On Sun, 2018-09-09 at 23:55 +0300, Arseny Maslennikov wrote:

On Sun, Sep 09, 2018 at 09:11:46PM +0300, Arseny Maslennikov wrote:

quoted

On Fri, Sep 07, 2018 at 09:43:59AM -0600, Jason Gunthorpe wrote:

quoted

On Thu, Sep 06, 2018 at 05:51:12PM +0300, Arseny Maslennikov wrote:

quoted

Some tools may currently be using only the deprecated attribute;
let's print an elaborate and clear deprecation notice to kmsg.

To do that, we have to replace the whole sysfs file, since we inherit
the original one from netdev.

Signed-off-by: Arseny Maslennikov <redacted>
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 30f840f874b3..74732726ec6f 100644
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c

@@ -2386,6 +2386,35 @@ int ipoib_add_pkey_attr(struct net_device *dev)
 	return device_create_file(&dev->dev, &dev_attr_pkey);
 }
 
+/*
+ * We erroneously exposed the iface's port number in the dev_id
+ * sysfs field long after dev_port was introduced for that purpose[1],
+ * and we need to stop everyone from relying on that.
+ * Let's overload the shower routine for the dev_id file here
+ * to gently bring the issue up.
+ *
+ * [1] https://www.spinics.net/lists/netdev/msg272123.html
+ */
+static ssize_t dev_id_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct net_device *ndev = to_net_dev(dev);
+
+	if (ndev->dev_id == ndev->dev_port)
+		netdev_info_once(ndev,
+			"\"%s\" wants to know my dev_id. Should it look at dev_port instead? See Documentation/ABI/testing/sysfs-class-net for more info.\n",
+			current->comm);
+
+	return sprintf(buf, "%#x\n", ndev->dev_id);
+}
+static DEVICE_ATTR_RO(dev_id);
+
+int ipoib_intercept_dev_id_attr(struct net_device *dev)
+{
+	device_remove_file(&dev->dev, &dev_attr_dev_id);
+	return device_create_file(&dev->dev, &dev_attr_dev_id);
+}

Isn't this racey with userspace? Ie what happens if udev is querying
the dev_id right here?

udev in particular does not use dev_id at all since 2014, because "why
would we keep using dev_id if it is not the right thing to use?".

quoted

Do we know there is no userspace doing this?

Not for sure.

If we move all the sysfs handling stuff we introduce in _add_port():
 - pkey
 - umcast
 - {create,delete}_child
 - connected/datagram mode
to _ndo_init(), which is called by register_netdev before it sends
the netlink message, would that suffice to eliminate the race?
(Sysfs files for {create,delete}_child go to _parent_init() then).

No, we can't, sorry for the noise. ndo_init() runs before the kobject
becomes available.

Anyway, our sysfs attributes being racy is unrelated to the patch series
subject, and I can't come up with any other ideas what to do with them
that do not involve adjustments to register_netdev.

Agreed (that fixing the race issues is a different patch series).

-- 
Doug Ledford [off-list ref]
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD