[patch net-next v5 0/4] export device physical port id to userspace

[patch net-next v5 0/4] 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 propagate
the change to userspace.

v1->v2: as suggested by Ben, handle -EOPNOTSUPP in rtnl code (wrapped up ndo call)
v2->v3: adjusted patch 1 commit message
v3->v4: used "%phN" for sysfs printf as suggested by DaveM
        added igb/igbvf implementation as requested by Or Gerlitz
v4->v5: used prandom_u32 to generate id in igb_probe
	removed duplicate code in ibgvf_probe
	pushed dev_err string into one line in igbvf_refresh_ppid

Jiri Pirko (4):
  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
  igb/igbvf: implement ndo_get_phys_port_id

 drivers/net/ethernet/intel/igb/e1000_mbx.h |  1 +
 drivers/net/ethernet/intel/igb/igb.h       |  2 ++
 drivers/net/ethernet/intel/igb/igb_main.c  | 29 +++++++++++++++++++++-
 drivers/net/ethernet/intel/igbvf/igbvf.h   |  3 +++
 drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
 drivers/net/ethernet/intel/igbvf/netdev.c  | 40 ++++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/igbvf/vf.c      | 34 +++++++++++++++++++++++++
 drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
 include/linux/netdevice.h                  | 20 +++++++++++++++
 include/uapi/linux/if_link.h               |  1 +
 net/core/dev.c                             | 18 ++++++++++++++
 net/core/net-sysfs.c                       | 22 ++++++++++++++++
 net/core/rtnetlink.c                       | 25 ++++++++++++++++++-
 13 files changed, 195 insertions(+), 2 deletions(-)

-- 
1.8.1.4

[patch net-next v5 1/4] 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. This is applicable not only for IOV, but for other
solutions (NPAR, multichannel) as well. Basically if there is possible
to have multiple netdevs on the single hw port.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>
---
 include/linux/netdevice.h | 20 ++++++++++++++++++++
 net/core/dev.c            | 18 ++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3ca60b0..875f869 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 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);
 };
 
 /*
@@ -2315,6 +2333,8 @@ extern int		dev_set_mac_address(struct net_device *,
 					    struct sockaddr *);
 extern int		dev_change_carrier(struct net_device *,
 					   bool new_carrier);
+extern int		dev_get_phys_port_id(struct net_device *dev,
+					     struct netdev_phys_port_id *ppid);
 extern int		dev_hard_start_xmit(struct sk_buff *skb,
 					    struct net_device *dev,
 					    struct netdev_queue *txq);
diff --git a/net/core/dev.c b/net/core/dev.c
index dfd9f5d..58eb802 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4989,6 +4989,24 @@ int dev_change_carrier(struct net_device *dev, bool new_carrier)
 EXPORT_SYMBOL(dev_change_carrier);
 
 /**
+ *	dev_get_phys_port_id - Get device physical port ID
+ *	@dev: device
+ *	@ppid: port ID
+ *
+ *	Get device physical port ID
+ */
+int dev_get_phys_port_id(struct net_device *dev,
+			 struct netdev_phys_port_id *ppid)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+
+	if (!ops->ndo_get_phys_port_id)
+		return -EOPNOTSUPP;
+	return ops->ndo_get_phys_port_id(dev, ppid);
+}
+EXPORT_SYMBOL(dev_get_phys_port_id);
+
+/**
  *	dev_new_index	-	allocate an ifindex
  *	@net: the applicable net namespace
  *
-- 
1.8.1.4

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

[Jiri Pirko]

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Narendra K <narendra_k@dell.com>
---
 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..0b2972c 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;
+
+	err = dev_get_phys_port_id(dev, &ppid);
+	if (err) {
+		if (err == -EOPNOTSUPP)
+			return 0;
+		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 v5 3/4] net: export physical port id via sysfs

[Jiri Pirko]

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Narendra K <narendra_k@dell.com>
---
 net/core/net-sysfs.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 981fed3..8826b0d 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -334,6 +334,27 @@ static ssize_t store_group(struct device *dev, struct device_attribute *attr,
 	return netdev_store(dev, attr, buf, len, change_group);
 }
 
+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 = -EINVAL;
+
+	if (!rtnl_trylock())
+		return restart_syscall();
+
+	if (dev_isalive(netdev)) {
+		struct netdev_phys_port_id ppid;
+
+		ret = dev_get_phys_port_id(netdev, &ppid);
+		if (!ret)
+			ret = sprintf(buf, "%*phN\n", ppid.id_len, ppid.id);
+	}
+	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 +376,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

[patch net-next v5 4/4] igb/igbvf: implement ndo_get_phys_port_id

[Jiri Pirko]

igb driver generated random number which will identify physical port.
This id is available via ndo_get_phys_port_id directly on igb netdev.
Also, id is passed to igbvf using mailbox. After that, it is available via
ndo_get_phys_port_id on igbvf netdev as well.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/ethernet/intel/igb/e1000_mbx.h |  1 +
 drivers/net/ethernet/intel/igb/igb.h       |  2 ++
 drivers/net/ethernet/intel/igb/igb_main.c  | 29 +++++++++++++++++++++-
 drivers/net/ethernet/intel/igbvf/igbvf.h   |  3 +++
 drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
 drivers/net/ethernet/intel/igbvf/netdev.c  | 40 ++++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/igbvf/vf.c      | 34 +++++++++++++++++++++++++
 drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
 8 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_mbx.h b/drivers/net/ethernet/intel/igb/e1000_mbx.h
index de9bba4..1788480 100644
--- a/drivers/net/ethernet/intel/igb/e1000_mbx.h
+++ b/drivers/net/ethernet/intel/igb/e1000_mbx.h
@@ -64,6 +64,7 @@
 #define E1000_VF_SET_LPE	0x05 /* VF requests to set VMOLR.LPE */
 #define E1000_VF_SET_PROMISC	0x06 /*VF requests to clear VMOLR.ROPE/MPME*/
 #define E1000_VF_SET_PROMISC_MULTICAST	(0x02 << E1000_VT_MSGINFO_SHIFT)
