[PATCH v2 net 0/6] Fix short frame transmission in enetc

[PATCH v2 net 0/6] Fix short frame transmission in enetc

[Vladimir Oltean]

This is a belated follow-up to Zefir Kurtisi's report:
https://lore.kernel.org/netdev/20260220132930.2521155-1-zefir.kurtisi@gmail.com/

which is that the ENETC misbehaves when transmitting packets smaller
than 16 octets sans FCS: it sends them but does not update the completed
index in the transmit BD ring. I did find a sentence in the reference
manual explicitly stating these short frames are not supported.

The original series (patches 5, 6) were focused on handling the invalid
frame geometries. However, review pointed out further issues in annex
code. Patches 1-4 handle those.

The main patch is 2/3, which Zefir already tested (in a simpler form)
and confirmed working in the thread from February. The code path should
be identical for his issue, so I've preserved the tag despite the
additions.

v1 at:
https://lore.kernel.org/netdev/20260401172246.1075883-1-vladimir.oltean@nxp.com/

Vladimir Oltean (6):
  net: enetc: increment error counter in enetc_xdp_xmit() on DMA mapping
    errors
  net: enetc: linearize PTP event packets with one-step TX timestamping
  net: enetc: ensure enetc_xdp_xmit() calls enetc_update_tx_ring_tail()
  net: enetc: fix bogus TX ring consumer index after reinitialization
  net: enetc: pad short frames in software
  net: enetc: pad short XDP frames coming from devmap

 drivers/net/ethernet/freescale/enetc/enetc.c | 38 +++++++++++++++++---
 drivers/net/ethernet/freescale/enetc/enetc.h |  2 ++
 include/net/xdp.h                            | 23 ++++++++++++
 3 files changed, 59 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH v2 net 1/6] net: enetc: increment error counter in enetc_xdp_xmit() on DMA mapping errors

[Vladimir Oltean]

Failures in enetc_xdp_frame_to_xdp_tx_swbd() should cause a bump in the
XDP TX drop counters, like failures of enetc_xdp_tx() do below.

