Re: trunk: keep interface up on port removal

2020-09-12 Thread Alexander Bluhm
OK bluhm@

On Sat, Sep 12, 2020 at 05:49:52PM +0200, Klemens Nanni wrote:
> Index: if_trunk.c
> ===
> RCS file: /cvs/src/sys/net/if_trunk.c,v
> retrieving revision 1.149
> diff -u -p -r1.149 if_trunk.c
> --- if_trunk.c28 Jul 2020 09:52:32 -  1.149
> +++ if_trunk.c12 Sep 2020 15:41:14 -
> @@ -423,10 +423,6 @@ trunk_port_destroy(struct trunk_port *tp
>   /* Remove multicast addresses from this port */
>   trunk_ether_cmdmulti(tp, SIOCDELMULTI);
>  
> - /* Port has to be down */
> - if (ifp->if_flags & IFF_UP)
> - if_down(ifp);
> -
>   ifpromisc(ifp, 0);
>  
>   if (tr->tr_port_destroy != NULL)



Re: basename(3) should have non-const arg, says POSIX

2020-09-12 Thread Christian Weisgerber
Todd C. Miller:

> This is probably the right thing to do but we should fix the warnings
> it generates.  In this new world order, passing a const char * to
> basename() or dirname() is unsafe.

FWIW, here's the list:

/usr/src/lib/libkvm/kvm.c:684:16: warning: passing 'const char *' to parameter 
of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
uf = basename(uf);
  ^~
/usr/src/bin/chio/parse.y:449:23: warning: passing 'const char *' to parameter 
of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
if (strcmp(basename(changer), p->name) == 0) {
^~~
/usr/src/sbin/pfctl/pfctl.c:2250:15: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
n = dirname(anchor);
^~
/usr/src/sbin/pfctl/pfctl.c:2253:16: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
n = basename(anchor);
 ^~
/usr/src/usr.bin/compress/main.c:537:19: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
name = basename(in);
^~
/usr/src/usr.bin/cvs/checkout.c:398:14: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
d = dirname(repo);
^~~~
/usr/src/usr.bin/cvs/checkout.c:399:15: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
f = basename(repo);
 ^~~~
/usr/src/usr.bin/ftp/fetch.c:219:24: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
savefile = basename(path);
^~~~
/usr/src/usr.bin/ftp/util.c:785:23: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
filename = basename(filename);
^~~~
/usr/src/usr.bin/patch/backupfile.c:61:34: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
base_versions = concat(basename(file), ".~");
^~~~
/usr/src/usr.bin/patch/backupfile.c:64:16: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
dir = dirname(file);
  ^~~~
/usr/src/usr.bin/rcs/rlog.c:367:27: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
if ((workfile = basename(fname)) == NULL)
 ^
/usr/src/usr.bin/sed/main.c:401:16: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
dirname(fname));
^
/usr/src/usr.sbin/hotplugd/hotplugd.c:166:24: warning: passing 'const char *' 
to parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
execl(file, basename(file), strclass, name, (char *)NULL);
 ^~~~
/usr/src/usr.sbin/vmd/vioqcow2.c:189:15: warning: passing 'const char *' to 
parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
s = dirname(dpath);
^
/usr/src/usr.sbin/vmctl/../vmd/vioqcow2.c:189:15: warning: passing 'const char 
*' to parameter of type 'char *' discards qualifiers 
[-Wincompatible-pointer-types-discards-qualifiers]
s = dirname(dpath);
^

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: sppp: add free() sizes

2020-09-12 Thread Stuart Henderson
On 2020/09/12 19:13, Martin Pieuchot wrote:
> Another approach would be to always use array of AUTHMAXLEN, I'm not sure
> the size justifies two malloc(9).

it used to do that, changed in if_sppsubr.c 1.74



Re: unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Florian Obser
On Sat, Sep 12, 2020 at 08:13:43PM +0300, Peter Pentchev wrote:
> On Sat, Sep 12, 2020 at 06:58:30PM +0200, Florian Obser wrote:
> > On Sat, Sep 12, 2020 at 05:15:08PM +0200, Klemens Nanni wrote:
> > > On Sat, Sep 12, 2020 at 05:11:00PM +0200, Klemens Nanni wrote:
> > > > Bit hard to read, what about aligning like this?
> > > > 
> > > > +   if ((rdns_proposal->src == 0 ||
> > > > +rdns_proposal->src == tmp->src) &&
> > > > +   (rdns_proposal->if_index == 0 ||
> > > > +   rdns_proposal->if_index == tmp->if_index))
> > > With a space here   ^
> > > 
> > 
> > I'd rather not, style(9):
> > Indentation is an 8 character tab.  Second level indents are four 
> > spaces.
> > 
> > Also, my editor would fix it up the next time I touch those lines.
> 
> Er... so what's the difference with the second line, two lines above
> that? "rdns_proposal->src == tmp->src" seems to have that space.

sure, because kn added it, have a look at the original diff, it's
formated completely different.

> 
> G'luck,
> Peter
> 
> -- 
> Peter Pentchev  r...@ringlet.net r...@debian.org p...@storpool.com
> PGP key:http://people.FreeBSD.org/~roam/roam.key.asc
> Key fingerprint 2EE7 A7A5 17FC 124C F115  C354 651E EFB0 2527 DF13



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



Re: unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Peter Pentchev
On Sat, Sep 12, 2020 at 06:58:30PM +0200, Florian Obser wrote:
> On Sat, Sep 12, 2020 at 05:15:08PM +0200, Klemens Nanni wrote:
> > On Sat, Sep 12, 2020 at 05:11:00PM +0200, Klemens Nanni wrote:
> > > Bit hard to read, what about aligning like this?
> > > 
> > > + if ((rdns_proposal->src == 0 ||
> > > +  rdns_proposal->src == tmp->src) &&
> > > + (rdns_proposal->if_index == 0 ||
> > > + rdns_proposal->if_index == tmp->if_index))
> > With a space here   ^
> > 
> 
> I'd rather not, style(9):
>   Indentation is an 8 character tab.  Second level indents are four 
> spaces.
> 
> Also, my editor would fix it up the next time I touch those lines.

Er... so what's the difference with the second line, two lines above
that? "rdns_proposal->src == tmp->src" seems to have that space.

G'luck,
Peter

-- 
Peter Pentchev  r...@ringlet.net r...@debian.org p...@storpool.com
PGP key:http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint 2EE7 A7A5 17FC 124C F115  C354 651E EFB0 2527 DF13


signature.asc
Description: PGP signature


Re: basename(3) should have non-const arg, says POSIX

2020-09-12 Thread Todd C . Miller
On Sat, 12 Sep 2020 17:05:02 +0200, Christian Weisgerber wrote:

> A make build with the patch below succeeds, but gains some new
> warnings "passing 'const char *' to parameter of type 'char *'
> discards qualifiers".
>
> This is a portability trap.  Code written on OpenBSD may not be
> prepared for basename() or dirname() to splat a '\0' into the input
> string, despite the warning in the man page.
>
> I'm in favor of moving to the POSIX prototypes, but I don't know
> if there are any hidden pitfalls I may be missing.  Opinions,
> comments?

This is probably the right thing to do but we should fix the warnings
it generates.  In this new world order, passing a const char * to
basename() or dirname() is unsafe.

 - todd



Re: sppp: add free() sizes

2020-09-12 Thread Martin Pieuchot
On 12/09/20(Sat) 14:49, Klemens Nanni wrote:
> These are the last free(buf, 0) occurences in if_pppoe.c and
> if_spppsubr.c changing to non-zero sizes.
> 
> I've been running with this the last week without any issues.
> 
> Feedback? OK?

Maybe store `pwdlen' and `idlen' in "struct sppp" instead of recomputing
it everytime? 

Another approach would be to always use array of AUTHMAXLEN, I'm not sure
the size justifies two malloc(9).

Anyway the diff is ok mpi@

> Index: if_spppsubr.c
> ===
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.186
> diff -u -p -r1.186 if_spppsubr.c
> --- if_spppsubr.c 22 Aug 2020 16:12:12 -  1.186
> +++ if_spppsubr.c 3 Sep 2020 21:43:54 -
> @@ -750,13 +750,15 @@ sppp_detach(struct ifnet *ifp)
>  
>   /* release authentication data */
>   if (sp->myauth.name != NULL)
> - free(sp->myauth.name, M_DEVBUF, 0);
> + free(sp->myauth.name, M_DEVBUF, strlen(sp->myauth.name) + 1);
>   if (sp->myauth.secret != NULL)
> - free(sp->myauth.secret, M_DEVBUF, 0);
> + free(sp->myauth.secret, M_DEVBUF,
> + strlen(sp->myauth.secret) + 1);
>   if (sp->hisauth.name != NULL)
> - free(sp->hisauth.name, M_DEVBUF, 0);
> + free(sp->hisauth.name, M_DEVBUF, strlen(sp->hisauth.name) + 1);
>   if (sp->hisauth.secret != NULL)
> - free(sp->hisauth.secret, M_DEVBUF, 0);
> + free(sp->hisauth.secret, M_DEVBUF,
> + strlen(sp->hisauth.secret) + 1);
>  }
>  
>  /*
> @@ -4579,9 +4587,11 @@ sppp_set_params(struct sppp *sp, struct 
>   if (spa->proto == 0) {
>   /* resetting auth */
>   if (auth->name != NULL)
> - free(auth->name, M_DEVBUF, 0);
> + free(auth->name, M_DEVBUF,
> + strlen(auth->name) + 1);
>   if (auth->secret != NULL)
> - free(auth->secret, M_DEVBUF, 0);
> + free(auth->secret, M_DEVBUF,
> + strlen(auth->secret) + 1);
>   bzero(auth, sizeof *auth);
>   explicit_bzero(sp->chap_challenge, sizeof 
> sp->chap_challenge);
>   } else {
> @@ -4594,7 +4604,8 @@ sppp_set_params(struct sppp *sp, struct 
>   p = malloc(len, M_DEVBUF, M_WAITOK);
>   strlcpy(p, spa->name, len);
>   if (auth->name != NULL)
> - free(auth->name, M_DEVBUF, 0);
> + free(auth->name, M_DEVBUF,
> + strlen(auth->name) + 1);
>   auth->name = p;
>  
>   if (spa->secret[0] != '\0') {
> @@ -4603,7 +4614,8 @@ sppp_set_params(struct sppp *sp, struct 
>   p = malloc(len, M_DEVBUF, M_WAITOK);
>   strlcpy(p, spa->secret, len);
>   if (auth->secret != NULL)
> - free(auth->secret, M_DEVBUF, 0);
> + free(auth->secret, M_DEVBUF,
> + strlen(auth->secret) + 1);
>   auth->secret = p;
>   } else if (!auth->secret) {
>   p = malloc(1, M_DEVBUF, M_WAITOK);
> 



Re: trunk: keep interface up on port removal

2020-09-12 Thread Florian Obser
Seems reasonable. OK

On Sat, Sep 12, 2020 at 05:49:52PM +0200, Klemens Nanni wrote:
> Unconfiguring a member interface from trunk(4) or simply destroying the
> trunk pulls the member down for no reason, both comment and code are
> there since import, but I see no justification for doing so.
> 
> aggr(4) does not pull its member down upon removal either.
> 
> I came across this after
> 
>   $ doas ifconfig trunk0 destroy
>   $ doas sh /etc/netstart trunk0
> 
> yielded no network and I had to manually pull up members.
> 
> Feedback? OK?
> 
> 
> Index: if_trunk.c
> ===
> RCS file: /cvs/src/sys/net/if_trunk.c,v
> retrieving revision 1.149
> diff -u -p -r1.149 if_trunk.c
> --- if_trunk.c28 Jul 2020 09:52:32 -  1.149
> +++ if_trunk.c12 Sep 2020 15:41:14 -
> @@ -423,10 +423,6 @@ trunk_port_destroy(struct trunk_port *tp
>   /* Remove multicast addresses from this port */
>   trunk_ether_cmdmulti(tp, SIOCDELMULTI);
>  
> - /* Port has to be down */
> - if (ifp->if_flags & IFF_UP)
> - if_down(ifp);
> -
>   ifpromisc(ifp, 0);
>  
>   if (tr->tr_port_destroy != NULL)
> 

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



Re: unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Florian Obser
On Sat, Sep 12, 2020 at 05:15:08PM +0200, Klemens Nanni wrote:
> On Sat, Sep 12, 2020 at 05:11:00PM +0200, Klemens Nanni wrote:
> > Bit hard to read, what about aligning like this?
> > 
> > +   if ((rdns_proposal->src == 0 ||
> > +rdns_proposal->src == tmp->src) &&
> > +   (rdns_proposal->if_index == 0 ||
> > +   rdns_proposal->if_index == tmp->if_index))
> With a space here   ^
> 

I'd rather not, style(9):
Indentation is an 8 character tab.  Second level indents are four 
spaces.

Also, my editor would fix it up the next time I touch those lines.

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



trunk: keep interface up on port removal

2020-09-12 Thread Klemens Nanni
Unconfiguring a member interface from trunk(4) or simply destroying the
trunk pulls the member down for no reason, both comment and code are
there since import, but I see no justification for doing so.

aggr(4) does not pull its member down upon removal either.

I came across this after

$ doas ifconfig trunk0 destroy
$ doas sh /etc/netstart trunk0

yielded no network and I had to manually pull up members.

Feedback? OK?


Index: if_trunk.c
===
RCS file: /cvs/src/sys/net/if_trunk.c,v
retrieving revision 1.149
diff -u -p -r1.149 if_trunk.c
--- if_trunk.c  28 Jul 2020 09:52:32 -  1.149
+++ if_trunk.c  12 Sep 2020 15:41:14 -
@@ -423,10 +423,6 @@ trunk_port_destroy(struct trunk_port *tp
/* Remove multicast addresses from this port */
trunk_ether_cmdmulti(tp, SIOCDELMULTI);
 
-   /* Port has to be down */
-   if (ifp->if_flags & IFF_UP)
-   if_down(ifp);
-
ifpromisc(ifp, 0);
 
if (tr->tr_port_destroy != NULL)



Re: rpki-client cleanup includes

2020-09-12 Thread Bob Beck


ok beck@

On Sat, Sep 12, 2020 at 05:42:39PM +0200, Claudio Jeker wrote:
> extern.h uses stuff from openssl/x509.h so put that include in there
> and remove all the various other openssl includes in other files that
> actually don't need x509 functions.
> 
> -- 
> :wq Claudio
> 
> Index: as.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/as.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 as.c
> --- as.c  27 Nov 2019 17:18:24 -  1.5
> +++ as.c  12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "extern.h"
>  
>  /*
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 cert.c
> --- cert.c28 Jul 2020 07:35:04 -  1.17
> +++ cert.c12 Sep 2020 15:02:20 -
> @@ -26,7 +26,6 @@
>  #include 
>  #include 
>  
> -#include 
>  #include  /* DIST_POINT */
>  
>  #include "extern.h"
> Index: crl.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 crl.c
> --- crl.c 2 Apr 2020 09:16:43 -   1.8
> +++ crl.c 12 Sep 2020 15:02:20 -
> @@ -26,8 +26,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "extern.h"
>  
>  X509_CRL *
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.33
> diff -u -p -r1.33 extern.h
> --- extern.h  12 Sep 2020 10:02:01 -  1.33
> +++ extern.h  12 Sep 2020 15:02:20 -
> @@ -20,6 +20,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  enum cert_as_type {
>   CERT_AS_ID, /* single identifier */
>   CERT_AS_INHERIT, /* inherit from parent */
> Index: io.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/io.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 io.c
> --- io.c  29 Nov 2019 05:09:50 -  1.8
> +++ io.c  12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "extern.h"
>  
>  void
> Index: ip.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/ip.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 ip.c
> --- ip.c  16 Apr 2020 14:39:44 -  1.12
> +++ ip.c  12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "extern.h"
>  
>  #define   PREFIX_SIZE(x)  (((x) + 7) / 8)
> Index: log.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/log.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 log.c
> --- log.c 29 Nov 2019 05:14:11 -  1.5
> +++ log.c 12 Sep 2020 15:02:20 -
> @@ -21,7 +21,6 @@
>  #include 
>  
>  #include 
> -#include 
>  
>  #include "extern.h"
>  
> Index: mft.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 mft.c
> --- mft.c 30 Jun 2020 12:52:44 -  1.15
> +++ mft.c 12 Sep 2020 15:02:20 -
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  
> -#include 
>  #include 
>  
>  #include "extern.h"
> Index: output-bgpd.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-bgpd.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 output-bgpd.c
> --- output-bgpd.c 28 Apr 2020 13:41:35 -  1.17
> +++ output-bgpd.c 12 Sep 2020 15:02:20 -
> @@ -16,7 +16,6 @@
>   */
>  
>  #include 
> -#include 
>  
>  #include "extern.h"
>  
> Index: output-bird.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-bird.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 output-bird.c
> --- output-bird.c 28 Apr 2020 15:03:39 -  1.9
> +++ output-bird.c 12 Sep 2020 15:02:20 -
> @@ -17,7 +17,6 @@
>   */
>  
>  #include 
> -#include 
>  
>  #include "extern.h"
>  
> Index: output-csv.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-csv.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 output-csv.c
> --- output-csv.c  28 Apr 2020 13:41:35 -  1.7
> +++ output-csv.c  12 Sep 2020 15:02:20 -
> @@ -16,7 +16,6 @@
>   */
>  
>  #include 
> -#include 
>  
>  #include "extern.h"
>  
> Index: output-json.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 output-json.c
> --- output-json.c 3 May 2020 20:24:02 -   1.12
> 

rpki-client cleanup includes

2020-09-12 Thread Claudio Jeker
extern.h uses stuff from openssl/x509.h so put that include in there
and remove all the various other openssl includes in other files that
actually don't need x509 functions.

-- 
:wq Claudio

Index: as.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/as.c,v
retrieving revision 1.5
diff -u -p -r1.5 as.c
--- as.c27 Nov 2019 17:18:24 -  1.5
+++ as.c12 Sep 2020 15:02:20 -
@@ -25,8 +25,6 @@
 #include 
 #include 
 
-#include 
-
 #include "extern.h"
 
 /*
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.17
diff -u -p -r1.17 cert.c
--- cert.c  28 Jul 2020 07:35:04 -  1.17
+++ cert.c  12 Sep 2020 15:02:20 -
@@ -26,7 +26,6 @@
 #include 
 #include 
 
-#include 
 #include  /* DIST_POINT */
 
 #include "extern.h"
Index: crl.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
retrieving revision 1.8
diff -u -p -r1.8 crl.c
--- crl.c   2 Apr 2020 09:16:43 -   1.8
+++ crl.c   12 Sep 2020 15:02:20 -
@@ -26,8 +26,6 @@
 #include 
 #include 
 
-#include 
-
 #include "extern.h"
 
 X509_CRL *
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.33
diff -u -p -r1.33 extern.h
--- extern.h12 Sep 2020 10:02:01 -  1.33
+++ extern.h12 Sep 2020 15:02:20 -
@@ -20,6 +20,8 @@
 #include 
 #include 
 
+#include 
+
 enum cert_as_type {
CERT_AS_ID, /* single identifier */
CERT_AS_INHERIT, /* inherit from parent */
Index: io.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/io.c,v
retrieving revision 1.8
diff -u -p -r1.8 io.c
--- io.c29 Nov 2019 05:09:50 -  1.8
+++ io.c12 Sep 2020 15:02:20 -
@@ -25,8 +25,6 @@
 #include 
 #include 
 
-#include 
-
 #include "extern.h"
 
 void
Index: ip.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/ip.c,v
retrieving revision 1.12
diff -u -p -r1.12 ip.c
--- ip.c16 Apr 2020 14:39:44 -  1.12
+++ ip.c12 Sep 2020 15:02:20 -
@@ -25,8 +25,6 @@
 #include 
 #include 
 
-#include 
-
 #include "extern.h"
 
 #define   PREFIX_SIZE(x)  (((x) + 7) / 8)
Index: log.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/log.c,v
retrieving revision 1.5
diff -u -p -r1.5 log.c
--- log.c   29 Nov 2019 05:14:11 -  1.5
+++ log.c   12 Sep 2020 15:02:20 -
@@ -21,7 +21,6 @@
 #include 
 
 #include 
-#include 
 
 #include "extern.h"
 
Index: mft.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
retrieving revision 1.15
diff -u -p -r1.15 mft.c
--- mft.c   30 Jun 2020 12:52:44 -  1.15
+++ mft.c   12 Sep 2020 15:02:20 -
@@ -24,7 +24,6 @@
 #include 
 #include 
 
-#include 
 #include 
 
 #include "extern.h"
Index: output-bgpd.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/output-bgpd.c,v
retrieving revision 1.17
diff -u -p -r1.17 output-bgpd.c
--- output-bgpd.c   28 Apr 2020 13:41:35 -  1.17
+++ output-bgpd.c   12 Sep 2020 15:02:20 -
@@ -16,7 +16,6 @@
  */
 
 #include 
-#include 
 
 #include "extern.h"
 
Index: output-bird.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/output-bird.c,v
retrieving revision 1.9
diff -u -p -r1.9 output-bird.c
--- output-bird.c   28 Apr 2020 15:03:39 -  1.9
+++ output-bird.c   12 Sep 2020 15:02:20 -
@@ -17,7 +17,6 @@
  */
 
 #include 
-#include 
 
 #include "extern.h"
 
Index: output-csv.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/output-csv.c,v
retrieving revision 1.7
diff -u -p -r1.7 output-csv.c
--- output-csv.c28 Apr 2020 13:41:35 -  1.7
+++ output-csv.c12 Sep 2020 15:02:20 -
@@ -16,7 +16,6 @@
  */
 
 #include 
-#include 
 
 #include "extern.h"
 
Index: output-json.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
retrieving revision 1.12
diff -u -p -r1.12 output-json.c
--- output-json.c   3 May 2020 20:24:02 -   1.12
+++ output-json.c   12 Sep 2020 15:02:20 -
@@ -19,7 +19,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "extern.h"
 
Index: output.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/output.c,v
retrieving revision 1.16
diff -u -p -r1.16 output.c
--- output.c14 May 2020 20:49:04 -  1.16
+++ output.c

Re: unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Klemens Nanni
On Sat, Sep 12, 2020 at 05:11:00PM +0200, Klemens Nanni wrote:
> Bit hard to read, what about aligning like this?
> 
> + if ((rdns_proposal->src == 0 ||
> +  rdns_proposal->src == tmp->src) &&
> + (rdns_proposal->if_index == 0 ||
> + rdns_proposal->if_index == tmp->if_index))
With a space here   ^



Re: unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Klemens Nanni
On Sat, Sep 12, 2020 at 04:36:28PM +0200, Florian Obser wrote:
> ... say if you pull a usb stick.
...or if you play with^W^Wrecreate your trunk0 { em0 athn0 } uplink
without checking unwind.

Without this diff, unwind cannot recover;  with it, stuff just works for
me across destroy/create.

OK kn

> diff --git resolver.c resolver.c
> index 874ad5e76b3..8cf72db7250 100644
> --- resolver.c
> +++ resolver.c
> @@ -1952,10 +1952,13 @@ replace_autoconf_forwarders(struct imsg_rdns_proposal 
> *rdns_proposal)
>   }
>  
>   TAILQ_FOREACH(tmp, _forwarder_list, entry) {
> - /* if_index of zero signals to clear all proposals */
> - if (rdns_proposal->src == tmp->src &&
> - (rdns_proposal->if_index == 0 || rdns_proposal->if_index ==
> - tmp->if_index))
> + /*
> +  * if_index of zero signals to clear all proposals
> +  * src of zero signals interface gone
> +  */
> + if ((rdns_proposal->src == 0 || rdns_proposal->src ==
> + tmp->src) && (rdns_proposal->if_index == 0 ||
> + rdns_proposal->if_index == tmp->if_index))
Bit hard to read, what about aligning like this?

+   if ((rdns_proposal->src == 0 ||
+rdns_proposal->src == tmp->src) &&
+   (rdns_proposal->if_index == 0 ||
+   rdns_proposal->if_index == tmp->if_index))

>   continue;
>   if ((uw_forwarder = calloc(1, sizeof(struct uw_forwarder))) ==
>   NULL)



basename(3) should have non-const arg, says POSIX

2020-09-12 Thread Christian Weisgerber
Our basename(3) and dirname(3) take a const argument:

  char*basename(const char *);
  char*dirname(const char *);

POSIX says otherwise...

  char *basename(char *path);
  char *dirname(char *path);

... and explicitly says the functions may modify the input string.


Our functions were const-ified in 1999 by espie@:

  proper const semantics for dirname & basename.
  (this follows FreeBSD and Linux. Single Unix 2 is still illogical)

Well, four years ago, FreeBSD finally switched to the POSIX prototypes
https://svnweb.freebsd.org/base/head/include/libgen.h?revision=303451=markup
and in fact now has an implementation that modifies the string.

Linux (GNU libc) has a bizarro solution where you get a const basename()
and no dirname() if you just include , but POSIX basename()
and dirname() if you instead or also include .

A make build with the patch below succeeds, but gains some new
warnings "passing 'const char *' to parameter of type 'char *'
discards qualifiers".

This is a portability trap.  Code written on OpenBSD may not be
prepared for basename() or dirname() to splat a '\0' into the input
string, despite the warning in the man page.

I'm in favor of moving to the POSIX prototypes, but I don't know
if there are any hidden pitfalls I may be missing.  Opinions,
comments?


Index: include/libgen.h
===
RCS file: /cvs/src/include/libgen.h,v
retrieving revision 1.9
diff -u -p -r1.9 libgen.h
--- include/libgen.h25 Jan 2019 00:19:25 -  1.9
+++ include/libgen.h11 Sep 2020 20:41:34 -
@@ -22,8 +22,8 @@
 #include 
 
 __BEGIN_DECLS
-char   *basename(const char *);
-char   *dirname(const char *);
+char   *basename(char *);
+char   *dirname(char *);
 __END_DECLS
 
 #endif /* _LIBGEN_H_ */
Index: lib/libc/gen/basename.3
===
RCS file: /cvs/src/lib/libc/gen/basename.3,v
retrieving revision 1.24
diff -u -p -r1.24 basename.3
--- lib/libc/gen/basename.3 25 Jan 2019 00:19:25 -  1.24
+++ lib/libc/gen/basename.3 11 Sep 2020 20:46:30 -
@@ -23,7 +23,7 @@
 .Sh SYNOPSIS
 .In libgen.h
 .Ft char *
-.Fn basename "const char *path"
+.Fn basename "char *path"
 .Sh DESCRIPTION
 The
 .Fn basename
Index: lib/libc/gen/basename.c
===
RCS file: /cvs/src/lib/libc/gen/basename.c,v
retrieving revision 1.16
diff -u -p -r1.16 basename.c
--- lib/libc/gen/basename.c 25 Jan 2019 00:19:25 -  1.16
+++ lib/libc/gen/basename.c 11 Sep 2020 20:43:13 -
@@ -22,7 +22,7 @@
 #include 
 
 char *
-basename(const char *path)
+basename(char *path)
 {
static char bname[PATH_MAX];
size_t len;
Index: lib/libc/gen/dirname.3
===
RCS file: /cvs/src/lib/libc/gen/dirname.3,v
retrieving revision 1.23
diff -u -p -r1.23 dirname.3
--- lib/libc/gen/dirname.3  8 Mar 2019 17:33:23 -   1.23
+++ lib/libc/gen/dirname.3  11 Sep 2020 20:47:08 -
@@ -23,7 +23,7 @@
 .Sh SYNOPSIS
 .In libgen.h
 .Ft char *
-.Fn dirname "const char *path"
+.Fn dirname "char *path"
 .Sh DESCRIPTION
 The
 .Fn dirname
Index: lib/libc/gen/dirname.c
===
RCS file: /cvs/src/lib/libc/gen/dirname.c,v
retrieving revision 1.16
diff -u -p -r1.16 dirname.c
--- lib/libc/gen/dirname.c  25 Jan 2019 00:19:25 -  1.16
+++ lib/libc/gen/dirname.c  11 Sep 2020 20:43:34 -
@@ -24,7 +24,7 @@
 /* A slightly modified copy of this file exists in libexec/ld.so */
 
 char *
-dirname(const char *path)
+dirname(char *path)
 {
static char dname[PATH_MAX];
size_t len;
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Florian Obser
... say if you pull a usb stick.

OK?

diff --git frontend.c frontend.c
index 87141f81e8c..b92cde8226c 100644
--- frontend.c
+++ frontend.c
@@ -913,8 +913,21 @@ handle_route_message(struct rt_msghdr *rtm, struct 
sockaddr **rti_info)
 {
struct imsg_rdns_proposalrdns_proposal;
struct sockaddr_rtdns   *rtdns;
+   struct if_announcemsghdr*ifan;
 
switch (rtm->rtm_type) {
+   case RTM_IFANNOUNCE:
+   ifan = (struct if_announcemsghdr *)rtm;
+   if (ifan->ifan_what == IFAN_ARRIVAL)
+   break;
+   rdns_proposal.if_index = ifan->ifan_index;
+   rdns_proposal.src = 0;
+   rdns_proposal.rtdns.sr_family = AF_INET;
+   rdns_proposal.rtdns.sr_len = offsetof(struct sockaddr_rtdns,
+   sr_dns);
+   frontend_imsg_compose_resolver(IMSG_REPLACE_DNS, 0,
+   _proposal, sizeof(rdns_proposal));
+   break;
case RTM_IFINFO:
frontend_imsg_compose_resolver(IMSG_NETWORK_CHANGED, 0, NULL,
0);
diff --git resolver.c resolver.c
index 874ad5e76b3..8cf72db7250 100644
--- resolver.c
+++ resolver.c
@@ -1952,10 +1952,13 @@ replace_autoconf_forwarders(struct imsg_rdns_proposal 
*rdns_proposal)
}
 
TAILQ_FOREACH(tmp, _forwarder_list, entry) {
-   /* if_index of zero signals to clear all proposals */
-   if (rdns_proposal->src == tmp->src &&
-   (rdns_proposal->if_index == 0 || rdns_proposal->if_index ==
-   tmp->if_index))
+   /*
+* if_index of zero signals to clear all proposals
+* src of zero signals interface gone
+*/
+   if ((rdns_proposal->src == 0 || rdns_proposal->src ==
+   tmp->src) && (rdns_proposal->if_index == 0 ||
+   rdns_proposal->if_index == tmp->if_index))
continue;
if ((uw_forwarder = calloc(1, sizeof(struct uw_forwarder))) ==
NULL)
diff --git unwind.c unwind.c
index b17bf7e413c..d7ae76d4274 100644
--- unwind.c
+++ unwind.c
@@ -266,7 +266,8 @@ main(int argc, char *argv[])
AF_INET)) == -1)
fatal("route socket");
 
-   rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL);
+   rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL)
+   | ROUTE_FILTER(RTM_IFANNOUNCE);
if (setsockopt(frontend_routesock, AF_ROUTE, ROUTE_MSGFILTER,
, sizeof(rtfilter)) == -1)
fatal("setsockopt(ROUTE_MSGFILTER)");


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



Re: acpiapplesmc(4)

2020-09-12 Thread Mark Kettenis
> Date: Sat, 12 Sep 2020 12:48:48 +0200
> From: Marcus Glocker 
> Cc: m...@umaxx.net, j...@jcs.org, tech@openbsd.org
> Content-Type: text/plain; charset=utf-8
> 
> On Sat, 12 Sep 2020 10:28:23 +0200 (CEST)
> Mark Kettenis  wrote:
> 
> > > Date: Sat, 12 Sep 2020 10:00:13 +0200
> > > From: Marcus Glocker 
> > > 
> > > On Sat, 12 Sep 2020 09:34:18 +0200 (CEST)
> > > Mark Kettenis  wrote:
> > >   
> > > > > Date: Sat, 12 Sep 2020 09:20:26 +0200
> > > > > From: Marcus Glocker 
> > > > > 
> > > > > On Fri, 11 Sep 2020 23:18:56 +0200 (CEST)
> > > > > Mark Kettenis  wrote:
> > > > > 
> > > > > > > Date: Fri, 11 Sep 2020 17:42:23 +0200
> > > > > > > From: Marcus Glocker 
> > > > > > > 
> > > > > > > On Thu, 10 Sep 2020 23:44:38 +0200
> > > > > > > Joerg Jung  wrote:
> > > > > > >   
> > > > > > > > Don’t give up so quickly ;) 
> > > > > > > > let’s try to make the driver work on your iMac, send me
> > > > > > > > dmesg and sysctl hw output please.
> > > > > > > > 
> > > > > > > > Your idea of converting it to ACPI is the right thing to
> > > > > > > > do anyways, would be nice to get this working.  
> > > > > > > 
> > > > > > > Here we go:
> > > > > > > 
> > > > > > > $ dmesg | grep smc
> > > > > > > asmc0 at acpi0: SMC_ (smc-piketon) addr 0x300/0x20: rev
> > > > > > > 1.64f564, 276 keys
> > > > > > > 
> > > > > > > $ sysctl -a | grep smc
> > > > > > > hw.sensors.asmc0.temp0=27.00 degC (TA0P ambient)
> > > > > > > hw.sensors.asmc0.temp1=42.00 degC (TC0H cpu0 heatsink)
> > > > > > > hw.sensors.asmc0.temp2=55.00 degC (TG0D gpu0 diode)
> > > > > > > hw.sensors.asmc0.temp3=53.00 degC (TG0H gpu0 heatsink)
> > > > > > > hw.sensors.asmc0.temp4=38.00 degC (TL0P lcd proximity)
> > > > > > > hw.sensors.asmc0.temp5=41.00 degC (TO0P optical drive)
> > > > > > > hw.sensors.asmc0.temp6=50.00 degC (Tm0P memory controller)
> > > > > > > hw.sensors.asmc0.fan0=998 RPM (ODD, right mid rear)
> > > > > > > hw.sensors.asmc0.fan1=1158 RPM (HDD, center mid rear)
> > > > > > > hw.sensors.asmc0.fan2=1200 RPM (CPU, left lower rear)
> > > > > > > 
> > > > > > > Does that work for you guys?  
> > > > > > 
> > > > > > $ dmesg | grep smc
> > > > > > asmc0 at acpi0: SMC_ (smc-napa) addr 0x300/0x20: rev 1.3f503,
> > > > > > 137 keys
> > > > > > 
> > > > > > $ sysctl -a | grep smc
> > > > > > hw.sensors.asmc0.temp0=63.00 degC (TC0D cpu0 die core)
> > > > > > hw.sensors.asmc0.temp1=55.00 degC (TC0H cpu0 heatsink)
> > > > > > hw.sensors.asmc0.temp2=58.00 degC (TC0P cpu0 proximity)
> > > > > > hw.sensors.asmc0.temp3=52.00 degC (TN0P northbridge proximity)
> > > > > > hw.sensors.asmc0.temp4=52.00 degC (TN1P northbridge 2)
> > > > > > hw.sensors.asmc0.fan0=2077 RPM (Master, left upper front)
> > > > > > 
> > > > > > So yes, this works for me.
> > > > > 
> > > > > Cool.
> > > > >  
> > > > > > You'll need to make changes to the i386 GENERIC kernel as
> > > > > > well.
> > > > > 
> > > > > Yep, done.
> > > > > 
> > > > > > And I'd like to ask you to make one small change...
> > > > > 
> > > > > > > +const char *acpiapplesmc_hids[] = {  
> > > > > > 
> > > > > > ...can you rename this variable to asmc_hids[]?
> > > > > 
> > > > > Of course, copy/pasto, thanks for spotting.
> > > > > 
> > > > > Also Joerg did suggest to hard code smc0 in GENERIC like it was
> > > > > before, since it won't be possible that there is more than 1 SMC
> > > > > available on a machine.
> > > > 
> > > > Meh.  We tend to only do that if there is a fundamental reason why
> > > > there can only be one.  But it doesn't hurt.  
> > > 
> > > Well, yeah, would be my initial preference as well.  Maybe Joerg can
> > > give a further explanation to underline why only one SMC can be
> > > found. If we are unsure we still can change it to 'asmc*'.  
> > 
> > In practice there will only be one.  But there is nothing in the ACPI
> > driver to prevent it from supporting multiple SMC chips.
> > 
> > For the ISA driver it made sense to have amsc0 at isa? since there can
> > only be one at the specified address.
> 
> Makes sense to me.  I would finally also prefer to go with 'asmc*', and
> it won't hurt either, right?

Yes. ok kettenis@

> Index: sys/arch/amd64/conf/GENERIC
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
> retrieving revision 1.491
> diff -u -p -u -p -r1.491 GENERIC
> --- sys/arch/amd64/conf/GENERIC   12 Sep 2020 07:47:26 -  1.491
> +++ sys/arch/amd64/conf/GENERIC   12 Sep 2020 10:46:53 -
> @@ -68,7 +68,7 @@ glkgpio*at acpi?
>  sdhc*at acpi?
>  acpicbkbd*   at acpi?
>  acpials* at acpi?
> -asmc0at acpi?# Apple SMC
> +asmc*at acpi?# Apple SMC
>  tpm* at acpi?
>  acpihve* at acpi?
>  acpisurface* at acpi?
> Index: sys/arch/i386/conf/GENERIC
> ===
> RCS file: 

sppp: add free() sizes

2020-09-12 Thread Klemens Nanni
These are the last free(buf, 0) occurences in if_pppoe.c and
if_spppsubr.c changing to non-zero sizes.

I've been running with this the last week without any issues.

Feedback? OK?


Index: if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.186
diff -u -p -r1.186 if_spppsubr.c
--- if_spppsubr.c   22 Aug 2020 16:12:12 -  1.186
+++ if_spppsubr.c   3 Sep 2020 21:43:54 -
@@ -750,13 +750,15 @@ sppp_detach(struct ifnet *ifp)
 
/* release authentication data */
if (sp->myauth.name != NULL)
-   free(sp->myauth.name, M_DEVBUF, 0);
+   free(sp->myauth.name, M_DEVBUF, strlen(sp->myauth.name) + 1);
if (sp->myauth.secret != NULL)
-   free(sp->myauth.secret, M_DEVBUF, 0);
+   free(sp->myauth.secret, M_DEVBUF,
+   strlen(sp->myauth.secret) + 1);
if (sp->hisauth.name != NULL)
-   free(sp->hisauth.name, M_DEVBUF, 0);
+   free(sp->hisauth.name, M_DEVBUF, strlen(sp->hisauth.name) + 1);
if (sp->hisauth.secret != NULL)
-   free(sp->hisauth.secret, M_DEVBUF, 0);
+   free(sp->hisauth.secret, M_DEVBUF,
+   strlen(sp->hisauth.secret) + 1);
 }
 
 /*
@@ -4579,9 +4587,11 @@ sppp_set_params(struct sppp *sp, struct 
if (spa->proto == 0) {
/* resetting auth */
if (auth->name != NULL)
-   free(auth->name, M_DEVBUF, 0);
+   free(auth->name, M_DEVBUF,
+   strlen(auth->name) + 1);
if (auth->secret != NULL)
-   free(auth->secret, M_DEVBUF, 0);
+   free(auth->secret, M_DEVBUF,
+   strlen(auth->secret) + 1);
bzero(auth, sizeof *auth);
explicit_bzero(sp->chap_challenge, sizeof 
sp->chap_challenge);
} else {
@@ -4594,7 +4604,8 @@ sppp_set_params(struct sppp *sp, struct 
p = malloc(len, M_DEVBUF, M_WAITOK);
strlcpy(p, spa->name, len);
if (auth->name != NULL)
-   free(auth->name, M_DEVBUF, 0);
+   free(auth->name, M_DEVBUF,
+   strlen(auth->name) + 1);
auth->name = p;
 
if (spa->secret[0] != '\0') {
@@ -4603,7 +4614,8 @@ sppp_set_params(struct sppp *sp, struct 
p = malloc(len, M_DEVBUF, M_WAITOK);
strlcpy(p, spa->secret, len);
if (auth->secret != NULL)
-   free(auth->secret, M_DEVBUF, 0);
+   free(auth->secret, M_DEVBUF,
+   strlen(auth->secret) + 1);
auth->secret = p;
} else if (!auth->secret) {
p = malloc(1, M_DEVBUF, M_WAITOK);



Re: undwind(8): request for data

2020-09-12 Thread Florian Obser
On Thu, Sep 03, 2020 at 06:13:41PM +0200, Florian Obser wrote:
> The point of this excercise is to work out if it's worthwhile to
> implement RFC 8806 "Running a Root Server Local to a Resolver" in
> unwind(8).
> 
> We are trading latency for bandwidth. Lower latency is almost always a
> win for unwind(8) usecases. But the work if we fetch the root zone is
> not trivial either.
> 
> The zone usually gets updated twice a day and is about 1.2MB. Due to the
> timing parameters unwind(8) would check every 30 minutes with a SOA
> query if a new zone is available:
> $ dig @k.root-servers.net +multiline +noall +answer . soa
> . 86400 IN SOA a.root-servers.net. 
> nstld.verisign-grs.com. (
>   2020090300 ; serial
>   1800   ; refresh (30 minutes)
>   900; retry (15 minutes)
>   604800 ; expire (1 week)
>   86400  ; minimum (1 day)
>   )
> 
> The code complexity should be managable, all the bits and pieces are
> there in libunbound.

Curiously nobody reported unwind(8) doing tcp queries.

The histogram for udp looks like this:

050 3
   50   100 3
  100   150 1
  150   200 1
  200   250 1
  300   350 1
  700   750 1
 1000  1050 1
 1700  1750 1
 1800  1850 1
 2050  2100 1
 5200  5250 1
 9900  9950 1
35050 35100 1

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



Re: acpiapplesmc(4)

2020-09-12 Thread Marcus Glocker
On Sat, 12 Sep 2020 10:28:23 +0200 (CEST)
Mark Kettenis  wrote:

> > Date: Sat, 12 Sep 2020 10:00:13 +0200
> > From: Marcus Glocker 
> > 
> > On Sat, 12 Sep 2020 09:34:18 +0200 (CEST)
> > Mark Kettenis  wrote:
> >   
> > > > Date: Sat, 12 Sep 2020 09:20:26 +0200
> > > > From: Marcus Glocker 
> > > > 
> > > > On Fri, 11 Sep 2020 23:18:56 +0200 (CEST)
> > > > Mark Kettenis  wrote:
> > > > 
> > > > > > Date: Fri, 11 Sep 2020 17:42:23 +0200
> > > > > > From: Marcus Glocker 
> > > > > > 
> > > > > > On Thu, 10 Sep 2020 23:44:38 +0200
> > > > > > Joerg Jung  wrote:
> > > > > >   
> > > > > > > Don’t give up so quickly ;) 
> > > > > > > let’s try to make the driver work on your iMac, send me
> > > > > > > dmesg and sysctl hw output please.
> > > > > > > 
> > > > > > > Your idea of converting it to ACPI is the right thing to
> > > > > > > do anyways, would be nice to get this working.  
> > > > > > 
> > > > > > Here we go:
> > > > > > 
> > > > > > $ dmesg | grep smc
> > > > > > asmc0 at acpi0: SMC_ (smc-piketon) addr 0x300/0x20: rev
> > > > > > 1.64f564, 276 keys
> > > > > > 
> > > > > > $ sysctl -a | grep smc
> > > > > > hw.sensors.asmc0.temp0=27.00 degC (TA0P ambient)
> > > > > > hw.sensors.asmc0.temp1=42.00 degC (TC0H cpu0 heatsink)
> > > > > > hw.sensors.asmc0.temp2=55.00 degC (TG0D gpu0 diode)
> > > > > > hw.sensors.asmc0.temp3=53.00 degC (TG0H gpu0 heatsink)
> > > > > > hw.sensors.asmc0.temp4=38.00 degC (TL0P lcd proximity)
> > > > > > hw.sensors.asmc0.temp5=41.00 degC (TO0P optical drive)
> > > > > > hw.sensors.asmc0.temp6=50.00 degC (Tm0P memory controller)
> > > > > > hw.sensors.asmc0.fan0=998 RPM (ODD, right mid rear)
> > > > > > hw.sensors.asmc0.fan1=1158 RPM (HDD, center mid rear)
> > > > > > hw.sensors.asmc0.fan2=1200 RPM (CPU, left lower rear)
> > > > > > 
> > > > > > Does that work for you guys?  
> > > > > 
> > > > > $ dmesg | grep smc
> > > > > asmc0 at acpi0: SMC_ (smc-napa) addr 0x300/0x20: rev 1.3f503,
> > > > > 137 keys
> > > > > 
> > > > > $ sysctl -a | grep smc
> > > > > hw.sensors.asmc0.temp0=63.00 degC (TC0D cpu0 die core)
> > > > > hw.sensors.asmc0.temp1=55.00 degC (TC0H cpu0 heatsink)
> > > > > hw.sensors.asmc0.temp2=58.00 degC (TC0P cpu0 proximity)
> > > > > hw.sensors.asmc0.temp3=52.00 degC (TN0P northbridge proximity)
> > > > > hw.sensors.asmc0.temp4=52.00 degC (TN1P northbridge 2)
> > > > > hw.sensors.asmc0.fan0=2077 RPM (Master, left upper front)
> > > > > 
> > > > > So yes, this works for me.
> > > > 
> > > > Cool.
> > > >  
> > > > > You'll need to make changes to the i386 GENERIC kernel as
> > > > > well.
> > > > 
> > > > Yep, done.
> > > > 
> > > > > And I'd like to ask you to make one small change...
> > > > 
> > > > > > +const char *acpiapplesmc_hids[] = {  
> > > > > 
> > > > > ...can you rename this variable to asmc_hids[]?
> > > > 
> > > > Of course, copy/pasto, thanks for spotting.
> > > > 
> > > > Also Joerg did suggest to hard code smc0 in GENERIC like it was
> > > > before, since it won't be possible that there is more than 1 SMC
> > > > available on a machine.
> > > 
> > > Meh.  We tend to only do that if there is a fundamental reason why
> > > there can only be one.  But it doesn't hurt.  
> > 
> > Well, yeah, would be my initial preference as well.  Maybe Joerg can
> > give a further explanation to underline why only one SMC can be
> > found. If we are unsure we still can change it to 'asmc*'.  
> 
> In practice there will only be one.  But there is nothing in the ACPI
> driver to prevent it from supporting multiple SMC chips.
> 
> For the ISA driver it made sense to have amsc0 at isa? since there can
> only be one at the specified address.

Makes sense to me.  I would finally also prefer to go with 'asmc*', and
it won't hurt either, right?


Index: sys/arch/amd64/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.491
diff -u -p -u -p -r1.491 GENERIC
--- sys/arch/amd64/conf/GENERIC 12 Sep 2020 07:47:26 -  1.491
+++ sys/arch/amd64/conf/GENERIC 12 Sep 2020 10:46:53 -
@@ -68,7 +68,7 @@ glkgpio*  at acpi?
 sdhc*  at acpi?
 acpicbkbd* at acpi?
 acpials*   at acpi?
-asmc0  at acpi?# Apple SMC
+asmc*  at acpi?# Apple SMC
 tpm*   at acpi?
 acpihve*   at acpi?
 acpisurface*   at acpi?
Index: sys/arch/i386/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/i386/conf/GENERIC,v
retrieving revision 1.852
diff -u -p -u -p -r1.852 GENERIC
--- sys/arch/i386/conf/GENERIC  12 Sep 2020 07:47:26 -  1.852
+++ sys/arch/i386/conf/GENERIC  12 Sep 2020 10:46:54 -
@@ -62,7 +62,7 @@ acpivideo*at acpi?
 acpivout*  at acpivideo?
 acpipwrres*at acpi?
 aibs*  at acpi?
-asmc0  at acpi?# Apple SMC
+asmc*  at acpi?# Apple SMC
 
 option PCIVERBOSE
 

Re: smtpd.conf add admd keyword

2020-09-12 Thread Giovanni Bechis
On 9/6/20 5:06 PM, Martijn van Duren wrote:
> EHLO,
> 
> RFC8601 defines the authentication-results header which can be used to
> show the verification-results of DKIM, SPF, DMARC, and others.
> 
I think it can be a good addition.
ok giovanni@

 Cheers
  Giovanni

> I can think of quite a few filters that could be build around this
> header:
> - the prior mentioned
> - detecting the header before accepting it into ones ADMD
> - using it to calculate some sort of spam-score by some other filter
> 
> These are the 3 main categories that spring to mind, with especially
> the first one having the option to be split in quite a few different
> filters on itself.
> 
> Since setting the authservid on every of these filters (once they
> arrive) will be cumbersome and error-prone I would like to propose to
> distribute this value from a single point in the smtpd.conf.
> 
> I already have a filter-admdscrub basically ready and I'm working on a
> filter-dkimverify every now and then (no where near done yet) which can
> use this feature.
> 
> OK?
> 
> martijn@
> 
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 lka_filter.c
> --- lka_filter.c  24 Apr 2020 11:34:07 -  1.62
> +++ lka_filter.c  6 Sep 2020 15:05:21 -
> @@ -210,6 +210,8 @@ lka_proc_config(struct processor_instanc
>   io_printf(pi->io, "config|subsystem|smtp-in\n");
>   if (pi->subsystems & FILTER_SUBSYSTEM_SMTP_OUT)
>   io_printf(pi->io, "config|subsystem|smtp-out\n");
> + io_printf(pi->io, "config|admd|%s\n",
> + env->sc_admd != NULL ? env->sc_admd : env->sc_hostname);
>   io_printf(pi->io, "config|ready\n");
>  }
>  
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.278
> diff -u -p -r1.278 parse.y
> --- parse.y   1 Jun 2020 05:21:30 -   1.278
> +++ parse.y   6 Sep 2020 15:05:21 -
> @@ -173,7 +173,7 @@ typedef struct {
>  
>  %}
>  
> -%token   ACTION ALIAS ANY ARROW AUTH AUTH_OPTIONAL
> +%token   ACTION ADMD ALIAS ANY ARROW AUTH AUTH_OPTIONAL
>  %token   BACKUP BOUNCE BYPASS
>  %token   CA CERT CHAIN CHROOT CIPHERS COMMIT COMPRESSION CONNECT
>  %token   DATA DATA_LINE DHE DISCONNECT DOMAIN
> @@ -209,6 +209,7 @@ grammar   : /* empty */
>   | grammar include '\n'
>   | grammar varset '\n'
>   | grammar bounce '\n'
> + | grammar admd '\n'
>   | grammar ca '\n'
>   | grammar mda '\n'
>   | grammar mta '\n'
> @@ -310,6 +311,21 @@ BOUNCE WARN_INTERVAL {
>  ;
>  
>  
> +admd:
> +ADMD STRING {
> + size_t i;
> +
> + for (i = 0; $2[i] != '\0'; i++) {
> + if (!isprint($2[i])) {
> + yyerror("not a valid admd");
> + free($2);
> + YYERROR;
> + }
> + }
> + conf->sc_admd = $2;
> +};
> +
> +
>  ca:
>  CA STRING {
>   char buf[HOST_NAME_MAX+1];
> @@ -2603,6 +2619,7 @@ lookup(char *s)
>   /* this has to be sorted always */
>   static const struct keywords keywords[] = {
>   { "action", ACTION },
> + { "admd",   ADMD },
>   { "alias",  ALIAS },
>   { "any",ANY },
>   { "auth",   AUTH },
> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.251
> diff -u -p -r1.251 smtpd.conf.5
> --- smtpd.conf.5  27 Aug 2020 08:58:30 -  1.251
> +++ smtpd.conf.5  6 Sep 2020 15:05:21 -
> @@ -313,6 +313,11 @@ which is useful on machines with multipl
>  If the list contains more than one address, all of them are used
>  in such a way that traffic is routed as efficiently as possible.
>  .El
> +.It Ic admd Ar authservid
> +The Administrative Management Domain this mailserver belongs to.
> +The authservid will be forwarded to filters using it to identify or mark
> +authentication-results headers.
> +If omitted it defaults to the server name.
>  .It Ic bounce Cm warn-interval Ar delay Op , Ar delay ...
>  Send warning messages to the envelope sender when temporary delivery
>  failures cause a message to remain on the queue for longer than
> Index: smtpd.h
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.656
> diff -u -p -r1.656 smtpd.h
> --- smtpd.h   8 Apr 2020 07:30:44 -   1.656
> +++ smtpd.h   6 Sep 2020 15:05:21 -
> @@ -624,6 +624,8 @@ struct smtpd {
>   char   *sc_srs_key;
>   char   *sc_srs_key_backup;
>   int 

Re: smtpd.conf add admd keyword

2020-09-12 Thread Martijn van Duren
Any takers?

On Sun, 2020-09-06 at 17:06 +0200, Martijn van Duren wrote:
> EHLO,
> 
> RFC8601 defines the authentication-results header which can be used to
> show the verification-results of DKIM, SPF, DMARC, and others.
> 
> I can think of quite a few filters that could be build around this
> header:
> - the prior mentioned
> - detecting the header before accepting it into ones ADMD
> - using it to calculate some sort of spam-score by some other filter
> 
> These are the 3 main categories that spring to mind, with especially
> the first one having the option to be split in quite a few different
> filters on itself.
> 
> Since setting the authservid on every of these filters (once they
> arrive) will be cumbersome and error-prone I would like to propose to
> distribute this value from a single point in the smtpd.conf.
> 
> I already have a filter-admdscrub basically ready and I'm working on a
> filter-dkimverify every now and then (no where near done yet) which can
> use this feature.
> 
> OK?
> 
> martijn@
> 
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 lka_filter.c
> --- lka_filter.c  24 Apr 2020 11:34:07 -  1.62
> +++ lka_filter.c  6 Sep 2020 15:05:21 -
> @@ -210,6 +210,8 @@ lka_proc_config(struct processor_instanc
>   io_printf(pi->io, "config|subsystem|smtp-in\n");
>   if (pi->subsystems & FILTER_SUBSYSTEM_SMTP_OUT)
>   io_printf(pi->io, "config|subsystem|smtp-out\n");
> + io_printf(pi->io, "config|admd|%s\n",
> + env->sc_admd != NULL ? env->sc_admd : env->sc_hostname);
>   io_printf(pi->io, "config|ready\n");
>  }
>  
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.278
> diff -u -p -r1.278 parse.y
> --- parse.y   1 Jun 2020 05:21:30 -   1.278
> +++ parse.y   6 Sep 2020 15:05:21 -
> @@ -173,7 +173,7 @@ typedef struct {
>  
>  %}
>  
> -%token   ACTION ALIAS ANY ARROW AUTH AUTH_OPTIONAL
> +%token   ACTION ADMD ALIAS ANY ARROW AUTH AUTH_OPTIONAL
>  %token   BACKUP BOUNCE BYPASS
>  %token   CA CERT CHAIN CHROOT CIPHERS COMMIT COMPRESSION CONNECT
>  %token   DATA DATA_LINE DHE DISCONNECT DOMAIN
> @@ -209,6 +209,7 @@ grammar   : /* empty */
>   | grammar include '\n'
>   | grammar varset '\n'
>   | grammar bounce '\n'
> + | grammar admd '\n'
>   | grammar ca '\n'
>   | grammar mda '\n'
>   | grammar mta '\n'
> @@ -310,6 +311,21 @@ BOUNCE WARN_INTERVAL {
>  ;
>  
>  
> +admd:
> +ADMD STRING {
> + size_t i;
> +
> + for (i = 0; $2[i] != '\0'; i++) {
> + if (!isprint($2[i])) {
> + yyerror("not a valid admd");
> + free($2);
> + YYERROR;
> + }
> + }
> + conf->sc_admd = $2;
> +};
> +
> +
>  ca:
>  CA STRING {
>   char buf[HOST_NAME_MAX+1];
> @@ -2603,6 +2619,7 @@ lookup(char *s)
>   /* this has to be sorted always */
>   static const struct keywords keywords[] = {
>   { "action", ACTION },
> + { "admd",   ADMD },
>   { "alias",  ALIAS },
>   { "any",ANY },
>   { "auth",   AUTH },
> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.251
> diff -u -p -r1.251 smtpd.conf.5
> --- smtpd.conf.5  27 Aug 2020 08:58:30 -  1.251
> +++ smtpd.conf.5  6 Sep 2020 15:05:21 -
> @@ -313,6 +313,11 @@ which is useful on machines with multipl
>  If the list contains more than one address, all of them are used
>  in such a way that traffic is routed as efficiently as possible.
>  .El
> +.It Ic admd Ar authservid
> +The Administrative Management Domain this mailserver belongs to.
> +The authservid will be forwarded to filters using it to identify or mark
> +authentication-results headers.
> +If omitted it defaults to the server name.
>  .It Ic bounce Cm warn-interval Ar delay Op , Ar delay ...
>  Send warning messages to the envelope sender when temporary delivery
>  failures cause a message to remain on the queue for longer than
> Index: smtpd.h
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.656
> diff -u -p -r1.656 smtpd.h
> --- smtpd.h   8 Apr 2020 07:30:44 -   1.656
> +++ smtpd.h   6 Sep 2020 15:05:21 -
> @@ -624,6 +624,8 @@ struct smtpd {
>   char   *sc_srs_key;
>   char   *sc_srs_key_backup;
>   int sc_srs_ttl;
> +
> + 

Re: acpiapplesmc(4)

2020-09-12 Thread Mark Kettenis
> Date: Sat, 12 Sep 2020 10:00:13 +0200
> From: Marcus Glocker 
> 
> On Sat, 12 Sep 2020 09:34:18 +0200 (CEST)
> Mark Kettenis  wrote:
> 
> > > Date: Sat, 12 Sep 2020 09:20:26 +0200
> > > From: Marcus Glocker 
> > > 
> > > On Fri, 11 Sep 2020 23:18:56 +0200 (CEST)
> > > Mark Kettenis  wrote:
> > >   
> > > > > Date: Fri, 11 Sep 2020 17:42:23 +0200
> > > > > From: Marcus Glocker 
> > > > > 
> > > > > On Thu, 10 Sep 2020 23:44:38 +0200
> > > > > Joerg Jung  wrote:
> > > > > 
> > > > > > Don’t give up so quickly ;) 
> > > > > > let’s try to make the driver work on your iMac, send me dmesg
> > > > > > and sysctl hw output please.
> > > > > > 
> > > > > > Your idea of converting it to ACPI is the right thing to do
> > > > > > anyways, would be nice to get this working.
> > > > > 
> > > > > Here we go:
> > > > > 
> > > > > $ dmesg | grep smc
> > > > > asmc0 at acpi0: SMC_ (smc-piketon) addr 0x300/0x20: rev
> > > > > 1.64f564, 276 keys
> > > > > 
> > > > > $ sysctl -a | grep smc
> > > > > hw.sensors.asmc0.temp0=27.00 degC (TA0P ambient)
> > > > > hw.sensors.asmc0.temp1=42.00 degC (TC0H cpu0 heatsink)
> > > > > hw.sensors.asmc0.temp2=55.00 degC (TG0D gpu0 diode)
> > > > > hw.sensors.asmc0.temp3=53.00 degC (TG0H gpu0 heatsink)
> > > > > hw.sensors.asmc0.temp4=38.00 degC (TL0P lcd proximity)
> > > > > hw.sensors.asmc0.temp5=41.00 degC (TO0P optical drive)
> > > > > hw.sensors.asmc0.temp6=50.00 degC (Tm0P memory controller)
> > > > > hw.sensors.asmc0.fan0=998 RPM (ODD, right mid rear)
> > > > > hw.sensors.asmc0.fan1=1158 RPM (HDD, center mid rear)
> > > > > hw.sensors.asmc0.fan2=1200 RPM (CPU, left lower rear)
> > > > > 
> > > > > Does that work for you guys?
> > > > 
> > > > $ dmesg | grep smc
> > > > asmc0 at acpi0: SMC_ (smc-napa) addr 0x300/0x20: rev 1.3f503, 137
> > > > keys
> > > > 
> > > > $ sysctl -a | grep smc
> > > > hw.sensors.asmc0.temp0=63.00 degC (TC0D cpu0 die core)
> > > > hw.sensors.asmc0.temp1=55.00 degC (TC0H cpu0 heatsink)
> > > > hw.sensors.asmc0.temp2=58.00 degC (TC0P cpu0 proximity)
> > > > hw.sensors.asmc0.temp3=52.00 degC (TN0P northbridge proximity)
> > > > hw.sensors.asmc0.temp4=52.00 degC (TN1P northbridge 2)
> > > > hw.sensors.asmc0.fan0=2077 RPM (Master, left upper front)
> > > > 
> > > > So yes, this works for me.  
> > > 
> > > Cool.
> > >
> > > > You'll need to make changes to the i386 GENERIC kernel as well.  
> > > 
> > > Yep, done.
> > >   
> > > > And I'd like to ask you to make one small change...  
> > >   
> > > > > +const char *acpiapplesmc_hids[] = {
> > > > 
> > > > ...can you rename this variable to asmc_hids[]?  
> > > 
> > > Of course, copy/pasto, thanks for spotting.
> > > 
> > > Also Joerg did suggest to hard code smc0 in GENERIC like it was
> > > before, since it won't be possible that there is more than 1 SMC
> > > available on a machine.  
> > 
> > Meh.  We tend to only do that if there is a fundamental reason why
> > there can only be one.  But it doesn't hurt.
> 
> Well, yeah, would be my initial preference as well.  Maybe Joerg can
> give a further explanation to underline why only one SMC can be found.
> If we are unsure we still can change it to 'asmc*'.

In practice there will only be one.  But there is nothing in the ACPI
driver to prevent it from supporting multiple SMC chips.

For the ISA driver it made sense to have amsc0 at isa? since there can
only be one at the specified address.

> > ok kettenis@
> 
> Thanks.
> 



Re: acpiapplesmc(4)

2020-09-12 Thread Marcus Glocker
On Sat, 12 Sep 2020 09:34:18 +0200 (CEST)
Mark Kettenis  wrote:

> > Date: Sat, 12 Sep 2020 09:20:26 +0200
> > From: Marcus Glocker 
> > 
> > On Fri, 11 Sep 2020 23:18:56 +0200 (CEST)
> > Mark Kettenis  wrote:
> >   
> > > > Date: Fri, 11 Sep 2020 17:42:23 +0200
> > > > From: Marcus Glocker 
> > > > 
> > > > On Thu, 10 Sep 2020 23:44:38 +0200
> > > > Joerg Jung  wrote:
> > > > 
> > > > > Don’t give up so quickly ;) 
> > > > > let’s try to make the driver work on your iMac, send me dmesg
> > > > > and sysctl hw output please.
> > > > > 
> > > > > Your idea of converting it to ACPI is the right thing to do
> > > > > anyways, would be nice to get this working.
> > > > 
> > > > Here we go:
> > > > 
> > > > $ dmesg | grep smc
> > > > asmc0 at acpi0: SMC_ (smc-piketon) addr 0x300/0x20: rev
> > > > 1.64f564, 276 keys
> > > > 
> > > > $ sysctl -a | grep smc
> > > > hw.sensors.asmc0.temp0=27.00 degC (TA0P ambient)
> > > > hw.sensors.asmc0.temp1=42.00 degC (TC0H cpu0 heatsink)
> > > > hw.sensors.asmc0.temp2=55.00 degC (TG0D gpu0 diode)
> > > > hw.sensors.asmc0.temp3=53.00 degC (TG0H gpu0 heatsink)
> > > > hw.sensors.asmc0.temp4=38.00 degC (TL0P lcd proximity)
> > > > hw.sensors.asmc0.temp5=41.00 degC (TO0P optical drive)
> > > > hw.sensors.asmc0.temp6=50.00 degC (Tm0P memory controller)
> > > > hw.sensors.asmc0.fan0=998 RPM (ODD, right mid rear)
> > > > hw.sensors.asmc0.fan1=1158 RPM (HDD, center mid rear)
> > > > hw.sensors.asmc0.fan2=1200 RPM (CPU, left lower rear)
> > > > 
> > > > Does that work for you guys?
> > > 
> > > $ dmesg | grep smc
> > > asmc0 at acpi0: SMC_ (smc-napa) addr 0x300/0x20: rev 1.3f503, 137
> > > keys
> > > 
> > > $ sysctl -a | grep smc
> > > hw.sensors.asmc0.temp0=63.00 degC (TC0D cpu0 die core)
> > > hw.sensors.asmc0.temp1=55.00 degC (TC0H cpu0 heatsink)
> > > hw.sensors.asmc0.temp2=58.00 degC (TC0P cpu0 proximity)
> > > hw.sensors.asmc0.temp3=52.00 degC (TN0P northbridge proximity)
> > > hw.sensors.asmc0.temp4=52.00 degC (TN1P northbridge 2)
> > > hw.sensors.asmc0.fan0=2077 RPM (Master, left upper front)
> > > 
> > > So yes, this works for me.  
> > 
> > Cool.
> >
> > > You'll need to make changes to the i386 GENERIC kernel as well.  
> > 
> > Yep, done.
> >   
> > > And I'd like to ask you to make one small change...  
> >   
> > > > +const char *acpiapplesmc_hids[] = {
> > > 
> > > ...can you rename this variable to asmc_hids[]?  
> > 
> > Of course, copy/pasto, thanks for spotting.
> > 
> > Also Joerg did suggest to hard code smc0 in GENERIC like it was
> > before, since it won't be possible that there is more than 1 SMC
> > available on a machine.  
> 
> Meh.  We tend to only do that if there is a fundamental reason why
> there can only be one.  But it doesn't hurt.

Well, yeah, would be my initial preference as well.  Maybe Joerg can
give a further explanation to underline why only one SMC can be found.
If we are unsure we still can change it to 'asmc*'.

> ok kettenis@

Thanks.



Re: acpiapplesmc(4)

2020-09-12 Thread Mark Kettenis
> Date: Sat, 12 Sep 2020 09:20:26 +0200
> From: Marcus Glocker 
> 
> On Fri, 11 Sep 2020 23:18:56 +0200 (CEST)
> Mark Kettenis  wrote:
> 
> > > Date: Fri, 11 Sep 2020 17:42:23 +0200
> > > From: Marcus Glocker 
> > > 
> > > On Thu, 10 Sep 2020 23:44:38 +0200
> > > Joerg Jung  wrote:
> > >   
> > > > Don’t give up so quickly ;) 
> > > > let’s try to make the driver work on your iMac, send me dmesg and
> > > > sysctl hw output please.
> > > > 
> > > > Your idea of converting it to ACPI is the right thing to do
> > > > anyways, would be nice to get this working.  
> > > 
> > > Here we go:
> > > 
> > > $ dmesg | grep smc
> > > asmc0 at acpi0: SMC_ (smc-piketon) addr 0x300/0x20: rev 1.64f564,
> > > 276 keys
> > > 
> > > $ sysctl -a | grep smc
> > > hw.sensors.asmc0.temp0=27.00 degC (TA0P ambient)
> > > hw.sensors.asmc0.temp1=42.00 degC (TC0H cpu0 heatsink)
> > > hw.sensors.asmc0.temp2=55.00 degC (TG0D gpu0 diode)
> > > hw.sensors.asmc0.temp3=53.00 degC (TG0H gpu0 heatsink)
> > > hw.sensors.asmc0.temp4=38.00 degC (TL0P lcd proximity)
> > > hw.sensors.asmc0.temp5=41.00 degC (TO0P optical drive)
> > > hw.sensors.asmc0.temp6=50.00 degC (Tm0P memory controller)
> > > hw.sensors.asmc0.fan0=998 RPM (ODD, right mid rear)
> > > hw.sensors.asmc0.fan1=1158 RPM (HDD, center mid rear)
> > > hw.sensors.asmc0.fan2=1200 RPM (CPU, left lower rear)
> > > 
> > > Does that work for you guys?  
> > 
> > $ dmesg | grep smc
> > asmc0 at acpi0: SMC_ (smc-napa) addr 0x300/0x20: rev 1.3f503, 137 keys
> > 
> > $ sysctl -a | grep smc
> > hw.sensors.asmc0.temp0=63.00 degC (TC0D cpu0 die core)
> > hw.sensors.asmc0.temp1=55.00 degC (TC0H cpu0 heatsink)
> > hw.sensors.asmc0.temp2=58.00 degC (TC0P cpu0 proximity)
> > hw.sensors.asmc0.temp3=52.00 degC (TN0P northbridge proximity)
> > hw.sensors.asmc0.temp4=52.00 degC (TN1P northbridge 2)
> > hw.sensors.asmc0.fan0=2077 RPM (Master, left upper front)
> > 
> > So yes, this works for me.
> 
> Cool.
>  
> > You'll need to make changes to the i386 GENERIC kernel as well.
> 
> Yep, done.
> 
> > And I'd like to ask you to make one small change...
> 
> > > +const char *acpiapplesmc_hids[] = {  
> > 
> > ...can you rename this variable to asmc_hids[]?
> 
> Of course, copy/pasto, thanks for spotting.
> 
> Also Joerg did suggest to hard code smc0 in GENERIC like it was before,
> since it won't be possible that there is more than 1 SMC available on a
> machine.

Meh.  We tend to only do that if there is a fundamental reason why
there can only be one.  But it doesn't hurt.

ok kettenis@

> Index: sys/arch/amd64/conf/GENERIC
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
> retrieving revision 1.490
> diff -u -p -u -p -r1.490 GENERIC
> --- sys/arch/amd64/conf/GENERIC   2 Jun 2020 16:24:24 -   1.490
> +++ sys/arch/amd64/conf/GENERIC   12 Sep 2020 07:17:29 -
> @@ -68,6 +68,7 @@ glkgpio*at acpi?
>  sdhc*at acpi?
>  acpicbkbd*   at acpi?
>  acpials* at acpi?
> +asmc0at acpi?# Apple SMC
>  tpm* at acpi?
>  acpihve* at acpi?
>  acpisurface* at acpi?
> @@ -132,7 +133,6 @@ lm*   at wbsio?
>  uguru0   at isa? disable port 0xe0   # ABIT uGuru
>  
>  aps0 at isa? port 0x1600 # ThinkPad Active Protection System
> -asmc0at isa? port 0x300  # Apple SMC
>  
>  piixpm*  at pci? # Intel PIIX PM
>  iic* at piixpm?
> Index: sys/arch/i386/conf/GENERIC
> ===
> RCS file: /cvs/src/sys/arch/i386/conf/GENERIC,v
> retrieving revision 1.851
> diff -u -p -u -p -r1.851 GENERIC
> --- sys/arch/i386/conf/GENERIC22 Jun 2020 00:33:28 -  1.851
> +++ sys/arch/i386/conf/GENERIC12 Sep 2020 07:17:30 -
> @@ -62,6 +62,7 @@ acpivideo*  at acpi?
>  acpivout*at acpivideo?
>  acpipwrres*  at acpi?
>  aibs*at acpi?
> +asmc0at acpi?# Apple SMC
>  
>  option   PCIVERBOSE
>  option   EISAVERBOSE
> @@ -148,7 +149,6 @@ uguru0at isa? disable port 0xe0   # ABIT 
>  fins0at isa? port 0x4e   # Fintek F71805 Super I/O
>  
>  aps0 at isa? port 0x1600 # ThinkPad Active Protection System
> -asmc0at isa? port 0x300  # Apple SMC
>  
>  itherm*  at pci? # Intel 3400 Thermal Sensor
>  adc* at iic? # Analog Devices AD7416/AD7417/7418
> Index: sys/dev/acpi/asmc.c
> ===
> RCS file: sys/dev/acpi/asmc.c
> diff -N sys/dev/acpi/asmc.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ sys/dev/acpi/asmc.c   12 Sep 2020 07:17:31 -
> @@ -0,0 +1,744 @@
> +/*   $OpenBSD: asmc.c,v 1.33 2020/08/26 03:29:06 visa Exp $  */
> +/*
> + * Copyright (c) 2015 Joerg Jung 
> + *
> + * Permission to use, copy, modify, and distribute this software 

Re: acpiapplesmc(4)

2020-09-12 Thread Marcus Glocker
On Fri, 11 Sep 2020 23:18:56 +0200 (CEST)
Mark Kettenis  wrote:

> > Date: Fri, 11 Sep 2020 17:42:23 +0200
> > From: Marcus Glocker 
> > 
> > On Thu, 10 Sep 2020 23:44:38 +0200
> > Joerg Jung  wrote:
> >   
> > > Don’t give up so quickly ;) 
> > > let’s try to make the driver work on your iMac, send me dmesg and
> > > sysctl hw output please.
> > > 
> > > Your idea of converting it to ACPI is the right thing to do
> > > anyways, would be nice to get this working.  
> > 
> > Here we go:
> > 
> > $ dmesg | grep smc
> > asmc0 at acpi0: SMC_ (smc-piketon) addr 0x300/0x20: rev 1.64f564,
> > 276 keys
> > 
> > $ sysctl -a | grep smc
> > hw.sensors.asmc0.temp0=27.00 degC (TA0P ambient)
> > hw.sensors.asmc0.temp1=42.00 degC (TC0H cpu0 heatsink)
> > hw.sensors.asmc0.temp2=55.00 degC (TG0D gpu0 diode)
> > hw.sensors.asmc0.temp3=53.00 degC (TG0H gpu0 heatsink)
> > hw.sensors.asmc0.temp4=38.00 degC (TL0P lcd proximity)
> > hw.sensors.asmc0.temp5=41.00 degC (TO0P optical drive)
> > hw.sensors.asmc0.temp6=50.00 degC (Tm0P memory controller)
> > hw.sensors.asmc0.fan0=998 RPM (ODD, right mid rear)
> > hw.sensors.asmc0.fan1=1158 RPM (HDD, center mid rear)
> > hw.sensors.asmc0.fan2=1200 RPM (CPU, left lower rear)
> > 
> > Does that work for you guys?  
> 
> $ dmesg | grep smc
> asmc0 at acpi0: SMC_ (smc-napa) addr 0x300/0x20: rev 1.3f503, 137 keys
> 
> $ sysctl -a | grep smc
> hw.sensors.asmc0.temp0=63.00 degC (TC0D cpu0 die core)
> hw.sensors.asmc0.temp1=55.00 degC (TC0H cpu0 heatsink)
> hw.sensors.asmc0.temp2=58.00 degC (TC0P cpu0 proximity)
> hw.sensors.asmc0.temp3=52.00 degC (TN0P northbridge proximity)
> hw.sensors.asmc0.temp4=52.00 degC (TN1P northbridge 2)
> hw.sensors.asmc0.fan0=2077 RPM (Master, left upper front)
> 
> So yes, this works for me.

Cool.
 
> You'll need to make changes to the i386 GENERIC kernel as well.

Yep, done.

> And I'd like to ask you to make one small change...

> > +const char *acpiapplesmc_hids[] = {  
> 
> ...can you rename this variable to asmc_hids[]?

Of course, copy/pasto, thanks for spotting.

Also Joerg did suggest to hard code smc0 in GENERIC like it was before,
since it won't be possible that there is more than 1 SMC available on a
machine.


Index: sys/arch/amd64/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.490
diff -u -p -u -p -r1.490 GENERIC
--- sys/arch/amd64/conf/GENERIC 2 Jun 2020 16:24:24 -   1.490
+++ sys/arch/amd64/conf/GENERIC 12 Sep 2020 07:17:29 -
@@ -68,6 +68,7 @@ glkgpio*  at acpi?
 sdhc*  at acpi?
 acpicbkbd* at acpi?
 acpials*   at acpi?
+asmc0  at acpi?# Apple SMC
 tpm*   at acpi?
 acpihve*   at acpi?
 acpisurface*   at acpi?
@@ -132,7 +133,6 @@ lm* at wbsio?
 uguru0 at isa? disable port 0xe0   # ABIT uGuru
 
 aps0   at isa? port 0x1600 # ThinkPad Active Protection System
-asmc0  at isa? port 0x300  # Apple SMC
 
 piixpm*at pci? # Intel PIIX PM
 iic*   at piixpm?
Index: sys/arch/i386/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/i386/conf/GENERIC,v
retrieving revision 1.851
diff -u -p -u -p -r1.851 GENERIC
--- sys/arch/i386/conf/GENERIC  22 Jun 2020 00:33:28 -  1.851
+++ sys/arch/i386/conf/GENERIC  12 Sep 2020 07:17:30 -
@@ -62,6 +62,7 @@ acpivideo*at acpi?
 acpivout*  at acpivideo?
 acpipwrres*at acpi?
 aibs*  at acpi?
+asmc0  at acpi?# Apple SMC
 
 option PCIVERBOSE
 option EISAVERBOSE
@@ -148,7 +149,6 @@ uguru0  at isa? disable port 0xe0   # ABIT 
 fins0  at isa? port 0x4e   # Fintek F71805 Super I/O
 
 aps0   at isa? port 0x1600 # ThinkPad Active Protection System
-asmc0  at isa? port 0x300  # Apple SMC
 
 itherm*at pci? # Intel 3400 Thermal Sensor
 adc*   at iic? # Analog Devices AD7416/AD7417/7418
Index: sys/dev/acpi/asmc.c
===
RCS file: sys/dev/acpi/asmc.c
diff -N sys/dev/acpi/asmc.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ sys/dev/acpi/asmc.c 12 Sep 2020 07:17:31 -
@@ -0,0 +1,744 @@
+/* $OpenBSD: asmc.c,v 1.33 2020/08/26 03:29:06 visa Exp $  */
+/*
+ * Copyright (c) 2015 Joerg Jung 
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA