On Wed, Sep 09, 2020 at 02:06:06PM +0200, Simon Wunderlich wrote:
if (unlikely(atomic_read(&bat_priv->bla.num_requests))) /* don't allow broadcasts while requests are in flight */
if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast)
if (is_multicast_ether_addr(ethhdr->h_dest) &&(!is_broadcast_ether_addr(ethhdr->h_dest) || is_bcast)) goto handled;Isn't this exactly the same logic as it was before?
is_multicast == 0, is_bcast = 0 => 0 is_multicast == 0, is_bcast = 1 => 0 is_multicast == 1, is_bcast = 0 => 0 is_multicast == 1, is_bcast = 1 => 1
The 3rd one should be different. Note that "is_bcast" is not the same as is_broadcast_ether_addr(ethhdr->h_dest). The former refers to the batman-adv packet header, while the latter refers to the destination MAC of the inner ethernet header.
On Friday, September 4, 2020 8:28:02 PM CEST Linus Lüssing wrote:
For DHCPv6: This is even trickier... DHCPv6 potentially uses non-broadcast multicast addresses. However according to RFC8415, section 7.1 it seems that currently multicast is only used from a DHCPv6 client to a DHCPv6 server, but not the other way round.
Working through the gateway feature part in batadv_interface_tx() it can be inferred that a DHCPv6 packet to a DHCP client would have been the only option for a DHCPv6 multicast packet to be sent via unicast through the gateway feature. Ergo, the newly introduced claim check won't wrongly drop a DHCPv6 packet received via the gateway feature either.
I also don't get this part. Maybe it helps if you can explain the two directions (client -> server, server -> client), and in which cases there can be multicast, and then describe why your check is sufficient?
Hm, actually it's not just the description that is messed up, I think. server->client is ok, but client->server isn't...
* DHCPv6 server -> client: -> Easy, according to RFC8415, section 7.1 this would always be unicast. So neither the Gateway nor Multicast feature would touch it.
* DHCPv6 client -> server: -> Actually both the gateway feature and multicast feature can use it. I misread the code...
I'm a bit uncertain how to solve the latter now... I see no way to distinguish gw vs. mcast feature as is. We also have no flags or reserved space in the batadv_unicast_packet available to make them distinguishable.
So the only solution I could think of for now is excluding DHCPv6 from multicast feature in TX of the originator... (in batadv_mcast_forw_mode_check_ipv6(), adding excludes for ff02::1:2 and ff05::1:3).
(Even though having the multicast feature handling it would have been nice(r) as it'd work without needing a user to set and maintain the gateway mode properly.)