Skip site navigation (1)Skip section navigation (2)
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:
>=20
> 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>    =20
> 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>=20
> 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> =20
> M>   /* Check arguments. */
> M> - KASSERT(SEQ_LEQ(rcv_start, rcv_end), ("rcv_start <=3D rcv_end"));
> M> + KASSERT(SEQ_LEQ(rcv_start, rcv_end), ("SEG_GT(rcv_start, =
rcv_end)"));
> M> =20
> M>   if ((rcv_start =3D=3D rcv_end) &&
> M>      (tp->rcv_numsacks >=3D 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> =20
> M> - KASSERT(tp->snd_numholes >=3D 0, ("tp->snd_numholes >=3D 0"));
> M> - KASSERT(V_tcp_sack_globalholes >=3D 0, ("tcp_sack_globalholes >=3D =
0"));
> M> + KASSERT(tp->snd_numholes >=3D 0, ("tp->snd_numholes < 0"));
> M> + KASSERT(V_tcp_sack_globalholes >=3D 0, ("tcp_sack_globalholes < =
0"));
>=20
> 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
>=20
> --=20
> Gleb Smirnoff




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8C353AC5-6E5E-45B6-AC48-D3001A37D4D8>