[WIP,RFT,RFC] generic: qca8k: add experimental patch for eth mdio #4828

Ansuel · 2021-12-07T03:39:35Z

Feature:

Add support for mdio read/write in Ethernet packet.
Add support for phy read/write in Ethernet packet.
Add support for MIB counter in Ethernet packet.
Bulk read/write. Read and write fdb entry in one go.

I need some stability testing and some perf stats.

DEPENDS

Dsa conversion patch for ipq806x ipq806x: add dsa support #4036
kernel 5.15 support patch ipq806x: add support for kernel 5.15 #4748

Current problem:

This slow down init. I still need to find the correct way to understand when the tagger/dsa starts to receive/send packet. Because of this every request has to timeout and use the legacy mdio way.
When the system starts to handle packet from the switch port the mdio read/write will use the Ethernet way and will be fasted. (With the help of the kernel dev we develop a way to track when the master port (and the tagger) can correctly receive and handle packet)

Reason:

To support multicpu port, we probably need to enable assisted learning for cpu. This cause an increase in mdio read/write and this new way should speedup everything and remove the problem of assisted learning.

I would really appreciate any test with this.

Additions:

This also include another pr that permit to skip some redundant mdio write to reduce even more mdio use.
I should also test if we can use the same logic for the mdio read. In theory yes...

WARNING

This is not compile tested on a normal openwrt... I compiled this on linux kernel and I directly modified the source in the build dir. So expect some problem with patch apply.

I got some report that this apply directly to 5.15 pr.

@robimarko wonder if you can give some comments on the implementation if you have any idea on how this can be improved...

Signed-off-by: Ansuel Smith ansuelsmth@gmail.com

Ansuel · 2021-12-07T03:57:34Z

.../linux/generic/pending-5.10/999-5-net-dsa-tag_qca-Add-support-for-handling-mdio-read-w.patch

+	header->data[0] = mdio_ethhdr->mdio_data;
+
+	/* Get the rest of the 12 byte of data */
+	memcpy(header->data + 1, skb->data, QCA_HDR_MDIO_DATA2_LEN);


I can check the cmd and skip this memcpy and the other set with write operation.

Ansuel · 2021-12-07T03:58:45Z

target/linux/generic/pending-5.10/999-6-net-dsa-qca8k-cache-lo-and-hi-for-mdio-write.patch

+ 	*page = regaddr & 0x3ff;
+ }
+
+static u16 qca8k_current_lo = 0xffff;


We should consider a more safe value.... But considering the first thing will be checking the switch id... this should be fine...

rbpp · 2021-12-10T21:13:33Z

@Ansuel : Hi Ansuel. Is this PR based on dsa or swconfig? I'm willing to test it but I need to know what config to use.

Ansuel · 2021-12-10T21:33:03Z

it's on dsa Il giorno ven 10 dic 2021 alle ore 22:13 rbpp ***@***.***> ha scritto:

…

@Ansuel <https://github.com/Ansuel> : Hi Ansuel. Is this PR based on dsa or swconfig? I'm willing to test it but I need to know what config to use. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4828 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE2ZMQTBNJEEJVKSUHBSMGLUQJUQRANCNFSM5JQHBDUA> .

rbpp · 2021-12-10T21:43:54Z

it's on dsa.

My question was a bit foolish, qca8k has to be on dsa... Anyway, since this PR does not contain the other required changes to work on dsa (e.g., dts changes and maybe others), do I have to apply this PR together with another one of your open PRs?

Ansuel · 2021-12-11T03:59:37Z

Yes sorry I rushed this and will change shortly as I'm stabilizing it with the kernel guy.
Anyway this require 5.15 and ipq806x patch
But if you want to wait I will push a totally different series that should also works better and have more features.
Also notice that 5.15 require firewall 4.

rbpp · 2021-12-11T15:35:22Z

Take your time, I'll wait for your magic!

Ansuel · 2021-12-12T01:29:27Z

