Re: [B.A.T.M.A.N.] [PATCHv12 1/4] batman-adv: Always flood IGMP/MLD reports

Simon Wunderlich
2016-04-07T13:03:11Z

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

On Wednesday 16 March 2016 05:06:42 Linus Lüssing wrote:
> With this patch IGMP or MLD reports are always flooded. This is
> necessary for the upcoming bridge integration to function without
> multicast packet loss.
>
> With the report handling so far bridges might miss interested multicast
> listeners, leading to wrongly excluding ports from multicast packet
> forwarding.
>
> Currently we are treating IGMP/MLD reports, the messages bridges use to
> learn about interested multicast listeners, just as any other multicast
> packet: We try to send them to nodes matching its multicast destination.
>
> Unfortunately, the destination address of reports of the older
> IGMPv2/MLDv1 protocol families do not strictly adhere to their own
> protocol: More precisely, the interested receiver, an IGMPv2 or MLDv1
> querier, itself usually does not listen to the multicast destination
> address of any reports.
>
> Therefore with this patch we are simply excluding IGMP/MLD reports from
> the multicast forwarding code path and keep flooding them. By that
> any bridge receives them and can properly learn about listeners.
>
> To avoid compatibility issues with older nodes not yet implementing this
> report handling, we need to force them to flood reports: We do this by
> bumping the multicast TVLV version to 2, effectively disabling their
> multicast optimization.

It looks like you register two containers, one with version 1 and version 2,
both with the same content. Do we really need to have both containers?
Wouldn't it be sufficient to register only version 2 and leave version 1 out?

Other than that, I just found a minor style thing, see below.

>
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
> index 8caa2c7..19cc404 100644
> --- a/net/batman-adv/multicast.c
> +++ b/net/batman-adv/multicast.c
> @@ -320,6 +391,31 @@ static int batadv_mcast_forw_mode_check_ipv4(struct
> batadv_priv *bat_priv, return 0;
> }
>
> +#if IS_ENABLED(CONFIG_IPV6)
> +/**
> + * batadv_mcast_is_report_ipv6 - check for MLD reports

put a space after the dash?

Cheers,
Simon

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