Re: [PATCH net v2 1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
Jason Xing <[email protected]>
| Newsgroups | dev.linux.lists.sashiko,org.kernel.vger.bpf |
|---|---|
| Message-ID | <CAL+tcoDp2ZgO629J0fw1TBJ=fdf0KmWz3i6mWODdLnSbmLv_sA@mail.gmail.com> |
On Tue, Apr 21, 2026 at 5:40 PM <[email protected]> wrote: > > Sashiko AI review found 1 potential issue(s): > - [High] The patch unconditionally rejects pool assignment for devices with IFF_TX_SKB_NO_LINEAR if tx_sw_csum is enabled, which incorrectly breaks native Zero-Copy (ZC) setups on those devices. > -- > > commit 5beba1aaed8c200424f98af3c07472ab4da8d451 > Author: Jason Xing <[email protected]> > > xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices > > This commit rejects UMEM bindings for devices that advertise > IFF_TX_SKB_NO_LINEAR when software checksumming (tx_sw_csum) is enabled. > This prevents a warning and panic in skb_checksum_help() which relies on a > linear skb head. > > Link: https://lore.kernel.org/all/[email protected]/#t > > > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c > > index cd7bc50872f6b..d981cfdd85357 100644 > > --- a/net/xdp/xsk_buff_pool.c > > +++ b/net/xdp/xsk_buff_pool.c > > @@ -175,6 +175,9 @@ int xp_assign_dev(struct xsk_buff_pool *pool, > > if (force_zc && force_copy) > > return -EINVAL; > > > > + if (pool->tx_sw_csum && (netdev->priv_flags & IFF_TX_SKB_NO_LINEAR)) > > + return -EOPNOTSUPP; > > + > > Does this unconditionally reject the pool assignment even for native > Zero-Copy (ZC) setups? It's weird. Nobody uses sw csum in zc mode.... If someone uses this and gets warned, he will surely root cause the reason and finally adjust the code in application. > > In native XDP_ZEROCOPY mode, AF_XDP descriptors are sent directly to the > hardware. No sk_buff is allocated, so skb_checksum_help() is never called, > making the tx_sw_csum flag irrelevant and safe to ignore. > > By placing this check before determining if native ZC is available, does > this break ZC mode on drivers like virtio_net for applications that pass > the XDP_UMEM_TX_SW_CSUM flag as a fallback? > > Should this check be deferred until we are certain we are using generic > copy mode, such as when force_copy is true or after native ZC setup fails? It's more complex. I'll keep my code as-is unless someone screams at me to handle this trivial case. Thanks, Jason > > > if (xsk_get_pool_from_qid(netdev, queue_id)) > > return -EBUSY; > > > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1