Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-enable network checksum TX offload on rockchip64 family #2623

Merged
merged 4 commits into from
Mar 23, 2021

Conversation

g-provost
Copy link
Member

@g-provost g-provost commented Feb 9, 2021

Description

There is no more reason to have TX offload disable for the whole rockchip64 family anymore.

We need additional test on others boards than Helios64.

Important: This is based on the fact that we use MTU1500, and not exceeding 3000

How Has This Been Tested?

  • 1GBE iperf3 with TX offload disabled (duration: 10min - both direction)
  • 1GBE iperf3 with TX offload enabled (duration: 10min - both direction)
  • 2.5GBE iperf3 with TX offload disabled (duration: 10min - both direction)
  • 2.5GBE iperf3 with TX offload enabled (duration: 10min - both direction)
  • Check TX offload is enabled by default on 1GBE and 2.5GBE

@piter75
Copy link
Member

piter75 commented Feb 9, 2021

IIRC TX offloading still needs to be disabled in legacy kernels for rk3399.
I think that was the reason for this particular script to come to life.
If that's still the case we should probably keep disabling TX offloading but only in legacy branches.

Tomorrow I may probably find some time to test it with legacy.

@g-provost
Copy link
Member Author

Arghhh I forgot about the GMAC on legacy :/ Can the same patch that you did which set the PBL can be applied to legacy ?

@g-provost
Copy link
Member Author

@piter75 In LK4.4, stmmac driver has pbl settings already which apply for both TX and RX, it's only later that the driver introduced txpbl and rxpbl. So it shouldn't be an issue to apply the same tweak to legacy.

snps,pbl = <0x4>;

https://elixir.bootlin.com/linux/v4.4.213/source/Documentation/networking/stmmac.txt#L152
https://elixir.bootlin.com/linux/v4.4.213/source/Documentation/devicetree/bindings/net/stmmac.txt#L20

@piter75
Copy link
Member

piter75 commented Feb 10, 2021

In LK4.4, stmmac driver has pbl settings already which apply for both TX and RX

Nice find!
Yes, we should definitely do that in legacy to be on par with current/dev solution.

@g-provost g-provost force-pushed the rockchip64-tx-offload branch from 65a9044 to 9d1851e Compare February 16, 2021 04:41
@g-provost
Copy link
Member Author

g-provost commented Feb 16, 2021

Can you guys test the pbl settings for legacy kernel I just added in last commit on rk3399 boards other than Helios64.

You should test with TX checksum offload enabled.

@igorpecovnik I'm not sure why the check test failed. Is it because I did a force pushed to revert to previous commit ?

@g-provost
Copy link
Member Author

@piter75 Can you please confirm if the PBL tweak is ok on legacy then we can merge this PR. Thanks.

@piter75
Copy link
Member

piter75 commented Mar 8, 2021

@g-provost I tested it with NanoPi M4V2 using rk3399-legacy and got ~940mbps stable both ways for 10 minutes with tx/rx offloading enabled but...

Now it occurred to me that we also need to change it for rk3328 which exhibits the same issues without either Programmable Buffer Length set or tx offloading disabled.
I will adjust your PR and test with one of the rk3328 boards in rockchip64-legacy.

@piter75
Copy link
Member

piter75 commented Mar 20, 2021

I merged latest changes from master, added snsp,pbl = <0x4> property to both rk3328's ethernet interfaces in rockchip64-legacy and tested it with offloading enabled on ROCK Pi E.

I used branch: https://github.com/armbian/build/compare/rockchip64-tx-offload
@g-provost can you cherry-pick this commit ea77ba2 into your branch?
I cannot push it to your repo (obviously) ;-)

TX:

[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-600.00 sec  64.4 GBytes   923 Mbits/sec   12             sender
[  5]   0.00-600.05 sec  64.4 GBytes   922 Mbits/sec                  receiver

RX:

[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-600.07 sec  65.7 GBytes   941 Mbits/sec    1             sender
[  5]   0.00-600.00 sec  65.7 GBytes   941 Mbits/sec                  receiver

@g-provost
Copy link
Member Author

g-provost commented Mar 22, 2021

Ok done I just pushed back the branch rockchip64-tx-offload on Armbian into ours on Kobol. Next time I do a PR i will use a branch directly on Armbian. I let you merge if you think all is good then.

BTW: I'm not sure why the PR CI test failed.

@igorpecovnik
Copy link
Member

I'm not sure why the PR CI test failed.

Bugs in test :)

@piter75 piter75 merged commit 5e69351 into armbian:master Mar 23, 2021
useful64 pushed a commit to useful64/build that referenced this pull request Feb 4, 2022
)

* Re-enable network checksum TX offload on rockchip64 family

* Adjust gmac PBL setting for rk3399 legacy to fix network issues with MTU 1500

* Adjust gmac PBL setting for rk3328 in rockchip64-legacy to fix network issues with MTU 1500

Co-authored-by: Piotr Szczepanik <[email protected]>
useful64 pushed a commit to useful64/build that referenced this pull request Feb 4, 2022
)

* Re-enable network checksum TX offload on rockchip64 family

* Adjust gmac PBL setting for rk3399 legacy to fix network issues with MTU 1500

* Adjust gmac PBL setting for rk3328 in rockchip64-legacy to fix network issues with MTU 1500

Co-authored-by: Piotr Szczepanik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants