[PATCH net-next] bridge: add igmpv3 and mldv2 query support

[PATCH net-next] bridge: add igmpv3 and mldv2 query support

[Hangbin Liu]

Add bridge IGMPv3 and MLDv2 query support. But before we think it is stable
enough, only enable it when declare in force_igmp/mld_version.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/bridge/br_multicast.c | 203 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 194 insertions(+), 9 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2136e45..9fb47f3 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -35,6 +35,10 @@
 
 #include "br_private.h"
 
+#define IGMP_V3_SEEN(in_dev) \
+	(IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) == 3 || \
+	 IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) == 3)
+
 static void br_multicast_start_querier(struct net_bridge *br,
 				       struct bridge_mcast_own_query *query);
 static void br_multicast_add_router(struct net_bridge *br,
@@ -360,9 +364,8 @@ static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max,
 	return 0;
 }
 
-static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
-						    __be32 group,
-						    u8 *igmp_type)
+static struct sk_buff *br_ip4_alloc_query_v2(struct net_bridge *br,
+					     __be32 group, u8 *igmp_type)
 {
 	struct sk_buff *skb;
 	struct igmphdr *ih;
@@ -428,10 +431,82 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
 	return skb;
 }
 
+static struct sk_buff *br_ip4_alloc_query_v3(struct net_bridge *br,
+					     __be32 group, u8 *igmp_type)
+{
+	struct sk_buff *skb;
+	struct igmpv3_query *ih3;
+	struct ethhdr *eth;
+	struct iphdr *iph;
+
+	skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*iph) +
+						 sizeof(*ih3) + 4);
+	if (!skb)
+		goto out;
+
+	skb->protocol = htons(ETH_P_IP);
+
+	skb_reset_mac_header(skb);
+	eth = eth_hdr(skb);
+
+	ether_addr_copy(eth->h_source, br->dev->dev_addr);
+	eth->h_dest[0] = 1;
+	eth->h_dest[1] = 0;
+	eth->h_dest[2] = 0x5e;
+	eth->h_dest[3] = 0;
+	eth->h_dest[4] = 0;
+	eth->h_dest[5] = 1;
+	eth->h_proto = htons(ETH_P_IP);
+	skb_put(skb, sizeof(*eth));
+
+	skb_set_network_header(skb, skb->len);
+	iph = ip_hdr(skb);
+
+	iph->version = 4;
+	iph->ihl = 6;
+	iph->tos = 0xc0;
+	iph->tot_len = htons(sizeof(*iph) + sizeof(*ih3) + 4);
+	iph->id = 0;
+	iph->frag_off = htons(IP_DF);
+	iph->ttl = 1;
+	iph->protocol = IPPROTO_IGMP;
+	iph->saddr = br->multicast_query_use_ifaddr ?
+		     inet_select_addr(br->dev, 0, RT_SCOPE_LINK) : 0;
+	iph->daddr = htonl(INADDR_ALLHOSTS_GROUP);
+	((u8 *)&iph[1])[0] = IPOPT_RA;
+	((u8 *)&iph[1])[1] = 4;
+	((u8 *)&iph[1])[2] = 0;
+	((u8 *)&iph[1])[3] = 0;
+	ip_send_check(iph);
+	skb_put(skb, 24);
+
+	skb_set_transport_header(skb, skb->len);
+	ih3 = igmpv3_query_hdr(skb);
+
+	*igmp_type = IGMP_HOST_MEMBERSHIP_QUERY;
+	ih3->type = IGMP_HOST_MEMBERSHIP_QUERY;
+	ih3->code = (group ? br->multicast_last_member_interval :
+			    br->multicast_query_response_interval) /
+		   (HZ / IGMP_TIMER_SCALE);
+	ih3->csum = 0;
+	ih3->group = group;
+	ih3->resv = 0;
+	ih3->suppress = 0;
+	ih3->qrv= 2;
+	ih3->qqic = br->multicast_query_interval / HZ;
+	ih3->nsrcs = 0;
+	ih3->csum = ip_compute_csum((void *)ih3, sizeof(struct igmpv3_query ));
+	skb_put(skb, sizeof(*ih3));
+
+	__skb_pull(skb, sizeof(*eth));
+
+out:
+	return skb;
+}
 #if IS_ENABLED(CONFIG_IPV6)
-static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
-						    const struct in6_addr *grp,
-						    u8 *igmp_type)
+static struct sk_buff *br_ip6_alloc_query_v1(struct net_bridge *br,
+					     const struct in6_addr *grp,
+					     u8 *igmp_type)
 {
 	struct sk_buff *skb;
 	struct ipv6hdr *ip6h;
@@ -514,19 +589,129 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
 out:
 	return skb;
 }