+#define E1000_VF_GET_PHYS_PORT_ID 0x07 /* VF requests to get physical port id */
 
 #define E1000_PF_CONTROL_MSG	0x0100 /* PF control message */
 
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 15ea8dc..12a25b1 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -444,6 +444,8 @@ struct igb_adapter {
 	struct i2c_algo_bit_data i2c_algo;
 	struct i2c_adapter i2c_adap;
 	struct i2c_client *i2c_client;
+
+	u32 phys_port_id;
 };
 
 #define IGB_FLAG_HAS_MSI		(1 << 0)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 6a0c1b6..9ecb696 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -58,6 +58,7 @@
 #include <linux/dca.h>
 #endif
 #include <linux/i2c.h>
+#include <linux/random.h>
 #include "igb.h"
 
 #define MAJ 5
@@ -1892,6 +1893,24 @@ static int igb_set_features(struct net_device *netdev,
 	return 0;
 }
 
+/**
+ * igb_get_phys_port_id - Get physical port ID
+ * @netdev: network interface device structure
+ * @ppid: pointer to a physical port id structure
+ *
+ * Returns 0 on success, negative on failure
+ **/
+static int igb_get_phys_port_id(struct net_device *netdev,
+				struct netdev_phys_port_id *ppid)
+{
+	struct igb_adapter *adapter = netdev_priv(netdev);
+
+	ppid->id_len = sizeof(adapter->phys_port_id);
+	memcpy(ppid->id, &adapter->phys_port_id, ppid->id_len);
+
+	return 0;
+}
+
 static const struct net_device_ops igb_netdev_ops = {
 	.ndo_open		= igb_open,
 	.ndo_stop		= igb_close,
@@ -1915,6 +1934,7 @@ static const struct net_device_ops igb_netdev_ops = {
 #endif
 	.ndo_fix_features	= igb_fix_features,
 	.ndo_set_features	= igb_set_features,
+	.ndo_get_phys_port_id	= igb_get_phys_port_id,
 };
 
 /**
@@ -2287,6 +2307,8 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * driver. */
 	igb_get_hw_control(adapter);
 
+	adapter->phys_port_id = prandom_u32();
+
 	strcpy(netdev->name, "eth%d");
 	err = register_netdev(netdev);
 	if (err)
@@ -5698,6 +5720,7 @@ static void igb_rcv_msg_from_vf(struct igb_adapter *adapter, u32 vf)
 	struct e1000_hw *hw = &adapter->hw;
 	struct vf_data_storage *vf_data = &adapter->vf_data[vf];
 	s32 retval;
+	u16 write_size = 1;
 
 	retval = igb_read_mbx(hw, msgbuf, E1000_VFMAILBOX_SIZE, vf);
 
@@ -5757,6 +5780,10 @@ static void igb_rcv_msg_from_vf(struct igb_adapter *adapter, u32 vf)
 		else
 			retval = igb_set_vf_vlan(adapter, msgbuf, vf);
 		break;
+	case E1000_VF_GET_PHYS_PORT_ID:
+		msgbuf[1] = adapter->phys_port_id;
+		write_size = 2;
+		break;
 	default:
 		dev_err(&pdev->dev, "Unhandled Msg %08x\n", msgbuf[0]);
 		retval = -1;
@@ -5771,7 +5798,7 @@ out:
 	else
 		msgbuf[0] |= E1000_VT_MSGTYPE_ACK;
 
-	igb_write_mbx(hw, msgbuf, 1, vf);
+	igb_write_mbx(hw, msgbuf, write_size, vf);
 }
 
 static void igb_msg_task(struct igb_adapter *adapter)
diff --git a/drivers/net/ethernet/intel/igbvf/igbvf.h b/drivers/net/ethernet/intel/igbvf/igbvf.h
index a1463e3..559ccaf 100644
--- a/drivers/net/ethernet/intel/igbvf/igbvf.h
+++ b/drivers/net/ethernet/intel/igbvf/igbvf.h
@@ -283,6 +283,9 @@ struct igbvf_adapter {
 
 	unsigned int flags;
 	unsigned long last_reset;
+
+	u32 phys_port_id;
+	bool phys_port_id_set;
 };
 
 struct igbvf_info {
diff --git a/drivers/net/ethernet/intel/igbvf/mbx.h b/drivers/net/ethernet/intel/igbvf/mbx.h
index 24370bc..1040d36 100644
--- a/drivers/net/ethernet/intel/igbvf/mbx.h
+++ b/drivers/net/ethernet/intel/igbvf/mbx.h
@@ -66,6 +66,7 @@
 #define E1000_VF_SET_MULTICAST    0x03 /* VF requests PF to set MC addr */
 #define E1000_VF_SET_VLAN         0x04 /* VF requests PF to set VLAN */
 #define E1000_VF_SET_LPE          0x05 /* VF requests PF to set VMOLR.LPE */
+#define E1000_VF_GET_PHYS_PORT_ID 0x07 /* VF requests to get physical port id */
 
 #define E1000_PF_CONTROL_MSG      0x0100 /* PF control message */
 
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 93eb7ee..7f46c26 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -1444,6 +1444,20 @@ static void igbvf_configure(struct igbvf_adapter *adapter)
 	                       igbvf_desc_unused(adapter->rx_ring));
 }
 
