| Message ID | 20180819070401.19678-1-yanjun.zhu@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] IB/rxe: remove redudant qpn check | expand |
On Sun, Aug 19, 2018 at 03:04:01PM +0800, Yanjun Zhu wrote: > In the commit 536ca245c512 ("IB/rxe: Drop QP0 silently"), if qpn is > zero, the function directly returns. So in the following function, > it is not necessary to check qpn. The qpn check in the function > check_keys is removed. But check_keys can be called from other places besides hdr_check, e.x rxe_rcv_mcast_pkt. > > Fixes: 536ca245c512 ("IB/rxe: Drop QP0 silently") > CC: Srinivas Eeda <srinivas.eeda@oracle.com> > CC: Junxiao Bi <junxiao.bi@oracle.com> > Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> > --- > drivers/infiniband/sw/rxe/rxe_recv.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c > index d30dbac24583..8dfbe2c2ca74 100644 > --- a/drivers/infiniband/sw/rxe/rxe_recv.c > +++ b/drivers/infiniband/sw/rxe/rxe_recv.c > @@ -122,7 +122,7 @@ static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > set_bad_pkey_cntr(port); > goto err1; > } > - } else if (qpn != 0) { > + } else { > if (unlikely(!pkey_match(pkey, > port->pkey_tbl[qp->attr.pkey_index] > ))) { > @@ -134,7 +134,7 @@ static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > } > > if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && > - qpn != 0 && pkt->mask) { > + pkt->mask) { > u32 qkey = (qpn == 1) ? GSI_QKEY : qp->attr.qkey; > > if (unlikely(deth_qkey(pkt) != qkey)) { > -- > 2.17.1 >
On 2018/8/20 15:09, Yuval Shaia wrote: > On Sun, Aug 19, 2018 at 03:04:01PM +0800, Yanjun Zhu wrote: >> In the commit 536ca245c512 ("IB/rxe: Drop QP0 silently"), if qpn is >> zero, the function directly returns. So in the following function, >> it is not necessary to check qpn. The qpn check in the function >> check_keys is removed. > But check_keys can be called from other places besides hdr_check, e.x > rxe_rcv_mcast_pkt. check_keys is called by rxe_rcv_mcast_pkt and hdr_check now. rxe_rcv_mcast_pkt is a static function in rxe_recv.c. Its call trace is as below. void rxe_rcv(struct sk_buff *skb) { ... err = hdr_check(pkt); if (unlikely(err)) goto drop; ... if (unlikely(bth_qpn(pkt) == IB_MULTICAST_QPN)) rxe_rcv_mcast_pkt(rxe, skb); else rxe_rcv_pkt(rxe, pkt, skb); ... drop: if (pkt->qp) rxe_drop_ref(pkt->qp); If qpn is zero, hdr_check will return error. Then the function rxe_rcv will goto label drop. The function rxe_rcv_mcast_pkt will not be called. So even though check_keys is called by rxe_rcv_mcast_pkt and hdr_check, when qpn is zero, hdr_check will not call check_keys and rxe_rcv_mcast_pkt will not be called. So this patch is safe. Zhu Yanjun > >> Fixes: 536ca245c512 ("IB/rxe: Drop QP0 silently") >> CC: Srinivas Eeda <srinivas.eeda@oracle.com> >> CC: Junxiao Bi <junxiao.bi@oracle.com> >> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> >> --- >> drivers/infiniband/sw/rxe/rxe_recv.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c >> index d30dbac24583..8dfbe2c2ca74 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_recv.c >> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c >> @@ -122,7 +122,7 @@ static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, >> set_bad_pkey_cntr(port); >> goto err1; >> } >> - } else if (qpn != 0) { >> + } else { >> if (unlikely(!pkey_match(pkey, >> port->pkey_tbl[qp->attr.pkey_index] >> ))) { >> @@ -134,7 +134,7 @@ static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, >> } >> >> if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && >> - qpn != 0 && pkt->mask) { >> + pkt->mask) { >> u32 qkey = (qpn == 1) ? GSI_QKEY : qp->attr.qkey; >> >> if (unlikely(deth_qkey(pkt) != qkey)) { >> -- >> 2.17.1 >>
On Mon, Aug 20, 2018 at 03:44:31PM +0800, Yanjun Zhu wrote: > > > On 2018/8/20 15:09, Yuval Shaia wrote: > > On Sun, Aug 19, 2018 at 03:04:01PM +0800, Yanjun Zhu wrote: > > > In the commit 536ca245c512 ("IB/rxe: Drop QP0 silently"), if qpn is > > > zero, the function directly returns. So in the following function, > > > it is not necessary to check qpn. The qpn check in the function > > > check_keys is removed. > > But check_keys can be called from other places besides hdr_check, e.x > > rxe_rcv_mcast_pkt. > check_keys is called by rxe_rcv_mcast_pkt and hdr_check now. > > rxe_rcv_mcast_pkt is a static function in rxe_recv.c. > Its call trace is as below. > > void rxe_rcv(struct sk_buff *skb) > { > ... > err = hdr_check(pkt); > if (unlikely(err)) > goto drop; > ... > if (unlikely(bth_qpn(pkt) == IB_MULTICAST_QPN)) > rxe_rcv_mcast_pkt(rxe, skb); > else > rxe_rcv_pkt(rxe, pkt, skb); > ... > drop: > if (pkt->qp) > rxe_drop_ref(pkt->qp); > > If qpn is zero, hdr_check will return error. Then the function rxe_rcv will > goto label drop. > The function rxe_rcv_mcast_pkt will not be called. > > So even though check_keys is called by rxe_rcv_mcast_pkt and hdr_check, when > qpn is zero, > hdr_check will not call check_keys and rxe_rcv_mcast_pkt will not be > called. > > So this patch is safe. Agree. Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> > > Zhu Yanjun > > > > > Fixes: 536ca245c512 ("IB/rxe: Drop QP0 silently") > > > CC: Srinivas Eeda <srinivas.eeda@oracle.com> > > > CC: Junxiao Bi <junxiao.bi@oracle.com> > > > Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> > > > --- > > > drivers/infiniband/sw/rxe/rxe_recv.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c > > > index d30dbac24583..8dfbe2c2ca74 100644 > > > --- a/drivers/infiniband/sw/rxe/rxe_recv.c > > > +++ b/drivers/infiniband/sw/rxe/rxe_recv.c > > > @@ -122,7 +122,7 @@ static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > > > set_bad_pkey_cntr(port); > > > goto err1; > > > } > > > - } else if (qpn != 0) { > > > + } else { > > > if (unlikely(!pkey_match(pkey, > > > port->pkey_tbl[qp->attr.pkey_index] > > > ))) { > > > @@ -134,7 +134,7 @@ static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > > > } > > > if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && > > > - qpn != 0 && pkt->mask) { > > > + pkt->mask) { > > > u32 qkey = (qpn == 1) ? GSI_QKEY : qp->attr.qkey; > > > if (unlikely(deth_qkey(pkt) != qkey)) { > > > -- > > > 2.17.1 > > > >
On Sun, 2018-08-19 at 15:04 +0800, Zhu Yanjun wrote: > In the commit 536ca245c512 ("IB/rxe: Drop QP0 silently"), if qpn is > zero, the function directly returns. So in the following function, > it is not necessary to check qpn. The qpn check in the function > check_keys is removed. > > Fixes: 536ca245c512 ("IB/rxe: Drop QP0 silently") > CC: Srinivas Eeda <srinivas.eeda@oracle.com> > CC: Junxiao Bi <junxiao.bi@oracle.com> > Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> Thanks, applied to for-next.
diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c index d30dbac24583..8dfbe2c2ca74 100644 --- a/drivers/infiniband/sw/rxe/rxe_recv.c +++ b/drivers/infiniband/sw/rxe/rxe_recv.c @@ -122,7 +122,7 @@ static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, set_bad_pkey_cntr(port); goto err1; } - } else if (qpn != 0) { + } else { if (unlikely(!pkey_match(pkey, port->pkey_tbl[qp->attr.pkey_index] ))) { @@ -134,7 +134,7 @@ static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, } if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && - qpn != 0 && pkt->mask) { + pkt->mask) { u32 qkey = (qpn == 1) ? GSI_QKEY : qp->attr.qkey; if (unlikely(deth_qkey(pkt) != qkey)) {
In the commit 536ca245c512 ("IB/rxe: Drop QP0 silently"), if qpn is zero, the function directly returns. So in the following function, it is not necessary to check qpn. The qpn check in the function check_keys is removed. Fixes: 536ca245c512 ("IB/rxe: Drop QP0 silently") CC: Srinivas Eeda <srinivas.eeda@oracle.com> CC: Junxiao Bi <junxiao.bi@oracle.com> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> --- drivers/infiniband/sw/rxe/rxe_recv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)