From: Willem de Bruijn <email@example.com> To: Yuri Benditovich <firstname.lastname@example.org> Cc: "David S. Miller" <email@example.com>, Jakub Kicinski <firstname.lastname@example.org>, "Michael S . Tsirkin" <email@example.com>, Jason Wang <firstname.lastname@example.org>, Alexei Starovoitov <email@example.com>, Daniel Borkmann <firstname.lastname@example.org>, Andrii Nakryiko <email@example.com>, Martin KaFai Lau <firstname.lastname@example.org>, Song Liu <email@example.com>, Yonghong Song <firstname.lastname@example.org>, John Fastabend <email@example.com>, KP Singh <firstname.lastname@example.org>, email@example.com, "Gustavo A . R . Silva" <firstname.lastname@example.org>, Herbert Xu <email@example.com>, Steffen Klassert <firstname.lastname@example.org>, Pablo Neira Ayuso <email@example.com>, firstname.lastname@example.org, email@example.com, Jakub Sitnicki <firstname.lastname@example.org>, Marco Elver <email@example.com>, Paolo Abeni <firstname.lastname@example.org>, Network Development <email@example.com>, linux-kernel <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, bpf <email@example.com>, Yan Vugenfirer <firstname.lastname@example.org> Subject: Re: [RFC PATCH 0/7] Support for virtio-net hash reporting Date: Tue, 12 Jan 2021 18:47:12 -0500 [thread overview] Message-ID: <CA+FuTSfhBZfEf8+LKNUJQpSxt8c5h1wMpARupekqFKuei6YBsA@mail.gmail.com> (raw) In-Reply-To: <CAOEp5Oc5qif_krU8oC6qhq6X0xRW-9GpWrBzWgPw0WevyhT8Mg@mail.gmail.com> On Tue, Jan 12, 2021 at 3:29 PM Yuri Benditovich <email@example.com> wrote: > > On Tue, Jan 12, 2021 at 9:49 PM Yuri Benditovich > <firstname.lastname@example.org> wrote: > > > > On Tue, Jan 12, 2021 at 9:41 PM Yuri Benditovich > > <email@example.com> wrote: > > > > > > Existing TUN module is able to use provided "steering eBPF" to > > > calculate per-packet hash and derive the destination queue to > > > place the packet to. The eBPF uses mapped configuration data > > > containing a key for hash calculation and indirection table > > > with array of queues' indices. > > > > > > This series of patches adds support for virtio-net hash reporting > > > feature as defined in virtio specification. It extends the TUN module > > > and the "steering eBPF" as follows: > > > > > > Extended steering eBPF calculates the hash value and hash type, keeps > > > hash value in the skb->hash and returns index of destination virtqueue > > > and the type of the hash. TUN module keeps returned hash type in > > > (currently unused) field of the skb. > > > skb->__unused renamed to 'hash_report_type'. > > > > > > When TUN module is called later to allocate and fill the virtio-net > > > header and push it to destination virtqueue it populates the hash > > > and the hash type into virtio-net header. > > > > > > VHOST driver is made aware of respective virtio-net feature that > > > extends the virtio-net header to report the hash value and hash report > > > type. > > > > Comment from Willem de Bruijn: > > > > Skbuff fields are in short supply. I don't think we need to add one > > just for this narrow path entirely internal to the tun device. > > > > We understand that and try to minimize the impact by using an already > existing unused field of skb. Not anymore. It was repurposed as a flags field very recently. This use case is also very narrow in scope. And a very short path from data producer to consumer. So I don't think it needs to claim scarce bits in the skb. tun_ebpf_select_queue stores the field, tun_put_user reads it and converts it to the virtio_net_hdr in the descriptor. tun_ebpf_select_queue is called from .ndo_select_queue. Storing the field in skb->cb is fragile, as in theory some code could overwrite that between field between ndo_select_queue and ndo_start_xmit/tun_net_xmit, from which point it is fully under tun control again. But in practice, I don't believe anything does. Alternatively an existing skb field that is used only on disjoint datapaths, such as ingress-only, could be viable. > > Instead, you could just run the flow_dissector in tun_put_user if the > > feature is negotiated. Indeed, the flow dissector seems more apt to me > > than BPF here. Note that the flow dissector internally can be > > overridden by a BPF program if the admin so chooses. > > > When this set of patches is related to hash delivery in the virtio-net > packet in general, > it was prepared in context of RSS feature implementation as defined in > virtio spec  > In case of RSS it is not enough to run the flow_dissector in tun_put_user: > in tun_ebpf_select_queue the TUN calls eBPF to calculate the hash, > hash type and queue index > according to the (mapped) parameters (key, hash types, indirection > table) received from the guest. TUNSETSTEERINGEBPF was added to support more diverse queue selection than the default in case of multiqueue tun. Not sure what the exact use cases are. But RSS is exactly the purpose of the flow dissector. It is used for that purpose in the software variant RPS. The flow dissector implements a superset of the RSS spec, and certainly computes a four-tuple for TCP/IPv6. In the case of RPS, it is skipped if the NIC has already computed a 4-tuple hash. 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. > Our intention is to keep the hash and hash type in the skb to populate them > into a virtio-net header later in tun_put_user. > Note that in this case the type of calculated hash is selected not > only from flow dissections > but also from limitations provided by the guest. > > This is already implemented in qemu (for case of vhost=off), see  > (virtio_net_process_rss) > For case of vhost=on there are WIP for qemu to load eBPF and attach it to TUN. > Note that exact way of selecting rx virtqueue depends on the guest, > it could be automatic steering (typical for Linux VM), RSS (typical > for Windows VM) or > any other steering mechanism implemented in loadable TUN steering BPF with > or without hash calculation. > >  https://github.com/oasis-tcs/virtio-spec/blob/master/content.tex#L3740 >  https://github.com/qemu/qemu/blob/master/hw/net/virtio-net.c#L1591 > > > This also hits on a deeper point with the choice of hash values, that > > I also noticed in my RFC patchset to implement the inverse . It > > is much more detailed than skb->hash + skb->l4_hash currently offers, > > and that can be gotten for free from most hardware. > > Unfortunately in the case of RSS we can't get this hash from the hardware as > this requires configuration of the NIC's hardware with key and hash types for > Toeplitz hash calculation. I don't understand. Toeplitz hash calculation is enabled by default for multiqueue devices, and many devices will pass the toeplitz hash along for free to avoid software flow dissection. > > In most practical > > cases, that information suffices. I added less specific fields > > VIRTIO_NET_HASH_REPORT_L4, VIRTIO_NET_HASH_REPORT_OTHER that work > > without explicit flow dissection. I understand that the existing > > fields are part of the standard. Just curious, what is their purpose > > beyond 4-tuple based flow hashing? > > The hash is used in combination with the indirection table to select > destination rx virtqueue. > The hash and hash type are to be reported in virtio-net header, if requested. > For Windows VM - in case the device does not report the hash (even if > it calculated it to > schedule the packet to a proper queue), the driver must do that for each packet > (this is a certification requirement). I understand the basics of RSS. My question is what the hash-type is intended to be used for by the guest. It is part of the virtio spec, so this point is somewhat moot: it has to be passed along with the hash value now. But it is not entirely moot. If most users are satisfied with knowing whether a hash is L4 or not, we could add two new types VIRTIO_NET_HASH_TYPE_L4 and VIRTIO_NET_HASH_TYPE_OTHER. And then pass the existing skb->hash as is, likely computed by the NIC.  https://firstname.lastname@example.org/ > > > >  https://patchwork.kernel.org/project/netdevbpf/list/?series=406859&state=* > >  https://github.com/wdebruij/linux/commit/0f77febf22cd6ffc242a575807fa8382a26e511e > > > > > > Yuri Benditovich (7): > > > skbuff: define field for hash report type > > > vhost: support for hash report virtio-net feature > > > tun: allow use of BPF_PROG_TYPE_SCHED_CLS program type > > > tun: free bpf_program by bpf_prog_put instead of bpf_prog_destroy > > > tun: add ioctl code TUNSETHASHPOPULATION > > > tun: populate hash in virtio-net header when needed > > > tun: report new tun feature IFF_HASH > > > > > > drivers/net/tun.c | 43 +++++++++++++++++++++++++++++++------ > > > drivers/vhost/net.c | 37 ++++++++++++++++++++++++------- > > > include/linux/skbuff.h | 7 +++++- > > > include/uapi/linux/if_tun.h | 2 ++ > > > 4 files changed, 74 insertions(+), 15 deletions(-) > > > > > > -- > > > 2.17.1 > > >
next prev parent reply other threads:[~2021-01-13 0:50 UTC|newest] Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-12 19:41 Yuri Benditovich 2021-01-12 19:41 ` [RFC PATCH 1/7] skbuff: define field for hash report type Yuri Benditovich 2021-01-12 19:41 ` [RFC PATCH 2/7] vhost: support for hash report virtio-net feature Yuri Benditovich 2021-01-12 19:41 ` [RFC PATCH 3/7] tun: allow use of BPF_PROG_TYPE_SCHED_CLS program type Yuri Benditovich 2021-01-12 19:46 ` Alexei Starovoitov 2021-01-12 20:33 ` Yuri Benditovich 2021-01-12 20:40 ` Yuri Benditovich 2021-01-12 20:55 ` Yuri Benditovich 2021-01-18 9:16 ` Yuri Benditovich 2021-01-20 18:44 ` Alexei Starovoitov 2021-01-24 11:52 ` Yuri Benditovich 2021-01-12 19:41 ` [RFC PATCH 4/7] tun: free bpf_program by bpf_prog_put instead of bpf_prog_destroy Yuri Benditovich 2021-01-12 19:41 ` [RFC PATCH 5/7] tun: add ioctl code TUNSETHASHPOPULATION Yuri Benditovich 2021-01-12 19:41 ` [RFC PATCH 6/7] tun: populate hash in virtio-net header when needed Yuri Benditovich 2021-01-12 19:41 ` [RFC PATCH 7/7] tun: report new tun feature IFF_HASH Yuri Benditovich 2021-01-12 19:49 ` [RFC PATCH 0/7] Support for virtio-net hash reporting Yuri Benditovich 2021-01-12 20:28 ` Yuri Benditovich 2021-01-12 23:47 ` Willem de Bruijn [this message] 2021-01-13 4:05 ` Jason Wang 2021-01-13 14:33 ` Willem de Bruijn 2021-01-14 3:38 ` Jason Wang 2021-01-17 7:57 ` Yuri Benditovich 2021-01-18 2:46 ` Jason Wang 2021-01-18 9:09 ` Yuri Benditovich 2021-01-18 15:19 ` Willem de Bruijn
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CA+FuTSfhBZfEf8+LKNUJQpSxt8c5h1wMpARupekqFKuei6YBsA@mail.gmail.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [RFC PATCH 0/7] Support for virtio-net hash reporting' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).