We keep expanding ethtool netlink api surface and this leads to
constantly playing catchup on the ynl spec side. There are a couple
of things that prevent us from fully converting to generating
the header from the spec (stats and cable tests), but we can
generate 95% of the header which is still better than maintaining
c header and spec separately. The series adds a couple of missing
features on the ynl-gen-c side and separates the parts
that we can generate into new ethtool_netlink_generated.h.

v3:
- s/Unsupported enum-model/Unsupported message enum-model/ (Jakub)
- add placeholder doc for header-flags (Jakub)

v2:
- attr-cnt-name -> enum-cnt-name (Jakub)
- add enum-cnt-name documentation (Jakub)
- __ETHTOOL_XXX_CNT -> __ethtool-xxx-cnt + c_upper (Jakub)
- keep and refine enum model check (Jakub)
- use 'header' presence as a signal to omit rendering instead of new
  'render' property (Jakub)
- new patch to reverse the order of header dependencies in xxx-user.h

Stanislav Fomichev (8):
  ynl: support enum-cnt-name attribute in legacy definitions
  ynl: skip rendering attributes with header property in uapi mode
  ynl: support directional specs in ynl-gen-c.py
  ynl: add missing pieces to ethtool spec to better match uapi header
  ynl: include uapi header after all dependencies
  ethtool: separate definitions that are gonna be generated
  ethtool: remove the comments that are not gonna be generated
  ethtool: regenerate uapi header from the spec

 Documentation/netlink/genetlink-c.yaml        |   3 +
 Documentation/netlink/genetlink-legacy.yaml   |   3 +
 Documentation/netlink/specs/ethtool.yaml      | 354 ++++++-
 .../userspace-api/netlink/c-code-gen.rst      |   4 +-
 MAINTAINERS                                   |   2 +-
 include/uapi/linux/ethtool_netlink.h          | 893 +-----------------
 .../uapi/linux/ethtool_netlink_generated.h    | 792 ++++++++++++++++
 tools/net/ynl/ynl-gen-c.py                    | 139 ++-
 8 files changed, 1250 insertions(+), 940 deletions(-)
 create mode 100644 include/uapi/linux/ethtool_netlink_generated.h

-- 
2.47.0

On Mon,  2 Dec 2024 08:29:28 -0800 Stanislav Fomichev wrote:
> We keep expanding ethtool netlink api surface and this leads to
> constantly playing catchup on the ynl spec side. There are a couple
> of things that prevent us from fully converting to generating
> the header from the spec (stats and cable tests), but we can
> generate 95% of the header which is still better than maintaining
> c header and spec separately. The series adds a couple of missing
> features on the ynl-gen-c side and separates the parts
> that we can generate into new ethtool_netlink_generated.h.
> 
> v3:
> - s/Unsupported enum-model/Unsupported message enum-model/ (Jakub)
> - add placeholder doc for header-flags (Jakub)
> 
> v2:
> - attr-cnt-name -> enum-cnt-name (Jakub)
> - add enum-cnt-name documentation (Jakub)
> - __ETHTOOL_XXX_CNT -> __ethtool-xxx-cnt + c_upper (Jakub)
> - keep and refine enum model check (Jakub)
> - use 'header' presence as a signal to omit rendering instead of new
>   'render' property (Jakub)
> - new patch to reverse the order of header dependencies in xxx-user.h
> 
> Stanislav Fomichev (8):
>   ynl: support enum-cnt-name attribute in legacy definitions
>   ynl: skip rendering attributes with header property in uapi mode
>   ynl: support directional specs in ynl-gen-c.py
>   ynl: add missing pieces to ethtool spec to better match uapi header
>   ynl: include uapi header after all dependencies
>   ethtool: separate definitions that are gonna be generated
>   ethtool: remove the comments that are not gonna be generated
>   ethtool: regenerate uapi header from the spec

Looks like doc codegen is unhappy about the missing type definitions:

Documentation/networking/netlink_spec/ethtool.rst:1122: WARNING: Bullet list ends without a blank line; unexpected unindent.
Documentation/networking/netlink_spec/ethtool.rst:2126: ERROR: Unknown target name: Documentation/networking/netlink_spec/ethtool.rst:2131: ERROR: Unknown target name: "ethtool_a_cable_result_code".
Documentation/networking/netlink_spec/ethtool.rst:2136: ERROR: Unknown target name: "ethtool_a_cable_inf_src".

We need to teach it to not link to external types?
-- 
pw-bot: cr

