| Message ID | 20250309175731.7185-1-jain.abhinav177@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | RDMA/core: Publish node GUID with the uevent for ib_device | expand |
On Sun, Mar 09, 2025 at 05:57:31PM +0000, Abhinav Jain wrote: > As per the comment, modify ib_device_uevent to publish the node > GUID alongside device name, upon device state change. > > Have compiled the file manually to ensure that it builds. Do not have > a readily available IB hardware to test. Confirmed with checkpatch > that the patch has no errors/warnings. I'm missing motivation for this patch. Why is this change needed? Thanks > > Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com> > --- > drivers/infiniband/core/device.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > index 0ded91f056f3..1812038f1a91 100644 > --- a/drivers/infiniband/core/device.c > +++ b/drivers/infiniband/core/device.c > @@ -499,12 +499,17 @@ static void ib_device_release(struct device *device) > static int ib_device_uevent(const struct device *device, > struct kobj_uevent_env *env) > { > - if (add_uevent_var(env, "NAME=%s", dev_name(device))) > + const struct ib_device *dev = > + container_of(device, struct ib_device, dev); > + > + if (add_uevent_var(env, "NAME=%s", dev_name(&dev->dev))) > return -ENOMEM; > > - /* > - * It would be nice to pass the node GUID with the event... > - */ > + __be64 node_guid_be = dev->node_guid; > + u64 node_guid = be64_to_cpu(node_guid_be); > + > + if (add_uevent_var(env, "NODE_GUID=0x%llx", node_guid)) > + return -ENOMEM; > > return 0; > } > -- > 2.34.1 >
On Sun, 9 Mar 2025 21:27:51 +0200, Leon Romanovsky wrote: >On Sun, Mar 09, 2025 at 05:57:31PM +0000, Abhinav Jain wrote: >> As per the comment, modify ib_device_uevent to publish the node >> GUID alongside device name, upon device state change. >> >> Have compiled the file manually to ensure that it builds. Do not have >> a readily available IB hardware to test. Confirmed with checkpatch >> that the patch has no errors/warnings. > >I'm missing motivation for this patch. Why is this change needed? > >Thanks Originally, I was looking at this function in order to solve a syzkaller bug. I noticed this comment from Jason and I assumed that the motivation would be to identify the node on which the event is happening. With the name, users can identify nodes however Subnet Manager uses node_guid for discovery and configuration of the nodes. To conclude, I think just using the node name might not be sufficient for unambiguous and reliable device management in the network. >> >> Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com> >> --- >> drivers/infiniband/core/device.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c >> index 0ded91f056f3..1812038f1a91 100644 >> --- a/drivers/infiniband/core/device.c >> +++ b/drivers/infiniband/core/device.c >> @@ -499,12 +499,17 @@ static void ib_device_release(struct device *device) >> static int ib_device_uevent(const struct device *device, >> struct kobj_uevent_env *env) >> { >> - if (add_uevent_var(env, "NAME=%s", dev_name(device))) >> + const struct ib_device *dev = >> + container_of(device, struct ib_device, dev); >> + >> + if (add_uevent_var(env, "NAME=%s", dev_name(&dev->dev))) >> return -ENOMEM; >> >> - /* >> - * It would be nice to pass the node GUID with the event... >> - */ >> + __be64 node_guid_be = dev->node_guid; >> + u64 node_guid = be64_to_cpu(node_guid_be); >> + >> + if (add_uevent_var(env, "NODE_GUID=0x%llx", node_guid)) >> + return -ENOMEM; >> >> return 0; >> } >> -- >> 2.34.1 >>
On Mon, Mar 10, 2025 at 07:01:56AM +0000, Abhinav Jain wrote: > On Sun, 9 Mar 2025 21:27:51 +0200, Leon Romanovsky wrote: > >On Sun, Mar 09, 2025 at 05:57:31PM +0000, Abhinav Jain wrote: > >> As per the comment, modify ib_device_uevent to publish the node > >> GUID alongside device name, upon device state change. > >> > >> Have compiled the file manually to ensure that it builds. Do not have > >> a readily available IB hardware to test. Confirmed with checkpatch > >> that the patch has no errors/warnings. > > > >I'm missing motivation for this patch. Why is this change needed? > > > >Thanks > > Originally, I was looking at this function in order to solve a syzkaller > bug. I noticed this comment from Jason and I assumed that the motivation > would be to identify the node on which the event is happening. > > With the name, users can identify nodes however Subnet Manager uses > node_guid for discovery and configuration of the nodes. To conclude, I > think just using the node name might not be sufficient for unambiguous > and reliable device management in the network. Up till now, it was sufficient. Let's add new uevent when actual use case will be needed. Thanks
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 0ded91f056f3..1812038f1a91 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -499,12 +499,17 @@ static void ib_device_release(struct device *device) static int ib_device_uevent(const struct device *device, struct kobj_uevent_env *env) { - if (add_uevent_var(env, "NAME=%s", dev_name(device))) + const struct ib_device *dev = + container_of(device, struct ib_device, dev); + + if (add_uevent_var(env, "NAME=%s", dev_name(&dev->dev))) return -ENOMEM; - /* - * It would be nice to pass the node GUID with the event... - */ + __be64 node_guid_be = dev->node_guid; + u64 node_guid = be64_to_cpu(node_guid_be); + + if (add_uevent_var(env, "NODE_GUID=0x%llx", node_guid)) + return -ENOMEM; return 0; }
As per the comment, modify ib_device_uevent to publish the node GUID alongside device name, upon device state change. Have compiled the file manually to ensure that it builds. Do not have a readily available IB hardware to test. Confirmed with checkpatch that the patch has no errors/warnings. Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com> --- drivers/infiniband/core/device.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)