[libvirt PATCH v6 2/3] Allow VF vlanid to be passed as a pointer

Dmitrii Shcherbakov dmitrii.shcherbakov at canonical.com
Thu Nov 18 17:15:19 UTC 2021


There should be a way to show no intent in programming a VLAN at all
(including clearing it). This allows handling error conditions
differently when VLAN clearing is explicit (vlan id == 0) vs implicit
(vlanid == NULL - try to clear it if possible).

Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov at canonical.com>
---
 src/hypervisor/virhostdev.c |  4 +++-
 src/util/virnetdev.c        | 36 ++++++++++++++++++++++++------------
 src/util/virnetdevpriv.h    |  4 ++--
 tests/virnetdevtest.c       | 27 +++++++++++++++++++++++----
 4 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index 14ea560309..515663222c 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -463,6 +463,7 @@ virHostdevSetNetConfig(virDomainHostdevDef *hostdev,
     const virNetDevVPortProfile *virtPort;
     int vf = -1;
     bool port_profile_associate = true;
+    bool setVlan = false;
 
     if (!virHostdevIsPCINetDevice(hostdev))
         return 0;
@@ -471,6 +472,7 @@ virHostdevSetNetConfig(virDomainHostdevDef *hostdev,
         return -1;
 
     vlan = virDomainNetGetActualVlan(hostdev->parentnet);
+    setVlan = vlan != NULL;
     virtPort = virDomainNetGetActualVirtPortProfile(hostdev->parentnet);
     if (virtPort) {
         if (vlan) {
@@ -486,7 +488,7 @@ virHostdevSetNetConfig(virDomainHostdevDef *hostdev,
             return -1;
     } else {
         if (virNetDevSetNetConfig(linkdev, vf, &hostdev->parentnet->mac,
-                                  vlan, NULL, true) < 0)
+                                  vlan, NULL, setVlan) < 0)
             return -1;
     }
 
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index f2f5fa2d95..d3b72c8f0b 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1607,20 +1607,25 @@ virNetDevSendVfSetLinkRequest(const char *ifname, int vfInfoType,
 }
 
 int
-virNetDevSetVfVlan(const char *ifname, int vf, int vlanid)
+virNetDevSetVfVlan(const char *ifname, int vf, const int *vlanid)
 {
     int rc = 0;
     int requestError = 0;
 
     struct ifla_vf_vlan ifla_vf_vlan = {
          .vf = vf,
-         .vlan = vlanid,
          .qos = 0,
+         /* If vlanid is NULL, assume it needs to be cleared. */
+         .vlan = 0,
     };
 
-    /* VLAN ids 0 and 4095 are reserved per 802.1Q but are valid values. */
-    if ((vlanid < 0 || vlanid > 4095)) {
-        return -ERANGE;
+    if (vlanid != NULL) {
+        /* VLAN ids 0 and 4095 are reserved per 802.1Q but are valid values. */
+        if ((*vlanid < 0 || *vlanid > 4095)) {
+            return -ERANGE;
+        } else {
+            ifla_vf_vlan.vlan = *vlanid;
+        }
     }
 
     requestError = virNetDevSendVfSetLinkRequest(ifname, IFLA_VF_VLAN,
@@ -1630,12 +1635,12 @@ virNetDevSetVfVlan(const char *ifname, int vf, int vlanid)
         virReportSystemError(-requestError,
                              _("Cannot set interface vlanid to %d "
                                "for ifname %s vf %d"),
-                             vlanid, ifname ? ifname : "(unspecified)", vf);
+                             ifla_vf_vlan.vlan, ifname ? ifname : "(unspecified)", vf);
         rc = requestError;
     }
     VIR_DEBUG("RTM_SETLINK %s vf %d vlanid=%d - %s",
               ifname, vf,
-              vlanid, rc < 0 ? "Fail" : "Success");
+              ifla_vf_vlan.vlan, rc < 0 ? "Fail" : "Success");
     return rc;
 }
 
@@ -1689,7 +1694,7 @@ virNetDevSetVfMac(const char *ifname, int vf,
 
 int
 virNetDevSetVfConfig(const char *ifname, int vf,
-                     const virMacAddr *macaddr, int vlanid,
+                     const virMacAddr *macaddr, const int *vlanid,
                      bool *allowRetry)
 {
     int rc = 0;
@@ -2225,7 +2230,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
     const char *pfDevName = NULL;
     g_autofree char *pfDevOrig = NULL;
     g_autofree char *vfDevOrig = NULL;
-    int vlanTag = -1;
+    g_autofree int *vlanTag = NULL;
     g_autoptr(virPCIDevice) vfPCIDevice = NULL;
 
     if (vf >= 0) {
@@ -2284,10 +2289,17 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
                 return -1;
             }
 
-            vlanTag = vlan->tag[0];
+            vlanTag = g_new0(int, 1);
+            *vlanTag = vlan->tag[0];
 
         } else if (setVlan) {
-            vlanTag = 0; /* assure any existing vlan tag is reset */
+            vlanTag = g_new0(int, 1);
+            /* Assure any existing vlan tag is reset. */
+            *vlanTag = 0;
+        } else {
+            /* Indicate that setting a VLAN has not been explicitly requested.
+             * This allows selected errors in clearing a VF VLAN to be ignored. */
+            vlanTag = NULL;
         }
     }
 
@@ -2369,7 +2381,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
         }
     }
 
