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

[Marek Lindner <mareklindner@neomailbox.ch>]

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

On Sunday, June 21, 2015 15:36:17 Linus Lüssing wrote:
> With this patch IGMP or MLD reports are always flooded. This is
> necessary for the upcoming bridge integration:
> 
> An IGMPv2/MLDv1 querier does not actively join the multicast group the
> reports are sent to. Because of this, this would lead to snooping
> bridges/switches not being able to learn of multicast listeners in the
> mesh and wrongly shutting down ports for multicast traffic to the mesh,
> leading to packet loss.

I am not quite clear on what the patch does. It helps to support a feature 
that is yet to come (upcoming bridge integration) or improves the situation 
today ?


>  /**
>   * batadv_mcast_forw_mode_check - check for optimized forwarding potential
> @@ -376,9 +434,11 @@ static int batadv_mcast_forw_mode_check(struct
> batadv_priv *bat_priv, case ETH_P_IP:
>  		return batadv_mcast_forw_mode_check_ipv4(bat_priv, skb,
>  							 is_unsnoopable);
> +#if IS_ENABLED(CONFIG_IPV6)
>  	case ETH_P_IPV6:
>  		return batadv_mcast_forw_mode_check_ipv6(bat_priv, skb,
>  							 is_unsnoopable);
> +#endif
>  	default:
>  		return -EINVAL;
>  	}

This hunk seems not really related to the patch itself ?


>  /**
> - * batadv_mcast_tvlv_ogm_handler_v1 - process incoming multicast tvlv
> container + * batadv_mcast_tvlv_ogm_handler_v2 - process incoming multicast
> tvlv container * @bat_priv: the bat priv with all the soft interface
> information * @orig: the orig_node of the ogm
>   * @flags: flags indicating the tvlv state (see batadv_tvlv_handler_flags)
>   * @tvlv_value: tvlv buffer containing the multicast data
>   * @tvlv_value_len: tvlv buffer length
>   */
> -static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
> +static void batadv_mcast_tvlv_ogm_handler_v2(struct batadv_priv *bat_priv,
>  					     struct batadv_orig_node *orig,
>  					     u8 flags,
>  					     void *tvlv_value,

By removing mcast v1 you effectively are breaking compatibility with all nodes 
running v1 and require everyone to upgrade to v2. Is there a good reason for 
that ? The multicast optimizations can't easily work with v1 and v2 ?

By reading your patch it does not become clear why v2 is needed in the first 
place. The tvlv's content seems to be identical ?


> @@ -206,6 +206,8 @@ static int batadv_interface_tx(struct sk_buff *skb,
>  	if (atomic_read(&bat_priv->mesh_state) != BATADV_MESH_ACTIVE)
>  		goto dropped;
> 
> +	skb_set_network_header(skb, ETH_HLEN);
> +
>  	soft_iface->trans_start = jiffies;
>  	vid = batadv_get_vid(skb, 0);
>  	ethhdr = eth_hdr(skb);

Why is this change part of the patch ?

Cheers,
Marek


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