On 17/12/2024 17:38, Sven Eckelmann wrote:
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.
I am not aware of 0 being used as a sane VID. I have seen it mostly used internally, but never truly used with something like "eth0.0".
Therefore I agree with Sven that this should not cause any real compat issue. If we truly break something, then we can probably assume that the scenario was already ill formed.
Regards,