netdev - Re: [PATCH v5 net-next 01/13] tcp: reorganize tcp_in_ack_event() and tcp_count

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fd509ff1-c97a-625f-6423-cf24871cf124@kernel.org>
Date: Thu, 7 Nov 2024 21:32:54 +0200 (EET)
From: Ilpo Järvinen <ij@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>
cc: chia-yu.chang@...ia-bell-labs.com, netdev@...r.kernel.org, 
    dsahern@...il.com, davem@...emloft.net, dsahern@...nel.org, 
    pabeni@...hat.com, joel.granados@...nel.org, kuba@...nel.org, 
    andrew+netdev@...n.ch, horms@...nel.org, pablo@...filter.org, 
    kadlec@...filter.org, netfilter-devel@...r.kernel.org, 
    coreteam@...filter.org, ncardwell@...gle.com, 
    koen.de_schepper@...ia-bell-labs.com, g.white@...lelabs.com, 
    ingemar.s.johansson@...csson.com, mirja.kuehlewind@...csson.com, 
    cheshire@...le.com, rs.ietf@....at, Jason_Livingood@...cast.com, 
    vidhi_goel@...le.com
Subject: Re: [PATCH v5 net-next 01/13] tcp: reorganize tcp_in_ack_event()
 and tcp_count_delivered()

On Thu, 7 Nov 2024, Eric Dumazet wrote:

> On Tue, Nov 5, 2024 at 11:07 AM <chia-yu.chang@...ia-bell-labs.com> wrote:
> >
> > From: Ilpo Järvinen <ij@...nel.org>
> >
> > - Move tcp_count_delivered() earlier and split tcp_count_delivered_ce()
> >   out of it
> > - Move tcp_in_ack_event() later
> > - While at it, remove the inline from tcp_in_ack_event() and let
> >   the compiler to decide
> >
> > Accurate ECN's heuristics does not know if there is going
> > to be ACE field based CE counter increase or not until after
> > rtx queue has been processed. Only then the number of ACKed
> > bytes/pkts is available. As CE or not affects presence of
> > FLAG_ECE, that information for tcp_in_ack_event is not yet
> > available in the old location of the call to tcp_in_ack_event().
> >
> > Signed-off-by: Ilpo Järvinen <ij@...nel.org>
> > Signed-off-by: Chia-Yu Chang <chia-yu.chang@...ia-bell-labs.com>
> > ---
> >  net/ipv4/tcp_input.c | 56 +++++++++++++++++++++++++-------------------
> >  1 file changed, 32 insertions(+), 24 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 5bdf13ac26ef..fc52eab4fcc9 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c

> > @@ -3856,12 +3861,23 @@ static void tcp_process_tlp_ack(struct sock *sk, u32 ack, int flag)
> >         }
> >  }
> >
> > -static inline void tcp_in_ack_event(struct sock *sk, u32 flags)
> > +static void tcp_in_ack_event(struct sock *sk, int flag)
> >  {
> >         const struct inet_connection_sock *icsk = inet_csk(sk);
> >
> > -       if (icsk->icsk_ca_ops->in_ack_event)
> > -               icsk->icsk_ca_ops->in_ack_event(sk, flags);
> > +       if (icsk->icsk_ca_ops->in_ack_event) {
> > +               u32 ack_ev_flags = 0;
> > +
> > +               if (flag & FLAG_WIN_UPDATE)
> > +                       ack_ev_flags |= CA_ACK_WIN_UPDATE;
> > +               if (flag & FLAG_SLOWPATH) {
> > +                       ack_ev_flags = CA_ACK_SLOWPATH;
> 
> This is removing the potential CA_ACK_WIN_UPDATE, I would suggest :
> 
> ack_ev_flags |= CA_ACK_SLOWPATH;

Yes, a good catch.

-- 
 i.


> > +                       if (flag & FLAG_ECE)
> > +                               ack_ev_flags |= CA_ACK_ECE;
> > +               }
> > +
> > +               icsk->icsk_ca_ops->in_ack_event(sk, ack_ev_flags);
> > +       }
> >  }
> >
> >
>