On 12/02, Jakub Kicinski wrote:
> On Mon,  2 Dec 2024 08:29:28 -0800 Stanislav Fomichev wrote:
> > We keep expanding ethtool netlink api surface and this leads to
> > constantly playing catchup on the ynl spec side. There are a couple
> > of things that prevent us from fully converting to generating
> > the header from the spec (stats and cable tests), but we can
> > generate 95% of the header which is still better than maintaining
> > c header and spec separately. The series adds a couple of missing
> > features on the ynl-gen-c side and separates the parts
> > that we can generate into new ethtool_netlink_generated.h.
> > 
> > v3:
> > - s/Unsupported enum-model/Unsupported message enum-model/ (Jakub)
> > - add placeholder doc for header-flags (Jakub)
> > 
> > v2:
> > - attr-cnt-name -> enum-cnt-name (Jakub)
> > - add enum-cnt-name documentation (Jakub)
> > - __ETHTOOL_XXX_CNT -> __ethtool-xxx-cnt + c_upper (Jakub)
> > - keep and refine enum model check (Jakub)
> > - use 'header' presence as a signal to omit rendering instead of new
> >   'render' property (Jakub)
> > - new patch to reverse the order of header dependencies in xxx-user.h
> > 
> > Stanislav Fomichev (8):
> >   ynl: support enum-cnt-name attribute in legacy definitions
> >   ynl: skip rendering attributes with header property in uapi mode
> >   ynl: support directional specs in ynl-gen-c.py
> >   ynl: add missing pieces to ethtool spec to better match uapi header
> >   ynl: include uapi header after all dependencies
> >   ethtool: separate definitions that are gonna be generated
> >   ethtool: remove the comments that are not gonna be generated
> >   ethtool: regenerate uapi header from the spec
> 
> Looks like doc codegen is unhappy about the missing type definitions:
> 
> Documentation/networking/netlink_spec/ethtool.rst:1122: WARNING: Bullet list ends without a blank line; unexpected unindent.
> Documentation/networking/netlink_spec/ethtool.rst:2126: ERROR: Unknown target name: Documentation/networking/netlink_spec/ethtool.rst:2131: ERROR: Unknown target name: "ethtool_a_cable_result_code".
> Documentation/networking/netlink_spec/ethtool.rst:2136: ERROR: Unknown target name: "ethtool_a_cable_inf_src".
> 
> We need to teach it to not link to external types?

The following calms it down on my side:

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index efa00665c191..859ae0cb1fd8 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -60,7 +60,8 @@ uapi-header: linux/ethtool_netlink_generated.h
     name-prefix: ethtool-c33-pse-ext-state-
     header: linux/ethtool.h
     entries:
-        - none
+        - name: none
+          doc: none
         -
           name: error-condition
           doc: Group of error_condition states
@@ -875,15 +876,15 @@ uapi-header: linux/ethtool_netlink_generated.h
         value: 0
       -
         name: pair
-        doc: ETHTOOL_A_CABLE_PAIR_
+        doc: ETHTOOL_A_CABLE_PAIR
         type: u8
       -
         name: code
-        doc: ETHTOOL_A_CABLE_RESULT_CODE_
+        doc: ETHTOOL_A_CABLE_RESULT_CODE
         type: u8
       -
         name: src
-        doc: ETHTOOL_A_CABLE_INF_SRC_
+        doc: ETHTOOL_A_CABLE_INF_SRC
         type: u32
   -
     name: cable-fault-length

The first one fixes the bullet list (seems like mixing entries with and
without docs confuses ynl-gen-rst.py). And removing trailing _ fixes the
rest (don't know why).

Any objections to folding it as is into v4? I can go on and try to
understand why ynl-gen-rst.py behaves exactly that way, but not sure
it would buy us anything?

On Mon, 2 Dec 2024 21:07:38 -0800 Stanislav Fomichev wrote:
> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> index efa00665c191..859ae0cb1fd8 100644
> --- a/Documentation/netlink/specs/ethtool.yaml
> +++ b/Documentation/netlink/specs/ethtool.yaml
> @@ -60,7 +60,8 @@ uapi-header: linux/ethtool_netlink_generated.h
>      name-prefix: ethtool-c33-pse-ext-state-
>      header: linux/ethtool.h
>      entries:
> -        - none
> +        - name: none
> +          doc: none
> 
> The first one fixes the bullet list (seems like mixing entries with and
> without docs confuses ynl-gen-rst.py). 

Ah, yes, that makes sense, either all entries should be objects or all
should be strings. I will spare you trying to figure out how to enforce
that in jsonschema :)

nit: "-" and "name: none" on separate lines

>          -
>            name: error-condition
>            doc: Group of error_condition states
> @@ -875,15 +876,15 @@ uapi-header: linux/ethtool_netlink_generated.h
>          value: 0
>        -
>          name: pair
> -        doc: ETHTOOL_A_CABLE_PAIR_
> +        doc: ETHTOOL_A_CABLE_PAIR
>          type: u8
>        -
>          name: code
> -        doc: ETHTOOL_A_CABLE_RESULT_CODE_
> +        doc: ETHTOOL_A_CABLE_RESULT_CODE
>          type: u8
>        -
>          name: src
> -        doc: ETHTOOL_A_CABLE_INF_SRC_
> +        doc: ETHTOOL_A_CABLE_INF_SRC
>          type: u32
>    -
>      name: cable-fault-length
>
> And removing trailing _ fixes the rest (don't know why).

Mmm. Trailing _ must mean something in ReST.
Can't decide now whether we care more about supporting ReST formatting
in YAML docs or protecting unsuspecting developers from this sort of a
surprise. So let's do the easier thing for now.

> Any objections to folding it as is into v4? I can go on and try to
> understand why ynl-gen-rst.py behaves exactly that way, but not sure
> it would buy us anything?

Yup, v4 with more or less the diff above SGTM!