linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] rds: transport module should be auto loaded when transport is set
@ 2020-05-27  8:17 rao.shoaib
  2020-05-29 23:41 ` David Miller
  2020-05-31 10:08 ` Leon Romanovsky
  0 siblings, 2 replies; 8+ messages in thread
From: rao.shoaib @ 2020-05-27  8:17 UTC (permalink / raw)
  To: netdev, linux-rdma, rao.shoaib; +Cc: Somasundaram Krishnasamy

From: Rao Shoaib <rao.shoaib@oracle.com>

This enhancement auto loads transport module when the transport
is set via SO_RDS_TRANSPORT socket option.

Orabug: 31032127

Reviewed-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
Reviewed-by: Håkon Bugge <haakon.bugge@oracle.com>
Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
Signed-off-by: Somasundaram Krishnasamy <somasundaram.krishnasamy@oracle.com>
---
 include/uapi/linux/rds.h |  2 +-
 net/rds/transport.c      | 26 +++++++++++++++++---------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
index cba368e55863..7273c681e6c1 100644
--- a/include/uapi/linux/rds.h
+++ b/include/uapi/linux/rds.h
@@ -64,7 +64,7 @@
 
 /* supported values for SO_RDS_TRANSPORT */
 #define	RDS_TRANS_IB	0
-#define	RDS_TRANS_IWARP	1
+#define	RDS_TRANS_GAP	1
 #define	RDS_TRANS_TCP	2
 #define RDS_TRANS_COUNT	3
 #define	RDS_TRANS_NONE	(~0)
diff --git a/net/rds/transport.c b/net/rds/transport.c
index 46f709a4b577..f8001ec80867 100644
--- a/net/rds/transport.c
+++ b/net/rds/transport.c
@@ -38,6 +38,12 @@
 #include "rds.h"
 #include "loop.h"
 
+static char * const rds_trans_modules[] = {
+	[RDS_TRANS_IB] = "rds_rdma",
+	[RDS_TRANS_GAP] = NULL,
+	[RDS_TRANS_TCP] = "rds_tcp",
+};
+
 static struct rds_transport *transports[RDS_TRANS_COUNT];
 static DECLARE_RWSEM(rds_trans_sem);
 
