[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACGkMEufffSj1GQMqwf598__-JgNtXRpyvsLtjSbr3angLmJXg@mail.gmail.com>
Date: Tue, 3 Jun 2025 11:19:03 +0800
From: Jason Wang <jasowang@...hat.com>
To: Akihiko Odaki <akihiko.odaki@...nix.com>
Cc: Jonathan Corbet <corbet@....net>, Willem de Bruijn <willemdebruijn.kernel@...il.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
"Michael S. Tsirkin" <mst@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Shuah Khan <shuah@...nel.org>, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, kvm@...r.kernel.org,
virtualization@...ts.linux-foundation.org, linux-kselftest@...r.kernel.org,
Yuri Benditovich <yuri.benditovich@...nix.com>, Andrew Melnychenko <andrew@...nix.com>,
Stephen Hemminger <stephen@...workplumber.org>, gur.stavi@...wei.com,
Lei Yang <leiyang@...hat.com>, Simon Horman <horms@...nel.org>
Subject: Re: [PATCH net-next v12 01/10] virtio_net: Add functions for hashing
On Fri, May 30, 2025 at 12:50 PM Akihiko Odaki <akihiko.odaki@...nix.com> wrote:
>
> They are useful to implement VIRTIO_NET_F_RSS and
> VIRTIO_NET_F_HASH_REPORT.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@...nix.com>
> Tested-by: Lei Yang <leiyang@...hat.com>
> ---
> include/linux/virtio_net.h | 188 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 188 insertions(+)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 02a9f4dc594d..426f33b4b824 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -9,6 +9,194 @@
> #include <uapi/linux/tcp.h>
> #include <uapi/linux/virtio_net.h>
>
> +struct virtio_net_hash {
> + u32 value;
> + u16 report;
> +};
> +
> +struct virtio_net_toeplitz_state {
> + u32 hash;
> + const u32 *key;
> +};
> +
> +#define VIRTIO_NET_SUPPORTED_HASH_TYPES (VIRTIO_NET_RSS_HASH_TYPE_IPv4 | \
> + VIRTIO_NET_RSS_HASH_TYPE_TCPv4 | \
> + VIRTIO_NET_RSS_HASH_TYPE_UDPv4 | \
> + VIRTIO_NET_RSS_HASH_TYPE_IPv6 | \
> + VIRTIO_NET_RSS_HASH_TYPE_TCPv6 | \
> + VIRTIO_NET_RSS_HASH_TYPE_UDPv6)
> +
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
> +
> +static inline void virtio_net_toeplitz_convert_key(u32 *input, size_t len)
> +{
> + while (len >= sizeof(*input)) {
> + *input = be32_to_cpu((__force __be32)*input);
> + input++;
> + len -= sizeof(*input);
> + }
> +}
> +
> +static inline void virtio_net_toeplitz_calc(struct virtio_net_toeplitz_state *state,
> + const __be32 *input, size_t len)
> +{
> + while (len >= sizeof(*input)) {
> + for (u32 map = be32_to_cpu(*input); map; map &= (map - 1)) {
> + u32 i = ffs(map);
> +
> + state->hash ^= state->key[0] << (32 - i) |
> + (u32)((u64)state->key[1] >> i);
> + }
> +
> + state->key++;
> + input++;
> + len -= sizeof(*input);
> + }
> +}
> +
> +static inline u8 virtio_net_hash_key_length(u32 types)
> +{
> + size_t len = 0;
> +
> + if (types & VIRTIO_NET_HASH_REPORT_IPv4)
> + len = max(len,
> + sizeof(struct flow_dissector_key_ipv4_addrs));
> +
> + if (types &
> + (VIRTIO_NET_HASH_REPORT_TCPv4 | VIRTIO_NET_HASH_REPORT_UDPv4))
> + len = max(len,
> + sizeof(struct flow_dissector_key_ipv4_addrs) +
> + sizeof(struct flow_dissector_key_ports));
> +
> + if (types & VIRTIO_NET_HASH_REPORT_IPv6)
> + len = max(len,
> + sizeof(struct flow_dissector_key_ipv6_addrs));
> +
> + if (types &
> + (VIRTIO_NET_HASH_REPORT_TCPv6 | VIRTIO_NET_HASH_REPORT_UDPv6))
> + len = max(len,
> + sizeof(struct flow_dissector_key_ipv6_addrs) +
> + sizeof(struct flow_dissector_key_ports));
> +
> + return len + sizeof(u32);
> +}
> +
> +static inline u32 virtio_net_hash_report(u32 types,
> + const struct flow_keys_basic *keys)
> +{
> + switch (keys->basic.n_proto) {
> + case cpu_to_be16(ETH_P_IP):
> + if (!(keys->control.flags & FLOW_DIS_IS_FRAGMENT)) {
> + if (keys->basic.ip_proto == IPPROTO_TCP &&
> + (types & VIRTIO_NET_RSS_HASH_TYPE_TCPv4))
> + return VIRTIO_NET_HASH_REPORT_TCPv4;
> +
> + if (keys->basic.ip_proto == IPPROTO_UDP &&
> + (types & VIRTIO_NET_RSS_HASH_TYPE_UDPv4))
> + return VIRTIO_NET_HASH_REPORT_UDPv4;
> + }
> +
> + if (types & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> + return VIRTIO_NET_HASH_REPORT_IPv4;
> +
> + return VIRTIO_NET_HASH_REPORT_NONE;
> +
> + case cpu_to_be16(ETH_P_IPV6):
> + if (!(keys->control.flags & FLOW_DIS_IS_FRAGMENT)) {
> + if (keys->basic.ip_proto == IPPROTO_TCP &&
> + (types & VIRTIO_NET_RSS_HASH_TYPE_TCPv6))
> + return VIRTIO_NET_HASH_REPORT_TCPv6;
> +
> + if (keys->basic.ip_proto == IPPROTO_UDP &&
> + (types & VIRTIO_NET_RSS_HASH_TYPE_UDPv6))
> + return VIRTIO_NET_HASH_REPORT_UDPv6;
> + }
> +
> + if (types & VIRTIO_NET_RSS_HASH_TYPE_IPv6)
> + return VIRTIO_NET_HASH_REPORT_IPv6;
> +
> + return VIRTIO_NET_HASH_REPORT_NONE;
> +
> + default:
> + return VIRTIO_NET_HASH_REPORT_NONE;
> + }
> +}
> +
> +static inline void virtio_net_hash_rss(const struct sk_buff *skb,
> + u32 types, const u32 *key,
> + struct virtio_net_hash *hash)
> +{
> + struct virtio_net_toeplitz_state toeplitz_state = { .key = key };
> + struct flow_keys flow;
> + struct flow_keys_basic flow_basic;
> + u16 report;
> +
> + if (!skb_flow_dissect_flow_keys(skb, &flow, 0)) {
> + hash->report = VIRTIO_NET_HASH_REPORT_NONE;
> + return;
> + }
> +
> + flow_basic = (struct flow_keys_basic) {
> + .control = flow.control,
> + .basic = flow.basic
> + };
> +
> + report = virtio_net_hash_report(types, &flow_basic);
> +
> + switch (report) {
> + case VIRTIO_NET_HASH_REPORT_IPv4:
> + virtio_net_toeplitz_calc(&toeplitz_state,
> + (__be32 *)&flow.addrs.v4addrs,
> + sizeof(flow.addrs.v4addrs));
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_TCPv4:
> + virtio_net_toeplitz_calc(&toeplitz_state,
> + (__be32 *)&flow.addrs.v4addrs,
> + sizeof(flow.addrs.v4addrs));
> + virtio_net_toeplitz_calc(&toeplitz_state, &flow.ports.ports,
> + sizeof(flow.ports.ports));
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_UDPv4:
> + virtio_net_toeplitz_calc(&toeplitz_state,
> + (__be32 *)&flow.addrs.v4addrs,
> + sizeof(flow.addrs.v4addrs));
> + virtio_net_toeplitz_calc(&toeplitz_state, &flow.ports.ports,
> + sizeof(flow.ports.ports));
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_IPv6:
> + virtio_net_toeplitz_calc(&toeplitz_state,
> + (__be32 *)&flow.addrs.v6addrs,
> + sizeof(flow.addrs.v6addrs));
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_TCPv6:
> + virtio_net_toeplitz_calc(&toeplitz_state,
> + (__be32 *)&flow.addrs.v6addrs,
> + sizeof(flow.addrs.v6addrs));
> + virtio_net_toeplitz_calc(&toeplitz_state, &flow.ports.ports,
> + sizeof(flow.ports.ports));
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_UDPv6:
> + virtio_net_toeplitz_calc(&toeplitz_state,
> + (__be32 *)&flow.addrs.v6addrs,
> + sizeof(flow.addrs.v6addrs));
> + virtio_net_toeplitz_calc(&toeplitz_state, &flow.ports.ports,
> + sizeof(flow.ports.ports));
> + break;
> +
> + default:
> + hash->report = VIRTIO_NET_HASH_REPORT_NONE;
> + return;
So I still think we need a comment here to explain why this is not an
issue if the device can report HASH_XXX_EX. Or we need to add the
support, since this is the code from the driver side, I don't think we
need to worry about the device implementation issues.
For the issue of the number of options, does the spec forbid fallback
to VIRTIO_NET_HASH_REPORT_NONE? If not, we can do that.
Thanks
> + }
> +
> + hash->value = toeplitz_state.hash;
> + hash->report = report;
> +}
> +
> static inline bool virtio_net_hdr_match_proto(__be16 protocol, __u8 gso_type)
> {
> switch (gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
>
> --
> 2.49.0
>
Powered by blists - more mailing lists