On Tue, May 08, 2018 at 03:04:59PM +0200, Martin Pieuchot wrote:
> On 08/05/18(Tue) 14:32, Alexander Bluhm wrote:
> > @@ -176,12 +176,12 @@ do { \
> > struct ifnet *ifp = NULL; \
> > if (m && (m->m_flags & M_PKTHDR)) \
> > ifp = if_get(m->m_pkthdr.ph_ifidx); \
> > - if ((tp)->t_flags & TF_DELACK || \
> > + if (TCP_TIMER_ISARMED(tp, TCPT_DELACK) || \
> > (tcp_ack_on_push && (tiflags) & TH_PUSH) || \
> > (ifp && (ifp->if_flags & IFF_LOOPBACK))) \
> > tp->t_flags |= TF_ACKNOW; \
> > else \
> > - TCP_SET_DELACK(tp); \
> > + TCP_TIMER_ARM_MSEC(tp, TCPT_DELACK, tcp_delack_msecs); \
>
> Here you introduce a behavior change. TCP_SET_DELACK() would not call
> timeout_add_msec() if TF_DELACK was set, you're now calling it
> unconditionally. Is it safe? Or should you add a TCP_TIMER_ISARMED()
> check?
The TCP_TIMER_ARM_MSEC() is in the else block. The if condition
is (TCP_TIMER_ISARMED() || ...), so the timer is only started if
it was not set before.
bluhm
