Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <49f933c3-7430-a133-9add-ed76c395023b@blackwall.org>
Date:   Wed, 10 Aug 2022 17:50:34 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior
 and add selftests
Content-Language: en-US
To:     Sevinj Aghayeva <sevinj.aghayeva@gmail.com>
Cc:     netdev@vger.kernel.org, aroulin@nvidia.com, sbrivio@redhat.com,
        roopa@nvidia.com, "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org,
        bridge@lists.linux-foundation.org
References: <cover.1660100506.git.sevinj.aghayeva@gmail.com>
 <94ec6182-0804-7a0e-dcba-42655ff19884@blackwall.org>
 <CAMWRUK45nbZS3PeSLR1X=Ko6oavrjKj2AWeh2F1wckMPrz_dEg@mail.gmail.com>
From:   Nikolay Aleksandrov <razor@blackwall.org>
In-Reply-To: <CAMWRUK45nbZS3PeSLR1X=Ko6oavrjKj2AWeh2F1wckMPrz_dEg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 10/08/2022 17:42, Sevinj Aghayeva wrote:
> 
> 
> On Wed, Aug 10, 2022 at 4:54 AM Nikolay Aleksandrov <razor@blackwall.org <mailto:razor@blackwall.org>> wrote:
> 
>     On 10/08/2022 06:11, 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: core: export call_netdevice_notifiers_info
>     >   net: 8021q: fix bridge binding behavior for vlan interfaces
>     >   selftests: net: tests for bridge binding behavior
>     >
>     >  include/linux/netdevice.h                     |   2 +
>     >  net/8021q/vlan.h                              |   2 +-
>     >  net/8021q/vlan_dev.c                          |  25 ++-
>     >  net/core/dev.c                                |   7 +-
>     >  tools/testing/selftests/net/Makefile          |   1 +
>     >  .../selftests/net/bridge_vlan_binding_test.sh | 143 ++++++++++++++++++
>     >  6 files changed, 172 insertions(+), 8 deletions(-)
>     >  create mode 100755 tools/testing/selftests/net/bridge_vlan_binding_test.sh
>     >
> 
>     Hi,
>     NETDEV_CHANGE event is already propagated when the vlan changes flags,
> 
> 
> I'm not sure if NETDEV_CHANGE is actually propagated when the vlan changes flags. The two functions in the bridge module that handle NETDEV_CHANGE are br_vlan_port_event  and br_vlan_bridge_event. I've installed probes for both, and when I'm changing flags using "sudo ip link set vlan10 type vlan bridge_binding on", I don't see any of those functions getting called, although I do see vlan_dev_change_flags getting called. I think there may be a bug in core/dev.c:__dev_notify_flags.

are both vlan and bridge interfaces up?
what exactly are you probing for?

I can see the NETDEV_CHANGE event go through when changing the loose binding.