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
| ||
|
Date: Tue, 3 Nov 2020 09:32:38 -0800 From: Florian Fainelli <f.fainelli@...il.com> To: Vladimir Oltean <vladimir.oltean@....com> Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "andrew@...n.ch" <andrew@...n.ch>, "vivien.didelot@...il.com" <vivien.didelot@...il.com>, "kuba@...nel.org" <kuba@...nel.org>, Christian Eggers <ceggers@...i.de>, Kurt Kanzenbach <kurt@...utronix.de> Subject: Re: [PATCH v3 net-next 09/12] net: dsa: tag_brcm: let DSA core deal with TX reallocation On 11/3/2020 2:51 AM, Vladimir Oltean wrote: > On Mon, Nov 02, 2020 at 12:34:11PM -0800, Florian Fainelli wrote: >> On 11/1/2020 11:16 AM, Vladimir Oltean wrote: >>> Now that we have a central TX reallocation procedure that accounts for >>> the tagger's needed headroom in a generic way, we can remove the >>> skb_cow_head call. >>> >>> Cc: Florian Fainelli <f.fainelli@...il.com> >>> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com> >> >> Reviewed-by: Florian Fainelli <f.fainelli@...il.com> > > Florian, I just noticed that tag_brcm.c has an __skb_put_padto call, > even though it is not a tail tagger. This comes from commit: > > commit bf08c34086d159edde5c54902dfa2caa4d9fbd8c > Author: Florian Fainelli <f.fainelli@...il.com> > Date: Wed Jan 3 22:13:00 2018 -0800 > > net: dsa: Move padding into Broadcom tagger > > Instead of having the different master network device drivers > potentially used by DSA/Broadcom tags, move the padding necessary for > the switches to accept short packets where it makes most sense: within > tag_brcm.c. This avoids multiplying the number of similar commits to > e.g: bgmac, bcmsysport, etc. > > Signed-off-by: Florian Fainelli <f.fainelli@...il.com> > Signed-off-by: David S. Miller <davem@...emloft.net> > > Do you remember why this was needed? Yes, it is explained in the comment surrounding the padding: /* The Ethernet switch we are interfaced with needs packets to be at * least 64 bytes (including FCS) otherwise they will be discarded when * they enter the switch port logic. When Broadcom tags are enabled, we * need to make sure that packets are at least 68 bytes * (including FCS and tag) because the length verification is done after * the Broadcom tag is stripped off the ingress packet. > As far as I understand, either the DSA master driver or the MAC itself > should pad frames automatically. Is that not happening on Broadcom SoCs, > or why do you need to pad from DSA? Some of the Ethernet MACs were not doing that automatic padding and/or had no option to turn this on. This is true for at least SYSTEMPORT (not Lite) and BGMAC. GENET is also commonly used but does support automatic RUNT frame padding. > How should we deal with this? Having tag_brcm.c still do some potential > reallocation defeats the purpose of doing it centrally, in a way. I was > trying to change the prototype of struct dsa_device_ops::xmit to stop > returning a struct sk_buff *, and I stumbled upon this. > Should we just go ahead and pad everything unconditionally in DSA? > This is really a problem specific to Broadcom tags and how the switch operate so it seems reasonable to leave those details down to the tagger. -- Florian
Powered by blists - more mailing lists