Let the caller specify the to-be-created subflow family. No
functional change intended, will be leveraged by the next
patch.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 2 +-
 net/mptcp/protocol.h | 3 ++-
 net/mptcp/subflow.c  | 9 +++++----
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 5e813580c394..f08ef9ca242a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -98,7 +98,7 @@ static int __mptcp_socket_create(struct mptcp_sock *msk)
 	struct socket *ssock;
 	int err;
 
-	err = mptcp_subflow_create_socket(sk, &ssock);
+	err = mptcp_subflow_create_socket(sk, sk->sk_family, &ssock);
 	if (err)
 		return err;
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3cd3270407b0..fcce8f03c1b5 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -643,7 +643,8 @@ bool mptcp_addresses_equal(const struct mptcp_addr_info *a,
 /* called with sk socket lock held */
 int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
 			    const struct mptcp_addr_info *remote);
-int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
+int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
+				struct socket **new_sock);
 void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
 			 struct sockaddr_storage *addr,
 			 unsigned short family);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index bd387d4b5a38..ec54413fb31f 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1547,7 +1547,7 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
 	if (!mptcp_is_fully_established(sk))
 		goto err_out;
 
-	err = mptcp_subflow_create_socket(sk, &sf);
+	err = mptcp_subflow_create_socket(sk, loc->family, &sf);
 	if (err)
 		goto err_out;
 
@@ -1660,7 +1660,9 @@ static void mptcp_subflow_ops_undo_override(struct sock *ssk)
 #endif
 		ssk->sk_prot = &tcp_prot;
 }
-int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
+
+int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
+				struct socket **new_sock)
 {
 	struct mptcp_subflow_context *subflow;
 	struct net *net = sock_net(sk);
@@ -1673,8 +1675,7 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
 	if (unlikely(!sk->sk_socket))
 		return -EINVAL;
 
-	err = sock_create_kern(net, sk->sk_family, SOCK_STREAM, IPPROTO_TCP,
-			       &sf);
+	err = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP, &sf);
 	if (err)
 		return err;
 
-- 
2.38.1

Hi Paolo,

On 23/12/2022 13:51, Paolo Abeni wrote:
> Let the caller specify the to-be-created subflow family. No
> functional change intended, will be leveraged by the next
> patch.

(...)

> @@ -1673,8 +1675,7 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
>  	if (unlikely(!sk->sk_socket))
>  		return -EINVAL;
>  
> -	err = sock_create_kern(net, sk->sk_family, SOCK_STREAM, IPPROTO_TCP,
> -			       &sf);
> +	err = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP, &sf);

Mmh, I don't know if we can do that: it means we no longer inherit the
'sk_family'. With the modification, it means if the initial socket is
created in v4 only (AF_INET), we will still be able to create subflows
in IPv6, right?

Somehow, I like the idea because with today's Internet, it is normal for
a client to have both IPv4 and IPv6 addresses. Then we avoid people
creating AF_INET sockets and complaining they cannot have additional
IPv6 subflows.

But on the other hand, there is no more way to force having v4 only
subflows, no?
And I guess listening sockets will have to be created with AF_INET6 to
accept additional v6 subflows.

WDYT?
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

On Fri, 2022-12-23 at 15:35 +0100, Matthieu Baerts wrote:
> Hi Paolo,
> 
> On 23/12/2022 13:51, Paolo Abeni wrote:
> > Let the caller specify the to-be-created subflow family. No
> > functional change intended, will be leveraged by the next
> > patch.
> 
> (...)
> 
> > @@ -1673,8 +1675,7 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
> >  	if (unlikely(!sk->sk_socket))
> >  		return -EINVAL;
> >  
> > -	err = sock_create_kern(net, sk->sk_family, SOCK_STREAM, IPPROTO_TCP,
> > -			       &sf);
> > +	err = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP, &sf);
> 
> Mmh, I don't know if we can do that: it means we no longer inherit the
> 'sk_family'. With the modification, it means if the initial socket is
> created in v4 only (AF_INET), we will still be able to create subflows
> in IPv6, right?

Yes. That was my understanding of issues/269 goal.

> Somehow, I like the idea because with today's Internet, it is normal for
> a client to have both IPv4 and IPv6 addresses. Then we avoid people
> creating AF_INET sockets and complaining they cannot have additional
> IPv6 subflows.
> 
> But on the other hand, there is no more way to force having v4 only
> subflows, no?
> And I guess listening sockets will have to be created with AF_INET6 to
> accept additional v6 subflows.
> 
> WDYT?

I'm not sure I understand. Do you mean that we should allow mixed
ipv4/ipv6 subflows only if mptcp family is ipv6? that is doable, I
think, just slightly different from what is implemented here.

Cheers,

Paolo

On 23/12/2022 17:01, Paolo Abeni wrote:
> On Fri, 2022-12-23 at 15:35 +0100, Matthieu Baerts wrote:
>> Hi Paolo,
>>
>> On 23/12/2022 13:51, Paolo Abeni wrote:
>>> Let the caller specify the to-be-created subflow family. No
>>> functional change intended, will be leveraged by the next
>>> patch.
>>
>> (...)
>>
>>> @@ -1673,8 +1675,7 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
>>>  	if (unlikely(!sk->sk_socket))
>>>  		return -EINVAL;
>>>  
>>> -	err = sock_create_kern(net, sk->sk_family, SOCK_STREAM, IPPROTO_TCP,
>>> -			       &sf);
>>> +	err = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP, &sf);
>>
>> Mmh, I don't know if we can do that: it means we no longer inherit the
>> 'sk_family'. With the modification, it means if the initial socket is
>> created in v4 only (AF_INET), we will still be able to create subflows
>> in IPv6, right?
> 
> Yes. That was my understanding of issues/269 goal.

I didn't think about that when creating issue 269.

>> Somehow, I like the idea because with today's Internet, it is normal for
>> a client to have both IPv4 and IPv6 addresses. Then we avoid people
>> creating AF_INET sockets and complaining they cannot have additional
>> IPv6 subflows.
>>
>> But on the other hand, there is no more way to force having v4 only
>> subflows, no?
>> And I guess listening sockets will have to be created with AF_INET6 to
>> accept additional v6 subflows.
>>
>> WDYT?
> 
> I'm not sure I understand. Do you mean that we should allow mixed
> ipv4/ipv6 subflows only if mptcp family is ipv6? that is doable, I
> think, just slightly different from what is implemented here.

Maybe we don't need to change, we just need to decide which behaviour we
want.

Is it OK to have a mix of v4 and v6 subflows if the socket has been
created with AF_INET family? In this case, the family is just for the
initial subflow and not the next ones.

That's maybe fine but it might be strange that for the server side,
AF_INET6 is required to accept both IPv4 and IPv6 subflows.

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

On Fri, 2022-12-23 at 17:08 +0100, Matthieu Baerts wrote:
> On 23/12/2022 17:01, Paolo Abeni wrote:
> 
> > Do you mean that we should allow mixed
> > ipv4/ipv6 subflows only if mptcp family is ipv6? that is doable, I
> > think, just slightly different from what is implemented here.
> 
> Maybe we don't need to change, we just need to decide which behaviour we
> want.
> 
> Is it OK to have a mix of v4 and v6 subflows if the socket has been
> created with AF_INET family? In this case, the family is just for the
> initial subflow and not the next ones.
> 
> That's maybe fine but it might be strange that for the server side,
> AF_INET6 is required to accept both IPv4 and IPv6 subflows.

Ok, adapting to that does not look like a big change. I'll try to cook
it.

Thanks,

Paolo