-    if (adminMAC || vlanTag >= 0) {
+    if (adminMAC) {
         /* Set vlanTag and admin MAC using an RTM_SETLINK request sent to
          * PFdevname+VF#, if mac != NULL this will set the "admin MAC" via
          * the PF, *not* the actual VF MAC - the admin MAC only takes
diff --git a/src/util/virnetdevpriv.h b/src/util/virnetdevpriv.h
index 7990bf5269..84a42fb747 100644
--- a/src/util/virnetdevpriv.h
+++ b/src/util/virnetdevpriv.h
@@ -31,7 +31,7 @@ virNetDevSendVfSetLinkRequest(const char *ifname, int vfInfoType,
                               const void *payload, const size_t payloadLen);
 
 int
-virNetDevSetVfVlan(const char *ifname, int vf, int vlanid);
+virNetDevSetVfVlan(const char *ifname, int vf, const int *vlanid);
 
 int
 virNetDevSetVfMac(const char *ifname, int vf,
@@ -40,5 +40,5 @@ virNetDevSetVfMac(const char *ifname, int vf,
 
 int
 virNetDevSetVfConfig(const char *ifname, int vf,
-                     const virMacAddr *macaddr, int vlanid,
+                     const virMacAddr *macaddr, const int *vlanid,
                      bool *allowRetry);
diff --git a/tests/virnetdevtest.c b/tests/virnetdevtest.c
index 15e8f75d8b..5a787fa0f1 100644
--- a/tests/virnetdevtest.c
+++ b/tests/virnetdevtest.c
@@ -73,7 +73,7 @@ int
 (*real_virNetDevSetVfMac)(const char *ifname, int vf, const virMacAddr *macaddr, bool *allowRetry);
 
 int
-(*real_virNetDevSetVfVlan)(const char *ifname, int vf, int vlanid);
+(*real_virNetDevSetVfVlan)(const char *ifname, int vf, const int *vlanid);
 
 static void
 init_syms(void)
@@ -107,7 +107,7 @@ virNetDevSetVfMac(const char *ifname, int vf,
 }
 
 int
-virNetDevSetVfVlan(const char *ifname, int vf, int vlanid)
+virNetDevSetVfVlan(const char *ifname, int vf, const int *vlanid)
 {
     init_syms();
 
@@ -208,6 +208,11 @@ testVirNetDevSetVfVlan(const void *opaque G_GNUC_UNUSED)
         const int vlan_id;
         const int rc;
     };
+    struct nullVlanTestCase {
+        const char *ifname;
+        const int vf_num;
+        const int rc;
+    };
     size_t i = 0;
     int rc = 0;
     const struct testCase testCases[] = {
@@ -228,13 +233,27 @@ testVirNetDevSetVfVlan(const void *opaque G_GNUC_UNUSED)
         { .ifname = "fakeiface-ok", .vf_num = 1, .vlan_id = 42, .rc = 0 },
     };
 
+    const struct nullVlanTestCase nullVLANTestCases[] = {
+        { .ifname = "fakeiface-eperm", .vf_num = 1, .rc = -EPERM },
+        { .ifname = "fakeiface-eagain", .vf_num = 1, .rc = -EAGAIN },
+        /* Successful requests with vlan id 0 need to have a zero return code. */
+        { .ifname = "fakeiface-ok", .vf_num = 1, .rc = 0 },
+    };
+
     for (i = 0; i < sizeof(testCases) / sizeof(struct testCase); ++i) {
-       rc = virNetDevSetVfVlan(testCases[i].ifname, testCases[i].vf_num, testCases[i].vlan_id);
+       rc = virNetDevSetVfVlan(testCases[i].ifname, testCases[i].vf_num, &testCases[i].vlan_id);
        if (rc != testCases[i].rc) {
            return -1;
        }
     }
 
+    for (i = 0; i < sizeof(nullVLANTestCases) / sizeof(struct nullVlanTestCase); ++i) {
+       rc = virNetDevSetVfVlan(nullVLANTestCases[i].ifname, nullVLANTestCases[i].vf_num, NULL);
+       if (rc != nullVLANTestCases[i].rc) {
+           return -1;
+       }
+    }
+
     return 0;
 }
 
@@ -262,7 +281,7 @@ testVirNetDevSetVfConfig(const void *opaque G_GNUC_UNUSED)
     };
 
     for (i = 0; i < sizeof(testCases) / sizeof(struct testCase); ++i) {
-       rc = virNetDevSetVfConfig(testCases[i].ifname, vfNum, &mac, vlanid, allowRetry);
+       rc = virNetDevSetVfConfig(testCases[i].ifname, vfNum, &mac, &vlanid, allowRetry);
        if (rc != testCases[i].rc) {
            return -1;
        }
-- 
2.32.0





More information about the libvir-list mailing list