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

From: Alexander Lobakin
Date: Mon Oct 21 2024 - 10:21:27 EST


From: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx>
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