Re: [libvirt PATCH v3 1/1] Ignore EPERM on attempts to clear VF VLAN ID

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

On 11/15/21 1:59 PM, Dmitrii Shcherbakov wrote:

[...]
diff --git a/tests/virnetdevtest.c b/tests/virnetdevtest.c
index aadbeb1ef4..bdaa94e83c 100644
--- a/tests/virnetdevtest.c
+++ b/tests/virnetdevtest.c
Maybe I'm just looking at it too superficially, but it seems like these test cases are testing the test jig itself more than any of the library code (the one exception is that testVirNetDevSetVfConfig() checks that the real virNetDevSetVfConfig() returns success when setting the vlan failed after setting the MAC succeeded). I'm not saying that anything better could be accomplished without mocking netlink itself, just wonder if it's worth all this extra bulk for the small amount of testing of actual library code that's accomplished. 



@@ -18,11 +18,17 @@
#include <config.h> +#include "internal.h" 
  #include "testutils.h"
+#include "virnetlink.h" 
+
+#define LIBVIRT_VIRNETDEVPRIV_H_ALLOW
+
  #ifdef __linux__
-# include "virnetdev.h" 
+# include "virmock.h"
+# include "virnetdevpriv.h"
# define VIR_FROM_THIS VIR_FROM_NONE @@ -59,6 +65,211 @@ testVirNetDevGetLinkInfo(const void *opaque) 
      return 0;
  }