+static void igbvf_refresh_ppid(struct igbvf_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	int err;
+
+	err = hw->mac.ops.ppid_get_vf(hw, &adapter->phys_port_id);
+	if (err) {
+		adapter->phys_port_id_set = false;
+		dev_err(&adapter->pdev->dev, "Failed to get physical port ID\n");
+	} else {
+		adapter->phys_port_id_set = true;
+	}
+}
+
 /* igbvf_reset - bring the hardware into a known good state
  *
  * This function boots the hardware and enables some settings that
@@ -1461,6 +1475,8 @@ static void igbvf_reset(struct igbvf_adapter *adapter)
 	if (mac->ops.reset_hw(hw))
 		dev_err(&adapter->pdev->dev, "PF still resetting\n");
 
+	igbvf_refresh_ppid(adapter);
+
 	mac->ops.init_hw(hw);
 
 	if (is_valid_ether_addr(adapter->hw.mac.addr)) {
@@ -1753,6 +1769,27 @@ static int igbvf_set_mac(struct net_device *netdev, void *p)
 	return 0;
 }
 
+/**
+ * igbvf_get_phys_port_id - Get physical port ID
+ * @netdev: network interface device structure
+ * @ppid: pointer to a physical port id structure
+ *
+ * Returns 0 on success, negative on failure
+ **/
+static int igbvf_get_phys_port_id(struct net_device *netdev,
+				  struct netdev_phys_port_id *ppid)
+{
+	struct igbvf_adapter *adapter = netdev_priv(netdev);
+
+	if (!adapter->phys_port_id_set)
+		return -EOPNOTSUPP;
+
+	ppid->id_len = sizeof(adapter->phys_port_id);
+	memcpy(ppid->id, &adapter->phys_port_id, ppid->id_len);
+
+	return 0;
+}
+
 #define UPDATE_VF_COUNTER(reg, name)                                    \
 	{                                                               \
 		u32 current_counter = er32(reg);                        \
@@ -2610,6 +2647,7 @@ static const struct net_device_ops igbvf_netdev_ops = {
 	.ndo_poll_controller            = igbvf_netpoll,
 #endif
 	.ndo_set_features               = igbvf_set_features,
+	.ndo_get_phys_port_id		= igbvf_get_phys_port_id,
 };
 
 /**
@@ -2759,6 +2797,8 @@ static int igbvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 			netdev->addr_len);
 	}
 
+	igbvf_refresh_ppid(adapter);
+
 	setup_timer(&adapter->watchdog_timer, &igbvf_watchdog,
 	            (unsigned long) adapter);
 
diff --git a/drivers/net/ethernet/intel/igbvf/vf.c b/drivers/net/ethernet/intel/igbvf/vf.c
index eea0e10..1a4afa5 100644
--- a/drivers/net/ethernet/intel/igbvf/vf.c
+++ b/drivers/net/ethernet/intel/igbvf/vf.c
@@ -39,6 +39,7 @@ static void e1000_update_mc_addr_list_vf(struct e1000_hw *hw, u8 *,
 static void e1000_rar_set_vf(struct e1000_hw *, u8 *, u32);
 static s32 e1000_read_mac_addr_vf(struct e1000_hw *);
 static s32 e1000_set_vfta_vf(struct e1000_hw *, u16, bool);
+static s32 e1000_ppid_get_vf(struct e1000_hw *hw, u32 *id);
 
 /**
  *  e1000_init_mac_params_vf - Inits MAC params
@@ -70,6 +71,8 @@ static s32 e1000_init_mac_params_vf(struct e1000_hw *hw)
 	mac->ops.read_mac_addr = e1000_read_mac_addr_vf;
 	/* set vlan filter table array */
 	mac->ops.set_vfta = e1000_set_vfta_vf;
+	/* get physical port ID */
+	mac->ops.ppid_get_vf = e1000_ppid_get_vf;
 
 	return E1000_SUCCESS;
 }
@@ -398,3 +401,34 @@ out:
 	return ret_val;
 }
 
