Re: [RFC PATCH] net: bridge: multicast querier per VLAN support

On Wed, 18 Apr 2018 16:14:26 +0300
Nikolay Aleksandrov [off-list ref] wrote:

On 18/04/18 16:07, Joachim Nilsson wrote:

quoted

On Wed, Apr 18, 2018 at 03:31:57PM +0300, Nikolay Aleksandrov wrote:  

quoted

On 18/04/18 15:07, Joachim Nilsson wrote:  

quoted

- First of all, is this patch useful to anyone  

Obviously to us as it's based on our patch. :-)
We actually recently discussed what will be needed to make it acceptable to upstream.  

Great! :)
  

quoted

quoted

- The current br_multicast.c is very complex.  The support for both IPv4
   and IPv6 is a no-brainer, but it also has #ifdef VLAN_FILTERING and
   'br->vlan_enabled' ... this has likely been discussed before, but if
   we could remove those code paths I believe what's left would be quite
   a bit easier to read and maintain.  

br->vlan_enabled has a wrapper that can be used without ifdefs, as does br_vlan_find()
so in short - you can remove the ifdefs and use the wrappers,  they'll degrade to always
false/null when vlans are disabled.  

Thanks, I'll have a look at that and prepare an RFC v2!
  

quoted

quoted

- Many per-bridge specific multicast sysfs settings may need to have a
   corresponding per-VLAN setting, e.g. snooping, query_interval, etc.
   How should we go about that? (For status reporting I have a proposal)  

We'll have to add more to the per-vlan context, but yes it has to happen.
It will be only netlink interface for config/retrieval, no sysfs.  

Some settings are possible to do with sysfs, like multicast_query_interval
and ...  

We want to avoid sysfs in general, all of networking config and stats
are moving to netlink. It is better controlled and structured for such
changes, also provides nice interfaces for automatic  type checks etc.

Also (but a minor reason) there is no tree/entity in sysfs for the vlans
where to add this. It will either have to be a file which does some
format string hack (like us currently) or will need to add new tree for
them which I'd really like to avoid for the bridge.

In general, all bridge attributes need to show in netlink and sysfs.
Sysfs is easier for scripting from languages.