| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130721072455.GA1944@minipsycho.orion>
Date: Sun, 21 Jul 2013 09:24:55 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Or Gerlitz <or.gerlitz@...il.com>
Cc: Narendra_K@...l.com, netdev@...r.kernel.org,
bhutchings@...arflare.com, john.r.fastabend@...el.com
Subject: Re: [PATCH net-next] net: Add phys_port identifier to struct
net_device and export it to sysfs
Sun, Jul 21, 2013 at 07:55:41AM CEST, or.gerlitz@...il.com wrote:
>On Mon, Jul 15, 2013 at 6:34 PM, <Narendra_K@...l.com> wrote:
>[...]
>> It is implemented in this patch here -
>> [RFC,net-next] net: Include phys_port identifier in the RTM_NEWLINK
>> message http://patchwork.ozlabs.org/patch/255435/
>>
>> Please find the patch with the changes incorporated. If the changes look
>> good, i will incorporate them in the next version of the patch.
>>
>> [PATCH net-next ] Add phys_port identifier to struct net_device and export it to sysfs
>>
>> It is useful to know if network interfaces from NIC partitions
>> 'map to/use the' same physical port. For example, when creating
>> bonding in fault tolerance mode, if two network interfaces map to/use
>> the same physical port, it might not have the desired result. This
>> information is not available today in a standard format or it is not
>> present. If this information can be made available in a generic way
>> to user space, tools such as NetworkManager or Libteam or Wicked can
>> make smarter bonding decisions (such as warn users when setting up
>> configurations which will not have desired effect).
>>
>> The requirement is to have a generic interface using which
>> kernel/drivers can provide information/hints to user space about the
>> physical port number used by a network interface.
>>
>> The following options were explored -
>>
>> 1. 'dev_id' sysfs attribute:
>>
>> In addition to being used to differentiate between devices that share
>> the same link layer address, it is being used to indicate the physical
>> port number used by a network interface.
>>
>> As dev_id exists to differentiate between devices sharing the same
>> link layer address, dev_id option is not selected.
>>
>> 2. Re-using 'if_port' field in 'struct net_device':
>>
>> if_port field exists to indicate the media type(please refer to
>> netdevice.h). It seemed like it was also used to indicate the physical
>> port number.
>>
>> As re-using 'if_port' might possibly break user space, this option is
>> not selected.
>>
>> 3. Add a new field 'phys_port' to 'struct net_device' and export it to sysfs:
>>
>> The 'phys_port' will be a universally unique identifier, which
>> would be a MAC-48 or EUI-64 or a 128 bit UUID value, but not
>> restricted to these spaces. It will uniquely identify the physical
>> port used by a network interface. The 'length' of the identifier will
>> be zero if the field is not set for a network interface.
>>
>> This patch implements option 3. It creates a new sysfs attribute
>> 'phys_port' -
>>
>> /sys/class/net/<interface name>/phys_port
>
>Hi Narendra, John, Ben, Jiri,
>
>Before diving to the eswitch use cases of this patch/approach, I was
>wondering if/how the new
>phys_port field could be of use for drivers that just want to place
>there an INT saying if this interface
>is associated with port #N of the PCI device (e.g N={1,2}). I wasn't
>sure how this is supposted
>to be done if the semantics of the new field is the one mentioned
>above in the change long
>(MAC-48 or EUI-64 or a 128 bit UUID value). Both mlx4 and ipoib driver
>use the dev_id field and would
>be happily ported to the new interface once introduced in a way that
>allows to meet the integer requirement.
The value could be anything. But note that you have to have different
values for card1-port1,2 and card2-port1,2
>
>Or.
>
>> References: http://marc.info/?l=linux-netdev&m=136920998009209&w=2
>> References: http://marc.info/?l=linux-netdev&m=136992041432498&w=2
>>
>> Signed-off-by: Narendra K <narendra_k@...l.com>
>> ---
>> include/linux/netdevice.h | 14 ++++++++++++++
>> net/core/net-sysfs.c | 22 ++++++++++++++++++++++
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 0741a1e..17db7e5 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1062,6 +1062,14 @@ struct net_device_ops {
>> bool new_carrier);
>> };
>>
>> +/* This structure holds a universally unique identifier to
>> + * identify the physical port used by a netdevice
>> + */
>> +struct phys_port_identifier {
>> + unsigned char port_id[MAX_ADDR_LEN];
>> + unsigned char port_id_len;
>> +};
>> +
>> /*
>> * The DEVICE structure.
>> * Actually, this whole structure is a big mistake. It mixes I/O
>> @@ -1181,6 +1189,11 @@ struct net_device {
>> * that share the same
>> link
>> * layer address
>> */
>> + struct phys_port_identifier phys_port; /* Universally unique
>> physical
>> + * port identifier,
>> MAC-48 or
>> + * EUI-64 or 128 bit
>> UUID,
>> + * length is zero if
>> not set
>> + */
>> spinlock_t addr_list_lock;
>> struct netdev_hw_addr_list uc; /* Unicast mac addresses
>> */
>> struct netdev_hw_addr_list mc; /* Multicast mac addresses
>> */
>> @@ -1633,6 +1646,7 @@ struct packet_offload {
>> #define NETDEV_NOTIFY_PEERS 0x0013
>> #define NETDEV_JOIN 0x0014
>> #define NETDEV_CHANGEUPPER 0x0015
>> +#define NETDEV_CHANGE_PHYS_PORT 0x0016
>>
>> extern int register_netdevice_notifier(struct notifier_block *nb);
>> extern int unregister_netdevice_notifier(struct notifier_block *nb);
>> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
>> index 981fed3..b77ebe6 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(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct net_device *net = to_net_dev(dev);
>> + ssize_t ret = -EINVAL;
>> + unsigned char len;
>> +
>> + read_lock(&dev_base_lock);
>> + if (dev_isalive(net)) {
>> + len = net->phys_port.port_id_len;
>> + if (!len)
>> + ret = 0;
>> + else
>> + ret = sysfs_format_mac(buf,
>> net->phys_port.port_id,
>> + len);
>> + }
>> + read_unlock(&dev_base_lock);
>> +
>> + 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, S_IRUGO, show_phys_port, NULL),
>> {}
>> };
>>
>> --
>> 1.8.0.1
>>
>> --
>> With regards,
>> Narendra K
>> Linux Engineering
>> Dell Inc.
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists