netdev - Re: [PATCH net-next 5/5] ixgbe: xsk: add TX multi-buffer support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL+tcoD0W2owb211ZAO7M3qWU=EFGx+S9O7GNKidj0+oowfpdw@mail.gmail.com>
Date: Fri, 25 Jul 2025 18:09:09 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: anthony.l.nguyen@...el.com, przemyslaw.kitszel@...el.com, 
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, bjorn@...nel.org, 
	magnus.karlsson@...el.com, jonathan.lemon@...il.com, sdf@...ichev.me, 
	ast@...nel.org, daniel@...earbox.net, hawk@...nel.org, 
	john.fastabend@...il.com, bpf@...r.kernel.org, 
	intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org, 
	Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next 5/5] ixgbe: xsk: add TX multi-buffer support

Hi Maciej,

On Fri, Jul 25, 2025 at 6:00 PM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> On Sun, Jul 20, 2025 at 05:11:23PM +0800, Jason Xing wrote:
> > From: Jason Xing <kernelxing@...cent.com>
> >
> > Use the common interface to see if the desc is the end of packets. If
> > so, set IXGBE_TXD_CMD_EOP bit instead of setting for all preceding
> > descriptors. This is also how i40e driver did in commit a92b96c4ae10
> > ("i40e: xsk: add TX multi-buffer support").
> >
> > Signed-off-by: Jason Xing <kernelxing@...cent.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++++
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 4 +++-
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index a59fd8f74b5e..c34737065f9e 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -52,6 +52,8 @@
> >  #include "ixgbe_txrx_common.h"
> >  #include "devlink/devlink.h"
> >
> > +#define IXGBE_MAX_BUFFER_TXD 4
> > +
> >  char ixgbe_driver_name[] = "ixgbe";
> >  static const char ixgbe_driver_string[] =
> >                             "Intel(R) 10 Gigabit PCI Express Network Driver";
> > @@ -11805,6 +11807,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >       netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
> >                              NETDEV_XDP_ACT_XSK_ZEROCOPY;
> >
> > +     netdev->xdp_zc_max_segs = IXGBE_MAX_BUFFER_TXD;
>
> Hi Jason,
>
> nack to this as you would allow fragmented frames on Rx side which is not
> supported even with your patchset.

I'm not sure about this one, to be honest when I observed no
performance impact with this patch. How could we support the idea of
this patch, I wonder? Do we need to correspondingly adjust the
hardware? Sorry that I wasn't able to find such information in the
datasheet :(

>
> Generally ixgbe needs some love, i have several patches in my backlog plus
> I think Larysa will be focusing on this driver.

Though ixgbe is an old driver, we still have thousands of machines
running with this driver. Looking forward to your patch then.

Thanks,
Jason

>
> please stick to enabling xsk batching on tx side.
>
> > +
> >       /* MTU range: 68 - 9710 */
> >       netdev->min_mtu = ETH_MIN_MTU;
> >       netdev->max_mtu = IXGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > index 9fe2c4bf8bc5..3d9fa4f2403e 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > @@ -424,7 +424,9 @@ static void ixgbe_xmit_pkt(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc,
> >       cmd_type = IXGBE_ADVTXD_DTYP_DATA |
> >                  IXGBE_ADVTXD_DCMD_DEXT |
> >                  IXGBE_ADVTXD_DCMD_IFCS;
> > -     cmd_type |= desc[i].len | IXGBE_TXD_CMD_EOP;
> > +     cmd_type |= desc[i].len;
> > +     if (xsk_is_eop_desc(&desc[i]))
> > +             cmd_type |= IXGBE_TXD_CMD_EOP;
> >       tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
> >       tx_desc->read.olinfo_status =
> >               cpu_to_le32(desc[i].len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > --
> > 2.41.3
> >