Page MenuHomeFreeBSD

[bhyve][virtio-net] Add MTU advice support
ClosedPublic

Authored by afedorov on Mar 5 2020, 3:56 PM.

Details

Summary

After r358180, all network backends (tap, netmap) support MTU > 1500. So, I think it would be useful to be able to give a hint to the guest system about host MTU.

The motivation for this change can be seen here: https://lists.oasis-open.org/archives/virtio-dev/201601/msg00002.html

Specification: https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html (5.1.3 Feature bits)

Please note, that this feature is optional. Some OS's are too old. Some, such as FreeBSD, do not support this.

I want to add support for the FreeBSD if_vtnet.c driver in a separate review.

Test Plan

You can add MTU advice to the virtio-net option:

bhyve .... -s X:Y:Z,virtio-net,[tapN|valeX:N],mtu=9000

I tested this patch on the following systems:

  • Ubuntu 18.04 - work
  • CentOS 7, 8 - work
  • Windows 10 - work
  • FreeBSD - not work (I have a patch)
  • Ubuntu 16.04 - not work (too old)

We have been using this patch in production for more than six months.

Test net_parsemtu

#include <assert.h>                                                                                                                                                                               
#include <errno.h>                                                                                                                                                                                
#include <stdlib.h>                                                                                                                                                                               
#include <stdio.h>                                                                                                                                                                                
#include <string.h>                                                                                                                                                                               
#include <limits.h>                                                                                                                                                                               
                                                                                                                                                                                                  
char *test_strings[] = {                                                                                                                                                                          
        "10",                                                                                                                                                                                     
        "20",                                                                                                                                                                                     
        "100",                                                                                                                                                                                    
        "1400",                                                                                                                                                                                   
        "1500",                                                                                                                                                                                   
        "2000",                                                                                                                                                                                   
        "9000",                                                                                                                                                                                   
        "16000",                                                                                                                                                                                  
        "65535",                                                                                                                                                                                  
        "90000",                                                                                                                                                                                  
        "-1",                                                                                                                                                                                     
        "-1480",                                                                                                                                                                                  
        "-50000",                                                                                                                                                                                 
        "-a10",                                                                                                                                                                                   
        "a10",                                                                                                                                                                                    
        "abcde",                                                                                                                                                                                  
        "xyz",                                                                                                                                                                                    
        "17-26",                                                                                                                                                                                  
        "15c4",                                                                                                                                                                                   
        "12+80",                                                                                                                                                                                  
        "1",                                                                                                                                                                                      
        "c",                                                                                                                                                                                      
        "-",                                                                                                                                                                                      
        "x",                                                                                                                                                                                      
        "+",                                                                                                                                                                                      
        "+1500",                                                                                                                                                                                  
        "\0",                                                                                                                                                                                     
        "",
        NULL
};

int
net_parsemtu(const char *mtu_str, unsigned long *mtu)
{
        char *end;
        unsigned long val;

        assert(mtu_str != NULL);

        if (*mtu_str == '-')
                goto err;

        val = strtoul(mtu_str, &end, 10);

        if (*end != '\0')
                goto err;

        if (val == ULONG_MAX)
                return (ERANGE);

        if (val == 0 && errno == EINVAL)
                return (EINVAL);

        *mtu = val;

        return (0);

err:
        errno = EINVAL;
        return (EINVAL);
}

int main(int argc, char **argv)
{
        for (int i = 0; test_strings[i] != NULL; i++) {
                unsigned long mtu = 1500;
                errno = 0;
                int ret = net_parsemtu(test_strings[i], &mtu);
                printf("Test: '%s' \tret: %3d \tmtu: %lu  \terr: %s\n",
                                test_strings[i], ret, mtu, strerror(errno));
        }

        return 0;
}

./test_parse_mtu

