The Linux tc actions expect that the action order starts from index
one. To accommodate this, add a start-index property to the ynl spec
for indexed arrays. This property allows the starting index to be
specified, ensuring compatibility with consumers that require a
non-zero-based index.
For example if we have "start_index = 1" then we get the following
diff.
ynl_attr_put_str(nlh, TCA_FLOWER_INDEV, obj->indev);
array = ynl_attr_nest_start(nlh, TCA_FLOWER_ACT);
for (i = 0; i < obj->_count.act; i++)
- tc_act_attrs_put(nlh, i, &obj->act[i]);
+ tc_act_attrs_put(nlh, i + 1, &obj->act[i]);
ynl_attr_nest_end(nlh, array);
Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
Documentation/netlink/netlink-raw.yaml | 13 +++++++++++++
Documentation/netlink/specs/tc.yaml | 7 +++++++
tools/net/ynl/pyynl/lib/nlspec.py | 1 +
tools/net/ynl/pyynl/ynl_gen_c.py | 6 +++++-
4 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/Documentation/netlink/netlink-raw.yaml b/Documentation/netlink/netlink-raw.yaml
index 246fa07bccf6..aafb7cb16beb 100644
--- a/Documentation/netlink/netlink-raw.yaml
+++ b/Documentation/netlink/netlink-raw.yaml
@@ -260,6 +260,9 @@ properties:
Sometimes, however, both forms are necessary, in which case header contains the enum
form while specific attributes may request to convert the values into a bitfield.
type: boolean
+ start-index:
+ description: For indexed arrays the first index value.
+ type: integer
checks:
description: Kernel input validation.
type: object
@@ -308,6 +311,16 @@ properties:
type: string
# End netlink-raw
+ # allow start index only for indexed arrays
+ if:
+ properties:
+ type:
+ const: indexed-array
+ then: {}
+ else:
+ not:
+ required: [ start-index ]
+
# Make sure name-prefix does not appear in subsets (subsets inherit naming)
dependencies:
name-prefix:
diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml
index b398f7a46dae..459aa51059ec 100644
--- a/Documentation/netlink/specs/tc.yaml
+++ b/Documentation/netlink/specs/tc.yaml
@@ -2044,6 +2044,7 @@ attribute-sets:
type: indexed-array
sub-type: nest
nested-attributes: act-attrs
+ start-index: 1
-
name: police
type: nest
@@ -2303,6 +2304,7 @@ attribute-sets:
type: indexed-array
sub-type: nest
nested-attributes: act-attrs
+ start-index: 1
-
name: police
type: nest
@@ -2493,6 +2495,7 @@ attribute-sets:
type: indexed-array
sub-type: nest
nested-attributes: act-attrs
+ start-index: 1
-
name: key-eth-dst
type: binary
@@ -3020,6 +3023,7 @@ attribute-sets:
type: indexed-array
sub-type: nest
nested-attributes: act-attrs
+ start-index: 1
-
name: mask
type: u32
@@ -3180,6 +3184,7 @@ attribute-sets:
type: indexed-array
sub-type: nest
nested-attributes: act-attrs
+ start-index: 1
-
name: flags
type: u32
@@ -3566,6 +3571,7 @@ attribute-sets:
type: indexed-array
sub-type: nest
nested-attributes: act-attrs
+ start-index: 1
-
name: taprio-attrs
name-prefix: tca-taprio-attr-
@@ -3798,6 +3804,7 @@ attribute-sets:
type: indexed-array
sub-type: nest
nested-attributes: act-attrs
+ start-index: 1
-
name: indev
type: string
diff --git a/tools/net/ynl/pyynl/lib/nlspec.py b/tools/net/ynl/pyynl/lib/nlspec.py
index 85c17fe01e35..08660602da9d 100644
--- a/tools/net/ynl/pyynl/lib/nlspec.py
+++ b/tools/net/ynl/pyynl/lib/nlspec.py
@@ -181,6 +181,7 @@ class SpecAttr(SpecElement):
self.display_hint = yaml.get('display-hint')
self.sub_message = yaml.get('sub-message')
self.selector = yaml.get('selector')
+ self.start_index = yaml.get('start-index', 0)
self.is_auto_scalar = self.type == "sint" or self.type == "uint"
diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index aadeb3abcad8..698d6089a856 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -852,7 +852,11 @@ class TypeIndexedArray(Type):
ri.cw.p(f"ynl_attr_put(nlh, i, {var}->{self.c_name}[i], {self.checks['exact-len']});")
elif self.sub_type == 'nest':
ri.cw.p(f'for (i = 0; i < {var}->_count.{self.c_name}; i++)')
- ri.cw.p(f"{self.nested_render_name}_put(nlh, i, &{var}->{self.c_name}[i]);")
+ ri.cw.p(
+ f"{self.nested_render_name}_put(nlh, "
+ f"i{f' + {self.start_index}' if self.start_index > 0 else ''}, "
+ f"&{var}->{self.c_name}[i]);"
+ )
else:
raise Exception(f"Put for IndexedArray sub-type {self.attr['sub-type']} not supported, yet")
ri.cw.p('ynl_attr_nest_end(nlh, array);')
--
2.51.0On Sat, 18 Oct 2025 17:17:37 +0200 Zahari Doychev wrote: > The Linux tc actions expect that the action order starts from index > one. To accommodate this, add a start-index property to the ynl spec > for indexed arrays. This property allows the starting index to be > specified, ensuring compatibility with consumers that require a > non-zero-based index. > > For example if we have "start_index = 1" then we get the following > diff. > > ynl_attr_put_str(nlh, TCA_FLOWER_INDEV, obj->indev); > array = ynl_attr_nest_start(nlh, TCA_FLOWER_ACT); > for (i = 0; i < obj->_count.act; i++) > - tc_act_attrs_put(nlh, i, &obj->act[i]); > + tc_act_attrs_put(nlh, i + 1, &obj->act[i]); > ynl_attr_nest_end(nlh, array); The first one is just silently skipped by the kernel right? We need to be selective about what API stupidity we try to cover up in YNL. Otherwise the specs will be unmanageably complex. IMO this one should be a comment in the spec explaining that action 0 is ignore and that's it.
On Mon, Oct 20, 2025 at 04:32:21PM -0700, Jakub Kicinski wrote: > On Sat, 18 Oct 2025 17:17:37 +0200 Zahari Doychev wrote: > > The Linux tc actions expect that the action order starts from index > > one. To accommodate this, add a start-index property to the ynl spec > > for indexed arrays. This property allows the starting index to be > > specified, ensuring compatibility with consumers that require a > > non-zero-based index. > > > > For example if we have "start_index = 1" then we get the following > > diff. > > > > ynl_attr_put_str(nlh, TCA_FLOWER_INDEV, obj->indev); > > array = ynl_attr_nest_start(nlh, TCA_FLOWER_ACT); > > for (i = 0; i < obj->_count.act; i++) > > - tc_act_attrs_put(nlh, i, &obj->act[i]); > > + tc_act_attrs_put(nlh, i + 1, &obj->act[i]); > > ynl_attr_nest_end(nlh, array); > > The first one is just silently skipped by the kernel right? yes, and then only the second action is being confiugred. The index defines the action order and the expectation is that they start from order 1. > > We need to be selective about what API stupidity we try to > cover up in YNL. Otherwise the specs will be unmanageably complex. > IMO this one should be a comment in the spec explaining that action > 0 is ignore and that's it. > I am not sure if this applies for all cases of indexed arrays. For sure it applies for the tc_act_attrs case but I need to check the rest again. Do you think it would be fine to start from 1 for all indexed arrays?
On 10/21/25 5:50 PM, Zahari Doychev wrote: > On Mon, Oct 20, 2025 at 04:32:21PM -0700, Jakub Kicinski wrote: >> We need to be selective about what API stupidity we try to >> cover up in YNL. Otherwise the specs will be unmanageably complex. >> IMO this one should be a comment in the spec explaining that action >> 0 is ignore and that's it. >> > > I am not sure if this applies for all cases of indexed arrays. For sure > it applies for the tc_act_attrs case but I need to check the rest again. > > Do you think it would be fine to start from 1 for all indexed arrays? Yes, AFAICT it would. Most of indexed-array attributes that are parsed by the kernel uses nla_for_each_nested(), and don't use the index. The TC actions are the only ones I found, that are parsed into a nlattr array. Disclaimer: I have only mapped out the indexed-arrays that are declared in the current specs. See patch 4-7 in this series for the full analysis: https://lore.kernel.org/netdev/20251022182701.250897-1-ast@fiberby.net/
On Wed, Oct 22, 2025 at 07:37:10PM +0000, Asbjørn Sloth Tønnesen wrote: > On 10/21/25 5:50 PM, Zahari Doychev wrote: > > On Mon, Oct 20, 2025 at 04:32:21PM -0700, Jakub Kicinski wrote: > > > We need to be selective about what API stupidity we try to > > > cover up in YNL. Otherwise the specs will be unmanageably complex. > > > IMO this one should be a comment in the spec explaining that action > > > 0 is ignore and that's it. > > > > > > > I am not sure if this applies for all cases of indexed arrays. For sure > > it applies for the tc_act_attrs case but I need to check the rest again. > > > > Do you think it would be fine to start from 1 for all indexed arrays? > Yes, AFAICT it would. Most of indexed-array attributes that are parsed by > the kernel uses nla_for_each_nested(), and don't use the index. The TC > actions are the only ones I found, that are parsed into a nlattr array. > > Disclaimer: I have only mapped out the indexed-arrays that are declared in > the current specs. > > See patch 4-7 in this series for the full analysis: > https://lore.kernel.org/netdev/20251022182701.250897-1-ast@fiberby.net/ > thanks, will try it out.
On Tue, 21 Oct 2025 20:50:03 +0300 Zahari Doychev wrote: > > We need to be selective about what API stupidity we try to > > cover up in YNL. Otherwise the specs will be unmanageably complex. > > IMO this one should be a comment in the spec explaining that action > > 0 is ignore and that's it. > > I am not sure if this applies for all cases of indexed arrays. For sure > it applies for the tc_act_attrs case but I need to check the rest again. > > Do you think it would be fine to start from 1 for all indexed arrays? Not sure off the top of my head
On 10/21/25 5:50 PM, Zahari Doychev wrote: > On Mon, Oct 20, 2025 at 04:32:21PM -0700, Jakub Kicinski wrote: >> We need to be selective about what API stupidity we try to >> cover up in YNL. Otherwise the specs will be unmanageably complex. >> IMO this one should be a comment in the spec explaining that action >> 0 is ignore and that's it. >> > > I am not sure if this applies for all cases of indexed arrays. For sure > it applies for the tc_act_attrs case but I need to check the rest again. > > Do you think it would be fine to start from 1 for all indexed arrays? I have a series, that I will try to get posted tomorrow, where I add a new attribute `ignore-index` which can be used to mark indexed arrays where the index is just an incremental value. This is a follow-up to an earlier discussion[1]. In that series, in order to add the new attribute to the existing specs, in the commit messages I walk through all the existing indexed arrays, and also include things like if they start their indexes from 0 or 1. [1] https://lore.kernel.org/netdev/7fff6b2f-f17e-4179-8507-397b76ea24bb@intel.com/
© 2016 - 2026 Red Hat, Inc.