[PATCH net-next 0/2] mptcp: Header fixups

[PATCH net-next 0/2] mptcp: Header fixups

[Mat Martineau]

Patch 1 makes the linux/mptcp.h header easier to use in userspace.

Patch 2 cleans up a magic number.


Geliang Tang (1):
  mptcp: move MPTCPOPT_HMAC_LEN to net/mptcp.h

Ossama Othman (1):
  mptcp: fix conflict with <netinet/in.h>

 include/net/mptcp.h        | 3 ++-
 include/uapi/linux/mptcp.h | 9 +++++----
 net/mptcp/protocol.h       | 1 -
 3 files changed, 7 insertions(+), 6 deletions(-)


base-commit: a84a434baf9427a1c49782fb1f0973d1308016df
-- 
2.36.1

[PATCH net-next 1/2] mptcp: fix conflict with <netinet/in.h>

[Mat Martineau]

From: Ossama Othman <ossama.othman@intel.com>

Including <linux/mptcp.h> before the C library <netinet/in.h> header
causes symbol redefinition errors at compile-time due to duplicate
declarations and definitions in the <linux/in.h> header included by
<linux/mptcp.h>.

Explicitly include <netinet/in.h> before <linux/in.h> in
<linux/mptcp.h> when __KERNEL__ is not defined so that the C library
compatibility logic in <linux/libc-compat.h> is enabled when including
<linux/mptcp.h> in user space code.

Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support")
Signed-off-by: Ossama Othman <ossama.othman@intel.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/uapi/linux/mptcp.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 921963589904..dfe19bf13f4c 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -2,16 +2,17 @@
 #ifndef _UAPI_MPTCP_H
 #define _UAPI_MPTCP_H
 
+#ifndef __KERNEL__
+#include <netinet/in.h>		/* for sockaddr_in and sockaddr_in6	*/
+#include <sys/socket.h>		/* for struct sockaddr			*/
+#endif
+
 #include <linux/const.h>
 #include <linux/types.h>
 #include <linux/in.h>		/* for sockaddr_in			*/
 #include <linux/in6.h>		/* for sockaddr_in6			*/
 #include <linux/socket.h>	/* for sockaddr_storage and sa_family	*/
 
-#ifndef __KERNEL__
-#include <sys/socket.h>		/* for struct sockaddr			*/
-#endif
-
 #define MPTCP_SUBFLOW_FLAG_MCAP_REM		_BITUL(0)
 #define MPTCP_SUBFLOW_FLAG_MCAP_LOC		_BITUL(1)
 #define MPTCP_SUBFLOW_FLAG_JOIN_REM		_BITUL(2)
-- 
2.36.1

[PATCH net-next 2/2] mptcp: move MPTCPOPT_HMAC_LEN to net/mptcp.h

[Mat Martineau]

From: Geliang Tang <geliang.tang@suse.com>

Move macro MPTCPOPT_HMAC_LEN definition from net/mptcp/protocol.h to
include/net/mptcp.h.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/net/mptcp.h  | 3 ++-
 net/mptcp/protocol.h | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 4d761ad530c9..ac9cf7271d46 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -39,6 +39,7 @@ struct mptcp_ext {
 			infinite_map:1;
 };
 
+#define MPTCPOPT_HMAC_LEN	20
 #define MPTCP_RM_IDS_MAX	8
 
 struct mptcp_rm_list {
@@ -89,7 +90,7 @@ struct mptcp_out_options {
 			u32 nonce;
 			u32 token;
 			u64 thmac;
-			u8 hmac[20];
+			u8 hmac[MPTCPOPT_HMAC_LEN];
 		};
 	};
 #endif
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 200f89f6d62f..8f03775a2f22 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -83,7 +83,6 @@
 
 /* MPTCP MP_JOIN flags */
 #define MPTCPOPT_BACKUP		BIT(0)
-#define MPTCPOPT_HMAC_LEN	20
 #define MPTCPOPT_THMAC_LEN	8
 
 /* MPTCP MP_CAPABLE flags */
-- 
2.36.1

Re: [PATCH net-next 1/2] mptcp: fix conflict with <netinet/in.h>

[Jakub Kicinski]

On Wed,  8 Jun 2022 12:19:18 -0700 Mat Martineau wrote:
> From: Ossama Othman <ossama.othman@intel.com>
> 
> Including <linux/mptcp.h> before the C library <netinet/in.h> header
> causes symbol redefinition errors at compile-time due to duplicate
> declarations and definitions in the <linux/in.h> header included by
> <linux/mptcp.h>.
> 
> Explicitly include <netinet/in.h> before <linux/in.h> in
> <linux/mptcp.h> when __KERNEL__ is not defined so that the C library
> compatibility logic in <linux/libc-compat.h> is enabled when including
> <linux/mptcp.h> in user space code.
> 
> Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support")

