[PATCH 0/7] IB/rxe: Bug fixes

[PATCH 0/7] IB/rxe: Bug fixes

[Andrew Boyer]

This patch set addresses a few issues which we have found in RXE.
The first patch fixes a serious issue that causes kernel panics during
network congestion.

Andrew Boyer (7):
  IB/rxe: Move refcounting earlier in rxe_send()
  IB/rxe: Disable completion upcalls when a CQ is destroyed
  IB/rxe: Remove dangling prototype
  IB/rxe: Fix up the responder's find_resources() function
  IB/rxe: Fix destination cache for IPv6
  IB/rxe: Fix up one more receive queue drain path that might prevent
    cleanup
  IB/rxe: Avoid ICRC errors by copying into the skb first

 drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h   |  4 ++--
 drivers/infiniband/sw/rxe/rxe_mr.c    | 12 ++++++------
 drivers/infiniband/sw/rxe/rxe_net.c   | 17 +++++++++++------
 drivers/infiniband/sw/rxe/rxe_qp.c    |  9 ++-------
 drivers/infiniband/sw/rxe/rxe_req.c   |  4 +++-
 drivers/infiniband/sw/rxe/rxe_resp.c  |  2 +-
 drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.h |  2 ++
 9 files changed, 48 insertions(+), 23 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] IB/rxe: Move refcounting earlier in rxe_send()

[Andrew Boyer]

The network stack will call nskb's destructor, rxe_skb_tx_dtor(), if the
packet gets dropped by ip_local_out()/ip6_local_out(). Thus we need to add
the QP ref before output to avoid extra dereferences during network
congestion. This could lead to unwanted destruction of the QP.

Fix up the skb_out accounting, too.

Fixes: fda85ce91240 ("IB/rxe: Fix kernel panic from skb destructor")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 08f3f90..0810f38 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -460,12 +460,17 @@ int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
 	nskb->destructor = rxe_skb_tx_dtor;
 	nskb->sk = pkt->qp->sk->sk;
 
+	rxe_add_ref(pkt->qp);
+	atomic_inc(&pkt->qp->skb_out);
+
 	if (av->network_type == RDMA_NETWORK_IPV4) {
 		err = ip_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
 	} else if (av->network_type == RDMA_NETWORK_IPV6) {
 		err = ip6_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
 	} else {
 		pr_err("Unknown layer 3 protocol: %d\n", av->network_type);
+		atomic_dec(&pkt->qp->skb_out);
+		rxe_drop_ref(pkt->qp);
 		kfree_skb(nskb);
 		return -EINVAL;
 	}
@@ -475,10 +480,7 @@ int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
 		return -EAGAIN;
 	}
 
-	rxe_add_ref(pkt->qp);
-	atomic_inc(&pkt->qp->skb_out);
 	kfree_skb(skb);
-
 	return 0;
 }
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] IB/rxe: Move refcounting earlier in rxe_send()

[Moni Shoua]

On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org> wrote:
> The network stack will call nskb's destructor, rxe_skb_tx_dtor(), if the
> packet gets dropped by ip_local_out()/ip6_local_out(). Thus we need to add
> the QP ref before output to avoid extra dereferences during network
> congestion. This could lead to unwanted destruction of the QP.
>
> Fix up the skb_out accounting, too.
>
> Fixes: fda85ce91240 ("IB/rxe: Fix kernel panic from skb destructor")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] IB/rxe: Disable completion upcalls when a CQ is destroyed

[Andrew Boyer]

This prevents the stack from accessing userspace objects while they
are being torn down.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h   |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.h |  1 +
 4 files changed, 24 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c
index 49fe42c..c4aabf7 100644
--- a/drivers/infiniband/sw/rxe/rxe_cq.c
+++ b/drivers/infiniband/sw/rxe/rxe_cq.c
@@ -69,6 +69,14 @@ int rxe_cq_chk_attr(struct rxe_dev *rxe, struct rxe_cq *cq,
 static void rxe_send_complete(unsigned long data)
 {
 	struct rxe_cq *cq = (struct rxe_cq *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cq->cq_lock, flags);
+	if (cq->is_dying) {
+		spin_unlock_irqrestore(&cq->cq_lock, flags);
+		return;
+	}
+	spin_unlock_irqrestore(&cq->cq_lock, flags);
 
 	cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
 }
@@ -97,6 +105,8 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe,
 	if (udata)
 		cq->is_user = 1;
 
+	cq->is_dying = false;
+
 	tasklet_init(&cq->comp_task, rxe_send_complete, (unsigned long)cq);
 
 	spin_lock_init(&cq->cq_lock);
@@ -156,6 +166,15 @@ int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited)
 	return 0;
 }
 
+void rxe_cq_disable(struct rxe_cq *cq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&cq->cq_lock, flags);
+	cq->is_dying = true;
+	spin_unlock_irqrestore(&cq->cq_lock, flags);
+}
+
 void rxe_cq_cleanup(struct rxe_pool_entry *arg)
 {
 	struct rxe_cq *cq = container_of(arg, typeof(*cq), pelem);
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index d6299ed..64f8fa1 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -64,6 +64,8 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe,
 
 int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited);
 
+void rxe_cq_disable(struct rxe_cq *cq);
+
 void rxe_cq_cleanup(struct rxe_pool_entry *arg);
 
 /* rxe_mcast.c */
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index af90a7d..02a39e8 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -960,6 +960,8 @@ static int rxe_destroy_cq(struct ib_cq *ibcq)
 {
 	struct rxe_cq *cq = to_rcq(ibcq);
 
+	rxe_cq_disable(cq);
+
 	rxe_drop_ref(cq);
 	return 0;
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 5a180fb..b09a9e2 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -89,6 +89,7 @@ struct rxe_cq {
 	struct rxe_queue	*queue;
 	spinlock_t		cq_lock;
 	u8			notify;
+	bool			is_dying;
 	int			is_user;
 	struct tasklet_struct	comp_task;
 };
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] IB/rxe: Disable completion upcalls when a CQ is destroyed

[Moni Shoua]

On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org> wrote:
> This prevents the stack from accessing userspace objects while they
> are being torn down.
>
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
>  drivers/infiniband/sw/rxe/rxe_loc.h   |  2 ++
>  drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
>  drivers/infiniband/sw/rxe/rxe_verbs.h |  1 +
>  4 files changed, 24 insertions(+)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c
> index 49fe42c..c4aabf7 100644
> --- a/drivers/infiniband/sw/rxe/rxe_cq.c
> +++ b/drivers/infiniband/sw/rxe/rxe_cq.c
> @@ -69,6 +69,14 @@ int rxe_cq_chk_attr(struct rxe_dev *rxe, struct rxe_cq *cq,
>  static void rxe_send_complete(unsigned long data)
>  {
>         struct rxe_cq *cq = (struct rxe_cq *)data;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&cq->cq_lock, flags);
> +       if (cq->is_dying) {
> +               spin_unlock_irqrestore(&cq->cq_lock, flags);
> +               return;
> +       }
> +       spin_unlock_irqrestore(&cq->cq_lock, flags);
What if CQ is destroyed here after you pass the is_dying test?
Maybe you should think of a solution based on ref counting.
>         cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] IB/rxe: Disable completion upcalls when a CQ is destroyed

[Boyer, Andrew]

On 7/27/17, 5:35 AM, "monisonlists-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org on behalf of Moni Shoua"
<monisonlists-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org on behalf of monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:

>On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>wrote:
>> This prevents the stack from accessing userspace objects while they
>> are being torn down.
>>
>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>> ---
>>  drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
>>  drivers/infiniband/sw/rxe/rxe_loc.h   |  2 ++
>>  drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
>>  drivers/infiniband/sw/rxe/rxe_verbs.h |  1 +
>>  4 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c
>>b/drivers/infiniband/sw/rxe/rxe_cq.c
>> index 49fe42c..c4aabf7 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_cq.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_cq.c
>> @@ -69,6 +69,14 @@ int rxe_cq_chk_attr(struct rxe_dev *rxe, struct
>>rxe_cq *cq,
>>  static void rxe_send_complete(unsigned long data)
>>  {
>>         struct rxe_cq *cq = (struct rxe_cq *)data;
>> +       unsigned long flags;
>> +
>> +       spin_lock_irqsave(&cq->cq_lock, flags);
>> +       if (cq->is_dying) {
>> +               spin_unlock_irqrestore(&cq->cq_lock, flags);
>> +               return;
>> +       }
>> +       spin_unlock_irqrestore(&cq->cq_lock, flags);
>What if CQ is destroyed here after you pass the is_dying test?
>Maybe you should think of a solution based on ref counting.
>>         cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
>>  }

Hello Moni,
Thank you for all of the reviews. I¹ll address commit messages etc. in a
revised series.

This is the situation that causes a crash here:
 - Userspace programs exits
 - ib_uverbs_cleanup_ucontext() runs, calling ib_destroy_qp(),
ib_destroy_cq(), etc. and releasing/freeing the UCQ
   - The QP still has tasklets running, so it isn¹t destroyed yet
   - The CQ is referenced (twice) by the QP, so the CQ isn¹t destroyed yet
   - The UCQ is kfree()'d!
 - A send work request completes
 - rxe_send_complete() calls cq->ibcq.comp_handler()
 - ib_uverbs_comp_handler() runs and crashes; the event queue is checked
for is_closed, but it has no way to check the ib_ucq_object

As you can see, the reference counting on the CQ doesn¹t protect us.
There¹s no interface I could find that would deregister the UCQ from the
CQ. I didn¹t think attempting to add reference counting to the UCQ was
going to be a good way to go since the solution I posted above is so much
simpler (if hacky).

It looks like ib_uverbs_cleanup_context() is gone in 4.12. I don¹t know if
whatever replaced it addresses this issue already, by accident or by
design.

Does this make sense? Do you have a better idea for a fix?

Thank you,
Andrew

P.S. Sorry for the Outlook garbage formatting.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/7] IB/rxe: Remove dangling prototype

[Andrew Boyer]

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_loc.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 64f8fa1..8ef2d3f 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -221,8 +221,6 @@ static inline void rxe_advance_resp_resource(struct rxe_qp *qp)
 void retransmit_timer(unsigned long data);
 void rnr_nak_timer(unsigned long data);
 
-void dump_qp(struct rxe_qp *qp);
-
 /* rxe_srq.c */
 #define IB_SRQ_INIT_MASK (~IB_SRQ_LIMIT)
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] IB/rxe: Remove dangling prototype

