Re: userland clock_gettime proof of concept

2020-05-15 Thread Paul Irofti
Here is an updated diff that addresses the following points mentioned by
kettenis@:

  - syscall fallback was implemented from the first version

  - the low resolution clock argument, I think, was shown not to be a
problem

  - TSC and HPET alternatives were discussed, and if we decide to add
them, I think that should be done by a separate diff

  - I think this version does proper wrapping (at least according to the
README); of course Philip's input would be greatly appreciated!

  - I will export the ELF bits after the diff gets in commitable shape

  - proper auxv number instead of 2004: I see that NetBSD
has taken 2004 for AT_SUN_LDELF; should I take 2015 which seems the
next one free?

Hopefully this version also fixes the init bug solene@ was seeing.

Paul

diff --git lib/libc/asr/asr.c lib/libc/asr/asr.c
index cd056c85719..2b25d49f32a 100644
--- lib/libc/asr/asr.c
+++ lib/libc/asr/asr.c
@@ -196,11 +196,11 @@ poll_intrsafe(struct pollfd *fds, nfds_t nfds, int 
timeout)
struct timespec pollstart, pollend, elapsed;
int r;
 
-   if (clock_gettime(CLOCK_MONOTONIC, &pollstart))
+   if (WRAP(clock_gettime)(CLOCK_MONOTONIC, &pollstart))
return -1;
 
while ((r = poll(fds, 1, timeout)) == -1 && errno == EINTR) {
-   if (clock_gettime(CLOCK_MONOTONIC, &pollend))
+   if (WRAP(clock_gettime)(CLOCK_MONOTONIC, &pollend))
return -1;
timespecsub(&pollend, &pollstart, &elapsed);
timeout -= elapsed.tv_sec * 1000 + elapsed.tv_nsec / 100;
@@ -418,7 +418,7 @@ asr_check_reload(struct asr *asr)
asr->a_rtime = 0;
}
 
-   if (clock_gettime(CLOCK_MONOTONIC, &ts) == -1)
+   if (WRAP(clock_gettime)(CLOCK_MONOTONIC, &ts) == -1)
return;
 
if ((ts.tv_sec - asr->a_rtime) < RELOAD_DELAY && asr->a_rtime != 0)
diff --git lib/libc/crypt/bcrypt.c lib/libc/crypt/bcrypt.c
index 82de8fa33b7..63edde9072e 100644
--- lib/libc/crypt/bcrypt.c
+++ lib/libc/crypt/bcrypt.c
@@ -31,6 +31,7 @@
  *
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -248,9 +249,9 @@ _bcrypt_autorounds(void)
char buf[_PASSWORD_LEN];
int duration;
 
-   clock_gettime(CLOCK_THREAD_CPUTIME_ID, &before);
+   WRAP(clock_gettime)(CLOCK_THREAD_CPUTIME_ID, &before);
bcrypt_newhash("testpassword", r, buf, sizeof(buf));
-   clock_gettime(CLOCK_THREAD_CPUTIME_ID, &after);
+   WRAP(clock_gettime)(CLOCK_THREAD_CPUTIME_ID, &after);
 
duration = after.tv_sec - before.tv_sec;
duration *= 100;
diff --git lib/libc/dlfcn/dlfcn_stubs.c lib/libc/dlfcn/dlfcn_stubs.c
index 78d728f66cb..7b75ec4582a 100644
--- lib/libc/dlfcn/dlfcn_stubs.c
+++ lib/libc/dlfcn/dlfcn_stubs.c
@@ -80,10 +80,14 @@ dlerror(void)
return "Wrong dl symbols!\n";
 }
 
+extern void *elf_aux_timekeep;
+extern int find_timekeep(void);
+
 int
 dl_iterate_phdr(int (*callback)(struct dl_phdr_info *, size_t, void *),
void *data)
 {
+   find_timekeep();
if (_dl_cb != NULL && _dl_cb->dl_iterate_phdr != NULL)
return _dl_cb->dl_iterate_phdr(callback, data);
 #ifndef PIC
diff --git lib/libc/dlfcn/init.c lib/libc/dlfcn/init.c
index 270f54aada5..0238bb50b0b 100644
--- lib/libc/dlfcn/init.c
+++ lib/libc/dlfcn/init.c
@@ -69,6 +69,9 @@ extern Elf_Ehdr __executable_start[] __attribute__((weak));
 /* provide definitions for these */
 const dl_cb *_dl_cb __relro = NULL;
 
+extern void *elf_aux_timekeep;
+extern int find_timekeep(void);
+
 void _libc_preinit(int, char **, char **, dl_cb_cb *) __dso_hidden;
 void
 _libc_preinit(int argc, char **argv, char **envp, dl_cb_cb *cb)
@@ -126,6 +129,7 @@ _libc_preinit(int argc, char **argv, char **envp, dl_cb_cb 
*cb)
if (cb == NULL)
setup_static_tib(phdr, phnum);
 #endif /* !PIC */
+   find_timekeep();
 }
 
 /* ARM just had to be different... */
