Re: [PATCH 1/3] net: bridge: Implement bridge flag local_receive

[Ido Schimmel <idosch@idosch.org>]

On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote:
> This patch implements the bridge flag local_receive. When this
> flag is cleared packets received on bridge ports will not be forwarded up.
> This makes is possible to only forward traffic between the port members
> of the bridge.
> 
> Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com>
> ---
>  include/linux/if_bridge.h      |  6 ++++++
>  include/net/switchdev.h        |  2 ++

Nik might ask you to split the offload part from the bridge
implementation. Please wait for his feedback as he might be AFK right
now

>  include/uapi/linux/if_bridge.h |  1 +
>  include/uapi/linux/if_link.h   |  1 +
>  net/bridge/br.c                | 18 ++++++++++++++++++
>  net/bridge/br_device.c         |  1 +
>  net/bridge/br_input.c          |  3 +++
>  net/bridge/br_ioctl.c          |  1 +
>  net/bridge/br_netlink.c        | 14 +++++++++++++-
>  net/bridge/br_private.h        |  2 ++
>  net/bridge/br_sysfs_br.c       | 23 +++++++++++++++++++++++

I believe the bridge doesn't implement sysfs for new attributes

>  net/bridge/br_vlan.c           |  8 ++++++++
>  12 files changed, 79 insertions(+), 1 deletion(-)

[...]

> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index e0c13fcc50ed..5864b61157d3 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  		break;
>  	}
>  
> +	if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE))
> +		local_rcv = false;
> +

I don't think the description in the commit message is accurate:
"packets received on bridge ports will not be forwarded up". From the
code it seems that if packets hit a local FDB entry, then they will be
"forwarded up". Instead, it seems that packets will not be flooded
towards the bridge. In which case, why not maintain the same granularity
we have for the rest of the ports and split this into unicast /
multicast / broadcast?

BTW, while the patch honors local FDB entries, it overrides host MDB
entries which seems wrong / inconsistent.

>  	if (dst) {
>  		unsigned long now = jiffies;