On 2023/01/12 04:49, Mikolaj Kucharski wrote:
> Hi,
>
> Is there anything else which I can do, to help this diff reviwed and
> increase the chance of getting in?
>
> Thread at https://marc.info/?t=163478298600001&r=1&w=2
>
> Last version of the diff at
> https://marc.info/?l=openbsd-tech&m=167185582521873&q=mbox
Inlining that for a few comments, otherwise it's ok sthen
: Index: sbin/ifconfig/ifconfig.c
: ===================================================================
: RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
: retrieving revision 1.460
: diff -u -p -u -r1.460 ifconfig.c
: --- sbin/ifconfig/ifconfig.c 18 Dec 2022 18:56:38 -0000 1.460
: +++ sbin/ifconfig/ifconfig.c 24 Dec 2022 00:49:05 -0000
: @@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
: void setwgpeeraip(const char *, int);
: void setwgpeerpsk(const char *, int);
: void setwgpeerpka(const char *, int);
: +void setwgpeerdesc(const char *, int);
: void setwgport(const char *, int);
: void setwgkey(const char *, int);
: void setwgrtable(const char *, int);
:
: void unsetwgpeer(const char *, int);
: void unsetwgpeerpsk(const char *, int);
: +void unsetwgpeerdesc(const char *, int);
: void unsetwgpeerall(const char *, int);
:
: void wg_status(int);
: @@ -623,11 +625,13 @@ const struct cmd {
: { "wgaip", NEXTARG, A_WIREGUARD, setwgpeeraip},
: { "wgpsk", NEXTARG, A_WIREGUARD, setwgpeerpsk},
: { "wgpka", NEXTARG, A_WIREGUARD, setwgpeerpka},
: + { "wgdesc", NEXTARG, A_WIREGUARD, setwgpeerdesc},
: { "wgport", NEXTARG, A_WIREGUARD, setwgport},
: { "wgkey", NEXTARG, A_WIREGUARD, setwgkey},
: { "wgrtable", NEXTARG, A_WIREGUARD, setwgrtable},
: { "-wgpeer", NEXTARG, A_WIREGUARD, unsetwgpeer},
: { "-wgpsk", 0, A_WIREGUARD, unsetwgpeerpsk},
: + { "-wgdesc", 0, A_WIREGUARD, unsetwgpeerdesc},
: { "-wgpeerall", 0, A_WIREGUARD, unsetwgpeerall},
:
: #else /* SMALL */
: @@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
: }
:
: void
: +setwgpeerdesc(const char *wgdesc, int param)
: +{
: + if (wg_peer == NULL)
: + errx(1, "wgdesc: wgpeer not set");
: + if (strlen(wgdesc))
: + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
: + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
: +}
: +
: +void
: setwgport(const char *port, int param)
: {
: const char *errmsg = NULL;
: @@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
: }
:
: void
: +unsetwgpeerdesc(const char *value, int param)
: +{
: + if (wg_peer == NULL)
: + errx(1, "wgdesc: wgpeer not set");
: + strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
: + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
I was a bit confused by this at first (wondering if it should use
"&= ~WG_PEER_SET_DESCRIPTION"). I understand it now but I think that
a different name would make it clearer. Maybe WG_PEER_UPDATE_DESCR?
: +}
: +
: +void
: unsetwgpeerall(const char *value, int param)
: {
: ensurewginterface();
: @@ -5961,6 +5984,9 @@ wg_status(int ifaliases)
: b64_ntop(wg_peer->p_public, WG_KEY_LEN,
: key, sizeof(key));
: printf("\twgpeer %s\n", key);
: +
: + if (strlen(wg_peer->p_description))
: + printf("\t\twgdesc %s\n",
wg_peer->p_description);
:
: if (wg_peer->p_flags & WG_PEER_HAS_PSK)
: printf("\t\twgpsk (present)\n");
: Index: share/man/man4/wg.4
: ===================================================================
: RCS file: /cvs/src/share/man/man4/wg.4,v
: retrieving revision 1.10
: diff -u -p -u -r1.10 wg.4
: --- share/man/man4/wg.4 14 Mar 2021 10:08:38 -0000 1.10
: +++ share/man/man4/wg.4 24 Dec 2022 00:49:05 -0000
: @@ -42,6 +42,19 @@ configuration file for
: .Xr netstart 8 .
: The interface itself can be configured with
: .Xr ifconfig 8 .
: +To display
: +.Cm wgpeer
: +information for each
: +.Nm wg
: +interface option
: +.Fl A
: +to
: +.Xr ifconfig 8
: +should be used or
: +.Nm wg
: +interface should be specified as an argument to
: +.Xr ifconfig 8
: +command.
: .Pp
: .Nm wg
: interfaces support the following
This isn't directly related to the descr diff so should be a separate
commit if done.
This diff displays like so:
-----
The interfaces can be created at runtime using the ifconfig wgN create
command or by setting up a hostname.if(5) configuration file for
netstart(8). The interface itself can be configured with ifconfig(8).
To display wgpeer information for each wg interface option -A to
ifconfig(8) should be used or wg interface should be specified as an
argument to ifconfig(8) command.
[...]
-----
I find this new text a bit confusing really, and really it's more
general to ifconfig rather than specific to wg, so I'm not sure how
much explanation is really needed in wg(4). How about just adding
a couple of lines to Examples instead?
Index: wg.4
===================================================================
RCS file: /cvs/src/share/man/man4/wg.4,v
retrieving revision 1.10
diff -u -p -r1.10 wg.4
--- wg.4 14 Mar 2021 10:08:38 -0000 1.10
+++ wg.4 14 Jan 2023 14:24:58 -0000
@@ -164,6 +164,11 @@ which resides in the default
Show the listening sockets:
.Pp
.Dl $ netstat -ln
+.Pp
+Show full information for peers on a single interface, or all interfaces:
+.Pp
+.Dl # ifconfig wg1
+.Dl # ifconfig -A
.Sh DIAGNOSTICS
The
.Nm
: Index: sys/net/if_wg.c
: ===================================================================
: RCS file: /cvs/src/sys/net/if_wg.c,v
: retrieving revision 1.26
: diff -u -p -u -r1.26 if_wg.c
: --- sys/net/if_wg.c 21 Jul 2022 11:26:50 -0000 1.26
: +++ sys/net/if_wg.c 24 Dec 2022 00:49:06 -0000
: @@ -221,6 +221,9 @@ struct wg_peer {
:
: SLIST_ENTRY(wg_peer) p_start_list;
: int p_start_onlist;
: +
: + struct mutex p_description_mtx;
: + char p_description[IFDESCRSIZE];
: };
:
: struct wg_softc {
: @@ -275,6 +278,7 @@ int wg_peer_get_sockaddr(struct wg_peer
: void wg_peer_clear_src(struct wg_peer *);
: void wg_peer_get_endpoint(struct wg_peer *, struct wg_endpoint *);
: void wg_peer_counters_add(struct wg_peer *, uint64_t, uint64_t);
: +void wg_peer_set_description(struct wg_peer *, char *);
:
: int wg_aip_add(struct wg_softc *, struct wg_peer *, struct wg_aip_io *);
: struct wg_peer *
: @@ -407,6 +411,9 @@ wg_peer_create(struct wg_softc *sc, uint
: peer->p_counters_tx = 0;
: peer->p_counters_rx = 0;
:
: + mtx_init(&peer->p_description_mtx, IPL_NET);
: + memset(peer->p_description, 0, IFDESCRSIZE);
: +
: mtx_init(&peer->p_endpoint_mtx, IPL_NET);
: bzero(&peer->p_endpoint, sizeof(peer->p_endpoint));
:
: @@ -581,6 +588,15 @@ wg_peer_counters_add(struct wg_peer *pee
: mtx_leave(&peer->p_counters_mtx);
: }
:
: +void
: +wg_peer_set_description(struct wg_peer *peer, char *description)
: +{
: + mtx_enter(&peer->p_description_mtx);
: + memset(peer->p_description, 0, IFDESCRSIZE);
: + strlcpy(peer->p_description, description, IFDESCRSIZE);
: + mtx_leave(&peer->p_description_mtx);
: +}
: +
: int
: wg_aip_add(struct wg_softc *sc, struct wg_peer *peer, struct wg_aip_io *d)
: {
: @@ -2320,6 +2336,10 @@ wg_ioctl_set(struct wg_softc *sc, struct
: }
: }
:
: + if (peer_o.p_flags & WG_PEER_SET_DESCRIPTION) {
: + wg_peer_set_description(peer, peer_o.p_description);
: + }
: +
: aip_p = &peer_p->p_aips[0];
: for (j = 0; j < peer_o.p_aips_count; j++) {
: if ((ret = copyin(aip_p, &aip_o, sizeof(aip_o))) != 0)
: @@ -2429,6 +2449,8 @@ wg_ioctl_get(struct wg_softc *sc, struct
: aip_count++;
: }
: peer_o.p_aips_count = aip_count;
: +
: + strlcpy(peer_o.p_description, peer->p_description, IFDESCRSIZE);
:
: if ((ret = copyout(&peer_o, peer_p, sizeof(peer_o))) != 0)
: goto unlock_and_ret_size;
: Index: sys/net/if_wg.h
: ===================================================================
: RCS file: /cvs/src/sys/net/if_wg.h,v
: retrieving revision 1.4
: diff -u -p -u -r1.4 if_wg.h
: --- sys/net/if_wg.h 22 Jun 2020 12:20:44 -0000 1.4
: +++ sys/net/if_wg.h 24 Dec 2022 00:49:06 -0000
: @@ -61,6 +61,7 @@ struct wg_aip_io {
: #define WG_PEER_REPLACE_AIPS (1 << 4)
: #define WG_PEER_REMOVE (1 << 5)
: #define WG_PEER_UPDATE (1 << 6)
: +#define WG_PEER_SET_DESCRIPTION (1 << 7)
:
: #define p_sa p_endpoint.sa_sa
: #define p_sin p_endpoint.sa_sin
: @@ -80,6 +81,7 @@ struct wg_peer_io {
: uint64_t p_txbytes;
: uint64_t p_rxbytes;
: struct timespec p_last_handshake; /* nanotime */
: + char p_description[IFDESCRSIZE];
: size_t p_aips_count;
: struct wg_aip_io p_aips[];
: };
:
: --
: Regards,
: Mikolaj
:
