Re: [patch] Fix closing socket twice bug in netcat program

2018-09-05 Thread Alexander Bluhm
On Tue, Sep 04, 2018 at 01:01:38PM +0800, Nan Xiao wrote:
> Before netcat program exits, it will check whether s is -1, and close
> socket if s is not -1:
> 
>   if (s != -1)
>   close(s);
> 
> The following patch fixes the issue that netcat will close socket twice
> if it works as a server:

I think it is a bug, but should be fixed differently.  The netstat
code has a lot of if and else for all the use cases.  Adding a s =
-1 at one place makes it even more confusing.

In general main() does not reset s to -1, it isets it at the
beginning.  Look at the client side.  There we close at the beginning
of the loop:

/* Cycle through portlist, connecting to each port. */
for (s = -1, i = 0; portlist[i] != NULL; i++) {
if (s != -1)
close(s);

So on the server side we should do the same, otherwise the code
will get more and more messy.  Use the same concept everywhere.
I think this would be better:

/* Allow only one connection at a time, but stay alive. */
for (;;) {
if (family != AF_UNIX) {
if (s != -1)
close(s);
s = local_listen(host, uport, hints);
}

bluhm



Re: Linux DRM

2018-09-05 Thread Chris Cappuccio
Joseph Mayer [joseph.ma...@protonmail.com] wrote:
> 
> For the one who has not reviewed the code, can you quantify and
> illustrate approximately how bad it is?
> 

Perhaps he was reading from someone who does know some detail of the code?

https://arcan-fe.com/2018/04/25/towards-secure-system-graphics-arcan-and-openbsd/



Mention errno in printf(3) man

2018-09-05 Thread geoff
Right now the printf(3) family of manpages make no reference to errno.
Glancing at __vfprintf, it appears that most of the erroneous paths do
set errno.

Should the manual be amended to reflect this behavior, so that users
have a documented way to determine the cause of failure in such cases?

Note this is a smaller concession than a full ERRORS section to
enumerate error codes, which would pin down the error API, making it
hard to change without user breakage. errno is still useful without
this section thanks to libc functions like strerror(3) and err(3).

The intended effect on userspace developers is to encourage picking
warn(3) over warnx(3) when logging, and other similar choices.


Index: lib/libc/stdio/printf.3
===
RCS file: /cvs/src/lib/libc/stdio/printf.3,v
retrieving revision 1.74
diff -u -p -u -r1.74 printf.3
--- lib/libc/stdio/printf.3 13 Oct 2015 12:25:04 -  1.74
+++ lib/libc/stdio/printf.3 5 Sep 2018 18:28:33 -
@@ -642,7 +642,9 @@ a field; if the result of a conversion i
 field is expanded to contain the conversion result.
 .Sh RETURN VALUES
 For all these functions if an output or encoding error occurs, a value
-less than 0 is returned.
+less than 0 is returned and the global variable
+.Va errno
+is set to indicate the error.
 .Pp
 The
 .Fn printf ,



Re: md5: convert from fgetln(3) to getline(3)

2018-09-05 Thread Todd C. Miller
On Thu, 23 Aug 2018 11:24:54 -0500, Scott Cheloha wrote:

> On Tue, Aug 14, 2018 at 03:11:47PM -0500, Scott Cheloha wrote:
> > This patch is ok cheloha@ and I can commit if someone else
> > is ok, too.
>
> 1 week bump.  Anyone else ok?

OK millert@

 - todd



Re: llvm: patch-update to release 5.0.1

2018-09-05 Thread Leonid Bobrov
I always wonder: where the fuck is lldb? I don't see it in base or ports



Re: md5: convert from fgetln(3) to getline(3)

2018-09-05 Thread Lauri Tirkkonen
On Thu, Aug 23 2018 11:24:54 -0500, Scott Cheloha wrote:
> On Tue, Aug 14, 2018 at 03:11:47PM -0500, Scott Cheloha wrote:
> > This patch is ok cheloha@ and I can commit if someone else
> > is ok, too.
> 
> 1 week bump.  Anyone else ok?

no takers?

-- 
Lauri Tirkkonen | lotheac @ IRCnet



Re: fix "_nfiles" references for crash dump

2018-09-05 Thread Martin Pieuchot
On 04/09/18(Tue) 14:59, Naoki Fukaumi wrote:
> hi tech@,
> 
> "_nfiles" was renamed to "_numfiles" by this commit,
>  https://marc.info/?l=openbsd-cvs=147199491615345=2
> 
> then fstat/pstat put following errors,
> 
>  # fstat -M bsd.0.core -N bsd.0
>  fstat: _nfiles: no such symbol
> 
>  # pstat -f -M bsd.0.core -N bsd.0
>  pstat: kvm_getfiles: _nfiles: no such symbol
> 
>  # pstat -T -M bsd.0.core -N bsd.0
>  pstat: cannot read nfile: invalid translation (invalid PTE)
> 
> this patch fixes the problem.

ok mpi@

> Index: lib/libkvm/kvm_file2.c
> ===
> RCS file: /cvs/src/lib/libkvm/kvm_file2.c,v
> retrieving revision 1.53
> diff -u -p -u -p -r1.53 kvm_file2.c
> --- lib/libkvm/kvm_file2.c2 Jan 2018 06:38:45 -   1.53
> +++ lib/libkvm/kvm_file2.c4 Sep 2018 04:19:00 -
> @@ -209,7 +209,7 @@ kvm_deadfile_byfile(kvm_t *kd, int op, i
>   int nfiles;
>  
>   nl[0].n_name = "_filehead";
> - nl[1].n_name = "_nfiles";
> + nl[1].n_name = "_numfiles";
>   nl[2].n_name = 0;
>  
>   if (kvm_nlist(kd, nl) != 0) {
> @@ -280,7 +280,7 @@ kvm_deadfile_byid(kvm_t *kd, int op, int
>   int i, nfiles;
>  
>   nl[0].n_name = "_filehead";
> - nl[1].n_name = "_nfiles";
> + nl[1].n_name = "_numfiles";
>   nl[2].n_name = "_allprocess";
>   nl[3].n_name = 0;
>  
> Index: usr.sbin/pstat/pstat.c
> ===
> RCS file: /cvs/src/usr.sbin/pstat/pstat.c,v
> retrieving revision 1.118
> diff -u -p -u -p -r1.118 pstat.c
> --- usr.sbin/pstat/pstat.c3 Aug 2018 14:39:55 -   1.118
> +++ usr.sbin/pstat/pstat.c4 Sep 2018 04:19:00 -
> @@ -69,7 +69,7 @@
>  
>  struct nlist vnodenl[] = {
>  #define  FNL_NFILE   0   /* sysctl */
> - {"_nfiles"},
> + {"_numfiles"},
>  #define FNL_MAXFILE  1   /* sysctl */
>   {"_maxfiles"},
>  #define TTY_NTTY 2   /* sysctl */
> 



Re: smtpd: flags cleanup in mta

2018-09-05 Thread Gilles Chehade
On Wed, Sep 05, 2018 at 03:04:08PM +0200, Eric Faurot wrote:
> With the recent changes in the smarthost syntax, and the removal of
> the "secure" keyword, it's now possible to clarify the mta code by
> changing the TLS option from a set flags to exclusive values.
> This is far less confusing.
> 
> More cleanup to come in mta_session.c after that.
> 

nice !


> Index: mta.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v
> retrieving revision 1.222
> diff -u -p -r1.222 mta.c
> --- mta.c 22 Aug 2018 10:11:43 -  1.222
> +++ mta.c 5 Sep 2018 12:42:19 -
> @@ -635,6 +635,7 @@ mta_handle_envelope(struct envelope *evp
>   }
>  
>   memset(, 0, sizeof(relayh));
> + relayh.tls = RELAY_TLS_OPPORTUNISTIC;
>   if (smarthost && !text_to_relayhost(, smarthost)) {
>   log_warnx("warn: Failed to parse smarthost %s", smarthost);
>   m_create(p_queue, IMSG_MTA_DELIVERY_TEMPFAIL, 0, 0, -1);
> @@ -1730,10 +1731,9 @@ mta_relay(struct envelope *e, struct rel
>   key.flags |= RELAY_MX;
>   } else {
>   key.domain = mta_domain(e->dest.domain, 0);
> - if (!(relayh->flags & RELAY_STARTTLS))
> - key.flags |= RELAY_TLS_OPTIONAL;
>   }
>  
> + key.tls = relayh->tls;
>   key.flags |= relayh->flags;
>   key.port = relayh->port;
>   key.authlabel = relayh->authlabel;
> @@ -1748,6 +1748,7 @@ mta_relay(struct envelope *e, struct rel
>   r = xcalloc(1, sizeof *r);
>   TAILQ_INIT(>tasks);
>   r->id = generate_uid();
> + r->tls = key.tls;
>   r->flags = key.flags;
>   r->domain = key.domain;
>   r->backupname = key.backupname ?
> @@ -1834,14 +1835,25 @@ mta_relay_to_text(struct mta_relay *rela
>   (void)strlcat(buf, tmp, sizeof buf);
>   }
>  
> - if (relay->flags & RELAY_STARTTLS) {
> - (void)strlcat(buf, sep, sizeof buf);
> - (void)strlcat(buf, "starttls", sizeof buf);
> - }
> -
> - if (relay->flags & RELAY_SMTPS) {
> - (void)strlcat(buf, sep, sizeof buf);
> + (void)strlcat(buf, sep, sizeof buf);
> + switch(relay->tls) {
> + case RELAY_TLS_OPPORTUNISTIC:
> + (void)strlcat(buf, "smtp", sizeof buf);
> + break;
> + case RELAY_TLS_STARTTLS:
> + (void)strlcat(buf, "smtp+tls", sizeof buf);
> + break;
> + case RELAY_TLS_SMTPS:
>   (void)strlcat(buf, "smtps", sizeof buf);
> + break;
> + case RELAY_TLS_NO:
> + if (relay->flags & RELAY_LMTP)
> + (void)strlcat(buf, "lmtp", sizeof buf);
> + else
> + (void)strlcat(buf, "smtp+notls", sizeof buf);
> + break;
> + default:
> + (void)strlcat(buf, "???", sizeof buf);
>   }
>  
>   if (relay->flags & RELAY_AUTH) {
> @@ -1993,6 +2005,11 @@ mta_relay_cmp(const struct mta_relay *a,
>   if (a->domain < b->domain)
>   return (-1);
>   if (a->domain > b->domain)
> + return (1);
> +
> + if (a->tls < b->tls)
> + return (-1);
> + if (a->tls > b->tls)
>   return (1);
>  
>   if (a->flags < b->flags)
> Index: mta_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/mta_session.c,v
> retrieving revision 1.109
> diff -u -p -r1.109 mta_session.c
> --- mta_session.c 5 Sep 2018 10:15:41 -   1.109
> +++ mta_session.c 5 Sep 2018 12:42:19 -
> @@ -199,24 +199,23 @@ mta_session(struct mta_relay *relay, str
>  
>   if (relay->flags & RELAY_LMTP)
>   s->flags |= MTA_LMTP;
> - switch (relay->flags & (RELAY_SSL|RELAY_TLS_OPTIONAL)) {
> - case RELAY_SSL:
> - s->flags |= MTA_FORCE_ANYSSL;
> - s->flags |= MTA_WANT_SECURE;
> - break;
> - case RELAY_SMTPS:
> + switch (relay->tls) {
> + case RELAY_TLS_SMTPS:
>   s->flags |= MTA_FORCE_SMTPS;
>   s->flags |= MTA_WANT_SECURE;
>   break;
> - case RELAY_STARTTLS:
> + case RELAY_TLS_STARTTLS:
>   s->flags |= MTA_FORCE_TLS;
>   s->flags |= MTA_WANT_SECURE;
>   break;
> - case RELAY_TLS_OPTIONAL:
> + case RELAY_TLS_OPPORTUNISTIC:
>   /* do not force anything, try tls then smtp */
>   break;
> - default:
> + case RELAY_TLS_NO:
>   s->flags |= MTA_FORCE_PLAIN;
> + break;
> + default:
> + fatalx("bad value for relay->tls: %d", relay->tls);
>   }
>  
>   if (relay->flags & RELAY_BACKUP)
> 

Re: [patch] Fix closing socket twice bug in netcat program

2018-09-05 Thread Nan Xiao
Ping tech@,

Could anyone spare a minute to check this patch? I think it is indeed a bug.

On 9/4/2018 1:01 PM, Nan Xiao wrote:
> Hi tech@,
> 
> Before netcat program exits, it will check whether s is -1, and close
> socket if s is not -1:
> 
>   if (s != -1)
>   close(s);
> 
> The following patch fixes the issue that netcat will close socket twice
> if it works as a server:
> 
> Index: netcat.c
> ===
> RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> retrieving revision 1.192
> diff -u -p -r1.192 netcat.c
> --- netcat.c  10 Aug 2018 17:15:22 -  1.192
> +++ netcat.c  4 Sep 2018 04:51:55 -
> @@ -622,9 +622,10 @@ main(int argc, char *argv[])
>   }
>   close(connfd);
>   }
> - if (family != AF_UNIX)
> + if (family != AF_UNIX) {
>   close(s);
> - else if (uflag) {
> + s = -1;
> + } else if (uflag) {
>   if (connect(s, NULL, 0) < 0)
>   err(1, "connect");
>   }
> 
> Thanks!
> 

-- 
Best Regards
Nan Xiao(肖楠)



smtpd: flags cleanup in mta

2018-09-05 Thread Eric Faurot
With the recent changes in the smarthost syntax, and the removal of
the "secure" keyword, it's now possible to clarify the mta code by
changing the TLS option from a set flags to exclusive values.
This is far less confusing.

More cleanup to come in mta_session.c after that.

Eric.

Index: mta.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v
retrieving revision 1.222
diff -u -p -r1.222 mta.c
--- mta.c   22 Aug 2018 10:11:43 -  1.222
+++ mta.c   5 Sep 2018 12:42:19 -
@@ -635,6 +635,7 @@ mta_handle_envelope(struct envelope *evp
}
 
memset(, 0, sizeof(relayh));
+   relayh.tls = RELAY_TLS_OPPORTUNISTIC;
if (smarthost && !text_to_relayhost(, smarthost)) {
log_warnx("warn: Failed to parse smarthost %s", smarthost);
m_create(p_queue, IMSG_MTA_DELIVERY_TEMPFAIL, 0, 0, -1);
@@ -1730,10 +1731,9 @@ mta_relay(struct envelope *e, struct rel
key.flags |= RELAY_MX;
} else {
key.domain = mta_domain(e->dest.domain, 0);
-   if (!(relayh->flags & RELAY_STARTTLS))
-   key.flags |= RELAY_TLS_OPTIONAL;
}
 
+   key.tls = relayh->tls;
key.flags |= relayh->flags;
key.port = relayh->port;
key.authlabel = relayh->authlabel;
@@ -1748,6 +1748,7 @@ mta_relay(struct envelope *e, struct rel
r = xcalloc(1, sizeof *r);
TAILQ_INIT(>tasks);
r->id = generate_uid();
+   r->tls = key.tls;
r->flags = key.flags;
r->domain = key.domain;
r->backupname = key.backupname ?
@@ -1834,14 +1835,25 @@ mta_relay_to_text(struct mta_relay *rela
(void)strlcat(buf, tmp, sizeof buf);
}
 
-   if (relay->flags & RELAY_STARTTLS) {
-   (void)strlcat(buf, sep, sizeof buf);
-   (void)strlcat(buf, "starttls", sizeof buf);
-   }
-
-   if (relay->flags & RELAY_SMTPS) {
-   (void)strlcat(buf, sep, sizeof buf);
+   (void)strlcat(buf, sep, sizeof buf);
+   switch(relay->tls) {
+   case RELAY_TLS_OPPORTUNISTIC:
+   (void)strlcat(buf, "smtp", sizeof buf);
+   break;
+   case RELAY_TLS_STARTTLS:
+   (void)strlcat(buf, "smtp+tls", sizeof buf);
+   break;
+   case RELAY_TLS_SMTPS:
(void)strlcat(buf, "smtps", sizeof buf);
+   break;
+   case RELAY_TLS_NO:
+   if (relay->flags & RELAY_LMTP)
+   (void)strlcat(buf, "lmtp", sizeof buf);
+   else
+   (void)strlcat(buf, "smtp+notls", sizeof buf);
+   break;
+   default:
+   (void)strlcat(buf, "???", sizeof buf);
}
 
if (relay->flags & RELAY_AUTH) {
@@ -1993,6 +2005,11 @@ mta_relay_cmp(const struct mta_relay *a,
if (a->domain < b->domain)
return (-1);
if (a->domain > b->domain)
+   return (1);
+
+   if (a->tls < b->tls)
+   return (-1);
+   if (a->tls > b->tls)
return (1);
 
if (a->flags < b->flags)
Index: mta_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mta_session.c,v
retrieving revision 1.109
diff -u -p -r1.109 mta_session.c
--- mta_session.c   5 Sep 2018 10:15:41 -   1.109
+++ mta_session.c   5 Sep 2018 12:42:19 -
@@ -199,24 +199,23 @@ mta_session(struct mta_relay *relay, str
 
if (relay->flags & RELAY_LMTP)
s->flags |= MTA_LMTP;
-   switch (relay->flags & (RELAY_SSL|RELAY_TLS_OPTIONAL)) {
-   case RELAY_SSL:
-   s->flags |= MTA_FORCE_ANYSSL;
-   s->flags |= MTA_WANT_SECURE;
-   break;
-   case RELAY_SMTPS:
+   switch (relay->tls) {
+   case RELAY_TLS_SMTPS:
s->flags |= MTA_FORCE_SMTPS;
s->flags |= MTA_WANT_SECURE;
break;
-   case RELAY_STARTTLS:
+   case RELAY_TLS_STARTTLS:
s->flags |= MTA_FORCE_TLS;
s->flags |= MTA_WANT_SECURE;
break;
-   case RELAY_TLS_OPTIONAL:
+   case RELAY_TLS_OPPORTUNISTIC:
/* do not force anything, try tls then smtp */
break;
-   default:
+   case RELAY_TLS_NO:
s->flags |= MTA_FORCE_PLAIN;
+   break;
+   default:
+   fatalx("bad value for relay->tls: %d", relay->tls);
}
 
if (relay->flags & RELAY_BACKUP)
Index: smtpd.h
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v

Re: icmp6_mtudisc_clone() tweak

2018-09-05 Thread Alexander Bluhm
On Wed, Sep 05, 2018 at 12:04:07AM +0800, Michael Mikonos wrote:
> The code executed on failure in icmp6_mtudisc_clone() is always
> the same so it can be declared in one place. Is anyone happy to
> OK this?

Yeah, why not.

But please keep icmp_mtudisc_clone() in sync.

bluhm



Re: Update to table(5) man page

2018-09-05 Thread Matt Schwartz
I will make those changes you want as soon as I can get to my computer.

On Wed, Sep 5, 2018, 6:23 AM Matt Schwartz 
wrote:

> Thanks guys. I really like working on documentation. Please let me know if
> I can help on anything in the future.
>
> On Wed, Sep 5, 2018, 3:22 AM Jason McIntyre  wrote:
>
>> On Tue, Sep 04, 2018 at 08:54:37AM -0400, Matt Schwartz wrote:
>> > Below is a diff to clear up the description of the Userinfo table in
>> > table(5). I also added an example of how it can be used with an Alias
>> > table.
>> >
>> > Thanks,
>> > Matt
>> >
>>
>> fixed now. thanks,
>> jmc
>>
>> > Index: table.5
>> > ===
>> > RCS file: /cvs/src/usr.sbin/smtpd/table.5,v
>> > retrieving revision 1.9
>> > diff -u -p -u -r1.9 table.5
>> > --- table.524 May 2018 20:40:21 -1.9
>> > +++ table.54 Sep 2018 12:52:47 -
>> > @@ -174,16 +174,12 @@ ipv6:::1
>> >  192.168.1.0/24
>> >  .Ed
>> >  .Ss Userinfo tables
>> > -User info tables are used to described virtual system users.
>> > -They are used in rule context to specify an alternate user base,
>> mapping
>> > -virtual users to local system UID, GID and home directory.
>> > +User info tables are used in rule context to specify an alternate user
>> base,
>> > +mapping virtual users to local system users by UID, GID and home
>> directory.
>> >  .Pp
>> >  .D1 Ic action Ar name method Cm userbase Pf < Ar table Ns >
>> >  .Pp
>> > -The userinfo table is a mapping from virtual user names to a set of
>> system user
>> > -ID, group ID and path to home directory.
>> > -.Pp
>> > -A userinfo table looks as follows:
>> > +An userinfo table looks as follows:
>> >  .Bd -literal -offset indent
>> >  joe1000:100:/home/virtual/joe
>> >  jack1000:100:/home/virtual/jack
>> > @@ -193,7 +189,15 @@ In this example, both joe and jack are v
>> >  system user with UID 1000 and GID 100, but different home directories.
>> >  These directories may contain a
>> >  .Xr forward 5
>> > -file.
>> > +file. This can be used in conjunction with an
>> > +.Cm Alias table
>> > +that maps an email address or the domain part to the desired virtual
>> > +username. For example:
>> > +.Bd -literal -offset indent
>> > +j...@example.orgjoe
>> > +j...@example.comjack
>> > +.Ed
>> > +.Pp
>> >  .Ss Source tables
>> >  Source tables are lists of IPv4 and IPv6 addresses.
>> >  They can only be used in the following context:
>> >
>>
>>


Re: Update to table(5) man page

2018-09-05 Thread Matt Schwartz
Thanks guys. I really like working on documentation. Please let me know if
I can help on anything in the future.

On Wed, Sep 5, 2018, 3:22 AM Jason McIntyre  wrote:

> On Tue, Sep 04, 2018 at 08:54:37AM -0400, Matt Schwartz wrote:
> > Below is a diff to clear up the description of the Userinfo table in
> > table(5). I also added an example of how it can be used with an Alias
> > table.
> >
> > Thanks,
> > Matt
> >
>
> fixed now. thanks,
> jmc
>
> > Index: table.5
> > ===
> > RCS file: /cvs/src/usr.sbin/smtpd/table.5,v
> > retrieving revision 1.9
> > diff -u -p -u -r1.9 table.5
> > --- table.524 May 2018 20:40:21 -1.9
> > +++ table.54 Sep 2018 12:52:47 -
> > @@ -174,16 +174,12 @@ ipv6:::1
> >  192.168.1.0/24
> >  .Ed
> >  .Ss Userinfo tables
> > -User info tables are used to described virtual system users.
> > -They are used in rule context to specify an alternate user base, mapping
> > -virtual users to local system UID, GID and home directory.
> > +User info tables are used in rule context to specify an alternate user
> base,
> > +mapping virtual users to local system users by UID, GID and home
> directory.
> >  .Pp
> >  .D1 Ic action Ar name method Cm userbase Pf < Ar table Ns >
> >  .Pp
> > -The userinfo table is a mapping from virtual user names to a set of
> system user
> > -ID, group ID and path to home directory.
> > -.Pp
> > -A userinfo table looks as follows:
> > +An userinfo table looks as follows:
> >  .Bd -literal -offset indent
> >  joe1000:100:/home/virtual/joe
> >  jack1000:100:/home/virtual/jack
> > @@ -193,7 +189,15 @@ In this example, both joe and jack are v
> >  system user with UID 1000 and GID 100, but different home directories.
> >  These directories may contain a
> >  .Xr forward 5
> > -file.
> > +file. This can be used in conjunction with an
> > +.Cm Alias table
> > +that maps an email address or the domain part to the desired virtual
> > +username. For example:
> > +.Bd -literal -offset indent
> > +j...@example.orgjoe
> > +j...@example.comjack
> > +.Ed
> > +.Pp
> >  .Ss Source tables
> >  Source tables are lists of IPv4 and IPv6 addresses.
> >  They can only be used in the following context:
> >
>
>


Re: iostat: add "sp" column for CP_SPIN

2018-09-05 Thread YASUOKA Masahiko
Hi,

Committed the diff for command.  Thanks.

As for the man page, it makes sense for me.

ok yasuoka

On Wed, 05 Sep 2018 10:33:37 +0200
Solene Rapenne  wrote:
> Solene Rapenne  wrote:
>> Naoki Fukaumi  wrote:
>> > hi tech@,
>> > 
>> > new cpu state, CP_SPIN, was added,
>> >  https://marc.info/?l=openbsd-cvs=152630109526317=2
>> > 
>> > but there is no column for it in the header of iostat,
>> > 
>> > $ iostat
>> >   tty  sd0 cpu
>> >  tin tout  KB/t  t/s  MB/s  us ni sy in id
>> >01 11.473  0.04   0  0  0  0  0100
>> > 
>> > this patch adds "sp" for CP_SPIN.
>> > 
>> > --
>> > FUKAUMI Naoki
>> > 
>> > Index: usr.sbin/iostat/iostat.c
>> > ===
>> > RCS file: /cvs/src/usr.sbin/iostat/iostat.c,v
>> > retrieving revision 1.40
>> > diff -u -p -u -p -r1.40 iostat.c
>> > --- usr.sbin/iostat/iostat.c   10 Feb 2018 19:49:50 -  1.40
>> > +++ usr.sbin/iostat/iostat.c   4 Sep 2018 04:19:14 -
>> > @@ -229,7 +229,7 @@ header(void)
>> >printf(" %16.16s ", cur.dk_name[i]);
>> >  
>> >if (ISSET(todo, SHOW_CPU))
>> > -  printf("cpu");
>> > +  printf("   cpu");
>> >printf("\n");
>> >  
>> >/* Sub-Headers. */
>> > @@ -254,7 +254,7 @@ header(void)
>> >printf(" KB  xfr time ");
>> >  
>> >if (ISSET(todo, SHOW_CPU))
>> > -  printf(" us ni sy in id");
>> > +  printf(" us ni sy sp in id");
>> >printf("\n");
>> >  }
>> >  
>> 
>> diff for iostat.8
>> 
>> not sure for the text though
>> is spinning time, time spent in locks?
>> 
>> The text could be "CPU time waiting for locks" maybe
>> 
>> Index: iostat.8
>> ===
>> RCS file: /cvs/src/usr.sbin/iostat/iostat.8,v
>> retrieving revision 1.26
>> diff -u -p -r1.26 iostat.8
>> --- iostat.817 Jan 2013 21:39:29 -  1.26
>> +++ iostat.85 Sep 2018 08:29:36 -
>> @@ -170,6 +170,8 @@ Seconds spent in disk activity
>>  % of CPU time in user mode running niced processes
>>  .It \
>>  % of CPU time in system mode
>> +.It \
>> +% of CPU time spent spinning
>>  .It \
>>  % of CPU time processing interrupts
>>  .It \
> 
> the right diff is this one
> 
> Index: iostat.8
> ===
> RCS file: /cvs/src/usr.sbin/iostat/iostat.8,v
> retrieving revision 1.26
> diff -u -p -r1.26 iostat.8
> --- iostat.817 Jan 2013 21:39:29 -  1.26
> +++ iostat.85 Sep 2018 08:29:36 -
> @@ -170,6 +170,8 @@ Seconds spent in disk activity
>  % of CPU time in user mode running niced processes
>  .It \
>  % of CPU time in system mode
> +.It \
> +% of CPU time spent spinning
>  .It \
>  % of CPU time processing interrupts
>  .It \
> 



Re: iostat: add "sp" column for CP_SPIN

2018-09-05 Thread Solene Rapenne
Solene Rapenne  wrote:
> Naoki Fukaumi  wrote:
> > hi tech@,
> > 
> > new cpu state, CP_SPIN, was added,
> >  https://marc.info/?l=openbsd-cvs=152630109526317=2
> > 
> > but there is no column for it in the header of iostat,
> > 
> > $ iostat
> >   tty  sd0 cpu
> >  tin tout  KB/t  t/s  MB/s  us ni sy in id
> >01 11.473  0.04   0  0  0  0  0100
> > 
> > this patch adds "sp" for CP_SPIN.
> > 
> > --
> > FUKAUMI Naoki
> > 
> > Index: usr.sbin/iostat/iostat.c
> > ===
> > RCS file: /cvs/src/usr.sbin/iostat/iostat.c,v
> > retrieving revision 1.40
> > diff -u -p -u -p -r1.40 iostat.c
> > --- usr.sbin/iostat/iostat.c10 Feb 2018 19:49:50 -  1.40
> > +++ usr.sbin/iostat/iostat.c4 Sep 2018 04:19:14 -
> > @@ -229,7 +229,7 @@ header(void)
> > printf(" %16.16s ", cur.dk_name[i]);
> >  
> > if (ISSET(todo, SHOW_CPU))
> > -   printf("cpu");
> > +   printf("   cpu");
> > printf("\n");
> >  
> > /* Sub-Headers. */
> > @@ -254,7 +254,7 @@ header(void)
> > printf(" KB  xfr time ");
> >  
> > if (ISSET(todo, SHOW_CPU))
> > -   printf(" us ni sy in id");
> > +   printf(" us ni sy sp in id");
> > printf("\n");
> >  }
> >  
> 
> diff for iostat.8
> 
> not sure for the text though
> is spinning time, time spent in locks?
> 
> The text could be "CPU time waiting for locks" maybe
> 
> Index: iostat.8
> ===
> RCS file: /cvs/src/usr.sbin/iostat/iostat.8,v
> retrieving revision 1.26
> diff -u -p -r1.26 iostat.8
> --- iostat.817 Jan 2013 21:39:29 -  1.26
> +++ iostat.85 Sep 2018 08:29:36 -
> @@ -170,6 +170,8 @@ Seconds spent in disk activity
>  % of CPU time in user mode running niced processes
>  .It \
>  % of CPU time in system mode
> +.It \
> +% of CPU time spent spinning
>  .It \
>  % of CPU time processing interrupts
>  .It \

the right diff is this one

Index: iostat.8
===
RCS file: /cvs/src/usr.sbin/iostat/iostat.8,v
retrieving revision 1.26
diff -u -p -r1.26 iostat.8
--- iostat.817 Jan 2013 21:39:29 -  1.26
+++ iostat.85 Sep 2018 08:29:36 -
@@ -170,6 +170,8 @@ Seconds spent in disk activity
 % of CPU time in user mode running niced processes
 .It \
 % of CPU time in system mode
+.It \
+% of CPU time spent spinning
 .It \
 % of CPU time processing interrupts
 .It \



Re: iostat: add "sp" column for CP_SPIN

2018-09-05 Thread Solene Rapenne
Naoki Fukaumi  wrote:
> hi tech@,
> 
> new cpu state, CP_SPIN, was added,
>  https://marc.info/?l=openbsd-cvs=152630109526317=2
> 
> but there is no column for it in the header of iostat,
> 
> $ iostat
>   tty  sd0 cpu
>  tin tout  KB/t  t/s  MB/s  us ni sy in id
>01 11.473  0.04   0  0  0  0  0100
> 
> this patch adds "sp" for CP_SPIN.
> 
> --
> FUKAUMI Naoki
> 
> Index: usr.sbin/iostat/iostat.c
> ===
> RCS file: /cvs/src/usr.sbin/iostat/iostat.c,v
> retrieving revision 1.40
> diff -u -p -u -p -r1.40 iostat.c
> --- usr.sbin/iostat/iostat.c  10 Feb 2018 19:49:50 -  1.40
> +++ usr.sbin/iostat/iostat.c  4 Sep 2018 04:19:14 -
> @@ -229,7 +229,7 @@ header(void)
>   printf(" %16.16s ", cur.dk_name[i]);
>  
>   if (ISSET(todo, SHOW_CPU))
> - printf("cpu");
> + printf("   cpu");
>   printf("\n");
>  
>   /* Sub-Headers. */
> @@ -254,7 +254,7 @@ header(void)
>   printf(" KB  xfr time ");
>  
>   if (ISSET(todo, SHOW_CPU))
> - printf(" us ni sy in id");
> + printf(" us ni sy sp in id");
>   printf("\n");
>  }
>  

diff for iostat.8

not sure for the text though
is spinning time, time spent in locks?

The text could be "CPU time waiting for locks" maybe

Index: iostat.8
===
RCS file: /cvs/src/usr.sbin/iostat/iostat.8,v
retrieving revision 1.26
diff -u -p -r1.26 iostat.8
--- iostat.817 Jan 2013 21:39:29 -  1.26
+++ iostat.85 Sep 2018 08:29:36 -
@@ -170,6 +170,8 @@ Seconds spent in disk activity
 % of CPU time in user mode running niced processes
 .It \
 % of CPU time in system mode
+.It \
+% of CPU time spent spinning
 .It \
 % of CPU time processing interrupts
 .It \



Re: iostat: add "sp" column for CP_SPIN

2018-09-05 Thread Stuart Henderson
On 2018/09/04 16:11, Landry Breuil wrote:
> On Tue, Sep 04, 2018 at 03:27:29PM +0200, Solene Rapenne wrote:
> > Stuart Henderson  wrote:
> > > On 2018/09/04 14:22, Solene Rapenne wrote:
> > > > About munin I'm trying to get a diff accepted upstream to fix cpu 
> > > > plugin and
> > > > talk about this with kirby@. The cpu plugin uses sysctl kern.cp_time. 
> > > > While
> > > > it's not related, I prefer to announce it here so people don't waste 
> > > > time
> > > > fixing it again by looking at the thread :)
> > > 
> > > Ah nice. When I worked on the initial munin port with mk I had intended
> > > to try to get upstream to take the openbsd plugins separately (rather
> > > than the current "copy similar OS and patch" approach), but I got fed up
> > > with rrdtool performance on OpenBSD and stopped using munin before I got
> > > round to it.. (and then I started using librenms and got fed up with
> > > rrdtool performance once again ;)
> > 
> > Using rrdcached the performances are really good. But that certainly depend 
> > on
> > the number of systems. It may not scale well with more than 50 systems, 
> > this is
> > also highly dependent on the number of plugins on each systems (as 1 value 
> > = 1
> > rrd file).
> 
> Seconded, rrdcached really helps a lot until you have waaayyy too many rrds,
> and then switch to a real tsdb

While other time-series DBs may perform better than rrdtool, the amount
of spin (kernel lock contention) seen with this relatively simple software
suggests it might be a worthwhile starting point to investigate what's
going on kernel-side.

>(hint: influxdb is in ports)

(as is prometheus. no graphite, though ;)



Re: smtpd: malloc+strlcpy -> strndup

2018-09-05 Thread Gilles Chehade
On Mon, Sep 03, 2018 at 11:43:02PM +0800, Michael Mikonos wrote:
> On Mon, Sep 03, 2018 at 02:24:49PM +0800, Michael Mikonos wrote:
> > On Sat, Sep 01, 2018 at 11:31:49PM +0200, Gilles Chehade wrote:
> > > On Sat, Sep 01, 2018 at 09:20:59PM +0800, Michael Mikonos wrote:
> > > > Hello,
> > > > 
> > > > Replace a malloc+strlcpy with strndup in cmdline_symset().
> > > > Parameter s is a "keyname=value" string and sym is the
> > > > "keyname" part.
> > > > 
> > > > If s is "=value", sym will be an empty string.
> > > > The patch doesn't change this behaviour although
> > > > it might be undesirable to call symset() with
> > > > an empty string. Possibly it could also return -1
> > > > if len is zero. Thoughts?
> > > > 
> > > 
> > > Not opposed to the diff but at this late hour I find it easier to read
> > > the malloc+strlcpy and be sure there's not an off-by-one than with the
> > > strndup version, I'll read again tomorrow.
> > 
> > In my understanding the length argument of strndup(3) doesn't include
> > the terminating NUL character. I think the linux manual for strndup(3)
> > is slightly clearer on this because it has the text:
> > 
> >   ... only n bytes are copied, and a terminating null byte ('\0') is
> >   added.
> > 
> > > Just wanted to remind you that this function is shared between daemons
> > > so this can't be an smtpd-only change :-)
> 
> Thanks for the reminder. Here is a new version of the patch to include
> other daemons. I also followed a suggestion from halex@ to remove the
> strlen() calls and determine length using val-s. Did I miss anything?
> 

this looks good to me


> Index: acme-client/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
> retrieving revision 1.29
> diff -u -p -u -r1.29 parse.y
> --- acme-client/parse.y   3 Aug 2018 17:57:21 -   1.29
> +++ acme-client/parse.y   3 Sep 2018 15:18:23 -
> @@ -839,17 +839,12 @@ cmdline_symset(char *s)
>  {
>   char*sym, *val;
>   int ret;
> - size_t  len;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return -1;
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(EXIT_FAILURE, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(EXIT_FAILURE, "%s: strndup", __func__);
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  
> Index: bgpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.331
> diff -u -p -u -r1.331 parse.y
> --- bgpd/parse.y  27 Aug 2018 19:32:37 -  1.331
> +++ bgpd/parse.y  3 Sep 2018 15:18:24 -
> @@ -3145,17 +3145,12 @@ cmdline_symset(char *s)
>  {
>   char*sym, *val;
>   int ret;
> - size_t  len;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - fatal("cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + fatal("%s: strndup", __func__);
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  
> Index: dvmrpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/parse.y,v
> retrieving revision 1.36
> diff -u -p -u -r1.36 parse.y
> --- dvmrpd/parse.y11 Jul 2018 07:39:22 -  1.36
> +++ dvmrpd/parse.y3 Sep 2018 15:18:24 -
> @@ -834,17 +834,12 @@ cmdline_symset(char *s)
>  {
>   char*sym, *val;
>   int ret;
> - size_t  len;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
>   ret = symset(sym, val + 1, 1);
>   free(sym);
>  
> Index: eigrpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/eigrpd/parse.y,v
> retrieving revision 1.27
> diff -u -p -u -r1.27 parse.y
> --- eigrpd/parse.y11 Jul 2018 07:39:22 -  1.27
> +++ eigrpd/parse.y3 Sep 2018 15:18:24 -
> @@ -1094,17 +1094,12 @@ cmdline_symset(char *s)
>  {
>   char*sym, *val;
>   int ret;
> - size_t  len;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> +

Re: Update to table(5) man page

2018-09-05 Thread Jason McIntyre
On Tue, Sep 04, 2018 at 08:54:37AM -0400, Matt Schwartz wrote:
> Below is a diff to clear up the description of the Userinfo table in
> table(5). I also added an example of how it can be used with an Alias
> table.
> 
> Thanks,
> Matt
> 

fixed now. thanks,
jmc

> Index: table.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/table.5,v
> retrieving revision 1.9
> diff -u -p -u -r1.9 table.5
> --- table.524 May 2018 20:40:21 -1.9
> +++ table.54 Sep 2018 12:52:47 -
> @@ -174,16 +174,12 @@ ipv6:::1
>  192.168.1.0/24
>  .Ed
>  .Ss Userinfo tables
> -User info tables are used to described virtual system users.
> -They are used in rule context to specify an alternate user base, mapping
> -virtual users to local system UID, GID and home directory.
> +User info tables are used in rule context to specify an alternate user base,
> +mapping virtual users to local system users by UID, GID and home directory.
>  .Pp
>  .D1 Ic action Ar name method Cm userbase Pf < Ar table Ns >
>  .Pp
> -The userinfo table is a mapping from virtual user names to a set of system 
> user
> -ID, group ID and path to home directory.
> -.Pp
> -A userinfo table looks as follows:
> +An userinfo table looks as follows:
>  .Bd -literal -offset indent
>  joe1000:100:/home/virtual/joe
>  jack1000:100:/home/virtual/jack
> @@ -193,7 +189,15 @@ In this example, both joe and jack are v
>  system user with UID 1000 and GID 100, but different home directories.
>  These directories may contain a
>  .Xr forward 5
> -file.
> +file. This can be used in conjunction with an
> +.Cm Alias table
> +that maps an email address or the domain part to the desired virtual
> +username. For example:
> +.Bd -literal -offset indent
> +j...@example.orgjoe
> +j...@example.comjack
> +.Ed
> +.Pp
>  .Ss Source tables
>  Source tables are lists of IPv4 and IPv6 addresses.
>  They can only be used in the following context:
> 



Re: Update to table(5) man page

2018-09-05 Thread Gilles Chehade
On Wed, Sep 05, 2018 at 07:08:39AM +0100, Jason McIntyre wrote:
> On Tue, Sep 04, 2018 at 08:54:37AM -0400, Matt Schwartz wrote:
> > Below is a diff to clear up the description of the Userinfo table in
> > table(5). I also added an example of how it can be used with an Alias
> > table.
> > 
> > Thanks,
> > Matt
> > 
> 
> [...]
>
> 
> i think your diff reads better than what's there now. gilles, eric?
> 

agreed



-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg



Re: Update to table(5) man page

2018-09-05 Thread Jason McIntyre
On Tue, Sep 04, 2018 at 08:54:37AM -0400, Matt Schwartz wrote:
> Below is a diff to clear up the description of the Userinfo table in
> table(5). I also added an example of how it can be used with an Alias
> table.
> 
> Thanks,
> Matt
> 

morning.

comments inline.

> Index: table.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/table.5,v
> retrieving revision 1.9
> diff -u -p -u -r1.9 table.5
> --- table.524 May 2018 20:40:21 -1.9
> +++ table.54 Sep 2018 12:52:47 -
> @@ -174,16 +174,12 @@ ipv6:::1
>  192.168.1.0/24
>  .Ed
>  .Ss Userinfo tables
> -User info tables are used to described virtual system users.
> -They are used in rule context to specify an alternate user base, mapping
> -virtual users to local system UID, GID and home directory.
> +User info tables are used in rule context to specify an alternate user base,
> +mapping virtual users to local system users by UID, GID and home directory.
>  .Pp
>  .D1 Ic action Ar name method Cm userbase Pf < Ar table Ns >
>  .Pp
> -The userinfo table is a mapping from virtual user names to a set of system 
> user
> -ID, group ID and path to home directory.
> -.Pp
> -A userinfo table looks as follows:
> +An userinfo table looks as follows:

"A" not "An"

>  .Bd -literal -offset indent
>  joe1000:100:/home/virtual/joe
>  jack1000:100:/home/virtual/jack
> @@ -193,7 +189,15 @@ In this example, both joe and jack are v
>  system user with UID 1000 and GID 100, but different home directories.
>  These directories may contain a
>  .Xr forward 5
> -file.
> +file. This can be used in conjunction with an

start new sentences on new lines in man pages

> +.Cm Alias table

i would not mark up "Alias table". i think plain "alias table", without
uppercase, is enough

> +that maps an email address or the domain part to the desired virtual
> +username. For example:

again, new sentence, new line

> +.Bd -literal -offset indent
> +j...@example.orgjoe
> +j...@example.comjack
> +.Ed
> +.Pp
>  .Ss Source tables
>  Source tables are lists of IPv4 and IPv6 addresses.
>  They can only be used in the following context:
>

i think your diff reads better than what's there now. gilles, eric?

jmc