[PATCH] netdev: storage address support

[PATCH] netdev: storage address support

[Jeff Kirsher]

From: Chris Leech <christopher.leech@intel.com>

Fibre Channel over Ethernet (FCoE) uses a second MAC address dedicated to FCoE
traffic on an Ethernet link.  There are two supported addressing modes,
negotiated at fabric discovery time with the gateway switch or Fibre Channel
Forwarder (FCF).

1) Fabric Provided MAC Address (FPMA)
The MAC address is dynamically generated from the assigned FC ID.  The FCoE
stack supports this today and adds the address to the device with the
dev_unicast_add() API.

2) Server Provided MAC Address (SPMA)
The server indicates to the FCF the address it would like to use for FCoE
traffic.  It's expected that SPMA capable interfaces will have a storage
dedicated MAC address programed into EPROM, flash, or other non-volatile
storage which the driver will be able to provide to the FCoE stack.

This adds a net_device_ops function to query a driver for a dedicated storage
address.

If ndo_get_storage_address is implemented, then the address will also be
exposed as a sysfs attribute.  In order to do that, a new optional attrs group
is added to the net_device, with the visibility of each attribute protected by
a call to netdev_show_optional_attr().

Signed-off-by: Chris Leech <christopher.leech@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 include/linux/netdevice.h |    3 ++-
 net/core/net-sysfs.c      |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3116745..3964ca6 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -602,6 +602,7 @@ struct net_device_ops {
 	int			(*ndo_fcoe_ddp_done)(struct net_device *dev,
 						     u16 xid);
 #endif
+	void *			(*ndo_get_storage_address)(struct net_device *dev);
 };
 
 /*
@@ -846,7 +847,7 @@ struct net_device
 	/* class/net/name entry */
 	struct device		dev;
 	/* space for optional statistics and wireless sysfs groups */
-	struct attribute_group  *sysfs_groups[3];
+	struct attribute_group  *sysfs_groups[4];
 
 	/* rtnetlink link ops */
 	const struct rtnl_link_ops *rtnl_link_ops;
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 2da59a0..da68ae6 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -266,6 +266,46 @@ static struct device_attribute net_class_attributes[] = {
 	{}
 };
 
+static ssize_t show_storage_address(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct net_device *net = to_net_dev(dev);
+	const struct net_device_ops *ops = net->netdev_ops;
+	u8 addr[MAX_ADDR_LEN];
+
+	if (!ops->ndo_get_storage_address)
+		return -EINVAL;
+	if (dev_isalive(net)) {
+		memcpy(addr, ops->ndo_get_storage_address(net), net->addr_len);
+		return sysfs_format_mac(buf, addr, net->addr_len);
+	}
+	return -EINVAL;
+}
+
+DEVICE_ATTR(storage_address, S_IRUGO, show_storage_address, NULL);
+
+static struct attribute *optional_attrs[] = {
+	&dev_attr_storage_address.attr,
+	NULL
+};
+
+static mode_t netdev_show_optional_attr(struct kobject *kobj,
+					struct attribute *attr, int i)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct net_device *net = to_net_dev(dev);
+	const struct net_device_ops *ops = net->netdev_ops;
+
+	if (attr == &dev_attr_storage_address.attr)
+		return ops->ndo_get_storage_address ? S_IRUGO : 0;
+	return 0;
+}
+
+static struct attribute_group optional_group = {
+	.is_visible = netdev_show_optional_attr,
+	.attrs = optional_attrs,
+};
+
 /* Show a given an attribute in the statistics group */
 static ssize_t netstat_show(const struct device *d,
 			    struct device_attribute *attr, char *buf,
@@ -502,6 +542,7 @@ int netdev_register_kobject(struct net_device *net)
 
 #ifdef CONFIG_SYSFS
 	*groups++ = &netstat_group;
+	*groups++ = &optional_group;
 
 #ifdef CONFIG_WIRELESS_EXT_SYSFS
 	if (net->wireless_handlers && net->wireless_handlers->get_wireless_stats)

Re: [PATCH] netdev: storage address support

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 24 Apr 2009 01:03:50 -0700

> 2) Server Provided MAC Address (SPMA)
> The server indicates to the FCF the address it would like to use for FCoE
> traffic.  It's expected that SPMA capable interfaces will have a storage
> dedicated MAC address programed into EPROM, flash, or other non-volatile
> storage which the driver will be able to provide to the FCoE stack.
> 
> This adds a net_device_ops function to query a driver for a dedicated storage
> address.
> 
> If ndo_get_storage_address is implemented, then the address will also be
> exposed as a sysfs attribute.  In order to do that, a new optional attrs group
> is added to the net_device, with the visibility of each attribute protected by
> a call to netdev_show_optional_attr().

This should be what is currently provided in netdev->perm_addr[]

I really see no difference.

Re: [PATCH] netdev: storage address support

[Chris Leech]

On Mon, Apr 27, 2009 at 03:01:03AM -0700, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Fri, 24 Apr 2009 01:03:50 -0700
> 
> > 2) Server Provided MAC Address (SPMA)
> > The server indicates to the FCF the address it would like to use for FCoE
> > traffic.  It's expected that SPMA capable interfaces will have a storage
> > dedicated MAC address programed into EPROM, flash, or other non-volatile
> > storage which the driver will be able to provide to the FCoE stack.
> > 
> > This adds a net_device_ops function to query a driver for a dedicated storage
> > address.
> > 
> > If ndo_get_storage_address is implemented, then the address will also be
> > exposed as a sysfs attribute.  In order to do that, a new optional attrs group
> > is added to the net_device, with the visibility of each attribute protected by
> > a call to netdev_show_optional_attr().
> 
> This should be what is currently provided in netdev->perm_addr[]
> 
> I really see no difference.

