mbox series

[ethtool,v2,00/25] initial netlink interface implementation for 5.6 release

Message ID cover.1583347351.git.mkubecek@suse.cz
Headers show
Series initial netlink interface implementation for 5.6 release | expand

Message

Michal Kubecek March 4, 2020, 8:24 p.m. UTC
This series adds initial support for ethtool netlink interface provided by
kernel since 5.6-rc1. The traditional ioctl interface is still supported
for compatibility with older kernels. The netlink interface and message
formats are documented in Documentation/networking/ethtool-netlink.rst file
in kernel source tree.

Netlink interface is preferred but ethtool falls back to ioctl if netlink
interface is not available (i.e. the "ethtool" genetlink family is not
registered). It also falls back if a particular command is not implemented
in netlink (kernel returns -EOPNOTSUPP). This allows new ethtool versions
to work with older kernel versions while support for ethool commands is
added in steps.

The series aims to touch existing ioctl code as little as possible in the
first phase to minimize the risk of introducing regressions. It is also
possible to build ethtool without netlink support if --disable-netlink is
passed to configure script. The most visible changes to existing code are

  - UAPI header copies are moved to uapi/ under original names
  - some variables and functions which are going to be shared with netlink
    code are moved from ethtool.c to common.c and common.h
  - args[] array in ethtool.c was rewritten to use named initializers

Except for changes to main(), all netlink specific code is in a separate
directory netlink/ and is divided into multiple files.

Changes in v2:
- add support for permanent hardware addres ("ethtool -P", patch 20)
- add support for pretty printing of netlink messages (patches 21-25)
- make output of "ethtool <dev>" closer to ioctl implementation
- two more kernel uapi header copies (patch 5)
- support for rtnetlink socket and requests (needed for "ethtool -P")
- some kerneldoc style comments

Michal Kubecek (25):
  move UAPI header copies to a separate directory
  update UAPI header copies
  add --debug option to control debugging messages
  use named initializers in command line option list
  netlink: add netlink related UAPI header files
  netlink: introduce the netlink interface
  netlink: message buffer and composition helpers
  netlink: netlink socket wrapper and helpers
  netlink: initialize ethtool netlink socket
  netlink: add support for string sets
  netlink: add notification monitor
  move shared code into a common file
  netlink: add bitset helpers
  netlink: partial netlink handler for gset (no option)
  netlink: support getting wake-on-lan and debugging settings
  netlink: add basic command line parsing helpers
  netlink: add bitset command line parser handlers
  netlink: add netlink handler for sset (-s)
  netlink: support tests with netlink enabled
  netlink: add handler for permaddr (-P)
  netlink: support for pretty printing netlink messages
  netlink: message format description for ethtool netlink
  netlink: message format descriptions for genetlink control
  netlink: message format descriptions for rtnetlink
  netlink: use pretty printing for ethtool netlink messages

 Makefile.am                                  |   31 +-
 common.c                                     |  145 +++
 common.h                                     |   26 +
 configure.ac                                 |   14 +-
 ethtool.8.in                                 |   48 +-
 ethtool.c                                    |  819 ++++++++------
 internal.h                                   |   31 +-
 netlink/bitset.c                             |  218 ++++
 netlink/bitset.h                             |   26 +
 netlink/desc-ethtool.c                       |  139 +++
 netlink/desc-genlctrl.c                      |   56 +
 netlink/desc-rtnl.c                          |   96 ++
 netlink/extapi.h                             |   46 +
 netlink/monitor.c                            |  229 ++++
 netlink/msgbuff.c                            |  255 +++++
 netlink/msgbuff.h                            |  117 ++
 netlink/netlink.c                            |  216 ++++
 netlink/netlink.h                            |   87 ++
 netlink/nlsock.c                             |  405 +++++++
 netlink/nlsock.h                             |   45 +
 netlink/parser.c                             | 1058 ++++++++++++++++++
 netlink/parser.h                             |  144 +++
 netlink/permaddr.c                           |  114 ++
 netlink/prettymsg.c                          |  237 ++++
 netlink/prettymsg.h                          |  118 ++
 netlink/settings.c                           |  955 ++++++++++++++++
 netlink/strset.c                             |  297 +++++
 netlink/strset.h                             |   25 +
 test-cmdline.c                               |   29 +-
 test-features.c                              |   11 +
 ethtool-copy.h => uapi/linux/ethtool.h       |   17 +
 uapi/linux/ethtool_netlink.h                 |  237 ++++
 uapi/linux/genetlink.h                       |   89 ++
 uapi/linux/if_link.h                         | 1051 +++++++++++++++++
 net_tstamp-copy.h => uapi/linux/net_tstamp.h |   27 +
 uapi/linux/netlink.h                         |  248 ++++
 uapi/linux/rtnetlink.h                       |  777 +++++++++++++
 37 files changed, 8102 insertions(+), 381 deletions(-)
 create mode 100644 common.c
 create mode 100644 common.h
 create mode 100644 netlink/bitset.c
 create mode 100644 netlink/bitset.h
 create mode 100644 netlink/desc-ethtool.c
 create mode 100644 netlink/desc-genlctrl.c
 create mode 100644 netlink/desc-rtnl.c
 create mode 100644 netlink/extapi.h
 create mode 100644 netlink/monitor.c
 create mode 100644 netlink/msgbuff.c
 create mode 100644 netlink/msgbuff.h
 create mode 100644 netlink/netlink.c
 create mode 100644 netlink/netlink.h
 create mode 100644 netlink/nlsock.c
 create mode 100644 netlink/nlsock.h
 create mode 100644 netlink/parser.c
 create mode 100644 netlink/parser.h
 create mode 100644 netlink/permaddr.c
 create mode 100644 netlink/prettymsg.c
 create mode 100644 netlink/prettymsg.h
 create mode 100644 netlink/settings.c
 create mode 100644 netlink/strset.c
 create mode 100644 netlink/strset.h
 rename ethtool-copy.h => uapi/linux/ethtool.h (99%)
 create mode 100644 uapi/linux/ethtool_netlink.h
 create mode 100644 uapi/linux/genetlink.h
 create mode 100644 uapi/linux/if_link.h
 rename net_tstamp-copy.h => uapi/linux/net_tstamp.h (84%)
 create mode 100644 uapi/linux/netlink.h
 create mode 100644 uapi/linux/rtnetlink.h

