netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* "ethtool" missing "master-slave" args in "do_sset" function.[TEXT/PLAIN]
@ 2020-12-27  4:34 Bruce LIU
  2020-12-27  9:46 ` Michal Kubecek
  0 siblings, 1 reply; 5+ messages in thread
From: Bruce LIU @ 2020-12-27  4:34 UTC (permalink / raw)
  To: mkubecek, netdev; +Cc: shlei

Hi Michal Kubecek and Network dev team,

Good day! Hope you are doing well.
This is Bruce from China, and please allow me to cc Rudy from Cisco 
Systems in China team.

We are facing a weird behavior about "master-slave configuration" 
function in ethtool.
Please correct me if I am wrong....

As you know, start from ethtool 5.8,  "master/slave configuration 
support" added.
https://lwn.net/Articles/828044/

========================================================================
Appeal:
Confirm and discuss workaround

========================================================================
Issue description:
As we test in lab, no "master-slave" option supported.

========================================================================
Issue reproduce:
root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
ethtool: bad command line argument(s)
For more information run ethtool -h

========================================================================
Environment:
debian-live-10.7.0-amd64-standard.iso
Kernel 5.4.79
ethtool 5.10
Source code: 
https://mirrors.edge.kernel.org/pub/software/network/ethtool/ethtool-5.10.tar.xz

========================================================================
Troubleshooting:
root@raspberrypi:~# ethtool -h
ethtool version 5.10
Usage:
         ethtool [ FLAGS ] DEVNAME       Display standard information
about device
         ethtool [ FLAGS ] -s|--change DEVNAME   Change generic options
                 [ speed %d ]
                 [ duplex half|full ]
                 [ port tp|aui|bnc|mii|fibre|da ]
                 [ mdix auto|on|off ]
                 [ autoneg on|off ]
                 [ advertise %x[/%x] | mode on|off ... [--] ]
                 [ phyad %d ]
                 [ xcvr internal|external ]
                 [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]
                 [ sopass %x:%x:%x:%x:%x:%x ]
                 [ msglvl %d[/%d] | type on|off ... [--] ]
                 [ master-slave
master-preferred|slave-preferred|master-force|slave-force ]

root@raspberrypi:~# ethtool -s eth0 [double tab here]
advertise  autoneg    duplex     mdix       msglvl     phyad port
       speed      wol        xcvr

========================================
Review 5.10 source code:
ethtool.c line:5616

static const struct option args[] = {
{
.opts = "-s|--change",
.func = do_sset,
.nlfunc = nl_sset,
.help = "Change generic options",
.xhelp = " [ speed %d ]\n"
  " [ duplex half|full ]\n"
  " [ port tp|aui|bnc|mii|fibre|da ]\n"
  " [ mdix auto|on|off ]\n"
  " [ autoneg on|off ]\n"
  " [ advertise %x[/%x] | mode on|off ... [--] ]\n"
  " [ phyad %d ]\n"
  " [ xcvr internal|external ]\n"
  " [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]\n"
  " [ sopass %x:%x:%x:%x:%x:%x ]\n"
  " [ msglvl %d[/%d] | type on|off ... [--] ]\n"
  " [ master-slave 
master-preferred|slave-preferred|master-force|slave-force ]\n"
},

========================================
ethtool.c line:2912  do_sset function
There is NOT an "else if" to catch "master-slave" option, and the
options matched final else, and print an error message   "ethtool: bad
command line argument(s)\n""For more information run ethtool -h\n""

ethtool.c line: 3069

   } else {
exit_bad_args();

========================================
root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
ethtool: bad command line argument(s)
For more information run ethtool -h

Look forward to your reply.

Cheers!
Bruce Liu (UTC +08)
Email: ccieliu@gmail.com

-- 
Cheers!

Bruce Liu    (UTC +08)
Email: ccieliu@gmail.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: "ethtool" missing "master-slave" args in "do_sset" function.[TEXT/PLAIN]
  2020-12-27  4:34 "ethtool" missing "master-slave" args in "do_sset" function.[TEXT/PLAIN] Bruce LIU
@ 2020-12-27  9:46 ` Michal Kubecek
  2020-12-28 13:35   ` Rudy Lei (shlei)
  0 siblings, 1 reply; 5+ messages in thread
From: Michal Kubecek @ 2020-12-27  9:46 UTC (permalink / raw)
  To: Bruce LIU; +Cc: netdev, shlei

On Sun, Dec 27, 2020 at 12:34:09PM +0800, Bruce LIU wrote:
> Hi Michal Kubecek and Network dev team,
> 
> Good day! Hope you are doing well.
> This is Bruce from China, and please allow me to cc Rudy from Cisco Systems
> in China team.
> 
> We are facing a weird behavior about "master-slave configuration" function
> in ethtool.
> Please correct me if I am wrong....
> 
> As you know, start from ethtool 5.8,  "master/slave configuration support"
> added.
> https://lwn.net/Articles/828044/
> 
> ========================================================================
> Appeal:
> Confirm and discuss workaround
> 
> ========================================================================
> Issue description:
> As we test in lab, no "master-slave" option supported.
> 
> ========================================================================
> Issue reproduce:
> root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
> ethtool: bad command line argument(s)
> For more information run ethtool -h
> 
> ========================================================================
> Environment:
> debian-live-10.7.0-amd64-standard.iso
> Kernel 5.4.79

This is the problem. Kernel support for this feature was added in
5.8-rc1 so that your kernel does not have it and there is no chance it
could possibly work. Newer ethtool has support for this feature but
kernel must support it as well for it to actually work.

But I agree that the error message is misleading. We handle subcommands
supported only in netlink with proper error message when ioctl fallback
is used but we don't do the same for new parameters of existing
subcommands which are generally supported by ioctl code. That's why the
command line parser used by ioctl code does not recognize the new
parameter and handles it as a syntax error.

We'll need to handle new parameters in ioctl parser so that it produces
more meaningful error for parameters only supported via netlink. Long
term, the proper solution would probably be using one parser for both
netlink and ioctl but that was something I wanted to avoid for now to
reduce the risk of introducing subtle changes in behaviour of existing
code.

Michal

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: "ethtool" missing "master-slave" args in "do_sset" function.[TEXT/PLAIN]
  2020-12-27  9:46 ` Michal Kubecek
@ 2020-12-28 13:35   ` Rudy Lei (shlei)
  0 siblings, 0 replies; 5+ messages in thread
From: Rudy Lei (shlei) @ 2020-12-28 13:35 UTC (permalink / raw)
  To: Michal Kubecek, Bruce LIU; +Cc: netdev


Hi Michal,
 
Merry Christmas and thanks a lot for the prompt reply.
After upgraded the kernel to 5.10 and we did see the expected logs:

[root@shlei-dev-machine ~]# ethtool -s ens32 master-slave preferred-master
netlink error: master/slave configuration not supported by device (offset 36)
netlink error: Operation not supported

 
Best Regards,
Rudy

On 2020/12/27, 5:46 PM, "Michal Kubecek" <mkubecek@suse.cz> wrote:

    On Sun, Dec 27, 2020 at 12:34:09PM +0800, Bruce LIU wrote:
    > Hi Michal Kubecek and Network dev team,
    > 
    > Good day! Hope you are doing well.
    > This is Bruce from China, and please allow me to cc Rudy from Cisco Systems
    > in China team.
    > 
    > We are facing a weird behavior about "master-slave configuration" function
    > in ethtool.
    > Please correct me if I am wrong....
    > 
    > As you know, start from ethtool 5.8,  "master/slave configuration support"
    > added.
    > https://lwn.net/Articles/828044/
    > 
    > ========================================================================
    > Appeal:
    > Confirm and discuss workaround
    > 
    > ========================================================================
    > Issue description:
    > As we test in lab, no "master-slave" option supported.
    > 
    > ========================================================================
    > Issue reproduce:
    > root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
    > ethtool: bad command line argument(s)
    > For more information run ethtool -h
    > 
    > ========================================================================
    > Environment:
    > debian-live-10.7.0-amd64-standard.iso
    > Kernel 5.4.79

    This is the problem. Kernel support for this feature was added in
    5.8-rc1 so that your kernel does not have it and there is no chance it
    could possibly work. Newer ethtool has support for this feature but
    kernel must support it as well for it to actually work.

    But I agree that the error message is misleading. We handle subcommands
    supported only in netlink with proper error message when ioctl fallback
    is used but we don't do the same for new parameters of existing
    subcommands which are generally supported by ioctl code. That's why the
    command line parser used by ioctl code does not recognize the new
    parameter and handles it as a syntax error.

    We'll need to handle new parameters in ioctl parser so that it produces
    more meaningful error for parameters only supported via netlink. Long
    term, the proper solution would probably be using one parser for both
    netlink and ioctl but that was something I wanted to avoid for now to
    reduce the risk of introducing subtle changes in behaviour of existing
    code.

    Michal


^ permalink raw reply	[flat|nested] 5+ messages in thread

* "ethtool" missing "master-slave" args in "do_sset" function.[TEXT/PLAIN]
@ 2020-12-23  8:32 Bruce Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Bruce Liu @ 2020-12-23  8:32 UTC (permalink / raw)
  To: mkubecek, netdev; +Cc: shlei

Hi Michal Kubecek and Network dev team,

Good day! Hope you are doing well.
This is Bruce from China, and please allow me to cc Rudy from Cisco
Systems in China team.

We are facing a weird behavior about "master-slave configuration"
function in ethtool.
Please correct me if I am wrong....

As you know, start from ethtool 5.8,  "master/slave configuration
support" added.
https://lwn.net/Articles/828044/

========================================================================
Appeal:
Confirm and discuss workaround

========================================================================
Issue description:
As we test in lab, no "master-slave" option supported.

========================================================================
Issue reproduce:
root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
ethtool: bad command line argument(s)
For more information run ethtool -h

========================================================================
Environment:
debian-live-10.7.0-amd64-standard.iso
Kernel 5.4.79
ethtool 5.10
Source code: https://mirrors.edge.kernel.org/pub/software/network/ethtool/ethtool-5.10.tar.xz

========================================================================
Troubleshooting:
root@raspberrypi:~# ethtool -h
ethtool version 5.10
Usage:
        ethtool [ FLAGS ] DEVNAME       Display standard information
about device
        ethtool [ FLAGS ] -s|--change DEVNAME   Change generic options
                [ speed %d ]
                [ duplex half|full ]
                [ port tp|aui|bnc|mii|fibre|da ]
                [ mdix auto|on|off ]
                [ autoneg on|off ]
                [ advertise %x[/%x] | mode on|off ... [--] ]
                [ phyad %d ]
                [ xcvr internal|external ]
                [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]
                [ sopass %x:%x:%x:%x:%x:%x ]
                [ msglvl %d[/%d] | type on|off ... [--] ]
                [ master-slave
master-preferred|slave-preferred|master-force|slave-force ]

root@raspberrypi:~# ethtool -s eth0 [double tab here]
advertise  autoneg    duplex     mdix       msglvl     phyad      port
      speed      wol        xcvr

========================================
Review 5.10 source code:
ethtool.c line:5616

static const struct option args[] = {
{
.opts = "-s|--change",
.func = do_sset,
.nlfunc = nl_sset,
.help = "Change generic options",
.xhelp = " [ speed %d ]\n"
 " [ duplex half|full ]\n"
 " [ port tp|aui|bnc|mii|fibre|da ]\n"
 " [ mdix auto|on|off ]\n"
 " [ autoneg on|off ]\n"
 " [ advertise %x[/%x] | mode on|off ... [--] ]\n"
 " [ phyad %d ]\n"
 " [ xcvr internal|external ]\n"
 " [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]\n"
 " [ sopass %x:%x:%x:%x:%x:%x ]\n"
 " [ msglvl %d[/%d] | type on|off ... [--] ]\n"
 " [ master-slave master-preferred|slave-preferred|master-force|slave-force ]\n"
},

========================================
ethtool.c line:2912  do_sset function
There is NOT an "else if" to catch "master-slave" option, and the
options matched final else, and print an error message   "ethtool: bad
command line argument(s)\n""For more information run ethtool -h\n""

ethtool.c line: 3069

  } else {
exit_bad_args();

========================================
root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
ethtool: bad command line argument(s)
For more information run ethtool -h

Look forward to your reply.

Cheers!
Bruce Liu (UTC +08)
Email: ccieliu@gmail.com


Cheers!

Bruce Liu    (UTC +08)
Email: ccieliu@gmail.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: "ethtool" missing "master-slave" args in "do_sset" function.[TEXT/PLAIN]
       [not found] <CAHZX3B_KBEzny0ejSP73q+1Y-n=f=39jTXjzBCDB8Lsrn5U2gg@mail.gmail.com>
@ 2020-12-23  8:31 ` Bruce Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Bruce Liu @ 2020-12-23  8:31 UTC (permalink / raw)
  To: mkubecek, netdev; +Cc: shlei

