2020-11-29 10:32:15

by Jean Pihet

[permalink] [raw]
Subject: [PATCH 1/2] net: dsa: ksz: pad frame to 64 bytes for transmission

Some ethernet controllers (e.g. TI CPSW) pad the frames to a minimum
of 64 bytes before the FCS is appended. This causes an issue with the
KSZ tail tag which could not be the last byte before the FCS.
Solve this by padding the frame to 64 bytes minus the tail tag size,
before the tail tag is added and the frame is passed for transmission.

Signed-off-by: Jean Pihet <[email protected]>
---
net/dsa/tag_ksz.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 0a5aa982c60d..0074702dcbbc 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -19,8 +19,13 @@ static struct sk_buff *ksz_common_xmit(struct sk_buff *skb,
{
struct sk_buff *nskb;
int padlen;
+ const int min_len = ETH_ZLEN + ETH_FCS_LEN;

- padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
+ /*
+ * Pad to the minimum ethernet frame size, minus the size of the
+ * tail tag which will be appended at the very end, before the FCS.
+ */
+ padlen = (skb->len >= min_len) ? 0 : min_len - skb->len - len;

if (skb_tailroom(skb) >= padlen + len) {
/* Let dsa_slave_xmit() free skb */
--
2.26.2


2020-11-29 17:00:08

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 1/2] net: dsa: ksz: pad frame to 64 bytes for transmission

Hi Jean

Please also include a patch 0/X which describes the patchset as a
whole. This will be used as the branch merge commit.

Andrew

2020-11-29 17:01:01

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 1/2] net: dsa: ksz: pad frame to 64 bytes for transmission

On Sun, Nov 29, 2020 at 11:23:59AM +0100, Jean Pihet wrote:
> Some ethernet controllers (e.g. TI CPSW) pad the frames to a minimum
> of 64 bytes before the FCS is appended. This causes an issue with the
> KSZ tail tag which could not be the last byte before the FCS.
> Solve this by padding the frame to 64 bytes minus the tail tag size,
> before the tail tag is added and the frame is passed for transmission.

Hi Jean

what tree is this based on? Have you seen

commit 88fda8eefd9a7a7175bf4dad1d02cc0840581111
Author: Christian Eggers <[email protected]>
Date: Sun Nov 1 21:16:10 2020 +0200

net: dsa: tag_ksz: don't allocate additional memory for padding/tagging

The caller (dsa_slave_xmit) guarantees that the frame length is at least
ETH_ZLEN and that enough memory for tail tagging is available.

2020-11-29 19:39:23

by Jean Pihet

[permalink] [raw]
Subject: Re: [PATCH 1/2] net: dsa: ksz: pad frame to 64 bytes for transmission

Hi Andrew,

On Sun, Nov 29, 2020 at 5:56 PM Andrew Lunn <[email protected]> wrote:
>
> On Sun, Nov 29, 2020 at 11:23:59AM +0100, Jean Pihet wrote:
> > Some ethernet controllers (e.g. TI CPSW) pad the frames to a minimum
> > of 64 bytes before the FCS is appended. This causes an issue with the
> > KSZ tail tag which could not be the last byte before the FCS.
> > Solve this by padding the frame to 64 bytes minus the tail tag size,
> > before the tail tag is added and the frame is passed for transmission.
>
> Hi Jean
>
> what tree is this based on? Have you seen
The patches are based on the latest mainline v5.10-rc5. Is this the
recommended version to submit new patches?

>
> commit 88fda8eefd9a7a7175bf4dad1d02cc0840581111
> Author: Christian Eggers <[email protected]>
> Date: Sun Nov 1 21:16:10 2020 +0200
>
> net: dsa: tag_ksz: don't allocate additional memory for padding/tagging
>
> The caller (dsa_slave_xmit) guarantees that the frame length is at least
> ETH_ZLEN and that enough memory for tail tagging is available.
>

I cannot find this commit. Which tree/branch is it from?

Thanks for reviewing,
Jean

2020-11-29 19:41:09

by Jean Pihet

[permalink] [raw]
Subject: Re: [PATCH 1/2] net: dsa: ksz: pad frame to 64 bytes for transmission

Andrew,

On Sun, Nov 29, 2020 at 5:57 PM Andrew Lunn <[email protected]> wrote:
>
> Hi Jean
>
> Please also include a patch 0/X which describes the patchset as a
> whole. This will be used as the branch merge commit.

Sure, will submit a v2 asap.

Thx,
Jean

>
> Andrew

2020-11-29 19:42:47

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 1/2] net: dsa: ksz: pad frame to 64 bytes for transmission

On Sun, Nov 29, 2020 at 08:34:27PM +0100, Jean Pihet wrote:
> Hi Andrew,
>
> On Sun, Nov 29, 2020 at 5:56 PM Andrew Lunn <[email protected]> wrote:
> >
> > On Sun, Nov 29, 2020 at 11:23:59AM +0100, Jean Pihet wrote:
> > > Some ethernet controllers (e.g. TI CPSW) pad the frames to a minimum
> > > of 64 bytes before the FCS is appended. This causes an issue with the
> > > KSZ tail tag which could not be the last byte before the FCS.
> > > Solve this by padding the frame to 64 bytes minus the tail tag size,
> > > before the tail tag is added and the frame is passed for transmission.
> >
> > Hi Jean
> >
> > what tree is this based on? Have you seen
> The patches are based on the latest mainline v5.10-rc5. Is this the
> recommended version to submit new patches?

No, that is old. Please take a read of:

https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html

Andrew

2020-11-29 19:53:49

by Jean Pihet

[permalink] [raw]
Subject: Re: [PATCH 1/2] net: dsa: ksz: pad frame to 64 bytes for transmission

Andrew,

On Sun, Nov 29, 2020 at 8:38 PM Andrew Lunn <[email protected]> wrote:
>
> On Sun, Nov 29, 2020 at 08:34:27PM +0100, Jean Pihet wrote:
> > Hi Andrew,
> >
> > On Sun, Nov 29, 2020 at 5:56 PM Andrew Lunn <[email protected]> wrote:
> > >
> > > On Sun, Nov 29, 2020 at 11:23:59AM +0100, Jean Pihet wrote:
> > > > Some ethernet controllers (e.g. TI CPSW) pad the frames to a minimum
> > > > of 64 bytes before the FCS is appended. This causes an issue with the
> > > > KSZ tail tag which could not be the last byte before the FCS.
> > > > Solve this by padding the frame to 64 bytes minus the tail tag size,
> > > > before the tail tag is added and the frame is passed for transmission.
> > >
> > > Hi Jean
> > >
> > > what tree is this based on? Have you seen
> > The patches are based on the latest mainline v5.10-rc5. Is this the
> > recommended version to submit new patches?
>
> No, that is old. Please take a read of:
>
> https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html

Ok got it, thx!

Found the commit 88fda8ee and its parent [1] with the following
comment, which seems to indicate that my patch is not needed anymore.
Can you confirm?

/* For tail taggers, we need to pad short frames ourselves, to ensure
+ * that the tail tag does not fail at its role of being at the end of
+ * the packet, once the master interface pads the frame. Account for
+ * that pad length here, and pad later.
...

[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=a3b0b6479700a5b0af2c631cb2ec0fb7a0d978f2

Thx,
Jean

>
> Andrew