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

Commit Message

Leon Romanovsky Sept. 7, 2016, 11:04 a.m. UTC

From: Yonatan Cohen <[e-mail]>

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 <[e-mail]>
Signed-off-by: Leon Romanovsky <[e-mail]>
---
 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(-)

--
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Or Gerlitz Sept. 7, 2016, 11:20 a.m. UTC | #1

On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <[e-mail]> wrote:
> From: Yonatan Cohen <[e-mail]>
>
> 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 <[e-mail]> (supporter:SOFT-ROCE DRIVER (rxe))
Doug Ledford <[e-mail]> (supporter:INFINIBAND SUBSYSTEM)
Sean Hefty <[e-mail]> (supporter:INFINIBAND SUBSYSTEM)
Hal Rosenstock <[e-mail]> (supporter:INFINIBAND SUBSYSTEM)
[e-mail] (open list:SOFT-ROCE DRIVER (rxe))
[e-mail] (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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Moni Shoua Sept. 7, 2016, 11:39 a.m. UTC | #2

U29ycnkgT3IgYnV0IEkgbWlnaHQgaGF2ZSBtaXNzZWQgeW91ciByZXZpZXcgY29tbWVudCByZWdh
cmRpbmcgdGhlIGNvbW1pdCBtZXNzYWdlDQpJIGNhbid0IGZpbmQgaXQgbm93IHNvIHBsZWFzZSBz
ZW5kIGl0IGFnYWluDQoNClRoYW5rcw0KDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpG
cm9tOiBPciBHZXJsaXR6IFttYWlsdG86Z2VybGl0ei5vckBnbWFpbC5jb21dIA0KU2VudDogV2Vk
bmVzZGF5LCBTZXB0ZW1iZXIgMDcsIDIwMTYgMjoyMSBQTQ0KVG86IExlb24gUm9tYW5vdnNreSA8
bGVvbnJvQG1lbGxhbm94LmNvbT47IE1vbmkgU2hvdWEgPG1vbmlzQG1lbGxhbm94LmNvbT4NCkNj
OiBEb3VnIExlZGZvcmQgPGRsZWRmb3JkQHJlZGhhdC5jb20+OyBsaW51eC1yZG1hQHZnZXIua2Vy
bmVsLm9yZzsgWW9uYXRhbiBDb2hlbiA8eW9uYXRhbmNAbWVsbGFub3guY29tPg0KU3ViamVjdDog
UmU6IFtQQVRDSCByZG1hLXJjIFYxIDEvNF0gSUIvcnhlOiBGaXgga2VybmVsIHBhbmljIGluIHVk
cF9zZXR1cF90dW5uZWwNCg0KT24gV2VkLCBTZXAgNywgMjAxNiBhdCAyOjA0IFBNLCBMZW9uIFJv
bWFub3Zza3kgPGxlb25Aa2VybmVsLm9yZz4gd3JvdGU6DQo+IEZyb206IFlvbmF0YW4gQ29oZW4g
PHlvbmF0YW5jQG1lbGxhbm94LmNvbT4NCj4NCj4gRGlzYWJsZSBjcmVhdGlvbiBvZiBhIFVEUCBz
b2NrZXQgZm9yIGlwdjYgd2hlbg0KPiBDT05GSUdfSVBWNiBpcyBub3QgZW5hYmVsZC4gU2luY2Ug
dWRwX3NvY2tfY3JlYXRlNigpIHJldHVybnMgMCB3aGVuIA0KPiBDT05GSUdfSVBWNiBpcyBub3Qg
c2V0DQo+DQo+IFsgICA0Ni44ODg2MzJdIElQOiBbPGMyMjA3MDVhPl0gc2V0dXBfdWRwX3R1bm5l
bF9zb2NrKzB4Ni8weDRmDQo+IFsgICA0Ni44OTEzNTVdICpwZHB0ID0gMDAwMDAwMDAwMDAwMDAw
MCAqcGRlID0gZjAwMGZmNTNmMDAwZmY1Mw0KPiBbICAgNDYuODkzOTE4XSBPb3BzOiAwMDAyIFsj
MV0gUFJFRU1QVA0KPiBbICAgNDYuODk2MDE0XSBDUFU6IDAgUElEOiAxIENvbW06IHN3YXBwZXIg
Tm90IHRhaW50ZWQgNC43LjAtcmM0LTAwMDAxLWc4NzAwZTNlICMxDQo+IFsgICA0Ni45MDAyODBd
IEhhcmR3YXJlIG5hbWU6IFFFTVUgU3RhbmRhcmQgUEMgKGk0NDBGWCArIFBJSVgsIDE5OTYpLCBC
SU9TIERlYmlhbi0xLjguMi0xIDA0LzAxLzIwMTQNCj4gWyAgIDQ2LjkwNDkwNV0gdGFzazogY2Yw
NmMwNDAgdGk6IGNmMDVlMDAwIHRhc2sudGk6IGNmMDVlMDAwDQo+IFsgICA0Ni45MDc4NTRdIEVJ
UDogMDA2MDpbPGMyMjA3MDVhPl0gRUZMQUdTOiAwMDIxMDI0NiBDUFU6IDANCj4gWyAgIDQ2Ljkx
MTEzN10gRUlQIGlzIGF0IHNldHVwX3VkcF90dW5uZWxfc29jaysweDYvMHg0Zg0KPiBbICAgNDYu
OTE0MDcwXSBFQVg6IDAwMDAwMDQ0IEVCWDogMDAwMDAwMDEgRUNYOiBjZjA1ZmVmMCBFRFg6IGNh
ODE0MmUwDQo+IFsgICA0Ni45MTcyMzZdIEVTSTogYzJjNDUwNWIgRURJOiBjZjA1ZmVmMCBFQlA6
IGNmMDVmZWQwIEVTUDogY2YwNWZlZDANCj4gWyAgIDQ2LjkxOTgzNl0gIERTOiAwMDdiIEVTOiAw
MDdiIEZTOiAwMDAwIEdTOiAwMGUwIFNTOiAwMDY4DQo+IFsgICA0Ni45MjIwNDZdIENSMDogODAw
NTAwMzMgQ1IyOiAwMDAwMDFmYyBDUjM6IDAyY2VjMDAwIENSNDogMDAwMDA2YjANCj4gWyAgIDQ2
LjkyNDU1MF0gU3RhY2s6DQo+IFsgICA0Ni45MjYwMTRdICBjZjA1ZmYxMCBjMWZkNDY1NyBjYTgx
NDJlMCAwMDAwMDAwYSAwMDAwMDAwMCAwMDAwMDAwMCAwMDAwYjcxMiAwMDAwMDAwOA0KPiBbICAg
NDYuOTMxMjc0XSAgMDAwMDAwMDAgNmJiNWJkMDEgYzFmZDQ4ZGUgMDAwMDAwMDAgMDAwMDAwMDAg
Y2YwNWZmMWMgMDAwMDAwMDAgMDAwMDAwMDANCj4gWyAgIDQ2LjkzNjEyMl0gIGNmMDVmZjFjIGMx
ZmQ0YmRmIDAwMDAwMDAwIGNmMDVmZjI4IGMyYzQ1MDdiIGZmZmZmZmZmIGNmMDVmZjg4IGMyYmYx
Yzc0DQo+IFsgICA0Ni45NDIzNTBdIENhbGwgVHJhY2U6DQo+IFsgICA0Ni45NDQ0MDNdICBbPGMx
ZmQ0NjU3Pl0gcnhlX3NldHVwX3VkcF90dW5uZWwrMHg4Zi8weDk5DQo+IFsgICA0Ni45NDc2ODld
ICBbPGMxZmQ0OGRlPl0gPyBuZXRfdG9fcnhlKzB4NGUvMHg0ZQ0KPiBbICAgNDYuOTUwNTY3XSAg
WzxjMWZkNGJkZj5dIHJ4ZV9uZXRfaW5pdCsweGUvMHhhNA0KPiBbICAgNDYuOTUzMTQ3XSAgWzxj
MmM0NTA3Yj5dIHJ4ZV9tb2R1bGVfaW5pdCsweDIwLzB4NGMNCj4gWyAgIDQ2Ljk1NTQ0OF0gIFs8
YzJiZjFjNzQ+XSBkb19vbmVfaW5pdGNhbGwrMHg4OS8weDExMw0KPiBbICAgNDYuOTU3Nzk3XSAg
WzxjMmJmMTVlYj5dID8gc2V0X2RlYnVnX3JvZGF0YSsweGYvMHhmDQo+IFsgICA0Ni45NTk5NjZd
ICBbPGMyYmYxZGJjPl0gPyBrZXJuZWxfaW5pdF9mcmVlYWJsZSsweGJlLzB4MTViDQo+IFsgICA0
Ni45NjIyNjJdICBbPGMyYmYxZGRjPl0ga2VybmVsX2luaXRfZnJlZWFibGUrMHhkZS8weDE1Yg0K
PiBbICAgNDYuOTY0NDE4XSAgWzxjMjMyZWI1ND5dIGtlcm5lbF9pbml0KzB4OC8weGQwDQo+IFsg
ICA0Ni45NjY2MThdICBbPGMyMzMzMTIyPl0gcmV0X2Zyb21fa2VybmVsX3RocmVhZCsweGUvMHgy
NA0KPiBbICAgNDYuOTY5NTkyXSAgWzxjMjMyZWI0Yz5dID8gcmVzdF9pbml0KzB4NmYvMHg2Zg0K
DQp3aGF0IGFuIHVubmljZSB3YXkgdG8gc2V0dGxlIGEgcmV2aWV3IGRpc2N1c3Npb24uIEkgcmVw
bGllZCB0byBhbGwgeW91ciBhcmd1bWVudHMgZXhwbGFpbmluZyB3aHkgdGhlIGNoYW5nZSBsb2cg
aGFzIHRvIGdvLCBhbmQgdGhlIG5leHQgc3RlcCBpcyB0byBpZ25vcmUgdGhpcyBkaXNjdXNzaW9u
IGFuZCBkbyBpdCBhZ2FpbiB3cm9uZyB3Lm8gcmVwbHlpbmc/IHNvLi4uIGFnYWluOg0KDQokIC4v
c2NyaXB0cy9nZXRfbWFpbnRhaW5lci5wbCBkcml2ZXJzL2luZmluaWJhbmQvc3cvcnhlIE1vbmkg
U2hvdWEgPG1vbmlzQG1lbGxhbm94LmNvbT4gKHN1cHBvcnRlcjpTT0ZULVJPQ0UgRFJJVkVSIChy
eGUpKSBEb3VnIExlZGZvcmQgPGRsZWRmb3JkQHJlZGhhdC5jb20+IChzdXBwb3J0ZXI6SU5GSU5J
QkFORCBTVUJTWVNURU0pIFNlYW4gSGVmdHkgPHNlYW4uaGVmdHlAaW50ZWwuY29tPiAoc3VwcG9y
dGVyOklORklOSUJBTkQgU1VCU1lTVEVNKSBIYWwgUm9zZW5zdG9jayA8aGFsLnJvc2Vuc3RvY2tA
Z21haWwuY29tPiAoc3VwcG9ydGVyOklORklOSUJBTkQgU1VCU1lTVEVNKSBsaW51eC1yZG1hQHZn
ZXIua2VybmVsLm9yZyAob3BlbiBsaXN0OlNPRlQtUk9DRSBEUklWRVIgKHJ4ZSkpIGxpbnV4LWtl
cm5lbEB2Z2VyLmtlcm5lbC5vcmcgKG9wZW4gbGlzdCkNCg0KTW9uaSwgY2FuIHlvdSBwbGVhc2Ug
cHV0IHlvdXIgbWFpbnRhaW5lciBzYXkgaGVyZS4NCg0KDQoNCk9yLg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Leon Romanovsky Sept. 7, 2016, 12:13 p.m. UTC | #3

On Wed, Sep 07, 2016 at 02:20:50PM +0300, Or Gerlitz wrote:
> On Wed, Sep 7, 2016 at 2:04 PM, Leon Romanovsky <[e-mail]> wrote:
> > From: Yonatan Cohen <[e-mail]>
> >
> > 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 <[e-mail]> (supporter:SOFT-ROCE DRIVER (rxe))
> Doug Ledford <[e-mail]> (supporter:INFINIBAND SUBSYSTEM)
> Sean Hefty <[e-mail]> (supporter:INFINIBAND SUBSYSTEM)
> Hal Rosenstock <[e-mail]> (supporter:INFINIBAND SUBSYSTEM)
> [e-mail] (open list:SOFT-ROCE DRIVER (rxe))
> [e-mail] (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 [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Or Gerlitz Sept. 7, 2016, 1:19 p.m. UTC | #4

On Wed, Sep 7, 2016 at 3:13 PM, Leon Romanovsky <[e-mail]> 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 <[e-mail]> wrote:
>> > From: Yonatan Cohen <[e-mail]>
>> >
>> > 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Or Gerlitz Sept. 7, 2016, 1:23 p.m. UTC | #5

On Wed, Sep 7, 2016 at 2:39 PM, Moni Shoua <[e-mail]> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Moni Shoua Sept. 7, 2016, 2:18 p.m. UTC | #6

> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Doug Ledford Sept. 7, 2016, 4:02 p.m. UTC | #7

On 9/7/2016 9:19 AM, Or Gerlitz wrote:
> On Wed, Sep 7, 2016 at 3:13 PM, Leon Romanovsky <[e-mail]> 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 <[e-mail]> wrote:
>>>> From: Yonatan Cohen <[e-mail]>
>>>>
>>>> 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.
>

Patch

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 */