[Moni Shoua]

On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org> wrote:
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_loc.h | 2 --
>  1 file changed, 2 deletions(-)
Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] IB/rxe: Fix up the responder's find_resources() function

[Andrew Boyer]

The resource array is sized by max_dest_rd_atomic, not max_rd_atomic.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index a958ee9..4240866 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -1055,7 +1055,7 @@ static struct resp_res *find_resource(struct rxe_qp *qp, u32 psn)
 {
 	int i;
 
-	for (i = 0; i < qp->attr.max_rd_atomic; i++) {
+	for (i = 0; i < qp->attr.max_dest_rd_atomic; i++) {
 		struct resp_res *res = &qp->resp.resources[i];
 
 		if (res->type == 0)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] IB/rxe: Fix up the responder's find_resources() function

[Moni Shoua]

On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org> wrote:
> The resource array is sized by max_dest_rd_atomic, not max_rd_atomic.
>
The patch seems correct. However the commit message lacks an explanation why.
Please improve

> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index a958ee9..4240866 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -1055,7 +1055,7 @@ static struct resp_res *find_resource(struct rxe_qp *qp, u32 psn)
>  {
>         int i;
>
> -       for (i = 0; i < qp->attr.max_rd_atomic; i++) {
> +       for (i = 0; i < qp->attr.max_dest_rd_atomic; i++) {
>                 struct resp_res *res = &qp->resp.resources[i];
>
>                 if (res->type == 0)
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] IB/rxe: Fix destination cache for IPv6

[Andrew Boyer]

To successfully match an IPv6 path, the path cookie must match. Store it
in the QP so that the IPv6 path can be reused.

Replace open-coded version of dst_check() with the actual call, fixing the
logic. The open-coded version skips the check call if dst->obsolete is 0
(DST_OBSOLETE_NONE), proceeding to replace the route. DST_OBSOLETE_NONE
means that the route may continue to be used, though.

Add dst_clone() in prepare_ipv6_hdr(). Otherwise the reference count goes
negative as packets complete.

Replace sk_dst_get()/dst_release() in rxe_qp_cleanup() with sk_dst_reset().
sk_dst_get() takes a new reference on dst, so the dst_release() doesn't
actually release the original reference, which was the design intent.

Remove unneeded initialization in prepare6().

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c   | 9 ++++++---
 drivers/infiniband/sw/rxe/rxe_qp.c    | 9 ++-------
 drivers/infiniband/sw/rxe/rxe_verbs.h | 1 +
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 0810f38..6aeda61 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -191,7 +191,7 @@ static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
 	if (qp_type(qp) == IB_QPT_RC)
 		dst = sk_dst_get(qp->sk->sk);
 
-	if (!dst || !(dst->obsolete && dst->ops->check(dst, 0))) {
+	if (!dst || !dst_check(dst, qp->dst_cookie)) {
 		if (dst)
 			dst_release(dst);
 
@@ -209,6 +209,9 @@ static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
 			saddr6 = &av->sgid_addr._sockaddr_in6.sin6_addr;
 			daddr6 = &av->dgid_addr._sockaddr_in6.sin6_addr;
 			dst = rxe_find_route6(rxe->ndev, saddr6, daddr6);
+			if (dst)
+				qp->dst_cookie =
+					rt6_get_cookie((struct rt6_info *)dst);
 		}
 	}
 
@@ -337,7 +340,7 @@ static void prepare_ipv6_hdr(struct dst_entry *dst, struct sk_buff *skb,
 	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
 	IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED
 			    | IPSKB_REROUTED);
-	skb_dst_set(skb, dst);
+	skb_dst_set(skb, dst_clone(dst));
 
 	__skb_push(skb, sizeof(*ip6h));
 	skb_reset_network_header(skb);
@@ -388,7 +391,7 @@ static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
 		    struct sk_buff *skb, struct rxe_av *av)
 {
 	struct rxe_qp *qp = pkt->qp;
-	struct dst_entry *dst = NULL;
+	struct dst_entry *dst;
 	struct in6_addr *saddr = &av->sgid_addr._sockaddr_in6.sin6_addr;
 	struct in6_addr *daddr = &av->dgid_addr._sockaddr_in6.sin6_addr;
 
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 80ccc7c..00bda93 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -851,13 +851,8 @@ void rxe_qp_cleanup(struct rxe_pool_entry *arg)
 		qp->resp.mr = NULL;
 	}
 
-	if (qp_type(qp) == IB_QPT_RC) {
-		struct dst_entry *dst = NULL;
-
-		dst = sk_dst_get(qp->sk->sk);
-		if (dst)
-			dst_release(dst);
-	}
+	if (qp_type(qp) == IB_QPT_RC)
+		sk_dst_reset(qp->sk->sk);
 
 	free_rd_atomic_resources(qp);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index b09a9e2..0c2dbe4 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -248,6 +248,7 @@ struct rxe_qp {
 	struct rxe_rq		rq;
 
 	struct socket		*sk;
+	u32			dst_cookie;
 
 	struct rxe_av		pri_av;
 	struct rxe_av		alt_av;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] IB/rxe: Fix destination cache for IPv6

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2349 bytes --]

Hi Andrew,

[auto build test ERROR on rdma/master]
[also build test ERROR on v4.13-rc2 next-20170726]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrew-Boyer/IB-rxe-Move-refcounting-earlier-in-rxe_send/20170726-141937
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git master
config: i386-randconfig-i1-07240017 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/infiniband//sw/rxe/rxe_net.c: In function 'rxe_find_route':
>> drivers/infiniband//sw/rxe/rxe_net.c:214:6: error: implicit declaration of function 'rt6_get_cookie' [-Werror=implicit-function-declaration]
         rt6_get_cookie((struct rt6_info *)dst);
         ^
   cc1: some warnings being treated as errors

vim +/rt6_get_cookie +214 drivers/infiniband//sw/rxe/rxe_net.c

   184	
   185	static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
   186						struct rxe_qp *qp,
   187						struct rxe_av *av)
   188	{
   189		struct dst_entry *dst = NULL;
   190	
   191		if (qp_type(qp) == IB_QPT_RC)
   192			dst = sk_dst_get(qp->sk->sk);
   193	
   194		if (!dst || !dst_check(dst, qp->dst_cookie)) {
   195			if (dst)
   196				dst_release(dst);
   197	
   198			if (av->network_type == RDMA_NETWORK_IPV4) {
   199				struct in_addr *saddr;
   200				struct in_addr *daddr;
   201	
   202				saddr = &av->sgid_addr._sockaddr_in.sin_addr;
   203				daddr = &av->dgid_addr._sockaddr_in.sin_addr;
   204				dst = rxe_find_route4(rxe->ndev, saddr, daddr);
   205			} else if (av->network_type == RDMA_NETWORK_IPV6) {
   206				struct in6_addr *saddr6;
   207				struct in6_addr *daddr6;
   208	
   209				saddr6 = &av->sgid_addr._sockaddr_in6.sin6_addr;
   210				daddr6 = &av->dgid_addr._sockaddr_in6.sin6_addr;
   211				dst = rxe_find_route6(rxe->ndev, saddr6, daddr6);
   212				if (dst)
   213					qp->dst_cookie =
 > 214						rt6_get_cookie((struct rt6_info *)dst);
   215			}
   216		}
   217	
   218		return dst;
   219	}
   220	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34162 bytes --]

Re: [PATCH 5/7] IB/rxe: Fix destination cache for IPv6

[Boyer, Andrew]

Looks like I forgot to check for CONFIG_IPV6.

On 7/27/17, 2:41 AM, "kbuild test robot" <lkp-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

>Hi Andrew,
>
>[auto build test ERROR on rdma/master]
>[also build test ERROR on v4.13-rc2 next-20170726]
>[if your patch is applied to the wrong git tree, please drop us a note to
>help improve the system]
>
>url:    
>https://github.com/0day-ci/linux/commits/Andrew-Boyer/IB-rxe-Move-refcount
>ing-earlier-in-rxe_send/20170726-141937
>base:   https://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git
>master
>config: i386-randconfig-i1-07240017 (attached as .config)
>compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
>reproduce:
>        # save the attached .config to linux build tree
>        make ARCH=i386
>
>All errors (new ones prefixed by >>):
>
>   drivers/infiniband//sw/rxe/rxe_net.c: In function 'rxe_find_route':
>>> drivers/infiniband//sw/rxe/rxe_net.c:214:6: error: implicit
>>>declaration of function 'rt6_get_cookie'
>>>[-Werror=implicit-function-declaration]
>         rt6_get_cookie((struct rt6_info *)dst);
>         ^
>   cc1: some warnings being treated as errors
>
>vim +/rt6_get_cookie +214 drivers/infiniband//sw/rxe/rxe_net.c
>
>   184	
>   185	static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
>   186						struct rxe_qp *qp,
>   187						struct rxe_av *av)
>   188	{
>   189		struct dst_entry *dst = NULL;
>   190	
>   191		if (qp_type(qp) == IB_QPT_RC)
>   192			dst = sk_dst_get(qp->sk->sk);
>   193	
>   194		if (!dst || !dst_check(dst, qp->dst_cookie)) {
>   195			if (dst)
>   196				dst_release(dst);
>   197	
>   198			if (av->network_type == RDMA_NETWORK_IPV4) {
>   199				struct in_addr *saddr;
>   200				struct in_addr *daddr;
>   201	
>   202				saddr = &av->sgid_addr._sockaddr_in.sin_addr;
>   203				daddr = &av->dgid_addr._sockaddr_in.sin_addr;
>   204				dst = rxe_find_route4(rxe->ndev, saddr, daddr);
>   205			} else if (av->network_type == RDMA_NETWORK_IPV6) {
>   206				struct in6_addr *saddr6;
>   207				struct in6_addr *daddr6;
>   208	
>   209				saddr6 = &av->sgid_addr._sockaddr_in6.sin6_addr;
>   210				daddr6 = &av->dgid_addr._sockaddr_in6.sin6_addr;
>   211				dst = rxe_find_route6(rxe->ndev, saddr6, daddr6);
>   212				if (dst)
>   213					qp->dst_cookie =
> > 214						rt6_get_cookie((struct rt6_info *)dst);
>   215			}
>   216		}
>   217	
>   218		return dst;
>   219	}
>   220	
>
>---
>0-DAY kernel test infrastructure                Open Source Technology
>Center
>https://lists.01.org/pipermail/kbuild-all                   Intel
>Corporation

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] IB/rxe: Fix destination cache for IPv6

