Feature #300
closedConvert sysfs config attributes to genl
Added by Sven Eckelmann about 8 years ago. Updated over 4 years ago.
100%
Description
Jiri Pirko is thinking that sysfs use to config attributes of object/batman-adv is "is a huge mistake to use sysfs for things like this." https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20161027190150.7880-4-sw@simonwunderlich.de/
Instead he "suggest to rework the api to use genl entirely."
Updated by Sven Eckelmann almost 8 years ago
- Assignee set to batman-adv developers
Updated by Sven Eckelmann about 6 years ago
- Status changed from New to In Progress
Some RFC patches for batctl and batman-adv are available:
Updated by Sven Eckelmann about 6 years ago
- Estimated time changed from 40.00 h to 80.00 h
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 attributeOk, 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
Updated by Sven Eckelmann almost 6 years ago
There is now a new patch series for batman-adv https://patchwork.open-mesh.org/project/b.a.t.m.a.n./list/?series=181
The batctl implementation can currently only receive the netlink messages of the "config" mcast group. But at least the GET replies will look the same: https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20181123161442.13461-2-sven@narfation.org/
Updated by Sven Eckelmann almost 6 years ago
There are now two new RFC series:
Updated by Sven Eckelmann almost 6 years ago
The RFC patch series for batman-adv was updated:
- batman-adv: https://patchwork.open-mesh.org/project/b.a.t.m.a.n./list/?series=202
- cover letter: https://patchwork.open-mesh.org/cover/17730/
The batctl patch series from #300#note-6 can still be used because the ABI didn't change.
Updated by Sven Eckelmann almost 6 years ago
- Status changed from In Progress to Resolved
- Target version set to 2019.1
- % Done changed from 0 to 100
The two patch series were now posted:
Updated by Sven Eckelmann almost 6 years ago
- Status changed from Resolved to Closed
Added to net-next and batman-adv master branch.