snmpd(8) reduce generic errors

2020-02-14 Thread Martijn van Duren
Apparently not many people check the error count in their snmp stats.
This appears to been here since day 1.

OK?

martijn@

Index: snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.60
diff -u -p -r1.60 snmpe.c
--- snmpe.c 24 Oct 2019 12:39:27 -  1.60
+++ snmpe.c 14 Feb 2020 14:41:44 -
@@ -766,6 +766,8 @@ snmpe_response(struct snmp_message *msg)
msg->sm_varbindresp = ober_unlink_elements(msg->sm_pduend);
 
switch (msg->sm_error) {
+   case SNMP_ERROR_NONE:
+   break;
case SNMP_ERROR_TOOBIG:
stats->snmp_intoobigs++;
break;



Re: dd(1) wording and style

2020-02-14 Thread Ingo Schwarze
Hi,

Jason McIntyre wrote on Fri, Feb 14, 2020 at 07:28:59AM +:
> On Thu, Feb 13, 2020 at 11:25:07PM +0100, Jan Stary wrote:

>> * Fix a factual error in the description of bs: it does not
>>   supersede ibs/obs, dd will error out when both are specified.

> i don;t want to comment on this change

In fact, that's a bug in the code.  POSIX requires:

  bs=expr
Set both input and output block sizes to expr bytes,
superseding ibs= and obs=. 

  (see https://pubs.opengroup.org/onlinepubs/9699919799/utilities/dd.html )

Yours,
  Ingo



Re: apmd: fix autoaction timeout

2020-02-14 Thread Scott Cheloha
On Thu, Feb 13, 2020 at 02:08:32PM +0100, Jeremie Courreges-Anglas wrote:
> On Wed, Feb 12 2020, Scott Cheloha  wrote:
> > On Wed, Feb 12, 2020 at 01:35:22PM +0100, Jeremie Courreges-Anglas wrote:
> >> On Wed, Feb 12 2020, Jeremie Courreges-Anglas  wrote:
> >> > On Sat, Jan 25 2020, Jeremie Courreges-Anglas  wrote:
> >> >> The diff below improves the way apmd -z/-Z may trigger.
> >> >>
> >> >> I think the current behavior is bogus, incrementing and checking
> >> >> apmtimeout like this doesn't make much sense.
> >> >>
> >> >> Here's a proposal:
> >> >> - on APM_POWER_CHANGE events, check the battery level and trigger
> >> >>   autoaction if needed.  This should be enough to make autoaction just
> >> >>   work with drivers like acpibat(4).
> >> >> - on kevent timeout (10mn by default now, maybe too long), also check
> >> >>   the battery level and suspend if needed.  This should be useful only
> >> >>   if your battery driver doesn't send any APM_POWER_CHANGE event.
> >> >>
> >> >> While here I also tweaked the warning.
> >> >
> >> > This has been committed, thanks Ted.
> >> >
> >> >> Some more context:
> >> >> - a subsequent diff would reorder the code to handle similarly the "!rv"
> >> >>   and "ev->ident == ctl_fd" paths
> >> >
> >> > Diff below.
> >> >
> >> >> - I think we want some throttling mechanism, like wait for 1mn after we
> >> >>   resume before autosuspending again.  But I want to fix obvious
> >> >>   problems first.
> >> 
> >> On top of the previous diff, here's a diff to block autoaction for 60
> >> seconds after:
> >> - autoaction triggers; this prevents apmd from sending multiple suspend
> >> requests before the system goes to sleep
> >> - a resume happens; this gives you 60 seconds to fetch and plug your AC
> >> cable if you notice you're low on power
> >> 
> >> A side effect is that apmd now ignores stale acpiac(4) data at resume
> >> time, so it apmd doesn't suspend the system again when you resume with
> >> a low battery and AC plugged.
> >> 
> >> cc'ing Scott since he has a thing for everything time-related. :)
> >
> > For the first case, is there any way you can detect that a suspend is
> > in-progress but not done yet?
> 
> Well, apmd could record that it asked the kernel for a suspend/hibernate
> and skip autoaction as long as it doesn't get a resume event.

Hmmm.  So what happens if the suspend/hibernate fails?  Could apmd(8)
get stuck waiting for a resume that will never happen?

> > I think that'd be cleaner (in some ways) than an autoaction cooldown
> > timer.
> >
> > Whenever I want to add an arbitrary delay that isn't per se required
> > by an interface I wonder whether I'm working around a deficiency in
> > the state machine instead of addressing the root cause.
> >
> > Sometimes it can't be helped, but I have to ask.
> 
> Initially I only cared about the second case, and then noticed that
> APM_POWER_CHANGE events can happen at any time.  Reusing the 60 seconds
> timer looked appealing (cheap) but please see the updated diff below.
> 
> > For the second case, I thought the design of autoaction was to (a)
> > note that the battery was below a particular threshold and (b) take
> > action to avert data loss.  If you resume from suspend with battery
> > below the threshold and no AC I think you would *want* autoaction to
> > trigger.  Like, it sounds like the state machine is working as
> > designed.
> >
> > If the machine is immediately suspending after resume shouldn't you
> > just plug it in before reattempting resume?  Isn't that better than
> > having the battery die on you?
> 
> We can't know when the battery will fail to feed the system.  I suspect
> that the resume sequence itself may drain more power than 60 seconds
> spent idling (wild guess, no power meter at hand).  So I see no
> convincing reason to prevent any use of the system.
> 
> Regarding the user experience, I think the user should be put into
> control.  60 seconds is enough to plug the power cable or take a quick
> look at a document, or even kill apmd if the laptop is really needed
> like, *right now*.  I know I've been in this kind of situation several
> times.

Fair enough.

> So here's an updated diff that:
> - disables autoaction for 60 seconds after resume.  This is still done
>   in a naive way, autoaction won't kick in exactly after 60 seconds
>   after resume.  Good enough for now, I think.

I think this part is fine.

You could use an alarm(3) to pull you out of kevent(2) prematurely.
That could be done in a separate change.

> - prevents autoaction to kick in several times before suspend/hibernate
> - improves naming (suggestions welcome)
> - logs why autoaction doesn't kick in
> - documents the 60 seconds grace period in the manpage
> 
> This still works around the issue with stale acpiac(4) data at resume
> time.
> 
> Thanks for your input so far, I hope I have addressed your concerns.
> 
> Comments / oks?

Just the question above.

> Index: apmd.c
> 

Re: minor bgpd cleanup

2020-02-14 Thread Sebastian Benoit
ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.02.14 14:06:37 +0100:
> Move and rename copy_filterset to rde_filter.c as filterset_copy.
> This way it matches the other filterset_* functions.
> 
> OK?
> -- 
> :wq Claudio
> 
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.400
> diff -u -p -r1.400 bgpd.h
> --- bgpd.h12 Feb 2020 10:33:56 -  1.400
> +++ bgpd.h14 Feb 2020 12:16:43 -
> @@ -1182,8 +1182,6 @@ voidfree_prefixtree(struct prefixset_t
>  void filterlist_free(struct filter_head *);
>  int  host(const char *, struct bgpd_addr *, u_int8_t *);
>  u_int32_tget_bgpid(void);
> -void copy_filterset(struct filter_set_head *,
> - struct filter_set_head *);
>  void expand_networks(struct bgpd_config *);
>  int  prefixset_cmp(struct prefixset_item *, struct prefixset_item *);
>  RB_PROTOTYPE(prefixset_tree, prefixset_item, entry, prefixset_cmp);
> @@ -1261,10 +1259,10 @@ int   pftable_addr_remove(struct pftable_m
>  int  pftable_commit(void);
>  
>  /* rde_filter.c */
> -void  filterset_free(struct filter_set_head *);
> -int   filterset_cmp(struct filter_set *, struct filter_set *);
> -void  filterset_move(struct filter_set_head *,
> - struct filter_set_head *);
> +void filterset_free(struct filter_set_head *);
> +int  filterset_cmp(struct filter_set *, struct filter_set *);
> +void filterset_move(struct filter_set_head *, struct filter_set_head *);
> +void filterset_copy(struct filter_set_head *, struct filter_set_head *);
>  const char   *filterset_name(enum action_types);
>  
>  /* rde_sets.c */
> Index: config.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 config.c
> --- config.c  28 Jan 2020 15:45:46 -  1.94
> +++ config.c  14 Feb 2020 12:21:44 -
> @@ -496,22 +496,6 @@ prepare_listeners(struct bgpd_config *co
>  }
>  
>  void
> -copy_filterset(struct filter_set_head *source, struct filter_set_head *dest)
> -{
> - struct filter_set   *s, *t;
> -
> - if (source == NULL)
> - return;
> -
> - TAILQ_FOREACH(s, source, entry) {
> - if ((t = malloc(sizeof(struct filter_set))) == NULL)
> - fatal(NULL);
> - memcpy(t, s, sizeof(struct filter_set));
> - TAILQ_INSERT_TAIL(dest, t, entry);
> - }
> -}
> -
> -void
>  expand_networks(struct bgpd_config *c)
>  {
>   struct network  *n, *m, *tmp;
> @@ -533,8 +517,7 @@ expand_networks(struct bgpd_config *c)
>   memcpy(>net.prefix, >p.addr,
>   sizeof(m->net.prefix));
>   m->net.prefixlen = psi->p.len;
> - TAILQ_INIT(>net.attrset);
> - copy_filterset(>net.attrset,
> + filterset_copy(>net.attrset,
>   >net.attrset);
>   TAILQ_INSERT_TAIL(nw, m, entry);
>   }
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.403
> diff -u -p -r1.403 parse.y
> --- parse.y   24 Jan 2020 05:44:05 -  1.403
> +++ parse.y   14 Feb 2020 12:21:59 -
> @@ -4076,8 +4076,7 @@ expand_rule(struct filter_rule *rule, st
>   memcpy(r, rule, sizeof(struct 
> filter_rule));
>   memcpy(>match, match,
>   sizeof(struct filter_match));
> - TAILQ_INIT(>set);
> - copy_filterset(set, >set);
> + filterset_copy(set, >set);
>  
>   if (rb != NULL)
>   strlcpy(r->rib, rb->name,
> Index: rde_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
> retrieving revision 1.122
> diff -u -p -r1.122 rde_filter.c
> --- rde_filter.c  13 Aug 2019 12:16:20 -  1.122
> +++ rde_filter.c  14 Feb 2020 12:21:28 -
> @@ -502,6 +502,10 @@ filterset_cmp(struct filter_set *a, stru
>   return (0);
>  }
>  
> +/*
> + * move filterset from source to dest. dest will be initialized first.
> + * After the move source is an empty list.
> + */
>  void
>  filterset_move(struct filter_set_head *source, struct filter_set_head *dest)
>  {
> @@ -509,6 +513,26 @@ filterset_move(struct filter_set_head *s
>   if (source == NULL)
>   return;
>   TAILQ_CONCAT(dest, source, entry);
> +}
> +
> +/*
> + * copy filterset from source 

Re: [PATCH] [www] faq/current.html - be consistent with naming of the 'id' attribute

2020-02-14 Thread Ingo Schwarze
Hi Raf,

Raf Czlonka wrote on Fri, Feb 14, 2020 at 01:53:03PM +:

> Small inconsistency.
> Personally, I prefer id *without* the 'r' but the below is "the odd
> one out" so...

Oops, sorry.

Fixed, thanks for noticing.
  Ingo


> Index: faq/current.html
> ===
> RCS file: /cvs/www/faq/current.html,v
> retrieving revision 1.1029
> diff -u -p -r1.1029 current.html
> --- faq/current.html  13 Feb 2020 16:29:21 -  1.1029
> +++ faq/current.html  14 Feb 2020 13:46:41 -
> @@ -229,7 +229,7 @@ root crontab or /etc/weekly.local<
>  /etc/locate.rc.
>  
>  
> -2020/02/13 - man.conf(5) _whatdb directive no longer 
> supported
> +2020/02/13 - man.conf(5) _whatdb directive no longer 
> supported
>  
>  In https://man.openbsd.org/man.conf.5;>man.conf(5),
>  change lines of the form



Re: [PATCH] [www] books.html - remove superfluous angle bracket

2020-02-14 Thread Raf Czlonka
Ping.

On Mon, Nov 25, 2019 at 11:16:09AM GMT, Raf Czlonka wrote:
> Regards,
> 
> Raf
> 
> Index: books.html
> ===
> RCS file: /cvs/www/books.html,v
> retrieving revision 1.117
> diff -u -p -r1.117 books.html
> --- books.html1 Jun 2019 23:12:47 -   1.117
> +++ books.html25 Nov 2019 11:15:11 -
> @@ -355,7 +355,7 @@ Lots of examples and real world code sni
>  
>  Network administration
>  
> ->Das SSH-Buch
> +Das SSH-Buch
>  (German)
>  by Timo Dotzauer and Tobias Ltticke
>  ISBN 3-938626-03-8



[PATCH] [www] faq/current.html - be consistent with naming of the 'id' attribute

2020-02-14 Thread Raf Czlonka
Hi all,

Small inconsistency.

Personally, I prefer id *without* the 'r' but the below is "the odd
one out" so...

Regards,

Raf

Index: faq/current.html
===
RCS file: /cvs/www/faq/current.html,v
retrieving revision 1.1029
diff -u -p -r1.1029 current.html
--- faq/current.html13 Feb 2020 16:29:21 -  1.1029
+++ faq/current.html14 Feb 2020 13:46:41 -
@@ -229,7 +229,7 @@ root crontab or /etc/weekly.local<
 /etc/locate.rc.
 
 
-2020/02/13 - man.conf(5) _whatdb directive no longer 
supported
+2020/02/13 - man.conf(5) _whatdb directive no longer 
supported
 
 In https://man.openbsd.org/man.conf.5;>man.conf(5),
 change lines of the form



Re: snmpd(8) reduce generic errors

2020-02-14 Thread Gerhard Roth

On 2/14/20 3:42 PM, Martijn van Duren wrote:

Apparently not many people check the error count in their snmp stats.
This appears to been here since day 1.

OK?

martijn@

Index: snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.60
diff -u -p -r1.60 snmpe.c
--- snmpe.c 24 Oct 2019 12:39:27 -  1.60
+++ snmpe.c 14 Feb 2020 14:41:44 -
@@ -766,6 +766,8 @@ snmpe_response(struct snmp_message *msg)
msg->sm_varbindresp = ober_unlink_elements(msg->sm_pduend);
  
  	switch (msg->sm_error) {

+   case SNMP_ERROR_NONE:
+   break;
case SNMP_ERROR_TOOBIG:
stats->snmp_intoobigs++;
break;



ok gerhard@



Re: [PATCH] [www] books.html - remove superfluous angle bracket

2020-02-14 Thread Ingo Schwarze
Hi Raf,

Raf Czlonka wrote on Fri, Feb 14, 2020 at 02:55:20PM +:
> On Mon, Nov 25, 2019 at 11:16:09AM GMT, Raf Czlonka wrote:

>> Index: books.html
>> ===
>> RCS file: /cvs/www/books.html,v
>> retrieving revision 1.117
>> diff -u -p -r1.117 books.html
>> --- books.html   1 Jun 2019 23:12:47 -   1.117
>> +++ books.html   25 Nov 2019 11:15:11 -
>> @@ -355,7 +355,7 @@ Lots of examples and real world code sni
>>  
>>  Network administration
>>  
>> ->Das SSH-Buch
>> +Das SSH-Buch
>>  (German)
>>  by Timo Dotzauer and Tobias Ltticke
>>  ISBN 3-938626-03-8

Committed, thanks.
  Ingo



Re: dd(1) wording and style

2020-02-14 Thread Ingo Schwarze
Hi,

Jason McIntyre wrote on Fri, Feb 14, 2020 at 07:28:59AM +:
> On Thu, Feb 13, 2020 at 11:25:07PM +0100, Jan Stary wrote:

>> -.It Cm seek= Ns Ar n
>> +.It Cm seek Ns = Ns Ar n
>>  Seek
>>  .Ar n
>>  blocks from the beginning of the output before copying.
>> -On non-tape devices, an
>> -.Xr lseek 2
>> -operation is used.
>> -Otherwise, existing blocks are read and the data discarded.
>> -If the user does not have read permission for the tape, it is positioned
>> -using the tape
>> +On a tape device, existing blocks are read and the data discarded;
>> +if the user does not have read permission for the tape,
>> +it is positioned using the tape
>>  .Xr ioctl 2
>>  function calls.
>> +On all other devices devices, an
>> +.Xr lseek 2
>> +operation is used.
>>  If the seek operation is past the end of file, space from the current
>>  end of file to the specified offset is filled with blocks of NUL bytes.

> i think this change is ok. however i think non-tape is clearer than "on
> all other devices". it's not a biggie, but i think the current stress on
> non-tape devices is probably intentional.

The patch is misleading.  The sentence "If the seek operation is
past the end of file..." only applies to tape devices, so it must
not follow a sentence about lseek(2).

I'm not convinced the text needs to be changed.

The existing order makes sense because the non-tape case can be
handled quickly, and then all the remaining complicated explanations
apply to tapes only.

Or what would you want to change, and why?


>> @@ -135,14 +137,10 @@ Otherwise, input data is read and discar
>>  For pipes, the correct number of bytes is read.
>>  For all other devices, the correct number of blocks is read without
>>  distinguishing between a partial or complete block being read.
>> -.It Xo
>> -.Sm off
>> -.Cm status= Ar value
>> -.Sm on
>> -.Xc
>> -Where
>> +.It Cm status Ns = Ns Ar value
>> +where

> you're correct that "Where" doesn;t really start a sentence, but this is
> sentence start position. using a small letter is also incorrect.
> 
> in all honesty i would leave this, because it is not easy to rewrite in
> a way that sounds natural. but maybe
> 
>   The
>   .Ar value
>   parameter is one of the symbols from the following list:

That would look ugly by causing the line to wrap, but what about
the patch below?

Yours,
  Ingo


Index: dd.1
===
RCS file: /cvs/src/bin/dd/dd.1,v
retrieving revision 1.36
diff -u -r1.36 dd.1
--- dd.114 Feb 2020 15:55:57 -  1.36
+++ dd.114 Feb 2020 16:33:23 -
@@ -136,9 +136,9 @@
 For all other devices, the correct number of blocks is read without
 distinguishing between a partial or complete block being read.
 .It Cm status Ns = Ns Ar value
-Where
+The
 .Ar value
-is one of the symbols from the following list.
+is one of the symbols from the following list:
 .Bl -tag -width unblock
 .It Cm noxfer
 Do not print the transfer statistics as the last line of status output.
@@ -147,9 +147,9 @@
 Error messages are shown; informational messages are not.
 .El
 .It Cm conv Ns = Ns Ar value Ns Op , Ns Ar value ...
-Where
+Each
 .Ar value
-is one of the symbols from the following list.
+is one of the symbols from the following list:
 .Bl -tag -width unblock
 .It Cm ascii
 The same as the



Re: macppc base-clang -msvr4-struct-return

2020-02-14 Thread Todd Mortimer
On Wed, Feb 12, 2020 at 04:05:38PM -0500, George Koehler wrote:
> This is the everything diff: it includes the clang diff that I sent to
> tech@, plus the major version bumps for libLLVM libc++ libc++abi.
> 
>  - distrib/sets/lists: put the new versions in the base sets.
>  - gnu/llvm/tools/clang: -msvr4-struct-return
>  - gnu/usr.bin/clang/libLLVM, lib/libcxx, lib/libcxxabi: new versions

I think this is good to go.

I built through the ABI change with this diff using the directions
George posted earlier:

> To update from source, you must build clang before you build the
> new versions of base libraries.
> First, change major=2 to major=1 in
> /usr/src/gnu/usr.bin/clang/libLLVM/shlib_version.
> 
> Then build clang:
> 
> # cd /usr/src/gnu/usr.bin/clang
> # make obj
> # make
> # make install
> 
> Then change major=1 back to major=2 in
> /usr/src/gnu/usr.bin/clang/libLLVM/shlib_version.
> Then build userland (including clang again).

Then I built a new snapshot and sysupgraded to that snapshot, all
without incident. The snapshot is in my home dir on cvs
(snapshots/macppc) if anyone wants to avoid the build. I checked the
tarballs and the right versions of the bumped libs are all there.

ok mortimer@


> 
> Index: distrib/sets/lists/base/md.amd64
> ===
> RCS file: /cvs/src/distrib/sets/lists/base/md.amd64,v
> retrieving revision 1.875
> diff -u -p -r1.875 md.amd64
> --- distrib/sets/lists/base/md.amd64  30 Dec 2019 02:20:33 -  1.875
> +++ distrib/sets/lists/base/md.amd64  12 Feb 2020 20:52:32 -
> @@ -81,9 +81,9 @@
>  ./usr/lib/gcc-lib/amd64-unknown-openbsd6.6/4.2.1/collect2
>  ./usr/lib/gcc-lib/amd64-unknown-openbsd6.6/4.2.1/libgcc.a
>  ./usr/lib/gcc-lib/amd64-unknown-openbsd6.6/4.2.1/specs
> -./usr/lib/libLLVM.so.1.0
> -./usr/lib/libc++.so.3.0
> -./usr/lib/libc++abi.so.1.0
> +./usr/lib/libLLVM.so.2.0
> +./usr/lib/libc++.so.4.0
> +./usr/lib/libc++abi.so.2.0
>  ./usr/lib/libcompiler_rt.a
>  ./usr/lib/libstdc++.so.57.0
>  ./usr/libdata/perl5/amd64-openbsd
> Index: distrib/sets/lists/base/md.arm64
> ===
> RCS file: /cvs/src/distrib/sets/lists/base/md.arm64,v
> retrieving revision 1.22
> diff -u -p -r1.22 md.arm64
> --- distrib/sets/lists/base/md.arm64  30 Dec 2019 02:20:34 -  1.22
> +++ distrib/sets/lists/base/md.arm64  12 Feb 2020 20:52:32 -
> @@ -37,9 +37,9 @@
>  ./usr/lib/crtbeginS.o
>  ./usr/lib/crtend.o
>  ./usr/lib/crtendS.o
> -./usr/lib/libLLVM.so.1.0
> -./usr/lib/libc++.so.3.0
> -./usr/lib/libc++abi.so.1.0
> +./usr/lib/libLLVM.so.2.0
> +./usr/lib/libc++.so.4.0
> +./usr/lib/libc++abi.so.2.0
>  ./usr/lib/libcompiler_rt.a
>  ./usr/libdata/perl5/aarch64-openbsd
>  ./usr/libdata/perl5/aarch64-openbsd/.packlist
> Index: distrib/sets/lists/base/md.armv7
> ===
> RCS file: /cvs/src/distrib/sets/lists/base/md.armv7,v
> retrieving revision 1.284
> diff -u -p -r1.284 md.armv7
> --- distrib/sets/lists/base/md.armv7  30 Dec 2019 02:20:34 -  1.284
> +++ distrib/sets/lists/base/md.armv7  12 Feb 2020 20:52:32 -
> @@ -37,9 +37,9 @@
>  ./usr/lib/crtbeginS.o
>  ./usr/lib/crtend.o
>  ./usr/lib/crtendS.o
> -./usr/lib/libLLVM.so.1.0
> -./usr/lib/libc++.so.3.0
> -./usr/lib/libc++abi.so.1.0
> +./usr/lib/libLLVM.so.2.0
> +./usr/lib/libc++.so.4.0
> +./usr/lib/libc++abi.so.2.0
>  ./usr/lib/libcompiler_rt.a
>  ./usr/libdata/perl5/arm-openbsd
>  ./usr/libdata/perl5/arm-openbsd/.packlist
> Index: distrib/sets/lists/base/md.i386
> ===
> RCS file: /cvs/src/distrib/sets/lists/base/md.i386,v
> retrieving revision 1.1254
> diff -u -p -r1.1254 md.i386
> --- distrib/sets/lists/base/md.i386   30 Dec 2019 02:20:34 -  1.1254
> +++ distrib/sets/lists/base/md.i386   12 Feb 2020 20:52:32 -
> @@ -78,9 +78,9 @@
>  ./usr/lib/crtbeginS.o
>  ./usr/lib/crtend.o
>  ./usr/lib/crtendS.o
> -./usr/lib/libLLVM.so.1.0
> -./usr/lib/libc++.so.3.0
> -./usr/lib/libc++abi.so.1.0
> +./usr/lib/libLLVM.so.2.0
> +./usr/lib/libc++.so.4.0
> +./usr/lib/libc++abi.so.2.0
>  ./usr/lib/libcompiler_rt.a
>  ./usr/libdata/perl5/i386-openbsd
>  ./usr/libdata/perl5/i386-openbsd/.packlist
> Index: distrib/sets/lists/base/md.loongson
> ===
> RCS file: /cvs/src/distrib/sets/lists/base/md.loongson,v
> retrieving revision 1.456
> diff -u -p -r1.456 md.loongson
> --- distrib/sets/lists/base/md.loongson   30 Dec 2019 02:20:34 -  
> 1.456
> +++ distrib/sets/lists/base/md.loongson   12 Feb 2020 20:52:32 -
> @@ -48,9 +48,9 @@
>  ./usr/lib/gcc-lib/mips64el-unknown-openbsd6.6/4.2.1/collect2
>  ./usr/lib/gcc-lib/mips64el-unknown-openbsd6.6/4.2.1/libgcc.a
>  ./usr/lib/gcc-lib/mips64el-unknown-openbsd6.6/4.2.1/specs
> -./usr/lib/libLLVM.so.1.0
> -./usr/lib/libc++.so.3.0
> -./usr/lib/libc++abi.so.1.0
> 

em(4) towards multiqueues

2020-02-14 Thread Martin Pieuchot
This diff introduces the concept of "queue" in the em(4) driver.  The
logic present in ix(4) has been matched for coherency.

Currently the driver uses a single queue and the diff below doesn't
change anything in that regard.  It can be viewed as the introduction
of an abstraction.

I'd like to get this in to reduce the differences between em(4) and
ix(4) when it comes to multiqueues.  My intend is to keep the upcoming
changes as small as possible such that we can concentrate our efforts on
the important bits.  So far we started discussing the interaction between
CPU and mapping MSIX vectors but other related topics will appear :o)

I'm running this on:

em0 at pci1 dev 0 function 0 "Intel I210" rev 0x03: msi
em0 at pci0 dev 20 function 0 "Intel I354 SGMII" rev 0x03: msi

More tests are always welcome ;)

Index: dev/pci/if_em.c
===
RCS file: /cvs/src/sys/dev/pci/if_em.c,v
retrieving revision 1.344
diff -u -p -r1.344 if_em.c
--- dev/pci/if_em.c 4 Feb 2020 10:59:23 -   1.344
+++ dev/pci/if_em.c 14 Feb 2020 17:13:06 -
@@ -257,25 +257,25 @@ void em_free_transmit_structures(struct 
 void em_free_receive_structures(struct em_softc *);
 void em_update_stats_counters(struct em_softc *);
 void em_disable_aspm(struct em_softc *);
-void em_txeof(struct em_softc *);
+void em_txeof(struct em_queue *);
 int  em_allocate_receive_structures(struct em_softc *);
 int  em_allocate_transmit_structures(struct em_softc *);
 int  em_allocate_desc_rings(struct em_softc *);
-int  em_rxfill(struct em_softc *);
+int  em_rxfill(struct em_queue *);
 void em_rxrefill(void *);
-int  em_rxeof(struct em_softc *);
+int  em_rxeof(struct em_queue *);
 void em_receive_checksum(struct em_softc *, struct em_rx_desc *,
 struct mbuf *);
-u_int  em_transmit_checksum_setup(struct em_softc *, struct mbuf *, u_int,
+u_int  em_transmit_checksum_setup(struct em_queue *, struct mbuf *, u_int,
u_int32_t *, u_int32_t *);
 void em_iff(struct em_softc *);
 #ifdef EM_DEBUG
 void em_print_hw_stats(struct em_softc *);
 #endif
 void em_update_link_status(struct em_softc *);
-int  em_get_buf(struct em_softc *, int);
+int  em_get_buf(struct em_queue *, int);
 void em_enable_hw_vlans(struct em_softc *);
-u_int em_encap(struct em_softc *, struct mbuf *);
+u_int em_encap(struct em_queue *, struct mbuf *);
 void em_smartspeed(struct em_softc *);
 int  em_82547_fifo_workaround(struct em_softc *, int);
 void em_82547_update_fifo_head(struct em_softc *, int);
@@ -286,8 +286,8 @@ int  em_dma_malloc(struct em_softc *, bu
 void em_dma_free(struct em_softc *, struct em_dma_alloc *);
 u_int32_t em_fill_descriptors(u_int64_t address, u_int32_t length,
  PDESC_ARRAY desc_array);
-void em_flush_tx_ring(struct em_softc *);
-void em_flush_rx_ring(struct em_softc *);
+void em_flush_tx_ring(struct em_queue *);
+void em_flush_rx_ring(struct em_queue *);
 void em_flush_desc_rings(struct em_softc *);
 
 /*
@@ -383,7 +383,6 @@ em_attach(struct device *parent, struct 
 
timeout_set(>timer_handle, em_local_timer, sc);
timeout_set(>tx_fifo_timer_handle, em_82547_move_tail, sc);
-   timeout_set(>rx_refill, em_rxrefill, sc);
 
/* Determine hardware revision */
em_identify_hardware(sc);
@@ -601,6 +600,7 @@ em_start(struct ifqueue *ifq)
u_int head, free, used;
struct mbuf *m;
int post = 0;
+   struct em_queue *que = sc->queues; /* Use only first queue. */
 
if (!sc->link_active) {
ifq_purge(ifq);
@@ -608,15 +608,15 @@ em_start(struct ifqueue *ifq)
}
 
/* calculate free space */
-   head = sc->sc_tx_desc_head;
-   free = sc->sc_tx_desc_tail;
+   head = que->tx.sc_tx_desc_head;
+   free = que->tx.sc_tx_desc_tail;
if (free <= head)
free += sc->sc_tx_slots;
free -= head;
 
if (sc->hw.mac_type != em_82547) {
-   bus_dmamap_sync(sc->sc_dmat, sc->sc_tx_dma.dma_map,
-   0, sc->sc_tx_dma.dma_map->dm_mapsize,
+   bus_dmamap_sync(sc->sc_dmat, que->tx.sc_tx_dma.dma_map,
+   0, que->tx.sc_tx_dma.dma_map->dm_mapsize,
BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
}
 
@@ -631,7 +631,7 @@ em_start(struct ifqueue *ifq)
if (m == NULL)
break;
 
-   used = em_encap(sc, m);
+   used = em_encap(que, m);
if (used == 0) {
m_freem(m);
continue;
@@ -656,8 +656,8 @@ em_start(struct ifqueue *ifq)
if (sc->link_duplex == HALF_DUPLEX)
em_82547_move_tail_locked(sc);
else {
-   E1000_WRITE_REG(>hw, TDT(0),
-   

iked.conf.5: Add BUGS section

2020-02-14 Thread Klemens Nanni
I'm experiencing some issues with iked(8) and the first one took
me much longer than appreciated:  order of commands matters.

This works:

ikev2 passive esp \
inet6 \
proto gre \
from A to B \
local any peer any

But switching `proto' and `inet' breaks it, not reporting any further
information.

I ran into the broken config in the first place, tried a few iterations
and finally removed `proto' to "unbreak" it, thinking this particular
command was broken in general.

Since fixing parse.y is another journey I'm currently not up for, the
BUGS section seems appropiate until order is truly irrelevant.

Feedback? OK?


Index: iked.conf.5
===
RCS file: /cvs/src/sbin/iked/iked.conf.5,v
retrieving revision 1.61
diff -u -p -r1.61 iked.conf.5
--- iked.conf.5 10 Feb 2020 13:18:20 -  1.61
+++ iked.conf.5 14 Feb 2020 19:48:38 -
@@ -1006,3 +1006,5 @@ The
 .Xr iked 8
 program was written by
 .An Reyk Floeter Aq Mt r...@openbsd.org .
+.Sh BUGS
+Commands must be specified in the same order as documented.



Re: dd(1) wording and style

2020-02-14 Thread Jason McIntyre
On Fri, Feb 14, 2020 at 05:37:27PM +0100, Ingo Schwarze wrote:
> Hi,
> 
> Jason McIntyre wrote on Fri, Feb 14, 2020 at 07:28:59AM +:
> > On Thu, Feb 13, 2020 at 11:25:07PM +0100, Jan Stary wrote:
> 
> >> -.It Cm seek= Ns Ar n
> >> +.It Cm seek Ns = Ns Ar n
> >>  Seek
> >>  .Ar n
> >>  blocks from the beginning of the output before copying.
> >> -On non-tape devices, an
> >> -.Xr lseek 2
> >> -operation is used.
> >> -Otherwise, existing blocks are read and the data discarded.
> >> -If the user does not have read permission for the tape, it is positioned
> >> -using the tape
> >> +On a tape device, existing blocks are read and the data discarded;
> >> +if the user does not have read permission for the tape,
> >> +it is positioned using the tape
> >>  .Xr ioctl 2
> >>  function calls.
> >> +On all other devices devices, an
> >> +.Xr lseek 2
> >> +operation is used.
> >>  If the seek operation is past the end of file, space from the current
> >>  end of file to the specified offset is filled with blocks of NUL bytes.
> 
> > i think this change is ok. however i think non-tape is clearer than "on
> > all other devices". it's not a biggie, but i think the current stress on
> > non-tape devices is probably intentional.
> 
> The patch is misleading.  The sentence "If the seek operation is
> past the end of file..." only applies to tape devices, so it must
> not follow a sentence about lseek(2).
> 
> I'm not convinced the text needs to be changed.
> 
> The existing order makes sense because the non-tape case can be
> handled quickly, and then all the remaining complicated explanations
> apply to tapes only.
> 
> Or what would you want to change, and why?
> 

i understand. so i agree with your assessment (i failed to read further
into the text!)

> 
> >> @@ -135,14 +137,10 @@ Otherwise, input data is read and discar
> >>  For pipes, the correct number of bytes is read.
> >>  For all other devices, the correct number of blocks is read without
> >>  distinguishing between a partial or complete block being read.
> >> -.It Xo
> >> -.Sm off
> >> -.Cm status= Ar value
> >> -.Sm on
> >> -.Xc
> >> -Where
> >> +.It Cm status Ns = Ns Ar value
> >> +where
> 
> > you're correct that "Where" doesn;t really start a sentence, but this is
> > sentence start position. using a small letter is also incorrect.
> > 
> > in all honesty i would leave this, because it is not easy to rewrite in
> > a way that sounds natural. but maybe
> > 
> > The
> > .Ar value
> > parameter is one of the symbols from the following list:
> 
> That would look ugly by causing the line to wrap, but what about
> the patch below?
>

line wrap?

anyway, i'm ok with your diff, but in all honesty, i'd just get over the
fact that it isn;t quite lovely, and just start

.Ar value
is...

but i'm fine with your approach too.
jmc

> Yours,
>   Ingo
> 
> 
> Index: dd.1
> ===
> RCS file: /cvs/src/bin/dd/dd.1,v
> retrieving revision 1.36
> diff -u -r1.36 dd.1
> --- dd.1  14 Feb 2020 15:55:57 -  1.36
> +++ dd.1  14 Feb 2020 16:33:23 -
> @@ -136,9 +136,9 @@
>  For all other devices, the correct number of blocks is read without
>  distinguishing between a partial or complete block being read.
>  .It Cm status Ns = Ns Ar value
> -Where
> +The
>  .Ar value
> -is one of the symbols from the following list.
> +is one of the symbols from the following list:
>  .Bl -tag -width unblock
>  .It Cm noxfer
>  Do not print the transfer statistics as the last line of status output.
> @@ -147,9 +147,9 @@
>  Error messages are shown; informational messages are not.
>  .El
>  .It Cm conv Ns = Ns Ar value Ns Op , Ns Ar value ...
> -Where
> +Each
>  .Ar value
> -is one of the symbols from the following list.
> +is one of the symbols from the following list:
>  .Bl -tag -width unblock
>  .It Cm ascii
>  The same as the
> 



proc.h & time to re-build kernel

2020-02-14 Thread Martin Pieuchot
During n2k20 we asked ourself why touching a header makes us rebuild a lot
of the kernel again.  New year or new cycle maybe, that's time for a new
round of cleanup ;o)

Today I figured out why when I touch  I have to rebuild too
much.  I came up with two cleanups, diff below is the first one.

 is the only sys/ header including .  It is
not always needed and do not necessarily require including .

The diff below removes it from the places where it isn't needed, replaces
it with  when that's what the file need, adds a forward
definition when it's enough and adds it in the places where a `proc' is
dereferenced.

Compiled for all affected architectures.

Ok?

Index: arch/armv7/omap/ommmc.c
===
RCS file: /cvs/src/sys/arch/armv7/omap/ommmc.c,v
retrieving revision 1.32
diff -u -p -r1.32 ommmc.c
--- arch/armv7/omap/ommmc.c 13 Jan 2020 13:30:42 -  1.32
+++ arch/armv7/omap/ommmc.c 14 Feb 2020 18:47:00 -
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
Index: arch/sparc64/dev/stp_sbus.c
===
RCS file: /cvs/src/sys/arch/sparc64/dev/stp_sbus.c,v
retrieving revision 1.10
diff -u -p -r1.10 stp_sbus.c
--- arch/sparc64/dev/stp_sbus.c 26 Jun 2008 05:42:13 -  1.10
+++ arch/sparc64/dev/stp_sbus.c 14 Feb 2020 18:47:29 -
@@ -42,7 +42,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
Index: dev/acpi/dwiic_acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/dwiic_acpi.c,v
retrieving revision 1.12
diff -u -p -r1.12 dwiic_acpi.c
--- dev/acpi/dwiic_acpi.c   4 Aug 2019 15:44:17 -   1.12
+++ dev/acpi/dwiic_acpi.c   14 Feb 2020 18:48:05 -
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
Index: dev/cardbus/cardslot.c
===
RCS file: /cvs/src/sys/dev/cardbus/cardslot.c,v
retrieving revision 1.21
diff -u -p -r1.21 cardslot.c
--- dev/cardbus/cardslot.c  15 Sep 2016 02:00:17 -  1.21
+++ dev/cardbus/cardslot.c  14 Feb 2020 18:48:10 -
@@ -34,7 +34,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
Index: dev/fdt/imxesdhc.c
===
RCS file: /cvs/src/sys/dev/fdt/imxesdhc.c,v
retrieving revision 1.12
diff -u -p -r1.12 imxesdhc.c
--- dev/fdt/imxesdhc.c  13 Jan 2020 13:30:00 -  1.12
+++ dev/fdt/imxesdhc.c  14 Feb 2020 18:51:33 -
@@ -22,7 +22,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
Index: dev/ic/dwiic.c
===
RCS file: /cvs/src/sys/dev/ic/dwiic.c,v
retrieving revision 1.9
diff -u -p -r1.9 dwiic.c
--- dev/ic/dwiic.c  11 Jan 2020 20:07:40 -  1.9
+++ dev/ic/dwiic.c  14 Feb 2020 18:48:30 -
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
Index: dev/ic/dwiicvar.h
===
RCS file: /cvs/src/sys/dev/ic/dwiicvar.h,v
retrieving revision 1.3
diff -u -p -r1.3 dwiicvar.h
--- dev/ic/dwiicvar.h   16 Mar 2019 02:40:43 -  1.3
+++ dev/ic/dwiicvar.h   14 Feb 2020 18:48:43 -
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
Index: dev/pci/drm/i915/intel_breadcrumbs.c
===
RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_breadcrumbs.c,v
retrieving revision 1.5
diff -u -p -r1.5 intel_breadcrumbs.c
--- dev/pci/drm/i915/intel_breadcrumbs.c30 Jan 2020 08:51:27 -  
1.5
+++ dev/pci/drm/i915/intel_breadcrumbs.c14 Feb 2020 18:50:59 -
@@ -28,7 +28,6 @@
 #else
 #include 
 #include 
-#include 
 #endif
 
 #include "i915_drv.h"
Index: dev/pci/drm/include/drm/drmP.h
===
RCS file: /cvs/src/sys/dev/pci/drm/include/drm/drmP.h,v
retrieving revision 1.5
diff -u -p -r1.5 drmP.h
--- dev/pci/drm/include/drm/drmP.h  18 Aug 2019 13:11:47 -  1.5
+++ dev/pci/drm/include/drm/drmP.h  14 Feb 2020 19:01:02 -
@@ -45,7 +45,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
Index: dev/pci/drm/include/linux/sched.h
===
RCS file: /cvs/src/sys/dev/pci/drm/include/linux/sched.h,v
retrieving revision 1.1
diff -u -p -r1.1 sched.h
--- dev/pci/drm/include/linux/sched.h   14 Apr 2019 10:14:53 -  1.1
+++ dev/pci/drm/include/linux/sched.h   14 Feb 2020 19:01:41 -
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -47,6 +46,7 @@ long schedule_timeout(long);
 
 #define io_schedule_timeout(x) schedule_timeout(x)
 
+struct proc;
 

Re: extern already declared

2020-02-14 Thread Ingo Schwarze
Hi,

Todd C. Miller wrote on Sun, Feb 09, 2020 at 09:49:35AM -0700:
> On Sun, 09 Feb 2020 17:46:51 +0100, Jan Stary wrote:
 
>> Whenever unistd.h declares getopt(3), it also declares
>> the extern optind and optarg, so files including unistd.h
>> don't need to declare those themselves, right?

> Correct.  Most of those date back from before optind and optarg
> were defined for you by unistd.h.

Committed, thanks.
  Ingo



diff: httpd, handling no content-lenght and not chunked trasfer

2020-02-14 Thread YASUOKA Masahiko
Hi,

When httpd received the following request,

  POST /cgi-bin/test.cgi HTTP/1.0
  Host: 127.0.0.1
  Content-Type: application/json
  Content-Length: 0

if the request is handled by fastcgi, STDIN is closed immediately.

But the problem is that STDIN is never closed if the request doesn't
have "Content-Length" header like the following.  read(STDIN) doesn't
return forever.

  POST /cgi-bin/test.cgi HTTP/1.0
  Host: 127.0.0.1
  Content-Type: application/json

In RFC7230 Section 3.3.3

|   6.  If this is a request message and none of the above are true, then
|   the message body length is zero (no message body is present).

it mentions the body length is zero if both Content-Length and
Content-Transfer-Encoding is not specified.

The diff is to fix the problm.

ok?

Treat the message body length as 0 byte if Content-Length is not
specified and chunked transfer is not used.

Index: usr.sbin/httpd/server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.136
diff -u -p -r1.136 server_http.c
--- usr.sbin/httpd/server_http.c14 Jan 2020 20:48:57 -  1.136
+++ usr.sbin/httpd/server_http.c4 Feb 2020 06:30:14 -
@@ -421,12 +421,12 @@ server_read_http(struct bufferevent *bev
/* HTTP request payload */
if (clt->clt_toread > 0)
bev->readcb = server_read_httpcontent;
-
-   /* Single-pass HTTP body */
-   if (clt->clt_toread < 0) {
-   clt->clt_toread = TOREAD_UNLIMITED;
-   bev->readcb = server_read;
-   }
+   else if (!desc->http_chunked)
+   /*
+* When no content-length and no chunked
+* transfer, it means body length is zero.
+*/
+   clt->clt_toread = 0;
break;
default:
server_abort_http(clt, 405, "method not allowed");



Re: apmd: fix autoaction timeout

2020-02-14 Thread Jeremie Courreges-Anglas
On Fri, Feb 14 2020, Scott Cheloha  wrote:
> On Thu, Feb 13, 2020 at 02:08:32PM +0100, Jeremie Courreges-Anglas wrote:
>> On Wed, Feb 12 2020, Scott Cheloha  wrote:
>> > On Wed, Feb 12, 2020 at 01:35:22PM +0100, Jeremie Courreges-Anglas wrote:
>> >> On Wed, Feb 12 2020, Jeremie Courreges-Anglas  wrote:
>> >> > On Sat, Jan 25 2020, Jeremie Courreges-Anglas  wrote:
>> >> >> The diff below improves the way apmd -z/-Z may trigger.
>> >> >>
>> >> >> I think the current behavior is bogus, incrementing and checking
>> >> >> apmtimeout like this doesn't make much sense.
>> >> >>
>> >> >> Here's a proposal:
>> >> >> - on APM_POWER_CHANGE events, check the battery level and trigger
>> >> >>   autoaction if needed.  This should be enough to make autoaction just
>> >> >>   work with drivers like acpibat(4).
>> >> >> - on kevent timeout (10mn by default now, maybe too long), also check
>> >> >>   the battery level and suspend if needed.  This should be useful only
>> >> >>   if your battery driver doesn't send any APM_POWER_CHANGE event.
>> >> >>
>> >> >> While here I also tweaked the warning.
>> >> >
>> >> > This has been committed, thanks Ted.
>> >> >
>> >> >> Some more context:
>> >> >> - a subsequent diff would reorder the code to handle similarly the 
>> >> >> "!rv"
>> >> >>   and "ev->ident == ctl_fd" paths
>> >> >
>> >> > Diff below.
>> >> >
>> >> >> - I think we want some throttling mechanism, like wait for 1mn after we
>> >> >>   resume before autosuspending again.  But I want to fix obvious
>> >> >>   problems first.
>> >> 
>> >> On top of the previous diff, here's a diff to block autoaction for 60
>> >> seconds after:
>> >> - autoaction triggers; this prevents apmd from sending multiple suspend
>> >> requests before the system goes to sleep
>> >> - a resume happens; this gives you 60 seconds to fetch and plug your AC
>> >> cable if you notice you're low on power
>> >> 
>> >> A side effect is that apmd now ignores stale acpiac(4) data at resume
>> >> time, so it apmd doesn't suspend the system again when you resume with
>> >> a low battery and AC plugged.
>> >> 
>> >> cc'ing Scott since he has a thing for everything time-related. :)
>> >
>> > For the first case, is there any way you can detect that a suspend is
>> > in-progress but not done yet?
>> 
>> Well, apmd could record that it asked the kernel for a suspend/hibernate
>> and skip autoaction as long as it doesn't get a resume event.
>
> Hmmm.  So what happens if the suspend/hibernate fails?  Could apmd(8)
> get stuck waiting for a resume that will never happen?

Well, if suspend fails maybe there's no point in having apmd retry
a suspend. Also what would get stuck is only the autoaction behavior,
the rest of apmd would keep on working as usual.

acpi_sleep_state seems to properly send a RESUME event even if
suspend/hibernate fails, except in one error case.

But depending on a resume event is not portable, the APM code in i386
and loongson doesn't notify userland about resumes. Something that ought
to be fixed.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



diff: nvme.c

2020-02-14 Thread YASUOKA Masahiko
Hi,

I have a problem that kernel core dumping always hangs around first
8MB.  The diff attached fixes the problem.

In nvme_poll():

 930 while (!ISSET(state.c.flags, htole16(NVME_CQE_PHASE))) {
 931 if (nvme_q_complete(sc, q) == 0)
 932 delay(10);
 933 
 934 /* XXX no timeout? */
 935 }

this loop is to wait commands completion when the system is cold.  If
CQE_PHASE flag on state.c.flags is set, it breaks the loop.  In
nvme_q_complete():

 979 int
 980 nvme_q_complete(struct nvme_softc *sc, struct nvme_queue *q)
 981 {
 982 struct nvme_ccb *ccb;
 983 struct nvme_cqe *ring = NVME_DMA_KVA(q->q_cq_dmamem), *cqe;
 984 u_int32_t head;
 985 u_int16_t flags;
 986 int rv = 0;
 987 
 988 if (!mtx_enter_try(>q_cq_mtx))
 989 return (-1);
 990 
 991 head = q->q_cq_head;
 992 
 993 nvme_dmamem_sync(sc, q->q_cq_dmamem, BUS_DMASYNC_POSTREAD);
 994 for (;;) {
 995 cqe = [head];
 996 flags = lemtoh16(>flags);
 997 if ((flags & NVME_CQE_PHASE) != q->q_cq_phase)
 998 break;
 999 
1000 ccb = >sc_ccbs[cqe->cid];
1001 ccb->ccb_done(sc, ccb, cqe);
1002 
1003 if (++head >= q->q_entries) {
1004 head = 0;
1005 q->q_cq_phase ^= NVME_CQE_PHASE;
1006 }
1007 
1008 rv = 1;
1009 }
1010 nvme_dmamem_sync(sc, q->q_cq_dmamem, BUS_DMASYNC_PREREAD);

See #997, the same CQE_PHASE frag is used for breaking the loop, but
see #1005.  It means is inverted when the ring buffer is looped.
Please note this.

In ccb->ccb_done()( which is actually nvme_poll_done()):

 954 void
 955 nvme_poll_done(struct nvme_softc *sc, struct nvme_ccb *ccb,
 956 struct nvme_cqe *cqe)
 957 {
 958 struct nvme_poll_state *state = ccb->ccb_cookie;
 959 
 960 SET(cqe->flags, htole16(NVME_CQE_PHASE));
 961 state->c = *cqe;
 962 }

struct nvme_poll_state *state is the same object "state" in
nvme_poll().  cqe is a mapped object of a physical queue.

On #960 set NVME_CQE_PHASE bit on "cqe" and copies it to "state".

I think nvme_poll_done() should change only the flag on "state", but
should not change the flag on "cqe".  Also let's remember that the
flag meaning on queue is inverted when the ring is looped.  As the
result of modifying the flag on the physical queue, it might happens
that the loop in nvme_q_complete() will never break.


comment? ok?

Index: sys/dev/ic/nvme.c
===
RCS file: /disk/cvs/openbsd/src/sys/dev/ic/nvme.c,v
retrieving revision 1.63
diff -u -p -r1.63 nvme.c
--- sys/dev/ic/nvme.c   27 Jul 2019 13:20:12 -  1.63
+++ sys/dev/ic/nvme.c   15 Feb 2020 02:16:22 -
@@ -957,8 +957,8 @@ nvme_poll_done(struct nvme_softc *sc, st
 {
struct nvme_poll_state *state = ccb->ccb_cookie;
 
-   SET(cqe->flags, htole16(NVME_CQE_PHASE));
state->c = *cqe;
+   SET(state->c.flags, htole16(NVME_CQE_PHASE));
 }
 
 void



Re: remove needless #ifdef

2020-02-14 Thread YASUOKA Masahiko
committed.  Thanks

On Fri, 14 Feb 2020 08:48:06 +0100
Claudio Jeker  wrote:
> On Thu, Feb 13, 2020 at 11:50:46PM +0100, Jan Stary wrote:
>> On Feb 10 09:28:38, yasu...@openbsd.org wrote:
>> > Hi,
>> > 
>> > On Sun, 09 Feb 2020 19:28:50 +0100
>> > Jeremie Courreges-Anglas  wrote:
>> > > On Sun, Feb 09 2020, Jan Stary  wrote:
>> > >> Currently, sys/net/pipex_local.h asks #ifdef __OpenBSD__
>> > >> and if so, defines "Static" to be nothing, to use it later.
>> > >> That can go away, right?
>> > > 
>> > > I believe that's something the IIJ folks want to keep, cc'ing Yasuoka.
>> > 
>> > I once thought keeping "static" is better for maintaining the code,
>> > but now I don't think it's necessary.  So it's ok to remove them.
>> 
>> So can we remove the please?
> 
> Yes. OK claudio
> 
>>  Jan
>> 
>> > >>
>> > >> Index: sys/net/pipex_local.h
>> > >> ===
>> > >> RCS file: /cvs/src/sys/net/pipex_local.h,v
>> > >> retrieving revision 1.30
>> > >> diff -u -p -r1.30 pipex_local.h
>> > >> --- sys/net/pipex_local.h   31 Jan 2019 18:01:14 -  1.30
>> > >> +++ sys/net/pipex_local.h   9 Feb 2020 15:26:51 -
>> > >> @@ -26,12 +26,6 @@
>> > >>   * SUCH DAMAGE.
>> > >>   */
>> > >>  
>> > >> -#ifdef __OpenBSD__
>> > >> -#define Static
>> > >> -#else
>> > >> -#define Static static
>> > >> -#endif
>> > >> -
>> > >>  #definePIPEX_PPTP  1
>> > >>  #definePIPEX_L2TP  1
>> > >>  #definePIPEX_PPPOE 1
>> > >> @@ -372,59 +366,56 @@ extern struct pipex_hash_head pipex_id_h
>> > >>  #define PIPEX_TCP_OPTLEN 40
>> > >>  #definePIPEX_L2TP_MINLEN   8
>> > >>  
>> > >> -/*
>> > >> - * static function prototypes
>> > >> - */
>> > >> -Static void  pipex_iface_start (struct 
>> > >> pipex_iface_context *);
>> > >> -Static void  pipex_iface_stop (struct 
>> > >> pipex_iface_context *);
>> > >> -Static int   pipex_add_session (struct 
>> > >> pipex_session_req *, struct pipex_iface_context *);
>> > >> -Static int   pipex_close_session (struct 
>> > >> pipex_session_close_req *);
>> > >> -Static int   pipex_config_session (struct 
>> > >> pipex_session_config_req *);
>> > >> -Static int   pipex_get_stat (struct 
>> > >> pipex_session_stat_req *);
>> > >> -Static int   pipex_get_closed (struct 
>> > >> pipex_session_list_req *);
>> > >> -Static int   pipex_destroy_session (struct 
>> > >> pipex_session *);
>> > >> -Static struct pipex_session  *pipex_lookup_by_ip_address (struct 
>> > >> in_addr);
>> > >> -Static struct pipex_session  *pipex_lookup_by_session_id (int, int);
>> > >> -Static void  pipex_ip_output (struct mbuf *, struct 
>> > >> pipex_session *);
>> > >> -Static void  pipex_ppp_output (struct mbuf *, struct 
>> > >> pipex_session *, int);
>> > >> -Static int   pipex_ppp_proto (struct mbuf *, struct 
>> > >> pipex_session *, int, int *);
>> > >> -Static void  pipex_ppp_input (struct mbuf *, struct 
>> > >> pipex_session *, int);
>> > >> -Static void  pipex_ip_input (struct mbuf *, struct 
>> > >> pipex_session *);
>> > >> +void  pipex_iface_start (struct pipex_iface_context *);
>> > >> +void  pipex_iface_stop (struct pipex_iface_context *);
>> > >> +int   pipex_add_session (struct pipex_session_req *, 
>> > >> struct pipex_iface_context *);
>> > >> +int   pipex_close_session (struct 
>> > >> pipex_session_close_req *);
>> > >> +int   pipex_config_session (struct 
>> > >> pipex_session_config_req *);
>> > >> +int   pipex_get_stat (struct pipex_session_stat_req *);
>> > >> +int   pipex_get_closed (struct pipex_session_list_req 
>> > >> *);
>> > >> +int   pipex_destroy_session (struct pipex_session *);
>> > >> +struct pipex_session  *pipex_lookup_by_ip_address (struct in_addr);
>> > >> +struct pipex_session  *pipex_lookup_by_session_id (int, int);
>> > >> +void  pipex_ip_output (struct mbuf *, struct 
>> > >> pipex_session *);
>> > >> +void  pipex_ppp_output (struct mbuf *, struct 
>> > >> pipex_session *, int);
>> > >> +int   pipex_ppp_proto (struct mbuf *, struct 
>> > >> pipex_session *, int, int *);
>> > >> +void  pipex_ppp_input (struct mbuf *, struct 
>> > >> pipex_session *, int);
>> > >> +void  pipex_ip_input (struct mbuf *, struct 
>> > >> pipex_session *);
>> > >>  #ifdef INET6
>> > >> -Static void  pipex_ip6_input (struct mbuf *, struct 
>> > >> pipex_session *);
>> > >> +void  pipex_ip6_input (struct mbuf *, struct 
>> > >> pipex_session *);
>> > >>  #endif
>> > >> -Static struct mbuf   *pipex_common_input(struct pipex_session 
>> > >> *, 

minor bgpd cleanup

2020-02-14 Thread Claudio Jeker
Move and rename copy_filterset to rde_filter.c as filterset_copy.
This way it matches the other filterset_* functions.

OK?
-- 
:wq Claudio

Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.400
diff -u -p -r1.400 bgpd.h
--- bgpd.h  12 Feb 2020 10:33:56 -  1.400
+++ bgpd.h  14 Feb 2020 12:16:43 -
@@ -1182,8 +1182,6 @@ void  free_prefixtree(struct prefixset_t
 void   filterlist_free(struct filter_head *);
 inthost(const char *, struct bgpd_addr *, u_int8_t *);
 u_int32_t  get_bgpid(void);
-void   copy_filterset(struct filter_set_head *,
-   struct filter_set_head *);
 void   expand_networks(struct bgpd_config *);
 intprefixset_cmp(struct prefixset_item *, struct prefixset_item *);
 RB_PROTOTYPE(prefixset_tree, prefixset_item, entry, prefixset_cmp);
@@ -1261,10 +1259,10 @@ int pftable_addr_remove(struct pftable_m
 intpftable_commit(void);
 
 /* rde_filter.c */
-voidfilterset_free(struct filter_set_head *);
-int filterset_cmp(struct filter_set *, struct filter_set *);
-voidfilterset_move(struct filter_set_head *,
-   struct filter_set_head *);
+void   filterset_free(struct filter_set_head *);
+intfilterset_cmp(struct filter_set *, struct filter_set *);
+void   filterset_move(struct filter_set_head *, struct filter_set_head *);
+void   filterset_copy(struct filter_set_head *, struct filter_set_head *);
 const char *filterset_name(enum action_types);
 
 /* rde_sets.c */
Index: config.c
===
RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
retrieving revision 1.94
diff -u -p -r1.94 config.c
--- config.c28 Jan 2020 15:45:46 -  1.94
+++ config.c14 Feb 2020 12:21:44 -
@@ -496,22 +496,6 @@ prepare_listeners(struct bgpd_config *co
 }
 
 void
-copy_filterset(struct filter_set_head *source, struct filter_set_head *dest)
-{
-   struct filter_set   *s, *t;
-
-   if (source == NULL)
-   return;
-
-   TAILQ_FOREACH(s, source, entry) {
-   if ((t = malloc(sizeof(struct filter_set))) == NULL)
-   fatal(NULL);
-   memcpy(t, s, sizeof(struct filter_set));
-   TAILQ_INSERT_TAIL(dest, t, entry);
-   }
-}
-
-void
 expand_networks(struct bgpd_config *c)
 {
struct network  *n, *m, *tmp;
@@ -533,8 +517,7 @@ expand_networks(struct bgpd_config *c)
memcpy(>net.prefix, >p.addr,
sizeof(m->net.prefix));
m->net.prefixlen = psi->p.len;
-   TAILQ_INIT(>net.attrset);
-   copy_filterset(>net.attrset,
+   filterset_copy(>net.attrset,
>net.attrset);
TAILQ_INSERT_TAIL(nw, m, entry);
}
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.403
diff -u -p -r1.403 parse.y
--- parse.y 24 Jan 2020 05:44:05 -  1.403
+++ parse.y 14 Feb 2020 12:21:59 -
@@ -4076,8 +4076,7 @@ expand_rule(struct filter_rule *rule, st
memcpy(r, rule, sizeof(struct 
filter_rule));
memcpy(>match, match,
sizeof(struct filter_match));
-   TAILQ_INIT(>set);
-   copy_filterset(set, >set);
+   filterset_copy(set, >set);
 
if (rb != NULL)
strlcpy(r->rib, rb->name,
Index: rde_filter.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
retrieving revision 1.122
diff -u -p -r1.122 rde_filter.c
--- rde_filter.c13 Aug 2019 12:16:20 -  1.122
+++ rde_filter.c14 Feb 2020 12:21:28 -
@@ -502,6 +502,10 @@ filterset_cmp(struct filter_set *a, stru
return (0);
 }
 
+/*
+ * move filterset from source to dest. dest will be initialized first.
+ * After the move source is an empty list.
+ */
 void
 filterset_move(struct filter_set_head *source, struct filter_set_head *dest)
 {
@@ -509,6 +513,26 @@ filterset_move(struct filter_set_head *s
if (source == NULL)
return;
TAILQ_CONCAT(dest, source, entry);
+}
+
+/*
+ * copy filterset from source to dest. dest will be initialized first.
+ */
+void
+filterset_copy(struct filter_set_head *source, struct filter_set_head *dest)
+{
+   struct filter_set   *s, *t;
+
+   TAILQ_INIT(dest);