[libvirt PATCH v3 1/1] Ignore EPERM on attempts to clear VF VLAN ID
Laine Stump
laine at redhat.com
Wed Nov 17 00:42:58 UTC 2021
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;
> }
>
>
More information about the libvir-list
mailing list