I pushed the """""""final""""""""" version of this... Hope someone can test this and give some result of stability / perf.
(this is the full conversion to eth packet
this use eth packet for

mib counter
mdio read/write
phy read/write
)

e9hack · 2021-12-12T09:35:47Z

target/linux/generic/pending-5.10/999-15-net-dsa-qca8k-cache-lo-and-hi-for-mdio-write.patch

+ 	*page = regaddr & 0x3ff;
+ }
+
+static u16 qca8k_current_lo = 0xffff;


qca8k_current_lo/high/page must be part of private data in structure qca8k_priv. If multiple qca8k switches does exist, currently they will share this three cached values.

The caching looks completely wrong. If we caching values, it must be done related to the registers.

Aside from the the global values that just got fixed, (still needs to be merged but i'm spending some time clearing the pr)

Caching the values looks wrong IMHO and this caching is on the regs level so we don't cache values, we cache the indirect regs used for mdio.

rbpp · 2021-12-12T14:07:24Z

@Ansuel : just loaded this PR on my dumb C2600 and it's working (it's with fw3, but I don't use firewall on this AP). Any test you want me to carry?

I can also test this PR on my main R7800 but I did not figure out yet how to compile a build with firewall4 in it.

Ansuel · 2021-12-12T15:31:04Z

You should check port stability and CPU load with traffic. What I need to know is if the switch stability is equal. Il Dom 12 Dic 2021, 15:07 rbpp ***@***.***> ha scritto:

…

@Ansuel <https://github.com/Ansuel> : just loaded this PR on my dumb C2600 and it's working (it's with fw3, but I don't use firewall on this AP). Any test you want me to carry? I can also test this PR on my main R7800 but I did not figure out yet how to compile a build with firewall4 in it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4828 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE2ZMQQJ255IPV65SXC7QG3UQSUCPANCNFSM5JQHBDUA> .

rbpp · 2021-12-13T21:45:29Z

I upgraded my R7800 with this PR and it's operational (with fw3). Bufferbloat test was not very good though but I can't tell if it's caused by this PR or by kernel 5.15.

Ansuel · 2021-12-13T23:14:15Z

@rbpp do you have any stats? bufferbloat probably doesn't work as no firewall.
port dropping anything?

rbpp · 2021-12-14T14:22:33Z

@rbpp do you have any stats? bufferbloat probably doesn't work as no firewall. port dropping anything?

Ports were stable, no drops in the logs. My C2600 was 3 days with this PR and everything looked good. As with my R7800, I used this PR only for a day because I did not want to have my main router exposed because of the problematic firewall, but it was otherwise stable during that period.

Ansuel · 2021-12-14T21:41:23Z

@rbpp was thinking, did you moved the patch to 5.15 dir or just applied this pr? Anyway the logic was wrong in the old series... I fixed it and make this more stable. Test it whatever you can :D

rbpp · 2021-12-14T22:15:53Z

@rbpp was thinking, did you moved the patch to 5.15 dir or just applied this pr?

I had applied this PR + #4748 (k5.15) + #4036 (ipq806x dsa), as per your instruction. I'll test this new version in the next few days.

KA2107 · 2021-12-15T23:38:44Z

@Ansuel Thank you for your work. Just one minor thing, "experimental" spelling is wrong in both the Pull Request title and one of the commits.

Ansuel · 2021-12-15T23:40:59Z

@KA2107 just notice that. Anyway I'm rushing this to make it at least compliable... I need to finish a feature and I should really start refreshing all these pr... (example the add experimental patch got merged upstream and I have to backport them to 5.15)

DSA tagger can now keep private data in the dsa switch. Backport these patch. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

Add support for mdio read/write in Ethernet packet. Add support for MIB counter in Ethernet packet. Add support for phy read/write in Ethernet packet. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

Add experimental patch to reduce mdio write by caching lo and hi part. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

mgmt Ethernet can read/write up to 16byte in one go. Add support for this with a new helper. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

Ansuel · 2022-02-02T17:15:45Z

Anyway got this merged upstream... Will close this and include backport patches in the 5.15 pr

robimarko · 2022-02-02T18:36:10Z

Damn, completely missed the upstream discussion lately.

This effectively gets you free-of-cost MIB polling.
Not to mention other speed improvements as you don't have to rely on MDIO which compared to ethernet is rather slow.
Now I gotta rebase my IPQ40xx refactoring tree on net-next instead of 5.17 like I was planning.

Nice job.

Ansuel · 2022-02-02T18:57:14Z

@robimarko i'm still very tempted to propose the code split... wonder if they will accepted it anyway even if there isn't a user

robimarko · 2022-02-02T18:58:34Z

No way it will get accepted without a user

Ansuel · 2022-02-02T19:20:10Z

Anyway I feel like this should be improved... I need to understand all the build_skb logic as it looks wrong that space is allocated and free multiple time for a packet of the same size...

current problem is that with build_skb the preallocated space will be free anyway by the stmmac driver... and incrementing the users for skb seems a bit of an hack. If you have any hint about that.

robimarko · 2022-02-02T19:24:07Z

I wouldnt worry too much, as long as its freed and not leaked its fine

Ansuel · 2022-02-02T19:25:20Z

it's problematic for ath79 devices as the cpu is not that fast

robimarko · 2022-02-02T19:26:49Z

It cant be that bad, its not like you have millions of packets per second for management.

Ansuel · 2022-02-02T19:48:46Z

@robimarko we have tons of packet for the state machine to check the port status 100 a seconds. but i assume they can be decreased by turning off polling.. it was enabled as the cpu port can be set to base-x but never understood why they didn't just limit it to that particular mode... But I have to investigate it... could be that polling mode can be set only on switch setup and not at the mac config call

Ansuel changed the title ~~generic: qca8k: add experiemntal patch for eth mdio~~ [WIP,RFT,RFC] generic: qca8k: add experiemntal patch for eth mdio Dec 7, 2021

Ansuel commented Dec 7, 2021

View reviewed changes

Ansuel marked this pull request as draft December 7, 2021 17:30

mans0n added kernel pull request/issue with Linux kernel related changes RFC pull request ready for comments RFT pull request ready for testing labels Dec 8, 2021

Ansuel force-pushed the qca8k-eth-mdio branch from e32be6c to 904ff9e Compare December 12, 2021 01:21

e9hack reviewed Dec 12, 2021

View reviewed changes

Ansuel force-pushed the qca8k-eth-mdio branch from 904ff9e to 39d9353 Compare December 14, 2021 21:39

Ansuel force-pushed the qca8k-eth-mdio branch 2 times, most recently from d2a131b to fbb1562 Compare December 14, 2021 21:52

Ansuel force-pushed the qca8k-eth-mdio branch 3 times, most recently from 98d3da9 to 6ee5892 Compare December 15, 2021 23:35

Ansuel force-pushed the qca8k-eth-mdio branch 2 times, most recently from 08f09a0 to ad22b0c Compare December 19, 2021 02:47

Ansuel changed the title ~~[WIP,RFT,RFC] generic: qca8k: add experiemntal patch for eth mdio~~ [WIP,RFT,RFC] generic: qca8k: add experimental patch for eth mdio Dec 19, 2021

Ansuel force-pushed the qca8k-eth-mdio branch from ad22b0c to d973019 Compare December 20, 2021 02:22

mans0n added work in progress pull request the author is still working on blocked pull request blocked by/waiting for another change labels Dec 25, 2021

Ansuel force-pushed the qca8k-eth-mdio branch from d973019 to 21e0241 Compare January 5, 2022 15:20

Ansuel added 4 commits January 16, 2022 21:52

generic: backport patch for tagger private data

2667269

DSA tagger can now keep private data in the dsa switch. Backport these patch. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

generic: qca8k: add experimental patch for eth mdio

0d3295a

Add support for mdio read/write in Ethernet packet. Add support for MIB counter in Ethernet packet. Add support for phy read/write in Ethernet packet. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

generic: qca8k: add experimental patch to reduce mdio write

99be537

Add experimental patch to reduce mdio write by caching lo and hi part. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

generic: qca8k: add experiemntal patch for bulk read/write

7b51f34

mgmt Ethernet can read/write up to 16byte in one go. Add support for this with a new helper. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

Ansuel force-pushed the qca8k-eth-mdio branch from 21e0241 to 7b51f34 Compare January 16, 2022 20:52

Ansuel closed this Feb 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP,RFT,RFC] generic: qca8k: add experimental patch for eth mdio #4828

