netdev - Re: [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+FuTScRp5hhkvETuVRsUxMRCZVU0wVrmd5_=a5UoKNLDv4LnA@mail.gmail.com>
Date:   Tue, 8 Feb 2022 15:58:27 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Andrew Melnychenko <andrew@...nix.com>
Cc:     netdev@...r.kernel.org, virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        jasowang@...hat.com, mst@...hat.com, yan@...nix.com,
        yuri.benditovich@...nix.com
Subject: Re: [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.

On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@...nix.com> wrote:
>
> Now it's possible to control supported hashflows.
> Added hashflow set/get callbacks.
> Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.

I don't follow this comment. Can you elaborate?

> TCP and UDP supports only:
> ethtool -U eth0 rx-flow-hash tcp4 sd
>     RXH_IP_SRC + RXH_IP_DST
> ethtool -U eth0 rx-flow-hash tcp4 sdfn
>     RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
>
> Signed-off-by: Andrew Melnychenko <andrew@...nix.com>
> ---
>  drivers/net/virtio_net.c | 141 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 140 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 543da2fbdd2d..88759d5e693c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -231,6 +231,7 @@ struct virtnet_info {
>         u8 rss_key_size;
>         u16 rss_indir_table_size;
>         u32 rss_hash_types_supported;
> +       u32 rss_hash_types_saved;

hash_types_active?

> +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> +{
> +       u32 new_hashtypes = vi->rss_hash_types_saved;
> +       bool is_disable = info->data & RXH_DISCARD;
> +       bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3);
> +
> +       /* supports only 'sd', 'sdfn' and 'r' */
> +       if (!((info->data == (RXH_IP_SRC | RXH_IP_DST)) | is_l4 | is_disable))

maybe add an is_l3

> +               return false;
> +
> +       switch (info->flow_type) {
> +       case TCP_V4_FLOW:
> +               new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_TCPv4);
> +               if (!is_disable)
> +                       new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> +                               | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv4 : 0);
> +               break;
> +       case UDP_V4_FLOW:
> +               new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_UDPv4);
> +               if (!is_disable)
> +                       new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> +                               | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv4 : 0);
> +               break;
> +       case IPV4_FLOW:
> +               new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> +               if (!is_disable)
> +                       new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> +               break;
> +       case TCP_V6_FLOW:
> +               new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_TCPv6);
> +               if (!is_disable)
> +                       new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> +                               | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv6 : 0);
> +               break;
> +       case UDP_V6_FLOW:
> +               new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_UDPv6);
> +               if (!is_disable)
> +                       new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> +                               | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv6 : 0);
> +               break;
> +       case IPV6_FLOW:
> +               new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> +               if (!is_disable)
> +                       new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> +               break;
> +       default:
> +               /* unsupported flow */
> +               return false;
> +       }
> +
> +       /* if unsupported hashtype was set */
> +       if (new_hashtypes != (new_hashtypes & vi->rss_hash_types_supported))
> +               return false;
> +
> +       if (new_hashtypes != vi->rss_hash_types_saved) {
> +               vi->rss_hash_types_saved = new_hashtypes;

should only be updated if the commit function returned success?