Re: [Idr] TCP & BGP: Some don't send terminate BGP when holdtimer expired, because TCP recv window is 0

[Enke Chen <[e-mail]>]

Hi, John and William:

My reading of the Linux function tcp_probe_timer()  is that the data in the
socket buffer is checked. More specifically, when there is no
un-acked data, but there is data in the socket buffer,
the "icsk_user_timeout" would be checked, and the probe timer would be set
again in tcp_send_probe0().

I am not sure what could have caused the failure that William observed. We
will need someone who is familiar with the TCP code to take a look.
There might be one potential issue in tcp_check_probe_timer() where the
probe timer is not started (please see below).

Thanks.   -- Enke

----------------------
Linux v5.10-rc7-149-g33dc961

*diff --git a/include/net/tcp.h b/include/net/tcp.h*

*index d4ef5bf..0b28af1 100644*

*--- a/include/net/tcp.h*

*+++ b/include/net/tcp.h*

@@ -1328,7 +1328,8 @@ static inline unsigned long tcp_probe0_when(const
struct sock *sk,



 static inline void tcp_check_probe_timer(struct sock *sk)

 {

-       if (!tcp_sk(sk)->packets_out && !inet_csk(sk)->icsk_pending)

+       if (!tcp_sk(sk)->packets_out &&

+           (inet_csk(sk)->icsk_pending != ICSK_TIME_PROBE0))

                tcp_reset_xmit_timer(sk, ICSK_TIME_PROBE0,

                                     tcp_probe0_base(sk), TCP_RTO_MAX);
 }

----------


static void tcp_probe_timer(struct sock *sk)

{

        struct inet_connection_sock *icsk = inet_csk(sk);

        struct sk_buff *skb = tcp_send_head(sk);

        struct tcp_sock *tp = tcp_sk(sk);

        int max_probes;


        if (tp->packets_out || !skb) {

                icsk->icsk_probes_out = 0;

                return;

        }


        /* RFC 1122 4.2.2.17 requires the sender to stay open indefinitely
as

         * long as the receiver continues to respond probes. We support
this by

         * default and reset icsk_probes_out with incoming ACKs. But if the

         * socket is orphaned or the user specifies TCP_USER_TIMEOUT, we

         * kill the socket when the retry count and the time exceeds the

         * corresponding system limit. We also implement similar policy when

         * we use RTO to probe window in tcp_retransmit_timer().

         */

        if (icsk->icsk_user_timeout) {

                u32 elapsed = tcp_model_timeout(sk, icsk->icsk_probes_out,

                                                tcp_probe0_base(sk));


                if (elapsed >= icsk->icsk_user_timeout)

                        goto abort;

        }


On Sat, Dec 19, 2020 at 2:38 AM William McCall <[e-mail]>
wrote:

> On Fri, Dec 18, 2020 at 10:33 PM John Scudder
> <jgs=[e-mail]> wrote:
> >
> > On Dec 18, 2020, at 1:09 PM, Enke Chen <[e-mail]>
> wrote:
> > >
> > > No, I am not assuming that packets are getting somewhere. The
> TCP_USER_TIMEOUT would work as long as there is "pending data" (either
> unacked, or locally queued). The data can be from the local BGP Keepalives
> or the TCP_KEEPALIVE.
> >
> > Apart from the other objections to relying on TCP_USER_TIMEOUT, which I
> think are sufficient, it’s not clear to me that implementations will
> provide the desired semantics. RFC 793 seems like it specifies the right
> semantics (“get this data to the peer within N seconds or close”):
> >
> >         The timeout, if present, permits the caller to set up a timeout
> >         for all data submitted to TCP.  If data is not successfully
> >         delivered to the destination within the timeout period, the TCP
> >         will abort the connection.  The present global default is five
> >         minutes.
> >
> > However the Linux man page documents different semantics:
> >
> >        TCP_USER_TIMEOUT (since Linux 2.6.37)
> >               This option takes an unsigned int as an argument.  When the
> >               value is greater than 0, it specifies the maximum amount of
> >               time in milliseconds that transmitted data may remain
> >               unacknowledged before TCP will forcibly close the
> >               corresponding connection and return ETIMEDOUT to the
> >               application.  If the option value is specified as 0, TCP
> will
> >               use the system default.
> >
> > The important difference being that whereas 793 implies data written to
> the socket, the Linux man page says “transmitted” data, which seems like it
> must mean data TCP has written to the network. These are two very different
> things! If Linux (or another stack) implements what the man page seems to
> say, it’s not useful for our purposes.
> >
> > —John
> > _______________________________________________
> > Idr mailing list
> > [e-mail]
> >
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_idr&d=DwIFaQ&c=V9IgWpI5PvzTw83UyHGVSoW3Uc1MFWe5J8PTfkrzVSo&r=OPLTTSu-451-QhDoSINhI2xYdwiMmfF5A2l8luvN11E&m=P-eZWmrFtootouPUugKAk40aIyuZdrP9wLMCSS7GUTU&s=6oYcnalNTtK-8ktoh-vivM6BlWM0bCrW3WuHw19s7zo&e=
>
> I was curious too. I read the manpage, relevant linux kernel code, the
> RFC, and hacked up a test case (unicast me if you want the code).
> Also, Cloudflare published a relevant blog entry[0]. For this specific
> scenario, see under the sub-heading "Zero window ESTAB is...
> forever?".
>
> TCP_USER_TIMEOUT doesn't appear to kick in until there is unACKed
> data, meaning that it has already been transmitted from TCP's
> perspective. Stuff hanging around in the buffers due to persist state
> doesn't seem to count, per the test results and the docs. Confirms
> your thoughts from the reading I think.
>
> [0]
> https://urldefense.proofpoint.com/v2/url?u=https-3A__blog.cloudflare.com_when-2Dtcp-2Dsockets-2Drefuse-2Dto-2Ddie_&d=DwIFaQ&c=V9IgWpI5PvzTw83UyHGVSoW3Uc1MFWe5J8PTfkrzVSo&r=OPLTTSu-451-QhDoSINhI2xYdwiMmfF5A2l8luvN11E&m=P-eZWmrFtootouPUugKAk40aIyuZdrP9wLMCSS7GUTU&s=M-HzefvcFBD2FU8OVERU_vTL_ObzcdQdlk0BUrADphk&e=
>
> --
> William McCall
>