Hello, my original idea was to have a one mutex per pfsync queue. however it creates things more complicated then necessary for queues, which keep pf(4) state. hrvoje@ hit some panics in this area recently. bluhm@ and I are still looking at those issues. However there is the first change in order to make code more simple: diff below merges the array of ->sc_mtx[] into a single mutex.
OK ? thanks and regards sashan --------8<---------------8<---------------8<------------------8<-------- diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c index edeb2209161..9a92fc4d21f 100644 --- a/sys/net/if_pfsync.c +++ b/sys/net/if_pfsync.c @@ -212,7 +212,7 @@ struct pfsync_softc { struct ip sc_template; struct pf_state_queue sc_qs[PFSYNC_S_COUNT]; - struct mutex sc_mtx[PFSYNC_S_COUNT]; + struct mutex sc_st_mtx; size_t sc_len; struct pfsync_upd_reqs sc_upd_req_list; @@ -324,13 +324,6 @@ pfsync_clone_create(struct if_clone *ifc, int unit) struct pfsync_softc *sc; struct ifnet *ifp; int q; - static const char *mtx_names[] = { - "iack_mtx", - "upd_c_mtx", - "del_mtx", - "ins_mtx", - "upd_mtx", - "" }; if (unit != 0) return (EINVAL); @@ -338,10 +331,9 @@ pfsync_clone_create(struct if_clone *ifc, int unit) pfsync_sync_ok = 1; sc = malloc(sizeof(*pfsyncif), M_DEVBUF, M_WAITOK|M_ZERO); - for (q = 0; q < PFSYNC_S_COUNT; q++) { + for (q = 0; q < PFSYNC_S_COUNT; q++) TAILQ_INIT(&sc->sc_qs[q]); - mtx_init_flags(&sc->sc_mtx[q], IPL_SOFTNET, mtx_names[q], 0); - } + mtx_init_flags(&sc->sc_st_mtx, IPL_SOFTNET, "st_mtx", 0); pool_init(&sc->sc_pool, PFSYNC_PLSIZE, 0, IPL_SOFTNET, 0, "pfsync", NULL); @@ -1585,9 +1577,7 @@ pfsync_grab_snapshot(struct pfsync_snapshot *sn, struct pfsync_softc *sc) sn->sn_sc = sc; - for (q = 0; q < PFSYNC_S_COUNT; q++) - mtx_enter(&sc->sc_mtx[q]); - + mtx_enter(&sc->sc_st_mtx); mtx_enter(&sc->sc_upd_req_mtx); mtx_enter(&sc->sc_tdb_mtx); @@ -1612,9 +1602,7 @@ pfsync_grab_snapshot(struct pfsync_snapshot *sn, struct pfsync_softc *sc) mtx_leave(&sc->sc_tdb_mtx); mtx_leave(&sc->sc_upd_req_mtx); - - for (q = (PFSYNC_S_COUNT - 1); q >= 0; q--) - mtx_leave(&sc->sc_mtx[q]); + mtx_leave(&sc->sc_st_mtx); } void @@ -2420,14 +2408,14 @@ pfsync_q_ins(struct pf_state *st, int q) panic("pfsync pkt len is too low %zd", sc->sc_len); #endif do { - mtx_enter(&sc->sc_mtx[q]); + mtx_enter(&sc->sc_st_mtx); /* * If two threads are competing to insert the same state, then * there must be just single winner. */ if (st->sync_state != PFSYNC_S_NONE) { - mtx_leave(&sc->sc_mtx[q]); + mtx_leave(&sc->sc_st_mtx); break; } @@ -2439,7 +2427,7 @@ pfsync_q_ins(struct pf_state *st, int q) sc_len = atomic_add_long_nv(&sc->sc_len, nlen); if (sc_len > sc->sc_if.if_mtu) { atomic_sub_long(&sc->sc_len, nlen); - mtx_leave(&sc->sc_mtx[q]); + mtx_leave(&sc->sc_st_mtx); pfsync_sendout(); continue; } @@ -2448,7 +2436,7 @@ pfsync_q_ins(struct pf_state *st, int q) TAILQ_INSERT_TAIL(&sc->sc_qs[q], st, sync_list); st->sync_state = q; - mtx_leave(&sc->sc_mtx[q]); + mtx_leave(&sc->sc_st_mtx); } while (0); } @@ -2456,18 +2444,19 @@ void pfsync_q_del(struct pf_state *st) { struct pfsync_softc *sc = pfsyncif; - int q = st->sync_state; + int q; KASSERT(st->sync_state != PFSYNC_S_NONE); - mtx_enter(&sc->sc_mtx[q]); + mtx_enter(&sc->sc_st_mtx); + q = st->sync_state; atomic_sub_long(&sc->sc_len, pfsync_qs[q].len); TAILQ_REMOVE(&sc->sc_qs[q], st, sync_list); if (TAILQ_EMPTY(&sc->sc_qs[q])) atomic_sub_long(&sc->sc_len, sizeof (struct pfsync_subheader)); - mtx_leave(&sc->sc_mtx[q]); - st->sync_state = PFSYNC_S_NONE; + mtx_leave(&sc->sc_st_mtx); + pf_state_unref(st); }