[PATCH net-next 0/6] tools: ynl-gen: generate flags better

[PATCH net-next 0/6] tools: ynl-gen: generate flags better

[Asbjørn Sloth Tønnesen]

This series focusses on increasing the quality of
the C code generated by ynl-gen for flags.

NB: I included a note in patch 6, on usage of the private
NETDEV_XDP_ACT_MASK in user-space.

Asbjørn Sloth Tønnesen (6):
  tools: ynl-gen: bitshift the flag values in the generated code
  tools: ynl-gen: refactor render-max enum generation
  tools: ynl-gen: use uapi mask definition in NLA_POLICY_MASK
  tools: ynl-gen: add generic p_wrap() helper
  tools: ynl-gen: construct bitflag masks in generated headers
  tools: ynl-gen: allow custom naming of render-max definitions

 Documentation/netlink/genetlink-c.yaml        |  3 +
 Documentation/netlink/genetlink-legacy.yaml   |  3 +
 .../userspace-api/netlink/c-code-gen.rst      |  7 +-
 include/uapi/linux/dpll.h                     |  6 +-
 .../uapi/linux/ethtool_netlink_generated.h    | 20 ++---
 include/uapi/linux/netdev.h                   | 34 ++++----
 net/psp/psp-nl-gen.h                          |  4 +-
 tools/include/uapi/linux/netdev.h             | 34 ++++----
 tools/net/ynl/pyynl/lib/nlspec.py             |  7 +-
 tools/net/ynl/pyynl/ynl_gen_c.py              | 79 +++++++++++--------
 10 files changed, 117 insertions(+), 80 deletions(-)

-- 
2.51.0

[PATCH net-next 1/6] tools: ynl-gen: bitshift the flag values in the generated code

[Asbjørn Sloth Tønnesen]

Instead of pre-computing the flag values within the code generator,
then move the bitshift operation into the generated code.

This IMHO makes the generated code read more like handwritten code.

No functional changes.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 include/uapi/linux/dpll.h                     |  6 ++--
 .../uapi/linux/ethtool_netlink_generated.h    | 20 ++++++-------
 include/uapi/linux/netdev.h                   | 28 +++++++++----------
 tools/include/uapi/linux/netdev.h             | 28 +++++++++----------
 tools/net/ynl/pyynl/lib/nlspec.py             |  7 +++--
 tools/net/ynl/pyynl/ynl_gen_c.py              |  2 +-
 6 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
