Re: Add ioctl for disk cache flush

2016-10-30 Thread Theo de Raadt
> > From: David Gwynne 
> > Date: Mon, 31 Oct 2016 10:38:49 +1000
> > 
> > > On 31 Oct 2016, at 09:18, Stefan Fritsch  wrote:
> > > 
> > > On Sun, 30 Oct 2016, Stefan Fritsch wrote:
> > >> I agree with all your comments (and should have reviewed the initial 
> > >> patch 
> > >> better). Here is an updated diff.
> > > 
> > > The FWRITE check was missing in wdioctl(). Next try:
> > 
> > the diff looks fine to me.
> > 
> > whats the use of the ioctl though? why do we need extra sync cache things?
> 
> Right.  This smells like papering over real bugs.

Here's the thoughts I have exchanged with sf.

This issue should not be per filesystem, not even per partition.  It
seems this should keep track of when any partition converts from RW ->
R access (either via direct open, or via mount), and if so schedule
the required sync operation (to occur sometime soon).  That way if
many such RW->R transitions occur, they won't serialize and create a
performance hit.

Furthermore I don't believe userland should have access to this.
We've made it 30 years without needing this, why do we need it now.



Re: Add ioctl for disk cache flush

2016-10-30 Thread Mark Kettenis
> From: David Gwynne 
> Date: Mon, 31 Oct 2016 10:38:49 +1000
> 
> > On 31 Oct 2016, at 09:18, Stefan Fritsch  wrote:
> > 
> > On Sun, 30 Oct 2016, Stefan Fritsch wrote:
> >> I agree with all your comments (and should have reviewed the initial patch 
> >> better). Here is an updated diff.
> > 
> > The FWRITE check was missing in wdioctl(). Next try:
> 
> the diff looks fine to me.
> 
> whats the use of the ioctl though? why do we need extra sync cache things?

Right.  This smells like papering over real bugs.



Re: Add ioctl for disk cache flush

2016-10-30 Thread David Gwynne

> On 31 Oct 2016, at 09:18, Stefan Fritsch  wrote:
> 
> On Sun, 30 Oct 2016, Stefan Fritsch wrote:
>> I agree with all your comments (and should have reviewed the initial patch 
>> better). Here is an updated diff.
> 
> The FWRITE check was missing in wdioctl(). Next try:

the diff looks fine to me.

whats the use of the ioctl though? why do we need extra sync cache things?

dlg

> 
> diff --git sys/dev/ata/wd.c sys/dev/ata/wd.c
> index 5e2461f..ca5ac90 100644
> --- sys/dev/ata/wd.c
> +++ sys/dev/ata/wd.c
> @@ -26,7 +26,7 @@
>  */
> 
> /*-
> - * Copyright (c) 1998 The NetBSD Foundation, Inc.
> + * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
>  * All rights reserved.
>  *
>  * This code is derived from software contributed to The NetBSD Foundation
> @@ -138,7 +138,7 @@ void  wdstart(void *);
> void  __wdstart(struct wd_softc*, struct buf *);
> void  wdrestart(void *);
> int   wd_get_params(struct wd_softc *, u_int8_t, struct ataparams *);
> -void  wd_flushcache(struct wd_softc *, int);
> +int   wd_flushcache(struct wd_softc *, int);
> void  wd_standby(struct wd_softc *, int);
> 
> /* XXX: these should go elsewhere */
> @@ -848,6 +848,14 @@ wdioctl(dev_t dev, u_long xfer, caddr_t addr, int flag, 
> struct proc *p)
>   }
> #endif
> 
> + case DIOCCACHESYNC:
> + if ((flag & FWRITE) == 0) {
> + error = EBADF;
> + goto exit;
> + }
> + error = wd_flushcache(wd, AT_WAIT);
> + goto exit;
> +
>   default:
>   error = wdc_ioctl(wd->drvp, xfer, addr, flag, p);
>   goto exit;
> @@ -1067,13 +1075,13 @@ wd_get_params(struct wd_softc *wd, u_int8_t flags, 
> struct ataparams *params)
>   }
> }
> 
> -void
> +int
> wd_flushcache(struct wd_softc *wd, int flags)
> {
>   struct wdc_command wdc_c;
> 
>   if (wd->drvp->ata_vers < 4) /* WDCC_FLUSHCACHE is here since ATA-4 */
> - return;
> + return EIO;
>   bzero(&wdc_c, sizeof(struct wdc_command));
>   wdc_c.r_command = (wd->sc_flags & WDF_LBA48 ? WDCC_FLUSHCACHE_EXT :
>   WDCC_FLUSHCACHE);
> @@ -1088,20 +1096,28 @@ wd_flushcache(struct wd_softc *wd, int flags)
>   if (wdc_exec_command(wd->drvp, &wdc_c) != WDC_COMPLETE) {
>   printf("%s: flush cache command didn't complete\n",
>   wd->sc_dev.dv_xname);
> + return EIO;
>   }
> + if (wdc_c.flags & ERR_NODEV)
> + return ENODEV;
>   if (wdc_c.flags & AT_TIMEOU) {
>   printf("%s: flush cache command timeout\n",
>   wd->sc_dev.dv_xname);
> + return EIO;
> + }
> + if (wdc_c.flags & AT_ERROR) {
> + if (wdc_c.r_error == WDCE_ABRT) /* command not supported */
> + return ENODEV;
> + printf("%s: flush cache command: error 0x%x\n",
> + wd->sc_dev.dv_xname, wdc_c.r_error);
> + return EIO;
>   }
>   if (wdc_c.flags & AT_DF) {
>   printf("%s: flush cache command: drive fault\n",
>   wd->sc_dev.dv_xname);
> + return EIO;
>   }
> - /*
> -  * Ignore error register, it shouldn't report anything else
> -  * than COMMAND ABORTED, which means the device doesn't support
> -  * flush cache
> -  */
> + return 0;
> }
> 
> void
> diff --git sys/scsi/sd.c sys/scsi/sd.c
> index 30fb36b..e5a0139 100644
> --- sys/scsi/sd.c
> +++ sys/scsi/sd.c
> @@ -2,7 +2,7 @@
> /*$NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $ */
> 
> /*-
> - * Copyright (c) 1998 The NetBSD Foundation, Inc.
> + * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
>  * All rights reserved.
>  *
>  * This code is derived from software contributed to The NetBSD Foundation
> @@ -96,7 +96,7 @@ int sd_vpd_block_limits(struct sd_softc *, int);
> int   sd_vpd_thin(struct sd_softc *, int);
> int   sd_thin_params(struct sd_softc *, int);
> int   sd_get_parms(struct sd_softc *, struct disk_parms *, int);
> -void sd_flush(struct sd_softc *, int);
> +int  sd_flush(struct sd_softc *, int);
> 
> void  viscpy(u_char *, u_char *, int);
> 
> @@ -999,6 +999,15 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, 
> struct proc *p)
>   error = sd_ioctl_cache(sc, cmd, (struct dk_cache *)addr);
>   goto exit;
> 
> + case DIOCCACHESYNC:
> + if (!ISSET(flag, FWRITE)) {
> + error = EBADF;
> + goto exit;
> + }
> + if ((sc->flags & SDF_DIRTY) != 0 || *(int *)addr != 0)
> + error = sd_flush(sc, 0);
> + goto exit;
> +
>   default:
>   if (part != RAW_PART) {
>   error = ENOTTY;
> @@ -1876,19 +1885,20 @@ die:
>   return (SDGP_RESULT_OFFLINE);
> }
> 
> -void
> +int
> sd_flush(struct sd_softc *sc, int flags)
> {
>   struct scsi_link *link;
>   struct scsi_xfer *xs;
>

Re: Add ioctl for disk cache flush

2016-10-30 Thread Stefan Fritsch
On Sun, 30 Oct 2016, Stefan Fritsch wrote:
> I agree with all your comments (and should have reviewed the initial patch 
> better). Here is an updated diff.

The FWRITE check was missing in wdioctl(). Next try:

diff --git sys/dev/ata/wd.c sys/dev/ata/wd.c
index 5e2461f..ca5ac90 100644
--- sys/dev/ata/wd.c
+++ sys/dev/ata/wd.c
@@ -26,7 +26,7 @@
  */
 
 /*-
- * Copyright (c) 1998 The NetBSD Foundation, Inc.
+ * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -138,7 +138,7 @@ void  wdstart(void *);
 void  __wdstart(struct wd_softc*, struct buf *);
 void  wdrestart(void *);
 int   wd_get_params(struct wd_softc *, u_int8_t, struct ataparams *);
-void  wd_flushcache(struct wd_softc *, int);
+int   wd_flushcache(struct wd_softc *, int);
 void  wd_standby(struct wd_softc *, int);
 
 /* XXX: these should go elsewhere */
@@ -848,6 +848,14 @@ wdioctl(dev_t dev, u_long xfer, caddr_t addr, int flag, 
struct proc *p)
}
 #endif
 
+   case DIOCCACHESYNC:
+   if ((flag & FWRITE) == 0) {
+   error = EBADF;
+   goto exit;
+   }
+   error = wd_flushcache(wd, AT_WAIT);
+   goto exit;
+
default:
error = wdc_ioctl(wd->drvp, xfer, addr, flag, p);
goto exit;
@@ -1067,13 +1075,13 @@ wd_get_params(struct wd_softc *wd, u_int8_t flags, 
struct ataparams *params)
}
 }
 
