[PATCH] Add missing ib_uverbs dependency from SoftiWARP

[PATCH] Add missing ib_uverbs dependency from SoftiWARP

[Tom Talpey]

When loading the siw module, ib_uverbs is needed so that consumers may
access it. However, siw references only inline functions in ib_uverbs.h,
so the kernel linker can not detect this, and the module is not loaded
automatically. Add a module dependency to ensure ib_uverbs is present.

Signed-off-by: Tom Talpey <tom@talpey.com>
---
  drivers/infiniband/sw/siw/siw_main.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/sw/siw/siw_main.c 
b/drivers/infiniband/sw/siw/siw_main.c
index dacc174604bf..372b37b18bac 100644
--- a/drivers/infiniband/sw/siw/siw_main.c
+++ b/drivers/infiniband/sw/siw/siw_main.c
@@ -628,3 +628,4 @@ module_init(siw_init_module);
  module_exit(siw_exit_module);

  MODULE_ALIAS_RDMA_LINK("siw");
+MODULE_SOFTDEP("ib_uverbs");
-- 
2.34.1

Re: [PATCH] Add missing ib_uverbs dependency from SoftiWARP

[Jason Gunthorpe]

On Wed, Sep 07, 2022 at 09:45:07AM -0400, Tom Talpey wrote:
> When loading the siw module, ib_uverbs is needed so that consumers may
> access it. However, siw references only inline functions in ib_uverbs.h,
> so the kernel linker can not detect this, and the module is not loaded
> automatically. Add a module dependency to ensure ib_uverbs is present.

No, that is not how things work.

Modern rdma-core will auto-load uverbs when something uses it, we
shouldn't have things like this.

Jason

RE: [PATCH] Add missing ib_uverbs dependency from SoftiWARP

[Bernard Metzler]

> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, 7 September 2022 15:51
> To: Tom Talpey <tom@talpey.com>
> Cc: Leon Romanovsky <leonro@nvidia.com>; Bernard Metzler
> <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH] Add missing ib_uverbs dependency from
> SoftiWARP
> 
> On Wed, Sep 07, 2022 at 09:45:07AM -0400, Tom Talpey wrote:
> > When loading the siw module, ib_uverbs is needed so that consumers may
> > access it. However, siw references only inline functions in
> ib_uverbs.h,
> > so the kernel linker can not detect this, and the module is not loaded
> > automatically. Add a module dependency to ensure ib_uverbs is present.
> 
> No, that is not how things work.
> 
> Modern rdma-core will auto-load uverbs when something uses it, we
> shouldn't have things like this.

Hmmm, if e.g. siw references ib_copy_to_udata(), it uses
ib_uverbs functionality, but the kernel build mechanics
do not bring in ib_uverbs.ko dependency, since ib_copy_to_udata()
is just an inline defined in ib_uverbs.h.

It seems drivers depend on using at least one 'real'
symbol out of that .ko. Maybe we shall not put functions into
header files?

Thanks
Bernard.

Re: [PATCH] Add missing ib_uverbs dependency from SoftiWARP

[Tom Talpey]

On 9/7/2022 9:51 AM, Jason Gunthorpe wrote:
> On Wed, Sep 07, 2022 at 09:45:07AM -0400, Tom Talpey wrote:
>> When loading the siw module, ib_uverbs is needed so that consumers may
>> access it. However, siw references only inline functions in ib_uverbs.h,
>> so the kernel linker can not detect this, and the module is not loaded
>> automatically. Add a module dependency to ensure ib_uverbs is present.
> 
> No, that is not how things work.
> 
> Modern rdma-core will auto-load uverbs when something uses it, we
> shouldn't have things like this.

But, it doesn't, for the reason stated - siw only consumes
inline functions from ib_uverbs.h, and the kernel linker has
no clue.

You can test it easily, just load siw on a laptop without any
other RDMA provider. The ib_uverbs module will not be loaded,
and the siw provider won't be seen, rping -s will run but peers
cannot connect for example.

My workaround has been to add a modprobe.d file, but it's a
hack and very non-obvious. Is there a simpler way to allow
ib_uverbs to be auto-loaded for siw?

Tom.

Re: [PATCH] Add missing ib_uverbs dependency from SoftiWARP

[Jason Gunthorpe]

On Wed, Sep 07, 2022 at 09:57:43AM -0400, Tom Talpey wrote:

> You can test it easily, just load siw on a laptop without any
> other RDMA provider. The ib_uverbs module will not be loaded,
> and the siw provider won't be seen, rping -s will run but peers
> cannot connect for example.

Perhaps there is something funky with rping, it works fine in simpler cases:

$ rdma link show
link siw0/1 state ACTIVE physical_state LINK_UP netdev enp0s31f6 
$ sudo rmmod ib_uverbs
$ build/bin/ibv_devinfo
hca_id:	siw0
	transport:			iWARP (1)
	fw_ver:				0.0.0
	node_guid:			7686:e2ff:fe28:63fc
	sys_image_guid:			7486:e228:63fc:0000
	vendor_id:			0x626d74
	vendor_part_id:			1
	hw_ver:				0x0
	phys_port_cnt:			1
		port:	1
			state:			PORT_ACTIVE (4)
			max_mtu:		1024 (3)
			active_mtu:		1024 (3)
			sm_lid:			0
			port_lid:		0
			port_lmc:		0x00
			link_layer:		Ethernet
