All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw at strlen.de>
To: mptcp at lists.01.org
Subject: [MPTCP] Re: [RFC mptcp-next] tcp: mptcp: use mptcp receive buffer space to select rcv window
Date: Wed, 18 Mar 2020 21:59:20 +0100	[thread overview]
Message-ID: <20200318205920.GK979@breakpoint.cc> (raw)
In-Reply-To: 95e08be8-9902-f998-6558-e7e574d783b0@gmail.com

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

Eric Dumazet <eric.dumazet(a)gmail.com> wrote:
> >>> +/* If ssk has an mptcp parent socket, use the mptcp rcvbuf occupancy,
> >>> + * not the ssk one.
> >>> + *
> >>> + * In mptcp, rwin is about the mptcp-level connection data.
> >>> + *
> >>> + * Data that is still on the ssk rx queue can thus be ignored,
> >>> + * as far as mptcp peer is concerened that data is still inflight.
> >>> + */
> >>> +void mptcp_space(const struct sock *ssk, int *space, int *full_space)
> >>> +{
> >>> +	const struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> >>> +	const struct sock *sk = READ_ONCE(subflow->conn);
> >>
> >> What are the rules protecting subflow->conn lifetime ?
> >>
> >> Why dereferencing sk after this line is safe ?
> > 
> > Subflow sockets hold a reference on the master/parent mptcp-socket.
> > 
> 
> Presence of READ_ONCE() tells something might happen on
> this pointer after you read it.

Right, sorry about this. The READ_ONCE() isn't needed anymore after
recent improvement from Paolo.

> Can this pointer be set while this thread is owning the socket lock ?

Only by the one holding the sk lock, so no race.

> If not, then you do not need READ_ONCE(), this is confusing.

Yes.

> If yes, then it means that whatever changes the pointer might also release the reference
> on the old object.

The reference is released only after aquiring the socket lock.

WARNING: multiple messages have this Message-ID (diff)
From: Florian Westphal <fw@strlen.de>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Florian Westphal <fw@strlen.de>,
	netdev@vger.kernel.org, mptcp@lists.01.org,
	Eric Dumazet <edumazet@google.com>
Subject: Re: [RFC mptcp-next] tcp: mptcp: use mptcp receive buffer space to select rcv window
Date: Wed, 18 Mar 2020 21:59:20 +0100	[thread overview]
Message-ID: <20200318205920.GK979@breakpoint.cc> (raw)
In-Reply-To: <95e08be8-9902-f998-6558-e7e574d783b0@gmail.com>

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >>> +/* If ssk has an mptcp parent socket, use the mptcp rcvbuf occupancy,
> >>> + * not the ssk one.
> >>> + *
> >>> + * In mptcp, rwin is about the mptcp-level connection data.
> >>> + *
> >>> + * Data that is still on the ssk rx queue can thus be ignored,
> >>> + * as far as mptcp peer is concerened that data is still inflight.
> >>> + */
> >>> +void mptcp_space(const struct sock *ssk, int *space, int *full_space)
> >>> +{
> >>> +	const struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> >>> +	const struct sock *sk = READ_ONCE(subflow->conn);
> >>
> >> What are the rules protecting subflow->conn lifetime ?
> >>
> >> Why dereferencing sk after this line is safe ?
> > 
> > Subflow sockets hold a reference on the master/parent mptcp-socket.
> > 
> 
> Presence of READ_ONCE() tells something might happen on
> this pointer after you read it.

Right, sorry about this. The READ_ONCE() isn't needed anymore after
recent improvement from Paolo.

> Can this pointer be set while this thread is owning the socket lock ?

Only by the one holding the sk lock, so no race.

> If not, then you do not need READ_ONCE(), this is confusing.

Yes.

> If yes, then it means that whatever changes the pointer might also release the reference
> on the old object.

The reference is released only after aquiring the socket lock.

             reply	other threads:[~2020-03-18 20:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-18 20:59 Florian Westphal [this message]
2020-03-18 20:59 ` [RFC mptcp-next] tcp: mptcp: use mptcp receive buffer space to select rcv window Florian Westphal
  -- strict thread matches above, loose matches on Subject: below --
2020-03-18 18:14 [MPTCP] " Eric Dumazet
2020-03-18 18:14 ` Eric Dumazet
2020-03-18 18:05 [MPTCP] " Florian Westphal
2020-03-18 18:05 ` Florian Westphal
2020-03-18 17:59 [MPTCP] " Eric Dumazet
2020-03-18 17:59 ` Eric Dumazet
2020-03-18 14:19 [MPTCP] " Florian Westphal
2020-03-18 14:19 ` Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200318205920.GK979@breakpoint.cc \
    --to=unknown@example.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.