Since commit ce6cb8113c ("tools: ynl-gen: individually free previous
values on double set"), specifying the "multi-attr" property raises an
error unless the "nested-attributes" property is specified as well:
File "tools/net/ynl/./pyynl/ynl_gen_c.py", line 1147, in _load_nested_sets
child = self.pure_nested_structs.get(nested)
^^^^^^
UnboundLocalError: cannot access local variable 'nested' where it is not associated with a value
This appears to be a bug since there are existing specs which omit
"nested-attributes" on "multi-attr" attributes. Also, according to
Documentation/userspace-api/netlink/specs.rst, multi-attr "is the
recommended way of implementing arrays (no extra nesting)", suggesting
that nesting should even be avoided in favor of multi-attr.
Fix the indentation of the if-block introduced by the commit to avoid
the error.
Fixes: ce6cb8113c ("tools: ynl-gen: individually free previous values on double set")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Link: https://patch.msgid.link/d6b58684b7e5bfb628f7313e6893d0097904e1d1.1746940107.git.lukas@wunner.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Support using a struct pointer for binary attrs. Len field is maintained
because the structs may grow with newer kernel versions. Or, which matters
more, be shorter if the binary is built against newer uAPI than kernel
against which it's executed. Since we are storing a pointer to a struct
type - always allocate at least the amount of memory needed by the struct
per current uAPI headers (unused mem is zeroed). Technically users should
check the length field but per modern ASAN checks storing a short object
under a pointer seems like a bad idea.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://patch.msgid.link/20250509154213.1747885-4-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Sub-type annotation on binary attributes may indicate that the attribute
carries an array of simple types (also referred to as "C array" in docs).
Support rendering them as such in the C user code. For example for u32,
instead of:
struct {
u32 arr;
} _len;
void *arr;
render:
struct {
u32 arr;
} _count;
__u32 *arr;
Note that count is the number of elements while len was the length in bytes.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://patch.msgid.link/20250509154213.1747885-2-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
When sending YNL CLI output into a pipe, closing the pipe causes a
BrokenPipeError. E.g. running the following and quitting less:
./tools/net/ynl/pyynl/cli.py --family rt-link --dump getlink | less
Traceback (most recent call last):
File "/home/donaldh/net-next/./tools/net/ynl/pyynl/cli.py", line 160, in <module>
main()
~~~~^^
File "/home/donaldh/net-next/./tools/net/ynl/pyynl/cli.py", line 142, in main
output(reply)
~~~~~~^^^^^^^
File "/home/donaldh/net-next/./tools/net/ynl/pyynl/cli.py", line 97, in output
pprint.PrettyPrinter().pprint(msg)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^
[...]
BrokenPipeError: [Errno 32] Broken pipe
Consolidate the try block for ops and notifications, and gracefully
handle the BrokenPipeError by adding an exception handler to the
consolidated try block.
Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://patch.msgid.link/20250508112102.63539-1-donald.hunter@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Fix a crash in the ethtool YNL implementation when Hardware Clock information
is not present in the response. This ensures graceful handling of devices or
drivers that do not provide this optional field. e.g.
Traceback (most recent call last):
File "/net/tools/net/ynl/pyynl/./ethtool.py", line 438, in <module>
main()
~~~~^^
File "/net/tools/net/ynl/pyynl/./ethtool.py", line 341, in main
print(f'PTP Hardware Clock: {tsinfo["phc-index"]}')
~~~~~~^^^^^^^^^^^^^
KeyError: 'phc-index'
Fixes: f3d07b02b2 ("tools: ynl: ethtool testing tool")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250508035414.82974-1-liuhangbin@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
While we reshuffle the presence members, move the counts as well.
Previously array count members would have been place directly in
the struct, so:
struct family_op_req {
struct {
u32 a:1;
u32 b:1;
} _present;
struct {
u32 bin;
} _len;
u32 a;
u64 b;
const unsigned char *bin;
u32 n_multi; << count
u32 *multi; << objects
};
Since len has been moved to its own presence struct move the count
as well:
struct family_op_req {
struct {
u32 a:1;
u32 b:1;
} _present;
struct {
u32 bin;
} _len;
struct {
u32 multi; << count
} _count;
u32 a;
u64 b;
const unsigned char *bin;
u32 *multi; << objects
};
This improves the consistency and allows us to remove some hacks
in the codegen. Unlike for len there is no known name collision
with the existing scheme.
Link: https://patch.msgid.link/20250505165208.248049-4-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Each YNL struct contains the data and a sub-struct indicating which
fields are valid. Something like:
struct family_op_req {
struct {
u32 a:1;
u32 b:1;
u32 bin_len;
} _present;
u32 a;
u64 b;
const unsigned char *bin;
};
Note that the bin object 'bin' has a length stored, and that length
has a _len suffix added to the field name. This breaks if there
is a explicit field called bin_len, which is the case for some
TC actions. Move the length fields out of the _present struct,
create a new struct called _len:
struct family_op_req {
struct {
u32 a:1;
u32 b:1;
} _present;
struct {
u32 bin;
} _len;
u32 a;
u64 b;
const unsigned char *bin;
};
This should prevent name collisions and help with the packing
of the struct.
Unfortunately this is a breaking change, but hopefully the migration
isn't too painful.
Link: https://patch.msgid.link/20250505165208.248049-3-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Internal change to the code gen. Rename how we indicate a type
has a single bit presence from using a 'bit' string to 'present'.
This is a noop in terms of generated code but will make next
breaking change easier.
Reviewed-by: David Wei <dw@davidwei.uk>
Link: https://patch.msgid.link/20250505165208.248049-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Strings from the kernel are guaranteed to be null terminated and
ynl_attr_validate() checks for this. But it doesn't check if the string
has a len of 0, which would cause problems when trying to access
data[len - 1]. Fix this by checking that len is positive.
Signed-off-by: David Wei <dw@davidwei.uk>
Link: https://patch.msgid.link/20250503043050.861238-1-dw@davidwei.uk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
rt-link has a vlan-protocols enum with:
name: 8021q value: 33024
name: 8021ad value: 34984
It's nice to have, since it converts the values to strings in Python.
For C, however, the codegen is trying to use enums to generate strict
policy checks. Parsing such sparse enums is not possible via policies.
Since for classic netlink we don't support kernel codegen and policy
generation - skip the auto-generation of checks from enums.
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Link: https://patch.msgid.link/20250429154704.2613851-12-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Binary types with struct are fixed size, relatively easy to
handle for multi attr. Declare the member as a pointer.
Count the members, allocate an array, copy in the data.
Allow the netlink attr to be smaller or larger than our view
of the struct in case the build headers are newer or older
than the running kernel.
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Link: https://patch.msgid.link/20250429154704.2613851-9-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Add support for multi attr strings (needed for link alt_names).
We record the length individual strings in a len member, to do
the same for multi-attr create a struct ynl_string in ynl.h
and use it as a layer holding both the string and its length.
Since strings may be arbitrary length dynamically allocate each
individual one.
Adjust arg_member and struct member to avoid spacing the double
pointers to get "type **name;" rather than "type * *name;"
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Link: https://patch.msgid.link/20250429154704.2613851-8-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Allow CRUD-style notification where the notification is more
like the response to the request, which can optionally be
looped back onto the requesting socket. Since the notification
and request are different ops in the spec, for example:
-
name: delrule
doc: Remove an existing FIB rule
attribute-set: fib-rule-attrs
do:
request:
value: 33
attributes: *fib-rule-all
-
name: delrule-ntf
doc: Notify a rule deletion
value: 33
notify: getrule
We need to find the request by ID. Ideally we'd detect this model
from the spec properties, rather than assume that its what all
classic netlink families do. But maybe that'd cause this model
to spread and its easy to get wrong. For now assume CRUD == classic.
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Link: https://patch.msgid.link/20250429154704.2613851-7-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Classic netlink makes extensive use of flags. Support specifying
them the same way as attributes are specified (using a helper),
for example:
rt_link_newlink_req_set_nlflags(req, NLM_F_CREATE | NLM_F_ECHO);
Wrap the code up in a RenderInfo predicate. I think that some
genetlink families may want this, too. It should be easy to
add a spec property later.
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Link: https://patch.msgid.link/20250429154704.2613851-5-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
When user calls request_attrA_set() multiple times (for the same
attribute), and attrA is of type which allocates memory -
we try to free the previously associated values. For array
types (including multi-attr) we have only freed the array,
but the array may have contained pointers.
Refactor the code generation for free attr and reuse the generated
lines in setters to flush out the previous state. Since setters
are static inlines in the header we need to add forward declarations
for the free helpers of pure nested structs. Track which types get
used by arrays and include the right forwad declarations.
At least ethtool string set and bit set would not be freed without
this. Tho, admittedly, overriding already set attribute twice is likely
a very very rare thing to do.
Fixes: be5bea1cc0 ("net: add basic C code generators for Netlink")
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20250414211851.602096-4-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
YNL C can now generate code for simple classic netlink families.
Include rt-route in the Makefile for generation and add a sample.
$ ./tools/net/ynl/samples/rt-route
oif: wlp0s20f3 gateway: 192.168.1.1
oif: wlp0s20f3 dst: 192.168.1.0/24
oif: vpn0 dst: fe80::/64
oif: wlp0s20f3 dst: fe80::/64
oif: wlp0s20f3 gateway: fe80::200:5eff:fe00:201
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://patch.msgid.link/20250410014658.782120-14-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
If the type for the response to do and dump are the same we don't
generate it twice. This is called "type_consistent" in the generator.
Consider operations which only have dump to also be consistent.
This removes unnecessary "_dump" from the names. There's a number
of GET ops in classic Netlink which only have dump handlers.
Make sure we output the "onesided" types, normally if the type
is consistent we only output it when we render the do structures.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20250410014658.782120-11-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
C codegen skips generating the structs if request/reply has no attrs.
In such cases the request op takes no argument and return int
(rather than response struct). In case of classic netlink a lot of
information gets passed using the fixed struct, however, so adjust
the logic to consider a request empty only if it has no attrs _and_
no fixed struct.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20250410014658.782120-9-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Classic netlink has static family IDs specified in YAML,
there is no family name -> ID lookup. Support providing
the ID info to the library via the generated struct and
make library use it. Since NETLINK_ROUTE is ID 0 we need
an extra boolean to indicate classic_id is to be used.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://patch.msgid.link/20250410014658.782120-8-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
