Messages in this thread Patch in this message | | | From | Linus Walleij <> | Date | Tue, 31 Oct 2023 23:45:33 +0100 | Subject | [PATCH net v3] net: dsa: tag_rtl4_a: Bump min packet size |
| |
It was reported that the "LuCI" web UI was not working properly with a device using the RTL8366RB switch. Disabling the egress port tagging code made the switch work again, but this is not a good solution as we want to be able to direct traffic to a certain port.
It turns out that packets between 1496 and 1500 bytes are dropped unless 4 extra blank bytes are added to the tail.
1496 is the size of a normal data frame minus the internal DSA tag. The number is intuitive, the explanation evades me. This is not a tail tag since frames below 1496 bytes does not need the extra bytes.
As we completely lack datasheet or any insight into how this switch works, this trial-and-error solution is the best we can do. FWIW this bug may very well be the reason why Realteks code drops are not using CPU tagging. The vendor routers using this switch are all hardwired to disable CPU tagging and all packets are pushed to all ports on TX. This is also the case in the old OpenWrt driver, derived from the vendor code drops.
Modifying the MTU on the switch (one setting affecting all ports) has no effect.
Before this patch:
PING 192.168.1.1 (192.168.1.1) 1470(1498) bytes of data. ^C --- 192.168.1.1 ping statistics --- 2 packets transmitted, 0 received, 100% packet loss, time 1048ms
PING 192.168.1.1 (192.168.1.1) 1472(1500) bytes of data. ^C --- 192.168.1.1 ping statistics --- 12 packets transmitted, 0 received, 100% packet loss, time 11267ms
After this patch:
PING 192.168.1.1 (192.168.1.1) 1470(1498) bytes of data. 1478 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=0.533 ms 1478 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=0.630 ms
PING 192.168.1.1 (192.168.1.1) 1472(1500) bytes of data. 1480 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=0.527 ms 1480 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=0.562 ms
Also LuCI starts working with the 1500 bytes frames it produces from the HTTP server.
Fixes: 9eb8bc593a5e ("net: dsa: tag_rtl4_a: fix egress tags") Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Changes in v3: - Do not pad out to 1518 bytes, just add 4 bytes to the tail consistently when the package is bigger than 1496 bytes. Use a combination of __skb_pad() and __skb_put(). This works fine. - Add tail in the first if-clause and pad to 60 bytes in the else-clause since they are mutually exclusive. - Edit comments and commit text. - Link to v2: https://lore.kernel.org/r/20231030-fix-rtl8366rb-v2-1-e66e1ef7dbd2@linaro.org
Changes in v2: - Pad packages >= 1496 bytes after some further tests - Use more to-the-point description - Provide ping results in the commit message - Link to v1: https://lore.kernel.org/r/20231027-fix-rtl8366rb-v1-1-d565d905535a@linaro.org --- net/dsa/tag_rtl4_a.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/net/dsa/tag_rtl4_a.c b/net/dsa/tag_rtl4_a.c index c327314b95e3..9c7f8a89cb82 100644 --- a/net/dsa/tag_rtl4_a.c +++ b/net/dsa/tag_rtl4_a.c @@ -41,9 +41,21 @@ static struct sk_buff *rtl4a_tag_xmit(struct sk_buff *skb, u8 *tag; u16 out; - /* Pad out to at least 60 bytes */ - if (unlikely(__skb_put_padto(skb, ETH_ZLEN, false))) - return NULL; + /* Packets over 1496 bytes get dropped unless 4 bytes are added + * on the tail. 1496 is ETH_DATA_LEN - tag which is hardly + * a coinicidence, and the 4 bytes correspond to the tag length + * and this is hardly a coinicidence so we use these defines + * here. + */ + if (skb->len >= (ETH_DATA_LEN - RTL4_A_HDR_LEN)) { + if (unlikely(__skb_pad(skb, RTL4_A_HDR_LEN, false))) + return NULL; + __skb_put(skb, RTL4_A_HDR_LEN); + } else { + /* Pad out to at least 60 bytes */ + if (unlikely(__skb_put_padto(skb, ETH_ZLEN, false))) + return NULL; + } netdev_dbg(dev, "add realtek tag to package to port %d\n", dp->index); --- base-commit: d9e164e4199bc465b3540d5fe3ffc8a9a4fc6157 change-id: 20231027-fix-rtl8366rb-e752bd5901ca Best regards, -- Linus Walleij <linus.walleij@linaro.org>
| |