Re: [PATCH] batman-adv: Map VID 0 to untagged TT VLAN

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 2932 bytes --]

On Tuesday, 17 December 2024 14:53:23 CET Linus Lüssing wrote:
> On Mon, Dec 16, 2024 at 07:37:12PM +0100, Sven Eckelmann wrote:
> > diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
> > index 8e0f44c71696f642d80304ec2724e8b5e56a5d93..333e947afcce7ca4128be8406f23295df723515c 100644
> > --- a/net/batman-adv/main.c
> > +++ b/net/batman-adv/main.c
> > @@ -637,6 +637,13 @@ unsigned short batadv_get_vid(struct sk_buff *skb, size_t header_len)
> >  
> >  	vhdr = (struct vlan_ethhdr *)(skb->data + header_len);
> >  	vid = ntohs(vhdr->h_vlan_TCI) & VLAN_VID_MASK;
> > +
> > +	/* VID 0 is only used to indicate "priority tag" frames which only
> > +	 * contain priority information and no VID.
> > +	 */
> > +	if (vid == 0)
> > +		return BATADV_NO_FLAGS;
> > +
> >  	vid |= BATADV_VLAN_HAS_TAG;
> >  
> >  	return vid;
> 
> I guess with this patch all TT entries previously in
> TT VLAN 0 would be moved to untagged/NO_FLAGS TT entries, right?

Yes, as specified by 802.1Q-2011, it is meant to transport only priority 
information and not a VID. For a switch, the PVID would be used but because 
batman-adv is here used as the lower device (for either a VLAN aware bridge or 
8021q device), we don't have to add the PVID - the VID is simply missing 
(because it is !BATADV_VLAN_HAS_TAG) and therefore has to check for the 
"untagged" TT global entries (or add entries to the "untagged" TT local part)

> Wouldn't that technically break compatibility? Let's say someone
> uses VLAN headers with VID 0 to be able to use priorities / QoS.

Then this person should have noticed that it broken at the moment and doesn't 
work as expected (to reach the "untagged remotes" with the priority tagged
packets)

> What if some old nodes still announced+used VLAN 0 in batman-adv
> while others used it after this patch, with the mapping to
> NO_FLAGS?

Then the misbehaving old node would still misbehave. Because you should 
actually be able to talk with VID 0 to the untagged global TT entries - which 
the old node fails to do. So I could also add

  Fixes: 0ffa9e8d86d6 ("batman-adv: use vid when computing local and global TT CRC")
  Fixes: 5d2c05b21337 ("batman-adv: add per VLAN interface attribute framework")

if you prefer and transmit it via the batadv/net queue.

But I considered VID 0 somewhat esoteric for in-Linux usage because most tools 
just use DSCP. I am only away of tools like isochron-send which just inject 
raw packets with the VLAN headers directly. And using another 8021q device 
with VID on one side is a good way to create a unidirectional communication 
(when you want a bidirectional one) because the other end will just reply
with a vanilla, untagged packet. And because of that, things like ARP will
not be able to "finish" because the answers are received on the non-VID0 
interface.

But maybe I am wrong about that.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]