LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
@ 2018-05-30 21:58 Arnd Bergmann
  2018-05-30 22:04 ` Jason Gunthorpe
  2018-06-04 15:36 ` Jason Gunthorpe
  0 siblings, 2 replies; 13+ messages in thread
From: Arnd Bergmann @ 2018-05-30 21:58 UTC (permalink / raw)
  To: Steve Wise, Doug Ledford, Jason Gunthorpe
  Cc: Arnd Bergmann, Yuval Shaia, linux-rdma, linux-kernel

The newly added fill_res_ep_entry function fails to link if
CONFIG_INFINIBAND_ADDR_TRANS is not set:

drivers/infiniband/hw/cxgb4/restrack.o: In function `fill_res_ep_entry':
restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'

This adds a Kconfig dependency for the driver.

Fixes: 116aeb887371 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/infiniband/hw/cxgb4/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/hw/cxgb4/Kconfig b/drivers/infiniband/hw/cxgb4/Kconfig
index 0a671a61fc92..e0522a5d5a06 100644
--- a/drivers/infiniband/hw/cxgb4/Kconfig
+++ b/drivers/infiniband/hw/cxgb4/Kconfig
@@ -1,6 +1,7 @@
 config INFINIBAND_CXGB4
 	tristate "Chelsio T4/T5 RDMA Driver"
 	depends on CHELSIO_T4 && INET
+	depends on INFINIBAND_ADDR_TRANS
 	select CHELSIO_LIB
 	select GENERIC_ALLOCATOR
 	---help---
-- 
2.9.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-30 21:58 [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency Arnd Bergmann
@ 2018-05-30 22:04 ` Jason Gunthorpe
  2018-05-30 22:10   ` Steve Wise
  2018-06-04 15:36 ` Jason Gunthorpe
  1 sibling, 1 reply; 13+ messages in thread
From: Jason Gunthorpe @ 2018-05-30 22:04 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Steve Wise, Doug Ledford, Yuval Shaia, linux-rdma, linux-kernel

On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
> The newly added fill_res_ep_entry function fails to link if
> CONFIG_INFINIBAND_ADDR_TRANS is not set:
> 
> drivers/infiniband/hw/cxgb4/restrack.o: In function `fill_res_ep_entry':
> restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
> restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
> 
> This adds a Kconfig dependency for the driver.
> 
> Fixes: 116aeb887371 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Oh, I think we need to solve this with maybe a header fill null stub
instead..

We don't want to disable drivers just because a user interface is
disabled.

Thanks,
Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-30 22:04 ` Jason Gunthorpe
@ 2018-05-30 22:10   ` Steve Wise
  2018-05-30 22:25     ` Jason Gunthorpe
  0 siblings, 1 reply; 13+ messages in thread
From: Steve Wise @ 2018-05-30 22:10 UTC (permalink / raw)
  To: Jason Gunthorpe, Arnd Bergmann
  Cc: Steve Wise, Doug Ledford, Yuval Shaia, linux-rdma, linux-kernel



On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
> On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
>> The newly added fill_res_ep_entry function fails to link if
>> CONFIG_INFINIBAND_ADDR_TRANS is not set:
>>
>> drivers/infiniband/hw/cxgb4/restrack.o: In function `fill_res_ep_entry':
>> restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
>> restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
>>
>> This adds a Kconfig dependency for the driver.
>>
>> Fixes: 116aeb887371 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
> Oh, I think we need to solve this with maybe a header fill null stub
> instead..
>
> We don't want to disable drivers just because a user interface is
> disabled.
>

Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko?  That
is not correct.

Steve.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-30 22:10   ` Steve Wise
@ 2018-05-30 22:25     ` Jason Gunthorpe
  2018-05-30 22:29       ` Steve Wise
  0 siblings, 1 reply; 13+ messages in thread
From: Jason Gunthorpe @ 2018-05-30 22:25 UTC (permalink / raw)
  To: Steve Wise
  Cc: Arnd Bergmann, Steve Wise, Doug Ledford, Yuval Shaia, linux-rdma,
	linux-kernel

