Re: iwm(4): re-add CCMP hardware offload support
On Sun, May 17, 2020 at 04:45:44PM +0200, Matthias Schmidt wrote: > I have your diff running since yesterday and noticed no regression. > > iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x78, msi > iwm0: hw rev 0x230, fw ver 34.0.1 > > Cheers > > Matthias Great, thanks for your help Matthias and others! It's committed now.
Re: iwm(4): re-add CCMP hardware offload support
Hi Stefan, * Stefan Sperling wrote: > On Sat, May 16, 2020 at 05:41:43PM +0200, Stefan Sperling wrote: > > On Fri, May 15, 2020 at 05:02:28PM +0200, Stefan Sperling wrote: > > > This has been attempted before, but had to backed out because in some > > > cases firmware was failing to decrypt a subset of received frames, which > > > resulted in packet loss. > > > > > > That was before we updated all iwm(4) devices to newer firmware in the > > > previous release cycle. I hope we won't see such problems again now. > > > > > > Please help out with getting this tested on the many iwm(4) variants: > > > 7260, 7265, 3160, 3165, 3168, 8260, 8265, 9260, and 9560. > > > > Turns out that my initial testing was insufficient. Sorry about that. > > > > My previous patch, which was based on code code written against older > > firmware, did not work on some devices. I missed that some newer firmware > > versions we use nowadays have different hardware encryption APIs. > > > > This new version has been tested by me on 7265, 8260, and 9260. > > It seems to work fine on those devices. > > > > Could someone test on any of the other devices? > > > > ok? > > Sorry, that diff was generated against the wrong base and contained unrelated > stuff. I have your diff running since yesterday and noticed no regression. iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x78, msi iwm0: hw rev 0x230, fw ver 34.0.1 Cheers Matthias
Re: iwm(4): re-add CCMP hardware offload support
On Sat, May 16, 2020 at 05:41:43PM +0200, Stefan Sperling wrote:
> On Fri, May 15, 2020 at 05:02:28PM +0200, Stefan Sperling wrote:
> > This has been attempted before, but had to backed out because in some
> > cases firmware was failing to decrypt a subset of received frames, which
> > resulted in packet loss.
> >
> > That was before we updated all iwm(4) devices to newer firmware in the
> > previous release cycle. I hope we won't see such problems again now.
> >
> > Please help out with getting this tested on the many iwm(4) variants:
> > 7260, 7265, 3160, 3165, 3168, 8260, 8265, 9260, and 9560.
>
> Turns out that my initial testing was insufficient. Sorry about that.
>
> My previous patch, which was based on code code written against older
> firmware, did not work on some devices. I missed that some newer firmware
> versions we use nowadays have different hardware encryption APIs.
>
> This new version has been tested by me on 7265, 8260, and 9260.
> It seems to work fine on those devices.
>
> Could someone test on any of the other devices?
>
> ok?
Sorry, that diff was generated against the wrong base and contained unrelated
stuff.
This is the intended diff:
blob - 906e910b02a49203bca9a703455a37fcb716b5da
blob + 9b4477cbba3795b6665b1ff0d33082ce205b9c9b
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -372,8 +372,10 @@ intiwm_rxmq_get_signal_strength(struct iwm_softc
*, s
void iwm_rx_rx_phy_cmd(struct iwm_softc *, struct iwm_rx_packet *,
struct iwm_rx_data *);
intiwm_get_noise(const struct iwm_statistics_rx_non_phy *);
-void iwm_rx_frame(struct iwm_softc *, struct mbuf *, int, int, int, uint32_t,
- struct ieee80211_rxinfo *, struct mbuf_list *);
+intiwm_ccmp_decap(struct iwm_softc *, struct mbuf *,
+ struct ieee80211_node *);
+void iwm_rx_frame(struct iwm_softc *, struct mbuf *, int, uint32_t, int, int,
+ uint32_t, struct ieee80211_rxinfo *, struct mbuf_list *);
void iwm_rx_tx_cmd_single(struct iwm_softc *, struct iwm_rx_packet *,
struct iwm_node *, int, int);
void iwm_rx_tx_cmd(struct iwm_softc *, struct iwm_rx_packet *,
@@ -452,6 +454,14 @@ intiwm_disassoc(struct iwm_softc *);
intiwm_run(struct iwm_softc *);
intiwm_run_stop(struct iwm_softc *);
struct ieee80211_node *iwm_node_alloc(struct ieee80211com *);
+intiwm_set_key_v1(struct ieee80211com *, struct ieee80211_node *,
+ struct ieee80211_key *);
+intiwm_set_key(struct ieee80211com *, struct ieee80211_node *,
+ struct ieee80211_key *);
+void iwm_delete_key_v1(struct ieee80211com *,
+ struct ieee80211_node *, struct ieee80211_key *);
+void iwm_delete_key(struct ieee80211com *,
+ struct ieee80211_node *, struct ieee80211_key *);
void iwm_calib_timeout(void *);
void iwm_setrates(struct iwm_node *, int);
intiwm_media_change(struct ifnet *);
@@ -3896,16 +3906,65 @@ iwm_get_noise(const struct iwm_statistics_rx_non_phy *
return (nbant == 0) ? -127 : (total / nbant) - 107;
}
+int
+iwm_ccmp_decap(struct iwm_softc *sc, struct mbuf *m, struct ieee80211_node *ni)
+{
+ struct ieee80211com *ic = &sc->sc_ic;
+ struct ieee80211_key *k = &ni->ni_pairwise_key;
+ struct ieee80211_frame *wh;
+ uint64_t pn, *prsc;
+ uint8_t *ivp;
+ uint8_t tid;
+ int hdrlen, hasqos;
+
+ wh = mtod(m, struct ieee80211_frame *);
+ hdrlen = ieee80211_get_hdrlen(wh);
+ ivp = (uint8_t *)wh + hdrlen;
+
+ /* Check that ExtIV bit is be set. */
+ if (!(ivp[3] & IEEE80211_WEP_EXTIV))
+ return 1;
+
+ hasqos = ieee80211_has_qos(wh);
+ tid = hasqos ? ieee80211_get_qos(wh) & IEEE80211_QOS_TID : 0;
+ prsc = &k->k_rsc[tid];
+
+ /* Extract the 48-bit PN from the CCMP header. */
+ pn = (uint64_t)ivp[0] |
+(uint64_t)ivp[1] << 8 |
+(uint64_t)ivp[4] << 16 |
+(uint64_t)ivp[5] << 24 |
+(uint64_t)ivp[6] << 32 |
+(uint64_t)ivp[7] << 40;
+ if (pn <= *prsc) {
+ ic->ic_stats.is_ccmp_replays++;
+ return 1;
+ }
+ /* Last seen packet number is updated in ieee80211_inputm(). */
+
+ /*
+* Some firmware versions strip the MIC, and some don't. It is not
+* clear which of the capability flags could tell us what to expect.
+* For now, keep things simple and just leave the MIC in place if
+* it is present.
+*
+* The IV will be stripped by ieee80211_inputm().
+*/
+ return 0;
+}
+
void
iwm_rx_frame(struct iwm_softc *sc, struct mbuf *m, int chanidx,
- int is_shortpre, int rate_n_flags, uint32_t device_timestamp,
- struct ieee80211_rxinfo *rxi, struct mbuf_list *ml)
+uint32_t rx_pkt_status, int is_shortpre, int rate_n_flags,
+uint32_t device_timestamp, struct ieee80211_rxinfo *rxi,
+struct mbuf_list *ml)
{
struct ieee80211
Re: iwm(4): re-add CCMP hardware offload support
On Fri, May 15, 2020 at 05:02:28PM +0200, Stefan Sperling wrote:
> This has been attempted before, but had to backed out because in some
> cases firmware was failing to decrypt a subset of received frames, which
> resulted in packet loss.
>
> That was before we updated all iwm(4) devices to newer firmware in the
> previous release cycle. I hope we won't see such problems again now.
>
> Please help out with getting this tested on the many iwm(4) variants:
> 7260, 7265, 3160, 3165, 3168, 8260, 8265, 9260, and 9560.
Turns out that my initial testing was insufficient. Sorry about that.
My previous patch, which was based on code code written against older
firmware, did not work on some devices. I missed that some newer firmware
versions we use nowadays have different hardware encryption APIs.
This new version has been tested by me on 7265, 8260, and 9260.
It seems to work fine on those devices.
Could someone test on any of the other devices?
ok?
diff refs/heads/master refs/heads/iwm-ccmp
blob - 9193ae3c7b5101a86b85b1b3ba48489d75f5150c
blob + 8bc189dc146562f7ab0af2f1690fa02676aecfcd
--- sys/dev/ic/ar5008.c
+++ sys/dev/ic/ar5008.c
@@ -794,9 +794,8 @@ ar5008_ccmp_decap(struct athn_softc *sc, struct mbuf *
struct ieee80211_frame *wh;
struct ieee80211_rx_ba *ba;
uint64_t pn, *prsc;
- u_int8_t *ivp, *mmie;
+ u_int8_t *ivp;
uint8_t tid;
- uint16_t kid;
int hdrlen, hasqos;
uintptr_t entry;
@@ -805,35 +804,8 @@ ar5008_ccmp_decap(struct athn_softc *sc, struct mbuf *
ivp = mtod(m, u_int8_t *) + hdrlen;
/* find key for decryption */
- if (!IEEE80211_IS_MULTICAST(wh->i_addr1)) {
- k = &ni->ni_pairwise_key;
- } else if ((wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK) !=
- IEEE80211_FC0_TYPE_MGT) {
- /* retrieve group data key id from IV field */
- /* check that IV field is present */
- if (m->m_len < hdrlen + 4)
- return 1;
- kid = ivp[3] >> 6;
- k = &ic->ic_nw_keys[kid];
- } else {
- /* retrieve integrity group key id from MMIE */
- if (m->m_len < sizeof(*wh) + IEEE80211_MMIE_LEN) {
- return 1;
- }
- /* it is assumed management frames are contiguous */
- mmie = (u_int8_t *)wh + m->m_len - IEEE80211_MMIE_LEN;
- /* check that MMIE is valid */
- if (mmie[0] != IEEE80211_ELEMID_MMIE || mmie[1] != 16) {
- return 1;
- }
- kid = LE_READ_2(&mmie[2]);
- if (kid != 4 && kid != 5) {
- return 1;
- }
- k = &ic->ic_nw_keys[kid];
- }
-
- if (k->k_cipher != IEEE80211_CIPHER_CCMP)
+ k = ieee80211_get_rxkey(ic, m, ni);
+ if (k == NULL || k->k_cipher != IEEE80211_CIPHER_CCMP)
return 1;
/* Sanity checks to ensure this is really a key we installed. */
@@ -853,7 +825,7 @@ ar5008_ccmp_decap(struct athn_softc *sc, struct mbuf *
hasqos = ieee80211_has_qos(wh);
tid = hasqos ? ieee80211_get_qos(wh) & IEEE80211_QOS_TID : 0;
ba = hasqos ? &ni->ni_rx_ba[tid] : NULL;
- prsc = &k->k_rsc[0];
+ prsc = &k->k_rsc[tid];
/* Extract the 48-bit PN from the CCMP header. */
pn = (uint64_t)ivp[0] |
@@ -863,30 +835,12 @@ ar5008_ccmp_decap(struct athn_softc *sc, struct mbuf *
(uint64_t)ivp[6] << 32 |
(uint64_t)ivp[7] << 40;
if (pn <= *prsc) {
- if (hasqos && ba->ba_state == IEEE80211_BA_AGREED) {
- /*
-* This is an A-MPDU subframe.
-* Such frames may be received out of order due to
-* legitimate retransmissions of failed subframes
-* in previous A-MPDUs. Duplicates will be handled
-* in ieee80211_inputm() as part of A-MPDU reordering.
-*
-* XXX TODO We can probably do better than this! Store
-* re-ordered PN in BA agreement state and check it?
-*/
- } else {
- ic->ic_stats.is_ccmp_replays++;
- return 1;
- }
+ ic->ic_stats.is_ccmp_replays++;
+ return 1;
}
- /* Update last seen packet number. */
- *prsc = pn;
+ /* Last seen packet number is updated in ieee80211_inputm(). */
- /* Clear Protected bit and strip IV. */
- wh->i_fc[1] &= ~IEEE80211_FC1_PROTECTED;
- memmove(mtod(m, caddr_t) + IEEE80211_CCMP_HDRLEN, wh, hdrlen);
- m_adj(m, IEEE80211_CCMP_HDRLEN);
- /* Strip MIC. */
+ /* Strip MIC. IV will be stripped by ieee80211_inputm(). */
m_adj(m, -IEEE80211_CCMP_MICLEN);
iwm(4): re-add CCMP hardware offload support
This has been attempted before, but had to backed out because in some
cases firmware was failing to decrypt a subset of received frames, which
resulted in packet loss.
That was before we updated all iwm(4) devices to newer firmware in the
previous release cycle. I hope we won't see such problems again now.
Please help out with getting this tested on the many iwm(4) variants:
7260, 7265, 3160, 3165, 3168, 8260, 8265, 9260, and 9560.
In particular, if you see an impact on system responsiveness while iwm(4)
is pushing a lot of traffic, please check if this patch makes a difference.
Note that you need an up-to-date tree. This patch depends on the CCMP replay
fix which I have just committed.
M sys/dev/pci/if_iwm.c
diff 726ed64885e754593692b3521ec292fecb37a563
6f4a43795c837cf1a31eb8cd5cbf23a908d22c4a
blob - 906e910b02a49203bca9a703455a37fcb716b5da
blob + dde808778b74b07f586bc5f982668459e15fd8cb
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -372,8 +372,10 @@ intiwm_rxmq_get_signal_strength(struct iwm_softc
*, s
void iwm_rx_rx_phy_cmd(struct iwm_softc *, struct iwm_rx_packet *,
struct iwm_rx_data *);
intiwm_get_noise(const struct iwm_statistics_rx_non_phy *);
-void iwm_rx_frame(struct iwm_softc *, struct mbuf *, int, int, int, uint32_t,
- struct ieee80211_rxinfo *, struct mbuf_list *);
+intiwm_ccmp_decap(struct iwm_softc *, struct mbuf *,
+ struct ieee80211_node *);
+void iwm_rx_frame(struct iwm_softc *, struct mbuf *, int, uint32_t, int, int,
+ uint32_t, struct ieee80211_rxinfo *, struct mbuf_list *);
void iwm_rx_tx_cmd_single(struct iwm_softc *, struct iwm_rx_packet *,
struct iwm_node *, int, int);
void iwm_rx_tx_cmd(struct iwm_softc *, struct iwm_rx_packet *,
@@ -452,6 +454,10 @@ intiwm_disassoc(struct iwm_softc *);
intiwm_run(struct iwm_softc *);
intiwm_run_stop(struct iwm_softc *);
struct ieee80211_node *iwm_node_alloc(struct ieee80211com *);
+intiwm_set_key(struct ieee80211com *, struct ieee80211_node *,
+ struct ieee80211_key *);
+void iwm_delete_key(struct ieee80211com *,
+ struct ieee80211_node *, struct ieee80211_key *);
void iwm_calib_timeout(void *);
void iwm_setrates(struct iwm_node *, int);
intiwm_media_change(struct ifnet *);
@@ -3896,16 +3902,60 @@ iwm_get_noise(const struct iwm_statistics_rx_non_phy *
return (nbant == 0) ? -127 : (total / nbant) - 107;
}
+int
+iwm_ccmp_decap(struct iwm_softc *sc, struct mbuf *m, struct ieee80211_node *ni)
+{
+ struct ieee80211com *ic = &sc->sc_ic;
+ struct ieee80211_key *k = &ni->ni_pairwise_key;
+ struct ieee80211_frame *wh;
+ uint64_t pn, *prsc;
+ uint8_t *ivp;
+ uint8_t tid;
+ int hdrlen, hasqos;
+
+ wh = mtod(m, struct ieee80211_frame *);
+ hdrlen = ieee80211_get_hdrlen(wh);
+ ivp = (uint8_t *)wh + hdrlen;
+
+ /* Check that ExtIV bit is be set. */
+ if (!(ivp[3] & IEEE80211_WEP_EXTIV)) {
+ DPRINTF(("CCMP decap ExtIV not set\n"));
+ return 1;
+ }
+ hasqos = ieee80211_has_qos(wh);
+ tid = hasqos ? ieee80211_get_qos(wh) & IEEE80211_QOS_TID : 0;
+ prsc = &k->k_rsc[tid];
+
+ /* Extract the 48-bit PN from the CCMP header. */
+ pn = (uint64_t)ivp[0] |
+(uint64_t)ivp[1] << 8 |
+(uint64_t)ivp[4] << 16 |
+(uint64_t)ivp[5] << 24 |
+(uint64_t)ivp[6] << 32 |
+(uint64_t)ivp[7] << 40;
+ if (pn <= *prsc) {
+ ic->ic_stats.is_ccmp_replays++;
+ return 1;
+ }
+ /* Last seen packet number is updated in ieee80211_inputm(). */
+
+ /* Strip MIC. IV will be stripped by ieee80211_inputm(). */
+ m_adj(m, -IEEE80211_CCMP_MICLEN);
+ return 0;
+}
+
void
iwm_rx_frame(struct iwm_softc *sc, struct mbuf *m, int chanidx,
- int is_shortpre, int rate_n_flags, uint32_t device_timestamp,
- struct ieee80211_rxinfo *rxi, struct mbuf_list *ml)
+uint32_t rx_pkt_status, int is_shortpre, int rate_n_flags,
+uint32_t device_timestamp, struct ieee80211_rxinfo *rxi,
+struct mbuf_list *ml)
{
struct ieee80211com *ic = &sc->sc_ic;
struct ieee80211_frame *wh;
struct ieee80211_node *ni;
struct ieee80211_channel *bss_chan;
uint8_t saved_bssid[IEEE80211_ADDR_LEN] = { 0 };
+ struct ifnet *ifp = IC2IFP(ic);
if (chanidx < 0 || chanidx >= nitems(ic->ic_channels))
chanidx = ieee80211_chan2ieee(ic, ic->ic_ibss_chan);
@@ -3922,6 +3972,38 @@ iwm_rx_frame(struct iwm_softc *sc, struct mbuf *m, int
}
ni->ni_chan = &ic->ic_channels[chanidx];
+ /* Handle hardware decryption. */
+ if (((wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK) != IEEE80211_FC0_TYPE_CTL)
+ && (wh->i_fc[1] & IEEE80211_FC1_PROTECTED) &&
+ !IEEE80211_IS_MULTICAST(wh->i_addr1) &&
+ (ni->
