[PATCH ethtool] ethtool: Accept long feature names reported by -k option as input to -K option

[PATCH ethtool] ethtool: Accept long feature names reported by -k option as input to -K option

[Ben Hutchings]

Before the generic features API was introduced, the ethtool -K option
took short names for various features, e.g. 'gso' but the -k option
reported their state using longer names,
e.g. 'generic-segmentation-offload'.

All newer features have a single kernel-provided name so input
and output are consistent.  But the old features still aren't, and
although their short names are documented it's not good to have
these exceptions.

Change the argument parsing code for -K so that the long names
reported by -k are also accepted.

Reported-by: Or Gerlitz <or.gerlitz@gmail.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
This turned out to be pretty easy to do as the argument processing is
all table-driven already.  I'll push this if it works for you.

Ben.

 ethtool.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index b06dfa3..4226d2e 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2053,25 +2053,31 @@ static int do_sfeatures(struct cmd_context *ctx)
 	/* Generate cmdline_info for legacy flags and kernel-named
 	 * features, and parse our arguments.
 	 */
-	cmdline_features = calloc(ARRAY_SIZE(off_flag_def) + defs->n_features,
+	cmdline_features = calloc(2 * ARRAY_SIZE(off_flag_def) +
+				  defs->n_features,
 				  sizeof(cmdline_features[0]));
 	if (!cmdline_features) {
 		perror("Cannot parse arguments");
 		return 1;
 	}
-	for (i = 0; i < ARRAY_SIZE(off_flag_def); i++)
+	for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) {
 		flag_to_cmdline_info(off_flag_def[i].short_name,
 				     off_flag_def[i].value,
 				     &off_flags_wanted, &off_flags_mask,
-				     &cmdline_features[i]);
+				     &cmdline_features[2 * i]);
+		flag_to_cmdline_info(off_flag_def[i].long_name,
+				     off_flag_def[i].value,
+				     &off_flags_wanted, &off_flags_mask,
+				     &cmdline_features[2 * i] + 1);
+	}
 	for (i = 0; i < defs->n_features; i++)
 		flag_to_cmdline_info(
 			defs->def[i].name, FEATURE_FIELD_FLAG(i),
 			&FEATURE_WORD(efeatures->features, i, requested),
 			&FEATURE_WORD(efeatures->features, i, valid),
-			&cmdline_features[ARRAY_SIZE(off_flag_def) + i]);
+			&cmdline_features[2 * ARRAY_SIZE(off_flag_def) + i]);
 	parse_generic_cmdline(ctx, &any_changed, cmdline_features,
-			      ARRAY_SIZE(off_flag_def) + defs->n_features);
+			      2 * ARRAY_SIZE(off_flag_def) + defs->n_features);
 	free(cmdline_features);
 
 	if (!any_changed) {

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [PATCH ethtool] ethtool: Accept long feature names reported by -k option as input to -K option

[Allan, Bruce W]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Ben Hutchings
> Sent: Wednesday, January 08, 2014 1:52 PM
> To: Or Gerlitz; Bill Fink; netdev@vger.kernel.org
> Subject: [PATCH ethtool] ethtool: Accept long feature names reported by -k
> option as input to -K option
> 
> Before the generic features API was introduced, the ethtool -K option
> took short names for various features, e.g. 'gso' but the -k option
> reported their state using longer names,
> e.g. 'generic-segmentation-offload'.
> 
> All newer features have a single kernel-provided name so input
> and output are consistent.  But the old features still aren't, and
> although their short names are documented it's not good to have
> these exceptions.
> 
> Change the argument parsing code for -K so that the long names
> reported by -k are also accepted.
> 
> Reported-by: Or Gerlitz <or.gerlitz@gmail.com>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> This turned out to be pretty easy to do as the argument processing is
> all table-driven already.  I'll push this if it works for you.
> 
> Ben.
> 
>  ethtool.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)

