[patch net-next RFC 0/3] export device physical port id to userspace

[patch net-next RFC 0/3] export device physical port id to userspace

[Jiri Pirko]

This patchset is based on patch by Narendra_K@Dell.com
Once device which can change phys port id during its lifetime adopts this,
NETDEV_CHANGEPHYSPORTID event will be added and driver will call
call_netdevice_notifiers(NETDEV_NETDEV_CHANGEPHYSPORTID, dev) to propagete
the change to userspace.

Jiri Pirko (3):
  net: add ndo to get id of physical port of the device
  rtnl: export physical port id via RT netlink
  net: export physical port id via sysfs

 include/linux/netdevice.h    | 18 ++++++++++++++++++
 include/uapi/linux/if_link.h |  1 +
 net/core/net-sysfs.c         | 41 +++++++++++++++++++++++++++++++++++++++++
 net/core/rtnetlink.c         | 25 ++++++++++++++++++++++++-
 4 files changed, 84 insertions(+), 1 deletion(-)

-- 
1.8.1.4

[patch net-next RFC 1/3] net: add ndo to get id of physical port of the device

[Jiri Pirko]

This patch adds a ndo for getting physical port of the device. Driver
which is aware of being virtual function of some physical port should
implement this ndo.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/netdevice.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0741a1e..e85f177 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -728,6 +728,16 @@ struct netdev_fcoe_hbainfo {
 };
 #endif
 
+#define MAX_PHYS_PORT_ID_LEN 32
+
+/* This structure holds a universally unique identifier to
+ * identify the physical port used by a netdevice
+ */
+struct netdev_phys_port_id {
+	unsigned char id[MAX_PHYS_PORT_ID_LEN];
+	unsigned char id_len;
+};
+
 /*
  * This structure defines the management hooks for network devices.
  * The following hooks can be defined; unless noted otherwise, they are
@@ -932,6 +942,12 @@ struct netdev_fcoe_hbainfo {
  *	that determine carrier state from physical hardware properties (eg
  *	network cables) or protocol-dependent mechanisms (eg
  *	USB_CDC_NOTIFY_NETWORK_CONNECTION) should NOT implement this function.
+ *
+ * int (*ndo_get_phys_port_id)(struct net_device *dev,
+ *			       struct netdev_phys_port_id *ppid);
+ *	Called to get ID of physical port of this device. If driver does
+ *	not implement this, it is assumed that the hw is not able to have
+ *	multiple net devices on single physical port.
  */
 struct net_device_ops {
 	int			(*ndo_init)(struct net_device *dev);
@@ -1060,6 +1076,8 @@ struct net_device_ops {
 						      struct nlmsghdr *nlh);
 	int			(*ndo_change_carrier)(struct net_device *dev,
 						      bool new_carrier);
+	int			(*ndo_get_phys_port_id)(struct net_device *dev,
+							struct netdev_phys_port_id *ppid);
 };
 
 /*
-- 
1.8.1.4

Re: [patch net-next RFC 1/3] net: add ndo to get id of physical port of the device

[Ben Hutchings]

On Mon, 2013-07-15 at 19:07 +0200, Jiri Pirko wrote:
> This patch adds a ndo for getting physical port of the device. Driver
> which is aware of being virtual function of some physical port should
> implement this ndo.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  include/linux/netdevice.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 0741a1e..e85f177 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -728,6 +728,16 @@ struct netdev_fcoe_hbainfo {
>  };
>  #endif
>  
> +#define MAX_PHYS_PORT_ID_LEN 32
> +
> +/* This structure holds a universally unique identifier to
> + * identify the physical port used by a netdevice
[...]

There is a slight problem with saying 'universally unique identifier'
which is that it may be taken to mean specifically a DCE 128-bit UUID.
Is there a slightly more generic term that would clearly allow for other
ID spaces here?  Or should we give a list of examples: 'such as a
128-bit UUID, globally unique MAC-48 or EUI-64'?

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [patch net-next RFC 1/3] net: add ndo to get id of physical port of the device

[Jiri Pirko]

Tue, Jul 16, 2013 at 12:01:57AM CEST, bhutchings@solarflare.com wrote:
>On Mon, 2013-07-15 at 19:07 +0200, Jiri Pirko wrote:
>> This patch adds a ndo for getting physical port of the device. Driver
>> which is aware of being virtual function of some physical port should
>> implement this ndo.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  include/linux/netdevice.h | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>> 
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 0741a1e..e85f177 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -728,6 +728,16 @@ struct netdev_fcoe_hbainfo {
>>  };
>>  #endif
>>  
>> +#define MAX_PHYS_PORT_ID_LEN 32
>> +
>> +/* This structure holds a universally unique identifier to
>> + * identify the physical port used by a netdevice
>[...]
>
>There is a slight problem with saying 'universally unique identifier'
>which is that it may be taken to mean specifically a DCE 128-bit UUID.
>Is there a slightly more generic term that would clearly allow for other
>ID spaces here?  Or should we give a list of examples: 'such as a
>128-bit UUID, globally unique MAC-48 or EUI-64'?

Well I do not have any speficic format in mind. I think that whatever
hash (driver's choice) would do. I will reword the comment.

Thanks Ben.

Jiri

>
>Ben.
>
>-- 
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>

[patch net-next RFC 2/3] rtnl: export physical port id via RT netlink

[Jiri Pirko]

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/uapi/linux/if_link.h |  1 +
 net/core/rtnetlink.c         | 25 ++++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 03f6170..04c0e7a 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -143,6 +143,7 @@ enum {
 	IFLA_NUM_TX_QUEUES,
 	IFLA_NUM_RX_QUEUES,
 	IFLA_CARRIER,
+	IFLA_PHYS_PORT_ID,
 	__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 3de7408..2bd0e67 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -767,7 +767,8 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + rtnl_vfinfo_size(dev, ext_filter_mask) /* IFLA_VFINFO_LIST */
 	       + rtnl_port_size(dev) /* IFLA_VF_PORTS + IFLA_PORT_SELF */
 	       + rtnl_link_get_size(dev) /* IFLA_LINKINFO */
