netdev - Re: [PATCH RESEND bpf-next 10/15] ice: Implement VLAN tag hint

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]

Date: Mon, 15 May 2023 17:45:54 +0200
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Jesper Dangaard Brouer <jbrouer@...hat.com>
CC: Stanislav Fomichev <sdf@...gle.com>, <brouer@...hat.com>,
	<bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann
	<daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Jakub Kicinski
	<kuba@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu
	<song@...nel.org>, Yonghong Song <yhs@...com>, John Fastabend
	<john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Jiri Olsa
	<jolsa@...nel.org>, Jesse Brandeburg <jesse.brandeburg@...el.com>, "Tony
 Nguyen" <anthony.l.nguyen@...el.com>, Anatoly Burakov
	<anatoly.burakov@...el.com>, Alexander Lobakin <alexandr.lobakin@...el.com>,
	Magnus Karlsson <magnus.karlsson@...il.com>, Maryam Tahhan
	<mtahhan@...hat.com>, <xdp-hints@...-project.net>, <netdev@...r.kernel.org>,
	<intel-wired-lan@...ts.osuosl.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND bpf-next 10/15] ice: Implement VLAN tag hint

On Mon, May 15, 2023 at 05:07:19PM +0200, Jesper Dangaard Brouer wrote:
> 
> 
> On 15/05/2023 15.41, Larysa Zaremba wrote:
> > > > +	*vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc);
> > > Should we also do the following:
> > > 
> > > if (!*vlan_tag)
> > > 	return -ENODATA;
> > > 
> > > ?
> > Oh, returning VLAN tag with zero value really made sense to me at the beginning,
> > but after playing with different kinds of packets, I think returning error makes
> > more sense. Will change.
> > 
> 
> IIRC then VLAN tag zero is also a valid id, right?

AFAIK, 0x000 is reseved and basically means "no vlan tag". When ice hardware 
returns such value in descriptor, it says "no vlan tag was stripped" and this 
doesn't necessarily mean there is no VLAN tag in the packet.

For example, let us consider a packet:

  Ether/802.1ad(s-tag)/802.1q(c-tag)/...

Hardware does not strip c-tag in such case and sends 0x000 in the descriptor, 
but packet clearly does contain a c-tag, so at least in ice, it is reasonable to
not consider '0' a reliable value.

I guess, for s-tag value of 0x000 should be more reliable, so maybe
'if (!*vlan_tag)' usage can be limited to c-tag function.

> 
> --Jesper
>