Skip to contentSkip to conversation

net: add support for phys_port_name

PATCH Superseded David Ahern 4 participants 10 messages Mar 17, 2015

Subsystems (3)
Networking Network drivers Rocker driver

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

David Ahern

author
#0 lore raw

PATCH v2 1/3 net: add support for phys_port_name

Similar to port id allow netdevices to specify port names and export the name via sysfs. Drivers can implement the netdevice operation to assist udev in having sane default names for the devices using the rule:

$ cat /etc/udev/rules.d/80-net-setup-link.rules
SUBSYSTEM=="net", ACTION=="add", ATTR{phys_port_name}!="",
NAME="$attr{phys_port_name}"

Use of phys_name versus phys_id was suggested-by Jiri Pirko.

v2:
- replaced netdev_phys_item_name with buffer and length per Jiri's comment

Signed-off-by David Ahern <[email protected]>
Cc Jiri Pirko <[email protected]>
Cc Scott Feldman <[email protected]>
---
 include/linux/netdevice.h    |  4 ++++
 include/uapi/linux/if_link.h |  1 +
 net/core/dev.c               | 18 ++++++++++++++++++
 net/core/net-sysfs.c         | 23 +++++++++++++++++++++++
 net/core/rtnetlink.c         | 21 +++++++++++++++++++++
 5 files changed, 67 insertions(+)
include/linux/netdevice.h +4 -0
index dd1d069758be..d4df818d6606 100644
@@ -1159,6 +1159,8 @@ struct net_device_ops {
 						      bool new_carrier);
 	int			(*ndo_get_phys_port_id)(struct net_device *dev,
 							struct netdev_phys_item_id *ppid);
+	int			(*ndo_get_phys_port_name)(struct net_device *dev,
+							  char *buf, int len);
 	void			(*ndo_add_vxlan_port)(struct  net_device *dev,
 						      sa_family_t sa_family,
 						      __be16 port);

@@ -2939,6 +2941,8 @@ int dev_set_mac_address(struct net_device *, struct sockaddr *);
 int dev_change_carrier(struct net_device *, bool new_carrier);
 int dev_get_phys_port_id(struct net_device *dev,
 			 struct netdev_phys_item_id *ppid);
+int dev_get_phys_port_name(struct net_device *dev,
+			   char *buf, int len);
 struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev);
 struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 				    struct netdev_queue *txq, int *ret);
include/uapi/linux/if_link.h +1 -0
index 756436e1ce89..7158fd00a109 100644
@@ -147,6 +147,7 @@ enum {
 	IFLA_CARRIER_CHANGES,
 	IFLA_PHYS_SWITCH_ID,
 	IFLA_LINK_NETNSID,
+	IFLA_PHYS_PORT_NAME,
 	__IFLA_MAX
 };
