[PATCH rdma-rc V1 0/4] RXE fixes for 4.8

[PATCH rdma-rc V1 0/4] RXE fixes for 4.8

[Leon Romanovsky]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Hi Doug,

Please find the following four patches from Yonatan which
fix numerous issues in RXE code.

Thanks

Changelog v0->v1:
 * Patch 1:
   * removed second call to register_netdevice_notifier()
   * fix indentation issues
   * separated rxe_net_init() into two functions

Yonatan Cohen (4):
  IB/rxe: Fix kernel panic in udp_setup_tunnel
  IB/rxe: Fix duplicate atomic request handling
  IB/rxe: Fix race condition between requester and completer
  IB/rxe: Fix kmem_cache leak

 drivers/infiniband/sw/rxe/rxe.c      | 25 ++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_comp.c | 13 ++++++++
 drivers/infiniband/sw/rxe/rxe_net.c  | 55 ++++++++++++++++------------------
 drivers/infiniband/sw/rxe/rxe_net.h  |  5 +++-
 drivers/infiniband/sw/rxe/rxe_req.c  | 57 ++++++++++++++++++++++++++++--------
 drivers/infiniband/sw/rxe/rxe_resp.c | 11 +++----
 6 files changed, 114 insertions(+), 52 deletions(-)

--
2.7.4

--
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 rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

[Leon Romanovsky]

From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Disable creation of a UDP socket for ipv6 when
CONFIG_IPV6 is not enabeld. Since udp_sock_create6()
returns 0 when CONFIG_IPV6 is not set

[   46.888632] IP: [<c220705a>] setup_udp_tunnel_sock+0x6/0x4f
[   46.891355] *pdpt = 0000000000000000 *pde = f000ff53f000ff53
[   46.893918] Oops: 0002 [#1] PREEMPT
[   46.896014] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc4-00001-g8700e3e #1
[   46.900280] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
[   46.904905] task: cf06c040 ti: cf05e000 task.ti: cf05e000
[   46.907854] EIP: 0060:[<c220705a>] EFLAGS: 00210246 CPU: 0
[   46.911137] EIP is at setup_udp_tunnel_sock+0x6/0x4f
[   46.914070] EAX: 00000044 EBX: 00000001 ECX: cf05fef0 EDX: ca8142e0
[   46.917236] ESI: c2c4505b EDI: cf05fef0 EBP: cf05fed0 ESP: cf05fed0
[   46.919836]  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
[   46.922046] CR0: 80050033 CR2: 000001fc CR3: 02cec000 CR4: 000006b0
[   46.924550] Stack:
[   46.926014]  cf05ff10 c1fd4657 ca8142e0 0000000a 00000000 00000000 0000b712 00000008
[   46.931274]  00000000 6bb5bd01 c1fd48de 00000000 00000000 cf05ff1c 00000000 00000000
[   46.936122]  cf05ff1c c1fd4bdf 00000000 cf05ff28 c2c4507b ffffffff cf05ff88 c2bf1c74
[   46.942350] Call Trace:
[   46.944403]  [<c1fd4657>] rxe_setup_udp_tunnel+0x8f/0x99
[   46.947689]  [<c1fd48de>] ? net_to_rxe+0x4e/0x4e
[   46.950567]  [<c1fd4bdf>] rxe_net_init+0xe/0xa4
[   46.953147]  [<c2c4507b>] rxe_module_init+0x20/0x4c
[   46.955448]  [<c2bf1c74>] do_one_initcall+0x89/0x113
[   46.957797]  [<c2bf15eb>] ? set_debug_rodata+0xf/0xf
[   46.959966]  [<c2bf1dbc>] ? kernel_init_freeable+0xbe/0x15b
[   46.962262]  [<c2bf1ddc>] kernel_init_freeable+0xde/0x15b
[   46.964418]  [<c232eb54>] kernel_init+0x8/0xd0
[   46.966618]  [<c2333122>] ret_from_kernel_thread+0xe/0x24
[   46.969592]  [<c232eb4c>] ? rest_init+0x6f/0x6f

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe.c     | 25 +++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_net.c | 55 +++++++++++++++++--------------------
 drivers/infiniband/sw/rxe/rxe_net.h |  5 +++-
 3 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index 55f0e8f..ddd5927 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -362,15 +362,34 @@ static int __init rxe_module_init(void)
 		return err;
 	}

-	err = rxe_net_init();
+	err = rxe_net_ipv4_init();
 	if (err) {
-		pr_err("rxe: unable to init\n");
+		pr_err("rxe: unable to init ipv4 tunnel\n");
 		rxe_cache_exit();
-		return err;
+		goto exit;
+	}
+
+	err = rxe_net_ipv6_init();
+	if (err) {
+		pr_err("rxe: unable to init ipv6 tunnel\n");
+		rxe_cache_exit();
+		goto exit;
 	}
+
+	err = register_netdevice_notifier(&rxe_net_notifier);
+	if (err) {
+		pr_err("rxe: Failed to rigister netdev notifier\n");
+		goto exit;
+	}
+
 	pr_info("rxe: loaded\n");

 	return 0;
