Re: [PATCH mptcp-next v2 07/21] mptcp: netlink: process IPv6 addrs in creating listening sockets

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2803 bytes --]

On Tue, 18 Jan 2022, Kishen Maloor wrote:

> On 1/14/22 2:09 PM, Mat Martineau wrote:
>> On Fri, 14 Jan 2022, Paolo Abeni wrote:
>>
>>> Hello,
>>>
>>> On Wed, 2022-01-12 at 17:15 -0500, Kishen Maloor wrote:
>>>> This change updates mptcp_pm_nl_create_listen_socket() to create
>>>> listening sockets bound to IPv6 addresses (where IPv6 is supported).
>>>>
>>
>> This tag -
>>
>> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
>>
>> would help with backporting to stable.
>>
>>>> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
>>>> ---
>>>>  net/mptcp/pm_netlink.c | 8 ++++++--
>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>>> index 4c1895dbc2a5..779ec9d375f0 100644
>>>> --- a/net/mptcp/pm_netlink.c
>>>> +++ b/net/mptcp/pm_netlink.c
>>>> @@ -986,6 +986,7 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
>>>>                          struct mptcp_pm_addr_entry *entry,
>>>>                          struct socket **lsk)
>>>>  {
>>>> +    int addrlen = sizeof(struct sockaddr_in);
>>>>      struct sockaddr_storage addr;
>>>>      struct mptcp_sock *msk;
>>>>      struct socket *ssock;
>>>> @@ -1010,8 +1011,11 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
>>>>      }
>>>>
>>>>      mptcp_info2sockaddr(&entry->addr, &addr, entry->addr.family);
>>>> -    err = kernel_bind(ssock, (struct sockaddr *)&addr,
>>>> -              sizeof(struct sockaddr_in));
>>>> +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
>>>> +    if (entry->addr.family == AF_INET6)
>>>> +        addrlen = sizeof(struct sockaddr_in6);
>>>> +#endif
>>>> +    err = kernel_bind(ssock, (struct sockaddr *)&addr, addrlen);
>>>>      if (err) {
>>>>          pr_warn("kernel_bind error, err=%d", err);
>>>>          goto out;
>>>
>>> This looks a bugfix for -net to me?
>
> Sure, we can record a bug and the fact that this commit fixes it.
>
>>>
>>> Possibly worthy additional an additional mp_join self-test for the ipv6
>>> case.
>>>
>>
>> I agree, this would be good for -net. Kishen can you add the suggested selftest too and repost separately for mptcp-net?
>>
>
> Actually, this path is currently exercised by self-tests in userspace_pm.sh through address
> advertisements from the namespace containing the MPTCP client, for which a listening
> socket is created to subsequently receive MPJs.

Ok, it's good that it's covered going forward. Re-reading Paolo's comment, 
maybe that was the main purpose of the requested test (rather than adding 
to self test coverage in -stable). Should be enough to have the kernel fix 
itself for the -net patch.

--
Mat Martineau
Intel