Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink
From: Jiri Pirko <jiri@resnulli.us>
Date: 2015-03-31 21:52:48
Tue, Mar 31, 2015 at 09:15:35PM CEST, ronen.arad@intel.com wrote:
quoted
-----Original Message----- From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Jiri Pirko Sent: Monday, March 30, 2015 10:53 PM To: Arad, Ronen Cc: Scott Feldman; Netdev; roopa; Guenter Roeck; Florian Fainelli Subject: Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink Tue, Mar 31, 2015 at 02:08:34AM CEST, ronen.arad@intel.com wrote:quoted
quoted
-----Original Message----- From: Scott Feldman [mailto:sfeldma@gmail.com] Sent: Monday, March 30, 2015 2:28 PM To: Arad, Ronen Cc: roopa; Netdev; Jirí Pírko; Guenter Roeck; Florian Fainelli Subject: Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink On Mon, Mar 30, 2015 at 1:46 PM, Arad, Ronen [off-list ref] wrote:quoted
quoted
-----Original Message----- From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]Onquoted
quoted
quoted
quoted
Behalf Of Scott Feldman Sent: Monday, March 30, 2015 1:20 PM To: roopa Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli Subject: Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink On Mon, Mar 30, 2015 at 6:23 AM, roopa [off-list ref] wrote:quoted
On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:quoted
From: Scott Feldman <redacted> New attr-based bridge_setlink can recurse lower devs and recover onerr,quoted
quoted
quoted
quoted
quoted
quoted
so remove old wrapper. Also, restore br_setlink back to original anddon'tquoted
quoted
quoted
quoted
quoted
quoted
call into SELF port driver. rtnetlink.c:bridge_setlink already does a call into port driver for SELF. Signed-off-by: Scott Feldman <redacted>removing this now requires every vlan add to be a two step process, why?quoted
quoted
quoted
quoted
No, that's not true. You want to use ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then using either vlan driver standalone or the bridge driver vlan support will work. Try it. Implement ndo_vlan_rx_add_vid in your port driver and verify you get called to add VLAN to port with either: bridge vlan add dev swp1 vid 10 -or- vconfig add swp1 10 Same for deleting a VLAN, either of these two commands call into the port driver ndo_vlan_rx_kill_vid: bridge vlan del dev swp1 vid 10 -or- vconfig rem swp1 10quoted
bridge vlan add dev swp1 vid 10 bridge vlan add dev swp1 vid 10 selfNot necessary. The first command is sufficient if using ndo_vlan_rx_add_vid.This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not provide the vinfo flags PVID and UNTAGGED. Therefore it is not an adequate replacement for propagating setlink/dellink messages to the swithport driver or an alternative via swdev_attr.Glad you bring that point up. I think these can get cast as port attrs and set using swdev_attr. This is something swdev attr should open up is allowing more settings to be pushed down to port driver. I'll look into this one and include it with v2.It could be beneficial to build extensibility into swdev_attr. An experimenter attribute designed to carry arbitrary data could allow for passing new attributes and implementation specific attributes without affecting any existing switchdev driver:Warning sign... I believe that we don't want this. It is very easy to add attribute for anything. Having this "universal attribuute" only allows wild things... Thanks. JiriLet's say a switch device has some behavior that is not common to all switch devices. Defining an explicit attribute might not be desirable in that case. For example let's say SOMEswitch device implements VLAN priority markdown using a table. It could be represented as a table of 8 bytes. To support this feature, there is a need to allow a user-space tool to program such table in SOMEswitch device. What mechanisms are available for that?
If you want to expose feature X that's ok, just do it. I see no problem in that. Please, just do not introduce kernel bypass. That simply won't fly...
This could be done via sysfs entries. However, I believe we're trying to make Netlink and iproute2 the protocol/tool for all switch device
It does not matter if universal attr is exposed via netlink or sysfs. Either way, I believe it is unacceptable.
configuration. Therefore, I think that a generic mechanism, integrated with existing switchdev supported tools would be best. Alternative approach would be for SOMEswitch driver to introduce its own generic netlink family and introduce SOMEswitch version of iproute2 or similar tool to augment SOMEswitch HW configuration where (or as long as) no common way to control such feature is available
This is definitelly unacceptable :/