+
+static struct sk_buff *br_ip6_alloc_query_v2(struct net_bridge *br,
+					     const struct in6_addr *grp,
+					     u8 *igmp_type)
+{
+	struct sk_buff *skb;
+	struct ipv6hdr *ip6h;
+	struct mld2_query *mld2q;
+	struct ethhdr *eth;
+	u8 *hopopt;
+	unsigned long interval;
+
+	skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*ip6h) +
+						 8 + sizeof(*mld2q));
+	if (!skb)
+		goto out;
+
+	skb->protocol = htons(ETH_P_IPV6);
+
+	/* Ethernet header */
+	skb_reset_mac_header(skb);
+	eth = eth_hdr(skb);
+
+	ether_addr_copy(eth->h_source, br->dev->dev_addr);
+	eth->h_proto = htons(ETH_P_IPV6);
+	skb_put(skb, sizeof(*eth));
+
+	/* IPv6 header + HbH option */
+	skb_set_network_header(skb, skb->len);
+	ip6h = ipv6_hdr(skb);
+
+	*(__force __be32 *)ip6h = htonl(0x60000000);
+	ip6h->payload_len = htons(8 + sizeof(*mld2q));
+	ip6h->nexthdr = IPPROTO_HOPOPTS;
+	ip6h->hop_limit = 1;
+	ipv6_addr_set(&ip6h->daddr, htonl(0xff020000), 0, 0, htonl(1));
+	if (ipv6_dev_get_saddr(dev_net(br->dev), br->dev, &ip6h->daddr, 0,
+			       &ip6h->saddr)) {
+		kfree_skb(skb);
+		br->has_ipv6_addr = 0;
+		return NULL;
+	}
+
+	br->has_ipv6_addr = 1;
+	ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
+
+	hopopt = (u8 *)(ip6h + 1);
+	hopopt[0] = IPPROTO_ICMPV6;		/* next hdr */
+	hopopt[1] = 0;				/* length of HbH */
+	hopopt[2] = IPV6_TLV_ROUTERALERT;	/* Router Alert */
+	hopopt[3] = 2;				/* Length of RA Option */
+	hopopt[4] = 0;				/* Type = 0x0000 (MLD) */
+	hopopt[5] = 0;
+	hopopt[6] = IPV6_TLV_PAD1;		/* Pad1 */
+	hopopt[7] = IPV6_TLV_PAD1;		/* Pad1 */
+
+	skb_put(skb, sizeof(*ip6h) + 8);
+
+	/* ICMPv6 */
+	skb_set_transport_header(skb, skb->len);
+	mld2q = (struct mld2_query *) icmp6_hdr(skb);
+
+	interval = ipv6_addr_any(grp) ?
+			br->multicast_query_response_interval :
+			br->multicast_last_member_interval;
+
+	*igmp_type = ICMPV6_MGM_QUERY;
+	mld2q->mld2q_type = ICMPV6_MGM_QUERY;
+	mld2q->mld2q_code = 0;
+	mld2q->mld2q_cksum = 0;
+	mld2q->mld2q_mrc = htons((u16)jiffies_to_msecs(interval));
+	mld2q->mld2q_resv1 = 0;
+	mld2q->mld2q_mca = *grp;
+	mld2q->mld2q_resv2 = 0;
+	mld2q->mld2q_suppress = 0;
+	mld2q->mld2q_qrv = 2;
+	mld2q->mld2q_qqic = br->multicast_query_interval / HZ;
+	mld2q->mld2q_nsrcs = 0;
+
+	/* checksum */
+	mld2q->mld2q_cksum = csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
+					  sizeof(*mld2q), IPPROTO_ICMPV6,
+					  csum_partial(mld2q,
+						       sizeof(*mld2q), 0));
+	skb_put(skb, sizeof(*mld2q));
+
+	__skb_pull(skb, sizeof(*eth));
+
+out:
+	return skb;
+}
 #endif
 
