All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: David Miller <davem@davemloft.net>, soheil.kdev@gmail.com
Cc: netdev@vger.kernel.org, ycheng@google.com, ncardwell@google.com,
	edumazet@google.com, willemb@google.com, soheil@google.com
Subject: Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read
Date: Mon, 30 Apr 2018 08:43:50 -0700	[thread overview]
Message-ID: <aec45003-3354-e49f-b032-5297e98722eb@gmail.com> (raw)
In-Reply-To: <20180430.113834.1760530542793231849.davem@davemloft.net>



On 04/30/2018 08:38 AM, David Miller wrote:
> From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
> Date: Fri, 27 Apr 2018 14:57:32 -0400
> 
>> Since the socket lock is not held when calculating the size of
>> receive queue, TCP_INQ is a hint.  For example, it can overestimate
>> the queue size by one byte, if FIN is received.
> 
> I think it is even worse than that.
> 
> If another application comes in and does a recvmsg() in parallel with
> these calculations, you could even report a negative value.
> 
> These READ_ONCE() make it look like some of these issues are being
> addressed but they are not.
> 
> You could freeze the values just by taking sk->sk_lock.slock, but I
> don't know if that cost is considered acceptable or not.
> 
> Another idea is to sample both values in a loop, similar to a sequence
> lock sequence:
> 
> again:
> 	tmp1 = A;
> 	tmp2 = B;
> 	barrier();
> 	tmp3 = A;
> 	if (tmp1 != tmp3)
> 		goto again;
> 
> But the current state of affairs is not going to work well.
> 

We want a hint, and max_t(int, 0, ....)  does not return a negative value ?

If the hint is wrong in 0.1 % of the cases, we really do not care, it is not meant
to replace the existing precise ( well, sort of ) mechanism.

I say sort of, because by the time we have any number, TCP might have received more packets anyway.

  reply	other threads:[~2018-04-30 15:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-27 18:57 [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read Soheil Hassas Yeganeh
2018-04-27 18:57 ` [PATCH V2 net-next 2/2] selftest: add test for TCP_INQ Soheil Hassas Yeganeh
2018-04-30 15:38 ` [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read David Miller
2018-04-30 15:43   ` Eric Dumazet [this message]
2018-04-30 15:56     ` David Miller
2018-04-30 16:01       ` Eric Dumazet
2018-04-30 16:10         ` David Miller
2018-04-30 16:59           ` Soheil Hassas Yeganeh
2018-04-30 15:59     ` Soheil Hassas Yeganeh

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=aec45003-3354-e49f-b032-5297e98722eb@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=soheil.kdev@gmail.com \
    --cc=soheil@google.com \
    --cc=willemb@google.com \
    --cc=ycheng@google.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.