Re: [RFC PATCH 0/7] Support for virtio-net hash reporting

quoted

quoted

quoted

quoted

quoted

quoted

What it does not give is a type indication, such as
VIRTIO_NET_HASH_TYPE_TCPv6. I don't understand how this would be used.
In datapaths where the NIC has already computed the four-tuple hash
and stored it in skb->hash --the common case for servers--, That type
field is the only reason to have to compute again.

The problem is there's no guarantee that the packet comes from the NIC,
it could be a simple VM2VM or host2VM packet.

And even if the packet is coming from the NIC that calculates the hash
there's no guarantee that it's the has that guest want (guest may use
different RSS keys).

Ah yes, of course.

I would still revisit the need to store a detailed hash_type along with
the hash, as as far I can tell that conveys no actionable information
to the guest.

Yes, need to figure out its usage. According to [1], it only mention
that storing has type is a charge of driver. Maybe Yuri can answer this.

For the case of Windows VM we can't know how exactly the network stack
uses provided hash data (including hash type). But: different releases
of Windows
enable different hash types (for example UDP hash is enabled only on
Server 2016 and up).

Indeed the Windows requires a little more from the network adapter/driver
than Linux does.

The addition of RSS support to virtio specification takes in account
the widest set of
requirements (i.e. Windows one), our initial impression is that this
should be enough also for Linux.

The NDIS specification in part of RSS is _mandatory_ and there are
certification tests
that check that the driver provides the hash data as expected. All the
high-performance
network adapters have such RSS functionality in the hardware.

Thanks for the context.

If Windows requires the driver to pass the hash-type along with the
hash data, then indeed this will be needed.

If it only requires the device to support a subset of of the possible
types, chosen at init, that would be different and it would be cheaper
for the driver to pass this config to the device one time.

quoted

quoted

With pre-RSS QEMU (i.e. where the virtio-net device does not indicate
the RSS support)
the virtio-net driver for Windows does all the job related to RSS:
- hash calculation
- hash/hash_type delivery
- reporting each packet on the correct CPU according to RSS settings

With RSS support in QEMU all the packets always come on a proper CPU and
the driver never needs to reschedule them. The driver still need to
calculate the
hash and report it to Windows. In this case we do the same job twice: the device
(QEMU or eBPF) does calculate the hash and get proper queue/CPU to deliver
the packet. But the hash is not delivered by the device, so the driver needs to
recalculate it and report to the Windows.

If we add HASH_REPORT support (current set of patches) and the device
indicates this
feature we can avoid hash recalculation in the driver assuming we
receive the correct hash
value and hash type. Otherwise the driver can't know which exactly
hash the device has calculated.

Please let me know if I did not answer the question.

I think I get you. The hash type is also a kind of classification (e.g
TCP or UDP). Any possibility that it can be deduced from the driver? (Or
it could be too expensive to do that).

The driver does it today (when the device does not offer any features)
and of course can continue doing it.
IMO if the device can't report the data according to the spec it
should not indicate support for the respective feature (or fallback to
vhost=off).
Again, IMO if Linux does not need the exact hash_type we can use (for
Linux) the way that Willem de Brujin suggested in his patchset:
- just add VIRTIO_NET_HASH_REPORT_L4 to the spec
- Linux can use MQ + hash delivery (and use VIRTIO_NET_HASH_REPORT_L4)
- Linux can use (if makes sense) RSS with VIRTIO_NET_HASH_REPORT_L4 and eBPF
- Windows gets what it needs + eBPF
So, everyone has what they need at the respective cost.

Regarding use of skb->cb for hash type:
Currently, if I'm not mistaken, there are 2 bytes at the end of skb->cb:
skb->cb is 48 bytes array
There is skb_gso_cb (14 bytes) at offset SKB_GSO_CB_OFFSET(32)
Is it possible to use one of these 2 bytes for hash_type?
If yes, shall we extend the skb_gso_cb and place the 1-bytes hash_type
in it or just emit compilation error if the skb_gso_cb grows beyond 15
bytes?

Good catch on segmentation taking place between .ndo_select_queue and
.ndo_start_xmit.

That also means that whatever field in the skb is used, has to be
copied to all segments in skb_segment. Which happens for cb. But this
feature is completely unrelated to the skb_gso_cb type. Perhaps
another field with a real type is more clear. For instance, an
extension to the union with napi_id and sender_cpu, as neither is used
in this egress path with .ndo_select_queue?
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization