| Message ID | 20240807173352.3501746-1-gal@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] ethtool: Fix context creation with no parameters | expand |
On 07/08/2024 18:33, Gal Pressman wrote: > The 'at least one change' requirement is not applicable for context > creation, skip the check in such case. > This allows a command such as 'ethtool -X eth0 context new' to work. > > The command works by mistake when using older versions of userspace > ethtool due to an incompatibility issue where rxfh.input_xfrm is passed > as zero (unset) instead of RXH_XFRM_NO_CHANGE as done with recent > userspace. This patch does not try to solve the incompatibility issue. > > Link: https://lore.kernel.org/netdev/05ae8316-d3aa-4356-98c6-55ed4253c8a7@nvidia.com/ > Fixes: 84a1d9c48200 ("net: ethtool: extend RXNFC API to support RSS spreading of filter matches") > Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com> > Reviewed-by: Jianbo Liu <jianbol@nvidia.com> > Signed-off-by: Gal Pressman <gal@nvidia.com> Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>
I'll make some minor modifications when applying.. On Wed, 7 Aug 2024 20:33:52 +0300 Gal Pressman wrote: > if ((rxfh.indir_size && > rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE && > rxfh.indir_size != dev_indir_size) || > - (rxfh.key_size && (rxfh.key_size != dev_key_size)) || > - (rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE && > + (rxfh.key_size && (rxfh.key_size != dev_key_size))) We should take this opportunity to remove the pointless brackets around key size comparison. Same clause for indir is not bracketed. > + return -EINVAL; > + > + /* Must request at least one change: indir size, hash key, function > + * or input transformation. > + * There's no need for any of it in case of context creation. > + */ > + if (!create && (rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE && > rxfh.key_size == 0 && rxfh.hfunc == ETH_RSS_HASH_NO_CHANGE && And here if (!create should probably be on a line of its own. Otherwise continuation lines don't align with the opening bracket.
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 8ca13208d240..118d69bc3c76 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -1369,15 +1369,17 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev, return -EOPNOTSUPP; create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC; - /* If either indir, hash key or function is valid, proceed further. - * Must request at least one change: indir size, hash key, function - * or input transformation. - */ if ((rxfh.indir_size && rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE && rxfh.indir_size != dev_indir_size) || - (rxfh.key_size && (rxfh.key_size != dev_key_size)) || - (rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE && + (rxfh.key_size && (rxfh.key_size != dev_key_size))) + return -EINVAL; + + /* Must request at least one change: indir size, hash key, function + * or input transformation. + * There's no need for any of it in case of context creation. + */ + if (!create && (rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE && rxfh.key_size == 0 && rxfh.hfunc == ETH_RSS_HASH_NO_CHANGE && rxfh.input_xfrm == RXH_XFRM_NO_CHANGE)) return -EINVAL;
The 'at least one change' requirement is not applicable for context creation, skip the check in such case. This allows a command such as 'ethtool -X eth0 context new' to work. The command works by mistake when using older versions of userspace ethtool due to an incompatibility issue where rxfh.input_xfrm is passed as zero (unset) instead of RXH_XFRM_NO_CHANGE as done with recent userspace. This patch does not try to solve the incompatibility issue. Link: https://lore.kernel.org/netdev/05ae8316-d3aa-4356-98c6-55ed4253c8a7@nvidia.com/ Fixes: 84a1d9c48200 ("net: ethtool: extend RXNFC API to support RSS spreading of filter matches") Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com> Reviewed-by: Jianbo Liu <jianbol@nvidia.com> Signed-off-by: Gal Pressman <gal@nvidia.com> --- v1->v2: https://lore.kernel.org/netdev/20240807132541.3460386-1-gal@nvidia.com/ * Split the check to two, one for validation and one for 'no change' * Only check for !change in the 'no change' check * Remove wrong comment --- net/ethtool/ioctl.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)