files.pci: sis missing dependency for mii_phy

[Alexander von Gernler]

Just like other interfaces, sis depends on mii_phy in order to
configure it via config(8).  Found out the hard way while building a custom
kernel for my Soekris net4501.

In GENERIC, this missing dependency is shadowed by many other
interfaces pulling in mii_phy in the first place.

Index: files.pci
===
RCS file: /cvs/openbsd/src/sys/dev/pci/files.pci,v
retrieving revision 1.337
diff -u -p -u -p -r1.337 files.pci
--- files.pci   29 Jul 2019 00:40:49 -  1.337
+++ files.pci   4 Aug 2019 15:21:47 -
@@ -448,7 +448,7 @@ attach  sf at pci with sf_pci
 file   dev/pci/if_sf_pci.c sf_pci
 
 # SiS 900/7016 ethernet
-device sis: ether, ifnet, mii, ifmedia
+device sis: ether, ifnet, mii, ifmedia, mii_phy
 attach sis at pci
 file   dev/pci/if_sis.csis
 

Re: iwm(4) WPA2 crypto hardware offload

[Stefan Sperling]

On Sun, Aug 04, 2019 at 10:20:24PM +1200, Richard Procter wrote:
> > On 2/08/2019, at 11:13 PM, Stefan Sperling  wrote:
> > @@ -4218,6 +4337,7 @@ iwm_tx(struct iwm_softc *sc, struct mbuf *m, struct ie
> > uint8_t tid, type;
> > int i, totlen, err, pad;
> > int hdrlen2, rtsthres = ic->ic_rtsthreshold;
> > +   uint8_t *ivp;
> 
> nit: shift declare up under const struct iwm_rate? (declare order by size)

Sure, fixed in my tree.

> > +   tx->sec_ctl = IWM_TX_CMD_SEC_CCM;
> > +   memcpy(tx->key, k->k_key, k->k_len);
> 
> The ring warrants an explicit_bzero() on teardown to clear these keys
> I think. The other drivers don’t do this that I can see; perhaps 
> best left for another diff. 

Yes, this needs a systematic approach.

> > +   /* Fallback to software crypto for other ciphers. */
> > +return (ieee80211_set_key(ic, ni, k));
> 
> nit: bad indent

Thanks for catching! Fixed in my tree.

> > +   }
> > +
> > +   cmd.key_flags = htole16(IWM_STA_KEY_FLG_CCM |
> > +   IWM_STA_KEY_FLG_WEP_KEY_MAP |
> > +   ((k->k_id << IWM_STA_KEY_FLG_KEYID_POS) &
> > +   IWM_STA_KEY_FLG_KEYID_MSK));
> > +   if (k->k_flags & IEEE80211_KEY_GROUP)
> > +   cmd.key_flags |= htole16(IWM_STA_KEY_MULTICAST);
> > +
> > +   memcpy(cmd.key, k->k_key, k->k_len);
> > +   cmd.key_offset = 0;
> > +   cmd.sta_id = IWM_STATION_ID;
> > +
> > +   return iwm_send_cmd_pdu(sc, IWM_ADD_STA_KEY, IWM_CMD_ASYNC,
> > +   sizeof(cmd), );
> 
> Might the async command open a race between loading keys and decrypting 
> received packets?

It can lead to all sorts of races but we have no other option because
net80211 runs lots of code in interrupt context. E.g. we might decide
to switch to a new SSID after a scan, which means we have to delete
the current key and perhaps set a new key. And all this happens in a
context where we're not allowed to sleep. So it has to be async, and
we don't bother checking for success/failure of the command. If this
command fails, the user will notice because net won't work. We could
of course check for a subsequent "key added" interrupt from the firmware
but what can we do when this fails? Reset the interface?

Regarding the small window between command initiation and success:
Encryption only affects unicast data frames, and we've already completed
a 4-way handshake with the peer which means newly arriving data frames will
be encrypted. Management frames will still pass. I guess the worst that
might happen is that some data frames sent before the key was installed
might be retransmitted because the firmware would fail to decrypt them.

FWIW, iwn(4) has had the same problem forever and I don't see anyone
complaining.

