[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8e0a0ceb-5816-60a6-6219-7306e75ce006@nvidia.com>
Date: Sat, 30 Jul 2022 18:54:52 -0700
From: Roopa Prabhu <roopa@...dia.com>
To: Sevinj Aghayeva <sevinj.aghayeva@...il.com>,
Nikolay Aleksandrov <razor@...ckwall.org>
Cc: aroulin@...dia.com, sbrivio@...hat.com,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, bridge@...ts.linux-foundation.org
Subject: Re: [PATCH net-next 0/3] net: vlan: fix bridge binding behavior and
add selftests
On 7/30/22 09:48, Sevinj Aghayeva wrote:
> (Resending this because the first email was rejected due to being in HTML.)
>
>
> On Sat, Jul 30, 2022 at 12:46 PM Sevinj Aghayeva
> <sevinj.aghayeva@...il.com> wrote:
>>
>>
>> On Sat, Jul 30, 2022 at 12:22 PM Nikolay Aleksandrov <razor@...ckwall.org> wrote:
>>> On 7/30/22 19:03, Sevinj Aghayeva wrote:
>>>> When bridge binding is enabled for a vlan interface, it is expected
>>>> that the link state of the vlan interface will track the subset of the
>>>> ports that are also members of the corresponding vlan, rather than
>>>> that of all ports.
>>>>
>>>> Currently, this feature works as expected when a vlan interface is
>>>> created with bridge binding enabled:
>>>>
>>>> ip link add link br name vlan10 type vlan id 10 protocol 802.1q \
>>>> bridge_binding on
>>>>
>>>> However, the feature does not work when a vlan interface is created
>>>> with bridge binding disabled, and then enabled later:
>>>>
>>>> ip link add link br name vlan10 type vlan id 10 protocol 802.1q \
>>>> bridge_binding off
>>>> ip link set vlan10 type vlan bridge_binding on
>>>>
>>>> After these two commands, the link state of the vlan interface
>>>> continues to track that of all ports, which is inconsistent and
>>>> confusing to users. This series fixes this bug and introduces two
>>>> tests for the valid behavior.
>>>>
>>>> Sevinj Aghayeva (3):
>>>> net: bridge: export br_vlan_upper_change
>>>> net: 8021q: fix bridge binding behavior for vlan interfaces
>>>> selftests: net: tests for bridge binding behavior
>>>>
>>>> include/linux/if_bridge.h | 9 ++
>>>> net/8021q/vlan.h | 2 +-
>>>> net/8021q/vlan_dev.c | 21 ++-
>>>> net/bridge/br_vlan.c | 7 +-
>>>> tools/testing/selftests/net/Makefile | 1 +
>>>> .../selftests/net/bridge_vlan_binding_test.sh | 143 ++++++++++++++++++
>>>> 6 files changed, 176 insertions(+), 7 deletions(-)
>>>> create mode 100755 tools/testing/selftests/net/bridge_vlan_binding_test.sh
>>>>
>>> Hmm.. I don't like this and don't think this bridge function should be
>>> exported at all.
>>>
>>> Calling bridge state changing functions from 8021q module is not the
>>> proper way to solve this. The problem is that the bridge doesn't know
>>> that the state has changed, so you can process NETDEV_CHANGE events and
>>> check for the bridge vlan which got its state changed and react based on
>>> it. I haven't checked in detail, but I think it should be doable. So all
>>> the logic is kept inside the bridge.
>>
>> Hi Nik,
>>
>> Can please elaborate on where I should process NETDEV_CHANGE events? I'm doing this as part of outreachy project and this is my first kernel task, so I don't know the bridging code that well.
>>
>> Thanks!
good point Nikolay.
Sevinj, see br_vlan_bridge_event and __vlan_device_eventĀ for how both
drivers react to netdev change events.
I have not looked at it in detail yet, but lets explore and discuss if
we can make use of events to achieve same results.
Powered by blists - more mailing lists