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

[Ido Schimmel <idosch@idosch.org>]

On Wed, Mar 02, 2022 at 07:27:25AM +0100, Mattias Forsblad wrote:
> On 2022-03-01 23:36, Nikolay Aleksandrov wrote:
> > On 1 March 2022 17:43:27 CET, Ido Schimmel <idosch@idosch.org> wrote:
> >> On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote:
> >>> 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?
> >>
> > 
> > Exactly my first thought - why not implement the same control for the bridge?
> > Also try to minimize the fast-path hit, you can keep the needed changes 
> > localized only to the cases where they are needed.
> > I'll send a few more comments in a reply to the patch.
> > 
> 
> Soo, if I understand you correctly, you want to have three different options?
> local_receive_unicast
> local_receive_multicast
> local_receive_broadcast

My understanding of the feature is that you want to prevent flooding
towards the bridge. In which case, it makes sense to keep the same
granularity as for regular bridge ports and also name the options
similarly. We already have several options that are applicable to both
the bridge and bridge ports (e.g., 'mcast_router').

I suggest:

$ ip link help bridge
Usage: ... bridge [ fdb_flush ]
                  ...
                  [ flood {on | off} ]
                  [ mcast_flood {on | off} ]
                  [ bcast_flood {on | off} ]

This is consistent with "bridge_slave".