netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch net-next v6 0/4] export device physical port id to userspace
@ 2013-07-29 16:16 Jiri Pirko
  2013-07-29 16:16 ` [patch net-next v6 1/4] net: add ndo to get id of physical port of the device Jiri Pirko
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Jiri Pirko @ 2013-07-29 16:16 UTC (permalink / raw)
  To: netdev
  Cc: davem, stephen, Narendra_K, bhutchings, john.r.fastabend,
	or.gerlitz, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
	peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
	tushar.n.dave, matthew.vick, mitch.a.williams, vyasevic, amwang,
	johannes

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
v5->v6: use uuid_le_gen for generating 16-byte phys port id for igb/igbvf
	as suggested by BenH

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

Currenty there's very hard to tell if two netdevs are using the same physical
port. For sr-iov this can be get by sysfs. For other mechanisms, like NPAR
there's very hard to do it (one must learn it from NIC BIOS). But even for
sr-iov there's no way to say if two netdevs are using the same phys port when
these are passed through to virtual guests.

This patchset provides the generic way of letting this information know to
userspace. This info can be used by apps like NetworkManager, teamd, Wicked,
ovs daemon, etc, to do smarter bonding decisions.

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?

For user to distinguish if two netdevs are using the same port, he only needs
to compare their phys port ids. Nothing else is needed. This id has no
structure for security reasons. VF should not know anything about PF.

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.

Most of the time only uniqueness needed is in scope of single machine.
There might be case when the id should be unique between couple of machines
in virtualization environment. Given that for example for igb/igbvf 16B uuid
is used, there is no problem for this case as well. But each driver can
implement this differently focusing the hw capabilities and needs.

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       |  3 +++
 drivers/net/ethernet/intel/igb/igb_main.c  | 37 ++++++++++++++++++++++++++++-
 drivers/net/ethernet/intel/igbvf/igbvf.h   |  4 ++++
 drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
 drivers/net/ethernet/intel/igbvf/netdev.c  | 38 ++++++++++++++++++++++++++++++
 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, 203 insertions(+), 2 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [patch net-next v6 1/4] net: add ndo to get id of physical port of the device
  2013-07-29 16:16 [patch net-next v6 0/4] export device physical port id to userspace Jiri Pirko
@ 2013-07-29 16:16 ` Jiri Pirko
  2013-07-29 16:16 ` [patch net-next v6 2/4] rtnl: export physical port id via RT netlink Jiri Pirko
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2013-07-29 16:16 UTC (permalink / raw)
  To: netdev
  Cc: davem, stephen, Narendra_K, bhutchings, john.r.fastabend,
	or.gerlitz, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
	peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
	tushar.n.dave, matthew.vick, mitch.a.williams, vyasevic, amwang,
	johannes

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next v6 2/4] rtnl: export physical port id via RT netlink
  2013-07-29 16:16 [patch net-next v6 0/4] export device physical port id to userspace Jiri Pirko
  2013-07-29 16:16 ` [patch net-next v6 1/4] net: add ndo to get id of physical port of the device Jiri Pirko
@ 2013-07-29 16:16 ` Jiri Pirko
  2013-07-29 16:16 ` [patch net-next v6 3/4] net: export physical port id via sysfs Jiri Pirko
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2013-07-29 16:16 UTC (permalink / raw)
  To: netdev
  Cc: davem, stephen, Narendra_K, bhutchings, john.r.fastabend,
	or.gerlitz, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
	peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
	tushar.n.dave, matthew.vick, mitch.a.williams, vyasevic, amwang,
	johannes, Narendra K

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next v6 3/4] net: export physical port id via sysfs
  2013-07-29 16:16 [patch net-next v6 0/4] export device physical port id to userspace Jiri Pirko
  2013-07-29 16:16 ` [patch net-next v6 1/4] net: add ndo to get id of physical port of the device Jiri Pirko
  2013-07-29 16:16 ` [patch net-next v6 2/4] rtnl: export physical port id via RT netlink Jiri Pirko
@ 2013-07-29 16:16 ` Jiri Pirko
  2013-07-29 16:16 ` [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id Jiri Pirko
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2013-07-29 16:16 UTC (permalink / raw)
  To: netdev
  Cc: davem, stephen, Narendra_K, bhutchings, john.r.fastabend,
	or.gerlitz, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
	peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
	tushar.n.dave, matthew.vick, mitch.a.williams, vyasevic, amwang,
	johannes, Narendra K

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id
  2013-07-29 16:16 [patch net-next v6 0/4] export device physical port id to userspace Jiri Pirko
                   ` (2 preceding siblings ...)
  2013-07-29 16:16 ` [patch net-next v6 3/4] net: export physical port id via sysfs Jiri Pirko
@ 2013-07-29 16:16 ` Jiri Pirko
  2013-08-22 10:39   ` Jeff Kirsher
  2013-07-29 22:23 ` [patch net-next v6 0/4] export device physical port id to userspace Jeff Kirsher
  2013-07-31  0:36 ` David Miller
  5 siblings, 1 reply; 13+ messages in thread
From: Jiri Pirko @ 2013-07-29 16:16 UTC (permalink / raw)
  To: netdev
  Cc: davem, stephen, Narendra_K, bhutchings, john.r.fastabend,
	or.gerlitz, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
	peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
	tushar.n.dave, matthew.vick, mitch.a.williams, vyasevic, amwang,
	johannes

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       |  3 +++
 drivers/net/ethernet/intel/igb/igb_main.c  | 37 ++++++++++++++++++++++++++++-
 drivers/net/ethernet/intel/igbvf/igbvf.h   |  4 ++++
 drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
 drivers/net/ethernet/intel/igbvf/netdev.c  | 38 ++++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/igbvf/vf.c      | 34 ++++++++++++++++++++++++++
 drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
 8 files changed, 118 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..fcbb1c7 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -444,6 +444,9 @@ struct igb_adapter {
 	struct i2c_algo_bit_data i2c_algo;
 	struct i2c_adapter i2c_adap;
 	struct i2c_client *i2c_client;
+
+#define IGB_PHYS_PORT_ID_LEN	16
+	u8 phys_port_id[IGB_PHYS_PORT_ID_LEN];
 };
 
 #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..8015e9f 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/uuid.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,
 };
 
 /**
@@ -1982,6 +2002,14 @@ static s32 igb_init_i2c(struct igb_adapter *adapter)
 	return status;
 }
 
+void igb_gen_phys_port_id(struct igb_adapter *adapter)
+{
+	uuid_le uuid;
+
+	uuid_le_gen(&uuid);
+	memcpy(adapter->phys_port_id, uuid.b, sizeof(adapter->phys_port_id));
+}
+
 /**
  *  igb_probe - Device Initialization Routine
  *  @pdev: PCI device information struct
@@ -2287,6 +2315,8 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * driver. */
 	igb_get_hw_control(adapter);
 