On Wed, May 30, 2018 at 05:10:35PM -0500, Steve Wise wrote:
> 
> 
> On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
> > On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
> >> The newly added fill_res_ep_entry function fails to link if
> >> CONFIG_INFINIBAND_ADDR_TRANS is not set:
> >>
> >> drivers/infiniband/hw/cxgb4/restrack.o: In function `fill_res_ep_entry':
> >> restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
> >> restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
> >>
> >> This adds a Kconfig dependency for the driver.
> >>
> >> Fixes: 116aeb887371 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >>  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
> >>  1 file changed, 1 insertion(+)
> > Oh, I think we need to solve this with maybe a header fill null stub
> > instead..
> >
> > We don't want to disable drivers just because a user interface is
> > disabled.
> >
> 
> Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko? That
> is not correct.

That seems like a reasonable thing to do..

But why does it break the compile? Those functions are in cma.c...

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-30 22:25     ` Jason Gunthorpe
@ 2018-05-30 22:29       ` Steve Wise
  2018-05-30 22:40         ` Arnd Bergmann
  0 siblings, 1 reply; 13+ messages in thread
From: Steve Wise @ 2018-05-30 22:29 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Arnd Bergmann, Steve Wise, Doug Ledford, Yuval Shaia, linux-rdma,
	linux-kernel



On 5/30/2018 5:25 PM, Jason Gunthorpe wrote:
> On Wed, May 30, 2018 at 05:10:35PM -0500, Steve Wise wrote:
>>
>> On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
>>> On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
>>>> The newly added fill_res_ep_entry function fails to link if
>>>> CONFIG_INFINIBAND_ADDR_TRANS is not set:
>>>>
>>>> drivers/infiniband/hw/cxgb4/restrack.o: In function `fill_res_ep_entry':
>>>> restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
>>>> restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
>>>>
>>>> This adds a Kconfig dependency for the driver.
>>>>
>>>> Fixes: 116aeb887371 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>>  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
>>>>  1 file changed, 1 insertion(+)
>>> Oh, I think we need to solve this with maybe a header fill null stub
>>> instead..
>>>
>>> We don't want to disable drivers just because a user interface is
>>> disabled.
>>>
>> Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko? That
>> is not correct.
> That seems like a reasonable thing to do..
rdma_ucm.ko is for usermode users, rdma_cm.ko is for kernel users, and
is required for iwarp drivers.  It seems rdma_cm.ko is not being
compiled if ADDR_TRANS is not set.


> But why does it break the compile? Those functions are in cma.c...
>
> Jason

Indeed, why. :)

Perhaps this line is wrong in drivers/infiniband/core/Makefile:

infiniband-$(CONFIG_INFINIBAND_ADDR_TRANS)      := rdma_cm.o

I don't understand the rules in that Makefile. 

Steve.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-30 22:29       ` Steve Wise
@ 2018-05-30 22:40         ` Arnd Bergmann
  2018-05-30 23:01           ` Jason Gunthorpe
  0 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2018-05-30 22:40 UTC (permalink / raw)
  To: Steve Wise
  Cc: Jason Gunthorpe, Steve Wise, Doug Ledford, Yuval Shaia,
	linux-rdma, Linux Kernel Mailing List

