Re: [PATCH net] net: ti: icssg-prueth: Fix skb handling for XDP_PASS

[Meghana Malladi]

Hi Jakub,

On 7/24/25 05:49, Jakub Kicinski wrote:
> On Mon, 21 Jul 2025 18:19:18 +0530 Meghana Malladi wrote:
> > diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> > index 12f25cec6255..a0e7def33e8e 100644
> > --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> > +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> > @@ -757,15 +757,12 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, u32 *xdp_state)
> >   		xdp_prepare_buff(&xdp, pa, PRUETH_HEADROOM, pkt_len, false);
> >   
> >   		*xdp_state = emac_run_xdp(emac, &xdp, page, &pkt_len);
> > -		if (*xdp_state == ICSSG_XDP_PASS)
> > -			skb = xdp_build_skb_from_buff(&xdp);
> > -		else
> > +		if (*xdp_state != ICSSG_XDP_PASS)
> >   			goto requeue;
> > -	} else {
> > -		/* prepare skb and send to n/w stack */
> > -		skb = napi_build_skb(pa, PAGE_SIZE);
> >   	}
> >   
> > +	/* prepare skb and send to n/w stack */
> > +	skb = napi_build_skb(pa, PAGE_SIZE);
> >   	if (!skb) {
> >   		ndev->stats.rx_dropped++;
> >   		page_pool_recycle_direct(pool, page);
>
> I'm not sure this is correct. We seem to hardcode headroom to
> PRUETH_HEADROOM lower in this function. If XDP adds or removes
> network headers and then returns XDP_PASS we'll look for the packet
> at the wrong offset.

Yes that makes sense. Thanks for pointing it out. I think I have the 
right fix in mind. Will post it shortly as v2.

> We just merged some XDP tests, could you try running
> tools/testing/selftests/drivers/net/xdp.py ?
> Some general instructions can be found here:
> https://github.com/linux-netdev/nipa/wiki/Running-driver-tests
>
> Not sure how stable the test is for all NICs but I think the
>   xdp.test_xdp_native_adjst_head_grow_data
> test case will exercise what I'm suspecting will fail.

It took me some time to install all the dependencies and get this tool 
running with the fix I mentioned above [1]. But I am not sure the error 
logs I got [2] are the one which you were expecting or some stability 
issue with the test. Can you please take a look at it.

[1]: 
https://gist.github.com/MeghanaMalladiTI/ce6a440f05fbdfb2d4363f672296d7d8
[2]:https://gist.github.com/MeghanaMalladiTI/52fe9d06c114562be08105d73f91ba62 


If the fix I attached here [1] makes sense, I will go ahead and post 
this as v2.

Thanks.