Comments

Michal Kubecek March 4, 2020, 8:34 p.m. UTC | #1
John,

If you would prefer that, feel free to use v1 and I'll send the later
updates as a shorter follow-up series.

Michal
John W. Linville March 5, 2020, 7:24 p.m. UTC | #2
On Wed, Mar 04, 2020 at 09:24:35PM +0100, Michal Kubecek wrote:
> This series adds initial support for ethtool netlink interface provided by
> kernel since 5.6-rc1. The traditional ioctl interface is still supported
> for compatibility with older kernels. The netlink interface and message
> formats are documented in Documentation/networking/ethtool-netlink.rst file
> in kernel source tree.
> 
> Netlink interface is preferred but ethtool falls back to ioctl if netlink
> interface is not available (i.e. the "ethtool" genetlink family is not
> registered). It also falls back if a particular command is not implemented
> in netlink (kernel returns -EOPNOTSUPP). This allows new ethtool versions
> to work with older kernel versions while support for ethool commands is
> added in steps.
> 
> The series aims to touch existing ioctl code as little as possible in the
> first phase to minimize the risk of introducing regressions. It is also
> possible to build ethtool without netlink support if --disable-netlink is
> passed to configure script. The most visible changes to existing code are
> 
>   - UAPI header copies are moved to uapi/ under original names
>   - some variables and functions which are going to be shared with netlink
>     code are moved from ethtool.c to common.c and common.h
>   - args[] array in ethtool.c was rewritten to use named initializers
> 
> Except for changes to main(), all netlink specific code is in a separate
> directory netlink/ and is divided into multiple files.
> 
> Changes in v2:
> - add support for permanent hardware addres ("ethtool -P", patch 20)
> - add support for pretty printing of netlink messages (patches 21-25)
> - make output of "ethtool <dev>" closer to ioctl implementation
> - two more kernel uapi header copies (patch 5)
> - support for rtnetlink socket and requests (needed for "ethtool -P")
> - some kerneldoc style comments
> 
> Michal Kubecek (25):
>   move UAPI header copies to a separate directory
>   update UAPI header copies
>   add --debug option to control debugging messages
>   use named initializers in command line option list
>   netlink: add netlink related UAPI header files
>   netlink: introduce the netlink interface
>   netlink: message buffer and composition helpers
>   netlink: netlink socket wrapper and helpers
>   netlink: initialize ethtool netlink socket
>   netlink: add support for string sets
>   netlink: add notification monitor
>   move shared code into a common file
>   netlink: add bitset helpers
>   netlink: partial netlink handler for gset (no option)
>   netlink: support getting wake-on-lan and debugging settings
>   netlink: add basic command line parsing helpers
>   netlink: add bitset command line parser handlers
>   netlink: add netlink handler for sset (-s)
>   netlink: support tests with netlink enabled
>   netlink: add handler for permaddr (-P)
>   netlink: support for pretty printing netlink messages
>   netlink: message format description for ethtool netlink
>   netlink: message format descriptions for genetlink control
>   netlink: message format descriptions for rtnetlink
>   netlink: use pretty printing for ethtool netlink messages
> 
>  Makefile.am                                  |   31 +-
>  common.c                                     |  145 +++
>  common.h                                     |   26 +
>  configure.ac                                 |   14 +-
>  ethtool.8.in                                 |   48 +-
>  ethtool.c                                    |  819 ++++++++------
>  internal.h                                   |   31 +-
>  netlink/bitset.c                             |  218 ++++
>  netlink/bitset.h                             |   26 +
>  netlink/desc-ethtool.c                       |  139 +++
>  netlink/desc-genlctrl.c                      |   56 +
>  netlink/desc-rtnl.c                          |   96 ++
>  netlink/extapi.h                             |   46 +
>  netlink/monitor.c                            |  229 ++++
>  netlink/msgbuff.c                            |  255 +++++
>  netlink/msgbuff.h                            |  117 ++
>  netlink/netlink.c                            |  216 ++++
>  netlink/netlink.h                            |   87 ++
>  netlink/nlsock.c                             |  405 +++++++
>  netlink/nlsock.h                             |   45 +
>  netlink/parser.c                             | 1058 ++++++++++++++++++
>  netlink/parser.h                             |  144 +++
>  netlink/permaddr.c                           |  114 ++
>  netlink/prettymsg.c                          |  237 ++++
>  netlink/prettymsg.h                          |  118 ++
>  netlink/settings.c                           |  955 ++++++++++++++++
>  netlink/strset.c                             |  297 +++++
>  netlink/strset.h                             |   25 +
>  test-cmdline.c                               |   29 +-
>  test-features.c                              |   11 +
>  ethtool-copy.h => uapi/linux/ethtool.h       |   17 +
>  uapi/linux/ethtool_netlink.h                 |  237 ++++
>  uapi/linux/genetlink.h                       |   89 ++
>  uapi/linux/if_link.h                         | 1051 +++++++++++++++++
>  net_tstamp-copy.h => uapi/linux/net_tstamp.h |   27 +
>  uapi/linux/netlink.h                         |  248 ++++
>  uapi/linux/rtnetlink.h                       |  777 +++++++++++++
>  37 files changed, 8102 insertions(+), 381 deletions(-)
>  create mode 100644 common.c
>  create mode 100644 common.h
>  create mode 100644 netlink/bitset.c
>  create mode 100644 netlink/bitset.h
>  create mode 100644 netlink/desc-ethtool.c
>  create mode 100644 netlink/desc-genlctrl.c
>  create mode 100644 netlink/desc-rtnl.c
>  create mode 100644 netlink/extapi.h
>  create mode 100644 netlink/monitor.c
>  create mode 100644 netlink/msgbuff.c
>  create mode 100644 netlink/msgbuff.h
>  create mode 100644 netlink/netlink.c
>  create mode 100644 netlink/netlink.h
>  create mode 100644 netlink/nlsock.c
>  create mode 100644 netlink/nlsock.h
>  create mode 100644 netlink/parser.c
>  create mode 100644 netlink/parser.h
>  create mode 100644 netlink/permaddr.c
>  create mode 100644 netlink/prettymsg.c
>  create mode 100644 netlink/prettymsg.h
>  create mode 100644 netlink/settings.c
>  create mode 100644 netlink/strset.c
>  create mode 100644 netlink/strset.h
>  rename ethtool-copy.h => uapi/linux/ethtool.h (99%)
>  create mode 100644 uapi/linux/ethtool_netlink.h
>  create mode 100644 uapi/linux/genetlink.h
>  create mode 100644 uapi/linux/if_link.h
>  rename net_tstamp-copy.h => uapi/linux/net_tstamp.h (84%)
>  create mode 100644 uapi/linux/netlink.h
>  create mode 100644 uapi/linux/rtnetlink.h
> 
> -- 
> 2.25.1