> > #define IWM_UCODE_TLV_API_BITS \
> > -   
> > "\020\10FRAGMENTED_SCAN\11WIFI_MCC_UPDATE\16WIDE_CMD_HDR\22LQ_SS_PARAMS\30EXT_SCAN_PRIO\33TX_POWER_CHAIN"
> > +   
> > "\020\10FRAGMENTED_SCAN\11WIFI_MCC_UPDATE\16WIDE_CMD_HDR\22LQ_SS_PARAMS\30EXT_SCAN_PRIO\33TX_POWER_CHAIN\35TKIP_MIC_KEYS"
> > 
> > /**
> >  * uCode capabilities
> 
> note: I copied this last hunk over from the linux driver; it’s unused by your 
> diff. 
> 

I think it's good to keep this because such definitions are the only
form of "hardware docs" we can get...

Re: iwm(4) WPA2 crypto hardware offload

[Richard Procter]

Hi Stefan, 

> On 2/08/2019, at 11:13 PM, Stefan Sperling  wrote:
> 
> This diff enables HW offload for WPA2 CCMP (AES) encrypted unicast
> frames in iwm(4). This is in preparation for Tx aggregation support.

Lightly tested on:

iwm0 at pci1 dev 0 function 0 "Intel Dual Band Wireless AC 7265" rev 0x69, msi
iwm0: hw rev 0x210, fw ver 16.242414.0, address xx:xx:xx:xx:xx:xx

pkg_add -u; download a few big files; ifconfig down; ifconfig up; 
change network.

comments inline. supposing the async set keys command is a non-issue then,
for what it’s worth from this 802.11 novice, ok procter@

thanks! 
Richard. 

> WEP and WPA1/TKIP ciphers are still handled in software, which mirrors
> what the older iwn(4) driver is doing. We don't enable 11n at all with
> those ciphers (see ieee80211_ht_negotiate()), and we won't aggregate
> non-encrypted frames (see ieee80211_can_use_ampdu()).
> 
> Based on an initial diff by procter@ and some code from iwn(4).
> 
> Tested on 7260, 7265, 8260, and 8265 in bsd.rd upgrade and with pkg_add -u.
> 
> ok?
> 
> diff refs/heads/master refs/heads/iwm-hwcrypt
> blob - 7add1e9e682ef5e22169ec1e89a182cda1af7e2a
> blob + 839c0a0f8b3a62115ba6d5e15adfa63158475c86
> --- sys/dev/pci/if_iwm.c
> +++ sys/dev/pci/if_iwm.c
> @@ -367,6 +367,8 @@ int   iwm_get_signal_strength(struct iwm_softc *, 
> struct
> void  iwm_rx_rx_phy_cmd(struct iwm_softc *, struct iwm_rx_packet *,
>   struct iwm_rx_data *);
> int   iwm_get_noise(const struct iwm_statistics_rx_non_phy *);
> +int  iwm_ccmp_decap(struct iwm_softc *, struct mbuf *,
> + struct ieee80211_node *);
> void  iwm_rx_rx_mpdu(struct iwm_softc *, struct iwm_rx_packet *,
>   struct iwm_rx_data *);
> void  iwm_enable_ht_cck_fallback(struct iwm_softc *, struct iwm_node *);
> @@ -448,6 +450,10 @@ int  iwm_disassoc(struct iwm_softc *);
> int   iwm_run(struct iwm_softc *);
> int   iwm_run_stop(struct iwm_softc *);
> struct ieee80211_node *iwm_node_alloc(struct ieee80211com *);
> +int  iwm_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 *);
> int   iwm_media_change(struct ifnet *);
> void  iwm_newstate_task(void *);
> @@ -3429,11 +3435,91 @@ 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_ic;
> + struct ieee80211_key *k = >ni_pairwise_key;
> + struct ieee80211_frame *wh;
> + struct ieee80211_rx_ba *ba;
> + 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;
> + ba = hasqos ? >ni_rx_ba[tid] : NULL;
> + prsc = >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) {
> + 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_input() as part of A-MPDU reordering.
> +  */
> + } else if (ieee80211_has_seq(wh)) {
> + /*
> +  * Not necessarily a replayed frame since we did not
> +  * check the sequence number of the 802.11 header yet.
> +  */
> + int nrxseq, orxseq;
> +
> + nrxseq = letoh16(*(u_int16_t *)wh->i_seq) >>
> + IEEE80211_SEQ_SEQ_SHIFT;
> + if (hasqos)
> + orxseq = ni->ni_qos_rxseqs[tid];
> + else
> + orxseq = ni->ni_rxseq;
> + if (nrxseq < orxseq) {
> + DPRINTF(("CCMP replayed (n=%d < o=%d)\n",
> + nrxseq, orxseq));
> + ic->ic_stats.is_ccmp_replays++;
> +