mptcp: fix conflict with <netinet/in.h>

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

Posted by Ossama Othman 3 years, 8 months ago

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.

Signed-off-by: Ossama Othman <ossama.othman@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.34.1

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

Posted by Mat Martineau 3 years, 8 months ago

On Thu, 19 May 2022, Ossama Othman wrote:

> 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.
>
> Signed-off-by: Ossama Othman <ossama.othman@intel.com>

Looks good to me, I think this makes things easier for userspace.

Reviewed-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.34.1
>
>
>

--
Mat Martineau
Intel

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

Posted by Matthieu Baerts 3 years, 8 months ago

Hi Ossama, Mat,

On 20/05/2022 02:05, Mat Martineau wrote:
> On Thu, 19 May 2022, Ossama Othman wrote:
> 
>> 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.
>>
>> Signed-off-by: Ossama Othman <ossama.othman@intel.com>
> 
> Looks good to me, I think this makes things easier for userspace.
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Thank you for the patch and the review!

Do you mind if I add this Fixes tag?

Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support")

Or do you prefer without it not to have it backported?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

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

Posted by Mat Martineau 3 years, 8 months ago

On Fri, 20 May 2022, Matthieu Baerts wrote:

> Hi Ossama, Mat,
>
> On 20/05/2022 02:05, Mat Martineau wrote:
>> On Thu, 19 May 2022, Ossama Othman wrote:
>>
>>> 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.
>>>
>>> Signed-off-by: Ossama Othman <ossama.othman@intel.com>
>>
>> Looks good to me, I think this makes things easier for userspace.
>>
>> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
>
> Thank you for the patch and the review!
>
> Do you mind if I add this Fixes tag?
>
> Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support")
>
> Or do you prefer without it not to have it backported?
>

Fixes tag is good, thanks! I will probably cc: stable on this one.

--
Mat Martineau
Intel

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

Posted by Matthieu Baerts 3 years, 8 months ago

Hi Ossama, Mat,

On 21/05/2022 02:18, Mat Martineau wrote:
> On Fri, 20 May 2022, Matthieu Baerts wrote:
> 
>> Hi Ossama, Mat,
>>
>> On 20/05/2022 02:05, Mat Martineau wrote:
>>> On Thu, 19 May 2022, Ossama Othman wrote:
>>>
>>>> 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.
>>>>
>>>> Signed-off-by: Ossama Othman <ossama.othman@intel.com>
>>>
>>> Looks good to me, I think this makes things easier for userspace.
>>>
>>> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
>>
>> Thank you for the patch and the review!
>>
>> Do you mind if I add this Fixes tag?
>>
>> Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support")
>>
>> Or do you prefer without it not to have it backported?
>>
> 
> Fixes tag is good, thanks!

Great, just did.

I also changed the author (FROM) to use Ossama's Intel email address. I
guess that was your intension, right?

Applied in our tree (fixes for net-next, easier as Mat said):

New patches for t/upstream:

- 334cee2f9c14: mptcp: fix conflict with <netinet/in.h>

- Results: c1baa3aff794..bbf8496f889f (export)


Builds and tests are now in progress:



https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220521T072520

https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

> I will probably cc: stable on this one.

Good idea!

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: mptcp: fix conflict with <netinet/in.h>: Tests Results

Posted by MPTCP CI 3 years, 8 months ago

Hi Ossama,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6207370031267840
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6207370031267840/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 2 failed test(s): selftest_diag selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/4799995147714560
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4799995147714560/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/9ca5881aed64


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)