linux-kernel - RE: [PATCH net-next] tools: ynl-gen: generate docs for <name>_max/

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 14 Jun 2023 12:48:14 +0000
From:   "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "chuck.lever@...cle.com" <chuck.lever@...cle.com>
Subject: RE: [PATCH net-next] tools: ynl-gen: generate docs for
 <name>_max/_mask enums

>From: Jakub Kicinski <kuba@...nel.org>
>Sent: Wednesday, June 14, 2023 2:59 AM
>
>On Wed, 14 Jun 2023 01:17:09 +0200 Arkadiusz Kubalewski wrote:
>> Including ynl generated uapi header files into source kerneldocs
>> (rst files in Documentation/) produces warnings during documentation
>> builds (i.e. make htmldocs)
>>
>> Prevent warnings by generating also description for enums where
>> rander_max was selected.
>
>Do you reckon that documenting the meta-values makes sense, or should
>we throw a:
>
>/* private: */
>

Most probably it doesn't..
Tried this:
/*
 [ other values description ]
 * private:
 * @__<NAME>_MAX
 */
and this:
/*
 [ other values description ]
 * private: @__<NAME>_MAX
 */

Both are not working as we would expect.

Do you mean to have double comments for enums? like:
/*
 [ other values description ]
 */
/*
 * private:
 * @__<NAME>_MAX
 */
 
>comment in front of them so that kdoc ignores them? Does user space
>have any use for those? If we want to document them...
>

Hmm, do you recall where I can find proper format of such ignore enum comment
for kdoc generation?
Or maybe we need to also submit patch to some kdoc build process to actually
change the current behavior?

>> diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
>> index 639524b59930..d78f7ae95092 100644
>> --- a/include/uapi/linux/netdev.h
>> +++ b/include/uapi/linux/netdev.h
>> @@ -24,6 +24,7 @@
>>   *   XDP buffer support in the driver napi callback.
>>   * @NETDEV_XDP_ACT_NDO_XMIT_SG: This feature informs if netdev implements
>>   *   non-linear XDP buffer support in ndo_xdp_xmit callback.
>> + * @NETDEV_XDP_ACT_MASK: valid values mask
>
>... I think we need to include some sort of indication that the value
>will change as the uAPI is extended. Unlike the other values which are
>set in stone, so to speak. "mask of currently defines values" ? Dunno.
>

Sure can fix this.

>>   */
>>  enum netdev_xdp_act {
>>  	NETDEV_XDP_ACT_BASIC = 1,
>> diff --git a/tools/include/uapi/linux/netdev.h
>>b/tools/include/uapi/linux/netdev.h
>> index 639524b59930..d78f7ae95092 100644
>> --- a/tools/include/uapi/linux/netdev.h
>> +++ b/tools/include/uapi/linux/netdev.h
>> @@ -24,6 +24,7 @@
>>   *   XDP buffer support in the driver napi callback.
>>   * @NETDEV_XDP_ACT_NDO_XMIT_SG: This feature informs if netdev
>>implements
>>   *   non-linear XDP buffer support in ndo_xdp_xmit callback.
>> + * @NETDEV_XDP_ACT_MASK: valid values mask
>>   */
>>  enum netdev_xdp_act {
>>  	NETDEV_XDP_ACT_BASIC = 1,
>> diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
>> index 0b5e0802a9a7..0d396bf98c27 100755
>> --- a/tools/net/ynl/ynl-gen-c.py
>> +++ b/tools/net/ynl/ynl-gen-c.py
>> @@ -2011,6 +2011,7 @@ def render_uapi(family, cw):
>>          # Write kdoc for enum and flags (one day maybe also structs)
>>          if const['type'] == 'enum' or const['type'] == 'flags':
>>              enum = family.consts[const['name']]
>> +            name_pfx = const.get('name-prefix', f"{family.name}-
>>{const['name']}-")
>>
>>              if enum.has_doc():
>>                  cw.p('/**')
>> @@ -2022,10 +2023,18 @@ def render_uapi(family, cw):
>>                      if entry.has_doc():
>>                          doc = '@' + entry.c_name + ': ' + entry['doc']
>>                          cw.write_doc_line(doc)
>> +                if const.get('render-max', False):
>> +                    if const['type'] == 'flags':
>> +                        doc = '@' + c_upper(name_pfx + 'mask') + ':
>>valid values mask'
>> +                        cw.write_doc_line(doc)
>> +                    else:
>> +                        doc = '@' + c_upper(name_pfx + 'max') + ': max
>>valid value'
>> +                        cw.write_doc_line(doc)
>> +                        doc = '@__' + c_upper(name_pfx + 'max') + ': do
>>not use'
>
>This one is definitely a candidate for /* private: */

Yep, makes sense, trying to find some way...

Thank you,
Arkadiusz

>
>> +                        cw.write_doc_line(doc)
>>                  cw.p(' */')
>>
>>              uapi_enum_start(family, cw, const, 'name')
>> -            name_pfx = const.get('name-prefix', f"{family.name}-
>{const['name']}-")
>>              for entry in enum.entries.values():
>>                  suffix = ','
>>                  if entry.value_change:
>
>--
>pw-bot: cr