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 >