-void
+int
 wd_flushcache(struct wd_softc *wd, int flags)
 {
struct wdc_command wdc_c;
 
if (wd->drvp->ata_vers < 4) /* WDCC_FLUSHCACHE is here since ATA-4 */
-   return;
+   return EIO;
bzero(&wdc_c, sizeof(struct wdc_command));
wdc_c.r_command = (wd->sc_flags & WDF_LBA48 ? WDCC_FLUSHCACHE_EXT :
WDCC_FLUSHCACHE);
@@ -1088,20 +1096,28 @@ wd_flushcache(struct wd_softc *wd, int flags)
if (wdc_exec_command(wd->drvp, &wdc_c) != WDC_COMPLETE) {
printf("%s: flush cache command didn't complete\n",
wd->sc_dev.dv_xname);
+   return EIO;
}
+   if (wdc_c.flags & ERR_NODEV)
+   return ENODEV;
if (wdc_c.flags & AT_TIMEOU) {
printf("%s: flush cache command timeout\n",
wd->sc_dev.dv_xname);
+   return EIO;
+   }
+   if (wdc_c.flags & AT_ERROR) {
+   if (wdc_c.r_error == WDCE_ABRT) /* command not supported */
+   return ENODEV;
+   printf("%s: flush cache command: error 0x%x\n",
+   wd->sc_dev.dv_xname, wdc_c.r_error);
+   return EIO;
}
if (wdc_c.flags & AT_DF) {
printf("%s: flush cache command: drive fault\n",
wd->sc_dev.dv_xname);
+   return EIO;
}
-   /*
-* Ignore error register, it shouldn't report anything else
-* than COMMAND ABORTED, which means the device doesn't support
-* flush cache
-*/
+   return 0;
 }
 
 void
diff --git sys/scsi/sd.c sys/scsi/sd.c
index 30fb36b..e5a0139 100644
--- sys/scsi/sd.c
+++ sys/scsi/sd.c
@@ -2,7 +2,7 @@
 /* $NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $ */
 
 /*-
- * Copyright (c) 1998 The NetBSD Foundation, Inc.
+ * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -96,7 +96,7 @@ int   sd_vpd_block_limits(struct sd_softc *, int);
 intsd_vpd_thin(struct sd_softc *, int);
 intsd_thin_params(struct sd_softc *, int);
 intsd_get_parms(struct sd_softc *, struct disk_parms *, int);
-void   sd_flush(struct sd_softc *, int);
+intsd_flush(struct sd_softc *, int);
 
 void   viscpy(u_char *, u_char *, int);
 
@@ -999,6 +999,15 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, 
struct proc *p)
error = sd_ioctl_cache(sc, cmd, (struct dk_cache *)addr);
goto exit;
 
+   case DIOCCACHESYNC:
+   if (!ISSET(flag, FWRITE)) {
+   error = EBADF;
+   goto exit;
+   }
+   if ((sc->flags & SDF_DIRTY) != 0 || *(int *)addr != 0)
+   error = sd_flush(sc, 0);
+   goto exit;
+
default:
if (part != RAW_PART) {
error = ENOTTY;
@@ -1876,19 +1885,20 @@ die:
return (SDGP_RESULT_OFFLINE);
 }
 
-void
+int
 sd_flush(struct sd_softc *sc, int flags)
 {
struct scsi_link *link;
struct scsi_xfer *xs;
struct scsi_synchronize_cache *cmd;
+   int error;
 
if (sc->flags & SDF_DYING)
-   return;
+   return (ENXIO);
link = sc->sc_link;
 
if (link->quirks & SDEV_NOSYNCCACHE)
-   return;
+   r

Re: acpivout(4): brightness adjustment quirk

2016-10-30 Thread Michel Behr
Not sure if it applies to your Dell machine (and probably too obvious), but
on my Lenovo lap-top the brightness hotkeys only worked after adjusting a
BIOS setting to enable them.

On Sun, Oct 30, 2016 at 12:48 PM, Anton Lindqvist  wrote:

> I'm trying to fix the brightness hotkeys on my Dell Latitude 3160.
> Adjusting the brightness using wsconsctl(1) works, as opposed of the
> keys. Turns out it's not possible to move the next or previous BCL level
> relative to the current one on this particular machine. Instead four
> steps is the minimum required in order for the brightness to actually
> change. Attached is a patch which finds the number of steps needed in
> order to get the brightness to change. Any feedback would be
> appreciated.
>
> Index: acpivout.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpivout.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 acpivout.c
> --- acpivout.c  29 Mar 2016 17:52:04 -  1.12
> +++ acpivout.c  30 Oct 2016 14:05:52 -
> @@ -57,6 +57,7 @@ struct acpivout_softc {
>
> int *sc_bcl;
> size_t  sc_bcl_len;
> +   size_t  sc_bcl_step;
>  };
>
>  void   acpivout_brightness_cycle(struct acpivout_softc *);
> @@ -178,8 +179,11 @@ acpivout_brightness_up(struct acpivout_s
> if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1])
> return;
>
> -   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++);
> -   acpivout_set_brightness(sc, sc->sc_bcl[i + 1]);
> +   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++)
> +   continue;
> +   KASSERT(i < sc->sc_bcl_len - 1);
> +   i += i + sc->sc_bcl_step < sc->sc_bcl_len ? sc->sc_bcl_step : 1;
> +   acpivout_set_brightness(sc, sc->sc_bcl[i]);
>  }
>
>  void
> @@ -197,8 +201,11 @@ acpivout_brightness_down(struct acpivout
> if (cur_level == sc->sc_bcl[0])
> return;
>
> -   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++);
> -   acpivout_set_brightness(sc, sc->sc_bcl[i - 1]);
> +   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++)
> +   continue;
> +   KASSERT(i > 0);
> +   i -= i > sc->sc_bcl_step ? sc->sc_bcl_step : 1;
> +   acpivout_set_brightness(sc, sc->sc_bcl[i]);
>  }
>
>  void
> @@ -262,8 +269,8 @@ acpivout_set_brightness(struct acpivout_
>  void
>  acpivout_get_bcl(struct acpivout_softc *sc)
>  {
> -   int i, j, value;
> -   struct aml_value res;
> +   struct aml_valueres;
> +   int cur_level, i, inc, j, value;
>
> DPRINTF(("Getting _BCL!"));
> aml_evalname(sc->sc_acpi, sc->sc_devnode, "_BCL", 0, NULL, &res);
> @@ -295,6 +302,30 @@ acpivout_get_bcl(struct acpivout_softc *
> sc->sc_bcl[j] = sc->sc_bcl[j - 1];
>  sc->sc_bcl[j] = value;
> }
> +
> +   /*
> +* Some implementations doesn't change the brightness when moving
> to the
> +* next or previous BCL value relative to the current one.
> Calculate the
> +* step needed by incrementally adjusting the brightness.
> +*/
> +   cur_level = acpivout_get_brightness(sc);
> +   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++)
> +   continue;
> +   inc = i < sc->sc_bcl_len/2 ? 1 : -1;
> +   sc->sc_bcl_step = 1;
> +   for (;;) {
> +   i += inc;
> +   if (i < 0 || i >= sc->sc_bcl_len)
> +   break;
> +
> +   acpivout_set_brightness(sc, sc->sc_bcl[i]);
> +   if (acpivout_get_brightness(sc) != cur_level)
> +   break;
> +
> +   sc->sc_bcl_step++;
> +   }
> +   /* Restore brightness */
> +   acpivout_set_brightness(sc, cur_level);
>
>  err:
> aml_freevalue(&res);
>
>


