From: Ido Schimmel <idosch@mellanox.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
Jakub Kicinski <jakub.kicinski@netronome.com>,
John Hurley <john.hurley@netronome.com>,
"open list:NETRONOME ETHERNET DRIVERS"
<oss-drivers@netronome.com>, Eric Dumazet <edumazet@google.com>,
Ioana Ciornei <ioana.ciornei@nxp.com>,
Tyler Hicks <tyhicks@canonical.com>,
Ivan Vecera <ivecera@redhat.com>, David Ahern <dsahern@gmail.com>,
Ioana Radulescu <ruxandra.radulescu@nxp.com>,
"open list:MELLANOX MLX5 core VPI driver"
<linux-rdma@vger.kernel.org>,
"moderated list:ETHERNET BRIDGE"
<bridge@lists.linux-foundation.org>,
Amritha Nambiar <amritha.nambiar@intel.com>,
Saeed Mahameed <saeedm@mellanox.com>,
Dirk van der Merwe <dirk.vandermerwe@netronome.com>,
Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
Vivien Didelot <vivien.didelot@gmail.com>,
Alexander Duyck <alexander.h.duyck@intel.com>open li
Subject: Re: [PATCH 12/12] net: Get rid of SWITCHDEV_ATTR_ID_PORT_PARENT_ID
Date: Tue, 5 Feb 2019 07:47:56 +0000 [thread overview]
Message-ID: <20190205074753.GA9588@splinter> (raw)
In-Reply-To: <20190204233633.20421-13-f.fainelli@gmail.com>
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.fainelli@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
>
next prev parent reply other threads:[~2019-02-05 7:47 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-04 23:36 [PATCH 00/12] net: Introduce ndo_get_port_parent_id() Florian Fainelli
2019-02-04 23:36 ` [PATCH 01/12] " Florian Fainelli
2019-02-04 23:36 ` [PATCH 02/12] bnxt: Implement ndo_get_port_parent_id() Florian Fainelli
2019-02-05 9:48 ` kbuild test robot
2019-02-05 13:24 ` kbuild test robot
2019-02-04 23:36 ` [PATCH 03/12] liquidio: " Florian Fainelli
2019-02-04 23:36 ` [PATCH 04/12] net/mlx5e: " Florian Fainelli
2019-02-04 23:36 ` [PATCH 05/12] mlxsw: " Florian Fainelli
2019-02-04 23:36 ` [PATCH 06/12] mscc: ocelot: " Florian Fainelli
2019-02-04 23:36 ` [PATCH 07/12] nfp: " Florian Fainelli
2019-02-05 3:49 ` Jakub Kicinski
2019-02-04 23:36 ` [PATCH 08/12] rocker: " Florian Fainelli
2019-02-04 23:36 ` [PATCH 09/12] netdevsim: " Florian Fainelli
2019-02-05 3:51 ` Jakub Kicinski
2019-02-05 9:23 ` kbuild test robot
2019-02-05 9:30 ` kbuild test robot
2019-02-04 23:36 ` [PATCH 10/12] staging: fsl-dpaa2: ethsw: " Florian Fainelli
2019-02-04 23:36 ` [PATCH 11/12] net: dsa: " Florian Fainelli
2019-02-04 23:36 ` [PATCH 12/12] net: Get rid of SWITCHDEV_ATTR_ID_PORT_PARENT_ID Florian Fainelli
2019-02-05 7:47 ` Ido Schimmel [this message]
2019-02-05 9:23 ` kbuild test robot
2019-02-05 11:17 ` kbuild test robot
2019-02-05 7:14 ` [PATCH 00/12] net: Introduce ndo_get_port_parent_id() Ido Schimmel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190205074753.GA9588@splinter \
--to=idosch@mellanox.com \
--cc=alexander.h.duyck@intel.com \
--cc=alexandre.belloni@bootlin.com \
--cc=amritha.nambiar@intel.com \
--cc=andrew@lunn.ch \
--cc=bridge@lists.linux-foundation.org \
--cc=dirk.vandermerwe@netronome.com \
--cc=dsahern@gmail.com \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=ioana.ciornei@nxp.com \
--cc=ivecera@redhat.com \
--cc=jakub.kicinski@netronome.com \
--cc=john.hurley@netronome.com \
--cc=kuznet@ms2.inr.ac.ru \
--cc=linux-rdma@vger.kernel.org \
--cc=oss-drivers@netronome.com \
--cc=ruxandra.radulescu@nxp.com \
--cc=saeedm@mellanox.com \
--cc=tyhicks@canonical.com \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).