* [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices
@ 2015-03-16 17:32 roopa
2015-03-16 17:36 ` Jiri Pirko
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: roopa @ 2015-03-16 17:32 UTC (permalink / raw)
To: davem, sfeldma, jiri; +Cc: netdev
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;
+
+ 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);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices
2015-03-16 17:32 [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices roopa
@ 2015-03-16 17:36 ` Jiri Pirko
2015-03-16 21:26 ` Florian Fainelli
2015-03-17 6:10 ` Scott Feldman
2 siblings, 0 replies; 6+ messages in thread
From: Jiri Pirko @ 2015-03-16 17:36 UTC (permalink / raw)
To: roopa; +Cc: davem, sfeldma, netdev
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!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices
2015-03-16 17:32 [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices roopa
2015-03-16 17:36 ` Jiri Pirko
@ 2015-03-16 21:26 ` Florian Fainelli
2015-03-17 22:39 ` roopa
2015-03-17 6:10 ` Scott Feldman
2 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2015-03-16 21:26 UTC (permalink / raw)
To: roopa, davem, sfeldma, jiri; +Cc: netdev
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);
>
>
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices
2015-03-16 17:32 [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices roopa
2015-03-16 17:36 ` Jiri Pirko
2015-03-16 21:26 ` Florian Fainelli
@ 2015-03-17 6:10 ` Scott Feldman
2015-03-17 12:42 ` roopa
2 siblings, 1 reply; 6+ messages in thread
From: Scott Feldman @ 2015-03-17 6:10 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: David S. Miller, Jiří Pírko, Netdev
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices
2015-03-17 6:10 ` Scott Feldman
@ 2015-03-17 12:42 ` roopa
0 siblings, 0 replies; 6+ messages in thread
From: roopa @ 2015-03-17 12:42 UTC (permalink / raw)
To: Scott Feldman; +Cc: David S. Miller, Jiří Pírko, Netdev
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices
2015-03-16 21:26 ` Florian Fainelli
@ 2015-03-17 22:39 ` roopa
0 siblings, 0 replies; 6+ messages in thread
From: roopa @ 2015-03-17 22:39 UTC (permalink / raw)
To: Florian Fainelli; +Cc: davem, sfeldma, jiri, netdev
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-17 22:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-16 17:32 [PATCH net-next v4] switchdev: fix stp update API to work with layered netdevices roopa
2015-03-16 17:36 ` Jiri Pirko
2015-03-16 21:26 ` Florian Fainelli
2015-03-17 22:39 ` roopa
2015-03-17 6:10 ` Scott Feldman
2015-03-17 12:42 ` roopa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.