Re: initial 11ac support for iwm(4)

2022-03-17 Thread Landry Breuil
Le Wed, Mar 16, 2022 at 11:17:47PM +0100, Stefan Sperling a écrit :
> On Wed, Mar 16, 2022 at 04:11:41PM +0100, Stefan Sperling wrote:
> > This patch adds initial 11ac support to the iwm(4) driver.
> > It allows use of 80 MHz channels and VHT MCS.
> 
> Updated patch. Fixes a fatal firmware error on devices which
> do not support MIMO, such as the 3160.

seems to work fine with 7265 on the x1c3 against a livebox 4 - my MSM430
doesnt support 11ac afaict. Havent tested performance as im too far away
from the AP.

iwm0 at pci2 dev 0 function 0 "Intel AC 7265" rev 0x59, msi
iwm0: hw rev 0x210, fw ver 17.3216344376.0, address 34:02:86:19:ff:0f
iwm0: associated with 90:4d:4a:aa:95:95 ssid "Livebox-9590" channel 108 start 
MCS 0 long preamble short slot time HT enabled VHT enabled

media hops between VHT-MCS0 & VHT-MCS1.

interestingly, when associated over ac to the livebox the background
scans only shows the 5ghz channels from both APs, but when im associated
to my regular AP the background scans shows both 2ghz and 5ghz chans.
not sure that matters.

Thanks !

Landry



Re: XBox One gamecontroller support

2022-03-17 Thread Thomas Frohwein
On Thu, Mar 17, 2022 at 10:58:21PM +0100, Solene Rapenne wrote:

[...]

> > 
> 
> ping
> 
> this diff is still applying to the kernel and allows to use a popular
> and affordable game controller

The diff was written fairly conservatively based on pre-existing code
and NetBSD's solution. It would be nice if someone with more experience
with the USB fundamentals would comment, but maybe it could just go in
in its current form.



document route sourceaddr limits with raw sockets

2022-03-17 Thread Denis Fondras
This is a recurring complaint so better document it.

Denis

Index: route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.102
diff -u -p -r1.102 route.8
--- route.8 26 Oct 2021 15:48:25 -  1.102
+++ route.8 17 Mar 2022 22:42:37 -
@@ -262,6 +262,9 @@ destination is on-link
 .It
 source address is assigned to a disabled interface
 .El
+.Pp
+Note that the preferred source address is not set when raw
+sockets are used.
 .El
 .Pp
 .Tg destination



Re: initial 11ac support for iwm(4)

2022-03-17 Thread Stefan Sperling
On Thu, Mar 17, 2022 at 02:43:14PM -0700, Mike Larkin wrote:
> On Wed, Mar 16, 2022 at 11:17:47PM +0100, Stefan Sperling wrote:
> > On Wed, Mar 16, 2022 at 04:11:41PM +0100, Stefan Sperling wrote:
> > > This patch adds initial 11ac support to the iwm(4) driver.
> > > It allows use of 80 MHz channels and VHT MCS.
> >
> > Updated patch. Fixes a fatal firmware error on devices which
> > do not support MIMO, such as the 3160.
> >
> 
> Works great here on
> 
> iwm0 at pci3 dev 0 function 0 "Intel AC 7265" rev 0x59, msi
> iwm0: hw rev 0x210, fw ver 17.3216344376.0
> 
> One thing I did notice is that after ZZZ/un-ZZZ, it won't move out of 11n mode
> until I ifconfig iwm0 down / up. Then it picks up the VHT rate again.

The device must have used an AP on a 2 GHz channel when it stayed
in 11n mode. This is expected because 11ac only works on 5GHz channels.

Our AP selection is not based on AP capabilities, only on the received
signal strength. I don't want to change this right now, the changes
are large enough as it is. But we could tweak this in the future such
that 11ac gets preferred over 11n if possible.



Re: initial 11ac support for iwm(4)

2022-03-17 Thread Mike Larkin
On Wed, Mar 16, 2022 at 11:17:47PM +0100, Stefan Sperling wrote:
> On Wed, Mar 16, 2022 at 04:11:41PM +0100, Stefan Sperling wrote:
> > This patch adds initial 11ac support to the iwm(4) driver.
> > It allows use of 80 MHz channels and VHT MCS.
>
> Updated patch. Fixes a fatal firmware error on devices which
> do not support MIMO, such as the 3160.
>

Works great here on

iwm0 at pci3 dev 0 function 0 "Intel AC 7265" rev 0x59, msi
iwm0: hw rev 0x210, fw ver 17.3216344376.0

One thing I did notice is that after ZZZ/un-ZZZ, it won't move out of 11n mode
until I ifconfig iwm0 down / up. Then it picks up the VHT rate again.

-ml



Re: initial 11ac support for iwm(4)

2022-03-17 Thread Stefan Sperling
On Thu, Mar 17, 2022 at 07:02:06PM +0100, Marcus MERIGHI wrote:
> Hello!
> 
> Thanks for your work on this!
> 
> s...@stsp.name (Stefan Sperling), 2022.03.16 (Wed) 16:11 (CET):
> > This patch adds initial 11ac support to the iwm(4) driver.
> > It allows use of 80 MHz channels and VHT MCS.
> [...]
> > So far, I have tested this successfully on iwm(4) 8265.
> 
> With this patch a "larger" file transfer will reliably panic my machine.
> "large" means even small files. 
> ICMP and NTP (and probably DNS) seem to work, though.
> I've tried it six times, it always crashed.
> 
> This is with:
> iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x88, msi
> iwm0: hw rev 0x230, fw ver 36.ca7b901d.0, address 00:28:f8:xx:yy:zz

If you have not done so yet, could you please try the revised
version of the patch which I posted here?
https://marc.info/?l=openbsd-tech=164746924001340=2
I believe this should fix your issue.

Below is the difference between the two versions, in case it is easier
for you to apply the fix on top of the first version of the patch.
 
diff 0191ab443b6994d9adb38f5f22f66ffea008c0b7 
00811a880ed9b9c98b475f172bf5f6fe3e31811f
blob - 3d13d9f024f349521cdf890955a4b6ce2e88fd5d
blob + dea634bcb86b2e41a7da27dd7be552b751eb1b66
--- sys/net80211/ieee80211_ra_vht.c
+++ sys/net80211/ieee80211_ra_vht.c
@@ -233,7 +233,7 @@ const struct ieee80211_vht_rateset *
 ieee80211_ra_vht_next_rateset(struct ieee80211_ra_vht_node *rn,
 struct ieee80211_node *ni)
 {
-   const struct ieee80211_vht_rateset *rs;
+   const struct ieee80211_vht_rateset *rs, *rsnext;
int next;
int sgi = ieee80211_ra_vht_use_sgi(ni);
int mcs = ni->ni_txmcs;
@@ -269,7 +269,11 @@ ieee80211_ra_vht_next_rateset(struct ieee80211_ra_vht_
} else
panic("%s: invalid probing mode %d", __func__, rn->probing);
 
-   return _std_ratesets_11ac[next];
+   rsnext = _std_ratesets_11ac[next];
+   if (rn->valid_rates[rsnext->num_ss - 1] == 0)
+   return NULL;
+
+   return rsnext;
 }
 
 int



Re: wg: remove argument names from prototypes

2022-03-17 Thread Theo Buehler
On Thu, Mar 17, 2022 at 07:17:40PM +0100, Martin Vahlensieck wrote:
> None of the other prototypes have argument names.

Committed, thanks



Re: rpki-client(8): properly zero terminate pretty printed key

2022-03-17 Thread Theo Buehler
On Thu, Mar 17, 2022 at 07:24:34PM +0100, Martin Vahlensieck wrote:
> Hi
> 
> It seems the pretty printed key is zero terminated only if the size
> of hex stays the same or increases between calls.  This diff fixes
> it, so it is always properly terminated.  While here, also drop
> *hex != '\0' from the if inside the loop, as it is checked directly
> above in the loop condition and constify the argument, as it is not
> modified.

ok tb

The ski and aki that are printed with this function are always SHA-1
digests, so they are actually NUL terminateed.



Re: two small typos

2022-03-17 Thread Stuart Henderson
thanks, committed.

