netdev - Re: [PATCH net-next v2 5/7] uapi: mptcp: use header file generated from YAML spec

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20231025111841.73a40904@kernel.org>
Date: Wed, 25 Oct 2023 11:18:41 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Davide Caratti <dcaratti@...hat.com>
Cc: Mat Martineau <martineau@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Matthieu Baerts <matttbe@...nel.org>,
 netdev@...r.kernel.org, mptcp@...ts.linux.dev, Simon Horman
 <horms@...nel.org>
Subject: Re: [PATCH net-next v2 5/7] uapi: mptcp: use header file generated
 from YAML spec

On Wed, 25 Oct 2023 18:40:52 +0200 Davide Caratti wrote:
> > > Do you want to intentionally move to the normal naming or would you
> > > prefer to keep the old names?  
> 
> given that nobody should use them, I'd prefer to move to the normal
> naming and drop the old definitions (_MPTCP_PM_CMD_AFTER_LAST and
> __MPTCP_ATTR_AFTER_LAST). I was unsure if I could do the drop thing
> actually, because applications using them would break the build then _
> hence these two "backward compatibility" lines.

It's up to you. Only mention on GitHub I see is this:
https://github.com/ngi-mptcp/mptcpanalyzer/blob/d6f5a4a61235f40dd17b1ef394a91ec50eda53f7/mptcp-pm/src/Net/Mptcp/V0/Constants.chs#L34
No idea what it is and whether the define disappearing will break it.

If you're confident that no code will break we can rename.
The downside (other than an angry user) is that if someone reports
breakage late we may need to keep both names, to avoid breaking any
code created in between..

> For the operation list, I see it's about exposing
> 
> cmd-cnt-name
> 
> to [ge]netlink*.yaml, and then do:
> 
>   9 max-by-define: true
>  10 kernel-policy: per-op
>  11 cmd-cnt-name: --mptcp-pm-cmd-after-last    <-- this
>  12
>  13 definitions:
> 
> the generated MPTCP #define(s) are the same as the ones we have in
> net-next now: no need to specify __MPTCP_PM_CMD_MAX anymore.

Ah, I was looking at the documentation which is clearly out of date
already..

> For the attributes, I thought I could use  'attr-cnt-name' like:
> 
> 169     name: attr
> 170     name-prefix: mptcp-pm-attr-
> 171     attr-cnt-name: --mptcp-attr-after-last <-- this
> 172     attributes:
> 
> as described in the [ge]netlink schema, but the tool seems to just ignore it.

Mm. Looks like we only use this one at the family level.

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 1c7474ad92dc..f9010fbbfdfd 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -789,9 +789,12 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                 pfx = f"{family.name}-a-{self.name}-"
             self.name_prefix = c_upper(pfx)
             self.max_name = c_upper(self.yaml.get('attr-max-name', f"{self.name_prefix}max"))
+            cnt_name = family.get('attr-cnt-name', f"__{self.name_prefix}MAX")
+            self.cnt_name = c_upper(self.yaml.get('attr-cnt-name', cnt_name))
         else:
             self.name_prefix = family.attr_sets[self.subset_of].name_prefix
             self.max_name = family.attr_sets[self.subset_of].max_name
+            self.cnt_name = family.attr_sets[self.subset_of].cnt_name

         # Added by resolve:
         self.c_name = None
@@ -2354,7 +2357,7 @@ _C_KW = {
         if attr_set.subset_of:
             continue

-        cnt_name = c_upper(family.get('attr-cnt-name', f"__{attr_set.name_prefix}MAX"))
+        cnt_name = attr_set.cnt_name
         max_value = f"({cnt_name} - 1)"

         val = 0