| Message ID | 20130614150624.GA1316@fedora-17-guest.blr.amer.dell.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
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. -- 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
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.
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. -- 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
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
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(-)