lkml.org 
[lkml]   [2024]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH net v2 1/2] virtio_net: Do not set rss_indir if RSS is not supported
> > On Wed, Mar 27, 2024 at 09:37:43AM +0800, Xuan Zhuo wrote:
> > > On Tue, 26 Mar 2024 08:19:08 -0700, Breno Leitao <leitao@debian.org> wrote:
> > > > Do not set virtnet_info->rss_indir_table_size if RSS is not available
> > > > for the device.
> > > >
> > > > Currently, rss_indir_table_size is set if either has_rss or
> > > > has_rss_hash_report is available, but, it should only be set if has_rss
> > > > is set.
> > > >
> > > > On the virtnet_set_rxfh(), return an invalid command if the request has
> > > > indirection table set, but virtnet does not support RSS.
> > > >
> > > > Suggested-by: Heng Qi <hengqi@linux.alibaba.com>
> > > > Signed-off-by: Breno Leitao <leitao@debian.org>
> > > > ---
> > > > drivers/net/virtio_net.c | 9 +++++++--
> > > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index c22d1118a133..c640fdf28fc5 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -3813,6 +3813,9 @@ static int virtnet_set_rxfh(struct net_device *dev,
> > > > rxfh->hfunc != ETH_RSS_HASH_TOP)
> > > > return -EOPNOTSUPP;
> > > >
> > > > + if (rxfh->indir && !vi->has_rss)
> > > > + return -EINVAL;
> > > > +
> > > > if (rxfh->indir) {
> > >
> > > Put !vi->has_rss here?
> >
> > I am not sure I understand the suggestion. Where do you suggest we have
> > !vi->has_rss?
> >
> > If we got this far, we either have:
> >
> > a) rxfh->indir set and vi->has_rss is also set
> > b) rxfh->indir not set. (vi->has_rss could be set or not).
>
>
> This function does two tasks.
> 1. update indir table
> 2. update rss key
>
> #1 only for has_rss
> #2 for has_rss or has_rss_hash_report
>
>
> So I would code:
>
> bool update = false
>
> if (rxfh->indir) {
> if (!vi->has_rss)
> return -EINVAL;
>
> for (i = 0; i < vi->rss_indir_table_size; ++i)
> vi->ctrl->rss.indirection_table[i] = rxfh->indir[i];
>
> update = true
> }
>
> if (rxfh->key) {
> if (!vi->has_rss && !vi->has_rss_hash_report)
> return -EINVAL;
>
> memcpy(vi->ctrl->rss.key, rxfh->key, vi->rss_key_size);
> update = true
> }
>
>
> if (update)
> virtnet_commit_rss_command(vi);
>
> Thanks.

This is a bit different from the previous proposal, but, I like this one
approach better. It makes the code easier to read.

How does the full patch looks like?


diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c22d1118a133..180f342f1898 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3807,20 +3807,35 @@ static int virtnet_set_rxfh(struct net_device *dev,
struct netlink_ext_ack *extack)
{
struct virtnet_info *vi = netdev_priv(dev);
+ bool update = false;
int i;

+ if (!vi->has_rss && !vi->has_rss_hash_report)
+ return -EOPNOTSUPP;
+
if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
rxfh->hfunc != ETH_RSS_HASH_TOP)
return -EOPNOTSUPP;

if (rxfh->indir) {
+ if (!vi->has_rss)
+ return -EINVAL;
+
for (i = 0; i < vi->rss_indir_table_size; ++i)
vi->ctrl->rss.indirection_table[i] = rxfh->indir[i];
+ update = true;
}
- if (rxfh->key)
+
+ if (rxfh->key) {
+ if (!vi->has_rss && !vi->has_rss_hash_report)
+ return -EINVAL;
+
memcpy(vi->ctrl->rss.key, rxfh->key, vi->rss_key_size);
+ update = true;
+ }

- virtnet_commit_rss_command(vi);
+ if (update)
+ virtnet_commit_rss_command(vi);

return 0;
}
@@ -4729,13 +4744,15 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
vi->has_rss_hash_report = true;

- if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
vi->has_rss = true;

- if (vi->has_rss || vi->has_rss_hash_report) {
vi->rss_indir_table_size =
virtio_cread16(vdev, offsetof(struct virtio_net_config,
rss_max_indirection_table_length));
+ }
+
+ if (vi->has_rss || vi->has_rss_hash_report) {
vi->rss_key_size =
virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));

\
 
 \ /
  Last update: 2024-03-28 15:37    [W:0.075 / U:0.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site