+static int mld_force_mld_version(const struct inet6_dev *idev)
+{
+	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
+		return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
+	else
+		return idev->cnf.force_mld_version;
+}
+
 static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br,
 						struct br_ip *addr,
 						u8 *igmp_type)
 {
+	struct in_device *in_dev = __in_dev_get_rcu(br->dev);
+	struct inet6_dev *idev = __in6_dev_get(br->dev);
 	switch (addr->proto) {
 	case htons(ETH_P_IP):
-		return br_ip4_multicast_alloc_query(br, addr->u.ip4, igmp_type);
+		if (IGMP_V3_SEEN(in_dev))
+			return br_ip4_alloc_query_v3(br, addr->u.ip4,
+						     igmp_type);
+		else
+			return br_ip4_alloc_query_v2(br, addr->u.ip4,
+						     igmp_type);
 #if IS_ENABLED(CONFIG_IPV6)
 	case htons(ETH_P_IPV6):
-		return br_ip6_multicast_alloc_query(br, &addr->u.ip6,
-						    igmp_type);
+		if (mld_force_mld_version(idev) == 2)
+			return br_ip6_alloc_query_v2(br, &addr->u.ip6,
+						     igmp_type);
+		else
+			return br_ip6_alloc_query_v1(br, &addr->u.ip6,
+						     igmp_type);
 #endif
 	}
 	return NULL;
-- 
2.5.5

Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support

[Nikolay Aleksandrov]

On 18/11/16 08:32, Hangbin Liu wrote:
> Add bridge IGMPv3 and MLDv2 query support. But before we think it is stable
> enough, only enable it when declare in force_igmp/mld_version.
>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  net/bridge/br_multicast.c | 203 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 194 insertions(+), 9 deletions(-)
>

(+CC Roopa)
Hi Hangbin,
This patch is not correct, you should not use the port net device IGMP config in the bridge.
These packets may never make it to the host and they may not be reflected, even more the
host may have very different igmp config for the port net device than the bridge does.
Also your current implementation switches to V3 only if it is globally set, and that should
be controlled on a per-bridge basis, even per-vlan later.
One more thing, if someone does a V2 leave for a group, you can end up sending them V3
group-specific query.

I have been working on an implementation for IGMPv3/MLDv2 querier for the bridge device, it re-uses
most of the current code and allows you to configure it per-bridge (and later per-vlan). The only
reason I've not yet sent it to upstream is that we (at Cumulus) are working out the compatibility
parts (e.g. RFC3376 sec 7, sec 8) of the RFC since it has some unclear cases.
But I can rip the compatibility out and send it early for review if you'd like, we can add the
compat code later.

Cheers,
  Nik

> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 2136e45..9fb47f3 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -35,6 +35,10 @@
>
>  #include "br_private.h"
>
> +#define IGMP_V3_SEEN(in_dev) \
> +	(IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) == 3 || \
> +	 IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) == 3)
> +
>  static void br_multicast_start_querier(struct net_bridge *br,
>  				       struct bridge_mcast_own_query *query);
>  static void br_multicast_add_router(struct net_bridge *br,
> @@ -360,9 +364,8 @@ static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max,
>  	return 0;
>  }
>
> -static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
> -						    __be32 group,
> -						    u8 *igmp_type)
> +static struct sk_buff *br_ip4_alloc_query_v2(struct net_bridge *br,
> +					     __be32 group, u8 *igmp_type)
>  {
>  	struct sk_buff *skb;
>  	struct igmphdr *ih;
> @@ -428,10 +431,82 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
>  	return skb;
>  }
>
> +static struct sk_buff *br_ip4_alloc_query_v3(struct net_bridge *br,
> +					     __be32 group, u8 *igmp_type)
> +{
> +	struct sk_buff *skb;
> +	struct igmpv3_query *ih3;
> +	struct ethhdr *eth;
> +	struct iphdr *iph;
> +
> +	skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*iph) +
> +						 sizeof(*ih3) + 4);
> +	if (!skb)
> +		goto out;
> +
> +	skb->protocol = htons(ETH_P_IP);
> +
> +	skb_reset_mac_header(skb);
> +	eth = eth_hdr(skb);
> +
> +	ether_addr_copy(eth->h_source, br->dev->dev_addr);
> +	eth->h_dest[0] = 1;
> +	eth->h_dest[1] = 0;
> +	eth->h_dest[2] = 0x5e;
> +	eth->h_dest[3] = 0;
> +	eth->h_dest[4] = 0;
> +	eth->h_dest[5] = 1;
> +	eth->h_proto = htons(ETH_P_IP);
> +	skb_put(skb, sizeof(*eth));
> +
> +	skb_set_network_header(skb, skb->len);
> +	iph = ip_hdr(skb);
> +
> +	iph->version = 4;
> +	iph->ihl = 6;
> +	iph->tos = 0xc0;
> +	iph->tot_len = htons(sizeof(*iph) + sizeof(*ih3) + 4);
> +	iph->id = 0;
> +	iph->frag_off = htons(IP_DF);
> +	iph->ttl = 1;
> +	iph->protocol = IPPROTO_IGMP;
> +	iph->saddr = br->multicast_query_use_ifaddr ?
> +		     inet_select_addr(br->dev, 0, RT_SCOPE_LINK) : 0;
> +	iph->daddr = htonl(INADDR_ALLHOSTS_GROUP);
> +	((u8 *)&iph[1])[0] = IPOPT_RA;
> +	((u8 *)&iph[1])[1] = 4;
> +	((u8 *)&iph[1])[2] = 0;
> +	((u8 *)&iph[1])[3] = 0;
> +	ip_send_check(iph);
> +	skb_put(skb, 24);
> +
> +	skb_set_transport_header(skb, skb->len);
> +	ih3 = igmpv3_query_hdr(skb);
> +
> +	*igmp_type = IGMP_HOST_MEMBERSHIP_QUERY;
> +	ih3->type = IGMP_HOST_MEMBERSHIP_QUERY;
> +	ih3->code = (group ? br->multicast_last_member_interval :
> +			    br->multicast_query_response_interval) /
> +		   (HZ / IGMP_TIMER_SCALE);
> +	ih3->csum = 0;
> +	ih3->group = group;
> +	ih3->resv = 0;
> +	ih3->suppress = 0;
> +	ih3->qrv= 2;
> +	ih3->qqic = br->multicast_query_interval / HZ;
> +	ih3->nsrcs = 0;
> +	ih3->csum = ip_compute_csum((void *)ih3, sizeof(struct igmpv3_query ));
> +	skb_put(skb, sizeof(*ih3));
> +
> +	__skb_pull(skb, sizeof(*eth));
> +
> +out:
> +	return skb;
> +}
>  #if IS_ENABLED(CONFIG_IPV6)
> -static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
> -						    const struct in6_addr *grp,
> -						    u8 *igmp_type)
> +static struct sk_buff *br_ip6_alloc_query_v1(struct net_bridge *br,
> +					     const struct in6_addr *grp,
> +					     u8 *igmp_type)
>  {
>  	struct sk_buff *skb;
>  	struct ipv6hdr *ip6h;
> @@ -514,19 +589,129 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
>  out:
>  	return skb;
>  }
> +
> +static struct sk_buff *br_ip6_alloc_query_v2(struct net_bridge *br,
> +					     const struct in6_addr *grp,
> +					     u8 *igmp_type)
> +{
> +	struct sk_buff *skb;
> +	struct ipv6hdr *ip6h;
> +	struct mld2_query *mld2q;
> +	struct ethhdr *eth;
> +	u8 *hopopt;
> +	unsigned long interval;
> +
> +	skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*ip6h) +
> +						 8 + sizeof(*mld2q));
> +	if (!skb)
> +		goto out;
> +
> +	skb->protocol = htons(ETH_P_IPV6);
> +
> +	/* Ethernet header */
> +	skb_reset_mac_header(skb);
> +	eth = eth_hdr(skb);
> +
> +	ether_addr_copy(eth->h_source, br->dev->dev_addr);
> +	eth->h_proto = htons(ETH_P_IPV6);
> +	skb_put(skb, sizeof(*eth));
> +
> +	/* IPv6 header + HbH option */
> +	skb_set_network_header(skb, skb->len);
> +	ip6h = ipv6_hdr(skb);
> +
> +	*(__force __be32 *)ip6h = htonl(0x60000000);
> +	ip6h->payload_len = htons(8 + sizeof(*mld2q));
> +	ip6h->nexthdr = IPPROTO_HOPOPTS;
> +	ip6h->hop_limit = 1;
> +	ipv6_addr_set(&ip6h->daddr, htonl(0xff020000), 0, 0, htonl(1));
> +	if (ipv6_dev_get_saddr(dev_net(br->dev), br->dev, &ip6h->daddr, 0,
> +			       &ip6h->saddr)) {
> +		kfree_skb(skb);
> +		br->has_ipv6_addr = 0;
> +		return NULL;
> +	}
> +
> +	br->has_ipv6_addr = 1;
> +	ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
> +
> +	hopopt = (u8 *)(ip6h + 1);
> +	hopopt[0] = IPPROTO_ICMPV6;		/* next hdr */
> +	hopopt[1] = 0;				/* length of HbH */
> +	hopopt[2] = IPV6_TLV_ROUTERALERT;	/* Router Alert */
> +	hopopt[3] = 2;				/* Length of RA Option */
> +	hopopt[4] = 0;				/* Type = 0x0000 (MLD) */
> +	hopopt[5] = 0;
> +	hopopt[6] = IPV6_TLV_PAD1;		/* Pad1 */
> +	hopopt[7] = IPV6_TLV_PAD1;		/* Pad1 */
> +
> +	skb_put(skb, sizeof(*ip6h) + 8);
> +
> +	/* ICMPv6 */
> +	skb_set_transport_header(skb, skb->len);
> +	mld2q = (struct mld2_query *) icmp6_hdr(skb);
> +
> +	interval = ipv6_addr_any(grp) ?
> +			br->multicast_query_response_interval :
> +			br->multicast_last_member_interval;
> +
> +	*igmp_type = ICMPV6_MGM_QUERY;
> +	mld2q->mld2q_type = ICMPV6_MGM_QUERY;
> +	mld2q->mld2q_code = 0;
> +	mld2q->mld2q_cksum = 0;
> +	mld2q->mld2q_mrc = htons((u16)jiffies_to_msecs(interval));
> +	mld2q->mld2q_resv1 = 0;
> +	mld2q->mld2q_mca = *grp;
> +	mld2q->mld2q_resv2 = 0;
> +	mld2q->mld2q_suppress = 0;
> +	mld2q->mld2q_qrv = 2;
> +	mld2q->mld2q_qqic = br->multicast_query_interval / HZ;
> +	mld2q->mld2q_nsrcs = 0;
> +
> +	/* checksum */
> +	mld2q->mld2q_cksum = csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
> +					  sizeof(*mld2q), IPPROTO_ICMPV6,
> +					  csum_partial(mld2q,
> +						       sizeof(*mld2q), 0));
> +	skb_put(skb, sizeof(*mld2q));
> +
> +	__skb_pull(skb, sizeof(*eth));
> +
> +out:
> +	return skb;
> +}
>  #endif
>
> +static int mld_force_mld_version(const struct inet6_dev *idev)
> +{
> +	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
> +		return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
> +	else
> +		return idev->cnf.force_mld_version;
> +}
> +
>  static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br,
>  						struct br_ip *addr,
>  						u8 *igmp_type)
>  {
> +	struct in_device *in_dev = __in_dev_get_rcu(br->dev);
> +	struct inet6_dev *idev = __in6_dev_get(br->dev);
>  	switch (addr->proto) {
>  	case htons(ETH_P_IP):
> -		return br_ip4_multicast_alloc_query(br, addr->u.ip4, igmp_type);
> +		if (IGMP_V3_SEEN(in_dev))
> +			return br_ip4_alloc_query_v3(br, addr->u.ip4,
> +						     igmp_type);
> +		else
> +			return br_ip4_alloc_query_v2(br, addr->u.ip4,
> +						     igmp_type);
>  #if IS_ENABLED(CONFIG_IPV6)
>  	case htons(ETH_P_IPV6):
> -		return br_ip6_multicast_alloc_query(br, &addr->u.ip6,
> -						    igmp_type);
> +		if (mld_force_mld_version(idev) == 2)
> +			return br_ip6_alloc_query_v2(br, &addr->u.ip6,
> +						     igmp_type);
> +		else
> +			return br_ip6_alloc_query_v1(br, &addr->u.ip6,
> +						     igmp_type);
>  #endif
>  	}
>  	return NULL;
>

Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support

[Nikolay Aleksandrov]

On 18/11/16 11:04, Nikolay Aleksandrov wrote:
> On 18/11/16 08:32, Hangbin Liu wrote:
>> Add bridge IGMPv3 and MLDv2 query support. But before we think it is stable
>> enough, only enable it when declare in force_igmp/mld_version.
>>
>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>> ---
>>  net/bridge/br_multicast.c | 203 ++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 194 insertions(+), 9 deletions(-)
>>
>
> (+CC Roopa)
> Hi Hangbin,
> This patch is not correct, you should not use the net device IGMP config in the bridge.
* bridge net device, sorry not port

> These packets may never make it to the host and they may not be reflected, even more the
> host may have very different igmp config for the port net device than the bridge does.
* again bridge net device, not port

> Also your current implementation switches to V3 only if it is globally set, and that should
> be controlled on a per-bridge basis, even per-vlan later.
* it is per-bridge, you use the global net device IGMP config, but it should be in the bridge
mcast control options, so we can do it per-vlan later and also be able to do the compat parts
of the RFC and in general, the bridge is configured via its own options not the host net device
because as I said these packets may never be received locally

> One more thing, if someone does a V2 leave for a group, you can end up sending them V3
> group-specific query.
>
> I have been working on an implementation for IGMPv3/MLDv2 querier for the bridge device, it re-uses
> most of the current code and allows you to configure it per-bridge (and later per-vlan). The only
> reason I've not yet sent it to upstream is that we (at Cumulus) are working out the compatibility
> parts (e.g. RFC3376 sec 7, sec 8) of the RFC since it has some unclear cases.
> But I can rip the compatibility out and send it early for review if you'd like, we can add the
> compat code later.
>
> Cheers,
>  Nik
>
>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>> index 2136e45..9fb47f3 100644
>> --- a/net/bridge/br_multicast.c
>> +++ b/net/bridge/br_multicast.c
>> @@ -35,6 +35,10 @@
>>
>>  #include "br_private.h"
>>
>> +#define IGMP_V3_SEEN(in_dev) \
>> +    (IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) == 3 || \
>> +     IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) == 3)
>> +
>>  static void br_multicast_start_querier(struct net_bridge *br,
>>                         struct bridge_mcast_own_query *query);
>>  static void br_multicast_add_router(struct net_bridge *br,
>> @@ -360,9 +364,8 @@ static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max,
>>      return 0;
>>  }
>>
>> -static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
>> -                            __be32 group,
>> -                            u8 *igmp_type)
>> +static struct sk_buff *br_ip4_alloc_query_v2(struct net_bridge *br,
>> +                         __be32 group, u8 *igmp_type)
>>  {
>>      struct sk_buff *skb;
>>      struct igmphdr *ih;
>> @@ -428,10 +431,82 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
>>      return skb;
>>  }
>>
>> +static struct sk_buff *br_ip4_alloc_query_v3(struct net_bridge *br,
>> +                         __be32 group, u8 *igmp_type)
>> +{
>> +    struct sk_buff *skb;
>> +    struct igmpv3_query *ih3;
>> +    struct ethhdr *eth;
>> +    struct iphdr *iph;
>> +
>> +    skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*iph) +
>> +                         sizeof(*ih3) + 4);
>> +    if (!skb)
>> +        goto out;
>> +
>> +    skb->protocol = htons(ETH_P_IP);
>> +
>> +    skb_reset_mac_header(skb);
>> +    eth = eth_hdr(skb);
>> +
>> +    ether_addr_copy(eth->h_source, br->dev->dev_addr);
>> +    eth->h_dest[0] = 1;
>> +    eth->h_dest[1] = 0;
>> +    eth->h_dest[2] = 0x5e;
>> +    eth->h_dest[3] = 0;
>> +    eth->h_dest[4] = 0;
>> +    eth->h_dest[5] = 1;
>> +    eth->h_proto = htons(ETH_P_IP);
>> +    skb_put(skb, sizeof(*eth));
>> +
>> +    skb_set_network_header(skb, skb->len);
>> +    iph = ip_hdr(skb);
>> +
>> +    iph->version = 4;
>> +    iph->ihl = 6;
>> +    iph->tos = 0xc0;
>> +    iph->tot_len = htons(sizeof(*iph) + sizeof(*ih3) + 4);
>> +    iph->id = 0;
>> +    iph->frag_off = htons(IP_DF);
>> +    iph->ttl = 1;
>> +    iph->protocol = IPPROTO_IGMP;
>> +    iph->saddr = br->multicast_query_use_ifaddr ?
>> +             inet_select_addr(br->dev, 0, RT_SCOPE_LINK) : 0;
>> +    iph->daddr = htonl(INADDR_ALLHOSTS_GROUP);
>> +    ((u8 *)&iph[1])[0] = IPOPT_RA;
>> +    ((u8 *)&iph[1])[1] = 4;
>> +    ((u8 *)&iph[1])[2] = 0;
>> +    ((u8 *)&iph[1])[3] = 0;
>> +    ip_send_check(iph);
>> +    skb_put(skb, 24);
>> +
>> +    skb_set_transport_header(skb, skb->len);
>> +    ih3 = igmpv3_query_hdr(skb);
>> +
>> +    *igmp_type = IGMP_HOST_MEMBERSHIP_QUERY;
>> +    ih3->type = IGMP_HOST_MEMBERSHIP_QUERY;
>> +    ih3->code = (group ? br->multicast_last_member_interval :
>> +                br->multicast_query_response_interval) /
>> +           (HZ / IGMP_TIMER_SCALE);
>> +    ih3->csum = 0;
>> +    ih3->group = group;
>> +    ih3->resv = 0;
>> +    ih3->suppress = 0;
>> +    ih3->qrv= 2;
>> +    ih3->qqic = br->multicast_query_interval / HZ;
>> +    ih3->nsrcs = 0;
>> +    ih3->csum = ip_compute_csum((void *)ih3, sizeof(struct igmpv3_query ));
>> +    skb_put(skb, sizeof(*ih3));
>> +
>> +    __skb_pull(skb, sizeof(*eth));
>> +
>> +out:
>> +    return skb;
>> +}
>>  #if IS_ENABLED(CONFIG_IPV6)
>> -static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
>> -                            const struct in6_addr *grp,
>> -                            u8 *igmp_type)
>> +static struct sk_buff *br_ip6_alloc_query_v1(struct net_bridge *br,
>> +                         const struct in6_addr *grp,
>> +                         u8 *igmp_type)
>>  {
>>      struct sk_buff *skb;
>>      struct ipv6hdr *ip6h;
>> @@ -514,19 +589,129 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
>>  out:
>>      return skb;
>>  }
>> +
>> +static struct sk_buff *br_ip6_alloc_query_v2(struct net_bridge *br,
>> +                         const struct in6_addr *grp,
>> +                         u8 *igmp_type)
>> +{
>> +    struct sk_buff *skb;
>> +    struct ipv6hdr *ip6h;
>> +    struct mld2_query *mld2q;
>> +    struct ethhdr *eth;
>> +    u8 *hopopt;
>> +    unsigned long interval;
>> +
>> +    skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*ip6h) +
>> +                         8 + sizeof(*mld2q));
>> +    if (!skb)
>> +        goto out;
>> +
>> +    skb->protocol = htons(ETH_P_IPV6);
>> +
>> +    /* Ethernet header */
>> +    skb_reset_mac_header(skb);
>> +    eth = eth_hdr(skb);
>> +
>> +    ether_addr_copy(eth->h_source, br->dev->dev_addr);
>> +    eth->h_proto = htons(ETH_P_IPV6);
>> +    skb_put(skb, sizeof(*eth));
>> +
>> +    /* IPv6 header + HbH option */
>> +    skb_set_network_header(skb, skb->len);
>> +    ip6h = ipv6_hdr(skb);
>> +
>> +    *(__force __be32 *)ip6h = htonl(0x60000000);
>> +    ip6h->payload_len = htons(8 + sizeof(*mld2q));
>> +    ip6h->nexthdr = IPPROTO_HOPOPTS;
>> +    ip6h->hop_limit = 1;
>> +    ipv6_addr_set(&ip6h->daddr, htonl(0xff020000), 0, 0, htonl(1));
>> +    if (ipv6_dev_get_saddr(dev_net(br->dev), br->dev, &ip6h->daddr, 0,
>> +                   &ip6h->saddr)) {
>> +        kfree_skb(skb);
>> +        br->has_ipv6_addr = 0;
>> +        return NULL;
>> +    }
>> +
>> +    br->has_ipv6_addr = 1;
>> +    ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
>> +
>> +    hopopt = (u8 *)(ip6h + 1);
>> +    hopopt[0] = IPPROTO_ICMPV6;        /* next hdr */
>> +    hopopt[1] = 0;                /* length of HbH */
>> +    hopopt[2] = IPV6_TLV_ROUTERALERT;    /* Router Alert */
>> +    hopopt[3] = 2;                /* Length of RA Option */
>> +    hopopt[4] = 0;                /* Type = 0x0000 (MLD) */
>> +    hopopt[5] = 0;
>> +    hopopt[6] = IPV6_TLV_PAD1;        /* Pad1 */
>> +    hopopt[7] = IPV6_TLV_PAD1;        /* Pad1 */
>> +
>> +    skb_put(skb, sizeof(*ip6h) + 8);
>> +
>> +    /* ICMPv6 */
>> +    skb_set_transport_header(skb, skb->len);
>> +    mld2q = (struct mld2_query *) icmp6_hdr(skb);
>> +
>> +    interval = ipv6_addr_any(grp) ?
>> +            br->multicast_query_response_interval :
>> +            br->multicast_last_member_interval;
>> +
>> +    *igmp_type = ICMPV6_MGM_QUERY;
>> +    mld2q->mld2q_type = ICMPV6_MGM_QUERY;
>> +    mld2q->mld2q_code = 0;
>> +    mld2q->mld2q_cksum = 0;
>> +    mld2q->mld2q_mrc = htons((u16)jiffies_to_msecs(interval));
>> +    mld2q->mld2q_resv1 = 0;
>> +    mld2q->mld2q_mca = *grp;
>> +    mld2q->mld2q_resv2 = 0;
>> +    mld2q->mld2q_suppress = 0;
>> +    mld2q->mld2q_qrv = 2;
>> +    mld2q->mld2q_qqic = br->multicast_query_interval / HZ;
>> +    mld2q->mld2q_nsrcs = 0;
>> +
>> +    /* checksum */
>> +    mld2q->mld2q_cksum = csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
>> +                      sizeof(*mld2q), IPPROTO_ICMPV6,
>> +                      csum_partial(mld2q,
>> +                               sizeof(*mld2q), 0));
>> +    skb_put(skb, sizeof(*mld2q));
>> +
>> +    __skb_pull(skb, sizeof(*eth));
>> +
>> +out:
>> +    return skb;
>> +}
>>  #endif
>>
>> +static int mld_force_mld_version(const struct inet6_dev *idev)
>> +{
>> +    if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
>> +        return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
>> +    else
>> +        return idev->cnf.force_mld_version;
>> +}
>> +
>>  static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br,
>>                          struct br_ip *addr,
>>                          u8 *igmp_type)
>>  {
>> +    struct in_device *in_dev = __in_dev_get_rcu(br->dev);
>> +    struct inet6_dev *idev = __in6_dev_get(br->dev);
>>      switch (addr->proto) {
>>      case htons(ETH_P_IP):
>> -        return br_ip4_multicast_alloc_query(br, addr->u.ip4, igmp_type);
>> +        if (IGMP_V3_SEEN(in_dev))
>> +            return br_ip4_alloc_query_v3(br, addr->u.ip4,
>> +                             igmp_type);
>> +        else
>> +            return br_ip4_alloc_query_v2(br, addr->u.ip4,
>> +                             igmp_type);
>>  #if IS_ENABLED(CONFIG_IPV6)
>>      case htons(ETH_P_IPV6):
>> -        return br_ip6_multicast_alloc_query(br, &addr->u.ip6,
>> -                            igmp_type);
>> +        if (mld_force_mld_version(idev) == 2)
>> +            return br_ip6_alloc_query_v2(br, &addr->u.ip6,
>> +                             igmp_type);
>> +        else
>> +            return br_ip6_alloc_query_v1(br, &addr->u.ip6,
>> +                             igmp_type);
>>  #endif
>>      }
>>      return NULL;
>>
>

Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support

[kbuild test robot]

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

Hi Hangbin,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Hangbin-Liu/bridge-add-igmpv3-and-mldv2-query-support/20161118-155854
config: i386-randconfig-r0-201646 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   net/bridge/br_multicast.c: In function 'mld_force_mld_version':
>> net/bridge/br_multicast.c:688:24: error: 'struct net' has no member named 'ipv6'
     if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
                           ^
   net/bridge/br_multicast.c:689:28: error: 'struct net' has no member named 'ipv6'
      return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
                               ^
   net/bridge/br_multicast.c: In function 'br_multicast_alloc_query':
>> net/bridge/br_multicast.c:699:27: error: implicit declaration of function '__in6_dev_get' [-Werror=implicit-function-declaration]
     struct inet6_dev *idev = __in6_dev_get(br->dev);
                              ^
>> net/bridge/br_multicast.c:699:27: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
>> net/bridge/br_multicast.c:699:20: warning: unused variable 'idev' [-Wunused-variable]
     struct inet6_dev *idev = __in6_dev_get(br->dev);
                       ^
   net/bridge/br_multicast.c: At top level:
   net/bridge/br_multicast.c:686:12: warning: 'mld_force_mld_version' defined but not used [-Wunused-function]
    static int mld_force_mld_version(const struct inet6_dev *idev)
               ^
   cc1: some warnings being treated as errors

vim +688 net/bridge/br_multicast.c

   682		return skb;
   683	}
   684	#endif
   685	
   686	static int mld_force_mld_version(const struct inet6_dev *idev)
   687	{
 > 688		if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
 > 689			return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
   690		else
   691			return idev->cnf.force_mld_version;
   692	}
   693	
   694	static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br,
   695							struct br_ip *addr,
   696							u8 *igmp_type)
   697	{
   698		struct in_device *in_dev = __in_dev_get_rcu(br->dev);
 > 699		struct inet6_dev *idev = __in6_dev_get(br->dev);
   700		switch (addr->proto) {
   701		case htons(ETH_P_IP):
   702			if (IGMP_V3_SEEN(in_dev))

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28193 bytes --]

Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support

[Nikolay Aleksandrov]

On 18/11/16 11:09, Nikolay Aleksandrov wrote:
> On 18/11/16 11:04, Nikolay Aleksandrov wrote:
>> (+CC Roopa)
>> Hi Hangbin,
>> This patch is not correct, you should not use the net device IGMP config in the bridge.
> * bridge net device, sorry not port
>
>> These packets may never make it to the host and they may not be reflected, even more the
>> host may have very different igmp config for the port net device than the bridge does.
> * again bridge net device, not port
>
>> Also your current implementation switches to V3 only if it is globally set, and that should
>> be controlled on a per-bridge basis, even per-vlan later.
> * it is per-bridge, you use the global net device IGMP config, but it should be in the bridge
> mcast control options, so we can do it per-vlan later and also be able to do the compat parts
> of the RFC and in general, the bridge is configured via its own options not the host net device
> because as I said these packets may never be received locally

Having the host netdev options affect the bridge-specific config is really undesirable and confusing.

>
>> One more thing, if someone does a V2 leave for a group, you can end up sending them V3
>> group-specific query.
>>
>> I have been working on an implementation for IGMPv3/MLDv2 querier for the bridge device, it re-uses
>> most of the current code and allows you to configure it per-bridge (and later per-vlan). The only
>> reason I've not yet sent it to upstream is that we (at Cumulus) are working out the compatibility
>> parts (e.g. RFC3376 sec 7, sec 8) of the RFC since it has some unclear cases.

Err, RFC3376 sec 7.3.1, 7.3.2, and RFC3810 sec 8.3.1, 8.3.2

I really should get coffee.. sorry for the multiple emails :-)

