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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <2e9a48b9-66cf-3908-7236-4e81be0dcecb@nvidia.com>
Date:   Tue, 1 Mar 2022 19:25:45 -0800
From:   Roopa Prabhu <roopa@...dia.com>
To:     Ido Schimmel <idosch@...sch.org>,
        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>,
        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 3/1/22 08:43, Ido Schimmel wrote:
> 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.

+1