[PATCH 03/17] batman-adv: Add network_coding and mcast sysfs files to README

From: Sven Eckelmann <redacted>

Signed-off-by: Sven Eckelmann <redacted>
Signed-off-by: Simon Wunderlich <redacted>
---
 Documentation/networking/batman-adv.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/batman-adv.txt b/Documentation/networking/batman-adv.txt
index d414e60..8afa991 100644
--- a/Documentation/networking/batman-adv.txt
+++ b/Documentation/networking/batman-adv.txt

@@ -71,10 +71,11 @@ All  mesh  wide  settings  can be found in batman's own interface
 folder:
 
 # ls /sys/class/net/bat0/mesh/
-#aggregated_ogms        distributed_arp_table  gw_sel_class    orig_interval
-#ap_isolation           fragmentation          hop_penalty     routing_algo
-#bonding                gw_bandwidth           isolation_mark  vlan0
-#bridge_loop_avoidance  gw_mode                log_level
+# aggregated_ogms        fragmentation  isolation_mark  routing_algo
+# ap_isolation           gw_bandwidth   log_level       vlan0
+# bonding                gw_mode        multicast_mode
+# bridge_loop_avoidance  gw_sel_class   network_coding
+# distributed_arp_table  hop_penalty    orig_interval
 
 There is a special folder for debugging information:
 

-- 
2.10.1

Thu, Oct 27, 2016 at 09:01:36PM CEST, sw@simonwunderlich.de wrote:

I strongly believe it is a huge mistake to use sysfs for things like
this. This should be done via generic netlink api.

On Samstag, 29. Oktober 2016 12:33:01 CEST Jiri Pirko wrote:
[...]

quoted

--- a/Documentation/networking/batman-adv.txt
+++ b/Documentation/networking/batman-adv.txt

@@ -71,10 +71,11 @@ All  mesh  wide  settings  can be found in batman's own interface

folder:

# ls /sys/class/net/bat0/mesh/
-#aggregated_ogms        distributed_arp_table  gw_sel_class    orig_interval
-#ap_isolation           fragmentation          hop_penalty     routing_algo
-#bonding                gw_bandwidth           isolation_mark  vlan0
-#bridge_loop_avoidance  gw_mode                log_level
+# aggregated_ogms        fragmentation  isolation_mark  routing_algo
+# ap_isolation           gw_bandwidth   log_level       vlan0
+# bonding                gw_mode        multicast_mode
+# bridge_loop_avoidance  gw_sel_class   network_coding
+# distributed_arp_table  hop_penalty    orig_interval

I strongly believe it is a huge mistake to use sysfs for things like
this. This should be done via generic netlink api.

This doesn't change the problem that it is already that way. This patch
only adds the list of available files to the README.

Kind regards,
	Sven

Sat, Oct 29, 2016 at 12:37:07PM CEST, sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org wrote:

On Samstag, 29. Oktober 2016 12:33:01 CEST Jiri Pirko wrote:
[...]

quoted

quoted

--- a/Documentation/networking/batman-adv.txt
+++ b/Documentation/networking/batman-adv.txt

@@ -71,10 +71,11 @@ All  mesh  wide  settings  can be found in batman's own interface

folder:

# ls /sys/class/net/bat0/mesh/
-#aggregated_ogms        distributed_arp_table  gw_sel_class    orig_interval
-#ap_isolation           fragmentation          hop_penalty     routing_algo
-#bonding                gw_bandwidth           isolation_mark  vlan0
-#bridge_loop_avoidance  gw_mode                log_level
+# aggregated_ogms        fragmentation  isolation_mark  routing_algo
+# ap_isolation           gw_bandwidth   log_level       vlan0
+# bonding                gw_mode        multicast_mode
+# bridge_loop_avoidance  gw_sel_class   network_coding
+# distributed_arp_table  hop_penalty    orig_interval

I strongly believe it is a huge mistake to use sysfs for things like
this. This should be done via generic netlink api.

This doesn't change the problem that it is already that way. This patch
only adds the list of available files to the README.

Sure. Just found out you did it like that. Therefore I commented. I
suggest to rework the api to use genl entirely.

Kind regards,
Sven

On Sat, Oct 29, 2016 at 12:56:28PM +0200, Jiri Pirko wrote:

quoted

quoted

I strongly believe it is a huge mistake to use sysfs for things like
this. This should be done via generic netlink api.

This doesn't change the problem that it is already that way. This patch
only adds the list of available files to the README.

Sure. Just found out you did it like that. Therefore I commented. I
suggest to rework the api to use genl entirely.

Hi Jiri,

Thanks for sharing your thoughts!

Could you explain a bit more on which disadvantages you see in
the usage of sysfs here?

Regards, Linus

On Dienstag, 27. März 2018 17:43:08 CEST Linus Lüssing wrote:

On Sat, Oct 29, 2016 at 12:56:28PM +0200, Jiri Pirko wrote:

quoted

quoted

quoted