Test: '10'      ret:   0        mtu: 10         err: No error: 0
Test: '20'      ret:   0        mtu: 20         err: No error: 0
Test: '100'     ret:   0        mtu: 100        err: No error: 0
Test: '1400'    ret:   0        mtu: 1400       err: No error: 0
Test: '1500'    ret:   0        mtu: 1500       err: No error: 0
Test: '2000'    ret:   0        mtu: 2000       err: No error: 0
Test: '9000'    ret:   0        mtu: 9000       err: No error: 0
Test: '16000'   ret:   0        mtu: 16000      err: No error: 0
Test: '65535'   ret:   0        mtu: 65535      err: No error: 0
Test: '90000'   ret:   0        mtu: 90000      err: No error: 0
Test: '-1'      ret:  22        mtu: 1500       err: Invalid argument
Test: '-1480'   ret:  22        mtu: 1500       err: Invalid argument
Test: '-50000'  ret:  22        mtu: 1500       err: Invalid argument
Test: '-a10'    ret:  22        mtu: 1500       err: Invalid argument
Test: 'a10'     ret:  22        mtu: 1500       err: Invalid argument
Test: 'abcde'   ret:  22        mtu: 1500       err: Invalid argument
Test: 'xyz'     ret:  22        mtu: 1500       err: Invalid argument
Test: '17-26'   ret:  22        mtu: 1500       err: Invalid argument
Test: '15c4'    ret:  22        mtu: 1500       err: Invalid argument
Test: '12+80'   ret:  22        mtu: 1500       err: Invalid argument
Test: '1'       ret:   0        mtu: 1          err: No error: 0
Test: 'c'       ret:  22        mtu: 1500       err: Invalid argument
Test: '-'       ret:  22        mtu: 1500       err: Invalid argument
Test: 'x'       ret:  22        mtu: 1500       err: Invalid argument
Test: '+'       ret:  22        mtu: 1500       err: Invalid argument
Test: '+1500'   ret:   0        mtu: 1500       err: No error: 0
Test: ''        ret:  22        mtu: 1500       err: Invalid argument
Test: ''        ret:  22        mtu: 1500       err: Invalid argument

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