+
+exit:
+	rxe_release_udp_tunnel(recv_sockets.sk4);
+	rxe_release_udp_tunnel(recv_sockets.sk6);
+	return err;
 }

 static void __exit rxe_module_exit(void)
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 0b8d2ea..eedf2f1 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -275,9 +275,10 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
 	return sock;
 }

-static void rxe_release_udp_tunnel(struct socket *sk)
+void rxe_release_udp_tunnel(struct socket *sk)
 {
-	udp_tunnel_sock_release(sk);
+	if (sk)
+		udp_tunnel_sock_release(sk);
 }

 static void prepare_udp_hdr(struct sk_buff *skb, __be16 src_port,
@@ -658,51 +659,45 @@ out:
 	return NOTIFY_OK;
 }

-static struct notifier_block rxe_net_notifier = {
+struct notifier_block rxe_net_notifier = {
 	.notifier_call = rxe_notify,
 };

-int rxe_net_init(void)
+int rxe_net_ipv4_init(void)
 {
-	int err;
-
 	spin_lock_init(&dev_list_lock);

-	recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
-			htons(ROCE_V2_UDP_DPORT), true);
-	if (IS_ERR(recv_sockets.sk6)) {
-		recv_sockets.sk6 = NULL;
-		pr_err("rxe: Failed to create IPv6 UDP tunnel\n");
-		return -1;
-	}
-
 	recv_sockets.sk4 = rxe_setup_udp_tunnel(&init_net,
-			htons(ROCE_V2_UDP_DPORT), false);
+				htons(ROCE_V2_UDP_DPORT), false);
 	if (IS_ERR(recv_sockets.sk4)) {
-		rxe_release_udp_tunnel(recv_sockets.sk6);
 		recv_sockets.sk4 = NULL;
-		recv_sockets.sk6 = NULL;
 		pr_err("rxe: Failed to create IPv4 UDP tunnel\n");
 		return -1;
 	}

-	err = register_netdevice_notifier(&rxe_net_notifier);
-	if (err) {
-		rxe_release_udp_tunnel(recv_sockets.sk6);
-		rxe_release_udp_tunnel(recv_sockets.sk4);
-		pr_err("rxe: Failed to rigister netdev notifier\n");
-	}
-
-	return err;
+	return 0;
 }

-void rxe_net_exit(void)
+int rxe_net_ipv6_init(void)
 {
-	if (recv_sockets.sk6)
-		rxe_release_udp_tunnel(recv_sockets.sk6);
+#if IS_ENABLED(CONFIG_IPV6)

-	if (recv_sockets.sk4)
-		rxe_release_udp_tunnel(recv_sockets.sk4);
+	spin_lock_init(&dev_list_lock);

+	recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
+						htons(ROCE_V2_UDP_DPORT), true);
+	if (IS_ERR(recv_sockets.sk6)) {
+		recv_sockets.sk6 = NULL;
+		pr_err("rxe: Failed to create IPv6 UDP tunnel\n");
+		return -1;
+	}
+#endif
+	return 0;
+}
+
+void rxe_net_exit(void)
+{
+	rxe_release_udp_tunnel(recv_sockets.sk6);
+	rxe_release_udp_tunnel(recv_sockets.sk4);
 	unregister_netdevice_notifier(&rxe_net_notifier);
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h
index 7b06f76..0daf7f0 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.h
+++ b/drivers/infiniband/sw/rxe/rxe_net.h
@@ -44,10 +44,13 @@ struct rxe_recv_sockets {
 };

 extern struct rxe_recv_sockets recv_sockets;
+extern struct notifier_block rxe_net_notifier;
+void rxe_release_udp_tunnel(struct socket *sk);

 struct rxe_dev *rxe_net_add(struct net_device *ndev);

-int rxe_net_init(void);
+int rxe_net_ipv4_init(void);
+int rxe_net_ipv6_init(void);
 void rxe_net_exit(void);

 #endif /* RXE_NET_H */
--
2.7.4

--
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 rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

[Or Gerlitz]

On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Disable creation of a UDP socket for ipv6 when
> CONFIG_IPV6 is not enabeld. Since udp_sock_create6()
> returns 0 when CONFIG_IPV6 is not set
>
> [   46.888632] IP: [<c220705a>] setup_udp_tunnel_sock+0x6/0x4f
> [   46.891355] *pdpt = 0000000000000000 *pde = f000ff53f000ff53
> [   46.893918] Oops: 0002 [#1] PREEMPT
> [   46.896014] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc4-00001-g8700e3e #1
> [   46.900280] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
> [   46.904905] task: cf06c040 ti: cf05e000 task.ti: cf05e000
> [   46.907854] EIP: 0060:[<c220705a>] EFLAGS: 00210246 CPU: 0
> [   46.911137] EIP is at setup_udp_tunnel_sock+0x6/0x4f
> [   46.914070] EAX: 00000044 EBX: 00000001 ECX: cf05fef0 EDX: ca8142e0
> [   46.917236] ESI: c2c4505b EDI: cf05fef0 EBP: cf05fed0 ESP: cf05fed0
> [   46.919836]  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
> [   46.922046] CR0: 80050033 CR2: 000001fc CR3: 02cec000 CR4: 000006b0
> [   46.924550] Stack:
> [   46.926014]  cf05ff10 c1fd4657 ca8142e0 0000000a 00000000 00000000 0000b712 00000008
> [   46.931274]  00000000 6bb5bd01 c1fd48de 00000000 00000000 cf05ff1c 00000000 00000000
> [   46.936122]  cf05ff1c c1fd4bdf 00000000 cf05ff28 c2c4507b ffffffff cf05ff88 c2bf1c74
> [   46.942350] Call Trace:
> [   46.944403]  [<c1fd4657>] rxe_setup_udp_tunnel+0x8f/0x99
> [   46.947689]  [<c1fd48de>] ? net_to_rxe+0x4e/0x4e
> [   46.950567]  [<c1fd4bdf>] rxe_net_init+0xe/0xa4
> [   46.953147]  [<c2c4507b>] rxe_module_init+0x20/0x4c
> [   46.955448]  [<c2bf1c74>] do_one_initcall+0x89/0x113
> [   46.957797]  [<c2bf15eb>] ? set_debug_rodata+0xf/0xf
> [   46.959966]  [<c2bf1dbc>] ? kernel_init_freeable+0xbe/0x15b
> [   46.962262]  [<c2bf1ddc>] kernel_init_freeable+0xde/0x15b
> [   46.964418]  [<c232eb54>] kernel_init+0x8/0xd0
> [   46.966618]  [<c2333122>] ret_from_kernel_thread+0xe/0x24
> [   46.969592]  [<c232eb4c>] ? rest_init+0x6f/0x6f

what an unnice way to settle a review discussion. I replied to all
your arguments explaining
why the change log has to go, and the next step is to ignore this
discussion and do it again
wrong w.o replying? so... again:

$ ./scripts/get_maintainer.pl drivers/infiniband/sw/rxe
Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> (supporter:SOFT-ROCE DRIVER (rxe))
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> (supporter:INFINIBAND SUBSYSTEM)
Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> (supporter:INFINIBAND SUBSYSTEM)
Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> (supporter:INFINIBAND SUBSYSTEM)
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list:SOFT-ROCE DRIVER (rxe))
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list)