index ab1725a954d74..28c9c8e5761b4 100644
--- a/include/uapi/linux/dpll.h
+++ b/include/uapi/linux/dpll.h
@@ -185,9 +185,9 @@ enum dpll_pin_state {
  * @DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE: pin state can be changed
  */
 enum dpll_pin_capabilities {
-	DPLL_PIN_CAPABILITIES_DIRECTION_CAN_CHANGE = 1,
-	DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE = 2,
-	DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE = 4,
+	DPLL_PIN_CAPABILITIES_DIRECTION_CAN_CHANGE = 1U << 0,
+	DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE = 1U << 1,
+	DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE = 1U << 2,
 };
 
 #define DPLL_PHASE_OFFSET_DIVIDER	1000
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index 0e8ac0d974e20..14c9baacde0e8 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -26,9 +26,9 @@ enum {
  * @ETHTOOL_FLAG_STATS: request statistics, if supported by the driver
  */
 enum ethtool_header_flags {
-	ETHTOOL_FLAG_COMPACT_BITSETS = 1,
-	ETHTOOL_FLAG_OMIT_REPLY = 2,
-	ETHTOOL_FLAG_STATS = 4,
+	ETHTOOL_FLAG_COMPACT_BITSETS = 1U << 0,
+	ETHTOOL_FLAG_OMIT_REPLY = 1U << 1,
+	ETHTOOL_FLAG_STATS = 1U << 2,
 };
 
 enum ethtool_tcp_data_split {
@@ -68,13 +68,13 @@ enum hwtstamp_source {
  *   power control from software
  */
 enum ethtool_pse_event {
-	ETHTOOL_PSE_EVENT_OVER_CURRENT = 1,
-	ETHTOOL_PSE_EVENT_OVER_TEMP = 2,
-	ETHTOOL_C33_PSE_EVENT_DETECTION = 4,
-	ETHTOOL_C33_PSE_EVENT_CLASSIFICATION = 8,
-	ETHTOOL_C33_PSE_EVENT_DISCONNECTION = 16,
-	ETHTOOL_PSE_EVENT_OVER_BUDGET = 32,
-	ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR = 64,
+	ETHTOOL_PSE_EVENT_OVER_CURRENT = 1U << 0,
+	ETHTOOL_PSE_EVENT_OVER_TEMP = 1U << 1,
+	ETHTOOL_C33_PSE_EVENT_DETECTION = 1U << 2,
+	ETHTOOL_C33_PSE_EVENT_CLASSIFICATION = 1U << 3,
+	ETHTOOL_C33_PSE_EVENT_DISCONNECTION = 1U << 4,
+	ETHTOOL_PSE_EVENT_OVER_BUDGET = 1U << 5,
+	ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR = 1U << 6,
 };
 
 enum {
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 48eb49aa03d41..db0526cb6672d 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -26,13 +26,13 @@
  *   non-linear XDP buffer support in ndo_xdp_xmit callback.
  */
 enum netdev_xdp_act {
-	NETDEV_XDP_ACT_BASIC = 1,
-	NETDEV_XDP_ACT_REDIRECT = 2,
-	NETDEV_XDP_ACT_NDO_XMIT = 4,
-	NETDEV_XDP_ACT_XSK_ZEROCOPY = 8,
-	NETDEV_XDP_ACT_HW_OFFLOAD = 16,
-	NETDEV_XDP_ACT_RX_SG = 32,
-	NETDEV_XDP_ACT_NDO_XMIT_SG = 64,
+	NETDEV_XDP_ACT_BASIC = 1U << 0,
+	NETDEV_XDP_ACT_REDIRECT = 1U << 1,
+	NETDEV_XDP_ACT_NDO_XMIT = 1U << 2,
+	NETDEV_XDP_ACT_XSK_ZEROCOPY = 1U << 3,
+	NETDEV_XDP_ACT_HW_OFFLOAD = 1U << 4,
+	NETDEV_XDP_ACT_RX_SG = 1U << 5,
+	NETDEV_XDP_ACT_NDO_XMIT_SG = 1U << 6,
 
 	/* private: */
 	NETDEV_XDP_ACT_MASK = 127,
@@ -48,9 +48,9 @@ enum netdev_xdp_act {
  *   packet VLAN tag via bpf_xdp_metadata_rx_vlan_tag().
  */
 enum netdev_xdp_rx_metadata {
-	NETDEV_XDP_RX_METADATA_TIMESTAMP = 1,
-	NETDEV_XDP_RX_METADATA_HASH = 2,
-	NETDEV_XDP_RX_METADATA_VLAN_TAG = 4,
+	NETDEV_XDP_RX_METADATA_TIMESTAMP = 1U << 0,
+	NETDEV_XDP_RX_METADATA_HASH = 1U << 1,
+	NETDEV_XDP_RX_METADATA_VLAN_TAG = 1U << 2,
 };
 
 /**
@@ -63,9 +63,9 @@ enum netdev_xdp_rx_metadata {
  *   by the driver.
  */
 enum netdev_xsk_flags {
-	NETDEV_XSK_FLAGS_TX_TIMESTAMP = 1,
-	NETDEV_XSK_FLAGS_TX_CHECKSUM = 2,
-	NETDEV_XSK_FLAGS_TX_LAUNCH_TIME_FIFO = 4,
+	NETDEV_XSK_FLAGS_TX_TIMESTAMP = 1U << 0,
+	NETDEV_XSK_FLAGS_TX_CHECKSUM = 1U << 1,
+	NETDEV_XSK_FLAGS_TX_LAUNCH_TIME_FIFO = 1U << 2,
 };
 
 enum netdev_queue_type {
@@ -74,7 +74,7 @@ enum netdev_queue_type {
 };
 
 enum netdev_qstats_scope {
-	NETDEV_QSTATS_SCOPE_QUEUE = 1,
+	NETDEV_QSTATS_SCOPE_QUEUE = 1U << 0,
 };
 
 enum netdev_napi_threaded {
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index 48eb49aa03d41..db0526cb6672d 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -26,13 +26,13 @@
  *   non-linear XDP buffer support in ndo_xdp_xmit callback.
  */
 enum netdev_xdp_act {
-	NETDEV_XDP_ACT_BASIC = 1,
-	NETDEV_XDP_ACT_REDIRECT = 2,
-	NETDEV_XDP_ACT_NDO_XMIT = 4,
-	NETDEV_XDP_ACT_XSK_ZEROCOPY = 8,
-	NETDEV_XDP_ACT_HW_OFFLOAD = 16,
-	NETDEV_XDP_ACT_RX_SG = 32,
-	NETDEV_XDP_ACT_NDO_XMIT_SG = 64,
+	NETDEV_XDP_ACT_BASIC = 1U << 0,
+	NETDEV_XDP_ACT_REDIRECT = 1U << 1,
+	NETDEV_XDP_ACT_NDO_XMIT = 1U << 2,
+	NETDEV_XDP_ACT_XSK_ZEROCOPY = 1U << 3,
+	NETDEV_XDP_ACT_HW_OFFLOAD = 1U << 4,
+	NETDEV_XDP_ACT_RX_SG = 1U << 5,
+	NETDEV_XDP_ACT_NDO_XMIT_SG = 1U << 6,
 
 	/* private: */
 	NETDEV_XDP_ACT_MASK = 127,
@@ -48,9 +48,9 @@ enum netdev_xdp_act {
  *   packet VLAN tag via bpf_xdp_metadata_rx_vlan_tag().
  */
 enum netdev_xdp_rx_metadata {
-	NETDEV_XDP_RX_METADATA_TIMESTAMP = 1,
-	NETDEV_XDP_RX_METADATA_HASH = 2,
-	NETDEV_XDP_RX_METADATA_VLAN_TAG = 4,
+	NETDEV_XDP_RX_METADATA_TIMESTAMP = 1U << 0,
+	NETDEV_XDP_RX_METADATA_HASH = 1U << 1,
+	NETDEV_XDP_RX_METADATA_VLAN_TAG = 1U << 2,
 };
 
 /**
@@ -63,9 +63,9 @@ enum netdev_xdp_rx_metadata {
  *   by the driver.
  */
 enum netdev_xsk_flags {
-	NETDEV_XSK_FLAGS_TX_TIMESTAMP = 1,
-	NETDEV_XSK_FLAGS_TX_CHECKSUM = 2,
-	NETDEV_XSK_FLAGS_TX_LAUNCH_TIME_FIFO = 4,
+	NETDEV_XSK_FLAGS_TX_TIMESTAMP = 1U << 0,
+	NETDEV_XSK_FLAGS_TX_CHECKSUM = 1U << 1,
+	NETDEV_XSK_FLAGS_TX_LAUNCH_TIME_FIFO = 1U << 2,
 };
 
 enum netdev_queue_type {
@@ -74,7 +74,7 @@ enum netdev_queue_type {
 };
 
 enum netdev_qstats_scope {
-	NETDEV_QSTATS_SCOPE_QUEUE = 1,
+	NETDEV_QSTATS_SCOPE_QUEUE = 1U << 0,
 };
 
 enum netdev_napi_threaded {
diff --git a/tools/net/ynl/pyynl/lib/nlspec.py b/tools/net/ynl/pyynl/lib/nlspec.py
index 85c17fe01e35a..465d8fd909a04 100644
--- a/tools/net/ynl/pyynl/lib/nlspec.py
+++ b/tools/net/ynl/pyynl/lib/nlspec.py
@@ -90,9 +90,12 @@ class SpecEnumEntry(SpecElement):
     def raw_value(self):
         return self.value
 
-    def user_value(self, as_flags=None):
+    def user_value(self, as_flags=None, as_c=None):
         if self.enum_set['type'] == 'flags' or as_flags:
-            return 1 << self.value
+            if as_c:
+                return f'1U << {self.value}'
+            else:
+                return 1 << self.value
         else:
             return self.value
 
diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 58086b1010573..e6df0e2b63a8c 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -3209,7 +3209,7 @@ def render_uapi(family, cw):
             for entry in enum.entries.values():
                 suffix = ','
                 if entry.value_change:
-                    suffix = f" = {entry.user_value()}" + suffix
+                    suffix = f" = {entry.user_value(as_c=True)}" + suffix
                 cw.p(entry.c_name + suffix)
 
             if const.get('render-max', False):
-- 
2.51.0

[PATCH net-next 2/6] tools: ynl-gen: refactor render-max enum generation

[Asbjørn Sloth Tønnesen]

This patch refactors the generation of the three render-max
private enum members: (__$pfx-MAX and $pfx-MAX) or $pfx-MASK.

The names, default or not, are now resolved in the EnumSet class.

This makes enum.enum_max_name re-usable for NLA_POLICY_MASK() in
the next patch in this series, so we don't have to re-define it.

This doesn't change the generated output.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index e6df0e2b63a8c..2666cc54d09c0 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -1060,7 +1060,9 @@ class EnumSet(SpecEnumSet):
 
         self.value_pfx = yaml.get('name-prefix', f"{family.ident_name}-{yaml['name']}-")
         self.header = yaml.get('header', None)
-        self.enum_cnt_name = yaml.get('enum-cnt-name', None)
+        self.enum_cnt_name = yaml.get('enum-cnt-name', f'--{self.value_pfx}max')
+        suffix = yaml['type'] == 'flags' and 'mask' or 'max'
+        self.enum_max_name = f'{self.value_pfx}{suffix}'
 
         super().__init__(family, yaml)
 
@@ -3205,7 +3207,6 @@ def render_uapi(family, cw):
                 cw.p(' */')
 
             uapi_enum_start(family, cw, const, 'name')
-            name_pfx = const.get('name-prefix', f"{family.ident_name}-{const['name']}-")
             for entry in enum.entries.values():
                 suffix = ','
                 if entry.value_change:
@@ -3215,17 +3216,14 @@ def render_uapi(family, cw):
             if const.get('render-max', False):
                 cw.nl()
                 cw.p('/* private: */')
+                max_name = c_upper(enum.enum_max_name)
                 if const['type'] == 'flags':
-                    max_name = c_upper(name_pfx + 'mask')
-                    max_val = f' = {enum.get_mask()},'
-                    cw.p(max_name + max_val)
+                    max_val = f'{enum.get_mask()},'
                 else:
-                    cnt_name = enum.enum_cnt_name
-                    max_name = c_upper(name_pfx + 'max')
-                    if not cnt_name:
-                        cnt_name = '__' + name_pfx + 'max'
-                    cw.p(c_upper(cnt_name) + ',')
-                    cw.p(max_name + ' = (' + c_upper(cnt_name) + ' - 1)')
+                    cnt_name = c_upper(enum.enum_cnt_name)
+                    cw.p(f'{cnt_name},')
+                    max_val = f'({cnt_name} - 1)'
+                cw.p(f'{max_name} = {max_val}')
             cw.block_end(line=';')
             cw.nl()
         elif const['type'] == 'const':
-- 
2.51.0

[PATCH net-next 3/6] tools: ynl-gen: use uapi mask definition in NLA_POLICY_MASK

[Asbjørn Sloth Tønnesen]

Currently when generating policies using NLA_POLICY_MASK(), then
we emit a pre-computed decimal mask.

When render-max is set, then we can re-use the mask definition,
that has been generated in the uapi header.

This IMHO makes the generated code read more like handwritten code.

This patch assumes that "kernel source" is only generated, when
"uapi header" is also generated through ynl-gen, when render-max is
set in the spec. AFAICT this is fine, as render-max is pointless
when uapi is not generated by ynl-gen.

Currently no generated policies are changed by this, as there are
no specs which are used for generation, which also has render-max.
In the future this might be used for code generation by wireguard.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 2666cc54d09c0..b00762721280c 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -418,12 +418,18 @@ class TypeScalar(Type):
         if 'flags-mask' in self.checks or self.is_bitfield:
             if self.is_bitfield:
                 enum = self.family.consts[self.attr['enum']]
-                mask = enum.get_mask(as_flags=True)
+                if enum.get('render-max', False):
+                    mask = c_upper(enum.enum_max_name)
+                else:
+                    mask = enum.get_mask(as_flags=True)
             else:
                 flags = self.family.consts[self.checks['flags-mask']]
                 flag_cnt = len(flags['entries'])
                 mask = (1 << flag_cnt) - 1
-            return f"NLA_POLICY_MASK({policy}, 0x{mask:x})"
+
+            if isinstance(mask, int):
+                mask = f'0x{mask:x}'
+            return f"NLA_POLICY_MASK({policy}, {mask})"
         elif 'full-range' in self.checks:
             return f"NLA_POLICY_FULL_RANGE({policy}, &{c_lower(self.enum_name)}_range)"
         elif 'range' in self.checks:
-- 
2.51.0

[PATCH net-next 4/6] tools: ynl-gen: add generic p_wrap() helper

[Asbjørn Sloth Tønnesen]

Previously only write_func_prot() was performing line wrapping.

As the next patch in this series also requires line wrapping,
then this patch introduces a generic line wrapping helper in
CodeWriter called p_wrap(), which can be used in both cases.

This patch causes a change in the generated prototype for
psp_device_get_locked(), as the skb argument actually fits
on the first line, while not exceeding 80 characters.

No functional changes.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 net/psp/psp-nl-gen.h             |  4 +--
 tools/net/ynl/pyynl/ynl_gen_c.py | 42 +++++++++++++++++++-------------
 2 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/net/psp/psp-nl-gen.h b/net/psp/psp-nl-gen.h
index 25268ed11fb56..ac5f59b4f498c 100644
--- a/net/psp/psp-nl-gen.h
+++ b/net/psp/psp-nl-gen.h
@@ -14,8 +14,8 @@
 /* Common nested types */
 extern const struct nla_policy psp_keys_nl_policy[PSP_A_KEYS_SPI + 1];
 
-int psp_device_get_locked(const struct genl_split_ops *ops,
-			  struct sk_buff *skb, struct genl_info *info);
+int psp_device_get_locked(const struct genl_split_ops *ops, struct sk_buff *skb,
+			  struct genl_info *info);
 int psp_assoc_device_get_locked(const struct genl_split_ops *ops,
 				struct sk_buff *skb, struct genl_info *info);
 void
diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index b00762721280c..1201c2ac352ea 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -1693,6 +1693,27 @@ class CodeWriter:
             ind += add_ind
         self._out.write('\t' * ind + line + '\n')
 
+    def p_wrap(self, prefix, parts):
+        assert(len(parts) > 0)
+        ts = 8
+        pfx_len = len(prefix)
+        pfx_ind_tabs = pfx_len // ts
+        pfx_ind = '\t' * pfx_ind_tabs + ' ' * (pfx_len % ts)
+        max_len = 80 - (self._ind * ts)
+        is_first_line = True
+        buf = f'{prefix}{parts[0]}'
+        for part in parts[1:]:
+            next_buf = f'{buf} {part}'
+            if len(next_buf) <= max_len:
+                buf = next_buf
+            else:
+                self.p(buf)
+                buf = f'{pfx_ind}{part}'
+                if is_first_line:
+                    max_len -= pfx_ind_tabs * (ts-1)
+                    is_first_line = False
+        self.p(buf)
+
     def nl(self):
         self._nl = True
 
@@ -1751,23 +1772,10 @@ class CodeWriter:
             v = ''
         elif qual_ret[-1] != '*':
             v += ' '
-        v += name + '('
-        ind = '\t' * (len(v) // 8) + ' ' * (len(v) % 8)
-        delta_ind = len(v) - len(ind)
-        v += args[0]
-        i = 1
-        while i < len(args):
-            next_len = len(v) + len(args[i])
-            if v[0] == '\t':
-                next_len += delta_ind
-            if next_len > 76:
-                self.p(v + ',')
-                v = ind
-            else:
-                v += ', '
-            v += args[i]
-            i += 1
-        self.p(v + ')' + suffix)
+
+        parts = [f'{arg},' for arg in args[:-1]]
+        parts.append(f'{args[-1]}){suffix}')
+        self.p_wrap(f'{v}{name}(', parts)
 
     def write_func_lvar(self, local_vars):
         if not local_vars:
-- 
2.51.0

[PATCH net-next 5/6] tools: ynl-gen: construct bitflag masks in generated headers

[Asbjørn Sloth Tønnesen]

Instead of pre-computing the bitflag mask within the code generator,
then generate the code to combine all the flags in the generated code.

This patch uses the new p_wrap() method to wrap long lines.

This IMHO makes the generated code read more like handwritten code.

No functional changes.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 include/uapi/linux/netdev.h       | 6 +++++-
 tools/include/uapi/linux/netdev.h | 6 +++++-
 tools/net/ynl/pyynl/ynl_gen_c.py  | 7 +++++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index db0526cb6672d..337f444178bbb 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -35,7 +35,11 @@ enum netdev_xdp_act {
 	NETDEV_XDP_ACT_NDO_XMIT_SG = 1U << 6,
 
 	/* private: */
-	NETDEV_XDP_ACT_MASK = 127,
+	NETDEV_XDP_ACT_MASK = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
+			      NETDEV_XDP_ACT_NDO_XMIT |
+			      NETDEV_XDP_ACT_XSK_ZEROCOPY |
+			      NETDEV_XDP_ACT_HW_OFFLOAD | NETDEV_XDP_ACT_RX_SG |
+			      NETDEV_XDP_ACT_NDO_XMIT_SG,
 };
 
 /**
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index db0526cb6672d..337f444178bbb 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -35,7 +35,11 @@ enum netdev_xdp_act {
 	NETDEV_XDP_ACT_NDO_XMIT_SG = 1U << 6,
 
 	/* private: */
-	NETDEV_XDP_ACT_MASK = 127,
+	NETDEV_XDP_ACT_MASK = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
+			      NETDEV_XDP_ACT_NDO_XMIT |
+			      NETDEV_XDP_ACT_XSK_ZEROCOPY |
+			      NETDEV_XDP_ACT_HW_OFFLOAD | NETDEV_XDP_ACT_RX_SG |
+			      NETDEV_XDP_ACT_NDO_XMIT_SG,
 };
 
 /**
diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 1201c2ac352ea..5e1c702143d86 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -3232,12 +3232,15 @@ def render_uapi(family, cw):
                 cw.p('/* private: */')
                 max_name = c_upper(enum.enum_max_name)
                 if const['type'] == 'flags':
-                    max_val = f'{enum.get_mask()},'
+                    values = list(enum.entries.values())
+                    parts = [f'{val.c_name} |' for val in values[:-1]]
+                    parts.append(f'{values[-1].c_name},')
+                    cw.p_wrap(f'{max_name} = ', parts)
                 else:
                     cnt_name = c_upper(enum.enum_cnt_name)
                     cw.p(f'{cnt_name},')
                     max_val = f'({cnt_name} - 1)'
-                cw.p(f'{max_name} = {max_val}')
+                    cw.p(f'{max_name} = {max_val}')
             cw.block_end(line=';')
             cw.nl()
         elif const['type'] == 'const':
-- 
2.51.0

[PATCH net-next 6/6] tools: ynl-gen: allow custom naming of render-max definitions

[Asbjørn Sloth Tønnesen]

When `render-max` is set for an enum, then it generates either
(`__$pfx-MAX` and `$pfx-MAX`) or (`$pfx-MASK` for flags).

The count definition `__$pfx-MAX` can already be overridden via
`enum-cnt-name` in the spec.

This patch adds a new `enum-max-name` attribute which can be used
to override the names for either `$pfx-MAX` or `$pfx-MASK`.

The existing `enum-cnt-name` is only described for the genetlink-c
and genetlink-legacy protocols, so I have only added `enum-max-name`
for those protocols.

This doesn't change the generated output.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---

Alternatively `enum-max-name` should be added to all protocols, so
that genetlink families can also choose to eg. have these private
variables prefixed with "__". As NETDEV_XDP_ACT_MASK leaked into
xdp-tools [v1.4.0..v1.5.7], then if we want to change the default
names[1], then we would still need to be able to use an override
to keep the current NETDEV_XDP_ACT_MASK name in the netdev family.

[1] https://lore.kernel.org/netdev/20230614211715.01940bbd@kernel.org/
---
 Documentation/netlink/genetlink-c.yaml             | 3 +++
 Documentation/netlink/genetlink-legacy.yaml        | 3 +++
 Documentation/userspace-api/netlink/c-code-gen.rst | 7 +++++--
 tools/net/ynl/pyynl/ynl_gen_c.py                   | 6 ++++--
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/genetlink-c.yaml b/Documentation/netlink/genetlink-c.yaml
index 5a234e9b5fa2e..755b24fb0c319 100644
--- a/Documentation/netlink/genetlink-c.yaml
+++ b/Documentation/netlink/genetlink-c.yaml
@@ -110,6 +110,9 @@ properties:
         enum-cnt-name:
           description: Name of the render-max counter enum entry.
           type: string
+        enum-max-name:
+          description: Name of the render-max max or mask enum entry.
+          type: string
         # End genetlink-c
 
   attribute-sets:
diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index 66fb8653a3442..ad4d69be6294e 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -124,6 +124,9 @@ properties:
         enum-cnt-name:
           description: Name of the render-max counter enum entry.
           type: string
+        enum-max-name:
+          description: Name of the render-max max or mask enum entry.
+          type: string
         # End genetlink-c
         # Start genetlink-legacy
         members:
diff --git a/Documentation/userspace-api/netlink/c-code-gen.rst b/Documentation/userspace-api/netlink/c-code-gen.rst
index 46415e6d646d2..413a56424012a 100644
--- a/Documentation/userspace-api/netlink/c-code-gen.rst
+++ b/Documentation/userspace-api/netlink/c-code-gen.rst
@@ -57,8 +57,11 @@ portion of the entry name.
 
 Boolean ``render-max`` controls creation of the max values
 (which are enabled by default for attribute enums). These max
-values are named ``__$pfx-MAX`` and ``$pfx-MAX``. The name
-of the first value can be overridden via ``enum-cnt-name`` property.
+values are named ``__$pfx-MAX`` and ``$pfx-MAX``, and can be
+overwritten via the properties ``enum-cnt-name`` and
+``enum-max-name`` respectively.
+For flags ``render-max`` will generate a mask with all flags set,
+which by default will be named ``$pfx-MASK``.
 
 Attributes
 ==========
diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 5e1c702143d86..a1a0b559b431b 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -1067,8 +1067,10 @@ class EnumSet(SpecEnumSet):
         self.value_pfx = yaml.get('name-prefix', f"{family.ident_name}-{yaml['name']}-")
         self.header = yaml.get('header', None)
         self.enum_cnt_name = yaml.get('enum-cnt-name', f'--{self.value_pfx}max')
-        suffix = yaml['type'] == 'flags' and 'mask' or 'max'
-        self.enum_max_name = f'{self.value_pfx}{suffix}'
+        self.enum_max_name = yaml.get('enum-max-name', None)
+        if not self.enum_max_name:
+            suffix = yaml['type'] == 'flags' and 'mask' or 'max'
+            self.enum_max_name = f'{self.value_pfx}{suffix}'
 
         super().__init__(family, yaml)
 
-- 
2.51.0

Re: [PATCH net-next 1/6] tools: ynl-gen: bitshift the flag values in the generated code

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 392 bytes --]



On 10/13/2025 9:49 AM, Asbjørn Sloth Tønnesen wrote:
> Instead of pre-computing the flag values within the code generator,
> then move the bitshift operation into the generated code.
> 
> This IMHO makes the generated code read more like handwritten code.
> 
> No functional changes.
> 

Could we use BIT() here? or is that not available within uAPI headers?

Thanks,
Jake

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 0/6] tools: ynl-gen: generate flags better

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1629 bytes --]



On 10/13/2025 9:49 AM, Asbjørn Sloth Tønnesen wrote:
> This series focusses on increasing the quality of
> the C code generated by ynl-gen for flags.
> 
> NB: I included a note in patch 6, on usage of the private
> NETDEV_XDP_ACT_MASK in user-space.
> 

Everything here is an improvement over the existing output. I would like
if we could use "BIT(N)" instead of "1U << N", but I think its better
than the current pre-computed values that you have to parse as bit values.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> Asbjørn Sloth Tønnesen (6):
>   tools: ynl-gen: bitshift the flag values in the generated code
>   tools: ynl-gen: refactor render-max enum generation
>   tools: ynl-gen: use uapi mask definition in NLA_POLICY_MASK
>   tools: ynl-gen: add generic p_wrap() helper
>   tools: ynl-gen: construct bitflag masks in generated headers
>   tools: ynl-gen: allow custom naming of render-max definitions
> 
>  Documentation/netlink/genetlink-c.yaml        |  3 +
>  Documentation/netlink/genetlink-legacy.yaml   |  3 +
>  .../userspace-api/netlink/c-code-gen.rst      |  7 +-
>  include/uapi/linux/dpll.h                     |  6 +-
>  .../uapi/linux/ethtool_netlink_generated.h    | 20 ++---
>  include/uapi/linux/netdev.h                   | 34 ++++----
>  net/psp/psp-nl-gen.h                          |  4 +-
>  tools/include/uapi/linux/netdev.h             | 34 ++++----
>  tools/net/ynl/pyynl/lib/nlspec.py             |  7 +-
>  tools/net/ynl/pyynl/ynl_gen_c.py              | 79 +++++++++++--------
>  10 files changed, 117 insertions(+), 80 deletions(-)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 1/6] tools: ynl-gen: bitshift the flag values in the generated code

[Jakub Kicinski]

On Mon, 13 Oct 2025 16:49:58 +0000 Asbjørn Sloth Tønnesen wrote:
> Instead of pre-computing the flag values within the code generator,
> then move the bitshift operation into the generated code.
> 
> This IMHO makes the generated code read more like handwritten code.

I like it the way it is. The values are irrelevant.

And returning a string from user_value() is quite ugly.

Re: [PATCH net-next 2/6] tools: ynl-gen: refactor render-max enum generation

[Jakub Kicinski]

On Mon, 13 Oct 2025 16:49:59 +0000 Asbjørn Sloth Tønnesen wrote:
> +        suffix = yaml['type'] == 'flags' and 'mask' or 'max'

This construct looks highly non-pythonic to me

> +        self.enum_max_name = f'{self.value_pfx}{suffix}'

sometimes its max sometimes is mask, so we shouldn't call it max always

Re: [PATCH net-next 3/6] tools: ynl-gen: use uapi mask definition in NLA_POLICY_MASK

[Jakub Kicinski]

On Mon, 13 Oct 2025 16:50:00 +0000 Asbjørn Sloth Tønnesen wrote:
> Currently when generating policies using NLA_POLICY_MASK(), then
> we emit a pre-computed decimal mask.
> 
> When render-max is set, then we can re-use the mask definition,
> that has been generated in the uapi header.

This will encourage people to render masks in uAPI which just pollutes
the uAPI files.

Re: [PATCH net-next 1/6] tools: ynl-gen: bitshift the flag values in the generated code

[Asbjørn Sloth Tønnesen]

On 10/13/25 11:07 PM, Jacob Keller wrote:
> On 10/13/2025 9:49 AM, Asbjørn Sloth Tønnesen wrote:
>> Instead of pre-computing the flag values within the code generator,
>> then move the bitshift operation into the generated code.
>>
>> This IMHO makes the generated code read more like handwritten code.
>>
>> No functional changes.
>>
> 
> Could we use BIT() here? or is that not available within uAPI headers?

Correct, BIT() is not exported to uAPI[1].

Thank you for the reviews!

[1] [PATCH v2] checkpatch: don't complain about BIT macro in uapi
https://lore.kernel.org/all/1468707033-16173-1-git-send-email-tomas.winkler@intel.com/

Re: [PATCH net-next 1/6] tools: ynl-gen: bitshift the flag values in the generated code

[Asbjørn Sloth Tønnesen]

On 10/14/25 12:53 AM, Jakub Kicinski wrote:
> On Mon, 13 Oct 2025 16:49:58 +0000 Asbjørn Sloth Tønnesen wrote:
>> Instead of pre-computing the flag values within the code generator,
>> then move the bitshift operation into the generated code.
>>
>> This IMHO makes the generated code read more like handwritten code.
> 
> I like it the way it is. The values are irrelevant.

Bit-shifting seams like the preferred way across the uAPI headers.

Would you be open to hexadecimal notation, if not bit-shifting?

Currently NLA_POLICY_MASK() is generated with a hexadecimal mask, and
with these patches, if render-max is not set. If using literal values
then we should properly consistently generate them as either decimal
or hexadecimal. I prefer hexadecimal over decimal.

> And returning a string from user_value() is quite ugly.
It only returns a string, when as_c is set, I am happy to duplicate
some code instead, and add a dedicated method always returning a string,
but can we please agree on the generated output, before implementation?

Re: [PATCH net-next 2/6] tools: ynl-gen: refactor render-max enum generation

[Asbjørn Sloth Tønnesen]

On 10/14/25 12:58 AM, Jakub Kicinski wrote:
> On Mon, 13 Oct 2025 16:49:59 +0000 Asbjørn Sloth Tønnesen wrote:
>> +        suffix = yaml['type'] == 'flags' and 'mask' or 'max'
> 
> This construct looks highly non-pythonic to me

I don't mind changing it to it's multi-line form, but this line might go
away (see below).

>> +        self.enum_max_name = f'{self.value_pfx}{suffix}'
> 
> sometimes its max sometimes is mask, so we shouldn't call it max always

I'm fine with splitting them to render-max, enum-max-name, render-mask and
enum-mask-name. I was just following along the current lines in the code,
as started in commit 96a611b6b60c.

Re: [PATCH net-next 3/6] tools: ynl-gen: use uapi mask definition in NLA_POLICY_MASK

[Asbjørn Sloth Tønnesen]

On 10/14/25 12:59 AM, Jakub Kicinski wrote:
> On Mon, 13 Oct 2025 16:50:00 +0000 Asbjørn Sloth Tønnesen wrote:
>> Currently when generating policies using NLA_POLICY_MASK(), then
>> we emit a pre-computed decimal mask.
>>
>> When render-max is set, then we can re-use the mask definition,
>> that has been generated in the uapi header.
> 
> This will encourage people to render masks in uAPI which just pollutes
> the uAPI files.

It might, but is that a problem, given that most flag-sets are rather small?

Example from include/uapi/linux/wireguard.h:
 > enum wgpeer_flag {
 >     WGPEER_F_REMOVE_ME = 1U << 0,
 >     WGPEER_F_REPLACE_ALLOWEDIPS = 1U << 1,
 >     WGPEER_F_UPDATE_ONLY = 1U << 2,
 >     __WGPEER_F_ALL = WGPEER_F_REMOVE_ME | WGPEER_F_REPLACE_ALLOWEDIPS |
 >                      WGPEER_F_UPDATE_ONLY
 > };

I agree that a private "WGPEER_F_ALL" would be pollution, but "__WGPEER_F_ALL"
is less likely to accidentally be used by user-space.

I get why Jason likes having the __WGPEER_F_ALL in a place where it is easy
to review that it has contains all flags, and why he don't like a policy like
NLA_POLICY_MASK(.., 0x7).

We could do the mask definition in the kernel code, like many handwritten
netlink families does, but we still need to keep NETDEV_XDP_ACT_MASK in
netdev.h or remove it's YNL-GEN header for some time.

Re: [PATCH net-next 1/6] tools: ynl-gen: bitshift the flag values in the generated code

[Jakub Kicinski]

On Tue, 14 Oct 2025 16:49:22 +0000 Asbjørn Sloth Tønnesen wrote:
> On 10/14/25 12:53 AM, Jakub Kicinski wrote:
> > On Mon, 13 Oct 2025 16:49:58 +0000 Asbjørn Sloth Tønnesen wrote:  
> >> Instead of pre-computing the flag values within the code generator,
> >> then move the bitshift operation into the generated code.
> >>
> >> This IMHO makes the generated code read more like handwritten code.  
> > 
> > I like it the way it is. The values are irrelevant.  
> 
> Bit-shifting seams like the preferred way across the uAPI headers.
> 
> Would you be open to hexadecimal notation, if not bit-shifting?
> 
> Currently NLA_POLICY_MASK() is generated with a hexadecimal mask, and
> with these patches, if render-max is not set. If using literal values
> then we should properly consistently generate them as either decimal
> or hexadecimal. I prefer hexadecimal over decimal.

Hm, hex could do. For the bit/1 << x i really don't like that the values
are not aligned to columns, so they visually mix in with the names. 
But aligning them would be more LoC than it's worth.

hex could be a reasonable compromise, but I make no promise that I will
like it once I see the result :)

> > And returning a string from user_value() is quite ugly.  
> It only returns a string, when as_c is set, I am happy to duplicate
> some code instead, and add a dedicated method always returning a string,
> but can we please agree on the generated output, before implementation?

nlspec.py was supposed to be a library that abstracts away things like
default values not being present, and simplifies indexing. So having a
"give me a format for C as result" arg is not great for layering.
That kind of logic belongs in the caller. 

Regarding LoC - great code is concise, but that doesn't mean that
making code shorter always makes it better.

Re: [PATCH net-next 2/6] tools: ynl-gen: refactor render-max enum generation

[Jakub Kicinski]

On Tue, 14 Oct 2025 17:04:14 +0000 Asbjørn Sloth Tønnesen wrote:
> On 10/14/25 12:58 AM, Jakub Kicinski wrote:
> > On Mon, 13 Oct 2025 16:49:59 +0000 Asbjørn Sloth Tønnesen wrote:  
> >> +        suffix = yaml['type'] == 'flags' and 'mask' or 'max'  
> > 
> > This construct looks highly non-pythonic to me  
> 
> I don't mind changing it to it's multi-line form, but this line might go
> away (see below).
> 
> >> +        self.enum_max_name = f'{self.value_pfx}{suffix}'  
> > 
> > sometimes its max sometimes is mask, so we shouldn't call it max always  
> 
> I'm fine with splitting them to render-max, enum-max-name, render-mask and
> enum-mask-name. I was just following along the current lines in the code,
> as started in commit 96a611b6b60c.

Ideally we'd find a general noun to describe both max and mask..
I don't have any great suggestions tho

Re: [PATCH net-next 3/6] tools: ynl-gen: use uapi mask definition in NLA_POLICY_MASK

[Jakub Kicinski]

On Tue, 14 Oct 2025 17:29:30 +0000 Asbjørn Sloth Tønnesen wrote:
> On 10/14/25 12:59 AM, Jakub Kicinski wrote:
> > On Mon, 13 Oct 2025 16:50:00 +0000 Asbjørn Sloth Tønnesen wrote:  
> >> Currently when generating policies using NLA_POLICY_MASK(), then
> >> we emit a pre-computed decimal mask.
> >>
> >> When render-max is set, then we can re-use the mask definition,
> >> that has been generated in the uapi header.  
> > 
> > This will encourage people to render masks in uAPI which just pollutes
> > the uAPI files.  
> 
> It might, but is that a problem, given that most flag-sets are rather small?

Problem is a strong word. But if the choice is having a constant in
auto-generated code, or pointless, cargo-cult'ed mask values in the 
uAPI headers - I choose the former.

> Example from include/uapi/linux/wireguard.h:
>  > enum wgpeer_flag {
>  >     WGPEER_F_REMOVE_ME = 1U << 0,
>  >     WGPEER_F_REPLACE_ALLOWEDIPS = 1U << 1,
>  >     WGPEER_F_UPDATE_ONLY = 1U << 2,
>  >     __WGPEER_F_ALL = WGPEER_F_REMOVE_ME | WGPEER_F_REPLACE_ALLOWEDIPS |
>  >                      WGPEER_F_UPDATE_ONLY
>  > };  
> 
> I agree that a private "WGPEER_F_ALL" would be pollution, but "__WGPEER_F_ALL"
> is less likely to accidentally be used by user-space.
> 
> I get why Jason likes having the __WGPEER_F_ALL in a place where it is easy
> to review that it has contains all flags, and why he don't like a policy like
> NLA_POLICY_MASK(.., 0x7).
> 
> We could do the mask definition in the kernel code, like many handwritten
> netlink families does, but we still need to keep NETDEV_XDP_ACT_MASK in
> netdev.h or remove it's YNL-GEN header for some time.

It's a transitional problem. People coming from hand-crafted code feel
like they need a human-readable mask. 6mo later once they are
comfortable with YNL codegen they won't care. But, sadly, at that point
it is too late to delete stuff from the uAPI header.