diff mbox series

[v6,net-next,8/9] net: ethtool: use the tracking array for get_rxfh on custom RSS contexts

Message ID 2f024e0b6d32880ff443c4e880af16ec2b5e456a.1718862050.git.ecree.xilinx@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ethtool: track custom RSS contexts in the core | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 842 this patch: 842
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 849 this patch: 849
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 849 this patch: 849
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 37 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2024-06-20--12-00 (tests: 659)

Commit Message

edward.cree@amd.com June 20, 2024, 5:47 a.m. UTC 
From: Edward Cree <ecree.xilinx@gmail.com>

On 'ethtool -x' with rss_context != 0, instead of calling the driver to
 read the RSS settings for the context, just get the settings from the
 rss_ctx xarray, and return them to the user with no driver involvement.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 net/ethtool/ioctl.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

Comments

Simon Horman June 20, 2024, 7:42 p.m. UTC | #1 
On Thu, Jun 20, 2024 at 06:47:11AM +0100, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> On 'ethtool -x' with rss_context != 0, instead of calling the driver to
>  read the RSS settings for the context, just get the settings from the
>  rss_ctx xarray, and return them to the user with no driver involvement.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
>  net/ethtool/ioctl.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 9d2d677770db..ac562ee3662e 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -1199,6 +1199,7 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
>  	const struct ethtool_ops *ops = dev->ethtool_ops;
>  	struct ethtool_rxfh_param rxfh_dev = {};
>  	u32 user_indir_size, user_key_size;
> +	struct ethtool_rxfh_context *ctx;
>  	struct ethtool_rxfh rxfh;
>  	u32 indir_bytes;
>  	u8 *rss_config;
> @@ -1246,11 +1247,25 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
>  	if (user_key_size)
>  		rxfh_dev.key = rss_config + indir_bytes;
>  
> -	rxfh_dev.rss_context = rxfh.rss_context;
> -
> -	ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
> -	if (ret)
> -		goto out;
> +	if (rxfh.rss_context) {
> +		ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
> +		if (!ctx) {
> +			ret = -ENOENT;
> +			goto out;
> +		}
> +		if (rxfh_dev.indir)
> +			memcpy(rxfh_dev.indir, ethtool_rxfh_context_indir(ctx),
> +			       indir_bytes);
> +		if (rxfh_dev.key)
> +			memcpy(rxfh_dev.key, ethtool_rxfh_context_key(ctx),
> +			       user_key_size);
> +		rxfh_dev.hfunc = ctx->hfunc;
> +		rxfh_dev.input_xfrm = ctx->input_xfrm;

Hi Edward,

The last line of this function is:

	return ret;

With this patch applied, Smatch complains that ret may be used there
when unintialised.

I think that occurs when the code reaches the line where this
commentary has been placed in this email.

> +	} else {
> +		ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
> +		if (ret)
> +			goto out;
> +	}
>  
>  	if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, hfunc),
>  			 &rxfh_dev.hfunc, sizeof(rxfh.hfunc))) {
>
Edward Cree June 24, 2024, 1:31 p.m. UTC | #2 
On 20/06/2024 20:42, Simon Horman wrote:
> On Thu, Jun 20, 2024 at 06:47:11AM +0100, edward.cree@amd.com wrote:
>> From: Edward Cree <ecree.xilinx@gmail.com>
>>
>> On 'ethtool -x' with rss_context != 0, instead of calling the driver to
>>  read the RSS settings for the context, just get the settings from the
>>  rss_ctx xarray, and return them to the user with no driver involvement.
>>
>> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
>> ---
>>  net/ethtool/ioctl.c | 25 ++++++++++++++++++++-----
>>  1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
>> index 9d2d677770db..ac562ee3662e 100644
>> --- a/net/ethtool/ioctl.c
>> +++ b/net/ethtool/ioctl.c
>> @@ -1199,6 +1199,7 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
>>  	const struct ethtool_ops *ops = dev->ethtool_ops;
>>  	struct ethtool_rxfh_param rxfh_dev = {};
>>  	u32 user_indir_size, user_key_size;
>> +	struct ethtool_rxfh_context *ctx;
>>  	struct ethtool_rxfh rxfh;
>>  	u32 indir_bytes;
>>  	u8 *rss_config;
>> @@ -1246,11 +1247,25 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
>>  	if (user_key_size)
>>  		rxfh_dev.key = rss_config + indir_bytes;
>>  
>> -	rxfh_dev.rss_context = rxfh.rss_context;
>> -
>> -	ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
>> -	if (ret)
>> -		goto out;
>> +	if (rxfh.rss_context) {
>> +		ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
>> +		if (!ctx) {
>> +			ret = -ENOENT;
>> +			goto out;
>> +		}
>> +		if (rxfh_dev.indir)
>> +			memcpy(rxfh_dev.indir, ethtool_rxfh_context_indir(ctx),
>> +			       indir_bytes);
>> +		if (rxfh_dev.key)
>> +			memcpy(rxfh_dev.key, ethtool_rxfh_context_key(ctx),
>> +			       user_key_size);
>> +		rxfh_dev.hfunc = ctx->hfunc;
>> +		rxfh_dev.input_xfrm = ctx->input_xfrm;
> 
> Hi Edward,
> 
> The last line of this function is:
> 
> 	return ret;
> 
> With this patch applied, Smatch complains that ret may be used there
> when unintialised.
> 
> I think that occurs when the code reaches the line where this
> commentary has been placed in this email.

You (and Smatch) are quite right.  Fixed for v7.
-ed

> 
>> +	} else {
>> +		ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
>> +		if (ret)
>> +			goto out;
>> +	}
>>  
>>  	if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, hfunc),
>>  			 &rxfh_dev.hfunc, sizeof(rxfh.hfunc))) {
>>
diff mbox series

Patch

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 9d2d677770db..ac562ee3662e 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1199,6 +1199,7 @@  static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
 	const struct ethtool_ops *ops = dev->ethtool_ops;
 	struct ethtool_rxfh_param rxfh_dev = {};
 	u32 user_indir_size, user_key_size;
+	struct ethtool_rxfh_context *ctx;
 	struct ethtool_rxfh rxfh;
 	u32 indir_bytes;
 	u8 *rss_config;
@@ -1246,11 +1247,25 @@  static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
 	if (user_key_size)
 		rxfh_dev.key = rss_config + indir_bytes;
 
-	rxfh_dev.rss_context = rxfh.rss_context;
-
-	ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
-	if (ret)
-		goto out;
+	if (rxfh.rss_context) {
+		ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
+		if (!ctx) {
+			ret = -ENOENT;
+			goto out;
+		}
+		if (rxfh_dev.indir)
+			memcpy(rxfh_dev.indir, ethtool_rxfh_context_indir(ctx),
+			       indir_bytes);
+		if (rxfh_dev.key)
+			memcpy(rxfh_dev.key, ethtool_rxfh_context_key(ctx),
+			       user_key_size);
+		rxfh_dev.hfunc = ctx->hfunc;
+		rxfh_dev.input_xfrm = ctx->input_xfrm;
+	} else {
+		ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
+		if (ret)
+			goto out;
+	}
 
 	if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, hfunc),
 			 &rxfh_dev.hfunc, sizeof(rxfh.hfunc))) {