| Message ID | 20220208181510.787069-5-andrew@daynix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | RSS support for VirtioNet. | expand |
On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.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@daynix.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?
Hi all, On Tue, Feb 8, 2022 at 10:59 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.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? I'll rephrase it in next version of patches. The idea is that VirtioNet RSS doesn't distinguish IP hashes between TCP and UDP. For TCP and UDP it's possible to set IP+PORT hashes. But disabling IP hashes will disable them for TCP and UDP simultaneously. It's possible to set IP+PORT for TCP and IP for everything else(UDP, ICMP etc.) > > > 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@daynix.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? I think "hash_types_saved" is more suitable for the current field. Idea is that the user may disable RSS/HASH and we need to save what hash type configurations previously were enabled. So, we can restore it when the user will enable RSS/HASH back. > > > +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 There used to be "is_l3", but that variable was used only in that condition statement. So I've decided to inplace it. > > > + 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? Not really, we already made all checks against "supported" hash types. Also, the commit function may not be called if RSS is disabled by the user.
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; /* Has control virtqueue */ bool has_cvq; @@ -2272,6 +2273,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi) int i = 0; vi->ctrl->rss.hash_types = vi->rss_hash_types_supported; + vi->rss_hash_types_saved = vi->rss_hash_types_supported; vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1; vi->ctrl->rss.unclassified_queue = 0; @@ -2286,6 +2288,121 @@ static void virtnet_init_default_rss(struct virtnet_info *vi) netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size); } +static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info) +{ + info->data = 0; + switch (info->flow_type) { + case TCP_V4_FLOW: + if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) { + info->data = RXH_IP_SRC | RXH_IP_DST | + RXH_L4_B_0_1 | RXH_L4_B_2_3; + } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) { + info->data = RXH_IP_SRC | RXH_IP_DST; + } + break; + case TCP_V6_FLOW: + if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) { + info->data = RXH_IP_SRC | RXH_IP_DST | + RXH_L4_B_0_1 | RXH_L4_B_2_3; + } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) { + info->data = RXH_IP_SRC | RXH_IP_DST; + } + break; + case UDP_V4_FLOW: + if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) { + info->data = RXH_IP_SRC | RXH_IP_DST | + RXH_L4_B_0_1 | RXH_L4_B_2_3; + } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) { + info->data = RXH_IP_SRC | RXH_IP_DST; + } + break; + case UDP_V6_FLOW: + if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) { + info->data = RXH_IP_SRC | RXH_IP_DST | + RXH_L4_B_0_1 | RXH_L4_B_2_3; + } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) { + info->data = RXH_IP_SRC | RXH_IP_DST; + } + break; + case IPV4_FLOW: + if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) + info->data = RXH_IP_SRC | RXH_IP_DST; + + break; + case IPV6_FLOW: + if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) + info->data = RXH_IP_SRC | RXH_IP_DST; + + break; + default: + info->data = 0; + break; + } +} + +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)) + 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; + vi->ctrl->rss.hash_types = vi->rss_hash_types_saved; + if (vi->dev->features & NETIF_F_RXHASH) + return virtnet_commit_rss_command(vi); + } + + return true; +} static void virtnet_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info) @@ -2571,6 +2688,27 @@ static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, switch (info->cmd) { case ETHTOOL_GRXRINGS: info->data = vi->curr_queue_pairs; + break; + case ETHTOOL_GRXFH: + virtnet_get_hashflow(vi, info); + break; + default: + rc = -EOPNOTSUPP; + } + + return rc; +} + +static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info) +{ + struct virtnet_info *vi = netdev_priv(dev); + int rc = 0; + + switch (info->cmd) { + case ETHTOOL_SRXFH: + if (!virtnet_set_hashflow(vi, info)) + rc = -EINVAL; + break; default: rc = -EOPNOTSUPP; @@ -2599,6 +2737,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = { .get_rxfh = virtnet_get_rxfh, .set_rxfh = virtnet_set_rxfh, .get_rxnfc = virtnet_get_rxnfc, + .set_rxnfc = virtnet_set_rxnfc, }; static void virtnet_freeze_down(struct virtio_device *vdev) @@ -2853,7 +2992,7 @@ static int virtnet_set_features(struct net_device *dev, if ((dev->features ^ features) & NETIF_F_RXHASH) { if (features & NETIF_F_RXHASH) - vi->ctrl->rss.hash_types = vi->rss_hash_types_supported; + vi->ctrl->rss.hash_types = vi->rss_hash_types_saved; else vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
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. 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@daynix.com> --- drivers/net/virtio_net.c | 141 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 140 insertions(+), 1 deletion(-)