usr.sbin/bhyve/net_utils.c
76 ↗(On Diff #69222)

Why restrict this to ETHER MTU's at all? And shouldnt that restriction actually be in the kernel code that implements this and not rely on some userland bounds checking?

usr.sbin/bhyve/pci_virtio_net.c
537 ↗(On Diff #69222)

Isnt this going to end up with an MTU of ETHERMIN if it is not specified on command line? If I did my math right this value is 46.

usr.sbin/bhyve/net_utils.c
76 ↗(On Diff #69222)

I think we need to follow the specification (sec 5.1.4.1)
According to the spec, the minimum suggested value is 1280 and the maximum value (hard limit) 65535 (inclusive).

usr.sbin/bhyve/pci_virtio_net.c
80 ↗(On Diff #69222)

If we add this field we should also export the VIRTIO_NET_F_MQ capability,
and set max_virtqueue_pairs to 1, since we do not really support multiqueue.

537 ↗(On Diff #69222)

+1, this is a default value, and should be ETHERMTU = 1500.

620 ↗(On Diff #69222)

Set max_virtqueue_pairs to 1.

  • Move checks of the MTU value to pci_virtio_net.c and add the corresponding min/max definitions from the virtio specification.
  • Fix default MTU value.
  • Add VIRTIO_NET_F_MQ capability, and initialize max_virtqueue_pairs.
rgrimes requested changes to this revision.Mar 10 2020, 5:59 PM

I have no idea why someone thinks a network device should have a minimum MTU of 1280, that is simply the IPv6 value, ethernet is very happy to transfer 64 byte packets. There should be some implementation detail of the in kernel vt driver that can at least go that small, and perhaps smaller as you do not have the collision detection minimum wire time that ethernet has(had).

usr.sbin/bhyve/pci_virtio_net.c
70 ↗(On Diff #69366)

I saw that this was refered to by vmaffione, and I know this constant as IPV6 minimum MTU, but it it NOT a link layer function of a network device. Ethernets minimum MTU is much smaller, and again unless there is some great reason to inforce these in the userland it should be the kernel that decides these limits, and it ultimately has to protect itself against out of range values anyway. This does NOT belong in userland code.

71 ↗(On Diff #69366)

Ditto, this does not belong here.

This revision now requires changes to proceed.Mar 10 2020, 5:59 PM

I have no idea why someone thinks a network device should have a minimum MTU of 1280, that is simply the IPv6 value, ethernet is very happy to transfer 64 byte packets. There should be some implementation detail of the in kernel vt driver that can at least go that small, and perhaps smaller as you do not have the collision detection minimum wire time that ethernet has(had).

So you suggest to violate RFC 6540 and disallow IPv6? ATM has a very small frame size, but the fragmentation FRF.8 is part of the standard. For IPv4 the minimum MTU is 512, for IPv6 it is 1280, even is the medium at layer 2 is able to transport smaller frames (which is perfectly allowed by a minimal maximum). 802.3 is even able to transport smaller packets than the minimum frame size ...
What do I miss?

I have no idea why someone thinks a network device should have a minimum MTU of 1280, that is simply the IPv6 value, ethernet is very happy to transfer 64 byte packets. There should be some implementation detail of the in kernel vt driver that can at least go that small, and perhaps smaller as you do not have the collision detection minimum wire time that ethernet has(had).

So you suggest to violate RFC 6540 and disallow IPv6? ATM has a very small frame size, but the fragmentation FRF.8 is part of the standard. For IPv4 the minimum MTU is 512, for IPv6 it is 1280, even is the medium at layer 2 is able to transport smaller frames (which is perfectly allowed by a minimal maximum). 802.3 is even able to transport smaller packets than the minimum frame size ...
What do I miss?

Why should RFC6540 apply here at ALL? This is a network layer/link layer device emulaiton, NOT an IPV6 layer. Though you might run ipv6 over it, it is usable for many other things.

The kernel device, vtnet, already does proper bounds check on mtu, returns an error if it is <ETHERMIN || >VTNET_MAX_MTU, adding range check code here just increases code maintanance, and if it is done it MUST much the kernel implementation (Which oddly has 65536 as VTNET_MAX_MTU, so even the 65535 above is a false collapse of capabilities.

I actually see an additional issue. According to the standard, VIRTIO_NET_F_MQ depends on VIRTIO_NET_F_CTRL_VQ.
So it looks like we should add support for VIRTIO_NET_F_CTRL_VQ as a first step.

usr.sbin/bhyve/pci_virtio_net.c
70 ↗(On Diff #69366)

Ok, so we can use the minimum mandated by the virtio standard, which is 68 bytes.
https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html

71 ↗(On Diff #69366)

But since this is a virtio device, we must follow the constraints dictated by the virtio standard, which limits the MTU to 65535. Then of course the kernel can enforce additional constraints.

633 ↗(On Diff #69366)

s/virtque/virtqueue

Let me try again to make my point of why it is wrong for this code to have ANY checks on the value of MTU other than to fit in the datatype.
The in kernel vtnet device ALREADY contains code that enforces the implementation specific limits of this value, and is a single place to make adjustments to those values.
See src/sys/dev/vertio/network/if_vtnet.c at about line 1044:

if (new_mtu < ETHERNMIN || new_mtu > VTNET_MAX_MTU)
    return (EINVAL);

This should be the sole place that limits on this value are enforced, and doing so any other place is just duplicate code that raises the work to maintain the code.

Now onto the "SPEC" calling:
1.1 Normative References

...
[IEEE 802] 	IEEE Standard for Local and Metropolitan Area Networks: Overview and Architecture,
                    http://standards.ieee.org/about/get/802/802.html, IEEE

5.1 Network Device The virtio network device is a virtual ethernet card, and is the most complex of the devices supported so far by virtio.

If this is to be a virtual ETHERNET CARD, it should conform to the IEEE ethernet specification which says the device minimum MTU is 64, doing anything else seems gratuitous, and in fact the current kernel if_vtnet.c code does exactly that.
From a quick glance at the MTU related stuff in the virtio spec it needs amended, someone has confused RFC IP specificaiton with Ethernet spcifications and are applying the wrong constants in the wrong places. The 68 byte MTU is an IPV4 minimum NOT an ethernet value. Or perhaps they should change the description of the device to "virtual IP device". There should be no mention of 1280 either, that is just conflating ethernet and IPv6.

usr.sbin/bhyve/pci_virtio_net.c
603 ↗(On Diff #69366)

Delete the above 4 lines, this is up to the kernel to decide what the limits are on these values.

I agree that the lower bound looks weird. And it came from Linux:
https://elixir.bootlin.com/linux/latest/source/drivers/net/virtio_net.c#L2978
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/if_ether.h#L40

As you can see, the Linux guest driver checks that MTU > 68, and if not, does not confirm the VIRTIO_NET_F_MTU flag and sets the default value.

I have no objection to remove the check of the lower boundary of the MTU. Or check it with ETHERMIN.
I prefer to keep checking the upper bound to prevent uin16_t type overflow. Thus, if the user started bhyve with "virtio-net, tapX, mtu=70000", EINVAL is returned.

I actually see an additional issue. According to the standard, VIRTIO_NET_F_MQ depends on VIRTIO_NET_F_CTRL_VQ.
So it looks like we should add support for VIRTIO_NET_F_CTRL_VQ as a first step.

I'm not sure, but is VIRTIO_NET_F_CTRL_VQ support really necessary for VIRTIO_NET_F_MTU?
Can we just do not negotiate on the flag VIRTIO_NET_F_MQ?

Il giorno mer 11 mar 2020 alle ore 03:11 rgrimes <
phabric-noreply@freebsd.org> ha scritto:

rgrimes added a comment.

Let me try again to make my point of why it is wrong for this code to

have ANY checks on the value of MTU other than to fit in the datatype.

The in kernel vtnet device ALREADY contains code that enforces the

implementation specific limits of this value, and is a single place to make
adjustments to those values.

See src/sys/dev/vertio/network/if_vtnet.c at about line 1044:

  if (new_mtu < ETHERNMIN || new_mtu > VTNET_MAX_MTU)
      return (EINVAL);

This should be the sole place that limits on this value are enforced,

and doing so any other place is just duplicate code that raises the work to
maintain the code.

Which kernel are referring to, exactly? Guest or host?
The MTU exposed by a virtio-net device in its configuration space is a
read-only value available to the guest kernel driver. The guest kernel can
be FreeBSD, Linux, Windows or others, so the limitations in
src/sys/dev/virtio/network/if_vtnet.c are not necessarily relevant.
In theory, a guest driver could not like a virtio-net device that exposes
an MTU that does not follow the standard (although I don't think this issue
is realistic).

Now onto the "SPEC" calling:
1.1 Normative References

  ...
  [IEEE 802]  IEEE Standard for Local and Metropolitan Area Networks:

Overview and Architecture,

http://standards.ieee.org/about/get/802/802.html,

IEEE

5.1 Network Device The virtio network device is a virtual ethernet card,

and is the most complex of the devices supported so far by virtio.

If this is to be a virtual ETHERNET CARD, it should conform to the IEEE

ethernet specification which says the device minimum MTU is 64, doing
anything else seems gratuitous, and in fact the current kernel if_vtnet.c
code does exactly that.

From a quick glance at the MTU related stuff in the virtio spec it needs

amended, someone has confused RFC IP specificaiton with Ethernet
spcifications and are applying the wrong constants in the wrong places.
The 68 byte MTU is an IPV4 minimum NOT an ethernet value. Or perhaps they
should change the description of the device to "virtual IP device". There
should be no mention of 1280 either, that is just conflating ethernet and
IPv6.

I'm ok with using the range 64..65536 rather than using 68..65535. After
all, the first range contains the second, and the difference is very small.
But I think we need to perform the check, because (1) we must make sure the
user does not provide a bogus input, and (2) we need to follow the standard.

INLINE COMMENTS

pci_virtio_net.c:603
+ errno = EINVAL;
+ break;
+ }

Delete the above 4 lines, this is up to the kernel to decide what the
limits are on these values.

CHANGES SINCE LAST ACTION

https://reviews.freebsd.org/D23971/new/

REVISION DETAIL

https://reviews.freebsd.org/D23971

EMAIL PREFERENCES

https://reviews.freebsd.org/settings/panel/emailpreferences/

To: aleksandr.fedorov_itglobal.com, vmaffione, jhb, grehan, krion,
bhyve, network, rgrimes, bryanv
Cc: lutz_donnerhacke.de, rgrimes, bcran

Il giorno mer 11 mar 2020 alle ore 11:51 aleksandr.fedorov_itglobal.com
(Aleksandr Fedorov) <phabric-noreply@freebsd.org> ha scritto:

aleksandr.fedorov_itglobal.com added a comment.

In D23971#528210 <https://reviews.freebsd.org/D23971#528210>,

@vmaffione wrote:

> I actually see an additional issue. According to the standard,

VIRTIO_NET_F_MQ depends on VIRTIO_NET_F_CTRL_VQ.

> So it looks like we should add support for VIRTIO_NET_F_CTRL_VQ as a

first step.

I'm not sure, but is VIRTIO_NET_F_CTRL_VQ support really necessary for

VIRTIO_NET_F_MTU?

Can we just do not negotiate on the flag VIRTIO_NET_F_MQ?

The VIRTIO_NET_F_CTRL_VQ is required by VIRTIO_NET_F_MQ. I did not notice
this when I earlier suggested negotiating also the VIRTIO_NET_F_MQ.
And I suggested negotiating VIRTIO_NET_F_MQ because the max_virtqueue_pairs
field comes *before* the mtu field in the virtio-net configuration space. I
thought this implied that VIRTIO_NET_F_MTU requires VIRTIO_NET_F_MQ.
But now that I looked into it a bit better, I think we can negotiate
VIRTIO_NET_F_MTU even if we do not negotiate VIRTIO_NET_F_MQ. This
circumvents the need to support the control VQ.
We should still keep max_virtqueue_pairs initialized to 1, for consistency.

CHANGES SINCE LAST ACTION

https://reviews.freebsd.org/D23971/new/

REVISION DETAIL

https://reviews.freebsd.org/D23971

EMAIL PREFERENCES

https://reviews.freebsd.org/settings/panel/emailpreferences/

To: aleksandr.fedorov_itglobal.com, vmaffione, jhb, grehan, krion,
bhyve, network, rgrimes, bryanv
Cc: lutz_donnerhacke.de, rgrimes, bcran

  • Fix typo.
  • Disable VIRTIO_NET_F_MQ flag.
  • Check the lower boundary of the MTU with ETHERMIN.

I assume you have done your tests adding mtu=9000 as a bhyve parameter.
Could you please check that you have no regression in case you don't add an mtu argument (and so with default MTU)?

usr.sbin/bhyve/net_backends.h
80 ↗(On Diff #69415)

s/multiqueues/multiple VQ pairs

Yes, I tested various MTU values ​​(including 9000) with different backends (if_tuntap(4), vale). I did not find any regressions without using the mtu argument.

But after I looked at upcoming changes from bryanv, I’m worried that if the flag VIRTIO_NET_F_MTU is always enabled, the MTU cannot be changed from the guest system.

I'm not sure if this is relevant code:
https://github.com/bryanv/freebsd/blob/virtio/sys/dev/virtio/network/if_vtnet.c#L728
https://github.com/bryanv/freebsd/blob/virtio/sys/dev/virtio/network/if_vtnet.c#L1247

Specifications also says:

5.1.3.1 Feature bit requirements

Device maximum MTU reporting is supported. If offered by the device, device advises driver about the value of its maximum MTU. If negotiated, the driver uses mtu as the maximum MTU value.

5.1.4.2 Driver Requirements: Device configuration layout

If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of size exceeding the value of mtu (plus low level ethernet header length) with gso_type NONE or ECN.

Linux also uses this value to buffer calculation: https://elixir.bootlin.com/linux/latest/source/drivers/net/virtio_net.c#L3110

So, I think we should not enable VIRTIO_NET_F_MTU capability without user provided mtu argument.

  • Fix typo.
  • Enable VIRTIO_NET_F_MTU only if user provide mtu argument.

Il giorno ven 13 mar 2020 alle ore 17:56 aleksandr.fedorov_itglobal.com
(Aleksandr Fedorov) <phabric-noreply@freebsd.org> ha scritto:

aleksandr.fedorov_itglobal.com added a comment.

Yes, I tested various MTU values (including 9000) with different

backends (if_tuntap(4), vale). I did not find any regressions without using
the mtu argument.

Ok.

But after I looked at upcoming changes from bryanv, I’m worried that if

the flag VIRTIO_NET_F_MTU is always enabled, the MTU cannot be changed from
the guest system.

I'm not sure if this is relevant code:

https://github.com/bryanv/freebsd/blob/virtio/sys/dev/virtio/network/if_vtnet.c#L728

https://github.com/bryanv/freebsd/blob/virtio/sys/dev/virtio/network/if_vtnet.c#L1247

If VIRTIO_NET_F_MTU is published by the host, the guest can change the

MTU, but it can never set it to a value greater than what the host
specifies. That's not an issue.

Specifications also says:

5.1.3.1 Feature bit requirements

> Device maximum MTU reporting is supported. If offered by the device,

device advises driver about the value of its maximum MTU. If negotiated,
the driver uses mtu as the maximum MTU value.

5.1.4.2 Driver Requirements: Device configuration layout

> If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit

packets of size exceeding the value of mtu (plus low level ethernet header
length) with gso_type NONE or ECN.

Linux also uses this value to buffer calculation:

https://elixir.bootlin.com/linux/latest/source/drivers/net/virtio_net.c#L3110

Yes, but still the guest can change the MTU. It just cannot make it too

large.

So, I think we should not enable VIRTIO_NET_F_MTU capability without

user provided mtu argument.

Yes, you are correct, but for a different motivation. The MTU feature is an
hint that the host passes to the guest. In case the bhyve user does not
provide any hint, you just not negotiate the feature. Indeed this is the
same that QEMU does.

CHANGES SINCE LAST ACTION

https://reviews.freebsd.org/D23971/new/

REVISION DETAIL

https://reviews.freebsd.org/D23971

EMAIL PREFERENCES

https://reviews.freebsd.org/settings/panel/emailpreferences/

To: aleksandr.fedorov_itglobal.com, vmaffione, jhb, grehan, krion,
bhyve, network, rgrimes, bryanv
Cc: lutz_donnerhacke.de, rgrimes, bcran

usr.sbin/bhyve/pci_virtio_net.c
541 ↗(On Diff #69487)

I would initialize this to ETHERMTU, to keep the compiler happy and (see below).

632 ↗(On Diff #69487)

I would initialize this unconditionally, because there is no harm, and having ETHERMTU in here makes more sense than having 0, in any case.

afedorov marked 2 inline comments as done.

I still object to range testing MTU here, but less so now that the values are closer to implementation limits. Base reason for objection is your range testing data at the producer, and it is the consumer that this should be, and I believe already is, done at.

You may also want to add a test with mtu=-1480. Yes, thats negative 1480. iirc strtoul is going to ignore the - and return 1500. I choose -1480 so you can detect it unique from 1500.

I understand your opinion, but because we do not know what kind of OS will be used as a guest, I think additional checks will not be superfluous. Also, I think it's easier to look for errors if bhyve crashes earlier.

afedorov edited the test plan for this revision. (Show Details)
  • Add additional checks (see Test Plan).

Also, if the bhyve user does not provide a value for mtu, this patch won't have any effect.

Can we come to some kind of consensus?

Just to recap:

  1. This patch has no effect if the user does not specify the mtu parameter explicitly. That is no effect in the default case.
  2. This patch is implementing a feature described by the virtio standard, and in the same way QEMU does.

As a result, I personally do not see any reason why we should not go ahead.

Just a note, we _have_ to check the MTU values in the device model. The device model cannot assume a FreeBSD kernel guest that will check MTU values, we have to handle other guests including malicious guests. Device models should always be independent and perform whatever checks the spec mandates similar to what a piece of hardware (or the hardware's firmware) would do.

Reading the spec, it doesn't say that you have to define MQ to use MTU. The language is a bit odd in that it says "only exists" when in practice it seems to mean "is only valid and certain to exist". The Linux driver doesn't mandate that MTU requires MQ, and it will just ignore the MQ field. I think we are probably fine with taking that approach. Neither NetBSD nor OpenBSD support the option yet, so Linux's behavior is the only one I've seen.

usr.sbin/bhyve/net_utils.c
78 ↗(On Diff #69572)

I would s/10/0/. If someone wants to enter an MTU in hex that seems fine to permit.

usr.sbin/bhyve/pci_virtio_net.c
632 ↗(On Diff #69487)

The only risk I see is what do old guest OS's do? If they ignore unknown feature bits then I agree with just setting it always.

In D23971#532136, @jhb wrote:

Reading the spec, it doesn't say that you have to define MQ to use MTU. The language is a bit odd in that it says "only exists" when in practice it seems to mean "is only valid and certain to exist". The Linux driver doesn't mandate that MTU requires MQ, and it will just ignore the MQ field. I think we are probably fine with taking that approach. Neither NetBSD nor OpenBSD support the option yet, so Linux's behavior is the only one I've seen.

Yes, indeed. On a second read I realized the same thing.

  • Allow entering MTU value in HEX.
usr.sbin/bhyve/pci_virtio_net.c
632 ↗(On Diff #69487)

I tested Ubuntu 16.04, it ignore unknown bits. But I prefer to enable the VIRTIO_NET_F_MQ flag only if the user provide the appropriate option, as is done in QEMU.

usr.sbin/bhyve/pci_virtio_net.c
632 ↗(On Diff #69487)

Yes, they ignore unknown feature bits. They don't read unknown fields.

usr.sbin/bhyve/pci_virtio_net.c
632 ↗(On Diff #69487)

Sorry, of course, I mean the VIRTIO_NET_F_MTU flag.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 7 2020, 5:06 PM
This revision was automatically updated to reflect the committed changes.