Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration

From: Jiri Pirko <[email-masked]>
Date: Fri, 19 Dec 2014 09:14:28 +0100

Thu, Dec 18, 2014 at 11:43:06PM CET, [email-masked] wrote:
>
>
>>-----Original Message-----
>>From: [email-masked] [mailto:[email-masked]] On
>>Behalf Of John Fastabend
>>Sent: Thursday, December 18, 2014 9:21 PM
>>To: Roopa Prabhu; Varlese, Marco
>>Cc: [email-masked]; Thomas Graf; Jiri Pirko; [email-masked]; linux-
>>[email-masked]
>>Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port
>>configuration
>>
>>On 12/18/2014 10:14 AM, Roopa Prabhu wrote:
>>> On 12/18/14, 10:02 AM, Varlese, Marco wrote:
>>>> Removed unnecessary content for ease of reading...
>>>>
>>>>>>>>>> +/* Switch Port Attributes section */
>>>>>>>>>> +
>>>>>>>>>> +enum {
>>>>>>>>>> +    IFLA_ATTR_UNSPEC,
>>>>>>>>>> +    IFLA_ATTR_LEARNING,
>>>>>>>>> Any reason you want learning here ?. This is covered as part  of
>>>>>>>>> the bridge setlink attributes.
>>>>>>>>>
>>>>>>>> Yes, because the user may _not_ want to go through a bridge
>>>>>>>> interface
>>>>>>> necessarily.
>>>>>>> But, the bridge setlink/getlink interface was changed to accommodate
>>>>> 'self'
>>>>>>> for exactly such cases.
>>>>>>> I kind of understand your case for the other attributes (these are
>>>>>>> per port settings that switch asics provide).
>>>>>>>
>>>>>>> However, i don't understand the reason to pull in bridge attributes
>>here.
>>>>>>>
>>>>>> Maybe, I am missing something so you might help. The learning attribute -
>>>>> in my case - it is like all other attributes: a port attribute (as you
>>said, port
>>>>> settings that the switch provides per port).
>>>>>> So, what I was saying is "why the user shall go through a bridge to
>>configure
>>>>> the learning attribute"? From my perspective, it is as any other attribute
>>and
>>>>> as such configurable on the port.
>>>>>
>>>>> Thinking about this some more, i don't see why any of these attributes
>>>>> (except loopback. I dont understand the loopback attribute) cant be part
>>of
>>>>> the birdge port attributes.
>>>>>
>>>>> With this we will end up adding l2 attributes in two places: the general
>>link
>>>>> attributes and bridge attributes.
>>>>>
>>>>> And since we have gone down the path of using ndo_bridge_setlink/getlink
>>>>> with 'self'....we should stick to that for all l2 attributes.
>>>>>
>>>>> The idea of overloading ndo_bridge_set/getlink, was to have the same set
>>of
>>>>> attributes but support both cases where the user wants to go through the
>>>>> bridge driver or directly to the switch port driver. So, you are not
>>really going
>>>>> through the bridge driver if you use 'self' and
>>ndo_bridge_setlink/getlink.
>>>>>
>>
>>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch
>>>> was that your patch and mine were supplementary ("I think Roopa's
>>>> patches are supplementary. Not all switchdev users will be backed
>>>> with a Linux Bridge. I therefore welcome your patches very
>>>> much")... I also understood by others that the patch made sense for
>>>> the same reason. I simply do not understand why these attributes
>>>> (and maybe others in the future) could not be configured directly
>>>> on a standard port but have to go through a bridge.
>>>>
>>> ok, i am very confused in that case. The whole moving of bridge
>>> attributes from the bridge driver to rtnetlink.c was to make the
>>> bridge attributes accessible to any driver who wants to set l2/bridge
>>> attributes on their switch ports. So, its unclear to me why we are
>>> doing this parallel thing again. This move to rtnetlink.c was done
>>> during the recent rocker support. so, maybe scott/jiri can elaborate
>>> more.
>>
>>
>>Not sure if this will add to the confusion or help. But you do not
>>need to have the bridge.ko loaded or netdev's attached to a bridge
>>to use the setlink/getlink ndo ops and netlink messages.
>
>No you don't need bridge.ko to implement ndo_bridge_setlink/getlink. Rtnetlink invokes those ndos from code which does not depend on CONFIG_BRIDGE or the presence of bridge.ko.
>Calling some bridge exported functions such as br_fdb_external_learn_add/del requires the presence of bridge.ko and it only makes sense when the switch port device is enslaved to a bridge.

Note I plan to change br_fdb_external_learn_add/del to (semi-)generic
notifier very soon.

>
>>
>>This was intentionally done. Its already used with NIC devices to
>>configure embedded bridge settings such as VEB/VEPA.
>>
>>I think I'm just repeating Roopa though.
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe netdev" in
>>the body of a message to [email-masked]
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html