Florian Fainelli
2019-Feb-22 23:59 UTC
[Bridge] [PATCH net-next 7/8] net: switchdev: Replace port attr set SDO with a notification
Drop switchdev_ops.switchdev_port_attr_set. Drop the uses of this field
from all clients, which were migrated to use switchdev notification in
the previous patches.
Add a new function switchdev_port_attr_notify() that sends the switchdev
notifications SWITCHDEV_PORT_ATTR_SET and takes care, depending on
SWITCHDEV_F_DEFER to call the blocking (process) or non-blocking
(atomic) notifier chain accordingly.
Drop __switchdev_port_attr_set() and update switchdev_port_attr_set()
likewise.
Signed-off-by: Florian Fainelli <f.fainelli at gmail.com>
---
net/switchdev/switchdev.c | 96 +++++++++++----------------------------
1 file changed, 26 insertions(+), 70 deletions(-)
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 94400f5b8e07..a1f16836ef89 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -174,81 +174,35 @@ static int switchdev_deferred_enqueue(struct net_device
*dev,
return 0;
}
-/**
- * switchdev_port_attr_get - Get port attribute
- *
- * @dev: port device
- * @attr: attribute to get
- */
-int switchdev_port_attr_get(struct net_device *dev, struct switchdev_attr
*attr)
+static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
+ struct net_device *dev,
+ const struct switchdev_attr *attr,
+ struct switchdev_trans *trans)
{
- const struct switchdev_ops *ops = dev->switchdev_ops;
- struct net_device *lower_dev;
- struct list_head *iter;
- struct switchdev_attr first = {
- .id = SWITCHDEV_ATTR_ID_UNDEFINED
- };
- int err = -EOPNOTSUPP;
+ int err;
+ int rc;
- if (ops && ops->switchdev_port_attr_get)
- return ops->switchdev_port_attr_get(dev, attr);
+ struct switchdev_notifier_port_attr_info attr_info = {
+ .attr = attr,
+ .trans = trans,
+ .handled = false,
+ };
- if (attr->flags & SWITCHDEV_F_NO_RECURSE)
+ if (attr & SWITCHDEV_F_DEFER)
+ rc = call_switchdev_blocking_notifiers(nt, dev,
+ &attr_info.info, NULL);
+ else
+ rc = call_switchdev_notifiers(nt, dev, &attr_info.info, NULL);
+ err = notifier_to_errno(rc);
+ if (err) {
+ WARN_ON(!attr_info.handled);
return err;
-
- /* Switch device port(s) may be stacked under
- * bond/team/vlan dev, so recurse down to get attr on
- * each port. Return -ENODATA if attr values don't
- * compare across ports.
- */
-
- netdev_for_each_lower_dev(dev, lower_dev, iter) {
- err = switchdev_port_attr_get(lower_dev, attr);
- if (err)
- break;
- if (first.id == SWITCHDEV_ATTR_ID_UNDEFINED)
- first = *attr;
- else if (memcmp(&first, attr, sizeof(*attr)))
- return -ENODATA;
}
- return err;
-}
-EXPORT_SYMBOL_GPL(switchdev_port_attr_get);
-
-static int __switchdev_port_attr_set(struct net_device *dev,
- const struct switchdev_attr *attr,
- struct switchdev_trans *trans)
-{
- const struct switchdev_ops *ops = dev->switchdev_ops;
- struct net_device *lower_dev;
- struct list_head *iter;
- int err = -EOPNOTSUPP;
-
- if (ops && ops->switchdev_port_attr_set) {
- err = ops->switchdev_port_attr_set(dev, attr, trans);
- goto done;
- }
-
- if (attr->flags & SWITCHDEV_F_NO_RECURSE)
- goto done;
-
- /* Switch device port(s) may be stacked under
- * bond/team/vlan dev, so recurse down to set attr on
- * each port.
- */
-
- netdev_for_each_lower_dev(dev, lower_dev, iter) {
- err = __switchdev_port_attr_set(lower_dev, attr, trans);
- if (err)
- break;
- }
-
-done:
- if (err == -EOPNOTSUPP && attr->flags &
SWITCHDEV_F_SKIP_EOPNOTSUPP)
- err = 0;
+ if (!attr_info.handled)
+ return -EOPNOTSUPP;
- return err;
+ return 0;
}
static int switchdev_port_attr_set_now(struct net_device *dev,
@@ -267,7 +221,8 @@ static int switchdev_port_attr_set_now(struct net_device
*dev,
*/
trans.ph_prepare = true;
- err = __switchdev_port_attr_set(dev, attr, &trans);
+ err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
+ &trans);
if (err) {
/* Prepare phase failed: abort the transaction. Any
* resources reserved in the prepare phase are
@@ -286,7 +241,8 @@ static int switchdev_port_attr_set_now(struct net_device
*dev,
*/
trans.ph_prepare = false;
- err = __switchdev_port_attr_set(dev, attr, &trans);
+ err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
+ &trans);
WARN(err, "%s: Commit of attribute (id=%d) failed.\n",
dev->name, attr->id);
switchdev_trans_items_warn_destroy(dev, &trans);
--
2.17.1
Ido Schimmel
2019-Feb-23 10:32 UTC
[Bridge] [PATCH net-next 7/8] net: switchdev: Replace port attr set SDO with a notification
On Fri, Feb 22, 2019 at 03:59:25PM -0800, Florian Fainelli wrote:> Drop switchdev_ops.switchdev_port_attr_set. Drop the uses of this field > from all clients, which were migrated to use switchdev notification in > the previous patches. > > Add a new function switchdev_port_attr_notify() that sends the switchdev > notifications SWITCHDEV_PORT_ATTR_SET and takes care, depending on > SWITCHDEV_F_DEFER to call the blocking (process) or non-blocking > (atomic) notifier chain accordingly. > > Drop __switchdev_port_attr_set() and update switchdev_port_attr_set() > likewise. > > Signed-off-by: Florian Fainelli <f.fainelli at gmail.com> > --- > net/switchdev/switchdev.c | 96 +++++++++++---------------------------- > 1 file changed, 26 insertions(+), 70 deletions(-) > > diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c > index 94400f5b8e07..a1f16836ef89 100644 > --- a/net/switchdev/switchdev.c > +++ b/net/switchdev/switchdev.c > @@ -174,81 +174,35 @@ static int switchdev_deferred_enqueue(struct net_device *dev, > return 0; > } > > -/** > - * switchdev_port_attr_get - Get port attributeHmm, why do you remove it here? Can't you remove it in a separate patch? I thought we already got rid of it :p> - * > - * @dev: port device > - * @attr: attribute to get > - */ > -int switchdev_port_attr_get(struct net_device *dev, struct switchdev_attr *attr) > +static int switchdev_port_attr_notify(enum switchdev_notifier_type nt, > + struct net_device *dev, > + const struct switchdev_attr *attr, > + struct switchdev_trans *trans) > { > - const struct switchdev_ops *ops = dev->switchdev_ops; > - struct net_device *lower_dev; > - struct list_head *iter; > - struct switchdev_attr first = { > - .id = SWITCHDEV_ATTR_ID_UNDEFINED > - }; > - int err = -EOPNOTSUPP; > + int err; > + int rc; > > - if (ops && ops->switchdev_port_attr_get) > - return ops->switchdev_port_attr_get(dev, attr); > + struct switchdev_notifier_port_attr_info attr_info = { > + .attr = attr, > + .trans = trans, > + .handled = false, > + }; > > - if (attr->flags & SWITCHDEV_F_NO_RECURSE) > + if (attr & SWITCHDEV_F_DEFER) > + rc = call_switchdev_blocking_notifiers(nt, dev, > + &attr_info.info, NULL); > + else > + rc = call_switchdev_notifiers(nt, dev, &attr_info.info, NULL);I don't believe this is needed. You're calling this function from switchdev_port_attr_set_now() which is always called from process context. switchdev_port_attr_set() takes care of that. Similar to switchdev_port_obj_add(). The event `SWITCHDEV_PORT_ATTR_SET` is therefore always blocking and drivers only need to take care of it from their blocking notifier.> + err = notifier_to_errno(rc); > + if (err) { > + WARN_ON(!attr_info.handled); > return err; > - > - /* Switch device port(s) may be stacked under > - * bond/team/vlan dev, so recurse down to get attr on > - * each port. Return -ENODATA if attr values don't > - * compare across ports. > - */ > - > - netdev_for_each_lower_dev(dev, lower_dev, iter) { > - err = switchdev_port_attr_get(lower_dev, attr); > - if (err) > - break; > - if (first.id == SWITCHDEV_ATTR_ID_UNDEFINED) > - first = *attr; > - else if (memcmp(&first, attr, sizeof(*attr))) > - return -ENODATA; > } > > - return err; > -} > -EXPORT_SYMBOL_GPL(switchdev_port_attr_get); > - > -static int __switchdev_port_attr_set(struct net_device *dev, > - const struct switchdev_attr *attr, > - struct switchdev_trans *trans) > -{ > - const struct switchdev_ops *ops = dev->switchdev_ops; > - struct net_device *lower_dev; > - struct list_head *iter; > - int err = -EOPNOTSUPP; > - > - if (ops && ops->switchdev_port_attr_set) { > - err = ops->switchdev_port_attr_set(dev, attr, trans); > - goto done; > - } > - > - if (attr->flags & SWITCHDEV_F_NO_RECURSE) > - goto done; > - > - /* Switch device port(s) may be stacked under > - * bond/team/vlan dev, so recurse down to set attr on > - * each port. > - */ > - > - netdev_for_each_lower_dev(dev, lower_dev, iter) { > - err = __switchdev_port_attr_set(lower_dev, attr, trans); > - if (err) > - break; > - } > - > -done: > - if (err == -EOPNOTSUPP && attr->flags & SWITCHDEV_F_SKIP_EOPNOTSUPP) > - err = 0; > + if (!attr_info.handled) > + return -EOPNOTSUPP; > > - return err; > + return 0; > } > > static int switchdev_port_attr_set_now(struct net_device *dev, > @@ -267,7 +221,8 @@ static int switchdev_port_attr_set_now(struct net_device *dev, > */ > > trans.ph_prepare = true; > - err = __switchdev_port_attr_set(dev, attr, &trans); > + err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr, > + &trans); > if (err) { > /* Prepare phase failed: abort the transaction. Any > * resources reserved in the prepare phase are > @@ -286,7 +241,8 @@ static int switchdev_port_attr_set_now(struct net_device *dev, > */ > > trans.ph_prepare = false; > - err = __switchdev_port_attr_set(dev, attr, &trans); > + err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr, > + &trans); > WARN(err, "%s: Commit of attribute (id=%d) failed.\n", > dev->name, attr->id); > switchdev_trans_items_warn_destroy(dev, &trans); > -- > 2.17.1 >