bgpd fix add-path capability encoding

2021-06-22 Thread Claudio Jeker 
Dumb copy paste error. The add-path capability is 4byte per AFI/SAFI
the 2 + is from graceful restart where two extra bytes are at the front of
the AFI/SAFI list.

-- 
:wq Claudio

Index: session.c
===
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.421
diff -u -p -r1.421 session.c
--- session.c   17 Jun 2021 16:05:26 -  1.421
+++ session.c   22 Jun 2021 11:50:08 -
@@ -1470,9 +1470,9 @@ session_open(struct peer *p)
u_int8_taplen;
 
if (mpcapa)
-   aplen = 2 + 4 * mpcapa;
+   aplen = 4 * mpcapa;
else/* AID_INET */
-   aplen = 2 + 4;
+   aplen = 4;
errs += session_capa_add(opb, CAPA_ADD_PATH, aplen);
if (mpcapa) {
for (i = AID_MIN; i < AID_MAX; i++) {

bgpd refactor some common code for add-path

2021-06-22 Thread Claudio Jeker 
Adjust the way nlri get extracted from the MP attrs. Instead of
switch statements with a while loop for each case move the while loop out
and only do the nlri_get_* call in the switch statement. The mpp and mplen
adjustmens and the call to rde_update_update and rde_update_withdraw are
also moved to the while loop.

-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.527
diff -u -p -r1.527 rde.c
--- rde.c   17 Jun 2021 16:05:26 -  1.527
+++ rde.c   22 Jun 2021 17:49:22 -
@@ -1338,9 +1338,9 @@ rde_update_dispatch(struct rde_peer *pee
rde_peer_recv_eor(peer, aid);
}
 
-   switch (aid) {
-   case AID_INET6:
-   while (mplen > 0) {
+   while (mplen > 0) {
+   switch (aid) {
+   case AID_INET6:
if ((pos = nlri_get_prefix6(mpp, mplen,
, )) == -1) {
log_peer_warnx(>conf,
@@ -1350,14 +1350,8 @@ rde_update_dispatch(struct rde_peer *pee
mpa.unreach, mpa.unreach_len);
goto done;
}
-   mpp += pos;
-   mplen -= pos;
-
-   rde_update_withdraw(peer, , prefixlen);
-   }
-   break;
-   case AID_VPN_IPv4:
-   while (mplen > 0) {
+   break;
+   case AID_VPN_IPv4:
if ((pos = nlri_get_vpn4(mpp, mplen,
, , 1)) == -1) {
log_peer_warnx(>conf,
@@ -1367,14 +1361,8 @@ rde_update_dispatch(struct rde_peer *pee
mpa.unreach, mpa.unreach_len);
goto done;
}
-   mpp += pos;
-   mplen -= pos;
-
-   rde_update_withdraw(peer, , prefixlen);
-   }
-   break;
-   case AID_VPN_IPv6:
-   while (mplen > 0) {
+   break;
+   case AID_VPN_IPv6:
if ((pos = nlri_get_vpn6(mpp, mplen,
, , 1)) == -1) {
log_peer_warnx(>conf,
@@ -1384,15 +1372,16 @@ rde_update_dispatch(struct rde_peer *pee
mpa.unreach_len);
goto done;
}
-   mpp += pos;
-   mplen -= pos;
-
-   rde_update_withdraw(peer, , prefixlen);
+   break;
+   default:
+   /* ignore unsupported multiprotocol AF */
+   break;
}
-   break;
-   default:
-   /* silently ignore unsupported multiprotocol AF */
-   break;
+
+   mpp += pos;
+   mplen -= pos;
+
+   rde_update_withdraw(peer, , prefixlen);
}
 
if ((state.aspath.flags & ~F_ATTR_MP_UNREACH) == 0)
@@ -1466,9 +1455,9 @@ rde_update_dispatch(struct rde_peer *pee
mpp += pos;
mplen -= pos;
 
-   switch (aid) {
-   case AID_INET6:
-   while (mplen > 0) {
+   while (mplen > 0) {
+   switch (aid) {
+   case AID_INET6:
if ((pos = nlri_get_prefix6(mpp, mplen,
, )) == -1) {
log_peer_warnx(>conf,
@@ -1478,16 +1467,8 @@ rde_update_dispatch(struct rde_peer *pee
mpa.reach, mpa.reach_len);
goto done;
}
-   mpp += pos;
-   mplen -= pos;
-
-   if (rde_update_update(peer, , ,
-   prefixlen) == -1)
-   goto done;
-   }
-   break;
-   case AID_VPN_IPv4:
-   while (mplen > 0) {
+   break;
+   case AID_VPN_IPv4:
if ((pos = nlri_get_vpn4(mpp,

Re: [External] : Re: parallel forwarding vs. bridges

2021-06-22 Thread Alexandr Nedvedicky 
Hello,



new diff still looks good to me. I could just catch typos
in comment.

> 
> I've been running versions of this diff in production at work, and have
> hit a few panics and asserts. All the issues we've hit should be
> addressed in this diff.
> 
> The first issue was that pfsync could be in the processing of sending a
> deferred packet while it's being removed from the state tree. Part of
> that removal process is stripping the state keys from the state, which
> pfsync uses to determine the address family so it knows which ip output
> routine to use. The quick and dirty fix to this is to have pfsync check
> if timeout state to see if the state is unlinked or not. This currently
> relies on pfsync undefer and pf being serialised by the NET_LOCK.
> 
> The second is that the timeout member on a state can change while the
> purge task is looking at it. We hit this assert in pf_state_expires:
> 
>   KASSERT(state->timeout != PFTM_UNLINKED);
> 
> pf_state_expires was called from the purge code like this:
> 
>   if ((cur->timeout == PFTM_UNLINKED) ||
>   (pf_state_expires(cur) <= getuptime()))
>   SLIST_INSERT_HEAD(, cur, gc_list);
> 

I see. I've missed those in earlier review. the pf(4) we have
in tree uses PF_STATE_ lock to keep cur->timeout consistent.
pf_purge_expired_states() executes the check under PF_STATE_ENTER_READ().

> 
> With my new locking scheme here, the state purge code is called without
> any of the locks that would serialise access the state->timeout
> variable. I think I found a solution to this without having to
> reintroduce extra locking, which should allow us to keep the purge scan
> running concurrently with pf actually handling packets.

I think it should work, because we make sure pf_remove_state()
gets called only once.

I've found two typos in comment, see below. otherwise looks good.

OK sashan
> + /*
> +  * pf_state_expires is used by the state purge task to
> +  * decide if a state is a candidate for cleanup, and by the
> +  * pfsync state export code to populate an expiry time.
> +  *
> +  * 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
s/chacks/checks
> +  * their view of the timeout in here for this function to use.
> +  * the only consequnce of using a stale timeout value is
s/consequnce/consequence

OK sashan

Re: disklabel(8): reduce usage()

2021-06-22 Thread Todd C . Miller 
On Tue, 22 Jun 2021 13:34:43 +0100, Jason McIntyre wrote:

> diff to reduce verbosity in disklabel(8) usage. before:

OK.  Should we also remove SEEALSO from sbin/disklabel/Makefile
and distrib/special/disklabel/Makefile?

 - todd

Re: crontab(1): reduce usage()

2021-06-22 Thread Todd C . Miller 
On Tue, 22 Jun 2021 13:42:19 +0100, Jason McIntyre wrote:

> diff to reduce verbosity in crontab(1) usage. before:

OK millert@

 - todd

Re: rdist(1): reduce usage()

2021-06-22 Thread Todd C . Miller 
On Tue, 22 Jun 2021 13:56:46 +0100, Jason McIntyre wrote:

> this one is more problematic. it uses two functions, getdistoptlist and
> msgprusage, which i think are only used within usage. so i killed them
> too.

OK millert@

 - todd

adb(4/macppc): fix adb_cuda_tickle() prototype

2021-06-22 Thread Scott Cheloha 
We seem to cast adb_cuda_tickle() to a void pointer to bypass
timeout_set(9)'s type checking

The actual function takes no parameters, but the timeout layer doesn't
know that.  Eek.  This is no good.

The fix is trivial.  Add an unused void pointer parameter to
adb_cuda_tickle().

ok?

Index: adb.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/adb.c,v
retrieving revision 1.43
diff -u -p -r1.43 adb.c
--- adb.c   11 Mar 2021 11:16:58 -  1.43
+++ adb.c   22 Jun 2021 19:26:07 -
@@ -228,7 +228,7 @@ voidprint_single(u_char *);
 void   adb_intr_cuda(void);
 void   adb_soft_intr(void);
 intsend_adb_cuda(u_char *, u_char *, void *, void *, int);
-void   adb_cuda_tickle(void);
+void   adb_cuda_tickle(void *);
 void   adb_pass_up(struct adbCommand *);
 void   adb_op_comprout(caddr_t, caddr_t, int);
 void   adb_reinit(struct adb_softc *);
@@ -276,7 +276,7 @@ print_single(u_char *str)
 #endif
 
 void
-adb_cuda_tickle(void)
+adb_cuda_tickle(void *unused)
 {
volatile int s;
 
@@ -1160,7 +1160,7 @@ adb_reinit(struct adb_softc *sc)
 #endif
 
if (adbHardware == ADB_HW_CUDA) {
-   timeout_set(_cuda_timeout, (void *)adb_cuda_tickle, NULL);
+   timeout_set(_cuda_timeout, adb_cuda_tickle, NULL);
timeout_add(_cuda_timeout, ADB_TICKLE_TICKS);
}

Re: tset(1): reduce usage()

2021-06-22 Thread Klemens Nanni 
On Tue, Jun 22, 2021 at 05:52:34PM +0200, Theo Buehler wrote:
> I think we should pull the assignment up. Our execve(2) guarantees that
> argc >= 1, so it's safe to do *argv. We need to do this before calling
> the internal err() the first time as that uses _nc_progname() internally.
Even better, OK kn.

iwm/iwx: firmware HT protection flags fix

2021-06-22 Thread Stefan Sperling 
According to zxystd from OpenIntelWireless, the following change fixed
fatal firmware errors seen during HT protection updates for them.

I have not seen the issue myself, but I don't think this change can hurt.
If it fixes firmware errors in an edge case, all the better.

This essentially aligns the flags we send to firmware with those used
by Linux iwlwifi. Setting the FAT protection flag while we're using
20 MHz channels doesn't really make a lot of sense to me but apparently
firmware expects it to be set regardless. And since Linux does not use the
CTS_EN flag it might be better to avoid using that flag?

ok?

The corresponding change in OpenIntelWireless is this one:
https://github.com/OpenIntelWireless/itlwm/commit/032a14185a434b7181c0e78a953dc2e1c21a0853
 
diff 97717a5af75b97bcfcce7ae087b98ea37fd88190 
c1fe4cc52d46fbf399a26531f8e540ff96486f00
blob - e33fe39b9784c5bd4f8b28099af9f97c04e49c65
blob + a8629aa87c1373f86ae105bf84ae3c2ce8aca7b3
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -7678,10 +7678,8 @@ iwm_mac_ctxt_cmd_common(struct iwm_softc *sc, struct i
case IEEE80211_HTPROT_NONMEMBER:
case IEEE80211_HTPROT_NONHT_MIXED:
cmd->protection_flags |=
-   htole32(IWM_MAC_PROT_FLG_HT_PROT);
-   if (ic->ic_protmode == IEEE80211_PROT_CTSONLY)
-   cmd->protection_flags |=
-   htole32(IWM_MAC_PROT_FLG_SELF_CTS_EN);
+   htole32(IWM_MAC_PROT_FLG_HT_PROT |
+   IWM_MAC_PROT_FLG_FAT_PROT);
break;
case IEEE80211_HTPROT_20MHZ:
if (ic->ic_htcaps & IEEE80211_HTCAP_CBW20_40) {
@@ -7689,9 +7687,6 @@ iwm_mac_ctxt_cmd_common(struct iwm_softc *sc, struct i
cmd->protection_flags |=
htole32(IWM_MAC_PROT_FLG_HT_PROT |
IWM_MAC_PROT_FLG_FAT_PROT);
-   if (ic->ic_protmode == IEEE80211_PROT_CTSONLY)
-   cmd->protection_flags |= htole32(
-   IWM_MAC_PROT_FLG_SELF_CTS_EN);
}
break;
default:
blob - 21bc8be34500730264ad206513951a32fcb2d702
blob + 95da2db085e4b8b03b21e67a26e4fee5e1dc75f5
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -5891,10 +5891,8 @@ iwx_mac_ctxt_cmd_common(struct iwx_softc *sc, struct i
case IEEE80211_HTPROT_NONMEMBER:
case IEEE80211_HTPROT_NONHT_MIXED:
cmd->protection_flags |=
-   htole32(IWX_MAC_PROT_FLG_HT_PROT);
-   if (ic->ic_protmode == IEEE80211_PROT_CTSONLY)
-   cmd->protection_flags |=
-   htole32(IWX_MAC_PROT_FLG_SELF_CTS_EN);
+   htole32(IWX_MAC_PROT_FLG_HT_PROT |
+   IWX_MAC_PROT_FLG_FAT_PROT);
break;
case IEEE80211_HTPROT_20MHZ:
if (ic->ic_htcaps & IEEE80211_HTCAP_CBW20_40) {
@@ -5902,9 +5900,6 @@ iwx_mac_ctxt_cmd_common(struct iwx_softc *sc, struct i
cmd->protection_flags |=
htole32(IWX_MAC_PROT_FLG_HT_PROT |
IWX_MAC_PROT_FLG_FAT_PROT);
-   if (ic->ic_protmode == IEEE80211_PROT_CTSONLY)
-   cmd->protection_flags |= htole32(
-   IWX_MAC_PROT_FLG_SELF_CTS_EN);
}
break;
default:

Re: ypmatch(1): reduce usage()

2021-06-22 Thread Todd C . Miller 
On Tue, 22 Jun 2021 13:44:27 +0100, Jason McIntyre wrote:

> diff to reduce verbosity in ypmatch(1) usage. before:

OK millert@

 - todd

Re: locate(1): reduce usage()

2021-06-22 Thread Todd C . Miller 
On Tue, 22 Jun 2021 13:49:35 +0100, Jason McIntyre wrote:

> diff to reduce verbosity in locate(1) usage. before:

OK millert@

 - todd

Re: disklabel(8): reduce usage()

2021-06-22 Thread Jason McIntyre 
On Tue, Jun 22, 2021 at 12:36:02PM -0600, Todd C. Miller wrote:
> On Tue, 22 Jun 2021 13:34:43 +0100, Jason McIntyre wrote:
> 
> > diff to reduce verbosity in disklabel(8) usage. before:
> 
> OK.  Should we also remove SEEALSO from sbin/disklabel/Makefile
> and distrib/special/disklabel/Makefile?
> 
>  - todd
> 

ah, now i see the point of SEEALSO! how about this?
(completely untested for distrib/special)

jmc

Index: distrib/special/disklabel/Makefile
===
RCS file: /cvs/src/distrib/special/disklabel/Makefile,v
retrieving revision 1.11
diff -u -p -r1.11 Makefile
--- distrib/special/disklabel/Makefile  21 Jul 2020 13:56:09 -  1.11
+++ distrib/special/disklabel/Makefile  22 Jun 2021 20:09:55 -
@@ -25,25 +25,6 @@ manual.c:disklabel.cat8
echo '};'; echo 'const int manpage_sz = sizeof(manpage);') > manual.c
 .endif
 
-.if ${MACHINE} == "alpha"
-CFLAGS+= -DSEEALSO="\"installboot(8)\""
-.endif
-
-.if ${MACHINE} == "amd64" || ${MACHINE} == "i386" || \
-${MACHINE} == "octeon" || ${MACHINE} == "loongson" || \
-${MACHINE} == "armv7" || ${MACHINE} == "arm64" || \
-${MACHINE} == "landisk" || ${MACHINE} == "powerpc64"
-CFLAGS+= -DSEEALSO="\"fdisk(8), installboot(8)\""
-.endif
-
-.if ${MACHINE} == "macppc"
-CFLAGS+= -DSEEALSO="\"fdisk(8), pdisk(8), installboot(8)\""
-.endif
-
-.if ${MACHINE} == "sparc64"
-CFLAGS+= -DSEEALSO="\"installboot(8)\"" -DSUN_CYLCHECK -DSUN_AAT0
-.endif
-
 .ifdef NOPIC
 CFLAGS+= -DSTATICLINKING
 .endif
Index: sbin/disklabel/Makefile
===
RCS file: /cvs/src/sbin/disklabel/Makefile,v
retrieving revision 1.68
diff -u -p -r1.68 Makefile
--- sbin/disklabel/Makefile 21 Jul 2020 13:56:08 -  1.68
+++ sbin/disklabel/Makefile 22 Jun 2021 20:09:55 -
@@ -25,23 +25,4 @@ manual.c:disklabel.cat8
echo '};'; echo 'const int manpage_sz = sizeof(manpage);') > manual.c
 .endif
 
-.if ${MACHINE} == "alpha"
-CFLAGS+= -DSEEALSO="\"installboot(8)\""
-.endif
-
-.if ${MACHINE} == "amd64" || ${MACHINE} == "i386" || \
-${MACHINE} == "octeon" || ${MACHINE} == "loongson" || \
-${MACHINE} == "armv7" || ${MACHINE} == "arm64" || \
-${MACHINE} == "landisk" || ${MACHINE} == "powerpc64"
-CFLAGS+= -DSEEALSO="\"fdisk(8), installboot(8)\""
-.endif
-
-.if ${MACHINE} == "macppc"
-CFLAGS+= -DSEEALSO="\"fdisk(8), pdisk(8), installboot(8)\""
-.endif
-
-.if ${MACHINE} == "sparc64"
-CFLAGS+= -DSEEALSO="\"installboot(8)\"" -DSUN_CYLCHECK -DSUN_AAT0
-.endif
-
 .include 
Index: sbin/disklabel/disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.236
diff -u -p -r1.236 disklabel.c
--- sbin/disklabel/disklabel.c  14 Nov 2020 20:53:31 -  1.236
+++ sbin/disklabel/disklabel.c  22 Jun 2021 20:09:55 -
@@ -1212,28 +1212,15 @@ void
 usage(void)
 {
fprintf(stderr,
-   "usage: disklabel[-Acdtv] [-h | -p unit] [-T file] 
disk\t(read)\n");
+   "usage: disklabel[-Acdtv] [-h | -p unit] [-T file] disk\n");
fprintf(stderr,
-   "   disklabel -w [-Acdnv] [-T file] disk disktype 
[packid]\t(write)\n");
+   "   disklabel -w [-Acdnv] [-T file] disk disktype [packid]\n");
fprintf(stderr,
-   "   disklabel -e [-Acdnv] [-T file] disk\t\t\t(edit)\n");
+   "   disklabel -e [-Acdnv] [-T file] disk\n");
fprintf(stderr,
-   "   disklabel -E [-Acdnv] [-F|-f file] [-T file] disk\t(simple 
editor)"
-   "\n");
+   "   disklabel -E [-Acdnv] [-F|-f file] [-T file] disk\n");
fprintf(stderr,
-   "   disklabel -R [-nv] [-F|-f file] disk 
protofile\t\t(restore)\n\n");
-   fprintf(stderr,
-   "`disk' may be of the form: sd0 or /dev/rsd0%c.\n", 'a'+RAW_PART);
-   fprintf(stderr,
-   "`disktype' is an entry from %s, see disktab(5) for more info.\n",
-   DISKTAB);
-   fprintf(stderr,
-   "`packid' is an identification string for the device.\n");
-   fprintf(stderr,
-   "`protofile' is the output from the read cmd form; -R is 
powerful.\n");
-#ifdef SEEALSO
-   fprintf(stderr,
-   "For procedures specific to this architecture see: %s\n", SEEALSO);
-#endif
+   "   disklabel -R [-nv] [-F|-f file] disk protofile\n");
+
exit(1);
 }

bgpd fix bad free() call when deflating aspath

2021-06-22 Thread Claudio Jeker 
I changed the way up_generate_attr() calls aspath_deflate() but did not
realize that aspath_deflate() frees the pdata at the end. That free should
no longer happen but for that also the mrt case where aspath_deflate()
needs to be adjusted.

With this both the mrt and as0 integration test pass again.
-- 
:wq Claudio

Index: rde_attr.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_attr.c,v
retrieving revision 1.124
diff -u -p -r1.124 rde_attr.c
--- rde_attr.c  16 Jan 2021 13:14:54 -  1.124
+++ rde_attr.c  22 Jun 2021 18:13:15 -
@@ -568,7 +568,6 @@ aspath_put(struct aspath *aspath)
 
 /*
  * convert a 4 byte aspath to a 2 byte one.
- * data is freed by aspath_deflate
  */
 u_char *
 aspath_deflate(u_char *data, u_int16_t *len, int *flagnew)
@@ -614,7 +613,6 @@ aspath_deflate(u_char *data, u_int16_t *
}
}
 
-   free(data);
*len = nlen;
return (ndata);
 }
Index: mrt.c
===
RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
retrieving revision 1.103
diff -u -p -r1.103 mrt.c
--- mrt.c   9 Jan 2020 11:55:25 -   1.103
+++ mrt.c   22 Jun 2021 18:12:45 -
@@ -160,15 +160,19 @@ mrt_attr_dump(struct ibuf *buf, struct r
return (-1);
 
/* aspath */
-   pdata = aspath_prepend(a->aspath, rde_local_as(), 0, );
+   plen = aspath_length(a->aspath);
+   pdata = aspath_dump(a->aspath);
+
if (!v2)
pdata = aspath_deflate(pdata, , );
if (attr_writebuf(buf, ATTR_WELL_KNOWN, ATTR_ASPATH, pdata,
plen) == -1) {
-   free(pdata);
+   if (!v2)
+   free(pdata);
return (-1);
}
-   free(pdata);
+   if (!v2)
+   free(pdata);
 
if (nexthop && nexthop->aid == AID_INET) {
/* nexthop, already network byte order */

Re: add table_procexec in smtpd

2021-06-22 Thread gilles 
June 22, 2021 11:14 PM, "Aisha Tammy"  wrote:

>> [...]
>> 
>> WRT to handshake, it has multiple uses ranging from ensuring the addons get 
>> their configuration
>> from the daemon to synchronising the daemon start with addons readiness.
>> Remember, we didn’t have this with filters and realised we couldn’t do 
>> without, it is the same for
>> tables, they need to get their “table name” at start so we need the daemon 
>> to pass them, and we
>> also need the daemon to not start using them before they are ready.
> 
> I am unsure what you mean by a handshake.
>

sure, so let's look at procexec for filters:

- when the server starts, it forks the filters and begins a handshake with each 
of them,
  emitting the following (for example):

config|smtpd-version|6.6.1
config|smtp-session-timeout|300
config|subsystem|smtp-in
config|ready

- when the filter receives the last line, it knows the server is done and it is 
its turn,
  it emits the following:

register|report|smtp-in|link-connect
register|ready

- at this point the handshake is over, the server is ready and the filter is 
ready too,
  they are both in a known state and synchronised before data flows to the 
filter.


The procexec tables should have a similar handshake to allow the server to pass 
them
information and make sure they are synchronised.

The only difference with filters would be that table would have a different 
"register""
line in the handshake but the config part should be similar.
This would an addon to implement both a filter and a table by registering for 
filtering,
and providing a lookup backend for example.


> Would something like the following be good - on exec the table process has to 
> print out a string
> "TABLE-READY" which ensures us that the process is ready.
>

Almost, on exec the daemon prints the config bits (exactly like it does for 
filter),
then it reads from the table backend for a register|ready,
but yes you have the idea.


> I am not exactly sure how I would implement this, my guess would be to use 
> event(3) with EV_READ on
> backend_r (or something like that).
> 

I haven't looked at this code in over a year now so I'm not sure what the right 
way
of doing it is on top of my head, but looking at how filters are handled is a 
good
startint point.

Re: add table_procexec in smtpd

2021-06-22 Thread Aisha Tammy 


> >> First, if the two implementations are not going to coexists,
> >> we can just replace table_proc.c.
> > 
> > True, though proc-exec was the name used for filters so it may be a good to 
> > unify and drop the legacy “proc” name.
> > This will be hidden to users so quite frankly it’s a matter of dev 
> > preference.

I've kept procexec for now.

> > 
> >> Second, since the goal is to expose the protocol directly, instead of
> >> relying on wrappers (through opensmtpd-extras api), we should take
> >> time to refine it properly before, and avoid having to bump it every
> >> other day.  For example, I don't see the point of adding timestamps in
> >> the request.  Do we want to be case-sensitive on the commands?  Do we
> >> need some kind of handshake?  Also, there can be long-term plan to use
> >> the same process for different tables, or even for other backends.
> > 
> > I’m unsure I understand your point:
> > 
> > The protocol is based on the filter protocol, follows the same logic and 
> > line header to solve the same issues, this is precisely so that there’s no 
> > need to bump every other day as we already figured what was needed for 
> > third party adding to interoperate with smtpd.
> > This also has the advantage that you can have a single parser handle these 
> > different API instead of having a filter protocol parser, a table protocol 
> > parser (and maybe in the future a queue protocol parser… etc).
> > 
> > WRT timestamps there were many uses for them ranging from timeout detection 
> > both in daemon / add-ons, profiling, logging the time an event was 
> > generated vs processes overhead, etc…
> > It allowed ensuring that all addons handling the same event have the exact 
> > same timestamp associated to the event.
> > 
> > WRT to case-sensitivity, I don’t recall using upper-case myself, to me the 
> > protocol is case-sensitive and as far as I can remember I always used 
> > lowercase :-/
> > I’m all for lowering case here.
> > 
> > WRT to handshake, it has multiple uses ranging from ensuring the addons get 
> > their configuration from the daemon to synchronising the daemon start with 
> > addons readiness. 
> > Remember, we didn’t have this with filters and realised we couldn’t do 
> > without, it is the same for tables, they need to get their “table name” at 
> > start so we need the daemon to pass them, and we also need the daemon to 
> > not start using them before they are ready.
> >

I am unsure what you mean by a handshake.
Would something like the following be good - on exec the table process has to 
print out a string "TABLE-READY" which ensures us that the process is ready.
I am not exactly sure how I would implement this, my guess would be to use 
event(3) with EV_READ on backend_r (or something like that).

> > 
> >> Finally the implementation could be factorized a bit, but that's a
> >> detail at this time. I think the close operation (is it really useful 
> >> anyway?)
> >> should use fclose() instead of kill(), and maybe wait() too?

I've moved it to fclose(), I've not used wait() for now.

> > 
> > The implementation can probably be improved, this was a PoC that allowed me 
> > to write various table backends but it was never meant to be committed in 
> > the first place.
> 

If you can clarify what the handshake means, that would be nice.

Cheers,
Aisha


diff --git a/usr.sbin/smtpd/parse.y b/usr.sbin/smtpd/parse.y
index 011e306ac61..1b0ee5ad38f 100644
--- a/usr.sbin/smtpd/parse.y
+++ b/usr.sbin/smtpd/parse.y
@@ -2557,13 +2557,6 @@ table: TABLE STRING STRING   {
config  = p+1;
}
}
-   if (config != NULL && *config != '/') {
-   yyerror("invalid backend parameter for table: 
%s",
-   $2);
-   free($2);
-   free($3);
-   YYERROR;
-   }
table = table_create(conf, backend, $2, config);
if (!table_config(table)) {
yyerror("invalid configuration file %s for 
table %s",
diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile
index ef8148be8c9..46831d647dc 100644
--- a/usr.sbin/smtpd/smtpctl/Makefile
+++ b/usr.sbin/smtpd/smtpctl/Makefile
@@ -47,7 +47,7 @@ SRCS+=table.c
 SRCS+= table_static.c
 SRCS+= table_db.c
 SRCS+= table_getpwnam.c
-SRCS+= table_proc.c
+SRCS+= table_procexec.c
 SRCS+= unpack_dns.c
 SRCS+= spfwalk.c
 
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index be934112103..221f24fbdc4 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1656,6 +1656,7 @@ int table_regex_match(const char *, const char *);
 void   table_open_all(struct smtpd *);
 void   table_dump_all(struct smtpd *);
 void   table_close_all(struct smtpd *);
+const char

Re: Z590 chipset + i7 10700 CPU - slowness, sysctl hw.cpuspeed/setperf weirdness

2021-06-22 Thread Ashton Fagg 
Mark Kettenis  writes:

> Sounds like Intel changed the way CPU frequency scaling is implemented
> on these new CPUs.  Somebody will have to look into this, but many
> OpenBSD developers prefer to buy machines with AMD CPU which is
> probably why this hasn't happened yet.

For those joining us on tech@, original bug report:

https://marc.info/?l=openbsd-bugs=162424580212241=2

I managed to work out what's happening here. There's a feature called
Intel Hardware P-States (HWP), which is known by the marketing gimmick
name of "Intel SpeedShift". This is apparently something distinct from
SpeedStep.

This is nothing new - it's been around since Skylake apparently. But, my
research indicates it is usually disabled by default. For whatever
reason - whether it be the fact my chipset is very very new, or just by
pure luck - my BIOS *enables* it by default.

Despite the fact there's a separate setting for SpeedStep, if SpeedShift
is enabled, according to Intel's documentation, the chip will no longer
repond to the usual scaling mechanisms (i.e. SpeedStep). So one solution
to this is just to turn it off in the BIOS - this works for me, even
once I re-enabled SpeedStep and C-States, leaving SpeedShift disabled
was enough to get good performance.

The issue is while you can detect that SpeedShift is on at
run-time, you can't turn it off without a reset. The Intel documentation
however suggests a workaround, which is implemented in the patch below.

https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-vol-3b-part-2-manual.pdf

Seconds 14.4 has all the necessary info.

The workaround consists of this:

- Detect if HWP is enabled and set a flag indicating the package
  supports it.
- If it is, don't enable SpeedStep (since it won't work anyway).

Then, for each core:

- Ask what the "maximum performance" is by querying the capabilities
  register.
- Request that the minimum and maximum performance be set to this and
  call it a day.

(there is an MSR defined in the manual for doing this step for the
*whole* package, but the manual indicates it's not always available - I
tried it on my hardware with no success)

This is kinda ugly as it basically runs the CPU flat-out as if no
scaling was enabled. Unfortunately, *not* doing the second step results
in very poor performance as I was experiencing before. After I found
this I wondered if just disabling the SpeedStep driver was sufficient -
apparently not.

Interestingly, my compile test went from 2min15sec with SpeedShift
disabled in the BIOS, to 1min30sec with this patch (compared to the
7mins in my initial report where scaling wasn't working properly).

I have no idea if anyone will be interested in committing this patch, I
just wanted to share what I learned. The ultimate solution is probably
to either write a driver for this or import the one that's in FreeBSD to
replace SpeedStep for CPUs where HWP is enabled. But I'm probably going
to just disable SpeedShift in the BIOS since that does the job. :-)

Thanks,

-ajf

Index: sys/arch/amd64//amd64/identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.118
diff -u -p -u -p -r1.118 identcpu.c
--- sys/arch/amd64//amd64/identcpu.c	31 Dec 2020 06:22:33 -	1.118
+++ sys/arch/amd64//amd64/identcpu.c	23 Jun 2021 02:58:59 -
@@ -64,6 +64,7 @@ int amd64_has_aesni;
 #endif
 int has_rdrand;
 int has_rdseed;
+int intel_has_hwp;
 
 const struct {
 	u_int32_t	bit;
@@ -214,6 +215,7 @@ const struct {
 }, cpu_tpm_eaxfeatures[] = {
 	{ TPM_SENSOR,		"SENSOR" },
 	{ TPM_ARAT,		"ARAT" },
+	{ TPM_HWP,		"HWP"  },
 }, cpu_cpuid_perf_eax[] = {
 	{ CPUIDEAX_VERID,	"PERF" },
 }, cpu_cpuid_apmi_edx[] = {
@@ -699,7 +701,24 @@ identifycpu(struct cpu_info *ci)
 setperf_setup = k1x_init;
 		}
 
-		if (cpu_ecxfeature & CPUIDECX_EST)
+		/*
+		 *  Intel HWP/"SpeedShift" - if enabled, CPU scaling won't work right.
+		 *  This detects that the package has this capability, but needs to be
+		 *  configured per core.
+		 */
+		intel_has_hwp = 0;
+		if (!strcmp(cpu_vendor, "GenuineIntel") && (ci->ci_feature_tpmflags & TPM_HWP))
+		{
+			uint64_t msr_hwp;
+
+			msr_hwp = rdmsr(IA32_PM_ENABLE);
+			if (msr_hwp & IA32_HWP_ENABLE) {
+printf("SpeedShift detected. SpeedStep will not be enabled, and CPU scaling won't work.\n");
+intel_has_hwp = 1;
+			}
+		}
+
+		if ((cpu_ecxfeature & CPUIDECX_EST) && !intel_has_hwp)
 			setperf_setup = est_init;
 #endif
 
@@ -729,6 +748,25 @@ identifycpu(struct cpu_info *ci)
 		sensor_task_register(ci, intelcore_update_sensor, 5);
 		sensor_attach(>ci_sensordev, >ci_sensor);
 		sensordev_install(>ci_sensordev);
+	}
+
+	if (intel_has_hwp)
+	{
+		uint64_t msr_cap, msr_req, max_perf;
+
+		msr_cap = rdmsr(IA32_HWP_CAPABILITIES);
+
+		/* Lowest byte is the "highest performance" capability */
+		max_perf = msr_cap & 0xFF;
+
+		/*
+		 * Least-significant byte is

scsi(8): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in scsi(8) usage. before:

$ /sbin/scsi -Z
scsi: unknown option -- Z
Usage:

  scsi -f device -d debug_level# To set debug level
  scsi -f device -m page [-P pc]   # To read mode pages
  scsi -f device [-v] [-s seconds] -c cmd_fmt [arg0 ... argn] # A command...
 -o count out_fmt [arg0 ... argn]  #   EITHER (data out)
 -i count in_fmt   #   OR (data in)

"out_fmt" can be "-" to read output data from stdin;
"in_fmt" can be "-" to write input data to stdout;

If debugging is not compiled in the kernel, "-d" will have no effect

after:

$ /usr/obj/sbin/scsi/scsi -Z
scsi: unknown option -- Z
usage: scsi -f device -d debug_level
   scsi -f device -m page [-e] [-P pc]
   scsi -f device [-v] [-s seconds] -c cmd_fmt [arg ...] -o count out_fmt
[arg ...] -i count in_fmt [arg ...]

ok?
jmc

Index: sbin/scsi/scsi.c
===
RCS file: /cvs/src/sbin/scsi/scsi.c,v
retrieving revision 1.30
diff -u -p -r1.30 scsi.c
--- sbin/scsi/scsi.c7 Jun 2016 01:29:38 -   1.30
+++ sbin/scsi/scsi.c22 Jun 2021 12:27:10 -
@@ -84,20 +84,11 @@ static void
 usage(void)
 {
fprintf(stderr,
-"Usage:\n"
-"\n"
-"  scsi -f device -d debug_level# To set debug level\n"
-"  scsi -f device -m page [-P pc]   # To read mode pages\n"
-"  scsi -f device [-v] [-s seconds] -c cmd_fmt [arg0 ... argn] # A 
command...\n"
-" -o count out_fmt [arg0 ... argn]  #   EITHER (data out)\n"
-" -i count in_fmt   #   OR (data in)\n"
-"\n"
-"\"out_fmt\" can be \"-\" to read output data from stdin;\n"
-"\"in_fmt\" can be \"-\" to write input data to stdout;\n"
-"\n"
-"If debugging is not compiled in the kernel, \"-d\" will have no effect\n"
-
-);
+"usage: scsi -f device -d debug_level\n"
+"   scsi -f device -m page [-e] [-P pc]\n"
+"   scsi -f device [-v] [-s seconds] -c cmd_fmt [arg ...]"
+" -o count out_fmt\n"
+"[arg ...] -i count in_fmt [arg ...]\n");
 
exit (1);
 }

crontab(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in crontab(1) usage. before:

$ /usr/bin/crontab -Z
crontab: unknown option -- Z
usage: crontab [-u user] file
   crontab [-e | -l | -r] [-u user]
(default operation is replace, per 1003.2)
-e  (edit user's crontab)
-l  (list user's crontab)
-r  (delete user's crontab)

after:

$ /usr/obj/usr.bin/crontab/crontab -Z
crontab: unknown option -- Z
usage: crontab [-u user] file
   crontab [-e | -l | -r] [-u user]

ok?
jmc

Index: crontab.c
===
RCS file: /cvs/src/usr.sbin/cron/crontab.c,v
retrieving revision 1.94
diff -u -p -r1.94 crontab.c
--- crontab.c   11 Feb 2020 12:42:02 -  1.94
+++ crontab.c   22 Jun 2021 12:41:42 -
@@ -70,11 +70,7 @@ usage(const char *msg)
warnx("usage error: %s", msg);
fprintf(stderr, "usage: %s [-u user] file\n", __progname);
fprintf(stderr, "   %s [-e | -l | -r] [-u user]\n", __progname);
-   fprintf(stderr,
-   "\t\t(default operation is replace, per 1003.2)\n"
-   "\t-e\t(edit user's crontab)\n"
-   "\t-l\t(list user's crontab)\n"
-   "\t-r\t(delete user's crontab)\n");
+
exit(EXIT_FAILURE);
 }

locate(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in locate(1) usage. before:

$ /usr/bin/locate -Z
locate: unknown option -- Z
usage: locate [-bciS] [-d database] [-l limit] pattern ...
default database: `/var/db/locate.database' or $LOCATE_PATH

after:

$ /usr/obj/usr.bin/locate/locate/locate -Z
locate: unknown option -- Z
usage: locate [-bciS] [-d database] [-l limit] pattern ...

ok?
jmc

Index: usr.bin/locate/locate/locate.c
===
RCS file: /cvs/src/usr.bin/locate/locate/locate.c,v
retrieving revision 1.33
diff -u -p -r1.33 locate.c
--- usr.bin/locate/locate/locate.c  17 Jan 2019 06:15:44 -  1.33
+++ usr.bin/locate/locate/locate.c  22 Jun 2021 12:49:13 -
@@ -252,8 +252,7 @@ usage(void)
 {
(void)fprintf(stderr, "usage: locate [-bciS] [-d database] ");
(void)fprintf(stderr, "[-l limit] pattern ...\n");
-   (void)fprintf(stderr, "default database: `%s' or $LOCATE_PATH\n",
-   _PATH_FCODES);
+
exit(1);
 }

Re: tls_load_file.3: tls_config_set_*_file() load files into memory

2021-06-22 Thread Klemens Nanni 
On Tue, Jun 22, 2021 at 06:35:44AM +0100, Jason McIntyre wrote:
> > -sets the files from which the public certificate, and private key will be 
> > read.
> > +loads two files from which the public certificate, and private key will be 
> > read.
> 
> this is a weird place for a comma. i would remove it.
> jmc

Agreed, but all the other sentences below have it as well so I left it
to keep my diff simple.  I'm sure this manual can be polished further
afterwards.

> >  .Pp
> >  .Fn tls_config_set_keypair_mem
> >  directly sets the public certificate, and private key from memory.
> >  .Pp
> >  .Fn tls_config_set_keypair_ocsp_file
> > -sets the files from which the public certificate, private key, and 
> > DER-encoded
> > -OCSP staple will be read.
> > +loads three files containing the public certificate, private key, and 
> > DER-encoded
> > +OCSP staple.
> >  .Pp
> >  .Fn tls_config_set_keypair_ocsp_mem
> >  directly sets the public certificate, private key, and DER-encoded OCSP 
> > staple
> > 
>

disklabel(8): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in disklabel(8) usage. before:

$ /sbin/disklabel -Z
disklabel: unknown option -- Z
usage: disklabel[-Acdtv] [-h | -p unit] [-T file] disk  (read)
   disklabel -w [-Acdnv] [-T file] disk disktype [packid]   (write)
   disklabel -e [-Acdnv] [-T file] disk (edit)
   disklabel -E [-Acdnv] [-F|-f file] [-T file] disk(simple editor)
   disklabel -R [-nv] [-F|-f file] disk protofile   (restore)

`disk' may be of the form: sd0 or /dev/rsd0c.
`disktype' is an entry from /etc/disktab, see disktab(5) for more info.
`packid' is an identification string for the device.
`protofile' is the output from the read cmd form; -R is powerful.
For procedures specific to this architecture see: fdisk(8), installboot(8)

after:

$ /usr/obj/sbin/disklabel/disklabel -Z
disklabel: unknown option -- Z
usage: disklabel[-Acdtv] [-h | -p unit] [-T file] disk
   disklabel -w [-Acdnv] [-T file] disk disktype [packid]
   disklabel -e [-Acdnv] [-T file] disk
   disklabel -E [-Acdnv] [-F|-f file] [-T file] disk
   disklabel -R [-nv] [-F|-f file] disk protofile

ok?
jmc

Index: sbin/disklabel/disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.236
diff -u -p -r1.236 disklabel.c
--- sbin/disklabel/disklabel.c  14 Nov 2020 20:53:31 -  1.236
+++ sbin/disklabel/disklabel.c  22 Jun 2021 12:34:14 -
@@ -1212,28 +1212,15 @@ void
 usage(void)
 {
fprintf(stderr,
-   "usage: disklabel[-Acdtv] [-h | -p unit] [-T file] 
disk\t(read)\n");
+   "usage: disklabel[-Acdtv] [-h | -p unit] [-T file] disk\n");
fprintf(stderr,
-   "   disklabel -w [-Acdnv] [-T file] disk disktype 
[packid]\t(write)\n");
+   "   disklabel -w [-Acdnv] [-T file] disk disktype [packid]\n");
fprintf(stderr,
-   "   disklabel -e [-Acdnv] [-T file] disk\t\t\t(edit)\n");
+   "   disklabel -e [-Acdnv] [-T file] disk\n");
fprintf(stderr,
-   "   disklabel -E [-Acdnv] [-F|-f file] [-T file] disk\t(simple 
editor)"
-   "\n");
+   "   disklabel -E [-Acdnv] [-F|-f file] [-T file] disk\n");
fprintf(stderr,
-   "   disklabel -R [-nv] [-F|-f file] disk 
protofile\t\t(restore)\n\n");
-   fprintf(stderr,
-   "`disk' may be of the form: sd0 or /dev/rsd0%c.\n", 'a'+RAW_PART);
-   fprintf(stderr,
-   "`disktype' is an entry from %s, see disktab(5) for more info.\n",
-   DISKTAB);
-   fprintf(stderr,
-   "`packid' is an identification string for the device.\n");
-   fprintf(stderr,
-   "`protofile' is the output from the read cmd form; -R is 
powerful.\n");
-#ifdef SEEALSO
-   fprintf(stderr,
-   "For procedures specific to this architecture see: %s\n", SEEALSO);
-#endif
+   "   disklabel -R [-nv] [-F|-f file] disk protofile\n");
+
exit(1);
 }

mkuboot(8): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in mkuboot(8) usage. i don;t have the means to
build this one.

ok?
jmc

Index: mkuboot.c
===
RCS file: /cvs/src/usr.sbin/mkuboot/mkuboot.c,v
retrieving revision 1.10
diff -u -p -r1.10 mkuboot.c
--- mkuboot.c   1 Jun 2021 02:59:01 -   1.10
+++ mkuboot.c   22 Jun 2021 12:36:08 -
@@ -395,16 +395,6 @@ usage(void)
(void)fprintf(stderr,
"usage: %s [-a arch] [-e entry] [-l loadaddr] [-n name] [-o os] "
"[-t type] infile outfile\n", __progname);
-   (void)fprintf(stderr,
-   "arch is one of:");
-   for (mapptr = archmap; mapptr->arch; mapptr++)
-   (void)fprintf(stderr, " %s", mapptr->arch);
-   (void)fprintf(stderr, "\n");
-   (void)fprintf(stderr,
-   "os is one of:");
-   for (osmapptr = osmap; osmapptr->arch; osmapptr++)
-   (void)fprintf(stderr, " %s", osmapptr->arch);
-   (void)fprintf(stderr, "\n");

exit(1);
 }

tset(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in tset(1) usage. before:

$ /usr/bin/tset -Z
tset: unknown option -- Z
Usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] [terminal]

Options:
  -c  set control characters
  -e ch   erase character
  -I  no initialization strings
  -i ch   interrupt character
  -k ch   kill character
  -m mapping  map identifier to type
  -Q  do not output control key settings
  -r  display term on stderr
  -s  output TERM set command
  -V  print curses-version
  -w  set window-size

after:

$ /usr/obj/usr.bin/tset/tset -Z
tset: unknown option -- Z
usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] [terminal]

ok?
jmc

Index: usr.bin/tset/tset.c
===
RCS file: /cvs/src/usr.bin/tset/tset.c,v
retrieving revision 1.41
diff -u -p -r1.41 tset.c
--- usr.bin/tset/tset.c 28 Jun 2019 13:35:05 -  1.41
+++ usr.bin/tset/tset.c 22 Jun 2021 12:46:16 -
@@ -1094,28 +1094,9 @@ obsolete(char **argv)
 static void
 usage(void)
 {
-static const char *tbl[] =
-{
-   ""
-   ,"Options:"
-   ,"  -c  set control characters"
-   ,"  -e ch   erase character"
-   ,"  -I  no initialization strings"
-   ,"  -i ch   interrupt character"
-   ,"  -k ch   kill character"
-   ,"  -m mapping  map identifier to type"
-   ,"  -Q  do not output control key settings"
-   ,"  -r  display term on stderr"
-   ,"  -s  output TERM set command"
-   ,"  -V  print curses-version"
-   ,"  -w  set window-size"
-};
-unsigned n;
-(void) fprintf(stderr, "Usage: %s [-cIQqrsVw] [-] "
-   "[-e ch] [-i ch] [-k ch] [-m mapping] [terminal]\n",
+(void) fprintf(stderr, "usage: %s [-cIQqrsVw] [-] "
+   "[-e ch] [-i ch] [-k ch] [-m mapping] [terminal]",
_nc_progname);
-for (n = 0; n < sizeof(tbl) / sizeof(tbl[0]); ++n)
-   fprintf(stderr, "%s\n", tbl[n]);
 
 exit_error();
 /* NOTREACHED */

lex(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in flex/lex usage. before:

$ /usr/bin/lex --help
Usage: lex [OPTIONS] [FILE]...
Generates programs that perform pattern-matching on text.

Table Compression:
  -Ca, --align  trade off larger tables for better memory alignment
  -Ce, --ecsconstruct equivalence classes
  -Cf   do not compress tables; use -f representation
  -CF   do not compress tables; use -F representation
  -Cm, --meta-ecs   construct meta-equivalence classes
  -Cr, --read   use read() instead of stdio for scanner input
  -f, --fullgenerate fast, large scanner. Same as -Cfr
  -F, --fastuse alternate table representation. Same as -CFr
  -Cem  default compression (same as --ecs --meta-ecs)

Debugging:
  -d, --debug enable debug mode in scanner
  -b, --backupwrite backing-up information to lex.backup
  -p, --perf-report   write performance report to stderr
  -s, --nodefault suppress default rule to ECHO unmatched text
  -T, --trace lex should run in trace mode
  -w, --nowarndo not generate warnings
  -v, --verbose   write summary of scanner statistics to stdout

Files:
  -o, --outfile=FILE  specify output filename
  -S, --skel=FILE specify skeleton file
  -t, --stdoutwrite scanner on stdout instead of lex.yy.c
  --yyclass=NAME  name of C++ class
  --header-file=FILE   create a C header file in addition to the scanner
  --tables-file[=FILE] write tables to FILE

Scanner behavior:
  -7, --7bit  generate 7-bit scanner
  -8, --8bit  generate 8-bit scanner
  -B, --batch generate batch scanner (opposite of -I)
  -i, --case-insensitive  ignore case in patterns
  -l, --lex-compatmaximal compatibility with original lex
  -X, --posix-compat  maximal compatibility with POSIX lex
  -I, --interactive   generate interactive scanner (opposite of -B)
  --yylineno  track line count in yylineno

Generated code:
  -+,  --c++   generate C++ scanner class
  -Dmacro[=defn]   #define macro defn  (default defn is '1')
  -L,  --nolinesuppress #line directives in scanner
  -P,  --prefix=STRING use STRING as prefix instead of "yy"
  -R,  --reentrant generate a reentrant C scanner
   --bison-bridge  scanner for bison pure parser.
   --bison-locations   include yylloc support.
   --stdinit   initialize yyin/yyout to stdin/stdout
   --noansi-definitions old-style function definitions
   --noansi-prototypes  empty parameter list in prototypes
   --nounistd  do not include 
   --noFUNCTIONdo not generate a particular FUNCTION

Miscellaneous:
  -n  do-nothing POSIX option
  -?
  -h, --help  produce this help message
  -V, --version   report lex version

after:

$ /usr/obj/usr.bin/lex/lex --help
usage: lex [-78BbFfhIiLlnpsTtVvw+?] [-C[aeFfmr]] [--help] [--version]
[-ooutput] [-Pprefix] [-Sskeleton] [file ...]

this one is more problematic as the usage is very different. i copied
over the structure from rdist, but did not declare "static void" at the
top as the compiler complained. i left it as "void". also inserted an
exit line at function end.

ok?
jmc

Index: usr.bin/lex/main.c
===
RCS file: /cvs/src/usr.bin/lex/main.c,v
retrieving revision 1.27
diff -u -p -r1.27 main.c
--- usr.bin/lex/main.c  21 Jan 2017 08:33:07 -  1.27
+++ usr.bin/lex/main.c  22 Jun 2021 12:51:48 -
@@ -1740,71 +1740,14 @@ basename2(path, strip_ext)
 }
 
 void
-usage()
+usage(void)
 {
-   FILE *f = stdout;
+   extern char *__progname;
 
-   if (!did_outfilename) {
-   snprintf(outfile_path, sizeof(outfile_path), outfile_template,
-   prefix, C_plus_plus ? "cc" : "c");
-   outfilename = outfile_path;
-   }
-   fprintf(f, _("Usage: %s [OPTIONS] [FILE]...\n"), program_name);
-   fprintf(f,
-   _
-   ("Generates programs that perform pattern-matching on text.\n"
-   "\n" "Table Compression:\n"
-   "  -Ca, --align  trade off larger tables for better memory 
alignment\n"
-   "  -Ce, --ecsconstruct equivalence classes\n"
-   "  -Cf   do not compress tables; use -f 
representation\n"
-   "  -CF   do not compress tables; use -F 
representation\n"
-   "  -Cm, --meta-ecs   construct meta-equivalence classes\n"
-   "  -Cr, --read   use read() instead of stdio for scanner 
input\n"
-   "  -f, --fullgenerate fast, large scanner. Same as 
-Cfr\n"
-   "  -F, --fastuse alternate table representation. Same 
as -CFr\n"
-   "  -Cem  default compression (same as --ecs

Re: scsi(8): reduce usage()

2021-06-22 Thread Mark Kettenis 
> Date: Tue, 22 Jun 2021 13:30:11 +0100
> From: Jason McIntyre 
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> hi.
> 
> diff to reduce verbosity in scsi(8) usage. before:
> 
> $ /sbin/scsi -Z
> scsi: unknown option -- Z
> Usage:
> 
>   scsi -f device -d debug_level# To set debug level
>   scsi -f device -m page [-P pc]   # To read mode pages
>   scsi -f device [-v] [-s seconds] -c cmd_fmt [arg0 ... argn] # A command...
>  -o count out_fmt [arg0 ... argn]  #   EITHER (data out)
>  -i count in_fmt   #   OR (data in)
> 
> "out_fmt" can be "-" to read output data from stdin;
> "in_fmt" can be "-" to write input data to stdout;
> 
> If debugging is not compiled in the kernel, "-d" will have no effect
> 
> after:
> 
> $ /usr/obj/sbin/scsi/scsi -Z
> scsi: unknown option -- Z
> usage: scsi -f device -d debug_level
>scsi -f device -m page [-e] [-P pc]
>scsi -f device [-v] [-s seconds] -c cmd_fmt [arg ...] -o count out_fmt
> [arg ...] -i count in_fmt [arg ...]
> 
> ok?

ok kettenis@

> Index: sbin/scsi/scsi.c
> ===
> RCS file: /cvs/src/sbin/scsi/scsi.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 scsi.c
> --- sbin/scsi/scsi.c  7 Jun 2016 01:29:38 -   1.30
> +++ sbin/scsi/scsi.c  22 Jun 2021 12:27:10 -
> @@ -84,20 +84,11 @@ static void
>  usage(void)
>  {
>   fprintf(stderr,
> -"Usage:\n"
> -"\n"
> -"  scsi -f device -d debug_level# To set debug level\n"
> -"  scsi -f device -m page [-P pc]   # To read mode pages\n"
> -"  scsi -f device [-v] [-s seconds] -c cmd_fmt [arg0 ... argn] # A 
> command...\n"
> -" -o count out_fmt [arg0 ... argn]  #   EITHER (data out)\n"
> -" -i count in_fmt   #   OR (data in)\n"
> -"\n"
> -"\"out_fmt\" can be \"-\" to read output data from stdin;\n"
> -"\"in_fmt\" can be \"-\" to write input data to stdout;\n"
> -"\n"
> -"If debugging is not compiled in the kernel, \"-d\" will have no effect\n"
> -
> -);
> +"usage: scsi -f device -d debug_level\n"
> +"   scsi -f device -m page [-e] [-P pc]\n"
> +"   scsi -f device [-v] [-s seconds] -c cmd_fmt [arg ...]"
> +" -o count out_fmt\n"
> +"[arg ...] -i count in_fmt [arg ...]\n");
>  
>   exit (1);
>  }
> 
>

ypmatch(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in ypmatch(1) usage. before:

$ /usr/bin/ypmatch -Z
ypmatch: unknown option -- Z
usage: ypmatch [-kt] [-d domain] key ... mapname
   ypmatch -x
where
mapname may be either a mapname or a nickname for a map.
-k prints keys as well as values.
-t inhibits map nickname translation.
-x dumps the map nickname translation table.

after:

$ /usr/obj/usr.bin/ypmatch/ypmatch -Z
ypmatch: unknown option -- Z
usage: ypmatch [-kt] [-d domain] key ... mapname
   ypmatch -x

ok?
jmc

Index: usr.bin/ypmatch/ypmatch.c
===
RCS file: /cvs/src/usr.bin/ypmatch/ypmatch.c,v
retrieving revision 1.16
diff -u -p -r1.16 ypmatch.c
--- usr.bin/ypmatch/ypmatch.c   8 Feb 2015 23:40:35 -   1.16
+++ usr.bin/ypmatch/ypmatch.c   22 Jun 2021 12:44:03 -
@@ -61,12 +61,7 @@ usage(void)
fprintf(stderr,
"usage: ypmatch [-kt] [-d domain] key ... mapname\n"
"   ypmatch -x\n");
-   fprintf(stderr,
-   "where\n"
-   "\tmapname may be either a mapname or a nickname for a map.\n"
-   "\t-k prints keys as well as values.\n"
-   "\t-t inhibits map nickname translation.\n"
-   "\t-x dumps the map nickname translation table.\n");
+
exit(1);
 }

rdist(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
hi.

diff to reduce verbosity in rdist(1) usage. before:

$ /usr/bin/rdist -Z
rdist: unknown option -- Z
usage: rdist [-DFnV] [-A num] [-a num] [-c mini_distfile]
[-d var=value] [-f distfile] [-L remote_logopts] [-l local_logopts]
[-M maxproc] [-m host] [-o distopts] [-P rsh-path] [-p rdistd-path]
[-t timeout] [name ...]

The values for  are:

chknfs,chkreadonly,chksym,compare,defgroup,defowner,follow,history,ignlnks,nochkgroup,nochkmode,nochkowner,nodescend,noexec,numchkgroup,numchkowner,quiet,remove,savetargets,sparse,updateperm,verify,whole,younger

Where  is of form
=,,...:=,...
Valid  names: stdout file syslog notify
Valid  names: change info notice nerror ferror warning verbose all debug

after:

$ /usr/obj/usr.bin/rdist -Z
rdist: unknown option -- Z
usage: rdist [-DFnV] [-A num] [-a num] [-c mini_distfile] [-d var=value]
[-f distfile] [-L remote_logopts] [-l local_logopts] [-M maxproc]
[-m host] [-o distopts] [-P rsh-path] [-p rdistd-path] [-t timeout]
[name ...]

this one is more problematic. it uses two functions, getdistoptlist and
msgprusage, which i think are only used within usage. so i killed them
too.

ok?
jmc

Index: usr.bin/rdist/client.h
===
RCS file: /cvs/src/usr.bin/rdist/client.h,v
retrieving revision 1.3
diff -u -p -r1.3 client.h
--- usr.bin/rdist/client.h  30 Aug 2017 07:42:52 -  1.3
+++ usr.bin/rdist/client.h  22 Jun 2021 12:54:54 -
@@ -161,7 +161,6 @@ int install(char *, char *, int, int , o
 
 /* distopt.c */
 int parsedistopts(char *, opt_t *, int);
-char *getdistoptlist(void);
 char *getondistoptlist(opt_t);
 
 /* docmd.c */
Index: usr.bin/rdist/defs.h
===
RCS file: /cvs/src/usr.bin/rdist/defs.h,v
retrieving revision 1.38
diff -u -p -r1.38 defs.h
--- usr.bin/rdist/defs.h21 Sep 2018 19:00:45 -  1.38
+++ usr.bin/rdist/defs.h22 Jun 2021 12:54:54 -
@@ -206,7 +206,6 @@ char *xbasename(char *);
 char *searchpath(char *);
 
 /* message.c */
-void msgprusage(void);
 void msgprconfig(void);
 char *msgparseopts(char *, int);
 void checkhostname(void);
Index: usr.bin/rdist/distopt.c
===
RCS file: /cvs/src/usr.bin/rdist/distopt.c,v
retrieving revision 1.13
diff -u -p -r1.13 distopt.c
--- usr.bin/rdist/distopt.c 20 Jan 2015 09:00:16 -  1.13
+++ usr.bin/rdist/distopt.c 22 Jun 2021 12:54:54 -
@@ -134,27 +134,6 @@ parsedistopts(char *str, opt_t *optptr, 
 }
 
 /*
- * Get a list of the Distfile Option Entries.
- */
-char *
-getdistoptlist(void)
-{
-   int i;
-   static char buf[1024];
-
-   for (i = 0, buf[0] = CNULL; distoptinfo[i].do_name; ++i) {
-   if (buf[0] == CNULL)
-   (void) strlcpy(buf, distoptinfo[i].do_name, sizeof buf);
-   else {
-   (void) strlcat(buf, ",", sizeof buf);
-   (void) strlcat(buf, distoptinfo[i].do_name, sizeof buf);
-   }
-   }
-
-   return(buf);
-}
-
-/*
  * Get a list of the Distfile Option Entries for each enabled 
  * value in "opts".
  */
Index: usr.bin/rdist/message.c
===
RCS file: /cvs/src/usr.bin/rdist/message.c,v
retrieving revision 1.29
diff -u -p -r1.29 message.c
--- usr.bin/rdist/message.c 28 Jun 2019 05:35:35 -  1.29
+++ usr.bin/rdist/message.c 22 Jun 2021 12:54:54 -
@@ -118,30 +118,6 @@ static void _error(const char *);
 static void _fatalerr(const char *);
 
 /*
- * Print message logging usage message
- */
-void
-msgprusage(void)
-{
-   int i, x;
-
-   (void) fprintf(stderr, "\nWhere  is of form\n");
-   (void) fprintf(stderr, 
-   "\t=,,...:=,...\n");
-
-   (void) fprintf(stderr, "Valid  names:");
-
-   for (i = 0; msgfacility[i].mf_name; ++i)
-   (void) fprintf(stderr, " %s", msgfacility[i].mf_name);
-
-   (void) fprintf(stderr, "\nValid  names:");
-   for (x = 0; msgtypes[x].mt_name; ++x)
-   (void) fprintf(stderr, " %s", msgtypes[x].mt_name);
-
-   (void) fprintf(stderr, "\n");
-}
-
-/*
  * Print enabled message logging info
  */
 void
Index: usr.bin/rdist/rdist.c
===
RCS file: /cvs/src/usr.bin/rdist/rdist.c,v
retrieving revision 1.31
diff -u -p -r1.31 rdist.c
--- usr.bin/rdist/rdist.c   9 Jul 2017 14:04:50 -   1.31
+++ usr.bin/rdist/rdist.c   22 Jun 2021 12:54:54 -
@@ -337,19 +337,13 @@ usage(void)
extern char *__progname;
 
(void) fprintf(stderr,
-   "usage: %s [-DFnV] [-A num] [-a num] "
-   "[-c mini_distfile]\n"
-   "\t[-d var=value] [-f distfile] [-L remote_logopts] "
-   "[-l local_logopts]\n"
-   "\t[-M

Re: tset(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
On Tue, Jun 22, 2021 at 02:19:32PM +, Klemens Nanni wrote:
> On Tue, Jun 22, 2021 at 01:47:13PM +0100, Jason McIntyre wrote:
> > after:
> > 
> > $ /usr/obj/usr.bin/tset/tset -Z
> > tset: unknown option -- Z
> > usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] [terminal]
> 
> OK kn
> 

thanks. i noticed sth else though:

$ reset -Z
reset: unknown option -- Z
usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] [terminal]

can we change it so that when you run tset as reset, the usage name
matches? i thought it would do that already but see now it does not. it
uses a weird _nc_progname argument in usage, which i do not understand
at all.

could it be easily fixed?

thanks,
jmc

Index: tset.c
===
RCS file: /cvs/src/usr.bin/tset/tset.c,v
retrieving revision 1.41
diff -u -p -r1.41 tset.c
--- tset.c  28 Jun 2019 13:35:05 -  1.41
+++ tset.c  22 Jun 2021 14:56:35 -
@@ -1094,28 +1094,9 @@ obsolete(char **argv)
 static void
 usage(void)
 {
-static const char *tbl[] =
-{
-   ""
-   ,"Options:"
-   ,"  -c  set control characters"
-   ,"  -e ch   erase character"
-   ,"  -I  no initialization strings"
-   ,"  -i ch   interrupt character"
-   ,"  -k ch   kill character"
-   ,"  -m mapping  map identifier to type"
-   ,"  -Q  do not output control key settings"
-   ,"  -r  display term on stderr"
-   ,"  -s  output TERM set command"
-   ,"  -V  print curses-version"
-   ,"  -w  set window-size"
-};
-unsigned n;
-(void) fprintf(stderr, "Usage: %s [-cIQqrsVw] [-] "
-   "[-e ch] [-i ch] [-k ch] [-m mapping] [terminal]\n",
+(void) fprintf(stderr, "usage: %s [-cIQqrsVw] [-] "
+   "[-e ch] [-i ch] [-k ch] [-m mapping] [terminal]",
_nc_progname);
-for (n = 0; n < sizeof(tbl) / sizeof(tbl[0]); ++n)
-   fprintf(stderr, "%s\n", tbl[n]);
 
 exit_error();
 /* NOTREACHED */

Re: tset(1): reduce usage()

2021-06-22 Thread Klemens Nanni 
On Tue, Jun 22, 2021 at 03:57:08PM +0100, Jason McIntyre wrote:
> On Tue, Jun 22, 2021 at 02:19:32PM +, Klemens Nanni wrote:
> > On Tue, Jun 22, 2021 at 01:47:13PM +0100, Jason McIntyre wrote:
> > > after:
> > > 
> > > $ /usr/obj/usr.bin/tset/tset -Z
> > > tset: unknown option -- Z
> > > usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] 
> > > [terminal]
> > 
> > OK kn
> > 
> 
> thanks. i noticed sth else though:
> 
> $ reset -Z
> reset: unknown option -- Z
> usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] [terminal]
> 
> can we change it so that when you run tset as reset, the usage name
> matches? i thought it would do that already but see now it does not. it
> uses a weird _nc_progname argument in usage, which i do not understand
> at all.
Right, this is using /usr/src/lib/libcurses/tinfo/access.c
_nc_rootname() here.

> could it be easily fixed?
I see no reason to not use getprogname(3) in usage(), with that our
usage will always match the getopt(3) warning for `-Z'.

iwm/iwx: fix mac context task synchronization

2021-06-22 Thread Stefan Sperling 
The mac context task gets triggered by changing parameters in beacons
received from our AP while ic_state indicates that we are in RUN state.
This can happen during regular operation and while a background scan
is in progress.

When a background scan has found a new AP, we trigger a RUN -> AUTH state
change by scheduling the newstate task. Any other driver tasks which are
pending are removed from the task queue now, before the newstate task is
inserted. So a pending mac context task will be removed at this point.
However, there is a chance that a mac context task gets inserted into
the task queue again, after the newstate task.

The newstate task has been scheduled, and when it runs it will issue a
long series of firmware commands to perform the state change, which involves
waiting for command responses. While this is going on the ic_state variable
still indicates that we are in RUN state, and any beacon we receive now could
contain a changed parameter and cause a mac context task to be scheduled.

Then, after completing the switch into AUTH state the scheduled mac context
task runs and sends its command to the firmware. This results in a fatal
firmware error along with the message "could not update MAC context" from
the driver.

We can prevent this problem by making the mac context task send its
command only if we are still in RUN state when the task gets to run.

Additionally, avoid scheduling a mac context task if a pending newstate
task is going to move us out of RUN state anyway (this part is just a
tiny optimization, it doesn't fix the bug).

Beacons are usually sent every 100ms, but parameters change less often.
Whether this issue triggers ultimately depends on timing and the behaviour
of the AP.

Issue debugged by zxystd in OpenIntelWireless itlwm; patch by me.

ok?
 
diff 18a81a05d6e3098eadc4b5e23d6656377f87591a /usr/src
blob - e33fe39b9784c5bd4f8b28099af9f97c04e49c65
file + sys/dev/pci/if_iwm.c
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -3223,7 +3223,8 @@ iwm_mac_ctxt_task(void *arg)
struct iwm_node *in = (void *)ic->ic_bss;
int err, s = splnet();
 
-   if (sc->sc_flags & IWM_FLAG_SHUTDOWN) {
+   if ((sc->sc_flags & IWM_FLAG_SHUTDOWN) ||
+   ic->ic_state != IEEE80211_S_RUN) {
refcnt_rele_wake(>task_refs);
splx(s);
return;
@@ -3242,7 +3243,8 @@ iwm_updateprot(struct ieee80211com *ic)
 {
struct iwm_softc *sc = ic->ic_softc;
 
-   if (ic->ic_state == IEEE80211_S_RUN)
+   if (ic->ic_state == IEEE80211_S_RUN &&
+   !task_pending(>newstate_task))
iwm_add_task(sc, systq, >mac_ctxt_task);
 }
 
@@ -3251,7 +3253,8 @@ iwm_updateslot(struct ieee80211com *ic)
 {
struct iwm_softc *sc = ic->ic_softc;
 
-   if (ic->ic_state == IEEE80211_S_RUN)
+   if (ic->ic_state == IEEE80211_S_RUN &&
+   !task_pending(>newstate_task))
iwm_add_task(sc, systq, >mac_ctxt_task);
 }
 
@@ -3260,7 +3263,8 @@ iwm_updateedca(struct ieee80211com *ic)
 {
struct iwm_softc *sc = ic->ic_softc;
 
-   if (ic->ic_state == IEEE80211_S_RUN)
+   if (ic->ic_state == IEEE80211_S_RUN &&
+   !task_pending(>newstate_task))
iwm_add_task(sc, systq, >mac_ctxt_task);
 }
 
blob - 21bc8be34500730264ad206513951a32fcb2d702
file + sys/dev/pci/if_iwx.c
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -2947,7 +2947,8 @@ iwx_mac_ctxt_task(void *arg)
struct iwx_node *in = (void *)ic->ic_bss;
int err, s = splnet();
 
-   if (sc->sc_flags & IWX_FLAG_SHUTDOWN) {
+   if ((sc->sc_flags & IWX_FLAG_SHUTDOWN) ||
+   ic->ic_state != IEEE80211_S_RUN) {
refcnt_rele_wake(>task_refs);
splx(s);
return;
@@ -2966,7 +2967,8 @@ iwx_updateprot(struct ieee80211com *ic)
 {
struct iwx_softc *sc = ic->ic_softc;
 
-   if (ic->ic_state == IEEE80211_S_RUN)
+   if (ic->ic_state == IEEE80211_S_RUN &&
+   !task_pending(>newstate_task))
iwx_add_task(sc, systq, >mac_ctxt_task);
 }
 
@@ -2975,7 +2977,8 @@ iwx_updateslot(struct ieee80211com *ic)
 {
struct iwx_softc *sc = ic->ic_softc;
 
-   if (ic->ic_state == IEEE80211_S_RUN)
+   if (ic->ic_state == IEEE80211_S_RUN &&
+   !task_pending(>newstate_task))
iwx_add_task(sc, systq, >mac_ctxt_task);
 }
 
@@ -2984,7 +2987,8 @@ iwx_updateedca(struct ieee80211com *ic)
 {
struct iwx_softc *sc = ic->ic_softc;
 
-   if (ic->ic_state == IEEE80211_S_RUN)
+   if (ic->ic_state == IEEE80211_S_RUN &&
+   !task_pending(>newstate_task))
iwx_add_task(sc, systq, >mac_ctxt_task);
 }

Re: tset(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
On Tue, Jun 22, 2021 at 03:22:34PM +, Klemens Nanni wrote:
> On Tue, Jun 22, 2021 at 03:57:08PM +0100, Jason McIntyre wrote:
> > On Tue, Jun 22, 2021 at 02:19:32PM +, Klemens Nanni wrote:
> > > On Tue, Jun 22, 2021 at 01:47:13PM +0100, Jason McIntyre wrote:
> > > > after:
> > > > 
> > > > $ /usr/obj/usr.bin/tset/tset -Z
> > > > tset: unknown option -- Z
> > > > usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] 
> > > > [terminal]
> > > 
> > > OK kn
> > > 
> > 
> > thanks. i noticed sth else though:
> > 
> > $ reset -Z
> > reset: unknown option -- Z
> > usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] [terminal]
> > 
> > can we change it so that when you run tset as reset, the usage name
> > matches? i thought it would do that already but see now it does not. it
> > uses a weird _nc_progname argument in usage, which i do not understand
> > at all.
> Right, this is using /usr/src/lib/libcurses/tinfo/access.c
> _nc_rootname() here.
> 
> > could it be easily fixed?
> I see no reason to not use getprogname(3) in usage(), with that our
> usage will always match the getopt(3) warning for `-Z'.
> 

would you mind tweaking my diff to do that, please? then commit with my
ok.

thanks!
jmc

Re: tls_load_file.3: tls_config_set_*_file() load files into memory

2021-06-22 Thread Jason McIntyre 
On Tue, Jun 22, 2021 at 01:29:59PM +, Klemens Nanni wrote:
> On Tue, Jun 22, 2021 at 06:35:44AM +0100, Jason McIntyre wrote:
> > > -sets the files from which the public certificate, and private key will 
> > > be read.
> > > +loads two files from which the public certificate, and private key will 
> > > be read.
> > 
> > this is a weird place for a comma. i would remove it.
> > jmc
> 
> Agreed, but all the other sentences below have it as well so I left it
> to keep my diff simple.  I'm sure this manual can be polished further
> afterwards.
> 

that is because they are lists of more than two items. putting a comma
in a list with two items is wrong (well, that's a simplification, but
it's wrong in your diff).

jmc

> > >  .Pp
> > >  .Fn tls_config_set_keypair_mem
> > >  directly sets the public certificate, and private key from memory.
> > >  .Pp
> > >  .Fn tls_config_set_keypair_ocsp_file
> > > -sets the files from which the public certificate, private key, and 
> > > DER-encoded
> > > -OCSP staple will be read.
> > > +loads three files containing the public certificate, private key, and 
> > > DER-encoded
> > > +OCSP staple.
> > >  .Pp
> > >  .Fn tls_config_set_keypair_ocsp_mem
> > >  directly sets the public certificate, private key, and DER-encoded OCSP 
> > > staple
> > > 
> > 
>

Re: lex(1): reduce usage()

2021-06-22 Thread Theo de Raadt 
Jason McIntyre  wrote:

> hi.
> 
> diff to reduce verbosity in flex/lex usage. before:

This seems better.  Rather than telling people to run --help, which is
now usage(), just run usage().  Also usage() does not need to exit(),
since all calls to usage() are followed by FLEX_EXIT().

[This is a crusty kind of old code from a time when people believed the
simplest things required abstraction...]

Normally we don't maintain diffs to code from outside, but I think we carry
a few diffs for lex already.

Index: main.c
===
RCS file: /cvs/src/usr.bin/lex/main.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 main.c
--- main.c  21 Jan 2017 08:33:07 -  1.27
+++ main.c  22 Jun 2021 13:46:50 -
@@ -977,14 +977,7 @@ flexinit(argc, argv)
while ((rv = scanopt(sopt, , )) != 0) {
 
if (rv < 0) {
-   /*
-* Scanopt has already printed an option-specific
-* error message.
-*/
-   fprintf(stderr,
-   _
-   ("Try `%s --help' for more information.\n"),
-   program_name);
+   usage();
FLEX_EXIT(1);
}
switch ((enum flexopt_flag_t) rv) {
@@ -1740,71 +1733,12 @@ basename2(path, strip_ext)
 }
 
 void
-usage()
+usage(void)
 {
-   FILE *f = stdout;
-
-   if (!did_outfilename) {
-   snprintf(outfile_path, sizeof(outfile_path), outfile_template,
-   prefix, C_plus_plus ? "cc" : "c");
-   outfilename = outfile_path;
-   }
-   fprintf(f, _("Usage: %s [OPTIONS] [FILE]...\n"), program_name);
-   fprintf(f,
-   _
-   ("Generates programs that perform pattern-matching on text.\n"
-   "\n" "Table Compression:\n"
-   "  -Ca, --align  trade off larger tables for better memory 
alignment\n"
-   "  -Ce, --ecsconstruct equivalence classes\n"
-   "  -Cf   do not compress tables; use -f 
representation\n"
-   "  -CF   do not compress tables; use -F 
representation\n"
-   "  -Cm, --meta-ecs   construct meta-equivalence classes\n"
-   "  -Cr, --read   use read() instead of stdio for scanner 
input\n"
-   "  -f, --fullgenerate fast, large scanner. Same as 
-Cfr\n"
-   "  -F, --fastuse alternate table representation. Same 
as -CFr\n"
-   "  -Cem  default compression (same as --ecs 
--meta-ecs)\n"
-   "\n" "Debugging:\n"
-   "  -d, --debug enable debug mode in scanner\n"
-   "  -b, --backupwrite backing-up information to %s\n"
-   "  -p, --perf-report   write performance report to stderr\n"
-   "  -s, --nodefault suppress default rule to ECHO 
unmatched text\n"
-   "  -T, --trace %s should run in trace mode\n"
-   "  -w, --nowarndo not generate warnings\n"
-   "  -v, --verbose   write summary of scanner statistics 
to stdout\n"
-   "\n" "Files:\n"
-   "  -o, --outfile=FILE  specify output filename\n"
-   "  -S, --skel=FILE specify skeleton file\n"
-   "  -t, --stdoutwrite scanner on stdout instead of 
%s\n"
-   "  --yyclass=NAME  name of C++ class\n"
-   "  --header-file=FILE   create a C header file in addition 
to the scanner\n"
-   "  --tables-file[=FILE] write tables to FILE\n" "\n"
-   "Scanner behavior:\n"
-   "  -7, --7bit  generate 7-bit scanner\n"
-   "  -8, --8bit  generate 8-bit scanner\n"
-   "  -B, --batch generate batch scanner (opposite of 
-I)\n"
-   "  -i, --case-insensitive  ignore case in patterns\n"
-   "  -l, --lex-compatmaximal compatibility with original 
lex\n"
-   "  -X, --posix-compat  maximal compatibility with POSIX 
lex\n"
-   "  -I, --interactive   generate interactive scanner 
(opposite of -B)\n"
-   "  --yylineno  track line count in yylineno\n"
-   "\n" "Generated code:\n"
-   "  -+,  --c++   generate C++ scanner class\n"
-   "  -Dmacro[=defn]   #define macro defn  (default defn 
is '1')\n"
-   "  -L,  --nolinesuppress #line directives in 
scanner\n"
-   "  -P,  --prefix=STRING use STRING as prefix instead of 
\"yy\"\n"
-   "  -R,  --reentrant generate a reentrant C scanner\n"
-   "   --bison-bridge  scanner for bison

Re: tset(1): reduce usage()

2021-06-22 Thread Klemens Nanni 
On Tue, Jun 22, 2021 at 01:47:13PM +0100, Jason McIntyre wrote:
> after:
> 
> $ /usr/obj/usr.bin/tset/tset -Z
> tset: unknown option -- Z
> usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] [terminal]

OK kn

Re: amd64: softintr_dispatch: remove kernel lock

2021-06-22 Thread Scott Cheloha 
On Mon, Jun 21, 2021 at 02:04:30PM +, Visa Hankala wrote:
> On Thu, May 27, 2021 at 07:40:26PM -0500, Scott Cheloha wrote:
> > On Sun, May 23, 2021 at 09:05:24AM +, Visa Hankala wrote:
> > > When a CPU starts processing a soft interrupt, it reserves the handler
> > > to prevent concurrent execution. If the soft interrupt gets rescheduled
> > > during processing, the handler is run again by the same CPU. This breaks
> > > FIFO ordering, though.
> > 
> > If you want to preserve FIFO you can reinsert the handler at the queue
> > tail.  That would be more fair.
> > 
> > If FIFO is the current behavior I think we ought to keep it.
> 
> I have updated the patch to preserve the FIFO order.
> 
> > > +STAILQ_HEAD(x86_soft_intr_queue, x86_soft_intrhand);
> > > +
> > > +struct x86_soft_intr_queue softintr_queue[X86_NSOFTINTR];
> > 
> > Why did we switch to STAILQ?  I know we don't have very many
> > softintr_disestablish() calls but isn't O(1) removal worth the extra
> > pointer?
> 
> I used STAILQ because it avoids the hassle of updating the list nodes'
> back pointers. softintr_disestablish() with multiple items pending in
> the queue is very rare in comparison to the normal softintr_schedule() /
> softintr_dispatch() cycle.
> 
> However, I have changed the code back to using TAILQ.

This looks good to me.  I mean, it looked good before, but it still
looks good.

I will run with it for a few days.

Assuming I hit no issues I'll come back with an OK.

Is there an easy way to exercise this code from userspace?  There
aren't many softintr users.

Maybe audio drivers?

Re: lex(1): reduce usage()

2021-06-22 Thread Jason McIntyre 
On Tue, Jun 22, 2021 at 07:50:10AM -0600, Theo de Raadt wrote:
> Jason McIntyre  wrote:
> 
> > hi.
> > 
> > diff to reduce verbosity in flex/lex usage. before:
> 
> This seems better.  Rather than telling people to run --help, which is
> now usage(), just run usage().  Also usage() does not need to exit(),
> since all calls to usage() are followed by FLEX_EXIT().
> 
> [This is a crusty kind of old code from a time when people believed the
> simplest things required abstraction...]
> 
> Normally we don't maintain diffs to code from outside, but I think we carry
> a few diffs for lex already.
> 

please go ahead. i was reluctant to touch some of the more complex ones
anyway.

jmc

> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/lex/main.c,v
> retrieving revision 1.27
> diff -u -p -u -r1.27 main.c
> --- main.c21 Jan 2017 08:33:07 -  1.27
> +++ main.c22 Jun 2021 13:46:50 -
> @@ -977,14 +977,7 @@ flexinit(argc, argv)
>   while ((rv = scanopt(sopt, , )) != 0) {
>  
>   if (rv < 0) {
> - /*
> -  * Scanopt has already printed an option-specific
> -  * error message.
> -  */
> - fprintf(stderr,
> - _
> - ("Try `%s --help' for more information.\n"),
> - program_name);
> + usage();
>   FLEX_EXIT(1);
>   }
>   switch ((enum flexopt_flag_t) rv) {
> @@ -1740,71 +1733,12 @@ basename2(path, strip_ext)
>  }
>  
>  void
> -usage()
> +usage(void)
>  {
> - FILE *f = stdout;
> -
> - if (!did_outfilename) {
> - snprintf(outfile_path, sizeof(outfile_path), outfile_template,
> - prefix, C_plus_plus ? "cc" : "c");
> - outfilename = outfile_path;
> - }
> - fprintf(f, _("Usage: %s [OPTIONS] [FILE]...\n"), program_name);
> - fprintf(f,
> - _
> - ("Generates programs that perform pattern-matching on text.\n"
> - "\n" "Table Compression:\n"
> - "  -Ca, --align  trade off larger tables for better memory 
> alignment\n"
> - "  -Ce, --ecsconstruct equivalence classes\n"
> - "  -Cf   do not compress tables; use -f 
> representation\n"
> - "  -CF   do not compress tables; use -F 
> representation\n"
> - "  -Cm, --meta-ecs   construct meta-equivalence classes\n"
> - "  -Cr, --read   use read() instead of stdio for scanner 
> input\n"
> - "  -f, --fullgenerate fast, large scanner. Same as 
> -Cfr\n"
> - "  -F, --fastuse alternate table representation. Same 
> as -CFr\n"
> - "  -Cem  default compression (same as --ecs 
> --meta-ecs)\n"
> - "\n" "Debugging:\n"
> - "  -d, --debug enable debug mode in scanner\n"
> - "  -b, --backupwrite backing-up information to %s\n"
> - "  -p, --perf-report   write performance report to stderr\n"
> - "  -s, --nodefault suppress default rule to ECHO 
> unmatched text\n"
> - "  -T, --trace %s should run in trace mode\n"
> - "  -w, --nowarndo not generate warnings\n"
> - "  -v, --verbose   write summary of scanner statistics 
> to stdout\n"
> - "\n" "Files:\n"
> - "  -o, --outfile=FILE  specify output filename\n"
> - "  -S, --skel=FILE specify skeleton file\n"
> - "  -t, --stdoutwrite scanner on stdout instead of 
> %s\n"
> - "  --yyclass=NAME  name of C++ class\n"
> - "  --header-file=FILE   create a C header file in addition 
> to the scanner\n"
> - "  --tables-file[=FILE] write tables to FILE\n" "\n"
> - "Scanner behavior:\n"
> - "  -7, --7bit  generate 7-bit scanner\n"
> - "  -8, --8bit  generate 8-bit scanner\n"
> - "  -B, --batch generate batch scanner (opposite of 
> -I)\n"
> - "  -i, --case-insensitive  ignore case in patterns\n"
> - "  -l, --lex-compatmaximal compatibility with original 
> lex\n"
> - "  -X, --posix-compat  maximal compatibility with POSIX 
> lex\n"
> - "  -I, --interactive   generate interactive scanner 
> (opposite of -B)\n"
> - "  --yylineno  track line count in yylineno\n"
> - "\n" "Generated code:\n"
> - "  -+,  --c++   generate C++ scanner class\n"
> - "  -Dmacro[=defn]   #define macro defn  (default defn 
> is '1')\n"
> - "  -L,  --nolinesuppress #line

Re: mkuboot(8): reduce usage()

2021-06-22 Thread Mark Kettenis 
> Date: Tue, 22 Jun 2021 13:36:47 +0100
> From: Jason McIntyre 
> 
> hi.
> 
> diff to reduce verbosity in mkuboot(8) usage. i don;t have the means to
> build this one.
> 
> ok?

ok kettenis@

> Index: mkuboot.c
> ===
> RCS file: /cvs/src/usr.sbin/mkuboot/mkuboot.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 mkuboot.c
> --- mkuboot.c 1 Jun 2021 02:59:01 -   1.10
> +++ mkuboot.c 22 Jun 2021 12:36:08 -
> @@ -395,16 +395,6 @@ usage(void)
>   (void)fprintf(stderr,
>   "usage: %s [-a arch] [-e entry] [-l loadaddr] [-n name] [-o os] "
>   "[-t type] infile outfile\n", __progname);
> - (void)fprintf(stderr,
> - "arch is one of:");
> - for (mapptr = archmap; mapptr->arch; mapptr++)
> - (void)fprintf(stderr, " %s", mapptr->arch);
> - (void)fprintf(stderr, "\n");
> - (void)fprintf(stderr,
> - "os is one of:");
> - for (osmapptr = osmap; osmapptr->arch; osmapptr++)
> - (void)fprintf(stderr, " %s", osmapptr->arch);
> - (void)fprintf(stderr, "\n");
>   
>   exit(1);
>  }
> 
>

Re: tset(1): reduce usage()

2021-06-22 Thread Theo Buehler 
On Tue, Jun 22, 2021 at 03:22:34PM +, Klemens Nanni wrote:
> On Tue, Jun 22, 2021 at 03:57:08PM +0100, Jason McIntyre wrote:
> > On Tue, Jun 22, 2021 at 02:19:32PM +, Klemens Nanni wrote:
> > > On Tue, Jun 22, 2021 at 01:47:13PM +0100, Jason McIntyre wrote:
> > > > after:
> > > > 
> > > > $ /usr/obj/usr.bin/tset/tset -Z
> > > > tset: unknown option -- Z
> > > > usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] 
> > > > [terminal]
> > > 
> > > OK kn
> > > 
> > 
> > thanks. i noticed sth else though:
> > 
> > $ reset -Z
> > reset: unknown option -- Z
> > usage: tset [-cIQqrsVw] [-] [-e ch] [-i ch] [-k ch] [-m mapping] [terminal]
> > 
> > can we change it so that when you run tset as reset, the usage name
> > matches? i thought it would do that already but see now it does not. it
> > uses a weird _nc_progname argument in usage, which i do not understand
> > at all.
> Right, this is using /usr/src/lib/libcurses/tinfo/access.c
> _nc_rootname() here.
> 
> > could it be easily fixed?
> I see no reason to not use getprogname(3) in usage(), with that our
> usage will always match the getopt(3) warning for `-Z'.
> 

I think we should pull the assignment up. Our execve(2) guarantees that
argc >= 1, so it's safe to do *argv. We need to do this before calling
the internal err() the first time as that uses _nc_progname() internally.

Index: tset.c
===
RCS file: /cvs/src/usr.bin/tset/tset.c,v
retrieving revision 1.41
diff -u -p -r1.41 tset.c
--- tset.c  28 Jun 2019 13:35:05 -  1.41
+++ tset.c  22 Jun 2021 15:50:44 -
@@ -110,7 +110,7 @@ extern char **environ;
 #undef CTRL
 #define CTRL(x)((x) & 0x1f)
 
-const char *_nc_progname = "tset";
+const char *_nc_progname;
 
 static TTY mode, oldmode, original;
 
@@ -1094,28 +1094,9 @@ obsolete(char **argv)
 static void
 usage(void)
 {
-static const char *tbl[] =
-{
-   ""
-   ,"Options:"
-   ,"  -c  set control characters"
-   ,"  -e ch   erase character"
-   ,"  -I  no initialization strings"
-   ,"  -i ch   interrupt character"
-   ,"  -k ch   kill character"
-   ,"  -m mapping  map identifier to type"
-   ,"  -Q  do not output control key settings"
-   ,"  -r  display term on stderr"
-   ,"  -s  output TERM set command"
-   ,"  -V  print curses-version"
-   ,"  -w  set window-size"
-};
-unsigned n;
-(void) fprintf(stderr, "Usage: %s [-cIQqrsVw] [-] "
-   "[-e ch] [-i ch] [-k ch] [-m mapping] [terminal]\n",
+(void) fprintf(stderr, "usage: %s [-cIQqrsVw] [-] "
+   "[-e ch] [-i ch] [-k ch] [-m mapping] [terminal]",
_nc_progname);
-for (n = 0; n < sizeof(tbl) / sizeof(tbl[0]); ++n)
-   fprintf(stderr, "%s\n", tbl[n]);
 
 exit_error();
 /* NOTREACHED */
@@ -1136,6 +1117,8 @@ main(int argc, char **argv)
 const char *p;
 const char *ttype;
 
+_nc_progname = _nc_rootname(*argv);
+
 if (pledge("stdio rpath wpath tty", NULL) == -1)
err("pledge: %s", strerror(errno));
 
@@ -1198,8 +1181,6 @@ main(int argc, char **argv)
usage();
}
 }
-
-_nc_progname = _nc_rootname(*argv);
 argc -= optind;
 argv += optind;