diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c index bcd5e96c2c..9a83f23854 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 96f969500f..b1d920d3d0 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1588,19 +1588,23 @@ virNetDevSendVfSetLinkRequest(const char *ifname, int virNetDevSetVfVlan(const char *ifname, int vf, - int vlanid) + const int *vlanid) { int ret = -1; struct ifla_vf_vlan ifla_vf_vlan = { .vf = vf, - .vlan = vlanid, + .vlan = 0, .qos = 0, }; - /* VLAN ids 0 and 4095 are reserved per 802.1Q but are valid values. */ - if ((vlanid < 0 || vlanid > 4095)) { - virReportError(ERANGE, _("vlanid out of range: %d"), vlanid); - return -ERANGE; + /* If vlanid is NULL, assume it needs to be cleared. */ + if (vlanid) { + /* VLAN ids 0 and 4095 are reserved per 802.1Q but are valid values. */ + if ((*vlanid < 0 || *vlanid > 4095)) { + virReportError(ERANGE, _("vlanid out of range: %d"), *vlanid); + return -ERANGE; + } + ifla_vf_vlan.vlan = *vlanid; } ret = virNetDevSendVfSetLinkRequest(ifname, IFLA_VF_VLAN, @@ -1609,11 +1613,11 @@ virNetDevSetVfVlan(const char *ifname, if (ret < 0) { virReportSystemError(-ret, _("Cannot set interface vlanid to %d for ifname %s vf %d"), - vlanid, ifname ? ifname : "(unspecified)", vf); + ifla_vf_vlan.vlan, ifname ? ifname : "(unspecified)", vf); } VIR_DEBUG("RTM_SETLINK %s vf %d vlanid=%d - %s", - ifname, vf, vlanid, ret < 0 ? "Fail" : "Success"); + ifname, vf, ifla_vf_vlan.vlan, ret < 0 ? "Fail" : "Success"); return ret; } @@ -1663,7 +1667,7 @@ int virNetDevSetVfConfig(const char *ifname, int vf, const virMacAddr *macaddr, - int vlanid, + const int *vlanid, bool *allowRetry) { int ret = -1; @@ -2200,7 +2204,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) { @@ -2259,10 +2263,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; } } @@ -2344,7 +2355,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 @@ -2442,7 +2453,7 @@ virNetDevSendVfSetLinkRequest(const char *ifname G_GNUC_UNUSED, int virNetDevSetVfVlan(const char *ifname G_GNUC_UNUSED, int vf G_GNUC_UNUSED, - int vlanid G_GNUC_UNUSED) + const int *vlanid G_GNUC_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Unable to set a VF VLAN on this platform")); @@ -2464,7 +2475,7 @@ int virNetDevSetVfConfig(const char *ifname G_GNUC_UNUSED, int vf G_GNUC_UNUSED, const virMacAddr *macaddr G_GNUC_UNUSED, - int vlanid G_GNUC_UNUSED, + const int *vlanid G_GNUC_UNUSED, bool *allowRetry G_GNUC_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/virnetdevpriv.h b/src/util/virnetdevpriv.h index 8730e30676..8c0094b2b5 100644 --- a/src/util/virnetdevpriv.h +++ b/src/util/virnetdevpriv.h @@ -35,7 +35,7 @@ virNetDevSendVfSetLinkRequest(const char *ifname, int virNetDevSetVfVlan(const char *ifname, int vf, - int vlanid); + const int *vlanid); int virNetDevSetVfMac(const char *ifname, @@ -47,5 +47,5 @@ int virNetDevSetVfConfig(const char *ifname, int vf, const virMacAddr *macaddr, - int vlanid, + const int *vlanid, bool *allowRetry); diff --git a/tests/virnetdevtest.c b/tests/virnetdevtest.c index 727a3bdff5..8fe7e7f86d 100644 --- a/tests/virnetdevtest.c +++ b/tests/virnetdevtest.c @@ -82,7 +82,7 @@ int int (*real_virNetDevSetVfVlan)(const char *ifname, int vf, - int vlanid); + const int *vlanid); static void init_syms(void) @@ -119,7 +119,7 @@ virNetDevSetVfMac(const char *ifname, int virNetDevSetVfVlan(const char *ifname, int vf, - int vlanid) + const int *vlanid) { init_syms(); @@ -222,6 +222,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[] = { @@ -242,13 +247,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; } @@ -276,7 +295,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; }