+	igb_gen_phys_port_id(adapter);
+
 	strcpy(netdev->name, "eth%d");
 	err = register_netdev(netdev);
 	if (err)
@@ -5698,6 +5728,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 +5788,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:
+		memcpy(&msgbuf[1], adapter->phys_port_id, IGB_PHYS_PORT_ID_LEN);
+		write_size += IGB_PHYS_PORT_ID_LEN / 4;
+		break;
 	default:
 		dev_err(&pdev->dev, "Unhandled Msg %08x\n", msgbuf[0]);
 		retval = -1;
@@ -5771,7 +5806,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..27ddbb0 100644
--- a/drivers/net/ethernet/intel/igbvf/igbvf.h
+++ b/drivers/net/ethernet/intel/igbvf/igbvf.h
@@ -283,6 +283,10 @@ struct igbvf_adapter {
 
 	unsigned int flags;
 	unsigned long last_reset;
+
+#define IGBVF_PHYS_PORT_ID_LEN	16
+	u8 phys_port_id[IGBVF_PHYS_PORT_ID_LEN];
+	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..46e4d16 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -1444,6 +1444,18 @@ 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;
+	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 +1473,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 +1767,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 +2645,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 +2795,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..291175d 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, u8 *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, u8 *id)
+{
+	struct e1000_mbx_info *mbx = &hw->mbx;
+	u32 msgbuf[5];
+	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, 5);
+	if (ret_val)
+		goto out;
+
+	if (msgbuf[0] & E1000_VT_MSGTYPE_NACK) {
+		ret_val = -E1000_ERR_MAC_INIT;
+		goto out;
+	}
+	memcpy(id, &msgbuf[1], 16);
+
+out:
+	return ret_val;
+}
diff --git a/drivers/net/ethernet/intel/igbvf/vf.h b/drivers/net/ethernet/intel/igbvf/vf.h
index 57db3c6..2b690bb 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 *, u8 *);
 };
 
 struct e1000_mac_info {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [patch net-next v6 0/4] export device physical port id to userspace
  2013-07-29 16:16 [patch net-next v6 0/4] export device physical port id to userspace Jiri Pirko
                   ` (3 preceding siblings ...)
  2013-07-29 16:16 ` [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id Jiri Pirko
@ 2013-07-29 22:23 ` Jeff Kirsher
  2013-07-31  0:36 ` David Miller
  5 siblings, 0 replies; 13+ messages in thread
From: Jeff Kirsher @ 2013-07-29 22:23 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, stephen, Narendra_K, bhutchings, john.r.fastabend,
	or.gerlitz, jesse.brandeburg, bruce.w.allan, carolyn.wyborny,
	donald.c.skidmore, gregory.v.rose, peter.p.waskiewicz.jr,
	alexander.h.duyck, john.ronciak, tushar.n.dave, matthew.vick,
	mitch.a.williams, vyasevic, amwang, johannes

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

On Mon, 2013-07-29 at 18:16 +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
> v5->v6: use uuid_le_gen for generating 16-byte phys port id for
> igb/igbvf
>         as suggested by BenH
> 
> 1) Why do we need this, and why do existing facilities fail to provide
>    a way to accomplish this?
> 
> Currenty there's very hard to tell if two netdevs are using the same
> physical
> port. For sr-iov this can be get by sysfs. For other mechanisms, like
> NPAR
> there's very hard to do it (one must learn it from NIC BIOS). But even
> for
> sr-iov there's no way to say if two netdevs are using the same phys
> port when
> these are passed through to virtual guests.
> 
> This patchset provides the generic way of letting this information
> know to
> userspace. This info can be used by apps like NetworkManager, teamd,
> Wicked,
> ovs daemon, etc, to do smarter bonding decisions.
> 
> 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?
> 
> For user to distinguish if two netdevs are using the same port, he
> only needs
> to compare their phys port ids. Nothing else is needed. This id has no
> structure for security reasons. VF should not know anything about PF.
> 
> 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.
> 
> Most of the time only uniqueness needed is in scope of single machine.
> There might be case when the id should be unique between couple of
> machines
> in virtualization environment. Given that for example for igb/igbvf
> 16B uuid
> is used, there is no problem for this case as well. But each driver
> can
> implement this differently focusing the hw capabilities and needs.
> 
> 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 

As before, I have applied this patch series so that we can get some
validation done on patch 4.  I do not want to hold up the patch series,
if Ben/Dave are fine with the recent changes, although I would like to
either get an ACK from Greg Rose or a thumbs up from our testers.

Thanks Jiri!

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch net-next v6 0/4] export device physical port id to userspace
  2013-07-29 16:16 [patch net-next v6 0/4] export device physical port id to userspace Jiri Pirko
                   ` (4 preceding siblings ...)
  2013-07-29 22:23 ` [patch net-next v6 0/4] export device physical port id to userspace Jeff Kirsher