+/**
+ *  e1000_ppid_get_vf - get device MAC physical port ID
+ *  @hw: pointer to the HW structure
+ *  @id: pointer where port ID will be written
+ **/
+static s32 e1000_ppid_get_vf(struct e1000_hw *hw, u32 *id)
+{
+	struct e1000_mbx_info *mbx = &hw->mbx;
+	u32 msgbuf[2];
+	s32 ret_val;
+
+	memset(msgbuf, 0, sizeof(msgbuf));
+	msgbuf[0] = E1000_VF_GET_PHYS_PORT_ID;
+
+	ret_val = mbx->ops.write_posted(hw, msgbuf, 1);
+	if (ret_val)
+		goto out;
+
+	ret_val = mbx->ops.read_posted(hw, msgbuf, 2);
+	if (ret_val)
+		goto out;
+
+	if (msgbuf[0] & E1000_VT_MSGTYPE_NACK) {
+		ret_val = -E1000_ERR_MAC_INIT;
+		goto out;
+	}
+	*id = msgbuf[1];
+
+out:
+	return ret_val;
+}
diff --git a/drivers/net/ethernet/intel/igbvf/vf.h b/drivers/net/ethernet/intel/igbvf/vf.h
index 57db3c6..aa04f46 100644
--- a/drivers/net/ethernet/intel/igbvf/vf.h
+++ b/drivers/net/ethernet/intel/igbvf/vf.h
@@ -188,6 +188,7 @@ struct e1000_mac_operations {
 	void (*rar_set)(struct e1000_hw *, u8*, u32);
 	s32  (*read_mac_addr)(struct e1000_hw *);
 	s32  (*set_vfta)(struct e1000_hw *, u16, bool);
+	s32  (*ppid_get_vf)(struct e1000_hw *, u32 *);
 };
 
 struct e1000_mac_info {
-- 
1.8.1.4

Re: [patch net-next v5 4/4] igb/igbvf: implement ndo_get_phys_port_id

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

On Fri, 2013-07-26 at 14:09 +0200, Jiri Pirko wrote:
> igb driver generated random number which will identify physical port.
> This id is available via ndo_get_phys_port_id directly on igb netdev.
> Also, id is passed to igbvf using mailbox. After that, it is available
> via
> ndo_get_phys_port_id on igbvf netdev as well.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  drivers/net/ethernet/intel/igb/e1000_mbx.h |  1 +
>  drivers/net/ethernet/intel/igb/igb.h       |  2 ++
>  drivers/net/ethernet/intel/igb/igb_main.c  | 29
> +++++++++++++++++++++-
>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  3 +++
>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
>  drivers/net/ethernet/intel/igbvf/netdev.c  | 40
> ++++++++++++++++++++++++++++++
>  drivers/net/ethernet/intel/igbvf/vf.c      | 34
> +++++++++++++++++++++++++
>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
>  8 files changed, 110 insertions(+), 1 deletion(-) 

Added this patch (actually the entire series) for testing.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch net-next v5 4/4] igb/igbvf: implement ndo_get_phys_port_id

[Ben Hutchings]

On Fri, 2013-07-26 at 14:09 +0200, Jiri Pirko wrote:
> igb driver generated random number which will identify physical port.
> This id is available via ndo_get_phys_port_id directly on igb netdev.
> Also, id is passed to igbvf using mailbox. After that, it is available via
> ndo_get_phys_port_id on igbvf netdev as well.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  drivers/net/ethernet/intel/igb/e1000_mbx.h |  1 +
>  drivers/net/ethernet/intel/igb/igb.h       |  2 ++
>  drivers/net/ethernet/intel/igb/igb_main.c  | 29 +++++++++++++++++++++-
>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  3 +++
>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
>  drivers/net/ethernet/intel/igbvf/netdev.c  | 40 ++++++++++++++++++++++++++++++
>  drivers/net/ethernet/intel/igbvf/vf.c      | 34 +++++++++++++++++++++++++
>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
>  8 files changed, 110 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/e1000_mbx.h b/drivers/net/ethernet/intel/igb/e1000_mbx.h
> index de9bba4..1788480 100644
> --- a/drivers/net/ethernet/intel/igb/e1000_mbx.h
> +++ b/drivers/net/ethernet/intel/igb/e1000_mbx.h
> @@ -64,6 +64,7 @@
>  #define E1000_VF_SET_LPE	0x05 /* VF requests to set VMOLR.LPE */
>  #define E1000_VF_SET_PROMISC	0x06 /*VF requests to clear VMOLR.ROPE/MPME*/
>  #define E1000_VF_SET_PROMISC_MULTICAST	(0x02 << E1000_VT_MSGINFO_SHIFT)
> +#define E1000_VF_GET_PHYS_PORT_ID 0x07 /* VF requests to get physical port id */
>  
>  #define E1000_PF_CONTROL_MSG	0x0100 /* PF control message */
>  
> diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
> index 15ea8dc..12a25b1 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -444,6 +444,8 @@ struct igb_adapter {
>  	struct i2c_algo_bit_data i2c_algo;
>  	struct i2c_adapter i2c_adap;
>  	struct i2c_client *i2c_client;
> +
> +	u32 phys_port_id;
[...]

If we're going to pick these IDs at random then 32 bits is fine for a
locally unique ID but not so much for a universally unique ID.  If we
want universal uniqueness (which is what you've specified) then I think
at least 64 bits are required.  Unless there's some constraint on length
then I would suggest using uuid_be_gen() in at least the single-PF case.

I had assumed that a stable ID was useful, but maybe it doesn't matter
(and I can see that it might sometimes be desirable to hide that from a
VM).

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 v5 4/4] igb/igbvf: implement ndo_get_phys_port_id

[Jiri Pirko]

Fri, Jul 26, 2013 at 05:09:47PM CEST, bhutchings@solarflare.com wrote:
>On Fri, 2013-07-26 at 14:09 +0200, Jiri Pirko wrote:
>> igb driver generated random number which will identify physical port.
>> This id is available via ndo_get_phys_port_id directly on igb netdev.
>> Also, id is passed to igbvf using mailbox. After that, it is available via
>> ndo_get_phys_port_id on igbvf netdev as well.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  drivers/net/ethernet/intel/igb/e1000_mbx.h |  1 +
>>  drivers/net/ethernet/intel/igb/igb.h       |  2 ++
>>  drivers/net/ethernet/intel/igb/igb_main.c  | 29 +++++++++++++++++++++-
>>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  3 +++
>>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
>>  drivers/net/ethernet/intel/igbvf/netdev.c  | 40 ++++++++++++++++++++++++++++++
>>  drivers/net/ethernet/intel/igbvf/vf.c      | 34 +++++++++++++++++++++++++
>>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
>>  8 files changed, 110 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/igb/e1000_mbx.h b/drivers/net/ethernet/intel/igb/e1000_mbx.h
>> index de9bba4..1788480 100644
>> --- a/drivers/net/ethernet/intel/igb/e1000_mbx.h
>> +++ b/drivers/net/ethernet/intel/igb/e1000_mbx.h
>> @@ -64,6 +64,7 @@
>>  #define E1000_VF_SET_LPE	0x05 /* VF requests to set VMOLR.LPE */
>>  #define E1000_VF_SET_PROMISC	0x06 /*VF requests to clear VMOLR.ROPE/MPME*/
>>  #define E1000_VF_SET_PROMISC_MULTICAST	(0x02 << E1000_VT_MSGINFO_SHIFT)
>> +#define E1000_VF_GET_PHYS_PORT_ID 0x07 /* VF requests to get physical port id */
>>  
>>  #define E1000_PF_CONTROL_MSG	0x0100 /* PF control message */
>>  
>> diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
>> index 15ea8dc..12a25b1 100644
>> --- a/drivers/net/ethernet/intel/igb/igb.h
>> +++ b/drivers/net/ethernet/intel/igb/igb.h
>> @@ -444,6 +444,8 @@ struct igb_adapter {
>>  	struct i2c_algo_bit_data i2c_algo;
>>  	struct i2c_adapter i2c_adap;
>>  	struct i2c_client *i2c_client;
>> +
>> +	u32 phys_port_id;
>[...]
>
>If we're going to pick these IDs at random then 32 bits is fine for a
>locally unique ID but not so much for a universally unique ID.  If we
>want universal uniqueness (which is what you've specified) then I think
>at least 64 bits are required.  Unless there's some constraint on length
>then I would suggest using uuid_be_gen() in at least the single-PF case.

I believe that locally unique ID is enough. I'm not sure if universally
would be ever needed for this particular item. There is need to identify
the ports within one machine.

But sure, for generated ids, lets use uuid_be_gen()

+ For NPAR for example, the id cannot be generated but must be computed
from hw info by individual netdev instances. And I imagine that collisions
in that case are even more likely than for 32-bit random number.

>
>I had assumed that a stable ID was useful, but maybe it doesn't matter
>(and I can see that it might sometimes be desirable to hide that from a
>VM).
>
>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 v5 0/4] export device physical port id to userspace

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]

On Fri, 2013-07-26 at 14:09 +0200, Jiri Pirko wrote:
> 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 propagate
> the change to userspace.
> 
> v1->v2: as suggested by Ben, handle -EOPNOTSUPP in rtnl code (wrapped up ndo call)
> v2->v3: adjusted patch 1 commit message
> v3->v4: used "%phN" for sysfs printf as suggested by DaveM
>         added igb/igbvf implementation as requested by Or Gerlitz
> v4->v5: used prandom_u32 to generate id in igb_probe
> 	removed duplicate code in ibgvf_probe
> 	pushed dev_err string into one line in igbvf_refresh_ppid
> 
> Jiri Pirko (4):
>   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
>   igb/igbvf: implement ndo_get_phys_port_id
> 

Jiri-
I do not want to hold up the entire series, especially since three of
the four patches are not against our drivers, but I would like to have
these changes tested in our validation labs.

I will apply the series internally for testing.  In the meantime, if I
get an ACK from Carolyn Wyborny and Greg Rose on the igb/igbvf patch, I
am good with Dave applying the series.

>  drivers/net/ethernet/intel/igb/e1000_mbx.h |  1 +
>  drivers/net/ethernet/intel/igb/igb.h       |  2 ++
>  drivers/net/ethernet/intel/igb/igb_main.c  | 29 +++++++++++++++++++++-
>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  3 +++
>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
>  drivers/net/ethernet/intel/igbvf/netdev.c  | 40 ++++++++++++++++++++++++++++++
>  drivers/net/ethernet/intel/igbvf/vf.c      | 34 +++++++++++++++++++++++++
>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
>  include/linux/netdevice.h                  | 20 +++++++++++++++
>  include/uapi/linux/if_link.h               |  1 +
>  net/core/dev.c                             | 18 ++++++++++++++
>  net/core/net-sysfs.c                       | 22 ++++++++++++++++
>  net/core/rtnetlink.c                       | 25 ++++++++++++++++++-
>  13 files changed, 195 insertions(+), 2 deletions(-)


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch net-next v5 0/4] export device physical port id to userspace

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 26 Jul 2013 14:09:52 +0200

