> On 21 Apr 2022, at 02:14, Alexander Bluhm <alexander.bl...@gmx.net> wrote:
>
> Hi,
>
> In pfsync there are some KASSERT hidden behind #ifdef PFSYNC_DEBUG.
> That does not make sense to me. Either they are correct, then they
> should actively check in production. Or they got wrong over time,
> then they should not make debugging harder.
>
> Some basic testing did not show problems.
>
> ok?
ok
>
> bluhm
>
> Index: net/if_pfsync.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v
> retrieving revision 1.303
> diff -u -p -r1.303 if_pfsync.c
> --- net/if_pfsync.c 14 Apr 2022 11:39:44 -0000 1.303
> +++ net/if_pfsync.c 20 Apr 2022 14:00:58 -0000
> @@ -1620,9 +1620,7 @@ pfsync_drop_snapshot(struct pfsync_snaps
>
> while ((st = TAILQ_FIRST(&sn->sn_qs[q])) != NULL) {
> TAILQ_REMOVE(&sn->sn_qs[q], st, sync_list);
> -#ifdef PFSYNC_DEBUG
> KASSERT(st->sync_state == q);
> -#endif
> st->sync_state = PFSYNC_S_NONE;
> pf_state_unref(st);
> }
> @@ -1857,9 +1855,7 @@ pfsync_sendout(void)
> count = 0;
> while ((st = TAILQ_FIRST(&sn.sn_qs[q])) != NULL) {
> TAILQ_REMOVE(&sn.sn_qs[q], st, sync_list);
> -#ifdef PFSYNC_DEBUG
> KASSERT(st->sync_state == q);
> -#endif
> st->sync_state = PFSYNC_S_NONE;
> pfsync_qs[q].write(st, m->m_data + offset);
> offset += pfsync_qs[q].len;
> @@ -1916,9 +1912,7 @@ pfsync_insert_state(struct pf_state *st)
> ISSET(st->state_flags, PFSTATE_NOSYNC))
> return;
>
> -#ifdef PFSYNC_DEBUG
> KASSERT(st->sync_state == PFSYNC_S_NONE);
> -#endif
>
> if (sc->sc_len == PFSYNC_MINPKT)
> timeout_add_sec(&sc->sc_tmo, 1);
> @@ -2403,7 +2397,7 @@ pfsync_q_ins(struct pf_state *st, int q)
> struct pfsync_softc *sc = pfsyncif;
> size_t nlen, sclen;
>
> -#if defined(PFSYNC_DEBUG)
> +#ifdef DIAGNOSTIC
> if (sc->sc_len < PFSYNC_MINPKT)
> panic("pfsync pkt len is too low %zd", sc->sc_len);
> #endif
>
