Il giorno mar 24 mar 2026 alle ore 09:06 Loktionov, Aleksandr
<[email protected]> ha scritto:
>
>
>
> > -----Original Message-----
> > From: Intel-wired-lan <[email protected]> On Behalf
> > Of Matteo Croce
> > Sent: Monday, March 23, 2026 7:28 PM
> > To: Nguyen, Anthony L <[email protected]>; Kitszel,
> > Przemyslaw <[email protected]>; Andrew Lunn
> > <[email protected]>; David S. Miller <[email protected]>; Eric
> > Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo
> > Abeni <[email protected]>; Alexei Starovoitov <[email protected]>; Daniel
> > Borkmann <[email protected]>; Jesper Dangaard Brouer
> > <[email protected]>; John Fastabend <[email protected]>; Mohsin
> > Bashir <[email protected]>
> > Cc: [email protected]; [email protected]; intel-wired-
> > [email protected]; [email protected]
> > Subject: [Intel-wired-lan] [PATCH net-next v4 1/2] e1000e: add basic
> > XDP support
> >
> > Add XDP support to the e1000e driver covering the actions defined by
> > NETDEV_XDP_ACT_BASIC: XDP_DROP, XDP_PASS, XDP_TX and XDP_ABORTED.
> >
> > Infrastructure:
> > - e1000_xdp_setup() / e1000_xdp() for program attach/detach with
> > MTU validation and close/open cycle
> > - ndo_bpf support in net_device_ops
> > - xdp_rxq_info registration in setup/free_rx_resources
> >
> > Receive path:
> > - e1000_alloc_rx_buffers_xdp() for page-based Rx buffer allocation
> > with XDP_PACKET_HEADROOM
> > - e1000_clean_rx_irq_xdp() as the XDP receive handler
> > - e1000_run_xdp() to execute the XDP program on received packets
> > - SKB building via napi_build_skb() for XDP_PASS with metadata,
> > checksum offload and RSS hash support
> >
> > Transmit path:
> > - e1000_xdp_xmit_ring() to DMA-map and enqueue an XDP frame
> > - e1000_xdp_xmit_back() to convert an xdp_buff to a frame and send it
> > - e1000_finalize_xdp() to flush the TX ring after XDP processing
> > - TX completion via xdp_return_frame() with buffer type tracking
> >
> > Assisted-by: claude-opus-4-6
> > Signed-off-by: Matteo Croce <[email protected]>
> > ---
> > drivers/net/ethernet/intel/Kconfig | 1 +
> > drivers/net/ethernet/intel/e1000e/e1000.h | 18 +-
> > drivers/net/ethernet/intel/e1000e/netdev.c | 533 ++++++++++++++++++++-
> > 3 files changed, 540 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/Kconfig
> > b/drivers/net/ethernet/intel/Kconfig
> > index 288fa8ce53af..46e37cb68e70 100644
> > --- a/drivers/net/ethernet/intel/Kconfig
> > +++ b/drivers/net/ethernet/intel/Kconfig
> > @@ -63,6 +63,7 @@ config E1000E
> > depends on PCI && (!SPARC32 || BROKEN)
> > depends on PTP_1588_CLOCK_OPTIONAL
> > select CRC32
> > + select PAGE_POOL
> > help
> > This driver supports the PCI-Express Intel(R) PRO/1000
> > gigabit
> > ethernet family of adapters. For PCI or PCI-X e1000 adapters,
> > diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h
> > b/drivers/net/ethernet/intel/e1000e/e1000.h
> > index 63ebe00376f5..4c1175d4e5cb 100644
> > --- a/drivers/net/ethernet/intel/e1000e/e1000.h
> > +++ b/drivers/net/ethernet/intel/e1000e/e1000.h
> > @@ -19,10 +19,13 @@
> > #include <linux/net_tstamp.h>
> > #include <linux/ptp_clock_kernel.h>
> > #include <linux/ptp_classify.h>
>
> ...
>
> > +/**
> > + * e1000_xdp_xmit_ring - transmit an XDP frame on the TX ring
> > + * @adapter: board private structure
> > + * @tx_ring: Tx descriptor ring
> > + * @xdpf: XDP frame to transmit
> > + *
> > + * Returns E1000_XDP_TX on success, E1000_XDP_CONSUMED on failure
> > **/
> > +static int e1000_xdp_xmit_ring(struct e1000_adapter *adapter,
> > + struct e1000_ring *tx_ring,
> > + struct xdp_frame *xdpf)
> > +{
> > + struct e1000_buffer *buffer_info;
> > + struct e1000_tx_desc *tx_desc;
> > + dma_addr_t dma;
> > + u16 i;
> > +
> > + if (e1000_desc_unused(tx_ring) < 1)
> > + return E1000_XDP_CONSUMED;
> > +
> > + i = tx_ring->next_to_use;
> Unsynchronized read of next_to_use. ndo_start_xmit on another CPU can be
> reading+writing this same field RIGHT NOW under __netif_tx_lock, which we do
> NOT hold.
> Isn't it ?
>
> > + buffer_info = &tx_ring->buffer_info[i];
> > +
> > + dma = dma_map_single(&adapter->pdev->dev, xdpf->data, xdpf-
> > >len,
> > + DMA_TO_DEVICE);
> > + if (dma_mapping_error(&adapter->pdev->dev, dma))
> > + return E1000_XDP_CONSUMED;
> > +
> > + buffer_info->xdpf = xdpf;
> > + buffer_info->type = E1000_TX_BUF_XDP;
> > + buffer_info->dma = dma;
> > + buffer_info->length = xdpf->len;
> > + buffer_info->time_stamp = jiffies;
> > + buffer_info->next_to_watch = i;
> > + buffer_info->segs = 1;
> > + buffer_info->bytecount = xdpf->len;
> > + buffer_info->mapped_as_page = 0;
> > +
> > + tx_desc = E1000_TX_DESC(*tx_ring, i);
> > + tx_desc->buffer_addr = cpu_to_le64(dma);
> Writing DMA descriptor that ndo_start_xmit may also be writing to
> at the same index - probably causes ring corruption
>
> > + tx_desc->lower.data = cpu_to_le32(adapter->txd_cmd |
> > + E1000_TXD_CMD_IFCS |
> > + xdpf->len);
> > + tx_desc->upper.data = 0;
> > +
> > + i++;
> > + if (i == tx_ring->count)
> > + i = 0;
> > + tx_ring->next_to_use = i;
> Unsynchronized store - races with the identical write in e1000_xmit_frame.
>
> > +
> > + return E1000_XDP_TX;
> > +}
> > +
>
> ...
>
> > kfree(adapter->tx_ring);
> > kfree(adapter->rx_ring);
> >
> > --
> > 2.53.0
>
You're right. I see two solutions here: taking __netif_tx_lock around
the XDP TX, or using a separate TX ring dedicated to XDP.
The latter would be a bigger change for e1000e since it only has one
TX queue, so I'd go with the lock.
Thanks,
--
Matteo Croce
perl -e 'for($t=0;;$t++){print chr($t*($t>>8|$t>>13)&255)}' |aplay