> 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 propagate
> the change to userspace.

I'm generally fine with these patches, but some things still need to be
done.

First of all, I want a real comprehensive description of this patch
series in the "0/4" posting.

It should answer, in detail, the following questions:

1) Why do we need this, and why do existing facilities fail to provide
   a way to accomplish this?

2) Why is the physical port ID defined as a 32 byte opaque cookie?
   What formats and layouts need to be accomodated, and which
   influenced the design of the ID?

3) Are IDs globally unique?  Why or why not?  If IDs should be
   globally unique, but only in certain cases, what exactly are those
   cases.

And please address Ben's concerns about global uniqueness in the
igb/igbvf changes, and whether a 32-bit value is sufficient.

Thanks.

Re: [patch net-next v5 0/4] export device physical port id to userspace

[Jiri Pirko]

Sun, Jul 28, 2013 at 10:15:31PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri, 26 Jul 2013 14:09:52 +0200
>
>> 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 propagate
>> the change to userspace.
>
>I'm generally fine with these patches, but some things still need to be
>done.
>
>First of all, I want a real comprehensive description of this patch
>series in the "0/4" posting.
>
>It should answer, in detail, the following questions:
>
>1) Why do we need this, and why do existing facilities fail to provide
>   a way to accomplish this?
>
>2) Why is the physical port ID defined as a 32 byte opaque cookie?
>   What formats and layouts need to be accomodated, and which
>   influenced the design of the ID?
>
>3) Are IDs globally unique?  Why or why not?  If IDs should be
>   globally unique, but only in certain cases, what exactly are those
>   cases.
>
>And please address Ben's concerns about global uniqueness in the
>igb/igbvf changes, and whether a 32-bit value is sufficient.

Will do. Thanks Dave

>
>Thanks.