netdev - Re: [PATCH net-next v2 12/18] xdp: add generic xdp_build_skb_from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4dda1ca6-f684-4b6a-8972-3915921cbf6d@intel.com>
Date: Mon, 21 Oct 2024 16:20:24 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
CC: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Toke Høiland-Jørgensen
	<toke@...hat.com>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann
	<daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>, "Andrii
 Nakryiko" <andrii@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, "Magnus
 Karlsson" <magnus.karlsson@...el.com>,
	<nex.sw.ncis.osdt.itp.upstreaming@...el.com>, <bpf@...r.kernel.org>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2 12/18] xdp: add generic
 xdp_build_skb_from_buff()

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