Re: [PATCH mptcp-next v8 0/8] Clarify when options can be used

[Matthieu Baerts <matthieu.baerts@tessares.net>]

Hi Geliang,

On 06/01/2022 03:58, Geliang Tang wrote:
> Hi Matt,
> 
> Thank you so much for this new version. Good catch on patch 2. It works on
> my test and all looks good to me...
> 
> Acked-and-tested-by: Geliang Tang <geliang.tang@suse.com>

Great, thank you for having checked that!

> ... except patch 6.
> 
> I thinks we'd better to drop patch 6 ("mptcp: allow sending both ADD_ADDR
> and RM_ADDR"). Since ADD_ADDR suboption is too big, we had made a lot of
> efforts to let ADD_ADDR not to be sent with other subopthins. I thinks it's
> better not to let ADD_ADDR sent with others by ourselves.

ADD_ADDR suboption can be too big in some cases but not all the time.
If I remember well, we decided to have a dedicated ACK to simplify the
code: not having the ADD_ADDR removing the DSS options in some cases but
not all, also simplifying packetdrill tests.

Here, the idea is to add the RM_ADDR if asked and if there is room, just
like what we do in mptcp_write_options().

> On the other hand, I'm not sure our code can deal with ADD_ADDR and RM_ADDR
> together correctly, it's not tested.

Good point, we need a test validating that.

I'm not even sure we can have a test because each netlink command will
cause the corresponding option to be sent: depending on the timing, when
the next packet is sent, we might have one or two options to be sent.

If we drop patch 6, we need to replace it with a patch updating the new
comment at the beginning of mptcp_write_options() and not allowing
ADD_ADDR to be sent with RM_ADDR + MP_PRIO as we currently do.

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