diff mbox

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

Message ID 1473077359-11546-2-git-send-email-leon@kernel.org (mailing list archive)
State Superseded
Headers show

Commit Message

Leon Romanovsky Sept. 5, 2016, 12:09 p.m. UTC
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

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Yonatan Cohen <yonatanc@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
---
 drivers/infiniband/sw/rxe/rxe.c     | 14 ++++++++++--
 drivers/infiniband/sw/rxe/rxe_net.c | 45 ++++++++++++++++++++++++-------------
 drivers/infiniband/sw/rxe/rxe_net.h |  3 ++-
 3 files changed, 43 insertions(+), 19 deletions(-)

Comments

Yuval Shaia Sept. 5, 2016, 1:24 p.m. UTC | #1
On Mon, Sep 05, 2016 at 03:09:16PM +0300, Leon Romanovsky 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
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Yonatan Cohen <yonatanc@mellanox.com>
> Signed-off-by: Leon Romanovsky <leon@kernel.org>
> ---
>  drivers/infiniband/sw/rxe/rxe.c     | 14 ++++++++++--
>  drivers/infiniband/sw/rxe/rxe_net.c | 45 ++++++++++++++++++++++++-------------
>  drivers/infiniband/sw/rxe/rxe_net.h |  3 ++-
>  3 files changed, 43 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
> index 55f0e8f..08e09d9 100644
> --- a/drivers/infiniband/sw/rxe/rxe.c
> +++ b/drivers/infiniband/sw/rxe/rxe.c
> @@ -362,12 +362,22 @@ 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;
>  	}
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +	err = rxe_net_ipv6_init();
> +	if (err) {
> +		pr_err("rxe: unable to init ipv6 tunnel\n");
> +		rxe_cache_exit();
> +		return err;
> +	}
> +#endif
> +
>  	pr_info("rxe: loaded\n");
>  
>  	return 0;
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 0b8d2ea..e0866b4 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -249,8 +249,8 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
>  	memset(&udp_cfg, 0, sizeof(udp_cfg));
>  
>  	if (ipv6) {
> -		udp_cfg.family = AF_INET6;
> -		udp_cfg.ipv6_v6only = 1;
> +	udp_cfg.family = AF_INET6;
> +	udp_cfg.ipv6_v6only = 1;
>  	} else {
>  		udp_cfg.family = AF_INET;
>  	}
> @@ -662,33 +662,22 @@ static 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");
>  	}
> @@ -696,11 +685,35 @@ int rxe_net_init(void)
>  	return err;
>  }
>  
> +int rxe_net_ipv6_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;
> +	}
> +
> +	err = register_netdevice_notifier(&rxe_net_notifier);
> +	if (err) {
> +		rxe_release_udp_tunnel(recv_sockets.sk6);
> +		pr_err("rxe: Failed to rigister netdev notifier\n");
> +	}
> +
> +	return err;
> +}
> +

Can this be also in IS_ENABLED(CONFIG_IPV6) scope?

>  void rxe_net_exit(void)
>  {
> +#if IS_ENABLED(CONFIG_IPV6)
>  	if (recv_sockets.sk6)
>  		rxe_release_udp_tunnel(recv_sockets.sk6);
> -
> +#endif
>  	if (recv_sockets.sk4)
>  		rxe_release_udp_tunnel(recv_sockets.sk4);
>  
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h
> index 7b06f76..b8e3284 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.h
> +++ b/drivers/infiniband/sw/rxe/rxe_net.h
> @@ -47,7 +47,8 @@ extern struct rxe_recv_sockets recv_sockets;
>  
>  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);

Can this be also in IS_ENABLED(CONFIG_IPV6) scope?

>  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@vger.kernel.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford Sept. 5, 2016, 1:32 p.m. UTC | #2
On 9/5/2016 8:09 AM, Leon Romanovsky wrote:
> From: Yonatan Cohen <yonatanc@mellanox.com>
> 

> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 0b8d2ea..e0866b4 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -249,8 +249,8 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
>  	memset(&udp_cfg, 0, sizeof(udp_cfg));
>  
>  	if (ipv6) {
> -		udp_cfg.family = AF_INET6;
> -		udp_cfg.ipv6_v6only = 1;
> +	udp_cfg.family = AF_INET6;
> +	udp_cfg.ipv6_v6only = 1;
>  	} else {
>  		udp_cfg.family = AF_INET;
>  	}