[Moni Shoua]

On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org> wrote:
> To successfully match an IPv6 path, the path cookie must match. Store it
> in the QP so that the IPv6 path can be reused.
>
> Replace open-coded version of dst_check() with the actual call, fixing the
> logic. The open-coded version skips the check call if dst->obsolete is 0
> (DST_OBSOLETE_NONE), proceeding to replace the route. DST_OBSOLETE_NONE
> means that the route may continue to be used, though.
>
> Add dst_clone() in prepare_ipv6_hdr(). Otherwise the reference count goes
> negative as packets complete.
>
> Replace sk_dst_get()/dst_release() in rxe_qp_cleanup() with sk_dst_reset().
> sk_dst_get() takes a new reference on dst, so the dst_release() doesn't
> actually release the original reference, which was the design intent.
>
> Remove unneeded initialization in prepare6().
>
> Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_net.c   | 9 ++++++---
>  drivers/infiniband/sw/rxe/rxe_qp.c    | 9 ++-------
>  drivers/infiniband/sw/rxe/rxe_verbs.h | 1 +
>  3 files changed, 9 insertions(+), 10 deletions(-)
>
Fixes seem to be correct but I identify here 3 unrelated patches.
Please split
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/7] IB/rxe: Fix up one more receive queue drain path that might prevent cleanup

[Andrew Boyer]

Fixes: 1217197142d1 ("rxe: fix broken receive queue draining")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 7ee465d..24fc0b8 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -594,8 +594,10 @@ int rxe_requester(void *arg)
 	rxe_add_ref(qp);
 
 next_wqe:
-	if (unlikely(!qp->valid))
+	if (unlikely(!qp->valid)) {
+		rxe_drain_req_pkts(qp, true);
 		goto exit;
+	}
 
 	if (unlikely(qp->req.state == QP_STATE_ERROR)) {
 		rxe_drain_req_pkts(qp, true);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] IB/rxe: Fix up one more receive queue drain path that might prevent cleanup

[Moni Shoua]

On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org> wrote:
> Fixes: 1217197142d1 ("rxe: fix broken receive queue draining")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_req.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
please improve documentation for this patch
Make sure that the commit message has a body, not just subject (which
you can make shorter and more general)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/7] IB/rxe: Avoid ICRC errors by copying into the skb first

[Andrew Boyer]

If the buffer contents are changing, the current code can generate a packet
with a bogus CRC. This can be seen with qperf's ud_bi_bw test.

No matter what behavior the client is expecting in this case, it shouldn't
result in a malformed packet.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_mr.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index e37cc89..5c2684b 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -367,11 +367,11 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		dest = (dir == to_mem_obj) ?
 			((void *)(uintptr_t)iova) : addr;
 
+		memcpy(dest, src, length);
+
 		if (crcp)
 			*crcp = rxe_crc32(to_rdev(mem->pd->ibpd.device),
-					*crcp, src, length);
-
-		memcpy(dest, src, length);
+					*crcp, dest, length);
 
 		return 0;
 	}
@@ -401,11 +401,11 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		if (bytes > length)
 			bytes = length;
 
+		memcpy(dest, src, bytes);
+
 		if (crcp)
 			crc = rxe_crc32(to_rdev(mem->pd->ibpd.device),
-					crc, src, bytes);
-
-		memcpy(dest, src, bytes);
+					crc, dest, bytes);
 
 		length	-= bytes;
 		addr	+= bytes;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] IB/rxe: Avoid ICRC errors by copying into the skb first

[Or Gerlitz]

On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org> wrote:
> If the buffer contents are changing,

can you please elaborate on that a little further? what are unchanged
contents and what are changing contents?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] IB/rxe: Avoid ICRC errors by copying into the skb first

[Boyer, Andrew]

On 7/25/17, 1:34 PM, "Or Gerlitz" <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

>On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>wrote:
>> If the buffer contents are changing,
>
>can you please elaborate on that a little further? what are unchanged
>contents and what are changing contents?

In qperf, the ud_bi_bw test allocates a single buffer on each side. Then
it posts that one buffer 1024 times for receive and 1024 times for send.
While RXE is building up the send packet, the buffer is being overwritten
by incoming receives.


The current process of first calculating the CRC and then copying the
buffer into the packet leaves a window in which the contents and the CRC
can get out of sync. By copying the buffer into the packet and then
calculating the CRC based on the packet contents we eliminate the window.

This seems like very strange/reckless client behavior, but whether the
client has mangled the buffer or not RXE should be able to transfer it
reliably.

Thanks,
Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] IB/rxe: Avoid ICRC errors by copying into the skb first

[Moni Shoua]

> This seems like very strange/reckless client behavior, but whether the
> client has mangled the buffer or not RXE should be able to transfer it
> reliably.
>
And what if was overwritten during the memcpy? Then we end up with
wrong ICRC again, right?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] IB/rxe: Bug fixes

[Moni Shoua]

On Tue, Jul 25, 2017 at 4:39 PM, Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org> wrote:
> This patch set addresses a few issues which we have found in RXE.
> The first patch fixes a serious issue that causes kernel panics during
> network congestion.
>
> Andrew Boyer (7):
>   IB/rxe: Move refcounting earlier in rxe_send()
>   IB/rxe: Disable completion upcalls when a CQ is destroyed
>   IB/rxe: Remove dangling prototype
>   IB/rxe: Fix up the responder's find_resources() function
>   IB/rxe: Fix destination cache for IPv6
>   IB/rxe: Fix up one more receive queue drain path that might prevent
>     cleanup
>   IB/rxe: Avoid ICRC errors by copying into the skb first
>
Thanks for the effort and time you invested here Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 00/11] IB/rxe: Bug fixes

[Andrew Boyer]

This patch set addresses a few issues which we have found in RXE.
The first patch fixes a serious issue that causes kernel panics during
network congestion.

Changes for v1:
* Address review comments and add review tags from Moni
* Split destination cache patch into 4 smaller patches
* Added CONFIG_IPV6 check to destination cache patch
* Added new NETDEV_CHANGE patch

Andrew Boyer (11):
  IB/rxe: Move refcounting earlier in rxe_send()
  IB/rxe: Disable completion upcalls when a CQ is destroyed
  IB/rxe: Remove dangling prototype
  IB/rxe: Fix up the responder's find_resources() function
  IB/rxe: Fix destination cache for IPv6
  IB/rxe: Add dst_clone() in prepare_ipv6_hdr()
  IB/rxe: Fix up rxe_qp_cleanup()
  IB/rxe: Remove unneeded initialization in prepare6()
  IB/rxe: Another fix for broken receive queue draining
  IB/rxe: Avoid ICRC errors by copying into the skb first
  IB/rxe: Handle NETDEV_CHANGE events

 drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h   |  4 ++--
 drivers/infiniband/sw/rxe/rxe_mr.c    | 12 ++++++------
 drivers/infiniband/sw/rxe/rxe_net.c   | 26 +++++++++++++++++++-------
 drivers/infiniband/sw/rxe/rxe_qp.c    |  9 ++-------
 drivers/infiniband/sw/rxe/rxe_req.c   |  4 +++-
 drivers/infiniband/sw/rxe/rxe_resp.c  |  2 +-
 drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.h |  2 ++
 9 files changed, 56 insertions(+), 24 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 01/11] IB/rxe: Move refcounting earlier in rxe_send()

[Andrew Boyer]

The network stack will call nskb's destructor, rxe_skb_tx_dtor(), if the
packet gets dropped by ip_local_out()/ip6_local_out(). Thus we need to add
the QP ref before output to avoid extra dereferences during network
congestion. This could lead to unwanted destruction of the QP.

Fix up the skb_out accounting, too.

Fixes: fda85ce91240 ("IB/rxe: Fix kernel panic from skb destructor")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 08f3f90..0810f38 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -460,12 +460,17 @@ int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
 	nskb->destructor = rxe_skb_tx_dtor;
 	nskb->sk = pkt->qp->sk->sk;
 
+	rxe_add_ref(pkt->qp);
+	atomic_inc(&pkt->qp->skb_out);
+
 	if (av->network_type == RDMA_NETWORK_IPV4) {
 		err = ip_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
 	} else if (av->network_type == RDMA_NETWORK_IPV6) {
 		err = ip6_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
 	} else {
 		pr_err("Unknown layer 3 protocol: %d\n", av->network_type);
+		atomic_dec(&pkt->qp->skb_out);
+		rxe_drop_ref(pkt->qp);
 		kfree_skb(nskb);
 		return -EINVAL;
 	}
@@ -475,10 +480,7 @@ int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
 		return -EAGAIN;
 	}
 
-	rxe_add_ref(pkt->qp);
-	atomic_inc(&pkt->qp->skb_out);
 	kfree_skb(skb);
-
 	return 0;
 }
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 01/11] IB/rxe: Move refcounting earlier in rxe_send()

[Yuval Shaia]

On Fri, Aug 25, 2017 at 03:05:46PM -0400, Andrew Boyer wrote:
> The network stack will call nskb's destructor, rxe_skb_tx_dtor(), if the
> packet gets dropped by ip_local_out()/ip6_local_out(). Thus we need to add
> the QP ref before output to avoid extra dereferences during network
> congestion. This could lead to unwanted destruction of the QP.
> 
> Fix up the skb_out accounting, too.
> 
> Fixes: fda85ce91240 ("IB/rxe: Fix kernel panic from skb destructor")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_net.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 08f3f90..0810f38 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -460,12 +460,17 @@ int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
>  	nskb->destructor = rxe_skb_tx_dtor;
>  	nskb->sk = pkt->qp->sk->sk;
>  
> +	rxe_add_ref(pkt->qp);
> +	atomic_inc(&pkt->qp->skb_out);
> +
>  	if (av->network_type == RDMA_NETWORK_IPV4) {
>  		err = ip_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
>  	} else if (av->network_type == RDMA_NETWORK_IPV6) {
>  		err = ip6_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
>  	} else {
>  		pr_err("Unknown layer 3 protocol: %d\n", av->network_type);
> +		atomic_dec(&pkt->qp->skb_out);
> +		rxe_drop_ref(pkt->qp);
>  		kfree_skb(nskb);
>  		return -EINVAL;
>  	}
> @@ -475,10 +480,7 @@ int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
>  		return -EAGAIN;
>  	}