Just a quick check -- executing "./autogen.sh ; ./configure ; make
distcheck" fails with the attached log output.

John
Michal Kubecek March 5, 2020, 8:32 p.m. UTC | #3
On Thu, Mar 05, 2020 at 02:24:16PM -0500, John W. Linville wrote:
> 
> Just a quick check -- executing "./autogen.sh ; ./configure ; make
> distcheck" fails with the attached log output.
[...]
> make[2]: Entering directory '/home/linville/git/ethtool/ethtool-5.4/_build/sub'
> gcc -DHAVE_CONFIG_H -I. -I../..    -I./uapi -Wall  -g -O2 -MT ethtool-ethtool.o -MD -MP -MF .deps/ethtool-ethtool.Tpo -c -o ethtool-ethtool.o `test -f 'ethtool.c' || echo '../../'`ethtool.c

I can see what is going on: this runs in subdirectory and correctly adds
"-I../.." but not "-I../../uapi". I'm afraid I'll have to dive into
automake documentation to see how to make it adjust that path as well.

> ../../ethtool.c: In function ‘do_get_phy_tunable’:
> ../../ethtool.c:4773:16: error: ‘ETHTOOL_PHY_EDPD’ undeclared (first use in this function); did you mean ‘ETHTOOL_PHYS_ID’?
>  4773 |   cont.ds.id = ETHTOOL_PHY_EDPD;
>       |                ^~~~~~~~~~~~~~~~
>       |                ETHTOOL_PHYS_ID

