All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sevinj Aghayeva <sevinj.aghayeva@gmail.com>
To: Nikolay Aleksandrov <razor@blackwall.org>
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
Subject: Re: [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests
Date: Wed, 10 Aug 2022 11:00:24 -0400	[thread overview]
Message-ID: <CAMWRUK4J6Dp7Cff=pN9iw6OwDN8g61dd4S=OVKQ75vBch-PxXQ@mail.gmail.com> (raw)
In-Reply-To: <49f933c3-7430-a133-9add-ed76c395023b@blackwall.org>

[-- Attachment #1: Type: text/plain, Size: 4778 bytes --]

On Wed, Aug 10, 2022 at 10:50 AM Nikolay Aleksandrov
<razor@blackwall.org> wrote:
>
> 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 first run the attached pre.sh script that sets up the environment
and creates a vlan interface with bridge binding off. I then start
recording with perf, and here's the list of probes:

$ sudo ./k/linux/tools/perf/perf probe -l
  probe:br_vlan_bridge_event (on br_vlan_bridge_event in bridge with event dev)
  probe:br_vlan_port_event (on br_vlan_port_event in bridge with event)
  probe:br_vlan_set_vlan_dev_state (on br_vlan_set_vlan_dev_state in
bridge with br vlan_dev)
  probe:register_vlan_dev (on register_vlan_dev in 8021q with dev)
  probe:vlan_changelink (on vlan_changelink in 8021q with dev)
  probe:vlan_dev_change_flags (on vlan_dev_change_flags in 8021q with dev)
  probe:vlan_dev_fix_features (on vlan_dev_fix_features in 8021q with dev)
  probe:vlan_dev_init  (on vlan_dev_init in 8021q with dev)
  probe:vlan_dev_ioctl (on vlan_dev_ioctl in 8021q with dev)
  probe:vlan_dev_open  (on vlan_dev_open in 8021q with dev)
  probe:vlan_dev_stop  (on vlan_dev_stop in 8021q with dev)
  probe:vlan_dev_uninit (on vlan_dev_uninit in 8021q with dev)
  probe:vlan_newlink   (on vlan_newlink in 8021q with dev)

I then run the following command to turn the bridge binding flag on:

$ sudo ip link set vlan10 type vlan bridge_binding on

Then I stop the recording and print out the events, and I see this. I
don't see br_vlan_port_event or br_vlan_bridge_event getting called.

              ip  5933 [003]  2204.722470:
probe:vlan_changelink: (ffffffffc1042b50) dev="vlan10"
              ip  5933 [003]  2204.722476:
probe:vlan_dev_change_flags: (ffffffffc1042600) dev="vlan10"

Am I doing something wrong?

Thanks



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


-- 

Sevinj.Aghayeva

[-- Attachment #2: pre.sh --]
[-- Type: application/x-sh, Size: 1128 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Sevinj Aghayeva <sevinj.aghayeva@gmail.com>
To: Nikolay Aleksandrov <razor@blackwall.org>
Cc: aroulin@nvidia.com, netdev@vger.kernel.org,
	bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	sbrivio@redhat.com, Eric Dumazet <edumazet@google.com>,
	roopa@nvidia.com, Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [Bridge] [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests
Date: Wed, 10 Aug 2022 11:00:24 -0400	[thread overview]
Message-ID: <CAMWRUK4J6Dp7Cff=pN9iw6OwDN8g61dd4S=OVKQ75vBch-PxXQ@mail.gmail.com> (raw)
In-Reply-To: <49f933c3-7430-a133-9add-ed76c395023b@blackwall.org>

[-- Attachment #1: Type: text/plain, Size: 4778 bytes --]

On Wed, Aug 10, 2022 at 10:50 AM Nikolay Aleksandrov
<razor@blackwall.org> wrote:
>
> 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 first run the attached pre.sh script that sets up the environment
and creates a vlan interface with bridge binding off. I then start
recording with perf, and here's the list of probes:

$ sudo ./k/linux/tools/perf/perf probe -l
  probe:br_vlan_bridge_event (on br_vlan_bridge_event in bridge with event dev)
  probe:br_vlan_port_event (on br_vlan_port_event in bridge with event)
  probe:br_vlan_set_vlan_dev_state (on br_vlan_set_vlan_dev_state in
bridge with br vlan_dev)
  probe:register_vlan_dev (on register_vlan_dev in 8021q with dev)
  probe:vlan_changelink (on vlan_changelink in 8021q with dev)
  probe:vlan_dev_change_flags (on vlan_dev_change_flags in 8021q with dev)
  probe:vlan_dev_fix_features (on vlan_dev_fix_features in 8021q with dev)
  probe:vlan_dev_init  (on vlan_dev_init in 8021q with dev)
  probe:vlan_dev_ioctl (on vlan_dev_ioctl in 8021q with dev)
  probe:vlan_dev_open  (on vlan_dev_open in 8021q with dev)
  probe:vlan_dev_stop  (on vlan_dev_stop in 8021q with dev)
  probe:vlan_dev_uninit (on vlan_dev_uninit in 8021q with dev)
  probe:vlan_newlink   (on vlan_newlink in 8021q with dev)

I then run the following command to turn the bridge binding flag on:

$ sudo ip link set vlan10 type vlan bridge_binding on

Then I stop the recording and print out the events, and I see this. I
don't see br_vlan_port_event or br_vlan_bridge_event getting called.

              ip  5933 [003]  2204.722470:
probe:vlan_changelink: (ffffffffc1042b50) dev="vlan10"
              ip  5933 [003]  2204.722476:
probe:vlan_dev_change_flags: (ffffffffc1042600) dev="vlan10"

Am I doing something wrong?

Thanks



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


-- 

Sevinj.Aghayeva

[-- Attachment #2: pre.sh --]
[-- Type: application/x-sh, Size: 1128 bytes --]

  reply	other threads:[~2022-08-10 15:00 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-10  3:11 [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests Sevinj Aghayeva
2022-08-10  3:11 ` [Bridge] " Sevinj Aghayeva
2022-08-10  3:11 ` [PATCH RFC net-next 1/3] net: core: export call_netdevice_notifiers_info Sevinj Aghayeva
2022-08-10  3:11   ` [Bridge] " Sevinj Aghayeva
2022-08-10  3:11 ` [PATCH RFC net-next 2/3] net: 8021q: fix bridge binding behavior for vlan interfaces Sevinj Aghayeva
2022-08-10  3:11   ` [Bridge] " Sevinj Aghayeva
2022-08-10  3:11 ` [PATCH RFC net-next 3/3] selftests: net: tests for bridge binding behavior Sevinj Aghayeva
2022-08-10  3:11   ` [Bridge] " Sevinj Aghayeva
2022-08-10  8:54 ` [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests Nikolay Aleksandrov
2022-08-10  8:54   ` [Bridge] " Nikolay Aleksandrov
2022-08-10 14:42   ` Sevinj Aghayeva
2022-08-10 14:50     ` Nikolay Aleksandrov
2022-08-10 14:50       ` [Bridge] " Nikolay Aleksandrov
2022-08-10 15:00       ` Sevinj Aghayeva [this message]
2022-08-10 15:00         ` Sevinj Aghayeva
2022-08-10 15:10         ` Nikolay Aleksandrov
2022-08-10 15:10           ` [Bridge] " Nikolay Aleksandrov
2022-08-10 15:21           ` Sevinj Aghayeva
2022-08-10 15:21             ` [Bridge] " Sevinj Aghayeva
2022-08-12 15:30   ` Sevinj Aghayeva
2022-08-12 15:30     ` [Bridge] " Sevinj Aghayeva
2022-08-14  7:38     ` Nikolay Aleksandrov
2022-08-14  7:38       ` [Bridge] " Nikolay Aleksandrov
2022-08-18 11:50       ` Sevinj Aghayeva
2022-08-18 11:50         ` [Bridge] " Sevinj Aghayeva
2022-08-18 12:00         ` Nikolay Aleksandrov
2022-08-18 12:00           ` [Bridge] " Nikolay Aleksandrov
2022-08-20 11:33           ` Sevinj Aghayeva
2022-08-20 11:33             ` [Bridge] " Sevinj Aghayeva
2022-08-22  8:01             ` Nikolay Aleksandrov
2022-08-22  8:01               ` [Bridge] " Nikolay Aleksandrov
2022-08-22 23:18               ` Sevinj Aghayeva
2022-08-29 20:22                 ` Sevinj Aghayeva
2022-08-31 10:16                   ` Nikolay Aleksandrov
2022-08-31 10:16                     ` [Bridge] " Nikolay Aleksandrov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMWRUK4J6Dp7Cff=pN9iw6OwDN8g61dd4S=OVKQ75vBch-PxXQ@mail.gmail.com' \
    --to=sevinj.aghayeva@gmail.com \
    --cc=aroulin@nvidia.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=roopa@nvidia.com \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.