| 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
| ||
|
Date: Thu, 4 Oct 2018 16:43:50 +0200
From: Michal Kubecek <mkubecek@...e.cz>
To: "John W. Linville" <linville@...driver.com>
Cc: Edward Cree <ecree@...arflare.com>, netdev <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH ethtool] ethtool: better syntax for combinations of
FEC modes
On Thu, Oct 04, 2018 at 10:08:14AM -0400, John W. Linville wrote:
> Ping?
>
> On Mon, Oct 01, 2018 at 02:59:10PM -0400, John W. Linville wrote:
> > Is this patch still RFC?
As far as I'm concerned, it can be taken as it is.
Michal Kubecek
> >
> > 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