diff mbox series

[ovs-dev,1/2] netdev-linux: update LAG in all cases

Message ID 20200507155412.3854062-2-aconole@redhat.com
State Superseded
Delegated to: Ilya Maximets
Headers show
Series TC flower support fixes | expand

Commit Message

Aaron Conole May 7, 2020, 3:54 p.m. UTC
In some cases, when processing a netlink change event, it's possible for
an alternate part of OvS (like the IPv6 endpoint processing) to hold an
active netdev interface.  This creates a race-condition, where sometimes
the OvS change processing will take the normal path.  This doesn't work
because the netdev device object won't actually be enslaved to the
ovs-system (for instance, a linux bond) and ingress qdisc entries will
be missing.

To address this, we update the LAG information in ALL cases where
LAG infromation could come in.

Fixes: d22f8927c3c9 ("netdev-linux: monitor and offload LAG slaves to TC")
Cc: Marcelo Leitner <mleitner@redhat.com>
Cc: John Hurley <john.hurley@netronome.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 lib/netdev-linux.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Roi Dayan May 10, 2020, 6:23 a.m. UTC | #1
On 2020-05-07 6:54 PM, Aaron Conole wrote:
> In some cases, when processing a netlink change event, it's possible for
> an alternate part of OvS (like the IPv6 endpoint processing) to hold an
> active netdev interface.  This creates a race-condition, where sometimes
> the OvS change processing will take the normal path.  This doesn't work
> because the netdev device object won't actually be enslaved to the
> ovs-system (for instance, a linux bond) and ingress qdisc entries will
> be missing.
> 
> To address this, we update the LAG information in ALL cases where
> LAG infromation could come in.
> 
> Fixes: d22f8927c3c9 ("netdev-linux: monitor and offload LAG slaves to TC")
> Cc: Marcelo Leitner <mleitner@redhat.com>
> Cc: John Hurley <john.hurley@netronome.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  lib/netdev-linux.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index b52071e92e..4006746bb3 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -655,10 +655,6 @@ netdev_linux_update_lag(struct rtnetlink_change *change)
>  {
>      struct linux_lag_slave *lag;
>  
> -    if (!rtnetlink_type_is_rtnlgrp_link(change->nlmsg_type)) {
> -        return;
> -    }
> -
>      if (change->slave && netdev_linux_kind_is_lag(change->slave)) {
>          lag = shash_find_data(&lag_shash, change->ifname);
>  
> @@ -756,8 +752,11 @@ netdev_linux_run(const struct netdev_class *netdev_class OVS_UNUSED)
>                      netdev_linux_update(netdev, nsid, &change);
>                      ovs_mutex_unlock(&netdev->mutex);
>                  }
> -                else if (!netdev_ && change.ifname) {
> -                    /* Netdev is not present in OvS but its master could be. */
> +
> +                if (change.ifname &&
> +                    rtnetlink_type_is_rtnlgrp_link(change.nlmsg_type)) {
> +
> +                    /* Need to try updating the LAG information */
>                      ovs_mutex_lock(&lag_mutex);
>                      netdev_linux_update_lag(&change);
>                      ovs_mutex_unlock(&lag_mutex);
> 

Acked-by: Roi Dayan <roid@mellanox.com>
diff mbox series

Patch

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index b52071e92e..4006746bb3 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -655,10 +655,6 @@  netdev_linux_update_lag(struct rtnetlink_change *change)
 {
     struct linux_lag_slave *lag;
 
-    if (!rtnetlink_type_is_rtnlgrp_link(change->nlmsg_type)) {
-        return;
-    }
-
     if (change->slave && netdev_linux_kind_is_lag(change->slave)) {
         lag = shash_find_data(&lag_shash, change->ifname);
 
@@ -756,8 +752,11 @@  netdev_linux_run(const struct netdev_class *netdev_class OVS_UNUSED)
                     netdev_linux_update(netdev, nsid, &change);
                     ovs_mutex_unlock(&netdev->mutex);
                 }
-                else if (!netdev_ && change.ifname) {
-                    /* Netdev is not present in OvS but its master could be. */
+
+                if (change.ifname &&
+                    rtnetlink_type_is_rtnlgrp_link(change.nlmsg_type)) {
+
+                    /* Need to try updating the LAG information */
                     ovs_mutex_lock(&lag_mutex);
                     netdev_linux_update_lag(&change);
                     ovs_mutex_unlock(&lag_mutex);