| Message ID | e44d323a-3b53-a7f2-1571-25af181d6467@dev.mellanox.co.il (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [opensm] osm_resp.c: No need to swap DR [D/S]LIDs in resp_make_resp_smp | expand |
> On 26 Nov 2018, at 14:27, Hal Rosenstock <hal@dev.mellanox.co.il> wrote: > > > C14-10.1.1 states that the DrSLID and DrDLID in the directed route response > SMP shall be copied as is from the request SMP. > > Pointed-out-by: Honggang Li <honli@redhat.com> > > Signed-off-by: Hal Rosenstock <hal@mellanox.com> > --- > opensm/osm_resp.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/opensm/osm_resp.c b/opensm/osm_resp.c > index 3f270e6..9a98df9 100644 > --- a/opensm/osm_resp.c > +++ b/opensm/osm_resp.c > @@ -85,8 +85,6 @@ static void resp_make_resp_smp(IN osm_sm_t * sm, IN const ib_smp_t * p_src_smp, > if (p_src_smp->mgmt_class == IB_MCLASS_SUBN_DIR) > p_dest_smp->status |= IB_SMP_DIRECTION; > > - p_dest_smp->dr_dlid = p_dest_smp->dr_slid; > - p_dest_smp->dr_slid = p_dest_smp->dr_dlid; > memcpy(&p_dest_smp->data, p_payload, IB_SMP_DATA_SIZE); This patch is correct, wrt. the IBTA spec, AFAICT. However, this bug must have been present for ~1/8 century, and partially DR SMPs may be negative affected by this commit. Any thoughts on that? Well, having raised the issue, then you have my: Reviewed-by: Håkon Bugge <haakon.bugge@oracle.com> Thxs, Håkon
On 11/26/2018 9:34 AM, Håkon Bugge wrote: > > >> On 26 Nov 2018, at 14:27, Hal Rosenstock <hal@dev.mellanox.co.il> wrote: >> >> >> C14-10.1.1 states that the DrSLID and DrDLID in the directed route response >> SMP shall be copied as is from the request SMP. >> >> Pointed-out-by: Honggang Li <honli@redhat.com> >> >> Signed-off-by: Hal Rosenstock <hal@mellanox.com> >> --- >> opensm/osm_resp.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/opensm/osm_resp.c b/opensm/osm_resp.c >> index 3f270e6..9a98df9 100644 >> --- a/opensm/osm_resp.c >> +++ b/opensm/osm_resp.c >> @@ -85,8 +85,6 @@ static void resp_make_resp_smp(IN osm_sm_t * sm, IN const ib_smp_t * p_src_smp, >> if (p_src_smp->mgmt_class == IB_MCLASS_SUBN_DIR) >> p_dest_smp->status |= IB_SMP_DIRECTION; >> >> - p_dest_smp->dr_dlid = p_dest_smp->dr_slid; >> - p_dest_smp->dr_slid = p_dest_smp->dr_dlid; >> memcpy(&p_dest_smp->data, p_payload, IB_SMP_DATA_SIZE); > > This patch is correct, wrt. the IBTA spec, AFAICT. > > However, this bug must have been present for ~1/8 century, and partially DR SMPs may be negative affected by this commit. Any thoughts on that? This routine is only invoked for SMInfo responding and sending TrapRepress. TrapRepress is only LID routed so Dr LIDs don't matter for this case. SMInfo responding can be either LID routed or DR. In the cases of SMInfo DR responding that I checked (2 instances in osm_req.c (osm_req_get and osm_prepare_req_set) where ib_smp_init_new is called and also using sminfo utility), the DrSLID = DrDLID (permissive LID) which is why the current code worked for more than the last 10-15 years. -- Hal > > Well, having raised the issue, then you have my: > > Reviewed-by: Håkon Bugge <haakon.bugge@oracle.com> > > > Thxs, Håkon > > > > > >
----- 原始邮件 -----
发件人: "Hal Rosenstock" <hal@dev.mellanox.co.il>
收件人: "Honggang LI" <honli@redhat.com>
抄送: linux-rdma@vger.kernel.org
发送时间: 星期一, 2018年 11 月 26日 下午 9:27:07
主题: [PATCH opensm] osm_resp.c: No need to swap DR [D/S]LIDs in resp_make_resp_smp
C14-10.1.1 states that the DrSLID and DrDLID in the directed route response
SMP shall be copied as is from the request SMP.
Pointed-out-by: Honggang Li <honli@redhat.com>
Signed-off-by: Hal Rosenstock <hal@mellanox.com>
---
opensm/osm_resp.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/opensm/osm_resp.c b/opensm/osm_resp.c
index 3f270e6..9a98df9 100644
--- a/opensm/osm_resp.c
+++ b/opensm/osm_resp.c
@@ -85,8 +85,6 @@ static void resp_make_resp_smp(IN osm_sm_t * sm, IN const ib_smp_t * p_src_smp,
if (p_src_smp->mgmt_class == IB_MCLASS_SUBN_DIR)
p_dest_smp->status |= IB_SMP_DIRECTION;
- p_dest_smp->dr_dlid = p_dest_smp->dr_slid;
- p_dest_smp->dr_slid = p_dest_smp->dr_dlid;
memcpy(&p_dest_smp->data, p_payload, IB_SMP_DATA_SIZE);
Exit:
diff --git a/opensm/osm_resp.c b/opensm/osm_resp.c index 3f270e6..9a98df9 100644 --- a/opensm/osm_resp.c +++ b/opensm/osm_resp.c @@ -85,8 +85,6 @@ static void resp_make_resp_smp(IN osm_sm_t * sm, IN const ib_smp_t * p_src_smp, if (p_src_smp->mgmt_class == IB_MCLASS_SUBN_DIR) p_dest_smp->status |= IB_SMP_DIRECTION; - p_dest_smp->dr_dlid = p_dest_smp->dr_slid; - p_dest_smp->dr_slid = p_dest_smp->dr_dlid; memcpy(&p_dest_smp->data, p_payload, IB_SMP_DATA_SIZE); Exit:
C14-10.1.1 states that the DrSLID and DrDLID in the directed route response SMP shall be copied as is from the request SMP. Pointed-out-by: Honggang Li <honli@redhat.com> Signed-off-by: Hal Rosenstock <hal@mellanox.com> --- opensm/osm_resp.c | 2 -- 1 file changed, 2 deletions(-)