| Message ID | 1404160596-25859-1-git-send-email-cascardo@linux.vnet.ibm.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
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 -- 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
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 -- 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
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 -- 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 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. -- 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 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. -- 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 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. -- 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 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 -- 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 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
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 -- 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/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;
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(-)