Re: keep track of HT protection in 11n mode

2016-01-24 Thread Stefan Sperling
On Thu, Jan 21, 2016 at 01:57:28AM +0100, Stefan Sperling wrote:
> On Wed, Jan 20, 2016 at 10:16:53PM +0100, Stefan Sperling wrote:
> > On Wed, Jan 20, 2016 at 10:04:11PM +0100, Stefan Sperling wrote:
> > > This diff makes us keep track of changes in the network's HT protection
> > > settings. These settings are advertised in beacons and change dynamically
> > > based on the nature of clients associated to an AP at a given moment.
> > > 
> > > Tracking these changes is rather important.
> > > If a non-11n client associates to an AP which previously had 11n clients
> > > only, we must update our wireless device's configuration accordingly or
> > > the new client might damage frames we send out.
> > 
> > This diff still has issues on iwn(4). Don't test there yet, please...
> 
> This diff works fine for me with both iwm(4) and iwn(4).
> 
> I couldn't figure out how to make proper use of iwn's RXON_ASSOC command.
> Linux uses this command to avoid having to restore a lot of state in 
> firmware when changing RXON flags. In my case sending RXON_ASSOC always
> broke Tx. I'm now using an implementation which uses RXON but works.

I know a few people have been testing this. Anyone who wants to
review? My plan is to commit this soon unless I hear objections.

> Index: dev/pci/if_iwm.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 if_iwm.c
> --- dev/pci/if_iwm.c  7 Jan 2016 23:08:38 -   1.75
> +++ dev/pci/if_iwm.c  21 Jan 2016 00:31:38 -
> @@ -294,6 +294,8 @@ int   iwm_nvm_read_section(struct iwm_soft
>   uint16_t *);
>  void iwm_init_channel_map(struct iwm_softc *, const uint16_t * const);
>  void iwm_setup_ht_rates(struct iwm_softc *);
> +void iwm_htprot_task(void *);
> +void iwm_update_htprot(struct ieee80211com *, struct ieee80211_node *);
>  int  iwm_ampdu_rx_start(struct ieee80211com *,
>   struct ieee80211_node *, uint8_t);
>  void iwm_ampdu_rx_stop(struct ieee80211com *,
> @@ -2602,6 +2604,34 @@ iwm_mvm_sta_rx_agg(struct iwm_softc *sc,
>  }
>  
>  void
> +iwm_htprot_task(void *arg)
> +{
> + struct iwm_softc *sc = arg;
> + struct ieee80211com *ic = >sc_ic;
> + struct iwm_node *in = (void *)ic->ic_bss;
> + int error;
> +
> + /* This call updates HT protection based on in->in_ni.ni_htop1. */
> + error = iwm_mvm_mac_ctxt_changed(sc, in);
> + if (error != 0)
> + printf("%s: could not change HT protection: error %d\n",
> + DEVNAME(sc), error);
> +}
> +
> +/*
> + * This function is called by upper layer when HT protection settings in
> + * beacons have changed.
> + */
> +void
> +iwm_update_htprot(struct ieee80211com *ic, struct ieee80211_node *ni)
> +{
> + struct iwm_softc *sc = ic->ic_softc;
> +
> + /* assumes that ni == ic->ic_bss */
> + task_add(systq, >htprot_task);
> +}
> +
> +void
>  iwm_ba_task(void *arg)
>  {
>   struct iwm_softc *sc = arg;
> @@ -5878,6 +5908,7 @@ iwm_stop(struct ifnet *ifp, int disable)
>   task_del(sc->sc_eswq, >sc_eswk);
>   task_del(systq, >setrates_task);
>   task_del(systq, >ba_task);
> + task_del(systq, >htprot_task);
>  
>   sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
>  
> @@ -6586,6 +6617,7 @@ iwm_preinit(struct iwm_softc *sc)
>   /* Override 802.11 state transition machine. */
>   sc->sc_newstate = ic->ic_newstate;
>   ic->ic_newstate = iwm_newstate;
> + ic->ic_update_htprot = iwm_update_htprot;
>   ic->ic_ampdu_rx_start = iwm_ampdu_rx_start;
>   ic->ic_ampdu_rx_stop = iwm_ampdu_rx_stop;
>  #ifdef notyet
> @@ -6822,6 +6854,7 @@ iwm_attach(struct device *parent, struct
>   task_set(>newstate_task, iwm_newstate_task, sc);
>   task_set(>setrates_task, iwm_setrates_task, sc);
>   task_set(>ba_task, iwm_ba_task, sc);
> + task_set(>htprot_task, iwm_htprot_task, sc);
>  
>   /*
>* We cannot read the MAC address without loading the
> Index: dev/pci/if_iwmvar.h
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwmvar.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 if_iwmvar.h
> --- dev/pci/if_iwmvar.h   5 Jan 2016 18:41:15 -   1.15
> +++ dev/pci/if_iwmvar.h   20 Jan 2016 17:37:06 -
> @@ -376,6 +376,9 @@ struct iwm_softc {
>   int ba_tid;
>   uint16_tba_ssn;
>  
> + /* Task for HT protection updates. */
> + struct task htprot_task;
> +
>   bus_space_tag_t sc_st;
>   bus_space_handle_t sc_sh;
>   bus_size_t sc_sz;
> Index: dev/pci/if_iwn.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
> retrieving revision 1.157
> diff -u -p -r1.157 if_iwn.c
> --- dev/pci/if_iwn.c  13 Jan 2016 14:39:35 -  1.157
> +++ dev/pci/if_iwn.c  21 Jan 2016 00:42:59 -
> @@ 

Re: keep track of HT protection in 11n mode

2016-01-20 Thread Stefan Sperling
On Wed, Jan 20, 2016 at 10:16:53PM +0100, Stefan Sperling wrote:
> On Wed, Jan 20, 2016 at 10:04:11PM +0100, Stefan Sperling wrote:
> > This diff makes us keep track of changes in the network's HT protection
> > settings. These settings are advertised in beacons and change dynamically
> > based on the nature of clients associated to an AP at a given moment.
> > 
> > Tracking these changes is rather important.
> > If a non-11n client associates to an AP which previously had 11n clients
> > only, we must update our wireless device's configuration accordingly or
> > the new client might damage frames we send out.
> 
> This diff still has issues on iwn(4). Don't test there yet, please...

This diff works fine for me with both iwm(4) and iwn(4).

I couldn't figure out how to make proper use of iwn's RXON_ASSOC command.
Linux uses this command to avoid having to restore a lot of state in 
firmware when changing RXON flags. In my case sending RXON_ASSOC always
broke Tx. I'm now using an implementation which uses RXON but works.

Index: dev/pci/if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.75
diff -u -p -r1.75 if_iwm.c
--- dev/pci/if_iwm.c7 Jan 2016 23:08:38 -   1.75
+++ dev/pci/if_iwm.c21 Jan 2016 00:31:38 -
@@ -294,6 +294,8 @@ int iwm_nvm_read_section(struct iwm_soft
uint16_t *);
 void   iwm_init_channel_map(struct iwm_softc *, const uint16_t * const);
 void   iwm_setup_ht_rates(struct iwm_softc *);
+void   iwm_htprot_task(void *);
+void   iwm_update_htprot(struct ieee80211com *, struct ieee80211_node *);
 intiwm_ampdu_rx_start(struct ieee80211com *,
struct ieee80211_node *, uint8_t);
 void   iwm_ampdu_rx_stop(struct ieee80211com *,
@@ -2602,6 +2604,34 @@ iwm_mvm_sta_rx_agg(struct iwm_softc *sc,
 }
 
 void
+iwm_htprot_task(void *arg)
+{
+   struct iwm_softc *sc = arg;
+   struct ieee80211com *ic = >sc_ic;
+   struct iwm_node *in = (void *)ic->ic_bss;
+   int error;
+
+   /* This call updates HT protection based on in->in_ni.ni_htop1. */
+   error = iwm_mvm_mac_ctxt_changed(sc, in);
+   if (error != 0)
+   printf("%s: could not change HT protection: error %d\n",
+   DEVNAME(sc), error);
+}
+
+/*
+ * This function is called by upper layer when HT protection settings in
+ * beacons have changed.
+ */
+void
+iwm_update_htprot(struct ieee80211com *ic, struct ieee80211_node *ni)
+{
+   struct iwm_softc *sc = ic->ic_softc;
+
+   /* assumes that ni == ic->ic_bss */
+   task_add(systq, >htprot_task);
+}
+
+void
 iwm_ba_task(void *arg)
 {
struct iwm_softc *sc = arg;
@@ -5878,6 +5908,7 @@ iwm_stop(struct ifnet *ifp, int disable)
task_del(sc->sc_eswq, >sc_eswk);
task_del(systq, >setrates_task);
task_del(systq, >ba_task);
+   task_del(systq, >htprot_task);
 
sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
 
@@ -6586,6 +6617,7 @@ iwm_preinit(struct iwm_softc *sc)
/* Override 802.11 state transition machine. */
sc->sc_newstate = ic->ic_newstate;
ic->ic_newstate = iwm_newstate;
+   ic->ic_update_htprot = iwm_update_htprot;
ic->ic_ampdu_rx_start = iwm_ampdu_rx_start;
ic->ic_ampdu_rx_stop = iwm_ampdu_rx_stop;
 #ifdef notyet
@@ -6822,6 +6854,7 @@ iwm_attach(struct device *parent, struct
task_set(>newstate_task, iwm_newstate_task, sc);
task_set(>setrates_task, iwm_setrates_task, sc);
task_set(>ba_task, iwm_ba_task, sc);
+   task_set(>htprot_task, iwm_htprot_task, sc);
 
/*
 * We cannot read the MAC address without loading the
Index: dev/pci/if_iwmvar.h
===
RCS file: /cvs/src/sys/dev/pci/if_iwmvar.h,v
retrieving revision 1.15
diff -u -p -r1.15 if_iwmvar.h
--- dev/pci/if_iwmvar.h 5 Jan 2016 18:41:15 -   1.15
+++ dev/pci/if_iwmvar.h 20 Jan 2016 17:37:06 -
@@ -376,6 +376,9 @@ struct iwm_softc {
int ba_tid;
uint16_tba_ssn;
 
+   /* Task for HT protection updates. */
+   struct task htprot_task;
+
bus_space_tag_t sc_st;
bus_space_handle_t sc_sh;
bus_size_t sc_sz;
Index: dev/pci/if_iwn.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.157
diff -u -p -r1.157 if_iwn.c
--- dev/pci/if_iwn.c13 Jan 2016 14:39:35 -  1.157
+++ dev/pci/if_iwn.c21 Jan 2016 00:42:59 -
@@ -226,6 +226,8 @@ int iwn_set_key(struct ieee80211com *, 
struct ieee80211_key *);
 void   iwn_delete_key(struct ieee80211com *, struct ieee80211_node *,
struct ieee80211_key *);
+void   iwn_update_htprot(struct ieee80211com *,
+   struct ieee80211_node *);
 int

Re: keep track of HT protection in 11n mode

2016-01-20 Thread Stefan Sperling
On Wed, Jan 20, 2016 at 10:04:11PM +0100, Stefan Sperling wrote:
> This diff makes us keep track of changes in the network's HT protection
> settings. These settings are advertised in beacons and change dynamically
> based on the nature of clients associated to an AP at a given moment.
> 
> Tracking these changes is rather important.
> If a non-11n client associates to an AP which previously had 11n clients
> only, we must update our wireless device's configuration accordingly or
> the new client might damage frames we send out.

This diff still has issues on iwn(4). Don't test there yet, please...



keep track of HT protection in 11n mode

2016-01-20 Thread Stefan Sperling
This diff makes us keep track of changes in the network's HT protection
settings. These settings are advertised in beacons and change dynamically
based on the nature of clients associated to an AP at a given moment.

Tracking these changes is rather important.
If a non-11n client associates to an AP which previously had 11n clients
only, we must update our wireless device's configuration accordingly or
the new client might damage frames we send out.

Index: dev/pci/if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.75
diff -u -p -r1.75 if_iwm.c
--- dev/pci/if_iwm.c7 Jan 2016 23:08:38 -   1.75
+++ dev/pci/if_iwm.c20 Jan 2016 20:42:23 -
@@ -294,6 +294,8 @@ int iwm_nvm_read_section(struct iwm_soft
uint16_t *);
 void   iwm_init_channel_map(struct iwm_softc *, const uint16_t * const);
 void   iwm_setup_ht_rates(struct iwm_softc *);
+void   iwm_htprot_task(void *);
+void   iwm_update_htprot(struct ieee80211com *, const struct ieee80211_node *);
 intiwm_ampdu_rx_start(struct ieee80211com *,
struct ieee80211_node *, uint8_t);
 void   iwm_ampdu_rx_stop(struct ieee80211com *,
@@ -2602,6 +2604,34 @@ iwm_mvm_sta_rx_agg(struct iwm_softc *sc,
 }
 
 void
+iwm_htprot_task(void *arg)
+{
+   struct iwm_softc *sc = arg;
+   struct ieee80211com *ic = >sc_ic;
+   struct iwm_node *in = (void *)ic->ic_bss;
+   int error;
+
+   /* This call updates HT protection based on in->in_ni.ni_htop1. */
+   error = iwm_mvm_mac_ctxt_changed(sc, in);
+   if (error != 0)
+   printf("%s: could not change HT protection: error %d\n",
+   DEVNAME(sc), error);
+}
+
+/*
+ * This function is called by upper layer when HT protection settings in
+ * beacons have changed.
+ */
+void
+iwm_update_htprot(struct ieee80211com *ic, const struct ieee80211_node *ni)
+{
+   struct iwm_softc *sc = ic->ic_softc;
+
+   /* assumes that ni == ic->ic_bss */
+   task_add(systq, >htprot_task);
+}
+
+void
 iwm_ba_task(void *arg)
 {
struct iwm_softc *sc = arg;
@@ -5878,6 +5908,7 @@ iwm_stop(struct ifnet *ifp, int disable)
task_del(sc->sc_eswq, >sc_eswk);
task_del(systq, >setrates_task);
task_del(systq, >ba_task);
+   task_del(systq, >htprot_task);
 
sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
 
@@ -6586,6 +6617,7 @@ iwm_preinit(struct iwm_softc *sc)
/* Override 802.11 state transition machine. */
sc->sc_newstate = ic->ic_newstate;
ic->ic_newstate = iwm_newstate;
+   ic->ic_update_htprot = iwm_update_htprot;
ic->ic_ampdu_rx_start = iwm_ampdu_rx_start;
ic->ic_ampdu_rx_stop = iwm_ampdu_rx_stop;
 #ifdef notyet
@@ -6822,6 +6854,7 @@ iwm_attach(struct device *parent, struct
task_set(>newstate_task, iwm_newstate_task, sc);
task_set(>setrates_task, iwm_setrates_task, sc);
task_set(>ba_task, iwm_ba_task, sc);
+   task_set(>htprot_task, iwm_htprot_task, sc);
 
/*
 * We cannot read the MAC address without loading the
Index: dev/pci/if_iwmvar.h
===
RCS file: /cvs/src/sys/dev/pci/if_iwmvar.h,v
retrieving revision 1.15
diff -u -p -r1.15 if_iwmvar.h
--- dev/pci/if_iwmvar.h 5 Jan 2016 18:41:15 -   1.15
+++ dev/pci/if_iwmvar.h 20 Jan 2016 17:37:06 -
@@ -376,6 +376,9 @@ struct iwm_softc {
int ba_tid;
uint16_tba_ssn;
 
+   /* Task for HT protection updates. */
+   struct task htprot_task;
+
bus_space_tag_t sc_st;
bus_space_handle_t sc_sh;
bus_size_t sc_sz;
Index: dev/pci/if_iwn.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.157
diff -u -p -r1.157 if_iwn.c
--- dev/pci/if_iwn.c13 Jan 2016 14:39:35 -  1.157
+++ dev/pci/if_iwn.c20 Jan 2016 20:45:40 -
@@ -226,6 +226,8 @@ int iwn_set_key(struct ieee80211com *, 
struct ieee80211_key *);
 void   iwn_delete_key(struct ieee80211com *, struct ieee80211_node *,
struct ieee80211_key *);
+void   iwn_update_htprot(struct ieee80211com *,
+   const struct ieee80211_node *);
 intiwn_ampdu_rx_start(struct ieee80211com *,
struct ieee80211_node *, uint8_t);
 void   iwn_ampdu_rx_stop(struct ieee80211com *,
@@ -515,6 +517,7 @@ iwn_attach(struct device *parent, struct
ic->ic_updateedca = iwn_updateedca;
ic->ic_set_key = iwn_set_key;
ic->ic_delete_key = iwn_delete_key;
+   ic->ic_update_htprot = iwn_update_htprot;
ic->ic_ampdu_rx_start = iwn_ampdu_rx_start;
ic->ic_ampdu_rx_stop = iwn_ampdu_rx_stop;
 #ifdef notyet
@@ -5009,6 +5012,44 @@ iwn_delete_key(struct ieee80211com