net/core/dev.c +18 -0
index 39fe369b46ad..ae2a1ecf33be 100644
@@ -5912,6 +5912,24 @@ int dev_get_phys_port_id(struct net_device *dev,
 EXPORT_SYMBOL(dev_get_phys_port_id);
 
 /**
+ *	dev_get_phys_port_name - Get device physical port name
+ *	@dev: device
+ *	@name: port name
+ *
+ *	Get device physical port name
+ */
+int dev_get_phys_port_name(struct net_device *dev,
+			   char *buf, int len)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+
+	if (!ops->ndo_get_phys_port_name)
+		return -EOPNOTSUPP;
+	return ops->ndo_get_phys_port_name(dev, buf, len);
+}
+EXPORT_SYMBOL(dev_get_phys_port_name);
+
+/**
  *	dev_new_index	-	allocate an ifindex
  *	@net: the applicable net namespace
  *
net/core/net-sysfs.c +23 -0
index cf30620a88e1..b2a673870c4b 100644
@@ -418,6 +418,28 @@ static ssize_t phys_port_id_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(phys_port_id);
 
+static ssize_t phys_port_name_show(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)) {
+		char name[IFNAMSIZ];
+
+		ret = dev_get_phys_port_name(netdev, name, sizeof(name));
+		if (!ret)
+			ret = sprintf(buf, "%s\n", name);
+	}
+	rtnl_unlock();
+
+	return ret;
+}
+static DEVICE_ATTR_RO(phys_port_name);
+
 static ssize_t phys_switch_id_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {

@@ -465,6 +487,7 @@ static struct attribute *net_class_attrs[] = {
 	&dev_attr_tx_queue_len.attr,
 	&dev_attr_gro_flush_timeout.attr,
 	&dev_attr_phys_port_id.attr,
+	&dev_attr_phys_port_name.attr,
 	&dev_attr_phys_switch_id.attr,
 	NULL,
 };
net/core/rtnetlink.c +21 -0
index 25b4b5d23485..7227b5695394 100644
@@ -982,6 +982,24 @@ static int rtnl_phys_port_id_fill(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
+static int rtnl_phys_port_name_fill(struct sk_buff *skb, struct net_device *dev)
+{
+	char buf[IFNAMSIZ];
+	int err;
+
+	err = dev_get_phys_port_name(dev, buf, sizeof(buf));
+	if (err) {
+		if (err == -EOPNOTSUPP)
+			return 0;
+		return err;
+	}
+
+	if (nla_put(skb, IFLA_PHYS_PORT_NAME, strlen(buf), buf))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
 static int rtnl_phys_switch_id_fill(struct sk_buff *skb, struct net_device *dev)
 {
 	int err;

@@ -1072,6 +1090,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	if (rtnl_phys_port_id_fill(skb, dev))
 		goto nla_put_failure;
 
+	if (rtnl_phys_port_name_fill(skb, dev))
+		goto nla_put_failure;
+
 	if (rtnl_phys_switch_id_fill(skb, dev))
 		goto nla_put_failure;
Signature · 1 line
2.2.1
author
#1 lore raw

PATCH v2 2/3 rocker: add support for phys_port_name

Implement the phys_port_name operation. Port names are pulled from the rocker hardware model in qemu and default to the qemu name + port id. e.g.,

sw1p1: flags=4098<BROADCAST,MULTICAST> mtu 1500
ether 52:54:00:12:35:01 txqueuelen 1000 (Ethernet) RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 0 bytes 0 (0.0 B)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0

where 'sw1' comes from the qemu command line -device rocker,name=sw1, and 'p1' is port 1.

Patch is adapted from Scott's phys_port_id patch.

v2:
- updated per changes in prior patch requested by Jiri

Signed-off-by David Ahern <[email protected]>
Cc Scott Feldman <[email protected]>
Cc Jiri Pirko <[email protected]>
---
 drivers/net/ethernet/rocker/rocker.c | 64 ++++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/rocker/rocker.h |  1 +
 2 files changed, 65 insertions(+)
drivers/net/ethernet/rocker/rocker.c +64 -0
index 2511ae22ccd8..e201e7ef9bcf 100644
@@ -30,6 +30,7 @@
 #include <linux/if_vlan.h>
 #include <linux/if_bridge.h>
 #include <linux/bitops.h>
+#include <linux/ctype.h>
 #include <net/switchdev.h>
 #include <net/rtnetlink.h>
 #include <net/ip_fib.h>

@@ -1630,6 +1631,53 @@ rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker,
 	return 0;
 }
 
+struct port_name {
+	char *buf;
+	int len;
+};
+
+static int
+rocker_cmd_get_port_settings_phys_name_proc(struct rocker *rocker,
+					    struct rocker_port *rocker_port,
+					    struct rocker_desc_info *desc_info,
+					    void *priv)
+{
+	struct rocker_tlv *info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_MAX + 1];
+	struct rocker_tlv *attrs[ROCKER_TLV_CMD_MAX + 1];
+	struct port_name *name = priv;
+	struct rocker_tlv *attr;
+	int i, j = 0, len;
+	char *str;
+
+	rocker_tlv_parse_desc(attrs, ROCKER_TLV_CMD_MAX, desc_info);
+	if (!attrs[ROCKER_TLV_CMD_INFO])
+		return -EIO;
+
+	rocker_tlv_parse_nested(info_attrs, ROCKER_TLV_CMD_PORT_SETTINGS_MAX,
+				attrs[ROCKER_TLV_CMD_INFO]);
+	attr = info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME];
+	if (!attr)
+		return -EIO;
+
+	len = min_t(int, rocker_tlv_len(attr), name->len);
+	str = rocker_tlv_data(attr);
+
+	/* make sure name only contains alphanumeric characters */
+	for (i = 0; i < len; ++i) {
+		if (isalnum(str[i])) {
+			name->buf[j] = str[i];
+			j++;
+		}
+	}
+
+	if (j == 0)
+		return -EIO;
+
+	name->buf[j] = '\0';
+
+	return 0;
+}
+
 static int
 rocker_cmd_set_port_settings_ethtool_prep(struct rocker *rocker,
 					  struct rocker_port *rocker_port,

@@ -4138,6 +4186,21 @@ static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				       rocker_port->brport_flags, mask);
 }
 
+static int rocker_port_get_phys_port_name(struct net_device *dev,
+					  char *buf, int len)
+{
+	struct rocker_port *rocker_port = netdev_priv(dev);
+	struct port_name name = { .buf = buf, .len = len };
+	int err;
+
+	err = rocker_cmd_exec(rocker_port->rocker, rocker_port,
+			      rocker_cmd_get_port_settings_prep, NULL,
+			      rocker_cmd_get_port_settings_phys_name_proc,
+			      &name, false);
+
+	return err ? -EOPNOTSUPP : 0;
+}
+
 static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_open			= rocker_port_open,
 	.ndo_stop			= rocker_port_stop,

@@ -4150,6 +4213,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_fdb_dump			= rocker_port_fdb_dump,
 	.ndo_bridge_setlink		= rocker_port_bridge_setlink,
 	.ndo_bridge_getlink		= rocker_port_bridge_getlink,
