All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Davide Caratti <dcaratti@redhat.com>
Cc: Mat Martineau <martineau@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Matthieu Baerts <matttbe@kernel.org>,
	netdev@vger.kernel.org, mptcp@lists.linux.dev,
	Simon Horman <horms@kernel.org>
Subject: Re: [PATCH net-next v2 5/7] uapi: mptcp: use header file generated from YAML spec
Date: Wed, 25 Oct 2023 11:18:41 -0700	[thread overview]
Message-ID: <20231025111841.73a40904@kernel.org> (raw)
In-Reply-To: <CAKa-r6vCj+gPEUKpv7AsXqM77N6pB0evuh7myHq=585RA3oD5g@mail.gmail.com>

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

  reply	other threads:[~2023-10-25 18:18 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-23 18:17 [PATCH net-next v2 0/7] mptcp: convert Netlink code to use YAML spec Mat Martineau
2023-10-23 18:17 ` [PATCH net-next v2 1/7] tools: ynl: add uns-admin-perm to genetlink legacy Mat Martineau
2023-10-23 18:17 ` [PATCH net-next v2 2/7] tools: ynl-gen: add support for exact-len validation Mat Martineau
2023-10-23 18:17 ` [PATCH net-next v2 3/7] net: mptcp: convert netlink from small_ops to ops Mat Martineau
2023-10-23 18:17 ` [PATCH net-next v2 4/7] Documentation: netlink: add a YAML spec for mptcp Mat Martineau
2023-10-23 18:17 ` [PATCH net-next v2 5/7] uapi: mptcp: use header file generated from YAML spec Mat Martineau
2023-10-24 19:59   ` Jakub Kicinski
2023-10-24 23:30     ` Mat Martineau
2023-10-24 23:49       ` Jakub Kicinski
2023-10-24 23:50         ` Jakub Kicinski
2023-10-25 16:40         ` Davide Caratti
2023-10-25 18:18           ` Jakub Kicinski [this message]
2023-10-23 18:17 ` [PATCH net-next v2 6/7] net: mptcp: rename netlink handlers to mptcp_pm_nl_<blah>_{doit,dumpit} Mat Martineau
2023-10-23 18:17 ` [PATCH net-next v2 7/7] net: mptcp: use policy generated by YAML spec Mat Martineau
2023-10-24 20:10 ` [PATCH net-next v2 0/7] mptcp: convert Netlink code to use " patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231025111841.73a40904@kernel.org \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dcaratti@redhat.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=martineau@kernel.org \
    --cc=matttbe@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.