Re: step missing from "2016/10/14" followig -current?

2016-10-30 Thread Jeremie Courreges-Anglas
Peter Hessler  writes:

> On 2016 Oct 30 (Sun) at 12:28:36 +0100 (+0100), Ingo Schwarze wrote:
> :> Everybody would have had, had it been included in /etc/skel/.cvsrc :^)
> :> Worth adding?
> :
> :I (mildly) oppose that.  Providing customization files by default
> :is an oxymoron that provokes exactly the problems seen here.  People
> :become so used to it that communication breaks down - some people
> :assume the default customization to be the default, others assume
> :the defaults to be the defaults.  These default customization files
> :are also undocumented, so they cause default behaviour of utilities
> :to differ from what is described in the manpages.  At least we should
> :not pile more poo on top of the turd.
> :
> :We want sane defaults, and we want simple defaults.  Where traditional
> :defaults are suboptimal but can't easily be changed, i still think
> :that having one set of defaults is saner and simpler than having
> :two sets of defaults:  One traditional, documented plus one arguably
> :more convenient, but undocumented set...
> :
> :Whatever, i expect others will disagree, so i'm unlikely to argue
> :this further, and i can certainly live with whatever happens in
> :/etc/skel/, i never use it anyway.
> :
> :Yours,
> :  Ingo
>
> On one hand, I 100% agree with Ingo.  Our instructions MUST NOT assume
> configuration files / environment have things we depend on.
>
> On the other hand, if we do provide environment files, we MUST ensure
> they are sane.  adding -d to cvs update is pretty much required when
> using OpenBSD's cvs tree.

Agreed.

>
> :> Index: etc/skel/dot.cvsrc
> :> ===
> :> RCS file: /cvs/src/etc/skel/dot.cvsrc,v
> :> retrieving revision 1.2
> :> diff -u -p -r1.2 dot.cvsrc
> :> --- etc/skel/dot.cvsrc 31 Mar 2015 18:34:40 -  1.2
> :> +++ etc/skel/dot.cvsrc 29 Oct 2016 17:44:30 -
> :> @@ -1,6 +1,6 @@
> :>  # $OpenBSD: dot.cvsrc,v 1.2 2015/03/31 18:34:40 naddy Exp $
> :>  #
> :>  diff -uNp
> :> -update -P
> :> +update -Pd
> :>  checkout -P
> :>  rdiff -u
> :
>
> OK

Also ok jca@

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



Re: update net/py-pcapy

2016-10-30 Thread Jeremie Courreges-Anglas
Peter Hessler  writes:

