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