netdev - RE: [PATCH ethtool] ethtool: fec: Change the prompt string to adapt to current situations

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <SN6PR13MB2414A83004A1D2B1C2FF4FB79AA29@SN6PR13MB2414.namprd13.prod.outlook.com>
Date:   Mon, 6 Jun 2022 03:13:31 +0000
From:   Macris Xiao <yu.xiao@...igine.com>
To:     Michal Kubecek <mkubecek@...e.cz>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        oss-drivers <oss-drivers@...igine.com>,
        Simon Horman <simon.horman@...igine.com>,
        Yinjun Zhang <yinjun.zhang@...igine.com>
Subject: RE: [PATCH ethtool] ethtool: fec: Change the prompt string to adapt
 to current situations



> -----Original Message-----
> From: Michal Kubecek <mkubecek@...e.cz>
> Sent: 2022年6月3日 2:55
> To: Simon Horman <simon.horman@...igine.com>
> Cc: netdev@...r.kernel.org; oss-drivers <oss-drivers@...igine.com>; Macris
> Xiao <yu.xiao@...igine.com>
> Subject: Re: [PATCH ethtool] ethtool: fec: Change the prompt string to adapt
> to current situations
> 
> On Thu, Jun 02, 2022 at 07:36:26AM +0200, Simon Horman wrote:
> > From: Yu Xiao <yu.xiao@...igine.com>
> >
> > Majority upstream drivers uses `Configured FEC encodings` to report
> > supported modes. At which point it is better to change the text in
> > ethtool user space that changes the meaning of the field, which is
> > better to suit for the current situations.
> >
> > So changing `Configured FEC encodings` to `Supported/Configured FEC
> > encodings` to adapt to both implementations.
> >
> > Signed-off-by: Yu Xiao <yu.xiao@...igine.com>
> > Signed-off-by: Simon Horman <simon.horman@...igine.com>
> > ---
> >
> > This patch resulted from a discussion on netdev regarding updating the
> > behaviour of the NFP driver. It was concluded in that thread that it
> > would be better to update the ethtool documentation to reflect current
> > implementations of the feature.
> >
> > Ref: [PATCH net] nfp: correct the output of `ethtool --show-fec <intf>`
> >
> > https://lore.kernel.org/netdev/20220530084842.21258-1-
> simon.horman@cor
> > igine.com/
> > ---
> >  ethtool.c     | 2 +-
> >  netlink/fec.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/ethtool.c b/ethtool.c
> > index 277253090245..8654f70de03b 100644
> > --- a/ethtool.c
> > +++ b/ethtool.c
> > @@ -5567,7 +5567,7 @@ static int do_gfec(struct cmd_context *ctx)
> >  	}
> >
> >  	fprintf(stdout, "FEC parameters for %s:\n", ctx->devname);
> > -	fprintf(stdout, "Configured FEC encodings:");
> > +	fprintf(stdout, "Supported/Configured FEC encodings:");
> 
> I'm OK with this part, even if I would prefer if we could unify what the drivers
> present.
> 
> >  	dump_fec(feccmd.fec);
> >  	fprintf(stdout, "\n");
> >
> > diff --git a/netlink/fec.c b/netlink/fec.c index
> > f2659199c157..1762ae349ca6 100644
> > --- a/netlink/fec.c
> > +++ b/netlink/fec.c
> > @@ -153,7 +153,7 @@ int fec_reply_cb(const struct nlmsghdr *nlhdr, void
> *data)
> >  	print_string(PRINT_ANY, "ifname", "FEC parameters for %s:\n",
> >  		     nlctx->devname);
> >
> > -	open_json_array("config", "Configured FEC encodings:");
> > +	open_json_array("support/config", "Supported/Configured FEC
> > +encodings:");
> 
> AFAICS this would result in backword incompatible change of the JSON
> output structure which is something we should avoid if we want people to
> use JSON for scripting rather than the human readable text output.
> 
> Michal
> 

Thank you for advice. So whether to use only the first part, and is this part not
needed?

Macris Xiao

> >  	fa = tb[ETHTOOL_A_FEC_AUTO] &&
> mnl_attr_get_u8(tb[ETHTOOL_A_FEC_AUTO]);
> >  	if (fa)
> >  		print_string(PRINT_ANY, NULL, " %s", "Auto");
> > --
> > 2.30.2
> >