Re: [MPTCP][PATCH v3 mptcp-next 2/5] mptcp: local addresses fullmesh

[Paolo Abeni <pabeni@redhat.com>]

On Mon, 2021-07-26 at 16:32 +0800, Geliang Tang wrote:
> Paolo Abeni <pabeni@redhat.com> 于2021年7月26日周一 下午4:24写道：
> > On Mon, 2021-07-26 at 12:12 +0800, Geliang Tang wrote:
> > > From: Geliang Tang <geliangtang@xiaomi.com>
> > > 
> > > In mptcp_pm_nl_add_addr_received(), fill a temporary allocate array of
> > > all local address corresponding to the fullmesh endpoint. If such array
> > > is empty, keep the current behavior.
> > > 
> > > Elsewhere loop on such array and create a subflow for each local address
> > > towards the given remote address
> > > 
> > > Suggested-by: Paolo Abeni <pabeni@redhat.com>
> > > Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
> > > ---
> > >  net/mptcp/pm_netlink.c | 35 ++++++++++++++++++++++++++++++++---
> > >  1 file changed, 32 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > > index f57db5b9a50f..a03e8fd3a584 100644
> > > --- a/net/mptcp/pm_netlink.c
> > > +++ b/net/mptcp/pm_netlink.c
> > > @@ -526,12 +526,17 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk)
> > > 
> > >  static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
> > >  {
> > > +     struct mptcp_pm_addr_entry entries[MPTCP_PM_ADDR_MAX];
> > >       struct sock *sk = (struct sock *)msk;
> > > +     struct mptcp_pm_addr_entry *entry;
> > >       unsigned int add_addr_accept_max;
> > > +     struct mptcp_pm_addr_entry local;
> > >       struct mptcp_addr_info remote;
> > > -     struct mptcp_addr_info local;
> > > +     struct pm_nl_pernet *pernet;
> > >       unsigned int subflows_max;
> > > +     int i, n = 0;
> > > 
> > > +     pernet = net_generic(sock_net(sk), pm_nl_pernet_id);
> > >       add_addr_accept_max = mptcp_pm_get_add_addr_accept_max(msk);
> > >       subflows_max = mptcp_pm_get_subflows_max(msk);
> > > 
> > > @@ -555,10 +560,34 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
> > >       if (!remote.port)
> > >               remote.port = sk->sk_dport;
> > >       memset(&local, 0, sizeof(local));
> > > -     local.family = remote.family;
> > > +     local.addr.family = remote.family;
> > > 
> > > +     entries[n++] = local;
> > 
> > If I read correctly, I think this entry should be added only if the
> > full-mesh selection below find no other entries. Otherwise, e.g. in the
> > simple scenario with 2 locals address, 3 subflows will be created.
> 
> local is the id 0 address, this address should always be used, and it's
> not on the local_addr_list. 

Uhm... if the user-space configures correctly the mptcp endpoints,
every relevant address should be in local_addr_list.

e.g.

The client has 2 IPs:

192.168.255.2/24 dev eth0
192.168.254.2/24 dev eth1

The user-space should create 2 endpoints with full-mesh flags, one for
each of the above IPs, at addresses creation time.

When the mptcp socket is established, it's first subflow will use as
local address one of such IPs/endpoints.

If mptcp_pm_nl_add_addr_received() adds to the 'entries' array an
'IPADDRANY' address, and than the 2 addresses above, the client will
end-up creating 3 subflows towards the newly signaled address, instead
of the expected 2.

WDYT?

Paolo