From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Subject: Re: [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
Date: Tue, 3 Sep 2013 23:00:11 +0200
Message-ID: <20130903210011.GB28889@order.stressinduktion.org>
References: <1378195178-21002-1-git-send-email-dborkman@redhat.com> <1378195178-21002-5-git-send-email-dborkman@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Cc: davem@davemloft.net, netdev@vger.kernel.org
To: Daniel Borkmann <dborkman@redhat.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from order.stressinduktion.org ([87.106.68.36]:35513 "EHLO
	order.stressinduktion.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S932324Ab3ICVAP (ORCPT
	<rfc822;netdev@vger.kernel.org>); Tue, 3 Sep 2013 17:00:15 -0400
Content-Disposition: inline
In-Reply-To: <1378195178-21002-5-git-send-email-dborkman@redhat.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hi Daniel!

On Tue, Sep 03, 2013 at 09:59:34AM +0200, Daniel Borkmann wrote:
> RFC3810, 10. Security Considerations says under subsection 10.1.
> Query Message:
> 
>   A forged Version 1 Query message will put MLDv2 listeners on that
>   link in MLDv1 Host Compatibility Mode. This scenario can be avoided
>   by providing MLDv2 hosts with a configuration option to ignore
>   Version 1 messages completely.
> 
> Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:
> 
>   echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version

I just played around with MLDv2-only mode and noticed that the commit message
diverges from the code:

> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index 005b22f..02cd0c5 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -1112,9 +1112,21 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
>  	return true;
>  }
>  
> +static bool mld_in_v2_mode_only(const struct inet6_dev *idev)
> +{
> +	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 2;

Maybe something like

int val = idev->cnf.force_mld_version ?: dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
return val == 2;

> +}
> +
> +static bool mld_in_v1_mode_only(const struct inet6_dev *idev)
> +{
> +	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1;

Likewise.

> +}
> +
>  static bool mld_in_v1_mode(const struct inet6_dev *idev)
>  {
> -	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1)
> +	if (mld_in_v2_mode_only(idev))
> +		return false;
> +	if (mld_in_v1_mode_only(idev))
>  		return true;
>  	if (idev->cnf.force_mld_version == 1)
>  		return true;

This last if statement could be dropped then.

Thanks,

  Hannes