[1/2] net: dsa: ksz: pad frame to 64 bytes for transmission
diff mbox series

Message ID 20201129102400.157786-1-jean.pihet@newoldbits.com
State New, archived
Headers show
Series
  • [1/2] net: dsa: ksz: pad frame to 64 bytes for transmission
Related show

Commit Message

Jean Pihet Nov. 29, 2020, 10:23 a.m. UTC
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 <jean.pihet@newoldbits.com>
---
 net/dsa/tag_ksz.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Andrew Lunn Nov. 29, 2020, 4:56 p.m. UTC | #1
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 <ceggers@arri.de>
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.
Andrew Lunn Nov. 29, 2020, 4:57 p.m. UTC | #2
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
Jean Pihet Nov. 29, 2020, 7:34 p.m. UTC | #3
Hi Andrew,

On Sun, Nov 29, 2020 at 5:56 PM Andrew Lunn <andrew@lunn.ch> 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 <ceggers@arri.de>
> 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
Jean Pihet Nov. 29, 2020, 7:35 p.m. UTC | #4
Andrew,

On Sun, Nov 29, 2020 at 5:57 PM Andrew Lunn <andrew@lunn.ch> 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
Andrew Lunn Nov. 29, 2020, 7:38 p.m. UTC | #5
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 <andrew@lunn.ch> 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
Jean Pihet Nov. 29, 2020, 7:48 p.m. UTC | #6
Andrew,

On Sun, Nov 29, 2020 at 8:38 PM Andrew Lunn <andrew@lunn.ch> 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 <andrew@lunn.ch> 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

Patch
diff mbox series

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 */