From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=209.85.128.50 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1792658044; b=MpUGWRn0CvX7dql0K9cOt4CW7XlKIzAdcPcfGUOhxQW7ft9FFLBD4tpGt+b9p2gSF2x8 P/f1JkSrzDNdig48glhZf6b5CCzsApT8Wq/O7R3T8F9iebmx1lMMplKlGePbaZGEv9CtI obJD0l+UHako7PibjjkAHVlwty+uDRAN+dy7ZyQk1YQrhyGcrCyo5ceArPjfep9d1Ymcd wqgEQmSoPDzY6sKMKavT5o05eUoZohpPCr4ZOPWDSt41RQ7sjin1a5W7AWlWZLQ05Eu9w PEffNM7yR+Nxx5bNvoVZ8J8l4UIJbxC71vbNgWu4cwaEtftDaLfJwZ879YJ9WSdjZRw== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1792658044; h=DMARC-Filter:Received:DKIM-Signature:X-Google-DKIM-Signature: X-Forwarded-Encrypted:X-Gm-Message-State:X-Gm-Gg:X-Received:Received: Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References:X-Mailer: MIME-Version:Content-Type:Content-Transfer-Encoding; bh=Yax36yyXPOjU+t0sLu2tNeMo+58jNxQtj3yAO7yym5E=; b=BVxWMmgulL4uzZ7v00Sz3qwStCNUKaxaN5BLdPAy44LXyXzk/8Ibvf3sZQT+dkqnM0Md tXiMojAoEF2zchB1YLOkRrlWsOx0HdykgqiSAWiFVjc3ECR5OYrZkyMlC0fOR1lY8m4Eo RhyxfZV43VlW+7xbtTwVvOQttvH/qiDe6O7zABTEGv019DxEV31YePcNMav7dg1ELT8Gt g8AU+uIqkSandmv+CE/sho5ZgETNKdedyjuZRkCD6hmL0I1gLLMGg6K/abbtVHyMIqmEy oXSbo/56KBs3oWyEenWahzc7CzZfQLA66kTi/tI5tfjgIESwIKeXhMIaw08QcZGcvdQ== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20251104 header.b=C7bQgVoA; arc=none smtp.remote-ip=209.85.128.50 Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E0071606A319 for ; Thu, 22 Oct 2026 08:34:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.2 smtp.subspace.kernel.org E0071606A319 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key, unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20251104 header.b=C7bQgVoA Received: by mail-wm1-f50.google.com with SMTP id 5b1f17b1804b1-4a1ec92f655so7369065e9.0 for ; Thu, 22 Oct 2026 01:34:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1792658042; x=1793262842; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=Yax36yyXPOjU+t0sLu2tNeMo+58jNxQtj3yAO7yym5E=; b=C7bQgVoAO4RA3HhlsxxWdnOhzaahrxQinb+MEjUCFSl9WedqaMJiMV4w3XHFlwirl3 Ynm+CBOqgVH+8HTYM0ctuRV3EXPa/5srVyOiIodR0KNGzZ6x4Vmatk5W0gLdUNulB2jQ YDTO7tPPUJHjhQ/kCbfbSYfJfkIMiC0EJw3zJFdM2RYmB9gZR6z8dq1ci3rZ1+Qs06mz 38Q1t5Vy8nTjvXlHhzfJza38tSEcDSLenwHAu6WNBCT1a2nA7MAqY58RVwmV/z2lKTTq 0JFPT8VN+Y+PENgONSenlJwZSRAC3T0Wy5Y0bF0RrW8UwP2pBYOECkCyhkOSVtTXdLKx Rarw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1792658042; x=1793262842; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=Yax36yyXPOjU+t0sLu2tNeMo+58jNxQtj3yAO7yym5E=; b=J/qzvELDgayvv0JCJ7F+SHdJ6acmDX7SuV31nvaZsYgXhwVeOFFhmyQOJgVUn2jD3S mtP5yX0F55VJOYgKygAm9sP0YuMZkzIHOzO3WLXuP3RwtrFvdH37i8D1wYD+wElh1Cw/ 8AwYhz7dhlnIWAqsaakzDuauKdjbSqH4qDBGD+AD6wgs+5sU6H3AkbMQieJ4MCaBkFFU cvoBRQMS7xmB++qqS2cuiglVzADLpIXLbWomkVGJrzk4dZpRcwpeJzzgV8HpFlCvZi07 4toj41vpmrPgvo7RGSAqEkA2Do3ehE8jKGrjW2HD3UJ789FFc6Ipp7SHvNyPToRup59k DIWw== X-Forwarded-Encrypted: i=1; AKwUvBwIFFG/yyozZjt0dm+8nqbVVbMwUGtmGiqUWs8m9nWOQwEr+8rQqHEzk6WsL4dFvqf2mGb71ToDCo6vlVI=@vger.kernel.org X-Gm-Message-State: AFq9FYJlPFDMV9z1E8lzq18g9Bw4LIUyxseE5BnytvyGHU92pXvtzAH+ rZLCd5AY5NOMwAM+XwltgxQ5guCL9e0GItQ0OrAfYdKZjPhVYAYg+yub X-Gm-Gg: AYBFou1QZJGHSPjbSKSO9w4OyAVHevomV7YJH1iRa0J40sB95TyM4cGaVGhG33AjjHS DT3G2Ab7r0EuHbTAYqd6555kzk0n1+g6gZ7wzoEZbOJl/268F9hfZ4wMFFjged1BSdqf0AlnSXx kGy/0QTneJOzSAOuC36j+RdkhmyvnTpob7oPun6ErvdPhWOcD+fhXPie+PL9O4wAFSPAWISx0XC J5G7psvMzPJ1DKPosF13nRKOkzrkwq9LgznSfQQD4AaKY9K2Z4AsIMgMbgp/Z4qW08lKCFIjxxp pdwkpknA/fJh/XUlHcWLl/E0OXtMRPWoIkAw/nPxqQaKdWfmlQzEJ2rDx4oETBsVPa24uQK/7yU thopOJgVQDaU28+R+DgYHVl/TsqPjNbZURzCb7GoVEqrWkRRUYwfx1ltz9MrFwGPdXBUb5YNcZk FpZOocBVVKU3T5bYyGS6c4c4b5Xd8ovuTzz1s5iInunWpOLh0/xs4koLD4KUY8zAwfZ5mMfztiL +veEGRhduXkllVKAUmGAmb3wWLEo6Ec5y59sNZbetSvXMUEwaGS5C5umvh7AfK/8KvL0/GNyfZD zXvt7Bobr60UvMnW X-Received: by 2002:a05:600c:a49:b0:49f:ffa2:1c72 with SMTP id 5b1f17b1804b1-4a204b20321mr33355645e9.11.1792658041966; Thu, 22 Oct 2026 01:34:01 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4a203b28f91sm51399915e9.5.2026.10.22.01.34.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Oct 2026 01:34:01 -0700 (PDT) Date: Thu, 22 Oct 2026 09:33:59 +0100 From: David Laight To: vladimir.oltean@nxp.com Cc: Andrew Lunn , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Claudiu Manoil , Wei Fang , Clark Wang , Ioana Ciornei , Zefir Kurtisi , Zefir Kurtisi , netdev@vger.kernel.org, imx@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 net 2/4] net: enetc: do not rely on mEMAC hardware frame padding Message-ID: <20261022093359.723c7e93@pumpkin> In-Reply-To: <20261021145314.1435049-3-vladimir.oltean@nxp.com> References: <20261021145314.1435049-1-vladimir.oltean@nxp.com> <20261021145314.1435049-3-vladimir.oltean@nxp.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 21 Oct 2026 17:53:12 +0300 vladimir.oltean@nxp.com wrote: > The driver sets the ENETC_PM0_CMD_TXP bit to pad short buffers. > This works, however sending short frames from user space and looking at > the padding bytes inserted by the MAC at the link partner, we notice on > LS1028A that the first octet of padding is sometimes not 0. > > This is fine per 802.3, which does not require that the padding is zero. > > However, there is an unproven suspicion that the padding bytes may > come from internal memories, and unfortunately I couldn't get sufficient > priority for this octet's origin to be investigated by hardware > engineers. > > To be defensive and not leak any sensitive information from the SoC, > let's pad all frames to ETH_ZLEN in software, so that the MAC doesn't > have to do anything. > > There are 2 distinct cases to handle, for GSO and non-GSO. For non-GSO > things are trivial; we need to call eth_skb_pad(). For GSO, things are > not simple, because user space may set gso_size to a very small value, > which could make the GSO super-skb smaller than ETH_ZLEN; That isn't sensible. The minimum frame is 64 bytes (including FCS), subtract that, the MAC header (14 bytes), minimum IP header (20), minimum TCP (20) and that leaves 6 bytes for user data. I doubt it is sensible to do GSO for anything under the minimum mtu (128 bytes rings a bell somewhere). But does this code allow for the VLAN tag that you forgot about in the other patch? I also noticed that the code seems to dma map the fragments for each gso segment separately, just mapping each skb fragment would be better. (But that is for another patch series.) David > padding that > would produce more segments than originally intended. What needs to be > done is to pad individual segments instead. > > When a GSO segment's hdr_len + data_len is below ETH_ZLEN, append a > zero-padding data BD and advertise the padded length in the header BD's > frm_len. > > The padding bytes are always the same zeros, so a single read-only > DMA-coherent buffer (tx_pad, ETH_ZLEN bytes, zero-initialized by > dma_alloc_coherent() and never written) is shared by all padding BDs of > the ring. Like the header BD, a padding BD carries no per-BD DMA address, > so cleanup does not unmap it. The pad BD becomes the segment's last BD, > so the F flag and the skb pointer move onto it. Segment checksums are > unaffected as they are computed over data_len only, before padding. > > The worst-case descriptor reservation grows by one BD per segment. > The pad BD also counts towards the per-frame chained BD limit (15 on > LS1028A, including the head and extension BD), so a padded segment may > use one data BD less. > > Only LS1028A (NETCv1) needs this. On i.MX95 (NETC v4.1), it was observed > that padding bytes are always zero, and the reference manual also > documents in "106.3.5.1.2.1.3.7 FCS generation and append on transmit" > that HTA Transmit pads short frames (including LSO segment frames) with > zeros before appending the FCS. On LS1028A it does not say this (padding > seems an exclusively mEMAC task). > > Nonetheless, for simplicity's sake, we keep the unconditional eth_skb_pad() > call in the common transmit path, but we leave the hardware LSO transmit > path unpadded in software. The latter is only present on NETC v4.1 and > later (thus already contains zeroes). > > Fixes: fb8629e2cbfc ("net: enetc: add support for software TSO") > Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers") > Signed-off-by: Vladimir Oltean > --- > v4->v5: > - account for the pad BD in the new enetc_tso_count_descs() helper > from "net: enetc: reserve VLAN extension BDs for software TSO", > instead of open-coding it in enetc_start_xmit() > - count the pad BD towards the per-frame chained BD limit, so a padded > segment may use one data BD less > - explain in the commit message why hardware LSO is not affected > v3->v4: > - split from "net: enetc: pad short frames in software", bring its own > justification: > https://lore.kernel.org/netdev/20260921112946.edo5uw6cl7jx6oez@skbuf/ > --- > drivers/net/ethernet/freescale/enetc/enetc.c | 61 ++++++++++++++++++-- > drivers/net/ethernet/freescale/enetc/enetc.h | 6 ++ > 2 files changed, 62 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c > index f6b352a3a49c..a9ea124f7835 100644 > --- a/drivers/net/ethernet/freescale/enetc/enetc.c > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c > @@ -524,7 +524,7 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb) > static int enetc_map_tx_tso_hdr(struct enetc_bdr *tx_ring, struct sk_buff *skb, > struct enetc_tx_swbd *tx_swbd, > union enetc_tx_bd *txbd, int *i, int hdr_len, > - int data_len) > + int data_len, int frm_len) > { > union enetc_tx_bd txbd_tmp; > u8 flags = 0, e_flags = 0; > @@ -541,7 +541,7 @@ static int enetc_map_tx_tso_hdr(struct enetc_bdr *tx_ring, struct sk_buff *skb, > txbd_tmp.buf_len = cpu_to_le16(hdr_len); > > /* first BD needs frm_len and offload flags set */ > - txbd_tmp.frm_len = cpu_to_le16(hdr_len + data_len); > + txbd_tmp.frm_len = cpu_to_le16(frm_len); > txbd_tmp.flags = flags; > > /* For the TSO header we do not set the dma address since we do not > @@ -613,6 +613,22 @@ static int enetc_map_tx_tso_data(struct enetc_bdr *tx_ring, struct sk_buff *skb, > return 0; > } > > +static void enetc_append_tx_pad(struct enetc_bdr *tx_ring, int i, int pad_len) > +{ > + struct enetc_tx_swbd *tx_swbd = &tx_ring->tx_swbd[i]; > + union enetc_tx_bd *txbd = ENETC_TXBD(*tx_ring, i); > + > + enetc_clear_tx_bd(txbd); > + txbd->addr = cpu_to_le64(tx_ring->tx_pad_dma); > + txbd->buf_len = cpu_to_le16(pad_len); > + txbd->flags = ENETC_TXBD_FLAGS_F; > + > + tx_swbd->len = pad_len; > + tx_swbd->is_eof = true; > + tx_swbd->do_twostep_tstamp = false; > + tx_swbd->check_wb = false; > +} > + > static __wsum enetc_tso_hdr_csum(struct tso_t *tso, struct sk_buff *skb, > char *hdr, int hdr_len, int *l4_hdr_len) > { > @@ -687,7 +703,8 @@ static int enetc_lso_count_descs(const struct sk_buff *skb) > > static int enetc_tso_count_descs(const struct sk_buff *skb) > { > - int count = tso_count_descs(skb); > + /* +gso_segs to cover one pad BD per segment */ > + int count = tso_count_descs(skb) + skb_shinfo(skb)->gso_segs; > > /* Each segment also needs an extension BD for VLAN insertion */ > if (skb_vlan_tag_present(skb)) > @@ -926,6 +943,9 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb > i = tx_ring->next_to_use; > > while (total_len > 0) { > + int max_data_bds; > + int frm_len; > + int pad_len; > char *hdr; > > /* Get the BD */ > @@ -937,6 +957,12 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb > data_len = min_t(int, skb_shinfo(skb)->gso_size, total_len); > total_len -= data_len; > > + /* Pad segments below ETH_ZLEN so the mEMAC does not pad them. */ > + frm_len = max_t(int, hdr_len + data_len, ETH_ZLEN); > + pad_len = frm_len - (hdr_len + data_len); > + /* The pad BD counts towards the max chained BD limit */ > + max_data_bds = priv->max_frags - !!pad_len; > + > /* prepare packet headers: MAC + IP + TCP */ > hdr = tx_ring->tso_headers + i * TSO_HEADER_SIZE; > tso_build_hdr(skb, hdr, &tso, data_len, total_len == 0); > @@ -944,7 +970,7 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb > /* compute the csum over the L4 header */ > csum = enetc_tso_hdr_csum(&tso, skb, hdr, hdr_len, &pos); > count += enetc_map_tx_tso_hdr(tx_ring, skb, tx_swbd, txbd, > - &i, hdr_len, data_len); > + &i, hdr_len, data_len, frm_len); > bd_data_num = 0; > > while (data_len > 0) { > @@ -966,8 +992,10 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb > csum = csum_block_add(csum, csum2, pos); > pos += size; > > + /* When padding, the pad BD is the last one, not this */ > err = enetc_map_tx_tso_data(tx_ring, skb, tx_swbd, txbd, > tso.data, size, > + !pad_len && > size == data_len); > if (err) { > if (i == 0) > @@ -982,12 +1010,19 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb > bd_data_num++; > tso_build_data(skb, &tso, size); > > - if (unlikely(bd_data_num >= priv->max_frags && data_len)) > + if (unlikely(bd_data_num >= max_data_bds && data_len)) > goto err_chained_bd; > } > > enetc_tso_complete_csum(tx_ring, &tso, skb, hdr, pos, csum); > > + if (pad_len) { > + enetc_bdr_idx_inc(tx_ring, &i); > + enetc_append_tx_pad(tx_ring, i, pad_len); > + tx_swbd = &tx_ring->tx_swbd[i]; > + count++; > + } > + > if (total_len == 0) > tx_swbd->skb = skb; > > @@ -1050,6 +1085,9 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff *skb, > enetc_unlock_mdio(); > } > } else { > + if (eth_skb_pad(skb)) > + return NETDEV_TX_OK; > + > if (unlikely(skb_shinfo(skb)->nr_frags > priv->max_frags)) > if (unlikely(skb_linearize(skb))) > goto drop_packet_err; > @@ -2285,8 +2323,18 @@ static int enetc_alloc_tx_resource(struct enetc_bdr_resource *res, > goto err_alloc_tso; > } > > + res->tx_pad = dma_alloc_coherent(dev, ETH_ZLEN, &res->tx_pad_dma, > + GFP_KERNEL); > + if (!res->tx_pad) { > + err = -ENOMEM; > + goto err_alloc_pad; > + } > + > return 0; > > +err_alloc_pad: > + dma_free_coherent(dev, bd_count * TSO_HEADER_SIZE, res->tso_headers, > + res->tso_headers_dma); > err_alloc_tso: > enetc_dma_free_bdr(res); > err_alloc_bdr: > @@ -2298,6 +2346,7 @@ static int enetc_alloc_tx_resource(struct enetc_bdr_resource *res, > > static void enetc_free_tx_resource(const struct enetc_bdr_resource *res) > { > + dma_free_coherent(res->dev, ETH_ZLEN, res->tx_pad, res->tx_pad_dma); > dma_free_coherent(res->dev, res->bd_count * TSO_HEADER_SIZE, > res->tso_headers, res->tso_headers_dma); > enetc_dma_free_bdr(res); > @@ -2425,6 +2474,8 @@ static void enetc_assign_tx_resource(struct enetc_bdr *tx_ring, > tx_ring->tx_swbd = res ? res->tx_swbd : NULL; > tx_ring->tso_headers = res ? res->tso_headers : NULL; > tx_ring->tso_headers_dma = res ? res->tso_headers_dma : 0; > + tx_ring->tx_pad = res ? res->tx_pad : NULL; > + tx_ring->tx_pad_dma = res ? res->tx_pad_dma : 0; > } > > static void enetc_assign_rx_resource(struct enetc_bdr *rx_ring, > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h > index d9e91832a9c1..db3b3bad0f59 100644 > --- a/drivers/net/ethernet/freescale/enetc/enetc.h > +++ b/drivers/net/ethernet/freescale/enetc/enetc.h > @@ -145,6 +145,8 @@ struct enetc_bdr_resource { > }; > char *tso_headers; > dma_addr_t tso_headers_dma; > + void *tx_pad; > + dma_addr_t tx_pad_dma; > }; > > struct enetc_bdr { > @@ -182,6 +184,10 @@ struct enetc_bdr { > /* DMA buffer for TSO headers */ > char *tso_headers; > dma_addr_t tso_headers_dma; > + > + /* Shared read-only zero buffer to pad short frames to ETH_ZLEN */ > + void *tx_pad; > + dma_addr_t tx_pad_dma; > } ____cacheline_aligned_in_smp; > > static inline void enetc_bdr_idx_inc(struct enetc_bdr *bdr, int *i)