Shouldn't we also dec and drop ref in case that net_xmit_eval returns
error?

>  
> -	rxe_add_ref(pkt->qp);
> -	atomic_inc(&pkt->qp->skb_out);
>  	kfree_skb(skb);
> -
>  	return 0;
>  }
>  
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 01/11] IB/rxe: Move refcounting earlier in rxe_send()

[Boyer, Andrew]

On 8/27/17, 8:01 AM, "linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org on behalf of Yuval
Shaia" <linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org on behalf of
yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

>On Fri, Aug 25, 2017 at 03:05:46PM -0400, Andrew Boyer wrote:
>> The network stack will call nskb's destructor, rxe_skb_tx_dtor(), if the
>> packet gets dropped by ip_local_out()/ip6_local_out(). Thus we need to
>>add
>> the QP ref before output to avoid extra dereferences during network
>> congestion. This could lead to unwanted destruction of the QP.
>> 
>> Fix up the skb_out accounting, too.
>> 
>> Fixes: fda85ce91240 ("IB/rxe: Fix kernel panic from skb destructor")
>> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>> Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> ---
>>  drivers/infiniband/sw/rxe/rxe_net.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
>>b/drivers/infiniband/sw/rxe/rxe_net.c
>> index 08f3f90..0810f38 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>> @@ -460,12 +460,17 @@ int rxe_send(struct rxe_dev *rxe, struct
>>rxe_pkt_info *pkt, struct sk_buff *skb)
>>  	nskb->destructor = rxe_skb_tx_dtor;
>>  	nskb->sk = pkt->qp->sk->sk;
>>  
>> +	rxe_add_ref(pkt->qp);
>> +	atomic_inc(&pkt->qp->skb_out);
>> +
>>  	if (av->network_type == RDMA_NETWORK_IPV4) {
>>  		err = ip_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
>>  	} else if (av->network_type == RDMA_NETWORK_IPV6) {
>>  		err = ip6_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
>>  	} else {
>>  		pr_err("Unknown layer 3 protocol: %d\n", av->network_type);
>> +		atomic_dec(&pkt->qp->skb_out);
>> +		rxe_drop_ref(pkt->qp);
>>  		kfree_skb(nskb);
>>  		return -EINVAL;
>>  	}
>> @@ -475,10 +480,7 @@ int rxe_send(struct rxe_dev *rxe, struct
>>rxe_pkt_info *pkt, struct sk_buff *skb)
>>  		return -EAGAIN;
>>  	}
>
>Shouldn't we also dec and drop ref in case that net_xmit_eval returns
>error?

That¹s the catch. net_xmit_eval() is just looking for error codes; it¹s
not doing the actual transmit. The transmit already started back in
ip_local_out()/ip6_local_out(), and if those end up failing the destructor
will be called. Any cleanup we do in this error path would be duplicated
in the destructor.

-Andrew

>
>>  
>> -	rxe_add_ref(pkt->qp);
>> -	atomic_inc(&pkt->qp->skb_out);
>>  	kfree_skb(skb);
>> -
>>  	return 0;
>>  }
>>  
>> -- 
>> 1.8.3.1
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>--
>To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 02/11] IB/rxe: Disable completion upcalls when a CQ is destroyed

[Andrew Boyer]

This prevents the stack from accessing userspace objects while they
are being torn down.

One possible sequence of events:
 - Userspace program exits
 - ib_uverbs_cleanup_ucontext() runs, calling ib_destroy_qp(),
   ib_destroy_cq(), etc. and releasing/freeing the UCQ
   - The QP still has tasklets running, so it isn't destroyed yet
   - The CQ is referenced by the QP, so the CQ isn't destroyed yet
   - The UCQ is kfree()'d anyway
 - A send work request completes
 - rxe_send_complete() calls cq->ibcq.comp_handler()
 - ib_uverbs_comp_handler() runs and crashes; the event queue is checked
   for is_closed, but it has no way to check the ib_ucq_object before
   accessing it

The reference counting on the CQ doesn't protect against this since the CQ
hasn't been destroyed yet.
There's no available interface to deregister the UCQ from the CQ, and it
didn't appear that attempting to add reference counting to the UCQ was
going to be a good way to go since this solution is much simpler.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h   |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.h |  1 +
 4 files changed, 24 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c
index 49fe42c..c4aabf7 100644
--- a/drivers/infiniband/sw/rxe/rxe_cq.c
+++ b/drivers/infiniband/sw/rxe/rxe_cq.c
@@ -69,6 +69,14 @@ int rxe_cq_chk_attr(struct rxe_dev *rxe, struct rxe_cq *cq,
 static void rxe_send_complete(unsigned long data)
 {
 	struct rxe_cq *cq = (struct rxe_cq *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cq->cq_lock, flags);
+	if (cq->is_dying) {
+		spin_unlock_irqrestore(&cq->cq_lock, flags);
+		return;
+	}
+	spin_unlock_irqrestore(&cq->cq_lock, flags);
 
 	cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
 }
@@ -97,6 +105,8 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe,
 	if (udata)
 		cq->is_user = 1;
 
+	cq->is_dying = false;
+
 	tasklet_init(&cq->comp_task, rxe_send_complete, (unsigned long)cq);
 
 	spin_lock_init(&cq->cq_lock);
@@ -156,6 +166,15 @@ int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited)
 	return 0;
 }
 
+void rxe_cq_disable(struct rxe_cq *cq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&cq->cq_lock, flags);
+	cq->is_dying = true;
+	spin_unlock_irqrestore(&cq->cq_lock, flags);
+}
+
 void rxe_cq_cleanup(struct rxe_pool_entry *arg)
 {
 	struct rxe_cq *cq = container_of(arg, typeof(*cq), pelem);
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index d6299ed..64f8fa1 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -64,6 +64,8 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe,
 
 int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited);
 
+void rxe_cq_disable(struct rxe_cq *cq);
+
 void rxe_cq_cleanup(struct rxe_pool_entry *arg);
 
 /* rxe_mcast.c */
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index af90a7d..02a39e8 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -960,6 +960,8 @@ static int rxe_destroy_cq(struct ib_cq *ibcq)
 {
 	struct rxe_cq *cq = to_rcq(ibcq);
 
+	rxe_cq_disable(cq);
+
 	rxe_drop_ref(cq);
 	return 0;
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 5a180fb..b09a9e2 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -89,6 +89,7 @@ struct rxe_cq {
 	struct rxe_queue	*queue;
 	spinlock_t		cq_lock;
 	u8			notify;
+	bool			is_dying;
 	int			is_user;
 	struct tasklet_struct	comp_task;
 };
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 03/11] IB/rxe: Remove dangling prototype

[Andrew Boyer]

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_loc.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 64f8fa1..8ef2d3f 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -221,8 +221,6 @@ static inline void rxe_advance_resp_resource(struct rxe_qp *qp)
 void retransmit_timer(unsigned long data);
 void rnr_nak_timer(unsigned long data);
 
-void dump_qp(struct rxe_qp *qp);
-
 /* rxe_srq.c */
 #define IB_SRQ_INIT_MASK (~IB_SRQ_LIMIT)
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 03/11] IB/rxe: Remove dangling prototype

[Yuval Shaia]

On Fri, Aug 25, 2017 at 03:05:48PM -0400, Andrew Boyer wrote:
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

> ---
>  drivers/infiniband/sw/rxe/rxe_loc.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 64f8fa1..8ef2d3f 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -221,8 +221,6 @@ static inline void rxe_advance_resp_resource(struct rxe_qp *qp)
>  void retransmit_timer(unsigned long data);
>  void rnr_nak_timer(unsigned long data);
>  
> -void dump_qp(struct rxe_qp *qp);
> -
>  /* rxe_srq.c */
>  #define IB_SRQ_INIT_MASK (~IB_SRQ_LIMIT)
>  
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 04/11] IB/rxe: Fix up the responder's find_resources() function

[Andrew Boyer]

The resource array is sized by max_dest_rd_atomic, not max_rd_atomic.
Iterating over max_rd_atomic entries of qp->resp.resources[] will cause
incorrect behavior when the two attributes are different (or even
crash if max_rd_atomic is larger).

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index a958ee9..4240866 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -1055,7 +1055,7 @@ static struct resp_res *find_resource(struct rxe_qp *qp, u32 psn)
 {
 	int i;
 
-	for (i = 0; i < qp->attr.max_rd_atomic; i++) {
+	for (i = 0; i < qp->attr.max_dest_rd_atomic; i++) {
 		struct resp_res *res = &qp->resp.resources[i];
 
 		if (res->type == 0)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 04/11] IB/rxe: Fix up the responder's find_resources() function

[Yuval Shaia]

On Fri, Aug 25, 2017 at 03:05:49PM -0400, Andrew Boyer wrote:
> The resource array is sized by max_dest_rd_atomic, not max_rd_atomic.
> Iterating over max_rd_atomic entries of qp->resp.resources[] will cause
> incorrect behavior when the two attributes are different (or even
> crash if max_rd_atomic is larger).
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>

Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

> ---
>  drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index a958ee9..4240866 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -1055,7 +1055,7 @@ static struct resp_res *find_resource(struct rxe_qp *qp, u32 psn)
>  {
>  	int i;
>  
> -	for (i = 0; i < qp->attr.max_rd_atomic; i++) {
> +	for (i = 0; i < qp->attr.max_dest_rd_atomic; i++) {
>  		struct resp_res *res = &qp->resp.resources[i];
>  
>  		if (res->type == 0)
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 05/11] IB/rxe: Fix destination cache for IPv6

[Andrew Boyer]

To successfully match an IPv6 path, the path cookie must match. Store it
in the QP so that the IPv6 path can be reused.

Replace open-coded version of dst_check() with the actual call, fixing the
logic. The open-coded version skips the check call if dst->obsolete is 0
(DST_OBSOLETE_NONE), proceeding to replace the route. DST_OBSOLETE_NONE
means that the route may continue to be used, though.

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c   | 7 ++++++-
 drivers/infiniband/sw/rxe/rxe_verbs.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 0810f38..a93ae3a 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -191,7 +191,7 @@ static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
 	if (qp_type(qp) == IB_QPT_RC)
 		dst = sk_dst_get(qp->sk->sk);
 
-	if (!dst || !(dst->obsolete && dst->ops->check(dst, 0))) {
+	if (!dst || !dst_check(dst, qp->dst_cookie)) {
 		if (dst)
 			dst_release(dst);
 
@@ -209,6 +209,11 @@ static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
 			saddr6 = &av->sgid_addr._sockaddr_in6.sin6_addr;
 			daddr6 = &av->dgid_addr._sockaddr_in6.sin6_addr;
 			dst = rxe_find_route6(rxe->ndev, saddr6, daddr6);
+#if IS_ENABLED(CONFIG_IPV6)
+			if (dst)
+				qp->dst_cookie =
+					rt6_get_cookie((struct rt6_info *)dst);
+#endif
 		}
 	}
 
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index b09a9e2..0c2dbe4 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -248,6 +248,7 @@ struct rxe_qp {
 	struct rxe_rq		rq;
 
 	struct socket		*sk;
+	u32			dst_cookie;
 
 	struct rxe_av		pri_av;
 	struct rxe_av		alt_av;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 06/11] IB/rxe: Add dst_clone() in prepare_ipv6_hdr()

[Andrew Boyer]

Otherwise the reference count goes negative as IPv6 packets complete.

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index a93ae3a..3fbbadc 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -342,7 +342,7 @@ static void prepare_ipv6_hdr(struct dst_entry *dst, struct sk_buff *skb,
 	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
 	IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED
 			    | IPSKB_REROUTED);
-	skb_dst_set(skb, dst);
+	skb_dst_set(skb, dst_clone(dst));
 
 	__skb_push(skb, sizeof(*ip6h));
 	skb_reset_network_header(skb);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 07/11] IB/rxe: Fix up rxe_qp_cleanup()

[Andrew Boyer]

Replace sk_dst_get()/dst_release() in rxe_qp_cleanup() with sk_dst_reset().
sk_dst_get() takes a new reference on dst, so the dst_release() doesn't
actually release the original reference, which was the design intent.

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_qp.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 80ccc7c..00bda93 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -851,13 +851,8 @@ void rxe_qp_cleanup(struct rxe_pool_entry *arg)
 		qp->resp.mr = NULL;
 	}
 
-	if (qp_type(qp) == IB_QPT_RC) {
-		struct dst_entry *dst = NULL;
-
-		dst = sk_dst_get(qp->sk->sk);
-		if (dst)
-			dst_release(dst);
-	}
+	if (qp_type(qp) == IB_QPT_RC)
+		sk_dst_reset(qp->sk->sk);
 
 	free_rd_atomic_resources(qp);
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 08/11] IB/rxe: Remove unneeded initialization in prepare6()

[Andrew Boyer]

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 3fbbadc..3ba76e7 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -393,7 +393,7 @@ static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
 		    struct sk_buff *skb, struct rxe_av *av)
 {
 	struct rxe_qp *qp = pkt->qp;
-	struct dst_entry *dst = NULL;
+	struct dst_entry *dst;
 	struct in6_addr *saddr = &av->sgid_addr._sockaddr_in6.sin6_addr;
 	struct in6_addr *daddr = &av->dgid_addr._sockaddr_in6.sin6_addr;
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 08/11] IB/rxe: Remove unneeded initialization in prepare6()

[Yuval Shaia]

On Fri, Aug 25, 2017 at 03:05:53PM -0400, Andrew Boyer wrote:
> Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>

Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

> ---
>  drivers/infiniband/sw/rxe/rxe_net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 3fbbadc..3ba76e7 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -393,7 +393,7 @@ static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
>  		    struct sk_buff *skb, struct rxe_av *av)
>  {
>  	struct rxe_qp *qp = pkt->qp;
> -	struct dst_entry *dst = NULL;
> +	struct dst_entry *dst;
>  	struct in6_addr *saddr = &av->sgid_addr._sockaddr_in6.sin6_addr;
>  	struct in6_addr *daddr = &av->dgid_addr._sockaddr_in6.sin6_addr;
>  
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 09/11] IB/rxe: Another fix for broken receive queue draining

[Andrew Boyer]

This fixes another path in rxe_requester() that might overlook stale SKBs,
preventing cleanup.

Fixes: 1217197142d1 ("rxe: fix broken receive queue draining")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 7ee465d..24fc0b8 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -594,8 +594,10 @@ int rxe_requester(void *arg)
 	rxe_add_ref(qp);
 
 next_wqe:
-	if (unlikely(!qp->valid))
+	if (unlikely(!qp->valid)) {
+		rxe_drain_req_pkts(qp, true);
 		goto exit;
+	}
 
 	if (unlikely(qp->req.state == QP_STATE_ERROR)) {
 		rxe_drain_req_pkts(qp, true);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 10/11] IB/rxe: Avoid ICRC errors by copying into the skb first

[Andrew Boyer]

The current process is to first calculate the CRC and then copy the client
data into the packet. This leaves a window in which the packet contents and
CRC can get out of sync, if the client changes the data after the CRC is
calculated but before the data is copied.

By copying the data into the packet and then calculating the CRC directly
from the packet contents we eliminate the window.

This can be seen with qperf's ud_bi_bw test. This seems like very
strange/reckless client behavior, but whether the client has mangled its
data or not RXE should be able to transfer it reliably.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_mr.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index e37cc89..5c2684b 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -367,11 +367,11 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		dest = (dir == to_mem_obj) ?
 			((void *)(uintptr_t)iova) : addr;
 
+		memcpy(dest, src, length);
+
 		if (crcp)
 			*crcp = rxe_crc32(to_rdev(mem->pd->ibpd.device),
-					*crcp, src, length);
-
-		memcpy(dest, src, length);
+					*crcp, dest, length);
 
 		return 0;
 	}
@@ -401,11 +401,11 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		if (bytes > length)
 			bytes = length;
 
+		memcpy(dest, src, bytes);
+
 		if (crcp)
 			crc = rxe_crc32(to_rdev(mem->pd->ibpd.device),
-					crc, src, bytes);
-
-		memcpy(dest, src, bytes);
+					crc, dest, bytes);
 
 		length	-= bytes;
 		addr	+= bytes;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Andrew Boyer]

Without this fix, ports configured on top of ixgbe miss link up
notifications. ibv_query_port() will continue to return IBV_PORT_DOWN even
though the port is up and working.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 3ba76e7..133c6c4 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block *not_blk,
 		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
 		rxe_set_mtu(rxe, ndev->mtu);
 		break;
-	case NETDEV_REBOOT:
 	case NETDEV_CHANGE:
+		if (netif_running(ndev) && netif_carrier_ok(ndev))
+			rxe_port_up(rxe, port_num);
+		else
+			rxe_port_down(rxe, port_num);
+		break;
+	case NETDEV_REBOOT:
 	case NETDEV_GOING_DOWN:
 	case NETDEV_CHANGEADDR:
 	case NETDEV_CHANGENAME:
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Yuval Shaia]

On Fri, Aug 25, 2017 at 03:05:56PM -0400, Andrew Boyer wrote:
> Without this fix, ports configured on top of ixgbe miss link up
> notifications. ibv_query_port() will continue to return IBV_PORT_DOWN even
> though the port is up and working.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> ---
>  drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 3ba76e7..133c6c4 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block *not_blk,
>  		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
>  		rxe_set_mtu(rxe, ndev->mtu);
>  		break;
> -	case NETDEV_REBOOT:
>  	case NETDEV_CHANGE:
> +		if (netif_running(ndev) && netif_carrier_ok(ndev))
> +			rxe_port_up(rxe, port_num);

On top of which branch/patch this patch is based on?

> +		else
> +			rxe_port_down(rxe, port_num);
> +		break;
> +	case NETDEV_REBOOT:
>  	case NETDEV_GOING_DOWN:
>  	case NETDEV_CHANGEADDR:
>  	case NETDEV_CHANGENAME:
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Boyer, Andrew]

On 8/27/17, 6:30 AM, "Yuval Shaia" <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

>On Fri, Aug 25, 2017 at 03:05:56PM -0400, Andrew Boyer wrote:
>> Without this fix, ports configured on top of ixgbe miss link up
>> notifications. ibv_query_port() will continue to return IBV_PORT_DOWN
>>even
>> though the port is up and working.
>> 
>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>> ---
>>  drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
>>b/drivers/infiniband/sw/rxe/rxe_net.c
>> index 3ba76e7..133c6c4 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>> @@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block
>>*not_blk,
>>  		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
>>  		rxe_set_mtu(rxe, ndev->mtu);
>>  		break;
>> -	case NETDEV_REBOOT:
>>  	case NETDEV_CHANGE:
>> +		if (netif_running(ndev) && netif_carrier_ok(ndev))
>> +			rxe_port_up(rxe, port_num);
>
>On top of which branch/patch this patch is based on?

Yikes, it relies on an internal patch that Doug wasn¹t interested in
taking. Will respin.

>
>> +		else
>> +			rxe_port_down(rxe, port_num);
>> +		break;
>> +	case NETDEV_REBOOT:
>>  	case NETDEV_GOING_DOWN:
>>  	case NETDEV_CHANGEADDR:
>>  	case NETDEV_CHANGENAME:
>> -- 
>> 1.8.3.1
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 1683 bytes --]

On 8/28/2017 8:38 AM, Boyer, Andrew wrote:
> 
> On 8/27/17, 6:30 AM, "Yuval Shaia" <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> 
>> On Fri, Aug 25, 2017 at 03:05:56PM -0400, Andrew Boyer wrote:
>>> Without this fix, ports configured on top of ixgbe miss link up
>>> notifications. ibv_query_port() will continue to return IBV_PORT_DOWN
>>> even
>>> though the port is up and working.
>>>
>>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>>> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>>> ---
>>>  drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
>>> b/drivers/infiniband/sw/rxe/rxe_net.c
>>> index 3ba76e7..133c6c4 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>>> @@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block
>>> *not_blk,
>>>  		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
>>>  		rxe_set_mtu(rxe, ndev->mtu);
>>>  		break;
>>> -	case NETDEV_REBOOT:
>>>  	case NETDEV_CHANGE:
>>> +		if (netif_running(ndev) && netif_carrier_ok(ndev))
>>> +			rxe_port_up(rxe, port_num);
>> On top of which branch/patch this patch is based on?
> Yikes, it relies on an internal patch that Doug wasn¹t interested in
> taking. Will respin.
> 

I don't remember seeing a patch come through that added rxe_port_up(),
let alone turning it away...

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH v1 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Boyer, Andrew]

On 8/28/17, 9:37 AM, "Doug Ledford" <dledford@redhat.com> wrote:

>On 8/28/2017 8:38 AM, Boyer, Andrew wrote:
>> 
>> On 8/27/17, 6:30 AM, "Yuval Shaia" <yuval.shaia@oracle.com> wrote:
>> 
>>> On Fri, Aug 25, 2017 at 03:05:56PM -0400, Andrew Boyer wrote:
>>>> Without this fix, ports configured on top of ixgbe miss link up
>>>> notifications. ibv_query_port() will continue to return IBV_PORT_DOWN
>>>> even
>>>> though the port is up and working.
>>>>
>>>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>>>> Signed-off-by: Andrew Boyer <andrew.boyer@dell.com>
>>>> ---
>>>>  drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
>>>> b/drivers/infiniband/sw/rxe/rxe_net.c
>>>> index 3ba76e7..133c6c4 100644
>>>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>>>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>>>> @@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block
>>>> *not_blk,
>>>>  		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
>>>>  		rxe_set_mtu(rxe, ndev->mtu);
>>>>  		break;
>>>> -	case NETDEV_REBOOT:
>>>>  	case NETDEV_CHANGE:
>>>> +		if (netif_running(ndev) && netif_carrier_ok(ndev))
>>>> +			rxe_port_up(rxe, port_num);
>>> On top of which branch/patch this patch is based on?
>> Yikes, it relies on an internal patch that Doug wasn¹t interested in
>> taking. Will respin.
>> 
>
>I don't remember seeing a patch come through that added rxe_port_up(),
>let alone turning it away...

It’s the port_num argument that doesn’t exist upstream. We added the
ability for RXE to support a second port (to better mimic an mlx4) but you
said it wasn’t needed.

-Andrew

>
>-- 
>Doug Ledford <dledford@redhat.com>
>    GPG Key ID: B826A3330E572FDD
>    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

Re: [PATCH v1 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 2211 bytes --]

On 8/28/2017 9:47 AM, Boyer, Andrew wrote:
> 
> 
> On 8/28/17, 9:37 AM, "Doug Ledford" <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 
>> On 8/28/2017 8:38 AM, Boyer, Andrew wrote:
>>>
>>> On 8/27/17, 6:30 AM, "Yuval Shaia" <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>>>
>>>> On Fri, Aug 25, 2017 at 03:05:56PM -0400, Andrew Boyer wrote:
>>>>> Without this fix, ports configured on top of ixgbe miss link up
>>>>> notifications. ibv_query_port() will continue to return IBV_PORT_DOWN
>>>>> even
>>>>> though the port is up and working.
>>>>>
>>>>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>>>>> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
>>>>> ---
>>>>>  drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
>>>>> b/drivers/infiniband/sw/rxe/rxe_net.c
>>>>> index 3ba76e7..133c6c4 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>>>>> @@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block
>>>>> *not_blk,
>>>>>  		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
>>>>>  		rxe_set_mtu(rxe, ndev->mtu);
>>>>>  		break;
>>>>> -	case NETDEV_REBOOT:
>>>>>  	case NETDEV_CHANGE:
>>>>> +		if (netif_running(ndev) && netif_carrier_ok(ndev))
>>>>> +			rxe_port_up(rxe, port_num);
>>>> On top of which branch/patch this patch is based on?
>>> Yikes, it relies on an internal patch that Doug wasn¹t interested in
>>> taking. Will respin.
>>>
>>
>> I don't remember seeing a patch come through that added rxe_port_up(),
>> let alone turning it away...
> 
> It’s the port_num argument that doesn’t exist upstream. We added the
> ability for RXE to support a second port (to better mimic an mlx4) but you
> said it wasn’t needed.

OK.  I still don't remember the event though ;-)

Anyway, I'll wait for you to send a fixed version of the code.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH v1 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Yuval Shaia]

On Mon, Aug 28, 2017 at 09:37:08AM -0400, Doug Ledford wrote:
> On 8/28/2017 8:38 AM, Boyer, Andrew wrote:
> > 
> > On 8/27/17, 6:30 AM, "Yuval Shaia" <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> > 
> >> On Fri, Aug 25, 2017 at 03:05:56PM -0400, Andrew Boyer wrote:
> >>> Without this fix, ports configured on top of ixgbe miss link up
> >>> notifications. ibv_query_port() will continue to return IBV_PORT_DOWN
> >>> even
> >>> though the port is up and working.
> >>>
> >>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> >>> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
> >>> ---
> >>>  drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
> >>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
> >>> b/drivers/infiniband/sw/rxe/rxe_net.c
> >>> index 3ba76e7..133c6c4 100644
> >>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> >>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> >>> @@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block
> >>> *not_blk,
> >>>  		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
> >>>  		rxe_set_mtu(rxe, ndev->mtu);
> >>>  		break;
> >>> -	case NETDEV_REBOOT:
> >>>  	case NETDEV_CHANGE:
> >>> +		if (netif_running(ndev) && netif_carrier_ok(ndev))
> >>> +			rxe_port_up(rxe, port_num);
> >> On top of which branch/patch this patch is based on?
> > Yikes, it relies on an internal patch that Doug wasn¹t interested in
> > taking. Will respin.
> > 
> 
> I don't remember seeing a patch come through that added rxe_port_up(),
> let alone turning it away...

rxe_port_up is already implemented but since RXE device has only one port i
don't see a reason to pass porn_num to this function.
I might be wrong.

> 
> -- 
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG Key ID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 11/11] IB/rxe: Handle NETDEV_CHANGE events

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]

Hi Andrew,

[auto build test ERROR on rdma/master]
[also build test ERROR on v4.13-rc6 next-20170825]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrew-Boyer/IB-rxe-Bug-fixes/20170828-060737
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git master
config: i386-randconfig-x001-201735 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/infiniband//sw/rxe/rxe_net.c: In function 'rxe_notify':
>> drivers/infiniband//sw/rxe/rxe_net.c:656:21: error: 'port_num' undeclared (first use in this function)
       rxe_port_up(rxe, port_num);
                        ^~~~~~~~
   drivers/infiniband//sw/rxe/rxe_net.c:656:21: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/infiniband//sw/rxe/rxe_net.c:656:4: error: too many arguments to function 'rxe_port_up'
       rxe_port_up(rxe, port_num);
       ^~~~~~~~~~~
   drivers/infiniband//sw/rxe/rxe_net.c:604:6: note: declared here
    void rxe_port_up(struct rxe_dev *rxe)
         ^~~~~~~~~~~
>> drivers/infiniband//sw/rxe/rxe_net.c:658:4: error: too many arguments to function 'rxe_port_down'
       rxe_port_down(rxe, port_num);
       ^~~~~~~~~~~~~
   drivers/infiniband//sw/rxe/rxe_net.c:617:6: note: declared here
    void rxe_port_down(struct rxe_dev *rxe)
         ^~~~~~~~~~~~~

vim +/port_num +656 drivers/infiniband//sw/rxe/rxe_net.c

   628	
   629	static int rxe_notify(struct notifier_block *not_blk,
   630			      unsigned long event,
   631			      void *arg)
   632	{
   633		struct net_device *ndev = netdev_notifier_info_to_dev(arg);
   634		struct rxe_dev *rxe = net_to_rxe(ndev);
   635	
   636		if (!rxe)
   637			goto out;
   638	
   639		switch (event) {
   640		case NETDEV_UNREGISTER:
   641			list_del(&rxe->list);
   642			rxe_remove(rxe);
   643			break;
   644		case NETDEV_UP:
   645			rxe_port_up(rxe);
   646			break;
   647		case NETDEV_DOWN:
   648			rxe_port_down(rxe);
   649			break;
   650		case NETDEV_CHANGEMTU:
   651			pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
   652			rxe_set_mtu(rxe, ndev->mtu);
   653			break;
   654		case NETDEV_CHANGE:
   655			if (netif_running(ndev) && netif_carrier_ok(ndev))
 > 656				rxe_port_up(rxe, port_num);
   657			else
 > 658				rxe_port_down(rxe, port_num);
   659			break;
   660		case NETDEV_REBOOT:
   661		case NETDEV_GOING_DOWN:
   662		case NETDEV_CHANGEADDR:
   663		case NETDEV_CHANGENAME:
   664		case NETDEV_FEAT_CHANGE:
   665		default:
   666			pr_info("ignoring netdev event = %ld for %s\n",
   667				event, ndev->name);
   668			break;
   669		}
   670	out:
   671		return NOTIFY_OK;
   672	}
   673	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30233 bytes --]

Re: [PATCH v1 00/11] IB/rxe: Bug fixes

[Yuval Shaia]

Hi Andrew,
I'm working on a bug that prevents RXE from routing between two instances of
RXE on the same host (https://patchwork.kernel.org/patch/9865227/).

When working on latest branches (since intro of 587fea741134) i started to
get system panic and i was not able to continue.

With your patch-set it works excellent!!!

So, besides some comments for one or two patches and besides some patches
which i skip the review because of lack of background, you have my:

Tested-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

And

Big-Thanks-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Yuval

On Fri, Aug 25, 2017 at 03:05:45PM -0400, Andrew Boyer wrote:
> This patch set addresses a few issues which we have found in RXE.
> The first patch fixes a serious issue that causes kernel panics during
> network congestion.
> 
> Changes for v1:
> * Address review comments and add review tags from Moni
> * Split destination cache patch into 4 smaller patches
> * Added CONFIG_IPV6 check to destination cache patch
> * Added new NETDEV_CHANGE patch
> 
> Andrew Boyer (11):
>   IB/rxe: Move refcounting earlier in rxe_send()
>   IB/rxe: Disable completion upcalls when a CQ is destroyed
>   IB/rxe: Remove dangling prototype
>   IB/rxe: Fix up the responder's find_resources() function
>   IB/rxe: Fix destination cache for IPv6
>   IB/rxe: Add dst_clone() in prepare_ipv6_hdr()
>   IB/rxe: Fix up rxe_qp_cleanup()
>   IB/rxe: Remove unneeded initialization in prepare6()
>   IB/rxe: Another fix for broken receive queue draining
>   IB/rxe: Avoid ICRC errors by copying into the skb first
>   IB/rxe: Handle NETDEV_CHANGE events
> 
>  drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
>  drivers/infiniband/sw/rxe/rxe_loc.h   |  4 ++--
>  drivers/infiniband/sw/rxe/rxe_mr.c    | 12 ++++++------
>  drivers/infiniband/sw/rxe/rxe_net.c   | 26 +++++++++++++++++++-------
>  drivers/infiniband/sw/rxe/rxe_qp.c    |  9 ++-------
>  drivers/infiniband/sw/rxe/rxe_req.c   |  4 +++-
>  drivers/infiniband/sw/rxe/rxe_resp.c  |  2 +-
>  drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
>  drivers/infiniband/sw/rxe/rxe_verbs.h |  2 ++
>  9 files changed, 56 insertions(+), 24 deletions(-)
> 
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 00/11] IB/rxe: Bug fixes

