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

["Malladi, Meghana" <m-malladi@ti.com>]

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@kernel.org>
>> 
>> 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@kernel.org>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> 
> [...]
> 
>> +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()
>