+	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
 };
 
 /********************
drivers/net/ethernet/rocker/rocker.h +1 -0
index 51e430d25138..a4e9591d7457 100644
@@ -158,6 +158,7 @@ enum {
 	ROCKER_TLV_CMD_PORT_SETTINGS_MACADDR,		/* binary */
 	ROCKER_TLV_CMD_PORT_SETTINGS_MODE,		/* u8 */
 	ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING,		/* u8 */
+	ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME,		/* binary */
 
 	__ROCKER_TLV_CMD_PORT_SETTINGS_MAX,
 	ROCKER_TLV_CMD_PORT_SETTINGS_MAX =
Signature · 1 line
2.2.1
#2 lore raw

Re: [PATCH v2 2/3] rocker: add support for phys_port_name

Inline

Quoted 40 lines · David Ahern

Implement the phys_port_name operation. Port names are pulled from the rocker hardware model in qemu and default to the qemu name + port id. e.g.,

sw1p1: flags=4098<BROADCAST,MULTICAST> mtu 1500
ether 52:54:00:12:35:01 txqueuelen 1000 (Ethernet) RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 0 bytes 0 (0.0 B)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0

where 'sw1' comes from the qemu command line -device rocker,name=sw1, and 'p1' is port 1.

Patch is adapted from Scott's phys_port_id patch.

v2:
- updated per changes in prior patch requested by Jiri

Signed-off-by David Ahern <[email protected]>
Cc Scott Feldman <[email protected]>
Cc Jiri Pirko <[email protected]>
---
 drivers/net/ethernet/rocker/rocker.c | 64 ++++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/rocker/rocker.h |  1 +
 2 files changed, 65 insertions(+)
drivers/net/ethernet/rocker/rocker.c +1 -0
index 2511ae22ccd8..e201e7ef9bcf 100644
@@ -30,6 +30,7 @@
 #include <linux/if_vlan.h>
 #include <linux/if_bridge.h>
 #include <linux/bitops.h>
+#include <linux/ctype.h>
 #include <net/switchdev.h>
 #include <net/rtnetlink.h>
 #include <net/ip_fib.h>

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1636

Quoted 6 lines
drivers/net/ethernet/rocker/rocker.c +3 -0
@@ -1630,6 +1631,53 @@ rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker,
 	return 0;
 }
 
+struct port_name {
+	char *buf;
+	int len;

I'd use "unsigned" for the len, since it's never going to be negative.

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1649

Quoted 13 lines
drivers/net/ethernet/rocker/rocker.c +13 -0
+};
+
+static int
+rocker_cmd_get_port_settings_phys_name_proc(struct rocker *rocker,
+					    struct rocker_port *rocker_port,
+					    struct rocker_desc_info *desc_info,
+					    void *priv)
+{
+	struct rocker_tlv *info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_MAX + 1];
+	struct rocker_tlv *attrs[ROCKER_TLV_CMD_MAX + 1];
+	struct port_name *name = priv;
+	struct rocker_tlv *attr;
+	int i, j = 0, len;

I'd make 'i' and 'j' unsigned.

Quoted 7 lines
drivers/net/ethernet/rocker/rocker.c +7 -0
+	char *str;
+
+	rocker_tlv_parse_desc(attrs, ROCKER_TLV_CMD_MAX, desc_info);
+	if (!attrs[ROCKER_TLV_CMD_INFO])
+		return -EIO;
+
+	rocker_tlv_parse_nested(info_attrs, ROCKER_TLV_CMD_PORT_SETTINGS_MAX,

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1666

Quoted 10 lines
drivers/net/ethernet/rocker/rocker.c +10 -0
+				attrs[ROCKER_TLV_CMD_INFO]);
+	attr = info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME];
+	if (!attr)
+		return -EIO;
+
+	len = min_t(int, rocker_tlv_len(attr), name->len);
+	str = rocker_tlv_data(attr);
+
+	/* make sure name only contains alphanumeric characters */
+	for (i = 0; i < len; ++i) {

Rather than hiding the initialization of 'j' above, I'd do:

i = j = 0;

instead.

Quoted 18 lines
drivers/net/ethernet/rocker/rocker.c +14 -0
+		if (isalnum(str[i])) {
+			name->buf[j] = str[i];
+			j++;
+		}
+	}
+
+	if (j == 0)
+		return -EIO;
+
+	name->buf[j] = '\0';
+
+	return 0;
+}
+
 static int
 rocker_cmd_set_port_settings_ethtool_prep(struct rocker *rocker,
 					  struct rocker_port *rocker_port,

drivers/net/ethernet/rocker/rocker.c static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq, · line 4190

Quoted 5 lines
drivers/net/ethernet/rocker/rocker.c +2 -0
@@ -4138,6 +4186,21 @@ static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				       rocker_port->brport_flags, mask);
 }
 
+static int rocker_port_get_phys_port_name(struct net_device *dev,
+					  char *buf, int len)

I'd use unsigned for 'len'.