On 2022/03/17 19:16, Martin Vahlensieck wrote:
> Index: if_wg.c
> ===
> RCS file: /home/reposync/cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 if_wg.c
> --- if_wg.c   22 Feb 2022 01:15:02 -  1.22
> +++ if_wg.c   15 Mar 2022 21:10:37 -
> @@ -2023,7 +2023,7 @@ wg_input(void *_sc, struct mbuf *m, stru
>  
>   /*
>* Ensure mbuf is contiguous over full length of packet. This is done
> -  * os we can directly read the handshake values in wg_handshake, and so
> +  * so we can directly read the handshake values in wg_handshake, and so
>* we can decrypt a transport packet by passing a single buffer to
>* noise_remote_decrypt in wg_decap.
>*/
> Index: pf.c
> ===
> RCS file: /home/reposync/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1125
> diff -u -p -r1.1125 pf.c
> --- pf.c  5 Mar 2022 10:43:32 -   1.1125
> +++ pf.c  10 Mar 2022 15:53:51 -
> @@ -1340,7 +1340,7 @@ pf_state_expires(const struct pf_state *
>* this function may be called by the state purge task while
>* the state is being modified. avoid inconsistent reads of
>* state->timeout by having the caller do the read (and any
> -  * chacks it needs to do on the same variable) and then pass
> +  * checks it needs to do on the same variable) and then pass
>* their view of the timeout in here for this function to use.
>* the only consequence of using a stale timeout value is
>* that the state won't be a candidate for purging until the
> 



rpki-client(8): properly zero terminate pretty printed key

2022-03-17 Thread Martin Vahlensieck
Hi

It seems the pretty printed key is zero terminated only if the size
of hex stays the same or increases between calls.  This diff fixes
it, so it is always properly terminated.  While here, also drop
*hex != '\0' from the if inside the loop, as it is checked directly
above in the loop condition and constify the argument, as it is not
modified.

Best,

Martin

Index: print.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
retrieving revision 1.5
diff -u -p -r1.5 print.c
--- print.c 10 Feb 2022 17:33:28 -  1.5
+++ print.c 17 Mar 2022 17:46:01 -
@@ -28,19 +28,21 @@
 #include "extern.h"
 
 static const char *
-pretty_key_id(char *hex)
+pretty_key_id(const char *hex)
 {
static char buf[128];   /* bigger than SHA_DIGEST_LENGTH * 3 */
size_t i;
 
for (i = 0; i < sizeof(buf) && *hex != '\0'; i++) {
-   if  (i % 3 == 2 && *hex != '\0')
+   if  (i % 3 == 2)
buf[i] = ':';
else
buf[i] = *hex++;
}
if (i == sizeof(buf))
memcpy(buf + sizeof(buf) - 4, "...", 4);
+   else
+   buf[i] = '\0';
return buf;
 }
 



wg: remove argument names from prototypes

2022-03-17 Thread Martin Vahlensieck
None of the other prototypes have argument names.

Index: if_wg.c
===
RCS file: /home/reposync/cvs/src/sys/net/if_wg.c,v
retrieving revision 1.22
diff -u -p -r1.22 if_wg.c
--- if_wg.c 22 Feb 2022 01:15:02 -  1.22
+++ if_wg.c 15 Mar 2022 21:10:37 -
@@ -325,7 +325,7 @@ voidwg_peer_send_buf(struct wg_peer *, 
 void   wg_send_initiation(void *);
 void   wg_send_response(struct wg_peer *);
 void   wg_send_cookie(struct wg_softc *, struct cookie_macs *, uint32_t,
-   struct wg_endpoint *e);
+   struct wg_endpoint *);
 void   wg_send_keepalive(void *);
 void   wg_peer_clear_secrets(void *);
 void   wg_handshake(struct wg_softc *, struct mbuf *);
Index: wg_cookie.c
===
RCS file: /home/reposync/cvs/src/sys/net/wg_cookie.c,v
retrieving revision 1.3
diff -u -p -r1.3 wg_cookie.c
--- wg_cookie.c 10 Mar 2021 10:21:48 -  1.3
+++ wg_cookie.c 15 Mar 2022 21:09:29 -
@@ -37,7 +37,7 @@ static void   cookie_macs_mac2(struct cook
 static int cookie_timer_expired(struct timespec *, time_t, long);
 static voidcookie_checker_make_cookie(struct cookie_checker *,
uint8_t[COOKIE_COOKIE_SIZE], struct sockaddr *);
-static int ratelimit_init(struct ratelimit *, struct pool *pool);
+static int ratelimit_init(struct ratelimit *, struct pool *);
 static voidratelimit_deinit(struct ratelimit *);
 static voidratelimit_gc(struct ratelimit *, int);
 static int ratelimit_allow(struct ratelimit *, struct sockaddr *);



two small typos

2022-03-17 Thread Martin Vahlensieck
Index: if_wg.c
===
RCS file: /home/reposync/cvs/src/sys/net/if_wg.c,v
retrieving revision 1.22
diff -u -p -r1.22 if_wg.c
--- if_wg.c 22 Feb 2022 01:15:02 -  1.22
+++ if_wg.c 15 Mar 2022 21:10:37 -
@@ -2023,7 +2023,7 @@ wg_input(void *_sc, struct mbuf *m, stru
 
/*
 * Ensure mbuf is contiguous over full length of packet. This is done
-* os we can directly read the handshake values in wg_handshake, and so
+* so we can directly read the handshake values in wg_handshake, and so
 * we can decrypt a transport packet by passing a single buffer to
 * noise_remote_decrypt in wg_decap.
 */
Index: pf.c
===
RCS file: /home/reposync/cvs/src/sys/net/pf.c,v
retrieving revision 1.1125
diff -u -p -r1.1125 pf.c
--- pf.c5 Mar 2022 10:43:32 -   1.1125
+++ pf.c10 Mar 2022 15:53:51 -
@@ -1340,7 +1340,7 @@ pf_state_expires(const struct pf_state *
 * this function may be called by the state purge task while
 * the state is being modified. avoid inconsistent reads of
 * state->timeout by having the caller do the read (and any
-* chacks it needs to do on the same variable) and then pass
+* checks it needs to do on the same variable) and then pass
 * their view of the timeout in here for this function to use.
 * the only consequence of using a stale timeout value is
 * that the state won't be a candidate for purging until the



Re: refcount btrace

2022-03-17 Thread Alexander Bluhm
On Thu, Mar 17, 2022 at 07:25:27AM +, Visa Hankala wrote:
> On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote:
> > I would like to use btrace to debug refernce counting.  The idea
> > is to a a tracepoint for every type of refcnt we have.  When it
> > changes, print the actual object, the current counter and the change
> > value.
> 
> > Do we want that feature?
> 
> I am against this in its current form. The code would become more
> complex, and the trace points can affect timing. There is a risk that
> the kernel behaves slightly differently when dt has been compiled in.

Can we get in this part then?

- Remove DIAGNOSTIC to keep similar in non DIAGNOSTIC case.
- Rename refcnt to refs.  refcnt is the struct, refs contains the
  r_refs value.
- Add KASSERT(refs != ~0) in refcnt_finalize().
- Always use u_int refs so I can insert my btrace diff easily.

Maybe I can optimize btrace diff later.

bluhm

Index: kern/kern_synch.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.184
diff -u -p -r1.184 kern_synch.c
--- kern/kern_synch.c   16 Mar 2022 14:13:01 -  1.184
+++ kern/kern_synch.c   17 Mar 2022 16:12:50 -
@@ -810,25 +810,21 @@ refcnt_init(struct refcnt *r)
 void
 refcnt_take(struct refcnt *r)
 {
-#ifdef DIAGNOSTIC
-   u_int refcnt;
+   u_int refs;
 
-   refcnt = atomic_inc_int_nv(>r_refs);
-   KASSERT(refcnt != 0);
-#else
-   atomic_inc_int(>r_refs);
-#endif
+   refs = atomic_inc_int_nv(>r_refs);
+   KASSERT(refs != 0);
+   (void)refs;
 }
 
 int
 refcnt_rele(struct refcnt *r)
 {
-   u_int refcnt;
+   u_int refs;
 
-   refcnt = atomic_dec_int_nv(>r_refs);
-   KASSERT(refcnt != ~0);
-
-   return (refcnt == 0);
+   refs = atomic_dec_int_nv(>r_refs);
+   KASSERT(refs != ~0);
+   return (refs == 0);
 }
 
 void
@@ -842,26 +838,33 @@ void
 refcnt_finalize(struct refcnt *r, const char *wmesg)
 {
struct sleep_state sls;
-   u_int refcnt;
+   u_int refs;
 
-   refcnt = atomic_dec_int_nv(>r_refs);
-   while (refcnt) {
+   refs = atomic_dec_int_nv(>r_refs);
+   KASSERT(refs != ~0);
+   while (refs) {
sleep_setup(, r, PWAIT, wmesg, 0);
-   refcnt = atomic_load_int(>r_refs);
-   sleep_finish(, refcnt);
+   refs = atomic_load_int(>r_refs);
+   sleep_finish(, refs);
}
 }
 
 int
 refcnt_shared(struct refcnt *r)
 {
-   return (atomic_load_int(>r_refs) > 1);
+   u_int refs;
+
+   refs = atomic_load_int(>r_refs);
+   return (refs > 1);
 }
 
 unsigned int
 refcnt_read(struct refcnt *r)
 {
-   return (atomic_load_int(>r_refs));
+   u_int refs;
+
+   refs = atomic_load_int(>r_refs);
+   return (refs);
 }
 
 void



Re: Use refcnt API with struct plimit

2022-03-17 Thread Alexander Bluhm
On Thu, Mar 17, 2022 at 04:07:24PM +, Visa Hankala wrote:
> Use the refcnt API with struct plimit.
> 
> OK?

OK bluhm@

> Index: kern/kern_resource.c
> ===
> RCS file: src/sys/kern/kern_resource.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 kern_resource.c
> --- kern/kern_resource.c  8 Feb 2021 10:51:01 -   1.71
> +++ kern/kern_resource.c  17 Mar 2022 15:59:52 -
> @@ -582,7 +582,7 @@ lim_startup(struct plimit *limit0)
>   limit0->pl_rlimit[RLIMIT_RSS].rlim_max = lim;
>   limit0->pl_rlimit[RLIMIT_MEMLOCK].rlim_max = lim;
>   limit0->pl_rlimit[RLIMIT_MEMLOCK].rlim_cur = lim / 3;
> - limit0->pl_refcnt = 1;
> + refcnt_init(>pl_refcnt);
>  }
>  
>  /*
> @@ -598,14 +598,14 @@ lim_copy(struct plimit *lim)
>   newlim = pool_get(_pool, PR_WAITOK);
>   memcpy(newlim->pl_rlimit, lim->pl_rlimit,
>   sizeof(struct rlimit) * RLIM_NLIMITS);
> - newlim->pl_refcnt = 1;
> + refcnt_init(>pl_refcnt);
>   return (newlim);
>  }
>  
>  void
>  lim_free(struct plimit *lim)
>  {
> - if (atomic_dec_int_nv(>pl_refcnt) > 0)
> + if (refcnt_rele(>pl_refcnt) == 0)
>   return;
>   pool_put(_pool, lim);
>  }
> @@ -617,7 +617,7 @@ lim_fork(struct process *parent, struct 
>  
>   mtx_enter(>ps_mtx);
>   limit = parent->ps_limit;
> - atomic_inc_int(>pl_refcnt);
> + refcnt_take(>pl_refcnt);
>   mtx_leave(>ps_mtx);
>  
>   child->ps_limit = limit;
> @@ -650,7 +650,7 @@ lim_write_begin(void)
>*/
>  
>   limit = p->p_p->ps_limit;
> - if (P_HASSIBLING(p) || limit->pl_refcnt > 1)
> + if (P_HASSIBLING(p) || refcnt_shared(>pl_refcnt))
>   limit = lim_copy(limit);
>  
>   return (limit);
> @@ -703,7 +703,7 @@ lim_read_enter(void)
>   if (limit != pr->ps_limit) {
>   mtx_enter(>ps_mtx);
>   limit = pr->ps_limit;
> - atomic_inc_int(>pl_refcnt);
> + refcnt_take(>pl_refcnt);
>   mtx_leave(>ps_mtx);
>   if (p->p_limit != NULL)
>   lim_free(p->p_limit);
> Index: sys/resourcevar.h
> ===
> RCS file: src/sys/sys/resourcevar.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 resourcevar.h
> --- sys/resourcevar.h 21 Jun 2019 09:39:48 -  1.24
> +++ sys/resourcevar.h 17 Mar 2022 15:59:52 -
> @@ -35,6 +35,7 @@
>  #ifndef  _SYS_RESOURCEVAR_H_
>  #define  _SYS_RESOURCEVAR_H_
>  
> +#include 
>  #include 
>  
>  /*
> @@ -44,7 +45,7 @@
>   */
>  struct plimit {
>   struct  rlimit pl_rlimit[RLIM_NLIMITS];
> - u_int   pl_refcnt;  /* number of references */
> + struct  refcnt pl_refcnt;
>  };
>  
>  /* add user profiling from AST */



Re: initial 11ac support for iwm(4)

2022-03-17 Thread Florian Obser
Still works fine on 9260.

While playing around with this I noticed something else which is
probably not a regression:

I have two SSIDs, "normal" and NAT64, they are on the same AP and just
come out on different vlans, they use the same channel. They are also on
2.4GHz.

Switching between them with
ifconfig iwm0 nwid MYSSID wpakey...
ifconfig iwm0 nwid MYSSID64 wpakey...
ifconfig iwm0 nwid MYSSID wpakey...
and so on sometimes makes iwm0 chose channel 1, i.e. 2.4GHz which is a
20MHz wide channel. And it really likes it there, it's not leaving even
when I switch the SSID around. ifconfig iwm0 mode 11ac makes it switch
over and then it stays there again, even if I do ifconfig iwm0 -mode.

It's a bit hard to trigger, it happens maybe 1 in 50 tries or so? Or
maybe it's a timing thing?

-- 
I'm not entirely sure you are real.



Use refcnt API with struct plimit

2022-03-17 Thread Visa Hankala
Use the refcnt API with struct plimit.

OK?

Index: kern/kern_resource.c
===
RCS file: src/sys/kern/kern_resource.c,v
retrieving revision 1.71
diff -u -p -r1.71 kern_resource.c
--- kern/kern_resource.c8 Feb 2021 10:51:01 -   1.71
+++ kern/kern_resource.c17 Mar 2022 15:59:52 -
@@ -582,7 +582,7 @@ lim_startup(struct plimit *limit0)
limit0->pl_rlimit[RLIMIT_RSS].rlim_max = lim;
limit0->pl_rlimit[RLIMIT_MEMLOCK].rlim_max = lim;
limit0->pl_rlimit[RLIMIT_MEMLOCK].rlim_cur = lim / 3;
-   limit0->pl_refcnt = 1;
+   refcnt_init(>pl_refcnt);
 }
 
 /*
@@ -598,14 +598,14 @@ lim_copy(struct plimit *lim)
newlim = pool_get(_pool, PR_WAITOK);
memcpy(newlim->pl_rlimit, lim->pl_rlimit,
sizeof(struct rlimit) * RLIM_NLIMITS);
-   newlim->pl_refcnt = 1;
+   refcnt_init(>pl_refcnt);
return (newlim);
 }
 
 void
 lim_free(struct plimit *lim)
 {
-   if (atomic_dec_int_nv(>pl_refcnt) > 0)
+   if (refcnt_rele(>pl_refcnt) == 0)
return;
pool_put(_pool, lim);
 }
@@ -617,7 +617,7 @@ lim_fork(struct process *parent, struct 
 
mtx_enter(>ps_mtx);
limit = parent->ps_limit;
-   atomic_inc_int(>pl_refcnt);
+   refcnt_take(>pl_refcnt);
mtx_leave(>ps_mtx);
 
child->ps_limit = limit;
@@ -650,7 +650,7 @@ lim_write_begin(void)
 */
 
limit = p->p_p->ps_limit;
-   if (P_HASSIBLING(p) || limit->pl_refcnt > 1)
+   if (P_HASSIBLING(p) || refcnt_shared(>pl_refcnt))
limit = lim_copy(limit);
 
return (limit);
@@ -703,7 +703,7 @@ lim_read_enter(void)
if (limit != pr->ps_limit) {
mtx_enter(>ps_mtx);
limit = pr->ps_limit;
-   atomic_inc_int(>pl_refcnt);
+   refcnt_take(>pl_refcnt);
mtx_leave(>ps_mtx);
if (p->p_limit != NULL)
lim_free(p->p_limit);
Index: sys/resourcevar.h
===
RCS file: src/sys/sys/resourcevar.h,v
retrieving revision 1.24
diff -u -p -r1.24 resourcevar.h
--- sys/resourcevar.h   21 Jun 2019 09:39:48 -  1.24
+++ sys/resourcevar.h   17 Mar 2022 15:59:52 -
@@ -35,6 +35,7 @@
 #ifndef_SYS_RESOURCEVAR_H_
 #define_SYS_RESOURCEVAR_H_
 
+#include 
 #include 
 
 /*
@@ -44,7 +45,7 @@
  */
 struct plimit {
struct  rlimit pl_rlimit[RLIM_NLIMITS];
-   u_int   pl_refcnt;  /* number of references */
+   struct  refcnt pl_refcnt;
 };
 
 /* add user profiling from AST */



Re: pcb mutex userland

2022-03-17 Thread Alexander Bluhm
On Thu, Mar 17, 2022 at 02:09:39PM +0100, Mark Kettenis wrote:
> I fear the fundamental problem is that we should not expose data
> structures internal to the kernel to userland.  What I don't
> understand though is how that happens.  The sysctl code doesn't seem
> to export "struct inpcb" instances directly, but instead it exports
> selected members through "struct kinfo_file".  So why is "struct
> inpcb" exposed to userland at all?

A few tools use it.  One thing is post mortem analysis of kernel
core dumps.  Sometimes I get dumps sent from customers.  They don't
have WITNESS.

Some traditional network debugging tools use these strutures.

lib/libkvm
sbin/sysctl
usr.bin/netstat
usr.bin/tcpbench
usr.sbin/trpt

As you need sysctl kern.allowkmem=1 for them this is only useful
on debugging machines.  Ob course they can be rewritten using sysctl.
A drawback is that you have to write a lot of copy code and post
mortem analysis code gets out of sync.  I see tools with -M -N
breaking over time.

The question is how to proceed.  For MP in the network stack I need
mutex in structs.  And I don't want to rewrite all tools before
making progress.

bluhm

> > Index: sys/netinet/in_pcb.h
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
> > retrieving revision 1.125
> > diff -u -p -r1.125 in_pcb.h
> > --- sys/netinet/in_pcb.h14 Mar 2022 22:38:43 -  1.125
> > +++ sys/netinet/in_pcb.h17 Mar 2022 00:44:54 -
> > @@ -65,6 +65,7 @@
> >  #define _NETINET_IN_PCB_H_
> >  
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > Index: sys/sys/mutex.h
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mutex.h,v
> > retrieving revision 1.18
> > diff -u -p -r1.18 mutex.h
> > --- sys/sys/mutex.h 23 Apr 2019 13:35:12 -  1.18
> > +++ sys/sys/mutex.h 17 Mar 2022 00:44:23 -
> > @@ -48,6 +48,8 @@ struct mutex {
> >  #endif
> >  };
> >  
> > +#ifdef _KERNEL
> > +
> >  /*
> >   * To prevent lock ordering problems with the kernel lock, we need to
> >   * make sure we block all interrupts that can grab the kernel lock.
> > @@ -148,7 +150,7 @@ void_mtx_init_flags(struct mutex *, int
> >  
> >  #endif /* WITNESS */
> >  
> > -#if defined(_KERNEL) && defined(DDB)
> > +#ifdef DDB
> >  
> >  struct db_mutex {
> > struct cpu_info *mtx_owner;
> > @@ -160,6 +162,8 @@ struct db_mutex {
> >  void   db_mtx_enter(struct db_mutex *);
> >  void   db_mtx_leave(struct db_mutex *);
> >  
> > -#endif /* _KERNEL && DDB */
> > +#endif /* DDB */
> > +
> > +#endif /* _KERNEL */
> >  
> >  #endif
> > 
> > 



Re: pcb mutex userland

2022-03-17 Thread Claudio Jeker
On Thu, Mar 17, 2022 at 02:09:39PM +0100, Mark Kettenis wrote:
> > Date: Thu, 17 Mar 2022 13:24:24 +0100
> > From: Alexander Bluhm 
> > 
> > On Thu, Mar 17, 2022 at 08:24:10AM +0100, Claudio Jeker wrote:
> > > On Thu, Mar 17, 2022 at 12:47:15AM +0100, Alexander Bluhm wrote:
> > > > Hi,
> > > > 
> > > > My previous atempt to add a mutex to in_pcb.h was reverted as it
> > > > broke userland build.
> > > > 
> > > > Is the correct fix to include sys/mutex.h in every .c file that
> > > > includes netinet/in_pcb.h ?  I made a release with it.
> > > > Or should I include sys/mutex.h in netinet/in_pcb.h ?
> > > 
> > > I would add sys/mutex.h in netinet/in_pcb.h. We do the same in other
> > > headers like sys/proc.h etc.
> > 
> > This survived make release.  It is similar to what we do in sys/proc.h
> > as suggested by claudio@ and has more #ifdef _KERNEL to please
> > kettenis@.
> > 
> > ok?
> 
> Sorry, but I don't think it is.  The problem is that "struct mutex"
> changes depending on whether WITNESS is defined.  This means that if
> you include mutex in a data structure exported to userland and you're
> running a kernel with WITNESS enabled, the data structures don't match
> up.
> 
> The reverse is also possible (although much less likely).  Since
> WITNESS is in the global namespace, code might define it and therefore
> accidentally change the data structure based and cause a mismatch when
> you're running on a normal kernel.
> 
> I fear the fundamental problem is that we should not expose data
> structures internal to the kernel to userland.  What I don't
> understand though is how that happens.  The sysctl code doesn't seem
> to export "struct inpcb" instances directly, but instead it exports
> selected members through "struct kinfo_file".  So why is "struct
> inpcb" exposed to userland at all?

The netstat -P code is using kvm so does -M using a core dump.
We probably have similar issues in other tools. At least the -M core -N
system options are also present in other tools like ps(1). So this issue
is not new.

Not sure how to fix this issue with altering struct sizes based on kernel
options. For netstat -P it would be possible to add a interface to give
access to this info using sysctl and struct kinfo_file (but the struct
would have to be extended a fair amount) or maybe add a kinfo_pcb for this
which works like kinfo_file but for pcbs.
 
> > Index: sys/netinet/in_pcb.h
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
> > retrieving revision 1.125
> > diff -u -p -r1.125 in_pcb.h
> > --- sys/netinet/in_pcb.h14 Mar 2022 22:38:43 -  1.125
> > +++ sys/netinet/in_pcb.h17 Mar 2022 00:44:54 -
> > @@ -65,6 +65,7 @@
> >  #define _NETINET_IN_PCB_H_
> >  
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > Index: sys/sys/mutex.h
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mutex.h,v
> > retrieving revision 1.18
> > diff -u -p -r1.18 mutex.h
> > --- sys/sys/mutex.h 23 Apr 2019 13:35:12 -  1.18
> > +++ sys/sys/mutex.h 17 Mar 2022 00:44:23 -
> > @@ -48,6 +48,8 @@ struct mutex {
> >  #endif
> >  };
> >  
> > +#ifdef _KERNEL
> > +
> >  /*
> >   * To prevent lock ordering problems with the kernel lock, we need to
> >   * make sure we block all interrupts that can grab the kernel lock.
> > @@ -148,7 +150,7 @@ void_mtx_init_flags(struct mutex *, int
> >  
> >  #endif /* WITNESS */
> >  
> > -#if defined(_KERNEL) && defined(DDB)
> > +#ifdef DDB
> >  
> >  struct db_mutex {
> > struct cpu_info *mtx_owner;
> > @@ -160,6 +162,8 @@ struct db_mutex {
> >  void   db_mtx_enter(struct db_mutex *);
> >  void   db_mtx_leave(struct db_mutex *);
> >  
> > -#endif /* _KERNEL && DDB */
> > +#endif /* DDB */
> > +
> > +#endif /* _KERNEL */
> >  
> >  #endif
> > 
> > 
> 

-- 
:wq Claudio



Re: pcb mutex userland

2022-03-17 Thread Mark Kettenis
> Date: Thu, 17 Mar 2022 13:24:24 +0100
> From: Alexander Bluhm 
> 
> On Thu, Mar 17, 2022 at 08:24:10AM +0100, Claudio Jeker wrote:
> > On Thu, Mar 17, 2022 at 12:47:15AM +0100, Alexander Bluhm wrote:
> > > Hi,
> > > 
> > > My previous atempt to add a mutex to in_pcb.h was reverted as it
> > > broke userland build.
> > > 
> > > Is the correct fix to include sys/mutex.h in every .c file that
> > > includes netinet/in_pcb.h ?  I made a release with it.
> > > Or should I include sys/mutex.h in netinet/in_pcb.h ?
> > 
> > I would add sys/mutex.h in netinet/in_pcb.h. We do the same in other
> > headers like sys/proc.h etc.
> 
> This survived make release.  It is similar to what we do in sys/proc.h
> as suggested by claudio@ and has more #ifdef _KERNEL to please
> kettenis@.
> 
> ok?

Sorry, but I don't think it is.  The problem is that "struct mutex"
changes depending on whether WITNESS is defined.  This means that if
you include mutex in a data structure exported to userland and you're
running a kernel with WITNESS enabled, the data structures don't match
up.

The reverse is also possible (although much less likely).  Since
WITNESS is in the global namespace, code might define it and therefore
accidentally change the data structure based and cause a mismatch when
you're running on a normal kernel.

I fear the fundamental problem is that we should not expose data
structures internal to the kernel to userland.  What I don't
understand though is how that happens.  The sysctl code doesn't seem
to export "struct inpcb" instances directly, but instead it exports
selected members through "struct kinfo_file".  So why is "struct
inpcb" exposed to userland at all?

> Index: sys/netinet/in_pcb.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
> retrieving revision 1.125
> diff -u -p -r1.125 in_pcb.h
> --- sys/netinet/in_pcb.h  14 Mar 2022 22:38:43 -  1.125
> +++ sys/netinet/in_pcb.h  17 Mar 2022 00:44:54 -
> @@ -65,6 +65,7 @@
>  #define _NETINET_IN_PCB_H_
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> Index: sys/sys/mutex.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mutex.h,v
> retrieving revision 1.18
> diff -u -p -r1.18 mutex.h
> --- sys/sys/mutex.h   23 Apr 2019 13:35:12 -  1.18
> +++ sys/sys/mutex.h   17 Mar 2022 00:44:23 -
> @@ -48,6 +48,8 @@ struct mutex {
>  #endif
>  };
>  
> +#ifdef _KERNEL
> +
>  /*
>   * To prevent lock ordering problems with the kernel lock, we need to
>   * make sure we block all interrupts that can grab the kernel lock.
> @@ -148,7 +150,7 @@ void  _mtx_init_flags(struct mutex *, int
>  
>  #endif /* WITNESS */
>  
> -#if defined(_KERNEL) && defined(DDB)
> +#ifdef DDB
>  
>  struct db_mutex {
>   struct cpu_info *mtx_owner;
> @@ -160,6 +162,8 @@ struct db_mutex {
>  void db_mtx_enter(struct db_mutex *);
>  void db_mtx_leave(struct db_mutex *);
>  
> -#endif /* _KERNEL && DDB */
> +#endif /* DDB */
> +
> +#endif /* _KERNEL */
>  
>  #endif
> 
> 



Re: initial 11ac support for iwm(4)

2022-03-17 Thread Stefan Sperling
On Thu, Mar 17, 2022 at 01:07:42AM +, Stuart Henderson wrote:
> 802.11 flags=0<>: beacon, ...

> 191:12 0xb109cb33aaff1806aaff1806, 192:5 0x00aaff,

Now is probably a good time to start pretty-printing these fields in tcpdump.

ok?

diff 140ae54c8a573c04824dd96957ebff4e069b2dfd /usr/src
blob - 4af7bdb9350f0feaff2e562ee2a6ec0982de7a70
file + usr.sbin/tcpdump/print-802_11.c
--- usr.sbin/tcpdump/print-802_11.c
+++ usr.sbin/tcpdump/print-802_11.c
@@ -101,6 +101,8 @@ void ieee80211_print_essid(u_int8_t *, u_int);
 voidieee80211_print_country(u_int8_t *, u_int);
 voidieee80211_print_htcaps(u_int8_t *, u_int);
 voidieee80211_print_htop(u_int8_t *, u_int);
+voidieee80211_print_vhtcaps(u_int8_t *, u_int);
+voidieee80211_print_vhtop(u_int8_t *, u_int);
 voidieee80211_print_rsncipher(u_int8_t []);
 voidieee80211_print_akm(u_int8_t []);
 voidieee80211_print_rsn(u_int8_t *, u_int);
@@ -607,6 +609,199 @@ ieee80211_print_htop(u_int8_t *data, u_int len)
 }
 
 void
+print_vht_mcsmap(uint16_t mcsmap)
+{
+   int nss, mcs;
+
+   for (nss = 1; nss < IEEE80211_VHT_NUM_SS; nss++) {
+   mcs = (mcsmap & IEEE80211_VHT_MCS_FOR_SS_MASK(nss)) >>
+   IEEE80211_VHT_MCS_FOR_SS_SHIFT(nss);
+   switch (mcs) {
+   case IEEE80211_VHT_MCS_0_9:
+   printf(" 0-9@%uSS", nss);
+   break;
+   case IEEE80211_VHT_MCS_0_8:
+   printf(" 0-8@%uSS", nss);
+   break;
+   case IEEE80211_VHT_MCS_0_7:
+   printf(" 0-7@%uSS", nss);
+   break;
+   case IEEE80211_VHT_MCS_SS_NOT_SUPP:
+   default:
+   break;
+   }
+   }
+}
+
+/* Caller checks len */
+void
+ieee80211_print_vhtcaps(u_int8_t *data, u_int len)
+{
+   uint32_t vhtcaps;
+   uint16_t rxmcs, txmcs, max_lgi;
+   uint32_t rxstbc, num_sts, max_ampdu, link_adapt;
+
+   if (len < 12) {
+   ieee80211_print_element(data, len);
+   return;
+   }
+
+   vhtcaps = (data[0] | (data[1] << 8) | data[2] << 16 |
+   data[3] << 24);
+   printf("=<");
+
+   /* max MPDU length */
+   switch (vhtcaps & IEEE80211_VHTCAP_MAX_MPDU_LENGTH_MASK) {
+   case IEEE80211_VHTCAP_MAX_MPDU_LENGTH_11454:
+   printf("max MPDU 11454");
+   break;
+   case IEEE80211_VHTCAP_MAX_MPDU_LENGTH_7991:
+   printf("max MPDU 7991");
+   break;
+   case IEEE80211_VHTCAP_MAX_MPDU_LENGTH_3895:
+   default:
+   printf("max MPDU 3895");
+   break;
+   }
+
+   /* supported channel widths */
+   switch ((vhtcaps & IEEE80211_VHTCAP_CHAN_WIDTH_MASK) <<
+   IEEE80211_VHTCAP_CHAN_WIDTH_SHIFT) {
+   case IEEE80211_VHTCAP_CHAN_WIDTH_160_8080:
+   printf(",80+80MHz");
+   /* fallthrough */
+   case IEEE80211_VHTCAP_CHAN_WIDTH_160:
+   printf(",160MHz");
+   /* fallthrough */
+   case IEEE80211_VHTCAP_CHAN_WIDTH_80:
+   default:
+   printf(",80MHz");
+   break;
+   }
+
+   /* LDPC coding */
+   if (vhtcaps & IEEE80211_VHTCAP_RX_LDPC)
+   printf(",LDPC");
+
+   /* short guard interval */
+   if (vhtcaps & IEEE80211_VHTCAP_SGI80)
+   printf(",SGI@80MHz");
+   if (vhtcaps & IEEE80211_VHTCAP_SGI160)
+   printf(",SGI@160MHz");
+
+   /* space-time block coding */
+   if (vhtcaps & IEEE80211_VHTCAP_TX_STBC)
+   printf(",TxSTBC");
+   rxstbc = (vhtcaps & IEEE80211_VHTCAP_RX_STBC_SS_MASK)
+   >> IEEE80211_VHTCAP_RX_STBC_SS_SHIFT;
+   if (rxstbc > 0 && rxstbc <= 7)
+   printf(",RxSTBC %d stream", rxstbc);
+
+   /* beamforming */
+   if (vhtcaps & IEEE80211_VHTCAP_SU_BEAMFORMER) {
+   printf(",beamformer");
+   num_sts = ((vhtcaps & IEEE80211_VHTCAP_NUM_STS_MASK) >>
+   IEEE80211_VHTCAP_NUM_STS_SHIFT);
+   if (num_sts)
+   printf(" STS %u", num_sts);
+   }
+   if (vhtcaps & IEEE80211_VHTCAP_SU_BEAMFORMEE) {
+   printf(",beamformee");
+   num_sts = ((vhtcaps & IEEE80211_VHTCAP_BEAMFORMEE_STS_MASK) >>
+   IEEE80211_VHTCAP_BEAMFORMEE_STS_SHIFT);
+   if (num_sts)
+   printf(" STS %u", num_sts);
+   }
+
+   if (vhtcaps & IEEE80211_VHTCAP_TXOP_PS)
+   printf(",TXOP PS");
+   if (vhtcaps & IEEE80211_VHTCAP_HTC_VHT)
+   printf(",+HTC VHT");
+
+   /* max A-MPDU length */
+   max_ampdu = ((vhtcaps & IEEE80211_VHTCAP_MAX_AMPDU_LEN_MASK) >>
+   IEEE80211_VHTCAP_MAX_AMPDU_LEN_SHIFT);
+   if (max_ampdu >= IEEE80211_VHTCAP_MAX_AMPDU_LEN_8K &&
+   max_ampdu <= IEEE80211_VHTCAP_MAX_AMPDU_LEN_1024K)

Re: network drivers' manual page consistency

2022-03-17 Thread Jason McIntyre
On Thu, Mar 17, 2022 at 09:17:04AM +, Miod Vallat wrote:
> I have noticed that a few manual page for network drivers mention the
> duplex options in square brackets. This is inconsistent with the vast
> majority of the network drivers' manual pages, so let's homogeneize
> this.
> 

hi.

i'm fine with this. the pages for the wired interfaces do have some
inconsistencies in how they're displayed, so i welcome any attempt to
clean it up.

jmc

> Index: bse.4
> ===
> RCS file: /OpenBSD/src/share/man/man4/bse.4,v
> retrieving revision 1.5
> diff -u -p -r1.5 bse.4
> --- bse.4 8 Sep 2021 20:29:21 -   1.5
> +++ bse.4 17 Mar 2022 09:14:08 -
> @@ -37,19 +37,19 @@ driver supports several media types, whi
>  command.
>  The supported media types are:
>  .Bl -tag -width "media" -offset indent
> -.It media autoselect
> +.It Cm media No autoselect
>  Attempt to autoselect the media type (default)
> -.It media 1000baseT mediaopt full-duplex
> +.It Cm media No 1000baseT Cm mediaopt No full-duplex
>  Use 1000baseT on copper, full duplex
> -.It media 1000baseT Op mediaopt half-duplex
> +.It Cm media No 1000baseT Cm mediaopt No half-duplex
>  Use 1000baseT on copper, half duplex
> -.It media 100baseTX  mediaopt full-duplex
> +.It Cm media No 100baseTX Cm mediaopt No full-duplex
>  Use 100baseTX, full duplex
> -.It media 100baseTX Op mediaopt half-duplex
> +.It Cm media No 100baseTX Cm mediaopt No half-duplex
>  Use 100baseTX, half duplex
> -.It media 10baseT  mediaopt full-duplex
> +.It Cm media No 10baseT Cm mediaopt No full-duplex
>  Use 10baseT, full duplex
> -.It media 10baseT Op mediaopt half-duplex
> +.It Cm media No 10baseT Cm mediaopt No half-duplex
>  Use 10baseT, half duplex
>  .El
>  .Sh SEE ALSO
> Index: cas.4
> ===
> RCS file: /OpenBSD/src/share/man/man4/cas.4,v
> retrieving revision 1.9
> diff -u -p -r1.9 cas.4
> --- cas.4 8 Sep 2021 20:29:21 -   1.9
> +++ cas.4 17 Mar 2022 09:14:08 -
> @@ -54,23 +54,23 @@ driver supports several media types, whi
>  command.
>  The supported media types are:
>  .Bl -tag -width "media" -offset indent
> -.It media autoselect
> +.It Cm media No autoselect
>  Attempt to autoselect the media type (default)
> -.It media 1000baseT mediaopt full-duplex
> +.It Cm media No 1000baseT Cm mediaopt No full-duplex
>  Use 1000baseT on copper, full duplex
> -.It media 1000baseT Op mediaopt half-duplex
> +.It Cm media No 1000baseT Cm mediaopt No half-duplex
>  Use 1000baseT on copper, half duplex
> -.It media 1000baseSX  mediaopt full-duplex
> +.It Cm media No 1000baseSX Cm mediaopt No full-duplex
>  Use 1000baseSX on fiber, full duplex
> -.It media 1000baseSX Op mediaopt half-duplex
> +.It Cm media No 1000baseSX Cm mediaopt No half-duplex
>  Use 1000baseSX on fiber, half duplex
> -.It media 100baseTX  mediaopt full-duplex
> +.It Cm media No 100baseTX Cm mediaopt No full-duplex
>  Use 100baseTX, full duplex
> -.It media 100baseTX Op mediaopt half-duplex
> +.It Cm media No 100baseTX Cm mediaopt No half-duplex
>  Use 100baseTX, half duplex
> -.It media 10baseT mediaopt full-duplex
> +.It Cm media No 10baseT Cm mediaopt No full-duplex
>  Use 10baseT, full duplex
> -.It media 10baseT Op mediaopt half-duplex
> +.It Cm media No 10baseT Cm mediaopt No half-duplex
>  Use 10baseT, half duplex
>  .El
>  .Sh SEE ALSO
> Index: dwge.4
> ===
> RCS file: /OpenBSD/src/share/man/man4/dwge.4,v
> retrieving revision 1.5
> diff -u -p -r1.5 dwge.4
> --- dwge.48 Sep 2021 20:29:21 -   1.5
> +++ dwge.417 Mar 2022 09:14:08 -
> @@ -38,15 +38,15 @@ driver supports several media types, whi
>  command.
>  The supported media types are:
>  .Bl -tag -width "media" -offset indent
> -.It media autoselect
> +.It Cm media No autoselect
>  Attempt to autoselect the media type (default)
> -.It media 1000baseT mediaopt full-duplex
> +.It Cm media No 1000baseT Cm mediaopt No full-duplex
>  Use 1000baseT on copper, full duplex
> -.It media 1000baseT Op mediaopt half-duplex
> +.It Cm media No 1000baseT Cm mediaopt No half-duplex
>  Use 1000baseT on copper, half duplex
> -.It media 100baseTX  mediaopt full-duplex
> +.It Cm media No 100baseTX Cm mediaopt No full-duplex
>  Use 100baseTX, full duplex
> -.It media 100baseTX Op mediaopt half-duplex
> +.It Cm media No 100baseTX Cm mediaopt No half-duplex
>  Use 100baseTX, half duplex
>  .El
>  .Sh SEE ALSO
> Index: dwxe.4
> ===
> RCS file: /OpenBSD/src/share/man/man4/dwxe.4,v
> retrieving revision 1.2
> diff -u -p -r1.2 dwxe.4
> --- dwxe.48 Sep 2021 20:29:21 -   1.2
> +++ dwxe.417 Mar 2022 09:14:08 -
> @@ -37,15 +37,15 @@ driver supports several media types, whi
>  command.
>  The supported media types are:
>  .Bl -tag -width "media" -offset indent
> -.It 

Re: pcb mutex userland

2022-03-17 Thread Alexander Bluhm
On Thu, Mar 17, 2022 at 08:24:10AM +0100, Claudio Jeker wrote:
> On Thu, Mar 17, 2022 at 12:47:15AM +0100, Alexander Bluhm wrote:
> > Hi,
> > 
> > My previous atempt to add a mutex to in_pcb.h was reverted as it
> > broke userland build.
> > 
> > Is the correct fix to include sys/mutex.h in every .c file that
> > includes netinet/in_pcb.h ?  I made a release with it.
> > Or should I include sys/mutex.h in netinet/in_pcb.h ?
> 
> I would add sys/mutex.h in netinet/in_pcb.h. We do the same in other
> headers like sys/proc.h etc.

This survived make release.  It is similar to what we do in sys/proc.h
as suggested by claudio@ and has more #ifdef _KERNEL to please
kettenis@.

ok?

bluhm

Index: sys/netinet/in_pcb.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
retrieving revision 1.125
diff -u -p -r1.125 in_pcb.h
--- sys/netinet/in_pcb.h14 Mar 2022 22:38:43 -  1.125
+++ sys/netinet/in_pcb.h17 Mar 2022 00:44:54 -
@@ -65,6 +65,7 @@
 #define _NETINET_IN_PCB_H_
 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: sys/sys/mutex.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mutex.h,v
retrieving revision 1.18
diff -u -p -r1.18 mutex.h
--- sys/sys/mutex.h 23 Apr 2019 13:35:12 -  1.18
+++ sys/sys/mutex.h 17 Mar 2022 00:44:23 -
@@ -48,6 +48,8 @@ struct mutex {
 #endif
 };
 
+#ifdef _KERNEL
+
 /*
  * To prevent lock ordering problems with the kernel lock, we need to
  * make sure we block all interrupts that can grab the kernel lock.
@@ -148,7 +150,7 @@ void_mtx_init_flags(struct mutex *, int
 
 #endif /* WITNESS */
 
-#if defined(_KERNEL) && defined(DDB)
+#ifdef DDB
 
 struct db_mutex {
struct cpu_info *mtx_owner;
@@ -160,6 +162,8 @@ struct db_mutex {
 void   db_mtx_enter(struct db_mutex *);
 void   db_mtx_leave(struct db_mutex *);
 
-#endif /* _KERNEL && DDB */
+#endif /* DDB */
+
+#endif /* _KERNEL */
 
 #endif



Re: fix boot timeout on arm64

2022-03-17 Thread Miod Vallat
> This issue has been fixed somewhat recently in U-Boot.  It only
> happens if you connect a USB keyboard to the machine.  With the
> exception of Apple machines and possibly the Raspberry Pi, I expect
> most people to use a serial console on arm64 (and armv7 and riscv64)
> machines.  And Apple machines will have a fixed U-Boot.
> 
> Note that this bug not only affects the timeout.  It makes loading the
> kernel from disk slow as well.  So we probably should patch the U-Boot
> version we ship and/or update to U-Boot 2022.04 when it is released in
> a few weeks.

That would be a good idea.

> It may still be worth doing something like this, but would a middle
> groun of making it loop something like a 100 times be an option?

Using 10 is already the middle ground between the existing code using
1000 and the pre-1999 behaviour of checking on every iteration, i.e. 1.
Until an U-Boot upgrade is available, using 100 would cause a
delay of 8 seconds between timeout checks.



Re: fix boot timeout on arm64

2022-03-17 Thread Mark Kettenis
> Date: Thu, 17 Mar 2022 11:02:00 +
> From: Miod Vallat 
> 
> By default (with no override in /etc/boot.conf), the arm64 boot loader
> will attempt to boot the kernel after a 5 second timeout.
> 
> On the RPi 4b here, it will indeed boot the kernel, but after about 80
> seconds.
> 
> The reason for this delay is that there is logic in the boot code, while
> waiting for keystrokes, to limit the timeout check to "only once every
> 1000 times the console reports no keystroke". That logic was introduced
> almost 23 years ago to cope with some slow i386 BIOses.
> 
> However, when there is no console activity, the EFI routine to check for
> a keystroke will itself wait for about 80 millisecond before returning
> failure, which in turn causes the timeout check to be performed only
> once every 80 seconds. Bummer.
> 
> The following diff introduces a compile-time symbol to reduce the loop
> counter from 1000 to 10, and enables this on arm64. This results in
> accurate timeout processing, and the system correctly boots after 5
> seconds of idleness.
> 
> That change might be needed on some other systems (armv7, riscv64?) as
> well.

This issue has been fixed somewhat recently in U-Boot.  It only
happens if you connect a USB keyboard to the machine.  With the
exception of Apple machines and possibly the Raspberry Pi, I expect
most people to use a serial console on arm64 (and armv7 and riscv64)
machines.  And Apple machines will have a fixed U-Boot.

Note that this bug not only affects the timeout.  It makes loading the
kernel from disk slow as well.  So we probably should patch the U-Boot
version we ship and/or update to U-Boot 2022.04 when it is released in
a few weeks.

It may still be worth doing something like this, but would a middle
groun of making it loop something like a 100 times be an option?


> Index: arch/arm64/stand/efiboot/Makefile
> ===
> RCS file: /OpenBSD/src/sys/arch/arm64/stand/efiboot/Makefile,v
> retrieving revision 1.15
> diff -u -p -r1.15 Makefile
> --- arch/arm64/stand/efiboot/Makefile 14 Mar 2022 19:09:32 -  1.15
> +++ arch/arm64/stand/efiboot/Makefile 17 Mar 2022 10:53:34 -
> @@ -49,6 +49,7 @@ CPPFLAGS+=  -I${EFIDIR}/include -I${EFIDI
>  CPPFLAGS+=   -D_STANDALONE
>  CPPFLAGS+=   -DSMALL -DSLOW -DNOBYFOUR -D__INTERNAL_LIBSA_CREAD
>  CPPFLAGS+=   -DNEEDS_HEAP_H -DMDRANDOM -DFWRANDOM
> +CPPFLAGS+=   -DSLOW_CNISCHAR
>  COPTS+=  -Wno-attributes -Wno-format
>  COPTS+=  -ffreestanding -fno-stack-protector
>  COPTS+=  -fshort-wchar -fPIC -fno-builtin
> Index: arch/arm64/stand/efiboot/conf.c
> ===
> RCS file: /OpenBSD/src/sys/arch/arm64/stand/efiboot/conf.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 conf.c
> --- arch/arm64/stand/efiboot/conf.c   14 Mar 2022 19:09:32 -  1.36
> +++ arch/arm64/stand/efiboot/conf.c   17 Mar 2022 10:53:34 -
> @@ -46,7 +46,7 @@
>  #include "efipxe.h"
>  #include "softraid_arm64.h"
>  
> -const char version[] = "1.8";
> +const char version[] = "1.9";
>  int  debug = 0;
>  
>  struct fs_ops file_system[] = {
> Index: stand/boot/cmd.c
> ===
> RCS file: /OpenBSD/src/sys/stand/boot/cmd.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 cmd.c
> --- stand/boot/cmd.c  24 Oct 2021 17:49:19 -  1.68
> +++ stand/boot/cmd.c  17 Mar 2022 10:53:35 -
> @@ -238,6 +238,18 @@ whatcmd(const struct cmd_table **ct, cha
>   return q;
>  }
>  
> +/*
> + * If there is a timeout, we want to honour it as best as possible, but
> + * the getsecs() call might be expensive, so we don't want to check for
> + * timeout too frequently... unless cnischar() itself takes a long time
> + * to report no activity.
> + */
> +#ifdef SLOW_CNISCHAR
> +#define  TIMEOUT_LOOP10
> +#else
> +#define  TIMEOUT_LOOP1000
> +#endif
> +
>  static int
>  readline(char *buf, size_t n, int to)
>  {
> @@ -254,10 +266,9 @@ readline(char *buf, size_t n, int to)
>   if (debug > 2)
>   printf ("readline: timeout(%d) at %u\n", to, tt);
>  #endif
> - /* check for timeout expiration less often
> -(for some very constrained archs) */
> + /* Check for timeout expiration */
>   while (!cnischar())
> - if (!(i++ % 1000) && (getsecs() >= tt))
> + if (!(i++ % TIMEOUT_LOOP) && (getsecs() >= tt))
>   break;
>  
>   if (!cnischar()) {
> 
> 



fix boot timeout on arm64

2022-03-17 Thread Miod Vallat
By default (with no override in /etc/boot.conf), the arm64 boot loader
will attempt to boot the kernel after a 5 second timeout.

On the RPi 4b here, it will indeed boot the kernel, but after about 80
seconds.

The reason for this delay is that there is logic in the boot code, while
waiting for keystrokes, to limit the timeout check to "only once every
1000 times the console reports no keystroke". That logic was introduced
almost 23 years ago to cope with some slow i386 BIOses.

However, when there is no console activity, the EFI routine to check for
a keystroke will itself wait for about 80 millisecond before returning
failure, which in turn causes the timeout check to be performed only
once every 80 seconds. Bummer.

The following diff introduces a compile-time symbol to reduce the loop
counter from 1000 to 10, and enables this on arm64. This results in
accurate timeout processing, and the system correctly boots after 5
seconds of idleness.

That change might be needed on some other systems (armv7, riscv64?) as
well.

Index: arch/arm64/stand/efiboot/Makefile
===
RCS file: /OpenBSD/src/sys/arch/arm64/stand/efiboot/Makefile,v
retrieving revision 1.15
diff -u -p -r1.15 Makefile
--- arch/arm64/stand/efiboot/Makefile   14 Mar 2022 19:09:32 -  1.15
+++ arch/arm64/stand/efiboot/Makefile   17 Mar 2022 10:53:34 -
@@ -49,6 +49,7 @@ CPPFLAGS+=-I${EFIDIR}/include -I${EFIDI
 CPPFLAGS+= -D_STANDALONE
 CPPFLAGS+= -DSMALL -DSLOW -DNOBYFOUR -D__INTERNAL_LIBSA_CREAD
 CPPFLAGS+= -DNEEDS_HEAP_H -DMDRANDOM -DFWRANDOM
+CPPFLAGS+= -DSLOW_CNISCHAR
 COPTS+=-Wno-attributes -Wno-format
 COPTS+=-ffreestanding -fno-stack-protector
 COPTS+=-fshort-wchar -fPIC -fno-builtin
Index: arch/arm64/stand/efiboot/conf.c
===
RCS file: /OpenBSD/src/sys/arch/arm64/stand/efiboot/conf.c,v
retrieving revision 1.36
diff -u -p -r1.36 conf.c
--- arch/arm64/stand/efiboot/conf.c 14 Mar 2022 19:09:32 -  1.36
+++ arch/arm64/stand/efiboot/conf.c 17 Mar 2022 10:53:34 -
@@ -46,7 +46,7 @@
 #include "efipxe.h"
 #include "softraid_arm64.h"
 
-const char version[] = "1.8";
+const char version[] = "1.9";
 intdebug = 0;
 
 struct fs_ops file_system[] = {
Index: stand/boot/cmd.c
===
RCS file: /OpenBSD/src/sys/stand/boot/cmd.c,v
retrieving revision 1.68
diff -u -p -r1.68 cmd.c
--- stand/boot/cmd.c24 Oct 2021 17:49:19 -  1.68
+++ stand/boot/cmd.c17 Mar 2022 10:53:35 -
@@ -238,6 +238,18 @@ whatcmd(const struct cmd_table **ct, cha
return q;
 }
 
+/*
+ * If there is a timeout, we want to honour it as best as possible, but
+ * the getsecs() call might be expensive, so we don't want to check for
+ * timeout too frequently... unless cnischar() itself takes a long time
+ * to report no activity.
+ */
+#ifdef SLOW_CNISCHAR
+#defineTIMEOUT_LOOP10
+#else
+#defineTIMEOUT_LOOP1000
+#endif
+
 static int
 readline(char *buf, size_t n, int to)
 {
@@ -254,10 +266,9 @@ readline(char *buf, size_t n, int to)
if (debug > 2)
printf ("readline: timeout(%d) at %u\n", to, tt);
 #endif
-   /* check for timeout expiration less often
-  (for some very constrained archs) */
+   /* Check for timeout expiration */
while (!cnischar())
-   if (!(i++ % 1000) && (getsecs() >= tt))
+   if (!(i++ % TIMEOUT_LOOP) && (getsecs() >= tt))
break;
 
if (!cnischar()) {



Re: sdmmc: simplify devlist2h

2022-03-17 Thread Jonathan Gray
On Thu, Mar 17, 2022 at 08:03:20AM +, Miod Vallat wrote:
> sys/dev/sdmmc/devlist2h.awk was based upon sys/dev/pcmcia/devlist2h.awk.
> The latter contains code to define optional CIS tuple overrides, which
> are not used in sdmmc - there is only one override and it is applied in
> sdmmc_check_cis_quirks().
> 
> The following diff removes this feature from devlist2h. As a result,
> there will no longer be SDMMC_CIS_* defines in sdmmcdevs.h.

ok jsg@

> 
> Index: devlist2h.awk
> ===
> RCS file: /OpenBSD/src/sys/dev/sdmmc/devlist2h.awk,v
> retrieving revision 1.2
> diff -u -p -r1.2 devlist2h.awk
> --- devlist2h.awk 2 Jun 2006 21:16:44 -   1.2
> +++ devlist2h.awk 17 Mar 2022 07:59:44 -
> @@ -80,7 +80,6 @@ NR == 1 {
>  $1 == "vendor" {
>   nvendors++
>  
> - vendorindex[$2] = nvendors; # record index for this name, 
> for later.
>   vendors[nvendors, 1] = $2;  # name
>   vendors[nvendors, 2] = $3;  # id
>   printf("#define\tSDMMC_VENDOR_%s\t%s\t", vendors[nvendors, 1],
> @@ -95,45 +94,8 @@ $1 == "product" {
>   products[nproducts, 1] = $2;# vendor name
>   products[nproducts, 2] = $3;# product id
>   products[nproducts, 3] = $4;# id
> -
> - f = 5;
> -
> - if ($4 == "{") {
> - products[nproducts, 3] = "SDMMC_PRODUCT_INVALID"
> - z = "{ "
> - for (i = 0; i < 4; i++) {
> - if (f <= NF) {
> - gsub("", " ", $f)
> - gsub("", "\t", $f)
> - gsub("", "\n", $f)
> - z = z $f " "
> - f++
> - }
> - else {
> - if (i == 3)
> - z = z "NULL "
> - else
> - z = z "NULL, "
> - }
> - }
> - products[nproducts, 4] = z $f
> - f++
> - }
> - else {
> - products[nproducts, 4] = "{ NULL, NULL, NULL, NULL }"
> - }
> - printf("#define\tSDMMC_CIS_%s_%s\t%s\n",
> - products[nproducts, 1], products[nproducts, 2],
> - products[nproducts, 4]) > hfile
>   printf("#define\tSDMMC_PRODUCT_%s_%s\t%s\n", products[nproducts, 1],
>   products[nproducts, 2], products[nproducts, 3]) > hfile
> -
> -#products[nproducts, 5] = collectline(f, line)
> -#
> -#printf("#define\tSDMMC_STR_%s_%s\t\"%s\"\n",
> -#products[nproducts, 1], products[nproducts, 2],
> -#products[nproducts, 5]) > hfile
> -
>   next
>  }
>  {
> 
> 



Re: bwfm@sdmmc: use symbolic constants for matching

2022-03-17 Thread Jonathan Gray
On Thu, Mar 17, 2022 at 08:05:38AM +, Miod Vallat wrote:
> The following diff declares the various devices bwfm@sdmmc checks for,
> and introduces no functional change.
> 
> Index: if_bwfm_sdio.c
> ===
> RCS file: /OpenBSD/src/sys/dev/sdmmc/if_bwfm_sdio.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 if_bwfm_sdio.c
> --- if_bwfm_sdio.c2 Nov 2021 14:49:53 -   1.42
> +++ if_bwfm_sdio.c17 Mar 2022 07:59:57 -
> @@ -45,6 +45,7 @@
>  
>  #include 
>  
> +#include 
>  #include 
>  
>  #include 
> @@ -207,27 +208,27 @@ bwfm_sdio_match(struct device *parent, v
>  
>   /* Look for Broadcom. */
>   cis = >sc->sc_fn0->cis;
> - if (cis->manufacturer != 0x02d0)
> + if (cis->manufacturer != SDMMC_VENDOR_BROADCOM)
>   return 0;
>  
>   /* Look for supported chips. */
>   switch (cis->product) {
> - case 0x4324:
> - case 0x4330:
> - case 0x4334:
> - case 0x4329:
> - case 0x4335:
> - case 0x4339:
> - case 0x4345:
> - case 0x4354:
> - case 0x4356:
> - case 0x4359:
> - case 0xa887:/* BCM43143 */
> - case 0xa94c:/* BCM43340 */
> - case 0xa94d:/* BCM43341 */
> - case 0xa962:/* BCM43362 */
> - case 0xa9a6:/* BCM43430 */
> - case 0xa9bf:/* BCM43364 */
> + case SDMMC_PRODUCT_BROADCOM_BCM4324:
> + case SDMMC_PRODUCT_BROADCOM_BCM4329:
> + case SDMMC_PRODUCT_BROADCOM_BCM4330:
> + case SDMMC_PRODUCT_BROADCOM_BCM4334:
> + case SDMMC_PRODUCT_BROADCOM_BCM4335:
> + case SDMMC_PRODUCT_BROADCOM_BCM4339:
> + case SDMMC_PRODUCT_BROADCOM_BCM4345:
> + case SDMMC_PRODUCT_BROADCOM_BCM4354:
> + case SDMMC_PRODUCT_BROADCOM_BCM4356:
> + case SDMMC_PRODUCT_BROADCOM_BCM4359:
> + case SDMMC_PRODUCT_BROADCOM_BCM43143:
> + case SDMMC_PRODUCT_BROADCOM_BCM43340:
> + case SDMMC_PRODUCT_BROADCOM_BCM43341:
> + case SDMMC_PRODUCT_BROADCOM_BCM43362:
> + case SDMMC_PRODUCT_BROADCOM_BCM43430:
> + case SDMMC_PRODUCT_BROADCOM_BCM43364:
>   break;
>   default:
>   return 0;
> Index: sdmmcdevs
> ===
> RCS file: /OpenBSD/src/sys/dev/sdmmc/sdmmcdevs,v
> retrieving revision 1.8
> diff -u -p -r1.8 sdmmcdevs
> --- sdmmcdevs 11 May 2007 17:16:16 -  1.8
> +++ sdmmcdevs 17 Mar 2022 07:59:57 -
> @@ -24,6 +24,7 @@ vendor CGUYS0x0092  C-guys, Inc.
>  vendor TOSHIBA   0x0098  Toshiba
>  vendor SOCKETCOM 0x0104  Socket Communications, Inc.
>  vendor ATHEROS   0x0271  Atheros
> +vendor BROADCOM  0x02d0  Broadcom
>  vendor SYCHIP0x02db  SyChip Inc.
>  vendor SPECTEC   0x02fe  Spectec Computer Co., Ltd
>  vendor GLOBALSAT 0x0501  Globalsat Technology Co.
> @@ -42,6 +43,24 @@ product ATHEROSAR6001_80x0108  AR6001
>  product ATHEROS  AR6001_90x0109  AR6001
>  product ATHEROS  AR6001_a0x010a  AR6001
>  product ATHEROS  AR6001_b0x010b  AR6001
> +
> +/* Broadcom */
> +product  BROADCOM BCM43240x4324  BCM4324
> +product  BROADCOM BCM43290x4329  BCM4329

the whitespace in your broadcom additions is different

 product ATHEROS^IAR6001_b^I0x010b^IAR6001$
+$
+/* Broadcom */$
+product^IBROADCOM BCM4324^I0x4324^IBCM4324$
+product^IBROADCOM BCM4329^I0x4329^IBCM4329$

pcidevs and usbdevs are product VENDOR DEVICE\t0x1234\tDevice

ok jsg@ if that is fixed

the existing atheros entries also get this wrong

> +product  BROADCOM BCM43300x4330  BCM4330
> +product  BROADCOM BCM43340x4334  BCM4334
> +product  BROADCOM BCM43350x4335  BCM4335
> +product  BROADCOM BCM43390x4339  BCM4339
> +product  BROADCOM BCM43450x4345  BCM4345
> +product  BROADCOM BCM43540x4354  BCM4354
> +product  BROADCOM BCM43560x4356  BCM4356
> +product  BROADCOM BCM43590x4359  BCM4359
> +product  BROADCOM BCM43143   0xa887  BCM43143
> +product  BROADCOM BCM43340   0xa94c  BCM43340
> +product  BROADCOM BCM43341   0xa94d  BCM43341
> +product  BROADCOM BCM43362   0xa962  BCM43362
> +product  BROADCOM BCM43430   0xa9a6  BCM43430
> +product  BROADCOM BCM43364   0xa9bf  BCM43364
>  
>  /* C-guys, Inc. */
>  product CGUYS TIACX100   0x0001  TI ACX100 SD-Link11b WiFi Card
> 
> 



network drivers' manual page consistency

2022-03-17 Thread Miod Vallat
I have noticed that a few manual page for network drivers mention the
duplex options in square brackets. This is inconsistent with the vast
majority of the network drivers' manual pages, so let's homogeneize
this.

Index: bse.4
===
RCS file: /OpenBSD/src/share/man/man4/bse.4,v
retrieving revision 1.5
diff -u -p -r1.5 bse.4
--- bse.4   8 Sep 2021 20:29:21 -   1.5
+++ bse.4   17 Mar 2022 09:14:08 -
@@ -37,19 +37,19 @@ driver supports several media types, whi
 command.
 The supported media types are:
 .Bl -tag -width "media" -offset indent
-.It media autoselect
+.It Cm media No autoselect
 Attempt to autoselect the media type (default)
-.It media 1000baseT mediaopt full-duplex
+.It Cm media No 1000baseT Cm mediaopt No full-duplex
 Use 1000baseT on copper, full duplex
-.It media 1000baseT Op mediaopt half-duplex
+.It Cm media No 1000baseT Cm mediaopt No half-duplex
 Use 1000baseT on copper, half duplex
-.It media 100baseTX  mediaopt full-duplex
+.It Cm media No 100baseTX Cm mediaopt No full-duplex
 Use 100baseTX, full duplex
-.It media 100baseTX Op mediaopt half-duplex
+.It Cm media No 100baseTX Cm mediaopt No half-duplex
 Use 100baseTX, half duplex
-.It media 10baseT  mediaopt full-duplex
+.It Cm media No 10baseT Cm mediaopt No full-duplex
 Use 10baseT, full duplex
-.It media 10baseT Op mediaopt half-duplex
+.It Cm media No 10baseT Cm mediaopt No half-duplex
 Use 10baseT, half duplex
 .El
 .Sh SEE ALSO
Index: cas.4
===
RCS file: /OpenBSD/src/share/man/man4/cas.4,v
retrieving revision 1.9
diff -u -p -r1.9 cas.4
--- cas.4   8 Sep 2021 20:29:21 -   1.9
+++ cas.4   17 Mar 2022 09:14:08 -
@@ -54,23 +54,23 @@ driver supports several media types, whi
 command.
 The supported media types are:
 .Bl -tag -width "media" -offset indent
-.It media autoselect
+.It Cm media No autoselect
 Attempt to autoselect the media type (default)
-.It media 1000baseT mediaopt full-duplex
+.It Cm media No 1000baseT Cm mediaopt No full-duplex
 Use 1000baseT on copper, full duplex
-.It media 1000baseT Op mediaopt half-duplex
+.It Cm media No 1000baseT Cm mediaopt No half-duplex
 Use 1000baseT on copper, half duplex
-.It media 1000baseSX  mediaopt full-duplex
+.It Cm media No 1000baseSX Cm mediaopt No full-duplex
 Use 1000baseSX on fiber, full duplex
-.It media 1000baseSX Op mediaopt half-duplex
+.It Cm media No 1000baseSX Cm mediaopt No half-duplex
 Use 1000baseSX on fiber, half duplex
-.It media 100baseTX  mediaopt full-duplex
+.It Cm media No 100baseTX Cm mediaopt No full-duplex
 Use 100baseTX, full duplex
-.It media 100baseTX Op mediaopt half-duplex
+.It Cm media No 100baseTX Cm mediaopt No half-duplex
 Use 100baseTX, half duplex
-.It media 10baseT mediaopt full-duplex
+.It Cm media No 10baseT Cm mediaopt No full-duplex
 Use 10baseT, full duplex
-.It media 10baseT Op mediaopt half-duplex
+.It Cm media No 10baseT Cm mediaopt No half-duplex
 Use 10baseT, half duplex
 .El
 .Sh SEE ALSO
Index: dwge.4
===
RCS file: /OpenBSD/src/share/man/man4/dwge.4,v
retrieving revision 1.5
diff -u -p -r1.5 dwge.4
--- dwge.4  8 Sep 2021 20:29:21 -   1.5
+++ dwge.4  17 Mar 2022 09:14:08 -
@@ -38,15 +38,15 @@ driver supports several media types, whi
 command.
 The supported media types are:
 .Bl -tag -width "media" -offset indent
-.It media autoselect
+.It Cm media No autoselect
 Attempt to autoselect the media type (default)
-.It media 1000baseT mediaopt full-duplex
+.It Cm media No 1000baseT Cm mediaopt No full-duplex
 Use 1000baseT on copper, full duplex
-.It media 1000baseT Op mediaopt half-duplex
+.It Cm media No 1000baseT Cm mediaopt No half-duplex
 Use 1000baseT on copper, half duplex
-.It media 100baseTX  mediaopt full-duplex
+.It Cm media No 100baseTX Cm mediaopt No full-duplex
 Use 100baseTX, full duplex
-.It media 100baseTX Op mediaopt half-duplex
+.It Cm media No 100baseTX Cm mediaopt No half-duplex
 Use 100baseTX, half duplex
 .El
 .Sh SEE ALSO
Index: dwxe.4
===
RCS file: /OpenBSD/src/share/man/man4/dwxe.4,v
retrieving revision 1.2
diff -u -p -r1.2 dwxe.4
--- dwxe.4  8 Sep 2021 20:29:21 -   1.2
+++ dwxe.4  17 Mar 2022 09:14:08 -
@@ -37,15 +37,15 @@ driver supports several media types, whi
 command.
 The supported media types are:
 .Bl -tag -width "media" -offset indent
-.It media autoselect
+.It Cm media No autoselect
 Attempt to autoselect the media type (default)
-.It media 1000baseT mediaopt full-duplex
+.It Cm media No 1000baseT Cm mediaopt No full-duplex
 Use 1000baseT on copper, full duplex
-.It media 1000baseT Op mediaopt half-duplex
+.It Cm media No 1000baseT Cm mediaopt No half-duplex
 Use 1000baseT on copper, half duplex
-.It media 100baseTX  mediaopt full-duplex
+.It Cm media No 100baseTX Cm mediaopt No 

Re: Remove data dependency barrier from atomic_load_*

2022-03-17 Thread Alexander Bluhm
On Thu, Mar 17, 2022 at 07:12:16AM +, Visa Hankala wrote:
> On Wed, Mar 16, 2022 at 11:09:12PM +0100, Alexander Bluhm wrote:
> > On Tue, Mar 15, 2022 at 09:15:34AM +, Visa Hankala wrote:
> > > However, some DEC Alpha CPUs have their data caches divided into cache
> > > banks to improve bandwidth. These cache banks are relatively
> > > independent. The system maintains coherency, but bus contention can
> > > delay propagation of cache updates. If the loads spanned different cache
> > > banks, the second load could deliver data which is older than the
> > > initial load's value. The data dependency barrier causes an interlock
> > > with cache updating, ensuring causal ordering.)
> > 
> > The code with the membar is copied from READ_ONCE() which is copied
> > from Linux.  The membar_datadep_consumer() has an #ifdef __alpha__
> > in it.  It is only used for that case.  I don't know whether we
> > want to support such CPU.  But if that is the case, we need the
> > membar.
> 
> Whether the membar is necessary or not depends on the use case.
> READ_ONCE(), and SMR_PTR_GET(), have it built in so that loaded
> pointers would work in the expected way in lockless contexts. This
> is intentional, the membar has not been just copied there.

With that explanation OK bluhm@ to remove the membar.



bwfm@sdmmc: use symbolic constants for matching

2022-03-17 Thread Miod Vallat
The following diff declares the various devices bwfm@sdmmc checks for,
and introduces no functional change.

Index: if_bwfm_sdio.c
===
RCS file: /OpenBSD/src/sys/dev/sdmmc/if_bwfm_sdio.c,v
retrieving revision 1.42
diff -u -p -r1.42 if_bwfm_sdio.c
--- if_bwfm_sdio.c  2 Nov 2021 14:49:53 -   1.42
+++ if_bwfm_sdio.c  17 Mar 2022 07:59:57 -
@@ -45,6 +45,7 @@
 
 #include 
 
+#include 
 #include 
 
 #include 
@@ -207,27 +208,27 @@ bwfm_sdio_match(struct device *parent, v
 
/* Look for Broadcom. */
cis = >sc->sc_fn0->cis;
-   if (cis->manufacturer != 0x02d0)
+   if (cis->manufacturer != SDMMC_VENDOR_BROADCOM)
return 0;
 
/* Look for supported chips. */
switch (cis->product) {
-   case 0x4324:
-   case 0x4330:
-   case 0x4334:
-   case 0x4329:
-   case 0x4335:
-   case 0x4339:
-   case 0x4345:
-   case 0x4354:
-   case 0x4356:
-   case 0x4359:
-   case 0xa887:/* BCM43143 */
-   case 0xa94c:/* BCM43340 */
-   case 0xa94d:/* BCM43341 */
-   case 0xa962:/* BCM43362 */
-   case 0xa9a6:/* BCM43430 */
-   case 0xa9bf:/* BCM43364 */
+   case SDMMC_PRODUCT_BROADCOM_BCM4324:
+   case SDMMC_PRODUCT_BROADCOM_BCM4329:
+   case SDMMC_PRODUCT_BROADCOM_BCM4330:
+   case SDMMC_PRODUCT_BROADCOM_BCM4334:
+   case SDMMC_PRODUCT_BROADCOM_BCM4335:
+   case SDMMC_PRODUCT_BROADCOM_BCM4339:
+   case SDMMC_PRODUCT_BROADCOM_BCM4345:
+   case SDMMC_PRODUCT_BROADCOM_BCM4354:
+   case SDMMC_PRODUCT_BROADCOM_BCM4356:
+   case SDMMC_PRODUCT_BROADCOM_BCM4359:
+   case SDMMC_PRODUCT_BROADCOM_BCM43143:
+   case SDMMC_PRODUCT_BROADCOM_BCM43340:
+   case SDMMC_PRODUCT_BROADCOM_BCM43341:
+   case SDMMC_PRODUCT_BROADCOM_BCM43362:
+   case SDMMC_PRODUCT_BROADCOM_BCM43430:
+   case SDMMC_PRODUCT_BROADCOM_BCM43364:
break;
default:
return 0;
Index: sdmmcdevs
===
RCS file: /OpenBSD/src/sys/dev/sdmmc/sdmmcdevs,v
retrieving revision 1.8
diff -u -p -r1.8 sdmmcdevs
--- sdmmcdevs   11 May 2007 17:16:16 -  1.8
+++ sdmmcdevs   17 Mar 2022 07:59:57 -
@@ -24,6 +24,7 @@ vendor CGUYS  0x0092  C-guys, Inc.
 vendor TOSHIBA 0x0098  Toshiba
 vendor SOCKETCOM   0x0104  Socket Communications, Inc.
 vendor ATHEROS 0x0271  Atheros
+vendor BROADCOM0x02d0  Broadcom
 vendor SYCHIP  0x02db  SyChip Inc.
 vendor SPECTEC 0x02fe  Spectec Computer Co., Ltd
 vendor GLOBALSAT   0x0501  Globalsat Technology Co.
@@ -42,6 +43,24 @@ product ATHEROS  AR6001_80x0108  AR6001
 product ATHEROSAR6001_90x0109  AR6001
 product ATHEROSAR6001_a0x010a  AR6001
 product ATHEROSAR6001_b0x010b  AR6001
+
+/* Broadcom */
+productBROADCOM BCM43240x4324  BCM4324
+productBROADCOM BCM43290x4329  BCM4329
+productBROADCOM BCM43300x4330  BCM4330
+productBROADCOM BCM43340x4334  BCM4334
+productBROADCOM BCM43350x4335  BCM4335
+productBROADCOM BCM43390x4339  BCM4339
+productBROADCOM BCM43450x4345  BCM4345
+productBROADCOM BCM43540x4354  BCM4354
+productBROADCOM BCM43560x4356  BCM4356
+productBROADCOM BCM43590x4359  BCM4359
+productBROADCOM BCM43143   0xa887  BCM43143
+productBROADCOM BCM43340   0xa94c  BCM43340
+productBROADCOM BCM43341   0xa94d  BCM43341
+productBROADCOM BCM43362   0xa962  BCM43362
+productBROADCOM BCM43430   0xa9a6  BCM43430
+productBROADCOM BCM43364   0xa9bf  BCM43364
 
 /* C-guys, Inc. */
 product CGUYS TIACX100 0x0001  TI ACX100 SD-Link11b WiFi Card



sdmmc: simplify devlist2h

2022-03-17 Thread Miod Vallat
sys/dev/sdmmc/devlist2h.awk was based upon sys/dev/pcmcia/devlist2h.awk.
The latter contains code to define optional CIS tuple overrides, which
are not used in sdmmc - there is only one override and it is applied in
sdmmc_check_cis_quirks().

The following diff removes this feature from devlist2h. As a result,
there will no longer be SDMMC_CIS_* defines in sdmmcdevs.h.

Index: devlist2h.awk
===
RCS file: /OpenBSD/src/sys/dev/sdmmc/devlist2h.awk,v
retrieving revision 1.2
diff -u -p -r1.2 devlist2h.awk
--- devlist2h.awk   2 Jun 2006 21:16:44 -   1.2
+++ devlist2h.awk   17 Mar 2022 07:59:44 -
@@ -80,7 +80,6 @@ NR == 1 {
 $1 == "vendor" {
nvendors++
 
-   vendorindex[$2] = nvendors; # record index for this name, 
for later.
vendors[nvendors, 1] = $2;  # name
vendors[nvendors, 2] = $3;  # id
printf("#define\tSDMMC_VENDOR_%s\t%s\t", vendors[nvendors, 1],
@@ -95,45 +94,8 @@ $1 == "product" {
products[nproducts, 1] = $2;# vendor name
products[nproducts, 2] = $3;# product id
products[nproducts, 3] = $4;# id
-
-   f = 5;
-
-   if ($4 == "{") {
-   products[nproducts, 3] = "SDMMC_PRODUCT_INVALID"
-   z = "{ "
-   for (i = 0; i < 4; i++) {
-   if (f <= NF) {
-   gsub("", " ", $f)
-   gsub("", "\t", $f)
-   gsub("", "\n", $f)
-   z = z $f " "
-   f++
-   }
-   else {
-   if (i == 3)
-   z = z "NULL "
-   else
-   z = z "NULL, "
-   }
-   }
-   products[nproducts, 4] = z $f
-   f++
-   }
-   else {
-   products[nproducts, 4] = "{ NULL, NULL, NULL, NULL }"
-   }
-   printf("#define\tSDMMC_CIS_%s_%s\t%s\n",
-   products[nproducts, 1], products[nproducts, 2],
-   products[nproducts, 4]) > hfile
printf("#define\tSDMMC_PRODUCT_%s_%s\t%s\n", products[nproducts, 1],
products[nproducts, 2], products[nproducts, 3]) > hfile
-
-#  products[nproducts, 5] = collectline(f, line)
-#
-#  printf("#define\tSDMMC_STR_%s_%s\t\"%s\"\n",
-#  products[nproducts, 1], products[nproducts, 2],
-#  products[nproducts, 5]) > hfile
-
next
 }
 {



Re: refcount btrace

2022-03-17 Thread Visa Hankala
On Thu, Mar 17, 2022 at 12:42:13AM +0100, Alexander Bluhm wrote:
> I would like to use btrace to debug refernce counting.  The idea
> is to a a tracepoint for every type of refcnt we have.  When it
> changes, print the actual object, the current counter and the change
> value.

> Do we want that feature?

I am against this in its current form. The code would become more
complex, and the trace points can affect timing. There is a risk that
the kernel behaves slightly differently when dt has been compiled in.



Re: pcb mutex userland

2022-03-17 Thread Claudio Jeker
On Thu, Mar 17, 2022 at 12:47:15AM +0100, Alexander Bluhm wrote:
> Hi,
> 
> My previous atempt to add a mutex to in_pcb.h was reverted as it
> broke userland build.
> 
> Is the correct fix to include sys/mutex.h in every .c file that
> includes netinet/in_pcb.h ?  I made a release with it.
> Or should I include sys/mutex.h in netinet/in_pcb.h ?

I would add sys/mutex.h in netinet/in_pcb.h. We do the same in other
headers like sys/proc.h etc.
 
> ok?
> 
> bluhm
> 
> Index: lib/libkvm/kvm_file2.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/lib/libkvm/kvm_file2.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 kvm_file2.c
> --- lib/libkvm/kvm_file2.c22 Feb 2022 17:35:01 -  1.57
> +++ lib/libkvm/kvm_file2.c16 Mar 2022 16:42:15 -
> @@ -74,6 +74,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> Index: sbin/sysctl/sysctl.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/sysctl/sysctl.c,v
> retrieving revision 1.258
> diff -u -p -r1.258 sysctl.c
> --- sbin/sysctl/sysctl.c  12 Jul 2021 15:09:19 -  1.258
> +++ sbin/sysctl/sysctl.c  15 Mar 2022 09:18:31 -
> @@ -42,9 +42,11 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> +
>  #include 
>  #include 
>  
> Index: usr.bin/netstat/inet.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/inet.c,v
> retrieving revision 1.173
> diff -u -p -r1.173 inet.c
> --- usr.bin/netstat/inet.c5 Dec 2021 22:36:19 -   1.173
> +++ usr.bin/netstat/inet.c16 Mar 2022 16:44:32 -
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #define _KERNEL
> @@ -41,6 +42,7 @@
>  #undef _KERNEL
>  
>  #include 
> +
>  #include 
>  #include 
>  #include 
> Index: usr.bin/tcpbench/tcpbench.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/tcpbench/tcpbench.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 tcpbench.c
> --- usr.bin/tcpbench/tcpbench.c   12 Jul 2021 15:09:20 -  1.65
> +++ usr.bin/tcpbench/tcpbench.c   16 Mar 2022 16:44:55 -
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> Index: usr.sbin/trpt/trpt.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/trpt/trpt.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 trpt.c
> --- usr.sbin/trpt/trpt.c  2 Dec 2019 21:47:54 -   1.39
> +++ usr.sbin/trpt/trpt.c  16 Mar 2022 16:45:23 -
> @@ -62,6 +62,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #define PRUREQUESTS
>  #include 
>  #define _KERNEL
> 

-- 
:wq Claudio



Re: Remove data dependency barrier from atomic_load_*

2022-03-17 Thread Visa Hankala
On Wed, Mar 16, 2022 at 11:09:12PM +0100, Alexander Bluhm wrote:
> On Tue, Mar 15, 2022 at 09:15:34AM +, Visa Hankala wrote:
> > However, some DEC Alpha CPUs have their data caches divided into cache
> > banks to improve bandwidth. These cache banks are relatively
> > independent. The system maintains coherency, but bus contention can
> > delay propagation of cache updates. If the loads spanned different cache
> > banks, the second load could deliver data which is older than the
> > initial load's value. The data dependency barrier causes an interlock
> > with cache updating, ensuring causal ordering.)
> 
> The code with the membar is copied from READ_ONCE() which is copied
> from Linux.  The membar_datadep_consumer() has an #ifdef __alpha__
> in it.  It is only used for that case.  I don't know whether we
> want to support such CPU.  But if that is the case, we need the
> membar.

Whether the membar is necessary or not depends on the use case.
READ_ONCE(), and SMR_PTR_GET(), have it built in so that loaded
pointers would work in the expected way in lockless contexts. This
is intentional, the membar has not been just copied there.

If you want to keep the memory barrier, then I suggest that the current
atomic_load_* and atomic_store_* functions are replaced with NetBSD's
atomic_load_relaxed(), atomic_load_consume(), atomic_load_acquire(),
atomic_store_relaxed() and atomic_store_release(). With these, it is
clear what ordering the operations provide and the programmer is able
to make the appropriate choice.

> What do you need refcnt_read() for?  Is it only for assert?  Then
> a refcnt_assert() without membar or atomic_load might be better.

I want to keep the API small. Even though refcnt_read() is possibly
dubious in general, it allows a degree of freedom that might be useful
for example in assertions.

The membar question will arise in many places, not just with refcnt.



Re: [External] : ipsec acquire mutex refcount

2022-03-17 Thread Alexandr Nedvedicky
Hello,


> > > +ipsp_delete_acquire_locked(struct ipsec_acquire *ipa)
> > > +{
> > > + if (timeout_del(>ipa_timeout) == 1)
> > > + refcnt_rele(>ipa_refcnt);
> >^^
> > can we also put ASSERT/check into this branch
> > to verify we are no releasing the last
> > reference to ipa. I suspect we might be doing
> > an extra reference drop here.
> 
> Later we call ipsp_unref_acquire_locked() and refcnt_rele() again.
> This will KASSERT(refcnt != ~0) in the case you describe.
> 
> > I believe this ASSERT
> > would be hit if we will compile kernel without IPSEC.
> > we grab the extra reference iff are adding a timer.
> 
> We release the extra reference only iff we delete the timer
> that was added before.

I see your point. it makes sense. thanks for clarification.


thanks and
regards
sashan