On 2022/07/14 09:37, Mikolaj Kucharski wrote: > Hi, > > Per other thread, Theo expressed dissatisfaction with long ifconfig(8) > for wg(4) interface. Stuart Henderson pointed me at direction, which > below diff makes it work. > > I guess to questions are: > > - Does the behaviour of ifconfig(8) make sense? > - Does the code which makes above, make sense?
I think so, and the diff works exactly as I would expect it to. > This is minimal diff, I would appreciate feedback, I did least > resistance approach. Looking at diff -wu shows even less changes > as wg_status() is mainly identation with if-statement. > > > Short output by default, only 6 lines, no wgpeers section: > > pce-0067# ifconfig.ifaliases -a | tail -n6 > wg0: flags=80c3<UP,BROADCAST,RUNNING,NOARP,MULTICAST> mtu 1420 > index 8 priority 0 llprio 3 > wgport 51820 > wgpubkey qcb... > groups: wg > inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64 > > > Long output with <if> as an argument, wgpeers section present: > > pce-0067# ifconfig.ifaliases wg0 > wg0: flags=80c3<UP,BROADCAST,RUNNING,NOARP,MULTICAST> mtu 1420 > index 8 priority 0 llprio 3 > wgport 51820 > wgpubkey qcb... > wgpeer klM... > wgpsk (present) > wgpka 25 (sec) > wgendpoint xxx.xxx.xxx.xxx 51820 > tx: 178764, rx: 65100 > last handshake: 7 seconds ago > wgaip fde4:f456:48c2:13c0::/64 > groups: wg > inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64 > > > Above long output works with group as an argument (ifconfig wg) and with > option -A (ifconfig -A), so I think from user experience perspective, > works as expected. > > Manual page changes not provided, as I'm not sure are they needed with > this diff. At least a small change is needed. Maybe some different text would be more appropriate though. Index: ifconfig.8 =================================================================== RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v retrieving revision 1.384 diff -u -p -r1.384 ifconfig.8 --- ifconfig.8 27 Jun 2022 16:27:03 -0000 1.384 +++ ifconfig.8 20 Aug 2022 13:22:43 -0000 @@ -75,7 +75,7 @@ Only the superuser may modify the config The following options are available: .Bl -tag -width Ds .It Fl A -Causes full interface alias information for each interface to +Causes full interface alias and wgpeer information for each interface to be displayed. .It Fl a Causes > Comments? > > > Index: ifconfig.c > =================================================================== > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v > retrieving revision 1.456 > diff -u -p -u -r1.456 ifconfig.c > --- ifconfig.c 8 Jul 2022 07:04:54 -0000 1.456 > +++ ifconfig.c 14 Jul 2022 09:25:21 -0000 > @@ -363,7 +363,7 @@ void unsetwgpeer(const char *, int); > void unsetwgpeerpsk(const char *, int); > void unsetwgpeerall(const char *, int); > > -void wg_status(); > +void wg_status(int); > #else > void setignore(const char *, int); > #endif > @@ -679,7 +679,7 @@ void printgroupattribs(char *); > void printif(char *, int); > void printb_status(unsigned short, unsigned char *); > const char *get_linkstate(int, int); > -void status(int, struct sockaddr_dl *, int); > +void status(int, struct sockaddr_dl *, int, int); > __dead void usage(void); > const char *get_string(const char *, const char *, u_int8_t *, int *); > int len_string(const u_int8_t *, int); > @@ -1195,7 +1195,7 @@ printif(char *name, int ifaliases) > continue; > ifdata = ifa->ifa_data; > status(1, (struct sockaddr_dl *)ifa->ifa_addr, > - ifdata->ifi_link_state); > + ifdata->ifi_link_state, ifaliases); > count++; > noinet = 1; > continue; > @@ -3316,7 +3316,7 @@ get_linkstate(int mt, int link_state) > * specified, show it and it only; otherwise, show them all. > */ > void > -status(int link, struct sockaddr_dl *sdl, int ls) > +status(int link, struct sockaddr_dl *sdl, int ls, int ifaliases) > { > const struct afswtch *p = afp; > struct ifmediareq ifmr; > @@ -3391,7 +3391,7 @@ status(int link, struct sockaddr_dl *sdl > mpls_status(); > pflow_status(); > umb_status(); > - wg_status(); > + wg_status(ifaliases); > #endif > trunk_status(); > getifgroups(); > @@ -5907,7 +5907,7 @@ process_wg_commands(void) > } > > void > -wg_status(void) > +wg_status(int ifaliases) > { > size_t i, j, last_size; > struct timespec now; > @@ -5942,45 +5942,47 @@ wg_status(void) > printf("\twgpubkey %s\n", key); > } > > - wg_peer = &wg_interface->i_peers[0]; > - for (i = 0; i < wg_interface->i_peers_count; i++) { > - b64_ntop(wg_peer->p_public, WG_KEY_LEN, > - key, sizeof(key)); > - printf("\twgpeer %s\n", key); > - > - if (wg_peer->p_flags & WG_PEER_HAS_PSK) > - printf("\t\twgpsk (present)\n"); > - > - if (wg_peer->p_flags & WG_PEER_HAS_PKA && wg_peer->p_pka) > - printf("\t\twgpka %u (sec)\n", wg_peer->p_pka); > - > - if (wg_peer->p_flags & WG_PEER_HAS_ENDPOINT) { > - if (getnameinfo(&wg_peer->p_sa, wg_peer->p_sa.sa_len, > - hbuf, sizeof(hbuf), sbuf, sizeof(sbuf), > - NI_NUMERICHOST | NI_NUMERICSERV) == 0) > - printf("\t\twgendpoint %s %s\n", hbuf, sbuf); > - else > - printf("\t\twgendpoint unable to print\n"); > - } > + if (ifaliases) { > + wg_peer = &wg_interface->i_peers[0]; > + for (i = 0; i < wg_interface->i_peers_count; i++) { > + b64_ntop(wg_peer->p_public, WG_KEY_LEN, > + key, sizeof(key)); > + printf("\twgpeer %s\n", key); > + > + if (wg_peer->p_flags & WG_PEER_HAS_PSK) > + printf("\t\twgpsk (present)\n"); > + > + if (wg_peer->p_flags & WG_PEER_HAS_PKA && > wg_peer->p_pka) > + printf("\t\twgpka %u (sec)\n", wg_peer->p_pka); > + > + if (wg_peer->p_flags & WG_PEER_HAS_ENDPOINT) { > + if (getnameinfo(&wg_peer->p_sa, > wg_peer->p_sa.sa_len, > + hbuf, sizeof(hbuf), sbuf, sizeof(sbuf), > + NI_NUMERICHOST | NI_NUMERICSERV) == 0) > + printf("\t\twgendpoint %s %s\n", hbuf, > sbuf); > + else > + printf("\t\twgendpoint unable to > print\n"); > + } > > - printf("\t\ttx: %llu, rx: %llu\n", > - wg_peer->p_txbytes, wg_peer->p_rxbytes); > + printf("\t\ttx: %llu, rx: %llu\n", > + wg_peer->p_txbytes, wg_peer->p_rxbytes); > > - if (wg_peer->p_last_handshake.tv_sec != 0) { > - timespec_get(&now, TIME_UTC); > - printf("\t\tlast handshake: %lld seconds ago\n", > - now.tv_sec - wg_peer->p_last_handshake.tv_sec); > - } > + if (wg_peer->p_last_handshake.tv_sec != 0) { > + timespec_get(&now, TIME_UTC); > + printf("\t\tlast handshake: %lld seconds ago\n", > + now.tv_sec - > wg_peer->p_last_handshake.tv_sec); > + } > > > - wg_aip = &wg_peer->p_aips[0]; > - for (j = 0; j < wg_peer->p_aips_count; j++) { > - inet_ntop(wg_aip->a_af, &wg_aip->a_addr, > - hbuf, sizeof(hbuf)); > - printf("\t\twgaip %s/%d\n", hbuf, wg_aip->a_cidr); > - wg_aip++; > + wg_aip = &wg_peer->p_aips[0]; > + for (j = 0; j < wg_peer->p_aips_count; j++) { > + inet_ntop(wg_aip->a_af, &wg_aip->a_addr, > + hbuf, sizeof(hbuf)); > + printf("\t\twgaip %s/%d\n", hbuf, > wg_aip->a_cidr); > + wg_aip++; > + } > + wg_peer = (struct wg_peer_io *)wg_aip; > } > - wg_peer = (struct wg_peer_io *)wg_aip; > } > out: > free(wgdata.wgd_interface); > > -- > Regards, > Mikolaj >