This is a result of the missing include path above: instead of
up-to-date uapi/linux/ethtool.h, older system file from /usr/include is
used so that new additions are missing. I have many more errors like
this and when I tried to rename /usr/include/linux/ethtool.h, the build
failed with

  ../../internal.h:56:10: fatal error: linux/ethtool.h: No such file or directory

I'll try to find what is the right way to add an include directory,
adding "-I./uapi" to AM_CFLAGS did the trick for regular build but
clearly isn't sufficient for other targets.

Michal
Michal Kubecek March 5, 2020, 9:14 p.m. UTC | #4
On Thu, Mar 05, 2020 at 09:32:59PM +0100, Michal Kubecek wrote:
> On Thu, Mar 05, 2020 at 02:24:16PM -0500, John W. Linville wrote:
> > 
> > Just a quick check -- executing "./autogen.sh ; ./configure ; make
> > distcheck" fails with the attached log output.
> [...]
> > make[2]: Entering directory '/home/linville/git/ethtool/ethtool-5.4/_build/sub'
> > gcc -DHAVE_CONFIG_H -I. -I../..    -I./uapi -Wall  -g -O2 -MT ethtool-ethtool.o -MD -MP -MF .deps/ethtool-ethtool.Tpo -c -o ethtool-ethtool.o `test -f 'ethtool.c' || echo '../../'`ethtool.c
> 
> I can see what is going on: this runs in subdirectory and correctly adds
> "-I../.." but not "-I../../uapi". I'm afraid I'll have to dive into
> automake documentation to see how to make it adjust that path as well.
> 
> > ../../ethtool.c: In function ‘do_get_phy_tunable’:
> > ../../ethtool.c:4773:16: error: ‘ETHTOOL_PHY_EDPD’ undeclared (first use in this function); did you mean ‘ETHTOOL_PHYS_ID’?
> >  4773 |   cont.ds.id = ETHTOOL_PHY_EDPD;
> >       |                ^~~~~~~~~~~~~~~~
> >       |                ETHTOOL_PHYS_ID
> 
> This is a result of the missing include path above: instead of
> up-to-date uapi/linux/ethtool.h, older system file from /usr/include is
> used so that new additions are missing. I have many more errors like
> this and when I tried to rename /usr/include/linux/ethtool.h, the build
> failed with
> 
>   ../../internal.h:56:10: fatal error: linux/ethtool.h: No such file or directory
> 
> I'll try to find what is the right way to add an include directory,
> adding "-I./uapi" to AM_CFLAGS did the trick for regular build but
> clearly isn't sufficient for other targets.

The change below seems to fix the problem but I'll run some more tests
tomorrow to see that it does not break anything.

Michal

diff --git a/Makefile.am b/Makefile.am
index 2fd79eb8c79a..eae5a55ce933 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,4 +1,5 @@
-AM_CFLAGS = -I./uapi -Wall
+AM_CFLAGS = -Wall
+AM_CPPFLAGS = -I$(top_srcdir)/uapi
 LDADD = -lm
 
 man_MANS = ethtool.8