Hi,

I tried to address what jmc@ mentioned below. I don't really know
mdoc(7) and English is not my native language, so I imagine there is
place for improvement in the wg(4) diff.

Full diff at the very end of this email, below here `diff -wu` to
visualize how minimal ifconfig.c changes are:


| Index: sbin/ifconfig/ifconfig.c
| ===================================================================
| RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
| retrieving revision 1.456
| diff -u -p -w -u -r1.456 ifconfig.c
| --- sbin/ifconfig/ifconfig.c  8 Jul 2022 07:04:54 -0000       1.456
| +++ sbin/ifconfig/ifconfig.c  3 Sep 2022 08:25:00 -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,6 +5942,7 @@ wg_status(void)
|               printf("\twgpubkey %s\n", key);
|       }
|  
| +     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,
| @@ -5981,6 +5982,7 @@ wg_status(void)
|                       wg_aip++;
|               }
|               wg_peer = (struct wg_peer_io *)wg_aip;
| +             }
|       }
|  out:
|       free(wgdata.wgd_interface);



On Sat, Aug 20, 2022 at 07:15:39PM +0100, Jason McIntyre wrote:
> On Sat, Aug 20, 2022 at 02:25:25PM +0100, Stuart Henderson wrote:
> > > 
> > > 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
> > 
> 
> hi.
> 
> i think this just muddies the simple explanation of -A. i don;t think
> it's worth it for a single exception. what about amending wg(1) to note
> that this info is available via -A? then wg users can find it.
> 
> jmc


Index: sbin/ifconfig/ifconfig.c
===================================================================
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.456
diff -u -p -u -r1.456 ifconfig.c
--- sbin/ifconfig/ifconfig.c    8 Jul 2022 07:04:54 -0000       1.456
+++ sbin/ifconfig/ifconfig.c    3 Sep 2022 08:54:53 -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);
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 3 Sep 2022 08:54:53 -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


-- 
Regards,
 Mikolaj

Reply via email to