-	       + rtnl_link_get_af_size(dev); /* IFLA_AF_SPEC */
+	       + rtnl_link_get_af_size(dev) /* IFLA_AF_SPEC */
+	       + nla_total_size(MAX_PHYS_PORT_ID_LEN); /* IFLA_PHYS_PORT_ID */
 }
 
 static int rtnl_vf_ports_fill(struct sk_buff *skb, struct net_device *dev)
@@ -846,6 +847,24 @@ static int rtnl_port_fill(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
+static int rtnl_phys_port_id_fill(struct sk_buff *skb, struct net_device *dev)
+{
+	int err;
+	struct netdev_phys_port_id ppid;
+
+	if (!dev->netdev_ops->ndo_get_phys_port_id)
+		return 0;
+
+	err = dev->netdev_ops->ndo_get_phys_port_id(dev, &ppid);
+	if (err)
+		return err;
+
+	if (nla_put(skb, IFLA_PHYS_PORT_ID, ppid.id_len, ppid.id))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
 			    unsigned int flags, u32 ext_filter_mask)
@@ -913,6 +932,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			goto nla_put_failure;
 	}
 
+	if (rtnl_phys_port_id_fill(skb, dev))
+		goto nla_put_failure;
+
 	attr = nla_reserve(skb, IFLA_STATS,
 			sizeof(struct rtnl_link_stats));
 	if (attr == NULL)
@@ -1113,6 +1135,7 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_PROMISCUITY]	= { .type = NLA_U32 },
 	[IFLA_NUM_TX_QUEUES]	= { .type = NLA_U32 },
 	[IFLA_NUM_RX_QUEUES]	= { .type = NLA_U32 },
+	[IFLA_PHYS_PORT_ID]	= { .type = NLA_BINARY, .len = MAX_PHYS_PORT_ID_LEN },
 };
 EXPORT_SYMBOL(ifla_policy);
 
-- 
1.8.1.4