Moni, can you please put your maintainer say here.



Or.
--
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 rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

[Moni Shoua]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3481 bytes --]

Sorry Or but I might have missed your review comment regarding the commit message
I can't find it now so please send it again

Thanks


-----Original Message-----
From: Or Gerlitz [mailto:gerlitz.or@gmail.com] 
Sent: Wednesday, September 07, 2016 2:21 PM
To: Leon Romanovsky <leonro@mellanox.com>; Moni Shoua <monis@mellanox.com>
Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Yonatan Cohen <yonatanc@mellanox.com>
Subject: Re: [PATCH rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <leon@kernel.org> wrote:
> From: Yonatan Cohen <yonatanc@mellanox.com>
>
> Disable creation of a UDP socket for ipv6 when
> CONFIG_IPV6 is not enabeld. Since udp_sock_create6() returns 0 when 
> CONFIG_IPV6 is not set
>
> [   46.888632] IP: [<c220705a>] setup_udp_tunnel_sock+0x6/0x4f
> [   46.891355] *pdpt = 0000000000000000 *pde = f000ff53f000ff53
> [   46.893918] Oops: 0002 [#1] PREEMPT
> [   46.896014] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc4-00001-g8700e3e #1
> [   46.900280] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
> [   46.904905] task: cf06c040 ti: cf05e000 task.ti: cf05e000
> [   46.907854] EIP: 0060:[<c220705a>] EFLAGS: 00210246 CPU: 0
> [   46.911137] EIP is at setup_udp_tunnel_sock+0x6/0x4f
> [   46.914070] EAX: 00000044 EBX: 00000001 ECX: cf05fef0 EDX: ca8142e0
> [   46.917236] ESI: c2c4505b EDI: cf05fef0 EBP: cf05fed0 ESP: cf05fed0
> [   46.919836]  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
> [   46.922046] CR0: 80050033 CR2: 000001fc CR3: 02cec000 CR4: 000006b0
> [   46.924550] Stack:
> [   46.926014]  cf05ff10 c1fd4657 ca8142e0 0000000a 00000000 00000000 0000b712 00000008
> [   46.931274]  00000000 6bb5bd01 c1fd48de 00000000 00000000 cf05ff1c 00000000 00000000
> [   46.936122]  cf05ff1c c1fd4bdf 00000000 cf05ff28 c2c4507b ffffffff cf05ff88 c2bf1c74
> [   46.942350] Call Trace:
> [   46.944403]  [<c1fd4657>] rxe_setup_udp_tunnel+0x8f/0x99
> [   46.947689]  [<c1fd48de>] ? net_to_rxe+0x4e/0x4e
> [   46.950567]  [<c1fd4bdf>] rxe_net_init+0xe/0xa4
> [   46.953147]  [<c2c4507b>] rxe_module_init+0x20/0x4c
> [   46.955448]  [<c2bf1c74>] do_one_initcall+0x89/0x113
> [   46.957797]  [<c2bf15eb>] ? set_debug_rodata+0xf/0xf
> [   46.959966]  [<c2bf1dbc>] ? kernel_init_freeable+0xbe/0x15b
> [   46.962262]  [<c2bf1ddc>] kernel_init_freeable+0xde/0x15b
> [   46.964418]  [<c232eb54>] kernel_init+0x8/0xd0
> [   46.966618]  [<c2333122>] ret_from_kernel_thread+0xe/0x24
> [   46.969592]  [<c232eb4c>] ? rest_init+0x6f/0x6f

what an unnice way to settle a review discussion. I replied to all your arguments explaining why the change log has to go, and the next step is to ignore this discussion and do it again wrong w.o replying? so... again:

$ ./scripts/get_maintainer.pl drivers/infiniband/sw/rxe Moni Shoua <monis@mellanox.com> (supporter:SOFT-ROCE DRIVER (rxe)) Doug Ledford <dledford@redhat.com> (supporter:INFINIBAND SUBSYSTEM) Sean Hefty <sean.hefty@intel.com> (supporter:INFINIBAND SUBSYSTEM) Hal Rosenstock <hal.rosenstock@gmail.com> (supporter:INFINIBAND SUBSYSTEM) linux-rdma@vger.kernel.org (open list:SOFT-ROCE DRIVER (rxe)) linux-kernel@vger.kernel.org (open list)

Moni, can you please put your maintainer say here.



Or.
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

[Or Gerlitz]

On Wed, Sep 7, 2016 at 2:39 PM, Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> I might have missed your review comment regarding the commit message
> I can't find it now so please send it again

http://marc.info/?t=147307741300005&r=1&w=2
--
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 rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

[Leon Romanovsky]

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

On Wed, Sep 07, 2016 at 02:20:50PM +0300, Or Gerlitz wrote:
> On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Disable creation of a UDP socket for ipv6 when
> > CONFIG_IPV6 is not enabeld. Since udp_sock_create6()
> > returns 0 when CONFIG_IPV6 is not set
> >
> > [   46.888632] IP: [<c220705a>] setup_udp_tunnel_sock+0x6/0x4f
> > [   46.891355] *pdpt = 0000000000000000 *pde = f000ff53f000ff53
> > [   46.893918] Oops: 0002 [#1] PREEMPT
> > [   46.896014] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc4-00001-g8700e3e #1
> > [   46.900280] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
> > [   46.904905] task: cf06c040 ti: cf05e000 task.ti: cf05e000
> > [   46.907854] EIP: 0060:[<c220705a>] EFLAGS: 00210246 CPU: 0
> > [   46.911137] EIP is at setup_udp_tunnel_sock+0x6/0x4f
> > [   46.914070] EAX: 00000044 EBX: 00000001 ECX: cf05fef0 EDX: ca8142e0
> > [   46.917236] ESI: c2c4505b EDI: cf05fef0 EBP: cf05fed0 ESP: cf05fed0
> > [   46.919836]  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
> > [   46.922046] CR0: 80050033 CR2: 000001fc CR3: 02cec000 CR4: 000006b0
> > [   46.924550] Stack:
> > [   46.926014]  cf05ff10 c1fd4657 ca8142e0 0000000a 00000000 00000000 0000b712 00000008
> > [   46.931274]  00000000 6bb5bd01 c1fd48de 00000000 00000000 cf05ff1c 00000000 00000000
> > [   46.936122]  cf05ff1c c1fd4bdf 00000000 cf05ff28 c2c4507b ffffffff cf05ff88 c2bf1c74
> > [   46.942350] Call Trace:
> > [   46.944403]  [<c1fd4657>] rxe_setup_udp_tunnel+0x8f/0x99
> > [   46.947689]  [<c1fd48de>] ? net_to_rxe+0x4e/0x4e
> > [   46.950567]  [<c1fd4bdf>] rxe_net_init+0xe/0xa4
> > [   46.953147]  [<c2c4507b>] rxe_module_init+0x20/0x4c
> > [   46.955448]  [<c2bf1c74>] do_one_initcall+0x89/0x113
> > [   46.957797]  [<c2bf15eb>] ? set_debug_rodata+0xf/0xf
> > [   46.959966]  [<c2bf1dbc>] ? kernel_init_freeable+0xbe/0x15b
> > [   46.962262]  [<c2bf1ddc>] kernel_init_freeable+0xde/0x15b
> > [   46.964418]  [<c232eb54>] kernel_init+0x8/0xd0
> > [   46.966618]  [<c2333122>] ret_from_kernel_thread+0xe/0x24
> > [   46.969592]  [<c232eb4c>] ? rest_init+0x6f/0x6f
>
> what an unnice way to settle a review discussion. I replied to all
> your arguments explaining
> why the change log has to go, and the next step is to ignore this
> discussion and do it again
> wrong w.o replying? so... again:

Or,

You didn't like the format of commit message, while I liked - that's
all, it is the end of discussion.

We are not in debate club here and while commit messages are
important, the code and bug fixes are much more.

If you think that kernel oops doesn't belong to commit message,
please fix SubmittingPatches [1] guide and don't forget to announce
it and enforce it in various subsystems.

Do you think that there is something in the code which needs to be
addressed before Doug's take this patchset and releases RXE with
less unaddressed issues?

>
> $ ./scripts/get_maintainer.pl drivers/infiniband/sw/rxe
> Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> (supporter:SOFT-ROCE DRIVER (rxe))
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> (supporter:INFINIBAND SUBSYSTEM)
> Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> (supporter:INFINIBAND SUBSYSTEM)
> Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> (supporter:INFINIBAND SUBSYSTEM)
> linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list:SOFT-ROCE DRIVER (rxe))
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list)
>
> Moni, can you please put your maintainer say here.

You are sitting next to him and you can freely ask him if he reviewed
it. The answer will be YES. Moni reviewed first version and I reviewed
the second one.

[1] http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L691

>
>
>
> Or.
> --
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

[Or Gerlitz]

On Wed, Sep 7, 2016 at 3:13 PM, Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> On Wed, Sep 07, 2016 at 02:20:50PM +0300, Or Gerlitz wrote:
>> On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >
>> > Disable creation of a UDP socket for ipv6 when
>> > CONFIG_IPV6 is not enabeld. Since udp_sock_create6()
>> > returns 0 when CONFIG_IPV6 is not set
>> >
>> > [   46.888632] IP: [<c220705a>] setup_udp_tunnel_sock+0x6/0x4f
>> > [   46.891355] *pdpt = 0000000000000000 *pde = f000ff53f000ff53
>> > [   46.893918] Oops: 0002 [#1] PREEMPT
>> > [   46.896014] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc4-00001-g8700e3e #1
>> > [   46.900280] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
>> > [   46.904905] task: cf06c040 ti: cf05e000 task.ti: cf05e000
>> > [   46.907854] EIP: 0060:[<c220705a>] EFLAGS: 00210246 CPU: 0
>> > [   46.911137] EIP is at setup_udp_tunnel_sock+0x6/0x4f
>> > [   46.914070] EAX: 00000044 EBX: 00000001 ECX: cf05fef0 EDX: ca8142e0
>> > [   46.917236] ESI: c2c4505b EDI: cf05fef0 EBP: cf05fed0 ESP: cf05fed0
>> > [   46.919836]  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
>> > [   46.922046] CR0: 80050033 CR2: 000001fc CR3: 02cec000 CR4: 000006b0
>> > [   46.924550] Stack:
>> > [   46.926014]  cf05ff10 c1fd4657 ca8142e0 0000000a 00000000 00000000 0000b712 00000008
>> > [   46.931274]  00000000 6bb5bd01 c1fd48de 00000000 00000000 cf05ff1c 00000000 00000000
>> > [   46.936122]  cf05ff1c c1fd4bdf 00000000 cf05ff28 c2c4507b ffffffff cf05ff88 c2bf1c74
>> > [   46.942350] Call Trace:
>> > [   46.944403]  [<c1fd4657>] rxe_setup_udp_tunnel+0x8f/0x99
>> > [   46.947689]  [<c1fd48de>] ? net_to_rxe+0x4e/0x4e
>> > [   46.950567]  [<c1fd4bdf>] rxe_net_init+0xe/0xa4
>> > [   46.953147]  [<c2c4507b>] rxe_module_init+0x20/0x4c
>> > [   46.955448]  [<c2bf1c74>] do_one_initcall+0x89/0x113
>> > [   46.957797]  [<c2bf15eb>] ? set_debug_rodata+0xf/0xf
>> > [   46.959966]  [<c2bf1dbc>] ? kernel_init_freeable+0xbe/0x15b
>> > [   46.962262]  [<c2bf1ddc>] kernel_init_freeable+0xde/0x15b
>> > [   46.964418]  [<c232eb54>] kernel_init+0x8/0xd0
>> > [   46.966618]  [<c2333122>] ret_from_kernel_thread+0xe/0x24
>> > [   46.969592]  [<c232eb4c>] ? rest_init+0x6f/0x6f
>>
>> what an unnice way to settle a review discussion. I replied to all
>> your arguments explaining
>> why the change log has to go, and the next step is to ignore this
>> discussion and do it again
>> wrong w.o replying? so... again:
>
> Or,
>
> You didn't like the format of commit message, while I liked - that's
> all, it is the end of discussion.

I made **concrete** **arguments** replying to yours why to my opinion
the oops stack listing doesn't help anyone and hence doesn't belong to
the kernel logs and need not be there forever. This is not liking thing, this
is technical talking thing.


> We are not in debate club here and while commit messages are
> important, the code and bug fixes are much more.

so strip this stupid stack trace which helps no one and set the patch free.

> Do you think that there is something in the code which needs to be
> addressed before Doug's take this patchset and releases RXE with
> less unaddressed issues?

yes, see my comments to the rest of the patches of the series, some
small fixes are needed.


>> Moni, can you please put your maintainer say here.

> You are sitting next to him and you can freely ask him if he reviewed it.

Your assumption is (1) wrong and (2) irrelevant to this discussion

> The answer will be YES. Moni reviewed first version and I reviewed
> the second one.

There's a reviewer discussion on a patch for a driver he (and not you)
maintain,
he has to put his maintainer say.

Or.
--
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 rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

[Doug Ledford]

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

On 9/7/2016 9:19 AM, Or Gerlitz wrote:
> On Wed, Sep 7, 2016 at 3:13 PM, Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> On Wed, Sep 07, 2016 at 02:20:50PM +0300, Or Gerlitz wrote:
>>> On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>> From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>>
>>>> Disable creation of a UDP socket for ipv6 when
>>>> CONFIG_IPV6 is not enabeld. Since udp_sock_create6()
>>>> returns 0 when CONFIG_IPV6 is not set
>>>>
>>>> [   46.888632] IP: [<c220705a>] setup_udp_tunnel_sock+0x6/0x4f
>>>> [   46.891355] *pdpt = 0000000000000000 *pde = f000ff53f000ff53
>>>> [   46.893918] Oops: 0002 [#1] PREEMPT
>>>> [   46.896014] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc4-00001-g8700e3e #1
>>>> [   46.900280] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
>>>> [   46.904905] task: cf06c040 ti: cf05e000 task.ti: cf05e000
>>>> [   46.907854] EIP: 0060:[<c220705a>] EFLAGS: 00210246 CPU: 0
>>>> [   46.911137] EIP is at setup_udp_tunnel_sock+0x6/0x4f
>>>> [   46.914070] EAX: 00000044 EBX: 00000001 ECX: cf05fef0 EDX: ca8142e0
>>>> [   46.917236] ESI: c2c4505b EDI: cf05fef0 EBP: cf05fed0 ESP: cf05fed0
>>>> [   46.919836]  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
>>>> [   46.922046] CR0: 80050033 CR2: 000001fc CR3: 02cec000 CR4: 000006b0
>>>> [   46.924550] Stack:
>>>> [   46.926014]  cf05ff10 c1fd4657 ca8142e0 0000000a 00000000 00000000 0000b712 00000008
>>>> [   46.931274]  00000000 6bb5bd01 c1fd48de 00000000 00000000 cf05ff1c 00000000 00000000
>>>> [   46.936122]  cf05ff1c c1fd4bdf 00000000 cf05ff28 c2c4507b ffffffff cf05ff88 c2bf1c74
>>>> [   46.942350] Call Trace:
>>>> [   46.944403]  [<c1fd4657>] rxe_setup_udp_tunnel+0x8f/0x99
>>>> [   46.947689]  [<c1fd48de>] ? net_to_rxe+0x4e/0x4e
>>>> [   46.950567]  [<c1fd4bdf>] rxe_net_init+0xe/0xa4
>>>> [   46.953147]  [<c2c4507b>] rxe_module_init+0x20/0x4c
>>>> [   46.955448]  [<c2bf1c74>] do_one_initcall+0x89/0x113
>>>> [   46.957797]  [<c2bf15eb>] ? set_debug_rodata+0xf/0xf
>>>> [   46.959966]  [<c2bf1dbc>] ? kernel_init_freeable+0xbe/0x15b
>>>> [   46.962262]  [<c2bf1ddc>] kernel_init_freeable+0xde/0x15b
>>>> [   46.964418]  [<c232eb54>] kernel_init+0x8/0xd0
>>>> [   46.966618]  [<c2333122>] ret_from_kernel_thread+0xe/0x24
>>>> [   46.969592]  [<c232eb4c>] ? rest_init+0x6f/0x6f
>>>
>>> what an unnice way to settle a review discussion. I replied to all
>>> your arguments explaining
>>> why the change log has to go, and the next step is to ignore this
>>> discussion and do it again
>>> wrong w.o replying? so... again:
>>
>> Or,
>>
>> You didn't like the format of commit message, while I liked - that's
>> all, it is the end of discussion.
> 
> I made **concrete** **arguments** replying to yours why to my opinion
> the oops stack listing doesn't help anyone

Your arguments were wrong though.  I myself have gone flipping through
git commit messages looking for a fix for a specific issue.  Having at
least the signature call trace in the commit message is very useful to
someone in that position.  *I* want the trace in the commit message.  I
may trim it down to just the relevant signature backtrace, but I would
still like it there.  So let's please drop this and move on.

 and hence doesn't belong to
> the kernel logs and need not be there forever. This is not liking thing, this
> is technical talking thing.
> 
> 
>> We are not in debate club here and while commit messages are
>> important, the code and bug fixes are much more.
> 
> so strip this stupid stack trace which helps no one and set the patch free.
> 
>> Do you think that there is something in the code which needs to be
>> addressed before Doug's take this patchset and releases RXE with
>> less unaddressed issues?
> 
> yes, see my comments to the rest of the patches of the series, some
> small fixes are needed.
> 
> 
>>> Moni, can you please put your maintainer say here.
> 
>> You are sitting next to him and you can freely ask him if he reviewed it.
> 
> Your assumption is (1) wrong and (2) irrelevant to this discussion
> 
>> The answer will be YES. Moni reviewed first version and I reviewed
>> the second one.
> 
> There's a reviewer discussion on a patch for a driver he (and not you)
> maintain,
> he has to put his maintainer say.
> 
> Or.
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: 0E572FDD


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

Re: [PATCH rdma-rc V1 1/4] IB/rxe: Fix kernel panic in udp_setup_tunnel

[Moni Shoua]

> Moni, can you please put your maintainer say here.
>

I see that panic messages are common in change logs and personally I
think it adds to info about the necessity of the patch.
Therefore I tend to agree with Leon.
--
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 rdma-rc V1 2/4] IB/rxe: Fix duplicate atomic request handling

[Leon Romanovsky]

From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

When handling ack for atomic opcodes like "fetch&add"
or "cmp&swp", the method send_atomic_ack() saves the ack
before sending it, in case it gets lost and never reach the
requester. In which case the method duplicate_request()
will need to find it using the duplicated request.psn.
But send_atomic_ack() used a wrong psn value and thus
the above ack was never found.
This fix uses the ack.psn to locate the ack in case
its needed.
This fix also copies the ack packet to the skb's control buffer
since duplicate_request() will need it when calling rxe_xmit_packet()

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index ebb03b4..3e0f0f2 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -972,11 +972,13 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 	free_rd_atomic_resource(qp, res);
 	rxe_advance_resp_resource(qp);

+	memcpy(SKB_TO_PKT(skb), &ack_pkt, sizeof(skb->cb));
+
 	res->type = RXE_ATOMIC_MASK;
 	res->atomic.skb = skb;
-	res->first_psn = qp->resp.psn;
-	res->last_psn = qp->resp.psn;
-	res->cur_psn = qp->resp.psn;
+	res->first_psn = ack_pkt.psn;
+	res->last_psn  = ack_pkt.psn;
+	res->cur_psn   = ack_pkt.psn;

 	rc = rxe_xmit_packet(rxe, qp, &ack_pkt, skb_copy);
 	if (rc) {
@@ -1116,8 +1118,7 @@ static enum resp_states duplicate_request(struct rxe_qp *qp,
 				rc = RESPST_CLEANUP;
 				goto out;
 			}
-			bth_set_psn(SKB_TO_PKT(skb_copy),
-				    qp->resp.psn - 1);
+
 			/* Resend the result. */
 			rc = rxe_xmit_packet(to_rdev(qp->ibqp.device), qp,
 					     pkt, skb_copy);
--
2.7.4

--
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 rdma-rc V1 2/4] IB/rxe: Fix duplicate atomic request handling

[Or Gerlitz]

On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> When handling ack for atomic opcodes like "fetch&add"
> or "cmp&swp", the method send_atomic_ack() saves the ack
> before sending it, in case it gets lost and never reach the
> requester. In which case the method duplicate_request()
> will need to find it using the duplicated request.psn.
> But send_atomic_ack() used a wrong psn value and thus
> the above ack was never found.
> This fix uses the ack.psn to locate the ack in case
> its needed.
> This fix also copies the ack packet to the skb's control buffer
> since duplicate_request() will need it when calling rxe_xmit_packet()

Are you using tiny screen? any special  reason to limit the change-log lines
to be ~55 chars long and not 70 or 80?
--
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 rdma-rc V1 3/4] IB/rxe: Fix race condition between requester and completer

[Leon Romanovsky]

From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

rxe_requester() is sending a pkt with rxe_xmit_packet() and
then calls rxe_update() to update the wqe and qp's psn values.
But sometimes the response is received before the requester
had time to update the wqe in which case the completer
acts on errornous wqe values.
This fix updates the wqe and qp before actually sending
the request and rolls back when xmit fails.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 57 ++++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 33b2d9d..13a848a 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -511,24 +511,21 @@ static int fill_packet(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 }

 static void update_wqe_state(struct rxe_qp *qp,
-			     struct rxe_send_wqe *wqe,
-			     struct rxe_pkt_info *pkt,
-			     enum wqe_state *prev_state)
+		struct rxe_send_wqe *wqe,
+		struct rxe_pkt_info *pkt)
 {
-	enum wqe_state prev_state_ = wqe->state;
-
 	if (pkt->mask & RXE_END_MASK) {
 		if (qp_type(qp) == IB_QPT_RC)
 			wqe->state = wqe_state_pending;
 	} else {
 		wqe->state = wqe_state_processing;
 	}
-
-	*prev_state = prev_state_;
 }