Quoted 36 lines
drivers/net/ethernet/rocker/rocker.c +14 -0
+{
+	struct rocker_port *rocker_port = netdev_priv(dev);
+	struct port_name name = { .buf = buf, .len = len };
+	int err;
+
+	err = rocker_cmd_exec(rocker_port->rocker, rocker_port,
+			      rocker_cmd_get_port_settings_prep, NULL,
+			      rocker_cmd_get_port_settings_phys_name_proc,
+			      &name, false);
+
+	return err ? -EOPNOTSUPP : 0;
+}
+
 static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_open			= rocker_port_open,
 	.ndo_stop			= rocker_port_stop,

@@ -4150,6 +4213,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_fdb_dump			= rocker_port_fdb_dump,
 	.ndo_bridge_setlink		= rocker_port_bridge_setlink,
 	.ndo_bridge_getlink		= rocker_port_bridge_getlink,
+	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
 };
 
 /********************
drivers/net/ethernet/rocker/rocker.h +1 -0
index 51e430d25138..a4e9591d7457 100644
@@ -158,6 +158,7 @@ enum {
 	ROCKER_TLV_CMD_PORT_SETTINGS_MACADDR,		/* binary */
 	ROCKER_TLV_CMD_PORT_SETTINGS_MODE,		/* u8 */
 	ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING,		/* u8 */
+	ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME,		/* binary */
 
 	__ROCKER_TLV_CMD_PORT_SETTINGS_MAX,
 	ROCKER_TLV_CMD_PORT_SETTINGS_MAX =
maintainer
#3 lore raw

Re: [PATCH v2 2/3] rocker: add support for phys_port_name

Quoted 46 lines · Tue, Mar 17, 2015 at 05:27:25AM CET, [email protected] wrote:

Inline

Quoted 43 lines · David Ahern

Implement the phys_port_name operation. Port names are pulled from the rocker hardware model in qemu and default to the qemu name + port id. e.g.,

sw1p1: flags=4098<BROADCAST,MULTICAST> mtu 1500
ether 52:54:00:12:35:01 txqueuelen 1000 (Ethernet) RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 0 bytes 0 (0.0 B)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0

where 'sw1' comes from the qemu command line -device rocker,name=sw1, and 'p1' is port 1.

Patch is adapted from Scott's phys_port_id patch.

v2:
- updated per changes in prior patch requested by Jiri

Signed-off-by David Ahern <[email protected]>
Cc Scott Feldman <[email protected]>
Cc Jiri Pirko <[email protected]>
---
 drivers/net/ethernet/rocker/rocker.c | 64 ++++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/rocker/rocker.h |  1 +
 2 files changed, 65 insertions(+)
drivers/net/ethernet/rocker/rocker.c +1 -0
index 2511ae22ccd8..e201e7ef9bcf 100644
@@ -30,6 +30,7 @@
 #include <linux/if_vlan.h>
 #include <linux/if_bridge.h>
 #include <linux/bitops.h>
+#include <linux/ctype.h>
 #include <net/switchdev.h>
 #include <net/rtnetlink.h>
 #include <net/ip_fib.h>

@@ -1630,6 +1631,53 @@ rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker,
 	return 0;
 }
Quoted 5 lines

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1636

Quoted 3 lines
drivers/net/ethernet/rocker/rocker.c +3 -0
+struct port_name {
+	char *buf;
+	int len;

I'd use "unsigned" for the len, since it's never going to be negative.

It think the best is just to use "size_t" for len

Quoted 105 lines
Quoted 4 lines
drivers/net/ethernet/rocker/rocker.c +3 -0
+};
+
+static int

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1649

Quoted 10 lines
drivers/net/ethernet/rocker/rocker.c +10 -0
+rocker_cmd_get_port_settings_phys_name_proc(struct rocker *rocker,
+					    struct rocker_port *rocker_port,
+					    struct rocker_desc_info *desc_info,
+					    void *priv)
+{
+	struct rocker_tlv *info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_MAX + 1];
+	struct rocker_tlv *attrs[ROCKER_TLV_CMD_MAX + 1];
+	struct port_name *name = priv;
+	struct rocker_tlv *attr;
+	int i, j = 0, len;

I'd make 'i' and 'j' unsigned.

Quoted 7 lines
drivers/net/ethernet/rocker/rocker.c +7 -0
+	char *str;
+
+	rocker_tlv_parse_desc(attrs, ROCKER_TLV_CMD_MAX, desc_info);
+	if (!attrs[ROCKER_TLV_CMD_INFO])
+		return -EIO;
+
+	rocker_tlv_parse_nested(info_attrs, ROCKER_TLV_CMD_PORT_SETTINGS_MAX,

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1666

Quoted 10 lines
drivers/net/ethernet/rocker/rocker.c +10 -0
+				attrs[ROCKER_TLV_CMD_INFO]);
+	attr = info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME];
+	if (!attr)
+		return -EIO;
+
+	len = min_t(int, rocker_tlv_len(attr), name->len);
+	str = rocker_tlv_data(attr);
+
+	/* make sure name only contains alphanumeric characters */
+	for (i = 0; i < len; ++i) {

Rather than hiding the initialization of 'j' above, I'd do:

i = j = 0;

instead.

Quoted 18 lines
drivers/net/ethernet/rocker/rocker.c +14 -0
+		if (isalnum(str[i])) {
+			name->buf[j] = str[i];
+			j++;
+		}
+	}
+
+	if (j == 0)
+		return -EIO;
+
+	name->buf[j] = '\0';
+
+	return 0;
+}
+
 static int
 rocker_cmd_set_port_settings_ethtool_prep(struct rocker *rocker,
 					  struct rocker_port *rocker_port,

drivers/net/ethernet/rocker/rocker.c static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq, · line 4190

Quoted 5 lines
drivers/net/ethernet/rocker/rocker.c +2 -0
@@ -4138,6 +4186,21 @@ static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				       rocker_port->brport_flags, mask);
 }
 
