Message-ID: <20150331215245.GA6515@nanopsycho.orion>
Date:	Tue, 31 Mar 2015 23:52:45 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	"Arad, Ronen" <ronen.arad@...el.com>
Cc:	Scott Feldman <sfeldma@...il.com>, Netdev <netdev@...r.kernel.org>,
	roopa <roopa@...ulusnetworks.com>,
	Guenter Roeck <linux@...ck-us.net>,
	Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH net-next 11/18] switchdev: remove old
 netdev_switch_port_bridge_setlink

Tue, Mar 31, 2015 at 09:15:35PM CEST, ronen.arad@...el.com wrote:
>
>
>>-----Original Message-----
>>From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.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@...el.com wrote:
>>>
>>>
>>>>-----Original Message-----
>>>>From: Scott Feldman [mailto:sfeldma@...il.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 <ronen.arad@...el.com> wrote:
>>>>>
>>>>>
>>>>>>-----Original Message-----
>>>>>>From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
>>On
>>>>>>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 <roopa@...ulusnetworks.com> wrote:
>>>>>>> On 3/30/15, 1:40 AM, sfeldma@...il.com wrote:
>>>>>>>>
>>>>>>>> From: Scott Feldman <sfeldma@...il.com>
>>>>>>>>
>>>>>>>> New attr-based bridge_setlink can recurse lower devs and recover on
>>err,
>>>>>>>> so
>>>>>>>> remove old wrapper.  Also, restore br_setlink back to original and
>>don't
>>>>>>>> call
>>>>>>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>>>>>>> into
>>>>>>>> port driver for SELF.
>>>>>>>>
>>>>>>>> Signed-off-by: Scott Feldman <sfeldma@...il.com>
>>>>>>>
>>>>>>> removing this now requires every vlan add to be a two step process, why
>>?
>>>>>>
>>>>>>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 10
>>>>>>
>>>>>>
>>>>>>> bridge vlan add dev swp1 vid 10
>>>>>>> bridge vlan add dev swp1 vid 10 self
>>>>>>
>>>>>>Not 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.
>>
>>Jiri
>>
>Let'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 :/

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists