Re: [EXTERNAL] Re: [PATCH net 3/3] net: ti: icssg-prueth: Add AF_XDP support

[Malladi, Meghana]

On 1/23/2025 10:55 PM, Ido Schimmel wrote:
> s/AF_XDP/XDP/ ? On Wed, Jan 22, 2025 at 06: 19: 51PM +0530, Meghana 
> Malladi wrote: > From: Roger Quadros <rogerq@ kernel. org> > > Add 
> native XDP support. We do not support zero copy yet. There are various 
> XDP features (e. g. , NETDEV_XDP_ACT_BASIC)
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> v9dnXbjPnQP15quJdjhJcrR0CoEiGINyhi1SO3vz-ZB8Sgif7YzLnG8ayC2XmAQz6g$>
> ZjQcmQRYFpfptBannerEnd
>
> s/AF_XDP/XDP/ ?

Will fix the typo.

> On Wed, Jan 22, 2025 at 06:19:51PM +0530, Meghana Malladi wrote:
> > From: Roger Quadros <rogerq@xxxxxxxxxx>
> >
> > Add native XDP support. We do not support zero copy yet.
>
> There are various XDP features (e.g., NETDEV_XDP_ACT_BASIC) to tell the
> stack what the driver supports. I don't see any of them being set here.

Ok, I will add the feature flags in the v2 patch series.

> > Signed-off-by: Roger Quadros <rogerq@xxxxxxxxxx>
> > Signed-off-by: MD Danish Anwar <danishanwar@xxxxxx>
>
> [...]
>
> > +static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, int *xdp_state)
> >  {
> >  	struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> >  	u32 buf_dma_len, pkt_len, port_id = 0;
> > @@ -560,10 +732,12 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
> >  	struct page *page, *new_page;
> >  	struct page_pool *pool;
> >  	struct sk_buff *skb;
> > +	struct xdp_buff xdp;
> >  	u32 *psdata;
> >  	void *pa;
> >  	int ret;
> >  
> > +	*xdp_state = 0;
> >  	pool = rx_chn->pg_pool;
> >  	ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
> >  	if (ret) {
> > @@ -602,8 +776,17 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id)
> >  		goto requeue;
> >  	}
> >  
> > -	/* prepare skb and send to n/w stack */
> >  	pa = page_address(page);
> > +	if (emac->xdp_prog) {
> > +		xdp_init_buff(&xdp, PAGE_SIZE, &rx_chn->xdp_rxq);
> > +		xdp_prepare_buff(&xdp, pa, PRUETH_HEADROOM, pkt_len, false);
> > +
> > +		*xdp_state = emac_run_xdp(emac, &xdp, page);
> > +		if (*xdp_state != ICSSG_XDP_PASS)
> > +			goto requeue;
> > +	}
> > +
> > +	/* prepare skb and send to n/w stack */
> >  	skb = build_skb(pa, prueth_rxbuf_total_len(pkt_len));
> >  	if (!skb) {
> >  		ndev->stats.rx_dropped++;
>
> XDP program could have changed the packet length, but driver seems to be

This will be true given, emac->xdp_prog is not NULL. What about when XDP 
is not enabled ?

> building the skb using original length read from the descriptor.
> Consider using xdp_build_skb_from_buff()