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

[Willem de Bruijn]

On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  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?