[Andrew Boyer]

This patch set addresses a few issues which we have found in RXE.
The first patch fixes a serious issue that causes kernel panics during
network congestion.

Changes for v2:
* Update NETDEV_CHANGE patch so it builds; original was inadvertently
  based on an internal branch

Changes for v1:
* Address review comments and add review tags from Moni
* Split destination cache patch into 4 smaller patches
* Added CONFIG_IPV6 check to destination cache patch
* Added new NETDEV_CHANGE patch

Andrew Boyer (11):
  IB/rxe: Move refcounting earlier in rxe_send()
  IB/rxe: Disable completion upcalls when a CQ is destroyed
  IB/rxe: Remove dangling prototype
  IB/rxe: Fix up the responder's find_resources() function
  IB/rxe: Fix destination cache for IPv6
  IB/rxe: Add dst_clone() in prepare_ipv6_hdr()
  IB/rxe: Fix up rxe_qp_cleanup()
  IB/rxe: Remove unneeded initialization in prepare6()
  IB/rxe: Another fix for broken receive queue draining
  IB/rxe: Avoid ICRC errors by copying into the skb first
  IB/rxe: Handle NETDEV_CHANGE events

 drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h   |  4 ++--
 drivers/infiniband/sw/rxe/rxe_mr.c    | 12 ++++++------
 drivers/infiniband/sw/rxe/rxe_net.c   | 26 +++++++++++++++++++-------
 drivers/infiniband/sw/rxe/rxe_qp.c    |  9 ++-------
 drivers/infiniband/sw/rxe/rxe_req.c   |  4 +++-
 drivers/infiniband/sw/rxe/rxe_resp.c  |  2 +-
 drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.h |  2 ++
 9 files changed, 56 insertions(+), 24 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 01/11] IB/rxe: Move refcounting earlier in rxe_send()

[Andrew Boyer]

The network stack will call nskb's destructor, rxe_skb_tx_dtor(), if the
packet gets dropped by ip_local_out()/ip6_local_out(). Thus we need to add
the QP ref before output to avoid extra dereferences during network
congestion. This could lead to unwanted destruction of the QP.

Fix up the skb_out accounting, too.

Fixes: fda85ce91240 ("IB/rxe: Fix kernel panic from skb destructor")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 08f3f90..0810f38 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -460,12 +460,17 @@ int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
 	nskb->destructor = rxe_skb_tx_dtor;
 	nskb->sk = pkt->qp->sk->sk;
 
+	rxe_add_ref(pkt->qp);
+	atomic_inc(&pkt->qp->skb_out);
+
 	if (av->network_type == RDMA_NETWORK_IPV4) {
 		err = ip_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
 	} else if (av->network_type == RDMA_NETWORK_IPV6) {
 		err = ip6_local_out(dev_net(skb_dst(skb)->dev), nskb->sk, nskb);
 	} else {
 		pr_err("Unknown layer 3 protocol: %d\n", av->network_type);
+		atomic_dec(&pkt->qp->skb_out);
+		rxe_drop_ref(pkt->qp);
 		kfree_skb(nskb);
 		return -EINVAL;
 	}
@@ -475,10 +480,7 @@ int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, struct sk_buff *skb)
 		return -EAGAIN;
 	}
 
-	rxe_add_ref(pkt->qp);
-	atomic_inc(&pkt->qp->skb_out);
 	kfree_skb(skb);
-
 	return 0;
 }
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/11] IB/rxe: Disable completion upcalls when a CQ is destroyed

[Andrew Boyer]

This prevents the stack from accessing userspace objects while they
are being torn down.

One possible sequence of events:
 - Userspace program exits
 - ib_uverbs_cleanup_ucontext() runs, calling ib_destroy_qp(),
   ib_destroy_cq(), etc. and releasing/freeing the UCQ
   - The QP still has tasklets running, so it isn't destroyed yet
   - The CQ is referenced by the QP, so the CQ isn't destroyed yet
   - The UCQ is kfree()'d anyway
 - A send work request completes
 - rxe_send_complete() calls cq->ibcq.comp_handler()
 - ib_uverbs_comp_handler() runs and crashes; the event queue is checked
   for is_closed, but it has no way to check the ib_ucq_object before
   accessing it

The reference counting on the CQ doesn't protect against this since the CQ
hasn't been destroyed yet.
There's no available interface to deregister the UCQ from the CQ, and it
didn't appear that attempting to add reference counting to the UCQ was
going to be a good way to go since this solution is much simpler.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h   |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
 drivers/infiniband/sw/rxe/rxe_verbs.h |  1 +
 4 files changed, 24 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_cq.c b/drivers/infiniband/sw/rxe/rxe_cq.c
index 49fe42c..c4aabf7 100644
--- a/drivers/infiniband/sw/rxe/rxe_cq.c
+++ b/drivers/infiniband/sw/rxe/rxe_cq.c
@@ -69,6 +69,14 @@ int rxe_cq_chk_attr(struct rxe_dev *rxe, struct rxe_cq *cq,
 static void rxe_send_complete(unsigned long data)
 {
 	struct rxe_cq *cq = (struct rxe_cq *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cq->cq_lock, flags);
+	if (cq->is_dying) {
+		spin_unlock_irqrestore(&cq->cq_lock, flags);
+		return;
+	}
+	spin_unlock_irqrestore(&cq->cq_lock, flags);
 
 	cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
 }
@@ -97,6 +105,8 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe,
 	if (udata)
 		cq->is_user = 1;
 
+	cq->is_dying = false;
+
 	tasklet_init(&cq->comp_task, rxe_send_complete, (unsigned long)cq);
 
 	spin_lock_init(&cq->cq_lock);
@@ -156,6 +166,15 @@ int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited)
 	return 0;
 }
 
+void rxe_cq_disable(struct rxe_cq *cq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&cq->cq_lock, flags);
+	cq->is_dying = true;
+	spin_unlock_irqrestore(&cq->cq_lock, flags);
+}
+
 void rxe_cq_cleanup(struct rxe_pool_entry *arg)
 {
 	struct rxe_cq *cq = container_of(arg, typeof(*cq), pelem);
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index d6299ed..64f8fa1 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -64,6 +64,8 @@ int rxe_cq_from_init(struct rxe_dev *rxe, struct rxe_cq *cq, int cqe,
 
 int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited);
 
+void rxe_cq_disable(struct rxe_cq *cq);
+
 void rxe_cq_cleanup(struct rxe_pool_entry *arg);
 
 /* rxe_mcast.c */
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index af90a7d..02a39e8 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -960,6 +960,8 @@ static int rxe_destroy_cq(struct ib_cq *ibcq)
 {
 	struct rxe_cq *cq = to_rcq(ibcq);
 
+	rxe_cq_disable(cq);
+
 	rxe_drop_ref(cq);
 	return 0;
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 5a180fb..b09a9e2 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -89,6 +89,7 @@ struct rxe_cq {
 	struct rxe_queue	*queue;
 	spinlock_t		cq_lock;
 	u8			notify;
+	bool			is_dying;
 	int			is_user;
 	struct tasklet_struct	comp_task;
 };
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 03/11] IB/rxe: Remove dangling prototype

[Andrew Boyer]

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
Acked-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_loc.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 64f8fa1..8ef2d3f 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -221,8 +221,6 @@ static inline void rxe_advance_resp_resource(struct rxe_qp *qp)
 void retransmit_timer(unsigned long data);
 void rnr_nak_timer(unsigned long data);
 
-void dump_qp(struct rxe_qp *qp);
-
 /* rxe_srq.c */
 #define IB_SRQ_INIT_MASK (~IB_SRQ_LIMIT)
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 04/11] IB/rxe: Fix up the responder's find_resources() function

[Andrew Boyer]

The resource array is sized by max_dest_rd_atomic, not max_rd_atomic.
Iterating over max_rd_atomic entries of qp->resp.resources[] will cause
incorrect behavior when the two attributes are different (or even
crash if max_rd_atomic is larger).

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index a958ee9..4240866 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -1055,7 +1055,7 @@ static struct resp_res *find_resource(struct rxe_qp *qp, u32 psn)
 {
 	int i;
 
-	for (i = 0; i < qp->attr.max_rd_atomic; i++) {
+	for (i = 0; i < qp->attr.max_dest_rd_atomic; i++) {
 		struct resp_res *res = &qp->resp.resources[i];
 
 		if (res->type == 0)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 05/11] IB/rxe: Fix destination cache for IPv6

[Andrew Boyer]

To successfully match an IPv6 path, the path cookie must match. Store it
in the QP so that the IPv6 path can be reused.

Replace open-coded version of dst_check() with the actual call, fixing the
logic. The open-coded version skips the check call if dst->obsolete is 0
(DST_OBSOLETE_NONE), proceeding to replace the route. DST_OBSOLETE_NONE
means that the route may continue to be used, though.

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c   | 7 ++++++-
 drivers/infiniband/sw/rxe/rxe_verbs.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 0810f38..a93ae3a 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -191,7 +191,7 @@ static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
 	if (qp_type(qp) == IB_QPT_RC)
 		dst = sk_dst_get(qp->sk->sk);
 
-	if (!dst || !(dst->obsolete && dst->ops->check(dst, 0))) {
+	if (!dst || !dst_check(dst, qp->dst_cookie)) {
 		if (dst)
 			dst_release(dst);
 
@@ -209,6 +209,11 @@ static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
 			saddr6 = &av->sgid_addr._sockaddr_in6.sin6_addr;
 			daddr6 = &av->dgid_addr._sockaddr_in6.sin6_addr;
 			dst = rxe_find_route6(rxe->ndev, saddr6, daddr6);
+#if IS_ENABLED(CONFIG_IPV6)
+			if (dst)
+				qp->dst_cookie =
+					rt6_get_cookie((struct rt6_info *)dst);
+#endif
 		}
 	}
 
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index b09a9e2..0c2dbe4 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -248,6 +248,7 @@ struct rxe_qp {
 	struct rxe_rq		rq;
 
 	struct socket		*sk;
+	u32			dst_cookie;
 
 	struct rxe_av		pri_av;
 	struct rxe_av		alt_av;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 06/11] IB/rxe: Add dst_clone() in prepare_ipv6_hdr()

[Andrew Boyer]

Otherwise the reference count goes negative as IPv6 packets complete.

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index a93ae3a..3fbbadc 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -342,7 +342,7 @@ static void prepare_ipv6_hdr(struct dst_entry *dst, struct sk_buff *skb,
 	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
 	IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED
 			    | IPSKB_REROUTED);
-	skb_dst_set(skb, dst);
+	skb_dst_set(skb, dst_clone(dst));
 
 	__skb_push(skb, sizeof(*ip6h));
 	skb_reset_network_header(skb);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 07/11] IB/rxe: Fix up rxe_qp_cleanup()

[Andrew Boyer]

Replace sk_dst_get()/dst_release() in rxe_qp_cleanup() with sk_dst_reset().
sk_dst_get() takes a new reference on dst, so the dst_release() doesn't
actually release the original reference, which was the design intent.

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_qp.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 80ccc7c..00bda93 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -851,13 +851,8 @@ void rxe_qp_cleanup(struct rxe_pool_entry *arg)
 		qp->resp.mr = NULL;
 	}
 
