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@xxxxxxxxxx>
>
> Zhu Yanjun
> >
> > > Fixes: 536ca245c512 ("IB/rxe: Drop QP0 silently")
> > > CC: Srinivas Eeda <srinivas.eeda@xxxxxxxxxx>
> > > CC: Junxiao Bi <junxiao.bi@xxxxxxxxxx>
> > > Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxxx>
> > > ---
> > > 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
> > >
>