[WIP,RFT,RFC] generic: qca8k: add experimental patch for eth mdio #4828

Ansuel commented Dec 7, 2021 •

edited

Ansuel Dec 7, 2021

Ansuel Dec 7, 2021

rbpp commented Dec 10, 2021

Ansuel commented Dec 10, 2021 via email

rbpp commented Dec 10, 2021

Ansuel commented Dec 11, 2021

rbpp commented Dec 11, 2021

Ansuel commented Dec 12, 2021

e9hack Dec 12, 2021 •

edited

Ansuel Dec 19, 2021

rbpp commented Dec 12, 2021

Ansuel commented Dec 12, 2021 via email

rbpp commented Dec 13, 2021

Ansuel commented Dec 13, 2021

rbpp commented Dec 14, 2021

Ansuel commented Dec 14, 2021

rbpp commented Dec 14, 2021

KA2107 commented Dec 15, 2021

Ansuel commented Dec 15, 2021

Ansuel commented Feb 2, 2022

robimarko commented Feb 2, 2022 •

edited

Ansuel commented Feb 2, 2022

robimarko commented Feb 2, 2022

Ansuel commented Feb 2, 2022 •

edited

robimarko commented Feb 2, 2022

Ansuel commented Feb 2, 2022

robimarko commented Feb 2, 2022

