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.faine...@gmail.com>
> > Signed-off-by: Vladimir Oltean <vladimir.olt...@nxp.com>
>
> Reviewed-by: Florian Fainelli <f.faine...@gmail.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.faine...@gmail.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.faine...@gmail.com>
    Signed-off-by: David S. Miller <da...@davemloft.net>

Do you remember why this was needed?
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?
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?

Reply via email to