-static void update_state(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
-			 struct rxe_pkt_info *pkt, int payload)
+static void update_wqe_psn(struct rxe_qp *qp,
+			   struct rxe_send_wqe *wqe,
+			   struct rxe_pkt_info *pkt,
+			   int payload)
 {
 	/* number of packets left to send including current one */
 	int num_pkt = (wqe->dma.resid + payload + qp->mtu - 1) / qp->mtu;
@@ -546,9 +543,34 @@ static void update_state(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 		qp->req.psn = (wqe->first_psn + num_pkt) & BTH_PSN_MASK;
 	else
 		qp->req.psn = (qp->req.psn + 1) & BTH_PSN_MASK;
+}

-	qp->req.opcode = pkt->opcode;
+static void save_state(struct rxe_send_wqe *wqe,
+		       struct rxe_qp *qp,
+		       struct rxe_send_wqe *rollback_wqe,
+		       struct rxe_qp *rollback_qp)
+{
+	rollback_wqe->state     = wqe->state;
+	rollback_wqe->first_psn = wqe->first_psn;
+	rollback_wqe->last_psn  = wqe->last_psn;
+	rollback_qp->req.psn    = qp->req.psn;
+}

+static void rollback_state(struct rxe_send_wqe *wqe,
+			   struct rxe_qp *qp,
+			   struct rxe_send_wqe *rollback_wqe,
+			   struct rxe_qp *rollback_qp)
+{
+	wqe->state     = rollback_wqe->state;
+	wqe->first_psn = rollback_wqe->first_psn;
+	wqe->last_psn  = rollback_wqe->last_psn;
+	qp->req.psn    = rollback_qp->req.psn;
+}
+
+static void update_state(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
+			 struct rxe_pkt_info *pkt, int payload)
+{
+	qp->req.opcode = pkt->opcode;

 	if (pkt->mask & RXE_END_MASK)
 		qp->req.wqe_index = next_index(qp->sq.queue, qp->req.wqe_index);
@@ -571,7 +593,8 @@ int rxe_requester(void *arg)
 	int mtu;
 	int opcode;
 	int ret;
-	enum wqe_state prev_state;
+	struct rxe_qp rollback_qp;
+	struct rxe_send_wqe rollback_wqe;

 next_wqe:
 	if (unlikely(!qp->valid || qp->req.state == QP_STATE_ERROR))
@@ -688,13 +711,21 @@ next_wqe:
 		goto err;
 	}

-	update_wqe_state(qp, wqe, &pkt, &prev_state);
+	/*
+	 * To prevent a race on wqe access between requester and completer,
+	 * wqe members state and psn need to be set before calling
+	 * rxe_xmit_packet().
+	 * Otherwise, completer might initiate an unjustified retry flow.
+	 */
+	save_state(wqe, qp, &rollback_wqe, &rollback_qp);
+	update_wqe_state(qp, wqe, &pkt);
+	update_wqe_psn(qp, wqe, &pkt, payload);
 	ret = rxe_xmit_packet(to_rdev(qp->ibqp.device), qp, &pkt, skb);
 	if (ret) {
 		qp->need_req_skb = 1;
 		kfree_skb(skb);

-		wqe->state = prev_state;
+		rollback_state(wqe, qp, &rollback_wqe, &rollback_qp);

 		if (ret == -EAGAIN) {
 			rxe_run_task(&qp->req.task, 1);
--
2.7.4

--
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 rdma-rc V1 3/4] IB/rxe: Fix race condition between requester and completer

[Or Gerlitz]

On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> rxe_requester() is sending a pkt with rxe_xmit_packet() and

pkt --> packet will make this much more readable
--
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 rdma-rc V1 4/4] IB/rxe: Fix kmem_cache leak

[Leon Romanovsky]

From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Decrement qp reference when handling error path
in completer to prevent kmem_cache leak.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_comp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 36f67de..1c59ef2 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -689,7 +689,14 @@ int rxe_completer(void *arg)
 					qp->req.need_retry = 1;
 					rxe_run_task(&qp->req.task, 1);
 				}
