Re: args vs arg ... in manuals

2022-12-20 Thread Jason McIntyre
On Wed, Dec 21, 2022 at 07:40:01AM +, Klemens Nanni wrote:
> On Tue, Dec 20, 2022 at 08:24:29PM +, Jason McIntyre wrote:
> > On Tue, Dec 20, 2022 at 06:24:45PM +, Klemens Nanni wrote:
> > > 
> > > Feedback? OK?
> > > 
> > 
> > ok, in general i'm fine with this. i do have some concerns though:
> 
> New diff with all points addressed, see inline for explanations.
> 

trimming this, but briefly: ok for your diff. i think this is fine. but
still one nit:

> Index: usr.bin/csplit/csplit.1
> ===
> RCS file: /cvs/src/usr.bin/csplit/csplit.1,v
> retrieving revision 1.12
> diff -u -p -r1.12 csplit.1
> --- usr.bin/csplit/csplit.1   24 Oct 2015 15:32:50 -  1.12
> +++ usr.bin/csplit/csplit.1   21 Dec 2022 06:56:23 -
> @@ -37,14 +37,15 @@
>  .Op Fl ks
>  .Op Fl f Ar prefix
>  .Op Fl n Ar number
> -.Ar file args ...
> +.Ar file
> +.Ar arg ...
>  .Sh DESCRIPTION
>  The
>  .Nm
>  utility splits
>  .Ar file
> -into pieces using the patterns
> -.Ar args .
> +into pieces using the pattern
> +.Ar arg .
>  If
>  .Ar file
>  is
> @@ -92,7 +93,7 @@ created.
>  .El
>  .Pp
>  The
> -.Ar args
> +.Ar arg
>  operands may be a combination of the following patterns:
>  .Bl -tag -width indent
>  .It Xo

so, is "arg" an operand or operands? well, it depends how you are
thinking of it. i would suggest we treat it as singular - it will read
better, and i don;t think anyone will read this text and complain that
it was misleading.

jmc



Re: args vs arg ... in manuals

2022-12-20 Thread Klemens Nanni
On Tue, Dec 20, 2022 at 08:24:29PM +, Jason McIntyre wrote:
> On Tue, Dec 20, 2022 at 06:24:45PM +, Klemens Nanni wrote:
> > 
> > Feedback? OK?
> > 
> 
> ok, in general i'm fine with this. i do have some concerns though:

New diff with all points addressed, see inline for explanations.

Just like the previous diff, this one only tries to fix 'Ar args' into
'Ar arg ...' in SYNOPSIS and DESCRIPTION texts, i.e. all spots marking
up the same argument.

> > Index: bin/expr/expr.1
> > ===
> > RCS file: /cvs/src/bin/expr/expr.1,v
> > retrieving revision 1.24
> > diff -u -p -r1.24 expr.1
> > --- bin/expr/expr.1 16 Aug 2017 20:10:58 -  1.24
> > +++ bin/expr/expr.1 20 Dec 2022 17:31:39 -
> > @@ -12,12 +12,12 @@
> >  .Nd evaluate expression
> >  .Sh SYNOPSIS
> >  .Nm expr
> > -.Ar expression
> > +.Ar expression ...
> >  .Sh DESCRIPTION
> >  The
> >  .Nm
> >  utility evaluates
> > -.Ar expression
> > +.Ar expression Ns s
> >  and writes the result on standard output.
> >  All operators are separate arguments to the
> >  .Nm
> 
> so first off i hate "Ar blah Ns s". i do understand why we might want
> it, but i;d rather avoid it. it can make the text read better, but it
> can also make it read worse (so i don;t think we should just slap it
> on). and the formatting is truly yucky.

No 'Ns s' added now to end markup after the singluar while keeping the
plural "s" withot markup.

Words are either inlined be removing 'Ar' or omitted as deemed unhelpful.

> 
> expr(1) is a good example. i think it doesn;t matter that expr can take
> one arg or many. when we want to descrive it at its basic level, we just
> want to say that it "evalautes expression". jumping through formatting
> hoops to try and indicate that more than one expression is possible is
> pointless. let's try and make the text clear and simple, and let
> formatting come second. here's some ideas:
> 
> - The
>   .Nm
>   utility evaluates
>   .Ar expression
>and writes ...
> 
> in this case we leave the reader to understand that SYNOPSIS and the
> description all clearly show that more than one arg is possible. nice
> and simple. this gets my vote.
> 
> - The
>   .Nm
>   utility evaluates one or more
>   .Ar expression
>and writes ...
> 
> now we are spelling it out. it is less ambiguous than the first example,
> but reads badly (one or more expression)
> 
> - The
>   .Nme
>   utility evaluates one or more expressions and writes ...
> 
> that reads better than the last example, but it misses the chance to
> link "expression" as the argument name, and people will complain.
> 
> i am really ok with any of these. but i don;t like "Ar expression Ns s"
> here because it both reads badly and formats ugly. i'm not saying we
> can't use "Ar .* Ns s", just that we should think about not using it
> willy nilly.

Again, expr(1) behaves differently if given a single argument containing
multiple expressions separated by whitespace or individual arguments on
the shell, so writing ... is important here.

I went with Theo's suggestion for this one.

> > Index: usr.bin/csplit/csplit.1
> > ===
> > RCS file: /cvs/src/usr.bin/csplit/csplit.1,v
> > retrieving revision 1.12
> > diff -u -p -r1.12 csplit.1
> > --- usr.bin/csplit/csplit.1 24 Oct 2015 15:32:50 -  1.12
> > +++ usr.bin/csplit/csplit.1 20 Dec 2022 17:27:24 -
> > @@ -37,14 +37,15 @@
> >  .Op Fl ks
> >  .Op Fl f Ar prefix
> >  .Op Fl n Ar number
> > -.Ar file args ...
> > +.Ar file
> > +.Ar arg ...
> 
> it's no biggie i guess, but it was simpler on one line. what is gained
> by splitting it?

Again, it clarifies that there may be only one 'file' but multiple 'arg' words.

> 
> >  .Sh DESCRIPTION
> >  The
> >  .Nm
> >  utility splits
> >  .Ar file
> >  into pieces using the patterns
> > -.Ar args .
> > +.Ar arg Ns s .
> 
> i think we should just say "the pattern Ar arg".

Done.

> 
> >  If
> >  .Ar file
> >  is
> > @@ -92,7 +93,7 @@ created.
> >  .El
> >  .Pp
> >  The
> > -.Ar args
> > +.Ar arg Ns s
> 
> ditto

Done.

> 
> >  operands may be a combination of the following patterns:
> >  .Bl -tag -width indent
> >  .It Xo

> > Index: usr.bin/mandoc/mandoc.1
> > ===
> > RCS file: /cvs/src/usr.bin/mandoc/mandoc.1,v
> > retrieving revision 1.189
> > diff -u -p -r1.189 mandoc.1
> > --- usr.bin/mandoc/mandoc.1 2 Aug 2022 11:55:51 -   1.189
> > +++ usr.bin/mandoc/mandoc.1 20 Dec 2022 17:58:29 -
> > @@ -765,7 +765,7 @@ Messages displayed by
> >  follow this format:
> >  .Bd -ragged -offset indent
> >  .Nm :
> > -.Ar file : Ns Ar line : Ns Ar column : level : message : macro arguments
> > +.Ar file : Ns Ar line : Ns Ar column : level : message : macro argument ...
> >  .Pq Ar os
> >  .Ed
> >  .Pp
> > @@ -786,7 +786,7 @@ strings are explained 

Re: [patch] add show.c style flag descriptions to route(8)

