Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 05:58 PM, Jason Gunthorpe wrote: > On Thu, Apr 16, 2015 at 10:08:10AM +0200, Michael Wang wrote: >> >> Use raw management helpers to reform cm related part in IB-core cma/ucm. >> >> These checks focus on the device cm type rather than the port capability, >> directly pass port 1 works currently, but can't support mixing cm type >> device in future. > > After the discussion settled, I ended up thinking that implementing > explicit device checks, for use by CM, and the BUG_ON at register to > require all ports have the same value was the best option. > > It also looks like hardwired 1 won't work on switch ports, so it is no-go. AFAIK, the current HW won't trigger such Bug, actually only mlx4 using port_num to check the link-layer (but still ib cm anyway), others are just static whatever the port_num is. Thus as long as the check is still count on transport type and link-layer, the BUG may never be triggered... Regards, Michael Wang > > Jason > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 07:30 PM, Hefty, Sean wrote: >>> To be confirmed: >>> PORT ASSIGNED >>> rdma_init_qp_attr Y >>> rdma_destroy_id unknown >>> cma_listen_on_dev N >>> cma_bind_loopback N > > Bind loopback will attach to a port, but the id does not have on entry. > >>> rdma_listen N >> >> Why "N"? rdma_listen() can be constrained to a single port, right? >> And even if wildcarded, it needs to act on multiple ports, which is >> to say, it will fail only if no ports are eligible. > > Rdma listen should be unknown. The id may be assigned to a port. It depends > on the source address. Agree, so for those 'N' or 'unknow', let's use port 1 directly. > >>> rdma_connectY >>> rdma_accept Y >>> rdma_reject Y >>> rdma_disconnect Y >>> ib_ucm_add_one N > > Others look correct. > > Btw, thanks for your work on this. I know this is becoming a much bigger > change than you originally intended. :) My pleasure :-) It do exceeded my expectations, fortunately I have you guys on my side, without your comments, it's impossible to make it so far :-) Regards, Michael Wang > > - Sean > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 07:21 PM, Tom Talpey wrote: > On 4/16/2015 11:22 AM, Michael Wang wrote: >> >> >> On 04/16/2015 04:31 PM, Hefty, Sean wrote: > This is equivalent to today where the checks are per node rather than > per port. > > Should all checks here be port 1 based or only certain ones like listen > ? For example, in connect/reject/disconnect, don't we already have port > ? Guess this can be dealt with later as this is not a regression from > the current implementation. Yeah, these parts of cma may need more carve in future, like some new callback for different CM type as Sean suggested. Maybe directly using 1 could help to highlight the problem ;-) >>> >>> Only a few checks need to be per device. I think I pointed those out >>> previously. Testing should show anywhere that we miss fairly quickly, >>> since port would still be 0. For the checks that can be updated to be per >>> port, I would rather go ahead and convert them. >> >> Got it, will be changed in next version :-) >> >> To be confirmed: >> PORT ASSIGNED >> rdma_init_qp_attrY >> rdma_destroy_idunknown >> cma_listen_on_devN >> cma_bind_loopbackN >> rdma_listenN > > Why "N"? rdma_listen() can be constrained to a single port, right? > And even if wildcarded, it needs to act on multiple ports, which is > to say, it will fail only if no ports are eligible. Yeah, it can or can't, maybe 'unknown' is better :-) Regards, Michael Wang > > Tom. > > >> rdma_connectY >> rdma_acceptY >> rdma_rejectY >> rdma_disconnectY >> ib_ucm_add_oneN >> >> Is this list correct? >> >> Regards, >> Michael Wang >> >>> >>> - Sean >>> >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 07:30 PM, Hefty, Sean wrote: To be confirmed: PORT ASSIGNED rdma_init_qp_attr Y rdma_destroy_id unknown cma_listen_on_dev N cma_bind_loopback N Bind loopback will attach to a port, but the id does not have on entry. rdma_listen N Why N? rdma_listen() can be constrained to a single port, right? And even if wildcarded, it needs to act on multiple ports, which is to say, it will fail only if no ports are eligible. Rdma listen should be unknown. The id may be assigned to a port. It depends on the source address. Agree, so for those 'N' or 'unknow', let's use port 1 directly. rdma_connectY rdma_accept Y rdma_reject Y rdma_disconnect Y ib_ucm_add_one N Others look correct. Btw, thanks for your work on this. I know this is becoming a much bigger change than you originally intended. :) My pleasure :-) It do exceeded my expectations, fortunately I have you guys on my side, without your comments, it's impossible to make it so far :-) Regards, Michael Wang - Sean -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 05:58 PM, Jason Gunthorpe wrote: On Thu, Apr 16, 2015 at 10:08:10AM +0200, Michael Wang wrote: Use raw management helpers to reform cm related part in IB-core cma/ucm. These checks focus on the device cm type rather than the port capability, directly pass port 1 works currently, but can't support mixing cm type device in future. After the discussion settled, I ended up thinking that implementing explicit device checks, for use by CM, and the BUG_ON at register to require all ports have the same value was the best option. It also looks like hardwired 1 won't work on switch ports, so it is no-go. AFAIK, the current HW won't trigger such Bug, actually only mlx4 using port_num to check the link-layer (but still ib cm anyway), others are just static whatever the port_num is. Thus as long as the check is still count on transport type and link-layer, the BUG may never be triggered... Regards, Michael Wang Jason -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 07:21 PM, Tom Talpey wrote: On 4/16/2015 11:22 AM, Michael Wang wrote: On 04/16/2015 04:31 PM, Hefty, Sean wrote: This is equivalent to today where the checks are per node rather than per port. Should all checks here be port 1 based or only certain ones like listen ? For example, in connect/reject/disconnect, don't we already have port ? Guess this can be dealt with later as this is not a regression from the current implementation. Yeah, these parts of cma may need more carve in future, like some new callback for different CM type as Sean suggested. Maybe directly using 1 could help to highlight the problem ;-) Only a few checks need to be per device. I think I pointed those out previously. Testing should show anywhere that we miss fairly quickly, since port would still be 0. For the checks that can be updated to be per port, I would rather go ahead and convert them. Got it, will be changed in next version :-) To be confirmed: PORT ASSIGNED rdma_init_qp_attrY rdma_destroy_idunknown cma_listen_on_devN cma_bind_loopbackN rdma_listenN Why N? rdma_listen() can be constrained to a single port, right? And even if wildcarded, it needs to act on multiple ports, which is to say, it will fail only if no ports are eligible. Yeah, it can or can't, maybe 'unknown' is better :-) Regards, Michael Wang Tom. rdma_connectY rdma_acceptY rdma_rejectY rdma_disconnectY ib_ucm_add_oneN Is this list correct? Regards, Michael Wang - Sean -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On Thu, Apr 16, 2015 at 01:38:07PM -0400, Hal Rosenstock wrote: > On 4/16/2015 11:58 AM, Jason Gunthorpe wrote: > > It also looks like hardwired 1 won't work on switch ports, so it is no-go. > > AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the > current code. There is no need for CM/RDMA CM on base switch port 0. Hurm, yet another reason to have a proper device level test to capture all this weirdness. I am surprised to hear this, actually. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
> > On 4/16/2015 11:58 AM, Jason Gunthorpe wrote: > > It also looks like hardwired 1 won't work on switch ports, so it is no-go. > > AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the current > code. There is no need for CM/RDMA CM on base switch port 0. I concur and I thought I mentioned that before. Ira -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 4/16/2015 11:58 AM, Jason Gunthorpe wrote: > It also looks like hardwired 1 won't work on switch ports, so it is no-go. AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the current code. There is no need for CM/RDMA CM on base switch port 0. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
> > To be confirmed: > > PORT ASSIGNED > > rdma_init_qp_attr Y > > rdma_destroy_id unknown > > cma_listen_on_dev N > > cma_bind_loopback N Bind loopback will attach to a port, but the id does not have on entry. > > rdma_listen N > > Why "N"? rdma_listen() can be constrained to a single port, right? > And even if wildcarded, it needs to act on multiple ports, which is > to say, it will fail only if no ports are eligible. Rdma listen should be unknown. The id may be assigned to a port. It depends on the source address. > > rdma_connectY > > rdma_accept Y > > rdma_reject Y > > rdma_disconnect Y > > ib_ucm_add_one N Others look correct. Btw, thanks for your work on this. I know this is becoming a much bigger change than you originally intended. :) - Sean
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 4/16/2015 11:22 AM, Michael Wang wrote: On 04/16/2015 04:31 PM, Hefty, Sean wrote: This is equivalent to today where the checks are per node rather than per port. Should all checks here be port 1 based or only certain ones like listen ? For example, in connect/reject/disconnect, don't we already have port ? Guess this can be dealt with later as this is not a regression from the current implementation. Yeah, these parts of cma may need more carve in future, like some new callback for different CM type as Sean suggested. Maybe directly using 1 could help to highlight the problem ;-) Only a few checks need to be per device. I think I pointed those out previously. Testing should show anywhere that we miss fairly quickly, since port would still be 0. For the checks that can be updated to be per port, I would rather go ahead and convert them. Got it, will be changed in next version :-) To be confirmed: PORT ASSIGNED rdma_init_qp_attr Y rdma_destroy_id unknown cma_listen_on_dev N cma_bind_loopback N rdma_listen N Why "N"? rdma_listen() can be constrained to a single port, right? And even if wildcarded, it needs to act on multiple ports, which is to say, it will fail only if no ports are eligible. Tom. rdma_connectY rdma_accept Y rdma_reject Y rdma_disconnect Y ib_ucm_add_one N Is this list correct? Regards, Michael Wang - Sean -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On Thu, Apr 16, 2015 at 04:55:10PM +, Hefty, Sean wrote: > > After the discussion settled, I ended up thinking that implementing > > explicit device checks, for use by CM, and the BUG_ON at register to > > require all ports have the same value was the best option. > > Sure, but why not update the other areas anyway? This way when > listens become per port, rather than per device, we only need to > update that portion of the code. I wasn't clear, I agree: Yes, update what is possible in the CM, but use an explicit device test for the areas we can't fix. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
> After the discussion settled, I ended up thinking that implementing > explicit device checks, for use by CM, and the BUG_ON at register to > require all ports have the same value was the best option. Sure, but why not update the other areas anyway? This way when listens become per port, rather than per device, we only need to update that portion of the code. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On Thu, Apr 16, 2015 at 10:08:10AM +0200, Michael Wang wrote: > > Use raw management helpers to reform cm related part in IB-core cma/ucm. > > These checks focus on the device cm type rather than the port capability, > directly pass port 1 works currently, but can't support mixing cm type > device in future. After the discussion settled, I ended up thinking that implementing explicit device checks, for use by CM, and the BUG_ON at register to require all ports have the same value was the best option. It also looks like hardwired 1 won't work on switch ports, so it is no-go. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 04:31 PM, Hefty, Sean wrote: >>> This is equivalent to today where the checks are per node rather than >>> per port. >>> >>> Should all checks here be port 1 based or only certain ones like listen >>> ? For example, in connect/reject/disconnect, don't we already have port >>> ? Guess this can be dealt with later as this is not a regression from >>> the current implementation. >> >> Yeah, these parts of cma may need more carve in future, like some new >> callback >> for different CM type as Sean suggested. >> >> Maybe directly using 1 could help to highlight the problem ;-) > > Only a few checks need to be per device. I think I pointed those out > previously. Testing should show anywhere that we miss fairly quickly, since > port would still be 0. For the checks that can be updated to be per port, I > would rather go ahead and convert them. Got it, will be changed in next version :-) To be confirmed: PORT ASSIGNED rdma_init_qp_attr Y rdma_destroy_id unknown cma_listen_on_dev N cma_bind_loopback N rdma_listen N rdma_connectY rdma_accept Y rdma_reject Y rdma_disconnect Y ib_ucm_add_one N Is this list correct? Regards, Michael Wang > > - Sean > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
> > This is equivalent to today where the checks are per node rather than
> > per port.
> >
> > Should all checks here be port 1 based or only certain ones like listen
> > ? For example, in connect/reject/disconnect, don't we already have port
> > ? Guess this can be dealt with later as this is not a regression from
> > the current implementation.
>
> Yeah, these parts of cma may need more carve in future, like some new
> callback
> for different CM type as Sean suggested.
>
> Maybe directly using 1 could help to highlight the problem ;-)
Only a few checks need to be per device. I think I pointed those out
previously. Testing should show anywhere that we miss fairly quickly, since
port would still be 0. For the checks that can be updated to be per port, I
would rather go ahead and convert them.
- Sean
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 03:10 PM, Hal Rosenstock wrote:
> On 4/16/2015 4:08 AM, Michael Wang wrote:
>>
>> Use raw management helpers to reform cm related part in IB-core cma/ucm.
>>
>> These checks focus on the device cm type rather than the port capability,
>> directly pass port 1 works currently, but can't support mixing cm type
>> device in future.
>
> This is equivalent to today where the checks are per node rather than
> per port.
>
> Should all checks here be port 1 based or only certain ones like listen
> ? For example, in connect/reject/disconnect, don't we already have port
> ? Guess this can be dealt with later as this is not a regression from
> the current implementation.
Yeah, these parts of cma may need more carve in future, like some new callback
for different CM type as Sean suggested.
Maybe directly using 1 could help to highlight the problem ;-)
Regards,
Michael Wanga
>
> -- Hal
>
>>
>> Cc: Steve Wise
>> Cc: Tom Talpey
>> Cc: Jason Gunthorpe
>> Cc: Doug Ledford
>> Cc: Ira Weiny
>> Cc: Sean Hefty
>> Signed-off-by: Michael Wang
>> ---
>> drivers/infiniband/core/cma.c | 81
>> +--
>> drivers/infiniband/core/ucm.c | 3 +-
>> 2 files changed, 26 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
>> index d570030..6b8a64d 100644
>> --- a/drivers/infiniband/core/cma.c
>> +++ b/drivers/infiniband/core/cma.c
>> @@ -735,8 +735,7 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct
>> ib_qp_attr *qp_attr,
>> int ret = 0;
>>
>> id_priv = container_of(id, struct rdma_id_private, id);
>> -switch (rdma_node_get_transport(id_priv->id.device->node_type)) {
>> -case RDMA_TRANSPORT_IB:
>> +if (rdma_ib_or_iboe(id_priv->id.device, 1)) {
>> if (!id_priv->cm_id.ib || (id_priv->id.qp_type == IB_QPT_UD))
>> ret = cma_ib_init_qp_attr(id_priv, qp_attr,
>> qp_attr_mask);
>> else
>> @@ -745,19 +744,15 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct
>> ib_qp_attr *qp_attr,
>>
>> if (qp_attr->qp_state == IB_QPS_RTR)
>> qp_attr->rq_psn = id_priv->seq_num;
>> -break;
>> -case RDMA_TRANSPORT_IWARP:
>> +} else if (rdma_tech_iwarp(id_priv->id.device, 1)) {
>> if (!id_priv->cm_id.iw) {
>> qp_attr->qp_access_flags = 0;
>> *qp_attr_mask = IB_QP_STATE | IB_QP_ACCESS_FLAGS;
>> } else
>> ret = iw_cm_init_qp_attr(id_priv->cm_id.iw, qp_attr,
>> qp_attr_mask);
>> -break;
>> -default:
>> +} else
>> ret = -ENOSYS;
>> -break;
>> -}
>>
>> return ret;
>> }
>> @@ -1037,17 +1032,12 @@ void rdma_destroy_id(struct rdma_cm_id *id)
>> mutex_unlock(_priv->handler_mutex);
>>
>> if (id_priv->cma_dev) {
>> -switch (rdma_node_get_transport(id_priv->id.device->node_type))
>> {
>> -case RDMA_TRANSPORT_IB:
>> +if (rdma_ib_or_iboe(id_priv->id.device, 1)) {
>> if (id_priv->cm_id.ib)
>> ib_destroy_cm_id(id_priv->cm_id.ib);
>> -break;
>> -case RDMA_TRANSPORT_IWARP:
>> +} else if (rdma_tech_iwarp(id_priv->id.device, 1)) {
>> if (id_priv->cm_id.iw)
>> iw_destroy_cm_id(id_priv->cm_id.iw);
>> -break;
>> -default:
>> -break;
>> }
>> cma_leave_mc_groups(id_priv);
>> cma_release_dev(id_priv);
>> @@ -1626,7 +1616,7 @@ static void cma_listen_on_dev(struct rdma_id_private
>> *id_priv,
>> int ret;
>>
>> if (cma_family(id_priv) == AF_IB &&
>> -rdma_node_get_transport(cma_dev->device->node_type) !=
>> RDMA_TRANSPORT_IB)
>> +!rdma_ib_or_iboe(cma_dev->device, 1))
>> return;
>>
>> id = rdma_create_id(cma_listen_handler, id_priv, id_priv->id.ps,
>> @@ -2028,7 +2018,7 @@ static int cma_bind_loopback(struct rdma_id_private
>> *id_priv)
>> mutex_lock();
>> list_for_each_entry(cur_dev, _list, list) {
>> if (cma_family(id_priv) == AF_IB &&
>> -rdma_node_get_transport(cur_dev->device->node_type) !=
>> RDMA_TRANSPORT_IB)
>> +!rdma_ib_or_iboe(cur_dev->device, 1))
>> continue;
>>
>> if (!cma_dev)
>> @@ -2060,7 +2050,7 @@ port_found:
>> goto out;
>>
>> id_priv->id.route.addr.dev_addr.dev_type =
>> -(rdma_port_get_link_layer(cma_dev->device, p) ==
>> IB_LINK_LAYER_INFINIBAND) ?
>> +(rdma_tech_ib(cma_dev->device, p)) ?
>> ARPHRD_INFINIBAND : ARPHRD_ETHER;
>>
>> rdma_addr_set_sgid(_priv->id.route.addr.dev_addr, );
>> @@ -2537,18 +2527,15 @@ int rdma_listen(struct
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 4/16/2015 4:08 AM, Michael Wang wrote:
>
> Use raw management helpers to reform cm related part in IB-core cma/ucm.
>
> These checks focus on the device cm type rather than the port capability,
> directly pass port 1 works currently, but can't support mixing cm type
> device in future.
This is equivalent to today where the checks are per node rather than
per port.
Should all checks here be port 1 based or only certain ones like listen
? For example, in connect/reject/disconnect, don't we already have port
? Guess this can be dealt with later as this is not a regression from
the current implementation.
-- Hal
>
> Cc: Steve Wise
> Cc: Tom Talpey
> Cc: Jason Gunthorpe
> Cc: Doug Ledford
> Cc: Ira Weiny
> Cc: Sean Hefty
> Signed-off-by: Michael Wang
> ---
> drivers/infiniband/core/cma.c | 81
> +--
> drivers/infiniband/core/ucm.c | 3 +-
> 2 files changed, 26 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index d570030..6b8a64d 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -735,8 +735,7 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct
> ib_qp_attr *qp_attr,
> int ret = 0;
>
> id_priv = container_of(id, struct rdma_id_private, id);
> - switch (rdma_node_get_transport(id_priv->id.device->node_type)) {
> - case RDMA_TRANSPORT_IB:
> + if (rdma_ib_or_iboe(id_priv->id.device, 1)) {
> if (!id_priv->cm_id.ib || (id_priv->id.qp_type == IB_QPT_UD))
> ret = cma_ib_init_qp_attr(id_priv, qp_attr,
> qp_attr_mask);
> else
> @@ -745,19 +744,15 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct
> ib_qp_attr *qp_attr,
>
> if (qp_attr->qp_state == IB_QPS_RTR)
> qp_attr->rq_psn = id_priv->seq_num;
> - break;
> - case RDMA_TRANSPORT_IWARP:
> + } else if (rdma_tech_iwarp(id_priv->id.device, 1)) {
> if (!id_priv->cm_id.iw) {
> qp_attr->qp_access_flags = 0;
> *qp_attr_mask = IB_QP_STATE | IB_QP_ACCESS_FLAGS;
> } else
> ret = iw_cm_init_qp_attr(id_priv->cm_id.iw, qp_attr,
>qp_attr_mask);
> - break;
> - default:
> + } else
> ret = -ENOSYS;
> - break;
> - }
>
> return ret;
> }
> @@ -1037,17 +1032,12 @@ void rdma_destroy_id(struct rdma_cm_id *id)
> mutex_unlock(_priv->handler_mutex);
>
> if (id_priv->cma_dev) {
> - switch (rdma_node_get_transport(id_priv->id.device->node_type))
> {
> - case RDMA_TRANSPORT_IB:
> + if (rdma_ib_or_iboe(id_priv->id.device, 1)) {
> if (id_priv->cm_id.ib)
> ib_destroy_cm_id(id_priv->cm_id.ib);
> - break;
> - case RDMA_TRANSPORT_IWARP:
> + } else if (rdma_tech_iwarp(id_priv->id.device, 1)) {
> if (id_priv->cm_id.iw)
> iw_destroy_cm_id(id_priv->cm_id.iw);
> - break;
> - default:
> - break;
> }
> cma_leave_mc_groups(id_priv);
> cma_release_dev(id_priv);
> @@ -1626,7 +1616,7 @@ static void cma_listen_on_dev(struct rdma_id_private
> *id_priv,
> int ret;
>
> if (cma_family(id_priv) == AF_IB &&
> - rdma_node_get_transport(cma_dev->device->node_type) !=
> RDMA_TRANSPORT_IB)
> + !rdma_ib_or_iboe(cma_dev->device, 1))
> return;
>
> id = rdma_create_id(cma_listen_handler, id_priv, id_priv->id.ps,
> @@ -2028,7 +2018,7 @@ static int cma_bind_loopback(struct rdma_id_private
> *id_priv)
> mutex_lock();
> list_for_each_entry(cur_dev, _list, list) {
> if (cma_family(id_priv) == AF_IB &&
> - rdma_node_get_transport(cur_dev->device->node_type) !=
> RDMA_TRANSPORT_IB)
> + !rdma_ib_or_iboe(cur_dev->device, 1))
> continue;
>
> if (!cma_dev)
> @@ -2060,7 +2050,7 @@ port_found:
> goto out;
>
> id_priv->id.route.addr.dev_addr.dev_type =
> - (rdma_port_get_link_layer(cma_dev->device, p) ==
> IB_LINK_LAYER_INFINIBAND) ?
> + (rdma_tech_ib(cma_dev->device, p)) ?
> ARPHRD_INFINIBAND : ARPHRD_ETHER;
>
> rdma_addr_set_sgid(_priv->id.route.addr.dev_addr, );
> @@ -2537,18 +2527,15 @@ int rdma_listen(struct rdma_cm_id *id, int backlog)
>
> id_priv->backlog = backlog;
> if (id->device) {
> - switch (rdma_node_get_transport(id->device->node_type)) {
> - case RDMA_TRANSPORT_IB:
> + if (rdma_ib_or_iboe(id->device, 1)) {
> ret = cma_ib_listen(id_priv);
>
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 03:10 PM, Hal Rosenstock wrote:
On 4/16/2015 4:08 AM, Michael Wang wrote:
Use raw management helpers to reform cm related part in IB-core cma/ucm.
These checks focus on the device cm type rather than the port capability,
directly pass port 1 works currently, but can't support mixing cm type
device in future.
This is equivalent to today where the checks are per node rather than
per port.
Should all checks here be port 1 based or only certain ones like listen
? For example, in connect/reject/disconnect, don't we already have port
? Guess this can be dealt with later as this is not a regression from
the current implementation.
Yeah, these parts of cma may need more carve in future, like some new callback
for different CM type as Sean suggested.
Maybe directly using 1 could help to highlight the problem ;-)
Regards,
Michael Wanga
-- Hal
Cc: Steve Wise sw...@opengridcomputing.com
Cc: Tom Talpey t...@talpey.com
Cc: Jason Gunthorpe jguntho...@obsidianresearch.com
Cc: Doug Ledford dledf...@redhat.com
Cc: Ira Weiny ira.we...@intel.com
Cc: Sean Hefty sean.he...@intel.com
Signed-off-by: Michael Wang yun.w...@profitbricks.com
---
drivers/infiniband/core/cma.c | 81
+--
drivers/infiniband/core/ucm.c | 3 +-
2 files changed, 26 insertions(+), 58 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d570030..6b8a64d 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -735,8 +735,7 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct
ib_qp_attr *qp_attr,
int ret = 0;
id_priv = container_of(id, struct rdma_id_private, id);
-switch (rdma_node_get_transport(id_priv-id.device-node_type)) {
-case RDMA_TRANSPORT_IB:
+if (rdma_ib_or_iboe(id_priv-id.device, 1)) {
if (!id_priv-cm_id.ib || (id_priv-id.qp_type == IB_QPT_UD))
ret = cma_ib_init_qp_attr(id_priv, qp_attr,
qp_attr_mask);
else
@@ -745,19 +744,15 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct
ib_qp_attr *qp_attr,
if (qp_attr-qp_state == IB_QPS_RTR)
qp_attr-rq_psn = id_priv-seq_num;
-break;
-case RDMA_TRANSPORT_IWARP:
+} else if (rdma_tech_iwarp(id_priv-id.device, 1)) {
if (!id_priv-cm_id.iw) {
qp_attr-qp_access_flags = 0;
*qp_attr_mask = IB_QP_STATE | IB_QP_ACCESS_FLAGS;
} else
ret = iw_cm_init_qp_attr(id_priv-cm_id.iw, qp_attr,
qp_attr_mask);
-break;
-default:
+} else
ret = -ENOSYS;
-break;
-}
return ret;
}
@@ -1037,17 +1032,12 @@ void rdma_destroy_id(struct rdma_cm_id *id)
mutex_unlock(id_priv-handler_mutex);
if (id_priv-cma_dev) {
-switch (rdma_node_get_transport(id_priv-id.device-node_type))
{
-case RDMA_TRANSPORT_IB:
+if (rdma_ib_or_iboe(id_priv-id.device, 1)) {
if (id_priv-cm_id.ib)
ib_destroy_cm_id(id_priv-cm_id.ib);
-break;
-case RDMA_TRANSPORT_IWARP:
+} else if (rdma_tech_iwarp(id_priv-id.device, 1)) {
if (id_priv-cm_id.iw)
iw_destroy_cm_id(id_priv-cm_id.iw);
-break;
-default:
-break;
}
cma_leave_mc_groups(id_priv);
cma_release_dev(id_priv);
@@ -1626,7 +1616,7 @@ static void cma_listen_on_dev(struct rdma_id_private
*id_priv,
int ret;
if (cma_family(id_priv) == AF_IB
-rdma_node_get_transport(cma_dev-device-node_type) !=
RDMA_TRANSPORT_IB)
+!rdma_ib_or_iboe(cma_dev-device, 1))
return;
id = rdma_create_id(cma_listen_handler, id_priv, id_priv-id.ps,
@@ -2028,7 +2018,7 @@ static int cma_bind_loopback(struct rdma_id_private
*id_priv)
mutex_lock(lock);
list_for_each_entry(cur_dev, dev_list, list) {
if (cma_family(id_priv) == AF_IB
-rdma_node_get_transport(cur_dev-device-node_type) !=
RDMA_TRANSPORT_IB)
+!rdma_ib_or_iboe(cur_dev-device, 1))
continue;
if (!cma_dev)
@@ -2060,7 +2050,7 @@ port_found:
goto out;
id_priv-id.route.addr.dev_addr.dev_type =
-(rdma_port_get_link_layer(cma_dev-device, p) ==
IB_LINK_LAYER_INFINIBAND) ?
+(rdma_tech_ib(cma_dev-device, p)) ?
ARPHRD_INFINIBAND : ARPHRD_ETHER;
rdma_addr_set_sgid(id_priv-id.route.addr.dev_addr, gid);
@@ -2537,18 +2527,15 @@ int rdma_listen(struct rdma_cm_id *id, int backlog)
id_priv-backlog = backlog;
if (id-device) {
-switch
RE: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
This is equivalent to today where the checks are per node rather than
per port.
Should all checks here be port 1 based or only certain ones like listen
? For example, in connect/reject/disconnect, don't we already have port
? Guess this can be dealt with later as this is not a regression from
the current implementation.
Yeah, these parts of cma may need more carve in future, like some new
callback
for different CM type as Sean suggested.
Maybe directly using 1 could help to highlight the problem ;-)
Only a few checks need to be per device. I think I pointed those out
previously. Testing should show anywhere that we miss fairly quickly, since
port would still be 0. For the checks that can be updated to be per port, I
would rather go ahead and convert them.
- Sean
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On Thu, Apr 16, 2015 at 10:08:10AM +0200, Michael Wang wrote: Use raw management helpers to reform cm related part in IB-core cma/ucm. These checks focus on the device cm type rather than the port capability, directly pass port 1 works currently, but can't support mixing cm type device in future. After the discussion settled, I ended up thinking that implementing explicit device checks, for use by CM, and the BUG_ON at register to require all ports have the same value was the best option. It also looks like hardwired 1 won't work on switch ports, so it is no-go. Jason -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 04/16/2015 04:31 PM, Hefty, Sean wrote: This is equivalent to today where the checks are per node rather than per port. Should all checks here be port 1 based or only certain ones like listen ? For example, in connect/reject/disconnect, don't we already have port ? Guess this can be dealt with later as this is not a regression from the current implementation. Yeah, these parts of cma may need more carve in future, like some new callback for different CM type as Sean suggested. Maybe directly using 1 could help to highlight the problem ;-) Only a few checks need to be per device. I think I pointed those out previously. Testing should show anywhere that we miss fairly quickly, since port would still be 0. For the checks that can be updated to be per port, I would rather go ahead and convert them. Got it, will be changed in next version :-) To be confirmed: PORT ASSIGNED rdma_init_qp_attr Y rdma_destroy_id unknown cma_listen_on_dev N cma_bind_loopback N rdma_listen N rdma_connectY rdma_accept Y rdma_reject Y rdma_disconnect Y ib_ucm_add_one N Is this list correct? Regards, Michael Wang - Sean -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
After the discussion settled, I ended up thinking that implementing explicit device checks, for use by CM, and the BUG_ON at register to require all ports have the same value was the best option. Sure, but why not update the other areas anyway? This way when listens become per port, rather than per device, we only need to update that portion of the code. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On Thu, Apr 16, 2015 at 04:55:10PM +, Hefty, Sean wrote: After the discussion settled, I ended up thinking that implementing explicit device checks, for use by CM, and the BUG_ON at register to require all ports have the same value was the best option. Sure, but why not update the other areas anyway? This way when listens become per port, rather than per device, we only need to update that portion of the code. I wasn't clear, I agree: Yes, update what is possible in the CM, but use an explicit device test for the areas we can't fix. Jason -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 4/16/2015 11:22 AM, Michael Wang wrote: On 04/16/2015 04:31 PM, Hefty, Sean wrote: This is equivalent to today where the checks are per node rather than per port. Should all checks here be port 1 based or only certain ones like listen ? For example, in connect/reject/disconnect, don't we already have port ? Guess this can be dealt with later as this is not a regression from the current implementation. Yeah, these parts of cma may need more carve in future, like some new callback for different CM type as Sean suggested. Maybe directly using 1 could help to highlight the problem ;-) Only a few checks need to be per device. I think I pointed those out previously. Testing should show anywhere that we miss fairly quickly, since port would still be 0. For the checks that can be updated to be per port, I would rather go ahead and convert them. Got it, will be changed in next version :-) To be confirmed: PORT ASSIGNED rdma_init_qp_attr Y rdma_destroy_id unknown cma_listen_on_dev N cma_bind_loopback N rdma_listen N Why N? rdma_listen() can be constrained to a single port, right? And even if wildcarded, it needs to act on multiple ports, which is to say, it will fail only if no ports are eligible. Tom. rdma_connectY rdma_accept Y rdma_reject Y rdma_disconnect Y ib_ucm_add_one N Is this list correct? Regards, Michael Wang - Sean -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 4/16/2015 4:08 AM, Michael Wang wrote:
Use raw management helpers to reform cm related part in IB-core cma/ucm.
These checks focus on the device cm type rather than the port capability,
directly pass port 1 works currently, but can't support mixing cm type
device in future.
This is equivalent to today where the checks are per node rather than
per port.
Should all checks here be port 1 based or only certain ones like listen
? For example, in connect/reject/disconnect, don't we already have port
? Guess this can be dealt with later as this is not a regression from
the current implementation.
-- Hal
Cc: Steve Wise sw...@opengridcomputing.com
Cc: Tom Talpey t...@talpey.com
Cc: Jason Gunthorpe jguntho...@obsidianresearch.com
Cc: Doug Ledford dledf...@redhat.com
Cc: Ira Weiny ira.we...@intel.com
Cc: Sean Hefty sean.he...@intel.com
Signed-off-by: Michael Wang yun.w...@profitbricks.com
---
drivers/infiniband/core/cma.c | 81
+--
drivers/infiniband/core/ucm.c | 3 +-
2 files changed, 26 insertions(+), 58 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d570030..6b8a64d 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -735,8 +735,7 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct
ib_qp_attr *qp_attr,
int ret = 0;
id_priv = container_of(id, struct rdma_id_private, id);
- switch (rdma_node_get_transport(id_priv-id.device-node_type)) {
- case RDMA_TRANSPORT_IB:
+ if (rdma_ib_or_iboe(id_priv-id.device, 1)) {
if (!id_priv-cm_id.ib || (id_priv-id.qp_type == IB_QPT_UD))
ret = cma_ib_init_qp_attr(id_priv, qp_attr,
qp_attr_mask);
else
@@ -745,19 +744,15 @@ int rdma_init_qp_attr(struct rdma_cm_id *id, struct
ib_qp_attr *qp_attr,
if (qp_attr-qp_state == IB_QPS_RTR)
qp_attr-rq_psn = id_priv-seq_num;
- break;
- case RDMA_TRANSPORT_IWARP:
+ } else if (rdma_tech_iwarp(id_priv-id.device, 1)) {
if (!id_priv-cm_id.iw) {
qp_attr-qp_access_flags = 0;
*qp_attr_mask = IB_QP_STATE | IB_QP_ACCESS_FLAGS;
} else
ret = iw_cm_init_qp_attr(id_priv-cm_id.iw, qp_attr,
qp_attr_mask);
- break;
- default:
+ } else
ret = -ENOSYS;
- break;
- }
return ret;
}
@@ -1037,17 +1032,12 @@ void rdma_destroy_id(struct rdma_cm_id *id)
mutex_unlock(id_priv-handler_mutex);
if (id_priv-cma_dev) {
- switch (rdma_node_get_transport(id_priv-id.device-node_type))
{
- case RDMA_TRANSPORT_IB:
+ if (rdma_ib_or_iboe(id_priv-id.device, 1)) {
if (id_priv-cm_id.ib)
ib_destroy_cm_id(id_priv-cm_id.ib);
- break;
- case RDMA_TRANSPORT_IWARP:
+ } else if (rdma_tech_iwarp(id_priv-id.device, 1)) {
if (id_priv-cm_id.iw)
iw_destroy_cm_id(id_priv-cm_id.iw);
- break;
- default:
- break;
}
cma_leave_mc_groups(id_priv);
cma_release_dev(id_priv);
@@ -1626,7 +1616,7 @@ static void cma_listen_on_dev(struct rdma_id_private
*id_priv,
int ret;
if (cma_family(id_priv) == AF_IB
- rdma_node_get_transport(cma_dev-device-node_type) !=
RDMA_TRANSPORT_IB)
+ !rdma_ib_or_iboe(cma_dev-device, 1))
return;
id = rdma_create_id(cma_listen_handler, id_priv, id_priv-id.ps,
@@ -2028,7 +2018,7 @@ static int cma_bind_loopback(struct rdma_id_private
*id_priv)
mutex_lock(lock);
list_for_each_entry(cur_dev, dev_list, list) {
if (cma_family(id_priv) == AF_IB
- rdma_node_get_transport(cur_dev-device-node_type) !=
RDMA_TRANSPORT_IB)
+ !rdma_ib_or_iboe(cur_dev-device, 1))
continue;
if (!cma_dev)
@@ -2060,7 +2050,7 @@ port_found:
goto out;
id_priv-id.route.addr.dev_addr.dev_type =
- (rdma_port_get_link_layer(cma_dev-device, p) ==
IB_LINK_LAYER_INFINIBAND) ?
+ (rdma_tech_ib(cma_dev-device, p)) ?
ARPHRD_INFINIBAND : ARPHRD_ETHER;
rdma_addr_set_sgid(id_priv-id.route.addr.dev_addr, gid);
@@ -2537,18 +2527,15 @@ int rdma_listen(struct rdma_cm_id *id, int backlog)
id_priv-backlog = backlog;
if (id-device) {
- switch (rdma_node_get_transport(id-device-node_type)) {
- case RDMA_TRANSPORT_IB:
+ if (rdma_ib_or_iboe(id-device, 1)) {
ret = cma_ib_listen(id_priv);
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 4/16/2015 11:58 AM, Jason Gunthorpe wrote: It also looks like hardwired 1 won't work on switch ports, so it is no-go. AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the current code. There is no need for CM/RDMA CM on base switch port 0. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On 4/16/2015 11:58 AM, Jason Gunthorpe wrote: It also looks like hardwired 1 won't work on switch ports, so it is no-go. AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the current code. There is no need for CM/RDMA CM on base switch port 0. I concur and I thought I mentioned that before. Ira -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
On Thu, Apr 16, 2015 at 01:38:07PM -0400, Hal Rosenstock wrote: On 4/16/2015 11:58 AM, Jason Gunthorpe wrote: It also looks like hardwired 1 won't work on switch ports, so it is no-go. AFAIK enhanced switch port 0 is not supported by CM/RDMA CM in the current code. There is no need for CM/RDMA CM on base switch port 0. Hurm, yet another reason to have a proper device level test to capture all this weirdness. I am surprised to hear this, actually. Jason -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 10/27] IB/Verbs: Reform cm related part in IB-core cma/ucm
To be confirmed: PORT ASSIGNED rdma_init_qp_attr Y rdma_destroy_id unknown cma_listen_on_dev N cma_bind_loopback N Bind loopback will attach to a port, but the id does not have on entry. rdma_listen N Why N? rdma_listen() can be constrained to a single port, right? And even if wildcarded, it needs to act on multiple ports, which is to say, it will fail only if no ports are eligible. Rdma listen should be unknown. The id may be assigned to a port. It depends on the source address. rdma_connectY rdma_accept Y rdma_reject Y rdma_disconnect Y ib_ucm_add_one N Others look correct. Btw, thanks for your work on this. I know this is becoming a much bigger change than you originally intended. :) - Sean
