Re: [patch net-next RFC 07/12] dsa: implement ndo_swdev_get_id

From: Florian Fainelli

2014-08-21 10:06 GMT-07:00 Jiri Pirko [off-list ref]:

quoted

Thu, Aug 21, 2014 at 06:56:13PM CEST, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:

quoted

2014-08-21 9:19 GMT-07:00 Jiri Pirko [off-list ref]:

quoted

Signed-off-by: Jiri Pirko <redacted>
---
 net/dsa/Kconfig |  2 +-
 net/dsa/slave.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index f5eede1..66c445a 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig

@@ -1,6 +1,6 @@
 config HAVE_NET_DSA
        def_bool y
-       depends on NETDEVICES && !S390
+       depends on NETDEVICES && NET_SWITCHDEV && !S390

 # Drivers must select NET_DSA and the appropriate tagging format

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 45a1e34..e069ba3 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c

@@ -171,6 +171,19 @@ static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int

cmd)

quoted

quoted

quoted

        return -EOPNOTSUPP;
 }

+static int dsa_slave_swdev_get_id(struct net_device *dev,
+                                 struct netdev_phys_item_id *psid)
+{
+       struct dsa_slave_priv *p = netdev_priv(dev);
+       struct dsa_switch *ds = p->parent;
+       u64 tmp = (u64) ds;
+
+       /* TODO: add more sophisticated id generation */
+       memcpy(&psid->id, &tmp, sizeof(tmp));
+       psid->id_len = sizeof(tmp);

There is already an unique id generated, which is the index in the
switch tree, and which is stored in struct dsa_switch, so this could
probably be simplified to:

psid->id = ds->index

That index is 0..n if I understand that correctly. That is not enough.
The point is to have unique id for every chip in the system. If we would
have 0,1,2... the collision is very likely.

Good point, so an unique index for DSA switches could look like the
DSA platform device id plus the switch index in the tree..., but then
we would need something like (pdev->id << N) | switch index, so that
would not give a consistent naming scheme across different devices.

Do you also need to worry about the 'lifetime' of these ids?
In which case some of the high bits need to be used as 'generation number'.

	David

Maybe we are just better with using the Linux IDR API in include/linux/idr.h?
--
Florian