From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [RFC PATCH] net: bridge: multicast querier per VLAN support Date: Wed, 18 Apr 2018 08:54:07 -0700 Message-ID: <20180418085407.4f5723de@xeon-e3> References: <20180418120713.GA10742@troglobit> <20180418130718.GA16044@troglobit> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Joachim Nilsson , netdev@vger.kernel.org, roopa To: Nikolay Aleksandrov Return-path: Received: from mail-pg0-f41.google.com ([74.125.83.41]:45847 "EHLO mail-pg0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751232AbeDRPyK (ORCPT ); Wed, 18 Apr 2018 11:54:10 -0400 Received: by mail-pg0-f41.google.com with SMTP id z21so1056898pgv.12 for ; Wed, 18 Apr 2018 08:54:10 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 18 Apr 2018 16:14:26 +0300 Nikolay Aleksandrov wrote: > On 18/04/18 16:07, Joachim Nilsson wrote: > > On Wed, Apr 18, 2018 at 03:31:57PM +0300, Nikolay Aleksandrov wrote: > >> On 18/04/18 15:07, Joachim Nilsson wrote: > >>> - 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! :) > > > >>> - 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! > > > >>> - 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.