$ lsmod | grep -i uverb
ib_uverbs             163840  0
ib_core               393216  2 ib_uverbs,siw

Jason

Re: [PATCH] Add missing ib_uverbs dependency from SoftiWARP

[Tom Talpey]

On 9/7/2022 10:06 AM, Jason Gunthorpe wrote:
> On Wed, Sep 07, 2022 at 09:57:43AM -0400, Tom Talpey wrote:
> 
>> You can test it easily, just load siw on a laptop without any
>> other RDMA provider. The ib_uverbs module will not be loaded,
>> and the siw provider won't be seen, rping -s will run but peers
>> cannot connect for example.
> 
> Perhaps there is something funky with rping, it works fine in simpler cases:
> 
> $ rdma link show
> link siw0/1 state ACTIVE physical_state LINK_UP netdev enp0s31f6
> $ sudo rmmod ib_uverbs
> $ build/bin/ibv_devinfo
> hca_id:	siw0
> 	transport:			iWARP (1)
> 	fw_ver:				0.0.0
> 	node_guid:			7686:e2ff:fe28:63fc
> 	sys_image_guid:			7486:e228:63fc:0000
> 	vendor_id:			0x626d74
> 	vendor_part_id:			1
> 	hw_ver:				0x0
> 	phys_port_cnt:			1
> 		port:	1
> 			state:			PORT_ACTIVE (4)
> 			max_mtu:		1024 (3)
> 			active_mtu:		1024 (3)
> 			sm_lid:			0
> 			port_lid:		0
> 			port_lmc:		0x00
> 			link_layer:		Ethernet
> $ lsmod | grep -i uverb
> ib_uverbs             163840  0
> ib_core               393216  2 ib_uverbs,siw


That's odd - your ib_uverbs "Used by" list is empty. Did some
module dependency cache force-load it? On my system, it doesn't
load at all. With the proposed siw softdep, it does.

Tom.

> 
> Jason
> 
> 

Re: [PATCH] Add missing ib_uverbs dependency from SoftiWARP

[Jason Gunthorpe]

On Wed, Sep 07, 2022 at 11:24:19AM -0400, Tom Talpey wrote:

> On 9/7/2022 10:06 AM, Jason Gunthorpe wrote:
> > On Wed, Sep 07, 2022 at 09:57:43AM -0400, Tom Talpey wrote:
> > 
> > > You can test it easily, just load siw on a laptop without any
> > > other RDMA provider. The ib_uverbs module will not be loaded,
> > > and the siw provider won't be seen, rping -s will run but peers
> > > cannot connect for example.
> > 
> > Perhaps there is something funky with rping, it works fine in simpler cases:
> > 
> > $ rdma link show
> > link siw0/1 state ACTIVE physical_state LINK_UP netdev enp0s31f6
> > $ sudo rmmod ib_uverbs
> > $ build/bin/ibv_devinfo
> > hca_id:	siw0
> > 	transport:			iWARP (1)
> > 	fw_ver:				0.0.0
> > 	node_guid:			7686:e2ff:fe28:63fc
> > 	sys_image_guid:			7486:e228:63fc:0000
> > 	vendor_id:			0x626d74
> > 	vendor_part_id:			1
> > 	hw_ver:				0x0
> > 	phys_port_cnt:			1
> > 		port:	1
> > 			state:			PORT_ACTIVE (4)
> > 			max_mtu:		1024 (3)
> > 			active_mtu:		1024 (3)
> > 			sm_lid:			0
> > 			port_lid:		0
> > 			port_lmc:		0x00
> > 			link_layer:		Ethernet
> > $ lsmod | grep -i uverb
> > ib_uverbs             163840  0
> > ib_core               393216  2 ib_uverbs,siw
> 
> 
> That's odd - your ib_uverbs "Used by" list is empty. Did some
> module dependency cache force-load it? On my system, it doesn't
> load at all. With the proposed siw softdep, it does.

As I said, modern rdma-core auto-loads it on its own. There is no
module dependency causing it to load.

Jason

Re: [PATCH] Add missing ib_uverbs dependency from SoftiWARP

[Tom Talpey]

On 9/7/2022 12:48 PM, Jason Gunthorpe wrote:
> On Wed, Sep 07, 2022 at 11:24:19AM -0400, Tom Talpey wrote:
>> That's odd - your ib_uverbs "Used by" list is empty. Did some
>> module dependency cache force-load it? On my system, it doesn't
>> load at all. With the proposed siw softdep, it does.
> 
> As I said, modern rdma-core auto-loads it on its own. There is no
> module dependency causing it to load.

Ok. I just packed up my test machines for shipping to the SDC IOLab
event next week, so I'll re-test then, and try to figure out why the
patch did change behavior.

BTW, there I'll be working on the kernel SMB3 server (ksmbd) RDMA
support, and testing over siw and rxe.

Tom.