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

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.

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

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 :(

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