Date: Thu, 7 Aug 2025 14:46:59 +0200 From: Michael Tuexen <tuexen@FreeBSD.org> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: feeb19201dca - main - tcp: improve consistency of KASSERTs in tcp_sack.c Message-ID: <8C353AC5-6E5E-45B6-AC48-D3001A37D4D8@FreeBSD.org> In-Reply-To: <aJPXT123XljlcxoX@cell.glebi.us> References: <202508060830.5768UUcN052133@gitrepo.freebsd.org> <aJPXT123XljlcxoX@cell.glebi.us>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 7. Aug 2025, at 00:29, Gleb Smirnoff <glebius@FreeBSD.org> wrote: > > On Wed, Aug 06, 2025 at 08:30:30AM +0000, Michael Tuexen wrote: > M> When panicing, don't print the condition, which was violated, > M> but the condition which holds at the time of the panic. > M> > M> Reviewed by: Nick Banks > M> MFC after: 1 week > M> Sponsored by: Netflix, Inc. > M> Differential Revision: https://reviews.freebsd.org/D51726 > M> --- > M> sys/netinet/tcp_sack.c | 30 +++++++++++++++++------------- > M> 1 file changed, 17 insertions(+), 13 deletions(-) > M> > M> diff --git a/sys/netinet/tcp_sack.c b/sys/netinet/tcp_sack.c > M> index 4405098a8620..f48e60207cc2 100644 > M> --- a/sys/netinet/tcp_sack.c > M> +++ b/sys/netinet/tcp_sack.c > M> @@ -283,7 +283,7 @@ tcp_update_sack_list(struct tcpcb *tp, tcp_seq rcv_start, tcp_seq rcv_end) > M> INP_WLOCK_ASSERT(tptoinpcb(tp)); > M> > M> /* Check arguments. */ > M> - KASSERT(SEQ_LEQ(rcv_start, rcv_end), ("rcv_start <= rcv_end")); > M> + KASSERT(SEQ_LEQ(rcv_start, rcv_end), ("SEG_GT(rcv_start, rcv_end)")); > M> > M> if ((rcv_start == rcv_end) && > M> (tp->rcv_numsacks >= 1) && > M> @@ -498,8 +498,8 @@ tcp_sackhole_free(struct tcpcb *tp, struct sackhole *hole) > M> tp->snd_numholes--; > M> atomic_subtract_int(&V_tcp_sack_globalholes, 1); > M> > M> - KASSERT(tp->snd_numholes >= 0, ("tp->snd_numholes >= 0")); > M> - KASSERT(V_tcp_sack_globalholes >= 0, ("tcp_sack_globalholes >= 0")); > M> + KASSERT(tp->snd_numholes >= 0, ("tp->snd_numholes < 0")); > M> + KASSERT(V_tcp_sack_globalholes >= 0, ("tcp_sack_globalholes < 0")); > > IMHO, better just through MPASS() instead of self-repetitive KASSERT. Up to now, tcp_sack.c does not use MPASS(). So I wanted to stay consistent.. I just go confused when looking at the panic messages from tcp_sack.c. Some messages printed the condition which should hold, some the condition which holds. Best regards Michael > > -- > Gleb Smirnoff
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8C353AC5-6E5E-45B6-AC48-D3001A37D4D8>