Jiri informed us that this approach is now frowned upon. Instead following implementation is preferred:
On Montag, 5. November 2018 11:54:12 CET Jiri Pirko wrote:
[...]
But let assume for now that you don't want to use this approach, we can also
have for example something like:
BATADV_CMD_SET_BRIDGE_LOOP_AVOIDANCE which then receives either one or no flag
BATADV_ATTR_BRIDGE_LOOP_AVOIDANCE to enable/disable bridge loop avoidance. And
a command BATADV_CMD_GET_BRIDGE_LOOP_AVOIDANCE which tells the client using
BATADV_ATTR_BRIDGE_LOOP_AVOIDANCE whether it was enabled or not. A dump
functionality for all options for this batadv object/class would then not
exist.
Got it. That is possible to do it this way. But if you would be able to
group the attrs somehow, that would be probably nice. 1:1 mapping
between cmds and attrs looks a bit weird.
Sidenote: I suggest not to use NLA_FLAG attribute type. Better to use
NLA_U8 of value 0/1. NLA_FLAG missing in message might mean 2 things:
1) user wants to set false
2) user is old and does not support this attribute
Ok, let me suggest something more specific (just to make sure we talk about
the same thing). We could use the commands:
> + /**
> + * @BATADV_CMD_GET_OPTION: Get option(s) from softif
> + */
> + BATADV_CMD_GET_OPTION,
> +
> + /**
> + * @BATADV_CMD_SET_OPTION: Set option for softif
> + */
> + BATADV_CMD_SET_OPTION,
> +
> + /**
> + * @BATADV_CMD_GET_OPTION_HARDIF: Get option(s) from a hardif of the
> + * current softif
> + */
> + BATADV_CMD_GET_OPTION_HARDIF,
> +
> + /**
> + * @BATADV_CMD_SET_OPTION_HARDIF: Set option for hardif of the
> + * current softif
> + */
> + BATADV_CMD_SET_OPTION_HARDIF,
> +
> + /**
> + * @BATADV_CMD_GET_OPTION_VLAN: Get option(s) from a VLAN of the
> + * current softif
> + */
> + BATADV_CMD_GET_OPTION_VLAN,
> +
> + /**
> + * @BATADV_CMD_SET_OPTION_VLAN: Set option for VLAN of the
> + * current softif
> + */
> + BATADV_CMD_SET_OPTION_VLAN,
> +
>
But instead of using BATADV_ATTR_OPTION_NAME+BATADV_ATTR_OPTION_TYPE+
BATADV_ATTR_OPTION_VALUE to send the infos up, we have a number of specific
netlink attributes [e.g. BATADV_ATTR_BRIDGE_LOOP_AVOIDANCE (u8)] that are used
to exchange the value directly. Multiple attributes can be in the messages
and dumping is possible but maybe requires to split all option in multiple
messages. Informational dumping (user readable) is then only possible when the
userspace part also knows all attributes. And sending things to the kernel is
only possible when the userspace also has explicit support for the attribute.
This is at least similar to how I did it in two previous projects but dropped
this approach when I saw that it was implemented differently for devlink and
team.
We can maybe modify the current way how options are registered to write/read directly to msg/attributes. But the main logic has to be modified anyway. And we cannot support something like the config command.
(A lot) more time has therefore to be spend on this topic before it is ready.
It should be checked whether NL80211_CMD_SET_WIPHY is a good reference.
And also NLA_FLAG should be dropped. Instead:
Sidenote: I suggest not to use NLA_FLAG attribute type. Better to use
NLA_U8 of value 0/1. NLA_FLAG missing in message might mean 2 things:
1) user wants to set false
2) user is old and does not support this attribute