| Message ID | 20201129102400.157786-1-jean.pihet@newoldbits.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] net: dsa: ksz: pad frame to 64 bytes for transmission | expand |
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.
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
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
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
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
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
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 */
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(-)