2022-12-20 Thread Jason McIntyre
On Tue, Dec 20, 2022 at 08:27:14PM -0500, Paul R. Tagliamonte wrote:
> Heyya tech@,
> 
> Please keep me on cc, I'm not subscribed
> 
> I've been working on porting some of my linux specific software to
> work on OpenBSD. While working with some routing-specific code,
> I needed to pull up show.c to map the letters in `route show` to the
> RTF_ values.
> 
> I figured while I was in there, I'd send a patch along adding that to
> the manpage in the hopes it's a bit easier for others in the future.
> Most of the descriptions were pulled from the .h
> 
> First time sending anything in to OpenBSD, please let me know if
> there's anything I can do better for next time! I'm wary of my MUA
> (GMail's web interface) destroying the patch, so I've attached it
> for redundancy.
> 
> paultag
> --
> :wq
> 

hi.

some of the relevant flags are already documented in route(8) ("Routes
have associated flags...). the entire list is documented in route(4),
but you have to explicitly ask for it (man 4 route) and again the flags
with detail in netstat(8) ("The mapping between letters and flags
is...").

jmc

> Index: route.8
> ===
> RCS file: /cvs/src/sbin/route/route.8,v
> retrieving revision 1.106
> diff -u -p -r1.106 route.8
> --- route.8 19 Nov 2022 19:23:37 - 1.106
> +++ route.8 21 Dec 2022 01:11:22 -
> @@ -571,6 +571,37 @@ and
>  .Dv RTM_CHANGE .
>  As such, only the superuser may modify
>  the routing tables.
> +.Sh FLAGS
> +When displaying routes with the
> +.Cm show
> +command, the output contains a
> +.Ar Flags
> +column indicating what flags have been set on the route. The possible
> +values and their corresponding header definition names are:
> +.Bl -column "U" "RTF_BLACKHOLE" "description"
> +.It U Ta Dv RTF_UP Ta "route is usable"
> +.It G Ta Dv RTF_GATEWAYTa "route traffic through a gateway"
> +.It H Ta Pf RTF_HOST   Ta "route target is a host entry"
> +.It R Ta Dv RTF_REJECT Ta "host or net is unreachable"
> +.It D Ta Dv RTF_DYNAMICTa "route created dynamically (by redirect)"
> +.It M Ta Pf RTF_MODIFIED   Ta "route modified dynamically (by redirect)"
> +.It C Ta Dv RTF_CLONINGTa "generate new routes on use"
> +.It m Ta Dv RTF_MULTICAST  Ta "route associated to a multicast addr"
> +.It L Ta Dv RTF_LLINFO Ta "route generated by ARP or NDP"
> +.It S Ta Dv RTF_STATIC Ta "route was manually added"
> +.It B Ta Dv RTF_BLACKHOLE  Ta "discard all traffic"
> +.It 3 Ta Dv RTF_PROTO3 Ta "protocol specific routing flag"
> +.It 2 Ta Dv RTF_PROTO2 Ta "protocol specific routing flag"
> +.It 1 Ta Dv RTF_PROTO1 Ta "protocol specific routing flag"
> +.It c Ta Dv RTF_CLONED Ta "route was cloned"
> +.It h Ta Dv RTF_CACHED Ta "referenced by gateway route"
> +.It P Ta Dv RTF_MPATH  Ta "multipath route"
> +.It T Ta Dv RTF_MPLS   Ta "MPLS route"
> +.It l Ta Dv RTF_LOCAL  Ta "route is local to the host"
> +.It F Ta Dv RTF_BFDTa "link state controlled by BFD"
> +.It b Ta Dv RTF_BROADCAST  Ta "route associated to a broadcast addr"
> +.It n Ta Dv RTF_CONNECTED  Ta "interface route"
> +.El
>  .Sh FILES
>  .Bl -tag -width "/etc/mygate" -compact
>  .It Pa /etc/hosts

> Index: route.8
> ===
> RCS file: /cvs/src/sbin/route/route.8,v
> retrieving revision 1.106
> diff -u -p -r1.106 route.8
> --- route.8   19 Nov 2022 19:23:37 -  1.106
> +++ route.8   21 Dec 2022 01:11:22 -
> @@ -571,6 +571,37 @@ and
>  .Dv RTM_CHANGE .
>  As such, only the superuser may modify
>  the routing tables.
> +.Sh FLAGS
> +When displaying routes with the
> +.Cm show
> +command, the output contains a
> +.Ar Flags
> +column indicating what flags have been set on the route. The possible
> +values and their corresponding header definition names are:
> +.Bl -column "U" "RTF_BLACKHOLE" "description"
> +.It U Ta Dv RTF_UP Ta "route is usable"
> +.It G Ta Dv RTF_GATEWAYTa "route traffic through a gateway"
> +.It H Ta Pf RTF_HOST   Ta "route target is a host entry"
> +.It R Ta Dv RTF_REJECT Ta "host or net is unreachable"
> +.It D Ta Dv RTF_DYNAMICTa "route created dynamically (by redirect)"
> +.It M Ta Pf RTF_MODIFIED   Ta "route modified dynamically (by redirect)"
> +.It C Ta Dv RTF_CLONINGTa "generate new routes on use"
> +.It m Ta Dv RTF_MULTICAST  Ta "route associated to a multicast addr"
> +.It L Ta Dv RTF_LLINFO Ta "route generated by ARP or NDP"
> +.It S Ta Dv RTF_STATIC Ta "route was manually added"
> +.It B Ta Dv RTF_BLACKHOLE  Ta "discard all traffic"
> +.It 3 Ta Dv RTF_PROTO3 Ta "protocol specific routing flag"
> +.It 2 Ta Dv RTF_PROTO2 Ta "protocol specific routing flag"
> +.It 1 Ta Dv RTF_PROTO1 Ta "protocol specific routing flag"
> +.It c Ta Dv RTF_CLONED Ta "route was cloned"
> +.It h Ta Dv RTF_CACHED Ta "referenced by gateway route"
> +.It P Ta Dv RTF_MPATH  Ta "multipath route"
> 

Re: args vs arg ... in manuals

2022-12-20 Thread Klemens Nanni
12/21/22 00:53, Theo de Raadt пишет:
> This went from solving one thing, which was largely agreed.
> 
> Now it is trying to introduce other changes as well.  Almost always that
> process stinks, unless it goes over the top at saying it is trying to solve
> other problems when the diff is re-introduced.  It is sneaky, and usually
> seems to be a way of trying to get people to sign off because they are tired.
> You really do know better.  The right way to handle someone doing that is for
> us to say "not ok".  BTW, this is not a new action by you.  I think it is fair
> to ask you to stop doing this.
> 
> And anyways, the way you do plural is with adding S, not adding 'S.

I don't see where I've changed anything wrt. an apostrophe.

Are you perhaps misunderstanding what the .Ns mdoc(7) macro does?

> 
> But what you want is more like this:
> 
>   -utility evaluates
>   +utility evaluates each
>   .Ar expression
>   and writes the result on standard output.
> 
> then you don't need to try this incorrect apostrophy.

That's a good idea, thanks.

> But it does not belong in this diff, and it does not belong in the same
> commit, because that is a procedure violation.  Yes this is just manual pages,
> but processes have a way of sneaking into other more complex parts of the
> tree, so the place to stop this methodology is here.

Those don't seem like separate issues to me as changing an argument's
markup in SYNOPSIS usually goes with doing the same in DESCRIPTION, no?

None of this is intended to be sneaky in any way, it surprises me that
you pick it up like that.



Re: ixv(4): porting Virtual Function driver for Intel 82599 series.

2022-12-20 Thread Yuichiro NAITO

I received an issue that ixv(4) doesn't detect linkdowns in personal email.
When linkdown happens, the PF (Primary Function) driver interrupts all VFs
(Virtual Function) via `mailbox`. But ixv(4) doesn't receive the interrupt.

According to NetBSD, this problem has been fixed by following commits.

http://www.nerv.org/netbsd/changeset.cgi?id=20171003T031229Z.85b67885ef5a26fe6778d9f35f3142c5e14d1de8#src/sys/dev/pci/ixgbe/ixv.c

http://www.nerv.org/netbsd/changeset.cgi?id=20171004T110321Z.7b02467efe35f10f476f3239901730fce3bdb494#src/sys/dev/pci/ixgbe/ixv.c

I ported these commits and updated my patch.

diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
index 3563126b2a1..e421c38e95f 100644
--- a/sys/arch/amd64/conf/GENERIC
+++ b/sys/arch/amd64/conf/GENERIC
@@ -522,6 +522,7 @@ msk*at mskc?#  each port of 
above
 em*at pci? # Intel Pro/1000 ethernet
 ixgb*  at pci? # Intel Pro/10Gb ethernet
 ix*at pci? # Intel 82598EB 10Gb ethernet
+ixv*   at pci? # Virtual Function of Intel 82598EB
 myx*   at pci? # Myricom Myri-10G 10Gb ethernet
 oce*   at pci? # Emulex OneConnect 10Gb ethernet
 txp*   at pci? # 3com 3CR990
diff --git a/sys/dev/pci/files.pci b/sys/dev/pci/files.pci
index a803dc9b659..7d6402e524e 100644
--- a/sys/dev/pci/files.pci
+++ b/sys/dev/pci/files.pci
@@ -350,13 +350,19 @@ file  dev/pci/ixgb_hw.c   ixgb
 # Intel 82598 10GbE
 device ix: ether, ifnet, ifmedia, intrmap, stoeplitz
 attach ix at pci
-file   dev/pci/if_ix.c ix
-file   dev/pci/ixgbe.c ix
-file   dev/pci/ixgbe_82598.c   ix
-file   dev/pci/ixgbe_82599.c   ix
-file   dev/pci/ixgbe_x540.cix
-file   dev/pci/ixgbe_x550.cix
-file   dev/pci/ixgbe_phy.c ix
+file   dev/pci/if_ix.c ix | ixv
+file   dev/pci/ixgbe.c ix | ixv
+file   dev/pci/ixgbe_82598.c   ix | ixv
+file   dev/pci/ixgbe_82599.c   ix | ixv
+file   dev/pci/ixgbe_x540.cix | ixv
+file   dev/pci/ixgbe_x550.cix | ixv
+file   dev/pci/ixgbe_phy.c ix | ixv
+
+# Virtual Function of i82599.
+device ixv: ether, ifnet, ifmedia, intrmap, stoeplitz
+attach ixv at pci
+file   dev/pci/if_ixv.cixv
+file   dev/pci/ixgbe_vf.c  ixv

 # Intel Ethernet 700 Series
 device ixl: ether, ifnet, ifmedia, intrmap, stoeplitz
diff --git a/sys/dev/pci/if_ix.c b/sys/dev/pci/if_ix.c
index b59ec28d9f1..4fb01d85778 100644
--- a/sys/dev/pci/if_ix.c
+++ b/sys/dev/pci/if_ix.c
@@ -507,7 +507,7 @@ ixgbe_start(struct ifqueue *ifq)
 * hardware that this frame is available to transmit.
 */
if (post)
-   IXGBE_WRITE_REG(>hw, IXGBE_TDT(txr->me),
+   IXGBE_WRITE_REG(>hw, txr->tail,
txr->next_avail_desc);
 }

@@ -706,7 +706,7 @@ ixgbe_watchdog(struct ifnet * ifp)
for (i = 0; i < sc->num_queues; i++, txr++) {
printf("%s: Queue(%d) tdh = %d, hw tdt = %d\n", ifp->if_xname, 
i,
IXGBE_READ_REG(hw, IXGBE_TDH(i)),
-   IXGBE_READ_REG(hw, IXGBE_TDT(i)));
+   IXGBE_READ_REG(hw, sc->tx_rings[i].tail));
printf("%s: TX(%d) Next TX to Clean = %d\n", ifp->if_xname,
i, txr->next_to_clean);
}
@@ -826,7 +826,7 @@ ixgbe_init(void *arg)
msec_delay(1);
}
IXGBE_WRITE_FLUSH(>hw);
-   IXGBE_WRITE_REG(>hw, IXGBE_RDT(i), rxr->last_desc_filled);
+   IXGBE_WRITE_REG(>hw, rxr[i].tail, rxr->last_desc_filled);
}

/* Set up VLAN support and filter */
@@ -2359,9 +2359,12 @@ ixgbe_initialize_transmit_units(struct ix_softc *sc)
IXGBE_WRITE_REG(hw, IXGBE_TDLEN(i),
sc->num_tx_desc * sizeof(struct ixgbe_legacy_tx_desc));

+   /* Set Tx Tail register */
+   txr->tail = IXGBE_TDT(i);
+
/* Setup the HW Tx Head and Tail descriptor pointers */
IXGBE_WRITE_REG(hw, IXGBE_TDH(i), 0);
-   IXGBE_WRITE_REG(hw, IXGBE_TDT(i), 0);
+   IXGBE_WRITE_REG(hw, txr->tail, 0);

/* Setup Transmit Descriptor Cmd Settings */
txr->txd_cmd = IXGBE_TXD_CMD_IFCS;
@@ -2834,7 +2837,7 @@ ixgbe_rxrefill(void *xrxr)

