Hi, > On 14/01/2021, at 8:33 PM, YASUOKA Masahiko <yasu...@openbsd.org> wrote: > > Hi, > > On Thu, 14 Jan 2021 08:54:36 +0900 > Yuichiro NAITO <naito.yuich...@gmail.com> wrote: >> Does anybody please review my code? >> >> Yasuoka-san is my coleague of my work. >> So, he is interested in this topic. That’s why I CCed this mail. >> I don’t mean he is an reviewer. >> >>> 2021/01/12 11:27、Yuichiro NAITO <naito.yuich...@gmail.com>のメール: >>> I have set up multiple peers in a wg0 interface, >>> and tried to remove more than one peers at once. >>> Ifconfig(1) only removes the first peer. >>> >>> Command line was like following. >>> >>> ``` >>> # ifconfig wg0 -wgpeer <keyA> -wgpeer <keyB> -wgpeer <keyC> >>> ``` >>> >>> Only <keyA> was removed. >>> >>> I think next peer pointer isn't calculated in case of removing peer >>> in sys/net/if_wg.c: wg_ioctl_set() function. >>> >>> I have tried following patch that can fix this problem. > > Yes, the diff seems good. > > I made the following whitespace change. > >> @@ -2333,6 +2333,11 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io >> *data) >> } >> >> peer_p = (struct wg_peer_io *)aip_p; >> + continue; >> + next_peer: >> + aip_p = &peer_p->p_aips[0]; >> + aip_p += peer_o.p_aips_count; >> + peer_p = (struct wg_peer_io *)aip_p; >> } >> >> error: > > It seems we prefer putting goto labels at the beginning of the line. > > > ok?
ok procter@ > > Fix wg(4) ioctl to be able to handle multiple wgpeers. > Diff from Yuichiro NAITO. > > Index: sys/net/if_wg.c > =================================================================== > RCS file: /cvs/src/sys/net/if_wg.c,v > retrieving revision 1.14 > diff -u -p -r1.14 if_wg.c > --- sys/net/if_wg.c 1 Sep 2020 19:06:59 -0000 1.14 > +++ sys/net/if_wg.c 14 Jan 2021 07:26:48 -0000 > @@ -2270,7 +2270,7 @@ wg_ioctl_set(struct wg_softc *sc, struct > > /* Peer must have public key */ > if (!(peer_o.p_flags & WG_PEER_HAS_PUBLIC)) > - continue; > + goto next_peer; > > /* 0 = latest protocol, 1 = this protocol */ > if (peer_o.p_protocol_version != 0) { > @@ -2283,7 +2283,7 @@ wg_ioctl_set(struct wg_softc *sc, struct > /* Get local public and check that peer key doesn't match */ > if (noise_local_keys(&sc->sc_local, public, NULL) == 0 && > bcmp(public, peer_o.p_public, WG_KEY_SIZE) == 0) > - continue; > + goto next_peer; > > /* Lookup peer, or create if it doesn't exist */ > if ((peer = wg_peer_lookup(sc, peer_o.p_public)) == NULL) { > @@ -2291,7 +2291,7 @@ wg_ioctl_set(struct wg_softc *sc, struct > * Also, don't create a new one if we only want to > * update. */ > if (peer_o.p_flags & (WG_PEER_REMOVE|WG_PEER_UPDATE)) > - continue; > + goto next_peer; > > if ((peer = wg_peer_create(sc, > peer_o.p_public)) == NULL) { > @@ -2303,7 +2303,7 @@ wg_ioctl_set(struct wg_softc *sc, struct > /* Remove peer and continue if specified */ > if (peer_o.p_flags & WG_PEER_REMOVE) { > wg_peer_destroy(peer); > - continue; > + goto next_peer; > } > > if (peer_o.p_flags & WG_PEER_HAS_ENDPOINT) > @@ -2332,6 +2332,11 @@ wg_ioctl_set(struct wg_softc *sc, struct > aip_p++; > } > > + peer_p = (struct wg_peer_io *)aip_p; > + continue; > +next_peer: > + aip_p = &peer_p->p_aips[0]; > + aip_p += peer_o.p_aips_count; > peer_p = (struct wg_peer_io *)aip_p; > } > >