+
+				if (pkt) {
+					rxe_drop_ref(pkt->qp);
+					kfree_skb(skb);
+				}
+
 				goto exit;
+
 			} else {
 				wqe->status = IB_WC_RETRY_EXC_ERR;
 				state = COMPST_ERROR;
@@ -716,6 +723,12 @@ int rxe_completer(void *arg)
 		case COMPST_ERROR:
 			do_complete(qp, wqe);
 			rxe_qp_error(qp);
+
+			if (pkt) {
+				rxe_drop_ref(pkt->qp);
+				kfree_skb(skb);
+			}
+
 			goto exit;
 		}
 	}
--
2.7.4

--
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 rdma-rc V1 4/4] IB/rxe: Fix kmem_cache leak

[Or Gerlitz]

On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Decrement qp reference when handling error path
> in completer to prevent kmem_cache leak.

"error path in completer" is pretty much cryptic, please user higher
language (completing what)
--
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 rdma-rc V1 0/4] RXE fixes for 4.8

[Doug Ledford]

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

On 9/7/2016 7:04 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Hi Doug,
> 
> Please find the following four patches from Yonatan which
> fix numerous issues in RXE code.
> 
> Thanks
> 
> Changelog v0->v1:
>  * Patch 1:
>    * removed second call to register_netdevice_notifier()
>    * fix indentation issues
>    * separated rxe_net_init() into two functions
> 
> Yonatan Cohen (4):
>   IB/rxe: Fix kernel panic in udp_setup_tunnel
>   IB/rxe: Fix duplicate atomic request handling
>   IB/rxe: Fix race condition between requester and completer
>   IB/rxe: Fix kmem_cache leak
> 
>  drivers/infiniband/sw/rxe/rxe.c      | 25 ++++++++++++++--
>  drivers/infiniband/sw/rxe/rxe_comp.c | 13 ++++++++
>  drivers/infiniband/sw/rxe/rxe_net.c  | 55 ++++++++++++++++------------------
>  drivers/infiniband/sw/rxe/rxe_net.h  |  5 +++-
>  drivers/infiniband/sw/rxe/rxe_req.c  | 57 ++++++++++++++++++++++++++++--------
>  drivers/infiniband/sw/rxe/rxe_resp.c | 11 +++----
>  6 files changed, 114 insertions(+), 52 deletions(-)
> 
> --
> 2.7.4
> 

Thanks, series applied.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: 0E572FDD


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