if (ixgbe_rxfill(rxr)) {
/* Advance the Rx Queue "Tail Pointer" */
-   IXGBE_WRITE_REG(>hw, IXGBE_RDT(rxr->me),
+   IXGBE_WRITE_REG(>hw, rxr->tail,
rxr->last_desc_filled);
} else if (if_rxr_inuse(>rx_ring) == 0)
timeout_add(>rx_refill, 1);
@@ -2928,6 +2931,9 @@ ixgbe_initialize_receive_units(struct ix_softc *sc)
srrctl = 

Re: Configure interface by lladdr during install

2022-12-20 Thread Andrew Hewus Fresh
On Fri, Dec 16, 2022 at 04:00:40PM -0800, Andrew Hewus Fresh wrote:
> On Wed, Dec 07, 2022 at 07:35:53PM -0800, Andrew Hewus Fresh wrote:
> > On Wed, Dec 07, 2022 at 09:51:51AM -0800, Andrew Hewus Fresh wrote:
> > > On Wed, Dec 07, 2022 at 10:28:05AM +, Stuart Henderson wrote:
> > > > On 2022/12/06 19:57, Andrew Hewus Fresh wrote:
> > > > > Which interface do you wish to configure? (name, lladdr, '?', or 
> > > > > 'done') [vio0] ?
> > > > > Available network interfaces are: vio0 vlan0.
> > > > >  vio0: lladdr fe:e1:bb:d1:dd:97
> > > > > Which interface do you wish to configure? (name, lladdr, '?', or 
> > > > > 'done') [vio0] fe:e1:bb:d1:dd:97
> > > > > IPv4 address for vio0? (or 'autoconf' or 'none') [autoconf] 
> > > > > IPv6 address for vio0? (or 'autoconf' or 'none') [none] 
> > > > > Available network interfaces are: vio0 vlan0.
> > > > 
> > > > It doesn't seem right to offer vio0 in the list if it was already
> > > > configured via lladdr?
> > > 
> > > The installer currently allows re-configuring interfaces and deletes the
> > > hostname.if before doing it.  I think the most useful solution is to
> > > check for "the other" configuration for that interface and remove it as
> > > well when reconfiguring.
> > 
> > Here's the next version that cleans up hostname.lladdr when configuring
> > by name and hostname.name when configuring by lladdr.
>  
> This version is mostly just rebasing onto the recent changes to swap
> priority.  There is a little  tidying of variables and such mixed in.
 
Just a couple of minor tidies and such, avoiding some weird constructs
and extra execs.  Some tweaks from kn@.


Index: distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1217
diff -u -p -r1.1217 install.sub
--- distrib/miniroot/install.sub16 Dec 2022 17:47:34 -  1.1217
+++ distrib/miniroot/install.sub21 Dec 2022 04:05:42 -
@@ -356,7 +356,7 @@ get_ifs() {
done
 }
 
-# Map an interface to its MAC address if it is unique
+# Map an interface to its MAC address if it is unique.
 if_name_to_lladdr() {
local _lladdr
 
@@ -365,6 +365,16 @@ if_name_to_lladdr() {
[[ -n $_lladdr && -n $(ifconfig -M "$_lladdr") ]] && echo $_lladdr
 }
 
+# Print a list of interface names and unique lladdr.
+get_ifs_and_lladdrs() {
+   local _if
+
+   for _if in $(get_ifs); do
+   echo $_if
+   if_name_to_lladdr $_if
+   done
+}
+
 # Return the device name of the disk device $1, which may be a disklabel UID.
 get_dkdev_name() {
local _dev=${1#/dev/} _d
@@ -1300,7 +1310,8 @@ ieee80211_config() {
 
 # Set up IPv4 and IPv6 interface configuration.
 configure_ifs() {
-   local _first _hn _if _name _p _vi _vn
+   local _first _hn _if _ifs _lladdr _name _p _q _vi _vn
+   resp=
 
# Always need lo0 configured.
ifconfig lo0 inet 127.0.0.1/8
@@ -1309,20 +1320,55 @@ configure_ifs() {
rm -f /tmp/i/mygate
 
while :; do
+   set -sA _ifs -- $(get_ifs)
+
# Discover last configured vlan interface and increment its
# minor for the next offered vlan interface.
-   _vi=$(get_ifs vlan | sed '$!d;s/^vlan//')
+   _vi=
+   for _if in "${_ifs[@]}"; do
+   [[ $_if = vlan+([[:digit:]]) ]] && _vi=${_if#vlan}
+   done
[[ -n $_vi ]] && ((_vi++))
-   [[ -n $(get_ifs) ]] && _vn="vlan${_vi:-0}"
+   [[ -n ${_ifs[*]} ]] && _vn="vlan${_vi:-0}"
+
+   echo "Available network interfaces are: ${_ifs[*]} $_vn."
+   if [[ $resp == '?' ]]; then
+   for _if in "${_ifs[@]}"; do
+   _lladdr=$(if_name_to_lladdr $_if)
+   [[ -n $_lladdr ]] && echo " $_if: lladdr 
$_lladdr"
+   done
+   fi
+
+   _q="Network interface to configure?"
+   ask_until "$_q (name, lladdr, '?', or 'done')" \
+   ${_p:-$( (get_ifs netboot; get_ifs) | sed q )}
 
-   ask_which "network interface" "do you wish to configure" \
-   "\$(get_ifs) $_vn" \
-   ${_p:-'$( (get_ifs netboot; get_ifs) | sed q )'}
[[ $resp == done ]] && break
+   [[ $resp == '?'  ]] && continue
+
+   # Quote $resp to prevent user from confusing isin() by
+   # entering something like 'a a'.
+   if ! isin "$resp" $(get_ifs_and_lladdrs) $_vn done; then
+   echo "'$resp' is not a valid choice."
+   $AI && [[ -n $AI_RESPFILE ]] && exit 1
+   continue
+   fi
 
_if=$resp
_hn=/tmp/i/hostname.$_if
rm -f $_hn
+
+   # Map lladdr to interface 

Re: is this rge crash known? - fixed

2022-12-20 Thread Kevin Lo
On Tue, Dec 20, 2022 at 01:02:23AM -0500, Geoff Steckel wrote:
> 
> On 12/19/22 21:07, Kevin Lo wrote:
> > On Mon, Dec 19, 2022 at 03:50:45PM -0500, Geoff Steckel wrote:
> > > Thanks for all the suggestions:
> > > 
> > > sysctl kern.pool_debug=1 = no change
> > > known working board in same slot = no change
> > > 
> > > hardware version is indeed 0609
> > > em(4) in same slot = works
> > > test using old rge(4) board between two Linux systems = works
> > > 
> > > Are any other drivers similar enough for me to compare with if_rge.c?
> > > Perhaps the AMD 5600G or the B550 chipset have quirks not seen before?
> > > 
> > > I could possibly install FreeBSD if that would give any information.
> > The diff below syncs up the Rx descriptor setup code to the upstream.
> > It should fix the problem.
> > 
> > Index: sys/dev/pci/if_rge.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/if_rge.c,v
> > retrieving revision 1.20
> > diff -u -p -u -p -r1.20 if_rge.c
> > --- sys/dev/pci/if_rge.c20 Nov 2022 23:47:51 -  1.20
> > +++ sys/dev/pci/if_rge.c20 Dec 2022 01:54:30 -
> > @@ -1104,24 +1104,16 @@ rge_newbuf(struct rge_queues *q)
> > /* Map the segments into RX descriptors. */
> > r = >q_rx.rge_rx_list[idx];
> > -   if (RGE_OWN(r)) {
> > -   printf("%s: tried to map busy RX descriptor\n",
> > -   sc->sc_dev.dv_xname);
> > -   m_freem(m);
> > -   return (ENOBUFS);
> > -   }
> > -
> > rxq->rxq_mbuf = m;
> > -   r->rge_extsts = 0;
> > -   r->rge_addrlo = htole32(RGE_ADDR_LO(rxmap->dm_segs[0].ds_addr));
> > -   r->rge_addrhi = htole32(RGE_ADDR_HI(rxmap->dm_segs[0].ds_addr));
> > +   r->hi_qword1.rx_qword4.rge_extsts = 0;
> > +   r->hi_qword0.rge_addr = htole64(rxmap->dm_segs[0].ds_addr);
> > -   r->rge_cmdsts = htole32(rxmap->dm_segs[0].ds_len);
> > +   r->hi_qword1.rx_qword4.rge_cmdsts = htole32(rxmap->dm_segs[0].ds_len);
> > if (idx == RGE_RX_LIST_CNT - 1)
> > -   r->rge_cmdsts |= htole32(RGE_RDCMDSTS_EOR);
> > +   r->hi_qword1.rx_qword4.rge_cmdsts |= htole32(RGE_RDCMDSTS_EOR);
> > -   r->rge_cmdsts |= htole32(RGE_RDCMDSTS_OWN);
> > +   r->hi_qword1.rx_qword4.rge_cmdsts |= htole32(RGE_RDCMDSTS_OWN);
> > bus_dmamap_sync(sc->sc_dmat, q->q_rx.rge_rx_list_map,
> > idx * sizeof(struct rge_rx_desc), sizeof(struct rge_rx_desc),
> > @@ -1140,11 +1132,11 @@ rge_discard_rxbuf(struct rge_queues *q,
> > r = >q_rx.rge_rx_list[idx];
> > -   r->rge_cmdsts = htole32(RGE_JUMBO_FRAMELEN);
> > -   r->rge_extsts = 0;
> > +   r->hi_qword1.rx_qword4.rge_cmdsts = htole32(RGE_JUMBO_FRAMELEN);
> > +   r->hi_qword1.rx_qword4.rge_extsts = 0;
> > if (idx == RGE_RX_LIST_CNT - 1)
> > -   r->rge_cmdsts |= htole32(RGE_RDCMDSTS_EOR);
> > -   r->rge_cmdsts |= htole32(RGE_RDCMDSTS_OWN);
> > +   r->hi_qword1.rx_qword4.rge_cmdsts |= htole32(RGE_RDCMDSTS_EOR);
> > +   r->hi_qword1.rx_qword4.rge_cmdsts |= htole32(RGE_RDCMDSTS_OWN);
> > bus_dmamap_sync(sc->sc_dmat, q->q_rx.rge_rx_list_map,
> > idx * sizeof(struct rge_rx_desc), sizeof(struct rge_rx_desc),
> > @@ -1219,8 +1211,8 @@ rge_rxeof(struct rge_queues *q)
> > if (RGE_OWN(cur_rx))
> > break;
> > -   rxstat = letoh32(cur_rx->rge_cmdsts);
> > -   extsts = letoh32(cur_rx->rge_extsts);
> > +   rxstat = letoh32(cur_rx->hi_qword1.rx_qword4.rge_cmdsts);
> > +   extsts = letoh32(cur_rx->hi_qword1.rx_qword4.rge_extsts);
> > 
> > total_len = RGE_RXBYTES(cur_rx);
> > rxq = >q_rx.rge_rxq[i];
> > @@ -1282,16 +1274,16 @@ rge_rxeof(struct rge_queues *q)
> > (total_len - ETHER_CRC_LEN);
> > /* Check IP header checksum. */
> > -   if (!(rxstat & RGE_RDCMDSTS_IPCSUMERR) &&
> > +   if (!(extsts & RGE_RDEXTSTS_IPCSUMERR) &&
> > (extsts & RGE_RDEXTSTS_IPV4))
> > m->m_pkthdr.csum_flags |= M_IPV4_CSUM_IN_OK;
> > /* Check TCP/UDP checksum. */
> > if ((extsts & (RGE_RDEXTSTS_IPV4 | RGE_RDEXTSTS_IPV6)) &&
> > -   (((rxstat & RGE_RDCMDSTS_TCPPKT) &&
> > -   !(rxstat & RGE_RDCMDSTS_TCPCSUMERR)) ||
> > -   ((rxstat & RGE_RDCMDSTS_UDPPKT) &&
> > -   !(rxstat & RGE_RDCMDSTS_UDPCSUMERR
> > +   (((extsts & RGE_RDEXTSTS_TCPPKT) &&
> > +   !(extsts & RGE_RDEXTSTS_TCPCSUMERR)) ||
> > +   ((extsts & RGE_RDEXTSTS_UDPPKT) &&
> > +   !(extsts & RGE_RDEXTSTS_UDPCSUMERR
> > m->m_pkthdr.csum_flags |= M_TCP_CSUM_IN_OK |
> > M_UDP_CSUM_IN_OK;
> > Index: sys/dev/pci/if_rgereg.h
> > ===
> > RCS file: /cvs/src/sys/dev/pci/if_rgereg.h,v
> > retrieving revision 1.8
> > diff -u -p -u -p -r1.8 if_rgereg.h
> > --- sys/dev/pci/if_rgereg.h 20 Nov 

[patch] add show.c style flag descriptions to route(8)

2022-12-20 Thread Paul R. Tagliamonte
Heyya tech@,

Please keep me on cc, I'm not subscribed

I've been working on porting some of my linux specific software to
work on OpenBSD. While working with some routing-specific code,
I needed to pull up show.c to map the letters in `route show` to the
RTF_ values.

I figured while I was in there, I'd send a patch along adding that to
the manpage in the hopes it's a bit easier for others in the future.
Most of the descriptions were pulled from the .h

First time sending anything in to OpenBSD, please let me know if
there's anything I can do better for next time! I'm wary of my MUA
(GMail's web interface) destroying the patch, so I've attached it
for redundancy.

paultag
--
:wq

Index: route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.106
diff -u -p -r1.106 route.8
--- route.8 19 Nov 2022 19:23:37 - 1.106
+++ route.8 21 Dec 2022 01:11:22 -
@@ -571,6 +571,37 @@ and
 .Dv RTM_CHANGE .
 As such, only the superuser may modify
 the routing tables.
+.Sh FLAGS
+When displaying routes with the
+.Cm show
+command, the output contains a
+.Ar Flags
+column indicating what flags have been set on the route. The possible
+values and their corresponding header definition names are:
+.Bl -column "U" "RTF_BLACKHOLE" "description"
+.It U Ta Dv RTF_UP Ta "route is usable"
+.It G Ta Dv RTF_GATEWAYTa "route traffic through a gateway"
+.It H Ta Pf RTF_HOST   Ta "route target is a host entry"
+.It R Ta Dv RTF_REJECT Ta "host or net is unreachable"
+.It D Ta Dv RTF_DYNAMICTa "route created dynamically (by redirect)"
+.It M Ta Pf RTF_MODIFIED   Ta "route modified dynamically (by redirect)"
+.It C Ta Dv RTF_CLONINGTa "generate new routes on use"
+.It m Ta Dv RTF_MULTICAST  Ta "route associated to a multicast addr"
+.It L Ta Dv RTF_LLINFO Ta "route generated by ARP or NDP"
+.It S Ta Dv RTF_STATIC Ta "route was manually added"
+.It B Ta Dv RTF_BLACKHOLE  Ta "discard all traffic"
+.It 3 Ta Dv RTF_PROTO3 Ta "protocol specific routing flag"
+.It 2 Ta Dv RTF_PROTO2 Ta "protocol specific routing flag"
+.It 1 Ta Dv RTF_PROTO1 Ta "protocol specific routing flag"
+.It c Ta Dv RTF_CLONED Ta "route was cloned"
+.It h Ta Dv RTF_CACHED Ta "referenced by gateway route"
+.It P Ta Dv RTF_MPATH  Ta "multipath route"
+.It T Ta Dv RTF_MPLS   Ta "MPLS route"
+.It l Ta Dv RTF_LOCAL  Ta "route is local to the host"
+.It F Ta Dv RTF_BFDTa "link state controlled by BFD"
+.It b Ta Dv RTF_BROADCAST  Ta "route associated to a broadcast addr"
+.It n Ta Dv RTF_CONNECTED  Ta "interface route"
+.El
 .Sh FILES
 .Bl -tag -width "/etc/mygate" -compact
 .It Pa /etc/hosts
Index: route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.106
diff -u -p -r1.106 route.8
--- route.8	19 Nov 2022 19:23:37 -	1.106
+++ route.8	21 Dec 2022 01:11:22 -
@@ -571,6 +571,37 @@ and
 .Dv RTM_CHANGE .
 As such, only the superuser may modify
 the routing tables.
+.Sh FLAGS
+When displaying routes with the
+.Cm show
+command, the output contains a
+.Ar Flags
+column indicating what flags have been set on the route. The possible
+values and their corresponding header definition names are:
+.Bl -column "U" "RTF_BLACKHOLE" "description"
+.It U Ta Dv RTF_UP Ta "route is usable"
+.It G Ta Dv RTF_GATEWAYTa "route traffic through a gateway"
+.It H Ta Pf RTF_HOST   Ta "route target is a host entry"
+.It R Ta Dv RTF_REJECT Ta "host or net is unreachable"
+.It D Ta Dv RTF_DYNAMICTa "route created dynamically (by redirect)"
+.It M Ta Pf RTF_MODIFIED   Ta "route modified dynamically (by redirect)"
+.It C Ta Dv RTF_CLONINGTa "generate new routes on use"
+.It m Ta Dv RTF_MULTICAST  Ta "route associated to a multicast addr"
+.It L Ta Dv RTF_LLINFO Ta "route generated by ARP or NDP"
+.It S Ta Dv RTF_STATIC Ta "route was manually added"
+.It B Ta Dv RTF_BLACKHOLE  Ta "discard all traffic"
+.It 3 Ta Dv RTF_PROTO3 Ta "protocol specific routing flag"
+.It 2 Ta Dv RTF_PROTO2 Ta "protocol specific routing flag"
+.It 1 Ta Dv RTF_PROTO1 Ta "protocol specific routing flag"
+.It c Ta Dv RTF_CLONED Ta "route was cloned"
+.It h Ta Dv RTF_CACHED Ta "referenced by gateway route"
+.It P Ta Dv RTF_MPATH  Ta "multipath route"
+.It T Ta Dv RTF_MPLS   Ta "MPLS route"
+.It l Ta Dv RTF_LOCAL  Ta "route is local to the host"
+.It F Ta Dv RTF_BFDTa "link state controlled by BFD"
+.It b Ta Dv RTF_BROADCAST  Ta "route associated to a broadcast addr"
+.It n Ta Dv RTF_CONNECTED  Ta "interface route"
+.El
 .Sh FILES
 .Bl -tag -width "/etc/mygate" -compact
 .It Pa /etc/hosts


Re: LLVM 15: mismatched bound errors

2022-12-20 Thread Todd C . Miller
On Tue, 20 Dec 2022 23:44:08 +0100, Patrick Wildt wrote:

> clang complains when the function is declared with a fixed array size in
> a parameter while the prototype is unbounded, like this:
>
> /usr/src/sys/net/pf.c:4353:54: error: argument 'sns' of type 'struct pf_src_n
> ode *[4]' with mismatched bound [-Werror,-Warray-parameter]
> struct pf_rule_actions *act, struct pf_src_node *sns[PF_SN_MAX])
>  ^
> /usr/src/sys/net/pf.c:203:28: note: previously declared as 'struct pf_src_nod
> e *[]' here
> struct pf_src_node *[]);
> ^
> 1 error generated.
>
> We have a few of that, and was wondering what the better solution is.
> clang apparently accepts using * instead of [].  The alternative would
> be to hardcode the size in the prototype as well.  Here's a diff for
> a three files for the first version, as example.

Using * instead of [] is the saner approach.  Hard-coding the sizes
into the prototype seems like a bad idea, even if clang is now smart
enough to complain about a mismatch.

 - todd



LLVM 15: mismatched bound errors

2022-12-20 Thread Patrick Wildt
Hi,

clang complains when the function is declared with a fixed array size in
a parameter while the prototype is unbounded, like this:

/usr/src/sys/net/pf.c:4353:54: error: argument 'sns' of type 'struct 
pf_src_node *[4]' with mismatched bound [-Werror,-Warray-parameter]
struct pf_rule_actions *act, struct pf_src_node *sns[PF_SN_MAX])
 ^
/usr/src/sys/net/pf.c:203:28: note: previously declared as 'struct pf_src_node 
*[]' here
struct pf_src_node *[]);
^
1 error generated.

We have a few of that, and was wondering what the better solution is.
clang apparently accepts using * instead of [].  The alternative would
be to hardcode the size in the prototype as well.  Here's a diff for
a three files for the first version, as example.

Which version is saner?

Patrick

diff --git a/sys/dev/ic/ar5008.c b/sys/dev/ic/ar5008.c
index cad0f142210..4c0126fd6d6 100644
--- a/sys/dev/ic/ar5008.c
+++ b/sys/dev/ic/ar5008.c
@@ -111,7 +111,7 @@ voidar5008_next_calib(struct athn_softc *);
 void   ar5008_calib_iq(struct athn_softc *);
 void   ar5008_calib_adc_gain(struct athn_softc *);
 void   ar5008_calib_adc_dc_off(struct athn_softc *);
-void   ar5008_write_txpower(struct athn_softc *, int16_t power[]);
+void   ar5008_write_txpower(struct athn_softc *, int16_t *);
 void   ar5008_set_viterbi_mask(struct athn_softc *, int);
 void   ar5008_hw_init(struct athn_softc *, struct ieee80211_channel *,
struct ieee80211_channel *);
@@ -119,9 +119,9 @@ uint8_t ar5008_get_vpd(uint8_t, const uint8_t *, const 
uint8_t *, int);
 void   ar5008_get_pdadcs(struct athn_softc *, uint8_t, struct athn_pier *,
struct athn_pier *, int, int, uint8_t, uint8_t *, uint8_t *);
 void   ar5008_get_lg_tpow(struct athn_softc *, struct ieee80211_channel *,
-   uint8_t, const struct ar_cal_target_power_leg *, int, uint8_t[]);
+   uint8_t, const struct ar_cal_target_power_leg *, int, uint8_t *);
 void   ar5008_get_ht_tpow(struct athn_softc *, struct ieee80211_channel *,
-   uint8_t, const struct ar_cal_target_power_ht *, int, uint8_t[]);
+   uint8_t, const struct ar_cal_target_power_ht *, int, uint8_t *);
 void   ar5008_set_noise_immunity_level(struct athn_softc *, int);
 void   ar5008_enable_ofdm_weak_signal(struct athn_softc *);
 void   ar5008_disable_ofdm_weak_signal(struct athn_softc *);
diff --git a/sys/dev/ic/ar9003.c b/sys/dev/ic/ar9003.c
index 565ea27c701..7ec3131f86b 100644
--- a/sys/dev/ic/ar9003.c
+++ b/sys/dev/ic/ar9003.c
@@ -114,7 +114,7 @@ int ar9003_init_calib(struct athn_softc *);
 void   ar9003_do_calib(struct athn_softc *);
 void   ar9003_next_calib(struct athn_softc *);
 void   ar9003_calib_iq(struct athn_softc *);
-intar9003_get_iq_corr(struct athn_softc *, int32_t[], int32_t[]);
+intar9003_get_iq_corr(struct athn_softc *, int32_t *, int32_t *);
 intar9003_calib_tx_iq(struct athn_softc *);
 void   ar9003_paprd_calib(struct athn_softc *, struct ieee80211_channel *);
 intar9003_get_desired_txgain(struct athn_softc *, int, int);
@@ -126,17 +126,17 @@ int   ar9003_compute_predistortion(struct athn_softc 
*, const uint32_t *,
 void   ar9003_enable_predistorter(struct athn_softc *, int);
 void   ar9003_paprd_enable(struct athn_softc *);
 void   ar9003_paprd_tx_tone_done(struct athn_softc *);
-void   ar9003_write_txpower(struct athn_softc *, int16_t power[]);
+void   ar9003_write_txpower(struct athn_softc *, int16_t *);
 void   ar9003_reset_rx_gain(struct athn_softc *, struct ieee80211_channel *);
 void   ar9003_reset_tx_gain(struct athn_softc *, struct ieee80211_channel *);
 void   ar9003_hw_init(struct athn_softc *, struct ieee80211_channel *,
struct ieee80211_channel *);
 void   ar9003_get_lg_tpow(struct athn_softc *, struct ieee80211_channel *,
uint8_t, const uint8_t *, const struct ar_cal_target_power_leg *,
-   int, uint8_t[]);
+   int, uint8_t *);
 void   ar9003_get_ht_tpow(struct athn_softc *, struct ieee80211_channel *,
uint8_t, const uint8_t *, const struct ar_cal_target_power_ht *,
-   int, uint8_t[]);
+   int, uint8_t *);
 void   ar9003_set_noise_immunity_level(struct athn_softc *, int);
 void   ar9003_enable_ofdm_weak_signal(struct athn_softc *);
 void   ar9003_disable_ofdm_weak_signal(struct athn_softc *);
diff --git a/sys/dev/ic/rtwn.c b/sys/dev/ic/rtwn.c
index 2d7551eaef3..d7f25d59919 100644
--- a/sys/dev/ic/rtwn.c
+++ b/sys/dev/ic/rtwn.c
@@ -151,19 +151,19 @@ void  rtwn_pa_bias_init(struct rtwn_softc *);
 void   rtwn_rxfilter_init(struct rtwn_softc *);
 void   rtwn_edca_init(struct rtwn_softc *);
 void   rtwn_rate_fallback_init(struct rtwn_softc *);
-void   rtwn_write_txpower(struct rtwn_softc *, int, uint16_t[]);
+void   rtwn_write_txpower(struct rtwn_softc *, int, uint16_t *);
 void   

use a zero-copy approach for vmd virtio devices

2022-12-20 Thread Dave Voutila
While working back and forth with dlg@ on his idea of adding in async
task queues to vmd devices, he started to introduce a cleaner way to
handle virtqueues. In short, we can just track the host virtual address
and read/write to that location.

This has the benefit of cleaning up a good chunk of code, mostly in the
vioscsi device. If my count is correct, it's close to 100 lines
removed.

Testing on a variety of systems, I see anywhere from a small (5%) to a
large (50%) gain in things like vionet performance depending on the host
hardware and the guest OS. (Linux guests push packets faster, on
average.)

If/when this lands, it'll be easier to introduce the async taskq diff
and also clean up more of the virtio codebase. Lots of duplicate logic
in basic virtio-pci config handling, like updating queue address or
size.

Testing is appreciated, but also looking for OKs.


diff refs/heads/master refs/heads/vmd-vq
commit - 9f38b5b2272f7ca113158ec7943c959ae9b22683
commit + 509bbee70e5a11cfb69b93202a24fb032c72ac43
blob - aea2fc1d5e8903c97cb7076bfb9e5f265f416378
blob + 3a9af7790b24914fb6e7716a049fd0428803af9b
--- usr.sbin/vmd/vioscsi.c
+++ usr.sbin/vmd/vioscsi.c
@@ -81,6 +81,7 @@ vioscsi_next_ring_item(struct vioscsi_dev *dev, struct
 {
used->ring[used->idx & VIOSCSI_QUEUE_MASK].id = idx;
used->ring[used->idx & VIOSCSI_QUEUE_MASK].len = desc->len;
+   __sync_synchronize();
used->idx++;

dev->vq[dev->cfg.queue_notify].last_avail =
@@ -157,7 +158,7 @@ vioscsi_reg_name(uint8_t reg)
switch (reg) {
case VIRTIO_CONFIG_DEVICE_FEATURES: return "device feature";
case VIRTIO_CONFIG_GUEST_FEATURES: return "guest feature";
-   case VIRTIO_CONFIG_QUEUE_ADDRESS: return "queue address";
+   case VIRTIO_CONFIG_QUEUE_PFN: return "queue pfn";
case VIRTIO_CONFIG_QUEUE_SIZE: return "queue size";
case VIRTIO_CONFIG_QUEUE_SELECT: return "queue select";
case VIRTIO_CONFIG_QUEUE_NOTIFY: return "queue notify";
@@ -1672,8 +1673,8 @@ vioscsi_io(int dir, uint16_t reg, uint32_t *data, uint
DPRINTF("%s: guest feature set to %u",
__func__, dev->cfg.guest_feature);
break;
-   case VIRTIO_CONFIG_QUEUE_ADDRESS:
-   dev->cfg.queue_address = *data;
+   case VIRTIO_CONFIG_QUEUE_PFN:
+   dev->cfg.queue_pfn = *data;
vioscsi_update_qa(dev);
break;
case VIRTIO_CONFIG_QUEUE_SELECT:
@@ -1692,7 +1693,7 @@ vioscsi_io(int dir, uint16_t reg, uint32_t *data, uint
if (dev->cfg.device_status == 0) {
log_debug("%s: device reset", __func__);
dev->cfg.guest_feature = 0;
-   dev->cfg.queue_address = 0;
+   dev->cfg.queue_pfn = 0;
vioscsi_update_qa(dev);
dev->cfg.queue_size = 0;
vioscsi_update_qs(dev);
@@ -1988,8 +1989,8 @@ vioscsi_io(int dir, uint16_t reg, uint32_t *data, uint
case VIRTIO_CONFIG_GUEST_FEATURES:
*data = dev->cfg.guest_feature;
break;
-   case VIRTIO_CONFIG_QUEUE_ADDRESS:
-   *data = dev->cfg.queue_address;
+   case VIRTIO_CONFIG_QUEUE_PFN:
+   *data = dev->cfg.queue_pfn;
break;
case VIRTIO_CONFIG_QUEUE_SIZE:
if (sz == 4)
@@ -2033,25 +2034,38 @@ vioscsi_update_qs(struct vioscsi_dev *dev)
 void
 vioscsi_update_qs(struct vioscsi_dev *dev)
 {
+   struct virtio_vq_info *vq_info;
+
/* Invalid queue? */
if (dev->cfg.queue_select >= VIRTIO_MAX_QUEUES) {
dev->cfg.queue_size = 0;
return;
}

-   /* Update queue address/size based on queue select */
-   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
-   dev->cfg.queue_size = dev->vq[dev->cfg.queue_select].qs;
+   vq_info = >vq[dev->cfg.queue_select];
+
+   /* Update queue pfn/size based on queue select */
+   dev->cfg.queue_pfn = vq_info->q_gpa >> 12;
+   dev->cfg.queue_size = vq_info->qs;
 }

 void
 vioscsi_update_qa(struct vioscsi_dev *dev)
 {
+   struct virtio_vq_info *vq_info;
+   void *hva = NULL;
+
/* Invalid queue? */
if (dev->cfg.queue_select >= VIRTIO_MAX_QUEUES)
return;

-   dev->vq[dev->cfg.queue_select].qa = dev->cfg.queue_address;
+   vq_info = >vq[dev->cfg.queue_select];
+   vq_info->q_gpa = (uint64_t)dev->cfg.queue_pfn * VIRTIO_PAGE_SIZE;
+
+   hva = hvaddr_mem(vq_info->q_gpa, vring_size(VIOSCSI_QUEUE_SIZE));
+   if (hva == NULL)
+   fatal("vioscsi_update_qa");
+   vq_info->q_hva = hva;
 }

 /*
@@ -2067,13 

Re: netcat: bump BUFSIZE to 64k?

2022-12-20 Thread Marco Pfatschbacher
On Sun, Dec 18, 2022 at 06:40:45PM +0100, Claudio Jeker wrote:
> 
> What confuses me is that atomicio() is not used in the main readwrite()
> loop. There nc polls on both fds and then read/write depending on return
> values.  atomicio() is only used by atelnet() and socks_connect() which do
> not depend on BUFSIZE.
>
> I assume this was modified when TLS support was added but I did
> not investigate further.

Thanks for the pointer. That was changed with the following commit
which means that stalling with atomicio() is not an issue anymore.


revision 1.125
date: 2014/10/30 16:06:07;  author: tedu;  state: Exp;  lines: +211 -47;
rework the poll loop to poll in both directions so it doesn't get stuck
if one pipe stalls out. from a diff by Arne Becker.
(buffer size left alone for now) 


There's also a longer discussion here

https://marc.info/?t=14023700382=1=2

   Marco



Re: args vs arg ... in manuals

2022-12-20 Thread Theo de Raadt
Jason McIntyre  wrote:

> On Tue, Dec 20, 2022 at 01:53:18PM -0700, Theo de Raadt wrote:
> > This went from solving one thing, which was largely agreed.
> > 
> > Now it is trying to introduce other changes as well.  Almost always that
> > process stinks, unless it goes over the top at saying it is trying to solve
> > other problems when the diff is re-introduced.  It is sneaky, and usually
> > seems to be a way of trying to get people to sign off because they are 
> > tired.
> > You really do know better.  The right way to handle someone doing that is 
> > for
> > us to say "not ok".  BTW, this is not a new action by you.  I think it is 
> > fair
> > to ask you to stop doing this.
> > 
> > And anyways, the way you do plural is with adding S, not adding 'S.
> > 
> > But what you want is more like this:
> > 
> >   -utility evaluates
> >   +utility evaluates each
> >   .Ar expression
> >   and writes the result on standard output.
> > 
> > then you don't need to try this incorrect apostrophy.
> > 
> > But it does not belong in this diff, and it does not belong in the same
> > commit, because that is a procedure violation.  Yes this is just manual 
> > pages,
> > but processes have a way of sneaking into other more complex parts of the
> > tree, so the place to stop this methodology is here.
> > 
> 
> hi.
> 
> i guess these comments are directed at me (i'm sorry if i have picked
> that up wrongly).

no, at kn, who keeps changing the diff without discussion, which is sneaky.

there are technical issues being explained wrong, and they can be fixed, but not
by sneaking additional new unexplained changes into the diff, which propose 
using
incorrect english and making the diff worse.

I think this is abusing 's.  I really want to know why an expression
owns a utility, and what does it do with the utilities it owns.  That is
what "expression's utility" means (if I ignore the following words).  this
is a possessive apostrophe.  's is used for pluralization in some cases,
but this does not feel like one of those cases.

But how about we let the english speaking pedants handle it, and not keep
interrupting with more sneaky tweaks?



Re: args vs arg ... in manuals

2022-12-20 Thread Jason McIntyre
On Tue, Dec 20, 2022 at 01:53:18PM -0700, Theo de Raadt wrote:
> This went from solving one thing, which was largely agreed.
> 
> Now it is trying to introduce other changes as well.  Almost always that
> process stinks, unless it goes over the top at saying it is trying to solve
> other problems when the diff is re-introduced.  It is sneaky, and usually
> seems to be a way of trying to get people to sign off because they are tired.
> You really do know better.  The right way to handle someone doing that is for
> us to say "not ok".  BTW, this is not a new action by you.  I think it is fair
> to ask you to stop doing this.
> 
> And anyways, the way you do plural is with adding S, not adding 'S.
> 
> But what you want is more like this:
> 
>   -utility evaluates
>   +utility evaluates each
>   .Ar expression
>   and writes the result on standard output.
> 
> then you don't need to try this incorrect apostrophy.
> 
> But it does not belong in this diff, and it does not belong in the same
> commit, because that is a procedure violation.  Yes this is just manual pages,
> but processes have a way of sneaking into other more complex parts of the
> tree, so the place to stop this methodology is here.
> 

hi.

i guess these comments are directed at me (i'm sorry if i have picked
that up wrongly).

i was trying to give some feedback. i dislike the implication that i'm
somehow trying to be sneaky or trying to tire people out. that was not
my intention.

thank you for your explanation about how plurals work.

jmc



Re: args vs arg ... in manuals

2022-12-20 Thread Klemens Nanni
12/21/22 00:24, Jason McIntyre пишет:
> On Tue, Dec 20, 2022 at 06:24:45PM +, Klemens Nanni wrote:
> expr(1) is a good example. i think it doesn;t matter that expr can take
> one arg or many.

But it does, which is why the expr(1) could be considered a bug fix:

$ expr '1 + 2'
1 + 2
$ expr 1 + 2
3

This is the difference between `expression' and `expression ...'.
DESCRIPTION explains that expressions must be one per argument.

> when we want to descrive it at its basic level, we just
> want to say that it "evalautes expression". jumping through formatting
> hoops to try and indicate that more than one expression is possible is
> pointless. let's try and make the text clear and simple, and let
> formatting come second. here's some ideas:
> 
> - The
>   .Nm
>   utility evaluates
>   .Ar expression
>and writes ...
> 
> in this case we leave the reader to understand that SYNOPSIS and the
> description all clearly show that more than one arg is possible. nice
> and simple. this gets my vote.
> 
> - The
>   .Nm
>   utility evaluates one or more
>   .Ar expression
>and writes ...
> 
> now we are spelling it out. it is less ambiguous than the first example,
> but reads badly (one or more expression)
> 
> - The
>   .Nme
>   utility evaluates one or more expressions and writes ...
> 
> that reads better than the last example, but it misses the chance to
> link "expression" as the argument name, and people will complain.
> 
> i am really ok with any of these. but i don;t like "Ar expression Ns s"
> here because it both reads badly and formats ugly. i'm not saying we
> can't use "Ar .* Ns s", just that we should think about not using it
> willy nilly.

I'm fine with just not doing the Ns hoop and writing argument names as
regular words without markups in those sentences.

> 
>> Index: sbin/route/route.8
>> ===
>> RCS file: /cvs/src/sbin/route/route.8,v
>> retrieving revision 1.106
>> diff -u -p -r1.106 route.8
>> --- sbin/route/route.8   19 Nov 2022 19:23:37 -  1.106
>> +++ sbin/route/route.8   20 Dec 2022 17:23:09 -
>> @@ -42,8 +42,8 @@
>>  .Op Fl T Ar rtable
>>  .Ar command
>>  .Oo
>> -.Op Ar modifiers
>> -.Ar args
>> +.Op Ar modifier ...
>> +.Ar arg ...
>>  .Oc
>>  .Sh DESCRIPTION
>>  .Nm
>> @@ -110,7 +110,7 @@ option.
>>  .Op Fl T Ar rtable
>>  .Tg
>>  .Cm flush
>> -.Op Ar modifiers
>> +.Op Ar modifier ...
>>  .Xc
>>  Delete all gateway entries from the routing table.
>>  When the address family is specified by any one of the
>> @@ -129,7 +129,7 @@ modifiers.
>>  .Op Fl T Ar rtable
>>  .Tg
>>  .Cm get
>> -.Op Ar modifiers
>> +.Op Ar modifier ...
>>  .Ar address
>>  .Xc
>>  Extract a routing entry from the kernel.
>> @@ -143,7 +143,7 @@ same address family as the destination a
>>  .Op Fl T Ar rtable
>>  .Tg
>>  .Cm monitor
>> -.Op Ar modifiers
>> +.Op Ar modifier ...
>>  .Xc
>>  Continuously report any changes to the routing information base,
>>  routing lookup misses, or suspected network partitionings.
>> @@ -278,7 +278,7 @@ have the syntax:
>>  .Op Fl T Ar rtable
>>  .Tg
>>  .Cm add
>> -.Op Ar modifiers
>> +.Op Ar modifier ...
>>  .Ar destination gateway
>>  .Xc
>>  .It Xo
>> @@ -287,7 +287,7 @@ have the syntax:
>>  .Op Fl T Ar rtable
>>  .Tg
>>  .Cm change
>> -.Op Ar modifiers
>> +.Op Ar modifier ...
>>  .Ar destination gateway
>>  .Xc
>>  .It Xo
>> @@ -297,7 +297,7 @@ have the syntax:
>>  .Tg delete
>>  .Tg
>>  .Cm del Ns Op Cm ete
>> -.Op Ar modifiers
>> +.Op Ar modifier ...
>>  .Ar destination gateway
>>  .Xc
>>  .El
>> Index: sbin/route/route.c
>> ===
>> RCS file: /cvs/src/sbin/route/route.c,v
>> retrieving revision 1.260
>> diff -u -p -r1.260 route.c
>> --- sbin/route/route.c   10 Nov 2021 20:24:22 -  1.260
>> +++ sbin/route/route.c   20 Dec 2022 17:23:06 -
>> @@ -135,9 +135,9 @@ usage(char *cp)
>>  warnx("botched keyword: %s", cp);
>>  fprintf(stderr,
>>  #ifndef SMALL
>> -"usage: %s [-dnqtv] [-T rtable] command [[modifiers] args]\n",
>> +"usage: %s [-dnqtv] [-T rtable] command [[modifier ...] arg ...]\n",
>>  #else
>> -"usage: %s [-dnqtv] command [[modifiers] args]\n",
>> +"usage: %s [-dnqtv] command [[modifier ...] arg ...]\n",
>>  #endif
>>  __progname);
>>  exit(1);
>> Index: usr.bin/cdio/cdio.1
>> ===
>> RCS file: /cvs/src/usr.bin/cdio/cdio.1,v
>> retrieving revision 1.67
>> diff -u -p -r1.67 cdio.1
>> --- usr.bin/cdio/cdio.1  31 Mar 2022 17:27:24 -  1.67
>> +++ usr.bin/cdio/cdio.1  20 Dec 2022 17:17:52 -
>> @@ -40,7 +40,7 @@
>>  .Op Fl sv
>>  .Op Fl d Ar host : Ns Ar port
>>  .Op Fl f Ar device
>> -.Op Ar command args ...
>> +.Op Ar command Op Ar arg ...
>>  .Sh DESCRIPTION
>>  The
>>  .Nm
>> 

Re: args vs arg ... in manuals

2022-12-20 Thread Theo de Raadt
This went from solving one thing, which was largely agreed.

Now it is trying to introduce other changes as well.  Almost always that
process stinks, unless it goes over the top at saying it is trying to solve
other problems when the diff is re-introduced.  It is sneaky, and usually
seems to be a way of trying to get people to sign off because they are tired.
You really do know better.  The right way to handle someone doing that is for
us to say "not ok".  BTW, this is not a new action by you.  I think it is fair
to ask you to stop doing this.

And anyways, the way you do plural is with adding S, not adding 'S.

But what you want is more like this:

  -utility evaluates
  +utility evaluates each
  .Ar expression
  and writes the result on standard output.

then you don't need to try this incorrect apostrophy.

But it does not belong in this diff, and it does not belong in the same
commit, because that is a procedure violation.  Yes this is just manual pages,
but processes have a way of sneaking into other more complex parts of the
tree, so the place to stop this methodology is here.



Re: args vs arg ... in manuals

2022-12-20 Thread Jason McIntyre
On Tue, Dec 20, 2022 at 06:24:45PM +, Klemens Nanni wrote:
> 
> Feedback? OK?
> 

ok, in general i'm fine with this. i do have some concerns though:

> 
> Index: share/man/man4/ddb.4
> ===
> RCS file: /cvs/src/share/man/man4/ddb.4,v
> retrieving revision 1.104
> diff -u -p -r1.104 ddb.4
> --- share/man/man4/ddb.4  11 Sep 2022 06:38:11 -  1.104
> +++ share/man/man4/ddb.4  20 Dec 2022 17:30:46 -
> @@ -1018,7 +1018,7 @@ A synonym for
>  .Tg machine
>  .It Xo
>  .Ic mac Ns Op Ic hine
> -.Ar subcommand Op Ar args ...
> +.Ar subcommand Op Ar arg ...
>  .Xc
>  Perform a platform-specific command.
>  .Pp
> Index: bin/expr/expr.1
> ===
> RCS file: /cvs/src/bin/expr/expr.1,v
> retrieving revision 1.24
> diff -u -p -r1.24 expr.1
> --- bin/expr/expr.1   16 Aug 2017 20:10:58 -  1.24
> +++ bin/expr/expr.1   20 Dec 2022 17:31:39 -
> @@ -12,12 +12,12 @@
>  .Nd evaluate expression
>  .Sh SYNOPSIS
>  .Nm expr
> -.Ar expression
> +.Ar expression ...
>  .Sh DESCRIPTION
>  The
>  .Nm
>  utility evaluates
> -.Ar expression
> +.Ar expression Ns s
>  and writes the result on standard output.
>  All operators are separate arguments to the
>  .Nm

so first off i hate "Ar blah Ns s". i do understand why we might want
it, but i;d rather avoid it. it can make the text read better, but it
can also make it read worse (so i don;t think we should just slap it
on). and the formatting is truly yucky.

expr(1) is a good example. i think it doesn;t matter that expr can take
one arg or many. when we want to descrive it at its basic level, we just
want to say that it "evalautes expression". jumping through formatting
hoops to try and indicate that more than one expression is possible is
pointless. let's try and make the text clear and simple, and let
formatting come second. here's some ideas:

-   The
.Nm
utility evaluates
.Ar expression
 and writes ...

in this case we leave the reader to understand that SYNOPSIS and the
description all clearly show that more than one arg is possible. nice
and simple. this gets my vote.

-   The
.Nm
utility evaluates one or more
.Ar expression
 and writes ...

now we are spelling it out. it is less ambiguous than the first example,
but reads badly (one or more expression)

-   The
.Nme
utility evaluates one or more expressions and writes ...

that reads better than the last example, but it misses the chance to
link "expression" as the argument name, and people will complain.

i am really ok with any of these. but i don;t like "Ar expression Ns s"
here because it both reads badly and formats ugly. i'm not saying we
can't use "Ar .* Ns s", just that we should think about not using it
willy nilly.

> Index: sbin/route/route.8
> ===
> RCS file: /cvs/src/sbin/route/route.8,v
> retrieving revision 1.106
> diff -u -p -r1.106 route.8
> --- sbin/route/route.819 Nov 2022 19:23:37 -  1.106
> +++ sbin/route/route.820 Dec 2022 17:23:09 -
> @@ -42,8 +42,8 @@
>  .Op Fl T Ar rtable
>  .Ar command
>  .Oo
> -.Op Ar modifiers
> -.Ar args
> +.Op Ar modifier ...
> +.Ar arg ...
>  .Oc
>  .Sh DESCRIPTION
>  .Nm
> @@ -110,7 +110,7 @@ option.
>  .Op Fl T Ar rtable
>  .Tg
>  .Cm flush
> -.Op Ar modifiers
> +.Op Ar modifier ...
>  .Xc
>  Delete all gateway entries from the routing table.
>  When the address family is specified by any one of the
> @@ -129,7 +129,7 @@ modifiers.
>  .Op Fl T Ar rtable
>  .Tg
>  .Cm get
> -.Op Ar modifiers
> +.Op Ar modifier ...
>  .Ar address
>  .Xc
>  Extract a routing entry from the kernel.
> @@ -143,7 +143,7 @@ same address family as the destination a
>  .Op Fl T Ar rtable
>  .Tg
>  .Cm monitor
> -.Op Ar modifiers
> +.Op Ar modifier ...
>  .Xc
>  Continuously report any changes to the routing information base,
>  routing lookup misses, or suspected network partitionings.
> @@ -278,7 +278,7 @@ have the syntax:
>  .Op Fl T Ar rtable
>  .Tg
>  .Cm add
> -.Op Ar modifiers
> +.Op Ar modifier ...
>  .Ar destination gateway
>  .Xc
>  .It Xo
> @@ -287,7 +287,7 @@ have the syntax:
>  .Op Fl T Ar rtable
>  .Tg
>  .Cm change
> -.Op Ar modifiers
> +.Op Ar modifier ...
>  .Ar destination gateway
>  .Xc
>  .It Xo
> @@ -297,7 +297,7 @@ have the syntax:
>  .Tg delete
>  .Tg
>  .Cm del Ns Op Cm ete
> -.Op Ar modifiers
> +.Op Ar modifier ...
>  .Ar destination gateway
>  .Xc
>  .El
> Index: sbin/route/route.c
> ===
> RCS file: /cvs/src/sbin/route/route.c,v
> retrieving revision 1.260
> diff -u -p -r1.260 route.c
> --- sbin/route/route.c10 Nov 2021 20:24:22 -  1.260
> +++ sbin/route/route.c20 Dec 2022 17:23:06 -
> @@ -135,9 +135,9 @@ usage(char *cp)
>   warnx("botched keyword: 

Re: args vs arg ... in manuals

2022-12-20 Thread Klemens Nanni
12/20/22 20:16, Ingo Schwarze пишет:
> Hi Klemens,
> 
> Jason McIntyre wrote on Tue, Dec 20, 2022 at 10:35:19AM +:
>> On Tue, Dec 20, 2022 at 09:36:39AM +, Klemens Nanni wrote:
> 
>>> Both styles are used, but I argue that the former fails to distinguish
>>> between
>>> $ program 'args in one shell word'
>>> and
>>> $ program one arg per shell word
>>>
>>> It's a minor thing, imho, but perhaps we can decide for one and stick to
>>> it throughout the tree?
>>>
>>> Triple dots also make it immediately clear that many arguments may
>>> follow, no matter what the "arg"ument is named.
>>>
>>> Here's one examplatory diff for timeout(1).
>>>
>>> Feedback?
> 
>> i prefer "arg ..." too.
> 
> Seconded.  Or rather, what, fourthed?

Thanks for the input.

>> by comparison, it would probably look weird to
>> write "files" instead of "file ...". indeed the Ar macro without args
>> outputs exactly that.
>>
>> still it would be good to get an idea of how many pages would be
>> affected by this.
> 
>   $ man -k Ar=args
> 
> gives me a relatively short list, and the only clear offenders appear
> to be: doas(1), openssl(1), timeout(1), route(8), and possibly boot(8)
> on a handful of platforms.
> 
> Strangely, in build-debug-info(1), cdio(1), csplit(1), tput(1),
> update-plist(1), ddb(4) we have "..." after plural "args" and i'm
> unsure what that is even intended to mean in some of the cases.
> 
> I guess i ought to fix mdoc(7) myself.

Sure, I'll leave that for you.

>> if we have only a few pages using "args" it would seem to make sense.
> 
> That seems to be the case.  I think kn@ is welcome to go ahead and fix
> them - not in a completely mechanical way, of course, the text should
> still be clear and read well afterwards.
> 
>> if it's 50/50 we might want to look for more complex
> 
> Certainly not 50/50:
> 
>   $ man -k Ar=... | wc -l
> 
> gives me well over three hundred pages.
> 
>> examples and figure out whether all the change will be worth it.
>>
>> also some things will use argument vs arg. for example ssh(1). do you
>> propose to change those?
> 
>   $ man -k Ar=argument
> 
> gives me fifty results.

Complete diff at the end for all tools I could find.

> I think that is a completely separate question [...]

Ack.

> There are also a handful of "Ar=arguments" that might deserve
> fixing, in particular su(1), tmux(1), bgpd.conf(5), bgpctl(8),
> and rcctl(8).
> 
> And time(1) and btrace(8) might need
> 
>   .Ar arguments -> .Ar argument Ns s
> 
> in the vein mentioned by millert@.

I've included these in the diff.

Feedback? OK?


Index: share/man/man4/ddb.4
===
RCS file: /cvs/src/share/man/man4/ddb.4,v
retrieving revision 1.104
diff -u -p -r1.104 ddb.4
--- share/man/man4/ddb.411 Sep 2022 06:38:11 -  1.104
+++ share/man/man4/ddb.420 Dec 2022 17:30:46 -
@@ -1018,7 +1018,7 @@ A synonym for
 .Tg machine
 .It Xo
 .Ic mac Ns Op Ic hine
-.Ar subcommand Op Ar args ...
+.Ar subcommand Op Ar arg ...
 .Xc
 Perform a platform-specific command.
 .Pp
Index: bin/expr/expr.1
===
RCS file: /cvs/src/bin/expr/expr.1,v
retrieving revision 1.24
diff -u -p -r1.24 expr.1
--- bin/expr/expr.1 16 Aug 2017 20:10:58 -  1.24
+++ bin/expr/expr.1 20 Dec 2022 17:31:39 -
@@ -12,12 +12,12 @@
 .Nd evaluate expression
 .Sh SYNOPSIS
 .Nm expr
-.Ar expression
+.Ar expression ...
 .Sh DESCRIPTION
 The
 .Nm
 utility evaluates
-.Ar expression
+.Ar expression Ns s
 and writes the result on standard output.
 All operators are separate arguments to the
 .Nm
Index: sbin/route/route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.106
diff -u -p -r1.106 route.8
--- sbin/route/route.8  19 Nov 2022 19:23:37 -  1.106
+++ sbin/route/route.8  20 Dec 2022 17:23:09 -
@@ -42,8 +42,8 @@
 .Op Fl T Ar rtable
 .Ar command
 .Oo
-.Op Ar modifiers
-.Ar args
+.Op Ar modifier ...
+.Ar arg ...
 .Oc
 .Sh DESCRIPTION
 .Nm
@@ -110,7 +110,7 @@ option.
 .Op Fl T Ar rtable
 .Tg
 .Cm flush
-.Op Ar modifiers
+.Op Ar modifier ...
 .Xc
 Delete all gateway entries from the routing table.
 When the address family is specified by any one of the
@@ -129,7 +129,7 @@ modifiers.
 .Op Fl T Ar rtable
 .Tg
 .Cm get
-.Op Ar modifiers
+.Op Ar modifier ...
 .Ar address
 .Xc
 Extract a routing entry from the kernel.
@@ -143,7 +143,7 @@ same address family as the destination a
 .Op Fl T Ar rtable
 .Tg
 .Cm monitor
-.Op Ar modifiers
+.Op Ar modifier ...
 .Xc
 Continuously report any changes to the routing information base,
 routing lookup misses, or suspected network partitionings.
@@ -278,7 +278,7 @@ have the syntax:
 .Op Fl T Ar rtable
 .Tg
 .Cm add
-.Op Ar modifiers
+.Op Ar modifier ...
 .Ar destination gateway
 .Xc
 .It Xo
@@ -287,7 +287,7 @@ have the syntax:
 .Op Fl T Ar rtable
 .Tg
 .Cm change
-.Op Ar modifiers

BROP mitigation

2022-12-20 Thread Theo de Raadt
A few weeks ago a conversation about retguard (a diff is probably
coming) caused me re-consider & re-read the BROP paper

https://www.scs.stanford.edu/brop/bittau-brop.pdf

After lots of details, page 8 has a table summarizing the attack process.

Step 5 contains the text "The attacker can now dump the entire binary to
find more gadgets".

This diff hinders that step.  It prevents download of immutable text
segments via system calls as a simple step.  This is valuable because
BROP needs this to be a simple step, because the write is the maximum
tooling the attacker has at the moment.

There is a difficulty in the way of "oh, just make code segments
non-readable".  Most MMU lack the ability to manage PROT_EXEC without
PROT_READ.  Being able to read the code using data instructions is
implicit in these architectures.  There is a very short list of
architectures and sub-architectures that could block read to code
regions if we wrote uvm/pmap code.  We really want this property for
security reasons, but since most MMU lack it we have not made a lot of
progress.  This illusive property, of block reads to code, is called
"X-only" in our group.

So that means user processe can read their own code.  Can't stop that.

The kernel also reads userland memory, when you do a system call like
write() or sendto().  It does this using copyin() against the userland
region, which again uses the MMU.  But the MMU lacks the ability we
desire. Changing this lookup to instead use higher-level virtual-memory
representation data structure inspection would introduce either races or
some pretty strong locks because the virtual memory layout can be
changed by threads, and therefore we would hurt threading performance.

So we cannot simply inspect the whole virtual-memory data structures
for the region.

So created a very small coherent cache of unpermitted regions which gets
looked up before copyin(), and after a few iterations of coding, I
managed to do it without locking!

Depending on binary type, this cache is an array of 2 to 4 text regions:
main program text, ld.so text, signal trampoline, and libc.so text.
Normally this would need management & updates when processes do
mprotect/mmap/munmap, but a few weeks ago I introduced mimmutable(), and
since all those text segements now immutable we know they cannot change!
So there is no need to update or delete entries in this cache.  Once we
know this table is complete, we don't need to lock lookups.

The kernel now prevents "write(fd, , length)" and "write(fd, ,
length)" by returning -1 EFAULT.

This protection is not made available to other libraries (like
libcrypto).  I've looked into doing this via a special system call, and
via an implicit arrangement inside mimmutable(), but the changes are
much more complicated and the security benefit is lower (so for now, I am
going to punt on that).

Someone is going to reply "but I'll copy libc text to other memory
before I do the write operation!"  Please show us at which step in the
BROP procedure on page 8 this copy operation is done, and how. BROP is
used when the attack tooling is insufficient for complicated sequences
like "copy elsewhere + write"; BROP is a method to collect powerful
gadgetry that you don't have for a next-round attack sequence.  In
particular, BROP would be used to learn the libc random relink, and in
particular to discover the location of all syscall stubs.


There are are unfinished pieces in here, but it seems to be working fine.
The next step will be to find out if we have any software in ports which
tries to perform output system calls with their text segments as data origin.

Let me know.

Index: sys/kern/exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.177
diff -u -p -u -r1.177 exec_elf.c
--- sys/kern/exec_elf.c 5 Dec 2022 23:18:37 -   1.177
+++ sys/kern/exec_elf.c 20 Dec 2022 07:10:34 -
@@ -621,9 +621,11 @@ exec_elf_makecmds(struct proc *p, struct
} else
addr = ELF_NO_ADDR;
 
-   /* Permit system calls in specific main-programs */
+   /*
+* Permit system calls in main-text static binaries.
+* Also block the ld.so syscall-grant
+*/
if (interp == NULL) {
-   /* statics. Also block the ld.so syscall-grant 
*/
syscall = VMCMD_SYSCALL;
p->p_vmspace->vm_map.flags |= 
VM_MAP_SYSCALL_ONCE;
}
Index: sys/kern/exec_subr.c
===
RCS file: /cvs/src/sys/kern/exec_subr.c,v
retrieving revision 1.64
diff -u -p -u -r1.64 exec_subr.c
--- sys/kern/exec_subr.c5 Dec 2022 23:18:37 -   1.64
+++ sys/kern/exec_subr.c20 Dec 2022 06:39:40 

Re: getdelim.3: Add a missing Vt

2022-12-20 Thread Ingo Schwarze
Hi Josiah,

committed, thanks!
  Ingo

Josiah Frentsos wrote on Tue, Dec 20, 2022 at 12:49:06PM -0500:

> Index: getdelim.3
> ===
> RCS file: /cvs/src/lib/libc/stdio/getdelim.3,v
> retrieving revision 1.6
> diff -u -p -r1.6 getdelim.3
> --- getdelim.317 Oct 2017 22:47:58 -  1.6
> +++ getdelim.320 Dec 2022 17:45:54 -
> @@ -57,7 +57,8 @@ The buffer is
>  and includes the delimiter.
>  The
>  .Fa delimiter
> -character must be representable as an unsigned char.
> +character must be representable as an
> +.Vt unsigned char .
>  .Pp
>  If
>  .Fa *n



getdelim.3: Add a missing Vt

2022-12-20 Thread Josiah Frentsos
Index: getdelim.3
===
RCS file: /cvs/src/lib/libc/stdio/getdelim.3,v
retrieving revision 1.6
diff -u -p -r1.6 getdelim.3
--- getdelim.3  17 Oct 2017 22:47:58 -  1.6
+++ getdelim.3  20 Dec 2022 17:45:54 -
@@ -57,7 +57,8 @@ The buffer is
 and includes the delimiter.
 The
 .Fa delimiter
-character must be representable as an unsigned char.
+character must be representable as an
+.Vt unsigned char .
 .Pp
 If
 .Fa *n



Re: args vs arg ... in manuals

2022-12-20 Thread Theo de Raadt
Todd C. Miller  wrote:

> On Tue, 20 Dec 2022 10:35:19 +, Jason McIntyre wrote:
> 
> > also some things will use argument vs arg. for example ssh(1). do you
> > propose to change those?
> 
> I have no strong opinion on "arg" vs. "argument".  It probably makes
> sense to standardize on whichever is most prevalent in our
> documentation.  But that can be handled separately, if at all.

I would like to avoid line-wraps in usage messages, which are often
kept in sync.  I don't think the abbreviation causes harm.



Re: args vs arg ... in manuals

2022-12-20 Thread Todd C . Miller
On Tue, 20 Dec 2022 10:35:19 +, Jason McIntyre wrote:

> also some things will use argument vs arg. for example ssh(1). do you
> propose to change those?

I have no strong opinion on "arg" vs. "argument".  It probably makes
sense to standardize on whichever is most prevalent in our
documentation.  But that can be handled separately, if at all.

 - todd



Re: args vs arg ... in manuals

2022-12-20 Thread Ingo Schwarze
Hi Klemens,

Jason McIntyre wrote on Tue, Dec 20, 2022 at 10:35:19AM +:
> On Tue, Dec 20, 2022 at 09:36:39AM +, Klemens Nanni wrote:

>> Both styles are used, but I argue that the former fails to distinguish
>> between
>>  $ program 'args in one shell word'
>> and
>>  $ program one arg per shell word
>> 
>> It's a minor thing, imho, but perhaps we can decide for one and stick to
>> it throughout the tree?
>> 
>> Triple dots also make it immediately clear that many arguments may
>> follow, no matter what the "arg"ument is named.
>> 
>> Here's one examplatory diff for timeout(1).
>> 
>> Feedback?

> i prefer "arg ..." too.

Seconded.  Or rather, what, fourthed?

> by comparison, it would probably look weird to
> write "files" instead of "file ...". indeed the Ar macro without args
> outputs exactly that.
> 
> still it would be good to get an idea of how many pages would be
> affected by this.

  $ man -k Ar=args

gives me a relatively short list, and the only clear offenders appear
to be: doas(1), openssl(1), timeout(1), route(8), and possibly boot(8)
on a handful of platforms.

Strangely, in build-debug-info(1), cdio(1), csplit(1), tput(1),
update-plist(1), ddb(4) we have "..." after plural "args" and i'm
unsure what that is even intended to mean in some of the cases.

I guess i ought to fix mdoc(7) myself.

> if we have only a few pages using "args" it would seem to make sense.

That seems to be the case.  I think kn@ is welcome to go ahead and fix
them - not in a completely mechanical way, of course, the text should
still be clear and read well afterwards.

> if it's 50/50 we might want to look for more complex

Certainly not 50/50:

  $ man -k Ar=... | wc -l

gives me well over three hundred pages.

> examples and figure out whether all the change will be worth it.
> 
> also some things will use argument vs arg. for example ssh(1). do you
> propose to change those?

  $ man -k Ar=argument

gives me fifty results.

I think that is a completely separate question and need not be handled
together with the "args vs. arg ..." issue.  FWIW, i personally like
.Ar argument ... , at least in many cases.  Avoiding unnessecary
abbreviations often makes text easier and more pleasant to read.

There are also a handful of "Ar=arguments" that might deserve
fixing, in particular su(1), tmux(1), bgpd.conf(5), bgpctl(8),
and rcctl(8).

And time(1) and btrace(8) might need

  .Ar arguments -> .Ar argument Ns s

in the vein mentioned by millert@.

Yours,
  Ingo


>> Index: timeout.1
>> ===
>> RCS file: /cvs/src/usr.bin/timeout/timeout.1,v
>> retrieving revision 1.3
>> diff -u -p -r1.3 timeout.1
>> --- timeout.14 Sep 2021 11:58:31 -   1.3
>> +++ timeout.129 Oct 2022 20:53:05 -
>> @@ -41,14 +41,14 @@
>>  .Op Fl -preserve-status
>>  .Ar duration
>>  .Ar command
>> -.Op Ar args
>> +.Op Ar arg ...
>>  .Sh DESCRIPTION
>>  The
>>  .Nm
>>  utility executes
>>  .Ar command ,
>>  with any
>> -.Ar args ,
>> +.Ar arg ... ,
>>  and kills it if it is still running after the
>>  specified
>>  .Ar duration .
>> Index: timeout.c
>> ===
>> RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
>> retrieving revision 1.21
>> diff -u -p -r1.21 timeout.c
>> --- timeout.c2 Jul 2022 19:00:35 -   1.21
>> +++ timeout.c29 Oct 2022 21:08:40 -
>> @@ -57,7 +57,7 @@ usage(void)
>>  fprintf(stderr,
>>  "usage: timeout [-k time] [-s sig] [--foreground]"
>>  " [--preserve-status] duration\n"
>> -"   command [args]\n");
>> +"   command [arg ...]\n");
>>  
>>  exit(1);
>>  }



Re: args vs arg ... in manuals

2022-12-20 Thread Klemens Nanni
12/20/22 19:26, Todd C. Miller пишет:
> On Tue, 20 Dec 2022 09:36:39 +, Klemens Nanni wrote:
> 
>> Both styles are used, but I argue that the former fails to distinguish
>> between
>>  $ program 'args in one shell word'
>> and
>>  $ program one arg per shell word
>>
>> It's a minor thing, imho, but perhaps we can decide for one and stick to
>> it throughout the tree?
>>
>> Triple dots also make it immediately clear that many arguments may
>> follow, no matter what the "arg"ument is named.
> 
> I agree, and as jmc@ points out, .Ar will emit "file ..." when used
> without an explicit argument.

Good point.

Let me come up with a list of manuals that could be changed, then we'll
have some numbers.

> 
>> Here's one examplatory diff for timeout(1).
>>
>> Feedback?
>>
>> Index: timeout.1
>> ===
>> RCS file: /cvs/src/usr.bin/timeout/timeout.1,v
>> retrieving revision 1.3
>> diff -u -p -r1.3 timeout.1
>> --- timeout.14 Sep 2021 11:58:31 -   1.3
>> +++ timeout.129 Oct 2022 20:53:05 -
>> @@ -41,14 +41,14 @@
>>  .Op Fl -preserve-status
>>  .Ar duration
>>  .Ar command
>> -.Op Ar args
>> +.Op Ar arg ...
>>  .Sh DESCRIPTION
>>  The
>>  .Nm
>>  utility executes
>>  .Ar command ,
>>  with any
>> -.Ar args ,
>> +.Ar arg ... ,
> 
> The change to the DESCRIPTION seems a little awkward to me.  I
> understand you want to match the SYNOPSIS but in this case it comes
> at the expense of readability.  Personally, I'd prefer:
> 
> .Ar arg Ns s ,

Neat, I like that much better.

> 
>>  and kills it if it is still running after the
>>  specified
>>  .Ar duration .
> 
>  - todd



Re: args vs arg ... in manuals

2022-12-20 Thread Todd C . Miller
On Tue, 20 Dec 2022 09:36:39 +, Klemens Nanni wrote:

> Both styles are used, but I argue that the former fails to distinguish
> between
>   $ program 'args in one shell word'
> and
>   $ program one arg per shell word
>
> It's a minor thing, imho, but perhaps we can decide for one and stick to
> it throughout the tree?
>
> Triple dots also make it immediately clear that many arguments may
> follow, no matter what the "arg"ument is named.

I agree, and as jmc@ points out, .Ar will emit "file ..." when used
without an explicit argument.

> Here's one examplatory diff for timeout(1).
>
> Feedback?
>
> Index: timeout.1
> ===
> RCS file: /cvs/src/usr.bin/timeout/timeout.1,v
> retrieving revision 1.3
> diff -u -p -r1.3 timeout.1
> --- timeout.1 4 Sep 2021 11:58:31 -   1.3
> +++ timeout.1 29 Oct 2022 20:53:05 -
> @@ -41,14 +41,14 @@
>  .Op Fl -preserve-status
>  .Ar duration
>  .Ar command
> -.Op Ar args
> +.Op Ar arg ...
>  .Sh DESCRIPTION
>  The
>  .Nm
>  utility executes
>  .Ar command ,
>  with any
> -.Ar args ,
> +.Ar arg ... ,

The change to the DESCRIPTION seems a little awkward to me.  I
understand you want to match the SYNOPSIS but in this case it comes
at the expense of readability.  Personally, I'd prefer:

.Ar arg Ns s ,

>  and kills it if it is still running after the
>  specified
>  .Ar duration .

 - todd



Re: 7.2: snmp mibtree command broken

2022-12-20 Thread Gerhard Roth
On Tue, 2022-12-20 at 11:34 +0100, Martijn van Duren wrote:
> On Tue, 2022-12-20 at 10:28 +, Gerhard Roth wrote:
> > Hi Martijn,
> > 
> > On Tue, 2022-12-20 at 11:10 +0100, Martijn van Duren wrote:
> > > On Tue, 2022-12-20 at 10:21 +0100, Matthias Pitzl wrote:
> > > > Hi,
> > > > 
> > > > Since the release of OpenBSD 7.2, snmp mibtree is broken:
> > > > > root@host:~# snmp mibtree
> > > > > snmp: No securityName specified
> > > > 
> > > > Greetings,
> > > > Matthias
> > > 
> > > Apparently no one has used this one in a long time (including me).
> > > This got broken in snmpc.c r1.37 on 2021/08/11 when changing snmp(1)'s
> > > default to v3.
> > > 
> > > This may not the prettiest solution, but this is the shortest I can come
> > > up with.
> > 
> > no sub-command that hasn't 'snmp_app->usecommonopt' set can have any of
> > the SNMPv2 or SNMPv3 credentials. Sure, currently 'mibtree' is the only
> > one. But for future updates, wouldn't it be better to check
> > 
> > if (!snmp_app->usecommonopt)
> > 
> > instead of
> > 
> > if (strcmp(snmp_app->name, "mibtree") == 0)
> > 
> > Greetings,
> > 
> > Gerhard
> > 
> > 
> You're right, that's better.

ok gerhard@

> 
> Index: snmpc.c
> ===
> RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 snmpc.c
> --- snmpc.c 21 Oct 2021 08:17:34 -  1.38
> +++ snmpc.c 20 Dec 2022 10:33:58 -
> @@ -468,7 +468,9 @@ main(int argc, char *argv[])
> argc -= optind;
> argv += optind;
>  
> -   if (version == SNMP_V1 || version == SNMP_V2C) {
> +   if (!snmp_app->usecommonopt) {
> +   /* No SNMP protocol settings */
> +   } else if (version == SNMP_V1 || version == SNMP_V2C) {
> if (community == NULL || community[0] == '\0')
> errx(1, "No community name specified.");
> } else if (version == SNMP_V3) {
> 



smime.p7s
Description: S/MIME cryptographic signature


Re: args vs arg ... in manuals

2022-12-20 Thread Jason McIntyre
On Tue, Dec 20, 2022 at 09:36:39AM +, Klemens Nanni wrote:
> Both styles are used, but I argue that the former fails to distinguish
> between
>   $ program 'args in one shell word'
> and
>   $ program one arg per shell word
> 
> It's a minor thing, imho, but perhaps we can decide for one and stick to
> it throughout the tree?
> 
> Triple dots also make it immediately clear that many arguments may
> follow, no matter what the "arg"ument is named.
> 
> Here's one examplatory diff for timeout(1).
> 
> Feedback?
> 

i prefer "arg ..." too. by comparison, it would probably look weird to
write "files" instead of "file ...". indeed the Ar macro without args
outputs exactly that.

still it would be good to get an idea of how many pages would be
affected by this. if we have only a few pages using "args" it would seem
to make sense. if it's 50/50 we might want to look for more complex
examples and figure out whether all the change will be worth it.

also some things will use argument vs arg. for example ssh(1). do you
propose to change those?

jmc

> Index: timeout.1
> ===
> RCS file: /cvs/src/usr.bin/timeout/timeout.1,v
> retrieving revision 1.3
> diff -u -p -r1.3 timeout.1
> --- timeout.1 4 Sep 2021 11:58:31 -   1.3
> +++ timeout.1 29 Oct 2022 20:53:05 -
> @@ -41,14 +41,14 @@
>  .Op Fl -preserve-status
>  .Ar duration
>  .Ar command
> -.Op Ar args
> +.Op Ar arg ...
>  .Sh DESCRIPTION
>  The
>  .Nm
>  utility executes
>  .Ar command ,
>  with any
> -.Ar args ,
> +.Ar arg ... ,
>  and kills it if it is still running after the
>  specified
>  .Ar duration .
> Index: timeout.c
> ===
> RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 timeout.c
> --- timeout.c 2 Jul 2022 19:00:35 -   1.21
> +++ timeout.c 29 Oct 2022 21:08:40 -
> @@ -57,7 +57,7 @@ usage(void)
>   fprintf(stderr,
>   "usage: timeout [-k time] [-s sig] [--foreground]"
>   " [--preserve-status] duration\n"
> - "   command [args]\n");
> + "   command [arg ...]\n");
>  
>   exit(1);
>  }
> 



Re: 7.2: snmp mibtree command broken

2022-12-20 Thread Martijn van Duren
On Tue, 2022-12-20 at 10:28 +, Gerhard Roth wrote:
> Hi Martijn,
> 
> On Tue, 2022-12-20 at 11:10 +0100, Martijn van Duren wrote:
> > On Tue, 2022-12-20 at 10:21 +0100, Matthias Pitzl wrote:
> > > Hi,
> > > 
> > > Since the release of OpenBSD 7.2, snmp mibtree is broken:
> > > > root@host:~# snmp mibtree
> > > > snmp: No securityName specified
> > > 
> > > Greetings,
> > > Matthias
> > 
> > Apparently no one has used this one in a long time (including me).
> > This got broken in snmpc.c r1.37 on 2021/08/11 when changing snmp(1)'s
> > default to v3.
> > 
> > This may not the prettiest solution, but this is the shortest I can come
> > up with.
> 
> no sub-command that hasn't 'snmp_app->usecommonopt' set can have any of
> the SNMPv2 or SNMPv3 credentials. Sure, currently 'mibtree' is the only
> one. But for future updates, wouldn't it be better to check
> 
>   if (!snmp_app->usecommonopt)
> 
> instead of
> 
>   if (strcmp(snmp_app->name, "mibtree") == 0)
> 
> Greetings,
> 
> Gerhard
> 
> 
You're right, that's better.

Index: snmpc.c
===
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.38
diff -u -p -r1.38 snmpc.c
--- snmpc.c 21 Oct 2021 08:17:34 -  1.38
+++ snmpc.c 20 Dec 2022 10:33:58 -
@@ -468,7 +468,9 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
 
-   if (version == SNMP_V1 || version == SNMP_V2C) {
+   if (!snmp_app->usecommonopt) {
+   /* No SNMP protocol settings */
+   } else if (version == SNMP_V1 || version == SNMP_V2C) {
if (community == NULL || community[0] == '\0')
errx(1, "No community name specified.");
} else if (version == SNMP_V3) {



Re: 7.2: snmp mibtree command broken

2022-12-20 Thread Gerhard Roth
Hi Martijn,

On Tue, 2022-12-20 at 11:10 +0100, Martijn van Duren wrote:
> On Tue, 2022-12-20 at 10:21 +0100, Matthias Pitzl wrote:
> > Hi,
> > 
> > Since the release of OpenBSD 7.2, snmp mibtree is broken:
> > > root@host:~# snmp mibtree
> > > snmp: No securityName specified
> > 
> > Greetings,
> > Matthias
> 
> Apparently no one has used this one in a long time (including me).
> This got broken in snmpc.c r1.37 on 2021/08/11 when changing snmp(1)'s
> default to v3.
> 
> This may not the prettiest solution, but this is the shortest I can come
> up with.

no sub-command that hasn't 'snmp_app->usecommonopt' set can have any of
the SNMPv2 or SNMPv3 credentials. Sure, currently 'mibtree' is the only
one. But for future updates, wouldn't it be better to check

if (!snmp_app->usecommonopt)

instead of

if (strcmp(snmp_app->name, "mibtree") == 0)

Greetings,

Gerhard

> 
> martijn@
> 
> Index: snmpc.c
> ===
> RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 snmpc.c
> --- snmpc.c 21 Oct 2021 08:17:34 -  1.38
> +++ snmpc.c 20 Dec 2022 10:08:29 -
> @@ -468,7 +468,9 @@ main(int argc, char *argv[])
> argc -= optind;
> argv += optind;
>  
> -   if (version == SNMP_V1 || version == SNMP_V2C) {
> +   if (strcmp(snmp_app->name, "mibtree") == 0) {
> +   /* No SNMP protocol settings */
> +   } else if (version == SNMP_V1 || version == SNMP_V2C) {
> if (community == NULL || community[0] == '\0')
> errx(1, "No community name specified.");
> } else if (version == SNMP_V3) {
> 



smime.p7s
Description: S/MIME cryptographic signature


Re: 7.2: snmp mibtree command broken

2022-12-20 Thread Martijn van Duren
On Tue, 2022-12-20 at 10:21 +0100, Matthias Pitzl wrote:
> Hi,
> 
> Since the release of OpenBSD 7.2, snmp mibtree is broken:
> > root@host:~# snmp mibtree
> > snmp: No securityName specified
> 
> Greetings,
> Matthias

Apparently no one has used this one in a long time (including me).
This got broken in snmpc.c r1.37 on 2021/08/11 when changing snmp(1)'s
default to v3.

This may not the prettiest solution, but this is the shortest I can come
up with.

martijn@

Index: snmpc.c
===
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.38
diff -u -p -r1.38 snmpc.c
--- snmpc.c 21 Oct 2021 08:17:34 -  1.38
+++ snmpc.c 20 Dec 2022 10:08:29 -
@@ -468,7 +468,9 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
 
-   if (version == SNMP_V1 || version == SNMP_V2C) {
+   if (strcmp(snmp_app->name, "mibtree") == 0) {
+   /* No SNMP protocol settings */
+   } else if (version == SNMP_V1 || version == SNMP_V2C) {
if (community == NULL || community[0] == '\0')
errx(1, "No community name specified.");
} else if (version == SNMP_V3) {



args vs arg ... in manuals

2022-12-20 Thread Klemens Nanni
Both styles are used, but I argue that the former fails to distinguish
between
$ program 'args in one shell word'
and
$ program one arg per shell word

It's a minor thing, imho, but perhaps we can decide for one and stick to
it throughout the tree?

Triple dots also make it immediately clear that many arguments may
follow, no matter what the "arg"ument is named.

Here's one examplatory diff for timeout(1).

Feedback?

Index: timeout.1
===
RCS file: /cvs/src/usr.bin/timeout/timeout.1,v
retrieving revision 1.3
diff -u -p -r1.3 timeout.1
--- timeout.1   4 Sep 2021 11:58:31 -   1.3
+++ timeout.1   29 Oct 2022 20:53:05 -
@@ -41,14 +41,14 @@
 .Op Fl -preserve-status
 .Ar duration
 .Ar command
-.Op Ar args
+.Op Ar arg ...
 .Sh DESCRIPTION
 The
 .Nm
 utility executes
 .Ar command ,
 with any
-.Ar args ,
+.Ar arg ... ,
 and kills it if it is still running after the
 specified
 .Ar duration .
Index: timeout.c
===
RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
retrieving revision 1.21
diff -u -p -r1.21 timeout.c
--- timeout.c   2 Jul 2022 19:00:35 -   1.21
+++ timeout.c   29 Oct 2022 21:08:40 -
@@ -57,7 +57,7 @@ usage(void)
fprintf(stderr,
"usage: timeout [-k time] [-s sig] [--foreground]"
" [--preserve-status] duration\n"
-   "   command [args]\n");
+   "   command [arg ...]\n");
 
exit(1);
 }



Re: vmt is not closed

2022-12-20 Thread YASUOKA Masahiko
Yes, my previous commit was wrong.  Calling vm_rpc_close() was missing.
Thank you for finding.

ok yasuoka

On Tue, 13 Dec 2022 19:26:05 +0900 (JST)
Masato Asou  wrote:
> From: Masato Asou 
> Date: Tue, 13 Dec 2022 18:26:22 +0900 (JST)
> 
> Delete #define VMT_DEBUG
> ok?
> --
> ASOU Masato
> 
>> comment, ok?
>> --
>> ASOU Masato
>> 
>> Index: sys/dev/pv/vmt.c
>> ===
>> RCS file: /cvs/src/sys/dev/pv/vmt.c,v
>> retrieving revision 1.27
>> diff -u -p -r1.27 vmt.c
>> --- sys/dev/pv/vmt.c 3 Dec 2022 10:57:04 -   1.27
>> +++ sys/dev/pv/vmt.c 13 Dec 2022 09:23:57 -
>> @@ -259,6 +259,7 @@ struct vmt_softc {
>>  char*sc_nic_info;
>>  };
>>  
>> +#define VMT_DEBUG
>>  #ifdef VMT_DEBUG
>>  #define DPRINTF(_arg...)printf(_arg)
>>  #else
>> @@ -534,7 +535,7 @@ vmt_kvop(void *arg, int op, char *key, c
>>  DPRINTF("%s: unable to send rpci command\n", DEVNAME(sc));
>>  sc->sc_rpc_error = 1;
>>  error = EIO;
>> -goto done;
>> +goto close;
>>  }
>>  
>>  if (vm_rpc_get_length(, , ) != 0) {
>> @@ -542,13 +543,13 @@ vmt_kvop(void *arg, int op, char *key, c
>>  DEVNAME(sc));
>>  sc->sc_rpc_error = 1;
>>  error = EIO;
>> -goto done;
>> +goto close;
>>  }
>>  
>>  if (rlen > 0) {
>>  if (rlen + 1 > valuelen) {
>>  error = EMSGSIZE;
>> -goto done;
>> +goto close;
>>  }
>>  
>>  if (vm_rpc_get_data(, value, rlen, ack) != 0) {
>> @@ -556,20 +557,23 @@ vmt_kvop(void *arg, int op, char *key, c
>>  DEVNAME(sc));
>>  sc->sc_rpc_error = 1;
>>  error = EIO;
>> -goto done;
>> +goto close;
>>  }
>>  /* test if response success  */
>>  if (rlen < 2 || value[0] != '1' || value[1] != ' ') {
>>  DPRINTF("%s: host rejected command: %s\n", DEVNAME(sc),
>>  buf);
>>  error = EINVAL;
>> -goto done;
>> +goto close;
>>  }
>>  /* skip response that was tested */
>>  bcopy(value + 2, value, valuelen - 2);
>>  value[rlen - 2] = '\0';
>>  }
>>  
>> + close:
>> +if (vm_rpc_close() != 0)
>> +DPRINTF("%s: unable to close rpci channel\n", DEVNAME(sc));
>>   done:
>>  free(buf, M_TEMP, bufsz);
>>  return (error);
> 
> Index: sys/dev/pv/vmt.c
> ===
> RCS file: /cvs/src/sys/dev/pv/vmt.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 vmt.c
> --- sys/dev/pv/vmt.c  3 Dec 2022 10:57:04 -   1.27
> +++ sys/dev/pv/vmt.c  13 Dec 2022 10:23:45 -
> @@ -534,7 +534,7 @@ vmt_kvop(void *arg, int op, char *key, c
>   DPRINTF("%s: unable to send rpci command\n", DEVNAME(sc));
>   sc->sc_rpc_error = 1;
>   error = EIO;
> - goto done;
> + goto close;
>   }
>  
>   if (vm_rpc_get_length(, , ) != 0) {
> @@ -542,13 +542,13 @@ vmt_kvop(void *arg, int op, char *key, c
>   DEVNAME(sc));
>   sc->sc_rpc_error = 1;
>   error = EIO;
> - goto done;
> + goto close;
>   }
>  
>   if (rlen > 0) {
>   if (rlen + 1 > valuelen) {
>   error = EMSGSIZE;
> - goto done;
> + goto close;
>   }
>  
>   if (vm_rpc_get_data(, value, rlen, ack) != 0) {
> @@ -556,20 +556,23 @@ vmt_kvop(void *arg, int op, char *key, c
>   DEVNAME(sc));
>   sc->sc_rpc_error = 1;
>   error = EIO;
> - goto done;
> + goto close;
>   }
>   /* test if response success  */
>   if (rlen < 2 || value[0] != '1' || value[1] != ' ') {
>   DPRINTF("%s: host rejected command: %s\n", DEVNAME(sc),
>   buf);
>   error = EINVAL;
> - goto done;
> + goto close;
>   }
>   /* skip response that was tested */
>   bcopy(value + 2, value, valuelen - 2);
>   value[rlen - 2] = '\0';
>   }
>  
> + close:
> + if (vm_rpc_close() != 0)
> +DPRINTF("%s: unable to close rpci channel\n", DEVNAME(sc));
>   done:
>   free(buf, M_TEMP, bufsz);
>   return (error);
>