Re: [PATCH net-next v2] bridge: multicast to unicast

[kbuild test robot]

Hi Felix,

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

url:
https://github.com/0day-ci/linux/commits/Linus-L-ssing/bridge-multicast-to-unicast/20170118-120345
config: x86_64-rhel-7.2 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   net/bridge/br_forward.c: In function 'br_multicast_flood':
>> net/bridge/br_forward.c:261:27: warning: 'port' may be used uninitialized in 
>> this function [-Wmaybe-uninitialized]
  struct net_bridge_port *port, *lport, *rport;
  ^~~~

vim +/port +261 net/bridge/br_forward.c

5cb5e947 Herbert Xu  2010-02-27  245  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
5cb5e947 Herbert Xu  2010-02-27  246  /* called with rcu_read_lock */
37b090e6 Nikolay Aleksandrov 2016-07-14  247  void br_multicast_flood(struct 
net_bridge_mdb_entry *mdst,
b35c5f63 Nikolay Aleksandrov 2016-07-14  248struct sk_buff 
*skb,
37b090e6 Nikolay Aleksandrov 2016-07-14  249bool local_rcv, 
bool local_orig)
5cb5e947 Herbert Xu  2010-02-27  250  {
5cb5e947 Herbert Xu  2010-02-27  251struct net_device *dev = 
BR_INPUT_SKB_CB(skb)->brdev;
1080ab95 Nikolay Aleksandrov 2016-06-28  252u8 igmp_type = 
br_multicast_igmp_type(skb);
5cb5e947 Herbert Xu  2010-02-27  253struct net_bridge *br = 
netdev_priv(dev);
afe0159d stephen hemminger   2010-04-27  254struct net_bridge_port *prev = 
NULL;
5cb5e947 Herbert Xu  2010-02-27  255struct net_bridge_port_group *p;
5cb5e947 Herbert Xu  2010-02-27  256struct hlist_node *rp;
5cb5e947 Herbert Xu  2010-02-27  257  
e8051688 Eric Dumazet2010-11-15  258rp = 
rcu_dereference(hlist_first_rcu(>router_list));
83f6a740 stephen hemminger   2010-04-27  259p = mdst ? 
rcu_dereference(mdst->ports) : NULL;
5cb5e947 Herbert Xu  2010-02-27  260while (p || rp) {
afe0159d stephen hemminger   2010-04-27 @261struct net_bridge_port 
*port, *lport, *rport;
afe0159d stephen hemminger   2010-04-27  262  
5cb5e947 Herbert Xu  2010-02-27  263lport = p ? p->port : 
NULL;
5cb5e947 Herbert Xu  2010-02-27  264rport = rp ? 
hlist_entry(rp, struct net_bridge_port, rlist) :
5cb5e947 Herbert Xu  2010-02-27  265 NULL;
5cb5e947 Herbert Xu  2010-02-27  266  
507962cd Felix Fietkau   2017-01-17  267if ((unsigned 
long)lport > (unsigned long)rport) {
507962cd Felix Fietkau   2017-01-17  268if (p->flags & 
MDB_PG_FLAGS_MCAST_TO_UCAST) {
507962cd Felix Fietkau   2017-01-17  269
maybe_deliver_addr(lport, skb, p->eth_addr,

:: The code at line 261 was first introduced by commit
:: afe0159d935ab731c682e811356914bb2be9470c bridge: multicast_flood cleanup

:: TO: stephen hemminger 
:: CC: David S. Miller 

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


.config.gz
Description: application/gzip

[PATCH net-next v2] bridge: multicast to unicast

[Linus Lüssing]

From: Felix Fietkau 

Implements an optional, per bridge port flag and feature to deliver
multicast packets to any host on the according port via unicast
individually. This is done by copying the packet per host and
changing the multicast destination MAC to a unicast one accordingly.

multicast-to-unicast works on top of the multicast snooping feature of
the bridge. Which means unicast copies are only delivered to hosts which
are interested in it and signalized this via IGMP/MLD reports
previously.

This feature is intended for interface types which have a more reliable
and/or efficient way to deliver unicast packets than broadcast ones
(e.g. wifi).

However, it should only be enabled on interfaces where no IGMPv2/MLDv1
report suppression takes place. This feature is disabled by default.

The initial patch and idea is from Felix Fietkau.

Signed-off-by: Felix Fietkau 
[linus.luess...@c0d3.blue: various bug + style fixes, commit message]
Signed-off-by: Linus Lüssing 

---

This feature is used and enabled by default in OpenWRT and LEDE for AP
interfaces for more than a year now to allow both a more robust multicast
delivery and multicast at higher rates (e.g. multicast streaming).

In OpenWRT/LEDE the IGMP/MLD report suppression issue is overcome by
the network daemon enabling AP isolation and by that separating all STAs.
Delivery of STA-to-STA IP mulitcast is made possible again by
enabling and utilizing the bridge hairpin mode, which considers the
incoming port as a potential outgoing port, too.

Hairpin-mode is performed after multicast snooping, therefore leading to
only deliver reports to STAs running a multicast router.

Changes in v2:
* netlink support (thanks Nik!)
* fixed switching between multicast_to_unicast on/off
  -> even after toggling an already existing entry would
 stale in its mode and would never be replaced
  -> new extra check in br_port_group_equal)
* reduced checks in br_multicast_flood() from two to one
  to address fast-path concerns (thanks Nik!)
* rev-christmas tree ordering (thanks Nik!)
* removed "net_bridge_port_group::unicast", using
  ::flags instead (thanks Nik!)
* BR_MULTICAST_TO_UCAST -> BR_MULTICAST_TO_UNICAST
  (BR_MULTICAST_FLAST_LEAVE has the same length anyway)
* simplified maybe_deliver_addr()
  (no return, no "prev" paramater -> was a NOP anyway)
* added "From: Felix Fietkau [...]"
* added "Signed-off-by: Felix Fietkau [...]"
---
 include/linux/if_bridge.h|  1 +
 include/uapi/linux/if_link.h |  1 +
 net/bridge/br_forward.c  | 37 -
 net/bridge/br_mdb.c  |  2 +-
 net/bridge/br_multicast.c| 96 
 net/bridge/br_netlink.c  |  5 +++
 net/bridge/br_private.h  |  8 ++--
 net/bridge/br_sysfs_if.c |  2 +
 8 files changed, 121 insertions(+), 31 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index c6587c0..debc9d5 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -46,6 +46,7 @@ struct br_ip_list {
 #define BR_LEARNING_SYNC   BIT(9)
 #define BR_PROXYARP_WIFI   BIT(10)
 #define BR_MCAST_FLOOD BIT(11)
+#define BR_MULTICAST_TO_UNICASTBIT(12)
 
 #define BR_DEFAULT_AGEING_TIME (300 * HZ)
 
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 6b13e59..4e59565 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -321,6 +321,7 @@ enum {
IFLA_BRPORT_MULTICAST_ROUTER,
IFLA_BRPORT_PAD,
IFLA_BRPORT_MCAST_FLOOD,
+   IFLA_BRPORT_MCAST_TO_UCAST,
__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 7cb41ae..75d041e 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -174,6 +174,29 @@ static struct net_bridge_port *maybe_deliver(
return p;
 }
 
+static void maybe_deliver_addr(struct net_bridge_port *p, struct sk_buff *skb,
+  const unsigned char *addr, bool local_orig)
+{
+   struct net_device *dev = BR_INPUT_SKB_CB(skb)->brdev;
+   const unsigned char *src = eth_hdr(skb)->h_source;
+
+   if (!should_deliver(p, skb))
+   return;
+
+   /* Even with hairpin, no soliloquies - prevent breaking IPv6 DAD */
+   if (skb->dev == p->dev && ether_addr_equal(src, addr))
+   return;
+
+   skb = skb_copy(skb, GFP_ATOMIC);
+   if (!skb) {
+   dev->stats.tx_dropped++;
+   return;
+   }
+
+   memcpy(eth_hdr(skb)->h_dest, addr, ETH_ALEN);
+   __br_forward(p, skb, local_orig);
+}
+
 /* called under rcu_read_lock */
 void br_flood(struct net_bridge *br, struct sk_buff *skb,
  enum br_pkt_type pkt_type, bool local_rcv, bool local_orig)
@@ -241,10 +264,20 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
rport = rp ? hlist_entry(rp,