@ 2013-07-31  0:36 ` David Miller
  5 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2013-07-31  0:36 UTC (permalink / raw)
  To: jiri
  Cc: netdev, stephen, Narendra_K, bhutchings, john.r.fastabend,
	or.gerlitz, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
	peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
	tushar.n.dave, matthew.vick, mitch.a.williams, vyasevic, amwang,
	johannes

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 29 Jul 2013 18:16:48 +0200

> 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

I applied all of the infrastructure patches (#1, #2, and #3) to net-next.

I'll let Jeff Kirsher pick up the Intel driver bits.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id
  2013-07-29 16:16 ` [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id Jiri Pirko
@ 2013-08-22 10:39   ` Jeff Kirsher
  2013-08-22 13:10     ` Jiri Pirko
  0 siblings, 1 reply; 13+ messages in thread
From: Jeff Kirsher @ 2013-08-22 10:39 UTC (permalink / raw)
  To: Jiri Pirko, aaron.f.brown
  Cc: netdev, davem, stephen, Narendra_K, bhutchings, or.gerlitz,
	carolyn.wyborny, gregory.v.rose, vyasevic, amwang, johannes

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

On Mon, 2013-07-29 at 18:16 +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       |  3 +++
>  drivers/net/ethernet/intel/igb/igb_main.c  | 37
> ++++++++++++++++++++++++++++-
>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  4 ++++
>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
>  drivers/net/ethernet/intel/igbvf/netdev.c  | 38
> ++++++++++++++++++++++++++++++
>  drivers/net/ethernet/intel/igbvf/vf.c      | 34
> ++++++++++++++++++++++++++
>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
>  8 files changed, 118 insertions(+), 1 deletion(-)

Jiri-

Validation ran into a couple of issues with this patch.  Here is what
Aaron found when testing this patch...

Aaron Brown wrote:
I think I have to fail this, it seems to have an issue with
initialization.  When I first create a vf via sysfs the pys_port_id file
is created along with the other sysfs files for the vf, however an
attempt to cat out the value returns " Operation not supported".  At
this point the vf is still down, if I bring it up (or simply unload /
reload the igbvf driver) I can then cat the file successfully and the vf
interface phys_port_id matches the phys_port_id of the pf.  This is
testing from bare metal, a console session showing this behavior
follows:

u1304:[0]/sys> find . -iname phys_port_id
./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
./devices/virtual/net/sit0/phys_port_id
./devices/virtual/net/lo/phys_port_id
u1304:[0]/sys> cat devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
5ece9fbd9cd51546982e15c1f2c11e25
u1304:[0]/sys>

So far so good, now make a few vfs and check for new phys_port_id sysfs files.

u1304:[0]/sys> find . -iname sriov_numvfs
./devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
./devices/pci0000:00/0000:00:01.0/0000:07:00.1/sriov_numvfs
u1304:[0]/sys> echo 2 > devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
u1304:[0]/sys> find . -iname phys_port_id                                       ./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
./devices/pci0000:00/0000:00:01.0/0000:07:10.2/net/eth3/phys_port_id
./devices/virtual/net/sit0/phys_port_id
./devices/virtual/net/lo/phys_port_id
u1304:[0]/sys>

The first vf is eth2, attempt to cat out it's phys_port_id

u1304:[0]/sys> cat ./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
cat: ./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id: Operation not supported
u1304:[0]/sys>

But, if I bring the interface up (or unload / load the igbvf driver) I then am able to cat the phys_port_id of the vf and it matches the phys_port_id of the physical interface.

u1304:[0]/sys> ifconfig eth2 u1304-2
u1304:[0]/sys> cat ./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
5ece9fbd9cd51546982e15c1f2c11e25
u1304:[0]/sys>

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id
  2013-08-22 10:39   ` Jeff Kirsher
@ 2013-08-22 13:10     ` Jiri Pirko
  2013-08-28  2:26       ` Brown, Aaron F
  0 siblings, 1 reply; 13+ messages in thread
From: Jiri Pirko @ 2013-08-22 13:10 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: aaron.f.brown, netdev, davem, stephen, Narendra_K, bhutchings,
	or.gerlitz, carolyn.wyborny, gregory.v.rose, vyasevic, amwang,
	johannes

Thu, Aug 22, 2013 at 12:39:10PM CEST, jeffrey.t.kirsher@intel.com wrote:
>On Mon, 2013-07-29 at 18:16 +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       |  3 +++
>>  drivers/net/ethernet/intel/igb/igb_main.c  | 37
>> ++++++++++++++++++++++++++++-
>>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  4 ++++
>>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
>>  drivers/net/ethernet/intel/igbvf/netdev.c  | 38
>> ++++++++++++++++++++++++++++++
>>  drivers/net/ethernet/intel/igbvf/vf.c      | 34
>> ++++++++++++++++++++++++++
>>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
>>  8 files changed, 118 insertions(+), 1 deletion(-)
>
>Jiri-
>
>Validation ran into a couple of issues with this patch.  Here is what
>Aaron found when testing this patch...

Interesting. So since igbvf_refresh_ppid() is called from igbvf_reset()
and igbvf_probe(), I believed that is enough. Looks like perm_addr
getting works in similar way. Can you please check if perm_addr is set
correctly in the same time reading phys_port_id gives -EOPNOTSUPP?


>
>Aaron Brown wrote:
>I think I have to fail this, it seems to have an issue with
>initialization.  When I first create a vf via sysfs the pys_port_id file
>is created along with the other sysfs files for the vf, however an
>attempt to cat out the value returns " Operation not supported".  At
>this point the vf is still down, if I bring it up (or simply unload /
>reload the igbvf driver) I can then cat the file successfully and the vf
>interface phys_port_id matches the phys_port_id of the pf.  This is
>testing from bare metal, a console session showing this behavior
>follows:
>
>u1304:[0]/sys> find . -iname phys_port_id
>./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
>./devices/virtual/net/sit0/phys_port_id
>./devices/virtual/net/lo/phys_port_id
>u1304:[0]/sys> cat devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>5ece9fbd9cd51546982e15c1f2c11e25
>u1304:[0]/sys>
>
>So far so good, now make a few vfs and check for new phys_port_id sysfs files.
>
>u1304:[0]/sys> find . -iname sriov_numvfs
>./devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
>./devices/pci0000:00/0000:00:01.0/0000:07:00.1/sriov_numvfs
>u1304:[0]/sys> echo 2 > devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
>u1304:[0]/sys> find . -iname phys_port_id                                       ./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
>./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>./devices/pci0000:00/0000:00:01.0/0000:07:10.2/net/eth3/phys_port_id
>./devices/virtual/net/sit0/phys_port_id
>./devices/virtual/net/lo/phys_port_id
>u1304:[0]/sys>
>
>The first vf is eth2, attempt to cat out it's phys_port_id
>
>u1304:[0]/sys> cat ./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>cat: ./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id: Operation not supported
>u1304:[0]/sys>
>
>But, if I bring the interface up (or unload / load the igbvf driver) I then am able to cat the phys_port_id of the vf and it matches the phys_port_id of the physical interface.
>
>u1304:[0]/sys> ifconfig eth2 u1304-2
>u1304:[0]/sys> cat ./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>5ece9fbd9cd51546982e15c1f2c11e25
>u1304:[0]/sys>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id
  2013-08-22 13:10     ` Jiri Pirko
@ 2013-08-28  2:26       ` Brown, Aaron F
  2013-08-28  6:05         ` Jiri Pirko
  0 siblings, 1 reply; 13+ messages in thread
From: Brown, Aaron F @ 2013-08-28  2:26 UTC (permalink / raw)
  To: Jiri Pirko, Kirsher, Jeffrey T
  Cc: netdev, davem, stephen, Narendra_K, bhutchings, or.gerlitz,
	Wyborny, Carolyn, Rose, Gregory V, vyasevic, amwang, johannes

Sorry, I was out sick towards the end of last week and playing catch up for the last few days...  Info inline.

> From: Jiri Pirko [mailto:jiri@resnulli.us]
> Sent: Thursday, August 22, 2013 6:10 AM
> To: Kirsher, Jeffrey T
> Cc: Brown, Aaron F; netdev@vger.kernel.org; davem@davemloft.net;
> stephen@networkplumber.org; Narendra_K@Dell.com;
> bhutchings@solarflare.com; or.gerlitz@gmail.com; Wyborny, Carolyn; Rose,
> Gregory V; vyasevic@redhat.com; amwang@redhat.com;
> johannes@sipsolutions.net
> Subject: Re: [patch net-next v6 4/4] igb/igbvf: implement
> ndo_get_phys_port_id
> 
> Thu, Aug 22, 2013 at 12:39:10PM CEST, jeffrey.t.kirsher@intel.com wrote:
> >On Mon, 2013-07-29 at 18:16 +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       |  3 +++
> >>  drivers/net/ethernet/intel/igb/igb_main.c  | 37
> >> ++++++++++++++++++++++++++++-
> >>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  4 ++++
> >>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
> >>  drivers/net/ethernet/intel/igbvf/netdev.c  | 38
> >> ++++++++++++++++++++++++++++++
> >>  drivers/net/ethernet/intel/igbvf/vf.c      | 34
> >> ++++++++++++++++++++++++++
> >>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
> >>  8 files changed, 118 insertions(+), 1 deletion(-)
> >
> >Jiri-
> >
> >Validation ran into a couple of issues with this patch.  Here is what
> >Aaron found when testing this patch...
> 
> Interesting. So since igbvf_refresh_ppid() is called from igbvf_reset()
> and igbvf_probe(), I believed that is enough. Looks like perm_addr getting
> works in similar way. Can you please check if perm_addr is set correctly
> in the same time reading phys_port_id gives -EOPNOTSUPP?

By perm_addr do you mean the MAC Address of the vf?  Yes, it is set up (and I can view it vi ip link or sysconfig) when I see the op not supported message (after I fi vfs via sysfs, before bringing the vf interface up or reloading igbvf.)

> 
> 
> >
> >Aaron Brown wrote:
> >I think I have to fail this, it seems to have an issue with
> >initialization.  When I first create a vf via sysfs the pys_port_id
> >file is created along with the other sysfs files for the vf, however an
> >attempt to cat out the value returns " Operation not supported".  At
> >this point the vf is still down, if I bring it up (or simply unload /
> >reload the igbvf driver) I can then cat the file successfully and the
> >vf interface phys_port_id matches the phys_port_id of the pf.  This is
> >testing from bare metal, a console session showing this behavior
> >follows:
> >
> >u1304:[0]/sys> find . -iname phys_port_id
> >./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
> >./devices/virtual/net/sit0/phys_port_id
> >./devices/virtual/net/lo/phys_port_id
> >u1304:[0]/sys> cat
> >devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
> >5ece9fbd9cd51546982e15c1f2c11e25
> >u1304:[0]/sys>
> >
> >So far so good, now make a few vfs and check for new phys_port_id sysfs
> files.
> >
> >u1304:[0]/sys> find . -iname sriov_numvfs
> >./devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/sriov_numvfs
> >u1304:[0]/sys> echo 2 >
> devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
> >u1304:[0]/sys> find . -iname phys_port_id
> ./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
> >./devices/pci0000:00/0000:00:01.0/0000:07:10.2/net/eth3/phys_port_id
> >./devices/virtual/net/sit0/phys_port_id
> >./devices/virtual/net/lo/phys_port_id
> >u1304:[0]/sys>
> >
> >The first vf is eth2, attempt to cat out it's phys_port_id
> >
> >u1304:[0]/sys> cat
> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
> >cat:
> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id:
> >Operation not supported u1304:[0]/sys>
> >
> >But, if I bring the interface up (or unload / load the igbvf driver) I
> then am able to cat the phys_port_id of the vf and it matches the
> phys_port_id of the physical interface.
> >
> >u1304:[0]/sys> ifconfig eth2 u1304-2
> >u1304:[0]/sys> cat
> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
> >5ece9fbd9cd51546982e15c1f2c11e25
> >u1304:[0]/sys>
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id
  2013-08-28  2:26       ` Brown, Aaron F
@ 2013-08-28  6:05         ` Jiri Pirko
  2013-08-28 19:06           ` Brown, Aaron F
  0 siblings, 1 reply; 13+ messages in thread
From: Jiri Pirko @ 2013-08-28  6:05 UTC (permalink / raw)
  To: Brown, Aaron F
  Cc: Kirsher, Jeffrey T, netdev, davem, stephen, Narendra_K,
	bhutchings, or.gerlitz, Wyborny, Carolyn, Rose, Gregory V,
	vyasevic, amwang, johannes

Wed, Aug 28, 2013 at 04:26:28AM CEST, aaron.f.brown@intel.com wrote:
>Sorry, I was out sick towards the end of last week and playing catch up for the last few days...  Info inline.
>
>> From: Jiri Pirko [mailto:jiri@resnulli.us]
>> Sent: Thursday, August 22, 2013 6:10 AM
>> To: Kirsher, Jeffrey T
>> Cc: Brown, Aaron F; netdev@vger.kernel.org; davem@davemloft.net;
>> stephen@networkplumber.org; Narendra_K@Dell.com;
>> bhutchings@solarflare.com; or.gerlitz@gmail.com; Wyborny, Carolyn; Rose,
>> Gregory V; vyasevic@redhat.com; amwang@redhat.com;
>> johannes@sipsolutions.net
>> Subject: Re: [patch net-next v6 4/4] igb/igbvf: implement
>> ndo_get_phys_port_id
>> 
>> Thu, Aug 22, 2013 at 12:39:10PM CEST, jeffrey.t.kirsher@intel.com wrote:
>> >On Mon, 2013-07-29 at 18:16 +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       |  3 +++
>> >>  drivers/net/ethernet/intel/igb/igb_main.c  | 37
>> >> ++++++++++++++++++++++++++++-
>> >>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  4 ++++
>> >>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
>> >>  drivers/net/ethernet/intel/igbvf/netdev.c  | 38
>> >> ++++++++++++++++++++++++++++++
>> >>  drivers/net/ethernet/intel/igbvf/vf.c      | 34
>> >> ++++++++++++++++++++++++++
>> >>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
>> >>  8 files changed, 118 insertions(+), 1 deletion(-)
>> >
>> >Jiri-
>> >
>> >Validation ran into a couple of issues with this patch.  Here is what
>> >Aaron found when testing this patch...
>> 
>> Interesting. So since igbvf_refresh_ppid() is called from igbvf_reset()
>> and igbvf_probe(), I believed that is enough. Looks like perm_addr getting
>> works in similar way. Can you please check if perm_addr is set correctly
>> in the same time reading phys_port_id gives -EOPNOTSUPP?
>
>By perm_addr do you mean the MAC Address of the vf?  Yes, it is set up (and I can view it vi ip link or sysconfig) when I see the op not supported message (after I fi vfs via sysfs, before bringing the vf interface up or reloading igbvf.)

Can you please send me dmesg log from the system you are testing this?

Looking at the code, it looks like whenever mac.ops.reset_hw() (which
sets up mac) is called, igbvf_refresh_ppid() is called as well.

Thanks

>
>> 
>> 
>> >
>> >Aaron Brown wrote:
>> >I think I have to fail this, it seems to have an issue with
>> >initialization.  When I first create a vf via sysfs the pys_port_id
>> >file is created along with the other sysfs files for the vf, however an
>> >attempt to cat out the value returns " Operation not supported".  At
>> >this point the vf is still down, if I bring it up (or simply unload /
>> >reload the igbvf driver) I can then cat the file successfully and the
>> >vf interface phys_port_id matches the phys_port_id of the pf.  This is
>> >testing from bare metal, a console session showing this behavior
>> >follows:
>> >
>> >u1304:[0]/sys> find . -iname phys_port_id
>> >./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
>> >./devices/virtual/net/sit0/phys_port_id
>> >./devices/virtual/net/lo/phys_port_id
>> >u1304:[0]/sys> cat
>> >devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>> >5ece9fbd9cd51546982e15c1f2c11e25
>> >u1304:[0]/sys>
>> >
>> >So far so good, now make a few vfs and check for new phys_port_id sysfs
>> files.
>> >
>> >u1304:[0]/sys> find . -iname sriov_numvfs
>> >./devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
>> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/sriov_numvfs
>> >u1304:[0]/sys> echo 2 >
>> devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
>> >u1304:[0]/sys> find . -iname phys_port_id
>> ./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
>> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>> >./devices/pci0000:00/0000:00:01.0/0000:07:10.2/net/eth3/phys_port_id
>> >./devices/virtual/net/sit0/phys_port_id
>> >./devices/virtual/net/lo/phys_port_id
>> >u1304:[0]/sys>
>> >
>> >The first vf is eth2, attempt to cat out it's phys_port_id
>> >
>> >u1304:[0]/sys> cat
>> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>> >cat:
>> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id:
>> >Operation not supported u1304:[0]/sys>
>> >
>> >But, if I bring the interface up (or unload / load the igbvf driver) I
>> then am able to cat the phys_port_id of the vf and it matches the
>> phys_port_id of the physical interface.
>> >
>> >u1304:[0]/sys> ifconfig eth2 u1304-2
>> >u1304:[0]/sys> cat
>> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>> >5ece9fbd9cd51546982e15c1f2c11e25
>> >u1304:[0]/sys>
>> 
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id
  2013-08-28  6:05         ` Jiri Pirko
@ 2013-08-28 19:06           ` Brown, Aaron F
  2014-02-21  8:02             ` Jiri Pirko
  0 siblings, 1 reply; 13+ messages in thread
From: Brown, Aaron F @ 2013-08-28 19:06 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Kirsher, Jeffrey T, netdev, davem, stephen, Narendra_K,
	bhutchings, or.gerlitz, Wyborny, Carolyn, Rose, Gregory V,
	vyasevic, amwang, johannes

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



> -----Original Message-----
> From: Jiri Pirko [mailto:jiri@resnulli.us]
> Sent: Tuesday, August 27, 2013 11:06 PM
> To: Brown, Aaron F
> Cc: Kirsher, Jeffrey T; netdev@vger.kernel.org; davem@davemloft.net;
> stephen@networkplumber.org; Narendra_K@Dell.com;
> bhutchings@solarflare.com; or.gerlitz@gmail.com; Wyborny, Carolyn; Rose,
> Gregory V; vyasevic@redhat.com; amwang@redhat.com;
> johannes@sipsolutions.net
> Subject: Re: [patch net-next v6 4/4] igb/igbvf: implement
> ndo_get_phys_port_id
> 
> Wed, Aug 28, 2013 at 04:26:28AM CEST, aaron.f.brown@intel.com wrote:
> >Sorry, I was out sick towards the end of last week and playing catch up
> for the last few days...  Info inline.
> >
> >> From: Jiri Pirko [mailto:jiri@resnulli.us]
> >> Sent: Thursday, August 22, 2013 6:10 AM
> >> To: Kirsher, Jeffrey T
> >> Cc: Brown, Aaron F; netdev@vger.kernel.org; davem@davemloft.net;
> >> stephen@networkplumber.org; Narendra_K@Dell.com;
> >> bhutchings@solarflare.com; or.gerlitz@gmail.com; Wyborny, Carolyn;
> >> Rose, Gregory V; vyasevic@redhat.com; amwang@redhat.com;
> >> johannes@sipsolutions.net
> >> Subject: Re: [patch net-next v6 4/4] igb/igbvf: implement
> >> ndo_get_phys_port_id
> >>
> >> Thu, Aug 22, 2013 at 12:39:10PM CEST, jeffrey.t.kirsher@intel.com
> wrote:
> >> >On Mon, 2013-07-29 at 18:16 +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       |  3 +++
> >> >>  drivers/net/ethernet/intel/igb/igb_main.c  | 37
> >> >> ++++++++++++++++++++++++++++-
> >> >>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  4 ++++
> >> >>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
> >> >>  drivers/net/ethernet/intel/igbvf/netdev.c  | 38
> >> >> ++++++++++++++++++++++++++++++
> >> >>  drivers/net/ethernet/intel/igbvf/vf.c      | 34
> >> >> ++++++++++++++++++++++++++
> >> >>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
> >> >>  8 files changed, 118 insertions(+), 1 deletion(-)
> >> >
> >> >Jiri-
> >> >
> >> >Validation ran into a couple of issues with this patch.  Here is
> >> >what Aaron found when testing this patch...
> >>
> >> Interesting. So since igbvf_refresh_ppid() is called from
> >> igbvf_reset() and igbvf_probe(), I believed that is enough. Looks
> >> like perm_addr getting works in similar way. Can you please check if
> >> perm_addr is set correctly in the same time reading phys_port_id gives
> -EOPNOTSUPP?
> >
> >By perm_addr do you mean the MAC Address of the vf?  Yes, it is set up
> >(and I can view it vi ip link or sysconfig) when I see the op not
> >supported message (after I fi vfs via sysfs, before bringing the vf
> >interface up or reloading igbvf.)
> 
> Can you please send me dmesg log from the system you are testing this?

Attached, dmesg dump from a fresh boot to the eopnosupp. I've been using a .config with a whole bunch of debug junk in it and this is putting out tons of kobject messages, I can re-build and send something with less junk if it's pushing anything relevant off the top.

> 
> Looking at the code, it looks like whenever mac.ops.reset_hw() (which sets
> up mac) is called, igbvf_refresh_ppid() is called as well.
> 
> Thanks
> 
> >
> >>
> >>
> >> >
> >> >Aaron Brown wrote:
> >> >I think I have to fail this, it seems to have an issue with
> >> >initialization.  When I first create a vf via sysfs the pys_port_id
> >> >file is created along with the other sysfs files for the vf, however
> >> >an attempt to cat out the value returns " Operation not supported".
> >> >At this point the vf is still down, if I bring it up (or simply
> >> >unload / reload the igbvf driver) I can then cat the file
> >> >successfully and the vf interface phys_port_id matches the
> >> >phys_port_id of the pf.  This is testing from bare metal, a console
> >> >session showing this behavior
> >> >follows:
> >> >
> >> >u1304:[0]/sys> find . -iname phys_port_id
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
> >> >./devices/virtual/net/sit0/phys_port_id
> >> >./devices/virtual/net/lo/phys_port_id
> >> >u1304:[0]/sys> cat
> >> >devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
> >> >5ece9fbd9cd51546982e15c1f2c11e25
> >> >u1304:[0]/sys>
> >> >
> >> >So far so good, now make a few vfs and check for new phys_port_id
> >> >sysfs
> >> files.
> >> >
> >> >u1304:[0]/sys> find . -iname sriov_numvfs
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/sriov_numvfs
> >> >u1304:[0]/sys> echo 2 >
> >> devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
> >> >u1304:[0]/sys> find . -iname phys_port_id
> >> ./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.2/net/eth3/phys_port_id
> >> >./devices/virtual/net/sit0/phys_port_id
> >> >./devices/virtual/net/lo/phys_port_id
> >> >u1304:[0]/sys>
> >> >
> >> >The first vf is eth2, attempt to cat out it's phys_port_id
> >> >
> >> >u1304:[0]/sys> cat
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
> >> >cat:
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id:
> >> >Operation not supported u1304:[0]/sys>
> >> >
> >> >But, if I bring the interface up (or unload / load the igbvf driver)
> >> >I
> >> then am able to cat the phys_port_id of the vf and it matches the
> >> phys_port_id of the physical interface.
> >> >
> >> >u1304:[0]/sys> ifconfig eth2 u1304-2 u1304:[0]/sys> cat
> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
> >> >5ece9fbd9cd51546982e15c1f2c11e25
> >> >u1304:[0]/sys>
> >>
> >

[-- Attachment #2: phys_port_id_eopnosupp_dmesg_from_boot.tgz --]
[-- Type: application/x-compressed, Size: 29823 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id
  2013-08-28 19:06           ` Brown, Aaron F
@ 2014-02-21  8:02             ` Jiri Pirko
  0 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2014-02-21  8:02 UTC (permalink / raw)
  To: Brown, Aaron F
  Cc: Kirsher, Jeffrey T, netdev, davem, stephen, Narendra_K,
	bhutchings, or.gerlitz, Wyborny, Carolyn, Rose, Gregory V,
	vyasevic, amwang, johannes

Wed, Aug 28, 2013 at 09:06:27PM CEST, aaron.f.brown@intel.com wrote:
>
>
>> -----Original Message-----
>> From: Jiri Pirko [mailto:jiri@resnulli.us]
>> Sent: Tuesday, August 27, 2013 11:06 PM
>> To: Brown, Aaron F
>> Cc: Kirsher, Jeffrey T; netdev@vger.kernel.org; davem@davemloft.net;
>> stephen@networkplumber.org; Narendra_K@Dell.com;
>> bhutchings@solarflare.com; or.gerlitz@gmail.com; Wyborny, Carolyn; Rose,
>> Gregory V; vyasevic@redhat.com; amwang@redhat.com;
>> johannes@sipsolutions.net
>> Subject: Re: [patch net-next v6 4/4] igb/igbvf: implement
>> ndo_get_phys_port_id
>> 
>> Wed, Aug 28, 2013 at 04:26:28AM CEST, aaron.f.brown@intel.com wrote:
>> >Sorry, I was out sick towards the end of last week and playing catch up
>> for the last few days...  Info inline.
>> >
>> >> From: Jiri Pirko [mailto:jiri@resnulli.us]
>> >> Sent: Thursday, August 22, 2013 6:10 AM
>> >> To: Kirsher, Jeffrey T
>> >> Cc: Brown, Aaron F; netdev@vger.kernel.org; davem@davemloft.net;
>> >> stephen@networkplumber.org; Narendra_K@Dell.com;
>> >> bhutchings@solarflare.com; or.gerlitz@gmail.com; Wyborny, Carolyn;
>> >> Rose, Gregory V; vyasevic@redhat.com; amwang@redhat.com;
>> >> johannes@sipsolutions.net
>> >> Subject: Re: [patch net-next v6 4/4] igb/igbvf: implement
>> >> ndo_get_phys_port_id
>> >>
>> >> Thu, Aug 22, 2013 at 12:39:10PM CEST, jeffrey.t.kirsher@intel.com
>> wrote:
>> >> >On Mon, 2013-07-29 at 18:16 +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       |  3 +++
>> >> >>  drivers/net/ethernet/intel/igb/igb_main.c  | 37
>> >> >> ++++++++++++++++++++++++++++-
>> >> >>  drivers/net/ethernet/intel/igbvf/igbvf.h   |  4 ++++
>> >> >>  drivers/net/ethernet/intel/igbvf/mbx.h     |  1 +
>> >> >>  drivers/net/ethernet/intel/igbvf/netdev.c  | 38
>> >> >> ++++++++++++++++++++++++++++++
>> >> >>  drivers/net/ethernet/intel/igbvf/vf.c      | 34
>> >> >> ++++++++++++++++++++++++++
>> >> >>  drivers/net/ethernet/intel/igbvf/vf.h      |  1 +
>> >> >>  8 files changed, 118 insertions(+), 1 deletion(-)
>> >> >
>> >> >Jiri-
>> >> >
>> >> >Validation ran into a couple of issues with this patch.  Here is
>> >> >what Aaron found when testing this patch...
>> >>
>> >> Interesting. So since igbvf_refresh_ppid() is called from
>> >> igbvf_reset() and igbvf_probe(), I believed that is enough. Looks
>> >> like perm_addr getting works in similar way. Can you please check if
>> >> perm_addr is set correctly in the same time reading phys_port_id gives
>> -EOPNOTSUPP?
>> >
>> >By perm_addr do you mean the MAC Address of the vf?  Yes, it is set up
>> >(and I can view it vi ip link or sysconfig) when I see the op not
>> >supported message (after I fi vfs via sysfs, before bringing the vf
>> >interface up or reloading igbvf.)
>> 
>> Can you please send me dmesg log from the system you are testing this?
>
>Attached, dmesg dump from a fresh boot to the eopnosupp. I've been using a .config with a whole bunch of debug junk in it and this is putting out tons of kobject messages, I can re-build and send something with less junk if it's pushing anything relevant off the top.


Hi Aaron, sorry for delay. I failed to find any error message related to
igb in the dmesg you attached. Can you please tell me what exact line do
you have in mind?

>
>> 
>> Looking at the code, it looks like whenever mac.ops.reset_hw() (which sets
>> up mac) is called, igbvf_refresh_ppid() is called as well.
>> 
>> Thanks
>> 
>> >
>> >>
>> >>
>> >> >
>> >> >Aaron Brown wrote:
>> >> >I think I have to fail this, it seems to have an issue with
>> >> >initialization.  When I first create a vf via sysfs the pys_port_id
>> >> >file is created along with the other sysfs files for the vf, however
>> >> >an attempt to cat out the value returns " Operation not supported".
>> >> >At this point the vf is still down, if I bring it up (or simply
>> >> >unload / reload the igbvf driver) I can then cat the file
>> >> >successfully and the vf interface phys_port_id matches the
>> >> >phys_port_id of the pf.  This is testing from bare metal, a console
>> >> >session showing this behavior
>> >> >follows:
>> >> >
>> >> >u1304:[0]/sys> find . -iname phys_port_id
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
>> >> >./devices/virtual/net/sit0/phys_port_id
>> >> >./devices/virtual/net/lo/phys_port_id
>> >> >u1304:[0]/sys> cat
>> >> >devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>> >> >5ece9fbd9cd51546982e15c1f2c11e25
>> >> >u1304:[0]/sys>
>> >> >
>> >> >So far so good, now make a few vfs and check for new phys_port_id
>> >> >sysfs
>> >> files.
>> >> >
>> >> >u1304:[0]/sys> find . -iname sriov_numvfs
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/sriov_numvfs
>> >> >u1304:[0]/sys> echo 2 >
>> >> devices/pci0000:00/0000:00:01.0/0000:07:00.0/sriov_numvfs
>> >> >u1304:[0]/sys> find . -iname phys_port_id
>> >> ./devices/pci0000:00/0000:00:01.0/0000:07:00.0/net/eth0/phys_port_id
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:00.1/net/eth1/phys_port_id
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.2/net/eth3/phys_port_id
>> >> >./devices/virtual/net/sit0/phys_port_id
>> >> >./devices/virtual/net/lo/phys_port_id
>> >> >u1304:[0]/sys>
>> >> >
>> >> >The first vf is eth2, attempt to cat out it's phys_port_id
>> >> >
>> >> >u1304:[0]/sys> cat
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>> >> >cat:
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id:
>> >> >Operation not supported u1304:[0]/sys>
>> >> >
>> >> >But, if I bring the interface up (or unload / load the igbvf driver)
>> >> >I
>> >> then am able to cat the phys_port_id of the vf and it matches the
>> >> phys_port_id of the physical interface.
>> >> >
>> >> >u1304:[0]/sys> ifconfig eth2 u1304-2 u1304:[0]/sys> cat
>> >> >./devices/pci0000:00/0000:00:01.0/0000:07:10.0/net/eth2/phys_port_id
>> >> >5ece9fbd9cd51546982e15c1f2c11e25
>> >> >u1304:[0]/sys>
>> >>
>> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-02-21  8:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-29 16:16 [patch net-next v6 0/4] export device physical port id to userspace Jiri Pirko
2013-07-29 16:16 ` [patch net-next v6 1/4] net: add ndo to get id of physical port of the device Jiri Pirko
2013-07-29 16:16 ` [patch net-next v6 2/4] rtnl: export physical port id via RT netlink Jiri Pirko
2013-07-29 16:16 ` [patch net-next v6 3/4] net: export physical port id via sysfs Jiri Pirko
2013-07-29 16:16 ` [patch net-next v6 4/4] igb/igbvf: implement ndo_get_phys_port_id Jiri Pirko
2013-08-22 10:39   ` Jeff Kirsher
2013-08-22 13:10     ` Jiri Pirko
2013-08-28  2:26       ` Brown, Aaron F
2013-08-28  6:05         ` Jiri Pirko
2013-08-28 19:06           ` Brown, Aaron F
2014-02-21  8:02             ` Jiri Pirko
2013-07-29 22:23 ` [patch net-next v6 0/4] export device physical port id to userspace Jeff Kirsher
2013-07-31  0:36 ` David Miller

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).