> On 2016 Oct 30 (Sun) at 15:17:34 +0100 (+0100), Jeremie Courreges-Anglas 
> wrote:
> :Peter Hessler  writes:
> :
> :> I would like to use pcapy in a python3 module, so we need to update it.
> :>
> :> However, when I run the test program, I get the following error:
> :>
> :> python2.7:/usr/local/lib/python2.7/site-packages/pcapy.so: undefined
> :> symbol '_Z10bpf_filterPK8bpf_insnPKhjj'
> :> Traceback (most recent call last):
> :>   File "/usr/local/tests/pcapytests.py", line 8, in 
> :> import pcapy
> :> ImportError: Cannot load specified object
> :>
> :> Based on what I can tell, pcapy.so is linked against libpcap, which
> :> should provide those symbols.
> :>
> :> $ ldd /usr/local/lib/python2.7/site-packages/pcapy.so
> :> /usr/local/lib/python2.7/site-packages/pcapy.so:
> :> StartEnd  Type Open Ref GrpRef Name
> :> 10ffb9019000 10ffb921f000 dlib 20   0  
> /usr/local/lib/python2.7/site-packages/pcapy.so
> :> 10ff88f9c000 10ff891c1000 rlib 01   0  
> /usr/lib/libpcap.so.8.1
> :> 10ff5d7d6000 10ff5daed000 rlib 01   0  
> /usr/lib/libstdc++.so.57.0
> :> 10ff9925f000 10ff99636000 rlib 01   0  
> /usr/local/lib/libpython2.7.so.0.0
> :> 10fee0ee9000 10fee111 rlib 03   0  
> /usr/lib/libm.so.10.0
> :> 10ff63fa3000 10ff641b2000 rlib 01   0  
> /usr/lib/libpthread.so.23.0
> :> 10fef2919000 10fef2b26000 rlib 01   0  
> /usr/lib/libutil.so.12.1
> :>
> :> Ideas?
> :
> :C++ name mangling is affecting the functions declared in net/bpf.h.
> :Probably a __BEGIN_DECLS / __END_DECLS pair missing.
> :
> :-- 
> :jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> :
>
> Yup, that was it.
>
> How risky is that?  Should this be tested in a bulk?

The potential for breakage seems low to me.  A failure case I can think
of: some c++ port now correctly detects the function, activates some
code, but this code fails to build for whatever reason.

> (moved to tech@) OK?

ok jca@

I wonder whether _bpf_filter should be exposed at all.

>
> Index: sys/net/bpf.h
> ===
> RCS file: /cvs/openbsd/src/sys/net/bpf.h,v
> retrieving revision 1.58
> diff -u -p -u -p -r1.58 bpf.h
> --- sys/net/bpf.h 12 Sep 2016 16:24:37 -  1.58
> +++ sys/net/bpf.h 30 Oct 2016 16:21:32 -
> @@ -276,11 +276,13 @@ struct bpf_ops {
>  #define BPF_STMT(code, k) { (u_int16_t)(code), 0, 0, k }
>  #define BPF_JUMP(code, k, jt, jf) { (u_int16_t)(code), jt, jf, k }
>  
> +__BEGIN_DECLS
>  u_int bpf_filter(const struct bpf_insn *, const u_char *, u_int, 
> u_int)
>   __bounded((__buffer__, 2, 4));
>  
>  u_int _bpf_filter(const struct bpf_insn *, const struct bpf_ops *,
>const void *, u_int);
> +__END_DECLS
>  
>  #ifdef _KERNEL
>  struct ifnet;

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



Re: update net/py-pcapy

2016-10-30 Thread Peter Hessler
On 2016 Oct 30 (Sun) at 15:17:34 +0100 (+0100), Jeremie Courreges-Anglas wrote:
:Peter Hessler  writes:
:
:> I would like to use pcapy in a python3 module, so we need to update it.
:>
:> However, when I run the test program, I get the following error:
:>
:> python2.7:/usr/local/lib/python2.7/site-packages/pcapy.so: undefined
:> symbol '_Z10bpf_filterPK8bpf_insnPKhjj'
:> Traceback (most recent call last):
:>   File "/usr/local/tests/pcapytests.py", line 8, in 
:> import pcapy
:> ImportError: Cannot load specified object
:>
:> Based on what I can tell, pcapy.so is linked against libpcap, which
:> should provide those symbols.
:>
:> $ ldd /usr/local/lib/python2.7/site-packages/pcapy.so
:> /usr/local/lib/python2.7/site-packages/pcapy.so:
:> StartEnd  Type Open Ref GrpRef Name
:> 10ffb9019000 10ffb921f000 dlib 20   0  
/usr/local/lib/python2.7/site-packages/pcapy.so
:> 10ff88f9c000 10ff891c1000 rlib 01   0  
/usr/lib/libpcap.so.8.1
:> 10ff5d7d6000 10ff5daed000 rlib 01   0  
/usr/lib/libstdc++.so.57.0
:> 10ff9925f000 10ff99636000 rlib 01   0  
/usr/local/lib/libpython2.7.so.0.0
:> 10fee0ee9000 10fee111 rlib 03   0  
/usr/lib/libm.so.10.0
:> 10ff63fa3000 10ff641b2000 rlib 01   0  
/usr/lib/libpthread.so.23.0
:> 10fef2919000 10fef2b26000 rlib 01   0  
/usr/lib/libutil.so.12.1
:>
:> Ideas?
:
:C++ name mangling is affecting the functions declared in net/bpf.h.
:Probably a __BEGIN_DECLS / __END_DECLS pair missing.
:
:-- 
:jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
:

Yup, that was it.

How risky is that?  Should this be tested in a bulk?

(moved to tech@) OK?


Index: sys/net/bpf.h
===
RCS file: /cvs/openbsd/src/sys/net/bpf.h,v
retrieving revision 1.58
diff -u -p -u -p -r1.58 bpf.h
--- sys/net/bpf.h   12 Sep 2016 16:24:37 -  1.58
+++ sys/net/bpf.h   30 Oct 2016 16:21:32 -
@@ -276,11 +276,13 @@ struct bpf_ops {
 #define BPF_STMT(code, k) { (u_int16_t)(code), 0, 0, k }
 #define BPF_JUMP(code, k, jt, jf) { (u_int16_t)(code), jt, jf, k }
 
+__BEGIN_DECLS
 u_int   bpf_filter(const struct bpf_insn *, const u_char *, u_int, u_int)
__bounded((__buffer__, 2, 4));
 
 u_int   _bpf_filter(const struct bpf_insn *, const struct bpf_ops *,
 const void *, u_int);
+__END_DECLS
 
 #ifdef _KERNEL
 struct ifnet;




-- 
Every program has two purposes -- one for which it was written and
another for which it wasn't.



acpivout(4): brightness adjustment quirk

2016-10-30 Thread Anton Lindqvist
I'm trying to fix the brightness hotkeys on my Dell Latitude 3160.
Adjusting the brightness using wsconsctl(1) works, as opposed of the
keys. Turns out it's not possible to move the next or previous BCL level
relative to the current one on this particular machine. Instead four
steps is the minimum required in order for the brightness to actually
change. Attached is a patch which finds the number of steps needed in
order to get the brightness to change. Any feedback would be
appreciated.

Index: acpivout.c
===
RCS file: /cvs/src/sys/dev/acpi/acpivout.c,v
retrieving revision 1.12
diff -u -p -r1.12 acpivout.c
--- acpivout.c  29 Mar 2016 17:52:04 -  1.12
+++ acpivout.c  30 Oct 2016 14:05:52 -
@@ -57,6 +57,7 @@ struct acpivout_softc {
 
int *sc_bcl;
size_t  sc_bcl_len;
+   size_t  sc_bcl_step;
 };
 
 void   acpivout_brightness_cycle(struct acpivout_softc *);
@@ -178,8 +179,11 @@ acpivout_brightness_up(struct acpivout_s
if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1])
return;
 
-   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++);
-   acpivout_set_brightness(sc, sc->sc_bcl[i + 1]);
+   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++)
+   continue;
+   KASSERT(i < sc->sc_bcl_len - 1);
+   i += i + sc->sc_bcl_step < sc->sc_bcl_len ? sc->sc_bcl_step : 1;
+   acpivout_set_brightness(sc, sc->sc_bcl[i]);
 }
 
 void