+int 
+(*real_virNetDevSendVfSetLinkRequest)(const char *ifname, int vfInfoType,
+                                      const void *payload, const size_t payloadLen);
+
+int
+(*real_virNetDevSetVfMac)(const char *ifname, int vf, const virMacAddr *macaddr, bool *allowRetry);
+
+int
+(*real_virNetDevSetVfVlan)(const char *ifname, int vf, int vlanid);
+
+static void
+init_syms(void)
+{
+    VIR_MOCK_REAL_INIT(virNetDevSendVfSetLinkRequest);
+    VIR_MOCK_REAL_INIT(virNetDevSetVfMac);
+    VIR_MOCK_REAL_INIT(virNetDevSetVfVlan);
+}
+
+int
+virNetDevSetVfMac(const char *ifname, int vf,
+                  const virMacAddr *macaddr,
+                  bool *allowRetry)
+{
+    init_syms();
+
+    if (STREQ_NULLABLE(ifname, "fakeiface-macerror")) {
+        return -1;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-altmacerror")) {
+        return -2;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-macerror-novlanerror")) {
+        return -1;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-macerror-vlanerror")) {
+        return -1;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-nomacerror-novlanerror")) {
+        return 0;
+    }
+    return real_virNetDevSetVfMac(ifname, vf, macaddr, allowRetry);
+}
+
+int
+virNetDevSetVfVlan(const char *ifname, int vf, int vlanid)
+{
+    init_syms();
+
+    if (STREQ_NULLABLE(ifname, "fakeiface-macerror-vlanerror")) {
+        return -1;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-macerror-novlanerror")) {
+        return 0;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-nomacerror-novlanerror")) {
+        return 0;
+    }
+    return real_virNetDevSetVfVlan(ifname, vf, vlanid);
+}
+
+int
+virNetDevSendVfSetLinkRequest(const char *ifname, int vfInfoType,
+                              const void *payload, const size_t payloadLen)
+{
+    init_syms();
+
+    if (STREQ_NULLABLE(ifname, "fakeiface-eperm")) {
+        return -EPERM;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-eagain")) {
+        return -EAGAIN;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-einval")) {
+        return -EINVAL;
+    } else if (STREQ_NULLABLE(ifname, "fakeiface-ok")) {
+        return 0;
+    }
+    return real_virNetDevSendVfSetLinkRequest(ifname, vfInfoType, payload, payloadLen);
+}
+
+static int
+testVirNetDevSetVfMac(const void *opaque G_GNUC_UNUSED)
+{
+    struct testCase {
+        const char *ifname;
+        const int vf_num;
+        const virMacAddr macaddr;
+        bool allow_retry;
+        const int rc;
+    };
+    size_t i = 0;
+    int rc = 0;
+    struct testCase testCases[] = {
+        { .ifname = "fakeiface-ok", .vf_num = 1,
+          .macaddr = { .addr = { 0, 0, 0, 0, 0, 0 } }, .allow_retry = false, .rc = 0 },
+        { .ifname = "fakeiface-ok", .vf_num = 2,
+          .macaddr = { .addr = { 0, 0, 0, 7, 7, 7 } }, .allow_retry = false, .rc = 0 },
+        { .ifname = "fakeiface-ok", .vf_num = 3,
+          .macaddr = { .addr = { 0, 0, 0, 0, 0, 0 } }, .allow_retry = true, .rc = 0 },
+        { .ifname = "fakeiface-ok", .vf_num = 4,
+          .macaddr = { .addr = { 0, 0, 0, 7, 7, 7 } }, .allow_retry = true, .rc = 0 },
+        { .ifname = "fakeiface-eperm", .vf_num = 5,
+          .macaddr = { .addr = { 0, 0, 0, 0, 0, 0 } }, .allow_retry = false, .rc = -1 },
+        { .ifname = "fakeiface-einval", .vf_num = 6,
+          .macaddr = { .addr = { 0, 0, 0, 0, 0, 0 } }, .allow_retry = false, .rc = -1 },
+        { .ifname = "fakeiface-einval", .vf_num = 7,
+          .macaddr = { .addr = { 0, 0, 0, 0, 0, 0 } }, .allow_retry = true, .rc = -2 },
+        { .ifname = "fakeiface-einval", .vf_num = 8,
+          .macaddr = { .addr = { 0, 0, 0, 7, 7, 7 } }, .allow_retry = false, .rc = -1 },
+        { .ifname = "fakeiface-einval", .vf_num = 9,
+          .macaddr = { .addr = { 0, 0, 0, 7, 7, 7 } }, .allow_retry = true, .rc = -1 },
+    };
+
+    for (i = 0; i < sizeof(testCases) / sizeof(struct testCase); ++i) {
+       rc = virNetDevSetVfMac(testCases[i].ifname, testCases[i].vf_num,
+                              &testCases[i].macaddr, &testCases[i].allow_retry);
+       if (rc != testCases[i].rc) {
+           return -1;
+       }
+    }
+    return 0;
+}
+
+static int
+testVirNetDevSetVfMissingMac(const void *opaque G_GNUC_UNUSED)
+{
+    bool allowRetry = false;
+    /* NULL MAC pointer. */
+    if (virNetDevSetVfMac("fakeiface-ok", 1, NULL, &allowRetry) != -1) {
+        return -1;
+    }
+    allowRetry = true;
+    if (virNetDevSetVfMac("fakeiface-ok", 1, NULL, &allowRetry) != -1) {
+        return -1;
+    }
+    return 0;
+}
+
+static int
+testVirNetDevSetVfVlan(const void *opaque G_GNUC_UNUSED)
+{
+    struct testCase {
+        const char *ifname;
+        const int vf_num;
+        const int vlan_id;
+        const int rc;
+    };
+    size_t i = 0;
+    int rc = 0;
+    const struct testCase testCases[] = {
+        /* VLAN ID is out of range of valid values (0-4095). */
+        { .ifname = "enxdeadbeefcafe", .vf_num = 1, .vlan_id = 4096, .rc = -1 },
+        { .ifname = "enxdeadbeefcafe", .vf_num = 1, .vlan_id = -1, .rc = -1 },
+        /* Requests with vlan id 0 (trying to clear a VLAN that return EPERM should
+         * not result in an error. They simply need to be ignored because it may
+         * indicate that the NIC switch functionality is not exposed to the host. */
+        { .ifname = "fakeiface-eperm", .vf_num = 1, .vlan_id = 0, .rc = 0 },
+        /* Requests with vlan id 0 with an error output different from EPERM are failures. */
+        { .ifname = "fakeiface-eagain", .vf_num = 1, .vlan_id = 0, .rc = -1 },
+        /* Successful requests with vlan id 0 need to have a zero return code. */
+        { .ifname = "fakeiface-ok", .vf_num = 1, .vlan_id = 0, .rc = 0 },
+        /* Requests with a non-zero VLAN ID that result in an EPERM need to result in failures.
+         * failures. */
+        { .ifname = "fakeiface-eperm", .vf_num = 1, .vlan_id = 42, .rc = -1 },
+        /* Requests with a non-zero VLAN ID that result in some other errors need to result in
+         * failures. */
+        { .ifname = "fakeiface-eagain", .vf_num = 1, .vlan_id = 42, .rc = -1 },
+        /* Successful requests with a non-zero VLAN ID */
+        { .ifname = "fakeiface-ok", .vf_num = 1, .vlan_id = 42, .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);
+       if (rc != testCases[i].rc) {
+           return -1;
+       }
+    }
+
+    return 0;
+}
+
+static int
+testVirNetDevSetVfConfig(const void *opaque G_GNUC_UNUSED)
+{
+    struct testCase {
+        const char *ifname;
+        const int rc;
+    };
+    int rc = 0;
+    size_t i = 0;
+    /* Nested functions are mocked so dummy values are used. */
+    const virMacAddr mac = { .addr = { 0xDE, 0xAD, 0xBE, 0xEF, 0xCA, 0xFE }};
+    const int vfNum = 1;
+    const int vlanid = 0;
+    bool *allowRetry = NULL;
+
+    const struct testCase testCases[] = {
+        { .ifname = "fakeiface-macerror", .rc = -1 },
+        { .ifname = "fakeiface-altmacerror", .rc = -2 },
+        { .ifname = "fakeiface-macerror-novlanerror", .rc = -1 },
+        { .ifname = "fakeiface-macerror-vlanerror", .rc = -1 },
+        { .ifname = "fakeiface-nomacerror-novlanerror", .rc = 0 },
+    };
+
+    for (i = 0; i < sizeof(testCases) / sizeof(struct testCase); ++i) {
+       rc = virNetDevSetVfConfig(testCases[i].ifname, vfNum, &mac, vlanid, allowRetry);
+       if (rc != testCases[i].rc) {
+           return -1;
+       }
+    }
+    return 0;
+}
+
  static int
  mymain(void)
  {
@@ -76,6 +287,15 @@ mymain(void)
      DO_TEST_LINK("lo", VIR_NETDEV_IF_STATE_UNKNOWN, 0);
      DO_TEST_LINK("eth0-broken", VIR_NETDEV_IF_STATE_DOWN, 0);
+ if (virTestRun("Set VF MAC", testVirNetDevSetVfMac, NULL) < 0) 
+        ret = -1;
+    if (virTestRun("Set VF MAC: missing MAC pointer", testVirNetDevSetVfMissingMac, NULL) < 0)
+        ret = -1;
+    if (virTestRun("Set VF VLAN", testVirNetDevSetVfVlan, NULL) < 0)
+        ret = -1;
+    if (virTestRun("Set VF Config", testVirNetDevSetVfConfig, NULL) < 0)
+        ret = -1;
+
      return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
  }
[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux