On 12/11/15(Thu) 20:22, Stefan Sperling wrote:
> This diff adds an initial implementation of 802.11n.
>
> [...]
Looks good. I think you should already put in the chunks below.
Especially the one needed for ifconfig(8).
I think I found a typo...
> Index: net/if_media.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if_media.h,v
> retrieving revision 1.34
> diff -u -p -r1.34 if_media.h
> --- net/if_media.h 11 Sep 2015 13:02:28 -0000 1.34
> +++ net/if_media.h 12 Nov 2015 15:33:07 -0000
ok mpi@
> Index: net80211/ieee80211.h
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211.h,v
> retrieving revision 1.53
> diff -u -p -r1.53 ieee80211.h
> --- net80211/ieee80211.h 10 Oct 2015 07:51:47 -0000 1.53
> +++ net80211/ieee80211.h 12 Nov 2015 15:33:07 -0000
ok mpi@
> Index: net80211/ieee80211_amrr.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_amrr.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 ieee80211_amrr.c
> --- net80211/ieee80211_amrr.c 23 Dec 2014 03:24:08 -0000 1.8
> +++ net80211/ieee80211_amrr.c 12 Nov 2015 15:33:07 -0000
This is ok and could go with the "struct ieee80211_node" growth.
I like how you explain your design choice for adding MCS support
and it would be nice to have that in the commit message.
> void
> ieee80211_amrr_node_init(const struct ieee80211_amrr *amrr,
> Index: net80211/ieee80211_crypto_ccmp.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_crypto_ccmp.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 ieee80211_crypto_ccmp.c
> --- net80211/ieee80211_crypto_ccmp.c 15 Jul 2015 22:16:42 -0000 1.16
> +++ net80211/ieee80211_crypto_ccmp.c 12 Nov 2015 15:33:07 -0000
> @@ -88,17 +88,18 @@ ieee80211_ccmp_phase1(rijndael_ctx *ctx,
> /* construct AAD (additional authenticated data) */
> aad = &auth[2]; /* skip l(a), will be filled later */
> *aad = wh->i_fc[0];
> - /* 11w: conditionnally mask subtype field */
> + /* 11w: conditionally mask subtype field */
> if ((wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK) ==
> IEEE80211_FC0_TYPE_DATA)
> - *aad &= ~IEEE80211_FC0_SUBTYPE_MASK;
> + *aad &= ~IEEE80211_FC0_SUBTYPE_MASK |
> + IEEE80211_FC0_SUBTYPE_QOS;
I believe this can go already with an explanation of the fix, right?
> aad++;
> /* protected bit is already set in wh */
> *aad = wh->i_fc[1];
> *aad &= ~(IEEE80211_FC1_RETRY | IEEE80211_FC1_PWR_MGT |
> IEEE80211_FC1_MORE_DATA);
> - /* 11n: conditionnally mask order bit */
> - if (ieee80211_has_htc(wh))
> + /* 11n: conditionally mask order bit */
> + if (ieee80211_has_qos(wh))
> *aad &= ~IEEE80211_FC1_ORDER;
> aad++;
> IEEE80211_ADDR_COPY(aad, wh->i_addr1); aad += IEEE80211_ADDR_LEN;
> @@ -112,6 +113,10 @@ ieee80211_ccmp_phase1(rijndael_ctx *ctx,
> aad += IEEE80211_ADDR_LEN;
> }
> if (ieee80211_has_qos(wh)) {
> + /*
> + * XXX 802.11-2012 11.4.3.3.3 g says the A-MSDU present bit
> + * should be set here if both STAs are SPP A-MSDU capable.
> + */
It's not clear to me why you left an XXX if you use "should".
> *aad++ = tid = ieee80211_get_qos(wh) & IEEE80211_QOS_TID;
> *aad++ = 0;
> }
> Index: net80211/ieee80211_ioctl.h
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.h,v
> retrieving revision 1.21
> diff -u -p -r1.21 ieee80211_ioctl.h
> --- net80211/ieee80211_ioctl.h 9 Jan 2015 20:34:21 -0000 1.21
> +++ net80211/ieee80211_ioctl.h 12 Nov 2015 15:33:07 -0000
ok mpi@
> Index: net80211/ieee80211_node.c
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
> retrieving revision 1.89
> diff -u -p -r1.89 ieee80211_node.c
> --- net80211/ieee80211_node.c 4 Nov 2015 12:12:00 -0000 1.89
> +++ net80211/ieee80211_node.c 12 Nov 2015 15:33:07 -0000
That could also go with the structure growth.
> Index: net80211/ieee80211_node.h
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
> retrieving revision 1.47
> diff -u -p -r1.47 ieee80211_node.h
> --- net80211/ieee80211_node.h 4 Nov 2015 12:12:00 -0000 1.47
> +++ net80211/ieee80211_node.h 12 Nov 2015 15:33:07 -0000
> @@ -112,8 +112,8 @@ struct ieee80211_tx_ba {
> struct ieee80211_node *ba_ni; /* backpointer for callbacks */
> struct timeout ba_to;
> int ba_timeout_val;
> -#define IEEE80211_BA_MIN_TIMEOUT (10 * 1000) /* 10msec */
> -#define IEEE80211_BA_MAX_TIMEOUT (10 * 1000 * 1000) /* 10sec */
> +#define IEEE80211_BA_MIN_TIMEOUT (10 * 1000 * 1000) /* 10 sec */
> +#define IEEE80211_BA_MAX_TIMEOUT (10 * 1000 * 1000 * 60) /* 60 sec */
60 sec should be (60 * 1000 * 1000) no?
Apart from that this file is ok mpi@
> int ba_state;
> #define IEEE80211_BA_INIT 0
> @@ -220,10 +220,31 @@ struct ieee80211_node {
> struct timeout ni_sa_query_to;
> int ni_sa_query_count;
>
> +#ifndef IEEE80211_NO_HT
> + /* HT capabilities */
> + uint16_t ni_htcaps;
> + uint8_t ni_ampdu_param;
> + uint8_t ni_rxmcs[howmany(80,NBBY)];
> + uint16_t ni_max_rxrate; /* in Mb/s, 0 <= rate <= 1023 */
> + uint8_t ni_tx_mcs_set;
> + uint16_t ni_htxcaps;
> + uint32_t ni_txbfcaps;
> + uint8_t ni_aselcaps;
> +
> + /* HT operation */
> + uint8_t ni_primary_chan; /* XXX corresponds to ni_chan
> */
> + uint8_t ni_htop0;
> + uint16_t ni_htop1;
> + uint16_t ni_htop2;
> + uint8_t ni_basic_mcs[howmany(128,NBBY)];
> +
> /* Block Ack records */
> struct ieee80211_tx_ba ni_tx_ba[IEEE80211_NUM_TID];
> struct ieee80211_rx_ba ni_rx_ba[IEEE80211_NUM_TID];
>
> + int ni_txmcs; /* current MCS used for TX */
> +#endif
> +
> /* others */
> u_int16_t ni_associd; /* assoc response */
> u_int16_t ni_txseq; /* seq to be transmitted */
> @@ -327,6 +348,12 @@ extern void ieee80211_iterate_nodes(stru
> ieee80211_iter_func *, void *);
> extern void ieee80211_clean_cached(struct ieee80211com *ic);
> extern void ieee80211_clean_nodes(struct ieee80211com *, int);
> +#ifndef IEEE80211_NO_HT
> +void ieee80211_setup_htcaps(struct ieee80211_node *, const uint8_t *,
> + uint8_t);
> +void ieee80211_setup_htop(struct ieee80211_node *, const uint8_t *,
> + uint8_t);
> +#endif
> extern int ieee80211_setup_rates(struct ieee80211com *,
> struct ieee80211_node *, const u_int8_t *, const u_int8_t *, int);
> extern int ieee80211_iserp_sta(const struct ieee80211_node *);
> Index: net80211/ieee80211_radiotap.h
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_radiotap.h,v
> retrieving revision 1.11
> diff -u -p -r1.11 ieee80211_radiotap.h
> --- net80211/ieee80211_radiotap.h 17 Jul 2010 16:25:09 -0000 1.11
> +++ net80211/ieee80211_radiotap.h 12 Nov 2015 15:33:07 -0000
ok mpi@
> Index: net80211/ieee80211_var.h
> ===================================================================
> RCS file: /cvs/src/sys/net80211/ieee80211_var.h,v
> retrieving revision 1.65
> diff -u -p -r1.65 ieee80211_var.h
> --- net80211/ieee80211_var.h 4 Nov 2015 12:12:00 -0000 1.65
> +++ net80211/ieee80211_var.h 12 Nov 2015 15:33:07 -0000
ok mpi@