@@ -197,8 +201,11 @@ acpivout_brightness_down(struct acpivout
if (cur_level == sc->sc_bcl[0])
return;
 
-   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++);
-   acpivout_set_brightness(sc, sc->sc_bcl[i - 1]);
+   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++)
+   continue;
+   KASSERT(i > 0);
+   i -= i > sc->sc_bcl_step ? sc->sc_bcl_step : 1;
+   acpivout_set_brightness(sc, sc->sc_bcl[i]);
 }
 
 void
@@ -262,8 +269,8 @@ acpivout_set_brightness(struct acpivout_
 void
 acpivout_get_bcl(struct acpivout_softc *sc)
 {
-   int i, j, value;
-   struct aml_value res;
+   struct aml_valueres;
+   int cur_level, i, inc, j, value;
 
DPRINTF(("Getting _BCL!"));
aml_evalname(sc->sc_acpi, sc->sc_devnode, "_BCL", 0, NULL, &res);
@@ -295,6 +302,30 @@ acpivout_get_bcl(struct acpivout_softc *
sc->sc_bcl[j] = sc->sc_bcl[j - 1];
 sc->sc_bcl[j] = value;
}
+
+   /*
+* Some implementations doesn't change the brightness when moving to the
+* next or previous BCL value relative to the current one. Calculate the
+* step needed by incrementally adjusting the brightness.
+*/
+   cur_level = acpivout_get_brightness(sc);
+   for (i = 0; i < sc->sc_bcl_len && cur_level != sc->sc_bcl[i]; i++)
+   continue;
+   inc = i < sc->sc_bcl_len/2 ? 1 : -1;
+   sc->sc_bcl_step = 1;
+   for (;;) {
+   i += inc;
+   if (i < 0 || i >= sc->sc_bcl_len)
+   break;
+
+   acpivout_set_brightness(sc, sc->sc_bcl[i]);
+   if (acpivout_get_brightness(sc) != cur_level)
+   break;
+
+   sc->sc_bcl_step++;
+   }
+   /* Restore brightness */
+   acpivout_set_brightness(sc, cur_level);
 
 err:
aml_freevalue(&res);



Re: step missing from "2016/10/14" followig -current?

2016-10-30 Thread Peter Hessler
On 2016 Oct 30 (Sun) at 12:28:36 +0100 (+0100), Ingo Schwarze wrote:
:> Everybody would have had, had it been included in /etc/skel/.cvsrc :^)
:> Worth adding?
:
:I (mildly) oppose that.  Providing customization files by default
:is an oxymoron that provokes exactly the problems seen here.  People
:become so used to it that communication breaks down - some people
:assume the default customization to be the default, others assume
:the defaults to be the defaults.  These default customization files
:are also undocumented, so they cause default behaviour of utilities
:to differ from what is described in the manpages.  At least we should
:not pile more poo on top of the turd.
:
:We want sane defaults, and we want simple defaults.  Where traditional
:defaults are suboptimal but can't easily be changed, i still think
:that having one set of defaults is saner and simpler than having
:two sets of defaults:  One traditional, documented plus one arguably
:more convenient, but undocumented set...
:
:Whatever, i expect others will disagree, so i'm unlikely to argue
:this further, and i can certainly live with whatever happens in
:/etc/skel/, i never use it anyway.
:
:Yours,
:  Ingo

On one hand, I 100% agree with Ingo.  Our instructions MUST NOT assume
configuration files / environment have things we depend on.

On the other hand, if we do provide environment files, we MUST ensure
they are sane.  adding -d to cvs update is pretty much required when
using OpenBSD's cvs tree.


:> Index: etc/skel/dot.cvsrc
:> ===
:> RCS file: /cvs/src/etc/skel/dot.cvsrc,v
:> retrieving revision 1.2
:> diff -u -p -r1.2 dot.cvsrc
:> --- etc/skel/dot.cvsrc   31 Mar 2015 18:34:40 -  1.2
:> +++ etc/skel/dot.cvsrc   29 Oct 2016 17:44:30 -
:> @@ -1,6 +1,6 @@
:>  # $OpenBSD: dot.cvsrc,v 1.2 2015/03/31 18:34:40 naddy Exp $
:>  #
:>  diff -uNp
:> -update -P
:> +update -Pd
:>  checkout -P
:>  rdiff -u
:

OK


-- 
Man 1:  Ask me what the most important thing about telling a good
joke is.

Man 2:  OK, what is the most impo --

Man 1:  TIMING!



Re: step missing from "2016/10/14" followig -current?

2016-10-30 Thread Ingo Schwarze
Hi,

Raf Czlonka wrote on Sat, Oct 29, 2016 at 06:46:56PM +0100:
> On Sat, Oct 29, 2016 at 08:39:47AM BST, Theo Buehler wrote:

>> I suppose everybody has 'update -Pd' in their ~/.cvsrc

  schwarze@isnote $ cat ~/.cvsrc ~/.Xdefaults ~/.login ~/.mailrc ~/.profile
  cat: /home/schwarze/.cvsrc: No such file or directory
  cat: /home/schwarze/.Xdefaults: No such file or directory
  cat: /home/schwarze/.login: No such file or directory
  cat: /home/schwarze/.mailrc: No such file or directory
  cat: /home/schwarze/.profile: No such file or directory

I'm probably repeating myself, but there are people who avoid as
much customization as they can, because customizing stuff makes
it harder to maintain systems and to work on unfamiliar systems.
In my personal opinion, saved keystrokes are not worth the above
downsides, finger memory does the trick just fine.  (I know that
many disagree, of course - no problem with that.  I don't say that
support for customization files should be deleted.)

>> or automatically knew to give the -d flag to create directories...
>> 'cvs up' should probably be expanded to the full 'cvs -q up -Pd'
>> on that page.  I'll fix it later when I'm on a good machine if
>> nobody beats me to it.

Thanks for fixing it.

> Everybody would have had, had it been included in /etc/skel/.cvsrc :^)
> Worth adding?

