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

[Samudrala, Sridhar]

On 12/18/2014 3:07 PM, Roopa Prabhu wrote:
> On 12/18/14, 11:21 AM, John Fastabend wrote:
> > 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.
> >
> > This was intentionally done. Its already used with NIC devices to
> > configure embedded bridge settings such as VEB/VEPA.
>
> that helps my case, thanks.

So the user interface to set/get the per-port attributes will be via 
'bridge', not 'ip'

    bridge link set dev sw0p1 port_attr bcast_flooding 1 self
    bridge link get dev sw0p1 port_attr bcast_flooding self

We also need an interface to set per-switch attributes. Can this work?
    bridge link set dev sw0 sw_attr bcast_flooding 1 master
where sw0 is a bridge representing the hardware switch.

> > 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 majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/