On Tue, Mar 08, 2022 at 07:17:33PM +0100, Stefan Sperling wrote:
> On Tue, Mar 08, 2022 at 03:55:48PM +0100, Stefan Sperling wrote:
> > On Mon, Mar 07, 2022 at 03:04:06PM -0700, Theo de Raadt wrote:
> > > > For now, the structs are identical so the code copying data out is
> > > > kept simple.
> > > 
> > > I think this is unwise, and you should write the field-by-field copying
> > > function at the same time, otherwise this is just asking for trouble.
> > > You really cannot wait until an intentional change.
> > 
> > Sure, here it is.
> 
> On second thought, avoiding the malloc/free dance is better.
> The struct is still small enough to fit on the stack.
> 
> diff refs/heads/master refs/heads/statsreq
> blob - 85d795d745eb21fd218056c2f3faf7fbc2c7fe49
> blob + 62938001ed22fc133a0c98e27ef5690c978e21f3
> --- sys/net80211/ieee80211_ioctl.c
> +++ sys/net80211/ieee80211_ioctl.c
> @@ -55,6 +55,8 @@ void         ieee80211_node2req(struct ieee80211com *,
>           const struct ieee80211_node *, struct ieee80211_nodereq *);
>  void  ieee80211_req2node(struct ieee80211com *,
>           const struct ieee80211_nodereq *, struct ieee80211_node *);
> +void ieee80211_stats2req(struct ieee80211_statsreq *,
> +         struct ieee80211_stats *);
>  
>  void
>  ieee80211_node2req(struct ieee80211com *ic, const struct ieee80211_node *ni,
> @@ -180,6 +182,89 @@ ieee80211_req2node(struct ieee80211com *ic, const stru
>  }
>  
>  void
> +ieee80211_stats2req(struct ieee80211_statsreq *req,
> +    struct ieee80211_stats *stats)
> +{
> +     memset(req, 0, sizeof(*req));
> +
> +     req->is_rx_badversion = stats->is_rx_badversion;
> +     req->is_rx_tooshort = stats->is_rx_tooshort;
> +     req->is_rx_wrongbss = stats->is_rx_wrongbss;
> +     req->is_rx_dup = stats->is_rx_dup;
> +     req->is_rx_wrongdir = stats->is_rx_wrongdir;
> +     req->is_rx_mcastecho = stats->is_rx_mcastecho;
> +     req->is_rx_notassoc = stats->is_rx_notassoc;
> +     req->is_rx_nowep = stats->is_rx_nowep;
> +     req->is_rx_unencrypted = stats->is_rx_unencrypted;
> +     req->is_rx_wepfail = stats->is_rx_wepfail;
> +     req->is_rx_decap = stats->is_rx_decap;
> +     req->is_rx_mgtdiscard = stats->is_rx_mgtdiscard;
> +     req->is_rx_ctl = stats->is_rx_ctl;
> +     req->is_rx_rstoobig = stats->is_rx_rstoobig;
> +     req->is_rx_elem_missing = stats->is_rx_elem_missing;
> +     req->is_rx_elem_toobig = stats->is_rx_elem_toobig;
> +     req->is_rx_elem_toosmall = stats->is_rx_elem_toosmall;
> +     req->is_rx_badchan = stats->is_rx_badchan;
> +     req->is_rx_chanmismatch = stats->is_rx_chanmismatch;
> +     req->is_rx_nodealloc = stats->is_rx_nodealloc;
> +     req->is_rx_ssidmismatch = stats->is_rx_ssidmismatch;
> +     req->is_rx_auth_unsupported = stats->is_rx_auth_unsupported;
> +     req->is_rx_auth_fail = stats->is_rx_auth_fail;
> +     req->is_rx_assoc_bss = stats->is_rx_assoc_bss;
> +     req->is_rx_assoc_notauth = stats->is_rx_assoc_notauth;
> +     req->is_rx_assoc_capmismatch = stats->is_rx_assoc_capmismatch;
> +     req->is_rx_assoc_norate = stats->is_rx_assoc_norate;
> +     req->is_rx_deauth = stats->is_rx_deauth;
> +     req->is_rx_disassoc = stats->is_rx_disassoc;
> +     req->is_rx_badsubtype = stats->is_rx_badsubtype;
> +     req->is_rx_nombuf = stats->is_rx_nombuf;
> +     req->is_rx_decryptcrc = stats->is_rx_decryptcrc;
> +     req->is_rx_ahdemo_mgt = stats->is_rx_ahdemo_mgt;
> +     req->is_rx_bad_auth = stats->is_rx_bad_auth;
> +     req->is_tx_nombuf = stats->is_tx_nombuf;
> +     req->is_tx_nonode = stats->is_tx_nonode;
> +     req->is_tx_unknownmgt = stats->is_tx_unknownmgt;
> +     req->is_scan_active = stats->is_scan_active;
> +     req->is_scan_passive = stats->is_scan_passive;
> +     req->is_node_timeout = stats->is_node_timeout;
> +     req->is_crypto_nomem = stats->is_crypto_nomem;
> +     req->is_rx_assoc_badrsnie = stats->is_rx_assoc_badrsnie;
> +     req->is_rx_unauth = stats->is_rx_unauth;
> +     req->is_tx_noauth = stats->is_tx_noauth;
> +     req->is_rx_eapol_key = stats->is_rx_eapol_key;
> +     req->is_rx_eapol_replay = stats->is_rx_eapol_replay;
> +     req->is_rx_eapol_badmic = stats->is_rx_eapol_badmic;
> +     req->is_rx_remmicfail = stats->is_rx_remmicfail;
> +     req->is_rx_locmicfail = stats->is_rx_locmicfail;
> +     req->is_tkip_replays = stats->is_tkip_replays;
> +     req->is_tkip_icv_errs = stats->is_tkip_icv_errs;
> +     req->is_ccmp_replays = stats->is_ccmp_replays;
> +     req->is_ccmp_dec_errs = stats->is_ccmp_dec_errs;
> +     req->is_cmac_replays = stats->is_cmac_replays;
> +     req->is_cmac_icv_errs = stats->is_cmac_icv_errs;
> +     req->is_pbac_errs = stats->is_pbac_errs;
> +     req->is_ht_nego_no_mandatory_mcs = stats->is_ht_nego_no_mandatory_mcs;
> +     req->is_ht_nego_no_basic_mcs = stats->is_ht_nego_no_basic_mcs;
> +     req->is_ht_nego_bad_crypto = stats->is_ht_nego_bad_crypto;
> +     req->is_ht_prot_change = stats->is_ht_prot_change;
> +     req->is_ht_rx_ba_agreements = stats->is_ht_rx_ba_agreements;
> +     req->is_ht_tx_ba_agreements = stats->is_ht_tx_ba_agreements;
> +     req->is_ht_rx_frame_below_ba_winstart =
> +         stats->is_ht_rx_frame_below_ba_winstart;
> +     req->is_ht_rx_frame_above_ba_winend =
> +         stats->is_ht_rx_frame_above_ba_winend;
> +     req->is_ht_rx_ba_window_slide = stats->is_ht_rx_ba_window_slide;
> +     req->is_ht_rx_ba_window_jump = stats->is_ht_rx_ba_window_jump;
> +     req->is_ht_rx_ba_no_buf = stats->is_ht_rx_ba_no_buf;
> +     req->is_ht_rx_ba_frame_lost = stats->is_ht_rx_ba_frame_lost;
> +     req->is_ht_rx_ba_window_gap_timeout =
> +         stats->is_ht_rx_ba_window_gap_timeout;
> +     req->is_ht_rx_ba_timeout = stats->is_ht_rx_ba_timeout;
> +     req->is_ht_tx_ba_timeout = stats->is_ht_tx_ba_timeout;
> +}
> +
> +
> +void
>  ieee80211_disable_wep(struct ieee80211com *ic)
>  {
>       struct ieee80211_key *k;
> @@ -471,6 +556,7 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t
>       struct ieee80211_node *ni;
>       struct ieee80211_chaninfo chaninfo;
>       struct ieee80211_chanreq_all *allchans;
> +     struct ieee80211_statsreq statsreq;
>       u_int32_t flags;
>  
>       switch (cmd) {
> @@ -810,17 +896,10 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t
>                               break;
>               }
>               break;
> -#if 0
> -     case SIOCG80211ZSTATS:
> -#endif
>       case SIOCG80211STATS:
>               ifr = (struct ifreq *)data;
> -             error = copyout(&ic->ic_stats, ifr->ifr_data,
> -                 sizeof(ic->ic_stats));
> -#if 0
> -             if (cmd == SIOCG80211ZSTATS)
> -                     memset(&ic->ic_stats, 0, sizeof(ic->ic_stats));
> -#endif
> +             ieee80211_stats2req(&statsreq, &ic->ic_stats);
> +             error = copyout(&statsreq, ifr->ifr_data, sizeof(statsreq));
>               break;
>       case SIOCS80211TXPOWER:
>               if ((error = suser(curproc)) != 0)
> blob - 65e93c23da2d86c0ad259f77b7c9affc85d9038b
> blob + fa95ad15b809e16868e2eba4dba66568b5a66cfe
> --- sys/net80211/ieee80211_ioctl.h
> +++ sys/net80211/ieee80211_ioctl.h
> @@ -37,8 +37,8 @@
>   * IEEE 802.11 ioctls.
>   */
>  
> -/* per-interface statistics */
> -struct ieee80211_stats {
> +/* per-interface statistics, corresponds to struct ieee80211_stats */
> +struct ieee80211_statsreq {
>       u_int32_t       is_rx_badversion;       /* rx frame with bad version */
>       u_int32_t       is_rx_tooshort;         /* rx frame too short */
>       u_int32_t       is_rx_wrongbss;         /* rx from wrong bssid */
> blob - 161853a629887a1aa997480d605d4febd66dd2db
> blob + d8845bfc6aa1331110ca22ec949a00932645907e
> --- sys/net80211/ieee80211_var.h
> +++ sys/net80211/ieee80211_var.h
> @@ -214,6 +214,81 @@ struct ieee80211_defrag {
>  
>  struct ieee80211_node_switch_bss_arg;
>  
> +/* per-interface statistics */
> +struct ieee80211_stats {
> +     u_int32_t       is_rx_badversion;       /* rx frame with bad version */
> +     u_int32_t       is_rx_tooshort;         /* rx frame too short */
> +     u_int32_t       is_rx_wrongbss;         /* rx from wrong bssid */
> +     u_int32_t       is_rx_dup;              /* rx discard 'cuz dup */
> +     u_int32_t       is_rx_wrongdir;         /* rx w/ wrong direction */
> +     u_int32_t       is_rx_mcastecho;        /* rx discard 'cuz mcast echo */
> +     u_int32_t       is_rx_notassoc;         /* rx discard 'cuz sta !assoc */
> +     u_int32_t       is_rx_nowep;            /* rx w/ wep but wep !config */
> +     u_int32_t       is_rx_unencrypted;      /* rx w/o wep but wep config */
> +     u_int32_t       is_rx_wepfail;          /* rx wep processing failed */
> +     u_int32_t       is_rx_decap;            /* rx decapsulation failed */
> +     u_int32_t       is_rx_mgtdiscard;       /* rx discard mgt frames */
> +     u_int32_t       is_rx_ctl;              /* rx discard ctrl frames */
> +     u_int32_t       is_rx_rstoobig;         /* rx rate set truncated */
> +     u_int32_t       is_rx_elem_missing;     /* rx required element missing*/
> +     u_int32_t       is_rx_elem_toobig;      /* rx element too big */
> +     u_int32_t       is_rx_elem_toosmall;    /* rx element too small */
> +     u_int32_t       is_rx_badchan;          /* rx frame w/ invalid chan */
> +     u_int32_t       is_rx_chanmismatch;     /* rx frame chan mismatch */
> +     u_int32_t       is_rx_nodealloc;        /* rx frame dropped */
> +     u_int32_t       is_rx_ssidmismatch;     /* rx frame ssid mismatch  */
> +     u_int32_t       is_rx_auth_unsupported; /* rx w/ unsupported auth alg */
> +     u_int32_t       is_rx_auth_fail;        /* rx sta auth failure */
> +     u_int32_t       is_rx_assoc_bss;        /* rx assoc from wrong bssid */
> +     u_int32_t       is_rx_assoc_notauth;    /* rx assoc w/o auth */
> +     u_int32_t       is_rx_assoc_capmismatch;/* rx assoc w/ cap mismatch */
> +     u_int32_t       is_rx_assoc_norate;     /* rx assoc w/ no rate match */
> +     u_int32_t       is_rx_deauth;           /* rx deauthentication */
> +     u_int32_t       is_rx_disassoc;         /* rx disassociation */
> +     u_int32_t       is_rx_badsubtype;       /* rx frame w/ unknown subtype*/
> +     u_int32_t       is_rx_nombuf;           /* rx failed for lack of mbuf */
> +     u_int32_t       is_rx_decryptcrc;       /* rx decrypt failed on crc */
> +     u_int32_t       is_rx_ahdemo_mgt;       /* rx discard ahdemo mgt frame*/
> +     u_int32_t       is_rx_bad_auth;         /* rx bad auth request */
> +     u_int32_t       is_tx_nombuf;           /* tx failed for lack of mbuf */
> +     u_int32_t       is_tx_nonode;           /* tx failed for no node */
> +     u_int32_t       is_tx_unknownmgt;       /* tx of unknown mgt frame */
> +     u_int32_t       is_scan_active;         /* active scans started */
> +     u_int32_t       is_scan_passive;        /* passive scans started */
> +     u_int32_t       is_node_timeout;        /* nodes timed out inactivity */
> +     u_int32_t       is_crypto_nomem;        /* no memory for crypto ctx */
> +     u_int32_t       is_rx_assoc_badrsnie;   /* rx assoc w/ bad RSN IE */
> +     u_int32_t       is_rx_unauth;           /* rx port not valid */
> +     u_int32_t       is_tx_noauth;           /* tx port not valid */
> +     u_int32_t       is_rx_eapol_key;        /* rx eapol-key frames */
> +     u_int32_t       is_rx_eapol_replay;     /* rx replayed eapol frames */
> +     u_int32_t       is_rx_eapol_badmic;     /* rx eapol frames w/ bad mic */
> +     u_int32_t       is_rx_remmicfail;       /* rx tkip remote mic fails */
> +     u_int32_t       is_rx_locmicfail;       /* rx tkip local mic fails */
> +     u_int32_t       is_tkip_replays;
> +     u_int32_t       is_tkip_icv_errs;
> +     u_int32_t       is_ccmp_replays;
> +     u_int32_t       is_ccmp_dec_errs;
> +     u_int32_t       is_cmac_replays;
> +     u_int32_t       is_cmac_icv_errs;
> +     u_int32_t       is_pbac_errs;
> +     u_int32_t       is_ht_nego_no_mandatory_mcs;
> +     u_int32_t       is_ht_nego_no_basic_mcs;
> +     u_int32_t       is_ht_nego_bad_crypto;
> +     u_int32_t       is_ht_prot_change;
> +     u_int32_t       is_ht_rx_ba_agreements;
> +     u_int32_t       is_ht_tx_ba_agreements;
> +     u_int32_t       is_ht_rx_frame_below_ba_winstart;
> +     u_int32_t       is_ht_rx_frame_above_ba_winend;
> +     u_int32_t       is_ht_rx_ba_window_slide;
> +     u_int32_t       is_ht_rx_ba_window_jump;
> +     u_int32_t       is_ht_rx_ba_no_buf;
> +     u_int32_t       is_ht_rx_ba_frame_lost;
> +     u_int32_t       is_ht_rx_ba_window_gap_timeout;
> +     u_int32_t       is_ht_rx_ba_timeout;
> +     u_int32_t       is_ht_tx_ba_timeout;
> +};
> +
>  struct ieee80211com {
>       struct arpcom           ic_ac;
>       LIST_ENTRY(ieee80211com) ic_list;       /* chain of all ieee80211com */
> blob - de136e4dd7a5da0f5a38c7de42fd2d646592fcd8
> blob + e2076ffeae43eb877108e997828b5481bef0a4e7
> --- usr.bin/netstat/net80211.c
> +++ usr.bin/netstat/net80211.c
> @@ -41,7 +41,7 @@ void
>  net80211_ifstats(char *ifname)
>  {
>       struct ifreq ifr;
> -     struct ieee80211_stats stats;
> +     struct ieee80211_statsreq stats;
>       int s;
>  
>  #define      p(f, m) printf(m, (unsigned long)stats.f, plural(stats.f))
> 
> 

Honestly I think this is overkill. There is no stat struct where we do
this dance. It is accepted that netstat needs to keep in sync for these
structs to work. Why is it necessary to disconnect the kernel and userland
for this?

-- 
:wq Claudio

Reply via email to