Skip to content

Commit 899aba8

Browse files
Bob Pearsonjgunthorpe
authored andcommitted
RDMA/rxe: Fix FIXME in rxe_udp_encap_recv()
rxe_udp_encap_recv() drops the reference to rxe->ib_dev taken by rxe_get_dev_from_net() which should be held until each received skb is freed. This patch moves the calls to ib_device_put() to each place a received skb is freed. It also takes references to the ib_device for each cloned skb created to process received multicast packets. Fixes: 4c173f5 ("RDMA/rxe: Use ib_device_get_by_netdev() instead of open coding") Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Bob Pearson <[email protected]> Signed-off-by: Jason Gunthorpe <[email protected]>
1 parent 5120bf0 commit 899aba8

File tree

4 files changed

+35
-35
lines changed

4 files changed

+35
-35
lines changed

drivers/infiniband/sw/rxe/rxe_comp.c

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,7 @@ static void rxe_drain_resp_pkts(struct rxe_qp *qp, bool notify)
515515
while ((skb = skb_dequeue(&qp->resp_pkts))) {
516516
rxe_drop_ref(qp);
517517
kfree_skb(skb);
518+
ib_device_put(qp->ibqp.device);
518519
}
519520

520521
while ((wqe = queue_head(qp->sq.queue))) {
@@ -527,6 +528,17 @@ static void rxe_drain_resp_pkts(struct rxe_qp *qp, bool notify)
527528
}
528529
}
529530

531+
static void free_pkt(struct rxe_pkt_info *pkt)
532+
{
533+
struct sk_buff *skb = PKT_TO_SKB(pkt);
534+
struct rxe_qp *qp = pkt->qp;
535+
struct ib_device *dev = qp->ibqp.device;
536+
537+
kfree_skb(skb);
538+
rxe_drop_ref(qp);
539+
ib_device_put(dev);
540+
}
541+
530542
int rxe_completer(void *arg)
531543
{
532544
struct rxe_qp *qp = (struct rxe_qp *)arg;
@@ -624,11 +636,8 @@ int rxe_completer(void *arg)
624636
break;
625637

626638
case COMPST_DONE:
627-
if (pkt) {
628-
rxe_drop_ref(pkt->qp);
629-
kfree_skb(skb);
630-
skb = NULL;
631-
}
639+
if (pkt)
640+
free_pkt(pkt);
632641
goto done;
633642

634643
case COMPST_EXIT:
@@ -671,12 +680,8 @@ int rxe_completer(void *arg)
671680
*/
672681
if (qp->comp.started_retry &&
673682
!qp->comp.timeout_retry) {
674-
if (pkt) {
675-
rxe_drop_ref(pkt->qp);
676-
kfree_skb(skb);
677-
skb = NULL;
678-
}
679-
683+
if (pkt)
684+
free_pkt(pkt);
680685
goto done;
681686
}
682687

@@ -699,13 +704,8 @@ int rxe_completer(void *arg)
699704
qp->comp.started_retry = 1;
700705
rxe_run_task(&qp->req.task, 0);
701706
}
702-
703-
if (pkt) {
704-
rxe_drop_ref(pkt->qp);
705-
kfree_skb(skb);
706-
skb = NULL;
707-
}
708-
707+
if (pkt)
708+
free_pkt(pkt);
709709
goto done;
710710

711711
} else {
@@ -726,9 +726,7 @@ int rxe_completer(void *arg)
726726
mod_timer(&qp->rnr_nak_timer,
727727
jiffies + rnrnak_jiffies(aeth_syn(pkt)
728728
& ~AETH_TYPE_MASK));
729-
rxe_drop_ref(pkt->qp);
730-
kfree_skb(skb);
731-
skb = NULL;
729+
free_pkt(pkt);
732730
goto exit;
733731
} else {
734732
rxe_counter_inc(rxe,
@@ -742,13 +740,8 @@ int rxe_completer(void *arg)
742740
WARN_ON_ONCE(wqe->status == IB_WC_SUCCESS);
743741
do_complete(qp, wqe);
744742
rxe_qp_error(qp);
745-
746-
if (pkt) {
747-
rxe_drop_ref(pkt->qp);
748-
kfree_skb(skb);
749-
skb = NULL;
750-
}
751-
743+
if (pkt)
744+
free_pkt(pkt);
752745
goto exit;
753746
}
754747
}

drivers/infiniband/sw/rxe/rxe_net.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,14 @@ static struct dst_entry *rxe_find_route(struct net_device *ndev,
152152
static int rxe_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
153153
{
154154
struct udphdr *udph;
155+
struct rxe_dev *rxe;
155156
struct net_device *ndev = skb->dev;
156-
struct rxe_dev *rxe = rxe_get_dev_from_net(ndev);
157157
struct rxe_pkt_info *pkt = SKB_TO_PKT(skb);
158158

159+
/* takes a reference on rxe->ib_dev
160+
* drop when skb is freed
161+
*/
162+
rxe = rxe_get_dev_from_net(ndev);
159163
if (!rxe)
160164
goto drop;
161165

@@ -174,12 +178,6 @@ static int rxe_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
174178

175179
rxe_rcv(skb);
176180

177-
/*
178-
* FIXME: this is in the wrong place, it needs to be done when pkt is
179-
* destroyed
180-
*/
181-
ib_device_put(&rxe->ib_dev);
182-
183181
return 0;
184182
drop:
185183
kfree_skb(skb);

drivers/infiniband/sw/rxe/rxe_recv.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,10 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb)
274274
*/
275275
if (mce->qp_list.next != &mcg->qp_list) {
276276
per_qp_skb = skb_clone(skb, GFP_ATOMIC);
277+
if (WARN_ON(!ib_device_try_get(&rxe->ib_dev))) {
278+
kfree_skb(per_qp_skb);
279+
continue;
280+
}
277281
} else {
278282
per_qp_skb = skb;
279283
/* show we have consumed the skb */
@@ -296,6 +300,7 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb)
296300
err1:
297301
/* free skb if not consumed */
298302
kfree_skb(skb);
303+
ib_device_put(&rxe->ib_dev);
299304
}
300305

301306
/**
@@ -405,4 +410,5 @@ void rxe_rcv(struct sk_buff *skb)
405410
rxe_drop_ref(pkt->qp);
406411

407412
kfree_skb(skb);
413+
ib_device_put(&rxe->ib_dev);
408414
}

drivers/infiniband/sw/rxe/rxe_resp.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ static inline enum resp_states get_req(struct rxe_qp *qp,
9999
while ((skb = skb_dequeue(&qp->req_pkts))) {
100100
rxe_drop_ref(qp);
101101
kfree_skb(skb);
102+
ib_device_put(qp->ibqp.device);
102103
}
103104

104105
/* go drain recv wr queue */
@@ -1012,6 +1013,7 @@ static enum resp_states cleanup(struct rxe_qp *qp,
10121013
skb = skb_dequeue(&qp->req_pkts);
10131014
rxe_drop_ref(qp);
10141015
kfree_skb(skb);
1016+
ib_device_put(qp->ibqp.device);
10151017
}
10161018

10171019
if (qp->resp.mr) {
@@ -1176,6 +1178,7 @@ static void rxe_drain_req_pkts(struct rxe_qp *qp, bool notify)
11761178
while ((skb = skb_dequeue(&qp->req_pkts))) {
11771179
rxe_drop_ref(qp);
11781180
kfree_skb(skb);
1181+
ib_device_put(qp->ibqp.device);
11791182
}
11801183

11811184
if (notify)

0 commit comments

Comments
 (0)