* [PATCH mptcp-net v6 1/5] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join
2021-06-25 2:48 [PATCH mptcp-net v6 0/5] Fix some mptcp syncookie process bugs wujianguo106
@ 2021-06-25 2:48 ` wujianguo106
2021-06-25 2:48 ` [PATCH mptcp-net v6 2/5] mptcp: remove redundant req destruct in subflow_check_req() wujianguo106
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: wujianguo106 @ 2021-06-25 2:48 UTC (permalink / raw)
To: mptcp; +Cc: pabeni, mathew.j.martineau
From: Jianguo Wu <wujianguo@chinatelecom.cn>
I did stress test with wrk(https://github.com/wg/wrk) and webfsd(https://github.com/ourway/webfsd)
with the assistance of mptcp-tools(https://github.com/pabeni/mptcp-tools):
Server side:
./use_mptcp.sh webfsd -4 -R /tmp/ -p 8099
Client side:
./use_mptcp.sh wrk -c 200 -d 30 -t 4 http://192.168.174.129:8099/
and got the following warning message:
[ 55.552626] TCP: request_sock_subflow: Possible SYN flooding on port 8099. Sending cookies. Check SNMP counters.
[ 55.553024] ------------[ cut here ]------------
[ 55.553027] WARNING: CPU: 0 PID: 10 at net/core/flow_dissector.c:984 __skb_flow_dissect+0x280/0x1650
...
[ 55.553117] CPU: 0 PID: 10 Comm: ksoftirqd/0 Not tainted 5.12.0+ #18
[ 55.553121] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020
[ 55.553124] RIP: 0010:__skb_flow_dissect+0x280/0x1650
...
[ 55.553133] RSP: 0018:ffffb79580087770 EFLAGS: 00010246
[ 55.553137] RAX: 0000000000000000 RBX: ffffffff8ddb58e0 RCX: ffffb79580087888
[ 55.553139] RDX: ffffffff8ddb58e0 RSI: ffff8f7e4652b600 RDI: 0000000000000000
[ 55.553141] RBP: ffffb79580087858 R08: 0000000000000000 R09: 0000000000000008
[ 55.553143] R10: 000000008c622965 R11: 00000000d3313a5b R12: ffff8f7e4652b600
[ 55.553146] R13: ffff8f7e465c9062 R14: 0000000000000000 R15: ffffb79580087888
[ 55.553149] FS: 0000000000000000(0000) GS:ffff8f7f75e00000(0000) knlGS:0000000000000000
[ 55.553152] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 55.553154] CR2: 00007f73d1d19000 CR3: 0000000135e10004 CR4: 00000000003706f0
[ 55.553160] Call Trace:
[ 55.553166] ? __sha256_final+0x67/0xd0
[ 55.553173] ? sha256+0x7e/0xa0
[ 55.553177] __skb_get_hash+0x57/0x210
[ 55.553182] subflow_init_req_cookie_join_save+0xac/0xc0
[ 55.553189] subflow_check_req+0x474/0x550
[ 55.553195] ? ip_route_output_key_hash+0x67/0x90
[ 55.553200] ? xfrm_lookup_route+0x1d/0xa0
[ 55.553207] subflow_v4_route_req+0x8e/0xd0
[ 55.553212] tcp_conn_request+0x31e/0xab0
[ 55.553218] ? selinux_socket_sock_rcv_skb+0x116/0x210
[ 55.553224] ? tcp_rcv_state_process+0x179/0x6d0
[ 55.553229] tcp_rcv_state_process+0x179/0x6d0
[ 55.553235] tcp_v4_do_rcv+0xaf/0x220
[ 55.553239] tcp_v4_rcv+0xce4/0xd80
[ 55.553243] ? ip_route_input_rcu+0x246/0x260
[ 55.553248] ip_protocol_deliver_rcu+0x35/0x1b0
[ 55.553253] ip_local_deliver_finish+0x44/0x50
[ 55.553258] ip_local_deliver+0x6c/0x110
[ 55.553262] ? ip_rcv_finish_core.isra.19+0x5a/0x400
[ 55.553267] ip_rcv+0xd1/0xe0
...
After debugging, I found in __skb_flow_dissect(), skb->dev and skb->sk are both NULL,
then net is NULL, and trigger WARN_ON_ONCE(!net), actually net is always NULL in this
code path, as skb->dev is set to NULL in tcp_v4_rcv(), and skb->sk is never set.
Code snippet in __skb_flow_dissect() that trigger warning:
975 if (skb) {
976 if (!net) {
977 if (skb->dev)
978 net = dev_net(skb->dev);
979 else if (skb->sk)
980 net = sock_net(skb->sk);
981 }
982 }
983
984 WARN_ON_ONCE(!net);
So, using seq and transport header derived hash.
Fixes: 9466a1ccebbe("mptcp: enable JOIN requests even if cookies are in use").
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/mptcp/syncookies.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
index abe0fd099746..37127781aee9 100644
--- a/net/mptcp/syncookies.c
+++ b/net/mptcp/syncookies.c
@@ -37,7 +37,21 @@ struct join_entry {
static u32 mptcp_join_entry_hash(struct sk_buff *skb, struct net *net)
{
- u32 i = skb_get_hash(skb) ^ net_hash_mix(net);
+ static u32 mptcp_join_hash_secret __read_mostly;
+ struct tcphdr *th = tcp_hdr(skb);
+ u32 seq, i;
+
+ net_get_random_once(&mptcp_join_hash_secret,
+ sizeof(mptcp_join_hash_secret));
+
+ if (th->syn)
+ seq = TCP_SKB_CB(skb)->seq;
+ else
+ seq = TCP_SKB_CB(skb)->seq - 1;
+
+ i = jhash_3words(seq, net_hash_mix(net),
+ (__force __u32)th->source << 16 | (__force __u32)th->dest,
+ mptcp_join_hash_secret);
return i % ARRAY_SIZE(join_entries);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH mptcp-net v6 2/5] mptcp: remove redundant req destruct in subflow_check_req()
2021-06-25 2:48 [PATCH mptcp-net v6 0/5] Fix some mptcp syncookie process bugs wujianguo106
2021-06-25 2:48 ` [PATCH mptcp-net v6 1/5] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join wujianguo106
@ 2021-06-25 2:48 ` wujianguo106
2021-06-25 2:48 ` [PATCH mptcp-net v6 3/5] mptcp: fix syncookie process if mptcp can not_accept new subflow wujianguo106
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: wujianguo106 @ 2021-06-25 2:48 UTC (permalink / raw)
To: mptcp; +Cc: pabeni, mathew.j.martineau
From: Jianguo Wu <wujianguo@chinatelecom.cn>
In subflow_check_req(), if subflow sport is mismatch, will put msk,
destroy token, and destruct req, then return -EPERM, which can be
done by subflow_req_destructor() via:
tcp_conn_request()
|--__reqsk_free()
|--subflow_req_destructor()
So we should remove these redundant code, otherwise will call
tcp_v4_reqsk_destructor() twice, and may double free inet_rsk(req)->ireq_opt.
Fixes: 5bc56388c74f ("mptcp: add port number check for MP_JOIN")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/mptcp/subflow.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 6b1cd4257edf..75ed530706c0 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -213,11 +213,6 @@ static int subflow_check_req(struct request_sock *req,
ntohs(inet_sk(sk_listener)->inet_sport),
ntohs(inet_sk((struct sock *)subflow_req->msk)->inet_sport));
if (!mptcp_pm_sport_in_anno_list(subflow_req->msk, sk_listener)) {
- sock_put((struct sock *)subflow_req->msk);
- mptcp_token_destroy_request(req);
- tcp_request_sock_ops.destructor(req);
- subflow_req->msk = NULL;
- subflow_req->mp_join = 0;
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MISMATCHPORTSYNRX);
return -EPERM;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH mptcp-net v6 3/5] mptcp: fix syncookie process if mptcp can not_accept new subflow
2021-06-25 2:48 [PATCH mptcp-net v6 0/5] Fix some mptcp syncookie process bugs wujianguo106
2021-06-25 2:48 ` [PATCH mptcp-net v6 1/5] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join wujianguo106
2021-06-25 2:48 ` [PATCH mptcp-net v6 2/5] mptcp: remove redundant req destruct in subflow_check_req() wujianguo106
@ 2021-06-25 2:48 ` wujianguo106
2021-06-25 2:48 ` [PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset wujianguo106
2021-06-25 2:48 ` [PATCH mptcp-net v6 5/5] selftests: mptcp: update case multiple subflows limited by server wujianguo106
4 siblings, 0 replies; 9+ messages in thread
From: wujianguo106 @ 2021-06-25 2:48 UTC (permalink / raw)
To: mptcp; +Cc: pabeni, mathew.j.martineau
From: Jianguo Wu <wujianguo@chinatelecom.cn>
Lots of "TCP: tcp_fin: Impossible, sk->sk_state=7" in client side
when doing stress testing using wrk and webfsd.
There are at least two cases may trigger this warning:
1.mptcp is in syncookie, and server recv MP_JOIN SYN request,
in subflow_check_req(), the mptcp_can_accept_new_subflow()
return false, so subflow_init_req_cookie_join_save() isn't
called, i.e. not store the data present in the MP_JOIN syn
request and the random nonce in hash table - join_entries[],
but still send synack. When recv 3rd-ack,
mptcp_token_join_cookie_init_state() will return false, and
3rd-ack is dropped, then if mptcp conn is closed by client,
client will send a DATA_FIN and a MPTCP FIN, the DATA_FIN
doesn't have MP_CAPABLE or MP_JOIN,
so mptcp_subflow_init_cookie_req() will return 0, and pass
the cookie check, MP_JOIN request is fallback to normal TCP.
Server will send a TCP FIN if closed, in client side,
when process TCP FIN, it will do reset, the code path is:
tcp_data_queue()->mptcp_incoming_options()
->check_fully_established()->mptcp_subflow_reset().
mptcp_subflow_reset() will set sock state to TCP_CLOSE,
so tcp_fin will hit TCP_CLOSE, and print the warning.
2.mptcp is in syncookie, and server recv 3rd-ack, in
mptcp_subflow_init_cookie_req(), mptcp_can_accept_new_subflow()
return false, and subflow_req->mp_join is not set to 1,
so in subflow_syn_recv_sock() will not reset the MP_JOIN
subflow, but fallback to normal TCP, and then the same thing
happens when server will send a TCP FIN if closed.
For case1, subflow_check_req() return -EPERM,
then tcp_conn_request() will drop MP_JOIN SYN.
For case2, let subflow_syn_recv_sock() call mptcp_can_accept_new_subflow(),
and do fatal fallback, send reset.
Fixes: 9466a1ccebbe("mptcp: enable JOIN requests even if cookies are in use")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/mptcp/subflow.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 75ed530706c0..6d98e19a20aa 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -224,6 +224,8 @@ static int subflow_check_req(struct request_sock *req,
if (unlikely(req->syncookie)) {
if (mptcp_can_accept_new_subflow(subflow_req->msk))
subflow_init_req_cookie_join_save(subflow_req, skb);
+ else
+ return -EPERM;
}
pr_debug("token=%u, remote_nonce=%u msk=%p", subflow_req->token,
@@ -263,9 +265,7 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req,
if (!mptcp_token_join_cookie_init_state(subflow_req, skb))
return -EINVAL;
- if (mptcp_can_accept_new_subflow(subflow_req->msk))
- subflow_req->mp_join = 1;
-
+ subflow_req->mp_join = 1;
subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq - 1;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset
2021-06-25 2:48 [PATCH mptcp-net v6 0/5] Fix some mptcp syncookie process bugs wujianguo106
` (2 preceding siblings ...)
2021-06-25 2:48 ` [PATCH mptcp-net v6 3/5] mptcp: fix syncookie process if mptcp can not_accept new subflow wujianguo106
@ 2021-06-25 2:48 ` wujianguo106
2021-06-25 6:40 ` kernel test robot
2021-06-25 2:48 ` [PATCH mptcp-net v6 5/5] selftests: mptcp: update case multiple subflows limited by server wujianguo106
4 siblings, 1 reply; 9+ messages in thread
From: wujianguo106 @ 2021-06-25 2:48 UTC (permalink / raw)
To: mptcp; +Cc: pabeni, mathew.j.martineau
From: Jianguo Wu <wujianguo@chinatelecom.cn>
If check_fully_established() causes a subflow reset, it should not
continue to process the packet in tcp_data_queue().
Add a return value to mptcp_incoming_options(), and return 0 if a
subflow has been reset, else return 1. Then drop the packet in
tcp_data_queue()/tcp_rcv_state_process() if mptcp_incoming_options()
return 0.
Fixes: d582484726c4 ("mptcp: fix fallback for MP_JOIN subflows")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
include/net/mptcp.h | 2 +-
net/ipv4/tcp_input.c | 19 +++++++++++++++----
net/mptcp/options.c | 22 ++++++++++++++--------
3 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index d61bbbf11979..b32333f95738 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -104,7 +104,7 @@ bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
unsigned int *size, unsigned int remaining,
struct mptcp_out_options *opts);
-void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb);
+int mptcp_incoming_options(struct sock *sk, struct sk_buff *skb);
void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
struct mptcp_out_options *opts);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 7d5e59f688de..4bacd7d2abd7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4247,6 +4247,9 @@ void tcp_reset(struct sock *sk, struct sk_buff *skb)
{
trace_tcp_receive_reset(sk);
+ /* mptcp can't tell us to ignore reset pkts,
+ * so just ignore the return value of mptcp_incoming_options().
+ */
if (sk_is_mptcp(sk))
mptcp_incoming_options(sk, skb);
@@ -4941,8 +4944,13 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
bool fragstolen;
int eaten;
- if (sk_is_mptcp(sk))
- mptcp_incoming_options(sk, skb);
+ /* If a subflow has been reset, the packet should not continue
+ * to be processed, drop the packet.
+ */
+ if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
+ __kfree_skb(skb);
+ return;
+ }
if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
__kfree_skb(skb);
@@ -6523,8 +6531,11 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
case TCP_CLOSING:
case TCP_LAST_ACK:
if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
- if (sk_is_mptcp(sk))
- mptcp_incoming_options(sk, skb);
+ /* If a subflow has been reset, the packet should not
+ * continue to be processed, drop the packet.
+ */
+ if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb))
+ goto discard;
break;
}
fallthrough;
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 1aec01686c1a..8a386556a23c 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -847,7 +847,8 @@ bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
struct mptcp_subflow_context *subflow,
struct sk_buff *skb,
- struct mptcp_options_received *mp_opt)
+ struct mptcp_options_received *mp_opt,
+ bool *subflow_is_rst)
{
/* here we can process OoO, in-window pkts, only in-sequence 4th ack
* will make the subflow fully established
@@ -926,6 +927,7 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
return true;
reset:
+ *subflow_is_rst = true;
mptcp_subflow_reset(ssk);
return false;
}
@@ -1022,12 +1024,14 @@ static bool add_addr_hmac_valid(struct mptcp_sock *msk,
return hmac == mp_opt->ahmac;
}
-void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
+/* Return 0 if a subflow has been reset, else return 1 */
+int mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
{
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
struct mptcp_sock *msk = mptcp_sk(subflow->conn);
struct mptcp_options_received mp_opt;
struct mptcp_ext *mpext;
+ bool subflow_is_rst = false;
if (__mptcp_check_fallback(msk)) {
/* Keep it simple and unconditionally trigger send data cleanup and
@@ -1040,12 +1044,12 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
__mptcp_check_push(subflow->conn, sk);
__mptcp_data_acked(subflow->conn);
mptcp_data_unlock(subflow->conn);
- return;
+ return 1;
}
mptcp_get_options(sk, skb, &mp_opt);
- if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
- return;
+ if (!check_fully_established(msk, sk, subflow, skb, &mp_opt, &subflow_is_rst))
+ return subflow_is_rst ? 0 : 1;
if (mp_opt.fastclose &&
msk->local_key == mp_opt.rcvr_key) {
@@ -1087,7 +1091,7 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
}
if (!mp_opt.dss)
- return;
+ return 1;
/* we can't wait for recvmsg() to update the ack_seq, otherwise
* monodirectional flows will stuck
@@ -1106,12 +1110,12 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
schedule_work(&msk->work))
sock_hold(subflow->conn);
- return;
+ return 1;
}
mpext = skb_ext_add(skb, SKB_EXT_MPTCP);
if (!mpext)
- return;
+ return 1;
memset(mpext, 0, sizeof(*mpext));
@@ -1140,6 +1144,8 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
if (mpext->csum_reqd)
mpext->csum = mp_opt.csum;
}
+
+ return 1;
}
static void mptcp_set_rwin(const struct tcp_sock *tp)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset
2021-06-25 2:48 ` [PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset wujianguo106
@ 2021-06-25 6:40 ` kernel test robot
2021-06-25 7:33 ` Jianguo Wu
0 siblings, 1 reply; 9+ messages in thread
From: kernel test robot @ 2021-06-25 6:40 UTC (permalink / raw)
To: wujianguo106, mptcp; +Cc: kbuild-all, pabeni, mathew.j.martineau
[-- Attachment #1: Type: text/plain, Size: 5448 bytes --]
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on mptcp/export]
[also build test ERROR on next-20210624]
[cannot apply to kselftest/next linus/master v5.13-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/wujianguo106-163-com/Fix-some-mptcp-syncookie-process-bugs/20210625-110557
base: https://github.com/multipath-tcp/mptcp_net-next.git export
config: i386-randconfig-a011-20210622 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/8bac278b64d6949fede4d0f383ca5f01fca11ef7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review wujianguo106-163-com/Fix-some-mptcp-syncookie-process-bugs/20210625-110557
git checkout 8bac278b64d6949fede4d0f383ca5f01fca11ef7
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/ipv4/tcp_input.c: In function 'tcp_data_queue':
>> net/ipv4/tcp_input.c:4950:25: error: invalid use of void expression
4950 | if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
| ^
net/ipv4/tcp_input.c: In function 'tcp_rcv_state_process':
net/ipv4/tcp_input.c:6537:27: error: invalid use of void expression
6537 | if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb))
| ^
vim +4950 net/ipv4/tcp_input.c
4940
4941 static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
4942 {
4943 struct tcp_sock *tp = tcp_sk(sk);
4944 bool fragstolen;
4945 int eaten;
4946
4947 /* If a subflow has been reset, the packet should not continue
4948 * to be processed, drop the packet.
4949 */
> 4950 if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
4951 __kfree_skb(skb);
4952 return;
4953 }
4954
4955 if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
4956 __kfree_skb(skb);
4957 return;
4958 }
4959 skb_dst_drop(skb);
4960 __skb_pull(skb, tcp_hdr(skb)->doff * 4);
4961
4962 tp->rx_opt.dsack = 0;
4963
4964 /* Queue data for delivery to the user.
4965 * Packets in sequence go to the receive queue.
4966 * Out of sequence packets to the out_of_order_queue.
4967 */
4968 if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
4969 if (tcp_receive_window(tp) == 0) {
4970 NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
4971 goto out_of_window;
4972 }
4973
4974 /* Ok. In sequence. In window. */
4975 queue_and_out:
4976 if (skb_queue_len(&sk->sk_receive_queue) == 0)
4977 sk_forced_mem_schedule(sk, skb->truesize);
4978 else if (tcp_try_rmem_schedule(sk, skb, skb->truesize)) {
4979 NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
4980 sk->sk_data_ready(sk);
4981 goto drop;
4982 }
4983
4984 eaten = tcp_queue_rcv(sk, skb, &fragstolen);
4985 if (skb->len)
4986 tcp_event_data_recv(sk, skb);
4987 if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
4988 tcp_fin(sk);
4989
4990 if (!RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
4991 tcp_ofo_queue(sk);
4992
4993 /* RFC5681. 4.2. SHOULD send immediate ACK, when
4994 * gap in queue is filled.
4995 */
4996 if (RB_EMPTY_ROOT(&tp->out_of_order_queue))
4997 inet_csk(sk)->icsk_ack.pending |= ICSK_ACK_NOW;
4998 }
4999
5000 if (tp->rx_opt.num_sacks)
5001 tcp_sack_remove(tp);
5002
5003 tcp_fast_path_check(sk);
5004
5005 if (eaten > 0)
5006 kfree_skb_partial(skb, fragstolen);
5007 if (!sock_flag(sk, SOCK_DEAD))
5008 tcp_data_ready(sk);
5009 return;
5010 }
5011
5012 if (!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt)) {
5013 tcp_rcv_spurious_retrans(sk, skb);
5014 /* A retransmit, 2nd most common case. Force an immediate ack. */
5015 NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
5016 tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
5017
5018 out_of_window:
5019 tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
5020 inet_csk_schedule_ack(sk);
5021 drop:
5022 tcp_drop(sk, skb);
5023 return;
5024 }
5025
5026 /* Out of window. F.e. zero window probe. */
5027 if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
5028 goto out_of_window;
5029
5030 if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
5031 /* Partial packet, seq < rcv_next < end_seq */
5032 tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, tp->rcv_nxt);
5033
5034 /* If window is closed, drop tail of packet. But after
5035 * remembering D-SACK for its head made in previous line.
5036 */
5037 if (!tcp_receive_window(tp)) {
5038 NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
5039 goto out_of_window;
5040 }
5041 goto queue_and_out;
5042 }
5043
5044 tcp_data_queue_ofo(sk, skb);
5045 }
5046
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47885 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset
2021-06-25 6:40 ` kernel test robot
@ 2021-06-25 7:33 ` Jianguo Wu
0 siblings, 0 replies; 9+ messages in thread
From: Jianguo Wu @ 2021-06-25 7:33 UTC (permalink / raw)
To: kernel test robot, mptcp; +Cc: kbuild-all, pabeni, mathew.j.martineau
Sorry for the mistake, please ignore this series, I will send v7.
On 2021/6/25 14:40, kernel test robot wrote:
> Hi,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on mptcp/export]
> [also build test ERROR on next-20210624]
> [cannot apply to kselftest/next linus/master v5.13-rc7]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/wujianguo106-163-com/Fix-some-mptcp-syncookie-process-bugs/20210625-110557
> base: https://github.com/multipath-tcp/mptcp_net-next.git export
> config: i386-randconfig-a011-20210622 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
> # https://github.com/0day-ci/linux/commit/8bac278b64d6949fede4d0f383ca5f01fca11ef7
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review wujianguo106-163-com/Fix-some-mptcp-syncookie-process-bugs/20210625-110557
> git checkout 8bac278b64d6949fede4d0f383ca5f01fca11ef7
> # save the attached .config to linux build tree
> make W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> net/ipv4/tcp_input.c: In function 'tcp_data_queue':
>>> net/ipv4/tcp_input.c:4950:25: error: invalid use of void expression
> 4950 | if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
> | ^
> net/ipv4/tcp_input.c: In function 'tcp_rcv_state_process':
> net/ipv4/tcp_input.c:6537:27: error: invalid use of void expression
> 6537 | if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb))
> | ^
>
>
> vim +4950 net/ipv4/tcp_input.c
>
> 4940
> 4941 static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
> 4942 {
> 4943 struct tcp_sock *tp = tcp_sk(sk);
> 4944 bool fragstolen;
> 4945 int eaten;
> 4946
> 4947 /* If a subflow has been reset, the packet should not continue
> 4948 * to be processed, drop the packet.
> 4949 */
>> 4950 if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
> 4951 __kfree_skb(skb);
> 4952 return;
> 4953 }
> 4954
> 4955 if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
> 4956 __kfree_skb(skb);
> 4957 return;
> 4958 }
> 4959 skb_dst_drop(skb);
> 4960 __skb_pull(skb, tcp_hdr(skb)->doff * 4);
> 4961
> 4962 tp->rx_opt.dsack = 0;
> 4963
> 4964 /* Queue data for delivery to the user.
> 4965 * Packets in sequence go to the receive queue.
> 4966 * Out of sequence packets to the out_of_order_queue.
> 4967 */
> 4968 if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
> 4969 if (tcp_receive_window(tp) == 0) {
> 4970 NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
> 4971 goto out_of_window;
> 4972 }
> 4973
> 4974 /* Ok. In sequence. In window. */
> 4975 queue_and_out:
> 4976 if (skb_queue_len(&sk->sk_receive_queue) == 0)
> 4977 sk_forced_mem_schedule(sk, skb->truesize);
> 4978 else if (tcp_try_rmem_schedule(sk, skb, skb->truesize)) {
> 4979 NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
> 4980 sk->sk_data_ready(sk);
> 4981 goto drop;
> 4982 }
> 4983
> 4984 eaten = tcp_queue_rcv(sk, skb, &fragstolen);
> 4985 if (skb->len)
> 4986 tcp_event_data_recv(sk, skb);
> 4987 if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
> 4988 tcp_fin(sk);
> 4989
> 4990 if (!RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
> 4991 tcp_ofo_queue(sk);
> 4992
> 4993 /* RFC5681. 4.2. SHOULD send immediate ACK, when
> 4994 * gap in queue is filled.
> 4995 */
> 4996 if (RB_EMPTY_ROOT(&tp->out_of_order_queue))
> 4997 inet_csk(sk)->icsk_ack.pending |= ICSK_ACK_NOW;
> 4998 }
> 4999
> 5000 if (tp->rx_opt.num_sacks)
> 5001 tcp_sack_remove(tp);
> 5002
> 5003 tcp_fast_path_check(sk);
> 5004
> 5005 if (eaten > 0)
> 5006 kfree_skb_partial(skb, fragstolen);
> 5007 if (!sock_flag(sk, SOCK_DEAD))
> 5008 tcp_data_ready(sk);
> 5009 return;
> 5010 }
> 5011
> 5012 if (!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt)) {
> 5013 tcp_rcv_spurious_retrans(sk, skb);
> 5014 /* A retransmit, 2nd most common case. Force an immediate ack. */
> 5015 NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
> 5016 tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
> 5017
> 5018 out_of_window:
> 5019 tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
> 5020 inet_csk_schedule_ack(sk);
> 5021 drop:
> 5022 tcp_drop(sk, skb);
> 5023 return;
> 5024 }
> 5025
> 5026 /* Out of window. F.e. zero window probe. */
> 5027 if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
> 5028 goto out_of_window;
> 5029
> 5030 if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
> 5031 /* Partial packet, seq < rcv_next < end_seq */
> 5032 tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, tp->rcv_nxt);
> 5033
> 5034 /* If window is closed, drop tail of packet. But after
> 5035 * remembering D-SACK for its head made in previous line.
> 5036 */
> 5037 if (!tcp_receive_window(tp)) {
> 5038 NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
> 5039 goto out_of_window;
> 5040 }
> 5041 goto queue_and_out;
> 5042 }
> 5043
> 5044 tcp_data_queue_ofo(sk, skb);
> 5045 }
> 5046
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH mptcp-net v6 5/5] selftests: mptcp: update case multiple subflows limited by server
2021-06-25 2:48 [PATCH mptcp-net v6 0/5] Fix some mptcp syncookie process bugs wujianguo106
` (3 preceding siblings ...)
2021-06-25 2:48 ` [PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset wujianguo106
@ 2021-06-25 2:48 ` wujianguo106
2021-06-25 3:24 ` Jianguo Wu
4 siblings, 1 reply; 9+ messages in thread
From: wujianguo106 @ 2021-06-25 2:48 UTC (permalink / raw)
To: mptcp; +Cc: pabeni, mathew.j.martineau
From: Jianguo Wu <wujianguo@chinatelecom.cn>
After patch "mptcp: fix syncookie process if mptcp can not_accept new subflow",
If a new subflow is limited, MP_JOIN SYN is dropped, and no SYN/ACK will be replied.
So in case "multiple subflows limited by server", the expected SYN/ACK number
should be 1.
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 523c7797f30a..37b7da3cd5ca 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1398,7 +1398,7 @@ syncookies_tests()
ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
run_tests $ns1 $ns2 10.0.1.1
- chk_join_nr "subflows limited by server w cookies" 2 2 1
+ chk_join_nr "subflows limited by server w cookies" 2 1 1
# test signal address with cookies
reset_with_cookies
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-net v6 5/5] selftests: mptcp: update case multiple subflows limited by server
2021-06-25 2:48 ` [PATCH mptcp-net v6 5/5] selftests: mptcp: update case multiple subflows limited by server wujianguo106
@ 2021-06-25 3:24 ` Jianguo Wu
0 siblings, 0 replies; 9+ messages in thread
From: Jianguo Wu @ 2021-06-25 3:24 UTC (permalink / raw)
To: mptcp; +Cc: pabeni, mathew.j.martineau
On 2021/6/25 10:48, wujianguo106@163.com wrote:
> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>
> After patch "mptcp: fix syncookie process if mptcp can not_accept new subflow",
> If a new subflow is limited, MP_JOIN SYN is dropped, and no SYN/ACK will be replied.
> So in case "multiple subflows limited by server", the expected SYN/ACK number
> should be 1.
>
Sorry, forgot to add:
Reported-by: kernel test robot <oliver.sang@intel.com>
> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 523c7797f30a..37b7da3cd5ca 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1398,7 +1398,7 @@ syncookies_tests()
> ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
> ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
> run_tests $ns1 $ns2 10.0.1.1
> - chk_join_nr "subflows limited by server w cookies" 2 2 1
> + chk_join_nr "subflows limited by server w cookies" 2 1 1
>
> # test signal address with cookies
> reset_with_cookies
>
^ permalink raw reply [flat|nested] 9+ messages in thread