Fixes: 9d2b68cc108d ("net: enetc: add support for XDP_REDIRECT")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is new
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index aa8a87124b10..ece10a58d44e 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1801,8 +1801,10 @@ int enetc_xdp_xmit(struct net_device *ndev, int num_frames,
 		xdp_tx_bd_cnt = enetc_xdp_frame_to_xdp_tx_swbd(tx_ring,
 							       xdp_redirect_arr,
 							       frames[k]);
-		if (unlikely(xdp_tx_bd_cnt < 0))
+		if (unlikely(xdp_tx_bd_cnt < 0)) {
+			tx_ring->stats.xdp_tx_drops++;
 			break;
+		}
 
 		if (unlikely(!enetc_xdp_tx(tx_ring, xdp_redirect_arr,
 					   xdp_tx_bd_cnt))) {
-- 
2.43.0

[PATCH v2 net 2/6] net: enetc: linearize PTP event packets with one-step TX timestamping

[Vladimir Oltean]

Sashiko reports that enetc_ptp_parse() uses ptp_parse_header(), which
does not handle fragmented headers, and expects the entire area between
skb_mac_header() and the end of the PTP header to be linear.

When the driver fails to parse a fragmented PTP frame to find the
offsets to the originTimestamp and correctionField, it falls back to
two-step timestamping, which is technically not what user space asked
for, and it may not be prepared to receive the timestamped packet
through the socket error queue.

The problem can be avoided relatively easily by linearizing packets with
one-step timestamping requests prior to calling enetc_ptp_parse(). These
are infrequent enough that this should not be a performance problem.

Fixes: 7294380c5211 ("enetc: support PTP Sync packet one-step timestamping")
Link: https://sashiko.dev/#/patchset/20260401172246.1075883-1-vladimir.oltean%40nxp.com
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is new
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index ece10a58d44e..ac6cad5605e4 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1059,6 +1059,11 @@ netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	/* Fall back to two-step timestamp if not one-step Sync packet */
 	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
+		if (unlikely(skb_linearize(skb))) {
+			dev_kfree_skb_any(skb);
+			return NETDEV_TX_OK;
+		}
+
 		if (enetc_ptp_parse(skb, &udp, &msgtype, &twostep,
 				    &offset1, &offset2) ||
 		    msgtype != PTP_MSGTYPE_SYNC || twostep != 0) {
-- 
2.43.0

[PATCH v2 net 3/6] net: enetc: ensure enetc_xdp_xmit() calls enetc_update_tx_ring_tail()

[Vladimir Oltean]

enetc_xdp_xmit() keeps track of 3 numbers:
- num_frames: total length of passed struct xdp_frame **frames array
- xdp_tx_frm_cnt: number of frames successfully sent
- k: index of currently sent frame from array

With "k != xdp_tx_frm_cnt", the intention was to detect an early break
due to an inability to send a frame, and to trigger a TX doorbell
anyway.

However, that doesn't work because every time when the loop breaks,
k and xdp_tx_frm_cnt are mathematically equal.

The correct condition on which we should ring the doorbell is when at
least one frame was sent, and either the caller required us to flush, or
we couldn't enqueue the entire passed array.

Fixes: 9d2b68cc108d ("net: enetc: add support for XDP_REDIRECT")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is new
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index ac6cad5605e4..5b97f9e668ba 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1823,7 +1823,8 @@ int enetc_xdp_xmit(struct net_device *ndev, int num_frames,
 		xdp_tx_frm_cnt++;
 	}
 
-	if (unlikely((flags & XDP_XMIT_FLUSH) || k != xdp_tx_frm_cnt))
+	if (unlikely(xdp_tx_frm_cnt && ((flags & XDP_XMIT_FLUSH) ||
+					xdp_tx_frm_cnt < num_frames)))
 		enetc_update_tx_ring_tail(tx_ring);
 
 	tx_ring->stats.xdp_tx += xdp_tx_frm_cnt;
-- 
2.43.0

[PATCH v2 net 4/6] net: enetc: fix bogus TX ring consumer index after reinitialization

[Vladimir Oltean]

The TBCIR (Transmit Buffer Descriptor Ring Consumer Index) register has
the BD index as the lower 16 bits, but the upper 16 bits contain this
field:

  STAT_ID: Status identifier. Incremented each time the BDR_INDEX is
  updated and an error status bit was set for one of the processed BDs.
  Clears on read.

If there was any transmit error prior to the ring reinitialization and
this is the first time we re-read the TBCIR register, reading it will
give us a value with non-zero upper bits, which is saved in
bdr->next_to_clean.

If subsequently NAPI gets invoked and enetc_clean_tx_ring() runs, this
will dereference the &tx_ring->tx_swbd[] for the bogus (and huge)
next_to_clean index, and will result in an out-of-bounds memory access.

Other places like enetc_bd_ready_count() do mask out the upper bits, so
let's do that here as well.

Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: none
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 5b97f9e668ba..a48ee3040b80 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -2604,7 +2604,8 @@ static void enetc_setup_txbdr(struct enetc_hw *hw, struct enetc_bdr *tx_ring)
 	 * adjust sw indexes
 	 */
 	tx_ring->next_to_use = enetc_txbdr_rd(hw, idx, ENETC_TBPIR);
-	tx_ring->next_to_clean = enetc_txbdr_rd(hw, idx, ENETC_TBCIR);
+	tx_ring->next_to_clean = enetc_txbdr_rd(hw, idx, ENETC_TBCIR) &
+				 ENETC_TBCIR_IDX_MASK;
 
 	if (tx_ring->next_to_use != tx_ring->next_to_clean &&
 	    !is_enetc_rev1(si)) {
-- 
2.43.0

[PATCH v2 net 5/6] net: enetc: pad short frames in software

[Vladimir Oltean]

The ENETC does not support BUF_LEN or FRM_LEN in TX buffer descriptors
less than 16. This is written in the reference manual of all SoCs
supported by the driver: LS1028A, i.MX943, i.MX95 etc.

  Frames must not have a FRM_LEN that is less than 16 bytes. Frames of
  0-15 bytes are not supported.
  (...)
  The first descriptor in a chain must not have a BUFF_LEN that is less
  than 16 bytes.

I don't think proper attention was paid to this during development, we
found the text at the end of a bug investigation. Therefore, the driver
does not enforce this.

But the frame length is out of the driver's control, and the network
stack can actually send packets with skb->len smaller than that. The
result is unpleasant, as will be explained below, so for simplicity
sake, we just pad anything shorter than ETH_ZLEN.

Zefir Kurtisi found a case where transmitting L2 WNM keep-alive frames
through ENETC would soft-lockup the host through an IRQ storm. He later
distilled this into a small enetc-killer.c user space program which
sends a packet with MAC DA, MAC SA and EtherType IPv4 (14 octets in
length) through an AF_PACKET raw socket.

The IRQ storm is actually a curious effect of a chain of events.

The hardware behaviour, when an invalid BD is put in its TX ring, is
that it would transmit the packet as normal, update counters, raise
completion interrupt as normal, but it would just not advance the
consumer index of the ring (TBaCIR) to signify that the BD has been
consumed and is available for software to free. The ring will also get
its TBaSR[BUSY] bit persistently set to 1 afterwards.

It deserves an explanation why the behaviour above would lead to an
IRQ storm, since ENETC interrupts are message-based (MSI-X), and an
unhandled interrupt would typically just be lost rather than retrigger
itself as a wired interrupt would.

NAPI processing in ENETC has 3 steps:

I. the enetc_msix() hardirq handler disables RBaIER, TBaIER and sets
   softirq processing to the 'pending' state.

II. the enetc_poll() softirq handler for the IRQ vector walks through
    the TX rings affine to that vector, checks which ones have a TBCIR
    updated since last time - enetc_bd_ready_count() - processes those
    completed frames, and clears pending interrupts in these updated TX
    rings by writing to TBaIDR. (I've excluded RX processing due to it
    being irrelevant).

III. After the softirq handler does its round of checking all RX and TX
     rings for updates, it re-enables all interrupts in RBaIER and
     TBaIER that were previously disabled by the hardirq handler, and
     exits.

Because the TX ring with the short frame is skipped at step II (TBCIR
wasn't updated as part of HW malfunction), its pending IRQ is not
cleared in TBaIDR by enetc_clean_tx_ring().

But because enetc_msix() disables TBaIER at step I and re-enables it at
step III, another MSI will be fired upon re-enabling it. This is what
completes the cycle and the driver goes back to step I.

So the driver misinterprets the mixed signals it's getting from the
hardware, and ends up causing a software-amplified IRQ storm.

Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers")
Reported-by: Zefir Kurtisi <zefir.kurtisi@westermo.com>
Closes: https://lore.kernel.org/netdev/b3d9136c-2803-4203-b1ea-1f9e62de80a1@gmail.com/
Tested-by: Zefir Kurtisi <zefir.kurtisi@westermo.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: handle the BUF_LEN limitation too, independently of FRM_LEN
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 13 +++++++++++++
 drivers/net/ethernet/freescale/enetc/enetc.h |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index a48ee3040b80..c70df45422e0 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1050,6 +1050,19 @@ netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev)
 	u8 udp, msgtype, twostep;
 	u16 offset1, offset2;
 
+	/* Hardware does not support transmit buffer descriptors with a total
+	 * length of less than 16 bytes, or a first buffer size of less than
+	 * 16 bytes.
+	 */
+	if (unlikely(skb_headlen(skb) < ENETC_MIN_BUFF_SIZE &&
+		     skb_linearize(skb))) {
+		dev_kfree_skb_any(skb);
+		return NETDEV_TX_OK;
+	}
+
+	if (eth_skb_pad(skb))
+		return NETDEV_TX_OK;
+
 	/* Mark tx timestamp type on enetc_cb->flag if requires */
 	if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
 	    (priv->active_offloads & ENETC_F_TX_TSTAMP_MASK))
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index aecd40aeef9c..819e643538b1 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -21,6 +21,8 @@
 #define ENETC_MAX_MTU		(ENETC_MAC_MAXFRM_SIZE - \
 				(ETH_FCS_LEN + ETH_HLEN + VLAN_HLEN))
 
+#define ENETC_MIN_BUFF_SIZE	16
+
 #define ENETC_CBD_DATA_MEM_ALIGN 64
 
 #define ENETC_MADDR_HASH_TBL_SZ	64
-- 
2.43.0

[PATCH v2 net 6/6] net: enetc: pad short XDP frames coming from devmap

[Vladimir Oltean]

Similar to the skb path issue explained in the previous change, ENETC
could end up transmitting short frames coming from XDP.

The way in which this could happen is a bit contrived, but it involves
XDP_REDIRECT from a veth interface pair.

As for enetc_xmit(), there are two separate limitations for the overall
FRM_LEN and for the head BUFF_LEN. For the head BUFF_LEN, we add a
direct restriction in enetc_xdp_xmit(), and for the overall FRM_LEN, we
introduce a xdp_frame_pad() best-effort generic helper which we call
from the same place.

This helper alters the frame, but that should be safe, because
ndo_xdp_xmit() is the hand-off function where the XDP frames become the
responsibility of the driver. AFAIU, struct xdp_frame doesn't have
multiple copies.

I say best-effort because xdp_frame_pad() can only expand the head
buffer of an XDP frame. It cannot expand the last fragment of a
multi-buffer XDP frame, because, unlike bpf_xdp_frags_increase_tail(),
it lacks access to the rxq->frag_size, aka the capacity of the chunk of
memory being pointed to by the fragment.

So, if the frame happens to be less than minimum Ethernet size, but
fragmented, callers of this function will have to drop it.

Fixes: 9d2b68cc108d ("net: enetc: add support for XDP_REDIRECT")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2:
- handle multi-buffer frames instead of being unaware of their
  multi-buffer quality
- add separate restriction for BUFF_LEN
- increment drop counter
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 10 ++++++++-
 include/net/xdp.h                            | 23 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index c70df45422e0..196b7828e2aa 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1816,9 +1816,17 @@ int enetc_xdp_xmit(struct net_device *ndev, int num_frames,
 	prefetchw(ENETC_TXBD(*tx_ring, tx_ring->next_to_use));
 
 	for (k = 0; k < num_frames; k++) {
+		struct xdp_frame *xdpf = frames[k];
+
+		if (unlikely(xdp_frame_pad(xdpf) ||
+			     xdpf->len < ENETC_MIN_BUFF_SIZE)) {
+			tx_ring->stats.xdp_tx_drops++;
+			break;
+		}
+
 		xdp_tx_bd_cnt = enetc_xdp_frame_to_xdp_tx_swbd(tx_ring,
 							       xdp_redirect_arr,
-							       frames[k]);
+							       xdpf);
 		if (unlikely(xdp_tx_bd_cnt < 0)) {
 			tx_ring->stats.xdp_tx_drops++;
 			break;
diff --git a/include/net/xdp.h b/include/net/xdp.h
index aa742f413c35..276afc9aa21d 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -477,6 +477,29 @@ xdp_get_frame_len(const struct xdp_frame *xdpf)
 	return len;
 }
 
+static inline int xdp_frame_pad(struct xdp_frame *xdpf)
+{
+	unsigned int total_len, pad;
+	void *sinfo;
+
+	total_len = xdp_get_frame_len(xdpf);
+	if (likely(total_len >= ETH_ZLEN))
+		return 0;
+
+	if (unlikely(xdp_frame_has_frags(xdpf)))
+		return -EOPNOTSUPP;
+
+	pad = ETH_ZLEN - total_len;
+	sinfo = xdp_get_shared_info_from_frame(xdpf);
+	if (unlikely(xdpf->data + xdpf->len + pad > sinfo))
+		return -ENOMEM;
+
+	memset(xdpf->data + xdpf->len, 0, pad);
+	xdpf->len += pad;
+
+	return 0;
+}
+
 int __xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq,
 		       struct net_device *dev, u32 queue_index,
 		       unsigned int napi_id, u32 frag_size);
-- 
2.43.0

RE: [PATCH v2 net 1/6] net: enetc: increment error counter in enetc_xdp_xmit() on DMA mapping errors

[Wei Fang]

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> b/drivers/net/ethernet/freescale/enetc/enetc.c
> index aa8a87124b10..ece10a58d44e 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -1801,8 +1801,10 @@ int enetc_xdp_xmit(struct net_device *ndev, int
> num_frames,
>  		xdp_tx_bd_cnt = enetc_xdp_frame_to_xdp_tx_swbd(tx_ring,
>  							       xdp_redirect_arr,
>  							       frames[k]);
> -		if (unlikely(xdp_tx_bd_cnt < 0))
> +		if (unlikely(xdp_tx_bd_cnt < 0)) {
> +			tx_ring->stats.xdp_tx_drops++;

xdp_tx_drops is also used for XDP_TX statistics, so it is difficult to distinguish
how many packets are dropped by XDP_TX and how many are dropped by
ndo_xdp_xmit(). Would it be better to add another statistic variable to track
the packets dropped by ndo_xdp_xmit()?

Additionally, I checked the code of bq_xmit_all(), when ndo_xdp_xmit()
returns due to failure, packets after 'sent' will be dropped by
xdp_return_frame_rx_napi(). Therefore, 'xdp_tx_drops' here seems more
like a statistic of the number of enetc_xdp_xmit() failures than a packet loss
statistic.

>  			break;
> +		}
> 
>  		if (unlikely(!enetc_xdp_tx(tx_ring, xdp_redirect_arr,
>  					   xdp_tx_bd_cnt))) {
> --
> 2.43.0

RE: [PATCH v2 net 2/6] net: enetc: linearize PTP event packets with one-step TX timestamping

[Wei Fang]

> Sashiko reports that enetc_ptp_parse() uses ptp_parse_header(), which
> does not handle fragmented headers, and expects the entire area between
> skb_mac_header() and the end of the PTP header to be linear.

I think we should correct the check in ptp_parse_header().

@@ -130,7 +130,7 @@ struct ptp_header *ptp_parse_header(struct sk_buff *skb, unsigned int type)
        ptr += ETH_HLEN;

        /* Ensure that the entire header is present in this packet. */
-       if (ptr + sizeof(struct ptp_header) > skb->data + skb->len)
+       if (ptr + sizeof(struct ptp_header) > skb->data + skb_headlen(skb))
                return NULL;

RE: [PATCH v2 net 3/6] net: enetc: ensure enetc_xdp_xmit() calls enetc_update_tx_ring_tail()

[Wei Fang]

> enetc_xdp_xmit() keeps track of 3 numbers:
> - num_frames: total length of passed struct xdp_frame **frames array
> - xdp_tx_frm_cnt: number of frames successfully sent
> - k: index of currently sent frame from array
> 
> With "k != xdp_tx_frm_cnt", the intention was to detect an early break
> due to an inability to send a frame, and to trigger a TX doorbell
> anyway.
> 
> However, that doesn't work because every time when the loop breaks,
> k and xdp_tx_frm_cnt are mathematically equal.

Since k is always equal to xdp_tx_frm_cnt, so we only need one of them,
there is no need to use two variables.

RE: [PATCH v2 net 4/6] net: enetc: fix bogus TX ring consumer index after reinitialization

[Wei Fang]

> The TBCIR (Transmit Buffer Descriptor Ring Consumer Index) register has
> the BD index as the lower 16 bits, but the upper 16 bits contain this
> field:
> 
>   STAT_ID: Status identifier. Incremented each time the BDR_INDEX is
>   updated and an error status bit was set for one of the processed BDs.
>   Clears on read.
> 
> If there was any transmit error prior to the ring reinitialization and
> this is the first time we re-read the TBCIR register, reading it will
> give us a value with non-zero upper bits, which is saved in
> bdr->next_to_clean.
> 
> If subsequently NAPI gets invoked and enetc_clean_tx_ring() runs, this
> will dereference the &tx_ring->tx_swbd[] for the bogus (and huge)
> next_to_clean index, and will result in an out-of-bounds memory access.
> 
> Other places like enetc_bd_ready_count() do mask out the upper bits, so
> let's do that here as well.
> 
> Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet
> drivers")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> v1->v2: none
> ---
>  drivers/net/ethernet/freescale/enetc/enetc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 5b97f9e668ba..a48ee3040b80 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -2604,7 +2604,8 @@ static void enetc_setup_txbdr(struct enetc_hw *hw,
> struct enetc_bdr *tx_ring)
>  	 * adjust sw indexes
>  	 */
>  	tx_ring->next_to_use = enetc_txbdr_rd(hw, idx, ENETC_TBPIR);
> -	tx_ring->next_to_clean = enetc_txbdr_rd(hw, idx, ENETC_TBCIR);
> +	tx_ring->next_to_clean = enetc_txbdr_rd(hw, idx, ENETC_TBCIR) &
> +				 ENETC_TBCIR_IDX_MASK;
> 
>  	if (tx_ring->next_to_use != tx_ring->next_to_clean &&
>  	    !is_enetc_rev1(si)) {
> --
> 2.43.0

Reviewed-by: Wei Fang <wei.fang@nxp.com>

RE: [PATCH v2 net 5/6] net: enetc: pad short frames in software

[Wei Fang]

> The ENETC does not support BUF_LEN or FRM_LEN in TX buffer descriptors
> less than 16. This is written in the reference manual of all SoCs
> supported by the driver: LS1028A, i.MX943, i.MX95 etc.
> 
>   Frames must not have a FRM_LEN that is less than 16 bytes. Frames of
>   0-15 bytes are not supported.
>   (...)
>   The first descriptor in a chain must not have a BUFF_LEN that is less
>   than 16 bytes.
> 
> I don't think proper attention was paid to this during development, we
> found the text at the end of a bug investigation. Therefore, the driver
> does not enforce this.
> 
> But the frame length is out of the driver's control, and the network
> stack can actually send packets with skb->len smaller than that. The
> result is unpleasant, as will be explained below, so for simplicity
> sake, we just pad anything shorter than ETH_ZLEN.
> 
> Zefir Kurtisi found a case where transmitting L2 WNM keep-alive frames
> through ENETC would soft-lockup the host through an IRQ storm. He later
> distilled this into a small enetc-killer.c user space program which
> sends a packet with MAC DA, MAC SA and EtherType IPv4 (14 octets in
> length) through an AF_PACKET raw socket.
> 
> The IRQ storm is actually a curious effect of a chain of events.
> 
> The hardware behaviour, when an invalid BD is put in its TX ring, is
> that it would transmit the packet as normal, update counters, raise
> completion interrupt as normal, but it would just not advance the
> consumer index of the ring (TBaCIR) to signify that the BD has been
> consumed and is available for software to free. The ring will also get
> its TBaSR[BUSY] bit persistently set to 1 afterwards.
> 
> It deserves an explanation why the behaviour above would lead to an
> IRQ storm, since ENETC interrupts are message-based (MSI-X), and an
> unhandled interrupt would typically just be lost rather than retrigger
> itself as a wired interrupt would.
> 
> NAPI processing in ENETC has 3 steps:
> 
> I. the enetc_msix() hardirq handler disables RBaIER, TBaIER and sets
>    softirq processing to the 'pending' state.
> 
> II. the enetc_poll() softirq handler for the IRQ vector walks through
>     the TX rings affine to that vector, checks which ones have a TBCIR
>     updated since last time - enetc_bd_ready_count() - processes those
>     completed frames, and clears pending interrupts in these updated TX
>     rings by writing to TBaIDR. (I've excluded RX processing due to it
>     being irrelevant).
> 
> III. After the softirq handler does its round of checking all RX and TX
>      rings for updates, it re-enables all interrupts in RBaIER and
>      TBaIER that were previously disabled by the hardirq handler, and
>      exits.
> 
> Because the TX ring with the short frame is skipped at step II (TBCIR
> wasn't updated as part of HW malfunction), its pending IRQ is not
> cleared in TBaIDR by enetc_clean_tx_ring().
> 
> But because enetc_msix() disables TBaIER at step I and re-enables it at
> step III, another MSI will be fired upon re-enabling it. This is what
> completes the cycle and the driver goes back to step I.
> 
> So the driver misinterprets the mixed signals it's getting from the
> hardware, and ends up causing a software-amplified IRQ storm.
> 
> Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet
> drivers")
> Reported-by: Zefir Kurtisi <zefir.kurtisi@westermo.com>
> Closes:
> https://lore.kernel.org/netdev/b3d9136c-2803-4203-b1ea-1f9e62de80a1@gma
> il.com/
> Tested-by: Zefir Kurtisi <zefir.kurtisi@westermo.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> v1->v2: handle the BUF_LEN limitation too, independently of FRM_LEN
> ---
>  drivers/net/ethernet/freescale/enetc/enetc.c | 13 +++++++++++++
>  drivers/net/ethernet/freescale/enetc/enetc.h |  2 ++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> b/drivers/net/ethernet/freescale/enetc/enetc.c
> index a48ee3040b80..c70df45422e0 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -1050,6 +1050,19 @@ netdev_tx_t enetc_xmit(struct sk_buff *skb, struct
> net_device *ndev)
>  	u8 udp, msgtype, twostep;
>  	u16 offset1, offset2;
> 
> +	/* Hardware does not support transmit buffer descriptors with a total
> +	 * length of less than 16 bytes, or a first buffer size of less than
> +	 * 16 bytes.
> +	 */
> +	if (unlikely(skb_headlen(skb) < ENETC_MIN_BUFF_SIZE &&
> +		     skb_linearize(skb))) {
> +		dev_kfree_skb_any(skb);
> +		return NETDEV_TX_OK;
> +	}
> +
> +	if (eth_skb_pad(skb))
> +		return NETDEV_TX_OK;
> +
>  	/* Mark tx timestamp type on enetc_cb->flag if requires */
>  	if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
>  	    (priv->active_offloads & ENETC_F_TX_TSTAMP_MASK))
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h
> b/drivers/net/ethernet/freescale/enetc/enetc.h
> index aecd40aeef9c..819e643538b1 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> @@ -21,6 +21,8 @@
>  #define ENETC_MAX_MTU		(ENETC_MAC_MAXFRM_SIZE - \
>  				(ETH_FCS_LEN + ETH_HLEN + VLAN_HLEN))
> 
> +#define ENETC_MIN_BUFF_SIZE	16
> +
>  #define ENETC_CBD_DATA_MEM_ALIGN 64
> 
>  #define ENETC_MADDR_HASH_TBL_SZ	64
> --
> 2.43.0

Reviewed-by: Wei Fang <wei.fang@nxp.com>