From: Zhu, Lingshan <lingshan.zhu@xxxxxxxxx>
Sent: Tuesday, August 16, 2022 12:19 AM
On 8/16/2022 10:32 AM, Parav Pandit wrote:
From: Zhu Lingshan <lingshan.zhu@xxxxxxxxx>
Sent: Monday, August 15, 2022 5:27 AM
Some fields of virtio-net device config space are conditional on the
feature bits, the spec says:
"The mac address field always exists
(though is only valid if VIRTIO_NET_F_MAC is set)"
"max_virtqueue_pairs only exists if VIRTIO_NET_F_MQ or
VIRTIO_NET_F_RSS is set"
"mtu only exists if VIRTIO_NET_F_MTU is set"
so we should read MTU, MAC and MQ in the device config space only
when these feature bits are offered.
Yes.
For MQ, if both VIRTIO_NET_F_MQ and VIRTIO_NET_F_RSS are not set,
the
virtio device should have one queue pair as default value, so when
userspace querying queue pair numbers, it should return mq=1 than zero.
No.
No need to treat mac and max_qps differently.
It is meaningless to differentiate when field exist/not-exists vs value
valid/not valid.
as we discussed before, MQ has a default value 1, to be a functional virtio-
net device, while MAC has no default value, if no VIRTIO_NET_F_MAC set,
the driver should generate a random MAC.
For MTU, if VIRTIO_NET_F_MTU is not set, we should not read MTU from
the device config sapce.
RFC894 <A Standard for the Transmission of IP Datagrams over Ethernet
Networks> says:"The minimum length of the data field of a packet sent
Networks> over
an Ethernet is 1500 octets, thus the maximum length of an IP datagram
sent over an Ethernet is 1500 octets. Implementations are encouraged
to support full-length packets"
This line in the RFC 894 of 1984 is wrong.
Errata already exists for it at [1].
[1] https://www.rfc-editor.org/errata_search.php?rfc=894&rec_status=0
OK, so I think we should return nothing if _F_MTU not set, like handling the
MAC
virtio spec says:"The virtio network device is a virtual ethernet
card", so the default MTU value should be 1500 for virtio-net.
Practically I have seen 1500 and highe mtu.
And this derivation is not good of what should be the default mtu as above
errata exists.
And I see the code below why you need to work so hard to define a default
value so that _MQ and _MTU can report default values.
There is really no need for this complexity and such a long commit
message.
Can we please expose feature bits as-is and report config space field which
are valid?
User space will be querying both.
I think MAC and MTU don't have default values, so return nothing if the
feature bits not set,
for MQ, it is still max_vq_paris == 1 by default.
+ if ((features & BIT_ULL(VIRTIO_NET_F_MTU)) == 0)
+ val_u16 = 1500;
+ else
+ val_u16 = __virtio16_to_cpu(true, config->mtu);
+
Need to work hard to find default values and that too turned out had
errata.
There are more fields that doesn’t have default values.
There is no point in kernel doing this guess work, that user space can figure
out of what is valid/invalid.
It's not guest work, when guest finds no feature bits set, it can decide what
to do.
