| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20181004140814.GB8557@tuxdriver.com>
Date: Thu, 4 Oct 2018 10:08:14 -0400
From: "John W. Linville" <linville@...driver.com>
To: Edward Cree <ecree@...arflare.com>
Cc: Michal Kubecek <mkubecek@...e.cz>, netdev <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH ethtool] ethtool: better syntax for combinations of
FEC modes
Ping?
On Mon, Oct 01, 2018 at 02:59:10PM -0400, John W. Linville wrote:
> Is this patch still RFC?
>
> On Wed, Sep 19, 2018 at 05:06:25PM +0100, Edward Cree wrote:
> > Instead of commas, just have them as separate argvs.
> >
> > The parsing state machine might look heavyweight, but it makes it easy to add
> > more parameters later and distinguish parameter names from encoding names.
> >
> > Suggested-by: Michal Kubecek <mkubecek@...e.cz>
> > Signed-off-by: Edward Cree <ecree@...arflare.com>
> > ---
> > ethtool.8.in | 6 +++---
> > ethtool.c | 63 ++++++++++++++++------------------------------------------
> > test-cmdline.c | 10 +++++-----
> > 3 files changed, 25 insertions(+), 54 deletions(-)
> >
> > diff --git a/ethtool.8.in b/ethtool.8.in
> > index 414eaa1..7ea2cc0 100644
> > --- a/ethtool.8.in
> > +++ b/ethtool.8.in
> > @@ -390,7 +390,7 @@ ethtool \- query or control network driver and hardware settings
> > .B ethtool \-\-set\-fec
> > .I devname
> > .B encoding
> > -.BR auto | off | rs | baser [ , ...]
> > +.BR auto | off | rs | baser \ [...]
> > .
> > .\" Adjust lines (i.e. full justification) and hyphenate.
> > .ad
> > @@ -1120,11 +1120,11 @@ current FEC mode, the driver or firmware must take the link down
> > administratively and report the problem in the system logs for users to correct.
> > .RS 4
> > .TP
> > -.BR encoding\ auto | off | rs | baser [ , ...]
> > +.BR encoding\ auto | off | rs | baser \ [...]
> >
> > Sets the FEC encoding for the device. Combinations of options are specified as
> > e.g.
> > -.B auto,rs
> > +.B encoding auto rs
> > ; the semantics of such combinations vary between drivers.
> > .TS
> > nokeep;
> > diff --git a/ethtool.c b/ethtool.c
> > index 9997930..2f7e96b 100644
> > --- a/ethtool.c
> > +++ b/ethtool.c
> > @@ -4979,39 +4979,6 @@ static int fecmode_str_to_type(const char *str)
> > return 0;
> > }
> >
> > -/* Takes a comma-separated list of FEC modes, returns the bitwise OR of their
> > - * corresponding ETHTOOL_FEC_* constants.
> > - * Accepts repetitions (e.g. 'auto,auto') and trailing comma (e.g. 'off,').
> > - */
> > -static int parse_fecmode(const char *str)
> > -{
> > - int fecmode = 0;
> > - char buf[6];
> > -
> > - if (!str)
> > - return 0;
> > - while (*str) {
> > - size_t next;
> > - int mode;
> > -
> > - next = strcspn(str, ",");
> > - if (next >= 6) /* Bad mode, longest name is 5 chars */
> > - return 0;
> > - /* Copy into temp buffer and nul-terminate */
> > - memcpy(buf, str, next);
> > - buf[next] = 0;
> > - mode = fecmode_str_to_type(buf);
> > - if (!mode) /* Bad mode encountered */
> > - return 0;
> > - fecmode |= mode;
> > - str += next;
> > - /* Skip over ',' (but not nul) */
> > - if (*str)
> > - str++;
> > - }
> > - return fecmode;
> > -}
> > -
> > static int do_gfec(struct cmd_context *ctx)
> > {
> > struct ethtool_fecparam feccmd = { 0 };
> > @@ -5041,22 +5008,26 @@ static int do_gfec(struct cmd_context *ctx)
> >
> > static int do_sfec(struct cmd_context *ctx)
> > {
> > - char *fecmode_str = NULL;
> > + enum { ARG_NONE, ARG_ENCODING } state = ARG_NONE;
> > struct ethtool_fecparam feccmd;
> > - struct cmdline_info cmdline_fec[] = {
> > - { "encoding", CMDL_STR, &fecmode_str, &feccmd.fec},
> > - };
> > - int changed;
> > - int fecmode;
> > - int rv;
> > + int fecmode = 0, newmode;
> > + int rv, i;
> >
> > - parse_generic_cmdline(ctx, &changed, cmdline_fec,
> > - ARRAY_SIZE(cmdline_fec));
> > -
> > - if (!fecmode_str)
> > + for (i = 0; i < ctx->argc; i++) {
> > + if (!strcmp(ctx->argp[i], "encoding")) {
> > + state = ARG_ENCODING;
> > + continue;
> > + }
> > + if (state == ARG_ENCODING) {
> > + newmode = fecmode_str_to_type(ctx->argp[i]);
> > + if (!newmode)
> > + exit_bad_args();
> > + fecmode |= newmode;
> > + continue;
> > + }
> > exit_bad_args();
> > + }
> >
> > - fecmode = parse_fecmode(fecmode_str);
> > if (!fecmode)
> > exit_bad_args();
> >
> > @@ -5265,7 +5236,7 @@ static const struct option {
> > " [ all ]\n"},
> > { "--show-fec", 1, do_gfec, "Show FEC settings"},
> > { "--set-fec", 1, do_sfec, "Set FEC settings",
> > - " [ encoding auto|off|rs|baser ]\n"},
> > + " [ encoding auto|off|rs|baser [...]]\n"},
> > { "-h|--help", 0, show_usage, "Show this help" },
> > { "--version", 0, do_version, "Show version number" },
> > {}
> > diff --git a/test-cmdline.c b/test-cmdline.c
> > index 9c51cca..84630a5 100644
> > --- a/test-cmdline.c
> > +++ b/test-cmdline.c
> > @@ -268,12 +268,12 @@ static struct test_case {
> > { 1, "--set-eee devname advertise foo" },
> > { 1, "--set-fec devname" },
> > { 0, "--set-fec devname encoding auto" },
> > - { 0, "--set-fec devname encoding off," },
> > - { 0, "--set-fec devname encoding baser,rs" },
> > - { 0, "--set-fec devname encoding auto,auto," },
> > + { 0, "--set-fec devname encoding off" },
> > + { 0, "--set-fec devname encoding baser rs" },
> > + { 0, "--set-fec devname encoding auto auto" },
> > { 1, "--set-fec devname encoding foo" },
> > - { 1, "--set-fec devname encoding auto,foo" },
> > - { 1, "--set-fec devname encoding auto,," },
> > + { 1, "--set-fec devname encoding auto foo" },
> > + { 1, "--set-fec devname encoding none" },
> > { 1, "--set-fec devname auto" },
> > /* can't test --set-priv-flags yet */
> > { 0, "-h" },
> >
>
> --
> John W. Linville Someday the world will need a hero, and you
> linville@...driver.com might be all we have. Be ready.
>
--
John W. Linville Someday the world will need a hero, and you
linville@...driver.com might be all we have. Be ready.
Powered by blists - more mailing lists