I (mildly) oppose that.  Providing customization files by default
is an oxymoron that provokes exactly the problems seen here.  People
become so used to it that communication breaks down - some people
assume the default customization to be the default, others assume
the defaults to be the defaults.  These default customization files
are also undocumented, so they cause default behaviour of utilities
to differ from what is described in the manpages.  At least we should
not pile more poo on top of the turd.

We want sane defaults, and we want simple defaults.  Where traditional
defaults are suboptimal but can't easily be changed, i still think
that having one set of defaults is saner and simpler than having
two sets of defaults:  One traditional, documented plus one arguably
more convenient, but undocumented set...

Whatever, i expect others will disagree, so i'm unlikely to argue
this further, and i can certainly live with whatever happens in
/etc/skel/, i never use it anyway.

Yours,
  Ingo


> Index: etc/skel/dot.cvsrc
> ===
> RCS file: /cvs/src/etc/skel/dot.cvsrc,v
> retrieving revision 1.2
> diff -u -p -r1.2 dot.cvsrc
> --- etc/skel/dot.cvsrc31 Mar 2015 18:34:40 -  1.2
> +++ etc/skel/dot.cvsrc29 Oct 2016 17:44:30 -
> @@ -1,6 +1,6 @@
>  # $OpenBSD: dot.cvsrc,v 1.2 2015/03/31 18:34:40 naddy Exp $
>  #
>  diff -uNp
> -update -P
> +update -Pd
>  checkout -P
>  rdiff -u



Re: Add ioctl for disk cache flush

2016-10-30 Thread Stefan Fritsch
On Mon, 17 Oct 2016, Alexander Bluhm wrote:
> > Part 1: Add an ioctl:
> > 
> > 
> > add DIOCCACHESYNC
> > 
> > Add an ioctl to tell storage devices to flush their internal caches.
> > Ported from netbsd by pedro@
> > 
> > OK?
> 
> Some remarks inline

