netdev - Re: [PATCH net v2] net: dsa: tag_rtl4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdaN2rTSHXDxwuS4czCzWyUkazY4Fn5vVLYosqF0=qi-Bw@mail.gmail.com>
Date: Mon, 30 Oct 2023 22:50:31 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] net: dsa: tag_rtl4_a: Bump min packet size

On Mon, Oct 30, 2023 at 3:16 PM Vladimir Oltean <olteanv@...il.com> wrote:

> which means that here, skb->len will be 1522, if it was originally 1496.
> So the code adds 26 extra octets, and only 4 of those are legitimate (a tag).

Yeah I know :/

> The rest is absolutely unexplained, which means that until there is a
> valid explanation for them:
>
> pw-bot: cr
>
> (sorry, but if it works and we don't know why it works, then at some
> point it will break and we won't know why it stopped working)

Yeah it broke now and we don't know why...

> you said that what increments is Dot1dTpPortInDiscards. 802.1Q-2018 says
> about it: "Count of received valid frames that were discarded (i.e.,
> filtered) by the Forwarding Process." which is odd enough to me, since
> packets sent by rtl4a_tag_xmit() should *not* be processed by the forwarding
> layer of the switch, but rather, force-delivered to the specified egress
> port.

No this was a coincidence, we can rule this out. There are always
a few (2-3) Dot1dTpPortInDiscards on the switch port when it
is connected, sorry for getting this wrong :/

What happens is way more disturbing: packets are dropped
*silently* if not padded.

I added the following patch:

@@ -37,6 +37,8 @@ static struct sk_buff *rtl4a_tag_xmit(struct sk_buff *skb,
                                      struct net_device *dev)
 {
        struct dsa_port *dp = dsa_slave_to_port(dev);
+       static u16 mask = BIT(6);
+       static int cnt = 0;
        __be16 *p;
        u8 *tag;
        u16 out;
@@ -60,6 +62,19 @@ static struct sk_buff *rtl4a_tag_xmit(struct sk_buff *skb,
        /* The lower bits indicate the port number */
        out |= BIT(dp->index);

+       if (skb->len >= (ETH_DATA_LEN - RTL4_A_HDR_LEN)) {
+               /* Test bits... */
+               out |= mask;
+               netdev_info(dev, "add mask %04x to big package\n", mask);
+               cnt ++;
+               if (cnt == 10) {
+                       cnt = 0;
+                       mask <<= 1;
+                       if (mask == BIT(15))
+                               mask = BIT(6);
+               }
+       }
+
        p = (__be16 *)(tag + 2);
        *p = htons(out);

This loops over all the bits not used by the port mask and test them
one by one to see if any of them help.

Then ran a few rounds of ping -s 1472 and ping -s 1470.

There are console prints:

realtek-smi switch lan0: add mask 0040 to big package
realtek-smi switch lan0: add mask 0040 to big package
realtek-smi switch lan0: add mask 0040 to big package
(...)

Then bits 6,7,8,9,10,11,12,13,14 and 15 are tested in succession.

No error counters increase in ethtool -S lan0.

I can see the big packets leave the eth0 interface
(from ethtool -S eth0)

     p05_EtherStatsPkts1024to1518Octe: 370

But they do not appear in the targeted switch port stats:

     EtherStatsPkts1024to1518Octets: 22

(these 22 are some unrelated -s 1400 packets I sent to test)

Yours,
Linus Walleij