On 02.11.2017 19:25, Eric Dumazet wrote:
On Thu, 2017-11-02 at 17:47 +0300, Vadim Fedorenko wrote:
On Thu, 2017-11-02 at 07:33 -7000, Eric Dumazet wrote:
On Thu, 2017-11-02 at 15:49 +0300, Vadim Fedorenko wrote:
Some time ago Eric Dumazet suggested a "hack the IFF_XMIT_DST_RELEASE
flag on the vlan netdev". But the last comment was "does not support
properly bonding/team.(If the real_dev->privflags IFF_XMIT_DST_RELEASE
bit changes, we want to update all the vlans at the same time )"

I've extended that patch to support changes of IFF_XMIT_DST_RELEASE in
bonding/team.
Both bonding and team call netdev_change_features() after recalculation
of features including priv_flags IFF_XMIT_DST_RELEASE bit. So the only
thing needed to support is to recheck this bit in
vlan_transfer_features().

Suggested-by: Eric Dumazet <eric.duma...@gmail.com>
Signed-off-by: Vadim Fedorenko <vfedore...@yandex-team.ru>
---
  net/8021q/vlan.c         | 3 +++
  net/8021q/vlan_netlink.c | 1 +
  2 files changed, 4 insertions(+)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 9649579..510986c 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -328,6 +328,9 @@ static void vlan_transfer_features(struct net_device *dev,
        vlandev->fcoe_ddp_xid = dev->fcoe_ddp_xid;
  #endif
+ vlandev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
+       vlandev->priv_flags |= (vlan->real_dev->priv_flags & 
IFF_XMIT_DST_RELEASE);
+
        netdev_update_features(vlandev);
  }
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 5e831de..9472de8 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -143,6 +143,7 @@ static int vlan_newlink(struct net *src_net, struct 
net_device *dev,
        vlan->vlan_proto = proto;
        vlan->vlan_id         = nla_get_u16(data[IFLA_VLAN_ID]);
        vlan->real_dev        = real_dev;
+       dev->priv_flags |= (real_dev->priv_flags & IFF_XMIT_DST_RELEASE);
        vlan->flags   = VLAN_FLAG_REORDER_HDR;
err = vlan_check_real_dev(real_dev, vlan->vlan_proto, vlan->vlan_id);

What happens if a bonding is composed of two slaves, with different
IFF_XMIT_DST_RELEASE settings ?

According to bond_compute_features() IFF_XMIT_DST_RELEASE flag is cleared from 
priv_flags
in such case, so it is cleared in vlan_transfer_features() and vlan device 
works as before
the patch.
The same code exists in team netdev.

I repeat the question.

What happens if a bonding is composed of two slaves A & B,
A has IFF_XMIT_DST_RELEASE set
B has IFF_XMIT_DST_RELEASE cleared.

1) A is added , then B is added.
2) B is added to the bond, then A is added.
3) Before, and after your patches.

Thanks.


Do you mean what happens with vlan device with real_dev is bonding ?

With patches:
1) A is added
  bond_enslave()
    bond_compute_features()
            -> bond_dev IFF_XMIT_DST_RELEASE is not changed (set)
      netdev_change_features()
        vlan_device_event(event=NETDEV_FEAT_CHANGE)
          vlan_transfer_features()
            -> vlan_dev IFF_XMIT_DST_RELEASE is not changed (still set)
Then B is added
  bond_enslave()
    bond_compute_features()
            -> bond_dev IFF_XMIT_DST_RELEASE is changed (cleared)
      netdev_change_features()
        vlan_device_event(event=NETDEV_FEAT_CHANGE)
          vlan_transfer_features()
            -> vlan_dev IFF_XMIT_DST_RELEASE is changed (cleared)

2) B is added
  bond_enslave()
    bond_compute_features()
            -> bond_dev IFF_XMIT_DST_RELEASE is changed (cleared)
      netdev_change_features()
        vlan_device_event(event=NETDEV_FEAT_CHANGE)
          vlan_transfer_features()
            -> vlan_dev IFF_XMIT_DST_RELEASE is changed (cleared)
Then A is added
  bond_enslave()
    bond_compute_features()
            -> bond_dev IFF_XMIT_DST_RELEASE is not changed (cleared)
      netdev_change_features()
        vlan_device_event(event=NETDEV_FEAT_CHANGE)
          vlan_transfer_features()
            -> vlan_dev IFF_XMIT_DST_RELEASE is not changed (cleared).

Without patches:
1) A is added
  bond_enslave()
    bond_compute_features()
            -> bond_dev IFF_XMIT_DST_RELEASE is not changed (set)
      netdev_change_features()
        vlan_device_event(event=NETDEV_FEAT_CHANGE)
          vlan_transfer_features()
             -> vlan_dev IFF_XMIT_DST_RELEASE is not changed (cleared)

Then B is added
  bond_enslave()
    bond_compute_features()
             -> bond_dev IFF_XMIT_DST_RELEASE is changed (cleared)
      netdev_change_features()
        vlan_device_event(event=NETDEV_FEAT_CHANGE)
          vlan_transfer_features()
             -> vlan_dev IFF_XMIT_DST_RELEASE is not changed(cleared)
2) B is added
  bond_enslave()
    bond_compute_features()
             -> bond_dev IFF_XMIT_DST_RELEASE is changed (cleared)
      netdev_change_features()
        vlan_device_event(event=NETDEV_FEAT_CHANGE)
          vlan_transfer_features()
             -> vlan_dev IFF_XMIT_DST_RELEASE is not changed(cleared)
Then A is added
  bond_enslave()
    bond_compute_features()
             -> bond_dev IFF_XMIT_DST_RELEASE is not changed (cleared)
      netdev_change_features()
        vlan_device_event(event=NETDEV_FEAT_CHANGE)
          vlan_transfer_features()
             -> vlan_dev IFF_XMIT_DST_RELEASE is not changed (cleared).

Do you mean that the problem is in the first case in the gap between netdev_change_features() and vlan_device_event() when bond_dev IFF_XMIT_DST_RELEASE is cleared and vlan_dev IFF_XMIT_DST_RELEASE is still set?

Reply via email to