Ansuel commented Feb 2, 2022

[WIP,RFT,RFC] generic: qca8k: add experimental patch for eth mdio #4828

[WIP,RFT,RFC] generic: qca8k: add experimental patch for eth mdio #4828

Conversation

Ansuel commented Dec 7, 2021 • edited

Feature:

DEPENDS

Current problem:

Reason:

Additions:

WARNING

Ansuel Dec 7, 2021

Choose a reason for hiding this comment

Ansuel Dec 7, 2021

Choose a reason for hiding this comment

rbpp commented Dec 10, 2021

Ansuel commented Dec 10, 2021 via email

rbpp commented Dec 10, 2021

Ansuel commented Dec 11, 2021

rbpp commented Dec 11, 2021

Ansuel commented Dec 12, 2021

e9hack Dec 12, 2021 • edited

Choose a reason for hiding this comment

Ansuel Dec 19, 2021

Choose a reason for hiding this comment

rbpp commented Dec 12, 2021

Ansuel commented Dec 12, 2021 via email

rbpp commented Dec 13, 2021

Ansuel commented Dec 13, 2021

rbpp commented Dec 14, 2021

Ansuel commented Dec 14, 2021

rbpp commented Dec 14, 2021

KA2107 commented Dec 15, 2021

Ansuel commented Dec 15, 2021

Ansuel commented Feb 2, 2022

robimarko commented Feb 2, 2022 • edited

Ansuel commented Feb 2, 2022

robimarko commented Feb 2, 2022

Ansuel commented Feb 2, 2022 • edited

robimarko commented Feb 2, 2022

Ansuel commented Feb 2, 2022

robimarko commented Feb 2, 2022

Ansuel commented Feb 2, 2022

Ansuel commented Dec 7, 2021 •

edited

e9hack Dec 12, 2021 •

edited

robimarko commented Feb 2, 2022 •

edited

Ansuel commented Feb 2, 2022 •

edited