| Message ID | 1617705423-15570-1-git-send-email-haakon.bugge@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [for-next] RDMA/core: Unify RoCE check and re-factor code | expand |
On Tue, Apr 06, 2021 at 12:37:03PM +0200, Håkon Bugge wrote: > In cm_req_handler(), unify the check for RoCE and re-factor to avoid > one test. > > Suggested-by: Jason Gunthorpe <jgg@nvidia.com> > Fixes: 8f9748602491 ("IB/cm: Reduce dependency on gid attribute ndev check") > Fixes: 194f64a3cad3 ("RDMA/core: Fix corrupted SL on passive side") > Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com> > --- > drivers/infiniband/core/cm.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c > index 32c836b..074faff 100644 > --- a/drivers/infiniband/core/cm.c > +++ b/drivers/infiniband/core/cm.c > @@ -2138,21 +2138,17 @@ static int cm_req_handler(struct cm_work *work) > goto destroy; > } > > - if (cm_id_priv->av.ah_attr.type != RDMA_AH_ATTR_TYPE_ROCE) > - cm_process_routed_req(req_msg, work->mad_recv_wc->wc); > - > memset(&work->path[0], 0, sizeof(work->path[0])); > if (cm_req_has_alt_path(req_msg)) > memset(&work->path[1], 0, sizeof(work->path[1])); > grh = rdma_ah_read_grh(&cm_id_priv->av.ah_attr); > gid_attr = grh->sgid_attr; > > - if (gid_attr && > - rdma_protocol_roce(work->port->cm_dev->ib_device, > - work->port->port_num)) { > + if (gid_attr && cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE) { I think your other note was right, the gid_attr cannot be NULL when in ROCE mode, so we can delete the 'gid_attr &&' term too Jason
> On 8 Apr 2021, at 14:25, Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Tue, Apr 06, 2021 at 12:37:03PM +0200, Håkon Bugge wrote: >> In cm_req_handler(), unify the check for RoCE and re-factor to avoid >> one test. >> >> Suggested-by: Jason Gunthorpe <jgg@nvidia.com> >> Fixes: 8f9748602491 ("IB/cm: Reduce dependency on gid attribute ndev check") >> Fixes: 194f64a3cad3 ("RDMA/core: Fix corrupted SL on passive side") >> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com> >> --- >> drivers/infiniband/core/cm.c | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c >> index 32c836b..074faff 100644 >> --- a/drivers/infiniband/core/cm.c >> +++ b/drivers/infiniband/core/cm.c >> @@ -2138,21 +2138,17 @@ static int cm_req_handler(struct cm_work *work) >> goto destroy; >> } >> >> - if (cm_id_priv->av.ah_attr.type != RDMA_AH_ATTR_TYPE_ROCE) >> - cm_process_routed_req(req_msg, work->mad_recv_wc->wc); >> - >> memset(&work->path[0], 0, sizeof(work->path[0])); >> if (cm_req_has_alt_path(req_msg)) >> memset(&work->path[1], 0, sizeof(work->path[1])); >> grh = rdma_ah_read_grh(&cm_id_priv->av.ah_attr); >> gid_attr = grh->sgid_attr; >> >> - if (gid_attr && >> - rdma_protocol_roce(work->port->cm_dev->ib_device, >> - work->port->port_num)) { >> + if (gid_attr && cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE) { > > I think your other note was right, the gid_attr cannot be NULL when in > ROCE mode, so we can delete the 'gid_attr &&' term too Shall I send a v2 or do you fix it when you merge? Håkon
> On 8 Apr 2021, at 15:24, Haakon Bugge <haakon.bugge@oracle.com> wrote: > > > >> On 8 Apr 2021, at 14:25, Jason Gunthorpe <jgg@nvidia.com> wrote: >> >> On Tue, Apr 06, 2021 at 12:37:03PM +0200, Håkon Bugge wrote: >>> In cm_req_handler(), unify the check for RoCE and re-factor to avoid >>> one test. >>> >>> Suggested-by: Jason Gunthorpe <jgg@nvidia.com> >>> Fixes: 8f9748602491 ("IB/cm: Reduce dependency on gid attribute ndev check") >>> Fixes: 194f64a3cad3 ("RDMA/core: Fix corrupted SL on passive side") >>> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com> >>> --- >>> drivers/infiniband/core/cm.c | 8 ++------ >>> 1 file changed, 2 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c >>> index 32c836b..074faff 100644 >>> --- a/drivers/infiniband/core/cm.c >>> +++ b/drivers/infiniband/core/cm.c >>> @@ -2138,21 +2138,17 @@ static int cm_req_handler(struct cm_work *work) >>> goto destroy; >>> } >>> >>> - if (cm_id_priv->av.ah_attr.type != RDMA_AH_ATTR_TYPE_ROCE) >>> - cm_process_routed_req(req_msg, work->mad_recv_wc->wc); >>> - >>> memset(&work->path[0], 0, sizeof(work->path[0])); >>> if (cm_req_has_alt_path(req_msg)) >>> memset(&work->path[1], 0, sizeof(work->path[1])); >>> grh = rdma_ah_read_grh(&cm_id_priv->av.ah_attr); >>> gid_attr = grh->sgid_attr; >>> >>> - if (gid_attr && >>> - rdma_protocol_roce(work->port->cm_dev->ib_device, >>> - work->port->port_num)) { >>> + if (gid_attr && cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE) { >> >> I think your other note was right, the gid_attr cannot be NULL when in >> ROCE mode, so we can delete the 'gid_attr &&' term too > > Shall I send a v2 or do you fix it when you merge? Have you made up you mind here :-) Håkon
On Mon, Apr 19, 2021 at 03:49:08PM +0000, Haakon Bugge wrote: > >> On 8 Apr 2021, at 14:25, Jason Gunthorpe <jgg@nvidia.com> wrote: > >> > >> On Tue, Apr 06, 2021 at 12:37:03PM +0200, Håkon Bugge wrote: > >>> In cm_req_handler(), unify the check for RoCE and re-factor to avoid > >>> one test. > >>> > >>> Suggested-by: Jason Gunthorpe <jgg@nvidia.com> > >>> Fixes: 8f9748602491 ("IB/cm: Reduce dependency on gid attribute ndev check") > >>> Fixes: 194f64a3cad3 ("RDMA/core: Fix corrupted SL on passive side") > >>> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com> > >>> drivers/infiniband/core/cm.c | 8 ++------ > >>> 1 file changed, 2 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c > >>> index 32c836b..074faff 100644 > >>> +++ b/drivers/infiniband/core/cm.c > >>> @@ -2138,21 +2138,17 @@ static int cm_req_handler(struct cm_work *work) > >>> goto destroy; > >>> } > >>> > >>> - if (cm_id_priv->av.ah_attr.type != RDMA_AH_ATTR_TYPE_ROCE) > >>> - cm_process_routed_req(req_msg, work->mad_recv_wc->wc); > >>> - > >>> memset(&work->path[0], 0, sizeof(work->path[0])); > >>> if (cm_req_has_alt_path(req_msg)) > >>> memset(&work->path[1], 0, sizeof(work->path[1])); > >>> grh = rdma_ah_read_grh(&cm_id_priv->av.ah_attr); > >>> gid_attr = grh->sgid_attr; > >>> > >>> - if (gid_attr && > >>> - rdma_protocol_roce(work->port->cm_dev->ib_device, > >>> - work->port->port_num)) { > >>> + if (gid_attr && cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE) { > >> > >> I think your other note was right, the gid_attr cannot be NULL when in > >> ROCE mode, so we can delete the 'gid_attr &&' term too > > > > Shall I send a v2 or do you fix it when you merge? > > Have you made up you mind here :-) Well, I marked it as changes-requested so I guess I did.. But anyway I'll fix it up, it does look obviously correct Applied to for-next Jason
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index 32c836b..074faff 100644 --- a/drivers/infiniband/core/cm.c +++ b/drivers/infiniband/core/cm.c @@ -2138,21 +2138,17 @@ static int cm_req_handler(struct cm_work *work) goto destroy; } - if (cm_id_priv->av.ah_attr.type != RDMA_AH_ATTR_TYPE_ROCE) - cm_process_routed_req(req_msg, work->mad_recv_wc->wc); - memset(&work->path[0], 0, sizeof(work->path[0])); if (cm_req_has_alt_path(req_msg)) memset(&work->path[1], 0, sizeof(work->path[1])); grh = rdma_ah_read_grh(&cm_id_priv->av.ah_attr); gid_attr = grh->sgid_attr; - if (gid_attr && - rdma_protocol_roce(work->port->cm_dev->ib_device, - work->port->port_num)) { + if (gid_attr && cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE) { work->path[0].rec_type = sa_conv_gid_to_pathrec_type(gid_attr->gid_type); } else { + cm_process_routed_req(req_msg, work->mad_recv_wc->wc); cm_path_set_rec_type( work->port->cm_dev->ib_device, work->port->port_num, &work->path[0],
In cm_req_handler(), unify the check for RoCE and re-factor to avoid one test. Suggested-by: Jason Gunthorpe <jgg@nvidia.com> Fixes: 8f9748602491 ("IB/cm: Reduce dependency on gid attribute ndev check") Fixes: 194f64a3cad3 ("RDMA/core: Fix corrupted SL on passive side") Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com> --- drivers/infiniband/core/cm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)