Re: iwm: set assoc ID in ADD_STA command

2021-10-06 Thread Stefan Sperling
On Tue, Oct 05, 2021 at 02:19:57PM +0200, Stefan Sperling wrote:
> While debugging iwm roaming issues which are now fixed in -current,
> I noticed a small difference between our driver and the Linux driver:
> 
> Set the assoc ID assigned by our AP when updating the firmware station
> with the ADD_STA command. Not sure if this strictly required but it
> doesn't seem to hurt. This assoc ID is also present in the MAC context
> data structure so presumably firmware might use this ID for something.
> 
> This needs to be done in the update case only because we don't know
> our ID yet when the station is first added to firmware. The station
> gets added before the association frame exchange begins.
> 
> Already tested by florian, bket, and myself as part of a larger diff.
> 
> ok?
>  
> diff 3995979b713a9ebcdddbee86864fdb4f14ca7112 
> 3f75e2890abe65d5050904183ad6752454ca8f3b
> blob - 35c4b352f905c1e493d64ccd360e022a77111e1e
> blob + 74545e96d1f0c4c07c3d3cb6fe58c0277628fbeb
> --- sys/dev/pci/if_iwm.c
> +++ sys/dev/pci/if_iwm.c
> @@ -6926,6 +6926,7 @@ iwm_add_sta_cmd(struct iwm_softc *sc, struct iwm_node 
>   if (update) {
>   add_sta_cmd.modify_mask |= (IWM_STA_MODIFY_QUEUES |
>   IWM_STA_MODIFY_TID_DISABLE_TX);
> + add_sta_cmd.assoc_id = htole32(in->in_ni.ni_associd);
>   }
>   add_sta_cmd.tid_disable_tx = htole16(in->tid_disable_ampdu);
>   add_sta_cmd.tfd_queue_msk = htole32(in->tfd_queue_msk);

Thinking about this some more, this change is probably only needed
for hostap mode. So I won't commit it, because a lot of additional
changes would be required to support hostap mode anyway.



iwm: set assoc ID in ADD_STA command

2021-10-05 Thread Stefan Sperling
While debugging iwm roaming issues which are now fixed in -current,
I noticed a small difference between our driver and the Linux driver:

Set the assoc ID assigned by our AP when updating the firmware station
with the ADD_STA command. Not sure if this strictly required but it
doesn't seem to hurt. This assoc ID is also present in the MAC context
data structure so presumably firmware might use this ID for something.

This needs to be done in the update case only because we don't know
our ID yet when the station is first added to firmware. The station
gets added before the association frame exchange begins.

Already tested by florian, bket, and myself as part of a larger diff.

ok?
 
diff 3995979b713a9ebcdddbee86864fdb4f14ca7112 
3f75e2890abe65d5050904183ad6752454ca8f3b
blob - 35c4b352f905c1e493d64ccd360e022a77111e1e
blob + 74545e96d1f0c4c07c3d3cb6fe58c0277628fbeb
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -6926,6 +6926,7 @@ iwm_add_sta_cmd(struct iwm_softc *sc, struct iwm_node 
if (update) {
add_sta_cmd.modify_mask |= (IWM_STA_MODIFY_QUEUES |
IWM_STA_MODIFY_TID_DISABLE_TX);
+   add_sta_cmd.assoc_id = htole32(in->in_ni.ni_associd);
}
add_sta_cmd.tid_disable_tx = htole16(in->tid_disable_ampdu);
add_sta_cmd.tfd_queue_msk = htole32(in->tfd_queue_msk);