[MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 0/8] Add REMOVE_ADDR support

[MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 0/8] Add REMOVE_ADDR support

[Geliang Tang]

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

Hi Paolo,

Thanks for your review.

Paolo Abeni <pabeni(a)redhat.com> 于2020年9月2日周三 上午12:02写道：
>
> On Mon, 2020-08-24 at 19:36 +0800, Geliang Tang wrote:
> > v8:
> >  - drop anno_list in v7.
> >    We don't need to add a new list, conn_list is enough for the signal
> > address and local subflow.
>
> I think this could be problematic.
>
> An MPTCP socket could refuse or delay the creation of new subflows for
> addresses received via ADD_ADDR.
>
> But an MPTCP should send REMOVE_ADDR for accounced addresses, even if
> no subflows exists on top of them.
>
> Removing the anno_list we lose this ability.

Yes, we can't cover this case, but the v7 code with anno_list can't cover
it either. Since anno_list only recorded the local announced addresses,
we need also record the received addresses on the remote side.

I'll write a patch to fix this problem later.

>
> I'd like to avoid additional interations, if possible, and eventually
> add squash-to fixes if/where needed. Other options more than welcome.

Since my another patchset 'add ADD_ADDR echo flag support' is accepted,
v8 will conflict with it. So here I simply updated the v8 patchset as
you suggested and sent out the v9 version.

>
> Cheers,
>
> Paolo
>

> very minor note: this does not respect the reverse x-mas tree order.
>
> Otherwise looks good.
>
> Note: I think you can submit this patch and the previous one (4/8)
> directly to -net (with an appropriate Fixes tag).
>

Since patch 5/8 is depended on the patch 'mptcp: add the incoming RM_ADDR
support', which is only on net-next's export branch, we can't directly
submit 4/8 and 5/8 to -net. So here I still keeped them in the v9 patchset.

Thanks.

-Geliang

[MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 0/8] Add REMOVE_ADDR support

[Geliang Tang]

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

Hi Paolo,

Paolo Abeni <pabeni(a)redhat.com> 于2020年9月2日周三 下午11:20写道：
>
> On Wed, 2020-09-02 at 19:30 +0800, Geliang Tang wrote:
> > Hi Paolo,
> >
> > Thanks for your review.
> >
> > Paolo Abeni <pabeni(a)redhat.com> 于2020年9月2日周三 上午12:02写道：
> > > On Mon, 2020-08-24 at 19:36 +0800, Geliang Tang wrote:
> > > > v8:
> > > >  - drop anno_list in v7.
> > > >    We don't need to add a new list, conn_list is enough for the signal
> > > > address and local subflow.
> > >
> > > I think this could be problematic.
> > >
> > > An MPTCP socket could refuse or delay the creation of new subflows for
> > > addresses received via ADD_ADDR.
> > >
> > > But an MPTCP should send REMOVE_ADDR for accounced addresses, even if
> > > no subflows exists on top of them.
> > >
> > > Removing the anno_list we lose this ability.
> >
> > Yes, we can't cover this case, but the v7 code with anno_list can't cover
> > it either. Since anno_list only recorded the local announced addresses,
> > we need also record the received addresses on the remote side.
>
> I think we don't need this latter list ?!? when the mptcp receives an
> ADD_ADDR option it will either create a new subflow with such address
> or will never use such address later.
>
> When the mptcp stack receives a REMOVE_ADDR option, it just need to
> check vs the existing subflows. If the relevant address has been
> ignored, no additional action is needed.
>

I have just sent out a squash-to patch named 'Squash-to: "mptcp: remove addr and
subflow in PM netlink v9"' to fix this. Please give me some
suggestions about it.

Thanks.

-Geliang

> Do I miss something?
>
> Thanks!
>
> Paolo
>

[MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 0/8] Add REMOVE_ADDR support

[Geliang Tang]

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

Hi Paolo,