Hi Michal Kubecek and Network dev team,

Good day! Hope you are doing well.
This is Bruce from China, and please allow me to cc Rudy from Cisco
Systems in China team.

We are facing a weird behavior about "master-slave configuration"
function in ethtool.
Please correct me if I am wrong....

As you know, start from ethtool 5.8,  "master/slave configuration
support" added.
https://lwn.net/Articles/828044/

========================================================================
Appeal:
Confirm and discuss workaround

========================================================================
Issue description:
As we test in lab, no "master-slave" option supported.

========================================================================
Issue reproduce:
root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
ethtool: bad command line argument(s)
For more information run ethtool -h

========================================================================
Environment:
debian-live-10.7.0-amd64-standard.iso
Kernel 5.4.79
ethtool 5.10
Source code: https://mirrors.edge.kernel.org/pub/software/network/ethtool/ethtool-5.10.tar.xz

========================================================================
Troubleshooting:
root@raspberrypi:~# ethtool -h
ethtool version 5.10
Usage:
        ethtool [ FLAGS ] DEVNAME       Display standard information
about device
        ethtool [ FLAGS ] -s|--change DEVNAME   Change generic options
                [ speed %d ]
                [ duplex half|full ]
                [ port tp|aui|bnc|mii|fibre|da ]
                [ mdix auto|on|off ]
                [ autoneg on|off ]
                [ advertise %x[/%x] | mode on|off ... [--] ]
                [ phyad %d ]
                [ xcvr internal|external ]
                [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]
                [ sopass %x:%x:%x:%x:%x:%x ]
                [ msglvl %d[/%d] | type on|off ... [--] ]
                [ master-slave
master-preferred|slave-preferred|master-force|slave-force ]