>> But I can rip the compatibility out and send it early for review if you'd like, we can add the
>> compat code later.
>>
>> Cheers,
>>  Nik
>>

Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support

[kbuild test robot]

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

Hi Hangbin,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Hangbin-Liu/bridge-add-igmpv3-and-mldv2-query-support/20161118-155854
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   net/bridge/br_multicast.c: In function 'mld_force_mld_version':
>> net/bridge/br_multicast.c:688:24: error: 'struct net' has no member named 'ipv6'; did you mean 'ipv4'?
     if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
                           ^~
   net/bridge/br_multicast.c:689:28: error: 'struct net' has no member named 'ipv6'; did you mean 'ipv4'?
      return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
                               ^~
   net/bridge/br_multicast.c: In function 'br_multicast_alloc_query':
   net/bridge/br_multicast.c:699:27: error: implicit declaration of function '__in6_dev_get' [-Werror=implicit-function-declaration]
     struct inet6_dev *idev = __in6_dev_get(br->dev);
                              ^~~~~~~~~~~~~
   net/bridge/br_multicast.c:699:27: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
   net/bridge/br_multicast.c:699:20: warning: unused variable 'idev' [-Wunused-variable]
     struct inet6_dev *idev = __in6_dev_get(br->dev);
                       ^~~~
   At top level:
   net/bridge/br_multicast.c:686:12: warning: 'mld_force_mld_version' defined but not used [-Wunused-function]
    static int mld_force_mld_version(const struct inet6_dev *idev)
               ^~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +688 net/bridge/br_multicast.c

   682		return skb;
   683	}
   684	#endif
   685	
   686	static int mld_force_mld_version(const struct inet6_dev *idev)
   687	{
 > 688		if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
   689			return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
   690		else
   691			return idev->cnf.force_mld_version;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22647 bytes --]

Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support

