On Mon, Feb 15, 2021 at 6:27 AM Bob Pearson <rpearsonhpe@xxxxxxxxx> wrote:
>
> Three errors occurred in the fix referenced below.
>
> 1) The on and off again 'if (skb)' got dropped but was really
> needed in rxe_rcv_mcast_pkt() to prevent calling ib_device_put()
> on the non-error path.
>
> 2) Extending the reference taken by rxe_get_dev_from_net() in
> rxe_udp_encap_recv() until each skb is freed was not matched by
> a reference in the loopback path resulting in underflows.
>
> 3) In rxe_comp.c the function free_pkt() did not clear skb which
> triggered a warning at done: and could possibly at exit: in
> rxe_completer(). The WARN_ONCE() calls are not required at done:
> and only in one place before going to exit.
>
> This patch fixes these errors.
>
> Fixes: 899aba891cab ("RDMA/rxe: Fix FIXME in rxe_udp_encap_recv()")
> Signed-off-by: Bob Pearson <rpearson@xxxxxxx>
> ---
> drivers/infiniband/sw/rxe/rxe_comp.c | 5 +++--
> drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
> drivers/infiniband/sw/rxe/rxe_recv.c | 6 ++++--
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
> index a8ac791a1bb9..13fc5a1cced1 100644
> --- a/drivers/infiniband/sw/rxe/rxe_comp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_comp.c
> @@ -671,6 +671,9 @@ int rxe_completer(void *arg)
> * it down the road or let it expire
> */
>
> + /* warn if we did receive a packet */
> + WARN_ON_ONCE(skb);
> +
> /* there is nothing to retry in this case */
> if (!wqe || (wqe->state == wqe_state_posted))
> goto exit;
> @@ -750,7 +753,6 @@ int rxe_completer(void *arg)
> /* we come here if we are done with processing and want the task to
> * exit from the loop calling us
> */
> - WARN_ON_ONCE(skb);
> rxe_drop_ref(qp);
> return -EAGAIN;
>
> @@ -758,7 +760,6 @@ int rxe_completer(void *arg)
> /* we come here if we have processed a packet we want the task to call
> * us again to see if there is anything else to do
> */
> - WARN_ON_ONCE(skb);
> rxe_drop_ref(qp);
> return 0;
> }
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 36d56163afac..8e81df578552 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -406,12 +406,17 @@ int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb)
>
> void rxe_loopback(struct sk_buff *skb)
> {
> + struct rxe_pkt_info *pkt = SKB_TO_PKT(skb);
> +
> if (skb->protocol == htons(ETH_P_IP))
> skb_pull(skb, sizeof(struct iphdr));
> else
> skb_pull(skb, sizeof(struct ipv6hdr));
>
> - rxe_rcv(skb);
> + if (WARN_ON(!ib_device_try_get(&pkt->rxe->ib_dev)))
> + kfree_skb(skb);
> + else
> + rxe_rcv(skb);
> }
>
> struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
> index 8a48a33d587b..a5e330e3bbce 100644
> --- a/drivers/infiniband/sw/rxe/rxe_recv.c
> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> @@ -299,8 +299,10 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb)
>
> err1:
> /* free skb if not consumed */
> - kfree_skb(skb);
> - ib_device_put(&rxe->ib_dev);
> + if (unlikely(skb)) {
>From Leon Romanovsky
"
Please don't put "if (a) kfree(a);" constructions unless you want to
deal with daily flux of patches with attempt to remove "if".
"
Zhu Yanjun
> + kfree_skb(skb);
> + ib_device_put(&rxe->ib_dev);
> + }
> }
>
> /**
> --
> 2.27.0
>
