| Message ID | 1426527159-8039-1-git-send-email-roopa@cumulusnetworks.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Mon, Mar 16, 2015 at 06:32:39PM CET, roopa@cumulusnetworks.com wrote: >From: Roopa Prabhu <roopa@cumulusnetworks.com> > >make it same as the netdev_switch_port_bridge_setlink/dellink >api (ie traverse lowerdevs to get to the switch port). > >removes "WARN_ON(!ops->ndo_switch_parent_id_get)" because >direct bridge ports can be stacked netdevices (like bonds >and team of switch ports) which may not imeplement this ndo. > >v2 to v3: > - remove changes to bond and team. Bring back the > transparently following lowerdevs like i initially > had for setlink/getlink > (http://www.spinics.net/lists/netdev/msg313436.html) > dave and scott feldman also seem to prefer it be that > way and move to non-transparent way of doing things > if we see a problem down the lane. > >v3 to v4: > - fix ret initialization > >Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Acked-by: Jiri Pirko <jiri@resnulli.us> Thanks Roopa! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16/03/15 10:32, roopa@cumulusnetworks.com wrote: > From: Roopa Prabhu <roopa@cumulusnetworks.com> > > make it same as the netdev_switch_port_bridge_setlink/dellink > api (ie traverse lowerdevs to get to the switch port). > > removes "WARN_ON(!ops->ndo_switch_parent_id_get)" because > direct bridge ports can be stacked netdevices (like bonds > and team of switch ports) which may not imeplement this ndo. > > v2 to v3: > - remove changes to bond and team. Bring back the > transparently following lowerdevs like i initially > had for setlink/getlink > (http://www.spinics.net/lists/netdev/msg313436.html) > dave and scott feldman also seem to prefer it be that > way and move to non-transparent way of doing things > if we see a problem down the lane. > > v3 to v4: > - fix ret initialization The changelog should be after "---" otherwise it will get included in the final git commit, we don't want that as this changelog only makes sense during the submission process. > > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> > --- > net/switchdev/switchdev.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c > index c9bfa00..e593b59 100644 > --- a/net/switchdev/switchdev.c > +++ b/net/switchdev/switchdev.c > @@ -47,11 +47,23 @@ EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get); > int netdev_switch_port_stp_update(struct net_device *dev, u8 state) > { > const struct swdev_ops *ops = dev->swdev_ops; > + struct net_device *lower_dev; > + struct list_head *iter; > + int ret = -EOPNOTSUPP, err; > > - if (!ops || !ops->swdev_port_stp_update) > - return -EOPNOTSUPP; > - WARN_ON(!ops->swdev_parent_id_get); > - return ops->swdev_port_stp_update(dev, state); > + if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD)) > + return ret; > + > + if (ops && ops->swdev_port_stp_update) > + return ops->swdev_port_stp_update(dev, state); > + > + netdev_for_each_lower_dev(dev, lower_dev, iter) { > + err = netdev_switch_port_stp_update(lower_dev, state); > + if (err && err != -EOPNOTSUPP) > + ret = err; > + } > + > + return ret; > } > EXPORT_SYMBOL_GPL(netdev_switch_port_stp_update); > >
On Mon, Mar 16, 2015 at 10:32 AM, <roopa@cumulusnetworks.com> wrote: > From: Roopa Prabhu <roopa@cumulusnetworks.com> > > make it same as the netdev_switch_port_bridge_setlink/dellink > api (ie traverse lowerdevs to get to the switch port). > > removes "WARN_ON(!ops->ndo_switch_parent_id_get)" because > direct bridge ports can be stacked netdevices (like bonds > and team of switch ports) which may not imeplement this ndo. > > v2 to v3: > - remove changes to bond and team. Bring back the > transparently following lowerdevs like i initially > had for setlink/getlink > (http://www.spinics.net/lists/netdev/msg313436.html) > dave and scott feldman also seem to prefer it be that > way and move to non-transparent way of doing things > if we see a problem down the lane. > > v3 to v4: > - fix ret initialization > > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> > --- > net/switchdev/switchdev.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c > index c9bfa00..e593b59 100644 > --- a/net/switchdev/switchdev.c > +++ b/net/switchdev/switchdev.c > @@ -47,11 +47,23 @@ EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get); > int netdev_switch_port_stp_update(struct net_device *dev, u8 state) > { > const struct swdev_ops *ops = dev->swdev_ops; > + struct net_device *lower_dev; > + struct list_head *iter; > + int ret = -EOPNOTSUPP, err; > > - if (!ops || !ops->swdev_port_stp_update) > - return -EOPNOTSUPP; > - WARN_ON(!ops->swdev_parent_id_get); > - return ops->swdev_port_stp_update(dev, state); > + if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD)) > + return ret; I would drop the NETIF_F_HW_SWITCH_OFFLOAD check. It's not telling you anything more than the next test for ops->swdev_port_stp_update. > + > + if (ops && ops->swdev_port_stp_update) > + return ops->swdev_port_stp_update(dev, state); > + > + netdev_for_each_lower_dev(dev, lower_dev, iter) { > + err = netdev_switch_port_stp_update(lower_dev, state); > + if (err && err != -EOPNOTSUPP) > + ret = err; Just return err here, on first failure. Otherwise you're overwriting previous err value; doesn't make sense. > + } > + > + return ret; > } > EXPORT_SYMBOL_GPL(netdev_switch_port_stp_update); > > -- > 1.7.10.4 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/16/15, 11:10 PM, Scott Feldman wrote: > On Mon, Mar 16, 2015 at 10:32 AM, <roopa@cumulusnetworks.com> wrote: >> From: Roopa Prabhu <roopa@cumulusnetworks.com> >> >> make it same as the netdev_switch_port_bridge_setlink/dellink >> api (ie traverse lowerdevs to get to the switch port). >> >> removes "WARN_ON(!ops->ndo_switch_parent_id_get)" because >> direct bridge ports can be stacked netdevices (like bonds >> and team of switch ports) which may not imeplement this ndo. >> >> v2 to v3: >> - remove changes to bond and team. Bring back the >> transparently following lowerdevs like i initially >> had for setlink/getlink >> (http://www.spinics.net/lists/netdev/msg313436.html) >> dave and scott feldman also seem to prefer it be that >> way and move to non-transparent way of doing things >> if we see a problem down the lane. >> >> v3 to v4: >> - fix ret initialization >> >> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> >> --- >> net/switchdev/switchdev.c | 20 ++++++++++++++++---- >> 1 file changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c >> index c9bfa00..e593b59 100644 >> --- a/net/switchdev/switchdev.c >> +++ b/net/switchdev/switchdev.c >> @@ -47,11 +47,23 @@ EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get); >> int netdev_switch_port_stp_update(struct net_device *dev, u8 state) >> { >> const struct swdev_ops *ops = dev->swdev_ops; >> + struct net_device *lower_dev; >> + struct list_head *iter; >> + int ret = -EOPNOTSUPP, err; >> >> - if (!ops || !ops->swdev_port_stp_update) >> - return -EOPNOTSUPP; >> - WARN_ON(!ops->swdev_parent_id_get); >> - return ops->swdev_port_stp_update(dev, state); >> + if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD)) >> + return ret; > I would drop the NETIF_F_HW_SWITCH_OFFLOAD check. It's not telling > you anything more than the next test for ops->swdev_port_stp_update. This was mainly to avoid lowerdev walk where you know that the dev does not have the feature. example: the bond carries this feature flag if any of its slaves have it. The lowerdev walk can be stopped at the bond. I don't mind removing the check, if you prefer that. thanks, Roopa -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/16/15, 2:26 PM, Florian Fainelli wrote: > On 16/03/15 10:32, roopa@cumulusnetworks.com wrote: >> From: Roopa Prabhu <roopa@cumulusnetworks.com> >> >> make it same as the netdev_switch_port_bridge_setlink/dellink >> api (ie traverse lowerdevs to get to the switch port). >> >> removes "WARN_ON(!ops->ndo_switch_parent_id_get)" because >> direct bridge ports can be stacked netdevices (like bonds >> and team of switch ports) which may not imeplement this ndo. >> >> v2 to v3: >> - remove changes to bond and team. Bring back the >> transparently following lowerdevs like i initially >> had for setlink/getlink >> (http://www.spinics.net/lists/netdev/msg313436.html) >> dave and scott feldman also seem to prefer it be that >> way and move to non-transparent way of doing things >> if we see a problem down the lane. >> >> v3 to v4: >> - fix ret initialization > The changelog should be after "---" otherwise it will get included in > the final git commit, we don't want that as this changelog only makes > sense during the submission process. > ack, thanks..will fix it when i respin the patch. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c index c9bfa00..e593b59 100644 --- a/net/switchdev/switchdev.c +++ b/net/switchdev/switchdev.c @@ -47,11 +47,23 @@ EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get); int netdev_switch_port_stp_update(struct net_device *dev, u8 state) { const struct swdev_ops *ops = dev->swdev_ops; + struct net_device *lower_dev; + struct list_head *iter; + int ret = -EOPNOTSUPP, err; - if (!ops || !ops->swdev_port_stp_update) - return -EOPNOTSUPP; - WARN_ON(!ops->swdev_parent_id_get); - return ops->swdev_port_stp_update(dev, state); + if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD)) + return ret; + + if (ops && ops->swdev_port_stp_update) + return ops->swdev_port_stp_update(dev, state); + + netdev_for_each_lower_dev(dev, lower_dev, iter) { + err = netdev_switch_port_stp_update(lower_dev, state); + if (err && err != -EOPNOTSUPP) + ret = err; + } + + return ret; } EXPORT_SYMBOL_GPL(netdev_switch_port_stp_update);