Sevinj Aghayeva
2022-Aug-10 15:00 UTC
[Bridge] [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests
On Wed, Aug 10, 2022 at 10:50 AM Nikolay Aleksandrov <razor at blackwall.org> wrote:> > On 10/08/2022 17:42, Sevinj Aghayeva wrote: > > > > > > On Wed, Aug 10, 2022 at 4:54 AM Nikolay Aleksandrov <razor at blackwall.org <mailto:razor at 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: pre.sh Type: application/x-sh Size: 1128 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/bridge/attachments/20220810/f2e6d784/attachment-0001.sh>
Nikolay Aleksandrov
2022-Aug-10 15:10 UTC
[Bridge] [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests
On 10/08/2022 18:00, Sevinj Aghayeva wrote:> On Wed, Aug 10, 2022 at 10:50 AM Nikolay Aleksandrov > <razor at blackwall.org> wrote: >> >> On 10/08/2022 17:42, Sevinj Aghayeva wrote: >>> >>> >>> On Wed, Aug 10, 2022 at 4:54 AM Nikolay Aleksandrov <razor at blackwall.org <mailto:razor at 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 > >You can't expect to see br_vlan_bridge_event() called because the notification target device is vlan10 and not the bridge. See br_device_event(): ... if (netif_is_bridge_master(dev)) { err = br_vlan_bridge_event(dev, event, ptr); if (err) return notifier_from_errno(err); ... Try probing for br_device_event(), you'll see it gets called every time you change the flag.