[PATCH 1/1 net-next] net: bridge: add support for host l2 mdb entries

[PATCH 1/1 net-next] net: bridge: add support for host l2 mdb entries

[Joachim Wiberg]

This patch expands on the earlier work on layer-2 mdb entries by adding
support for host entries.  Due to the fact that host joined entries do
not have any flag field, we infer the permanent flag when reporting the
entries to userspace, which otherwise would be listed as 'temp'.

Before patch:

    ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee permanent
    Error: bridge: Flags are not allowed for host groups.
    ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee
    Error: bridge: Only permanent L2 entries allowed.

After patch:

    ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee permanent
    ~# bridge mdb show
    dev br0 port br0 grp 01:00:00:c0:ff:ee permanent vid 1

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
 net/bridge/br_mdb.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 4556d913955b..9ba5c5cc2f3d 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -257,8 +257,10 @@ static int __mdb_fill_info(struct sk_buff *skb,
 	else if (mp->addr.proto == htons(ETH_P_IPV6))
 		e.addr.u.ip6 = mp->addr.dst.ip6;
 #endif
-	else
+	else {
 		ether_addr_copy(e.addr.u.mac_addr, mp->addr.dst.mac_addr);
+		e.state = MDB_PG_FLAGS_PERMANENT;
+	}
 	e.addr.proto = mp->addr.proto;
 	nest_ent = nla_nest_start_noflag(skb,
 					 MDBA_MDB_ENTRY_INFO);
@@ -873,8 +875,8 @@ static int br_mdb_add_group(struct net_bridge *br, struct net_bridge_port *port,
 		return -EINVAL;
 
 	/* host join errors which can happen before creating the group */
-	if (!port) {
-		/* don't allow any flags for host-joined groups */
+	if (!port && !br_group_is_l2(&group)) {
+		/* don't allow any flags for host-joined IP groups */
 		if (entry->state) {
 			NL_SET_ERR_MSG_MOD(extack, "Flags are not allowed for host groups");
 			return -EINVAL;
-- 
2.25.1

Re: [PATCH 1/1 net-next] net: bridge: add support for host l2 mdb entries

[Nikolay Aleksandrov]

On 23/02/2022 19:24, Joachim Wiberg wrote:
> This patch expands on the earlier work on layer-2 mdb entries by adding
> support for host entries.  Due to the fact that host joined entries do
> not have any flag field, we infer the permanent flag when reporting the
> entries to userspace, which otherwise would be listed as 'temp'.
> 
> Before patch:
> 
>     ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee permanent
>     Error: bridge: Flags are not allowed for host groups.
>     ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee
>     Error: bridge: Only permanent L2 entries allowed.
> 
> After patch:
> 
>     ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee permanent
>     ~# bridge mdb show
>     dev br0 port br0 grp 01:00:00:c0:ff:ee permanent vid 1
> 
> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
> ---
>  net/bridge/br_mdb.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 

It would be nice to add a selftest for L2 entries. You can send it as a follow-up.
The patch looks good to me.

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH 1/1 net-next] net: bridge: add support for host l2 mdb entries

[Joachim Wiberg]

On Thu, Feb 24, 2022 at 13:26, Nikolay Aleksandrov <razor@blackwall.org> wrote:
> On 23/02/2022 19:24, Joachim Wiberg wrote:
>> This patch expands on the earlier work on layer-2 mdb entries by adding
>> support for host entries.
> It would be nice to add a selftest for L2 entries. You can send it as a follow-up.

OK, will do!  It's on my immediate backlog.

Thanks
 /Joachim
 

Re: [PATCH 1/1 net-next] net: bridge: add support for host l2 mdb entries

[Jakub Kicinski]

On Thu, 24 Feb 2022 13:26:22 +0200 Nikolay Aleksandrov wrote:
> On 23/02/2022 19:24, Joachim Wiberg wrote:
> > This patch expands on the earlier work on layer-2 mdb entries by adding
> > support for host entries.  Due to the fact that host joined entries do
> > not have any flag field, we infer the permanent flag when reporting the
> > entries to userspace, which otherwise would be listed as 'temp'.
> > 
> > Before patch:
> > 
> >     ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee permanent
> >     Error: bridge: Flags are not allowed for host groups.
> >     ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee
> >     Error: bridge: Only permanent L2 entries allowed.
> > 
> > After patch:
> > 
> >     ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee permanent
> >     ~# bridge mdb show
> >     dev br0 port br0 grp 01:00:00:c0:ff:ee permanent vid 1
> > 
> > Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
>
> It would be nice to add a selftest for L2 entries. You can send it as a follow-up.

Let's wait for that, also checkpatch says you need to balance brackets
to hold kernel coding style.

> Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH 1/1 net-next] net: bridge: add support for host l2 mdb entries

[Joachim Wiberg]

On Thu, Feb 24, 2022 at 08:06, Jakub Kicinski <kuba@kernel.org> wrote:
> On Thu, 24 Feb 2022 13:26:22 +0200 Nikolay Aleksandrov wrote:
>> On 23/02/2022 19:24, Joachim Wiberg wrote:
>> > This patch expands on the earlier work on layer-2 mdb entries by adding
>> > support for host entries.
>> It would be nice to add a selftest for L2 entries. You can send it as a follow-up.
> Let's wait for that, also checkpatch says you need to balance brackets
> to hold kernel coding style.

Jakub, by "wait for that" do you mean you'd prefer I add the selftests
to this?  Otherwise I'll send a v2 with the style fixes.

Best regards
 /Joachim
 

Re: [PATCH 1/1 net-next] net: bridge: add support for host l2 mdb entries

[Jakub Kicinski]

On Thu, 24 Feb 2022 17:29:35 +0100 Joachim Wiberg wrote:
> On Thu, Feb 24, 2022 at 08:06, Jakub Kicinski <kuba@kernel.org> wrote:
> > On Thu, 24 Feb 2022 13:26:22 +0200 Nikolay Aleksandrov wrote:  
> >> On 23/02/2022 19:24, Joachim Wiberg wrote:  
>  [...]  
> >> It would be nice to add a selftest for L2 entries. You can send it as a follow-up.  
> > Let's wait for that, also checkpatch says you need to balance brackets
> > to hold kernel coding style.  
> 
> Jakub, by "wait for that" do you mean you'd prefer I add the selftests
> to this?

Yes, add a selftest as a separate patch but in the same series.

Re: [PATCH 1/1 net-next] net: bridge: add support for host l2 mdb entries

[Joachim Wiberg]

On Thu, Feb 24, 2022 at 08:48, Jakub Kicinski <kuba@kernel.org> wrote:
> On Thu, 24 Feb 2022 17:29:35 +0100 Joachim Wiberg wrote:
>> On Thu, Feb 24, 2022 at 08:06, Jakub Kicinski <kuba@kernel.org> wrote:
>> > On Thu, 24 Feb 2022 13:26:22 +0200 Nikolay Aleksandrov wrote:  
>> >> On 23/02/2022 19:24, Joachim Wiberg wrote:  
>>  [...]  
>> >> It would be nice to add a selftest for L2 entries. You can send it as a follow-up.  
>> > Let's wait for that, also checkpatch says you need to balance brackets
>> > to hold kernel coding style.  
>> Jakub, by "wait for that" do you mean you'd prefer I add the selftests
>> to this?
> Yes, add a selftest as a separate patch but in the same series.

OK, will do, thanks! :)

Regards
 /Joachim
 

[PATCH v2 net-next 0/2] net: bridge: add support for host l2 mdb entries

[Joachim Wiberg]

Fix to an obvious omissions for layer-2 host mdb entries, this v2 adds
the missing selftest and some minor style fixes.

Note: this patch revealed some worrying problems in how the bridge
      forwards unknown BUM traffic and also how unknown multicast is
      forwarded when a IP multicast router is known, which a another
      (RFC) patch series intend to address.  That series will build
      on this selftest, hence the name of the test.

 /Joachim

v2:
  - Add braces to other if/else clauses (Jakub)
  - Add selftest to verify add/del of mac/ipv4/ipv6 mdb entries (Jakub)

Joachim Wiberg (2):
  net: bridge: add support for host l2 mdb entries
  selftests: forwarding: new test, verify host mdb entries

 net/bridge/br_mdb.c                           |  12 +-
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../selftests/net/forwarding/bridge_mdb.sh    | 103 ++++++++++++++++++
 3 files changed, 111 insertions(+), 5 deletions(-)
 create mode 100755 tools/testing/selftests/net/forwarding/bridge_mdb.sh

-- 
2.25.1

[PATCH v2 net-next 1/2] net: bridge: add support for host l2 mdb entries

[Joachim Wiberg]

This patch expands on the earlier work on layer-2 mdb entries by adding
support for host entries.  Due to the fact that host joined entries do
not have any flag field, we infer the permanent flag when reporting the
entries to userspace, which otherwise would be listed as 'temp'.

Before patch:

    ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee permanent
    Error: bridge: Flags are not allowed for host groups.
    ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee
    Error: bridge: Only permanent L2 entries allowed.

After patch:

    ~# bridge mdb add dev br0 port br0 grp 01:00:00:c0:ff:ee permanent
    ~# bridge mdb show
    dev br0 port br0 grp 01:00:00:c0:ff:ee permanent vid 1

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_mdb.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 4556d913955b..fdcc641fc89a 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -251,14 +251,16 @@ static int __mdb_fill_info(struct sk_buff *skb,
 	__mdb_entry_fill_flags(&e, flags);
 	e.ifindex = ifindex;
 	e.vid = mp->addr.vid;
-	if (mp->addr.proto == htons(ETH_P_IP))
+	if (mp->addr.proto == htons(ETH_P_IP)) {
 		e.addr.u.ip4 = mp->addr.dst.ip4;
 #if IS_ENABLED(CONFIG_IPV6)
-	else 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
+	} else {
 		ether_addr_copy(e.addr.u.mac_addr, mp->addr.dst.mac_addr);
+		e.state = MDB_PG_FLAGS_PERMANENT;
+	}
 	e.addr.proto = mp->addr.proto;
 	nest_ent = nla_nest_start_noflag(skb,
 					 MDBA_MDB_ENTRY_INFO);
@@ -873,8 +875,8 @@ static int br_mdb_add_group(struct net_bridge *br, struct net_bridge_port *port,
 		return -EINVAL;
 
 	/* host join errors which can happen before creating the group */
-	if (!port) {
-		/* don't allow any flags for host-joined groups */
+	if (!port && !br_group_is_l2(&group)) {
+		/* don't allow any flags for host-joined IP groups */
 		if (entry->state) {
 			NL_SET_ERR_MSG_MOD(extack, "Flags are not allowed for host groups");
 			return -EINVAL;
-- 
2.25.1

[PATCH v2 net-next 2/2] selftests: forwarding: new test, verify host mdb entries

[Joachim Wiberg]

Boiler plate for testing static mdb entries.  This first test verifies
adding and removing host mdb entries for all supported types: IPv4,
IPv6, and MAC multicast.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../selftests/net/forwarding/bridge_mdb.sh    | 103 ++++++++++++++++++
 2 files changed, 104 insertions(+)
 create mode 100755 tools/testing/selftests/net/forwarding/bridge_mdb.sh

diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile
index 8fa97ae9af9e..ae80c2aef577 100644
--- a/tools/testing/selftests/net/forwarding/Makefile
+++ b/tools/testing/selftests/net/forwarding/Makefile
@@ -2,6 +2,7 @@
 
 TEST_PROGS = bridge_igmp.sh \
 	bridge_locked_port.sh \
+	bridge_mdb.sh \
 	bridge_port_isolation.sh \
 	bridge_sticky_fdb.sh \
 	bridge_vlan_aware.sh \
diff --git a/tools/testing/selftests/net/forwarding/bridge_mdb.sh b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
new file mode 100755
index 000000000000..b1ba6876dd86
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
@@ -0,0 +1,103 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Verify that adding host mdb entries work as intended for all types of
+# multicast filters: ipv4, ipv6, and mac
+
+ALL_TESTS="mdb_add_del_test"
+NUM_NETIFS=2
+
+TEST_GROUP_IP4="225.1.2.3"
+TEST_GROUP_IP6="ff02::42"
+TEST_GROUP_MAC="01:00:01:c0:ff:ee"
+
+source lib.sh
+
+h1_create()
+{
+	simple_if_init $h1 192.0.2.1/24 2001:db8:1::1/64
+}
+
+h1_destroy()
+{
+	simple_if_fini $h1 192.0.2.1/24 2001:db8:1::1/64
+}
+
+switch_create()
+{
+	# Enable multicast filtering
+	ip link add dev br0 type bridge mcast_snooping 1
+
+	ip link set dev $swp1 master br0
+
+	ip link set dev br0 up
+	ip link set dev $swp1 up
+}
+
+switch_destroy()
+{
+	ip link set dev $swp1 down
+	ip link del dev br0
+}
+
+setup_prepare()
+{
+	h1=${NETIFS[p1]}
+	swp1=${NETIFS[p2]}
+
+	vrf_prepare
+
+	h1_create
+	switch_create
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	switch_destroy
+	h1_destroy
+
+	vrf_cleanup
+}
+
+do_mdb_add_del()
+{
+	local group=$1
+	local flag=$2
+
+	RET=0
+	bridge mdb add dev br0 port br0 grp $group $flag 2>/dev/null
+	check_err $? "Failed adding $group to br0, port br0"
+
+	if [ -z "$flag" ]; then
+	    flag="temp"
+	fi
+
+	bridge mdb show dev br0 | grep $group | grep -q $flag 2>/dev/null
+	check_err $? "$group not added with $flag flag"
+
+	bridge mdb del dev br0 port br0 grp $group 2>/dev/null
+	check_err $? "Failed deleting $group from br0, port br0"
+
+	bridge mdb show dev br0 | grep -q $group >/dev/null
+	check_err_fail 1 $? "$group still in mdb after delete"
+
+	log_test "MDB add/del group $group to bridge port br0"
+}
+
+mdb_add_del_test()
+{
+	do_mdb_add_del $TEST_GROUP_MAC permanent
+	do_mdb_add_del $TEST_GROUP_IP4
+	do_mdb_add_del $TEST_GROUP_IP6
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
-- 
2.25.1

Re: [PATCH v2 net-next 0/2] net: bridge: add support for host l2 mdb entries

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 11 Apr 2022 10:40:52 +0200 you wrote:
> Fix to an obvious omissions for layer-2 host mdb entries, this v2 adds
> the missing selftest and some minor style fixes.
> 
> Note: this patch revealed some worrying problems in how the bridge
>       forwards unknown BUM traffic and also how unknown multicast is
>       forwarded when a IP multicast router is known, which a another
>       (RFC) patch series intend to address.  That series will build
>       on this selftest, hence the name of the test.
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/2] net: bridge: add support for host l2 mdb entries
    https://git.kernel.org/netdev/net-next/c/e65693b0179e
  - [v2,net-next,2/2] selftests: forwarding: new test, verify host mdb entries
    https://git.kernel.org/netdev/net-next/c/50fe062c806e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html