On Thu, May 31, 2018 at 12:29 AM, Steve Wise
<swise@opengridcomputing.com> wrote:
>
>
> On 5/30/2018 5:25 PM, Jason Gunthorpe wrote:
>> On Wed, May 30, 2018 at 05:10:35PM -0500, Steve Wise wrote:
>>>
>>> On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
>>>> On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
>>>>> The newly added fill_res_ep_entry function fails to link if
>>>>> CONFIG_INFINIBAND_ADDR_TRANS is not set:
>>>>>
>>>>> drivers/infiniband/hw/cxgb4/restrack.o: In function `fill_res_ep_entry':
>>>>> restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
>>>>> restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
>>>>>
>>>>> This adds a Kconfig dependency for the driver.
>>>>>
>>>>> Fixes: 116aeb887371 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
>>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>>>  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>> Oh, I think we need to solve this with maybe a header fill null stub
>>>> instead..
>>>>
>>>> We don't want to disable drivers just because a user interface is
>>>> disabled.
>>>>
>>> Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko? That
>>> is not correct.
>> That seems like a reasonable thing to do..
> rdma_ucm.ko is for usermode users, rdma_cm.ko is for kernel users, and
> is required for iwarp drivers.  It seems rdma_cm.ko is not being
> compiled if ADDR_TRANS is not set.
>
>
>> But why does it break the compile? Those functions are in cma.c...
>>
>> Jason
>
> Indeed, why. :)
>
> Perhaps this line is wrong in drivers/infiniband/core/Makefile:
>
> infiniband-$(CONFIG_INFINIBAND_ADDR_TRANS)      := rdma_cm.o
>
> I don't understand the rules in that Makefile.

You need to look at these three lines:

infiniband-$(CONFIG_INFINIBAND_ADDR_TRANS)      := rdma_cm.o

obj-$(CONFIG_INFINIBAND) +=             ib_core.o ib_cm.o iw_cm.o \
                                        $(infiniband-y)

rdma_cm-y :=                    cma.o

cma.o is built into rdma_cm.ko if both CONFIG_INFINIBAND_ADDR_TRANS
and CONFIG_INFINIBAND are enabled. The first line is just another
syntax for writing

ifdef CONFIG_INFINIBAND_ADDR_TRANS
obj-$(CONFIG_INFINIBAND) += rdma_cm.o
endif

        Arnd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-30 22:40         ` Arnd Bergmann
@ 2018-05-30 23:01           ` Jason Gunthorpe
  2018-05-31  4:03             ` Greg Thelen
  0 siblings, 1 reply; 13+ messages in thread
From: Jason Gunthorpe @ 2018-05-30 23:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Steve Wise, Steve Wise, Doug Ledford, Yuval Shaia, linux-rdma,
	Greg Thelen, Linux Kernel Mailing List

On Thu, May 31, 2018 at 12:40:54AM +0200, Arnd Bergmann wrote:
> > On 5/30/2018 5:25 PM, Jason Gunthorpe wrote:
> >> On Wed, May 30, 2018 at 05:10:35PM -0500, Steve Wise wrote:
> >>>
> >>> On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
> >>>> On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
> >>>>> The newly added fill_res_ep_entry function fails to link if
> >>>>> CONFIG_INFINIBAND_ADDR_TRANS is not set:
> >>>>>
> >>>>> drivers/infiniband/hw/cxgb4/restrack.o: In function `fill_res_ep_entry':
> >>>>> restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
> >>>>> restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
> >>>>>
> >>>>> This adds a Kconfig dependency for the driver.
> >>>>>
> >>>>> Fixes: 116aeb887371 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
> >>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >>>>>  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
> >>>>>  1 file changed, 1 insertion(+)
> >>>> Oh, I think we need to solve this with maybe a header fill null stub
> >>>> instead..
> >>>>
> >>>> We don't want to disable drivers just because a user interface is
> >>>> disabled.
> >>>>
> >>> Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko? That
> >>> is not correct.
> >> That seems like a reasonable thing to do..
> > rdma_ucm.ko is for usermode users, rdma_cm.ko is for kernel users, and
> > is required for iwarp drivers.  It seems rdma_cm.ko is not being
> > compiled if ADDR_TRANS is not set.

I think the intention was to completely disable rdma-cm, including all
support for rx'ing remote packets? Greg?

If this is required for iwarp then Arnd's patch is probably the right
way to go..

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-30 23:01           ` Jason Gunthorpe
@ 2018-05-31  4:03             ` Greg Thelen
  2018-05-31 18:40               ` Doug Ledford
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Thelen @ 2018-05-31  4:03 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Arnd Bergmann, swise, swise, Doug Ledford, yuval.shaia,
	linux-rdma, LKML, Tarick Bedeir

