Re: [PATCH net-next v2 12/18] xdp: add generic xdp_build_skb_from_buff()

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: Thu, 17 Oct 2024 14:34:33 +0200

> On Tue, Oct 15, 2024 at 04:53:44PM +0200, Alexander Lobakin wrote:
>> The code which builds an skb from an &xdp_buff keeps multiplying itself
>> around the drivers with almost no changes. Let's try to stop that by
>> adding a generic function.

[...]

>> +	skb_record_rx_queue(skb, rxq->queue_index);
>> +
>> +	if (unlikely(nr_frags)) {
>> +		u32 ts;
> 
> nit: spell out truesize? ts confuse my brain with timestamp TBH

Ack.

> 
>> +
>> +		ts = sinfo->xdp_frags_truesize ? : nr_frags * xdp->frame_sz;
>> +		xdp_update_skb_shared_info(skb, nr_frags,
>> +					   sinfo->xdp_frags_size, ts,
>> +					   xdp_buff_is_frag_pfmemalloc(xdp));
>> +	}
>> +
>> +	skb->protocol = eth_type_trans(skb, rxq->dev);
> 
> could we leave this out to be set by drivers? i see in ice for example
> netdev ptr is retrieved in different ways here.

As for ice, it's for representors only + unlikely() + we could override
it manually there later (set skb->dev + call eth_skb_pkt_type()).
OTOH this looks more consistent with xdp_build_skb_from_frame() + saves
some object code (you won't need to generate a call to eth_type_trans()
in each driver).

> 
>> +
>> +	return skb;
>> +}
>> +EXPORT_SYMBOL_GPL(xdp_build_skb_from_buff);
>> +
>>  struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
>>  					   struct sk_buff *skb,
>>  					   struct net_device *dev)
>> -- 
>> 2.46.2

Thanks,
Olek