Re: [PATCH v4 net-next] net: bridge: mcast: add support for raw L2 multicast groups

2020-10-30 Thread Jakub Kicinski
On Thu, 29 Oct 2020 01:38:31 +0200 Vladimir Oltean wrote:
> From: Nikolay Aleksandrov 
> 
> Extend the bridge multicast control and data path to configure routes
> for L2 (non-IP) multicast groups.
> 
> The uapi struct br_mdb_entry union u is extended with another variant,
> mac_addr, which does not change the structure size, and which is valid
> when the proto field is zero.
> 
> To be compatible with the forwarding code that is already in place,
> which acts as an IGMP/MLD snooping bridge with querier capabilities, we
> need to declare that for L2 MDB entries (for which there exists no such
> thing as IGMP/MLD snooping/querying), that there is always a querier.
> Otherwise, these entries would be flooded to all bridge ports and not
> just to those that are members of the L2 multicast group.
> 
> Needless to say, only permanent L2 multicast groups can be installed on
> a bridge port.
> 
> Signed-off-by: Nikolay Aleksandrov 
> Signed-off-by: Vladimir Oltean 

Applied, thanks!


[PATCH v4 net-next] net: bridge: mcast: add support for raw L2 multicast groups

2020-10-28 Thread Vladimir Oltean
From: Nikolay Aleksandrov 

Extend the bridge multicast control and data path to configure routes
for L2 (non-IP) multicast groups.

The uapi struct br_mdb_entry union u is extended with another variant,
mac_addr, which does not change the structure size, and which is valid
when the proto field is zero.

To be compatible with the forwarding code that is already in place,
which acts as an IGMP/MLD snooping bridge with querier capabilities, we
need to declare that for L2 MDB entries (for which there exists no such
thing as IGMP/MLD snooping/querying), that there is always a querier.
Otherwise, these entries would be flooded to all bridge ports and not
just to those that are members of the L2 multicast group.

Needless to say, only permanent L2 multicast groups can be installed on
a bridge port.

Signed-off-by: Nikolay Aleksandrov 
Signed-off-by: Vladimir Oltean 
---
Changes in v4:
- Check for MDB group being permanent before calling br_multicast_new_group,
  to avoid having empty groups that can't be deleted due to errors.

Changes in v3:
- Removed some noise in the diff.

Changes in v2:
- Removed redundant MDB_FLAGS_L2 (we are simply signalling an L2 entry
  through proto == 0)
- Moved mac_addr inside union dst of struct br_ip.
- Validation that L2 multicast address is indeed multicast

 include/linux/if_bridge.h  |  1 +
 include/uapi/linux/if_bridge.h |  1 +
 net/bridge/br_device.c |  2 +-
 net/bridge/br_input.c  |  2 +-
 net/bridge/br_mdb.c| 24 ++--
 net/bridge/br_multicast.c  | 13 +
 net/bridge/br_private.h| 10 --
 7 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 556caed00258..b979005ea39c 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -25,6 +25,7 @@ struct br_ip {
 #if IS_ENABLED(CONFIG_IPV6)
struct in6_addr ip6;
 #endif
+   unsigned char   mac_addr[ETH_ALEN];
} dst;
__be16  proto;
__u16   vid;
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 4c687686aa8f..281777477616 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -526,6 +526,7 @@ struct br_mdb_entry {
union {
__be32  ip4;
struct in6_addr ip6;
+   unsigned char mac_addr[ETH_ALEN];
} u;
__be16  proto;
} addr;
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 6f742fee874a..06c28753b911 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -93,7 +93,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct 
net_device *dev)
 
mdst = br_mdb_get(br, skb, vid);
if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
-   br_multicast_querier_exists(br, eth_hdr(skb)))
+   br_multicast_querier_exists(br, eth_hdr(skb), mdst))
br_multicast_flood(mdst, skb, false, true);
else
br_flood(br, skb, BR_PKT_MULTICAST, false, true);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 59a318b9f646..d31b5c18c6a1 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -134,7 +134,7 @@ int br_handle_frame_finish(struct net *net, struct sock 
*sk, struct sk_buff *skb
case BR_PKT_MULTICAST:
mdst = br_mdb_get(br, skb, vid);
if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
-   br_multicast_querier_exists(br, eth_hdr(skb))) {
+   br_multicast_querier_exists(br, eth_hdr(skb), mdst)) {
if ((mdst && mdst->host_joined) ||
br_multicast_is_router(br)) {
local_rcv = true;
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index e15bab19a012..3c8863418d0b 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -87,6 +87,8 @@ static void __mdb_entry_to_br_ip(struct br_mdb_entry *entry, 
struct br_ip *ip,
ip->src.ip6 = 
nla_get_in6_addr(mdb_attrs[MDBE_ATTR_SOURCE]);
break;
 #endif
+   default:
+   ether_addr_copy(ip->dst.mac_addr, entry->addr.u.mac_addr);
}
 
 }
@@ -174,9 +176,11 @@ static int __mdb_fill_info(struct sk_buff *skb,
if (mp->addr.proto == htons(ETH_P_IP))
e.addr.u.ip4 = mp->addr.dst.ip4;
 #if IS_ENABLED(CONFIG_IPV6)
-   if (mp->addr.proto == htons(ETH_P_IPV6))
+   else if (mp->addr.proto == htons(ETH_P_IPV6))
e.addr.u.ip6 = mp->addr.dst.ip6;
 #endif
+   else
+   ether_addr_copy(e.addr.u.mac_addr, mp->addr.dst.mac_addr);
e.addr.proto = mp->addr.proto;
nest_ent = nla_nest_start_noflag(skb,