[patch net-next RFC 3/3] net: export physical port id via sysfs

[Jiri Pirko]

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/core/net-sysfs.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 981fed3..fb19ede 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -334,6 +334,46 @@ static ssize_t store_group(struct device *dev, struct device_attribute *attr,
 	return netdev_store(dev, attr, buf, len, change_group);
 }
 
+static size_t _format_port_id(char *buf, int buflen,
+			      const unsigned char *id, int len)
+{
+	int i;
+	char *cp = buf;
+
+	for (i = 0; i < len; i++)
+		cp += scnprintf(cp, buflen - (cp - buf), "%02x", id[i]);
+	return cp - buf;
+}
+
+ssize_t sysfs_format_port_id(char *buf, const unsigned char *id, int len)
+{
+	size_t l;
+
+	l = _format_port_id(buf, PAGE_SIZE, id, len);
+	l += scnprintf(buf + l, PAGE_SIZE - l, "\n");
+	return (ssize_t)l;
+}
+static ssize_t show_phys_port_id(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct net_device *netdev = to_net_dev(dev);
+	ssize_t ret = 0;
+
+	if (!rtnl_trylock())
+		return restart_syscall();
+
+	if (dev_isalive(netdev) && netdev->netdev_ops->ndo_get_phys_port_id) {
+		struct netdev_phys_port_id ppid;
+
+		ret = netdev->netdev_ops->ndo_get_phys_port_id(netdev, &ppid);
+		if (!ret)
+			ret = sysfs_format_port_id(buf, ppid.id, ppid.id_len);
+	}
+	rtnl_unlock();
+
+	return ret;
+}
+
 static struct device_attribute net_class_attributes[] = {
 	__ATTR(addr_assign_type, S_IRUGO, show_addr_assign_type, NULL),
 	__ATTR(addr_len, S_IRUGO, show_addr_len, NULL),
@@ -355,6 +395,7 @@ static struct device_attribute net_class_attributes[] = {
 	__ATTR(tx_queue_len, S_IRUGO | S_IWUSR, show_tx_queue_len,
 	       store_tx_queue_len),
 	__ATTR(netdev_group, S_IRUGO | S_IWUSR, show_group, store_group),
+	__ATTR(phys_port_id, S_IRUGO, show_phys_port_id, NULL),
 	{}
 };
 
-- 
1.8.1.4

RE: [patch net-next RFC 3/3] net: export physical port id via sysfs

[Narendra_K]

> -----Original Message-----
> From: Jiri Pirko [mailto:jiri@resnulli.us]
> Sent: Monday, July 15, 2013 10:37 PM
> To: netdev@vger.kernel.org
> Cc: K, Narendra; bhutchings@solarflare.com; john.r.fastabend@intel.com
> Subject: [patch net-next RFC 3/3] net: export physical port id via sysfs
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  net/core/net-sysfs.c | 41
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index
> 981fed3..fb19ede 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -334,6 +334,46 @@ static ssize_t store_group(struct device *dev, struct
> device_attribute *attr,
>  	return netdev_store(dev, attr, buf, len, change_group);  }
> 
> +static size_t _format_port_id(char *buf, int buflen,
> +			      const unsigned char *id, int len) {
> +	int i;
> +	char *cp = buf;
> +
> +	for (i = 0; i < len; i++)
> +		cp += scnprintf(cp, buflen - (cp - buf), "%02x", id[i]);
> +	return cp - buf;
> +}
> +
> +ssize_t sysfs_format_port_id(char *buf, const unsigned char *id, int
> +len) {
> +	size_t l;
> +
> +	l = _format_port_id(buf, PAGE_SIZE, id, len);
> +	l += scnprintf(buf + l, PAGE_SIZE - l, "\n");
> +	return (ssize_t)l;
> +}
> +static ssize_t show_phys_port_id(struct device *dev,
> +				 struct device_attribute *attr, char *buf) {
> +	struct net_device *netdev = to_net_dev(dev);
> +	ssize_t ret = 0;
> +
> +	if (!rtnl_trylock())
> +		return restart_syscall();
> +
> +	if (dev_isalive(netdev) && netdev->netdev_ops-
> >ndo_get_phys_port_id) {
> +		struct netdev_phys_port_id ppid;
> +
> +		ret = netdev->netdev_ops->ndo_get_phys_port_id(netdev,
> &ppid);
[>] 

Hello,
I am thinking if "ndo_get_phys_port_id()" is required.  With it, the driver needs to generate port_id  every time "ndo_get_phys_port_id()" is called, but the "netdev->phys_port.port_id" might not have changed.  If 'phys_port'  structure is part of 'struct net_device',  then 'netdev->phys_port.port_id' and port_id_len are set by driver before calling 'register_netdev' and are available to the core.  Driver not implementing it would indicate it by a "port_id_len" of zero.     If the "netdev->phys_port.port_id" changes for some reason then the notification sent by the driver would ensure that the interested kernel components and user space are notified of the change.

With regards,
Narendra K
Linux Engineering
Dell Inc.

Re: [patch net-next RFC 3/3] net: export physical port id via sysfs

[Jiri Pirko]

Tue, Jul 16, 2013 at 10:03:04PM CEST, Narendra_K@Dell.com wrote:
>> -----Original Message-----
>> From: Jiri Pirko [mailto:jiri@resnulli.us]
>> Sent: Monday, July 15, 2013 10:37 PM
>> To: netdev@vger.kernel.org
>> Cc: K, Narendra; bhutchings@solarflare.com; john.r.fastabend@intel.com
>> Subject: [patch net-next RFC 3/3] net: export physical port id via sysfs
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  net/core/net-sysfs.c | 41
>> +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 41 insertions(+)
>> 
>> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index
>> 981fed3..fb19ede 100644
>> --- a/net/core/net-sysfs.c
>> +++ b/net/core/net-sysfs.c
>> @@ -334,6 +334,46 @@ static ssize_t store_group(struct device *dev, struct
>> device_attribute *attr,
>>  	return netdev_store(dev, attr, buf, len, change_group);  }
>> 
>> +static size_t _format_port_id(char *buf, int buflen,
>> +			      const unsigned char *id, int len) {
>> +	int i;
>> +	char *cp = buf;
>> +
>> +	for (i = 0; i < len; i++)
>> +		cp += scnprintf(cp, buflen - (cp - buf), "%02x", id[i]);
>> +	return cp - buf;
>> +}
>> +
>> +ssize_t sysfs_format_port_id(char *buf, const unsigned char *id, int
>> +len) {
>> +	size_t l;
>> +
>> +	l = _format_port_id(buf, PAGE_SIZE, id, len);
>> +	l += scnprintf(buf + l, PAGE_SIZE - l, "\n");
>> +	return (ssize_t)l;
>> +}
>> +static ssize_t show_phys_port_id(struct device *dev,
>> +				 struct device_attribute *attr, char *buf) {
>> +	struct net_device *netdev = to_net_dev(dev);
>> +	ssize_t ret = 0;
>> +
>> +	if (!rtnl_trylock())
>> +		return restart_syscall();
>> +
>> +	if (dev_isalive(netdev) && netdev->netdev_ops-
>> >ndo_get_phys_port_id) {
>> +		struct netdev_phys_port_id ppid;
>> +
>> +		ret = netdev->netdev_ops->ndo_get_phys_port_id(netdev,
>> &ppid);
>[>] 
>
>Hello,
>I am thinking if "ndo_get_phys_port_id()" is required.  With it, the driver needs to generate port_id  every time "ndo_get_phys_port_id()" is called, but the "netdev->phys_port.port_id" might not have changed.  If 'phys_port'  structure is part of 'struct net_device',  then 'netdev->phys_port.port_id' and port_id_len are set by driver before calling 'register_netdev' and are available to the core.  Driver not implementing it would indicate it by a "port_id_len" of zero.     If the "netdev->phys_port.port_id" changes for some reason then the notification sent by the driver would ensure that the interested kernel components and user space are notified of the change.

Although this can be done by extending netdevice structure by another
item, I believe it is cleaner to do it by ndo. Driver has a flexibility
to either compute the phys port on fly of compute it once, store it in
it's private data and use it when ndo is called.
	
>
>With regards,
>Narendra K
>Linux Engineering
>Dell Inc.

RE: [patch net-next RFC 3/3] net: export physical port id via sysfs

[Narendra_K]

> -----Original Message-----
> From: Jiri Pirko [mailto:jiri@resnulli.us]
> Sent: Wednesday, July 17, 2013 2:00 PM
> To: K, Narendra
> Cc: netdev@vger.kernel.org; bhutchings@solarflare.com;
> john.r.fastabend@intel.com
> Subject: Re: [patch net-next RFC 3/3] net: export physical port id via sysfs
> 
> Tue, Jul 16, 2013 at 10:03:04PM CEST, Narendra_K@Dell.com wrote:
[...] 
> >> +static ssize_t show_phys_port_id(struct device *dev,
> >> +				 struct device_attribute *attr, char *buf) {
> >> +	struct net_device *netdev = to_net_dev(dev);
> >> +	ssize_t ret = 0;
> >> +
> >> +	if (!rtnl_trylock())
> >> +		return restart_syscall();
> >> +
> >> +	if (dev_isalive(netdev) && netdev->netdev_ops-
> >> >ndo_get_phys_port_id) {
> >> +		struct netdev_phys_port_id ppid;
> >> +
> >> +		ret = netdev->netdev_ops->ndo_get_phys_port_id(netdev,
> >> &ppid);
> >[>]
> >
> >Hello,
> >I am thinking if "ndo_get_phys_port_id()" is required.  With it, the driver
> needs to generate port_id  every time "ndo_get_phys_port_id()" is called,
> but the "netdev->phys_port.port_id" might not have changed.  If 'phys_port'
> structure is part of 'struct net_device',  then 'netdev->phys_port.port_id'
> and port_id_len are set by driver before calling 'register_netdev' and are
> available to the core.  Driver not implementing it would indicate it by a
> "port_id_len" of zero.     If the "netdev->phys_port.port_id" changes for
> some reason then the notification sent by the driver would ensure that the
> interested kernel components and user space are notified of the change.
> 
> Although this can be done by extending netdevice structure by another item,
> I believe it is cleaner to do it by ndo. Driver has a flexibility to either compute
> the phys port on fly of compute it once, store it in it's private data and use it
> when ndo is called.

It seems to me that phys_port identifier is a net_device property,  similar to netdev->dev_addr or netdev->if_port, and could be part of struct net_device  as discussed in [1].  My understanding may not be correct.

Hello Ben,  for the hybrid guest networking scenario, would it be required/helpful for phys_port to be part of net_device structure or either of the approaches would be fine.

With regards,
Narendra K
Linux Engineering
Dell Inc.

[1] http://marc.info/?l=linux-netdev&m=136920998009209&w=2

Re: [patch net-next RFC 3/3] net: export physical port id via sysfs

[Ben Hutchings]

On Thu, 2013-07-18 at 01:09 +0530, Narendra_K@Dell.com wrote:
> > -----Original Message-----
> > From: Jiri Pirko [mailto:jiri@resnulli.us]
[...]
> > Although this can be done by extending netdevice structure by another item,
> > I believe it is cleaner to do it by ndo. Driver has a flexibility to either compute
> > the phys port on fly of compute it once, store it in it's private data and use it
> > when ndo is called.

Doesn't the same argument apply to perm_addr?  And the flexibility there
turned out to be completely pointless.

> It seems to me that phys_port identifier is a net_device property,
> similar to netdev->dev_addr or netdev->if_port, and could be part of
> struct net_device  as discussed in [1].  My understanding may not be
> correct.
> 
> Hello Ben,  for the hybrid guest networking scenario, would it be
> required/helpful for phys_port to be part of net_device structure or
> either of the approaches would be fine.

I don't think it matters.  Either would be fine.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.