[MPTCP] Re: [MPTCP][PATCH v5 mptcp-next 0/9] RM_ADDR: remove a list of addrs

[MPTCP] Re: [MPTCP][PATCH v5 mptcp-next 0/9] RM_ADDR: remove a list of addrs

[Mat Martineau]

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


On Mon, 8 Feb 2021, Geliang Tang wrote:

> v5:
> - use struct mptcp_rm_list instead of the ids array.
> - simplify mptcp_pm_remove_addrs_and_subflows.
> - apply: tag export/20210207T062839 + patchset "move to next addr and
> testcases".
>

Thanks Geliang!

I only had a couple of minor comments doing visual code review, they 
affect individual patches but not the final code. That's all I saw in 
visual code review, I will look at the test output and pcaps tomorrow.

Have you tested what happens if there's an incoming rm_addr with id 0, 
since we never send an ADD_ADDR for the initial subflow?

--
Mat Martineau
Intel

[MPTCP] Re: [MPTCP][PATCH v5 mptcp-next 0/9] RM_ADDR: remove a list of addrs

[Geliang Tang]

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

Hi Mat,

Thanks for your reviews.

Mat Martineau <mathew.j.martineau(a)linux.intel.com> 于2021年2月9日周二 上午9:57写道：
>
>
> On Mon, 8 Feb 2021, Geliang Tang wrote:
>
> > v5:
> > - use struct mptcp_rm_list instead of the ids array.
> > - simplify mptcp_pm_remove_addrs_and_subflows.
> > - apply: tag export/20210207T062839 + patchset "move to next addr and
> > testcases".
> >
>
> Thanks Geliang!
>
> I only had a couple of minor comments doing visual code review, they
> affect individual patches but not the final code. That's all I saw in
> visual code review, I will look at the test output and pcaps tomorrow.
>
> Have you tested what happens if there's an incoming rm_addr with id 0,
> since we never send an ADD_ADDR for the initial subflow?

Id 0 is still not support in this removing address version, since we only
use this code to shutdown and close the subsocket in
mptcp_pm_nl_rm_addr_received/mptcp_pm_nl_rm_subflow_received:

    mptcp_subflow_shutdown(sk, ssk, how);
    mptcp_close_ssk(sk, ssk, subflow);

It's problematic to use them to shutdown and close the initial subflow.

I think we need to use something like:

    mptcp_shutdown(sk, how);
    mptcp_close(sk, timeout);

to deal with the initial subflow, but it doesn't work too. It will take
me some time to solve it.

-Geliang

>
> --
> Mat Martineau
> Intel