diff mbox series

[net-next,v2,1/3] devlink: Introduce PCI PF port flavour and port attribute

Message ID 20190705073711.37854-2-parav@mellanox.com
State Superseded
Delegated to: David Miller
Headers show
Series devlink: Introduce PCI PF, VF ports and attributes | expand

Commit Message

Parav Pandit July 5, 2019, 7:37 a.m. UTC 
In an eswitch, PCI PF may have port which is normally represented
using a representor netdevice.
To have better visibility of eswitch port, its association with
PF and a representor netdevice, introduce a PCI PF port
flavour and port attriute.

When devlink port flavour is PCI PF, fill up PCI PF attributes of the
port.

Extend port name creation using PCI PF number on best effort basis.
So that vendor drivers can skip defining their own scheme.

$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0

Signed-off-by: Parav Pandit <parav@mellanox.com>

---
Changelog:
v1->v2:
 - Limited port_num attribute to physical ports
 - Updated PCI PF attribute set API to not have port_number
---
 include/net/devlink.h        | 15 ++++++-
 include/uapi/linux/devlink.h |  5 +++
 net/core/devlink.c           | 80 +++++++++++++++++++++++++++++-------
 3 files changed, 84 insertions(+), 16 deletions(-)

Comments

Jakub Kicinski July 5, 2019, 7:17 p.m. UTC | #1 
On Fri,  5 Jul 2019 02:37:09 -0500, Parav Pandit wrote:
> @@ -38,14 +38,24 @@ struct devlink {
>  	char priv[0] __aligned(NETDEV_ALIGN);
>  };
>  
> +struct devlink_port_pci_pf_attrs {
> +	u16 pf;	/* Associated PCI PF for this port. */
> +};
> +
>  struct devlink_port_attrs {
>  	u8 set:1,
>  	   split:1,
>  	   switch_port:1;
>  	enum devlink_port_flavour flavour;
> -	u32 port_number; /* same value as "split group" */
> +	u32 port_number; /* same value as "split group".
> +			  * Valid only when a port is physical and visible
> +			  * to the user for a given port flavour.
> +			  */

port_number can be in the per-flavour union below.

>  	u32 split_subport_number;

As can split_subport_number.

>  	struct netdev_phys_item_id switch_id;
> +	union {
> +		struct devlink_port_pci_pf_attrs pci_pf;
> +	};
>  };
>  
>  struct devlink_port {

> @@ -515,8 +523,14 @@ 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))
> +	if (is_devlink_phy_port_num_supported(devlink_port) &&
> +	    nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
>  		return -EMSGSIZE;
> +	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
> +		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
> +				attrs->pci_pf.pf))
> +			return -EMSGSIZE;
> +	}
>  	if (!attrs->split)
>  		return 0;
>  	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs->port_number))

Split attributes as well, please:

On Tue, 2 Jul 2019 16:42:52 -0700, Jakub Kicinski wrote:
> port_number, and split attributes should not be exposed for PCI ports.

We have no clear semantics for those, yet, and the phys_port_name
implementation in this patch doesn't handle split PCI, so let's leave
them out for now.
Parav Pandit July 6, 2019, 6:12 a.m. UTC | #2 
> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Saturday, July 6, 2019 12:47 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: netdev@vger.kernel.org; Jiri Pirko <jiri@mellanox.com>; Saeed Mahameed
> <saeedm@mellanox.com>
> Subject: Re: [PATCH net-next v2 1/3] devlink: Introduce PCI PF port flavour and
> port attribute
> 
> On Fri,  5 Jul 2019 02:37:09 -0500, Parav Pandit wrote:
> > @@ -38,14 +38,24 @@ struct devlink {
> >  	char priv[0] __aligned(NETDEV_ALIGN);  };
> >
> > +struct devlink_port_pci_pf_attrs {
> > +	u16 pf;	/* Associated PCI PF for this port. */
> > +};
> > +
> >  struct devlink_port_attrs {
> >  	u8 set:1,
> >  	   split:1,
> >  	   switch_port:1;
> >  	enum devlink_port_flavour flavour;
> > -	u32 port_number; /* same value as "split group" */
> > +	u32 port_number; /* same value as "split group".
> > +			  * Valid only when a port is physical and visible
> > +			  * to the user for a given port flavour.
> > +			  */
> 
> port_number can be in the per-flavour union below.
> 
Ack.
> >  	u32 split_subport_number;
> 
> As can split_subport_number.
> 
Ack.

> >  	struct netdev_phys_item_id switch_id;
> > +	union {
> > +		struct devlink_port_pci_pf_attrs pci_pf;
> > +	};
> >  };
> >
> >  struct devlink_port {
> 
> > @@ -515,8 +523,14 @@ 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))
> > +	if (is_devlink_phy_port_num_supported(devlink_port) &&
> > +	    nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs-
> >port_number))
> >  		return -EMSGSIZE;
> > +	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
> > +		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
> > +				attrs->pci_pf.pf))
> > +			return -EMSGSIZE;
> > +	}
> >  	if (!attrs->split)
> >  		return 0;
> >  	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
> > attrs->port_number))
> 
> Split attributes as well, please:
> 
Ack.
> On Tue, 2 Jul 2019 16:42:52 -0700, Jakub Kicinski wrote:
> > port_number, and split attributes should not be exposed for PCI ports.
> 
> We have no clear semantics for those, yet, and the phys_port_name
> implementation in this patch doesn't handle split PCI, so let's leave them out
> for now.
Ok. Sending v3.
Thanks for the review.
diff mbox series