On Wed, May 30, 2018 at 4:01 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Thu, May 31, 2018 at 12:40:54AM +0200, Arnd Bergmann wrote:
> > > On 5/30/2018 5:25 PM, Jason Gunthorpe wrote:
> > >> On Wed, May 30, 2018 at 05:10:35PM -0500, Steve Wise wrote:
> > >>>
> > >>> On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
> > >>>> On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
> > >>>>> The newly added fill_res_ep_entry function fails to link if
> > >>>>> CONFIG_INFINIBAND_ADDR_TRANS is not set:
> > >>>>>
> > >>>>> drivers/infiniband/hw/cxgb4/restrack.o: In function
`fill_res_ep_entry':
> > >>>>> restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
> > >>>>> restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
> > >>>>>
> > >>>>> This adds a Kconfig dependency for the driver.
> > >>>>>
> > >>>>> Fixes: 116aeb887371 ("iw_cxgb4: provide detailed
provider-specific CM_ID information")
> > >>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > >>>>>  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
> > >>>>>  1 file changed, 1 insertion(+)
> > >>>> Oh, I think we need to solve this with maybe a header fill null
stub
> > >>>> instead..
> > >>>>
> > >>>> We don't want to disable drivers just because a user interface is
> > >>>> disabled.
> > >>>>
> > >>> Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko?
That
> > >>> is not correct.
> > >> That seems like a reasonable thing to do..
> > > rdma_ucm.ko is for usermode users, rdma_cm.ko is for kernel users, and
> > > is required for iwarp drivers.  It seems rdma_cm.ko is not being
> > > compiled if ADDR_TRANS is not set.

> I think the intention was to completely disable rdma-cm, including all
> support for rx'ing remote packets? Greg?

Yes.  That's my goal when INFINIBAND_ADDR_TRANS is unset.

> If this is required for iwarp then Arnd's patch is probably the right
> way to go..

> Jason

Agreed.
Acked-by: Greg Thelen <gthelen@google.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-31  4:03             ` Greg Thelen
@ 2018-05-31 18:40               ` Doug Ledford
  2018-06-04 23:07                 ` Jason Gunthorpe
  0 siblings, 1 reply; 13+ messages in thread
From: Doug Ledford @ 2018-05-31 18:40 UTC (permalink / raw)
  To: Greg Thelen, Jason Gunthorpe
  Cc: Arnd Bergmann, swise, swise, yuval.shaia, linux-rdma, LKML,
	Tarick Bedeir

[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]

