[GIT] Sync with net-next on 20190131: a few conflicts
by Matthieu Baerts
Hello,
It seems that recently, 'net-next' has been synced with 'net' which
contains some modifications in MPTCP code. We then had a few conflicts
due to Florian's patch-set, see: 55c6ab4b368 (Merge branch
'mptcp-fix-sockopt-crash-and-lockdep-splats')
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id...
The first conflict is with: ae2dd7164943 (mptcp: handle tcp fallback
when using syn cookies)
diff --cc net/mptcp/subflow.c
index 65122edf60aa,8b5945266890..000000000000
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@@ -772,7 -863,7 +868,11 @@@ static void subflow_ulp_clone(const str
struct mptcp_subflow_context *old_ctx = mptcp_subflow_ctx(newsk);
struct mptcp_subflow_context *new_ctx;
++<<<<<<< t/mptcp-Add-handling-of-incoming-MP_JOIN-requests base content
+ if (!tcp_rsk(req)->is_mptcp || !subflow_req->mp_capable) {
++=======
+ if (!subflow_req->mp_capable && !subflow_req->mp_join) {
++>>>>>>> remotes/origin/t/mptcp-Add-handling-of-incoming-MP_JOIN-requests
subflow_ulp_fallback(newsk, old_ctx);
return;
}
The resolution:
diff --cc net/mptcp/subflow.c
index 8b5945266890,65122edf60aa..000000000000
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@@ -863,7 -772,7 +868,8 @@@ static void subflow_ulp_clone(const str
struct mptcp_subflow_context *old_ctx = mptcp_subflow_ctx(newsk);
struct mptcp_subflow_context *new_ctx;
- if (!subflow_req->mp_capable && !subflow_req->mp_join) {
- if (!tcp_rsk(req)->is_mptcp || !subflow_req->mp_capable) {
++ if (!tcp_rsk(req)->is_mptcp ||
++ (!subflow_req->mp_capable && !subflow_req->mp_join)) {
subflow_ulp_fallback(newsk, old_ctx);
return;
}
→ 05f97b84e5d6: conflict in
t/mptcp-Add-handling-of-incoming-MP_JOIN-requests
Another one caused by another patch from Florian: b2c5b614ca6e (mptcp:
avoid a lockdep splat when mcast group was joined):
diff --cc net/mptcp/protocol.c
index 95bc6f9c6745,766a4b15c586..000000000000
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@@ -691,6 -794,11 +799,14 @@@ static void __mptcp_close(struct sock *
__mptcp_close_ssk(sk, ssk, subflow, timeout);
}
++<<<<<<< t/mptcp-queue-data-for-mptcp-level-retransmission base content
++=======
+ if (msk->cached_ext)
+ __skb_ext_put(msk->cached_ext);
+ __mptcp_clear_xmit(sk);
+ release_sock(sk);
+
++>>>>>>> remotes/origin/t/mptcp-queue-data-for-mptcp-level-retransmission
sk_common_release(sk);
}
The resolution:
diff --cc net/mptcp/protocol.c
index 766a4b15c586,95bc6f9c6745..000000000000
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@@ -788,7 -681,11 +789,13 @@@ static void __mptcp_close(struct sock *
mptcp_token_destroy(msk->token);
inet_sk_state_store(sk, TCP_CLOSE);
- list_for_each_entry_safe(subflow, tmp, &msk->conn_list, node) {
+ list_splice_init(&msk->conn_list, &conn_list);
+
++ __mptcp_clear_xmit(sk);
++
+ release_sock(sk);
+
+ list_for_each_entry_safe(subflow, tmp, &conn_list, node) {
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
__mptcp_close_ssk(sk, ssk, subflow, timeout);
→ b62d04c9cdd9: conflict in
t/mptcp-queue-data-for-mptcp-level-retransmission
Another conflict due to yet another patch from the series: c9fd9c5f4b93
(mptcp: defer freeing of cached ext until last moment)
diff --cc net/mptcp/protocol.c
index 65141c3d1574,60aca199fea2..000000000000
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@@ -1034,7 -1012,10 +1038,14 @@@ static struct sock *mptcp_accept(struc
static void mptcp_destroy(struct sock *sk)
{
++<<<<<<< t/mptcp-implement-memory-accounting-for-mptcp-rtx-queue
+ sk_sockets_allocated_dec(sk);
++=======
+ struct mptcp_sock *msk = mptcp_sk(sk);
+
+ if (msk->cached_ext)
+ __skb_ext_put(msk->cached_ext);
++>>>>>>> top-bases/t/mptcp-implement-memory-accounting-for-mptcp-rtx-queue
}
static int mptcp_setsockopt(struct sock *sk, int level, int optname,
The resolution:
diff --cc net/mptcp/protocol.c
index 65141c3d1574,60aca199fea2..000000000000
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@@ -1034,7 -1012,10 +1038,12 @@@ static struct sock *mptcp_accept(struc
static void mptcp_destroy(struct sock *sk)
{
+ struct mptcp_sock *msk = mptcp_sk(sk);
+
+ if (msk->cached_ext)
+ __skb_ext_put(msk->cached_ext);
++
+ sk_sockets_allocated_dec(sk);
}
static int mptcp_setsockopt(struct sock *sk, int level, int optname,
→ ccc5451794ad: conflict in
t/mptcp-implement-memory-accounting-for-mptcp-rtx-queue
Two last conflicts this time due to b2c5b614ca6e (mptcp: avoid a lockdep
splat when mcast group was joined) again:
diff --cc net/mptcp/protocol.c
index fd6f434455df,e9ae42a8c49d..000000000000
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@@ -1052,13 -947,6 +1059,16 @@@ static void __mptcp_close(struct sock *
__mptcp_close_ssk(sk, ssk, subflow, timeout);
}
++<<<<<<< t/mptcp-implement-and-use-MPTCP-level-retransmission
+ if (msk->cached_ext)
+ __skb_ext_put(msk->cached_ext);
+ __mptcp_clear_xmit(sk);
+ release_sock(sk);
+
+ mptcp_cancel_work(sk);
+
++=======
++>>>>>>> top-bases/t/mptcp-implement-and-use-MPTCP-level-retransmission
sk_common_release(sk);
}
and the resolution:
diff --cc net/mptcp/protocol.c
index fd6f434455df,e9ae42a8c49d..000000000000
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@@ -1052,13 -947,6 +1059,8 @@@ static void __mptcp_close(struct sock *
__mptcp_close_ssk(sk, ssk, subflow, timeout);
}
- if (msk->cached_ext)
- __skb_ext_put(msk->cached_ext);
- __mptcp_clear_xmit(sk);
- release_sock(sk);
-
+ mptcp_cancel_work(sk);
+
sk_common_release(sk);
}
→ c4534101b55b: conflict in
t/mptcp-implement-and-use-MPTCP-level-retransmission
And again due to the same patch:
diff --cc net/mptcp/protocol.c
index 0b5bf17419ac,76b11e9e6642..000000000000
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@@ -1111,9 -1096,13 +1112,19 @@@ static void __mptcp_close(struct sock *
mptcp_token_destroy(msk->token);
inet_sk_state_store(sk, TCP_CLOSE);
++<<<<<<< t/subflow-place-further-subflows-on-new-join_list
+ __mptcp_flush_join_list(msk);
+
+ list_for_each_entry_safe(subflow, tmp, &msk->conn_list, node) {
++=======
+ list_splice_init(&msk->conn_list, &conn_list);
+
+ __mptcp_clear_xmit(sk);
+
+ release_sock(sk);
+
+ list_for_each_entry_safe(subflow, tmp, &conn_list, node) {
++>>>>>>> top-bases/t/subflow-place-further-subflows-on-new-join_list
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
__mptcp_close_ssk(sk, ssk, subflow, timeout);
and the resolution:
diff --cc net/mptcp/protocol.c
index 0b5bf17419ac,76b11e9e6642..000000000000
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@@ -1111,9 -1096,13 +1112,15 @@@ static void __mptcp_close(struct sock *
mptcp_token_destroy(msk->token);
inet_sk_state_store(sk, TCP_CLOSE);
+ __mptcp_flush_join_list(msk);
+
- list_for_each_entry_safe(subflow, tmp, &msk->conn_list, node) {
+ list_splice_init(&msk->conn_list, &conn_list);
+
+ __mptcp_clear_xmit(sk);
+
+ release_sock(sk);
+
+ list_for_each_entry_safe(subflow, tmp, &conn_list, node) {
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
__mptcp_close_ssk(sk, ssk, subflow, timeout);
→ fc30e7873372: conflict in
t/subflow-place-further-subflows-on-new-join_list
I hope it is what you had in mind!
Tests + export are running!
Cheers,
Matt
--
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
2 years, 3 months
[syzkaller] KASAN: use-after-free Write in __lock_sock
by Christoph Paasch
Hello,
syzkaller hit another one (on top of latest net-tree). C-repro is attached
to the e-mail.
====
netlink: 8 bytes leftover after parsing attributes in process `syz-executor.5'.
TCP: request_sock_TCPv6: Possible SYN flooding on port 20002. Sending cookies. Check SNMP counters.
==================================================================
BUG: KASAN: use-after-free in atomic_try_cmpxchg include/asm-generic/atomic-instrumented.h:693 [inline]
BUG: KASAN: use-after-free in queued_spin_lock include/asm-generic/qspinlock.h:78 [inline]
BUG: KASAN: use-after-free in do_raw_spin_lock include/linux/spinlock.h:181 [inline]
BUG: KASAN: use-after-free in __raw_spin_lock_bh include/linux/spinlock_api_smp.h:136 [inline]
BUG: KASAN: use-after-free in _raw_spin_lock_bh+0x71/0xd0 kernel/locking/spinlock.c:175
Write of size 4 at addr ffff8880491a0e88 by task syz-executor.1/2083
CPU: 1 PID: 2083 Comm: syz-executor.1 Not tainted 5.5.0 #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xb7/0xfe lib/dump_stack.c:118
print_address_description.constprop.0+0x36/0x50 mm/kasan/report.c:374
__kasan_report.cold+0x1a/0x32 mm/kasan/report.c:506
kasan_report+0xe/0x20 mm/kasan/common.c:639
check_memory_region_inline mm/kasan/generic.c:185 [inline]
check_memory_region+0x130/0x1a0 mm/kasan/generic.c:192
atomic_try_cmpxchg include/asm-generic/atomic-instrumented.h:693 [inline]
queued_spin_lock include/asm-generic/qspinlock.h:78 [inline]
do_raw_spin_lock include/linux/spinlock.h:181 [inline]
__raw_spin_lock_bh include/linux/spinlock_api_smp.h:136 [inline]
_raw_spin_lock_bh+0x71/0xd0 kernel/locking/spinlock.c:175
spin_lock_bh include/linux/spinlock.h:343 [inline]
__lock_sock+0x105/0x190 net/core/sock.c:2414
lock_sock_nested+0x10f/0x140 net/core/sock.c:2938
lock_sock include/net/sock.h:1516 [inline]
mptcp_setsockopt+0x2f/0x1f0 net/mptcp/protocol.c:800
__sys_setsockopt+0x152/0x240 net/socket.c:2130
__do_sys_setsockopt net/socket.c:2146 [inline]
__se_sys_setsockopt net/socket.c:2143 [inline]
__x64_sys_setsockopt+0xba/0x150 net/socket.c:2143
do_syscall_64+0xb7/0x3d0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7fe1942da469
Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d ff 49 2b 00 f7 d8 64 89 01 48
RSP: 002b:00007fe194946dd8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 000000000066c1a0 RCX: 00007fe1942da469
RDX: 0000000000000039 RSI: 0000000000000029 RDI: 0000000000000006
RBP: 00000000ffffffff R08: 0000000000000001 R09: 0000000000000000
R10: 0000000020000180 R11: 0000000000000246 R12: 0000000000000a59
R13: 000000000041d0d6 R14: 00007fe1949475c0 R15: 0000000000000003
Allocated by task 2163:
save_stack+0x1b/0x80 mm/kasan/common.c:72
set_track mm/kasan/common.c:80 [inline]
__kasan_kmalloc mm/kasan/common.c:513 [inline]
__kasan_kmalloc.constprop.0+0xc2/0xd0 mm/kasan/common.c:486
slab_post_alloc_hook mm/slab.h:584 [inline]
slab_alloc_node mm/slub.c:2759 [inline]
slab_alloc mm/slub.c:2767 [inline]
kmem_cache_alloc+0xc1/0x250 mm/slub.c:2772
sk_prot_alloc+0x5f/0x2c0 net/core/sock.c:1597
sk_alloc+0x32/0x8c0 net/core/sock.c:1657
inet6_create net/ipv6/af_inet6.c:180 [inline]
inet6_create+0x292/0xd70 net/ipv6/af_inet6.c:107
__sock_create+0x213/0x4d0 net/socket.c:1433
sock_create net/socket.c:1484 [inline]
__sys_socket+0xef/0x200 net/socket.c:1526
__do_sys_socket net/socket.c:1535 [inline]
__se_sys_socket net/socket.c:1533 [inline]
__x64_sys_socket+0x6f/0xb0 net/socket.c:1533
do_syscall_64+0xb7/0x3d0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Freed by task 2069:
save_stack+0x1b/0x80 mm/kasan/common.c:72
set_track mm/kasan/common.c:80 [inline]
kasan_set_free_info mm/kasan/common.c:335 [inline]
__kasan_slab_free+0x12f/0x180 mm/kasan/common.c:474
slab_free_hook mm/slub.c:1425 [inline]
slab_free_freelist_hook mm/slub.c:1458 [inline]
slab_free mm/slub.c:3005 [inline]
kmem_cache_free+0x80/0x2b0 mm/slub.c:3021
sk_prot_free net/core/sock.c:1638 [inline]
__sk_destruct+0x459/0x5a0 net/core/sock.c:1724
sk_destruct+0xc6/0x100 net/core/sock.c:1739
__sk_free+0xef/0x3d0 net/core/sock.c:1750
sk_free+0x78/0xa0 net/core/sock.c:1761
sock_put include/net/sock.h:1719 [inline]
sk_common_release+0x24a/0x370 net/core/sock.c:3200
__mptcp_close+0x3c3/0x530 net/mptcp/protocol.c:662
__mptcp_fallback_to_tcp net/mptcp/protocol.c:75 [inline]
__mptcp_tcp_fallback net/mptcp/protocol.c:116 [inline]
__mptcp_tcp_fallback+0x716/0x970 net/mptcp/protocol.c:100
mptcp_sendmsg+0xe1/0x14e0 net/mptcp/protocol.c:334
inet6_sendmsg+0x115/0x140 net/ipv6/af_inet6.c:576
sock_sendmsg_nosec net/socket.c:652 [inline]
sock_sendmsg+0xee/0x190 net/socket.c:672
__sys_sendto+0x21a/0x330 net/socket.c:1998
__do_sys_sendto net/socket.c:2010 [inline]
__se_sys_sendto net/socket.c:2006 [inline]
__x64_sys_sendto+0xdd/0x1b0 net/socket.c:2006
do_syscall_64+0xb7/0x3d0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x44/0xa9
The buggy address belongs to the object at ffff8880491a0e00
which belongs to the cache MPTCPv6 of size 1616
The buggy address is located 136 bytes inside of
1616-byte region [ffff8880491a0e00, ffff8880491a1450)
The buggy address belongs to the page:
page:ffffea0001246800 refcount:1 mapcount:0 mapping:ffff888116aa8800 index:0x0 compound_mapcount: 0
raw: 0100000000010200 dead000000000100 dead000000000122 ffff888116aa8800
raw: 0000000000000000 0000000080120012 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8880491a0d80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8880491a0e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff8880491a0e80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8880491a0f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8880491a0f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Cheers,
Christoph
2 years, 3 months
Fwd: Re: [multipathtcp] RFC6824bis edits based on implementation feedback
by Christoph Paasch
Hello,
the IETF working-group is still arguing whether or not to allow early/late
DSS-mappings.
If you feel strongly about it, feel free to express your support on the IETF
mailing-list (thanks to Paolo for already doing so).
Cheers,
Christoph
----- Forwarded message from philip.eardley(a)bt.com -----
From: philip.eardley(a)bt.com
To: anil(a)csir4pi.in, alan.ford(a)gmail.com
Cc: multipathtcp(a)ietf.org
Date: Thu, 23 Jan 2020 10:51:11 +0000
Subject: Re: [multipathtcp] RFC6824bis edits based on implementation feedback
I haven’t seen any responses about this. Please follow-up if you care about the issue.
Alan – please consider Change 1 as agreed.
From: multipathtcp <multipathtcp-bounces(a)ietf.org> On Behalf Of V Anil Kumar
Sent: 13 January 2020 18:25
To: alan ford <alan.ford(a)gmail.com>
Cc: multipathtcp(a)ietf.org
Subject: Re: [multipathtcp] RFC6824bis edits based on implementation feedback
Hi,
I have some points related to the modifications (Change 2) being proposed on data sequence map. Please see them inline. Though I am putting forward the below points, if the consensus is in favour of the proposed change for reducing implementation complexity, I am also OK with that as well.
________________________________
From: "alan ford" <alan.ford(a)gmail.com<mailto:alan.ford@gmail.com>>
To: multipathtcp(a)ietf.org<mailto:multipathtcp@ietf.org>
Sent: Thursday, January 2, 2020 4:21:32 AM
Subject: [multipathtcp] RFC6824bis edits based on implementation feedback
Hi all,
We’d love to get this to a state of completion as soon as possible, and to this end I am starting a new thread on this topic. In discussion with the chairs, it is possible to make the desired changes in AUTH48 as long as there is WG consensus. The discussion so far has been fairly limited in terms of participation.
I would ask the chairs please if it was possible to specify a time bound for this discussion and a default conclusion.
Regarding the changes, in summary, there are two areas where changes have been requested by the implementation community. As we are the IETF we obviously have strong focus on “running code” and so ease of implementing standards-compliant code is strongly desirable. However, we do not wish to reduce functionality agreed by the IETF community if it is considered a required feature by the community.
Change 1
Change the sentence reading:
If B has data to send first, then the reliable delivery of the ACK + MP_CAPABLE can be inferred by the receipt of this data with an MPTCP Data Sequence Signal (DSS) option (Section 3.3).
To:
If B has data to send first, then the reliable delivery of the ACK + MP_CAPABLE is ensured by the receipt of this data with an MPTCP Data Sequence Signal (DSS) option (Section 3.3) containing a DATA_ACK for the MP_CAPABLE (which is the first octet of the data sequence space).
What this means:
The current text is concerned only with ensuring a path is MPTCP capable, and so only cares that DSS option occurs on a data packet. However, the MP_CAPABLE option is defined to occupy the first octet of data sequence space and thus, if analogous to TCP, must be acknowledged. >From an implementation point of view it would make sense not to have this hanging around forever and instead define it is acknowledged at the connection level as soon as received. This change ensures the first data packet also DATA_ACKs this MP_CAPABLE octet.
Change 2
Change the sentence reading:
A Data Sequence Mapping does not need to be included in every MPTCP packet, as long as the subflow sequence space in that packet is covered by a mapping known at the receiver.
To:
The mapping provided by a Data Sequence Mapping MUST apply to some or all of the subflow sequence space in the TCP segment which carries the option. It does not need to be included in every MPTCP packet, as long as the subflow sequence space in that packet is covered by a mapping known at the receiver.
What this means:
The current text does not place any restrictions on where a mapping could appear. In theory a sender could define a thousand different mappings up front, send them all, and expect a receiver to store this and reassemble data according to these mappings as it arrives. Indeed, this was never explicitly disallowed since it “might have been useful”. The implementation community, however, has expressed concerns over the difficulty of implementing this open-endedly. How many mappings is it reasonable to store? Is there a DoS risk here? Instead, it has been requested that thee specification restricts the placement of the DSS option to being within the subflow sequence space to which it applies.
Below are my comments on this. I had shared some of these points in a previous thread that you had initiated in the same context.
Transmitting large number of non-contiguous data sequence maps could be a misbehaviour (map-flooding), though it is not clear whether this can go to the extent of causing a potential DoS to the data receiver. So some sort of restriction on this could be useful. One approach could be to insist that the data sender should ensure that the map being transmitted is for in-window data, as per the receiver advertised window. A receiver should anyhow be willing to store the maps for in-window data to deal with packet loss. For example, when a window of data segments (say 1 to 64) is transmitted, each carrying its corresponding map, and segment-1 is lost, the maps for the remaining 63 need to be stored till the lost segment is retransmitted. Of course, in this case the maps will be stored at the receiver side along with their corresponding data. But the need to store multiple maps for in-window data would still be there.
The problem with the proposed change (restriction) is that a data sender may find it difficult, in case a need arise to slightly delay the map delivery by few segments, i.e., sending some data first without map, and then send the corresponding map in a later segment, as shown below:
subflow-1: segment-1 segment-3 segment-4 segment-7
bytes:1-100 bytes:201-300 bytes:301-400 bytes:601-700
no map map for 1-100 map for 201-400 map for 601-700
subflow-2: segment-2 segment-5 segment-6 segment-8
bytes: 101-200 bytes:401-500 bytes: 501-600 bytes:701-800
map for 101-200 map for 401-600 no map map for 701-800
In the above case, segment-1 goes without map and its map is included later in segment-3, the next data segment in the same subflow. Further, in the above scheduling pattern, the map in segment-3 cannot cover the data in segment-1 and segment-3, as some data in between (segment-2) is transmitted through another subflow. With the proposed change, the map in segment-3 will become invalid and this will eventually break subflow-1, though this could be a corner case.
The question at this stage is why would segment-1 be transmitted without its map. In the case of bidirectional data transfer, there could be a need to pack both timestamp and SACK options in a data segment, i.e., piggybacking of SACK with data. If we consider that timestamp takes 12 bytes and SACK, even with single block, takes another 10 bytes, the remaining 18 bytes option space is not adequate to carry data sequence signal with map, especially when DSN is 64 bit long. So the delivery of either of the two (SACK or map) would be delayed.
As far as I understand, RFC 2018 (TCP Selective Acknowledgement Options) implies that SACK should not be delayed. It states "If sent at all, SACK options SHOULD be included in all ACKs which do not ACK the highest sequence number in the data receiver's queue". It also says "If data receiver generates SACK options under any circumstance, it SHOULD generate them under all permitted circumstances". So, as part of meeting the RFC 2018 requirements, if the combination of SACK and timestamp is given preference over DSS, data segments could be transmitted without their map.
Another case of delaying map could arise if the data sender prefers to send ADD_ADDR option, instead of map, in a data segment. It is nice that ADD_ADDR option can be delivered reliably in a pure ACK, but I think this is not the case with DSS at present.
If we adopt the proposed change, I think it might also be helpful to spell out how the receiver is supposed to behave, if it gets maps not meeting the MUST condition in the proposed change. For example termination of the subflow with MP_TCPRST option (section 3.6 in RFC 6824-bis) with appropriate reason code and T flag value to intimate the data sender the cause for subflow termination.
With regards,
Anil
Please can members of the WG express whether they are happy with these changes, or concerned.
Best regards,
Alan
_______________________________________________
multipathtcp mailing list
multipathtcp(a)ietf.org<mailto:multipathtcp@ietf.org>
https://www.ietf.org/mailman/listinfo/multipathtcp
_______________________________________________
multipathtcp mailing list
multipathtcp(a)ietf.org
https://www.ietf.org/mailman/listinfo/multipathtcp
----- End forwarded message -----
2 years, 3 months
[Weekly meetings] MoM - 30th of January 2020
by Matthieu Baerts
Hello,
Yesterday, we had our 84th meeting with Mat and Ossama (Intel OTC),
Christoph (Apple), Paolo, Florian, Davide (RedHat) and myself (Tessares).
Thanks again for this new good meeting!
Here are the minutes of the meeting:
Accepted patches:
- The list of accepted patches can be seen on PatchWork:
https://patchwork.ozlabs.org/project/mptcp/list/?state=3
- By Florian Westphal, Geert Uytterhoeven, Mat Martineau, Randy Dunlap
- All these patches have been directly sent to netdev ML, not
applied in our TopGit tree (export branch)
1229123 [net-next,2/2] mptcp: Fix code formatting
1229125 [net-next,1/2] mptcp: do not inherit inet proto ops
1231348 mptcp: Fix undefined mptcp_handle_ipv6_mapped for modular IPV6
1231046 mptcp: MPTCP_HMAC_TEST should depend on MPTCP
1231044 mptcp: Fix incorrect IPV6 dependency check
1231042 [net] Revert "MAINTAINERS: mptcp@ mailing list is moderated"
1230708 MAINTAINERS: mptcp@ mailing list is moderated
1230404 [net-next,v1,4/4] mptcp: handle tcp fallback when using syn coo
1230403 [net-next,v2,3/4] mptcp: mptcp_close: avoid a lockdep splat whe
1230402 [net-next,v4,2/4] mptcp: protocol: fix panic on user pointer ac
1230400 [net-next,v1,1/4] mptcp: protocol: defer freeing of cached ext
Pending patches:
- The list of pending patches can be seen on PatchWork:
https://patchwork.ozlabs.org/project/mptcp/list/?state=*
- By Matthieu Baerts
1196109: RFC: [10/10,RFC] selftests:mptcp: decrease timeout to 100 sec:
- note that the fix for selftests core has been applied: it now
takes into account the timeout we set.
- but we still have a bug when there is no re-ordering
FYI: Current Roadmap: (before the meeting)
- Part 1 (mainly TCP changes, will be sent with Part 2):
- Merged!
- Part 2 (minimum set for MPTCP, up to MPTCPv1 including
KSelftests, one subflow):
- Merged!
- Kernel 5.6:
- fix issues reported by users and Syzkaller
- Part 3 (to be sent when the next merge window opens)
- Full DATA_FIN support [WIP]
- Shared recv window (drop data received on other subflows) [TODO]
- Active backup support [WIP]
- Opti in TCP option structures (unions) [to be rebased)
- ADD_ADDR for MPTCPv1 [TODO]
- PM basic [WIP]
- Part 4 (to fully support MPTCP):
- Shared recv window (full support)
- IPv6 - IPv4 mapped support
- not dropping MPTCP options (ADD_ADDR, etc.)
- FAST_CLOSE
- full MPTCP v1 support (reliable add_addr, etc.)
- after a few attempts of failed MPTCP, we fallback to TCP
(like TFO is doing)
- PM server
- Part 5 (extra needed for prod):
- opti/perfs
- TFO
- PM netlink
- PM bpf
- Scheduler bpf
- syncookies
Initial submission to net-next:
- Part 2:
- v3 has been merged!
- We got more likes than Wireguard :-D
https://twitter.com/davem_dokebi/status/1220715287365943298
Kernel 5.6:
- stabilisation
- run Syzkaller
- doc?:
- userspace devs:
- maybe better to wait for the next kernel with multiple
subflows support
- kernelspace devs:
- Is it something needed? To be discussed next week
- Jakub Sitnicki new Net co-maintainer: does it change anything
for us?:
- he was already maintaining the net tree for some time, should
not change anything
Where to store patches for "net" and "net-next"?:
- a new repo?
- maybe easier with branches?: → should be fine
- fixes-net
- fixes-net-next
- → we would force push on them because we will continue to
send patches to netdev
- → if not OK, we can change that later
- these branches can be used for having a pre-review
- would be easier to have the patches on Git to run CI jobs
- we could have CI jobs on public CI (appveyor.com, travis (but
annoying to store the .yml file), etc.):
- *Matth* can look at that
About patches sent by other people directly to netdev ML:
- do we have to send ACK-by/Reviewed-By?
- for the moment we had fixes to fix compilation issues, better to
merge them directly, net maintainers will not wait for us
- for the rest, net maintainers might wait for us to review. Most
of the time, there are no explicit request except if it is for a long
time in "waiting for review" status.
- we should then try to review these patches ASAP when directly
posted to netdev from other people
Part 3 remaining stuff:
- Review commit messages:
- needed for the next merge window: not urgent
- reduce locks when mptcp_stream_accept (currently 3 locks) but we
might need to add an helper in inet API:
- Paolo started to look at that
- memleak if a request comes after a socket is closed:
- Florian plans to look at that
- PM:
- maybe need more work: locking ??? (at least check which are
the missing pieces, possibly some basic locking schema is missing)
- do we want to ship it?
- see the discussions below
- MP_JOIN self-tests:
- we need to cover this part
- packetdrill:
- latest state: https://github.com/dcaratti/packetdrill/tree/dss_v1
- there are also a couple of patches at the end of the series we
will have to rebase/squash:
- to be discussed next week
- ADD_ADDR for MPTCPv1:
- Peter plans to work on that
- Path Manager basic with netlink interface.
- Active backup support:
- Florian is working on it, patches have been shared
- DATA_FIN:
- Mat is working on it
For the next iteration:
- PM:
- locking
- basic → netlink support
- MP_JOIN
- ADD_ADDR for MPTCP v1
- clean the current patches
- need to make sure we continue to support the protocol → shared
received windows
last data packet is ACKed when the app read data:
- in Out-Of-Tree (OOT) MPTCP and iOS: data are ACKed directly
- maybe not nice to rely on the application to ACK the last one:
userspace can take time, the server can then timeout, not nice
ACK:
- in OOT MPTCP: we ACK the last in order chunk. Once it's ordered,
we ACK everything
- in OOT MPTCP, there is a single lock per connection → easier but
more intrusive to TCP
- alternative solution:
- share a pointer for the lock but would not be accepted
- a workqueue that is triggered to send a ACK
- can be useful to keep in mind: it's rare to have more than 3
subflows.
MP_JOIN:
- addr ID: it's related to the sender's address
- for the moment in the "export" branch, we mix things → TODO
ADD_ADDR echo reply:
- it's too long, would be easier with only the ID
- but (too) late to change the RFC
- this cannot fit in a "normal" packet transporting data
- we would need to send a dedicated ACK
- can be seen as a dup ACK
- (same for dedicated ADD_ADDR, RM_ADDR, etc.)
- TCP CC should ignore these dup ACK containing these MPTCP options
(according to the RFC)
- sounds not like a bit deal, it's not often it happens
Extra tests:
- news about Syzkaller? (see prev meeting):
- an instance running at Apple (Christoph)
- Christoph has modifications for Syzkaller to take into
account MPTCP
- That means syzbot will test that and report issues on netdev ML
- Christoph will send the Pull Request
- and test the end of the "export" branch
- news about Intel's kbuild? (Mat):
- they are going to test out "export" branch
- running our kselftests and more
- "export" branch is re-written every day: should be OK
Socket option whitelisting (from Mat):
- Revisit socket option whitelisting?
- In discussion of patch to allow TCP_CONGESTION, changes were
dropped so we allow all socket options to pass through to subflows.
- will we need a 'if (level == SO_MPTCP)'?
- we would like to whitelist some but later
- if we pass option to existing subflows, what happens when there
is a new subflow created: we keep the modifications in a list and we
send options to subflows
- could we pass options only to some subflows, not all
- → to be discussed next week
Path Manager: (Paolo):
- Basic but using netlink instead of sysctl
- revisiting the current pm.c
- idea would be to fix issues with the locks, then improve PM basic
to support netlink, should be compatible with OOT Netlink PM
- discussion to be continue next week or on the ML
Move the parent branch of export from net-next to net:
- if net-next is behind net for a long time, better to switch
- [after meeting note]: it seems net-next was updated during the
meeting
- we just need to resolve conflicts
Next meeting:
- We propose to have the next meeting on Thursday, the 6th of February.
- Usual time: 17:00 UTC (9am PST, 6pm CET)
- Still open to everyone!
- https://annuel2.framapad.org/p/mptcp_upstreaming_20200206
Feel free to comment on these points and propose new ones for the next
meeting!
Talk to you next week,
Matt
--
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
2 years, 3 months
[PATCH] mptcp: Fix undefined mptcp_handle_ipv6_mapped for modular IPV6
by Geert Uytterhoeven
If CONFIG_MPTCP=y, CONFIG_MPTCP_IPV6=n, and CONFIG_IPV6=m:
ERROR: "mptcp_handle_ipv6_mapped" [net/ipv6/ipv6.ko] undefined!
This does not happen if CONFIG_MPTCP_IPV6=y, as CONFIG_MPTCP_IPV6
selects CONFIG_IPV6, and thus forces CONFIG_IPV6 builtin.
As exporting a symbol for an empty function would be a bit wasteful, fix
this by providing a dummy version of mptcp_handle_ipv6_mapped() for the
CONFIG_MPTCP_IPV6=n case.
Rename mptcp_handle_ipv6_mapped() to mptcpv6_handle_mapped(), to make it
clear this is a pure-IPV6 function, just like mptcpv6_init().
Fixes: cec37a6e41aae7bf ("mptcp: Handle MP_CAPABLE options for outgoing connections")
Signed-off-by: Geert Uytterhoeven <geert(a)linux-m68k.org>
---
include/net/mptcp.h | 9 +++------
net/ipv6/tcp_ipv6.c | 6 +++---
net/mptcp/subflow.c | 6 +++---
3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 27627e2d1bc2e9a1..c971d25431ea9412 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -174,15 +174,12 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
#endif /* CONFIG_MPTCP */
-void mptcp_handle_ipv6_mapped(struct sock *sk, bool mapped);
-
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
int mptcpv6_init(void);
+void mptcpv6_handle_mapped(struct sock *sk, bool mapped);
#elif IS_ENABLED(CONFIG_IPV6)
-static inline int mptcpv6_init(void)
-{
- return 0;
-}
+static inline int mptcpv6_init(void) { return 0; }
+static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
#endif
#endif /* __NET_MPTCP_H */
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 33a578a3eb3abadb..eaf09e6b78442e09 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -239,7 +239,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
icsk->icsk_af_ops = &ipv6_mapped;
if (sk_is_mptcp(sk))
- mptcp_handle_ipv6_mapped(sk, true);
+ mptcpv6_handle_mapped(sk, true);
sk->sk_backlog_rcv = tcp_v4_do_rcv;
#ifdef CONFIG_TCP_MD5SIG
tp->af_specific = &tcp_sock_ipv6_mapped_specific;
@@ -251,7 +251,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
icsk->icsk_ext_hdr_len = exthdrlen;
icsk->icsk_af_ops = &ipv6_specific;
if (sk_is_mptcp(sk))
- mptcp_handle_ipv6_mapped(sk, false);
+ mptcpv6_handle_mapped(sk, false);
sk->sk_backlog_rcv = tcp_v6_do_rcv;
#ifdef CONFIG_TCP_MD5SIG
tp->af_specific = &tcp_sock_ipv6_specific;
@@ -1208,7 +1208,7 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
inet_csk(newsk)->icsk_af_ops = &ipv6_mapped;
if (sk_is_mptcp(newsk))
- mptcp_handle_ipv6_mapped(newsk, true);
+ mptcpv6_handle_mapped(newsk, true);
newsk->sk_backlog_rcv = tcp_v4_do_rcv;
#ifdef CONFIG_TCP_MD5SIG
newtp->af_specific = &tcp_sock_ipv6_mapped_specific;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 1662e117894900a8..de04b03fa41e70a6 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -582,9 +582,9 @@ subflow_default_af_ops(struct sock *sk)
return &subflow_specific;
}
-void mptcp_handle_ipv6_mapped(struct sock *sk, bool mapped)
-{
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+void mptcpv6_handle_mapped(struct sock *sk, bool mapped)
+{
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
struct inet_connection_sock *icsk = inet_csk(sk);
struct inet_connection_sock_af_ops *target;
@@ -599,8 +599,8 @@ void mptcp_handle_ipv6_mapped(struct sock *sk, bool mapped)
subflow->icsk_af_ops = icsk->icsk_af_ops;
icsk->icsk_af_ops = target;
-#endif
}
+#endif
int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
{
--
2.17.1
2 years, 3 months
[PATCH] mptcp: MPTCP_HMAC_TEST should depend on MPTCP
by Geert Uytterhoeven
As the MPTCP HMAC test is integrated into the MPTCP code, it can be
built only when MPTCP is enabled. Hence when MPTCP is disabled, asking
the user if the test code should be enabled is futile.
Wrap the whole block of MPTCP-specific config options inside a check for
MPTCP. While at it, drop the "default n" for MPTCP_HMAC_TEST, as that
is the default anyway.
Fixes: 65492c5a6ab5df50 ("mptcp: move from sha1 (v0) to sha256 (v1)")
Signed-off-by: Geert Uytterhoeven <geert(a)linux-m68k.org>
---
net/mptcp/Kconfig | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/mptcp/Kconfig b/net/mptcp/Kconfig
index 5db56d2218c518c8..49f6054e7f4ebc15 100644
--- a/net/mptcp/Kconfig
+++ b/net/mptcp/Kconfig
@@ -10,17 +10,19 @@ config MPTCP
uses the TCP protocol, and TCP options carry header information for
MPTCP.
+if MPTCP
+
config MPTCP_IPV6
bool "MPTCP: IPv6 support for Multipath TCP"
- depends on MPTCP
select IPV6
default y
config MPTCP_HMAC_TEST
bool "Tests for MPTCP HMAC implementation"
- default n
help
This option enable boot time self-test for the HMAC implementation
used by the MPTCP code
Say N if you are unsure.
+
+endif
--
2.17.1
2 years, 3 months
[PATCH] mptcp: Fix incorrect IPV6 dependency check
by Geert Uytterhoeven
If CONFIG_MPTCP=y, CONFIG_MPTCP_IPV6=n, and CONFIG_IPV6=m:
net/mptcp/protocol.o: In function `__mptcp_tcp_fallback':
protocol.c:(.text+0x786): undefined reference to `inet6_stream_ops'
Fix this by checking for CONFIG_MPTCP_IPV6 instead of CONFIG_IPV6, like
is done in all other places in the mptcp code.
Fixes: 8ab183deb26a3b79 ("mptcp: cope with later TCP fallback")
Signed-off-by: Geert Uytterhoeven <geert(a)linux-m68k.org>
---
net/mptcp/protocol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 39fdca79ce90137e..096dfd1074540c8a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -28,7 +28,7 @@ static void __mptcp_close(struct sock *sk, long timeout);
static const struct proto_ops *tcp_proto_ops(struct sock *sk)
{
-#if IS_ENABLED(CONFIG_IPV6)
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
if (sk->sk_family == AF_INET6)
return &inet6_stream_ops;
#endif
--
2.17.1
2 years, 3 months
[PATCH] MAINTAINERS: mptcp@ mailing list is moderated
by Randy Dunlap
From: Randy Dunlap <rdunlap(a)infradead.org>
Note that mptcp(a)lists.01.org is moderated, like we note for
other mailing lists.
Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org>
Cc: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
Cc: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Cc: netdev(a)vger.kernel.org
Cc: mptcp(a)lists.01.org
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- mmotm-2020-0128-2005.orig/MAINTAINERS
+++ mmotm-2020-0128-2005/MAINTAINERS
@@ -11718,7 +11718,7 @@ NETWORKING [MPTCP]
M: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
M: Matthieu Baerts <matthieu.baerts(a)tessares.net>
L: netdev(a)vger.kernel.org
-L: mptcp(a)lists.01.org
+L: mptcp(a)lists.01.org (moderated for non-subscribers)
W: https://github.com/multipath-tcp/mptcp_net-next/wiki
B: https://github.com/multipath-tcp/mptcp_net-next/issues
S: Maintained
2 years, 3 months
[PATCH net] Revert "MAINTAINERS: mptcp@ mailing list is moderated"
by Mat Martineau
This reverts commit 74759e1693311a8d1441de836c4080c192374238.
mptcp(a)lists.01.org accepts messages from non-subscribers. There was an
invisible and unexpected server-wide rule limiting the number of
recipients for subscribers and non-subscribers alike, and that has now
been turned off for this list.
Cc: Randy Dunlap <rdunlap(a)infradead.org>
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 58e4eb554d0e..d51ce1a0a817 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11641,7 +11641,7 @@ NETWORKING [MPTCP]
M: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
M: Matthieu Baerts <matthieu.baerts(a)tessares.net>
L: netdev(a)vger.kernel.org
-L: mptcp(a)lists.01.org (moderated for non-subscribers)
+L: mptcp(a)lists.01.org
W: https://github.com/multipath-tcp/mptcp_net-next/wiki
B: https://github.com/multipath-tcp/mptcp_net-next/issues
S: Maintained
--
2.25.0
2 years, 3 months
