Now pipex(4) session can't be grabbed while going to be destroyed. So `pppx_ifs' tree can't be broken by NET_LOCK() dances in pppx_if_destroy() and I guess it's time to remove panic()'s from pppx(4).
claudio@ pointed in [1] we can avoid two lookups while inserting `pxi' into `pppx_ifs' so I removed RBT_FIND() from pppx_add_session(). Also there is no context switch between RBT_INSERT() and RBT_REMOVE() in error path so additional checks for RBT_REMOVE() are unnecessary. I replaced panic() by KASSERT() in pppx_if_destroy(). It's also unnecessary but, if I remember correct, claudio@ privately said he like to keep assertion here. 1. https://marc.info/?l=openbsd-tech&m=158625333217729&w=2 Index: sys/net/if_pppx.c =================================================================== RCS file: /cvs/src/sys/net/if_pppx.c,v retrieving revision 1.88 diff -u -p -r1.88 if_pppx.c --- sys/net/if_pppx.c 18 Jun 2020 14:20:12 -0000 1.88 +++ sys/net/if_pppx.c 18 Jun 2020 16:40:18 -0000 @@ -696,14 +696,10 @@ pppx_add_session(struct pppx_dev *pxd, s pxi->pxi_key.pxik_protocol = req->pr_protocol; pxi->pxi_dev = pxd; - /* this is safe without splnet since we're not modifying it */ - if (RBT_FIND(pppx_ifs, &pppx_ifs, pxi) != NULL) { + if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL) { error = EADDRINUSE; goto out; } - - if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL) - panic("%s: pppx_ifs modified while lock was held", __func__); LIST_INSERT_HEAD(&pxd->pxd_pxis, pxi, pxi_list); snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit); @@ -776,8 +772,7 @@ pppx_add_session(struct pppx_dev *pxd, s return (error); remove: - if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL) - panic("%s: pppx_ifs modified while lock was held", __func__); + RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi); LIST_REMOVE(pxi, pxi_list); out: pool_put(pppx_if_pl, pxi); @@ -870,8 +865,7 @@ pppx_if_destroy(struct pppx_dev *pxd, st NET_LOCK(); pipex_rele_session(session); - if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL) - panic("%s: pppx_ifs modified while lock was held", __func__); + KASSERT(RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) != NULL); LIST_REMOVE(pxi, pxi_list); pool_put(pppx_if_pl, pxi);
