netdev - Re: [PATCH 1/3] net: bridge: Implement bridge flag local

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yi9tgOQ32q2l2TxD@shredder>
Date:   Mon, 14 Mar 2022 18:29:52 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     mattias.forsblad+netdev@...il.com
Cc:     Nikolay Aleksandrov <razor@...ckwall.org>, netdev@...r.kernel.org,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Roopa Prabhu <roopa@...dia.com>
Subject: Re: [PATCH 1/3] net: bridge: Implement bridge flag local_receive

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@...sch.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".