On Mon, Feb 04, 2019 at 03:36:33PM -0800, Florian Fainelli wrote:
> Now that we have a dedicated NDO for getting a port's parent ID, get rid
> of SWITCHDEV_ATTR_ID_PORT_PARENT_ID and convert all callers to use the
> NDO exclusively. This is a preliminary change to getting rid of
> switchdev_ops eventually.
>
> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
> ---
> include/net/switchdev.h | 2 --
> net/bridge/br_switchdev.c | 11 +++--------
> net/core/net-sysfs.c | 14 +++-----------
> net/core/rtnetlink.c | 16 ++++------------
> net/ipv4/ipmr.c | 14 ++++----------
> net/switchdev/switchdev.c | 20 +++++++++-----------
> 6 files changed, 23 insertions(+), 54 deletions(-)
>
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index 63843ae5dc81..e1a5e8bc24b8 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -43,7 +43,6 @@ static inline bool switchdev_trans_ph_commit(struct
> switchdev_trans *trans)
>
> enum switchdev_attr_id {
> SWITCHDEV_ATTR_ID_UNDEFINED,
> - SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
> SWITCHDEV_ATTR_ID_PORT_STP_STATE,
> SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
> SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
> @@ -61,7 +60,6 @@ struct switchdev_attr {
> void *complete_priv;
> void (*complete)(struct net_device *dev, int err, void *priv);
> union {
> - struct netdev_phys_item_id ppid; /* PORT_PARENT_ID */
> u8 stp_state; /* PORT_STP_STATE */
> unsigned long brport_flags; /* PORT_BRIDGE_FLAGS */
> unsigned long brport_flags_support; /*
> PORT_BRIDGE_FLAGS_SUPPORT */
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index 501a4221220a..620fd645f6f1 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -24,18 +24,13 @@ static int br_switchdev_mark_get(struct net_bridge *br,
> struct net_device *dev)
> int nbp_switchdev_mark_set(struct net_bridge_port *p)
> {
> const struct net_device_ops *ops = p->dev->netdev_ops;
> - struct switchdev_attr attr = {
> - .orig_dev = p->dev,
> - .id = SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
> - };
> - int err;
> + struct netdev_phys_item_id ppid = { };
> + int err = -EOPNOTSUPP;
>
> ASSERT_RTNL();
>
> if (ops->ndo_get_port_parent_id)
> - err = ops->ndo_get_port_parent_id(p->dev, &attr.u.ppid);
> - else
> - err = switchdev_port_attr_get(p->dev, &attr);
> + err = ops->ndo_get_port_parent_id(p->dev, &ppid);
> if (err) {
> if (err == -EOPNOTSUPP)
> return 0;
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index cc05e8b72657..1d2d76930afd 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -12,7 +12,6 @@
> #include <linux/capability.h>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> -#include <net/switchdev.h>
> #include <linux/if_arp.h>
> #include <linux/slab.h>
> #include <linux/sched/signal.h>
> @@ -502,19 +501,12 @@ static ssize_t phys_switch_id_show(struct device *dev,
> return restart_syscall();
>
> if (dev_isalive(netdev)) {
> - struct switchdev_attr attr = {
> - .orig_dev = netdev,
> - .id = SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
> - .flags = SWITCHDEV_F_NO_RECURSE,
Florian, note this flag. We should not return the parent id in case this
is called for a stacked device. Maybe extend the ndo with a new
parameter called 'recurse' ? Or you can check here if device has lower
devices and bail.
> - };
> + struct netdev_phys_item_id ppid = { };
>
> if (ops->ndo_get_port_parent_id)
> - ret = ops->ndo_get_port_parent_id(netdev, &attr.u.ppid);
> - else
> - ret = switchdev_port_attr_get(netdev, &attr);
> + ret = ops->ndo_get_port_parent_id(netdev, &ppid);
> if (!ret)
> - ret = sprintf(buf, "%*phN\n", attr.u.ppid.id_len,
> - attr.u.ppid.id);
> + ret = sprintf(buf, "%*phN\n", ppid.id_len, ppid.id);
> }
> rtnl_unlock();
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index ed8564eb97c8..27bccf68538e 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -46,7 +46,6 @@
>
> #include <linux/inet.h>
> #include <linux/netdevice.h>
> -#include <net/switchdev.h>
> #include <net/ip.h>
> #include <net/protocol.h>
> #include <net/arp.h>
> @@ -1147,25 +1146,18 @@ static int rtnl_phys_port_name_fill(struct sk_buff
> *skb, struct net_device *dev)
> static int rtnl_phys_switch_id_fill(struct sk_buff *skb, struct net_device
> *dev)
> {
> const struct net_device_ops *ops = dev->netdev_ops;
> - int err;
> - struct switchdev_attr attr = {
> - .orig_dev = dev,
> - .id = SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
> - .flags = SWITCHDEV_F_NO_RECURSE,
Same here.
> - };
> + struct netdev_phys_item_id ppid = { };
> + int err = -EOPNOTSUPP;
>
> if (ops->ndo_get_port_parent_id)
> - err = ops->ndo_get_port_parent_id(dev, &attr.u.ppid);
> - else
> - err = switchdev_port_attr_get(dev, &attr);
> + err = ops->ndo_get_port_parent_id(dev, &ppid);
> if (err) {
> if (err == -EOPNOTSUPP)
> return 0;
> return err;
> }
>
> - if (nla_put(skb, IFLA_PHYS_SWITCH_ID, attr.u.ppid.id_len,
> - attr.u.ppid.id))
> + if (nla_put(skb, IFLA_PHYS_SWITCH_ID, ppid.id_len, ppid.id))
> return -EMSGSIZE;
>
> return 0;
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 9f67394b6691..ac592bd07be1 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -67,7 +67,6 @@
> #include <net/fib_rules.h>
> #include <linux/netconf.h>
> #include <net/nexthop.h>
> -#include <net/switchdev.h>
>
> #include <linux/nospec.h>
>
> @@ -837,11 +836,9 @@ static void ipmr_update_thresholds(struct mr_table *mrt,
> struct mr_mfc *cache,
> static int vif_add(struct net *net, struct mr_table *mrt,
> struct vifctl *vifc, int mrtsock)
> {
> + struct netdev_phys_item_id ppid = { };
> const struct net_device_ops *ops;
> int vifi = vifc->vifc_vifi;
> - struct switchdev_attr attr = {
> - .id = SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
> - };
> struct vif_device *v = &mrt->vif_table[vifi];
> struct net_device *dev;
> struct in_device *in_dev;
> @@ -923,12 +920,9 @@ static int vif_add(struct net *net, struct mr_table *mrt,
> attr.orig_dev = dev;
> ops = dev->netdev_ops;
> if (ops->ndo_get_port_parent_id &&
> - !ops->ndo_get_port_parent_id(dev, &attr.ppid)) {
> - memcpy(v->dev_parent_id.id, attr.u.ppid.id, attr.u.ppid.id_len);
> - v->dev_parent_id.id_len = attr.u.ppid.id_len;
> - } else if (!switchdev_port_attr_get(dev, &attr)) {
> - memcpy(v->dev_parent_id.id, attr.u.ppid.id, attr.u.ppid.id_len);
> - v->dev_parent_id.id_len = attr.u.ppid.id_len;
> + !ops->ndo_get_port_parent_id(dev, &ppid)) {
> + memcpy(v->dev_parent_id.id, ppid.id, ppid.id_len);
> + v->dev_parent_id.id_len = ppid.id_len;
> } else {
> v->dev_parent_id.id_len = 0;
> }
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index cd78253de31d..57e0bfde67b4 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -595,20 +595,18 @@ EXPORT_SYMBOL_GPL(call_switchdev_blocking_notifiers);
> bool switchdev_port_same_parent_id(struct net_device *a,
> struct net_device *b)
I suggest to rename this function and move it to net/core/dev.c
> {
> - struct switchdev_attr a_attr = {
> - .orig_dev = a,
> - .id = SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
> - };
> - struct switchdev_attr b_attr = {
> - .orig_dev = b,
> - .id = SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
> - };
> + struct netdev_phys_item_id id_a = { };
> + struct netdev_phys_item_id id_b = { };
> +
> + if (!a->netdev_ops->ndo_get_port_parent_id ||
> + !b->netdev_ops->ndo_get_port_parent_id)
> + return -EOPNOTSUPP;
>
> - if (switchdev_port_attr_get(a, &a_attr) ||
> - switchdev_port_attr_get(b, &b_attr))
> + if (a->netdev_ops->ndo_get_port_parent_id(a, &id_a) ||
> + b->netdev_ops->ndo_get_port_parent_id(b, &id_b))
> return false;
>
> - return netdev_phys_item_id_same(&a_attr.u.ppid, &b_attr.u.ppid);
> + return netdev_phys_item_id_same(&id_a, &id_b);
> }
> EXPORT_SYMBOL_GPL(switchdev_port_same_parent_id);
>
> --
> 2.17.1
>
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