I strongly believe it is a huge mistake to use sysfs for things like
this. This should be done via generic netlink api.

This doesn't change the problem that it is already that way. This patch
only adds the list of available files to the README.

Sure. Just found out you did it like that. Therefore I commented. I
suggest to rework the api to use genl entirely.

Hi Jiri,

Thanks for sharing your thoughts!

Could you explain a bit more on which disadvantages you see in
the usage of sysfs here?

Linus is asking because of following patch: 
https://patchwork.open-mesh.org/patch/17340/

Kind regards,
	Sven

Hi Jiri,

seems like you still haven't answered Linus' question.

On Montag, 7. Mai 2018 08:34:16 CEST Sven Eckelmann wrote:

On Dienstag, 27. März 2018 17:43:08 CEST Linus Lüssing wrote:

quoted

On Sat, Oct 29, 2016 at 12:56:28PM +0200, Jiri Pirko wrote:

quoted

quoted

quoted

I strongly believe it is a huge mistake to use sysfs for things like
this. This should be done via generic netlink api.

This doesn't change the problem that it is already that way. This patch
only adds the list of available files to the README.

Sure. Just found out you did it like that. Therefore I commented. I
suggest to rework the api to use genl entirely.

Hi Jiri,

Thanks for sharing your thoughts!

Could you explain a bit more on which disadvantages you see in
the usage of sysfs here?

Linus is asking because of following patch: 
https://patchwork.open-mesh.org/patch/17340/

The next patch with a similar problem would be 
https://patchwork.open-mesh.org/patch/17372/. It is rather important that you 
are discussing this with Linus Luessing and Marek Lindner.

Kind regards,
	Sven

Tue, Mar 27, 2018 at 05:43:08PM CEST, linus.luessing-djzkFPsfvsizQB+pC5nmwQ@public.gmane.org wrote:

On Sat, Oct 29, 2016 at 12:56:28PM +0200, Jiri Pirko wrote:

quoted

quoted

quoted

I strongly believe it is a huge mistake to use sysfs for things like
this. This should be done via generic netlink api.

This doesn't change the problem that it is already that way. This patch
only adds the list of available files to the README.

Sure. Just found out you did it like that. Therefore I commented. I
suggest to rework the api to use genl entirely.

Hi Jiri,

Thanks for sharing your thoughts!

Could you explain a bit more on which disadvantages you see in
the usage of sysfs here?

There are 2 major disadvantages.
1) You don't have any events on a change. An app has to poll in order to
   know what changed in kernel. Netlink handles this by sending
   multicast messages on a specific socket while whoever is interested
   gets the messages.
2) In sysfs, everything is string. There are even mixed values like
   "1 (means something)". There are no well defined values. Every driver
   can expose same things differently. In Netlink, you have well-defined
   attributes, with typed values. You can pass multiple attributes for
   the same value if needed.

In general, usage of sysfs in netdev subsystem is frowned upon. I would
suggest to convert your iface to Generic Netlink API and let the
existing sysfs API to rot.

Regards, Linus

Hi Jiri,

On 20/05/18 14:19, Jiri Pirko wrote:

Tue, Mar 27, 2018 at 05:43:08PM CEST, linus.luessing@c0d3.blue wrote:

quoted

On Sat, Oct 29, 2016 at 12:56:28PM +0200, Jiri Pirko wrote:

quoted

quoted

quoted

I strongly believe it is a huge mistake to use sysfs for things like
this. This should be done via generic netlink api.

This doesn't change the problem that it is already that way. This patch
only adds the list of available files to the README.

Sure. Just found out you did it like that. Therefore I commented. I
suggest to rework the api to use genl entirely.

Hi Jiri,

Thanks for sharing your thoughts!

Could you explain a bit more on which disadvantages you see in
the usage of sysfs here?

There are 2 major disadvantages.
1) You don't have any events on a change. An app has to poll in order to
   know what changed in kernel. Netlink handles this by sending
   multicast messages on a specific socket while whoever is interested
   gets the messages.
2) In sysfs, everything is string. There are even mixed values like
   "1 (means something)". There are no well defined values. Every driver
   can expose same things differently. In Netlink, you have well-defined
   attributes, with typed values. You can pass multiple attributes for
   the same value if needed.

In general, usage of sysfs in netdev subsystem is frowned upon. I would
suggest to convert your iface to Generic Netlink API and let the
existing sysfs API to rot.

Do you have any pointer about where this discussion took place? I
imagine it happened in conjunction with some patches intended to other
drivers/netdev changes.

Reading that could give us a sense of how strict/important/severe this
decision was and how to prioritize future work.

I am asking because we have been working on a new feature since several
months and this feature introduces a new sysfs knob.

Now, although I understand the recommendation of switching to netlink, I
find it a bit impractical to delay a new (and fairly big) feature,
simply because it uses a potentially obsolete, but current, API.

Any opinion about this?


Thanks a lot


Regards,


-- 
Antonio Quartulli