+static int rocker_port_get_phys_port_name(struct net_device *dev,
+					  char *buf, int len)

I'd use unsigned for 'len'.

Quoted 36 lines
drivers/net/ethernet/rocker/rocker.c +14 -0
+{
+	struct rocker_port *rocker_port = netdev_priv(dev);
+	struct port_name name = { .buf = buf, .len = len };
+	int err;
+
+	err = rocker_cmd_exec(rocker_port->rocker, rocker_port,
+			      rocker_cmd_get_port_settings_prep, NULL,
+			      rocker_cmd_get_port_settings_phys_name_proc,
+			      &name, false);
+
+	return err ? -EOPNOTSUPP : 0;
+}
+
 static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_open			= rocker_port_open,
 	.ndo_stop			= rocker_port_stop,

@@ -4150,6 +4213,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_fdb_dump			= rocker_port_fdb_dump,
 	.ndo_bridge_setlink		= rocker_port_bridge_setlink,
 	.ndo_bridge_getlink		= rocker_port_bridge_getlink,
+	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
 };
 
 /********************
drivers/net/ethernet/rocker/rocker.h +1 -0
index 51e430d25138..a4e9591d7457 100644
@@ -158,6 +158,7 @@ enum {
 	ROCKER_TLV_CMD_PORT_SETTINGS_MACADDR,		/* binary */
 	ROCKER_TLV_CMD_PORT_SETTINGS_MODE,		/* u8 */
 	ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING,		/* u8 */
+	ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME,		/* binary */
 
 	__ROCKER_TLV_CMD_PORT_SETTINGS_MAX,
 	ROCKER_TLV_CMD_PORT_SETTINGS_MAX =
#4 lore raw

Re: [PATCH v2 2/3] rocker: add support for phys_port_name

Inline

Quoted 47 lines · Jiri Pirko
Quoted 46 lines · Tue, Mar 17, 2015 at 05:27:25AM CET, [email protected] wrote:

Inline

Quoted 43 lines · David Ahern

Implement the phys_port_name operation. Port names are pulled from the rocker hardware model in qemu and default to the qemu name + port id. e.g.,

sw1p1: flags=4098<BROADCAST,MULTICAST> mtu 1500
ether 52:54:00:12:35:01 txqueuelen 1000 (Ethernet) RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 0 bytes 0 (0.0 B)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0

where 'sw1' comes from the qemu command line -device rocker,name=sw1, and 'p1' is port 1.

Patch is adapted from Scott's phys_port_id patch.

v2:
- updated per changes in prior patch requested by Jiri

Signed-off-by David Ahern <[email protected]>
Cc Scott Feldman <[email protected]>
Cc Jiri Pirko <[email protected]>
---
 drivers/net/ethernet/rocker/rocker.c | 64 ++++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/rocker/rocker.h |  1 +
 2 files changed, 65 insertions(+)
drivers/net/ethernet/rocker/rocker.c +1 -0
index 2511ae22ccd8..e201e7ef9bcf 100644
@@ -30,6 +30,7 @@
 #include <linux/if_vlan.h>
 #include <linux/if_bridge.h>
 #include <linux/bitops.h>
+#include <linux/ctype.h>
 #include <net/switchdev.h>
 #include <net/rtnetlink.h>
 #include <net/ip_fib.h>

@@ -1630,6 +1631,53 @@ rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker,
 	return 0;
 }
Quoted 5 lines
Quoted 4 lines

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1636

Quoted 3 lines
drivers/net/ethernet/rocker/rocker.c +3 -0
+struct port_name {
+	char *buf;
+	int len;

I'd use "unsigned" for the len, since it's never going to be negative.

It think the best is just to use "size_t" for len

Works for me.

-Philip

Quoted 104 lines
Quoted 98 lines

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1649

Quoted 13 lines
drivers/net/ethernet/rocker/rocker.c +13 -0
+};
+
+static int
+rocker_cmd_get_port_settings_phys_name_proc(struct rocker *rocker,
+					    struct rocker_port *rocker_port,
+					    struct rocker_desc_info *desc_info,
+					    void *priv)
+{
+	struct rocker_tlv *info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_MAX + 1];
+	struct rocker_tlv *attrs[ROCKER_TLV_CMD_MAX + 1];
+	struct port_name *name = priv;
+	struct rocker_tlv *attr;
+	int i, j = 0, len;