Is my mailer displaying this wrong, or is this hunk just intentionally
introducing indent breakage?
Leon Romanovsky Sept. 5, 2016, 2:03 p.m. UTC | #3
On Mon, Sep 05, 2016 at 04:24:02PM +0300, Yuval Shaia wrote:
> On Mon, Sep 05, 2016 at 03:09:16PM +0300, Leon Romanovsky 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
> >
> > Fixes: 8700e3e7c485 ("Soft RoCE driver")
> > Signed-off-by: Yonatan Cohen <yonatanc@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leon@kernel.org>
> > ---
> >  drivers/infiniband/sw/rxe/rxe.c     | 14 ++++++++++--
> >  drivers/infiniband/sw/rxe/rxe_net.c | 45 ++++++++++++++++++++++++-------------
> >  drivers/infiniband/sw/rxe/rxe_net.h |  3 ++-
> >  3 files changed, 43 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
> > index 55f0e8f..08e09d9 100644
> > --- a/drivers/infiniband/sw/rxe/rxe.c
> > +++ b/drivers/infiniband/sw/rxe/rxe.c
> > @@ -362,12 +362,22 @@ 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;
> >  	}
> > +
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +	err = rxe_net_ipv6_init();
> > +	if (err) {
> > +		pr_err("rxe: unable to init ipv6 tunnel\n");
> > +		rxe_cache_exit();
> > +		return err;
> > +	}
> > +#endif
> > +
> >  	pr_info("rxe: loaded\n");
> >
> >  	return 0;
> > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> > index 0b8d2ea..e0866b4 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_net.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> > @@ -249,8 +249,8 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
> >  	memset(&udp_cfg, 0, sizeof(udp_cfg));
> >
> >  	if (ipv6) {
> > -		udp_cfg.family = AF_INET6;
> > -		udp_cfg.ipv6_v6only = 1;
> > +	udp_cfg.family = AF_INET6;
> > +	udp_cfg.ipv6_v6only = 1;
> >  	} else {
> >  		udp_cfg.family = AF_INET;
> >  	}
> > @@ -662,33 +662,22 @@ static 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");
> >  	}
> > @@ -696,11 +685,35 @@ int rxe_net_init(void)
> >  	return err;
> >  }
> >
> > +int rxe_net_ipv6_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;
> > +	}
> > +
> > +	err = register_netdevice_notifier(&rxe_net_notifier);
> > +	if (err) {
> > +		rxe_release_udp_tunnel(recv_sockets.sk6);
> > +		pr_err("rxe: Failed to rigister netdev notifier\n");
> > +	}
> > +
> > +	return err;
> > +}
> > +
>
> Can this be also in IS_ENABLED(CONFIG_IPV6) scope?
>
> >  void rxe_net_exit(void)
> >  {
> > +#if IS_ENABLED(CONFIG_IPV6)
> >  	if (recv_sockets.sk6)
> >  		rxe_release_udp_tunnel(recv_sockets.sk6);
> > -
> > +#endif
> >  	if (recv_sockets.sk4)
> >  		rxe_release_udp_tunnel(recv_sockets.sk4);
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h
> > index 7b06f76..b8e3284 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_net.h
> > +++ b/drivers/infiniband/sw/rxe/rxe_net.h
> > @@ -47,7 +47,8 @@ extern struct rxe_recv_sockets recv_sockets;
> >
> >  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);
>
> Can this be also in IS_ENABLED(CONFIG_IPV6) scope?

In current implementation it should.
I personally would implement these config checks in rxe_net_ipv6_init()
function and not outside.

>
> >  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@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Sept. 5, 2016, 2:04 p.m. UTC | #4
On Mon, Sep 05, 2016 at 09:32:42AM -0400, Doug Ledford wrote:
> On 9/5/2016 8:09 AM, Leon Romanovsky wrote:
> > From: Yonatan Cohen <yonatanc@mellanox.com>
> >
>
> > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> > index 0b8d2ea..e0866b4 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_net.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> > @@ -249,8 +249,8 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
> >  	memset(&udp_cfg, 0, sizeof(udp_cfg));
> >
> >  	if (ipv6) {
> > -		udp_cfg.family = AF_INET6;
> > -		udp_cfg.ipv6_v6only = 1;
> > +	udp_cfg.family = AF_INET6;
> > +	udp_cfg.ipv6_v6only = 1;
> >  	} else {
> >  		udp_cfg.family = AF_INET;
> >  	}
>
> Is my mailer displaying this wrong, or is this hunk just intentionally
> introducing indent breakage?

You are right, it is unintentionally. I'll repost.

>
>
> --
> Doug Ledford <dledford@redhat.com>
>     GPG Key ID: 0E572FDD
>
Or Gerlitz Sept. 5, 2016, 7:21 p.m. UTC | #5
> On Mon, Sep 5, 2016 at 3:09 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

Hi Yoni, in this case, the trace contributes practically nothing to the
driver and kernel maintainers, please remove it.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Sept. 6, 2016, 5:05 a.m. UTC | #6
On Mon, Sep 05, 2016 at 10:21:15PM +0300, Or Gerlitz wrote:
> > On Mon, Sep 5, 2016 at 3:09 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
>
> Hi Yoni, in this case, the trace contributes practically nothing to the
> driver and kernel maintainers, please remove it.

Hi Or,

I like your self-confident.

In this case, I asked from Yoni to add this call trace for a number of
reasons:
1. We got this call stack from external user and he wants to know that
it is fixed.
2. I'm looking on the oops posted to the list to get better
understanding if such cases exist in other places too.
3. We are in late -rc stage and kernel trace shows that it is real issue
which justifies late acceptance/pull request.

Thanks

> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz Sept. 6, 2016, 6:06 a.m. UTC | #7
On Tue, Sep 6, 2016 at 8:05 AM, Leon Romanovsky <leonro@mellanox.com> wrote:
> On Mon, Sep 05, 2016 at 10:21:15PM +0300, Or Gerlitz wrote:

>> Hi Yoni, in this case, the trace contributes practically nothing to the
>> driver and kernel maintainers, please remove it.

> 1. We got this call stack from external user and he wants to know that
> it is fixed.

good to know, Yoni - we should give them credit! add Reported-by: line
after your author S.O.B line

> 2. I'm looking on the oops posted to the list to get better
> understanding if such cases exist in other places too.

for both (1) and (2) you can put the track trace after the --- line of
the submitted patch,
such that when the maintainer does git am, we don't carry that forever
in the kernel logs.

> 3. We are in late -rc stage and kernel trace shows that it is real issue
> which justifies late acceptance/pull request.

You can solve it with two words "kernel crash" in the change log

The crash originates from small wrongish behaviour of the rxe driver, not from
sophisticated and complex bug. The crash trace doesn't contribute
anything to future
maintainship of the driver and/or the IB subsystem.

> I like your self-confident.
> In this case, I asked from Yoni to add this call trace for a number of reasons:

$ ./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)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Sept. 6, 2016, 8:59 a.m. UTC | #8
On Tue, Sep 06, 2016 at 09:06:46AM +0300, Or Gerlitz wrote:
> On Tue, Sep 6, 2016 at 8:05 AM, Leon Romanovsky <leonro@mellanox.com> wrote:
> > On Mon, Sep 05, 2016 at 10:21:15PM +0300, Or Gerlitz wrote:
>
> >> Hi Yoni, in this case, the trace contributes practically nothing to the
> >> driver and kernel maintainers, please remove it.
>
> > 1. We got this call stack from external user and he wants to know that
> > it is fixed.
>
> good to know, Yoni - we should give them credit! add Reported-by: line
> after your author S.O.B line

Internally, It was asked too, and we decided to skip it.
It is uncommon to put jenkins bot name.
Or Gerlitz Sept. 6, 2016, 9:21 a.m. UTC | #9
On Tue, Sep 6, 2016 at 11:59 AM, Leon Romanovsky <leonro@mellanox.com> wrote:
> On Tue, Sep 06, 2016 at 09:06:46AM +0300, Or Gerlitz wrote:
>> On Tue, Sep 6, 2016 at 8:05 AM, Leon Romanovsky <leonro@mellanox.com> wrote:
>> > On Mon, Sep 05, 2016 at 10:21:15PM +0300, Or Gerlitz wrote:
>>
>> >> Hi Yoni, in this case, the trace contributes practically nothing to the
>> >> driver and kernel maintainers, please remove it.
>>
>> > 1. We got this call stack from external user and he wants to know that
>> > it is fixed.
>>
>> good to know, Yoni - we should give them credit! add Reported-by: line
>> after your author S.O.B line
>
> Internally, It was asked too, and we decided to skip it.
> It is uncommon to put jenkins bot name.

I couldn't realize from your email that it was a robot finding that
and not a human, you
can skip the robot name or creditize the person that read the robot's
email, if this is
the author of the patch, no need for double credit.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index 55f0e8f..08e09d9 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -362,12 +362,22 @@  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;
 	}
+
+#if IS_ENABLED(CONFIG_IPV6)
+	err = rxe_net_ipv6_init();
+	if (err) {
+		pr_err("rxe: unable to init ipv6 tunnel\n");
+		rxe_cache_exit();
+		return err;
+	}
+#endif
+
 	pr_info("rxe: loaded\n");
 
 	return 0;
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 0b8d2ea..e0866b4 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -249,8 +249,8 @@  static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
 	memset(&udp_cfg, 0, sizeof(udp_cfg));
 
 	if (ipv6) {
-		udp_cfg.family = AF_INET6;
-		udp_cfg.ipv6_v6only = 1;
+	udp_cfg.family = AF_INET6;
+	udp_cfg.ipv6_v6only = 1;
 	} else {
 		udp_cfg.family = AF_INET;
 	}
@@ -662,33 +662,22 @@  static 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");
 	}
@@ -696,11 +685,35 @@  int rxe_net_init(void)
 	return err;
 }
 
+int rxe_net_ipv6_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;
+	}
+
+	err = register_netdevice_notifier(&rxe_net_notifier);
+	if (err) {
+		rxe_release_udp_tunnel(recv_sockets.sk6);
+		pr_err("rxe: Failed to rigister netdev notifier\n");
+	}
+
+	return err;
+}
+
 void rxe_net_exit(void)
 {
+#if IS_ENABLED(CONFIG_IPV6)
 	if (recv_sockets.sk6)
 		rxe_release_udp_tunnel(recv_sockets.sk6);
-
+#endif
 	if (recv_sockets.sk4)
 		rxe_release_udp_tunnel(recv_sockets.sk4);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h
index 7b06f76..b8e3284 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.h
+++ b/drivers/infiniband/sw/rxe/rxe_net.h
@@ -47,7 +47,8 @@  extern struct rxe_recv_sockets recv_sockets;
 
 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 */