netdev - Re: [PATCH v4 bpf-next 15/22] xsk: add multi-buffer documentation

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: <87zg4idm1q.fsf@toke.dk>
Date: Thu, 29 Jun 2023 22:57:05 +0200
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: Magnus Karlsson <magnus.karlsson@...il.com>, Jakub Kicinski
 <kuba@...nel.org>, bpf@...r.kernel.org, ast@...nel.org,
 daniel@...earbox.net, andrii@...nel.org, netdev@...r.kernel.org,
 magnus.karlsson@...el.com, bjorn@...nel.org, tirthendu.sarkar@...el.com,
 simon.horman@...igine.com
Subject: Re: [PATCH v4 bpf-next 15/22] xsk: add multi-buffer documentation

Maciej Fijalkowski <maciej.fijalkowski@...el.com> writes:

> On Wed, Jun 28, 2023 at 11:02:06PM +0200, Toke Høiland-Jørgensen wrote:
>> > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
>> > index a4270fafdf11..b24244f768e3 100644
>> > --- a/net/core/netdev-genl.c
>> > +++ b/net/core/netdev-genl.c
>> > @@ -19,6 +19,8 @@ netdev_nl_dev_fill(struct net_device *netdev, struct sk_buff *rsp,
>> >  		return -EMSGSIZE;
>> >  
>> >  	if (nla_put_u32(rsp, NETDEV_A_DEV_IFINDEX, netdev->ifindex) ||
>> > +	    nla_put_u32(rsp, NETDEV_A_DEV_XDP_ZC_MAX_SEGS,
>> > +			netdev->xdp_zc_max_segs) ||
>> 
>> Should this be omitted if the driver doesn't support zero-copy at all?
>
> This is now set independently when allocing net_device struct, so this can
> be read without issues. Furthermore this value should not be used to find
> out if underlying driver supports ZC or not - let us keep using
> xdp_features for that.
>
> Does it make sense?

Yes, I agree we shouldn't use this field for that. However, I am not
sure I trust all userspace applications to get that right, so I fear
some will end up looking at the field even when the flag is not set,
which will lead to confused users. So why not just omit the property
entirely when the flag is not set? :)

-Toke