[ Upstream commit a44f2eb106a46f2275a79de54ce0ea63e4f3d8c8 ]
NLMSG_DONE contains an error code, it has to be extracted.
Prior to this change all dumps will end in success,
and in case of failure the result is silently truncated.
Fixes: e4b48ed460 ("tools: ynl: add a completely generic client")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://lore.kernel.org/r/20240420020827.3288615-1-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit f6c8f5e8694c7a78c94e408b628afa6255cc428a ]
When we set members of simple nested structures in requests
we need to set "presence" bits for all the nesting layers
below. This has nothing to do with the presence type of
the last layer.
Fixes: be5bea1cc0 ("net: add basic C code generators for Netlink")
Reviewed-by: Breno Leitao <leitao@debian.org>
Link: https://lore.kernel.org/r/20240321020214.1250202-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit b6c65eb20ffa8e3bd89f551427dbeee2876d72ca ]
We never increment the group number iterator, so all groups
get recorded into index 0 of the mcast_groups[] array.
As a result YNL can only handle using the last group.
For example using the "netdev" sample on kernel with
page pool commands results in:
$ ./samples/netdev
YNL: Multicast group 'mgmt' not found
Most families have only one multicast group, so this hasn't
been noticed. Plus perhaps developers usually test the last
group which would have worked.
Fixes: 86878f14d7 ("tools: ynl: user space helpers")
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Link: https://lore.kernel.org/r/20240226214019.1255242-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 5d78b73e851455d525a064f3b042b29fdc0c1a4a ]
Make sure to free the already-parsed mcast_groups if
we don't get an ack from the kernel when reading family info.
This is part of the ynl_sock_create() error path, so we won't
get a call to ynl_sock_destroy() to free them later.
Fixes: 86878f14d7 ("tools: ynl: user space helpers")
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Link: https://lore.kernel.org/r/20240220161112.2735195-3-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit e4fe082c38cd74a8fa384bc7542cf3edf1cb7318 ]
There is one common error handler in ynl - ynl_cb_error().
It expects priv to be a pointer to struct ynl_parse_arg AKA yarg.
To avoid potential crashes if we encounter a stray NLMSG_ERROR
always pass yarg as priv (or a struct which has it as the first
member).
ynl_cb_null() has a similar problem directly - it expects yarg
but priv passed by the caller is ys.
Found by code inspection.
Fixes: 86878f14d7 ("tools: ynl: user space helpers")
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Link: https://lore.kernel.org/r/20240220161112.2735195-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Make sure that the command values used for replies are correct. This is
only affecting generated userspace helpers, no change on kernel code.
Fixes: 7199c86247 ("netlink: specs: devlink: add commands that do per-instance dump")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Link: https://lore.kernel.org/r/20231012115811.298129-1-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add support for using NLM_F_REPLACE, _EXCL, _CREATE and _APPEND flags
in requests.
Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://lore.kernel.org/r/20230825122756.7603-10-donald.hunter@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add support for the 'array-nest' attribute type that is used by several
netlink-raw families.
Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://lore.kernel.org/r/20230825122756.7603-9-donald.hunter@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Move decode_fixed_header into YnlFamily and add a _fixed_header_size
method to allow extack decoding to skip the fixed header.
Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://lore.kernel.org/r/20230825122756.7603-7-donald.hunter@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
We use a tempfile for code generation, to avoid wiping the target
file out if the code generator crashes. File contents are copied
from tempfile to actual destination at the end of main().
uAPI generation is relatively simple so when generating the uAPI
header we return from main() early, and never reach the "copy code
over" stage. Since commit under Fixes uAPI headers are not updated
by ynl-gen.
Move the copy/commit of the code into CodeWriter, to make it
easier to call at any point in time. Hook it into the destructor
to make sure we don't miss calling it.
Fixes: f65f305ae0 ("tools: ynl-gen: use temporary file for rendering")
Link: https://lore.kernel.org/r/20230824212431.1683612-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Differentiate between empty list and None for member lists.
New families may want to create request responses with no attribute.
If we treat those the same as None we end up rendering
a full parsing policy in user space, instead of an empty one.
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://lore.kernel.org/r/20230824003056.1436637-5-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
We look for attributes inside do.request, but there's another
layer of nesting in the spec, look inside do.request.attributes.
This bug had no effect as all global policies we generate (fou)
seem to be full, anyway, and we treat full and empty the same.
Next patch will change the treatment of empty policies.
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://lore.kernel.org/r/20230824003056.1436637-4-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Recent changes made us assume that input for binary data is in hex.
When using YNL as a Python library it's possible to pass in raw bytes.
Bring the ability to do that back.
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://lore.kernel.org/r/20230824003056.1436637-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Currently any error during render leads to output an empty file.
That is quite annoying when using tools/net/ynl/ynl-regen.sh
which git greps files with content of "YNL-GEN.." and therefore ignores
empty files. So once you fail to regen, you have to checkout the file.
Avoid that by rendering to a temporary file first, only at the end
copy the content to the actual destination.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
When developing specs its useful to know which attr space
YNL was trying to find an attribute in on key error.
Instead of printing:
KeyError: 0
add info about the space:
Exception: Space 'vport' has no attribute with value '0'
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://lore.kernel.org/r/20230814205627.2914583-4-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Allow user to pass port index for health reporter dump request.
Re-generate the related code.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230811155714.1736405-14-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Extend per-instance dump command definitions to accept instance
attributes. Allow parsing of devlink handle attributes so they could
be used for instance selection.
Re-generate the related code.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230811155714.1736405-12-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add the definitions for the commands that do per-instance dump
and re-generate the related code.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230811155714.1736405-8-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When dont-validate flags are filtered out for do/dump op, the list may
be empty. In that case, avoid rendering the validate field.
Fixes: fa8ba3502a ("ynl-gen-c.py: render netlink policies static for split ops")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230808090344.1368874-1-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add missing dump op for info-get command and re-generate related
devlink-user.[ch] code.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230803111340.1074067-10-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When policies are rendered for split ops, they are consumed in the same
file. No need to expose them for user outside, make them static.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230803111340.1074067-5-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Directional model limitation is only applicable for uapi mode.
For kernel mode, the code is generated correctly using right cmd values
for do/dump requests. Lift the limitation.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230803111340.1074067-4-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
For split ops, do and dump has different meaningful values in
validate field.
Fix the rendering to allow the values per op type as follows:
do: strict
dump: dump, strict-dump
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230803111340.1074067-3-jiri@resnulli.us
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Technically we don't have to keep extending the sample, but it
feels useful to run these tools locally to confirm everything
is working.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230727163001.3952878-5-sdf@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Also add support to pass topdir to ynl-regen.sh (Jakub) and call
it from the makefile to update the UAPI headers.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Co-developed-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230727163001.3952878-4-sdf@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Simon mentioned in another thread that it makes kdoc happy
and Jakub confirms that commit e27cb89a22 ("scripts: kernel-doc: support
private / public marking for enums") actually added the needed
support.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20230727163001.3952878-3-sdf@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When attribute is enum type and marked as multi-attr, the netlink
respond is not parsed, fails with stack trace:
Traceback (most recent call last):
File "/net-next/tools/net/ynl/./test.py", line 520, in <module>
main()
File "/net-next/tools/net/ynl/./test.py", line 488, in main
dplls=dplls_get(282574471561216)
File "/net-next/tools/net/ynl/./test.py", line 48, in dplls_get
reply=act(args)
File "/net-next/tools/net/ynl/./test.py", line 41, in act
reply = ynl.dump(args.dump, attrs)
File "/net-next/tools/net/ynl/lib/ynl.py", line 598, in dump
return self._op(method, vals, dump=True)
File "/net-next/tools/net/ynl/lib/ynl.py", line 584, in _op
rsp_msg = self._decode(gm.raw_attrs, op.attr_set.name)
File "/net-next/tools/net/ynl/lib/ynl.py", line 451, in _decode
self._decode_enum(rsp, attr_spec)
File "/net-next/tools/net/ynl/lib/ynl.py", line 408, in _decode_enum
value = enum.entries_by_val[raw].name
TypeError: unhashable type: 'list'
error: 1
Redesign _decode_enum(..) to take a enum int value and translate
it to either a bitmask or enum name as expected.
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://lore.kernel.org/r/20230725101642.267248-3-arkadiusz.kubalewski@intel.com
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Remove wrong index adjustment, which is leftover from adding
support for sparse enums.
enum.entries_by_val() function shall not subtract the start-value, as
it is indexed with real enum value.
Fixes: c311aaa74c ("tools: ynl: fix enum-as-flags in the generic CLI")
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Link: https://lore.kernel.org/r/20230725101642.267248-2-arkadiusz.kubalewski@intel.com
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Przemek suggests that I shouldn't accuse GCC of witchcraft,
there is a simpler explanation for why we need manual define.
scripts/headers_install.sh modifies the guard, removing _UAPI.
That's why including a kernel header from the tree and from
/usr leads to duplicate definitions.
This also solves the mystery of why I needed to include
the header conditionally. I had the wrong guards for most
cases but ethtool.
Suggested-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Link: https://lore.kernel.org/r/20230621231719.2728928-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The inability to include the uAPI headers directly in tools/
is one of the bigger annoyances of compiling user space code.
Most projects trade the pain for smaller inconvenience of having
to copy the headers under tools/include.
In case of netlink headers I think that we can avoid both.
Netlink family headers are simple and should be self-contained.
We can try to twiddle the Makefile a little to force-include
just the family header, and use system headers for the rest.
This works fairly well. There are two warts - for some reason
if we specify -include $path/family.h as a compilation flag,
the #ifdef header guard does not seem to work. So we need
to throw the guard in on the command line as well. Seems like
GCC detects that the header is different and tries to include
both. Second problem is that make wants hash sign to be escaped
or not depending on the version. Sigh.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Instead of reimplementing policies in MutliAttr for every
underlying type forward the calls to the base type.
This will be needed for DPLL which uses a multi-attr nest,
and currently gets an invalid NLA_NEST policy generated.
Reviewed-by: Jacob Keller <Jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Scalar range validation assumes enums start at 0.
Teach it to properly calculate the value range.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Configuring / reading ring sizes and counts is a fairly common
operation for ethtool netlink. Present a sample doing that with
YNL:
$ ./ethtool
Channels:
enp1s0: combined 1
eni1np1: combined 1
eni2np1: combined 1
Rings:
enp1s0: rx 256 tx 256
eni1np1: rx 0 tx 0
eni2np1: rx 0 tx 0
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Generate the protocol code for ethtool. Skip the stats
for now, they are the only outlier in terms of complexity.
Stats are a sort-of semi-polymorphic (attr space of a nest
depends on value of another attr) or a type-value-scalar,
depending on how one wants to look at it...
A challenge for another time.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Ethtool has an attribute set called stringset, from which
we'll generate struct ethtool_stringset. Unfortunately,
the old ethtool header declares enum ethtool_stringset
(the same name), to which compilers object.
This seems unavoidable. Check struct names against known
constants and append an underscore if conflict is detected.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
If attr set or enum has empty enum name we need to use u32 or int
as function arguments and struct members.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Ethtool's PSE PoDL has a attr nest with different prefixes:
/* Power Sourcing Equipment */
enum {
ETHTOOL_A_PSE_UNSPEC,
ETHTOOL_A_PSE_HEADER, /* nest - _A_HEADER_* */
ETHTOOL_A_PODL_PSE_ADMIN_STATE, /* u32 */
ETHTOOL_A_PODL_PSE_ADMIN_CONTROL, /* u32 */
ETHTOOL_A_PODL_PSE_PW_D_STATUS, /* u32 */
Header has a prefix of ETHTOOL_A_PSE_ and other attrs prefix of
ETHTOOL_A_PODL_PSE_ we can't cover them uniformly.
If PODL was after PSE life would be easy.
Now we either need to add prefixes to attr names which is yucky
or support setting prefix name per attr.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
ynl-regen needs to know the arguments used to generate a file.
Record excluded ops and, while at it, user headers.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
The ethtool family has a small handful of quite tricky ops
and a lot of simple very useful ops. Teach ynl-gen to skip
ops so that we can bypass the tricky ones.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Netlink specs support both events and notifications (former can
define their own message contents). Plug in missing code to
generate types, parsers and include events into notification
tables.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Don't modify the raw dicts (as loaded from YAML) to pretend
that the notify attributes also exist on the ops. This makes
the code easier to follow.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Common notification handler was supposed to be a way for the user
to parse the notifications from a socket synchronously.
I don't think we'll end up using it, ynl_ntf_check() works for
all known use cases.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Jakub Kicinski