Will there be an update to the man page, too?

Re: [PATCH ethtool] ethtool: Accept long feature names reported by -k option as input to -K option

[Ben Hutchings]

On Wed, 2014-01-08 at 22:00 +0000, Allan, Bruce W wrote:
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-
> > owner@vger.kernel.org] On Behalf Of Ben Hutchings
> > Sent: Wednesday, January 08, 2014 1:52 PM
> > To: Or Gerlitz; Bill Fink; netdev@vger.kernel.org
> > Subject: [PATCH ethtool] ethtool: Accept long feature names reported by -k
> > option as input to -K option
> > 
> > Before the generic features API was introduced, the ethtool -K option
> > took short names for various features, e.g. 'gso' but the -k option
> > reported their state using longer names,
> > e.g. 'generic-segmentation-offload'.
> > 
> > All newer features have a single kernel-provided name so input
> > and output are consistent.  But the old features still aren't, and
> > although their short names are documented it's not good to have
> > these exceptions.
> > 
> > Change the argument parsing code for -K so that the long names
> > reported by -k are also accepted.
> > 
> > Reported-by: Or Gerlitz <or.gerlitz@gmail.com>
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> > This turned out to be pretty easy to do as the argument processing is
> > all table-driven already.  I'll push this if it works for you.
> > 
> > Ben.
> > 
> >  ethtool.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> Will there be an update to the man page, too?

I wasn't intending to do so.  The set of feature names is not known at
compile time so I think it should only list the names that *aren't*
reported by ethtool -K (the short names).

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH ethtool] ethtool: Accept long feature names reported by -k option as input to -K option

[Ben Hutchings]

On Wed, 2014-01-08 at 22:00 +0000, Allan, Bruce W wrote:
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-
> > owner@vger.kernel.org] On Behalf Of Ben Hutchings
> > Sent: Wednesday, January 08, 2014 1:52 PM
> > To: Or Gerlitz; Bill Fink; netdev@vger.kernel.org
> > Subject: [PATCH ethtool] ethtool: Accept long feature names reported by -k
> > option as input to -K option
> > 
> > Before the generic features API was introduced, the ethtool -K option
> > took short names for various features, e.g. 'gso' but the -k option
> > reported their state using longer names,
> > e.g. 'generic-segmentation-offload'.
> > 
> > All newer features have a single kernel-provided name so input
> > and output are consistent.  But the old features still aren't, and
> > although their short names are documented it's not good to have
> > these exceptions.
> > 
> > Change the argument parsing code for -K so that the long names
> > reported by -k are also accepted.
> > 
> > Reported-by: Or Gerlitz <or.gerlitz@gmail.com>
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> > This turned out to be pretty easy to do as the argument processing is
> > all table-driven already.  I'll push this if it works for you.
> > 
> > Ben.
> > 
> >  ethtool.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> Will there be an update to the man page, too?

I wasn't intending to do so.  The set of feature names is not known at
compile time so I think it should only list the names that *aren't*
reported by ethtool -k (i.e. the short names).

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH ethtool] ethtool: Accept long feature names reported by -k option as input to -K option

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

On Wed, 2014-01-08 at 21:52 +0000, Ben Hutchings wrote:
> Before the generic features API was introduced, the ethtool -K option
> took short names for various features, e.g. 'gso' but the -k option
> reported their state using longer names,
> e.g. 'generic-segmentation-offload'.
> 
> All newer features have a single kernel-provided name so input
> and output are consistent.  But the old features still aren't, and
> although their short names are documented it's not good to have
> these exceptions.
> 
> Change the argument parsing code for -K so that the long names
> reported by -k are also accepted.
> 
> Reported-by: Or Gerlitz <or.gerlitz@gmail.com>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> This turned out to be pretty easy to do as the argument processing is
> all table-driven already.  I'll push this if it works for you.
[...]

Can you confirm whether this does what you wanted?

Ben.

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]