@@ -110,18 +116,20 @@ struct rds_transport *rds_trans_get(int t_type)
 {
 	struct rds_transport *ret = NULL;
 	struct rds_transport *trans;
-	unsigned int i;
 
 	down_read(&rds_trans_sem);
-	for (i = 0; i < RDS_TRANS_COUNT; i++) {
-		trans = transports[i];
-
-		if (trans && trans->t_type == t_type &&
-		    (!trans->t_owner || try_module_get(trans->t_owner))) {
-			ret = trans;
-			break;
-		}
+	trans = transports[t_type];
+	if (!trans) {
+		up_read(&rds_trans_sem);
+		if (rds_trans_modules[t_type])
+			request_module(rds_trans_modules[t_type]);
+		down_read(&rds_trans_sem);
+		trans = transports[t_type];
 	}
+	if (trans && trans->t_type == t_type &&
+	    (!trans->t_owner || try_module_get(trans->t_owner)))
+		ret = trans;
+
 	up_read(&rds_trans_sem);
 
 	return ret;
-- 
2.16.6


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

* Re: [PATCH net-next] rds: transport module should be auto loaded when transport is set
  2020-05-27  8:17 [PATCH net-next] rds: transport module should be auto loaded when transport is set rao.shoaib
@ 2020-05-29 23:41 ` David Miller
  2020-06-01 16:59   ` Rao Shoaib
  2020-05-31 10:08 ` Leon Romanovsky
  1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2020-05-29 23:41 UTC (permalink / raw)
  To: rao.shoaib; +Cc: netdev, linux-rdma, somasundaram.krishnasamy

From: rao.shoaib@oracle.com
Date: Wed, 27 May 2020 01:17:42 -0700

> diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
> index cba368e55863..7273c681e6c1 100644
> --- a/include/uapi/linux/rds.h
> +++ b/include/uapi/linux/rds.h
> @@ -64,7 +64,7 @@
>  
>  /* supported values for SO_RDS_TRANSPORT */
>  #define	RDS_TRANS_IB	0
> -#define	RDS_TRANS_IWARP	1
> +#define	RDS_TRANS_GAP	1
>  #define	RDS_TRANS_TCP	2
>  #define RDS_TRANS_COUNT	3
>  #define	RDS_TRANS_NONE	(~0)

You can't break user facing UAPI like this, sorry.

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

* Re: [PATCH net-next] rds: transport module should be auto loaded when transport is set
  2020-05-27  8:17 [PATCH net-next] rds: transport module should be auto loaded when transport is set rao.shoaib
  2020-05-29 23:41 ` David Miller
@ 2020-05-31 10:08 ` Leon Romanovsky
  2020-06-01 17:08   ` Rao Shoaib
  1 sibling, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2020-05-31 10:08 UTC (permalink / raw)
  To: rao.shoaib; +Cc: netdev, linux-rdma, Somasundaram Krishnasamy

On Wed, May 27, 2020 at 01:17:42AM -0700, rao.shoaib@oracle.com wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
>
> This enhancement auto loads transport module when the transport
> is set via SO_RDS_TRANSPORT socket option.
>
> Orabug: 31032127

I think that it is internal to Oracle and should not be in the commit
message.

Thanks

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

* Re: [PATCH net-next] rds: transport module should be auto loaded when transport is set
  2020-05-29 23:41 ` David Miller
@ 2020-06-01 16:59   ` Rao Shoaib
  2020-06-02  6:17     ` Leon Romanovsky
  0 siblings, 1 reply; 8+ messages in thread
From: Rao Shoaib @ 2020-06-01 16:59 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-rdma, somasundaram.krishnasamy


On 5/29/20 4:41 PM, David Miller wrote:
> From: rao.shoaib@oracle.com
> Date: Wed, 27 May 2020 01:17:42 -0700
>
>> diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
>> index cba368e55863..7273c681e6c1 100644
>> --- a/include/uapi/linux/rds.h
>> +++ b/include/uapi/linux/rds.h
>> @@ -64,7 +64,7 @@
>>   
>>   /* supported values for SO_RDS_TRANSPORT */
>>   #define	RDS_TRANS_IB	0
>> -#define	RDS_TRANS_IWARP	1
>> +#define	RDS_TRANS_GAP	1
>>   #define	RDS_TRANS_TCP	2
>>   #define RDS_TRANS_COUNT	3
>>   #define	RDS_TRANS_NONE	(~0)
> You can't break user facing UAPI like this, sorry.

I was hoping that this could be considered an exception as IWARP has 
been deprecated for almost a decade and there is no current product 
using it. With the change any old binary will continue to work, a new 
compilation fill fail so that the code can be examined, otherwise we 
will never be able to reuse this number.

If the above is not acceptable I can revert this part of the change.

Shoaib


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

* Re: [PATCH net-next] rds: transport module should be auto loaded when transport is set
  2020-05-31 10:08 ` Leon Romanovsky
@ 2020-06-01 17:08   ` Rao Shoaib
  2020-06-02  6:18     ` Leon Romanovsky
  0 siblings, 1 reply; 8+ messages in thread
From: Rao Shoaib @ 2020-06-01 17:08 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: netdev, linux-rdma, Somasundaram Krishnasamy


On 5/31/20 3:08 AM, Leon Romanovsky wrote:
> On Wed, May 27, 2020 at 01:17:42AM -0700, rao.shoaib@oracle.com wrote:
>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>
>> This enhancement auto loads transport module when the transport
>> is set via SO_RDS_TRANSPORT socket option.
>>
>> Orabug: 31032127
> I think that it is internal to Oracle and should not be in the commit
> message.
>
> Thanks

There are logs that have internals bug numbers mentioned in them. I do 
agree with you and will take the bug number out.

Thanks for the comment.

Shoaib


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

* Re: [PATCH net-next] rds: transport module should be auto loaded when transport is set
  2020-06-01 16:59   ` Rao Shoaib
@ 2020-06-02  6:17     ` Leon Romanovsky
  2020-06-02 18:19       ` Rao Shoaib
  0 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2020-06-02  6:17 UTC (permalink / raw)
  To: Rao Shoaib; +Cc: David Miller, netdev, linux-rdma, somasundaram.krishnasamy

On Mon, Jun 01, 2020 at 09:59:30AM -0700, Rao Shoaib wrote:
>
> On 5/29/20 4:41 PM, David Miller wrote:
> > From: rao.shoaib@oracle.com
> > Date: Wed, 27 May 2020 01:17:42 -0700
> >
> > > diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
> > > index cba368e55863..7273c681e6c1 100644
> > > --- a/include/uapi/linux/rds.h
> > > +++ b/include/uapi/linux/rds.h
> > > @@ -64,7 +64,7 @@
> > >   /* supported values for SO_RDS_TRANSPORT */
> > >   #define	RDS_TRANS_IB	0
> > > -#define	RDS_TRANS_IWARP	1
> > > +#define	RDS_TRANS_GAP	1
> > >   #define	RDS_TRANS_TCP	2
> > >   #define RDS_TRANS_COUNT	3
> > >   #define	RDS_TRANS_NONE	(~0)
> > You can't break user facing UAPI like this, sorry.
>
> I was hoping that this could be considered an exception as IWARP has been
> deprecated for almost a decade and there is no current product using it.
> With the change any old binary will continue to work, a new compilation fill
> fail so that the code can be examined, otherwise we will never be able to
> reuse this number.
>
> If the above is not acceptable I can revert this part of the change.

Nothing prohibits you from adding the following lines:

+ /* don't use RDS_TRANS_IWARP - it is deprecated */
+ #define  RDS_TRANS_GAP RDS_TRANS_IWARP

>
> Shoaib
>

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

* Re: [PATCH net-next] rds: transport module should be auto loaded when transport is set
  2020-06-01 17:08   ` Rao Shoaib
@ 2020-06-02  6:18     ` Leon Romanovsky
  0 siblings, 0 replies; 8+ messages in thread
From: Leon Romanovsky @ 2020-06-02  6:18 UTC (permalink / raw)
  To: Rao Shoaib; +Cc: netdev, linux-rdma, Somasundaram Krishnasamy

On Mon, Jun 01, 2020 at 10:08:44AM -0700, Rao Shoaib wrote:
>
> On 5/31/20 3:08 AM, Leon Romanovsky wrote:
> > On Wed, May 27, 2020 at 01:17:42AM -0700, rao.shoaib@oracle.com wrote:
> > > From: Rao Shoaib <rao.shoaib@oracle.com>
> > >
> > > This enhancement auto loads transport module when the transport
> > > is set via SO_RDS_TRANSPORT socket option.
> > >
> > > Orabug: 31032127
> > I think that it is internal to Oracle and should not be in the commit
> > message.
> >
> > Thanks
>
> There are logs that have internals bug numbers mentioned in them.

Right, it was missed in the review.

> I do agree with you and will take the bug number out.
>
> Thanks for the comment.
>
> Shoaib
>

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

* Re: [PATCH net-next] rds: transport module should be auto loaded when transport is set
  2020-06-02  6:17     ` Leon Romanovsky
@ 2020-06-02 18:19       ` Rao Shoaib
  0 siblings, 0 replies; 8+ messages in thread
From: Rao Shoaib @ 2020-06-02 18:19 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: David Miller, netdev, linux-rdma, somasundaram.krishnasamy


On 6/1/20 11:17 PM, Leon Romanovsky wrote:
> On Mon, Jun 01, 2020 at 09:59:30AM -0700, Rao Shoaib wrote:
>> On 5/29/20 4:41 PM, David Miller wrote:
>>> From: rao.shoaib@oracle.com
>>> Date: Wed, 27 May 2020 01:17:42 -0700
>>>
>>>> diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
>>>> index cba368e55863..7273c681e6c1 100644
>>>> --- a/include/uapi/linux/rds.h
>>>> +++ b/include/uapi/linux/rds.h
>>>> @@ -64,7 +64,7 @@
>>>>    /* supported values for SO_RDS_TRANSPORT */
>>>>    #define	RDS_TRANS_IB	0
>>>> -#define	RDS_TRANS_IWARP	1
>>>> +#define	RDS_TRANS_GAP	1
>>>>    #define	RDS_TRANS_TCP	2
>>>>    #define RDS_TRANS_COUNT	3
>>>>    #define	RDS_TRANS_NONE	(~0)
>>> You can't break user facing UAPI like this, sorry.
>> I was hoping that this could be considered an exception as IWARP has been
>> deprecated for almost a decade and there is no current product using it.
>> With the change any old binary will continue to work, a new compilation fill
>> fail so that the code can be examined, otherwise we will never be able to
>> reuse this number.
>>
>> If the above is not acceptable I can revert this part of the change.
> Nothing prohibits you from adding the following lines:
>
> + /* don't use RDS_TRANS_IWARP - it is deprecated */
> + #define  RDS_TRANS_GAP RDS_TRANS_IWARP

Correct. That is what I was planning on doing in case I could not get 
rid of RDS_TRANS_IWARP. I will resubmit the patch.

Shoaib

>
>> Shoaib
>>

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

end of thread, other threads:[~2020-06-02 18:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27  8:17 [PATCH net-next] rds: transport module should be auto loaded when transport is set rao.shoaib
2020-05-29 23:41 ` David Miller
2020-06-01 16:59   ` Rao Shoaib
2020-06-02  6:17     ` Leon Romanovsky
2020-06-02 18:19       ` Rao Shoaib
2020-05-31 10:08 ` Leon Romanovsky
2020-06-01 17:08   ` Rao Shoaib
2020-06-02  6:18     ` Leon Romanovsky

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).