I'd make 'i' and 'j' unsigned.

Quoted 7 lines
drivers/net/ethernet/rocker/rocker.c +7 -0
+	char *str;
+
+	rocker_tlv_parse_desc(attrs, ROCKER_TLV_CMD_MAX, desc_info);
+	if (!attrs[ROCKER_TLV_CMD_INFO])
+		return -EIO;
+
+	rocker_tlv_parse_nested(info_attrs, ROCKER_TLV_CMD_PORT_SETTINGS_MAX,

drivers/net/ethernet/rocker/rocker.c rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker, · line 1666

Quoted 10 lines
drivers/net/ethernet/rocker/rocker.c +10 -0
+				attrs[ROCKER_TLV_CMD_INFO]);
+	attr = info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME];
+	if (!attr)
+		return -EIO;
+
+	len = min_t(int, rocker_tlv_len(attr), name->len);
+	str = rocker_tlv_data(attr);
+
+	/* make sure name only contains alphanumeric characters */
+	for (i = 0; i < len; ++i) {

Rather than hiding the initialization of 'j' above, I'd do:

i = j = 0;

instead.

Quoted 18 lines
drivers/net/ethernet/rocker/rocker.c +14 -0
+		if (isalnum(str[i])) {
+			name->buf[j] = str[i];
+			j++;
+		}
+	}
+
+	if (j == 0)
+		return -EIO;
+
+	name->buf[j] = '\0';
+
+	return 0;
+}
+
 static int
 rocker_cmd_set_port_settings_ethtool_prep(struct rocker *rocker,
 					  struct rocker_port *rocker_port,

drivers/net/ethernet/rocker/rocker.c static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq, · line 4190

Quoted 5 lines
drivers/net/ethernet/rocker/rocker.c +2 -0
@@ -4138,6 +4186,21 @@ static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				       rocker_port->brport_flags, mask);
 }
 
+static int rocker_port_get_phys_port_name(struct net_device *dev,
+					  char *buf, int len)

I'd use unsigned for 'len'.

Quoted 34 lines
drivers/net/ethernet/rocker/rocker.c +14 -0
+{
+	struct rocker_port *rocker_port = netdev_priv(dev);
+	struct port_name name = { .buf = buf, .len = len };
+	int err;
+
+	err = rocker_cmd_exec(rocker_port->rocker, rocker_port,
+			      rocker_cmd_get_port_settings_prep, NULL,
+			      rocker_cmd_get_port_settings_phys_name_proc,
+			      &name, false);
+
+	return err ? -EOPNOTSUPP : 0;
+}
+
 static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_open			= rocker_port_open,
 	.ndo_stop			= rocker_port_stop,

@@ -4150,6 +4213,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_fdb_dump			= rocker_port_fdb_dump,
 	.ndo_bridge_setlink		= rocker_port_bridge_setlink,
 	.ndo_bridge_getlink		= rocker_port_bridge_getlink,
+	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
 };
 
 /********************
drivers/net/ethernet/rocker/rocker.h +1 -0
index 51e430d25138..a4e9591d7457 100644
@@ -158,6 +158,7 @@ enum {
 	ROCKER_TLV_CMD_PORT_SETTINGS_MACADDR,		/* binary */
 	ROCKER_TLV_CMD_PORT_SETTINGS_MODE,		/* u8 */
 	ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING,		/* u8 */
+	ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME,		/* binary */
Quoted 2 lines

drivers/net/ethernet/rocker/rocker.h enum { · line 164

Quoted 2 lines
drivers/net/ethernet/rocker/rocker.h +0 -0
 	__ROCKER_TLV_CMD_PORT_SETTINGS_MAX,
 	ROCKER_TLV_CMD_PORT_SETTINGS_MAX =

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Acked-by on v2 2/3
#5 lore raw

Re: [PATCH v2 2/3] rocker: add support for phys_port_name

In reply to David Ahern
Quoted 14 lines · David Ahern

Implement the phys_port_name operation. Port names are pulled from the rocker hardware model in qemu and default to the qemu name + port id. e.g.,

sw1p1: flags=4098<BROADCAST,MULTICAST> mtu 1500
ether 52:54:00:12:35:01 txqueuelen 1000 (Ethernet) RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 0 bytes 0 (0.0 B)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0

where 'sw1' comes from the qemu command line -device rocker,name=sw1, and 'p1' is port 1.

Quoted 8 lines

Patch is adapted from Scott's phys_port_id patch.

v2:
- updated per changes in prior patch requested by Jiri

Signed-off-by David Ahern <[email protected]>
Cc Scott Feldman <[email protected]>
Cc Jiri Pirko <[email protected]>
Acked-by Scott Feldman <[email protected]>
author
#6 lore raw

PATCH 3/3 iproute2: Add support for phys_port_name

In reply to David Ahern

Display phys_port_name attribute if returned:

./ip link show sw1p1
4: sw1p1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop port name sw1p1 state DOWN mode DEFAULT group default qlen 1000

    link/ether 52:54:00:12:35:01 brd ff:ff:ff:ff:ff:ff
Signed-off-by David Ahern <[email protected]>
Cc Jiri Pirko <[email protected]>
Cc Scott Feldman <[email protected]>
---
 include/linux/if_link.h | 1 +
 ip/ipaddress.c          | 3 +++
 2 files changed, 4 insertions(+)
include/linux/if_link.h +1 -0
index 3450c3fbdc65..c5ee8c91b1c6 100644
@@ -147,6 +147,7 @@ enum {
 	IFLA_CARRIER_CHANGES,
 	IFLA_PHYS_SWITCH_ID,
 	IFLA_LINK_NETNSID,
+	IFLA_PHYS_PORT_NAME,
 	__IFLA_MAX
 };
ip/ipaddress.c +3 -0
index 99a6ab5977e3..9e65015b7a48 100644
@@ -645,6 +645,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
 				      b1, sizeof(b1)));
 	}
 