On Wed, 2018-05-30 at 21:03 -0700, Greg Thelen wrote:
> On Wed, May 30, 2018 at 4:01 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> 
> > On Thu, May 31, 2018 at 12:40:54AM +0200, Arnd Bergmann wrote:
> > > > On 5/30/2018 5:25 PM, Jason Gunthorpe wrote:
> > > > > On Wed, May 30, 2018 at 05:10:35PM -0500, Steve Wise wrote:
> > > > > > 
> > > > > > On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
> > > > > > > On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
> > > > > > > > The newly added fill_res_ep_entry function fails to link if
> > > > > > > > CONFIG_INFINIBAND_ADDR_TRANS is not set:
> > > > > > > > 
> > > > > > > > drivers/infiniband/hw/cxgb4/restrack.o: In function
> 
> `fill_res_ep_entry':
> > > > > > > > restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
> > > > > > > > restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
> > > > > > > > 
> > > > > > > > This adds a Kconfig dependency for the driver.
> > > > > > > > 
> > > > > > > > Fixes: 116aeb887371 ("iw_cxgb4: provide detailed
> 
> provider-specific CM_ID information")
> > > > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > > > > >  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
> > > > > > > >  1 file changed, 1 insertion(+)
> > > > > > > 
> > > > > > > Oh, I think we need to solve this with maybe a header fill null
> 
> stub
> > > > > > > instead..
> > > > > > > 
> > > > > > > We don't want to disable drivers just because a user interface is
> > > > > > > disabled.
> > > > > > > 
> > > > > > 
> > > > > > Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko?
> 
> That
> > > > > > is not correct.
> > > > > 
> > > > > That seems like a reasonable thing to do..
> > > > 
> > > > rdma_ucm.ko is for usermode users, rdma_cm.ko is for kernel users, and
> > > > is required for iwarp drivers.  It seems rdma_cm.ko is not being
> > > > compiled if ADDR_TRANS is not set.
> > I think the intention was to completely disable rdma-cm, including all
> > support for rx'ing remote packets? Greg?
> 
> Yes.  That's my goal when INFINIBAND_ADDR_TRANS is unset.
> 
> > If this is required for iwarp then Arnd's patch is probably the right
> > way to go..
> > Jason
> 
> Agreed.
> Acked-by: Greg Thelen <gthelen@google.com>

If that's the case, then there should be a NOTE: in the Kconfig that
disabling the connection manager completely disables iWARP hardware.

I don't really think I like this approach though.  At a minimum if you
are going to make iWARP totally dependent on rdmacm, then there is zero
reason that iw_cm.o should be part of the obj-$(CONFIG_INFINIBAND)
Makefile recipe when ADDR_TRANS is disabled.

We can take this patch as a band-aid, but IMO it's either incomplete or
simply not the right solution.

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-30 21:58 [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency Arnd Bergmann
  2018-05-30 22:04 ` Jason Gunthorpe
@ 2018-06-04 15:36 ` Jason Gunthorpe
  1 sibling, 0 replies; 13+ messages in thread
From: Jason Gunthorpe @ 2018-06-04 15:36 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Steve Wise, Doug Ledford, Yuval Shaia, linux-rdma, linux-kernel

On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
> The newly added fill_res_ep_entry function fails to link if
> CONFIG_INFINIBAND_ADDR_TRANS is not set:
> 
> drivers/infiniband/hw/cxgb4/restrack.o: In function `fill_res_ep_entry':
> restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
> restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
> 
> This adds a Kconfig dependency for the driver.
> 
> Fixes: 116aeb887371 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Greg Thelen <gthelen@google.com>
> ---
>  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Applied to for-next