Patch

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 6625ea068d5e..32badf7e0810 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -38,14 +38,24 @@  struct devlink {
 	char priv[0] __aligned(NETDEV_ALIGN);
 };
 
+struct devlink_port_pci_pf_attrs {
+	u16 pf;	/* Associated PCI PF for this port. */
+};
+
 struct devlink_port_attrs {
 	u8 set:1,
 	   split:1,
 	   switch_port:1;
 	enum devlink_port_flavour flavour;
-	u32 port_number; /* same value as "split group" */
+	u32 port_number; /* same value as "split group".
+			  * Valid only when a port is physical and visible
+			  * to the user for a given port flavour.
+			  */
 	u32 split_subport_number;
 	struct netdev_phys_item_id switch_id;
+	union {
+		struct devlink_port_pci_pf_attrs pci_pf;
+	};
 };
 
 struct devlink_port {
@@ -590,6 +600,9 @@  void devlink_port_attrs_set(struct devlink_port *devlink_port,
 			    u32 split_subport_number,
 			    const unsigned char *switch_id,
 			    unsigned char switch_id_len);
+void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
+				   const unsigned char *switch_id,
+				   unsigned char switch_id_len, u16 pf);
 int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
 			u32 size, u16 ingress_pools_count,
 			u16 egress_pools_count, u16 ingress_tc_count,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 5287b42c181f..f7323884c3fe 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -169,6 +169,10 @@  enum devlink_port_flavour {
 	DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
 				   * interconnect port.
 				   */
+	DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
+				      * the PCI PF. It is an internal
+				      * port that faces the PCI PF.
+				      */
 };
 
 enum devlink_param_cmode {
@@ -337,6 +341,7 @@  enum devlink_attr {
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE,	/* u64 */
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL,	/* u64 */
 
+	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u16 */
 	/* 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 89c533778135..c9418f1ce025 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -506,6 +506,14 @@  static void devlink_notify(struct devlink *devlink, enum devlink_command cmd)
 				msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
 }
 
+static bool
+is_devlink_phy_port_num_supported(const struct devlink_port *dl_port)
+{
+	return (dl_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL ||
+		dl_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_CPU ||
+		dl_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_DSA);
+}
+
 static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 				     struct devlink_port *devlink_port)
 {
@@ -515,8 +523,14 @@  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))
+	if (is_devlink_phy_port_num_supported(devlink_port) &&
+	    nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
 		return -EMSGSIZE;
+	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
+		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
+				attrs->pci_pf.pf))
+			return -EMSGSIZE;
+	}
 	if (!attrs->split)
 		return 0;
 	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs->port_number))
@@ -5738,6 +5752,30 @@  void devlink_port_type_clear(struct devlink_port *devlink_port)
 }
 EXPORT_SYMBOL_GPL(devlink_port_type_clear);
 
+static void __devlink_port_attrs_set(struct devlink_port *devlink_port,
+				     enum devlink_port_flavour flavour,
+				     u32 port_number,
+				     const unsigned char *switch_id,
+				     unsigned char switch_id_len)
+{
+	struct devlink_port_attrs *attrs = &devlink_port->attrs;
+
+	if (WARN_ON(devlink_port->registered))
+		return;
+	attrs->set = true;
+	attrs->flavour = flavour;
+	attrs->port_number = port_number;
+	if (switch_id) {
+		attrs->switch_port = true;
+		if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
+			switch_id_len = MAX_PHYS_ITEM_ID_LEN;
+		memcpy(attrs->switch_id.id, switch_id, switch_id_len);
+		attrs->switch_id.id_len = switch_id_len;
+	} else {
+		attrs->switch_port = false;
+	}
+}
+
 /**
  *	devlink_port_attrs_set - Set port attributes
  *
@@ -5761,25 +5799,34 @@  void devlink_port_attrs_set(struct devlink_port *devlink_port,
 {
 	struct devlink_port_attrs *attrs = &devlink_port->attrs;
 
-	if (WARN_ON(devlink_port->registered))
-		return;
-	attrs->set = true;
-	attrs->flavour = flavour;
-	attrs->port_number = port_number;
+	__devlink_port_attrs_set(devlink_port, flavour, port_number,
+				 switch_id, switch_id_len);
 	attrs->split = split;
 	attrs->split_subport_number = split_subport_number;
-	if (switch_id) {
-		attrs->switch_port = true;
-		if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
-			switch_id_len = MAX_PHYS_ITEM_ID_LEN;
-		memcpy(attrs->switch_id.id, switch_id, switch_id_len);
-		attrs->switch_id.id_len = switch_id_len;
-	} else {
-		attrs->switch_port = false;
-	}
 }
 EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
 
+/**
+ *	devlink_port_attrs_pci_pf_set - Set PCI PF port attributes
+ *
+ *	@devlink_port: devlink port
+ *	@pf: associated PF for the devlink port instance
+ *	@switch_id: if the port is part of switch, this is buffer with ID,
+ *	            otwerwise this is NULL
+ *	@switch_id_len: length of the switch_id buffer
+ */
+void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
+				   const unsigned char *switch_id,
+				   unsigned char switch_id_len, u16 pf)
+{
+	struct devlink_port_attrs *attrs = &devlink_port->attrs;
+
+	__devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_PCI_PF,
+				 0, switch_id, switch_id_len);
+	attrs->pci_pf.pf = pf;
+}
+EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_pf_set);
+
 static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
 					     char *name, size_t len)
 {
@@ -5804,6 +5851,9 @@  static int __devlink_port_phys_port_name_get(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.pf);
+		break;
 	}
 
 	if (n >= len)