diff --git lib/libc/gen/times.c lib/libc/gen/times.c
index 02e4dd44b5c..36841810d1b 100644
--- lib/libc/gen/times.c
+++ lib/libc/gen/times.c
@@ -52,7 +52,7 @@ times(struct tms *tp)
return ((clock_t)-1);
tp->tms_cutime = CONVTCK(ru.ru_utime);
tp->tms_cstime = CONVTCK(ru.ru_stime);
-   if (clock_gettime(CLOCK_MONOTONIC, &ts) == -1)
+   if (WRAP(clock_gettime)(CLOCK_MONOTONIC, &ts) == -1)
return ((clock_t)-1);
return (ts.tv_sec * CLK_TCK + ts.tv_nsec / (10 / CLK_TCK));
 }
diff --git lib/libc/gen/timespec_get.c lib/libc/gen/timespec_get.c
index 520a5954025..b2bdcd15a4d 100644
--- lib/libc/gen/timespec_get.c
+++ lib/libc/gen/timespec_get.c
@@ -30,6 +30,7 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include 
 #include 
 
 int
@@ -37,7 +38,7 @@ timespec_get(struct timespec *ts, int base)
 {
switch (base) {
case TIME_UTC:
-   if (clock_gettime(CLOCK_REALTIME, ts) == -1)
+   if (WRAP(clock_gettime)(CLOCK_REALTIME, ts) == -1

libpcap: allow breaking out of loop when using savefile

2020-05-15 Thread Caspar Schutijser
Hi,

Below is a patch that makes breaking out of the loop work when using
a savefile.

The pcap_breakloop() function was backported from tcpdump.org libpcap
to OpenBSD libpcap by djm@ on Nov 18, 2005. The bits to make
pcap_breakloop() work were backported to pcap-bpf.c [1] but not to
savefile.c even though tcpdump.org implemented support there too [2].

The diff below backports this piece of code to savefile.c after all.

Thanks,
Caspar Schutijser

[1] 
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libpcap/pcap-bpf.c.diff?r1=1.16&r2=1.17
[2] 
https://github.com/the-tcpdump-group/libpcap/commit/991d444f7116bef16893826b46f3950f62281507#diff-95d4d29d0f11145ff40b850860667a97R745


Index: savefile.c
===
RCS file: /cvs/src/lib/libpcap/savefile.c,v
retrieving revision 1.16
diff -u -p -r1.16 savefile.c
--- savefile.c  22 Dec 2015 19:51:04 -  1.16
+++ savefile.c  15 May 2020 19:03:44 -
@@ -307,6 +307,23 @@ pcap_offline_read(pcap_t *p, int cnt, pc
while (status == 0) {
struct pcap_pkthdr h;
 
+   /*
+* Has "pcap_breakloop()" been called?
+* If so, return immediately - if we haven't read any
+* packets, clear the flag and return -2 to indicate
+* that we were told to break out of the loop, otherwise
+* leave the flag set, so that the *next* call will break
+* out of the loop without having read any packets, and
+* return the number of packets we've processed so far.
+*/
+   if (p->break_loop) {
+   if (n == 0) {
+   p->break_loop = 0;
+   return (PCAP_ERROR_BREAK);
+   } else
+   return (n);
+   }
+
status = sf_next_packet(p, &h, p->buffer, p->bufsize);
if (status) {
if (status == 1)



Re: iwx: fix phy context command

2020-05-15 Thread sven falempin
>
> # patch -p0 -l < ./patch.diff
>>
> --
> |-- sys/dev/pci/if_iwx.c
> |+++ sys/dev/pci/if_iwx.c
> --
> Patching file sys/dev/pci/if_iwx.c using Plan A...
> Hunk #1 succeeded at 343.
> Hunk #2 succeeded at 3771.
> Hunk #3 succeeded at 3810.
> Hmm...  The next patch looks like a unified diff to me...
> The text leading up to this was:
> --
> |blob - bf3209c2d3bb508f36dff179ab3e7cf5f45725dc
> |blob + 210e2e89a61c004c04afe87cc829ecc2d37cfcd3
> |--- sys/dev/pci/if_iwxreg.h
> |+++ sys/dev/pci/if_iwxreg.h
> --
> Patching file sys/dev/pci/if_iwxreg.h using Plan A...
> Hunk #1 succeeded at 2794.
> Hunk #2 succeeded at 2815.
> done
>
> It worked here
>
> iwx0: hw rev 0x340, fw ver 46.393952838.0, address f8:e4:e3:23:3c:46
>
> without the Debug IWX_DEBUG, will reboot a few time more to be sure
>
> --
> --
>

Because time is relative or something

# cat /etc/rc.local
D=$(date)

if ifconfig iwx0 | grep -q 'lladdr 00:00:00:00:00:00'
then
echo FAILED $D >> /var/db/iwTest
dmesg | grep 'iwx0:' > /var/db/iwFail/$D
else
echo OK $D >> /var/db/iwTest
ifconfig iwx0 scan >> /var/db/iwScans/$D
fi

test `wc -l < /var/db/iwTest` -lt 50 && reboot

# cat /var/db/iwTest
OK Fri May 15 13:37:31 EDT 2020
OK Fri May 15 13:38:10 EDT 2020
OK Fri May 15 13:38:49 EDT 2020
OK Fri May 15 13:39:28 EDT 2020
OK Fri May 15 13:40:07 EDT 2020
FAILED Fri May 15 13:40:50 EDT 2020
OK Fri May 15 13:41:29 EDT 2020
FAILED Fri May 15 13:42:12 EDT 2020
OK Fri May 15 13:42:51 EDT 2020
OK Fri May 15 13:43:30 EDT 2020
OK Fri May 15 13:44:09 EDT 2020
OK Fri May 15 13:44:50 EDT 2020
OK Fri May 15 13:45:29 EDT 2020
OK Fri May 15 13:46:08 EDT 2020
OK Fri May 15 13:46:47 EDT 2020
FAILED Fri May 15 13:47:30 EDT 2020
OK Fri May 15 13:48:09 EDT 2020
OK Fri May 15 13:48:48 EDT 2020
OK Fri May 15 13:49:27 EDT 2020
FAILED Fri May 15 13:50:11 EDT 2020
FAILED Fri May 15 13:50:55 EDT 2020
FAILED Fri May 15 13:51:38 EDT 2020
OK Fri May 15 13:52:17 EDT 2020
FAILED Fri May 15 13:53:02 EDT 2020
OK Fri May 15 13:53:42 EDT 2020
FAILED Fri May 15 13:54:24 EDT 2020
FAILED Fri May 15 13:55:07 EDT 2020
OK Fri May 15 13:55:46 EDT 2020
OK Fri May 15 13:56:25 EDT 2020
FAILED Fri May 15 13:57:08 EDT 2020
OK Fri May 15 13:57:50 EDT 2020
OK Fri May 15 13:58:30 EDT 2020
OK Fri May 15 13:59:09 EDT 2020
OK Fri May 15 13:59:48 EDT 2020
FAILED Fri May 15 14:00:31 EDT 2020
OK Fri May 15 14:01:10 EDT 2020
OK Fri May 15 14:02:05 EDT 2020
OK Fri May 15 14:02:45 EDT 2020
FAILED Fri May 15 14:03:28 EDT 2020
OK Fri May 15 14:04:09 EDT 2020
OK Fri May 15 14:04:48 EDT 2020
OK Fri May 15 14:05:27 EDT 2020
OK Fri May 15 14:06:06 EDT 2020
OK Fri May 15 14:06:45 EDT 2020
FAILED Fri May 15 14:07:28 EDT 2020
OK Fri May 15 14:08:08 EDT 2020
OK Fri May 15 14:08:47 EDT 2020
OK Fri May 15 14:09:26 EDT 2020
FAILED Fri May 15 14:10:09 EDT 2020

The greped dmesg are in the attached tar

-- 
--
-
Knowing is not enough; we must apply. Willing is not enough; we must do


iwDmesg.tar.gz
Description: GNU Zip compressed data


Re: iwx: fix phy context command

2020-05-15 Thread sven falempin
On Fri, May 15, 2020 at 9:29 AM Stefan Sperling  wrote:

> iwx(4) firmware understands two different variants of the "PHY_CONTEXT"
> command. Both variants are documented with the same command API version
> number, but they use different sizes for an embedded struct that contains
> information about channels.
>
> Which variant to use depends on whether the firmware advertises support for
> the "ULTRA_HB_CHANNELS" feature (we don't use this feature; it's about 6
> GHz
> channels; but if the command we send has the wrong size the firmware will
> reject the command).
>
> The code I wrote to handle both of these cases has a bug in case this
> feature is *not* supported. Our current iwx(4) firmware supports the
> feature, so there is no actual problem for now.
>
> I have considered removing the command variant which is currently not
> needed.
> However, additional devices may be added to this driver at some point.
> And some of those are kind of hybrids between the 9k and the ax200 device
> generations. I do not know which variant those devices will need. So this
> diff fixes the bug in the non-UHB code path, in case it will be needed.
> We can always remove this later.
>
> ok?
>
>  M  sys/dev/pci/if_iwx.c
>  M  sys/dev/pci/if_iwxreg.h
>
> diff f896dbcaba91a37b23dc6cbeb42839fba3e329be
> 24bd56723f1f029278b7c9cc71d56349db3c5950
> blob - 64c3641a2d0d07a9d899c0b7ccdbe46d46e17b96
> blob + d01db87880cc583922d1610de8319e491e7f148c
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -343,10 +343,8 @@ void   iwx_rx_tx_cmd(struct iwx_softc *, struct
> iwx_rx_p
>  void   iwx_rx_bmiss(struct iwx_softc *, struct iwx_rx_packet *,
> struct iwx_rx_data *);
>  intiwx_binding_cmd(struct iwx_softc *, struct iwx_node *, uint32_t);
> -void   iwx_phy_ctxt_cmd_hdr(struct iwx_softc *, struct iwx_phy_ctxt *,
> -   struct iwx_phy_context_cmd *, uint32_t, uint32_t);
> -void   iwx_phy_ctxt_cmd_data(struct iwx_softc *, struct
> iwx_phy_context_cmd *,
> -   struct ieee80211_channel *, uint8_t, uint8_t);
> +intiwx_phy_ctxt_cmd_uhb(struct iwx_softc *, struct iwx_phy_ctxt *,
> uint8_t,
> +   uint8_t, uint32_t, uint32_t);
>  intiwx_phy_ctxt_cmd(struct iwx_softc *, struct iwx_phy_ctxt *,
> uint8_t,
> uint8_t, uint32_t, uint32_t);
>  intiwx_send_cmd(struct iwx_softc *, struct iwx_host_cmd *);
> @@ -3773,52 +3771,38 @@ iwx_binding_cmd(struct iwx_softc *sc, struct
> iwx_node
> return err;
>  }
>
> -void
> -iwx_phy_ctxt_cmd_hdr(struct iwx_softc *sc, struct iwx_phy_ctxt *ctxt,
> -struct iwx_phy_context_cmd *cmd, uint32_t action, uint32_t apply_time)
> +int
> +iwx_phy_ctxt_cmd_uhb(struct iwx_softc *sc, struct iwx_phy_ctxt *ctxt,
> +uint8_t chains_static, uint8_t chains_dynamic, uint32_t action,
> +uint32_t apply_time)
>  {
> -   memset(cmd, 0, sizeof(struct iwx_phy_context_cmd));
> +   struct ieee80211com *ic = &sc->sc_ic;
> +   struct iwx_phy_context_cmd_uhb cmd;
> +   uint8_t active_cnt, idle_cnt;
> +   struct ieee80211_channel *chan = ctxt->channel;
>
> -   cmd->id_and_color = htole32(IWX_FW_CMD_ID_AND_COLOR(ctxt->id,
> +   memset(&cmd, 0, sizeof(cmd));
> +   cmd.id_and_color = htole32(IWX_FW_CMD_ID_AND_COLOR(ctxt->id,
> ctxt->color));
> -   cmd->action = htole32(action);
> -   cmd->apply_time = htole32(apply_time);
> -}
> +   cmd.action = htole32(action);
> +   cmd.apply_time = htole32(apply_time);
>
> -void
> -iwx_phy_ctxt_cmd_data(struct iwx_softc *sc, struct iwx_phy_context_cmd
> *cmd,
> -struct ieee80211_channel *chan, uint8_t chains_static,
> -uint8_t chains_dynamic)
> -{
> -   struct ieee80211com *ic = &sc->sc_ic;
> -   uint8_t active_cnt, idle_cnt;
> +   cmd.ci.band = IEEE80211_IS_CHAN_2GHZ(chan) ?
> +   IWX_PHY_BAND_24 : IWX_PHY_BAND_5;
> +   cmd.ci.channel = htole32(ieee80211_chan2ieee(ic, chan));
> +   cmd.ci.width = IWX_PHY_VHT_CHANNEL_MODE20;
> +   cmd.ci.ctrl_pos = IWX_PHY_VHT_CTRL_POS_1_BELOW;
>
> -   if (isset(sc->sc_enabled_capa,
> IWX_UCODE_TLV_CAPA_ULTRA_HB_CHANNELS)) {
> -   cmd->ci.band = IEEE80211_IS_CHAN_2GHZ(chan) ?
> -   IWX_PHY_BAND_24 : IWX_PHY_BAND_5;
> -   cmd->ci.channel = htole32(ieee80211_chan2ieee(ic, chan));
> -   cmd->ci.width = IWX_PHY_VHT_CHANNEL_MODE20;
> -   cmd->ci.ctrl_pos = IWX_PHY_VHT_CTRL_POS_1_BELOW;
> -   } else {
> -   struct iwx_fw_channel_info_v1 *ci_v1;
> -   ci_v1 = (struct iwx_fw_channel_info_v1 *)&cmd->ci;
> -   ci_v1->band = IEEE80211_IS_CHAN_2GHZ(chan) ?
> -   IWX_PHY_BAND_24 : IWX_PHY_BAND_5;
> -   ci_v1->channel = ieee80211_chan2ieee(ic, chan);
> -   ci_v1->width = IWX_PHY_VHT_CHANNEL_MODE20;
> -   ci_v1->ctrl_pos = IWX_PHY_VHT_CTRL_POS_1_BELOW;
> -   }
> -   /* Set rx the chains */
> idle_cnt = chains_static;
> active_

Re: 6.7 snaps upgrade went fine - Intel ax200ngw not so much

2020-05-15 Thread Stefan Sperling
On Fri, May 15, 2020 at 11:11:44AM -0400, sven falempin wrote:
> Index: if_iwx.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwx.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 if_iwx.c
> --- if_iwx.c29 Apr 2020 13:13:30 -  1.11
> +++ if_iwx.c15 May 2020 15:08:45 -
> @@ -3222,6 +3222,9 @@ iwx_run_init_mvm_ucode(struct iwx_softc
>  * Send init config command to mark that we are sending NVM
>  * access commands
>  */
> +   printf("%s: DELAYING\n", DEVNAME(sc));
> +   DELAY(5000);
> +
> err = iwx_send_cmd_pdu(sc, IWX_WIDE_ID(IWX_SYSTEM_GROUP,
> IWX_INIT_EXTENDED_CFG_CMD), 0, sizeof(init_cfg), &init_cfg);
> if (err)
> 
> Gave
> 
> iwx0: DELAYING
> iwx0: dumping device error log
> iwx0: Start Error Log Dump:
> iwx0: Status: 0x1, count: 6
> iwx0: 0x0071 | NMI_INTERRUPT_UMAC_FATAL
> iwx0: 0020A2F0 | trm_hw_status0
> iwx0:  | trm_hw_status1
> iwx0: 004FC308 | branchlink2
> iwx0: 00016E5A | interruptlink1
> iwx0: 00016E5A | interruptlink2
> iwx0: 004F9F62 | data1
> iwx0: 1000 | data2
> iwx0: F008 | data3
> iwx0:  | beacon time
> iwx0: 000115E1 | tsf low
> iwx0:  | tsf hi
> iwx0:  | time gp1
> iwx0: 000115E2 | time gp2
> iwx0: 0001 | uCode revision type
> iwx0: 002E | uCode version major
> iwx0: 177B3E46 | uCode version minor
> iwx0: 0340 | hw version
> iwx0: 18889000 | board version
> iwx0: 800AFD0C | hcmd
> iwx0: 2002 | isr0
> iwx0:  | isr1
> iwx0: 18F2 | isr2
> iwx0: 00CC | isr3
> iwx0:  | isr4
> iwx0:  | last cmd Id
> iwx0: 004F9F62 | wait_event
> iwx0:  | l2p_control
> iwx0: 0020 | l2p_duration
> iwx0:  | l2p_mhvalid
> iwx0:  | l2p_addr_match
> iwx0: 0009 | lmpm_pmg_sel
> iwx0: 19071335 | timestamp
> iwx0: 0828 | flow_handler
> iwx0: Start UMAC Error Log Dump:
> iwx0: Status: 0x1, count: 7
> iwx0: 0x201010A3 | ADVANCED_SYSASSERT
> iwx0: 0x | umac branchlink1
> iwx0: 0xC008B1C0 | umac branchlink2
> iwx0: 0xC0084E04 | umac interruptlink1
> iwx0: 0x | umac interruptlink2
> iwx0: 0x0002 | umac data1
> iwx0: 0x0001 | umac data2
> iwx0: 0xDEADBEEF | umac data3
> iwx0: 0x002E | umac major
> iwx0: 0x177B3E46 | umac minor
> iwx0: 0x000115D2 | frame pointer
> iwx0: 0xC0886C6C | stack pointer
> iwx0: 0x00050C00 | last host cmd
> iwx0: 0x | isr status reg
> driver status:
>   tx ring  0: qid=0  cur=6   queued=0
>   tx ring  1: qid=1  cur=0   queued=0
>   tx ring  2: qid=2  cur=0   queued=0
>   tx ring  3: qid=3  cur=0   queued=0
>   tx ring  4: qid=4  cur=0   queued=0
>   tx ring  5: qid=5  cur=0   queued=0
>   tx ring  6: qid=6  cur=0   queued=0
>   tx ring  7: qid=7  cur=0   queued=0
>   tx ring  8: qid=8  cur=0   queued=0
>   tx ring  9: qid=9  cur=0   queued=0
>   tx ring 10: qid=10 cur=0   queued=0
>   tx ring 11: qid=11 cur=0   queued=0
>   tx ring 12: qid=12 cur=0   queued=0
>   tx ring 13: qid=13 cur=0   queued=0
>   tx ring 14: qid=14 cur=0   queued=0
>   tx ring 15: qid=15 cur=0   queued=0
>   tx ring 16: qid=16 cur=0   queued=0
>   tx ring 17: qid=17 cur=0   queued=0
>   tx ring 18: qid=18 cur=0   queued=0
>   tx ring 19: qid=19 cur=0   queued=0
>   tx ring 20: qid=20 cur=0   queued=0
>   tx ring 21: qid=21 cur=0   queued=0
>   tx ring 22: qid=22 cur=0   queued=0
>   tx ring 23: qid=23 cur=0   queued=0
>   tx ring 24: qid=24 cur=0   queued=0
>   tx ring 25: qid=25 cur=0   queued=0
>   tx ring 26: qid=26 cur=0   queued=0
>   tx ring 27: qid=27 cur=0   queued=0
>   tx ring 28: qid=28 cur=0   queued=0
>   tx ring 29: qid=29 cur=0   queued=0
>   tx ring 30: qid=30 cur=0   queued=0
>   rx ring: cur=265
>   802.11 state INIT
> iwx0: fatal firmware error
> 
> I think the delay must be somewhere after.

Ouch. Yes, looks like that's a bad spot.

Though it is an interesting observation that waiting there for a long time
causes another problem.

> I know it s a bit silly , but would a test with
> 
> #ifdef IWX_DEBUG
> #define DPRINTF(x)  do { if (iwx_debug > 0) printf x; } while (0)
> #define DPRINTFN(n, x)  do { if (iwx_debug >= (n)) printf x; } while (0)
> int iwx_debug = 1;
> #else
> #define DPRINTF(x)  do { DELAY(10); } while (1)
> #define DPRINTFN(n, x)  do { DELAY(10); } while (1)
> #endif
> 
> makes sense ?

Not really. That just puts DELAY(10) in some arbitrary places.
What we need to know is where exactly the driver is going too fast.



Re: 6.7 snaps upgrade went fine - Intel ax200ngw not so much

2020-05-15 Thread sven falempin
On Thu, May 14, 2020 at 5:55 AM Stefan Sperling  wrote:

> On Wed, May 13, 2020 at 07:55:02PM -0400, sven falempin wrote:
> > 'good news'
> >
> > I build a custom kernel with the DEBUG flag for the driver
>
> > I 'works' ,
>
> This means that the driver is doing something too fast on your hardware,
> and some miscommunication happens with the card as a result.
>
> One way to work around this is to add DELAY calls. It is not the ideal
> solution but would be a good first step to get the card working.
>
> Can you disable debugging again and try the patch below instead?
> If the problem re-appears, try to increase the amount of delay (up to 5000
> seems reasonable). If increasing the DELAY value does not help, try to move
> the DELAY call further down until it works.
>
> The DELAY may even need to be moved into the while loop inside
> iwx_nvm_init().
> But please try using the DELAY outside of a loop first.
>
> Finding the right spot might take some time. Welcome to driver development
> :)
>
> If you cannot find a spot for the DELAY that makes this work, then we will
> have to wait for someone else who is seeing the same problem and tries
> harder.
>
> diff 4a0fa473f5ea308b63ffd39645f73b2195291973 /usr/src
> blob - 64c3641a2d0d07a9d899c0b7ccdbe46d46e17b96
> file + sys/dev/pci/if_iwx.c
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -3222,6 +3222,7 @@ iwx_run_init_mvm_ucode(struct iwx_softc *sc, int
> readn
>  * Send init config command to mark that we are sending NVM
>  * access commands
>  */
> +   DELAY(1000);
> err = iwx_send_cmd_pdu(sc, IWX_WIDE_ID(IWX_SYSTEM_GROUP,
> IWX_INIT_EXTENDED_CFG_CMD), 0, sizeof(init_cfg), &init_cfg);
> if (err)
>


Index: if_iwx.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwx.c,v
retrieving revision 1.11
diff -u -p -r1.11 if_iwx.c
--- if_iwx.c29 Apr 2020 13:13:30 -  1.11
+++ if_iwx.c15 May 2020 15:08:45 -
@@ -3222,6 +3222,9 @@ iwx_run_init_mvm_ucode(struct iwx_softc
 * Send init config command to mark that we are sending NVM
 * access commands
 */
+   printf("%s: DELAYING\n", DEVNAME(sc));
+   DELAY(5000);
+
err = iwx_send_cmd_pdu(sc, IWX_WIDE_ID(IWX_SYSTEM_GROUP,
IWX_INIT_EXTENDED_CFG_CMD), 0, sizeof(init_cfg), &init_cfg);
if (err)

Gave

iwx0: DELAYING
iwx0: dumping device error log
iwx0: Start Error Log Dump:
iwx0: Status: 0x1, count: 6
iwx0: 0x0071 | NMI_INTERRUPT_UMAC_FATAL
iwx0: 0020A2F0 | trm_hw_status0
iwx0:  | trm_hw_status1
iwx0: 004FC308 | branchlink2
iwx0: 00016E5A | interruptlink1
iwx0: 00016E5A | interruptlink2
iwx0: 004F9F62 | data1
iwx0: 1000 | data2
iwx0: F008 | data3
iwx0:  | beacon time
iwx0: 000115E1 | tsf low
iwx0:  | tsf hi
iwx0:  | time gp1
iwx0: 000115E2 | time gp2
iwx0: 0001 | uCode revision type
iwx0: 002E | uCode version major
iwx0: 177B3E46 | uCode version minor
iwx0: 0340 | hw version
iwx0: 18889000 | board version
iwx0: 800AFD0C | hcmd
iwx0: 2002 | isr0
iwx0:  | isr1
iwx0: 18F2 | isr2
iwx0: 00CC | isr3
iwx0:  | isr4
iwx0:  | last cmd Id
iwx0: 004F9F62 | wait_event
iwx0:  | l2p_control
iwx0: 0020 | l2p_duration
iwx0:  | l2p_mhvalid
iwx0:  | l2p_addr_match
iwx0: 0009 | lmpm_pmg_sel
iwx0: 19071335 | timestamp
iwx0: 0828 | flow_handler
iwx0: Start UMAC Error Log Dump:
iwx0: Status: 0x1, count: 7
iwx0: 0x201010A3 | ADVANCED_SYSASSERT
iwx0: 0x | umac branchlink1
iwx0: 0xC008B1C0 | umac branchlink2
iwx0: 0xC0084E04 | umac interruptlink1
iwx0: 0x | umac interruptlink2
iwx0: 0x0002 | umac data1
iwx0: 0x0001 | umac data2
iwx0: 0xDEADBEEF | umac data3
iwx0: 0x002E | umac major
iwx0: 0x177B3E46 | umac minor
iwx0: 0x000115D2 | frame pointer
iwx0: 0xC0886C6C | stack pointer
iwx0: 0x00050C00 | last host cmd
iwx0: 0x | isr status reg
driver status:
  tx ring  0: qid=0  cur=6   queued=0
  tx ring  1: qid=1  cur=0   queued=0
  tx ring  2: qid=2  cur=0   queued=0
  tx ring  3: qid=3  cur=0   queued=0
  tx ring  4: qid=4  cur=0   queued=0
  tx ring  5: qid=5  cur=0   queued=0
  tx ring  6: qid=6  cur=0   queued=0
  tx ring  7: qid=7  cur=0   queued=0
  tx ring  8: qid=8  cur=0   queued=0
  tx ring  9: qid=9  cur=0   queued=0
  tx ring 10: qid=10 cur=0   queued=0
  tx ring 11: qid=11 cur=0   queued=0
  tx ring 12: qid=12 cur=0   queued=0
  tx ring 13: qid=13 cur=0   queued=0
  tx ring 14: qid=14 cur=0   queued=0
  tx ring 15: qid=15 cur=0   queued=0
  tx ring 16: qid=16 cur=0   queued=0
  tx ring 17: qid=17 cur=0   queued=0
  tx ring 18: qid=18 cur=0   queued=0
  tx ring 19: qid=19 cur=0   queued=0
  tx ring 20: qid=20 cur=0   queued=0
  tx ring 21: qid=21 cur=0   queued=0
  tx ring 22: qid=22 cur=0   queued=0
  tx ring 23: qid=23 cur=0   queued=0
  tx ring 24: qid=24 cur=0   queued=0
  tx ring 25: qid=25

iwm(4): re-add CCMP hardware offload support

2020-05-15 Thread Stefan Sperling
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->

Re: iwn/athn/wpi: fix CCMP replay check with HW crypto

2020-05-15 Thread Solene Rapenne
Le Fri, 15 May 2020 11:02:46 +0200,
Stefan Sperling  a écrit :

> On Fri, May 08, 2020 at 10:53:30AM +0200, Stefan Sperling wrote:
> > So the diff below contains just the reordering fix for drivers using
> > hardware acceleration for WPA.  
> 
> Is there anybody who wants to OK this?
> 
> To recap:
> 
> Currently, CCMP replay checking is skipped for aggregated 11n frames
> on drivers which use CCMP hardware acceleration: athn(4) and iwn(4).
> 
> This diff makes reply checking in that case possible without false
> positives, by updating the last-seen packet number after the
> aggregated subframes, which may be received out of order, have been
> put back into their intended order by ieee80211_input_ba(). The old
> code only works in the single-frame case for 11a/b/g modes where no
> such legitimate re-transmissions can occur.
> 
> > diff refs/heads/master refs/heads/pn
> > 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);
> > return 0;
> >  }
> > blob - 6b6331d167a881a

iwx: fix phy context command

2020-05-15 Thread Stefan Sperling
iwx(4) firmware understands two different variants of the "PHY_CONTEXT"
command. Both variants are documented with the same command API version
number, but they use different sizes for an embedded struct that contains
information about channels.

Which variant to use depends on whether the firmware advertises support for
the "ULTRA_HB_CHANNELS" feature (we don't use this feature; it's about 6 GHz
channels; but if the command we send has the wrong size the firmware will
reject the command).

The code I wrote to handle both of these cases has a bug in case this
feature is *not* supported. Our current iwx(4) firmware supports the
feature, so there is no actual problem for now.

I have considered removing the command variant which is currently not needed.
However, additional devices may be added to this driver at some point.
And some of those are kind of hybrids between the 9k and the ax200 device
generations. I do not know which variant those devices will need. So this
diff fixes the bug in the non-UHB code path, in case it will be needed.
We can always remove this later.

ok?

 M  sys/dev/pci/if_iwx.c
 M  sys/dev/pci/if_iwxreg.h

diff f896dbcaba91a37b23dc6cbeb42839fba3e329be 
24bd56723f1f029278b7c9cc71d56349db3c5950
blob - 64c3641a2d0d07a9d899c0b7ccdbe46d46e17b96
blob + d01db87880cc583922d1610de8319e491e7f148c
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -343,10 +343,8 @@ void   iwx_rx_tx_cmd(struct iwx_softc *, struct 
iwx_rx_p
 void   iwx_rx_bmiss(struct iwx_softc *, struct iwx_rx_packet *,
struct iwx_rx_data *);
 intiwx_binding_cmd(struct iwx_softc *, struct iwx_node *, uint32_t);
-void   iwx_phy_ctxt_cmd_hdr(struct iwx_softc *, struct iwx_phy_ctxt *,
-   struct iwx_phy_context_cmd *, uint32_t, uint32_t);
-void   iwx_phy_ctxt_cmd_data(struct iwx_softc *, struct iwx_phy_context_cmd *,
-   struct ieee80211_channel *, uint8_t, uint8_t);
+intiwx_phy_ctxt_cmd_uhb(struct iwx_softc *, struct iwx_phy_ctxt *, uint8_t,
+   uint8_t, uint32_t, uint32_t);
 intiwx_phy_ctxt_cmd(struct iwx_softc *, struct iwx_phy_ctxt *, uint8_t,
uint8_t, uint32_t, uint32_t);
 intiwx_send_cmd(struct iwx_softc *, struct iwx_host_cmd *);
@@ -3773,52 +3771,38 @@ iwx_binding_cmd(struct iwx_softc *sc, struct iwx_node 
return err;
 }
 
-void
-iwx_phy_ctxt_cmd_hdr(struct iwx_softc *sc, struct iwx_phy_ctxt *ctxt,
-struct iwx_phy_context_cmd *cmd, uint32_t action, uint32_t apply_time)
+int
+iwx_phy_ctxt_cmd_uhb(struct iwx_softc *sc, struct iwx_phy_ctxt *ctxt,
+uint8_t chains_static, uint8_t chains_dynamic, uint32_t action,
+uint32_t apply_time)
 {
-   memset(cmd, 0, sizeof(struct iwx_phy_context_cmd));
+   struct ieee80211com *ic = &sc->sc_ic;
+   struct iwx_phy_context_cmd_uhb cmd;
+   uint8_t active_cnt, idle_cnt;
+   struct ieee80211_channel *chan = ctxt->channel;
 
-   cmd->id_and_color = htole32(IWX_FW_CMD_ID_AND_COLOR(ctxt->id,
+   memset(&cmd, 0, sizeof(cmd));
+   cmd.id_and_color = htole32(IWX_FW_CMD_ID_AND_COLOR(ctxt->id,
ctxt->color));
-   cmd->action = htole32(action);
-   cmd->apply_time = htole32(apply_time);
-}
+   cmd.action = htole32(action);
+   cmd.apply_time = htole32(apply_time);
 
-void
-iwx_phy_ctxt_cmd_data(struct iwx_softc *sc, struct iwx_phy_context_cmd *cmd,
-struct ieee80211_channel *chan, uint8_t chains_static,
-uint8_t chains_dynamic)
-{
-   struct ieee80211com *ic = &sc->sc_ic;
-   uint8_t active_cnt, idle_cnt;
+   cmd.ci.band = IEEE80211_IS_CHAN_2GHZ(chan) ?
+   IWX_PHY_BAND_24 : IWX_PHY_BAND_5;
+   cmd.ci.channel = htole32(ieee80211_chan2ieee(ic, chan));
+   cmd.ci.width = IWX_PHY_VHT_CHANNEL_MODE20;
+   cmd.ci.ctrl_pos = IWX_PHY_VHT_CTRL_POS_1_BELOW;
 
-   if (isset(sc->sc_enabled_capa, IWX_UCODE_TLV_CAPA_ULTRA_HB_CHANNELS)) {
-   cmd->ci.band = IEEE80211_IS_CHAN_2GHZ(chan) ?
-   IWX_PHY_BAND_24 : IWX_PHY_BAND_5;
-   cmd->ci.channel = htole32(ieee80211_chan2ieee(ic, chan));
-   cmd->ci.width = IWX_PHY_VHT_CHANNEL_MODE20;
-   cmd->ci.ctrl_pos = IWX_PHY_VHT_CTRL_POS_1_BELOW;
-   } else {
-   struct iwx_fw_channel_info_v1 *ci_v1;
-   ci_v1 = (struct iwx_fw_channel_info_v1 *)&cmd->ci;
-   ci_v1->band = IEEE80211_IS_CHAN_2GHZ(chan) ?
-   IWX_PHY_BAND_24 : IWX_PHY_BAND_5;
-   ci_v1->channel = ieee80211_chan2ieee(ic, chan);
-   ci_v1->width = IWX_PHY_VHT_CHANNEL_MODE20;
-   ci_v1->ctrl_pos = IWX_PHY_VHT_CTRL_POS_1_BELOW;
-   }
-   /* Set rx the chains */
idle_cnt = chains_static;
active_cnt = chains_dynamic;
-
-   cmd->rxchain_info = htole32(iwx_fw_valid_rx_ant(sc) <<
+   cmd.rxchain_info = htole32(iwx_fw_valid_rx_ant(sc) <<
IWX_PHY_RX_CHAIN_VALID_POS);
-   cmd->rxchain_info |= htole32(idle_cnt <<

Re: ospfctl json support

2020-05-15 Thread Richard Chivers
Hi,

I have now resolved the spacing/tabbing issues I think correctly
following style(9), along with a couple of other indent issues.

Would appreciate a cursory look at this stage to spot any further common issues.

I have read through style(9) and can't spot anything else, but I may
be missing something

>From a workflow perspective can anyone suggest good tools that work
with style(9) rules? I have spotted indent.1, is this generally in
peoples workflow or is it typically a manual approach to each file?

Cheers

Richard

On Thu, May 14, 2020 at 7:33 PM Denis Fondras  wrote:
>
> On Thu, May 14, 2020 at 07:15:41PM +0100, Richard Chivers wrote:
> > Shall I effectively fix issues in the original code at this stage, or only
> > where I have moved and refactored?
> >
>
> Thanks. Limit the changes to what is relative to json support. The diff is
> already big enough :)


ospfctl.diff
Description: Binary data


Re: iwn/athn/wpi: fix CCMP replay check with HW crypto

2020-05-15 Thread Stefan Sperling
On Fri, May 08, 2020 at 10:53:30AM +0200, Stefan Sperling wrote:
> So the diff below contains just the reordering fix for drivers using
> hardware acceleration for WPA.

Is there anybody who wants to OK this?

To recap:

Currently, CCMP replay checking is skipped for aggregated 11n frames on
drivers which use CCMP hardware acceleration: athn(4) and iwn(4).

This diff makes reply checking in that case possible without false positives,
by updating the last-seen packet number after the aggregated subframes, which
may be received out of order, have been put back into their intended order by
ieee80211_input_ba(). The old code only works in the single-frame case for
11a/b/g modes where no such legitimate re-transmissions can occur.

> diff refs/heads/master refs/heads/pn
> 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);
>   return 0;
>  }
> blob - 6b6331d167a881a526b9d0a30f7f452882fee19a
> blob + 620cbd4c138516398a4683d83c1b6bf8aac57c82
> --- sys/dev/pci/if_iwn.c
> +++ sys/dev/pci/if_iwn.c
> @@ -1936,47 +1936,13 @