Not possible to connect() after a disconnect for a passive socket
#290
Assignees
Labels
Comments
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Jan 13, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overal this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: multipath-tcp/mptcp_net-next#290 Signed-off-by: Paolo Abeni <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Jan 28, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overal this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: multipath-tcp/mptcp_net-next#290 Signed-off-by: Paolo Abeni <[email protected]>
matttbe
pushed a commit
that referenced
this issue
Jan 30, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
jenkins-tessares
pushed a commit
that referenced
this issue
Jan 31, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
matttbe
pushed a commit
that referenced
this issue
Jan 31, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
matttbe
pushed a commit
that referenced
this issue
Jan 31, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
jenkins-tessares
pushed a commit
that referenced
this issue
Feb 1, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
matttbe
pushed a commit
that referenced
this issue
Feb 2, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
matttbe
pushed a commit
that referenced
this issue
Feb 2, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
jenkins-tessares
pushed a commit
that referenced
this issue
Feb 3, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
jenkins-tessares
pushed a commit
that referenced
this issue
Feb 4, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
jenkins-tessares
pushed a commit
that referenced
this issue
Feb 5, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
jenkins-tessares
pushed a commit
that referenced
this issue
Feb 7, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
matttbe
pushed a commit
that referenced
this issue
Feb 8, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
jenkins-tessares
pushed a commit
that referenced
this issue
Feb 9, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
jenkins-tessares
pushed a commit
that referenced
this issue
Feb 10, 2023
After the previous patch the first subflow is allocated as needed at bind, connect, listen time. We don't need anymore to keep alive the first subflow after a disconnect just to be able to perform such syscall. Overall, this change makes the passive and active sockets consistent: even passive sockets will be allowed to complete life cycle after disconnect. Closes: #290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Matthieu Baerts <[email protected]>
|
The patch initially fixing this issue has been reverted, see #352 |
|
should be addressed by the recent msk->subflow refactor: b5b6973 mptcp: get rid of msk->subflow |
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Aug 14, 2023
Such field is now unused just as a flag to control the first subflow deletion at close() time. Introduce a new bit flag for that and finally drop the mentioned field. As an intended side effect, now the first subflow sock is not freed before close() even for passive sockets. The msk has no open/active subflows if the first one is closed and the subflow list is singular, update accordingly the state check in mptcp_stream_accept(). Among other benefits, the subflow removal, reduces the amount of memory used on the client side for each mptcp connection, allows passive sockets to go through successful accept()/disconnect()/connect() and makes return error code consistent for failing both passive and active sockets. Closes: multipath-tcp/mptcp_net-next#290 Signed-off-by: Paolo Abeni <[email protected]> Reviewed-by: Mat Martineau <[email protected]> Signed-off-by: Matthieu Baerts <[email protected]> Signed-off-by: David S. Miller <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this issue
Aug 14, 2023
Matthieu Baerts says: ==================== mptcp: get rid of msk->subflow The MPTCP protocol maintains an additional struct socket per connection, mainly to be able to easily use tcp-level struct socket operations. This leads to several side effects, beyond the quite unfortunate / confusing 'subflow' field name: - active and passive sockets behaviour is inconsistent: only active ones have a not NULL msk->subflow, leading to different error handling and different error code returned to the user-space in several places. - active sockets uses an unneeded, larger amount of memory - passive sockets can't successfully go through accept(), disconnect(), accept() sequence, see [1] for more details. The 13 first patches of this series are from Paolo and address all the above, finally getting rid of the blamed field: - The first patch is a minor clean-up. - In the next 11 patches, msk->subflow usage is systematically removed from the MPTCP protocol, replacing it with direct msk->first usage, eventually introducing new core helpers when needed. - The 13th patch finally disposes the field, and it's the only patch in the series intended to produce functional changes. The last and 14th patch is from Kuniyuki and it is not linked to the previous ones: it is a small clean-up to get rid of an unnecessary check in mptcp_init_sock(). [1] multipath-tcp/mptcp_net-next#290 ==================== Signed-off-by: Matthieu Baerts <[email protected]>
matttbe
pushed a commit
that referenced
this issue
Jan 5, 2024
Now the APIs virtqueue_dma_sync_single_range_for_{cpu,device} ignore
the parameter 'dir', that is a mistake.
[ 6.101666] ------------[ cut here ]------------
[ 6.102079] DMA-API: virtio-pci 0000:00:04.0: device driver syncs DMA memory with different direction [device address=0x00000000ae010000] [size=32752 bytes] [mapped with DMA_FROM_DEVICE] [synced with DMA_BIDIRECTIONAL]
[ 6.103630] WARNING: CPU: 6 PID: 0 at kernel/dma/debug.c:1125 check_sync+0x53e/0x6c0
[ 6.107420] CPU: 6 PID: 0 Comm: swapper/6 Tainted: G E 6.6.0+ #290
[ 6.108030] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
[ 6.108936] RIP: 0010:check_sync+0x53e/0x6c0
[ 6.109289] Code: 24 10 e8 f5 d9 74 00 4c 8b 4c 24 10 4c 8b 44 24 18 48 8b 4c 24 20 48 89 c6 41 56 4c 89 ea 48 c7 c7 b0 f1 50 82 e8 32 fc f3 ff <0f> 0b 48 c7 c7 48 4b 4a 82 e8 74 d9 fc ff 8b 73 4c 48 8d 7b 50 31
[ 6.110750] RSP: 0018:ffffc90000180cd8 EFLAGS: 00010092
[ 6.111178] RAX: 00000000000000ce RBX: ffff888100aa5900 RCX: 0000000000000000
[ 6.111744] RDX: 0000000000000104 RSI: ffffffff824c3208 RDI: 00000000ffffffff
[ 6.112316] RBP: ffffc90000180d40 R08: 0000000000000000 R09: 00000000fffeffff
[ 6.112893] R10: ffffc90000180b98 R11: ffffffff82f63308 R12: ffffffff83d5af00
[ 6.113460] R13: ffff888100998200 R14: ffffffff824a4b5f R15: 0000000000000286
[ 6.114027] FS: 0000000000000000(0000) GS:ffff88842fd80000(0000) knlGS:0000000000000000
[ 6.114665] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 6.115128] CR2: 00007f10f1e03030 CR3: 0000000108272004 CR4: 0000000000770ee0
[ 6.115701] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 6.116272] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 6.116842] PKRU: 55555554
[ 6.117069] Call Trace:
[ 6.117275] <IRQ>
[ 6.117452] ? __warn+0x84/0x140
[ 6.117727] ? check_sync+0x53e/0x6c0
[ 6.118034] ? __report_bug+0xea/0x100
[ 6.118353] ? check_sync+0x53e/0x6c0
[ 6.118653] ? report_bug+0x41/0xc0
[ 6.118944] ? handle_bug+0x3c/0x70
[ 6.119237] ? exc_invalid_op+0x18/0x70
[ 6.119551] ? asm_exc_invalid_op+0x1a/0x20
[ 6.119900] ? check_sync+0x53e/0x6c0
[ 6.120199] ? check_sync+0x53e/0x6c0
[ 6.120499] debug_dma_sync_single_for_cpu+0x5c/0x70
[ 6.120906] ? dma_sync_single_for_cpu+0xb7/0x100
[ 6.121291] virtnet_rq_unmap+0x158/0x170 [virtio_net]
[ 6.121716] virtnet_receive+0x196/0x220 [virtio_net]
[ 6.122135] virtnet_poll+0x48/0x1b0 [virtio_net]
[ 6.122524] __napi_poll+0x29/0x1b0
[ 6.123083] net_rx_action+0x282/0x360
[ 6.123612] __do_softirq+0xf3/0x2fb
[ 6.124138] __irq_exit_rcu+0x8e/0xf0
[ 6.124663] common_interrupt+0xbc/0xe0
[ 6.125202] </IRQ>
We need to enable CONFIG_DMA_API_DEBUG and work with need sync mode(such
as swiotlb) to reproduce this warn.
Fixes: 8bd2f71 ("virtio_ring: introduce dma sync api for virtqueue")
Reported-by: "Ning, Hongyu" <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Suggested-by: Jason Wang <[email protected]>
Signed-off-by: Xuan Zhuo <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Reviewed-by: Parav Pandit <[email protected]>
Acked-by: Jason Wang <[email protected]>
Tested-by: Hongyu Ning <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
@pabeni reported that it is not possible to do a
connect()after a disconnect for a passive socket.It fails because the passive MPTCP socket -- created after an
accept()-- has thestruct socketmissing for the first subflow.A solution would be to allocate new
struct socketat the disconnect time but then unexpected errors might happen, e.g. ENOMEM. Quite unexpected at the disconnect time, it might not be handled properly on the userspace side.The text was updated successfully, but these errors were encountered: