[PATCH] Use dev_port for the ID of a network device.

[PATCH] Use dev_port for the ID of a network device.

[Thadeu Lima de Souza Cascardo]

For network devices on the same PCI function, dev_id should not be used,
since its purpose is for IPv6 support on interfaces with the same MAC
address.

The new dev_port sysfs attribute should be used when it is found. When
it is not, using dev_id might work.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 src/udev/udev-builtin-net_id.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
index c80c30a..6de9c98 100644
--- a/src/udev/udev-builtin-net_id.c
+++ b/src/udev/udev-builtin-net_id.c
@@ -186,9 +186,14 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) {
                 return -ENOENT;
 
         /* kernel provided multi-device index */
-        attr = udev_device_get_sysattr_value(dev, "dev_id");
-        if (attr)
+        attr = udev_device_get_sysattr_value(dev, "dev_port");
+        if (attr) {
                 dev_id = strtol(attr, NULL, 16);
+        } else {
+                attr = udev_device_get_sysattr_value(dev, "dev_id");
+                if (attr)
+                        dev_id = strtol(attr, NULL, 16);
+        }
 
         /* compose a name based on the raw kernel's PCI bus, slot numbers */
         s = names->pci_path;
-- 
1.7.1

Re: [PATCH] Use dev_port for the ID of a network device.

[Kay Sievers]

On Mon, Jun 30, 2014 at 10:36 PM, Thadeu Lima de Souza Cascardo
<cascardo@linux.vnet.ibm.com> wrote:
> For network devices on the same PCI function, dev_id should not be used,
> since its purpose is for IPv6 support on interfaces with the same MAC
> address.
>
> The new dev_port sysfs attribute should be used when it is found. When
> it is not, using dev_id might work.

I don't see a problem switching this over, but why would we keep using
dev_id if it is not the right thing to use?

Kay

Re: [systemd-devel] [PATCH] Use dev_port for the ID of a network device.

[Thadeu Lima de Souza Cascardo]

On Tue, Jul 01, 2014 at 02:33:19AM +0200, Kay Sievers wrote:
> On Mon, Jun 30, 2014 at 10:36 PM, Thadeu Lima de Souza Cascardo
> <cascardo@linux.vnet.ibm.com> wrote:
> > For network devices on the same PCI function, dev_id should not be used,
> > since its purpose is for IPv6 support on interfaces with the same MAC
> > address.
> >
> > The new dev_port sysfs attribute should be used when it is found. When
> > it is not, using dev_id might work.
> 
> I don't see a problem switching this over, but why would we keep using
> dev_id if it is not the right thing to use?
> 
> Kay
> 

Because dev_port has only been introduced into Linux 3.15, and some
drivers used dev_id before Linux 3.10. It's not an ideal situation, so I
think it's important to keep some compatibility for the time being. Or
we could simply drop dev_id, and force everyone to do the right thing
and backport dev_port support and fix their drivers.

Cascardo.

Re: [systemd-devel] [PATCH] Use dev_port for the ID of a network device.

[Tom Gundersen]

On Tue, Jul 1, 2014 at 2:32 PM, Thadeu Lima de Souza Cascardo
<cascardo@linux.vnet.ibm.com> wrote:
> On Tue, Jul 01, 2014 at 02:33:19AM +0200, Kay Sievers wrote:
>> On Mon, Jun 30, 2014 at 10:36 PM, Thadeu Lima de Souza Cascardo
>> <cascardo@linux.vnet.ibm.com> wrote:
>> > For network devices on the same PCI function, dev_id should not be used,
>> > since its purpose is for IPv6 support on interfaces with the same MAC
>> > address.
>> >
>> > The new dev_port sysfs attribute should be used when it is found. When
>> > it is not, using dev_id might work.
>>
>> I don't see a problem switching this over, but why would we keep using
>> dev_id if it is not the right thing to use?
>>
>> Kay
>>
>
> Because dev_port has only been introduced into Linux 3.15, and some
> drivers used dev_id before Linux 3.10. It's not an ideal situation, so I
> think it's important to keep some compatibility for the time being. Or
> we could simply drop dev_id, and force everyone to do the right thing
> and backport dev_port support and fix their drivers.

If this fixes a real issue, I guess it would be nice to push the
dev_port stuff to -stable. If it is accepted it would solve the
backwards compatibility problem, and we could just drop dev_id from
udev.

Cheers,

Tom

Re: [systemd-devel] [PATCH] Use dev_port for the ID of a network device.

[Kay Sievers]

On Tue, Jul 1, 2014 at 2:45 PM, Tom Gundersen <teg@jklm.no> wrote:
> On Tue, Jul 1, 2014 at 2:32 PM, Thadeu Lima de Souza Cascardo
> <cascardo@linux.vnet.ibm.com> wrote:
>> On Tue, Jul 01, 2014 at 02:33:19AM +0200, Kay Sievers wrote:
>>> On Mon, Jun 30, 2014 at 10:36 PM, Thadeu Lima de Souza Cascardo
>>> <cascardo@linux.vnet.ibm.com> wrote:
>>> > For network devices on the same PCI function, dev_id should not be used,
>>> > since its purpose is for IPv6 support on interfaces with the same MAC
>>> > address.
>>> >
>>> > The new dev_port sysfs attribute should be used when it is found. When
>>> > it is not, using dev_id might work.
>>>
>>> I don't see a problem switching this over, but why would we keep using
>>> dev_id if it is not the right thing to use?
>>>
>>> Kay
>>>
>>
>> Because dev_port has only been introduced into Linux 3.15, and some
>> drivers used dev_id before Linux 3.10. It's not an ideal situation, so I
>> think it's important to keep some compatibility for the time being. Or
>> we could simply drop dev_id, and force everyone to do the right thing
>> and backport dev_port support and fix their drivers.
>
> If this fixes a real issue, I guess it would be nice to push the
> dev_port stuff to -stable. If it is accepted it would solve the
> backwards compatibility problem, and we could just drop dev_id from
> udev.

Let's drop it now and mention it in NEWS. That way, distros which
really care can add the compat hack to the code.

Kay

Re: [systemd-devel] [PATCH] Use dev_port for the ID of a network device.

[Amir Vadai]

On 7/1/2014 3:59 PM, Kay Sievers wrote:
> On Tue, Jul 1, 2014 at 2:45 PM, Tom Gundersen <teg@jklm.no> wrote:
>> On Tue, Jul 1, 2014 at 2:32 PM, Thadeu Lima de Souza Cascardo
>> <cascardo@linux.vnet.ibm.com> wrote:
>>> On Tue, Jul 01, 2014 at 02:33:19AM +0200, Kay Sievers wrote:
>>>> On Mon, Jun 30, 2014 at 10:36 PM, Thadeu Lima de Souza Cascardo
>>>> <cascardo@linux.vnet.ibm.com> wrote:
>>>>> For network devices on the same PCI function, dev_id should not be used,
>>>>> since its purpose is for IPv6 support on interfaces with the same MAC
>>>>> address.
>>>>>
>>>>> The new dev_port sysfs attribute should be used when it is found. When
>>>>> it is not, using dev_id might work.
>>>>
>>>> I don't see a problem switching this over, but why would we keep using
>>>> dev_id if it is not the right thing to use?
>>>>
>>>> Kay
>>>>
>>>
>>> Because dev_port has only been introduced into Linux 3.15, and some
>>> drivers used dev_id before Linux 3.10. It's not an ideal situation, so I
>>> think it's important to keep some compatibility for the time being. Or
>>> we could simply drop dev_id, and force everyone to do the right thing
>>> and backport dev_port support and fix their drivers.
>>
>> If this fixes a real issue, I guess it would be nice to push the
>> dev_port stuff to -stable. If it is accepted it would solve the
>> backwards compatibility problem, and we could just drop dev_id from
>> udev.
> 
> Let's drop it now and mention it in NEWS. That way, distros which
> really care can add the compat hack to the code.
> 
> Kay
> --
> 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
> 

Acked-by: Amir Vadai <amirv@mellanox.com>

mlx4_en is one of those drivers that wrongly used dev_id and changed to
dev_port.
I guess that having a bug in driver that used dev_id the right way is
worse than bad interface name for drivers that wrongly used it.
Never the less - we should pull dev_port to stable, to minimize the damage.

Amir

Re: [PATCH] Use dev_port for the ID of a network device.

[Bjørn Mork]

Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> writes:

> diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
> index c80c30a..6de9c98 100644
> --- a/src/udev/udev-builtin-net_id.c
> +++ b/src/udev/udev-builtin-net_id.c
> @@ -186,9 +186,14 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) {
>                  return -ENOENT;
>  
>          /* kernel provided multi-device index */
> -        attr = udev_device_get_sysattr_value(dev, "dev_id");
> -        if (attr)
> +        attr = udev_device_get_sysattr_value(dev, "dev_port");
> +        if (attr) {
>                  dev_id = strtol(attr, NULL, 16);
> +        } else {
> +                attr = udev_device_get_sysattr_value(dev, "dev_id");
> +                if (attr)
> +                        dev_id = strtol(attr, NULL, 16);
> +        }
>  
>          /* compose a name based on the raw kernel's PCI bus, slot numbers */
>          s = names->pci_path;

Note that the base of the new attribute is 10, not 16:

bjorn@nemi:/usr/local/src/git/linux$ git grep dev_port net/core/
net/core/net-sysfs.c:NETDEVICE_SHOW_RO(dev_port, fmt_dec);
net/core/net-sysfs.c:   &dev_attr_dev_port.attr,
bjorn@nemi:/usr/local/src/git/linux$ git grep dev_id net/core/
net/core/net-sysfs.c:NETDEVICE_SHOW_RO(dev_id, fmt_hex);
net/core/net-sysfs.c:   &dev_attr_dev_id.attr,



Bjørn

Re: [PATCH] Use dev_port for the ID of a network device.

[Bjørn Mork]

FYI: I just got this back from systemd-devel-owner@lists.freedesktop.org :

 Please subscribe to this mailing list, all non-subscriber emails are
 automatically rejected by the server.
 
 If you actually don't want to receive the mails, stay subscribed but
 just disable the delivery in the mailman web interface. You will still
 be able to send messages that way.
 
 We were unable to cope with the amount of SPAM, and the moderation
 queue was not actually moderated by anybody, so rejection seemed more
 appropriate than no answer and no delivery to the list.
 
 Thanks for understanding.


Please do not crosspost between netdev and closed mailing lists.  It
splits the followup threads between those who are inside the closed
community and those who are not.

I've removed systemd-devel from the CC list of this email.

Thanks for understanding.


Bjørn

Re: [PATCH] Use dev_port for the ID of a network device.

[Thadeu Lima de Souza Cascardo]

On Tue, Jul 01, 2014 at 09:40:57AM +0200, Bjørn Mork wrote:
> FYI: I just got this back from systemd-devel-owner@lists.freedesktop.org :
> 
>  Please subscribe to this mailing list, all non-subscriber emails are
>  automatically rejected by the server.
>  
>  If you actually don't want to receive the mails, stay subscribed but
>  just disable the delivery in the mailman web interface. You will still
>  be able to send messages that way.
>  
>  We were unable to cope with the amount of SPAM, and the moderation
>  queue was not actually moderated by anybody, so rejection seemed more
>  appropriate than no answer and no delivery to the list.
>  
>  Thanks for understanding.
> 
> 
> Please do not crosspost between netdev and closed mailing lists.  It
> splits the followup threads between those who are inside the closed
> community and those who are not.
> 
> I've removed systemd-devel from the CC list of this email.
> 
> Thanks for understanding.
> 
> 
> Bjørn
> 

Not sure how, but my patch got through, and I don't recall being
subscribed, so I thought it was a an open list. Anyway, it's something
that I considered important to be considered by the two communities.

Kay raises up the matter of still using dev_id. Since we had drivers
that used that in the past, should we use it as a fallback for older
kernels where dev_port is not there?

Regards.
Cascardo.

Re: [PATCH] Use dev_port for the ID of a network device.

[Thadeu Lima de Souza Cascardo]

On Tue, Jul 01, 2014 at 08:45:56AM +0200, Bjørn Mork wrote:
> Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> writes:
> 
> > diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
> > index c80c30a..6de9c98 100644
> > --- a/src/udev/udev-builtin-net_id.c
> > +++ b/src/udev/udev-builtin-net_id.c
> > @@ -186,9 +186,14 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) {
> >                  return -ENOENT;
> >  
> >          /* kernel provided multi-device index */
> > -        attr = udev_device_get_sysattr_value(dev, "dev_id");
> > -        if (attr)
> > +        attr = udev_device_get_sysattr_value(dev, "dev_port");
> > +        if (attr) {
> >                  dev_id = strtol(attr, NULL, 16);
> > +        } else {
> > +                attr = udev_device_get_sysattr_value(dev, "dev_id");
> > +                if (attr)
> > +                        dev_id = strtol(attr, NULL, 16);
> > +        }
> >  
> >          /* compose a name based on the raw kernel's PCI bus, slot numbers */
> >          s = names->pci_path;
> 
> Note that the base of the new attribute is 10, not 16:
> 
> bjorn@nemi:/usr/local/src/git/linux$ git grep dev_port net/core/
> net/core/net-sysfs.c:NETDEVICE_SHOW_RO(dev_port, fmt_dec);
> net/core/net-sysfs.c:   &dev_attr_dev_port.attr,
> bjorn@nemi:/usr/local/src/git/linux$ git grep dev_id net/core/
> net/core/net-sysfs.c:NETDEVICE_SHOW_RO(dev_id, fmt_hex);
> net/core/net-sysfs.c:   &dev_attr_dev_id.attr,
> 
> 
> 
> Bjørn
> 

Thanks for noticing, I'll fix that.

Cascardo.