root@raspberrypi:~# ethtool -s eth0 [double tab here]
advertise  autoneg    duplex     mdix       msglvl     phyad      port
      speed      wol        xcvr

========================================
Review 5.10 source code:
ethtool.c line:5616

static const struct option args[] = {
{
.opts = "-s|--change",
.func = do_sset,
.nlfunc = nl_sset,
.help = "Change generic options",
.xhelp = " [ speed %d ]\n"
 " [ duplex half|full ]\n"
 " [ port tp|aui|bnc|mii|fibre|da ]\n"
 " [ mdix auto|on|off ]\n"
 " [ autoneg on|off ]\n"
 " [ advertise %x[/%x] | mode on|off ... [--] ]\n"
 " [ phyad %d ]\n"
 " [ xcvr internal|external ]\n"
 " [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]\n"
 " [ sopass %x:%x:%x:%x:%x:%x ]\n"
 " [ msglvl %d[/%d] | type on|off ... [--] ]\n"
 " [ master-slave master-preferred|slave-preferred|master-force|slave-force ]\n"
},

========================================
ethtool.c line:2912  do_sset function
There is NOT an "else if" to catch "master-slave" option, and the
options matched final else, and print an error message   "ethtool: bad
command line argument(s)\n""For more information run ethtool -h\n""

ethtool.c line: 3069

  } else {
exit_bad_args();

========================================
root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
ethtool: bad command line argument(s)
For more information run ethtool -h

Look forward to your reply.

Cheers!
Bruce Liu (UTC +08)
Email: ccieliu@gmail.com


Cheers!

Bruce Liu    (UTC +08)
Email: ccieliu@gmail.com



Cheers!

Bruce Liu    (UTC +08)
Email: ccieliu@gmail.com


On Wed, Dec 23, 2020 at 3:49 PM Bruce Liu <ccieliu@gmail.com> wrote:
>
> Hi Michal Kubecek and Network dev team,
>
> Good day! Hope you are doing well.
> This is Bruce from China, and please allow me to cc Rudy from Cisco Systems in China team.
>
> We are facing a weird behavior about "master-slave configuration" function in ethtool.
> Please correct me if I am wrong....
>
> As you know, start from ethtool 5.8,  "master/slave configuration support" added.
> https://lwn.net/Articles/828044/
>
> ========================================================================
> Appeal:
> Confirm and discuss workaround
>
> ========================================================================
> Issue description:
> As we test in lab, no "master-slave" option supported.
>
> ========================================================================
> Issue reproduce:
> root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
> ethtool: bad command line argument(s)
> For more information run ethtool -h
>
> ========================================================================
> Environment:
> debian-live-10.7.0-amd64-standard.iso
> Kernel 5.4.79
> ethtool 5.10
> Source code: https://mirrors.edge.kernel.org/pub/software/network/ethtool/ethtool-5.10.tar.xz
>
> ========================================================================
> Troubleshooting:
> root@raspberrypi:~# ethtool -h
> ethtool version 5.10
> Usage:
>         ethtool [ FLAGS ] DEVNAME       Display standard information about device
>         ethtool [ FLAGS ] -s|--change DEVNAME   Change generic options
>                 [ speed %d ]
>                 [ duplex half|full ]
>                 [ port tp|aui|bnc|mii|fibre|da ]
>                 [ mdix auto|on|off ]
>                 [ autoneg on|off ]
>                 [ advertise %x[/%x] | mode on|off ... [--] ]
>                 [ phyad %d ]
>                 [ xcvr internal|external ]
>                 [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]
>                 [ sopass %x:%x:%x:%x:%x:%x ]
>                 [ msglvl %d[/%d] | type on|off ... [--] ]
>                 [ master-slave master-preferred|slave-preferred|master-force|slave-force ]
>
> root@raspberrypi:~# ethtool -s eth0 [double tab here]
> advertise  autoneg    duplex     mdix       msglvl     phyad      port       speed      wol        xcvr
>
> ========================================
> Review 5.10 source code:
> ethtool.c line:5616
>
> static const struct option args[] = {
> {
> .opts = "-s|--change",
> .func = do_sset,
> .nlfunc = nl_sset,
> .help = "Change generic options",
> .xhelp = " [ speed %d ]\n"
>  " [ duplex half|full ]\n"
>  " [ port tp|aui|bnc|mii|fibre|da ]\n"
>  " [ mdix auto|on|off ]\n"
>  " [ autoneg on|off ]\n"
>  " [ advertise %x[/%x] | mode on|off ... [--] ]\n"
>  " [ phyad %d ]\n"
>  " [ xcvr internal|external ]\n"
>  " [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]\n"
>  " [ sopass %x:%x:%x:%x:%x:%x ]\n"
>  " [ msglvl %d[/%d] | type on|off ... [--] ]\n"
>  " [ master-slave master-preferred|slave-preferred|master-force|slave-force ]\n"
> },
>
> ========================================
> ethtool.c line:2912  do_sset function
> There is NOT an "else if" to catch "master-slave" option, and the options matched final else, and print an error message   "ethtool: bad command line argument(s)\n""For more information run ethtool -h\n""
>
> ethtool.c line: 3069
>
>   } else {
> exit_bad_args();
>
> ========================================
> root@raspberrypi:~# ethtool -s eth0 master-slave master-preferred
> ethtool: bad command line argument(s)
> For more information run ethtool -h
>
> Look forward to your reply.
>
> Cheers!
> Bruce Liu (UTC +08)
> Email: ccieliu@gmail.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-28 13:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-27  4:34 "ethtool" missing "master-slave" args in "do_sset" function.[TEXT/PLAIN] Bruce LIU
2020-12-27  9:46 ` Michal Kubecek
2020-12-28 13:35   ` Rudy Lei (shlei)
  -- strict thread matches above, loose matches on Subject: below --
2020-12-23  8:32 Bruce Liu
     [not found] <CAHZX3B_KBEzny0ejSP73q+1Y-n=f=39jTXjzBCDB8Lsrn5U2gg@mail.gmail.com>
2020-12-23  8:31 ` Bruce Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).