[PATCH ethtool v2 0/2] netlink: require a value for hds-thresh and the TDR parameters

[PATCH ethtool v2 0/2] netlink: require a value for hds-thresh and the TDR parameters

[Dave Seddon]

hds-thresh (-G) and first, last, step and pair (--cable-test-tdr) take
a value but leave min_argc at 0, so a missing value prints
"invalid value '(null)'" instead of "no value for parameter". Each
patch sets min_argc to 1 for one command.

Changes in v2:
- Split into one patch per command, each with its own Fixes tag
  (Andrew Lunn).
- v1: https://lore.kernel.org/netdev/20261102023919.1472495-1-dave.seddon.ca@gmail.com/

Checked by hand on a build of each patch on bae2dbd: the parameters
it covers print "no value for parameter", the others are unchanged.
make check passes.

Claude found the issue while porting the code from C to Go, and drafted
the patches; I reviewed them and take responsibility for them.

Dave Seddon (2):
  netlink: rings: require a value for hds-thresh
  netlink: cable_test: require a value for the TDR parameters

 netlink/cable_test.c | 4 ++++
 netlink/rings.c      | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.56.0

[PATCH ethtool v2 1/2] netlink: rings: require a value for hds-thresh

[Dave Seddon]

hds-thresh (-G) takes a value, but its parser entry sets min_argc to 0.
Given as the last word on the line, the value handler reads the NULL
that ends argv and the error message prints it:

  # ethtool -G eth0 hds-thresh
  ethtool (-G): invalid value '(null)' for parameter 'hds-thresh'

Set min_argc to 1, as every other value-taking parameter does, so the
parser reports the missing value itself:

  # ethtool -G eth0 hds-thresh
  ethtool (-G): no value for parameter 'hds-thresh'

Only the message changes: the exit status was already 1 and no request
was sent.

Fixes: 66e5dcf295c0 ("ethtool: Add support for configuring hds-thresh")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Dave Seddon <dave.seddon.ca@gmail.com>
---
 netlink/rings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/netlink/rings.c b/netlink/rings.c
index 5c695ab..22552df 100644
--- a/netlink/rings.c
+++ b/netlink/rings.c
@@ -202,7 +202,7 @@ static const struct param_parser sring_params[] = {
 		.arg		= "hds-thresh",
 		.type		= ETHTOOL_A_RINGS_HDS_THRESH,
 		.handler	= nl_parse_direct_u32,
-		.min_argc	= 0,
+		.min_argc	= 1,
 	},
 	{}
 };
-- 
2.56.0

[PATCH ethtool v2 2/2] netlink: cable_test: require a value for the TDR parameters

[Dave Seddon]

first, last, step and pair (--cable-test-tdr) take a value, but their
parser entries leave min_argc at 0. Given as the last word on the
line, the value handler reads the NULL that ends argv and the error
message prints it:

  # ethtool --cable-test-tdr eth0 first 1 last
  ethtool (--cable-test-tdr): invalid value '(null)' for parameter 'last'

Set min_argc to 1, as every other value-taking parameter does, so the
parser reports the missing value itself:

  # ethtool --cable-test-tdr eth0 first 1 last
  ethtool (--cable-test-tdr): no value for parameter 'last'

Only the message changes: the exit status was already 1 and no request
was sent.

Fixes: 9561db9b76f4 ("Add cable test TDR support")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Dave Seddon <dave.seddon.ca@gmail.com>
---
 netlink/cable_test.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/netlink/cable_test.c b/netlink/cable_test.c
index fdb046e..648bf74 100644
--- a/netlink/cable_test.c
+++ b/netlink/cable_test.c
@@ -544,24 +544,28 @@ static const struct param_parser tdr_params[] = {
 		.type		= ETHTOOL_A_CABLE_TEST_TDR_CFG_FIRST,
 		.group		= ETHTOOL_A_CABLE_TEST_TDR_CFG,
 		.handler	= nl_parse_direct_m2cm,
+		.min_argc	= 1,
 	},
 	{
 		.arg		= "last",
 		.type		= ETHTOOL_A_CABLE_TEST_TDR_CFG_LAST,
 		.group		= ETHTOOL_A_CABLE_TEST_TDR_CFG,
 		.handler	= nl_parse_direct_m2cm,
+		.min_argc	= 1,
 	},
 	{
 		.arg		= "step",
 		.type		= ETHTOOL_A_CABLE_TEST_TDR_CFG_STEP,
 		.group		= ETHTOOL_A_CABLE_TEST_TDR_CFG,
 		.handler	= nl_parse_direct_m2cm,
+		.min_argc	= 1,
 	},
 	{
 		.arg		= "pair",
 		.type		= ETHTOOL_A_CABLE_TEST_TDR_CFG_PAIR,
 		.group		= ETHTOOL_A_CABLE_TEST_TDR_CFG,
 		.handler	= nl_parse_direct_u8,
+		.min_argc	= 1,
 	},
 	{}
 };
-- 
2.56.0

Re: [PATCH ethtool v2 2/2] netlink: cable_test: require a value for the TDR parameters

[Andrew Lunn]

On Mon, Nov 02, 2026 at 10:51:21AM -0800, Dave Seddon wrote:
> first, last, step and pair (--cable-test-tdr) take a value, but their
> parser entries leave min_argc at 0. Given as the last word on the
> line, the value handler reads the NULL that ends argv and the error
> message prints it:
> 
>   # ethtool --cable-test-tdr eth0 first 1 last
>   ethtool (--cable-test-tdr): invalid value '(null)' for parameter 'last'
> 
> Set min_argc to 1, as every other value-taking parameter does, so the
> parser reports the missing value itself:
> 
>   # ethtool --cable-test-tdr eth0 first 1 last
>   ethtool (--cable-test-tdr): no value for parameter 'last'
> 
> Only the message changes: the exit status was already 1 and no request
> was sent.
> 
> Fixes: 9561db9b76f4 ("Add cable test TDR support")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Dave Seddon <dave.seddon.ca@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew