All of lore.kernel.org
 help / color / mirror / Atom feed
From: Davide Caratti <dcaratti@redhat.com>
To: Mat Martineau <mathew.j.martineau@linux.intel.com>,
	Matthieu Baerts <matthieu.baerts@tessares.net>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Geliang Tang <geliangtang@gmail.com>,
	netdev@vger.kernel.org
Subject: [PATCH net-next v3] mptcp: fix length of MP_PRIO suboption
Date: Mon,  1 Feb 2021 14:05:26 +0100	[thread overview]
Message-ID: <846cdd41e6ad6ec88ef23fee1552ab39c2f5a3d1.1612184361.git.dcaratti@redhat.com> (raw)

With version 0 of the protocol it was legal to encode the 'Subflow Id' in
the MP_PRIO suboption, to specify which subflow would change its 'Backup'
flag. This has been removed from v1 specification: thus, according to RFC
8684 §3.3.8, the resulting 'Length' for MP_PRIO changed from 4 to 3 byte.

Current Linux generates / parses MP_PRIO according to the old spec, using
'Length' equal to 4, and hardcoding 1 as 'Subflow Id'; RFC compliance can
improve if we change 'Length' in other to become 3, leaving a 'Nop' after
the MP_PRIO suboption. In this way the kernel will emit and accept *only*
MP_PRIO suboptions that are compliant to version 1 of the MPTCP protocol.

 unpatched 5.11-rc kernel:
 [root@bottarga ~]# tcpdump -tnnr unpatched.pcap | grep prio
 reading from file unpatched.pcap, link-type LINUX_SLL (Linux cooked v1)
 dropped privs to tcpdump
 IP 10.0.3.2.48433 > 10.0.1.1.10006: Flags [.], ack 1, win 502, options [nop,nop,TS val 4032325513 ecr 1876514270,mptcp prio non-backup id 1,mptcp dss ack 14084896651682217737], length 0

 patched 5.11-rc kernel:
 [root@bottarga ~]# tcpdump -tnnr patched.pcap | grep prio
 reading from file patched.pcap, link-type LINUX_SLL (Linux cooked v1)
 dropped privs to tcpdump
 IP 10.0.3.2.49735 > 10.0.1.1.10006: Flags [.], ack 1, win 502, options [nop,nop,TS val 1276737699 ecr 2686399734,mptcp prio non-backup,nop,mptcp dss ack 18433038869082491686], length 0

Changes since v2:
 - when accounting for option space, don't increment 'TCPOLEN_MPTCP_PRIO'
   and use 'TCPOLEN_MPTCP_PRIO_ALIGN' instead, thanks to Matthieu Baerts.
Changes since v1:
 - refactor patch to avoid using 'TCPOLEN_MPTCP_PRIO' with its old value,
   thanks to Geliang Tang.

Fixes: 067065422fcd ("mptcp: add the outgoing MP_PRIO support")
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 net/mptcp/options.c  | 5 +++--
 net/mptcp/protocol.h | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index c9643344a8d7..17ad42c65087 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -699,10 +699,11 @@ static bool mptcp_established_options_mp_prio(struct sock *sk,
 	if (!subflow->send_mp_prio)
 		return false;
 
-	if (remaining < TCPOLEN_MPTCP_PRIO)
+	/* account for the trailing 'nop' option */
+	if (remaining < TCPOLEN_MPTCP_PRIO_ALIGN)
 		return false;
 
-	*size = TCPOLEN_MPTCP_PRIO;
+	*size = TCPOLEN_MPTCP_PRIO_ALIGN;
 	opts->suboptions |= OPTION_MPTCP_PRIO;
 	opts->backup = subflow->request_bkup;
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 1460705aaad0..07ee319f7847 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -60,7 +60,8 @@
 #define TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT	24
 #define TCPOLEN_MPTCP_PORT_LEN		4
 #define TCPOLEN_MPTCP_RM_ADDR_BASE	4
-#define TCPOLEN_MPTCP_PRIO		4
+#define TCPOLEN_MPTCP_PRIO		3
+#define TCPOLEN_MPTCP_PRIO_ALIGN	4
 #define TCPOLEN_MPTCP_FASTCLOSE		12
 
 /* MPTCP MP_JOIN flags */
-- 
2.29.2


             reply	other threads:[~2021-02-01 13:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-01 13:05 Davide Caratti [this message]
2021-02-01 15:24 ` [PATCH net-next v3] mptcp: fix length of MP_PRIO suboption Matteo Croce
2021-02-03  2:00 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=846cdd41e6ad6ec88ef23fee1552ab39c2f5a3d1.1612184361.git.dcaratti@redhat.com \
    --to=dcaratti@redhat.com \
    --cc=davem@davemloft.net \
    --cc=geliangtang@gmail.com \
    --cc=kuba@kernel.org \
    --cc=mathew.j.martineau@linux.intel.com \
    --cc=matthieu.baerts@tessares.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.