netdev-linux: Fix inability to apply QoS on ports with custom qdiscs.

tc_del_qdisc() function only removes qdiscs with handle '1:0'.  If for
some reason the interface has a qdisc with non-zero handle attached,
tc_del_qdisc() will not delete it and subsequent tc_install() will fail
to install a new qdisc.

The problem is that Libvirt by default is setting noqueue qdisc for all
tap interfaces it creates.  This is done for performance reasons to
ensure lockless xmit.

The issue is causing non-working QoS in OpenStack setups since new
versions of Libvirt started to use OVS to configure it.  In the past,
Libvirt configured TC on its own, bypassing OVS.

Removing the handle value from the deletion request, so any qdisc can
be removed.  Changing the error checking to also pass ENOENT, since
that is the error reported if only default qdisc is present.
Alternative solution might be to use NLM_F_REPLACE, but that will be
a larger change with a potential need of refactoring.

Potential side effect of the change is that OVS may start removing
qdiscs that it didn't remove before.  Though it's not a new issue and
'linux-noop' QoS type should be used for ports that OVS should not
touch.  Otherwise, OVS owns qdiscs on all interfaces attached to it.

While at it, adding more logs as errors are not logged in any way
at the moment making the issue hard to debug.

Reported-at: https://bugzilla.redhat.com/2138339
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-October/052088.html
Reported-at: https://github.com/openvswitch/ovs-issues/issues/268
Suggested-by: Slawek Kaplonski <skaplons@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

1 files changed, 9 insertions, 4 deletions

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 7ea4070c2..59e8dc0ae 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2984,12 +2984,18 @@ netdev_linux_set_qos(struct netdev *netdev_,
         /* Delete existing qdisc. */
         error = tc_del_qdisc(netdev_);
         if (error) {
+            VLOG_WARN_RL(&rl, "%s: Failed to delete existing qdisc: %s",
+                         netdev_get_name(netdev_), ovs_strerror(error));
             goto exit;
         }
         ovs_assert(netdev->tc == NULL);
 
         /* Install new qdisc. */
         error = new_ops->tc_install(netdev_, details);
+        if (error) {
+            VLOG_WARN_RL(&rl, "%s: Failed to install new qdisc: %s",
+                         netdev_get_name(netdev_), ovs_strerror(error));
+        }
         ovs_assert((error == 0) == (netdev->tc != NULL));
     }
 
@@ -6143,13 +6149,12 @@ tc_del_qdisc(struct netdev *netdev_)
     if (!tcmsg) {
         return ENODEV;
     }
-    tcmsg->tcm_handle = tc_make_handle(1, 0);
     tcmsg->tcm_parent = TC_H_ROOT;
 
     error = tc_transact(&request, NULL);
-    if (error == EINVAL) {
-        /* EINVAL probably means that the default qdisc was in use, in which
-         * case we've accomplished our purpose. */
+    if (error == EINVAL || error == ENOENT) {
+        /* EINVAL or ENOENT probably means that the default qdisc was in use,
+         * in which case we've accomplished our purpose. */
         error = 0;
     }
     if (!error && netdev->tc) {