[Hangbin Liu]

2016-11-18 18:31 GMT+08:00 Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
> On 18/11/16 11:09, Nikolay Aleksandrov wrote:
>>
>> On 18/11/16 11:04, Nikolay Aleksandrov wrote:
>>>
>>> (+CC Roopa)
>>> Hi Hangbin,
>>> This patch is not correct, you should not use the net device IGMP config
>>> in the bridge.
>>
>> * bridge net device, sorry not port

Hi Nikolay,

Thanks for the comments. You are right. I was just thinking to make the IGMP/MLD
version configurable and use existing variables directly. But we
should control it
per-bridge basis.

>>
>>> These packets may never make it to the host and they may not be
>>> reflected, even more the
>>> host may have very different igmp config for the port net device than the
>>> bridge does.
>>
>> * again bridge net device, not port
>>
>>> Also your current implementation switches to V3 only if it is globally
>>> set, and that should
>>> be controlled on a per-bridge basis, even per-vlan later.
>>
>> * it is per-bridge, you use the global net device IGMP config, but it
>> should be in the bridge
>> mcast control options, so we can do it per-vlan later and also be able to
>> do the compat parts
>> of the RFC and in general, the bridge is configured via its own options
>> not the host net device
>> because as I said these packets may never be received locally
>
>
> Having the host netdev options affect the bridge-specific config is really
> undesirable and confusing.
>
>>
>>> One more thing, if someone does a V2 leave for a group, you can end up
>>> sending them V3
>>> group-specific query.
>>>
>>> I have been working on an implementation for IGMPv3/MLDv2 querier for the
>>> bridge device, it re-uses
>>> most of the current code and allows you to configure it per-bridge (and
>>> later per-vlan). The only
>>> reason I've not yet sent it to upstream is that we (at Cumulus) are
>>> working out the compatibility
>>> parts (e.g. RFC3376 sec 7, sec 8) of the RFC since it has some unclear
>>> cases.

Yeah, even in the host igmp/mld code we have some vague areas and different
handles.

>
>
> Err, RFC3376 sec 7.3.1, 7.3.2, and RFC3810 sec 8.3.1, 8.3.2
>
> I really should get coffee.. sorry for the multiple emails :-)
>
>
>>> But I can rip the compatibility out and send it early for review if you'd
>>> like, we can add the
>>> compat code later.

OK, then I will wait for your patch.

Thanks
Hangbin