Sorry for the long delay, I was very busy in the last weeks.

I agree with all your comments (and should have reviewed the initial patch 
better). Here is an updated diff.

Cheers,
Stefan

diff --git sys/dev/ata/wd.c sys/dev/ata/wd.c
index 5e2461f..a644cbf 100644
--- sys/dev/ata/wd.c
+++ sys/dev/ata/wd.c
@@ -26,7 +26,7 @@
  */
 
 /*-
- * Copyright (c) 1998 The NetBSD Foundation, Inc.
+ * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -138,7 +138,7 @@ void  wdstart(void *);
 void  __wdstart(struct wd_softc*, struct buf *);
 void  wdrestart(void *);
 int   wd_get_params(struct wd_softc *, u_int8_t, struct ataparams *);
-void  wd_flushcache(struct wd_softc *, int);
+int   wd_flushcache(struct wd_softc *, int);
 void  wd_standby(struct wd_softc *, int);
 
 /* XXX: these should go elsewhere */
@@ -848,6 +848,10 @@ wdioctl(dev_t dev, u_long xfer, caddr_t addr, int flag, 
struct proc *p)
}
 #endif
 
+   case DIOCCACHESYNC:
+   error = wd_flushcache(wd, AT_WAIT);
+   goto exit;
+
default:
error = wdc_ioctl(wd->drvp, xfer, addr, flag, p);
goto exit;
@@ -1067,13 +1071,13 @@ wd_get_params(struct wd_softc *wd, u_int8_t flags, 
struct ataparams *params)
}
 }
 
-void
+int
 wd_flushcache(struct wd_softc *wd, int flags)
 {
struct wdc_command wdc_c;
 
if (wd->drvp->ata_vers < 4) /* WDCC_FLUSHCACHE is here since ATA-4 */
-   return;
+   return EIO;
bzero(&wdc_c, sizeof(struct wdc_command));
wdc_c.r_command = (wd->sc_flags & WDF_LBA48 ? WDCC_FLUSHCACHE_EXT :
WDCC_FLUSHCACHE);
@@ -1088,20 +1092,28 @@ wd_flushcache(struct wd_softc *wd, int flags)
if (wdc_exec_command(wd->drvp, &wdc_c) != WDC_COMPLETE) {
printf("%s: flush cache command didn't complete\n",
wd->sc_dev.dv_xname);
+   return EIO;
}
+   if (wdc_c.flags & ERR_NODEV)
+   return ENODEV;
if (wdc_c.flags & AT_TIMEOU) {
printf("%s: flush cache command timeout\n",
wd->sc_dev.dv_xname);
+   return EIO;
+   }
+   if (wdc_c.flags & AT_ERROR) {
+   if (wdc_c.r_error == WDCE_ABRT) /* command not supported */
+   return ENODEV;
+   printf("%s: flush cache command: error 0x%x\n",
+   wd->sc_dev.dv_xname, wdc_c.r_error);
+   return EIO;
}
if (wdc_c.flags & AT_DF) {
printf("%s: flush cache command: drive fault\n",
wd->sc_dev.dv_xname);
+   return EIO;
}
-   /*
-* Ignore error register, it shouldn't report anything else
-* than COMMAND ABORTED, which means the device doesn't support
-* flush cache
-*/
+   return 0;
 }
 
 void
diff --git sys/scsi/sd.c sys/scsi/sd.c
index 30fb36b..e5a0139 100644
--- sys/scsi/sd.c
+++ sys/scsi/sd.c
@@ -2,7 +2,7 @@
 /* $NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $ */
 
 /*-
- * Copyright (c) 1998 The NetBSD Foundation, Inc.
+ * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -96,7 +96,7 @@ int   sd_vpd_block_limits(struct sd_softc *, int);
 intsd_vpd_thin(struct sd_softc *, int);
 intsd_thin_params(struct sd_softc *, int);
 intsd_get_parms(struct sd_softc *, struct disk_parms *, int);
-void   sd_flush(struct sd_softc *, int);
+intsd_flush(struct sd_softc *, int);
 
 void   viscpy(u_char *, u_char *, int);
 
@@ -999,6 +999,15 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, 
struct proc *p)
error = sd_ioctl_cache(sc, cmd, (struct dk_cache *)addr);
goto exit;
 
+   case DIOCCACHESYNC:
+   if (!ISSET(flag, FWRITE)) {
+   error = EBADF;
+   goto exit;
+   }
+   if ((sc->flags & SDF_DIRTY) != 0 || *(int *)addr != 0)
+   error = sd_flush(sc, 0);
+   goto exit;
+
default:
if (part != RAW_PART) {
error = ENOTTY;
@@ -1876,19 +1885,20 @@ die:
return (SDGP_RESULT_OFFLINE);
 }
 
-void
+int
 sd_flush(struct sd_softc *sc, int flags)
 {
struct scsi_link *link;
struct scsi_xfer *xs;
struct scsi_synchronize_cache *cmd;
+   int error;
 
if (sc->flags & SDF_DYING)
-   return;
+   return (ENXIO);
link = sc->