[RFC PATCH net-next] mlx4_en: Do not set dev_id to port number

[RFC PATCH net-next] mlx4_en: Do not set dev_id to port number

[Narendra_K]

Port number should not be stored in dev_id. 'dev_id' field was
intended to be used to differentiate between multiple devices
which share the same MAC address.

Signed-off-by: Narendra K <narendra_k@dell.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index ade276c..ca618e0 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2138,7 +2138,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	netif_set_real_num_rx_queues(dev, prof->rx_ring_num);
 
 	SET_NETDEV_DEV(dev, &mdev->dev->pdev->dev);
-	dev->dev_id =  port - 1;
 
 	/*
 	 * Initialize driver private data
-- 
1.8.0.1

-- 
With regards,
Narendra K
Linux Engineering
Dell Inc.

Re: [RFC PATCH net-next] mlx4_en: Do not set dev_id to port number

[Or Gerlitz]

On Fri, Jun 14, 2013 at 6:06 PM, <Narendra_K@dell.com> wrote:
> Port number should not be stored in dev_id. 'dev_id' field was
> intended to be used to differentiate between multiple devices
> which share the same MAC address.

Maybe you want to say that dev_id is **now** intended to differentiate
between multiple devices which share the same MAC address? the reason
that mlx4_en and IPoIB use this field is

1. the Mellanox exposes one PCI function but has two ports with a
netdevice set on each

2. there was no standard way to expose the port number used by two
netdevices that share the same PCI function

3. here http://patchwork.ozlabs.org/patch/53547/ we were suggested by
the netdev maintainers to use that field

This patch introduces a regression and can't get in without a
replacment mechanism.

Or.

Re: [RFC PATCH net-next] mlx4_en: Do not set dev_id to port number

[Ben Hutchings]

On Fri, 2013-06-14 at 22:55 +0300, Or Gerlitz wrote:
> On Fri, Jun 14, 2013 at 6:06 PM, <Narendra_K@dell.com> wrote:
> > Port number should not be stored in dev_id. 'dev_id' field was
> > intended to be used to differentiate between multiple devices
> > which share the same MAC address.
> 
> Maybe you want to say that dev_id is **now** intended to differentiate
> between multiple devices which share the same MAC address?

That was the original intent but it wasn't well-documented.  (Hence
several of us driver writers have made a similar mistake.)

> the reason that mlx4_en and IPoIB use this field is
> 
> 1. the Mellanox exposes one PCI function but has two ports with a
> netdevice set on each
> 
> 2. there was no standard way to expose the port number used by two
> netdevices that share the same PCI function
> 
> 3. here http://patchwork.ozlabs.org/patch/53547/ we were suggested by
> the netdev maintainers to use that field
> 
> This patch introduces a regression and can't get in without a
> replacment mechanism.

If you have userland tools relying on this then I agree it should be
left alone, but maybe add a comment explaining why the field is used in
an unusual way.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC PATCH net-next] mlx4_en: Do not set dev_id to port number

[Or Gerlitz]

On Fri, Jun 14, 2013 at 11:14 PM, Ben Hutchings
<bhutchings@solarflare.com> wrote:

> If you have userland tools relying on this then I agree it should be
> left alone, but maybe add a comment explaining why the field is used in
> an unusual way.

we were not aware that it used in unsual way, just used that b/c were told
too, not sure what to put in the comment. As for userland tools, I know
about internal regression tools and some clould related scripts that use
that.