All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] IB/ipoib: Use dev_port to disambiguate port numbers
@ 2018-08-30 18:22 Arseny Maslennikov
  2018-08-30 18:22 ` [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface " Arseny Maslennikov
  2018-08-30 18:22 ` [PATCH v2 2/2] Documentation/ABI: document /sys/class/net/*/dev_port Arseny Maslennikov
  0 siblings, 2 replies; 6+ messages in thread
From: Arseny Maslennikov @ 2018-08-30 18:22 UTC (permalink / raw)
  To: linux-rdma; +Cc: Arseny Maslennikov, Doug Ledford, Jason Gunthorpe, netdev

Pre-3.15 userspace had trouble distinguishing different ports
of a NIC on a single PCI bus/device/function. To solve this,
a sysfs field `dev_port' was introduced quite a while ago
(commit v3.14-rc3-739-g3f85944fe207), and some relevant device
drivers were fixed to use it, but not in case of IPoIB.

The convention for some reason never got documented in the kernel, but
was immediately adopted by userspace (notably udev[1][2], biosdevname[3])

3/3 documents the sysfs field — that's why I'm CC-ing netdev.

This series was tested on and applies to 4.19-rc1.

[1] https://lists.freedesktop.org/archives/systemd-devel/2014-June/020788.html
[2] https://lists.freedesktop.org/archives/systemd-devel/2014-July/020804.html
[3] https://github.com/CloudAutomationNTools/biosdevname/blob/c795d51dd93a5309652f0d635f12a3ecfabfaa72/src/eths.c#L38

v1->v2: replace a line instead of inserting and then removing.

Arseny Maslennikov (2):
  IB/ipoib: Use dev_port to expose network interface port numbers
  Documentation/ABI: document /sys/class/net/*/dev_port

 Documentation/ABI/testing/sysfs-class-net | 10 ++++++++++
 drivers/infiniband/ulp/ipoib/ipoib_main.c |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
2.18.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface port numbers
  2018-08-30 18:22 [PATCH v2 0/2] IB/ipoib: Use dev_port to disambiguate port numbers Arseny Maslennikov
@ 2018-08-30 18:22 ` Arseny Maslennikov
  2018-08-30 20:17   ` Doug Ledford
  2018-08-30 18:22 ` [PATCH v2 2/2] Documentation/ABI: document /sys/class/net/*/dev_port Arseny Maslennikov
  1 sibling, 1 reply; 6+ messages in thread
From: Arseny Maslennikov @ 2018-08-30 18:22 UTC (permalink / raw)
  To: linux-rdma; +Cc: Arseny Maslennikov, Doug Ledford, Jason Gunthorpe, netdev

Some InfiniBand network devices have multiple ports on the same PCI
function. This initializes the `dev_port' sysfs field of those
network interfaces with their port number.

Prior to this the kernel erroneously used the `dev_id' sysfs
field of those network interfaces to convey the port number to userspace.

The use of `dev_id' was considered correct until Linux 3.15,
when another field, `dev_port', was defined for this particular
purpose and `dev_id' was reserved for distinguishing stacked ifaces
(e.g: VLANs) with the same hardware address as their parent device.

Similar fixes to net/mlx4_en and many other drivers, which started
exporting this information through `dev_id' before 3.15, were accepted
into the kernel 4 years ago.
See 76a066f2a2a0 (`net/mlx4_en: Expose port number through sysfs').

Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index e3d28f9ad9c0..ba16a63ee303 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -1880,7 +1880,7 @@ static int ipoib_parent_init(struct net_device *ndev)
 	       sizeof(union ib_gid));
 
 	SET_NETDEV_DEV(priv->dev, priv->ca->dev.parent);