Thanks,
Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-05-31 18:40               ` Doug Ledford
@ 2018-06-04 23:07                 ` Jason Gunthorpe
  2018-06-07  8:10                   ` Greg Thelen
  0 siblings, 1 reply; 13+ messages in thread
From: Jason Gunthorpe @ 2018-06-04 23:07 UTC (permalink / raw)
  To: Doug Ledford
  Cc: Greg Thelen, Arnd Bergmann, swise, swise, yuval.shaia,
	linux-rdma, LKML, Tarick Bedeir

On Thu, May 31, 2018 at 02:40:59PM -0400, Doug Ledford wrote:
> On Wed, 2018-05-30 at 21:03 -0700, Greg Thelen wrote:
> > On Wed, May 30, 2018 at 4:01 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > 
> > > On Thu, May 31, 2018 at 12:40:54AM +0200, Arnd Bergmann wrote:
> > > > > On 5/30/2018 5:25 PM, Jason Gunthorpe wrote:
> > > > > > On Wed, May 30, 2018 at 05:10:35PM -0500, Steve Wise wrote:
> > > > > > > 
> > > > > > > On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
> > > > > > > > On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
> > > > > > > > > The newly added fill_res_ep_entry function fails to link if
> > > > > > > > > CONFIG_INFINIBAND_ADDR_TRANS is not set:
> > > > > > > > > 
> > > > > > > > > drivers/infiniband/hw/cxgb4/restrack.o: In function
> > 
> > `fill_res_ep_entry':
> > > > > > > > > restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
> > > > > > > > > restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
> > > > > > > > > 
> > > > > > > > > This adds a Kconfig dependency for the driver.
> > > > > > > > > 
> > > > > > > > > Fixes: 116aeb887371 ("iw_cxgb4: provide detailed
> > 
> > provider-specific CM_ID information")
> > > > > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > > > > > >  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
> > > > > > > > >  1 file changed, 1 insertion(+)
> > > > > > > > 
> > > > > > > > Oh, I think we need to solve this with maybe a header fill null
> > 
> > stub
> > > > > > > > instead..
> > > > > > > > 
> > > > > > > > We don't want to disable drivers just because a user interface is
> > > > > > > > disabled.
> > > > > > > > 
> > > > > > > 
> > > > > > > Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko?
> > 
> > That
> > > > > > > is not correct.
> > > > > > 
> > > > > > That seems like a reasonable thing to do..
> > > > > 
> > > > > rdma_ucm.ko is for usermode users, rdma_cm.ko is for kernel users, and
> > > > > is required for iwarp drivers.  It seems rdma_cm.ko is not being
> > > > > compiled if ADDR_TRANS is not set.
> > > I think the intention was to completely disable rdma-cm, including all
> > > support for rx'ing remote packets? Greg?
> > 
> > Yes.  That's my goal when INFINIBAND_ADDR_TRANS is unset.
> > 
> > > If this is required for iwarp then Arnd's patch is probably the right
> > > way to go..
> > > Jason
> > 
> > Agreed.
> > Acked-by: Greg Thelen <gthelen@google.com>
> 
> If that's the case, then there should be a NOTE: in the Kconfig that
> disabling the connection manager completely disables iWARP hardware.
> 
> I don't really think I like this approach though.  At a minimum if you
> are going to make iWARP totally dependent on rdmacm, then there is zero
> reason that iw_cm.o should be part of the obj-$(CONFIG_INFINIBAND)
> Makefile recipe when ADDR_TRANS is disabled.

This makes sense, Greg, can you send a followup patch with these
items? I know this series has been tough..

Thanks,
Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-06-04 23:07                 ` Jason Gunthorpe
@ 2018-06-07  8:10                   ` Greg Thelen
  2018-06-07 18:00                     ` Jason Gunthorpe
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Thelen @ 2018-06-07  8:10 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, Arnd Bergmann, swise, swise, yuval.shaia,
	linux-rdma, LKML, Tarick Bedeir

On Mon, Jun 4, 2018 at 4:07 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, May 31, 2018 at 02:40:59PM -0400, Doug Ledford wrote:
> > On Wed, 2018-05-30 at 21:03 -0700, Greg Thelen wrote:
> > > On Wed, May 30, 2018 at 4:01 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > > On Thu, May 31, 2018 at 12:40:54AM +0200, Arnd Bergmann wrote:
> > > > > > On 5/30/2018 5:25 PM, Jason Gunthorpe wrote:
> > > > > > > On Wed, May 30, 2018 at 05:10:35PM -0500, Steve Wise wrote:
> > > > > > > >
> > > > > > > > On 5/30/2018 5:04 PM, Jason Gunthorpe wrote:
> > > > > > > > > On Wed, May 30, 2018 at 11:58:18PM +0200, Arnd Bergmann wrote:
> > > > > > > > > > The newly added fill_res_ep_entry function fails to link if
> > > > > > > > > > CONFIG_INFINIBAND_ADDR_TRANS is not set:
> > > > > > > > > >
> > > > > > > > > > drivers/infiniband/hw/cxgb4/restrack.o: In function
> > >
> > > `fill_res_ep_entry':
> > > > > > > > > > restrack.c:(.text+0x3cc): undefined reference to `rdma_res_to_id'
> > > > > > > > > > restrack.c:(.text+0x3d0): undefined reference to `rdma_iw_cm_id'
> > > > > > > > > >
> > > > > > > > > > This adds a Kconfig dependency for the driver.
> > > > > > > > > >
> > > > > > > > > > Fixes: 116aeb887371 ("iw_cxgb4: provide detailed
> > >
> > > provider-specific CM_ID information")
> > > > > > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > > > > > > >  drivers/infiniband/hw/cxgb4/Kconfig | 1 +
> > > > > > > > > >  1 file changed, 1 insertion(+)
> > > > > > > > >
> > > > > > > > > Oh, I think we need to solve this with maybe a header fill null
> > >
> > > stub
> > > > > > > > > instead..
> > > > > > > > >
> > > > > > > > > We don't want to disable drivers just because a user interface is
> > > > > > > > > disabled.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Why does CONFIG_INFINIBAND_ADDR_TRANS disable building rdma_cm.ko?
> > >
> > > That
> > > > > > > > is not correct.
> > > > > > >
> > > > > > > That seems like a reasonable thing to do..
> > > > > >
> > > > > > rdma_ucm.ko is for usermode users, rdma_cm.ko is for kernel users, and
> > > > > > is required for iwarp drivers.  It seems rdma_cm.ko is not being
> > > > > > compiled if ADDR_TRANS is not set.
> > > > I think the intention was to completely disable rdma-cm, including all
> > > > support for rx'ing remote packets? Greg?
> > >
> > > Yes.  That's my goal when INFINIBAND_ADDR_TRANS is unset.
> > >
> > > > If this is required for iwarp then Arnd's patch is probably the right
> > > > way to go..
> > > > Jason
> > >
> > > Agreed.
> > > Acked-by: Greg Thelen <gthelen@google.com>
> >
> > If that's the case, then there should be a NOTE: in the Kconfig that
> > disabling the connection manager completely disables iWARP hardware.
> >
> > I don't really think I like this approach though.  At a minimum if you
> > are going to make iWARP totally dependent on rdmacm, then there is zero
> > reason that iw_cm.o should be part of the obj-$(CONFIG_INFINIBAND)
> > Makefile recipe when ADDR_TRANS is disabled.
>
> This makes sense, Greg, can you send a followup patch with these
> items? I know this series has been tough..
>
> Thanks,
> Jason

Ack.  Sorry for delay.  I'll take a look at this in a day or two.  Let
me know if it's more urgent.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency
  2018-06-07  8:10                   ` Greg Thelen
@ 2018-06-07 18:00                     ` Jason Gunthorpe
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Gunthorpe @ 2018-06-07 18:00 UTC (permalink / raw)
  To: Greg Thelen
  Cc: Doug Ledford, Arnd Bergmann, swise, swise, yuval.shaia,
	linux-rdma, LKML, Tarick Bedeir

On Thu, Jun 07, 2018 at 01:10:39AM -0700, Greg Thelen wrote:
> > > I don't really think I like this approach though.  At a minimum if you
> > > are going to make iWARP totally dependent on rdmacm, then there is zero
> > > reason that iw_cm.o should be part of the obj-$(CONFIG_INFINIBAND)
> > > Makefile recipe when ADDR_TRANS is disabled.
> >
> > This makes sense, Greg, can you send a followup patch with these
> > items? I know this series has been tough..
> >
> > Thanks,
> > Jason
> 
> Ack.  Sorry for delay.  I'll take a look at this in a day or two.  Let
> me know if it's more urgent.

No rush, sometime in the next several weeks is fine

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-06-07 18:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-30 21:58 [PATCH] iw_cxgb4: add INFINIBAND_ADDR_TRANS dependency Arnd Bergmann
2018-05-30 22:04 ` Jason Gunthorpe
2018-05-30 22:10   ` Steve Wise
2018-05-30 22:25     ` Jason Gunthorpe
2018-05-30 22:29       ` Steve Wise
2018-05-30 22:40         ` Arnd Bergmann
2018-05-30 23:01           ` Jason Gunthorpe
2018-05-31  4:03             ` Greg Thelen
2018-05-31 18:40               ` Doug Ledford
2018-06-04 23:07                 ` Jason Gunthorpe
2018-06-07  8:10                   ` Greg Thelen
2018-06-07 18:00                     ` Jason Gunthorpe
2018-06-04 15:36 ` Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).