+	if (tb[IFLA_PHYS_PORT_NAME])
+		fprintf(fp, "port name %s ", rta_getattr_str(tb[IFLA_PHYS_PORT_NAME]));
+
 	if (tb[IFLA_OPERSTATE])
 		print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
Signature · 1 line
2.2.1
maintainer Acked-by on v1 3/3
#7 lore raw

Re: [PATCH 3/3] iproute2: Add support for phys_port_name

Quoted 9 lines · Tue, Mar 17, 2015 at 04:27:17AM CET, [email protected] wrote:

Display phys_port_name attribute if returned:

./ip link show sw1p1
4: sw1p1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop port name sw1p1 state DOWN mode DEFAULT group default qlen 1000
link/ether 52:54:00:12:35:01 brd ff:ff:ff:ff:ff:ff

Signed-off-by David Ahern <[email protected]>
Cc Jiri Pirko <[email protected]>
Cc Scott Feldman <[email protected]>
Acked-by Jiri Pirko <[email protected]>
Quoted 35 lines

---
include/linux/if_link.h | 1 +
ip/ipaddress.c | 3 +++

2 files changed, 4 insertions(+)
include/linux/if_link.h +1 -0
index 3450c3fbdc65..c5ee8c91b1c6 100644
@@ -147,6 +147,7 @@ enum {
 	IFLA_CARRIER_CHANGES,
 	IFLA_PHYS_SWITCH_ID,
 	IFLA_LINK_NETNSID,
+	IFLA_PHYS_PORT_NAME,
 	__IFLA_MAX
 };
ip/ipaddress.c +3 -0
index 99a6ab5977e3..9e65015b7a48 100644
@@ -645,6 +645,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
 				      b1, sizeof(b1)));
 	}
 
+	if (tb[IFLA_PHYS_PORT_NAME])
+		fprintf(fp, "port name %s ", rta_getattr_str(tb[IFLA_PHYS_PORT_NAME]));
+
 	if (tb[IFLA_OPERSTATE])
 		print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
Signature · 1 line
2.2.1
Acked-by on v2 1/3
#8 lore raw
In reply to David Ahern
Quoted 9 lines · David Ahern

Similar to port id allow netdevices to specify port names and export the name via sysfs. Drivers can implement the netdevice operation to assist udev in having sane default names for the devices using the rule:

$ cat /etc/udev/rules.d/80-net-setup-link.rules
SUBSYSTEM=="net", ACTION=="add", ATTR{phys_port_name}!="",
NAME="$attr{phys_port_name}"
Quoted 8 lines

Use of phys_name versus phys_id was suggested-by Jiri Pirko.

v2:
- replaced netdev_phys_item_name with buffer and length per Jiri's comment

Signed-off-by David Ahern <[email protected]>
Cc Jiri Pirko <[email protected]>
Cc Scott Feldman <[email protected]>
Acked-by Scott Feldman <[email protected]>
maintainer Acked-by on v2 1/3
#9 lore raw
In reply to David Ahern
Quoted 30 lines · Tue, Mar 17, 2015 at 04:27:15AM CET, [email protected] wrote:

Similar to port id allow netdevices to specify port names and export the name via sysfs. Drivers can implement the netdevice operation to assist udev in having sane default names for the devices using the rule:

$ cat /etc/udev/rules.d/80-net-setup-link.rules
SUBSYSTEM=="net", ACTION=="add", ATTR{phys_port_name}!="",
NAME="$attr{phys_port_name}"

Use of phys_name versus phys_id was suggested-by Jiri Pirko.

v2:
- replaced netdev_phys_item_name with buffer and length per Jiri's comment

Signed-off-by David Ahern <[email protected]>
Cc Jiri Pirko <[email protected]>
Cc Scott Feldman <[email protected]>

---