Geliang Tang <geliangtang(a)gmail.com> 于2020年9月2日周三 下午7:30写道：
>
> Hi Paolo,
>
> Thanks for your review.
>
> Paolo Abeni <pabeni(a)redhat.com> 于2020年9月2日周三 上午12:02写道：
> >
> > On Mon, 2020-08-24 at 19:36 +0800, Geliang Tang wrote:
> > > v8:
> > >  - drop anno_list in v7.
> > >    We don't need to add a new list, conn_list is enough for the signal
> > > address and local subflow.
> >
> > I think this could be problematic.
> >
> > An MPTCP socket could refuse or delay the creation of new subflows for
> > addresses received via ADD_ADDR.
> >
> > But an MPTCP should send REMOVE_ADDR for accounced addresses, even if
> > no subflows exists on top of them.
> >
> > Removing the anno_list we lose this ability.
>
> Yes, we can't cover this case, but the v7 code with anno_list can't cover
> it either. Since anno_list only recorded the local announced addresses,
> we need also record the received addresses on the remote side.
>
> I'll write a patch to fix this problem later.
>
> >
> > I'd like to avoid additional interations, if possible, and eventually
> > add squash-to fixes if/where needed. Other options more than welcome.
>
> Since my another patchset 'add ADD_ADDR echo flag support' is accepted,
> v8 will conflict with it. So here I simply updated the v8 patchset as
> you suggested and sent out the v9 version.
>
> >
> > Cheers,
> >
> > Paolo
> >
>
> > very minor note: this does not respect the reverse x-mas tree order.
> >
> > Otherwise looks good.
> >
> > Note: I think you can submit this patch and the previous one (4/8)
> > directly to -net (with an appropriate Fixes tag).
> >
>
> Since patch 5/8 is depended on the patch 'mptcp: add the incoming RM_ADDR
> support', which is only on net-next's export branch, we can't directly
> submit 4/8 and 5/8 to -net. So here I still keeped them in the v9 patchset.
>

I have just sent out a Squash-to patch for "mptcp: add the incoming RM_ADDR
support", to drop the remote_id fixing code in it. Then we can submit
4/8 and 5/8
directly to -net.

-Geliang

> Thanks.
>
> -Geliang

[MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 0/8] Add REMOVE_ADDR support

[Paolo Abeni]

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

On Wed, 2020-09-02 at 19:30 +0800, Geliang Tang wrote:
> Hi Paolo,
> 
> Thanks for your review.
> 
> Paolo Abeni <pabeni(a)redhat.com> 于2020年9月2日周三 上午12:02写道：
> > On Mon, 2020-08-24 at 19:36 +0800, Geliang Tang wrote:
> > > v8:
> > >  - drop anno_list in v7.
> > >    We don't need to add a new list, conn_list is enough for the signal
> > > address and local subflow.
> > 
> > I think this could be problematic.
> > 
> > An MPTCP socket could refuse or delay the creation of new subflows for
> > addresses received via ADD_ADDR.
> > 
> > But an MPTCP should send REMOVE_ADDR for accounced addresses, even if
> > no subflows exists on top of them.
> > 
> > Removing the anno_list we lose this ability.
> 
> Yes, we can't cover this case, but the v7 code with anno_list can't cover
> it either. Since anno_list only recorded the local announced addresses,
> we need also record the received addresses on the remote side.

I think we don't need this latter list ?!? when the mptcp receives an
ADD_ADDR option it will either create a new subflow with such address
or will never use such address later.

When the mptcp stack receives a REMOVE_ADDR option, it just need to
check vs the existing subflows. If the relevant address has been
ignored, no additional action is needed.

Do I miss something?

Thanks!

Paolo

[MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 0/8] Add REMOVE_ADDR support

[Paolo Abeni]

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

On Mon, 2020-08-24 at 19:36 +0800, Geliang Tang wrote:
> v8:
>  - drop anno_list in v7.
>    We don't need to add a new list, conn_list is enough for the signal
> address and local subflow.

I think this could be problematic.

An MPTCP socket could refuse or delay the creation of new subflows for
addresses received via ADD_ADDR. 

But an MPTCP should send REMOVE_ADDR for accounced addresses, even if
no subflows exists on top of them.

Removing the anno_list we lose this ability.

I'd like to avoid additional interations, if possible, and eventually
add squash-to fixes if/where needed. Other options more than welcome.

Cheers,

Paolo