This has a benefit of being able to handle error codes for those
operations separately which is useful when drivers allow setting a MAC
address but do not allow setting a VLAN (which is the case with some
SmartNIC DPUs).
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@xxxxxxxxxxxxx>
---
src/libvirt_private.syms | 7 ++
src/util/virnetdev.c | 188 ++++++++++++++++++++-------------
src/util/virnetdevpriv.h | 44 ++++++++
tests/virnetdevtest.c | 222 ++++++++++++++++++++++++++++++++++++++-
4 files changed, 388 insertions(+), 73 deletions(-)
create mode 100644 src/util/virnetdevpriv.h
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a7bc50a4d1..e681e69d77 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2849,6 +2849,13 @@ virNetDevOpenvswitchSetTimeout;
virNetDevOpenvswitchUpdateVlan;
+#util/virnetdevpriv.h
+virNetDevSendVfSetLinkRequest;
+virNetDevSetVfConfig;
+virNetDevSetVfMac;
+virNetDevSetVfVlan;
+
+
# util/virnetdevtap.h
virNetDevTapAttachBridge;
virNetDevTapCreate;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 58f7360a0f..f2f5fa2d95 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -19,7 +19,9 @@
#include <config.h>
#include <math.h>
-#include "virnetdev.h"
+#define LIBVIRT_VIRNETDEVPRIV_H_ALLOW
+
+#include "virnetdevpriv.h"
#include "viralloc.h"
#include "virnetlink.h"
#include "virmacaddr.h"
@@ -1527,16 +1529,12 @@ static struct nla_policy ifla_vfstats_policy[IFLA_VF_STATS_MAX+1] = {
[IFLA_VF_STATS_MULTICAST] = { .type = NLA_U64 },
};
-
-static int
-virNetDevSetVfConfig(const char *ifname, int vf,
- const virMacAddr *macaddr, int vlanid,
- bool *allowRetry)
+int
+virNetDevSendVfSetLinkRequest(const char *ifname, int vfInfoType,
+ const void *payload, const size_t payloadLen)
{
- int rc = -1;
- char macstr[VIR_MAC_STRING_BUFLEN];
g_autofree struct nlmsghdr *resp = NULL;
- struct nlmsgerr *err;
+ struct nlmsgerr *err = NULL;
unsigned int recvbuflen = 0;
struct nl_msg *nl_msg;
struct nlattr *vfinfolist, *vfinfo;
@@ -1544,9 +1542,11 @@ virNetDevSetVfConfig(const char *ifname, int vf,
.ifi_family = AF_UNSPEC,
.ifi_index = -1,
};
+ int rc = 0;
- if (!macaddr && vlanid < 0)
+ if (payload == NULL || payloadLen == 0) {
return -1;
+ }
nl_msg = virNetlinkMsgNew(RTM_SETLINK, NLM_F_REQUEST);
@@ -1564,30 +1564,8 @@ virNetDevSetVfConfig(const char *ifname, int vf,
if (!(vfinfo = nla_nest_start(nl_msg, IFLA_VF_INFO)))
goto buffer_too_small;
- if (macaddr) {
- struct ifla_vf_mac ifla_vf_mac = {
- .vf = vf,
- .mac = { 0, },
- };
-
- virMacAddrGetRaw(macaddr, ifla_vf_mac.mac);
-
- if (nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac),
- &ifla_vf_mac) < 0)
- goto buffer_too_small;
- }
-
- if (vlanid >= 0) {
- struct ifla_vf_vlan ifla_vf_vlan = {
- .vf = vf,
- .vlan = vlanid,
- .qos = 0,
- };
-
- if (nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan),
- &ifla_vf_vlan) < 0)
- goto buffer_too_small;
- }
+ if (nla_put(nl_msg, vfInfoType, payloadLen, payload) < 0)
+ goto buffer_too_small;
nla_nest_end(nl_msg, vfinfo);
nla_nest_end(nl_msg, vfinfolist);
@@ -1600,48 +1578,20 @@ virNetDevSetVfConfig(const char *ifname, int vf,
goto malformed_resp;
switch (resp->nlmsg_type) {
- case NLMSG_ERROR:
- err = (struct nlmsgerr *)NLMSG_DATA(resp);
- if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+ case NLMSG_ERROR:
+ err = (struct nlmsgerr *)NLMSG_DATA(resp);
+ if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+ goto malformed_resp;
+ rc = err->error;
+ break;
+ case NLMSG_DONE:
+ rc = 0;
+ break;
+ default:
goto malformed_resp;
-
- /* if allowRetry is true and the error was EINVAL, then
- * silently return a failure so the caller can retry with a
- * different MAC address
- */
- if (err->error == -EINVAL && *allowRetry &&
- macaddr && !virMacAddrCmp(macaddr, &zeroMAC)) {
- goto cleanup;
- } else if (err->error) {
- /* other errors are permanent */
- virReportSystemError(-err->error,
- _("Cannot set interface MAC/vlanid to %s/%d "
- "for ifname %s vf %d"),
- (macaddr
- ? virMacAddrFormat(macaddr, macstr)
- : "(unchanged)"),
- vlanid,
- ifname ? ifname : "(unspecified)",
- vf);
- *allowRetry = false; /* no use retrying */
- goto cleanup;
- }
- break;
-
- case NLMSG_DONE:
- break;
-
- default:
- goto malformed_resp;
}
- rc = 0;
cleanup:
- VIR_DEBUG("RTM_SETLINK %s vf %d MAC=%s vlanid=%d - %s",
- ifname, vf,
- macaddr ? virMacAddrFormat(macaddr, macstr) : "(unchanged)",
- vlanid, rc < 0 ? "Fail" : "Success");
-
nlmsg_free(nl_msg);
return rc;
@@ -1656,6 +1606,100 @@ virNetDevSetVfConfig(const char *ifname, int vf,
goto cleanup;
}
+int
+virNetDevSetVfVlan(const char *ifname, int vf, int vlanid)
+{
+ int rc = 0;
+ int requestError = 0;
+
+ struct ifla_vf_vlan ifla_vf_vlan = {
+ .vf = vf,
+ .vlan = vlanid,
+ .qos = 0,
+ };
+
+ /* VLAN ids 0 and 4095 are reserved per 802.1Q but are valid values. */
+ if ((vlanid < 0 || vlanid > 4095)) {
+ return -ERANGE;
+ }
+
+ requestError = virNetDevSendVfSetLinkRequest(ifname, IFLA_VF_VLAN,
+ &ifla_vf_vlan, sizeof(ifla_vf_vlan));
+
+ if (requestError) {
+ virReportSystemError(-requestError,
+ _("Cannot set interface vlanid to %d "
+ "for ifname %s vf %d"),
+ vlanid, ifname ? ifname : "(unspecified)", vf);
+ rc = requestError;
+ }
+ VIR_DEBUG("RTM_SETLINK %s vf %d vlanid=%d - %s",
+ ifname, vf,
+ vlanid, rc < 0 ? "Fail" : "Success");
+ return rc;
+}
+
+int
+virNetDevSetVfMac(const char *ifname, int vf,
+ const virMacAddr *macaddr,
+ bool *allowRetry)
+{
+ int rc = 0;
+ int requestError = 0;
+ char macstr[VIR_MAC_STRING_BUFLEN];
+
+ struct ifla_vf_mac ifla_vf_mac = {
+ .vf = vf,
+ .mac = { 0, },
+ };
+
+ if (macaddr == NULL || allowRetry == NULL)
+ return -EINVAL;
+
+ virMacAddrGetRaw(macaddr, ifla_vf_mac.mac);
+
+ requestError = virNetDevSendVfSetLinkRequest(ifname, IFLA_VF_MAC,
+ &ifla_vf_mac, sizeof(ifla_vf_mac));
+ /* if allowRetry is true and the error was EINVAL, then
+ * silently return a failure so the caller can retry with a
+ * different MAC address. */
+ if (requestError == -EINVAL && *allowRetry && !virMacAddrCmp(macaddr, &zeroMAC)) {
+ rc = requestError;
+ } else if (requestError) {
+ /* other errors are permanent */
+ virReportSystemError(-requestError,
+ _("Cannot set interface MAC to %s "
+ "for ifname %s vf %d"),
+ (macaddr
+ ? virMacAddrFormat(macaddr, macstr)
+ : "(unchanged)"),
+ ifname ? ifname : "(unspecified)",
+ vf);
+ *allowRetry = false; /* no use retrying */
+ rc = requestError;
+ } else {
+ rc = 0;
+ }
+ VIR_DEBUG("RTM_SETLINK %s vf %d MAC=%s - %s",
+ ifname, vf,
+ macaddr ? virMacAddrFormat(macaddr, macstr) : "(unchanged)",
+ rc < 0 ? "Fail" : "Success");
+ return rc;
+}
+
+int
+virNetDevSetVfConfig(const char *ifname, int vf,
+ const virMacAddr *macaddr, int vlanid,
+ bool *allowRetry)
+{
+ int rc = 0;
+ if ((rc = virNetDevSetVfMac(ifname, vf, macaddr, allowRetry)) < 0)
+ return rc;
+ if ((rc = virNetDevSetVfVlan(ifname, vf, vlanid)) < 0)
+ return rc;
+ return rc;
+}
+
/**
* virNetDevParseVfInfo:
* Get the VF interface information from kernel by netlink, To make netlink
diff --git a/src/util/virnetdevpriv.h b/src/util/virnetdevpriv.h
new file mode 100644
index 0000000000..7990bf5269
--- /dev/null
+++ b/src/util/virnetdevpriv.h
@@ -0,0 +1,44 @@
+/*
+ * virnetdevpriv.h: private virnetdev header for unit testing
+ *
+ * Copyright (C) 2021 Canonical Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef LIBVIRT_VIRNETDEVPRIV_H_ALLOW
+# error "virnetdevpriv.h may only be included by virnetdev.c or test suites"
+#endif /* LIBVIRT_VIRNETDEVPRIV_H_ALLOW */
+
+#pragma once
+
+#include "virnetdev.h"
+
+int
+virNetDevSendVfSetLinkRequest(const char *ifname, int vfInfoType,
+ const void *payload, const size_t payloadLen);
+
+int
+virNetDevSetVfVlan(const char *ifname, int vf, int vlanid);
+
+int
+virNetDevSetVfMac(const char *ifname, int vf,
+ const virMacAddr *macaddr,
+ bool *allowRetry);
+
+int
+virNetDevSetVfConfig(const char *ifname, int vf,
+ const virMacAddr *macaddr, int vlanid,
+ bool *allowRetry);
diff --git a/tests/virnetdevtest.c b/tests/virnetdevtest.c
index aadbeb1ef4..15e8f75d8b 100644
--- a/tests/virnetdevtest.c
+++ b/tests/virnetdevtest.c
@@ -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 -EBUSY;
+ } else if (STREQ_NULLABLE(ifname, "fakeiface-altmacerror")) {
+ return -EINVAL;
+ } else if (STREQ_NULLABLE(ifname, "fakeiface-macerror-novlanerror")) {
+ return -EAGAIN;
+ } else if (STREQ_NULLABLE(ifname, "fakeiface-macerror-vlanerror")) {
+ return -ENODEV;
+ } else if (STREQ_NULLABLE(ifname, "fakeiface-nomacerror-vlanerror")) {
+ return 0;
+ } 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 -EPERM;
+ } else if (STREQ_NULLABLE(ifname, "fakeiface-nomacerror-vlanerror")) {
+ return -EPERM;
+ } 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 = -EPERM },
+ { .ifname = "fakeiface-einval", .vf_num = 6,
+ .macaddr = { .addr = { 0, 0, 0, 0, 0, 0 } }, .allow_retry = false, .rc = -EINVAL },
+ { .ifname = "fakeiface-einval", .vf_num = 7,
+ .macaddr = { .addr = { 0, 0, 0, 0, 0, 0 } }, .allow_retry = true, .rc = -EINVAL },
+ { .ifname = "fakeiface-einval", .vf_num = 8,
+ .macaddr = { .addr = { 0, 0, 0, 7, 7, 7 } }, .allow_retry = false, .rc = -EINVAL },
+ { .ifname = "fakeiface-einval", .vf_num = 9,
+ .macaddr = { .addr = { 0, 0, 0, 7, 7, 7 } }, .allow_retry = true, .rc = -EINVAL },
+ };
+
+ 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) != -EINVAL) {
+ return -1;
+ }
+ allowRetry = true;
+ if (virNetDevSetVfMac("fakeiface-ok", 1, NULL, &allowRetry) != -EINVAL) {
+ 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 = -ERANGE },
+ { .ifname = "enxdeadbeefcafe", .vf_num = 1, .vlan_id = -1, .rc = -ERANGE },
+ { .ifname = "fakeiface-eperm", .vf_num = 1, .vlan_id = 0, .rc = -EPERM },
+ { .ifname = "fakeiface-eagain", .vf_num = 1, .vlan_id = 0, .rc = -EAGAIN },
+ /* 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 = -EPERM },
+ /* 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 = -EAGAIN },
+ /* 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 = -EBUSY },
+ { .ifname = "fakeiface-altmacerror", .rc = -EINVAL },
+ { .ifname = "fakeiface-macerror-novlanerror", .rc = -EAGAIN },
+ { .ifname = "fakeiface-macerror-vlanerror", .rc = -ENODEV },
+ { .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;
}
--
2.32.0
