From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ariel Almog <arielalmogworkemails@gmail.com>
Subject: Re: [PATCH ethtool] ethtool: support combinations of FEC modes
Date: Wed, 26 Sep 2018 11:47:45 +0300
Message-ID: <CABvr3-Hvq4nsNEfJH+ic_pFGup2iAUGjcDfXni33a6LZoSZOYw@mail.gmail.com>
References: <518b8b8b-0151-1053-3798-6009044ed53a@solarflare.com> <811cf92b-51ed-4a8f-4b69-113cdd8473df@mellanox.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Cc: linville@tuxdriver.com, Linux Netdev List <netdev@vger.kernel.org>,
        ganeshgr@chelsio.com, jakub.kicinski@netronome.com,
        dustin@cumulusnetworks.com, dirk.vandermerwe@netronome.com,
        shayag@mellanox.com, ariela@mellanox.com
To: ecree@solarflare.com
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-qt1-f172.google.com ([209.85.160.172]:36783 "EHLO
        mail-qt1-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726593AbeIZO7u (ORCPT
        <rfc822;netdev@vger.kernel.org>); Wed, 26 Sep 2018 10:59:50 -0400
Received: by mail-qt1-f172.google.com with SMTP id e26-v6so4213550qtq.3
        for <netdev@vger.kernel.org>; Wed, 26 Sep 2018 01:47:58 -0700 (PDT)
In-Reply-To: <811cf92b-51ed-4a8f-4b69-113cdd8473df@mellanox.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -4967,20 +4967,48 @@ static int do_set_phy_tunable(struct cmd_context
> *ctx)
>    static int fecmode_str_to_type(const char *str)
>   {
> +       if (!strcasecmp(str, "auto"))
> +               return ETHTOOL_FEC_AUTO;
> +       if (!strcasecmp(str, "off"))
> +               return ETHTOOL_FEC_OFF;
> +       if (!strcasecmp(str, "rs"))
> +               return ETHTOOL_FEC_RS;
> +       if (!strcasecmp(str, "baser"))
> +               return ETHTOOL_FEC_BASER;
> +
> +       return 0;
> +}
I was won
> +
> +/* 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 fecmode;
> -
> -       if (!strcasecmp(str, "auto"))
> -               fecmode |= ETHTOOL_FEC_AUTO;
> -       else if (!strcasecmp(str, "off"))
> -               fecmode |= ETHTOOL_FEC_OFF;
> -       else if (!strcasecmp(str, "rs"))
> -               fecmode |= ETHTOOL_FEC_RS;
> -       else if (!strcasecmp(str, "baser"))
> -               fecmode |= ETHTOOL_FEC_BASER;
> +               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;

I would like to apologize for my late response.

I find the ability to set off, auto and specific FEC mode in the same
command confusing.
Here are some examples

1. What is the expected result of 'off' & other FEC mode such as 'RS'?
  -'off'?
  -'RS'?
  -automatic selection {'off','RS'}? w/o setting of auto?

2. What is the expected result of 'off', 'RS' and 'auto'?
  - automatic selection from the set of {'RS','off'}
    - if that is the case, what is the different from 'off' and 'RS'
with out 'auto'?
  - allowing the device to use all three modes
    - automatic selection {auto, rs, off}. what is the meaning of auto of auto?

I think that we shall have some mutual configuration limitation :

1. if 'auto' was set, any other configuration from within the set
{'off', 'RS', 'base-r'}
    will imply the set of configuration to be selected by auto mode
    i.e. 'auto', 'RS' and 'off' configuration will result with
automatic selection between {'off', 'RS'}
2. if 'auto' was not set, only one configuration from within {'off',
'RS', 'base-r'} can
    be set (and from that, 'off' cannot be set with other configuration)

Thanks
Ariel Almog
Mellanox technologies