From: Jiri Pirko <jiri@resnulli.us>
To: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: davem@davemloft.net, oss-drivers@netronome.com, netdev@vger.kernel.org
Subject: Re: [PATCH net-next 2/8] devlink: add PF and VF port flavours
Date: Wed, 27 Feb 2019 13:16:44 +0100 [thread overview]
Message-ID: <20190227121644.GA2240@nanopsycho> (raw)
In-Reply-To: <20190226182436.23811-3-jakub.kicinski@netronome.com>
Tue, Feb 26, 2019 at 07:24:30PM CET, jakub.kicinski@netronome.com wrote:
>Current port flavours cover simple switches and DSA. Add PF
>and VF flavours to cover "switchdev" SR-IOV NICs.
>
>Example devlink user space output:
>
>$ devlink port
>pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
>pci/0000:82:00.0/10000: type eth netdev eth0 flavour pcie_pf pf 0
>pci/0000:82:00.0/10001: type eth netdev eth1 flavour pcie_vf pf 0 vf 0
>pci/0000:82:00.0/10002: type eth netdev eth2 flavour pcie_vf pf 0 vf 1
I believe that that this output should be in sync with attr names. So
the names should be:
pci_vf
pci_pf
pf_number
vf_number
Like:
pci/0000:82:00.0/10002: type eth netdev eth2 flavour pci_vf pf_number 0 vf_number 1
But that is comment to the userspace part.
>
>$ devlink -jp port
>{
> "port": {
> "pci/0000:82:00.0/0": {
> "type": "eth",
> "netdev": "p4p1",
> "flavour": "physical"
> },
> "pci/0000:82:00.0/10000": {
> "type": "eth",
> "netdev": "eth0",
> "flavour": "pci_pf",
> "pf": 0,
> },
> "pci/0000:82:00.0/10001": {
> "type": "eth",
> "netdev": "eth1",
> "flavour": "pci_vf",
> "pf": 0,
> "vf": 0
> },
> "pci/0000:82:00.0/10002": {
> "type": "eth",
> "netdev": "eth2",
> "flavour": "pci_vf",
> "pf": 0,
> "vf": 1
> }
> }
>}
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> include/net/devlink.h | 25 ++++++++++--
> include/uapi/linux/devlink.h | 5 +++
> net/core/devlink.c | 73 +++++++++++++++++++++++++++++++-----
> 3 files changed, 91 insertions(+), 12 deletions(-)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 7f5a0bdca228..b5376ef492f1 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -42,9 +42,19 @@ struct devlink {
> struct devlink_port_attrs {
> bool set;
> enum devlink_port_flavour flavour;
>- u32 port_number; /* same value as "split group" */
>- bool split;
>- u32 split_subport_number;
>+ union { /* port identifiers differ per-flavour */
>+ /* PHYSICAL, CPU, DSA */
>+ struct {
>+ bool split;
>+ u32 split_subport_number;
>+ u32 port_number; /* same value as "split group" */
>+ };
>+ /* PCI_PF, PCI_VF */
>+ struct {
>+ u32 pf_number;
>+ u32 vf_number;
>+ } pci;
>+ };
> };
>
> struct devlink_port {
>@@ -568,6 +578,9 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
> enum devlink_port_flavour flavour,
> u32 port_number, bool split,
> u32 split_subport_number);
>+void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
>+ enum devlink_port_flavour flavour,
>+ u32 pf_number, u32 vf_number);
> int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> char *name, size_t len);
> int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>@@ -782,6 +795,12 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port,
> {
> }
>
>+static inline void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
>+ enum devlink_port_flavour flavour,
>+ u32 pf_number, u32 vf_number)
>+{
>+}
>+
> static inline int
> devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> char *name, size_t len)
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index 5bb4ea67d84f..9ce76d4f640d 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -167,6 +167,8 @@ enum devlink_port_flavour {
> DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
> * interconnect port.
> */
>+ DEVLINK_PORT_FLAVOUR_PCI_PF, /* PCI Physical function port */
>+ DEVLINK_PORT_FLAVOUR_PCI_VF, /* PCI Physical function port */
> };
>
> enum devlink_param_cmode {
>@@ -332,6 +334,9 @@ enum devlink_attr {
> DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME, /* string */
> DEVLINK_ATTR_FLASH_UPDATE_COMPONENT, /* string */
>
>+ DEVLINK_ATTR_PORT_PCI_PF_NUMBER, /* u32 */
>+ DEVLINK_ATTR_PORT_PCI_VF_NUMBER, /* u32 */
>+
> /* add new attributes above here, update the policy in devlink.c */
>
> __DEVLINK_ATTR_MAX,
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index a49dee67e66f..af177284830b 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -516,16 +516,35 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
> return 0;
> if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
> return -EMSGSIZE;
>- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
>- return -EMSGSIZE;
>- if (!attrs->split)
>+
>+ switch (attrs->flavour) {
>+ case DEVLINK_PORT_FLAVOUR_PHYSICAL:
>+ case DEVLINK_PORT_FLAVOUR_CPU:
>+ case DEVLINK_PORT_FLAVOUR_DSA:
>+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER,
>+ attrs->port_number))
>+ return -EMSGSIZE;
>+
>+ if (attrs->split &&
>+ (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
>+ attrs->port_number) ||
>+ nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
>+ attrs->split_subport_number)))
>+ return -EMSGSIZE;
> return 0;
>- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs->port_number))
>- return -EMSGSIZE;
>- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
>- attrs->split_subport_number))
>- return -EMSGSIZE;
>- return 0;
>+ case DEVLINK_PORT_FLAVOUR_PCI_VF:
>+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_PCI_VF_NUMBER,
>+ attrs->pci.vf_number))
>+ return -EMSGSIZE;
>+ /* fall through */
>+ case DEVLINK_PORT_FLAVOUR_PCI_PF:
>+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
>+ attrs->pci.pf_number))
>+ return -EMSGSIZE;
>+ return 0;
>+ default:
>+ return -EINVAL;
>+ }
> }
>
> static int devlink_nl_port_fill(struct sk_buff *msg, struct devlink *devlink,
>@@ -5410,6 +5429,9 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
> {
> struct devlink_port_attrs *attrs = &devlink_port->attrs;
>
>+ WARN_ON(flavour == DEVLINK_PORT_FLAVOUR_PCI_PF ||
>+ flavour == DEVLINK_PORT_FLAVOUR_PCI_VF);
>+
> attrs->set = true;
> attrs->flavour = flavour;
> attrs->port_number = port_number;
>@@ -5419,6 +5441,32 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
> }
> EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
>
>+/**
>+ * devlink_port_attrs_pci_set - Set port attributes for a PCI port
>+ *
>+ * @devlink_port: devlink port
>+ * @flavour: flavour of the port (PF or VF only)
>+ * @pf_number: PCI PF number, in multi-host mapping to hosts depends
>+ * on the platform
>+ * @vf_number: PCI VF number within given PF (ignored for PF itself)
>+ */
>+void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
>+ enum devlink_port_flavour flavour,
>+ u32 pf_number, u32 vf_number)
Maybe nicer would be to have this static and have 2 helpers:
void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
u32 pf_number)
void devlink_port_attrs_pci_vf_set(struct devlink_port *devlink_port,
u32 pf_number, u32 vf_number)
Then you need no warnon. Also you won't have dead arg in driver api
case of pf
Other than this the patch looks good to me.
>+{
>+ struct devlink_port_attrs *attrs = &devlink_port->attrs;
>+
>+ WARN_ON(flavour != DEVLINK_PORT_FLAVOUR_PCI_PF &&
>+ flavour != DEVLINK_PORT_FLAVOUR_PCI_VF);
>+
>+ attrs->set = true;
>+ attrs->flavour = flavour;
>+ attrs->pci.pf_number = pf_number;
>+ attrs->pci.vf_number = vf_number;
>+ devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
>+}
>+EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_set);
>+
> int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> char *name, size_t len)
> {
>@@ -5443,6 +5491,13 @@ int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> */
> WARN_ON(1);
> return -EINVAL;
>+ case DEVLINK_PORT_FLAVOUR_PCI_PF:
>+ n = snprintf(name, len, "pf%u", attrs->pci.pf_number);
>+ break;
>+ case DEVLINK_PORT_FLAVOUR_PCI_VF:
>+ n = snprintf(name, len, "pf%uvf%u",
>+ attrs->pf_number, attrs->pci.vf_number);
>+ break;
> }
>
> if (n >= len)
>--
>2.19.2
>
next prev parent reply other threads:[~2019-02-27 12:16 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-26 18:24 [PATCH net-next 0/8] devlink: add PF and VF port flavours Jakub Kicinski
2019-02-26 18:24 ` [PATCH net-next 1/8] nfp: split devlink port init from registration Jakub Kicinski
2019-02-26 18:24 ` [PATCH net-next 2/8] devlink: add PF and VF port flavours Jakub Kicinski
2019-02-27 12:16 ` Jiri Pirko [this message]
2019-03-04 4:59 ` Parav Pandit
2019-03-04 7:30 ` Jiri Pirko
2019-03-20 17:29 ` Abodunrin, Akeem G
2019-03-21 12:26 ` Jiri Pirko
2019-02-27 12:23 ` Jiri Pirko
2019-02-27 12:41 ` Jiri Pirko
2019-02-27 17:23 ` Jakub Kicinski
2019-02-27 20:17 ` Jiri Pirko
2019-02-27 22:42 ` Jakub Kicinski
2019-02-28 8:44 ` Jiri Pirko
2019-02-28 16:08 ` Jakub Kicinski
2019-02-28 16:24 ` David Ahern
2019-02-26 18:24 ` [PATCH net-next 3/8] nfp: register devlink ports of all reprs Jakub Kicinski
2019-02-26 18:24 ` [PATCH net-next 4/8] devlink: allow subports on devlink PCI ports Jakub Kicinski
2019-02-27 12:37 ` Jiri Pirko
2019-02-27 18:30 ` Jakub Kicinski
2019-02-28 8:56 ` Jiri Pirko
2019-02-28 13:32 ` Jiri Pirko
2019-02-28 16:24 ` Jakub Kicinski
2019-03-01 7:25 ` Jiri Pirko
2019-03-01 16:04 ` Jakub Kicinski
2019-03-01 16:20 ` Jiri Pirko
2019-03-04 16:15 ` Jason Gunthorpe
2019-03-05 1:03 ` Jakub Kicinski
2019-03-05 1:30 ` Jason Gunthorpe
2019-03-05 2:11 ` Jakub Kicinski
2019-03-05 22:11 ` Jason Gunthorpe
2019-03-04 5:00 ` Parav Pandit
2019-02-26 18:24 ` [PATCH net-next 5/8] nfp: switch to devlink_port_get_phys_port_name() Jakub Kicinski
2019-02-26 18:24 ` [PATCH net-next 6/8] devlink: introduce port's peer netdevs Jakub Kicinski
2019-02-27 13:08 ` Jiri Pirko
2019-02-27 18:47 ` Jakub Kicinski
2019-02-28 9:00 ` Jiri Pirko
2019-02-28 16:36 ` Jakub Kicinski
2019-03-01 7:37 ` Jiri Pirko
2019-03-01 16:05 ` Jakub Kicinski
2019-03-04 5:07 ` Parav Pandit
2019-02-26 18:24 ` [PATCH net-next 7/8] nfp: expose PF " Jakub Kicinski
2019-02-26 18:24 ` [PATCH net-next 8/8] devlink: fix kdoc Jakub Kicinski
2019-02-27 13:13 ` Jiri Pirko
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=20190227121644.GA2240@nanopsycho \
--to=jiri@resnulli.us \
--cc=davem@davemloft.net \
--cc=jakub.kicinski@netronome.com \
--cc=netdev@vger.kernel.org \
--cc=oss-drivers@netronome.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 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.