What does it break, tho? The commit under Fixes is in net, if it's
really a fix it needs to go to net. If it's just prep for another
change we don't need to fixes tag.

Re: [PATCH net-next 1/2] mptcp: fix conflict with <netinet/in.h>

[Mat Martineau]

On Thu, 9 Jun 2022, Jakub Kicinski wrote:

> On Wed,  8 Jun 2022 12:19:18 -0700 Mat Martineau wrote:
>> From: Ossama Othman <ossama.othman@intel.com>
>>
>> Including <linux/mptcp.h> before the C library <netinet/in.h> header
>> causes symbol redefinition errors at compile-time due to duplicate
>> declarations and definitions in the <linux/in.h> header included by
>> <linux/mptcp.h>.
>>
>> Explicitly include <netinet/in.h> before <linux/in.h> in
>> <linux/mptcp.h> when __KERNEL__ is not defined so that the C library
>> compatibility logic in <linux/libc-compat.h> is enabled when including
>> <linux/mptcp.h> in user space code.
>>
>> Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support")
>
> What does it break, tho? The commit under Fixes is in net, if it's
> really a fix it needs to go to net. If it's just prep for another
> change we don't need to fixes tag.
>

Hi Jakub -

This is a minor "fix" to be sure, which I thought did not meet the bar for 
net and therefore submitted for net-next. It's not prep for another 
change, it's something Ossama and I noticed when doing code review for a 
userspace program that included the header. There's no problem with kernel 
compilation, and there's also no issue if the userspace program happens to 
include netinet/in.h before linux/mptcp.h


If my threshold for the net branch is too high, I have no objection to 
having this patch applied there and will recalibrate :)

Do you prefer to have no Fixes: tags in net-next, or did that just seem 
ambiguous in this case?

--
Mat Martineau
Intel

Re: [PATCH net-next 1/2] mptcp: fix conflict with <netinet/in.h>

[Jakub Kicinski]

On Fri, 10 Jun 2022 11:00:28 -0700 (PDT) Mat Martineau wrote:
> This is a minor "fix" to be sure, which I thought did not meet the bar for 
> net and therefore submitted for net-next. It's not prep for another 
> change, it's something Ossama and I noticed when doing code review for a 
> userspace program that included the header. There's no problem with kernel 
> compilation, and there's also no issue if the userspace program happens to 
> include netinet/in.h before linux/mptcp.h
> 
> 
> If my threshold for the net branch is too high, I have no objection to 
> having this patch applied there and will recalibrate :)
> 
> Do you prefer to have no Fixes: tags in net-next, or did that just seem 
> ambiguous in this case?

The important point is that the middle ground of marking things as fixes
and at the same time putting them in -next, to still get them
backported but with an extended settling time -- that middle ground
does not exist.

If we look at the patch from the "do we want it backported or not"
perspective I think the answer is yes, hence I'd lean towards net.
If you think it doesn't matter enough for backport - we can drop the
fixes tag and go with net-next. Unfortunately I don't have enough
direct experience to tell how annoying this will be to the user space.
netinet/in.h vs linux/in.h is a mess :(

Re: [PATCH net-next 1/2] mptcp: fix conflict with <netinet/in.h>

[Mat Martineau]

On Fri, 10 Jun 2022, Jakub Kicinski wrote:

> On Fri, 10 Jun 2022 11:00:28 -0700 (PDT) Mat Martineau wrote:
>> This is a minor "fix" to be sure, which I thought did not meet the bar for
>> net and therefore submitted for net-next. It's not prep for another
>> change, it's something Ossama and I noticed when doing code review for a
>> userspace program that included the header. There's no problem with kernel
>> compilation, and there's also no issue if the userspace program happens to
>> include netinet/in.h before linux/mptcp.h
>>
>>
>> If my threshold for the net branch is too high, I have no objection to
>> having this patch applied there and will recalibrate :)
>>
>> Do you prefer to have no Fixes: tags in net-next, or did that just seem
>> ambiguous in this case?
>
> The important point is that the middle ground of marking things as fixes
> and at the same time putting them in -next, to still get them
> backported but with an extended settling time -- that middle ground
> does not exist.
>
> If we look at the patch from the "do we want it backported or not"
> perspective I think the answer is yes, hence I'd lean towards net.
> If you think it doesn't matter enough for backport - we can drop the
> fixes tag and go with net-next. Unfortunately I don't have enough
> direct experience to tell how annoying this will be to the user space.
> netinet/in.h vs linux/in.h is a mess :(
>

By that criteria, I lean towards net too. Thanks Jakub.

--
Mat Martineau
Intel