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: <Yh5NL1SY7+3rLW5O@shredder>
Date:   Tue, 1 Mar 2022 18:43:27 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Mattias Forsblad <mattias.forsblad@...il.com>
Cc:     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>,
        Nikolay Aleksandrov <razor@...ckwall.org>,
        Mattias Forsblad <mattias.forsblad+netdev@...il.com>
Subject: Re: [PATCH 1/3] net: bridge: Implement bridge flag local_receive

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@...il.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;