-	priv->dev->dev_id = priv->port - 1;
+	priv->dev->dev_port = priv->port - 1;
 
 	return 0;
 }
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] Documentation/ABI: document /sys/class/net/*/dev_port
  2018-08-30 18:22 [PATCH v2 0/2] IB/ipoib: Use dev_port to disambiguate port numbers Arseny Maslennikov
  2018-08-30 18:22 ` [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface " Arseny Maslennikov
@ 2018-08-30 18:22 ` Arseny Maslennikov
  1 sibling, 0 replies; 6+ messages in thread
From: Arseny Maslennikov @ 2018-08-30 18:22 UTC (permalink / raw)
  To: linux-rdma; +Cc: Arseny Maslennikov, Doug Ledford, Jason Gunthorpe, netdev

The sysfs field was introduced 4 years ago along with fixes to various
drivers that erroneously used `dev_id' for that purpose, but it was not
properly documented anywhere.
See commit v3.14-rc3-739-g3f85944fe207.

Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
 Documentation/ABI/testing/sysfs-class-net | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-net b/Documentation/ABI/testing/sysfs-class-net
index 2f1788111cd9..1593d8997ade 100644
--- a/Documentation/ABI/testing/sysfs-class-net
+++ b/Documentation/ABI/testing/sysfs-class-net
@@ -91,6 +91,16 @@ Description:
 		stacked (e.g: VLAN interfaces) but still have the same MAC
 		address as their parent device.
 
+What:		/sys/class/net/<iface>/dev_port
+Date:		February 2014
+KernelVersion:	3.15
+Contact:	netdev@vger.kernel.org
+Description:
+		Indicates the port number of this network device, formatted
+		as a decimal value. Some NICs have multiple independent ports
+		on the same PCI bus, device and function. This field allows
+		userspace to distinguish the respective interfaces.
+
 What:		/sys/class/net/<iface>/dormant
 Date:		March 2006
 KernelVersion:	2.6.17
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface port numbers
  2018-08-30 18:22 ` [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface " Arseny Maslennikov
@ 2018-08-30 20:17   ` Doug Ledford
  2018-08-31  8:57     ` Arseny Maslennikov
  0 siblings, 1 reply; 6+ messages in thread
From: Doug Ledford @ 2018-08-30 20:17 UTC (permalink / raw)
  To: Arseny Maslennikov, linux-rdma; +Cc: Jason Gunthorpe, netdev

[-- Attachment #1: Type: text/plain, Size: 2112 bytes --]

On Thu, 2018-08-30 at 21:22 +0300, Arseny Maslennikov wrote:
> Some InfiniBand network devices have multiple ports on the same PCI
> function. This initializes the `dev_port' sysfs field of those
> network interfaces with their port number.
> 
> Prior to this the kernel erroneously used the `dev_id' sysfs
> field of those network interfaces to convey the port number to userspace.
> 
> The use of `dev_id' was considered correct until Linux 3.15,
> when another field, `dev_port', was defined for this particular
> purpose and `dev_id' was reserved for distinguishing stacked ifaces
> (e.g: VLANs) with the same hardware address as their parent device.
> 
> Similar fixes to net/mlx4_en and many other drivers, which started
> exporting this information through `dev_id' before 3.15, were accepted
> into the kernel 4 years ago.
> See 76a066f2a2a0 (`net/mlx4_en: Expose port number through sysfs').
> 
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index e3d28f9ad9c0..ba16a63ee303 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -1880,7 +1880,7 @@ static int ipoib_parent_init(struct net_device *ndev)
>  	       sizeof(union ib_gid));
>  
>  	SET_NETDEV_DEV(priv->dev, priv->ca->dev.parent);
> -	priv->dev->dev_id = priv->port - 1;
> +	priv->dev->dev_port = priv->port - 1;

I don't know that we can't do this.  At least not yet.  Expose the new
item to make us compliant with the new docs, and deprecate the old sysfs
item, but we can't just yank the old item.  Existing tools/scripts might
(probably) rely on it (existing tools already special case IPoIB
interfaces and we'll need to make sure they don't special case this
element too).

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface port numbers
  2018-08-30 20:17   ` Doug Ledford
@ 2018-08-31  8:57     ` Arseny Maslennikov
  2018-08-31 20:58       ` Doug Ledford
  0 siblings, 1 reply; 6+ messages in thread
From: Arseny Maslennikov @ 2018-08-31  8:57 UTC (permalink / raw)
  To: Doug Ledford; +Cc: linux-rdma, Jason Gunthorpe, netdev

[-- Attachment #1: Type: text/plain, Size: 2680 bytes --]

On Thu, Aug 30, 2018 at 04:17:58PM -0400, Doug Ledford wrote:
> On Thu, 2018-08-30 at 21:22 +0300, Arseny Maslennikov wrote:
> > Some InfiniBand network devices have multiple ports on the same PCI
> > function. This initializes the `dev_port' sysfs field of those
> > network interfaces with their port number.
> > 
> > Prior to this the kernel erroneously used the `dev_id' sysfs
> > field of those network interfaces to convey the port number to userspace.
> > 
> > The use of `dev_id' was considered correct until Linux 3.15,
> > when another field, `dev_port', was defined for this particular
> > purpose and `dev_id' was reserved for distinguishing stacked ifaces
> > (e.g: VLANs) with the same hardware address as their parent device.
> > 
> > Similar fixes to net/mlx4_en and many other drivers, which started
> > exporting this information through `dev_id' before 3.15, were accepted
> > into the kernel 4 years ago.
> > See 76a066f2a2a0 (`net/mlx4_en: Expose port number through sysfs').
> > 
> > Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> > ---
> >  drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > index e3d28f9ad9c0..ba16a63ee303 100644
> > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > @@ -1880,7 +1880,7 @@ static int ipoib_parent_init(struct net_device *ndev)
> >  	       sizeof(union ib_gid));
> >  
> >  	SET_NETDEV_DEV(priv->dev, priv->ca->dev.parent);
> > -	priv->dev->dev_id = priv->port - 1;
> > +	priv->dev->dev_port = priv->port - 1;
> 
> I don't know that we can't do this.  At least not yet.  Expose the new
> item to make us compliant with the new docs, and deprecate the old sysfs
> item, but we can't just yank the old item.  Existing tools/scripts might
> (probably) rely on it (existing tools already special case IPoIB
> interfaces and we'll need to make sure they don't special case this
> element too).

I'm good with keeping both items for a (probably long) while to not break
things. But how exactly should we notify users of the deprecation, so they
don't special case this again? A comment in the code seems too little —
everyone's obviously too busy to look there and stumble upon that.
A distinct notice in the doc seems too much. I can't think of another place
for the deprecation notice where people would take note of it, however.

Anyway: would it be OK to just restore both items and put a small note in
dev_id's doc entry? If yes, I'll then send a v3.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface port numbers
  2018-08-31  8:57     ` Arseny Maslennikov
@ 2018-08-31 20:58       ` Doug Ledford
  0 siblings, 0 replies; 6+ messages in thread
From: Doug Ledford @ 2018-08-31 20:58 UTC (permalink / raw)
  To: Arseny Maslennikov; +Cc: linux-rdma, Jason Gunthorpe, netdev

[-- Attachment #1: Type: text/plain, Size: 3249 bytes --]

On Fri, 2018-08-31 at 11:57 +0300, Arseny Maslennikov wrote:
> On Thu, Aug 30, 2018 at 04:17:58PM -0400, Doug Ledford wrote:
> > On Thu, 2018-08-30 at 21:22 +0300, Arseny Maslennikov wrote:
> > > Some InfiniBand network devices have multiple ports on the same PCI
> > > function. This initializes the `dev_port' sysfs field of those
> > > network interfaces with their port number.
> > > 
> > > Prior to this the kernel erroneously used the `dev_id' sysfs
> > > field of those network interfaces to convey the port number to userspace.
> > > 
> > > The use of `dev_id' was considered correct until Linux 3.15,
> > > when another field, `dev_port', was defined for this particular
> > > purpose and `dev_id' was reserved for distinguishing stacked ifaces
> > > (e.g: VLANs) with the same hardware address as their parent device.
> > > 
> > > Similar fixes to net/mlx4_en and many other drivers, which started
> > > exporting this information through `dev_id' before 3.15, were accepted
> > > into the kernel 4 years ago.
> > > See 76a066f2a2a0 (`net/mlx4_en: Expose port number through sysfs').
> > > 
> > > Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> > > ---
> > >  drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > index e3d28f9ad9c0..ba16a63ee303 100644
> > > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > @@ -1880,7 +1880,7 @@ static int ipoib_parent_init(struct net_device *ndev)
> > >  	       sizeof(union ib_gid));
> > >  
> > >  	SET_NETDEV_DEV(priv->dev, priv->ca->dev.parent);
> > > -	priv->dev->dev_id = priv->port - 1;
> > > +	priv->dev->dev_port = priv->port - 1;
> > 
> > I don't know that we can't do this.  At least not yet.  Expose the new
> > item to make us compliant with the new docs, and deprecate the old sysfs
> > item, but we can't just yank the old item.  Existing tools/scripts might
> > (probably) rely on it (existing tools already special case IPoIB
> > interfaces and we'll need to make sure they don't special case this
> > element too).
> 
> I'm good with keeping both items for a (probably long) while to not break
> things. But how exactly should we notify users of the deprecation, so they
> don't special case this again? A comment in the code seems too little —
> everyone's obviously too busy to look there and stumble upon that.
> A distinct notice in the doc seems too much. I can't think of another place
> for the deprecation notice where people would take note of it, however.
> 
> Anyway: would it be OK to just restore both items and put a small note in
> dev_id's doc entry? If yes, I'll then send a v3.

A warn_on_once in the code so that when someone reads the dev_id entry,
we get a deprecation warning in the dmesg output at info level would be
my suggestion.  Have it output the command name as part of the warning
so we know what tools are using it.

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-08-31 20:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-30 18:22 [PATCH v2 0/2] IB/ipoib: Use dev_port to disambiguate port numbers Arseny Maslennikov
2018-08-30 18:22 ` [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface " Arseny Maslennikov
2018-08-30 20:17   ` Doug Ledford
2018-08-31  8:57     ` Arseny Maslennikov
2018-08-31 20:58       ` Doug Ledford
2018-08-30 18:22 ` [PATCH v2 2/2] Documentation/ABI: document /sys/class/net/*/dev_port Arseny Maslennikov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.