Re: git: 7121e9414f29 - main - wg: Improve wg_peer_alloc() to simplify the calling
- In reply to: Kyle Evans : "git: 7121e9414f29 - main - wg: Improve wg_peer_alloc() to simplify the calling"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 18 Apr 2025 00:31:38 UTC
On 4/17/25 19:30, Kyle Evans wrote: > The branch main has been updated by kevans: > > URL: https://cgit.FreeBSD.org/src/commit/?id=7121e9414f294d116caeadd07ebd969136d3a631 > > commit 7121e9414f294d116caeadd07ebd969136d3a631 > Author: Aaron LI <aly@aaronly.me> > AuthorDate: 2025-04-18 00:30:11 +0000 > Commit: Kyle Evans <kevans@FreeBSD.org> > CommitDate: 2025-04-18 00:30:11 +0000 > > wg: Improve wg_peer_alloc() to simplify the calling > > Move the necessary extra logics (i.e., noise_remote_enable() and > TAILQ_INSERT_TAIL()) from wg_ioctl_set() to wg_peer_alloc(), and thus > make it easier to be called. Actually, the updated version is more > asymmetric to wg_peer_destroy() and thus less likely to be misused. > Meanwhile, rename it to wg_peer_create() to look more consistent with > wg_peer_destroy(). > > Reviewed by: aly_aaronly.me (diff), markj > Obtained from: DragonflyBSD 902964ab24ba (with some changes) > --- > sys/dev/wg/if_wg.c | 42 ++++++++++++++++++++++++++---------------- > 1 file changed, 26 insertions(+), 16 deletions(-) > Sigh, sorry, forgot to tag this one: Differential Revision: https://reviews.freebsd.org/D49796 Thanks, Kyle Evans > diff --git a/sys/dev/wg/if_wg.c b/sys/dev/wg/if_wg.c > index 83e5d9e5ceb3..5a3b60e45b7a 100644 > --- a/sys/dev/wg/if_wg.c > +++ b/sys/dev/wg/if_wg.c > @@ -315,7 +315,8 @@ static void wg_timers_run_persistent_keepalive(void *); > static int wg_aip_add(struct wg_softc *, struct wg_peer *, sa_family_t, const void *, uint8_t); > static struct wg_peer *wg_aip_lookup(struct wg_softc *, sa_family_t, void *); > static void wg_aip_remove_all(struct wg_softc *, struct wg_peer *); > -static struct wg_peer *wg_peer_alloc(struct wg_softc *, const uint8_t [WG_KEY_SIZE]); > +static struct wg_peer *wg_peer_create(struct wg_softc *, > + const uint8_t [WG_KEY_SIZE], int *); > static void wg_peer_free_deferred(struct noise_remote *); > static void wg_peer_destroy(struct wg_peer *); > static void wg_peer_destroy_all(struct wg_softc *); > @@ -378,18 +379,26 @@ static void wg_module_deinit(void); > > /* TODO Peer */ > static struct wg_peer * > -wg_peer_alloc(struct wg_softc *sc, const uint8_t pub_key[WG_KEY_SIZE]) > +wg_peer_create(struct wg_softc *sc, const uint8_t pub_key[WG_KEY_SIZE], > + int *errp) > { > struct wg_peer *peer; > > sx_assert(&sc->sc_lock, SX_XLOCKED); > > peer = malloc(sizeof(*peer), M_WG, M_WAITOK | M_ZERO); > + > peer->p_remote = noise_remote_alloc(sc->sc_local, peer, pub_key); > - peer->p_tx_bytes = counter_u64_alloc(M_WAITOK); > - peer->p_rx_bytes = counter_u64_alloc(M_WAITOK); > + if ((*errp = noise_remote_enable(peer->p_remote)) != 0) { > + noise_remote_free(peer->p_remote, NULL); > + free(peer, M_WG); > + return (NULL); > + } > + > peer->p_id = peer_counter++; > peer->p_sc = sc; > + peer->p_tx_bytes = counter_u64_alloc(M_WAITOK); > + peer->p_rx_bytes = counter_u64_alloc(M_WAITOK); > > cookie_maker_init(&peer->p_cookie, pub_key); > > @@ -420,6 +429,13 @@ wg_peer_alloc(struct wg_softc *sc, const uint8_t pub_key[WG_KEY_SIZE]) > LIST_INIT(&peer->p_aips); > peer->p_aips_num = 0; > > + TAILQ_INSERT_TAIL(&sc->sc_peers, peer, p_entry); > + sc->sc_peers_num++; > + > + if (if_getlinkstate(sc->sc_ifp) == LINK_STATE_UP) > + wg_timers_enable(peer); > + > + DPRINTF(sc, "Peer %" PRIu64 " created\n", peer->p_id); > return (peer); > } > > @@ -2376,7 +2392,7 @@ wg_peer_add(struct wg_softc *sc, const nvlist_t *nvl) > size_t size; > struct noise_remote *remote; > struct wg_peer *peer = NULL; > - bool need_insert = false; > + bool need_cleanup = false; > > sx_assert(&sc->sc_lock, SX_XLOCKED); > > @@ -2408,8 +2424,10 @@ wg_peer_add(struct wg_softc *sc, const nvlist_t *nvl) > wg_aip_remove_all(sc, peer); > } > if (peer == NULL) { > - peer = wg_peer_alloc(sc, pub_key); > - need_insert = true; > + peer = wg_peer_create(sc, pub_key, &err); > + if (peer == NULL) > + goto out; > + need_cleanup = true; > } > if (nvlist_exists_binary(nvl, "endpoint")) { > endpoint = nvlist_get_binary(nvl, "endpoint", &size); > @@ -2467,19 +2485,11 @@ wg_peer_add(struct wg_softc *sc, const nvlist_t *nvl) > } > } > } > - if (need_insert) { > - if ((err = noise_remote_enable(peer->p_remote)) != 0) > - goto out; > - TAILQ_INSERT_TAIL(&sc->sc_peers, peer, p_entry); > - sc->sc_peers_num++; > - if (if_getlinkstate(sc->sc_ifp) == LINK_STATE_UP) > - wg_timers_enable(peer); > - } > if (remote != NULL) > noise_remote_put(remote); > return (0); > out: > - if (need_insert) /* If we fail, only destroy if it was new. */ > + if (need_cleanup) /* If we fail, only destroy if it was new. */ > wg_peer_destroy(peer); > if (remote != NULL) > noise_remote_put(remote);