[PATCH net] xsk: restore UMEM flags after failed pool assignment

[PATCH net] xsk: restore UMEM flags after failed pool assignment

[Paul Moses]

xp_assign_dev() sets XDP_UMEM_SG_FLAG before checking whether the
device supports multibuffer zero-copy. If assignment fails, xsk_bind()
destroys the pool but retains the registered UMEM. A subsequent bind
without XDP_USE_SG inherits the stale flag.

The flag also changes RX buffer sizing after DMA mapping. On igc, a
failed SG bind followed by a non-SG retry with 4096-byte chunks and
1792 bytes of headroom passes admission with 2048 bytes, but programs
a 1024-byte RX buffer. On I225-V hardware, an ordinary 1514-byte frame
is truncated to 1024 bytes or 1008 with RX timestamps enabled.
Restoring the original flags eliminates the truncation in both cases.

Save the UMEM flags before assignment and restore them on failure.

Fixes: d609f3d228a8 ("xsk: add multi-buffer support for sockets sharing umem")
Cc: stable@vger.kernel.org
Signed-off-by: Paul Moses <p@1g4.org>
---
 net/xdp/xsk_buff_pool.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 0ebd4d46f7ab..7750d2ec4ee2 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -166,6 +166,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
 {
 	u32 needed = netdev->mtu + ETH_PAD_LEN;
 	u32 segs = netdev->xdp_zc_max_segs;
+	u8 umem_flags = pool->umem->flags;
 	bool mbuf = flags & XDP_USE_SG;
 	bool force_zc, force_copy;
 	struct netdev_bpf bpf;
@@ -268,6 +269,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
 	if (!force_zc)
 		err = 0; /* fallback to copy mode */
 	if (err) {
+		pool->umem->flags = umem_flags;
 		xsk_clear_pool_at_qid(netdev, queue_id);
 		hlist_del(&pool->dev_node);
 		pool->netdev = NULL;

-- 
2.56.GIT

Re: [PATCH net] xsk: restore UMEM flags after failed pool assignment

[Maciej Fijalkowski]

On Wed, Oct 21, 2026 at 10:47:33PM +0000, Paul Moses wrote:
> xp_assign_dev() sets XDP_UMEM_SG_FLAG before checking whether the
> device supports multibuffer zero-copy. If assignment fails, xsk_bind()
> destroys the pool but retains the registered UMEM. A subsequent bind
> without XDP_USE_SG inherits the stale flag.

so you opened a socket, tried binding with XDP_USE_SG, failed then retried
without the flag and you noticed unexpected rx frame size due to kept
XDP_UMEM_SG_FLAG - do i get this right?

socket not being closed kept the umem alive...

> 
> The flag also changes RX buffer sizing after DMA mapping. On igc, a
> failed SG bind followed by a non-SG retry with 4096-byte chunks and
> 1792 bytes of headroom passes admission with 2048 bytes, but programs
> a 1024-byte RX buffer. On I225-V hardware, an ordinary 1514-byte frame
> is truncated to 1024 bytes or 1008 with RX timestamps enabled.
> Restoring the original flags eliminates the truncation in both cases.

It should be enough to state that sg flag presence takes into account
tailroom size and therefore has influence on final rx frame size provided
to driver.

> 
> Save the UMEM flags before assignment and restore them on failure.

Regarding the patch content:
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

> 
> Fixes: d609f3d228a8 ("xsk: add multi-buffer support for sockets sharing umem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Paul Moses <p@1g4.org>
> ---
>  net/xdp/xsk_buff_pool.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 0ebd4d46f7ab..7750d2ec4ee2 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -166,6 +166,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
>  {
>  	u32 needed = netdev->mtu + ETH_PAD_LEN;
>  	u32 segs = netdev->xdp_zc_max_segs;
> +	u8 umem_flags = pool->umem->flags;
>  	bool mbuf = flags & XDP_USE_SG;
>  	bool force_zc, force_copy;
>  	struct netdev_bpf bpf;
> @@ -268,6 +269,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
>  	if (!force_zc)
>  		err = 0; /* fallback to copy mode */
>  	if (err) {
> +		pool->umem->flags = umem_flags;
>  		xsk_clear_pool_at_qid(netdev, queue_id);
>  		hlist_del(&pool->dev_node);
>  		pool->netdev = NULL;
> 
> -- 
> 2.56.GIT
> 
> 

Re: [PATCH net] xsk: restore UMEM flags after failed pool assignment

[Paul Moses]

On Thursday, October 22nd, 2026 at 3:34 AM, Maciej Fijalkowski <maciej.fijalkowski@intel.com> wrote:

> On Wed, Oct 21, 2026 at 10:47:33PM +0000, Paul Moses wrote:
> > xp_assign_dev() sets XDP_UMEM_SG_FLAG before checking whether the
> > device supports multibuffer zero-copy. If assignment fails, xsk_bind()
> > destroys the pool but retains the registered UMEM. A subsequent bind
> > without XDP_USE_SG inherits the stale flag.
>
> so you opened a socket, tried binding with XDP_USE_SG, failed then retried
> without the flag and you noticed unexpected rx frame size due to kept
> XDP_UMEM_SG_FLAG - do i get this right?
>
> socket not being closed kept the umem alive...
>
> >
> > The flag also changes RX buffer sizing after DMA mapping. On igc, a
> > failed SG bind followed by a non-SG retry with 4096-byte chunks and
> > 1792 bytes of headroom passes admission with 2048 bytes, but programs
> > a 1024-byte RX buffer. On I225-V hardware, an ordinary 1514-byte frame
> > is truncated to 1024 bytes or 1008 with RX timestamps enabled.
> > Restoring the original flags eliminates the truncation in both cases.
>
> It should be enough to state that sg flag presence takes into account
> tailroom size and therefore has influence on final rx frame size provided
> to driver.
>
> >
> > Save the UMEM flags before assignment and restore them on failure.
>
> Regarding the patch content:
> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> >
> > Fixes: d609f3d228a8 ("xsk: add multi-buffer support for sockets sharing umem")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Paul Moses <p@1g4.org>
> > ---
> >  net/xdp/xsk_buff_pool.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> > index 0ebd4d46f7ab..7750d2ec4ee2 100644
> > --- a/net/xdp/xsk_buff_pool.c
> > +++ b/net/xdp/xsk_buff_pool.c
> > @@ -166,6 +166,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
> >  {
> >  	u32 needed = netdev->mtu + ETH_PAD_LEN;
> >  	u32 segs = netdev->xdp_zc_max_segs;
> > +	u8 umem_flags = pool->umem->flags;
> >  	bool mbuf = flags & XDP_USE_SG;
> >  	bool force_zc, force_copy;
> >  	struct netdev_bpf bpf;
> > @@ -268,6 +269,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
> >  	if (!force_zc)
> >  		err = 0; /* fallback to copy mode */
> >  	if (err) {
> > +		pool->umem->flags = umem_flags;
> >  		xsk_clear_pool_at_qid(netdev, queue_id);
> >  		hlist_del(&pool->dev_node);
> >  		pool->netdev = NULL;
> >
> > --
> > 2.56.GIT
> >
> >
>

Right, evidence suggests igc handles it gracefully, I had attempted 
to provide multiple examples of real world impact to satiate Sashiko,
but only igc survived final edit. It needs to be open for the failed 
bind and successful retry, it can be closed after and bad state 
remains. If you're looking for greater impact, look at stmmac. I'll
send out a v2 without naming drivers specifically soon.

Re: [PATCH net] xsk: restore UMEM flags after failed pool assignment

[Maciej Fijalkowski]

On Thu, Oct 22, 2026 at 04:55:59PM +0000, Paul Moses wrote:
> On Thursday, October 22nd, 2026 at 3:34 AM, Maciej Fijalkowski <maciej.fijalkowski@intel.com> wrote:
> 
> > On Wed, Oct 21, 2026 at 10:47:33PM +0000, Paul Moses wrote:
> > > xp_assign_dev() sets XDP_UMEM_SG_FLAG before checking whether the
> > > device supports multibuffer zero-copy. If assignment fails, xsk_bind()
> > > destroys the pool but retains the registered UMEM. A subsequent bind
> > > without XDP_USE_SG inherits the stale flag.
> >
> > so you opened a socket, tried binding with XDP_USE_SG, failed then retried
> > without the flag and you noticed unexpected rx frame size due to kept
> > XDP_UMEM_SG_FLAG - do i get this right?
> >
> > socket not being closed kept the umem alive...
> >
> > >
> > > The flag also changes RX buffer sizing after DMA mapping. On igc, a
> > > failed SG bind followed by a non-SG retry with 4096-byte chunks and
> > > 1792 bytes of headroom passes admission with 2048 bytes, but programs
> > > a 1024-byte RX buffer. On I225-V hardware, an ordinary 1514-byte frame
> > > is truncated to 1024 bytes or 1008 with RX timestamps enabled.
> > > Restoring the original flags eliminates the truncation in both cases.
> >
> > It should be enough to state that sg flag presence takes into account
> > tailroom size and therefore has influence on final rx frame size provided
> > to driver.
> >
> > >
> > > Save the UMEM flags before assignment and restore them on failure.
> >
> > Regarding the patch content:
> > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> >
> > >
> > > Fixes: d609f3d228a8 ("xsk: add multi-buffer support for sockets sharing umem")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Paul Moses <p@1g4.org>
> > > ---
> > >  net/xdp/xsk_buff_pool.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> > > index 0ebd4d46f7ab..7750d2ec4ee2 100644
> > > --- a/net/xdp/xsk_buff_pool.c
> > > +++ b/net/xdp/xsk_buff_pool.c
> > > @@ -166,6 +166,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
> > >  {
> > >  	u32 needed = netdev->mtu + ETH_PAD_LEN;
> > >  	u32 segs = netdev->xdp_zc_max_segs;
> > > +	u8 umem_flags = pool->umem->flags;
> > >  	bool mbuf = flags & XDP_USE_SG;
> > >  	bool force_zc, force_copy;
> > >  	struct netdev_bpf bpf;
> > > @@ -268,6 +269,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
> > >  	if (!force_zc)
> > >  		err = 0; /* fallback to copy mode */
> > >  	if (err) {
> > > +		pool->umem->flags = umem_flags;
> > >  		xsk_clear_pool_at_qid(netdev, queue_id);
> > >  		hlist_del(&pool->dev_node);
> > >  		pool->netdev = NULL;
> > >
> > > --
> > > 2.56.GIT
> > >
> > >
> >
> 
> Right, evidence suggests igc handles it gracefully, I had attempted 
> to provide multiple examples of real world impact to satiate Sashiko,
> but only igc survived final edit. It needs to be open for the failed 
> bind and successful retry, it can be closed after and bad state 
> remains. If you're looking for greater impact, look at stmmac. I'll
> send out a v2 without naming drivers specifically soon.

Not really needed to go with v2 with reworded message. Let us wait for
bot reports first.