Re: tcp keep-alives sent without timestamps

[Mike Belopuhov]

On Tue, Apr 14, 2015 at 22:08 +0300, Lauri Tirkkonen wrote:
> On Tue, Apr 14 2015 20:40:58 +0200, Mike Belopuhov wrote:
> > According to 3.2 in RFC 7323:
> > 
> >    Once TSopt has been successfully negotiated, that is both <SYN> and
> >    <SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
> >    segment for the duration of the connection, and SHOULD be sent in an
> >    <RST> segment (see Section 5.2 for details).  The TCP SHOULD remember
> >    this state by setting a flag, referred to as Snd.TS.OK, to one.  If a
> >    non-<RST> segment is received without a TSopt, a TCP SHOULD silently
> >    drop the segment.  A TCP MUST NOT abort a TCP connection because any
> >    segment lacks an expected TSopt.
> 
> Thank you, I somehow missed the existence of this RFC.
>

Does anyone else want to comment on this?

> > I had a stab at adding timestamp support to tcp_respond but couldn't
> > test yet.  If you feel like giving it a try, please be my guest.
> 
> With your patch, I confirm that timestamps are present on keep-alive
> messages.
> 

The patch needs a small (but crucial) amendment: tcp pcb can be NULL...

diff --git sys/netinet/tcp_subr.c sys/netinet/tcp_subr.c
index c8c8e77..6f17af0 100644
--- sys/netinet/tcp_subr.c
+++ sys/netinet/tcp_subr.c
@@ -295,13 +295,10 @@ tcp_template(tp)
  * attached mbufs.
  *
  * In any case the ack and sequence number of the transmitted
  * segment are as specified by the parameters.
  */
-#ifdef INET6
-/* This function looks hairy, because it was so IPv4-dependent. */
-#endif /* INET6 */
 void
 tcp_respond(struct tcpcb *tp, caddr_t template, struct tcphdr *th0,
     tcp_seq ack, tcp_seq seq, int flags, u_int rtableid)
 {
        int tlen;
@@ -370,14 +367,10 @@ tcp_respond(struct tcpcb *tp, caddr_t template, struct 
tcphdr *th0,
                xchg(th->th_dport, th->th_sport, u_int16_t);
        else
                flags = TH_ACK;
 #undef xchg
 
-       m->m_len = tlen;
-       m->m_pkthdr.len = tlen;
-       m->m_pkthdr.rcvif = (struct ifnet *) 0;
-       m->m_pkthdr.csum_flags |= M_TCP_CSUM_OUT;
        th->th_seq = htonl(seq);
        th->th_ack = htonl(ack);
        th->th_x2 = 0;
        th->th_off = sizeof (struct tcphdr) >> 2;
        th->th_flags = flags;
@@ -386,10 +379,26 @@ tcp_respond(struct tcpcb *tp, caddr_t template, struct 
tcphdr *th0,
        if (win > TCP_MAXWIN)
                win = TCP_MAXWIN;
        th->th_win = htons((u_int16_t)win);
        th->th_urp = 0;
 
+       if (tp && (tp->t_flags & (TF_REQ_TSTMP|TF_NOOPT)) == TF_REQ_TSTMP &&
+           (flags & TH_RST) == 0 && (tp->t_flags & TF_RCVD_TSTMP)) {
+               u_int32_t *lp = (u_int32_t *)(th + 1);
+               /* Form timestamp option as shown in appendix A of RFC 1323. */
+               *lp++ = htonl(TCPOPT_TSTAMP_HDR);
+               *lp++ = htonl(tcp_now + tp->ts_modulate);
+               *lp   = htonl(tp->ts_recent);
+               tlen += TCPOLEN_TSTAMP_APPA;
+               th->th_off = (sizeof(struct tcphdr) + TCPOLEN_TSTAMP_APPA) >> 2;
+       }
+
+       m->m_len = tlen;
+       m->m_pkthdr.len = tlen;
+       m->m_pkthdr.rcvif = (struct ifnet *) 0;
+       m->m_pkthdr.csum_flags |= M_TCP_CSUM_OUT;
+
        /* force routing table */
        if (tp)
                m->m_pkthdr.ph_rtableid = tp->t_inpcb->inp_rtableid;
        else
                m->m_pkthdr.ph_rtableid = rtableid;