-	if (qp_type(qp) == IB_QPT_RC) {
-		struct dst_entry *dst = NULL;
-
-		dst = sk_dst_get(qp->sk->sk);
-		if (dst)
-			dst_release(dst);
-	}
+	if (qp_type(qp) == IB_QPT_RC)
+		sk_dst_reset(qp->sk->sk);
 
 	free_rd_atomic_resources(qp);
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 08/11] IB/rxe: Remove unneeded initialization in prepare6()

[Andrew Boyer]

Fixes: 4ed6ad1eb30e ("IB/rxe: Cache dst in QP instead of getting it...")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 3fbbadc..3ba76e7 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -393,7 +393,7 @@ static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
 		    struct sk_buff *skb, struct rxe_av *av)
 {
 	struct rxe_qp *qp = pkt->qp;
-	struct dst_entry *dst = NULL;
+	struct dst_entry *dst;
 	struct in6_addr *saddr = &av->sgid_addr._sockaddr_in6.sin6_addr;
 	struct in6_addr *daddr = &av->dgid_addr._sockaddr_in6.sin6_addr;
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 09/11] IB/rxe: Another fix for broken receive queue draining

[Andrew Boyer]

This fixes another path in rxe_requester() that might overlook stale SKBs,
preventing cleanup.

Fixes: 1217197142d1 ("rxe: fix broken receive queue draining")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 7ee465d..24fc0b8 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -594,8 +594,10 @@ int rxe_requester(void *arg)
 	rxe_add_ref(qp);
 
 next_wqe:
-	if (unlikely(!qp->valid))
+	if (unlikely(!qp->valid)) {
+		rxe_drain_req_pkts(qp, true);
 		goto exit;
+	}
 
 	if (unlikely(qp->req.state == QP_STATE_ERROR)) {
 		rxe_drain_req_pkts(qp, true);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 10/11] IB/rxe: Avoid ICRC errors by copying into the skb first

[Andrew Boyer]

The current process is to first calculate the CRC and then copy the client
data into the packet. This leaves a window in which the packet contents and
CRC can get out of sync, if the client changes the data after the CRC is
calculated but before the data is copied.

By copying the data into the packet and then calculating the CRC directly
from the packet contents we eliminate the window.

This can be seen with qperf's ud_bi_bw test. This seems like very
strange/reckless client behavior, but whether the client has mangled its
data or not RXE should be able to transfer it reliably.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_mr.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index e37cc89..5c2684b 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -367,11 +367,11 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		dest = (dir == to_mem_obj) ?
 			((void *)(uintptr_t)iova) : addr;
 
+		memcpy(dest, src, length);
+
 		if (crcp)
 			*crcp = rxe_crc32(to_rdev(mem->pd->ibpd.device),
-					*crcp, src, length);
-
-		memcpy(dest, src, length);
+					*crcp, dest, length);
 
 		return 0;
 	}
@@ -401,11 +401,11 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		if (bytes > length)
 			bytes = length;
 
+		memcpy(dest, src, bytes);
+
 		if (crcp)
 			crc = rxe_crc32(to_rdev(mem->pd->ibpd.device),
-					crc, src, bytes);
-
-		memcpy(dest, src, bytes);
+					crc, dest, bytes);
 
 		length	-= bytes;
 		addr	+= bytes;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Andrew Boyer]

Without this fix, ports configured on top of ixgbe miss link up
notifications. ibv_query_port() will continue to return IBV_PORT_DOWN even
though the port is up and working.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 3ba76e7..59dee10 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block *not_blk,
 		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
 		rxe_set_mtu(rxe, ndev->mtu);
 		break;
-	case NETDEV_REBOOT:
 	case NETDEV_CHANGE:
+		if (netif_running(ndev) && netif_carrier_ok(ndev))
+			rxe_port_up(rxe);
+		else
+			rxe_port_down(rxe);
+		break;
+	case NETDEV_REBOOT:
 	case NETDEV_GOING_DOWN:
 	case NETDEV_CHANGEADDR:
 	case NETDEV_CHANGENAME:
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 11/11] IB/rxe: Handle NETDEV_CHANGE events

[Yuval Shaia]

On Mon, Aug 28, 2017 at 04:11:59PM -0400, Andrew Boyer wrote:
> Without this fix, ports configured on top of ixgbe miss link up
> notifications. ibv_query_port() will continue to return IBV_PORT_DOWN even
> though the port is up and working.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>

Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

> ---
>  drivers/infiniband/sw/rxe/rxe_net.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 3ba76e7..59dee10 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -651,8 +651,13 @@ static int rxe_notify(struct notifier_block *not_blk,
>  		pr_info("%s changed mtu to %d\n", ndev->name, ndev->mtu);
>  		rxe_set_mtu(rxe, ndev->mtu);
>  		break;
> -	case NETDEV_REBOOT:
>  	case NETDEV_CHANGE:
> +		if (netif_running(ndev) && netif_carrier_ok(ndev))
> +			rxe_port_up(rxe);
> +		else
> +			rxe_port_down(rxe);
> +		break;
> +	case NETDEV_REBOOT:
>  	case NETDEV_GOING_DOWN:
>  	case NETDEV_CHANGEADDR:
>  	case NETDEV_CHANGENAME:
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/11] IB/rxe: Bug fixes

[Doug Ledford]

On Mon, 2017-08-28 at 16:11 -0400, Andrew Boyer wrote:
> This patch set addresses a few issues which we have found in RXE.
> The first patch fixes a serious issue that causes kernel panics
> during
> network congestion.
> 
> Changes for v2:
> * Update NETDEV_CHANGE patch so it builds; original was inadvertently
>   based on an internal branch
> 
> Changes for v1:
> * Address review comments and add review tags from Moni
> * Split destination cache patch into 4 smaller patches
> * Added CONFIG_IPV6 check to destination cache patch
> * Added new NETDEV_CHANGE patch
> 
> Andrew Boyer (11):
>   IB/rxe: Move refcounting earlier in rxe_send()
>   IB/rxe: Disable completion upcalls when a CQ is destroyed
>   IB/rxe: Remove dangling prototype
>   IB/rxe: Fix up the responder's find_resources() function
>   IB/rxe: Fix destination cache for IPv6
>   IB/rxe: Add dst_clone() in prepare_ipv6_hdr()
>   IB/rxe: Fix up rxe_qp_cleanup()
>   IB/rxe: Remove unneeded initialization in prepare6()
>   IB/rxe: Another fix for broken receive queue draining
>   IB/rxe: Avoid ICRC errors by copying into the skb first
>   IB/rxe: Handle NETDEV_CHANGE events

Thanks, series applied.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/11] IB/rxe: Bug fixes

[Yuval Shaia]

On Mon, Aug 28, 2017 at 04:11:48PM -0400, Andrew Boyer wrote:
> This patch set addresses a few issues which we have found in RXE.
> The first patch fixes a serious issue that causes kernel panics during
> network congestion.
> 
> Changes for v2:
> * Update NETDEV_CHANGE patch so it builds; original was inadvertently
>   based on an internal branch

For v2:

Tested-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

> 
> Changes for v1:
> * Address review comments and add review tags from Moni
> * Split destination cache patch into 4 smaller patches
> * Added CONFIG_IPV6 check to destination cache patch
> * Added new NETDEV_CHANGE patch
> 
> Andrew Boyer (11):
>   IB/rxe: Move refcounting earlier in rxe_send()
>   IB/rxe: Disable completion upcalls when a CQ is destroyed
>   IB/rxe: Remove dangling prototype
>   IB/rxe: Fix up the responder's find_resources() function
>   IB/rxe: Fix destination cache for IPv6
>   IB/rxe: Add dst_clone() in prepare_ipv6_hdr()
>   IB/rxe: Fix up rxe_qp_cleanup()
>   IB/rxe: Remove unneeded initialization in prepare6()
>   IB/rxe: Another fix for broken receive queue draining
>   IB/rxe: Avoid ICRC errors by copying into the skb first
>   IB/rxe: Handle NETDEV_CHANGE events
> 
>  drivers/infiniband/sw/rxe/rxe_cq.c    | 19 +++++++++++++++++++
>  drivers/infiniband/sw/rxe/rxe_loc.h   |  4 ++--
>  drivers/infiniband/sw/rxe/rxe_mr.c    | 12 ++++++------
>  drivers/infiniband/sw/rxe/rxe_net.c   | 26 +++++++++++++++++++-------
>  drivers/infiniband/sw/rxe/rxe_qp.c    |  9 ++-------
>  drivers/infiniband/sw/rxe/rxe_req.c   |  4 +++-
>  drivers/infiniband/sw/rxe/rxe_resp.c  |  2 +-
>  drivers/infiniband/sw/rxe/rxe_verbs.c |  2 ++
>  drivers/infiniband/sw/rxe/rxe_verbs.h |  2 ++
>  9 files changed, 56 insertions(+), 24 deletions(-)
> 
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html