include/linux/netdevice.h    |  4 ++++
include/uapi/linux/if_link.h |  1 +
net/core/dev.c               | 18 ++++++++++++++++++
net/core/net-sysfs.c         | 23 +++++++++++++++++++++++
net/core/rtnetlink.c         | 21 +++++++++++++++++++++
5 files changed, 67 insertions(+)
include/linux/netdevice.h +0 -0
index dd1d069758be..d4df818d6606 100644

include/linux/netdevice.h struct net_device_ops { · line 1163

Quoted 5 lines
include/linux/netdevice.h +2 -0
@@ -1159,6 +1159,8 @@ struct net_device_ops {
 						      bool new_carrier);
 	int			(*ndo_get_phys_port_id)(struct net_device *dev,
 							struct netdev_phys_item_id *ppid);
+	int			(*ndo_get_phys_port_name)(struct net_device *dev,
+							  char *buf, int len);

please change this to "char *name" and "size_t len"

With that changes, feel free to add my

Acked-by Jiri Pirko <[email protected]>

Thanks David for taking care of this.

Quoted 136 lines
include/linux/netdevice.h +2 -0
 	void			(*ndo_add_vxlan_port)(struct  net_device *dev,
 						      sa_family_t sa_family,
 						      __be16 port);

@@ -2939,6 +2941,8 @@ int dev_set_mac_address(struct net_device *, struct sockaddr *);
 int dev_change_carrier(struct net_device *, bool new_carrier);
 int dev_get_phys_port_id(struct net_device *dev,
 			 struct netdev_phys_item_id *ppid);
+int dev_get_phys_port_name(struct net_device *dev,
+			   char *buf, int len);
 struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev);
 struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 				    struct netdev_queue *txq, int *ret);
include/uapi/linux/if_link.h +1 -0
index 756436e1ce89..7158fd00a109 100644
@@ -147,6 +147,7 @@ enum {
 	IFLA_CARRIER_CHANGES,
 	IFLA_PHYS_SWITCH_ID,
 	IFLA_LINK_NETNSID,
+	IFLA_PHYS_PORT_NAME,
 	__IFLA_MAX
 };
net/core/dev.c +18 -0
index 39fe369b46ad..ae2a1ecf33be 100644
@@ -5912,6 +5912,24 @@ int dev_get_phys_port_id(struct net_device *dev,
 EXPORT_SYMBOL(dev_get_phys_port_id);
 
 /**
+ *	dev_get_phys_port_name - Get device physical port name
+ *	@dev: device
+ *	@name: port name
+ *
+ *	Get device physical port name
+ */
+int dev_get_phys_port_name(struct net_device *dev,
+			   char *buf, int len)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+
+	if (!ops->ndo_get_phys_port_name)
+		return -EOPNOTSUPP;
+	return ops->ndo_get_phys_port_name(dev, buf, len);
+}
+EXPORT_SYMBOL(dev_get_phys_port_name);
+
+/**
 *	dev_new_index	-	allocate an ifindex
 *	@net: the applicable net namespace
 *
net/core/net-sysfs.c +23 -0
index cf30620a88e1..b2a673870c4b 100644
@@ -418,6 +418,28 @@ static ssize_t phys_port_id_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(phys_port_id);
 
+static ssize_t phys_port_name_show(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)) {
+		char name[IFNAMSIZ];
+
+		ret = dev_get_phys_port_name(netdev, name, sizeof(name));
+		if (!ret)
+			ret = sprintf(buf, "%s\n", name);
+	}
+	rtnl_unlock();
+
+	return ret;
+}
+static DEVICE_ATTR_RO(phys_port_name);
+
 static ssize_t phys_switch_id_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {

@@ -465,6 +487,7 @@ static struct attribute *net_class_attrs[] = {
 	&dev_attr_tx_queue_len.attr,
 	&dev_attr_gro_flush_timeout.attr,
 	&dev_attr_phys_port_id.attr,
+	&dev_attr_phys_port_name.attr,
 	&dev_attr_phys_switch_id.attr,
 	NULL,
 };
net/core/rtnetlink.c +21 -0
index 25b4b5d23485..7227b5695394 100644
@@ -982,6 +982,24 @@ static int rtnl_phys_port_id_fill(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
+static int rtnl_phys_port_name_fill(struct sk_buff *skb, struct net_device *dev)
+{
+	char buf[IFNAMSIZ];
+	int err;
+
+	err = dev_get_phys_port_name(dev, buf, sizeof(buf));
+	if (err) {
+		if (err == -EOPNOTSUPP)
+			return 0;
+		return err;
+	}
+
+	if (nla_put(skb, IFLA_PHYS_PORT_NAME, strlen(buf), buf))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
 static int rtnl_phys_switch_id_fill(struct sk_buff *skb, struct net_device *dev)
 {
 	int err;

@@ -1072,6 +1090,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	if (rtnl_phys_port_id_fill(skb, dev))
 		goto nla_put_failure;
 
+	if (rtnl_phys_port_name_fill(skb, dev))
+		goto nla_put_failure;
+
 	if (rtnl_phys_switch_id_fill(skb, dev))
 		goto nla_put_failure;
Signature · 1 line
2.2.1