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

Roopa Prabhu
2022-03-02T03:25:52Z

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

+1