Re: [MPTCP] [RFC PATCH mptcp-next v2 1/8] mptcp: add skeleton to sync msk socket options to subflows

[Paolo Abeni <pabeni@redhat.com>]

On Thu, 2021-03-25 at 15:06 +0100, Florian Westphal wrote:
> Paolo Abeni <pabeni@redhat.com> wrote:
> > > Ok, so you propose to init ssk->sockopt_seq to msk->sockopt_seq in
> > > mptcp_accept()?
> > 
> > in subflow_syn_recv_sock(), but only if we can avoid incrementing  msk-
> > > sockopt_seq on per setsockopt basis, otherwise things will be
> > crippled:
> > 
> > socket(s1)
> > setsockopt(s1)   // s1->seq 1
> > listen(s1)
> > 
> > s2 = accept() s2->seq == s1->seq == ssk->seq
> > 
> > setsockopt(s2)   s2->seq = 2
> > setsockopt(s1, <a different sockopt from the above>) s1->seq = 2
> > 
> > At this point, if we increment sockopt_seq after everu sockopt, s1-
> > > sockopt_seq and s2->sockopt_seq will be equal (and we will not sync
> > any mpj subflow accepted later if propagate sockopt_seq to ssk). 
> 
> Sorry, I am not following.
> 
> The ssk (s2 msk->first) has ssk->seq 1, s2 msk has 2, so this would be
> synced again on completion.
> 
> I don't understand how s1 and s2 are related.
> This is too complicated for me :(

s1 is an msk listener socket. It will get 2 setsockopt() syscall, so
s1->sockopt_seq == 2.

Even s2->sockopt_seq == 2, as you noted.

If we copy msk->sockopt_seq into ssk->sockopt_seq, any additional MPJ
subflow joining s2 created after the above syscalls sequence, will
get sockopt_seq == 2, (inherited by s1->sockopt_seq), so will not be
synched, even if synchronization will be needed.

Not sure if the above is less confusing ?!?

> > Instead, if we set sockopt_seq to some socket status related value
> > after every setsockopt(), s2->sockopt_seq and s1->sockopt_seq will be
> > different and we will sync later MPJ subflows.
> 
> What is a 'socket status related value'?

I mean something that changes when 'sk->sk_state' changes. Even plain
'sk->sk_state'.

> > Perhaps we can keep the things as-is to and ev change/improve in a
> > second time?
> 
> I am confused, you mean leave it as-is, i.e. scratch this patch set
> and retry (new per-subflow sockopt api for instance)?
> 
> Or leave this RFC v2 as-is and see if we can improve the sync strategy
> later?

the latter, of course! I mean, keep this v2 as is.

Sorry for the lack of clarity,

/P