[MPTCP] Re: [PATCH RFC 10/11] mptcp: enable JOIN requests even if cookies are in use

[Florian Westphal <fw at strlen.de>]

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

Paolo Abeni <pabeni(a)redhat.com> wrote:
> On Mon, 2020-07-27 at 11:02 +0200, Florian Westphal wrote:
> > Paolo Abeni <pabeni(a)redhat.com> wrote:
> > > > +static bool mptcp_join_store(struct mptcp_subflow_request_sock *req, struct sock *sk_listener)
> > > > +{
> > > > +	struct mptcp_sock *msk = req->msk;
> > > > +	struct inet_connection_sock *icsk;
> > > > +	struct sock *sk;
> > > > +
> > > > +	icsk = &msk->sk;
> > > > +	sk = &icsk->icsk_inet.sk;
> > > > +
> > > > +	if (inet_csk_reqsk_queue_len(sk) >= MPTCP_SUBFLOWS_MAX)
> > > > +		return false;
> > > 
> > > Isn't this test a little too relaxed? e.g. shouldn't we check something
> > > alike:
> > > 
> > > 	if (inet_csk_reqsk_queue_len(sk) + READ_ONCE(pm->subflows) >
> > > 	    READ_ONCE(pm->subflows_max))
> > > 
> > > Possibly we can account the pending MP_JOIN subflows in a new PM field,
> > > so we could do the above check and the follow-up increment in a
> > > consistent way with a single atomic opetation (the pm spinlock)
> > 
> > Not following here -- inet_csk_reqsk_queue_len() is supposed to return
> > the number of all subflows, both established and half-open.
> > 
> > Why would I need to add pm->subflows?
> 
> Currently the PM netlink does not accept any additional subflows
> when pm->subflows >= pm->subflows_max.

You mean:

if (inet_csk_reqsk_queue_len(sk) >= MPTCP_SUBFLOWS_MAX ||
      pm->subflows >= pm->subflows_max)
	return false;

?
(I ommitted R_ONCE for brevity only).