There seems to be some desire to use separate MAC address for LAN and
SAN traffic on a converged network, even when using the server provided
addressing mode for FCoE.  So I'm looking at a device that has an extra
MAC address in it's EEPROM, that's intended to be used for SAN traffic
only.

At first glance Jiri's "list of device addresses" patch seems to be
heading towards a more general approach to having multiple MAC
addresses, without providing any sort of intent on how they should be
used.  But given that it's trying to solve a bonding + bridging issue,
the addresses involved seem fairly dynamic and I'm not sure how it
differs much from the existing uc_list.

Ignoring the issue of intended use for the moment, if an ethernet driver
wanted to advertise several MAC addresses to the system how should it go
about that?

- Chris

Re: [PATCH] netdev: storage address support

[David Miller]

From: Chris Leech <christopher.leech@intel.com>
Date: Mon, 27 Apr 2009 15:28:00 -0700

> Ignoring the issue of intended use for the moment, if an ethernet driver
> wanted to advertise several MAC addresses to the system how should it go
> about that?

Now that's a more interesting question.

It seems better to formalize this.   It can be an ethtool
callback or whatever, but what it should do is return
an array of addresses, types, and perhaps even indexes
with types.

So you could return LAN1, LAN2, LAN3, and SAN1.

Re: [PATCH] netdev: storage address support

[Jiri Pirko]

Tue, Apr 28, 2009 at 03:48:54AM CEST, davem@davemloft.net wrote:
>From: Chris Leech <christopher.leech@intel.com>
>Date: Mon, 27 Apr 2009 15:28:00 -0700
>
>> Ignoring the issue of intended use for the moment, if an ethernet driver
>> wanted to advertise several MAC addresses to the system how should it go
>> about that?
>
>Now that's a more interesting question.
>
>It seems better to formalize this.   It can be an ethtool
>callback or whatever, but what it should do is return
>an array of addresses, types, and perhaps even indexes
>with types.

Well the list of device addresses from my patch can pretty much solve this.
There can be additional field in struct netdev_hw_addr to store flags to
identify the mac address type (like primary, lan, san, slave, etc).

I think it would be better to do this in general (in struct net_device) then
inside each driver exported by ethtool or whatever.

>
>So you could return LAN1, LAN2, LAN3, and SAN1.
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netdev: storage address support

[David Miller]

From: Jiri Pirko <jpirko@redhat.com>
Date: Tue, 28 Apr 2009 10:30:48 +0200

> Tue, Apr 28, 2009 at 03:48:54AM CEST, davem@davemloft.net wrote:
>>From: Chris Leech <christopher.leech@intel.com>
>>Date: Mon, 27 Apr 2009 15:28:00 -0700
>>
>>> Ignoring the issue of intended use for the moment, if an ethernet driver
>>> wanted to advertise several MAC addresses to the system how should it go
>>> about that?
>>
>>Now that's a more interesting question.
>>
>>It seems better to formalize this.   It can be an ethtool
>>callback or whatever, but what it should do is return
>>an array of addresses, types, and perhaps even indexes
>>with types.
> 
> Well the list of device addresses from my patch can pretty much solve this.
> There can be additional field in struct netdev_hw_addr to store flags to
> identify the mac address type (like primary, lan, san, slave, etc).
> 
> I think it would be better to do this in general (in struct net_device) then
> inside each driver exported by ethtool or whatever.

Agreed, can you update your patch to provide such a 'type' tag?

Thanks!

Re: [PATCH] netdev: storage address support

[Jiri Pirko]

Tue, Apr 28, 2009 at 11:12:49AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jpirko@redhat.com>
>Date: Tue, 28 Apr 2009 10:30:48 +0200
>
>> Tue, Apr 28, 2009 at 03:48:54AM CEST, davem@davemloft.net wrote:
>>>From: Chris Leech <christopher.leech@intel.com>
>>>Date: Mon, 27 Apr 2009 15:28:00 -0700
>>>
>>>> Ignoring the issue of intended use for the moment, if an ethernet driver
>>>> wanted to advertise several MAC addresses to the system how should it go
>>>> about that?
>>>
>>>Now that's a more interesting question.
>>>
>>>It seems better to formalize this.   It can be an ethtool
>>>callback or whatever, but what it should do is return
>>>an array of addresses, types, and perhaps even indexes
>>>with types.
>> 
>> Well the list of device addresses from my patch can pretty much solve this.
>> There can be additional field in struct netdev_hw_addr to store flags to
>> identify the mac address type (like primary, lan, san, slave, etc).
>> 
>> I think it would be better to do this in general (in struct net_device) then
>> inside each driver exported by ethtool or whatever.
>
>Agreed, can you update your patch to provide such a 'type' tag?

Ok, I'll do that. What types do you have on the top of your head?

>
>Thanks!

Re: [PATCH] netdev: storage address support

[David Miller]

From: Jiri Pirko <jpirko@redhat.com>
Date: Tue, 28 Apr 2009 12:18:17 +0200

> Tue, Apr 28, 2009 at 11:12:49AM CEST, davem@davemloft.net wrote:
>>Agreed, can you update your patch to provide such a 'type' tag?
> 
> Ok, I'll do that. What types do you have on the top of your head?

Just start with SAN, LAN, and the ones you need for your work.