* [PATCH v2 net-next] ethtool: Remove inactive code snippet in ethtool_set_rxfh_indir
@ 2014-02-19 7:50 Venkat Duvvuru
2014-02-19 21:42 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Venkat Duvvuru @ 2014-02-19 7:50 UTC (permalink / raw)
To: netdev; +Cc: Venkat Duvvuru
ethtool_rxfh_indir_default is invoked inside ethtool_set_rxfh_indir when
user_size is zero. However user_size can only be zero when dev_size is
zero in which case ethtool_get_rxfh_indir itself would return
EOPNOTSUPP error. "user_size = 0" can never happen unless userspace
ethtool sends user_size as zero beforing invoking the ioctl for
ETHTOOL_SRSSH, in which case we should return -EINVAL.
v2: Modified commit message.
Signed-off-by: Venkat Duvvuru <VenkatKumar.Duvvuru@Emulex.com>
---
net/core/ethtool.c | 33 ++++++++++++++-------------------
1 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 30071de..9d96516 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -626,7 +626,7 @@ static noinline_for_stack int ethtool_set_rxfh_indir(struct net_device *dev,
sizeof(user_size)))
return -EFAULT;
- if (user_size != 0 && user_size != dev_size)
+ if (!user_size || user_size != dev_size)
return -EINVAL;
indir = kcalloc(dev_size, sizeof(indir[0]), GFP_USER);
@@ -638,25 +638,20 @@ static noinline_for_stack int ethtool_set_rxfh_indir(struct net_device *dev,
if (ret)
goto out;
- if (user_size == 0) {
- for (i = 0; i < dev_size; i++)
- indir[i] = ethtool_rxfh_indir_default(i, rx_rings.data);
- } else {
- if (copy_from_user(indir,
- useraddr +
- offsetof(struct ethtool_rxfh_indir,
- ring_index[0]),
- dev_size * sizeof(indir[0]))) {
- ret = -EFAULT;
- goto out;
- }
+ if (copy_from_user(indir,
+ useraddr +
+ offsetof(struct ethtool_rxfh_indir,
+ ring_index[0]),
+ dev_size * sizeof(indir[0]))) {
+ ret = -EFAULT;
+ goto out;
+ }
- /* Validate ring indices */
- for (i = 0; i < dev_size; i++) {
- if (indir[i] >= rx_rings.data) {
- ret = -EINVAL;
- goto out;
- }
+ /* Validate ring indices */
+ for (i = 0; i < dev_size; i++) {
+ if (indir[i] >= rx_rings.data) {
+ ret = -EINVAL;
+ goto out;
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next] ethtool: Remove inactive code snippet in ethtool_set_rxfh_indir
2014-02-19 7:50 [PATCH v2 net-next] ethtool: Remove inactive code snippet in ethtool_set_rxfh_indir Venkat Duvvuru
@ 2014-02-19 21:42 ` David Miller
2014-02-21 10:28 ` Venkata Duvvuru
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2014-02-19 21:42 UTC (permalink / raw)
To: VenkatKumar.Duvvuru; +Cc: netdev
From: Venkat Duvvuru <VenkatKumar.Duvvuru@Emulex.com>
Date: Wed, 19 Feb 2014 13:20:25 +0530
> ethtool_rxfh_indir_default is invoked inside ethtool_set_rxfh_indir when
> user_size is zero. However user_size can only be zero when dev_size is
> zero in which case ethtool_get_rxfh_indir itself would return
> EOPNOTSUPP error. "user_size = 0" can never happen unless userspace
> ethtool sends user_size as zero beforing invoking the ioctl for
> ETHTOOL_SRSSH, in which case we should return -EINVAL.
>
> v2: Modified commit message.
>
> Signed-off-by: Venkat Duvvuru <VenkatKumar.Duvvuru@Emulex.com>
It looks like the author of this code intended user_size == 0 as a way
to reset the indirection table to the default, did this ever work?
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2 net-next] ethtool: Remove inactive code snippet in ethtool_set_rxfh_indir
2014-02-19 21:42 ` David Miller
@ 2014-02-21 10:28 ` Venkata Duvvuru
2014-02-21 17:14 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Venkata Duvvuru @ 2014-02-21 10:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
> From: Venkat Duvvuru <VenkatKumar.Duvvuru@Emulex.com>
> Date: Wed, 19 Feb 2014 13:20:25 +0530
>
> > ethtool_rxfh_indir_default is invoked inside ethtool_set_rxfh_indir
> > when user_size is zero. However user_size can only be zero when
> > dev_size is zero in which case ethtool_get_rxfh_indir itself would
> > return EOPNOTSUPP error. "user_size = 0" can never happen unless
> > userspace ethtool sends user_size as zero beforing invoking the ioctl
> > for ETHTOOL_SRSSH, in which case we should return -EINVAL.
> >
> > v2: Modified commit message.
> >
> > Signed-off-by: Venkat Duvvuru <VenkatKumar.Duvvuru@Emulex.com>
>
> It looks like the author of this code intended user_size == 0 as a way to reset
> the indirection table to the default, did this ever work?
There is no option for --set-rxfh-indir to reset the indirection table. So ethtool cannot know that it has to fill user_size = 0 to reset the indirection table to default.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next] ethtool: Remove inactive code snippet in ethtool_set_rxfh_indir
2014-02-21 10:28 ` Venkata Duvvuru
@ 2014-02-21 17:14 ` David Miller
2014-02-21 20:19 ` Ben Hutchings
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2014-02-21 17:14 UTC (permalink / raw)
To: VenkatKumar.Duvvuru; +Cc: netdev
From: Venkata Duvvuru <VenkatKumar.Duvvuru@Emulex.Com>
Date: Fri, 21 Feb 2014 10:28:24 +0000
> There is no option for --set-rxfh-indir to reset the indirection table.
But this can be easily added to ethtool.
My point is that the intention is _clearly_ there to provide this
facility.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next] ethtool: Remove inactive code snippet in ethtool_set_rxfh_indir
2014-02-21 17:14 ` David Miller
@ 2014-02-21 20:19 ` Ben Hutchings
0 siblings, 0 replies; 5+ messages in thread
From: Ben Hutchings @ 2014-02-21 20:19 UTC (permalink / raw)
To: David Miller; +Cc: VenkatKumar.Duvvuru, netdev
[-- Attachment #1: Type: text/plain, Size: 934 bytes --]
On Fri, 2014-02-21 at 12:14 -0500, David Miller wrote:
> From: Venkata Duvvuru <VenkatKumar.Duvvuru@Emulex.Com>
> Date: Fri, 21 Feb 2014 10:28:24 +0000
>
> > There is no option for --set-rxfh-indir to reset the indirection table.
>
> But this can be easily added to ethtool.
>
> My point is that the intention is _clearly_ there to provide this
> facility.
This is indeed intentional, and was added by:
commit 278bc4296bd64ffd1d3913b487dc8a520e423a7a
Author: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu Dec 15 13:56:49 2011 +0000
ethtool: Define and apply a default policy for RX flow hash indirection
I can't remember whether I wrote a patch for ethtool to use this feature
but I must have tested it somehow... The ethtool command isn't the only
user of the ethtool API, in any case.
Ben.
--
Ben Hutchings
A free society is one where it is safe to be unpopular. - Adlai Stevenson
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-02-21 20:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-19 7:50 [PATCH v2 net-next] ethtool: Remove inactive code snippet in ethtool_set_rxfh_indir Venkat Duvvuru
2014-02-19 21:42 ` David Miller
2014-02-21 10:28 ` Venkata Duvvuru
2014-02-21 17:14 ` David Miller
2014-02-21 20:19 ` Ben Hutchings
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).