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@xxxxxxxxxxxxx>
---
src/hypervisor/virhostdev.c | 4 +++-
src/util/virnetdev.c | 41 ++++++++++++++++++++++++-------------
src/util/virnetdevpriv.h | 4 ++--
tests/virnetdevtest.c | 27 ++++++++++++++++++++----
4 files changed, 55 insertions(+), 21 deletions(-)
diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index e44eb7f1d3..929b0a0805 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 043225d31f..eacdc3bf1f 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1589,20 +1589,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,
@@ -1612,12 +1617,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;
}
@@ -1671,7 +1676,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;
@@ -2207,7 +2212,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) {
@@ -2266,10 +2271,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;
}
}
@@ -2351,7 +2363,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
@@ -2446,7 +2458,8 @@ virNetDevSendVfSetLinkRequest(const char *ifname G_GNUC_UNUSED, int vfInfoType G
}
int
-virNetDevSetVfVlan(const char *ifname G_GNUC_UNUSED, int vf G_GNUC_UNUSED, int vlanid G_GNUC_UNUSED)
+virNetDevSetVfVlan(const char *ifname G_GNUC_UNUSED, int vf G_GNUC_UNUSED,
+ const int *vlanid G_GNUC_UNUSED)
{
virReportSystemError(ENOSYS, "%s",
_("Unable to set a VF VLAN on this platform"));
@@ -2465,7 +2478,7 @@ virNetDevSetVfMac(const char *ifname G_GNUC_UNUSED, int vf G_GNUC_UNUSED,
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 virMacAddr *macaddr 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 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 c8dba05c31..f5f54653bb 100644
--- a/tests/virnetdevtest.c
+++ b/tests/virnetdevtest.c
@@ -75,7 +75,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)
@@ -109,7 +109,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();
@@ -210,6 +210,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[] = {
@@ -230,13 +235,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;
}
@@ -264,7 +283,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
