Messages in this thread | | | From | Yafang Shao <> | Date | Mon, 30 Jul 2018 10:06:02 +0800 | Subject | Re: [PATCH net-next 1/2] tcp: call tcp_drop() in tcp collapse |
| |
On Sun, Jul 29, 2018 at 12:28 AM, Eric Dumazet <edumazet@google.com> wrote: > On Sat, Jul 28, 2018 at 12:43 AM Yafang Shao <laoar.shao@gmail.com> wrote: >> >> On Sat, Jul 28, 2018 at 11:38 AM, Eric Dumazet <edumazet@google.com> wrote: >> > On Fri, Jul 27, 2018 at 8:35 PM Yafang Shao <laoar.shao@gmail.com> wrote: >> > >> >> So what about LINUX_MIB_TCPOFOMERGE ? >> >> Regarding LINUX_MIB_TCPOFOMERGE, a skb is already covered by another >> >> skb, is that dropping the packet or simply lowering the memory >> >> overhead ? >> > >> > What do you think ? >> > >> > If you receive two times the same payload, don't you have to drop one >> > of the duplicate ? >> > >> > There is a a big difference between the two cases. >> >> If the drop caused some data lost (which may then cause retransmition >> or something), then this is a really DROP. >> While if the drop won't cause any data lost, meaning it is a >> non-harmful behavior, I think it should not be defined as DROP. >> This is my suggestion anyway. > > Sigh. > > We count drops, not because they are ' bad or something went wrong'. > > If TCP stack receives twice the same sequence (same payload), we > _drop_ one of the duplicate, so we account for this event. > > When ' collapsing' we reorganize our own storage, not because we have > to drop a payload, > but for some memory pressure reason.
Thanks for you clarification. So what about LINUX_MIB_TCPOFODROP ?
if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) { NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP); tcp_drop(sk, skb); return; }
It is also because of our own memory pressure, but we call tcp_drop() here.
I am not mean to disagree with you. I am just confused and want to make it clear.
> We have specific SNMP counters to account for these, we do not want to > pretend a packet was ' dropped' since it was not. > > If we have to _drop_ some packets, it is called Pruning, and we do > properly account for these drops.
Agreed.
Thanks Yafang
| |