Re: ping graphical display

2021-02-19 Thread Landry Breuil
On Fri, Feb 19, 2021 at 03:19:49PM +, Stuart Henderson wrote:
> This diff adds something similar to cisco's ping display, giving a
> visual display of good/dropped pings. Any interest in it? Example
> output (with a couple of ^T during the run):

fwiw, noping from net/liboping in ports has this feature builtin.

Landry



Re: use /dev/dri/ in xenocara

2021-02-18 Thread Landry Breuil
On Thu, Feb 18, 2021 at 11:11:15PM +1100, Jonathan Gray wrote:
> On Thu, Feb 18, 2021 at 11:34:19AM +, Stuart Henderson wrote:
> > On 2021/02/18 22:24, Jonathan Gray wrote:
> > > On Thu, Feb 18, 2021 at 12:01:28PM +0100, Mark Kettenis wrote:
> > > > > Date: Thu, 18 Feb 2021 21:18:51 +1100
> > > > > From: Jonathan Gray 
> > > > 
> > > > I suspect that there are some ports that need to get their unveils
> > > > updated if we do this.
> > > 
> > > firefox ports were updated.  Not aware of anything else in ports that
> > > unveils /dev/drm.
> > 
> > unveils: not afaik
> > 
> > others: gdm already handled it, some other ports will need patches changing:
> > 
> > graphics/clutter/cogl/patches/patch-cogl_winsys_cogl-winsys-egl-kms_c
> > graphics/waffle/patches/patch-src_waffle_gbm_wgbm_display_c
> > x11/compton/patches/patch-src_compton_c
> > x11/slim/patches/patch-slim_conf
> 
> This is a display manager like xdm/gdm.  The last upstream release was
> in 2013.  I can patch it after the xenocara changes go in or perhaps we
> remove it as landry suggested in

there's a less dead upstream at https://github.com/PeteGozz/slim.
FreeBSD and debian still ship a package for slim :)

I *might* have a look at updating the port to this github fork.

Landry



Re: uhidpp(4): logitech hid++ device driver

2021-02-05 Thread Landry Breuil
On Fri, Jan 22, 2021 at 08:18:51AM +0100, Anton Lindqvist wrote:
> Hi,
> Here's a new driver for Logitech HID++ devices, currently limited to
> exposing battery sensors. Here's an example using a Logitech M330 mouse:
> 
>   $ dmesg | grep uhidpp
>   uhidpp0 at uhidev1 device 1 mouse "B330/M330/M3" serial c7-2f-a8-33
>   $ sysctl hw.sensors.uhidpp0
>   hw.sensors.uhidpp0.raw0=2 (battery levels)
>   hw.sensors.uhidpp0.percent0=70.00% (battery level), OK

thanks anton, now that this got commited i realize i have a pair of
devices:

device 1 (M235 according to bottom) previously reported this in dmesg:

uhidev0 at uhub0 port 2 configuration 1 interface 0 "Logitech USB Receiver" rev 
2.00/12.03 addr 2
uhidev0: iclass 3/1
ukbd0 at uhidev0: 8 variable keys, 6 key codes
wskbd1 at ukbd0 mux 1
uhidev1 at uhub0 port 2 configuration 1 interface 1 "Logitech USB Receiver" rev 
2.00/12.03 addr 2
uhidev1: iclass 3/1, 8 report ids
ums0 at uhidev1 reportid 2: 16 buttons, Z and W dir
wsmouse2 at ums0 mux 0
uhid0 at uhidev1 reportid 3: input=4, output=0, feature=0
uhid1 at uhidev1 reportid 4: input=1, output=0, feature=0
uhid2 at uhidev1 reportid 8: input=1, output=0, feature=0
uhidev2 at uhub0 port 2 configuration 1 interface 2 "Logitech USB Receiver" rev 
2.00/12.03 addr 2
uhidev2: iclass 3/0, 33 report ids
uhid3 at uhidev2 reportid 16: input=6, output=6, feature=0
uhid4 at uhidev2 reportid 17: input=19, output=19, feature=0
uhid5 at uhidev2 reportid 32: input=14, output=14, feature=0
uhid6 at uhidev2 reportid 33: input=31, output=31, feature=0

device 2 (M235 2nd gen according to bottom) previously reported this in dmesg
(same thing, but the rev/addr values):

uhidev1 at uhub0 port 4 configuration 1 interface 0 "Logitech USB Receiver" rev 
2.00/24.07 addr 3
uhidev1: iclass 3/1
ukbd1 at uhidev1: 8 variable keys, 6 key codes
wskbd2 at ukbd1 mux 1
uhidev2 at uhub0 port 4 configuration 1 interface 1 "Logitech USB Receiver" rev 
2.00/24.07 addr 3
uhidev2: iclass 3/1, 8 report ids
ums0 at uhidev2 reportid 2: 16 buttons, Z and W dir
wsmouse2 at ums0 mux 0
uhid0 at uhidev2 reportid 3: input=4, output=0, feature=0
uhid1 at uhidev2 reportid 4: input=1, output=0, feature=0
uhid2 at uhidev2 reportid 8: input=1, output=0, feature=0
uhidev3 at uhub0 port 4 configuration 1 interface 2 "Logitech USB Receiver" rev 
2.00/24.07 addr 3
uhidev3: iclass 3/0, 33 report ids
uhid3 at uhidev3 reportid 16: input=6, output=6, feature=0
uhid4 at uhidev3 reportid 17: input=19, output=19, feature=0
uhid5 at uhidev3 reportid 32: input=14, output=14, feature=0
uhid6 at uhidev3 reportid 33: input=31, output=31, feature=0

now device 1/uhidev2 is properly identified by uhidpp:

uhidev2 at uhub0 port 2 configuration 1 interface 2 "Logitech USB Receiver" rev 
2.00/12.03 addr 2
uhidev2: iclass 3/0, 33 report ids
uhidpp0 at uhidev2 device 1 mouse "M325" serial 69-b1-af-84

and properly reports sensors:
hw.sensors.uhidpp0.raw0=2 (battery levels)
hw.sensors.uhidpp0.percent0=70.00% (battery level), OK

and device2 (plugged on the same laptop so device renumbered) also works with 
uhidpp:

uhidev2 at uhub0 port 2 configuration 1 interface 2 "Logitech USB Receiver" rev 
2.00/24.07 addr 2
uhidev2: iclass 3/0, 33 report ids
uhidpp0 at uhidev2 device 1 mouse "Wireless Mouse" serial f3-94-18-8c

same level reported in sensors for a different mouse (?):
hw.sensors.uhidpp0.raw0=2 (battery levels)
hw.sensors.uhidpp0.percent0=70.00% (battery level), OK

let me know if you need more details/infos from those devices (usbdevs dumps 
etc..)

i'll eventually look at adding support for uhidpp to upower so mouse battery
levels can be reported in xfce4-power-manager and gnome, like it does on
linux..

Thanks !

Landry



Re: IPPROTO_SCTP

2021-01-18 Thread Landry Breuil
On Mon, Jan 18, 2021 at 12:13:32PM +, Stuart Henderson wrote:
> can I add IPPROTO_SCTP to in.h? only one port wants it at the
> moment, but I think I've seen others in the past.

https://searchfox.org/mozilla-central/search?q=ipproto_sctp has some
hits, especially in the chromium-bundled libevent copy.



Re: WANTLIB problems and possible solution: the libset design

2020-10-11 Thread Landry Breuil
On Sun, Oct 11, 2020 at 05:03:31PM +0200, Marc Espie wrote:
> On Sun, Oct 11, 2020 at 04:53:18PM +0200, Christian Weisgerber wrote:
> > Marc Espie:
> > 
> > > The new design:
> > > 
> > > The idea behind "libset" is to be able to specify a "set" of wantlib that
> > > corresponds to our package, AND to just write WANTLIB wrt that libset for
> > > that specific set of libraries.
> > 
> > I'm struggling to understand whether this libset records the libraries
> > a port depends on, the libraries the port provides, or both.
> > 
> > Let's say--slightly simplified from reality--we have devel/gettext
> > that provides libintl and depends on iconv from converters/libiconv.
> > What would gettext's LIBSET entry look like?
> > 
> > (1) LIBSET = iconv
> > (2) LIBSET = intl
> > (3) LIBSET = intl iconv
> 
> 
> 
> I assume any user of gettext will need both iconv and intl, so
> 
> LIBSET = intl iconv

Hmm, pretty sure some consumers of gettext only have one in their
WANTLIB and not both, in that case those ones shouldnt you the libset to
avoid an extra WANTLIB ?



uvideo_querycap(): set device_caps field to unbreak video device detection with firefox 78

2020-07-04 Thread Landry Breuil
Hi,

since firefox 78, video(4) devices arent detected anymore. Upstream
added code in https://bugzilla.mozilla.org/show_bug.cgi?id=1637319 to
check for the V4L2_CAP_VIDEO_CAPTURE capacity, and the code added in
https://hg.mozilla.org/integration/autoland/rev/33facf191f23 checks for
that in the 'device_caps' field of v4l2_capability struct.

problem is, we only set the 'capabilities' field in uvideo(4). Looking
at the linux doc in
https://www.kernel.org/doc/html/v4.14/media/uapi/v4l/vidioc-querycap.html#description,
both fields are used to distinguish multiple devices created by the same
driver. We dont have such a feature, but still, 'device_caps' should/could be
filled for compatibility ? The utvfu(4) driver apparently does it this way:
https://github.com/openbsd/src/blob/master/sys/dev/usb/utvfu.c#L490

so let's just do the same in uvideo(4). With this diff, i can use my
webcam again from firefox 78. No apparent difference in lsusb -vv.

ok ?

Landry

Index: usb/uvideo.c
===
RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
retrieving revision 1.207
diff -u -r1.207 uvideo.c
--- usb/uvideo.c30 May 2020 09:01:04 -  1.207
+++ usb/uvideo.c4 Jul 2020 15:00:14 -
@@ -2942,9 +2942,10 @@
strlcpy(caps->bus_info, "usb", sizeof(caps->bus_info));
 
caps->version = 1;
-   caps->capabilities = V4L2_CAP_VIDEO_CAPTURE
+   caps->device_caps = V4L2_CAP_VIDEO_CAPTURE
| V4L2_CAP_STREAMING
| V4L2_CAP_READWRITE;
+   caps->capabilities = caps->device_caps | V4L2_CAP_DEVICE_CAPS;
 
return (0);
 }



Re: 11n Tx aggregation for iwm(4)

2020-06-27 Thread Landry Breuil
On Fri, Jun 26, 2020 at 06:14:48PM +0200, Landry Breuil wrote:
> On Fri, Jun 26, 2020 at 02:45:53PM +0200, Stefan Sperling wrote:
> > This patch adds support for 11n Tx aggregation to iwm(4).
> > 
> > Please help with testing if you can by running the patch and using wifi
> > as usual. Nothing should change, except that Tx speed may potentially
> > improve. If you have time to run before/after performance measurements with
> > tcpbench or such, that would be nice. But it's not required for testing.
> > 
> > If Tx aggregation is active then netstat will show a non-zero output block 
> > ack
> > agreement counter:
> > 
> > $ netstat -W iwm0 | grep 'output block'
> > 3 new output block ack agreements
> > 0 output block ack agreements timed out
> > 
> > It would be great to get at least one test for all the chipsets the driver
> > supports: 7260, 7265, 3160, 3165, 3168, 8260, 8265, 9260, 9560
> > The behaviour of the access point also matters a great deal. It won't
> > hurt to test the same chipset against several different access points.
> > 
> > I have tested this version on 8265 only so far. I've run older revisions
> > of this patch on 7265 so I'm confident that this chip will work, too.
> > So far, the APs I have tested against are athn(4) in 11a mode and in 11n
> > mode with the 'nomimo' nwflag, and a Sagemcom 11ac AP. All on 5Ghz channels.
> 
> no difference on X1c3 w/
> iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless AC 7265" rev 0x59, msi
> iwm0: hw rev 0x210, fw ver 17.3216344376.0,
> 
> using a crappy old fonera as AP, serving as a bridge to gw w/ tcpbench.
> 
> bandwidth min/avg/max/std-dev = 22.519/22.704/22.995/0.162 Mbps
> 
> same bw both ways it seems.

so no change against this old AP, which selects:
media: IEEE802.11 autoselect (OFDM48 mode 11g)
or sometimes
media: IEEE802.11 autoselect (OFDM12 mode 11g)
or
media: IEEE802.11 autoselect (OFDM6 mode 11g)

but if i connect to the ISP's box wifi, which selects:
media: IEEE802.11 autoselect (HT-MCS8 mode 11n)

the performance is horrible, i have a lot of lag, and tcpbench says:
bandwidth min/avg/max/std-dev = 0.000/1.576/10.069/2.781 Mbps

i have some iwm firmware errors in dmesg.

without the patch, its a bit the same:
bandwidth min/avg/max/std-dev = 0.000/1.836/9.846/2.292 Mbps

but no firmware errors afaict.
so dunno if the patch itself changes something, but the perf with the
ISP AP is awful. Cant remember if it was the case before as i seldomly
use it with OpenBSD as a client..

Landry



Re: 11n Tx aggregation for iwm(4)

2020-06-26 Thread Landry Breuil
On Fri, Jun 26, 2020 at 02:45:53PM +0200, Stefan Sperling wrote:
> This patch adds support for 11n Tx aggregation to iwm(4).
> 
> Please help with testing if you can by running the patch and using wifi
> as usual. Nothing should change, except that Tx speed may potentially
> improve. If you have time to run before/after performance measurements with
> tcpbench or such, that would be nice. But it's not required for testing.
> 
> If Tx aggregation is active then netstat will show a non-zero output block ack
> agreement counter:
> 
> $ netstat -W iwm0 | grep 'output block'
> 3 new output block ack agreements
>   0 output block ack agreements timed out
> 
> It would be great to get at least one test for all the chipsets the driver
> supports: 7260, 7265, 3160, 3165, 3168, 8260, 8265, 9260, 9560
> The behaviour of the access point also matters a great deal. It won't
> hurt to test the same chipset against several different access points.
> 
> I have tested this version on 8265 only so far. I've run older revisions
> of this patch on 7265 so I'm confident that this chip will work, too.
> So far, the APs I have tested against are athn(4) in 11a mode and in 11n
> mode with the 'nomimo' nwflag, and a Sagemcom 11ac AP. All on 5Ghz channels.

no difference on X1c3 w/
iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless AC 7265" rev 0x59, msi
iwm0: hw rev 0x210, fw ver 17.3216344376.0,

using a crappy old fonera as AP, serving as a bridge to gw w/ tcpbench.

bandwidth min/avg/max/std-dev = 22.519/22.704/22.995/0.162 Mbps

same bw both ways it seems.

Landry



Re: libossaudio: start using sndio

2020-04-02 Thread Landry Breuil
On Wed, Apr 01, 2020 at 07:27:12PM +0200, Alexandre Ratchov wrote:
> ping!
> 
> FWIW, the diff below affects the following ports:
>   - emulators/gambatte
>   - gstreamer-0.10 mixer plugin
>   - sysutils/conky
>   - sysutils/gkrellm
>   - sysutil/tpb
>   - telephony/iaxclient
>   - anything depending on gstreamer, ex xfce4-mixer
> 
> If you use one of above ports and want to test, just apply this diff
> to src/lib/libossaudio in base, rebuild it and reinstall it. The ABI
> is not changing, so no need to rebuild or update ports.

I've briefly tested it with xfce4-mixer which uses gst 0.10 - on a t430u
with:

azalia0 at pci0 dev 27 function 0 "Intel 7 Series HD Audio" rev 0x04: msi
azalia0: codecs: Realtek ALC269, Intel/0x2806, using Realtek ALC269

previously, it saw two controls, one for the output acting/wired to
mixerctl values inputs.dac-0:1 & inputs.dac-2:3 (ie both changes upon
control changes), and one for the input wired to inputs.mic

with the diff, xfce4-mixer now sees the 3 expected controls:
* 'volume', wired to aucatctl 'master' / sndioctl 'output.level' value
* 'input gain', wired to mixerctl record.volume, record.adc-0:1 &
* record.adc-2:3 (and to sndioctl hw/input.level)
* 'output gain', wired to mixerctl outputs.master, inputs.dac-0:1 &
* inputs.dac-2:3 (and to sndioctl hw/output.level)

trying to toggle the 'mute' button in xfce4-mixer (for the 'input gain'
only) leads to 'Error setting mixer recording devices (0x0): Invalid
argument' msgs on stderr but im not sure it matters.

toggling the mute button for volume and input gain set the level to 0
instead of toggling the hw/output.mute button but im not sure it's a big
deal either.

all in all that seems an improvement to me, at least does what's
expected for gst 0.10/xfce4-mixer.



Re: iked.conf.5: Provide GRE tunnel in transport mode example

2020-02-22 Thread Landry Breuil
On Sat, Feb 22, 2020 at 12:24:36PM +0100, Klemens Nanni wrote:
> On Sat, Feb 22, 2020 at 10:19:27AM +0100, Tobias Heider wrote:
> > This is not what dstid does. When setting 'dstid D.example.com' the policy 
> > still
> > only applies if the peer sends 'D.example.com' as it's identity in the ID 
> > payload.
> > Not setting dstid explicitly means iked will fall back to the value of 
> > "peer",
> > which in your case would be the same: "D.example.com".
> > 
> > Setting dstid is only necessary if you are using the IP address in the
> > "peer" option but still want to use a FQDN as ID, which is really only the
> > case with certificate authentication where the ID must match the
> > subjectAltName.
> I can double check yet again, but I'm pretty sure that setting dstid
> was what made iked find the public key.  So far, I have not used literal
> IPs in my configuration - that I know for sure.

that was also my experience when working on faq17, srcid/dstid were used
to lookup the cert/key in /etc/iked...



Re: Audio control API, part 2: add new sndioctl(1) utility

2020-02-09 Thread Landry Breuil
On Sun, Feb 09, 2020 at 01:14:47PM +0100, Alexandre Ratchov wrote:
> Here's a new sndioctl utility similar to mixerctl(1) but using the new
> sndio API. Example:
> 
> $ sndioctl 
> output.level=127
> app/aucat0.level=127
> app/firefox0.level=127
> app/firefox1.level=12
> app/midisyn0.level=127
> app/mpv0.level=127
> app/prog5.level=127
> app/prog6.level=127
> app/prog7.level=127
> hw/input.level=62
> hw/input.mute=0
> hw/output.level=63
> hw/output.mute=0

i suppose that replaces audio/aucatctl port ? audio/cmixer relies on the
latter but i'll have no issue migrating to it if sndioctl gets added.



Re: man.cgi(8): turn off HTML5 autocomplete for the query input field

2020-01-09 Thread Landry Breuil
On Thu, Jan 09, 2020 at 09:18:25PM -0600, Tim Baumgard wrote:
> This turns off HTML5 autocomplete for the query input field for
> man.cgi(8). This essentially makes smartphones and tablets behave the
> same as PCs since they otherwise would try to annoyingly capitalize the
> first character and change things like "mandoc" to "man doctor."

hilarious.. 'smart'phones :)



Re: change to bsd.port.mk to help debug packages a bit

2019-12-02 Thread Landry Breuil
On Wed, Nov 27, 2019 at 05:55:47PM +0100, Marc Espie wrote:
> This should get rid of the weird error (don't know how to make
> .../all/debug-*.tgz) when switching from !DEBUG_PACKAGES to DEBUG_PACKAGES.
> 
> Basically, this introduces a "build two targets at once" in bsd.port.mk.
> 
> make has some glue to figure out whether this is a "duplicate the work" or
> "one target builds two things" (already used for yacc and friends), by
> looking for variations on $@... which includes $@D for better or worse, hence
> the small tweak.
> 
> This should work on all settings, including running make -jN package
> (which doesn't make a lot of sense, but whatever).
> 
> Please check/test

Fixes the error i was seeing, looks ok to me.



Re: [patch] cwm: filter duplicate hostnames in ssh menu

2019-05-06 Thread Landry Breuil
On Mon, May 06, 2019 at 08:04:02AM +0200, Bruno Flückiger wrote:
> On 01.05., Marcus MERIGHI wrote:
> > Hello,
> >
> > o...@demirmen.com (Okan Demirmen), 2019.04.29 (Mon) 16:19 (CEST):
> > > On Fri 2019.04.26 at 07:15 +0200, Bruno Fl?ckiger wrote:
> > > > Hi,
> > > >
> > > > The ssh menu of cwm(1) doesn't filter duplicated hostnames when reading
> > > > them from ~/.ssh/known_hosts. This patch makes sure each hostname is
> > > > only displayed once to the menu.
> > >
> > > Sure, maybe; but why again do we even have this inside a window manager?
> > > Really, the known_hosts parsing is incomplete at best; either the entire
> > > parsing code needs to be lifted from ssh or this (mis)feature should be
> > > removed from cwm. I prefer the latter.
> >
> > FWIW, i use "M-period" a lot... are there easy alternatives?
> >
> > Marcus
> >
> 
> I'm thinking about transforming the ssh menu into a standalone
> application. This way the (mis)feature could be removed, but those who
> like using would get an adequate replacement. Any opinions about that?

there are already standalone applications that do this among other
things, https://github.com/davatorium/rofi comes to my mind (yes, it's in
ports).

Landry



Re: OpenBSD 6.5 released -- Apr 24 2019

2019-04-26 Thread Landry Breuil
On Fri, Apr 26, 2019 at 07:48:26PM +0100, Andrew Grillet wrote:
> Install was very quick on my Sun V100. Congratulations to all involved.
> 
> Any news on if/when there will be packages for Sparc64?

When they're done, still at least 3 weeks.



Re: Neuter shm calls from X swrast_dri.so

2019-02-24 Thread Landry Breuil
On Sun, Feb 24, 2019 at 10:44:25AM -0500, Todd Mortimer wrote:
> A few weeks ago I noticed that firefox tabs were getting killed for
> running afoul of pledge(2). It seems that the problem was some calls to
> shmget(2) from the X swrast_dri.so lib that seem to have come from the
> new mesa code that was recently imported. Since the shm syscalls aren't
> covered by any pledge the system was killing the firefox tabs when they
> called into X and X went down this code path.
> 
> The shm calls are guarded by a #ifdef, so the patch below just
> modifies the conditions so OpenBSD does not include the shm function and
> falls back to ordinary malloc. With this patch my firefox works again.
> 
> The alternative is to add shm to pledge(2) somewhere. I expect that
> adding a syscall to pledge for a single program is unwanted, but in this
> case it would be for any program using X with this DRI module. A quick
> check in xenocara finds a small number of other references to the shm
> functions (lib/libXvMC/src/XvMC.c, lib/xcb-util-image/), but I don't
> know if we use these.

Thanks for looking into this, and nice findings !

There has been some discussion to add an 'shm' pledge class, but no real
usage surfaced so far, and the usual stance was to neuter the shmget
calls so that fallback codepaths were used like in
https://bugzilla.mozilla.org/show_bug.cgi?id=1457092. - maybe there are
more in other programs.. but there would have been a lot of pledge
reports if so.



NMEA: add more gps sensor values (altitude)(take 3)

2019-01-24 Thread Landry Breuil
Hi,

simplified diff from my last mail, only adding speed & altitude, now
with 100% less floating point in the kernel !

Use a #define KNOTTOMS (51444 / 100) instead of the wrong (1000 / 1.9438)
formula to convert from thousands of knots (nmea_atoi() returns an
integer which takes 3 extra digits from the input, ie '0.110' yields
110) to um/s (the unit expected by the sensor).

Built on amd64, i386 & sparc64, bsd.rd too (i don't think tty_nmea.c is
built in RAMDISK as it has needs-flag).

nmea(4) updated too; see diff.

Last chance for comments - oks welcome too :)

Landry
Index: tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.47
diff -u -r1.47 tty_nmea.c
--- tty_nmea.c  1 Sep 2018 06:09:26 -   1.47
+++ tty_nmea.c  24 Jan 2019 19:12:43 -
@@ -38,6 +38,7 @@
 
 #define NMEAMAX82
 #define MAXFLDS32
+#define KNOTTOMS   (51444 / 100)
 #ifdef NMEA_DEBUG
 #define TRUSTTIME  30
 #else
@@ -52,6 +53,8 @@
struct ksensor  signal; /* signal status */
struct ksensor  latitude;
struct ksensor  longitude;
+   struct ksensor  altitude;
+   struct ksensor  speed;
struct ksensordev   timedev;
struct timespec ts; /* current timestamp */
struct timespec lts;/* timestamp of last '$' seen */
@@ -70,6 +73,7 @@
 /* NMEA decoding */
 void   nmea_scan(struct nmea *, struct tty *);
 void   nmea_gprmc(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void   nmea_decode_gga(struct nmea *, struct tty *, char *fld[], int fldcnt);
 
 /* date and time conversion */
 intnmea_date_to_nano(char *s, int64_t *nano);
@@ -77,6 +81,7 @@
 
 /* longitude and latitude conversion */
 intnmea_degrees(int64_t *dst, char *src, int neg);
+intnmea_atoi(int64_t *dst, char *src);
 
 /* degrade the timedelta sensor */
 void   nmea_timeout(void *);
@@ -126,6 +131,20 @@
strlcpy(np->longitude.desc, "Longitude", sizeof(np->longitude.desc));
sensor_attach(>timedev, >longitude);
 
+   np->altitude.type = SENSOR_DISTANCE;
+   np->altitude.status = SENSOR_S_UNKNOWN;
+   np->altitude.flags = SENSOR_FINVALID;
+   np->altitude.value = 0;
+   strlcpy(np->altitude.desc, "Altitude", sizeof(np->altitude.desc));
+   sensor_attach(>timedev, >altitude);
+
+   np->speed.type = SENSOR_VELOCITY;
+   np->speed.status = SENSOR_S_UNKNOWN;
+   np->speed.flags = SENSOR_FINVALID;
+   np->speed.value = 0;
+   strlcpy(np->speed.desc, "Ground speed", sizeof(np->speed.desc));
+   sensor_attach(>timedev, >speed);
+
np->sync = 1;
tp->t_sc = (caddr_t)np;
 
@@ -261,7 +280,7 @@
}
 
/*
-* we only look at the RMC message, which can come from different 
'talkers',
+* we only look at the messages coming from well-known sources or 
'talkers',
 * distinguished by the two-chars prefix, the most common being:
 * GPS (GP)
 * Glonass (GL)
@@ -269,11 +288,16 @@
 * Galileo (GA)
 * 'Any kind/a mix of GNSS systems' (GN)
 */
-   if (strcmp(fld[0], "BDRMC") &&
-   strcmp(fld[0], "GARMC") &&
-   strcmp(fld[0], "GLRMC") &&
-   strcmp(fld[0], "GNRMC") &&
-   strcmp(fld[0], "GPRMC"))
+   if (strncmp(fld[0], "BD", 2) &&
+   strncmp(fld[0], "GA", 2) &&
+   strncmp(fld[0], "GL", 2) &&
+   strncmp(fld[0], "GN", 2) &&
+   strncmp(fld[0], "GP", 2))
+   return;
+   
+   /* we look for the RMC & GGA messages */
+   if (strncmp(fld[0] + 2, "RMC", 3) &&
+   strncmp(fld[0] + 2, "GGA", 3))
return;
 
/* if we have a checksum, verify it */
@@ -299,7 +323,10 @@
return;
}
}
-   nmea_gprmc(np, tp, fld, fldcnt);
+   if (strncmp(fld[0] + 2, "RMC", 3) == 0)
+   nmea_gprmc(np, tp, fld, fldcnt);
+   if (strncmp(fld[0] + 2, "GGA", 3) == 0)
+   nmea_decode_gga(np, tp, fld, fldcnt);
 }
 
 /* Decode the recommended minimum specific GPS/TRANSIT data. */
@@ -385,9 +412,11 @@
np->signal.status = SENSOR_S_OK;
np->latitude.status = SENSOR_S_OK;
np->longitude.status = SENSOR_S_OK;
+   np->speed.status = SENSOR_S_OK;
np->time.flags &= ~SENSOR_FINVALID;
np->latitude.flags &= ~SENSOR_FINVALID;
np->longitude.flags &= ~SENSOR_FINVALID;
+   np->speed.flags &= ~SENSOR_FINVALID;
break;
case 'V':   /*
 * The GPS indicates a warning status, do not add to
@@ -399,6 +428,7 @@
np->signal.status = SENSOR_S_CRIT;
np->latitude.status = SENSOR_S_WARN;

Re: video(1) pledge

2019-01-21 Thread Landry Breuil
On Mon, Jan 21, 2019 at 09:44:59PM +0100, Matthieu Herrb wrote:
> On Mon, Jan 21, 2019 at 09:27:57PM +0100, Landry Breuil wrote:
> > Hi,
> >
> 
> Hi,
> 
> > now that the 'video' promise is in, looking for okays to pledge
> > video(1).
> > 
> > with help & hints from semarie@.
> 
> One comment in-line.
> > 
> > Index: video.c
> > ===
> > RCS file: /cvs/xenocara/app/video/video.c,v
> > retrieving revision 1.25
> > diff -u -r1.25 video.c
> > --- video.c 9 Apr 2018 18:16:44 -   1.25
> > +++ video.c 30 Dec 2018 09:39:27 -
> > @@ -1961,6 +1961,8 @@
> > argv += optind;
> >  
> > if (vid.mode & M_QUERY) {
> > +   if (pledge("stdio rpath wpath video", NULL) == -1)
> > +   err(1, "pledge");
> > dev_dump_query();
> > cleanup(, 0);
> > }
> > @@ -1970,6 +1972,14 @@
> >  
> > if (!setup())
> > cleanup(, 1);
> > +
> > +   if (vid.mode & M_IN_FILE) {
> > +   if (pledge("stdio", NULL) == -1)
> 
> Like people have found out the hard way recently, X libs need "rpath"
> in case the X error handler needs to be called.

Right, forgot about that issue. new diff :)

Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.26
diff -u -r1.26 video.c
--- video.c 4 Jan 2019 17:45:00 -   1.26
+++ video.c 21 Jan 2019 20:50:06 -
@@ -1961,6 +1961,8 @@
argv += optind;
 
if (vid.mode & M_QUERY) {
+   if (pledge("stdio rpath wpath video", NULL) == -1)
+   err(1, "pledge");
dev_dump_query();
cleanup(, 0);
}
@@ -1970,6 +1972,14 @@
 
if (!setup())
cleanup(, 1);
+
+   if (vid.mode & M_IN_FILE) {
+   if (pledge("stdio rpath", NULL) == -1)
+   err(1, "pledge");
+   } else {
+   if (pledge("stdio rpath video", NULL) == -1)
+   err(1, "pledge");
+   }
 
if (!stream())
cleanup(, 1);



video(1) pledge

2019-01-21 Thread Landry Breuil
Hi,

now that the 'video' promise is in, looking for okays to pledge
video(1).

with help & hints from semarie@.

Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.25
diff -u -r1.25 video.c
--- video.c 9 Apr 2018 18:16:44 -   1.25
+++ video.c 30 Dec 2018 09:39:27 -
@@ -1961,6 +1961,8 @@
argv += optind;
 
if (vid.mode & M_QUERY) {
+   if (pledge("stdio rpath wpath video", NULL) == -1)
+   err(1, "pledge");
dev_dump_query();
cleanup(, 0);
}
@@ -1970,6 +1972,14 @@
 
if (!setup())
cleanup(, 1);
+
+   if (vid.mode & M_IN_FILE) {
+   if (pledge("stdio", NULL) == -1)
+   err(1, "pledge");
+   } else {
+   if (pledge("stdio rpath video", NULL) == -1)
+   err(1, "pledge");
+   }
 
if (!stream())
cleanup(, 1);



Re: NMEA: add more gps sensor values (2nd take)

2018-12-30 Thread Landry Breuil
On Sun, Dec 30, 2018 at 02:23:59PM -0700, Theo de Raadt wrote:
> Landry Breuil  wrote:
> 
> > On Sun, Dec 30, 2018 at 01:37:26PM -0700, Theo de Raadt wrote:
> > > That's a big step you are taking there
> > > 
> > > "ldattach -t nmea" was written to extract time information for ntpd.
> > > 
> > > It was not written to extract any other information.
> > > 
> > > Until you came up with a diff which extracts other information.
> > > 
> > > The time information was not privacy sensitive, so the previous design
> > > of exporting to all processes via sysctl was acceptable.  However now
> > > there is privacy sensitive information, and sysctl export to all processes
> > > seems unsuitable.
> > 
> > seems this 'position' has changed over the years...
> 
> Yes, positions have changed.
> 
> Where do you stand?

On the 'make things usable' side.

> Shall we enable video and the microphone by default, to all
> applications?

I never said/proposed that. Now, can we come back to the technical
implementation details ?  After all, this is tech@..

I've been privately suggested to have hw.sensors.nmea values only
available to root as done for net.inet6.ip6.soiikey. What are you
proposing ?



Re: NMEA: add more gps sensor values (2nd take)

2018-12-30 Thread Landry Breuil
On Sun, Dec 30, 2018 at 01:37:26PM -0700, Theo de Raadt wrote:
> That's a big step you are taking there
> 
> "ldattach -t nmea" was written to extract time information for ntpd.
> 
> It was not written to extract any other information.
> 
> Until you came up with a diff which extracts other information.
> 
> The time information was not privacy sensitive, so the previous design
> of exporting to all processes via sysctl was acceptable.  However now
> there is privacy sensitive information, and sysctl export to all processes
> seems unsuitable.

seems this 'position' has changed over the years...

revision 1.37
date: 2010/04/21 23:43:39;  author: sthen;  state: Exp;  lines: +63 -52;
Provide nmea(4) position information using the new angle sensor type.
Use SENSOR_FINVALID until we have good data, suggested by deraadt@
"i am happy" deraadt@

revision 1.30
date: 2008/07/22 06:06:47;  author: mbalmer;  state: Exp;  lines: +13-8;
deactivate the code to display location in the sensor description

revision 1.28
date: 2008/07/06 21:03:13;  author: mbalmer;  state: Exp;  lines: +58-1;
Add the position to the sensor description.

discussed with otto, sthen, ckuethe.  ok otto


the position information is public *right now* (and since 8 years) if
you plug an nmea(4) device with ldattach to use it as a timedelta
source.

> > This is imo orthogonal to the privacy discussion, and i personally have
> > no plans to dig into adding knobs for restricting/allowing fine-grained
> > access to devices.
> 
> So you have no plans to address the privacy concerns?

Personally, no. But i'll welcome and test diffs from anyone who has
serious concerns about it, and actual plans to improve the current
situation.



Re: NMEA: add more gps sensor values (2nd take)

2018-12-30 Thread Landry Breuil
On Sun, Dec 30, 2018 at 05:17:54AM -0700, Theo de Raadt wrote:
> I think you've gone overboard adding sensors.
> 
> Some of them aren't informing a fact about the world, but about the GPS device
> operation.
> 
> Therefore I think nsat, quality, hdop, vdop, pdop are irrelevant.  These
> are basically protocol-specific / device-specific "error bars".
> 
> I think you should delete those.

New smaller, simpler diff only adding altitude and speed. As a bonus
nmea_atoi() loses the now useless decimal argument.

Index: tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.47
diff -u -r1.47 tty_nmea.c
--- tty_nmea.c  1 Sep 2018 06:09:26 -   1.47
+++ tty_nmea.c  30 Dec 2018 14:28:21 -
@@ -52,6 +52,8 @@
struct ksensor  signal; /* signal status */
struct ksensor  latitude;
struct ksensor  longitude;
+   struct ksensor  altitude;
+   struct ksensor  speed;
struct ksensordev   timedev;
struct timespec ts; /* current timestamp */
struct timespec lts;/* timestamp of last '$' seen */
@@ -70,6 +72,7 @@
 /* NMEA decoding */
 void   nmea_scan(struct nmea *, struct tty *);
 void   nmea_gprmc(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void   nmea_decode_gga(struct nmea *, struct tty *, char *fld[], int fldcnt);
 
 /* date and time conversion */
 intnmea_date_to_nano(char *s, int64_t *nano);
@@ -77,6 +80,7 @@
 
 /* longitude and latitude conversion */
 intnmea_degrees(int64_t *dst, char *src, int neg);
+intnmea_atoi(int64_t *dst, char *src);
 
 /* degrade the timedelta sensor */
 void   nmea_timeout(void *);
@@ -126,6 +130,20 @@
strlcpy(np->longitude.desc, "Longitude", sizeof(np->longitude.desc));
sensor_attach(>timedev, >longitude);
 
+   np->altitude.type = SENSOR_DISTANCE;
+   np->altitude.status = SENSOR_S_UNKNOWN;
+   np->altitude.flags = SENSOR_FINVALID;
+   np->altitude.value = 0;
+   strlcpy(np->altitude.desc, "Altitude", sizeof(np->altitude.desc));
+   sensor_attach(>timedev, >altitude);
+
+   np->speed.type = SENSOR_VELOCITY;
+   np->speed.status = SENSOR_S_UNKNOWN;
+   np->speed.flags = SENSOR_FINVALID;
+   np->speed.value = 0;
+   strlcpy(np->speed.desc, "Ground speed", sizeof(np->speed.desc));
+   sensor_attach(>timedev, >speed);
+
np->sync = 1;
tp->t_sc = (caddr_t)np;
 
@@ -261,7 +279,7 @@
}
 
/*
-* we only look at the RMC message, which can come from different 
'talkers',
+* we only look at the messages coming from well-known sources or 
'talkers',
 * distinguished by the two-chars prefix, the most common being:
 * GPS (GP)
 * Glonass (GL)
@@ -269,11 +287,16 @@
 * Galileo (GA)
 * 'Any kind/a mix of GNSS systems' (GN)
 */
-   if (strcmp(fld[0], "BDRMC") &&
-   strcmp(fld[0], "GARMC") &&
-   strcmp(fld[0], "GLRMC") &&
-   strcmp(fld[0], "GNRMC") &&
-   strcmp(fld[0], "GPRMC"))
+   if (strncmp(fld[0], "BD", 2) &&
+   strncmp(fld[0], "GA", 2) &&
+   strncmp(fld[0], "GL", 2) &&
+   strncmp(fld[0], "GN", 2) &&
+   strncmp(fld[0], "GP", 2))
+   return;
+   
+   /* we look for the RMC & GGA messages */
+   if (strncmp(fld[0] + 2, "RMC", 3) &&
+   strncmp(fld[0] + 2, "GGA", 3))
return;
 
/* if we have a checksum, verify it */
@@ -299,7 +322,10 @@
return;
}
}
-   nmea_gprmc(np, tp, fld, fldcnt);
+   if (strncmp(fld[0] + 2, "RMC", 3) == 0)
+   nmea_gprmc(np, tp, fld, fldcnt);
+   if (strncmp(fld[0] + 2, "GGA", 3) == 0)
+   nmea_decode_gga(np, tp, fld, fldcnt);
 }
 
 /* Decode the recommended minimum specific GPS/TRANSIT data. */
@@ -385,9 +411,11 @@
np->signal.status = SENSOR_S_OK;
np->latitude.status = SENSOR_S_OK;
np->longitude.status = SENSOR_S_OK;
+   np->speed.status = SENSOR_S_OK;
np->time.flags &= ~SENSOR_FINVALID;
np->latitude.flags &= ~SENSOR_FINVALID;
np->longitude.flags &= ~SENSOR_FINVALID;
+   np->speed.flags &= ~SENSOR_FINVALID;
break;
case 'V':   /*
 * The GPS indicates a warning status, do not add to
@@ -399,6 +427,7 @@
np->signal.status = SENSOR_S_CRIT;
np->latitude.status = SENSOR_S_WARN;
np->longitude.status = SENSOR_S_WARN;
+   np->speed.status = SENSOR_S_WARN;
break;
}
if (nmea_degrees(>latitude.value, fld[3], *fld[4] == 'S' ? 1 : 0))
@@ -406,6 +435,11 @@
if 

Re: NMEA: add more gps sensor values (2nd take)

2018-12-30 Thread Landry Breuil
On Sun, Dec 30, 2018 at 06:04:17AM -0700, Theo de Raadt wrote:
> Anyways, I haven't seen a specific consumer ready to use this
> information from sysctl.  I'm sure such programs exist and will be
> adapted to use sysctl (or a file, will we make it mode 600 by
> default?)  rather than some linux interface.  But I'm pretty sure I
> don't want a system call making this feature available to all my
> software, including some of the gigantic software I run which will
> communicate my whereabouts to the world, such that the information can
> be used against me.
> 
> In many ways this is similar to the video pledge for firefox.  firefox
> main-process wants to do EVERYTHING.  That diff is an attempt to
> continue enforce pledge "everything" in the main-process, by adding
> all missing features to pledge such that the main-process can be
> pledged (but really, pledge in name alone, since it really requests
> all features to work).
> 
> So if we add a security control feature to block access to sensors,
> will firefox want a way to enable GPS coordinate data so that it can
> give it to the cloud?
> 
> You will all sense I am becoming quite cynical about where this is going.

Well, this drifted far away from the original diff, but i'll reply
anyway since i'm feeling lucky. I'm mostly interested in it to record
offline traces of trips/hikes for my personal use. After all, that's why
i bought a gps, plugged it to my laptop, and i thought it would be
simpler to integrate the missing bits (speed/altitude) in the sensors
framework rather than running gpsd.

Let's be clear since you seem worried about that, i have no plans to
make firefox use that information.

It *could* be possible though if people show interest, provided that:

- one enables location sharing in firefox when a website asks for it:
  well, that also means you trust it to not share the location when you
said you dont want to. At that point, if you dont trust it, dont run it.
Do you trust your smartphone to disable the gps when you tell it so ?
Your laptop vendor to disable the webcam when you say so in the bios ?
Where is the line ?

- one implements the missing bits between firefox and geoclue
  (https://bugzilla.mozilla.org/show_bug.cgi?id=1063572) - so far nobody
showed interest upstream, and location is done through the public IP (as
the browser doesnt have access to the list of nearby wifi networks,
which is what's done on other OSes) cf
https://support.mozilla.org/en-US/kb/does-firefox-share-my-location-websites

- one implements the missing bits in geoclue to read location from
  hw.sensors.nmea. I had a quick look a while ago and never managed to
get anything out of geoclue.

If i had to choose and wanted to share my exact location with a website
(for $reasons), i'd rather do it through a passive gps device rather
than actively pinging google location services..

Landry



Re: NMEA: add more gps sensor values (2nd take)

2018-12-30 Thread Landry Breuil
On Sun, Dec 30, 2018 at 05:17:54AM -0700, Theo de Raadt wrote:
> I think you've gone overboard adding sensors.
> 
> Some of them aren't informing a fact about the world, but about the GPS device
> operation.
> 
> Therefore I think nsat, quality, hdop, vdop, pdop are irrelevant.  These
> are basically protocol-specific / device-specific "error bars".
> 
> I think you should delete those.

Sure, i dont mind. I'm mostly interested in adding speed & altitude to
lat/lon in the sensors. Others showed interest in other values.

> We should also discuss the privacy implications of allowing all local software
> to know the location of the user's machine, and then possibly broadcasting
> into the cloud.  I consider this similar to our recent decision to have
> the microphone disabled by default, and same thing with /dev/video0 being
> restricted access.  What is the plan for hiding this sensitive information?

Except that most devices around have a builtin microphone, all laptops
have builtin webcams, and none have a builtin gps device. When you buy
and plug a specific devices, you plan to make use of it..

Note that right now, if you plug a gps device, you need to be root (or
in dialer group) to directly read on /dev/cuaU0. You also need it to run
ldattach and make the information available via sensors. At that point,
i think the admin has made the choice to make this information
'available to userland' - which is right now lat/lon, and my diff only
proposes to add speed/altitude.

This is imo orthogonal to the privacy discussion, and i personally have
no plans to dig into adding knobs for restricting/allowing fine-grained
access to devices. But i'll gladly test diffs, be it for video or nmea.

Landry



Re: NMEA: add more gps sensor values (2nd take)

2018-12-30 Thread Landry Breuil
On Sat, Dec 29, 2018 at 12:45:07AM +0200, Paul Irofti wrote:
> > so now that the sensor types got adjusted, here's the tty_nmea.c diff
> > again, adding 7 new sensors:
> > - # satellites (raw)
> > - GPS/DGPS fix (or not) (raw)
> > - HDOP/VDOP/PDOP (raw)
> > - Altitude (m)
> > - Speed (m/s)
> 
> Yay sensors!
> 
> > i'd welcome eyes on the logic, and especially on the nmea_atoi()
> > implementation - it is inspired by nmea_degrees() just below but it
> > feels awkward. I can of course only add altitude/speed as a start..
> 
> I had a stab at it. Read ok, but I also tested it. If this is the
> desired output in the corner cases (e.g. ".1"), then its OK by me.

Yes, i had more or less done the same tests, and setting nmeadebug above
2 shows the actual values and their translation in the kernel buffer:
559.1 -> 559100 (altitude in mm)
2.79 -> 2790
1.11 -> 1110
2.56 -> 2560
0.039 -> 39 (speed in mm/s)

> > And testing with other nmea devices would be great ! So far i've tested
> > this in various static locations and recording data points when driving,
> > and it matched the gps data from my smartphone.
> 
> Unfortunately I don't have any devices (that I know of) to test.
> 
> The diff looks OK to me. The only thing that bugs me is a few scalar
> values that you use instead of defines. Mainly fld[] entries and vdop
> checks.

Right.. for the DOP warning threshold, added a #define NMEA_DOP_WARN. As
for the fld[] indexes, those are the position of the interesting value
in the NMEA sentence, so i dont think using #define for this is useful.
Adding comments and sentence examples before nmea_decode_gsa() &
nmea_decode_gga() might be better, as shown in the attached new diff.

While here i rechecked the NMEA spec for the GGA/GSA field definitions,
and understood why RMC could be 12 or 13 - a new field was added in NMEA
2.3 for some sentences only, per
http://www.catb.org/gpsd/NMEA.html#_sentence_mixes_and_nmea_variations
"In NMEA 2.3, several sentences (APB, BWC, BWR, GLL, RMA, RMB, RMC, VTG,
WCV, and XTE) got a new last field carrying the signal integrity
information needed by the FAA. " so this didnt apply to GSA/GGA.

Also attached is a poor try at updating nmea(4).
Index: nmea.4
===
RCS file: /cvs/src/share/man/man4/nmea.4,v
retrieving revision 1.26
diff -u -r1.26 nmea.4
--- nmea.4  2 Sep 2018 08:14:25 -   1.26
+++ nmea.4  30 Dec 2018 11:26:57 -
@@ -37,16 +37,19 @@
 maintains timedelta and position sensors using the NMEA data.
 The sensors will appear as nmea* in the list.
 The timedelta (nanoseconds difference between the received time
-information and the local time), and position (calculated
-latitude and longitude in degrees) can be accessed through the
+information and the local time), position (calculated latitude
+and longitude in degrees), altitude, speed and precision can be
+accessed through the
 .Xr sysctl 8
 interface.
 .Pp
 The
 .Nm
-line discipline decodes NMEA 0183
-Recommended Minimum Specific GPS/TRANSIT Data sentences.
-The time and date information and position are extracted.
+line discipline decodes the following NMEA 0183 sentences:
+.Bl -tag -width "RMCXX"
+.It RMC
+Recommended Minimum Specific GPS/TRANSIT Data.
+The time and date information, position and speed are extracted.
 The warning indication is used to provide the sensor status (see below).
 If the attached device sends the RMC message in the 13-field format,
 the operation mode of the GPS device is reported in the sensor description.
@@ -54,8 +57,16 @@
 is being used and tty timestamping has been turned on.
 Otherwise the sensor timestamp is taken when the initial `$' character of
 a message block is received from the NMEA device.
+.It GGA
+current fix data.
+The fix type (DGPS or GPS), amount of satellites used and altitude in meters 
are extracted.
+.It GSA
+precision data.
+The Positional (PDOP), Horizontal (HDOP) and Vertical (VDOP) dilution of
+precision are extracted. A value above 2 is considered not precise enough.
+.El
 .Pp
-RMC messages source are recognised by the first two characters of the
+Messages source are recognised by the first two characters of the NMEA
 sentence according to the following prefixes:
 .Pp
 .Bl -tag -width "X" -offset indent -compact
Index: tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.47
diff -u -r1.47 tty_nmea.c
--- tty_nmea.c  1 Sep 2018 06:09:26 -   1.47
+++ tty_nmea.c  30 Dec 2018 11:08:31 -
@@ -28,7 +28,11 @@
 
 #ifdef NMEA_DEBUG
 #define DPRINTFN(n, x) do { if (nmeadebug > (n)) printf x; } while (0)
-int nmeadebug = 0;
+int nmeadebug = 2;
+/*
+ * 1 = print interesting messages
+ * 2 = print all messages
+ */
 #else
 #define DPRINTFN(n, x)
 #endif
@@ -38,6 +42,7 @@
 
 #define NMEAMAX82
 #define MAXFLDS32
+#define NMEA_DOP_WARN  2000
 #ifdef NMEA_DEBUG
 #define 

Re: video(1) pledge (& updated kernel diff)

2018-12-30 Thread Landry Breuil
On Sat, Dec 29, 2018 at 09:30:22AM +0100, Sebastien Marie wrote:
> On Fri, Dec 28, 2018 at 09:41:06PM +0100, Landry Breuil wrote:
> > Hi,
> > 
> > so i've updated my 'video' class for pledge and also did an initial
> > naive pledging of xenocara/app/video:
> 
> just a small note: video(1) is the sole customer for ioctl(VIDIOC_*) in
> base. other potential customers are in ports.
> 
> personally, with such promise, I would be interested in pledging a SIP
> program (like telephony/baresip, but I didn't check if this program is
> designed well enough for be pledgeable). I will look at it in order to
> have a third program to check the relevance of the ioctls.
> 
> > as for the video(1) diff:
> > - fixed a typo
> > - renamed err to errs to avoid shadowing err()
> 
> I would separate the addition of pledge(2) and unrelated fixes.

Right, two separated diffs attached for this.

> I think you pledged too early.
> 
> "dns unix" are here for XOpenDisplay(), but if the display is remote,
> "inet" could be needed too. Even if Xv isn't possible on remote display,
> XOpenDisplay() will be killed by pledge.
> 
> So ideally, pledge(2) should occurs later.

Yup, after some discussions, moving the pledge calls after setup()
(which does the file & xv opening) allows to only need stdio in the -i
case, and 'stdio rpath video' in the others. i thought rpath could be
dropped but in my testing with video -vvv -O i had a crash upon some
keys where x tried to open /usr/X11R6/share/X11/locale/locale.alias..

in addition we can also pledge the video -q case which needs
stdio/rpath/wpath/video.


> > +#define PLEDGE_VIDEO   0x00200800ULL   /* video ioctls */
> 
> I suspect a copy/paste error: PLEDGE_VIDEO contains 2 bits sets (there is a 
> '8' after the '2').
> and space vs tab between the name and the value.

Yes that was a typo, i think when i originally started this the next
available define was 8000ULL :)

Landry
Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.25
diff -u -r1.25 video.c
--- video.c 9 Apr 2018 18:16:44 -   1.25
+++ video.c 30 Dec 2018 09:39:21 -
@@ -1854,7 +1854,7 @@
struct xdsp *x = 
const char *errstr;
size_t len;
-   int ch, err = 0;
+   int ch, errs = 0;
 
bzero(, sizeof(struct video));
 
@@ -1872,21 +1872,21 @@
x->cur_adap = strtonum(optarg, 0, 4, );
if (errstr != NULL) {
warnx("Xv adaptor '%s' is %s", optarg, errstr);
-   err++;
+   errs++;
}
break;
case 'e':
vid.enc = find_enc(optarg);
if (vid.enc >= ENC_LAST) {
warnx("encoding '%s' is invalid", optarg);
-   err++;
+   errs++;
}
break;
case 'f':
len = strlcpy(d->path, optarg, sizeof(d->path));
if (len >= sizeof(d->path)) {
warnx("file path is too long: %s", optarg);
-   err++;
+   errs++;
}
break;
case 'g':
@@ -1894,8 +1894,8 @@
break;
case 'i':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
-   warnx("only one input or ouput file allowed");
-   err++;
+   warnx("only one input or output file allowed");
+   errs++;
} else {
vid.mode = (vid.mode & ~M_IN_DEV) | M_IN_FILE;
vid.mmap_on = 0; /* mmap mode does not work for 
files */
@@ -1904,15 +1904,15 @@
if (len >= sizeof(vid.iofile)) {
warnx("input path is too long: %s",
optarg);
-   err++;
+   errs++;
}
}
break;
case 'o':
case 'O':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
-   warnx("only one input or ouput file allowed");
-   err++;
+ 

NMEA: add more gps sensor values (2nd take)

2018-12-28 Thread Landry Breuil
Hi,

so now that the sensor types got adjusted, here's the tty_nmea.c diff
again, adding 7 new sensors:
- # satellites (raw)
- GPS/DGPS fix (or not) (raw)
- HDOP/VDOP/PDOP (raw)
- Altitude (m)
- Speed (m/s)

i'd welcome eyes on the logic, and especially on the nmea_atoi()
implementation - it is inspired by nmea_degrees() just below but it
feels awkward. I can of course only add altitude/speed as a start..

And testing with other nmea devices would be great ! So far i've tested
this in various static locations and recording data points when driving,
and it matched the gps data from my smartphone.

Landry
Index: tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.47
diff -u -r1.47 tty_nmea.c
--- tty_nmea.c  1 Sep 2018 06:09:26 -   1.47
+++ tty_nmea.c  28 Dec 2018 20:47:20 -
@@ -29,6 +29,10 @@
 #ifdef NMEA_DEBUG
 #define DPRINTFN(n, x) do { if (nmeadebug > (n)) printf x; } while (0)
 int nmeadebug = 0;
+/*
+ * 1 = print interesting messages
+ * 2 = print all messages
+ */
 #else
 #define DPRINTFN(n, x)
 #endif
@@ -52,6 +56,13 @@
struct ksensor  signal; /* signal status */
struct ksensor  latitude;
struct ksensor  longitude;
+   struct ksensor  altitude;
+   struct ksensor  speed;
+   struct ksensor  nsat;
+   struct ksensor  quality;
+   struct ksensor  hdop;
+   struct ksensor  vdop;
+   struct ksensor  pdop;
struct ksensordev   timedev;
struct timespec ts; /* current timestamp */
struct timespec lts;/* timestamp of last '$' seen */
@@ -70,6 +81,8 @@
 /* NMEA decoding */
 void   nmea_scan(struct nmea *, struct tty *);
 void   nmea_gprmc(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void   nmea_decode_gsa(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void   nmea_decode_gga(struct nmea *, struct tty *, char *fld[], int fldcnt);
 
 /* date and time conversion */
 intnmea_date_to_nano(char *s, int64_t *nano);
@@ -77,6 +90,7 @@
 
 /* longitude and latitude conversion */
 intnmea_degrees(int64_t *dst, char *src, int neg);
+intnmea_atoi(int64_t *dst, char *src, int decimal);
 
 /* degrade the timedelta sensor */
 void   nmea_timeout(void *);
@@ -126,6 +140,55 @@
strlcpy(np->longitude.desc, "Longitude", sizeof(np->longitude.desc));
sensor_attach(>timedev, >longitude);
 
+   np->altitude.type = SENSOR_DISTANCE;
+   np->altitude.status = SENSOR_S_UNKNOWN;
+   np->altitude.flags = SENSOR_FINVALID;
+   np->altitude.value = 0;
+   strlcpy(np->altitude.desc, "Altitude", sizeof(np->altitude.desc));
+   sensor_attach(>timedev, >altitude);
+
+   np->speed.type = SENSOR_VELOCITY;
+   np->speed.status = SENSOR_S_UNKNOWN;
+   np->speed.flags = SENSOR_FINVALID;
+   np->speed.value = 0;
+   strlcpy(np->speed.desc, "Ground speed", sizeof(np->speed.desc));
+   sensor_attach(>timedev, >speed);
+
+   np->nsat.type = SENSOR_INTEGER;
+   np->nsat.status = SENSOR_S_UNKNOWN;
+   np->nsat.flags = SENSOR_FINVALID;
+   np->nsat.value = 0;
+   strlcpy(np->nsat.desc, "Nb satellites", sizeof(np->nsat.desc));
+   sensor_attach(>timedev, >nsat);
+
+   np->quality.type = SENSOR_INTEGER;
+   np->quality.status = SENSOR_S_UNKNOWN;
+   np->quality.flags = SENSOR_FINVALID;
+   np->quality.value = 0;
+   strlcpy(np->quality.desc, "Fix Quality", sizeof(np->quality.desc));
+   sensor_attach(>timedev, >quality);
+
+   np->hdop.type = SENSOR_INTEGER;
+   np->hdop.status = SENSOR_S_UNKNOWN;
+   np->hdop.flags = SENSOR_FINVALID;
+   np->hdop.value = 0;
+   strlcpy(np->hdop.desc, "HDOP", sizeof(np->hdop.desc));
+   sensor_attach(>timedev, >hdop);
+
+   np->vdop.type = SENSOR_INTEGER;
+   np->vdop.status = SENSOR_S_UNKNOWN;
+   np->vdop.flags = SENSOR_FINVALID;
+   np->vdop.value = 0;
+   strlcpy(np->vdop.desc, "VDOP", sizeof(np->vdop.desc));
+   sensor_attach(>timedev, >vdop);
+
+   np->pdop.type = SENSOR_INTEGER;
+   np->pdop.status = SENSOR_S_UNKNOWN;
+   np->pdop.flags = SENSOR_FINVALID;
+   np->pdop.value = 0;
+   strlcpy(np->pdop.desc, "PDOP", sizeof(np->pdop.desc));
+   sensor_attach(>timedev, >pdop);
+
np->sync = 1;
tp->t_sc = (caddr_t)np;
 
@@ -182,7 +245,7 @@
np->ts.tv_nsec = ts.tv_nsec;
 
 #ifdef NMEA_DEBUG
-   if (nmeadebug > 0) {
+   if (nmeadebug > 1) {
linesw[TTYDISC].l_rint('[', tp);
linesw[TTYDISC].l_rint('0' + np->gapno++, tp);
linesw[TTYDISC].l_rint(']', tp);
@@ -261,7 +324,7 @@
}
 
/*
-* we only look at the RMC message, which can come from different 
'talkers',
+* 

video(1) pledge (& updated kernel diff)

2018-12-28 Thread Landry Breuil
Hi,

so i've updated my 'video' class for pledge and also did an initial
naive pledging of xenocara/app/video:

the kernel diff is the same subset as the diff from some months ago,
except that it adds:
- VIDIOC_QUERYCTRL/VIDIOC_G_CTRL/VIDIOC_S_CTRL: those 3 are used by
  video(1) to set gamma/contrast/etc controls on the device on the fly
(via A/a, B/b, C/c - etc when running)
- VIDIOC_TRY_FMT, used by
  
https://searchfox.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/linux/device_info_linux.cc#418
since https://bugzilla.mozilla.org/show_bug.cgi?id=1376873 (yes, i
checked with ktrace, all the other ioctls are still used)

with that kernel diff, i'm able to record video in firefox with the
video pledge added to the main process.

as for the video(1) diff:
- fixed a typo
- renamed err to errs to avoid shadowing err()

and i've identified 4 main modes:
- -o needs to write to the fs (so wpath cpath) and read from the device
  (so video)
- -O needs the same subset and dns unix in addition, as it talks to Xv
- -i only needs rpath & dns/unix for xv, no access to the video device
  needed
- no option needs dns unix for xv, video for ioctls, and wpath as the
  video device is open with O_RDWR.

tested those 4 modes without aborts so far, video -q exits before
reaching pledge. Not sure if those pledge calls are at the best spot,
but that's a start.

I know video(1) sucks and doesnt work directly with Xv with most modern
builtin devices on laptops as they often dont support the few encodings
supported by video(1) for Xv, but with my old-school cameras lying
around it works fine.

Landry
? ktrace.out
? video
? video.d
Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.25
diff -u -r1.25 video.c
--- video.c 9 Apr 2018 18:16:44 -   1.25
+++ video.c 28 Dec 2018 20:22:36 -
@@ -1854,7 +1854,7 @@
struct xdsp *x = 
const char *errstr;
size_t len;
-   int ch, err = 0;
+   int ch, errs = 0;
 
bzero(, sizeof(struct video));
 
@@ -1872,21 +1872,21 @@
x->cur_adap = strtonum(optarg, 0, 4, );
if (errstr != NULL) {
warnx("Xv adaptor '%s' is %s", optarg, errstr);
-   err++;
+   errs++;
}
break;
case 'e':
vid.enc = find_enc(optarg);
if (vid.enc >= ENC_LAST) {
warnx("encoding '%s' is invalid", optarg);
-   err++;
+   errs++;
}
break;
case 'f':
len = strlcpy(d->path, optarg, sizeof(d->path));
if (len >= sizeof(d->path)) {
warnx("file path is too long: %s", optarg);
-   err++;
+   errs++;
}
break;
case 'g':
@@ -1894,8 +1894,8 @@
break;
case 'i':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
-   warnx("only one input or ouput file allowed");
-   err++;
+   warnx("only one input or output file allowed");
+   errs++;
} else {
vid.mode = (vid.mode & ~M_IN_DEV) | M_IN_FILE;
vid.mmap_on = 0; /* mmap mode does not work for 
files */
@@ -1904,15 +1904,15 @@
if (len >= sizeof(vid.iofile)) {
warnx("input path is too long: %s",
optarg);
-   err++;
+   errs++;
}
}
break;
case 'o':
case 'O':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
-   warnx("only one input or ouput file allowed");
-   err++;
+   warnx("only one input or output file allowed");
+   errs++;
} else {
vid.mode |= M_OUT_FILE;
if (ch != 'O')
@@ -1922,7 +1922,7 @@
if (len >= sizeof(vid.iofile)) {
warnx("output path is too long: %s",
optarg);
-   err++;
+   

Re: undocumented nfs mount option "intr"

2018-12-20 Thread Landry Breuil
On Thu, Dec 20, 2018 at 09:26:33AM +0100, Solene Rapenne wrote:
> Hi
> 
> fstab(5) has an example for a nfs mountpoint using the "intr" option.
> 
> That option isn't documented in mount(8) or mount_nfs(8) and in fact, I've not
> been able to find it anywhere. What is it doing?

I think it's the fstab shortcut for -i option in mount_nfs(8).



add velocity sensor type, change distance unit

2018-12-08 Thread Landry Breuil
Hi,

followup on a diff from last month, updated to -current:

- adds SENSOR_VELOCITY, internal unit um/s, display unit m/s
- changes SENSOR_DISTANCE to display as meters, with 3 decimals

which gives:
hw.sensors.nmea0.distance0=335.600 m (Altitude), OK
hw.sensors.nmea0.velocity0=18.337 m/s (Ground speed), OK

should the LAST-UPDATED field in the snmp sensors mib be updated to the
commit date ?

looking for okays so that i can make progress on the nmea changes for
altitude/speed.

Landry
Index: sys/sys/sensors.h
===
RCS file: /cvs/src/sys/sys/sensors.h,v
retrieving revision 1.35
diff -u -r1.35 sensors.h
--- sys/sys/sensors.h   8 Apr 2017 04:06:01 -   1.35
+++ sys/sys/sensors.h   8 Dec 2018 09:51:00 -
@@ -52,6 +52,7 @@
SENSOR_DISTANCE,/* distance (uMeter) */
SENSOR_PRESSURE,/* pressure (mPa) */
SENSOR_ACCEL,   /* acceleration (u m/s^2) */
+   SENSOR_VELOCITY,/* velocity (u m/s) */
SENSOR_MAX_TYPES
 };
 
@@ -78,6 +79,7 @@
"distance",
"pressure",
"acceleration",
+   "velocity",
"undefined"
 };
 #endif /* !_KERNEL */
Index: share/snmp/OPENBSD-SENSORS-MIB.txt
===
RCS file: /cvs/src/share/snmp/OPENBSD-SENSORS-MIB.txt,v
retrieving revision 1.6
diff -u -r1.6 OPENBSD-SENSORS-MIB.txt
--- share/snmp/OPENBSD-SENSORS-MIB.txt  2 Sep 2016 12:17:33 -   1.6
+++ share/snmp/OPENBSD-SENSORS-MIB.txt  8 Dec 2018 09:51:00 -
@@ -26,7 +26,7 @@
FROM SNMPv2-CONF;
 
 sensorsMIBObjects MODULE-IDENTITY
-   LAST-UPDATED "20120920Z"
+   LAST-UPDATED "20181103Z"
ORGANIZATION "OpenBSD"
CONTACT-INFO
"Editor:Reyk Floeter
@@ -39,6 +39,9 @@
DESCRIPTION
"The MIB module for gathering information from
OpenBSD's kernel sensor framework."
+   REVISION "20181103Z"
+   DESCRIPTION
+   "Add new sensor types."
REVISION "20120920Z"
DESCRIPTION
"Add new sensor types."
@@ -136,7 +139,8 @@
angle(17),
distance(18),
pressure(19),
-   accel(20)
+   accel(20),
+   velocity(21)
}
MAX-ACCESS  read-only
STATUS  current
Index: usr.sbin/snmpd/mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.91
diff -u -r1.91 mib.c
--- usr.sbin/snmpd/mib.c31 Aug 2018 05:20:36 -  1.91
+++ usr.sbin/snmpd/mib.c8 Dec 2018 09:51:06 -
@@ -2658,7 +2658,7 @@
 static const char * const sensor_unit_s[SENSOR_MAX_TYPES + 1] = {
"degC", "RPM", "V DC", "V AC", "Ohm", "W", "A", "Wh", "Ah",
"", "", "%", "lx", "", "sec", "%RH", "Hz", "degree", 
-   "mm", "Pa", "m/s^2", ""
+   "m", "Pa", "m/s^2", "m/s", ""
 };
 
 const char *
@@ -2690,6 +2690,8 @@
case SENSOR_LUX:
case SENSOR_FREQ:
case SENSOR_ACCEL:
+   case SENSOR_VELOCITY:
+   case SENSOR_DISTANCE:
ret = asprintf(, "%.2f", s->value / 100.0);
break;
case SENSOR_INDICATOR:
@@ -2699,7 +2701,6 @@
case SENSOR_HUMIDITY:
ret = asprintf(, "%.2f", s->value / 1000.0);
break;
-   case SENSOR_DISTANCE:
case SENSOR_PRESSURE:
ret = asprintf(, "%.2f", s->value / 1000.0);
break;
Index: usr.sbin/sensorsd/sensorsd.c
===
RCS file: /cvs/src/usr.sbin/sensorsd/sensorsd.c,v
retrieving revision 1.62
diff -u -r1.62 sensorsd.c
--- usr.sbin/sensorsd/sensorsd.c22 Oct 2018 16:20:09 -  1.62
+++ usr.sbin/sensorsd/sensorsd.c8 Dec 2018 09:51:06 -
@@ -692,7 +692,7 @@
snprintf(fbuf, RFBUFSIZ, "%lld", value);
break;
case SENSOR_DISTANCE:
-   snprintf(fbuf, RFBUFSIZ, "%.2f mm", value / 1000.0);
+   snprintf(fbuf, RFBUFSIZ, "%.3f m", value / 100.0);
break;
case SENSOR_PRESSURE:
snprintf(fbuf, RFBUFSIZ, "%.2f Pa", value / 1000.0);
@@ -700,6 +700,9 @@
case SENSOR_ACCEL:
snprintf(fbuf, RFBUFSIZ, "%2.4f m/s^2", value / 100.0);
break;
+   case SENSOR_VELOCITY:
+   snprintf(fbuf, RFBUFSIZ, "%4.3f m/s", value / 100.0);
+   break;
default:
snprintf(fbuf, RFBUFSIZ, "%lld ???", value);
}
@@ -813,13 +816,14 @@
case SENSOR_LUX:
case SENSOR_FREQ:
case SENSOR_ACCEL:
+   case SENSOR_DISTANCE:
+   case SENSOR_VELOCITY:
 

Re: pvclock(4)

2018-11-22 Thread Landry Breuil
On Thu, Nov 22, 2018 at 05:24:10PM +0100, Landry Breuil wrote:
> On Thu, Nov 22, 2018 at 07:44:01AM -0800, Mike Larkin wrote:
> > On Thu, Nov 22, 2018 at 04:37:49PM +0100, Reyk Floeter wrote:
> > > On Mon, Nov 19, 2018 at 01:12:46PM +0100, Reyk Floeter wrote:
> > > > the attached diff is another attempt at implementing a pvclock(4)
> > > > guest driver.  This improves the clock on KVM and replaces the need
> > > > for using the VM-expensive acpihpet(4).
> > > > 
> > > 
> > > So far I only got positive reports.  Where are the problems? ;)
> > > 
> > > Otherwise: OK?
> > > 
> > > Reyk
> > > 
> > 
> > Reads ok. One question - you mention in pvclock.c that this is supported
> > on i386 and amd64 but I only see GENERIC changes for amd64?
> > 
> > ok mlarkin in any case, but I'd either add GENERIC changes for i386 or
> > make this for sure amd64 only.
> 
> I'm giving it a shot on my i386 vm.

Also seems to work fine there, i'm stressing the vm (well,
unpacking tarball/building firefox) and ntpd still manages to reduce the drift:


[17:49] c32:~/ $while sleep 10 ; do ntpctl -sa|grep clock ; date ; done 
  
4/4 peers valid, constraint offset -1s, clock unsynced, clock offset is 
404.382ms 
Thu Nov 22 17:49:49 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced, clock offset is 
354.382ms 
Thu Nov 22 17:49:59 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced, clock offset is 
304.382ms 
Thu Nov 22 17:50:09 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced, clock offset is 
254.382ms 
Thu Nov 22 17:50:19 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced, clock offset is 
204.382ms 
Thu Nov 22 17:50:29 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced, clock offset is 
154.382ms 
Thu Nov 22 17:50:39 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced, clock offset is 
104.382ms 
Thu Nov 22 17:50:49 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced, clock offset is 
54.382ms  
Thu Nov 22 17:50:59 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced
Thu Nov 22 17:51:09 CET 2018
4/4 peers valid, constraint offset -1s, clock unsynced
Thu Nov 22 17:51:19 CET 2018
4/4 peers valid, constraint offset -1s, clock synced, stratum 3
Thu Nov 22 17:51:29 CET 2018

dunno how valid a testcase this is

dmesg below, this time emulating a QEMU Standard PC (i440FX + PIIX, 1996)

Landry
OpenBSD 6.4-current (GENERIC.MP) #4: Thu Nov 22 17:40:05 CET 2018
landry@c32.proxmox2:/usr/src/sys/arch/i386/compile/GENERIC.MP
real mem  = 3220561920 (3071MB)
avail mem = 3146719232 (3000MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: date 06/23/99, BIOS32 rev. 0 @ 0xfd2b1, SMBIOS rev. 2.8 @ 
0xf5980 (10 entries)
bios0: vendor SeaBIOS version 
"rel-1.11.0-0-g63451fca13-prebuilt.qemu-project.org" date 04/01/2014
bios0: QEMU Standard PC (i440FX + PIIX, 1996)
acpi0 at bios0: rev 0
acpi0: sleep states S3 S4 S5
acpi0: tables DSDT FACP APIC HPET
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Common KVM processor ("GenuineIntel" 686-class) 2.94 GHz, 0f-06-01
cpu0: 
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,CX16,x2APIC,HV,NXE,LONG,LAHF,MELTDOWN
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 999MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Common KVM processor ("GenuineIntel" 686-class) 2.94 GHz, 0f-06-01
cpu1: 
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,CX16,x2APIC,HV,NXE,LONG,LAHF,MELTDOWN
ioapic0 at mainbus0: apid 0 pa 0xfec0, version 11, 24 pins
acpihpet0 at acpi0: 1 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpicpu0 at acpi0: C1(@1 halt!)
acpicpu1 at acpi0: C1(@1 halt!)
"ACPI0006" at acpi0 not configured
"PNP0A03" at acpi0 not configured
acpicmos0 at acpi0
"PNP0A06" at acpi0 not configured
"PNP0A06" at acpi0 not configured
"PNP0A06" at acpi0 not configured
"QEMU0002" at acpi0 not configured
"ACPI0010" at acpi0 not configured
bios0: ROM list: 0xc

Re: pvclock(4)

2018-11-22 Thread Landry Breuil
On Thu, Nov 22, 2018 at 07:44:01AM -0800, Mike Larkin wrote:
> On Thu, Nov 22, 2018 at 04:37:49PM +0100, Reyk Floeter wrote:
> > On Mon, Nov 19, 2018 at 01:12:46PM +0100, Reyk Floeter wrote:
> > > the attached diff is another attempt at implementing a pvclock(4)
> > > guest driver.  This improves the clock on KVM and replaces the need
> > > for using the VM-expensive acpihpet(4).
> > > 
> > 
> > So far I only got positive reports.  Where are the problems? ;)
> > 
> > Otherwise: OK?
> > 
> > Reyk
> > 
> 
> Reads ok. One question - you mention in pvclock.c that this is supported
> on i386 and amd64 but I only see GENERIC changes for amd64?
> 
> ok mlarkin in any case, but I'd either add GENERIC changes for i386 or
> make this for sure amd64 only.

I'm giving it a shot on my i386 vm.



Re: pvclock(4)

2018-11-20 Thread Landry Breuil
On Mon, Nov 19, 2018 at 01:12:46PM +0100, Reyk Floeter wrote:
> Hi,
> 
> the attached diff is another attempt at implementing a pvclock(4)
> guest driver.  This improves the clock on KVM and replaces the need
> for using the VM-expensive acpihpet(4).
> 
> While pvclock(4) is available on KVM, Xen, Hyper-V (in a modified
> form), it currently only attaches under KVM:
> 
> pvbus0 at mainbus0: KVM
> pvclock0 at pvbus0

Works fine on my proxmox 5.2 / KVM 2.11 dpb build cluster emulating 6x
"QEMU Standard PC (Q35 + ICH9, 2009)". Before i had to restart ntpd
every 5mn on each hosts, time would drift like crazy under build load.
With pvclock it seems the slave clocks are stable and ntpd stays synced.

kern.timecounter.tick=1
kern.timecounter.timestepwarnings=0
kern.timecounter.hardware=pvclock0
kern.timecounter.choice=i8254(0) pvclock0(1500) acpihpet0(1000) 
acpitimer0(1000) dummy(-100)

Landry
OpenBSD 6.4-current (GENERIC.MP) #0: Tue Nov 20 18:52:03 CET 2018
landry@c64.proxmox2:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 4276924416 (4078MB)
avail mem = 4138016768 (3946MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf5970 (10 entries)
bios0: vendor SeaBIOS version 
"rel-1.11.0-0-g63451fca13-prebuilt.qemu-project.org" date 04/01/2014
bios0: QEMU Standard PC (Q35 + ICH9, 2009)
acpi0 at bios0: rev 0
acpi0: sleep states S3 S4 S5
acpi0: tables DSDT FACP APIC HPET MCFG
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Common KVM processor, 2933.90 MHz, 0f-06-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,CX16,x2APIC,HV,NXE,LONG,LAHF,MELTDOWN
cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 64b/line 
16-way L2 cache
cpu0: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 999MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Common KVM processor, 2933.54 MHz, 0f-06-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,CX16,x2APIC,HV,NXE,LONG,LAHF,MELTDOWN
cpu1: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 64b/line 
16-way L2 cache
cpu1: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu1: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu1: smt 0, core 1, package 0
ioapic0 at mainbus0: apid 0 pa 0xfec0, version 11, 24 pins
acpihpet0 at acpi0: 1 Hz
acpimcfg0 at acpi0
acpimcfg0: addr 0xb000, bus 0-255
acpiprt0 at acpi0: bus 0 (PCI0)
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
no _STA method
acpicpu0 at acpi0: C1(@1 halt!)
acpicpu1 at acpi0: C1(@1 halt!)
"ACPI0006" at acpi0 not configured
acpipci0 at acpi0 PCI0: 0x 0x0011 0x0001
acpicmos0 at acpi0
"PNP0A06" at acpi0 

Re: NMEA: add more gps sensor values (altitude, precision...)

2018-11-04 Thread Landry Breuil
On Sat, Nov 03, 2018 at 06:21:38PM -0400, Ted Unangst wrote:
> Mark Kettenis wrote:
> > SENSOR_VELOCITY would make sense.  It should be m/s with some
> > appropriate scaling.  We don't need to represent speeds higer than
> > 299792458 m/s, so micrometers per second would be a good choice.
> 
> is it reasonable to use mm/s so that distance and velocity are consistent?

We're talking about the internal unit here, and afaict using um/s would
be consistent with distance which is in um per sensors.h:

SENSOR_DISTANCE,/* distance (uMeter) */
SENSOR_ACCEL,   /* acceleration (u m/s^2) */
SENSOR_VELOCITY,/* velocity (u m/s) */



Re: add altitude & speed sensor types

2018-11-03 Thread Landry Breuil
On Sat, Nov 03, 2018 at 08:40:33PM +0100, Mark Kettenis wrote:
> > Date: Sat, 3 Nov 2018 20:09:32 +0100
> > From: Landry Breuil 
> > 
> > On Sat, Nov 03, 2018 at 05:27:03PM +0100, Landry Breuil wrote:
> > > Hi,
> > > 
> > > complementary to the previous diff, adds altitude sensor type (in
> > > meters) and speed (in m/s, as it seems that's the common denominator and
> > > most accepted unit, even if it doesnt 'mean' anything to humans...)
> > > i still have to ponder about the decimals & intervals..
> > 
> > New diff after feedback from kettenis@, this one:
> > - adds SENSOR_VELOCITY, internal unit um/s, display unit m/s
> > - changes SENSOR_DISTANCE to display as meters, i kept 5 decimals
> 
> That's a strange number.  3 decimals would be more reasonable I'd say.

Well that was only in case there's a proximity sensor with
sub-millimeter precision... but 3 is fine for me too.



Re: NMEA: add more gps sensor values (altitude, precision, speed, #satellites...)

2018-11-03 Thread Landry Breuil
On Sat, Nov 03, 2018 at 05:00:47PM +0100, Landry Breuil wrote:
> Hi,

New diff adding speed and # of satellites, uses SENSOR_VELOCITY as
posted in the previous diff, and relies on SENSOR_DISTANCE being shown
as meters. I redid the message type matching to account for
constellation types which was commented out in the previous diff. More
proofreading welcome, especially in the conversions and the handrolled
atoi implem...

That gives:

nmea0.indicator0   OnOKSignal
nmea0.raw0  5 rawOKNb satellites
nmea0.raw1  1 rawOKGPS fix
nmea0.raw2   2030 raw WARNING  HDOP
nmea0.raw3   2330 raw WARNING  VDOP
nmea0.raw4   3080 raw WARNING  PDOP
nmea0.timedelta030.311 msOKGPS autonomous
nmea0.angle0  xx. degreesOKLatitude
nmea0.angle1   z. degreesOKLongitude
nmea0.distance0   381.0 mOKAltitude
nmea0.velocity0 0.319 m/sOKGround speed

I need to do more velocity testing when moving.. as GPS is not reliable
when *not* moving :)

When there's no signal (ie, staying inside without satellite coverage)

nmea0.indicator0  Off CRITICAL Signal
nmea0.raw0  0 rawOKNb satellites
nmea0.raw1  0 raw CRITICAL Invalid
nmea0.raw2  0 raw WARNING  HDOP
nmea0.raw3  0 raw WARNING  VDOP
nmea0.raw4  0 raw WARNING  PDOP
nmea0.timedelta027.195 msOKGPS invalid
nmea0.angle0  xx. degrees WARNING  Latitude
nmea0.angle1   z. degrees WARNING  Longitude
nmea0.distance0 0.0 m WARNING  Altitude
nmea0.velocity0 0.000 m/s WARNING  Ground speed

Most statuses should just be CRITICAL, and i think in that case the position
is 'last known'.

Landry
Index: kern/tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.47
diff -u -r1.47 tty_nmea.c
--- kern/tty_nmea.c 1 Sep 2018 06:09:26 -   1.47
+++ kern/tty_nmea.c 3 Nov 2018 19:08:47 -
@@ -29,6 +29,10 @@
 #ifdef NMEA_DEBUG
 #define DPRINTFN(n, x) do { if (nmeadebug > (n)) printf x; } while (0)
 int nmeadebug = 0;
+/*
+ * 1 = print interesting messages
+ * 2 = print all messages
+ */
 #else
 #define DPRINTFN(n, x)
 #endif
@@ -52,6 +56,13 @@
struct ksensor  signal; /* signal status */
struct ksensor  latitude;
struct ksensor  longitude;
+   struct ksensor  altitude;
+   struct ksensor  speed;
+   struct ksensor  nsat;
+   struct ksensor  quality;
+   struct ksensor  hdop;
+   struct ksensor  vdop;
+   struct ksensor  pdop;
struct ksensordev   timedev;
struct timespec ts; /* current timestamp */
struct timespec lts;/* timestamp of last '$' seen */
@@ -70,6 +81,8 @@
 /* NMEA decoding */
 void   nmea_scan(struct nmea *, struct tty *);
 void   nmea_gprmc(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void   nmea_decode_gsa(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void   nmea_decode_gga(struct nmea *, struct tty *, char *fld[], int fldcnt);
 
 /* date and time conversion */
 intnmea_date_to_nano(char *s, int64_t *nano);
@@ -77,6 +90,7 @@
 
 /* longitude and latitude conversion */
 intnmea_degrees(int64_t *dst, char *src, int neg);
+intnmea_atoi(int64_t *dst, char *src, int decimal);
 
 /* degrade the timedelta sensor */
 void   nmea_timeout(void *);
@@ -126,6 +140,55 @@
strlcpy(np->longitude.desc, "Longitude", sizeof(np->longitude.desc));
sensor_attach(>timedev, >longitude);
 
+   np->altitude.type = SENSOR_DISTANCE;
+   np->altitude.status = SENSOR_S_UNKNOWN;
+   np->altitude.flags = SENSOR_FINVALID;
+   np->altitude.value = 0;
+   strlcpy(np->altitude.desc, "Altitude", sizeof(np->altitude.desc));
+   sensor_attach(>timedev, >altitude);
+
+   np->speed.type = SENSOR_VELOCITY;
+   np->speed.status = SENSOR_S_UNKNOWN;
+   np->speed.flags = SENSOR_FINVALID;
+   np->speed.value = 0;
+   strlcpy(np->speed.desc, "Ground speed", sizeof(np->speed.desc));
+   sensor_attach(>timedev, >speed);
+
+   np->nsat.type = SENSOR_INTEGER;
+   np->nsat.status = SENS

Re: add altitude & speed sensor types

2018-11-03 Thread Landry Breuil
On Sat, Nov 03, 2018 at 05:27:03PM +0100, Landry Breuil wrote:
> Hi,
> 
> complementary to the previous diff, adds altitude sensor type (in
> meters) and speed (in m/s, as it seems that's the common denominator and
> most accepted unit, even if it doesnt 'mean' anything to humans...)
> i still have to ponder about the decimals & intervals..

New diff after feedback from kettenis@, this one:
- adds SENSOR_VELOCITY, internal unit um/s, display unit m/s
- changes SENSOR_DISTANCE to display as meters, i kept 5 decimals

I think my math should be good, but more eyes are always welcome :)

Landry
Index: sys/sys/sensors.h
===
RCS file: /cvs/src/sys/sys/sensors.h,v
retrieving revision 1.35
diff -u -r1.35 sensors.h
--- sys/sys/sensors.h   8 Apr 2017 04:06:01 -   1.35
+++ sys/sys/sensors.h   3 Nov 2018 18:11:53 -
@@ -52,6 +52,7 @@
SENSOR_DISTANCE,/* distance (uMeter) */
SENSOR_PRESSURE,/* pressure (mPa) */
SENSOR_ACCEL,   /* acceleration (u m/s^2) */
+   SENSOR_VELOCITY,/* velocity (u m/s) */
SENSOR_MAX_TYPES
 };
 
@@ -78,6 +79,7 @@
"distance",
"pressure",
"acceleration",
+   "velocity",
"undefined"
 };
 #endif /* !_KERNEL */
Index: share/snmp/OPENBSD-SENSORS-MIB.txt
===
RCS file: /cvs/src/share/snmp/OPENBSD-SENSORS-MIB.txt,v
retrieving revision 1.6
diff -u -r1.6 OPENBSD-SENSORS-MIB.txt
--- share/snmp/OPENBSD-SENSORS-MIB.txt  2 Sep 2016 12:17:33 -   1.6
+++ share/snmp/OPENBSD-SENSORS-MIB.txt  3 Nov 2018 18:11:53 -
@@ -26,7 +26,7 @@
FROM SNMPv2-CONF;
 
 sensorsMIBObjects MODULE-IDENTITY
-   LAST-UPDATED "20120920Z"
+   LAST-UPDATED "20181103Z"
ORGANIZATION "OpenBSD"
CONTACT-INFO
"Editor:Reyk Floeter
@@ -39,6 +39,9 @@
DESCRIPTION
"The MIB module for gathering information from
OpenBSD's kernel sensor framework."
+   REVISION "20181103Z"
+   DESCRIPTION
+   "Add new sensor types."
REVISION "20120920Z"
DESCRIPTION
"Add new sensor types."
@@ -136,7 +139,8 @@
angle(17),
distance(18),
pressure(19),
-   accel(20)
+   accel(20),
+   velocity(21)
}
MAX-ACCESS  read-only
STATUS  current
Index: usr.sbin/snmpd/mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.85
diff -u -r1.85 mib.c
--- usr.sbin/snmpd/mib.c18 Dec 2017 05:51:53 -  1.85
+++ usr.sbin/snmpd/mib.c3 Nov 2018 18:11:54 -
@@ -2617,7 +2617,7 @@
 static const char * const sensor_unit_s[SENSOR_MAX_TYPES + 1] = {
"degC", "RPM", "V DC", "V AC", "Ohm", "W", "A", "Wh", "Ah",
"", "", "%", "lx", "", "sec", "%RH", "Hz", "degree", 
-   "mm", "Pa", "m/s^2", ""
+   "m", "Pa", "m/s^2", "m/s", ""
 };
 
 const char *
@@ -2649,6 +2649,8 @@
case SENSOR_LUX:
case SENSOR_FREQ:
case SENSOR_ACCEL:
+   case SENSOR_VELOCITY:
+   case SENSOR_DISTANCE:
ret = asprintf(, "%.2f", s->value / 100.0);
break;
case SENSOR_INDICATOR:
@@ -2658,7 +2660,6 @@
case SENSOR_HUMIDITY:
ret = asprintf(, "%.2f", s->value / 1000.0);
break;
-   case SENSOR_DISTANCE:
case SENSOR_PRESSURE:
ret = asprintf(, "%.2f", s->value / 1000.0);
break;
Index: usr.sbin/sensorsd/sensorsd.c
===
RCS file: /cvs/src/usr.sbin/sensorsd/sensorsd.c,v
retrieving revision 1.62
diff -u -r1.62 sensorsd.c
--- usr.sbin/sensorsd/sensorsd.c22 Oct 2018 16:20:09 -  1.62
+++ usr.sbin/sensorsd/sensorsd.c3 Nov 2018 18:11:55 -
@@ -692,7 +692,7 @@
snprintf(fbuf, RFBUFSIZ, "%lld", value);
break;
case SENSOR_DISTANCE:
-   snprintf(fbuf, RFBUFSIZ, "%.2f mm", value / 1000.0);
+   snprintf(fbuf, RFBUFSIZ, "%.5f m", value / 100.0);
break;
c

Re: NMEA: add more gps sensor values (altitude, precision...)

2018-11-03 Thread Landry Breuil
On Sat, Nov 03, 2018 at 05:42:26PM +0100, Mark Kettenis wrote:
> > Date: Sat, 3 Nov 2018 17:00:47 +0100
> > From: Landry Breuil 

> > 
> > two problems with this display:
> > - DOPs are usually decimal values, ie 1.3, 1.53.. but raw sensors only
> > support integers, hence *1000.
> 
> We could add a SENSOR_FIXEDPOINT if we can come up with a reasonable
> representation.  Probably should be binary, and maybe we'd simply put
> the radix point smack in the middle.  But I'm not convinced DUP is a
> good enough reason to introduce this.

Same, so that might stay this way for now. The DOPs are interesting, but
a bit abstract to me..

> > - GGA messages provide altitude as '355.5' in meters (i *think* the unit
> >   is fixed in the protocol, because altitude in feets is provided by
> > PGRMZ) - and we only have a SENSOR_DISTANCE type in mm (which doesnt
> > seem used by any driver..). Thus altitude shown in milimeters..
> > This sensor types were added in
> > https://github.com/openbsd/src/commit/d95200b806051887b8c69294833e22dad6302828
> > but no driver apparently makes use of it.
> 
> Well, altitude really is just a vertical distance, so SENSOR_DISTANCE
> is the right one.  You obviously should convert the altitude from
> meters into millimeters.  If you think it would be more appropriate to
> display distances in meters, we (you?) can adjust sysctl(8).

Agreed, from what i understand there's a difference between 'the unit in
which the value is stored internally in the int64_t' and the 'the unit
used when displaying the value'.

As nothing uses SENSOR_DISTANCE for now, we can decide to switch all
'users' to show it as meter right now, don't change the current internal
storage unit (ie, uMeter per the sensors.h comment ?) and if there's a
proximity sensor someday (with its native unit being millimeters) it
will be displayed as '0.00022 m' for 0.22mm, right ?

> > >From that point, questions:
> > - should i add more 'interesting' sensor values, like amount of satellites
> >   seen/used ?
> 
> Number of satellites is probably interesting.

Ok, will add that one. I dont necessarly want to clutter the hw tree
with all sensors/values available, only keep the relevant ones..

> > - i want to add speed value (as RMC has it in knots?, and VTG in various
> >   units, per http://aprs.gids.nl/nmea/#vtg), but we only have
> > SENSOR_ACCEL type in m.s-2 (which is only used by asmc(4)). Should we
> > add a 'speed' sensor type ?  in m/s ? in km/h ? knots ?
> 
> SENSOR_VELOCITY would make sense.  It should be m/s with some
> appropriate scaling.  We don't need to represent speeds higer than
> 299792458 m/s, so micrometers per second would be a good choice.

In that case, if i understand it right, that would be 'store internally
as micrometers per second' then 'display to user as meter per second' ?

> > - should i add a SENSOR_ALTITUDE type in meters ?
> 
> I don't think so.

Ok, will withdraw that part, add SENSOR_VELOCITY and reuse SENSOR_DISTANCE.

Thanks for your comments !



add altitude & speed sensor types

2018-11-03 Thread Landry Breuil
Hi,

complementary to the previous diff, adds altitude sensor type (in
meters) and speed (in m/s, as it seems that's the common denominator and
most accepted unit, even if it doesnt 'mean' anything to humans...)
i still have to ponder about the decimals & intervals..

builds, not used yet.

Landry
Index: share/snmp/OPENBSD-SENSORS-MIB.txt
===
RCS file: /cvs/src/share/snmp/OPENBSD-SENSORS-MIB.txt,v
retrieving revision 1.6
diff -u -r1.6 OPENBSD-SENSORS-MIB.txt
--- share/snmp/OPENBSD-SENSORS-MIB.txt  2 Sep 2016 12:17:33 -   1.6
+++ share/snmp/OPENBSD-SENSORS-MIB.txt  3 Nov 2018 16:23:57 -
@@ -39,6 +39,9 @@
DESCRIPTION
"The MIB module for gathering information from
OpenBSD's kernel sensor framework."
+   REVISION "20181103Z"
+   DESCRIPTION
+   "Add new sensor types."
REVISION "20120920Z"
DESCRIPTION
"Add new sensor types."
@@ -136,7 +139,9 @@
angle(17),
distance(18),
pressure(19),
-   accel(20)
+   accel(20),
+   altitude(21),
+   speed(22)
}
MAX-ACCESS  read-only
STATUS  current
Index: usr.sbin/snmpd/mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.85
diff -u -r1.85 mib.c
--- usr.sbin/snmpd/mib.c18 Dec 2017 05:51:53 -  1.85
+++ usr.sbin/snmpd/mib.c3 Nov 2018 16:23:59 -
@@ -2617,7 +2617,7 @@
 static const char * const sensor_unit_s[SENSOR_MAX_TYPES + 1] = {
"degC", "RPM", "V DC", "V AC", "Ohm", "W", "A", "Wh", "Ah",
"", "", "%", "lx", "", "sec", "%RH", "Hz", "degree", 
-   "mm", "Pa", "m/s^2", ""
+   "mm", "Pa", "m/s^2", "m", "m/s", ""
 };
 
 const char *
@@ -2659,6 +2659,8 @@
ret = asprintf(, "%.2f", s->value / 1000.0);
break;
case SENSOR_DISTANCE:
+   case SENSOR_ALTITUDE:
+   case SENSOR_SPEED:
case SENSOR_PRESSURE:
ret = asprintf(, "%.2f", s->value / 1000.0);
break;
Index: usr.sbin/sensorsd/sensorsd.c
===
RCS file: /cvs/src/usr.sbin/sensorsd/sensorsd.c,v
retrieving revision 1.62
diff -u -r1.62 sensorsd.c
--- usr.sbin/sensorsd/sensorsd.c22 Oct 2018 16:20:09 -  1.62
+++ usr.sbin/sensorsd/sensorsd.c3 Nov 2018 16:24:01 -
@@ -700,6 +700,12 @@
case SENSOR_ACCEL:
snprintf(fbuf, RFBUFSIZ, "%2.4f m/s^2", value / 100.0);
break;
+   case SENSOR_ALTITUDE:
+   snprintf(fbuf, RFBUFSIZ, "%4.3f m", value / 1000.0);
+   break;
+   case SENSOR_SPEED:
+   snprintf(fbuf, RFBUFSIZ, "%4.3f m/s", value / 1000.0);
+   break;
default:
snprintf(fbuf, RFBUFSIZ, "%lld ???", value);
}
@@ -821,6 +827,8 @@
case SENSOR_HUMIDITY:
case SENSOR_DISTANCE:
case SENSOR_PRESSURE:
+   case SENSOR_ALTITUDE:
+   case SENSOR_SPEED:
rval = val * 1000.0;
break;
default:
Index: sbin/sysctl/sysctl.c
===
RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
retrieving revision 1.237
diff -u -r1.237 sysctl.c
--- sbin/sysctl/sysctl.c29 Sep 2018 04:29:48 -  1.237
+++ sbin/sysctl/sysctl.c3 Nov 2018 16:24:02 -
@@ -2661,6 +2661,12 @@
case SENSOR_ACCEL:
printf("%2.4f m/s^2", s->value / 100.0);
break;
+   case SENSOR_ALTITUDE:
+   printf("%4.3f m", s->value / 1000.0);
+   break;
+   case SENSOR_SPEED:
+   printf("%4.3f m/s", s->value / 1000.0);
+   break;
default:
printf("unknown");
}
Index: usr.bin/systat/sensors.c
===
RCS file: /cvs/src/usr.bin/systat/sensors.c,v
retrieving revision 1.30
diff -u -r1.30 sensors.c
--- usr.bin/systat/sensors.c16 Jan 2015 00:03:38 -  1.30
+++ usr.bin/systat/sensors.c3 Nov 2018 16:24:02 -
@@ -288,6 +288,12 @@
case SENSOR_ACCEL:
tbprintf("%2.4f m/s^2", s->sn_value / 100.0);
break;
+   case SENSOR_ALTITUDE:
+   tbprintf("%4.3f m", s->sn_value / 1000.0);
+   break;
+   case SENSOR_SPEED:
+   tbprintf("%4.3f m/s", s->sn_value / 1000.0);
+   break;
default:
tbprintf("%10lld", s->sn_value);
 

NMEA: add more gps sensor values (altitude, precision...)

2018-11-03 Thread Landry Breuil
Hi,

here's a diff to add 5 new sensor values to nmea: altitude, quality,
hdop, vdop & pdop. altitude and quality are provided by GGA messages:
http://aprs.gids.nl/nmea/#gga, quality is either 0 (no fix), 1 (gps fix)
or 2 (dgps fix).

The last 3 are 'Dilution of precision' values, respectively horizontal,
vertical & positional (ie 3d), cf
https://en.wikipedia.org/wiki/Dilution_of_precision_(navigation) &
http://www.trakgps.com/en/index.php/information/gps-articles-information/65-gps-accuracy
and are provided by GSA messages: http://aprs.gids.nl/nmea/#gsa it is
generally considered good precision when the DOP is below 2, so i've set
the sensor status warning accordingly.

this provides for example (angle values hidden for 'privacy', just after
the gps got a fix):
nmea0.indicator0   OnOKSignal
nmea0.raw0  1 rawOKGPS fix
nmea0.raw1   1920 rawOKHDOP
nmea0.raw2   2520 raw WARNING  VDOP
nmea0.raw3   3170 raw WARNING  PDOP
nmea0.timedelta024.867 msOKGPS autonomous
nmea0.angle0  xx. degreesOKLatitude
nmea0.angle1   z. degreesOKLongitude
nmea0.distance0 371.50 mmOKAltitude

and after a while, when precision improved:
nmea0.indicator0   OnOKSignal
nmea0.raw0  1 rawOKGPS fix
nmea0.raw1800 rawOKHDOP
nmea0.raw2   1300 rawOKVDOP
nmea0.raw3   1530 rawOKPDOP
nmea0.timedelta0  -296.499 msOKGPS autonomous
nmea0.angle0  xx. degreesOKLatitude
nmea0.angle1   z. degreesOKLongitude
nmea0.distance0 355.50 mmOKAltitude

two problems with this display:
- DOPs are usually decimal values, ie 1.3, 1.53.. but raw sensors only
support integers, hence *1000.

- GGA messages provide altitude as '355.5' in meters (i *think* the unit
  is fixed in the protocol, because altitude in feets is provided by
PGRMZ) - and we only have a SENSOR_DISTANCE type in mm (which doesnt
seem used by any driver..). Thus altitude shown in milimeters..
This sensor types were added in
https://github.com/openbsd/src/commit/d95200b806051887b8c69294833e22dad6302828
but no driver apparently makes use of it.

>From that point, questions:
- should i add more 'interesting' sensor values, like amount of satellites
  seen/used ?

- i want to add speed value (as RMC has it in knots?, and VTG in various
  units, per http://aprs.gids.nl/nmea/#vtg), but we only have
SENSOR_ACCEL type in m.s-2 (which is only used by asmc(4)). Should we
add a 'speed' sensor type ?  in m/s ? in km/h ? knots ?

- should i add a SENSOR_ALTITUDE type in meters ?

- is there any interest in all this, from the sensors framework POV ?
Otherwise i can leave it as is and just use gpsd in userspace, which
works 100% fine...

All this done with an
umodem0 at uhub1 port 1 configuration 1 interface 0 "u-blox AG - www.u-blox.com 
u-blox GNSS receiver" rev 1.10/2.01 addr 3

which works fine with ldattach & gpsd, ie i run doas gpsd -N -D2 $(doas
ldattach -p nmea /dev/cuaU0) and gpsmon shows me the msgs received by
the device, sent to the kernel via ldattach, and forwarded to gpsd.

comments welcome, diff not to be commited as is of course (nmea_atoi is
*horrible*, i know..)

Landry
Index: tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.47
diff -u -r1.47 tty_nmea.c
--- tty_nmea.c  1 Sep 2018 06:09:26 -   1.47
+++ tty_nmea.c  3 Nov 2018 15:28:11 -
@@ -29,6 +29,11 @@
 #ifdef NMEA_DEBUG
 #define DPRINTFN(n, x) do { if (nmeadebug > (n)) printf x; } while (0)
 int nmeadebug = 0;
+/*
+ * 1 = print interesting messages
+ * 2 = print all messages
+int nmeadebug = 2;
+ */
 #else
 #define DPRINTFN(n, x)
 #endif
@@ -52,6 +57,11 @@
struct ksensor  signal; /* signal status */
struct ksensor  latitude;
struct ksensor  longitude;
+   struct ksensor  altitude;
+   struct ksensor  quality;
+   struct ksensor  hdop;
+   struct ksensor  vdop;
+   struct ksensor  pdop;
struct ksensordev   timedev;
struct timespec ts; /* current timestamp */
struct timespec lts;/* timestamp of last '$' seen */
@@ -70,6 +80,8 @@
 /* NMEA decoding */
 void   nmea_scan(struct nmea *, struct tty *);
 void   nmea_gprmc(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void   

Re: iostat: add "sp" column for CP_SPIN

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

Seconded, rrdcached really helps a lot until you have waaayyy too many rrds,
and then switch to a real tsdb (hint: influxdb is in ports)



Re: nmea(4): support GNRMC messages sent by gps devices supporting multiple networks

2018-08-29 Thread Landry Breuil
On Wed, Aug 29, 2018 at 02:13:01PM -0600, Theo de Raadt wrote:
> Landry Breuil  wrote:
> 
> > Hi,
> > 
> > was playing with an usb gps that works ootb with gpsd, but our nmea line
> > discipline doesnt recognize it, and the 'indicator' sensor stays at its
> > default 'unknown'.
> > 
> > The gps is a navilock nl-8002u:
> > umodem0 at uhub3 port 2 configuration 1 interface 0 "u-blox AG - 
> > www.u-blox.com u-blox GNSS receiver" rev 1.10/2.01 addr 2
> > umodem0: data interface 1, has CM over data, has no break
> > umodem0: status change notification available
> > ucom1 at umodem0
> > 
> > The RMC message our tty_nmea.c code looks for is like this w/ this device:
> > $GNRMC,194858.00,A,,N,yyy,E,0.236,,290818,,,A*6E
> > 
> > Turns out the NMEA 0183 spec says that RMC message types are prefixed by 
> > various
> > codes for 'talkers' which says if the data comes from GPS (GP), Glonass 
> > (GL),
> > Galileo (GA), Beidou (BD), or 'Global navigation satellite' (GN) for the
> > 'generic term' (cf http://freenmea.net/docs or
> > https://github.com/mvglasow/satstat/wiki/NMEA-IDs) and mine uses GNRMC, 
> > while
> > tty_nmea.c looks for GPRMC.
> > 
> > Poking at the string comparison, i finally have sensors working:
> > 
> > $doas ldattach -d nmea /dev/cuaU1
> > ldattach[93876]: attach nmea on /dev/cuaU1
> > 
> > hw.sensors.nmea0.indicator0=On (Signal), OK
> > hw.sensors.nmea0.timedelta0=0.031653 secs (GPS autonomous), OK, Wed Aug 29 
> > 21:48:52.031
> > hw.sensors.nmea0.angle0=xx degrees (Latitude), OK
> > hw.sensors.nmea0.angle1=yy degrees (Longitude), OK
> > 
> > ntpd thinks the sensor is invalid, but that's for more poking later.
> > 
> > $ntpctl -s Sensors
> > sensor
> >wt gd st  next  poll  offset  correction
> > nmea0
> > 1  0  02s   15s - sensor not valid -
> > 
> > So here's the diff.. i wonder if we should just match the 3 chars for the 
> > 'RMC'
> > message.
> 
> I think it should match all the currently known ones, including the first
> two characters.

Well, let's match on the 5 'most common' used, otherwise there's many
more.. cf http://www.catb.org/gpsd/NMEA.html#_talker_ids

Would there be interest in adding support for speed/elevation to sensors
provided by nmea(4) ? As the data is available anyway on the wire.. (altitude
in the GGA messages, and RMC has the ground speed in knots)

Index: tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.46
diff -u -r1.46 tty_nmea.c
--- tty_nmea.c  19 Feb 2018 08:59:52 -  1.46
+++ tty_nmea.c  30 Aug 2018 05:46:29 -
@@ -260,8 +260,20 @@
}
}
 
-   /* we only look at the GPRMC message */
-   if (strcmp(fld[0], "GPRMC"))
+   /*
+* we only look at the RMC message, which can come from different 
'talkers',
+* distinguished by the two-chars prefix, the most common being:
+* GPS (GP)
+* Glonass (GL)
+* BeiDou (BD)
+* Galileo (GA)
+* 'Any kind/a mix of GNSS systems' (GN)
+*/
+   if (strcmp(fld[0], "BDRMC") &&
+   strcmp(fld[0], "GARMC") &&
+   strcmp(fld[0], "GLRMC") &&
+   strcmp(fld[0], "GNRMC") &&
+   strcmp(fld[0], "GPRMC"))
return;
 
/* if we have a checksum, verify it */



nmea(4): support GNRMC messages sent by gps devices supporting multiple networks

2018-08-29 Thread Landry Breuil
Hi,

was playing with an usb gps that works ootb with gpsd, but our nmea line
discipline doesnt recognize it, and the 'indicator' sensor stays at its
default 'unknown'.

The gps is a navilock nl-8002u:
umodem0 at uhub3 port 2 configuration 1 interface 0 "u-blox AG - www.u-blox.com 
u-blox GNSS receiver" rev 1.10/2.01 addr 2
umodem0: data interface 1, has CM over data, has no break
umodem0: status change notification available
ucom1 at umodem0

The RMC message our tty_nmea.c code looks for is like this w/ this device:
$GNRMC,194858.00,A,,N,yyy,E,0.236,,290818,,,A*6E

Turns out the NMEA 0183 spec says that RMC message types are prefixed by various
codes for 'talkers' which says if the data comes from GPS (GP), Glonass (GL),
Galileo (GA), Beidou (BD), or 'Global navigation satellite' (GN) for the
'generic term' (cf http://freenmea.net/docs or
https://github.com/mvglasow/satstat/wiki/NMEA-IDs) and mine uses GNRMC, while
tty_nmea.c looks for GPRMC.

Poking at the string comparison, i finally have sensors working:

$doas ldattach -d nmea /dev/cuaU1
ldattach[93876]: attach nmea on /dev/cuaU1

hw.sensors.nmea0.indicator0=On (Signal), OK
hw.sensors.nmea0.timedelta0=0.031653 secs (GPS autonomous), OK, Wed Aug 29 
21:48:52.031
hw.sensors.nmea0.angle0=xx degrees (Latitude), OK
hw.sensors.nmea0.angle1=yy degrees (Longitude), OK

ntpd thinks the sensor is invalid, but that's for more poking later.

$ntpctl -s Sensors
sensor
   wt gd st  next  poll  offset  correction
nmea0
1  0  02s   15s - sensor not valid -

So here's the diff.. i wonder if we should just match the 3 chars for the 'RMC'
message.

Index: tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.46
diff -u -r1.46 tty_nmea.c
--- tty_nmea.c  19 Feb 2018 08:59:52 -  1.46
+++ tty_nmea.c  29 Aug 2018 19:49:24 -
@@ -261,7 +261,7 @@
}
 
/* we only look at the GPRMC message */
-   if (strcmp(fld[0], "GPRMC"))
+   if (strcmp(fld[0], "GPRMC") && strcmp(fld[0], "GNRMC"))
return;
 
/* if we have a checksum, verify it */



Re: Add 'video' pledge

2018-07-25 Thread Landry Breuil
On Wed, Jul 25, 2018 at 12:29:10PM -0600, Theo de Raadt wrote:
> >I went over this for a while and i don't see how firefox could be adapted to
> >avoid this new pledge class. The other option is to move lots of code around 
> >so
> >that the video device is opened/configured inconditionally by the main 
> >process
> >before pledging (but then you'd still need the various ioctls getting buffers
> >etc), but that feels stupid: why would you want to open the video device if
> >you're not going to actually use it ?
> 
> Most privsep programs contain a process which isn't pledged.

Except the privsep programs usually follow a model of 'a master process
doing init, opening priviledged devices, then orchestrating the
others/passing fds'.

Here, we're talking about the main firefox process, which does shitloads of
things, among those orchestrate the content processes, but also & mostly
filesystem access, network access, etc, etc . You'd prefer having it unpledged ?
I don't understand your argument here.

> >That of course doesn't try to solve the video device access/ownership which 
> >is
> >a separate issue.
> 
> I fear it solves no issues at all.
> 
> As I said before, I am uncomfortable pushing this policy mechanism into
> the kernel to be used by *only one program*.

I never said it was *only* for firefox. Right now, any program using the
v4l api can't be pledged or then loses the ability to talk to the camera
device. Ok, in base there's only video(1), but in ports
there's mplayer, ffmpeg, vlc, gstreamer, sane, etc..

I know blindly adding pledge everywhere in the ports tree isnt a primary
target, but i think huge programs with big attack surface (like all the
video players) would be good contenders.

I'm not volunteering for them, but i'd like to allow OpenBSD users to
'easily' use video chat in their browser, without having to resort to
skype or hangouts on their jesus laptops, or to go through knobs to
disable pledge.

> Sorry, but that isn't how pledge is developed / extended.

I thought it was developed by experimenting with things, and then
iterating on them. As far as i see from the cvs log for the pledge
kernel subsystem and the basesystem programs conversion, that's how it
was developed. Start with a set of syscalls subsets, then add a new
syscall to a subset when needed because it's a valid usecase (or change
the program behaviour to hoist the syscall usage), or separate a set of
syscalls in a new class.

I don't see how different the video pledge i'm proposing is from the
other classes of syscalls that were added for
bpf/drm/tape/audio/disklabel/pf/tty/route.  They were all added to allow
for a subset of ioctls on a particular type of device.

How is my approach different here ?



Re: Add 'video' pledge

2018-07-25 Thread Landry Breuil
On Thu, May 24, 2018 at 07:15:05PM +0200, Landry Breuil wrote:
> Hi,
> 
> here's two simple diffs (one for the kernel, one for the pledge.2
> manpage) that allow me to use my webcam again within firefox when
> pledged,, adding 'video' to the main process pledges.
> 
> The kernel changes are similar to what was done for 'audio' pledge, and
> i took the ioctl list from the ones found in the mozilla codebase:
> https://dxr.mozilla.org/mozilla-central/search?q=vidioc=false
> 
> Comments and feedback welcome. Of course, to test it using firefox, you
> still need to grant your user access to /dev/video*.

After having been poked offline about why this diff hadnt been commited,
sending an updated diff including the pledge.h change, and updated after unveil
addition. Among other bits, you'll need this if you want to use webrtc in
firefox while being still pledged - the other solution right now is of course
disabling pledge, which is a pity.

I went over this for a while and i don't see how firefox could be adapted to
avoid this new pledge class. The other option is to move lots of code around so
that the video device is opened/configured inconditionally by the main process
before pledging (but then you'd still need the various ioctls getting buffers
etc), but that feels stupid: why would you want to open the video device if
you're not going to actually use it ?

Right now the devices are listed/opened *when* camera access is requested by
a page aiming to use the camera, ie during the process lifetime, so until
upstream decides to separate video device access in a separate process (which
isnt afaik on the radar) adding this pledge class is the only solution i see
for now.

That of course doesn't try to solve the video device access/ownership which is
a separate issue.

Feedback appreciated.

Index: sys/pledge.h
===
RCS file: /cvs/src/sys/sys/pledge.h,v
retrieving revision 1.37
diff -u -r1.37 pledge.h
--- sys/pledge.h13 Jul 2018 09:25:23 -  1.37
+++ sys/pledge.h25 Jul 2018 17:59:15 -
@@ -62,6 +62,7 @@
 #define PLEDGE_ERROR   0x0004ULL   /* ENOSYS instead of kill */
 #define PLEDGE_WROUTE  0x0008ULL   /* interface address ioctls */
 #define PLEDGE_UNVEIL  0x0010ULL   /* allow unveil() */
+#define PLEDGE_VIDEO   0x00200800ULL   /* video ioctls */
 
 /*
  * Bits outside PLEDGE_USERSET are used by the kernel itself
@@ -112,6 +113,7 @@
{ PLEDGE_ERROR, "error" },
{ PLEDGE_WROUTE,"wroute" },
{ PLEDGE_UNVEIL,"unveil" },
+   { PLEDGE_VIDEO, "video" },
{ 0, NULL },
 };
 #endif
Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.237
diff -u -r1.237 kern_pledge.c
--- kern/kern_pledge.c  15 Jul 2018 12:44:09 -  1.237
+++ kern/kern_pledge.c  25 Jul 2018 17:59:15 -
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -396,6 +397,7 @@
{ "tty",PLEDGE_TTY },
{ "unix",   PLEDGE_UNIX },
{ "unveil", PLEDGE_UNVEIL },
+   { "video",  PLEDGE_VIDEO },
{ "vminfo", PLEDGE_VMINFO },
{ "vmm",PLEDGE_VMM },
{ "wpath",  PLEDGE_WPATH },
@@ -1151,6 +1153,30 @@
break;
}
}
+
+   if ((p->p_p->ps_pledge & PLEDGE_VIDEO)) {
+   switch (com) {
+   case VIDIOC_QUERYCAP:
+   case VIDIOC_ENUM_FMT:
+   case VIDIOC_S_FMT:
+   case VIDIOC_G_PARM:
+   case VIDIOC_S_PARM:
+   case VIDIOC_REQBUFS:
+   case VIDIOC_QBUF:
+   case VIDIOC_DQBUF:
+   case VIDIOC_QUERYBUF:
+   case VIDIOC_STREAMON:
+   case VIDIOC_STREAMOFF:
+   case VIDIOC_ENUM_FRAMESIZES:
+   case VIDIOC_ENUM_FRAMEINTERVALS:
+   if (fp->f_type == DTYPE_VNODE &&
+   vp->v_type == VCHR &&
+   cdevsw[major(vp->v_rdev)].d_open == videoopen)
+   return (0);
+   break;
+   }
+   }
+
 
 #if NPF > 0
if ((p->p_p->ps_pledge & PLEDGE_PF)) {



Re: uaudio: fix detection of a bunch of logitech webcams

2018-07-07 Thread Landry Breuil
On Fri, Jul 06, 2018 at 03:38:00PM +0200, Remco wrote:
> > @@ -1814,6 +1827,10 @@
> > sc->sc_audio_rev = UGETW(acdp->bcdADC);
> > DPRINTFN(2,("%s: found AC header, vers=%03x, len=%d\n",
> >  __func__, sc->sc_audio_rev, aclen));
> > +
> > +   /* Some webcams descriptors advertise an off-by-one wTotalLength */
> > +   if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
> > +   aclen++;
> > sc->sc_nullalt = -1;
> > 
> > 
> 
> I'm not in a position to test yet, but I have a comment on the aclen usage.
> AFAICS aclen is used in two places with the fix in between:
> 
>   aclen = UGETW(acdp->wTotalLength);
>   if (offs + aclen > size)
>   return (USBD_INVAL);
> ...
>   if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
>   aclen++;
> ...
>   ibufend = ibuf + aclen;
> 
> It looks more correct to fix aclen first, but does that still work ?
> e.g.:
>   aclen = UGETW(acdp->wTotalLength);
>   if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
>   aclen++;
>   if (offs + aclen > size)
>   return (USBD_INVAL);
> ...
>   ibufend = ibuf + aclen;
> 

It might be 'better' logically speaking, but i know the (offs + aclen >
size) check wasnt triggered, as the debug printf 'found AC header' is
reached.

The one triggering the 'descriptor make no sense' error codepath is the
one checking (ibuf + dp->bLength > ibufend), and ibufend is computed
with aclen>



Re: uaudio: fix detection of a bunch of logitech webcams

2018-07-06 Thread Landry Breuil
On Fri, Jul 06, 2018 at 01:52:57PM +0200, Landry Breuil wrote:
> On Thu, Jul 05, 2018 at 11:13:42AM +0200, Landry Breuil wrote:
> > On Thu, Jul 05, 2018 at 11:04:34AM +0200, Martin Pieuchot wrote:
> > > On 05/07/18(Thu) 09:28, Landry Breuil wrote:
> > > > On Fri, Jun 29, 2018 at 01:50:24PM +0200, Landry Breuil wrote:
> > > > > On Fri, Jun 29, 2018 at 09:05:56AM +0200, Remco wrote:
> > > > > > > > If you don't find any better solution, I'd suggest using a 
> > > > > > > > device ID
> > > > > > > > check rather than adding a quirk.  Because such quirk cannot be 
> > > > > > > > generic.
> > > > > > > > In that case you have an off-by-one, but another device might 
> > > > > > > > have a
> > > > > > > > different problem.
> > > > > > > 
> > > > > > > That was in the case we'd encounter other devices where the 
> > > > > > > descriptor
> > > > > > > is bogus and has the same issue, but i agree this is unlikely.
> > > > > > > 
> > > > > > 
> > > > > > More likely for Logitech USB webcams I'm afraid.
> > > > > > My dad's Logitech C500 showed the same issue several years back.
> > > > > > (I'll try to find some time to borrow the device and test your 
> > > > > > patch)
> > > > > > There's also a report that suggests a C200 may be affected as well.
> > > > > > (https://marc.info/?l=openbsd-misc=127952275021290=2)
> > > > > 
> > > > > Aaah, very interesting data point. Thanks for the pointer.. maybe a
> > > > > 'generic fix' would be to ignore size checks for all logitech webcams,
> > > > > or to print a warning but not error out in this case. Is there some 
> > > > > kind
> > > > > of 'usbdevs -v/lsusb -v' database somewhere ?
> > > > 
> > > > So after feedback from lots of logitech webcam users (thanks!), i got a 
> > > > list
> > > > for working vs non-working devices, and i think a new quirk would do as 
> > > > they
> > > > all expose the same issue (bLength for FEATURE_UNIT is 9 while it is 8 
> > > > on
> > > > working devices) - and they would all be 'fixed' by the attached diff.
> > > 
> > > If you know that some descriptors have an off-by-one, I'd suggest you add
> > > 1 to the corresponding buffer length instead of completely removing the
> > > check.  Removing it completely now open the door to other overflows and
> > > adding 1 auto-document the hardware bug ;)
> > 
> > Well that was the intent of the original diff. New version...
> 
> New diff adding the C250 which also exhibits the same issue. I have an
> okay from mpi@ on the previous diff, but i'd like actual reports that it
> makes the audio device actually working (mine doesnt now, i'm digging
> into it, but i'm pretty sure at some point i had it working...)
> 
> To test:
> 
> sysctl kern.audio.record=1
> aucat -f rsnd/1 -o /tmp/foo.wav (if the uaudio device attaches as audio1)
> 
> If this actually records sound (mplayer /tmp/foo.wav to check), you've
> won. if foo.wav is 68 bytes long, then recording doesnt work, interrupts
> are triggered but no transfers happen..

I should add to this testing part that you should 'revert' to usb2 for
proper uaudio(4) testing, usb3/xhci doesnt really work in this usecase
and will trigger 'error creating pipe: err=INVAL endpt=0x86' errors in
dmesg.

If you can, ensure that you use usb2 by disabling usb3 in the bios or
xhci in the kernel config.

Landry



Re: uaudio: fix detection of a bunch of logitech webcams

2018-07-06 Thread Landry Breuil
On Thu, Jul 05, 2018 at 11:13:42AM +0200, Landry Breuil wrote:
> On Thu, Jul 05, 2018 at 11:04:34AM +0200, Martin Pieuchot wrote:
> > On 05/07/18(Thu) 09:28, Landry Breuil wrote:
> > > On Fri, Jun 29, 2018 at 01:50:24PM +0200, Landry Breuil wrote:
> > > > On Fri, Jun 29, 2018 at 09:05:56AM +0200, Remco wrote:
> > > > > > > If you don't find any better solution, I'd suggest using a device 
> > > > > > > ID
> > > > > > > check rather than adding a quirk.  Because such quirk cannot be 
> > > > > > > generic.
> > > > > > > In that case you have an off-by-one, but another device might 
> > > > > > > have a
> > > > > > > different problem.
> > > > > > 
> > > > > > That was in the case we'd encounter other devices where the 
> > > > > > descriptor
> > > > > > is bogus and has the same issue, but i agree this is unlikely.
> > > > > > 
> > > > > 
> > > > > More likely for Logitech USB webcams I'm afraid.
> > > > > My dad's Logitech C500 showed the same issue several years back.
> > > > > (I'll try to find some time to borrow the device and test your patch)
> > > > > There's also a report that suggests a C200 may be affected as well.
> > > > > (https://marc.info/?l=openbsd-misc=127952275021290=2)
> > > > 
> > > > Aaah, very interesting data point. Thanks for the pointer.. maybe a
> > > > 'generic fix' would be to ignore size checks for all logitech webcams,
> > > > or to print a warning but not error out in this case. Is there some kind
> > > > of 'usbdevs -v/lsusb -v' database somewhere ?
> > > 
> > > So after feedback from lots of logitech webcam users (thanks!), i got a 
> > > list
> > > for working vs non-working devices, and i think a new quirk would do as 
> > > they
> > > all expose the same issue (bLength for FEATURE_UNIT is 9 while it is 8 on
> > > working devices) - and they would all be 'fixed' by the attached diff.
> > 
> > If you know that some descriptors have an off-by-one, I'd suggest you add
> > 1 to the corresponding buffer length instead of completely removing the
> > check.  Removing it completely now open the door to other overflows and
> > adding 1 auto-document the hardware bug ;)
> 
> Well that was the intent of the original diff. New version...

New diff adding the C250 which also exhibits the same issue. I have an
okay from mpi@ on the previous diff, but i'd like actual reports that it
makes the audio device actually working (mine doesnt now, i'm digging
into it, but i'm pretty sure at some point i had it working...)

To test:

sysctl kern.audio.record=1
aucat -f rsnd/1 -o /tmp/foo.wav (if the uaudio device attaches as audio1)

If this actually records sound (mplayer /tmp/foo.wav to check), you've
won. if foo.wav is 68 bytes long, then recording doesnt work, interrupts
are triggered but no transfers happen..

Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.128
diff -u -r1.128 uaudio.c
--- uaudio.c30 Dec 2017 23:08:29 -  1.128
+++ uaudio.c6 Jul 2018 11:49:36 -
@@ -172,6 +172,7 @@
 #define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but 
works */
 #define UAUDIO_FLAG_DEPENDENT   0x0020 /* play and record params must equal */
 #define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_BAD_ADC_LEN 0x0080 /* bad audio control descriptor 
size */
 
 struct uaudio_devs {
struct usb_devno uv_dev;
@@ -222,6 +223,18 @@
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC200 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC210 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC250 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC270 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC500 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
UAUDIO_FLAG_NO_FRAC }
 };
@@ -1814,6 +1827,10 @@
sc->sc_audio_rev = UGETW(acdp->bcdADC);
DPRINTFN(2,("%s: found AC header, vers=%03x, len=%d\n",
 __func__, sc->sc_audio_rev, aclen));
+
+   /* Some webcams descriptors advertise an off-by-one wTotalLength */
+   if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
+   aclen++;
 
sc->sc_nullalt = -1;
 



add logitech c250

2018-07-06 Thread Landry Breuil
Hi,

thx to Raf Czlonka who sent me the usb descriptor for this model, it's
also affected by the 'wrong feature_unit bLength' issue so should be
added to usbdevs too.

okay ? I'll of course add it to the other uaudio diff..

Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.687
diff -u -r1.687 usbdevs
--- usbdevs 3 Jul 2018 07:03:18 -   1.687
+++ usbdevs 6 Jul 2018 11:46:22 -
@@ -2573,6 +2573,7 @@
 product LOGITECH PAGESCAN  0x040f  PageScan
 product LOGITECH QUICKCAMWEB   0x0801  QuickCam Web
 product LOGITECH WEBCAMC2000x0802  Webcam C200
+product LOGITECH WEBCAMC2500x0804  Webcam C250
 product LOGITECH WEBCAMC5000x0807  Webcam C500
 product LOGITECH QUICKCAMPRO   0x0810  QuickCam Pro
 product LOGITECH WEBCAMC2100x0819  Webcam C210
Index: usbdevs.h
===
RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v
retrieving revision 1.699
diff -u -r1.699 usbdevs.h
--- usbdevs.h   3 Jul 2018 07:04:21 -   1.699
+++ usbdevs.h   6 Jul 2018 11:46:23 -
@@ -1,10 +1,10 @@
-/* $OpenBSD: usbdevs.h,v 1.699 2018/07/03 07:04:21 landry Exp $*/
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMATICALLY GENERATED.  DO NOT EDIT.
  *
  * generated from:
- * OpenBSD: usbdevs,v 1.686 2018/06/28 15:02:06 kevlo Exp 
+ * OpenBSD: usbdevs,v 1.687 2018/07/03 07:03:18 landry Exp 
  */
 /* $NetBSD: usbdevs,v 1.322 2003/05/10 17:47:14 hamajima Exp $ */
 
@@ -2580,6 +2580,7 @@
 #defineUSB_PRODUCT_LOGITECH_PAGESCAN   0x040f  /* PageScan */
 #defineUSB_PRODUCT_LOGITECH_QUICKCAMWEB0x0801  /* 
QuickCam Web */
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC200 0x0802  /* Webcam C200 
*/
+#defineUSB_PRODUCT_LOGITECH_WEBCAMC250 0x0804  /* Webcam C250 
*/
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC500 0x0807  /* Webcam C500 
*/
 #defineUSB_PRODUCT_LOGITECH_QUICKCAMPRO0x0810  /* 
QuickCam Pro */
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC210 0x0819  /* Webcam C210 
*/
Index: usbdevs_data.h
===
RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v
retrieving revision 1.693
diff -u -r1.693 usbdevs_data.h
--- usbdevs_data.h  3 Jul 2018 07:04:21 -   1.693
+++ usbdevs_data.h  6 Jul 2018 11:46:23 -
@@ -1,10 +1,10 @@
-/* $OpenBSD: usbdevs_data.h,v 1.693 2018/07/03 07:04:21 landry Exp $   
*/
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMATICALLY GENERATED.  DO NOT EDIT.
  *
  * generated from:
- * OpenBSD: usbdevs,v 1.686 2018/06/28 15:02:06 kevlo Exp 
+ * OpenBSD: usbdevs,v 1.687 2018/07/03 07:03:18 landry Exp 
  */
 /* $NetBSD: usbdevs,v 1.322 2003/05/10 17:47:14 hamajima Exp $ */
 
@@ -5704,6 +5704,10 @@
{
USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC200,
"Webcam C200",
+   },
+   {
+   USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC250,
+   "Webcam C250",
},
{
USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC500,



Re: uaudio: fix detection of a bunch of logitech webcams

2018-07-05 Thread Landry Breuil
On Thu, Jul 05, 2018 at 11:04:34AM +0200, Martin Pieuchot wrote:
> On 05/07/18(Thu) 09:28, Landry Breuil wrote:
> > On Fri, Jun 29, 2018 at 01:50:24PM +0200, Landry Breuil wrote:
> > > On Fri, Jun 29, 2018 at 09:05:56AM +0200, Remco wrote:
> > > > > > If you don't find any better solution, I'd suggest using a device ID
> > > > > > check rather than adding a quirk.  Because such quirk cannot be 
> > > > > > generic.
> > > > > > In that case you have an off-by-one, but another device might have a
> > > > > > different problem.
> > > > > 
> > > > > That was in the case we'd encounter other devices where the descriptor
> > > > > is bogus and has the same issue, but i agree this is unlikely.
> > > > > 
> > > > 
> > > > More likely for Logitech USB webcams I'm afraid.
> > > > My dad's Logitech C500 showed the same issue several years back.
> > > > (I'll try to find some time to borrow the device and test your patch)
> > > > There's also a report that suggests a C200 may be affected as well.
> > > > (https://marc.info/?l=openbsd-misc=127952275021290=2)
> > > 
> > > Aaah, very interesting data point. Thanks for the pointer.. maybe a
> > > 'generic fix' would be to ignore size checks for all logitech webcams,
> > > or to print a warning but not error out in this case. Is there some kind
> > > of 'usbdevs -v/lsusb -v' database somewhere ?
> > 
> > So after feedback from lots of logitech webcam users (thanks!), i got a list
> > for working vs non-working devices, and i think a new quirk would do as they
> > all expose the same issue (bLength for FEATURE_UNIT is 9 while it is 8 on
> > working devices) - and they would all be 'fixed' by the attached diff.
> 
> If you know that some descriptors have an off-by-one, I'd suggest you add
> 1 to the corresponding buffer length instead of completely removing the
> check.  Removing it completely now open the door to other overflows and
> adding 1 auto-document the hardware bug ;)

Well that was the intent of the original diff. New version...

Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.128
diff -u -r1.128 uaudio.c
--- uaudio.c30 Dec 2017 23:08:29 -  1.128
+++ uaudio.c5 Jul 2018 09:12:42 -
@@ -172,6 +172,7 @@
 #define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but 
works */
 #define UAUDIO_FLAG_DEPENDENT   0x0020 /* play and record params must equal */
 #define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_BAD_ADC_LEN 0x0080 /* bad audio control descriptor 
size */
 
 struct uaudio_devs {
struct usb_devno uv_dev;
@@ -222,6 +223,16 @@
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC200 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC210 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC270 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC500 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
UAUDIO_FLAG_NO_FRAC }
 };
@@ -1814,6 +1825,10 @@
sc->sc_audio_rev = UGETW(acdp->bcdADC);
DPRINTFN(2,("%s: found AC header, vers=%03x, len=%d\n",
 __func__, sc->sc_audio_rev, aclen));
+
+   /* Some webcams descriptors advertise an off-by-one wTotalLength */
+   if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
+   aclen++;
 
sc->sc_nullalt = -1;
 



uaudio: fix detection of a bunch of logitech webcams

2018-07-05 Thread Landry Breuil
On Fri, Jun 29, 2018 at 01:50:24PM +0200, Landry Breuil wrote:
> On Fri, Jun 29, 2018 at 09:05:56AM +0200, Remco wrote:
> > > > If you don't find any better solution, I'd suggest using a device ID
> > > > check rather than adding a quirk.  Because such quirk cannot be generic.
> > > > In that case you have an off-by-one, but another device might have a
> > > > different problem.
> > > 
> > > That was in the case we'd encounter other devices where the descriptor
> > > is bogus and has the same issue, but i agree this is unlikely.
> > > 
> > 
> > More likely for Logitech USB webcams I'm afraid.
> > My dad's Logitech C500 showed the same issue several years back.
> > (I'll try to find some time to borrow the device and test your patch)
> > There's also a report that suggests a C200 may be affected as well.
> > (https://marc.info/?l=openbsd-misc=127952275021290=2)
> 
> Aaah, very interesting data point. Thanks for the pointer.. maybe a
> 'generic fix' would be to ignore size checks for all logitech webcams,
> or to print a warning but not error out in this case. Is there some kind
> of 'usbdevs -v/lsusb -v' database somewhere ?

So after feedback from lots of logitech webcam users (thanks!), i got a list
for working vs non-working devices, and i think a new quirk would do as they
all expose the same issue (bLength for FEATURE_UNIT is 9 while it is 8 on
working devices) - and they would all be 'fixed' by the attached diff.

If an overflow is detected on the descriptor size (ie the (ibuf +
dp->bLength > ibufend) check but we know we're on a broken device (with
UAUDIO_FLAG_BAD_ADC_LEN quirk) we silently avoid returning USBD_INVAL,
and that allows uaudio(4) detection to succeed. That's a different take than
adding 1 to aclen in the first version of the diff, i dunno which is better.

Now, i'm pretty sure i managed to properly test the device some days ago
and record sound with it, but now i only manage to produce 68-bytes
empty wav files, using 'aucat -f rsnd/1 -o /tmp/test.wav' after having
ensured kern.audio.record was 1. Something is fishy

Oks, testing & feedback welcome, so that i can move forward and get more
testing/debugging done.

Landry

Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.128
diff -u -r1.128 uaudio.c
--- uaudio.c30 Dec 2017 23:08:29 -  1.128
+++ uaudio.c3 Jul 2018 06:44:28 -
@@ -172,6 +172,7 @@
 #define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but 
works */
 #define UAUDIO_FLAG_DEPENDENT   0x0020 /* play and record params must equal */
 #define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_BAD_ADC_LEN 0x0080 /* bad audio control descriptor 
size */
 
 struct uaudio_devs {
struct usb_devno uv_dev;
@@ -222,6 +223,16 @@
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC200 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC210 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC270 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC500 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
UAUDIO_FLAG_NO_FRAC }
 };
@@ -1832,7 +1843,8 @@
if (ibuf >= ibufend)
break;
dp = (const usb_descriptor_t *)ibuf;
-   if (ibuf + dp->bLength > ibufend) {
+   if (ibuf + dp->bLength > ibufend &&
+   !(sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)) {
free(iot, M_TEMP, 0);
return (USBD_INVAL);
}



Re: add a bunch of logitech webcam ids to usbdevs

2018-07-03 Thread Landry Breuil
On Mon, Jul 02, 2018 at 09:29:04PM +0200, Mark Kettenis wrote:
> > Date: Mon, 2 Jul 2018 21:10:26 +0200
> > From: Landry Breuil 
> > 
> > ok ?
> 
> Why?  How do they show up right now?  We typically only add USB IDs
> for hardware that needs quirks or doesn't have a useful string
> embedded in the hardware.

New diffs that only adds the ones needed for the new quirk.

Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.686
diff -u -r1.686 usbdevs
--- usbdevs 28 Jun 2018 15:02:06 -  1.686
+++ usbdevs 3 Jul 2018 06:39:14 -
@@ -2575,8 +2575,10 @@
 product LOGITECH WEBCAMC2000x0802  Webcam C200
 product LOGITECH WEBCAMC5000x0807  Webcam C500
 product LOGITECH QUICKCAMPRO   0x0810  QuickCam Pro
+product LOGITECH WEBCAMC2100x0819  Webcam C210
 product LOGITECH WEBCAMC3100x081b  Webcam C310
 product LOGITECH HDPROC910 0x0821  HD Pro Webcam C910
+product LOGITECH WEBCAMC2700x0825  Webcam C270
 product LOGITECH QUICKCAMEXP   0x0840  QuickCam Express
 product LOGITECH QUICKCAM  0x0850  QuickCam
 product LOGITECH QUICKCAMNBDLX 0x08a9  QuickCam Notebook Deluxe
Index: usbdevs.h
===
RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v
retrieving revision 1.698
diff -u -r1.698 usbdevs.h
--- usbdevs.h   28 Jun 2018 15:04:00 -  1.698
+++ usbdevs.h   3 Jul 2018 06:39:17 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: usbdevs.h,v 1.698 2018/06/28 15:04:00 kevlo Exp $ */
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMATICALLY GENERATED.  DO NOT EDIT.
@@ -2582,8 +2582,10 @@
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC200 0x0802  /* Webcam C200 
*/
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC500 0x0807  /* Webcam C500 
*/
 #defineUSB_PRODUCT_LOGITECH_QUICKCAMPRO0x0810  /* 
QuickCam Pro */
+#defineUSB_PRODUCT_LOGITECH_WEBCAMC210 0x0819  /* Webcam C210 
*/
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC310 0x081b  /* Webcam C310 
*/
 #defineUSB_PRODUCT_LOGITECH_HDPROC910  0x0821  /* HD Pro 
Webcam C910 */
+#defineUSB_PRODUCT_LOGITECH_WEBCAMC270 0x0825  /* Webcam C270 
*/
 #defineUSB_PRODUCT_LOGITECH_QUICKCAMEXP0x0840  /* 
QuickCam Express */
 #defineUSB_PRODUCT_LOGITECH_QUICKCAM   0x0850  /* QuickCam */
 #defineUSB_PRODUCT_LOGITECH_QUICKCAMNBDLX  0x08a9  /* 
QuickCam Notebook Deluxe */
Index: usbdevs_data.h
===
RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v
retrieving revision 1.692
diff -u -r1.692 usbdevs_data.h
--- usbdevs_data.h  28 Jun 2018 15:04:00 -  1.692
+++ usbdevs_data.h  3 Jul 2018 06:39:19 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: usbdevs_data.h,v 1.692 2018/06/28 15:04:00 kevlo Exp $
*/
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMATICALLY GENERATED.  DO NOT EDIT.
@@ -5714,12 +5714,20 @@
"QuickCam Pro",
},
{
+   USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC210,
+   "Webcam C210",
+   },
+   {
USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310,
"Webcam C310",
},
{
USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_HDPROC910,
"HD Pro Webcam C910",
+   },
+   {
+   USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC270,
+   "Webcam C270",
},
{
USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMEXP,



Re: add a bunch of logitech webcam ids to usbdevs

2018-07-02 Thread Landry Breuil
On Mon, Jul 02, 2018 at 09:29:04PM +0200, Mark Kettenis wrote:
> > Date: Mon, 2 Jul 2018 21:10:26 +0200
> > From: Landry Breuil 
> > 
> > ok ?
> 
> Why?  How do they show up right now?  We typically only add USB IDs
> for hardware that needs quirks or doesn't have a useful string
> embedded in the hardware.

the C210 & C270 ones are needed for a new quirk i'm working on to fix
uaudio(4) detection. The other ones appear properly, so i can add only
the ones that need a quirk if preferred.

Landry



add a bunch of logitech webcam ids to usbdevs

2018-07-02 Thread Landry Breuil
ok ?

Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.686
diff -u -r1.686 usbdevs
--- usbdevs 28 Jun 2018 15:02:06 -  1.686
+++ usbdevs 2 Jul 2018 19:08:08 -
@@ -2575,8 +2575,12 @@
 product LOGITECH WEBCAMC2000x0802  Webcam C200
 product LOGITECH WEBCAMC5000x0807  Webcam C500
 product LOGITECH QUICKCAMPRO   0x0810  QuickCam Pro
+product LOGITECH WEBCAMC2100x0819  Webcam C210
 product LOGITECH WEBCAMC3100x081b  Webcam C310
+product LOGITECH WEBCAMC5100x081d  HD Webcam C510
 product LOGITECH HDPROC910 0x0821  HD Pro Webcam C910
+product LOGITECH WEBCAMC2700x0825  Webcam C270
+product LOGITECH HDPROC920 0x082d  HD Pro Webcam C920
 product LOGITECH QUICKCAMEXP   0x0840  QuickCam Express
 product LOGITECH QUICKCAM  0x0850  QuickCam
 product LOGITECH QUICKCAMNBDLX 0x08a9  QuickCam Notebook Deluxe
Index: usbdevs.h
===
RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v
retrieving revision 1.698
diff -u -r1.698 usbdevs.h
--- usbdevs.h   28 Jun 2018 15:04:00 -  1.698
+++ usbdevs.h   2 Jul 2018 19:08:10 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: usbdevs.h,v 1.698 2018/06/28 15:04:00 kevlo Exp $ */
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMATICALLY GENERATED.  DO NOT EDIT.
@@ -2582,8 +2582,12 @@
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC200 0x0802  /* Webcam C200 
*/
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC500 0x0807  /* Webcam C500 
*/
 #defineUSB_PRODUCT_LOGITECH_QUICKCAMPRO0x0810  /* 
QuickCam Pro */
+#defineUSB_PRODUCT_LOGITECH_WEBCAMC210 0x0819  /* Webcam C210 
*/
 #defineUSB_PRODUCT_LOGITECH_WEBCAMC310 0x081b  /* Webcam C310 
*/
+#defineUSB_PRODUCT_LOGITECH_WEBCAMC510 0x081d  /* HD Webcam 
C510 */
 #defineUSB_PRODUCT_LOGITECH_HDPROC910  0x0821  /* HD Pro 
Webcam C910 */
+#defineUSB_PRODUCT_LOGITECH_WEBCAMC270 0x0825  /* Webcam C270 
*/
+#defineUSB_PRODUCT_LOGITECH_HDPROC920  0x082d  /* HD Pro 
Webcam C920 */
 #defineUSB_PRODUCT_LOGITECH_QUICKCAMEXP0x0840  /* 
QuickCam Express */
 #defineUSB_PRODUCT_LOGITECH_QUICKCAM   0x0850  /* QuickCam */
 #defineUSB_PRODUCT_LOGITECH_QUICKCAMNBDLX  0x08a9  /* 
QuickCam Notebook Deluxe */
Index: usbdevs_data.h
===
RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v
retrieving revision 1.692
diff -u -r1.692 usbdevs_data.h
--- usbdevs_data.h  28 Jun 2018 15:04:00 -  1.692
+++ usbdevs_data.h  2 Jul 2018 19:08:12 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: usbdevs_data.h,v 1.692 2018/06/28 15:04:00 kevlo Exp $
*/
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMATICALLY GENERATED.  DO NOT EDIT.
@@ -5714,12 +5714,28 @@
"QuickCam Pro",
},
{
+   USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC210,
+   "Webcam C210",
+   },
+   {
USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310,
"Webcam C310",
},
{
+   USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC510,
+   "HD Webcam C510",
+   },
+   {
USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_HDPROC910,
"HD Pro Webcam C910",
+   },
+   {
+   USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC270,
+   "Webcam C270",
+   },
+   {
+   USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_HDPROC920,
+   "HD Pro Webcam C920",
},
{
USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMEXP,



Looking for logitech webcams testers/device info

2018-06-30 Thread Landry Breuil
Hi,

sending this to a wider audience on misc@, to fix the microphone (cf
https://marc.info/?t=15298427072=1=2) on a
variety of logitech webcams (mostly the Cxxx{,HD}?) i'd need the lsusb
-v output for the corresponding devices.

If you have a logitech webcam where the mic doesnt work (looking for
'uaudio0: audio descriptors make no sense, error=4' in dmesg) this is
your chance to help fix it.
Of course if you have a logitech webcam where the mic works out of the
box, this information is also valuable !

Install usbutils package, run lsusb to find the device ids
corresponding to the logitech device (starts with 046d:, per
https://usb-ids.gowdy.us/read/UD/046d) and send me privately the output
of:

lsusb -v -d 046d:

where  matches your webcam.

if you receive directly this e-mail in bcc, it's because i've found
occurences of 'audio descriptors make no sense' corresponding to a
logitech webcam in our dmesg archive sent by you, this information is
definitely valuable so please keep sending dmesg per
https://www.openbsd.org/faq/faq4.html#SendDmesg :)

Landry



Re: uaudio: fix logitech c310 integrated mic

2018-06-29 Thread Landry Breuil
On Fri, Jun 29, 2018 at 09:05:56AM +0200, Remco wrote:
> > > If you don't find any better solution, I'd suggest using a device ID
> > > check rather than adding a quirk.  Because such quirk cannot be generic.
> > > In that case you have an off-by-one, but another device might have a
> > > different problem.
> > 
> > That was in the case we'd encounter other devices where the descriptor
> > is bogus and has the same issue, but i agree this is unlikely.
> > 
> 
> More likely for Logitech USB webcams I'm afraid.
> My dad's Logitech C500 showed the same issue several years back.
> (I'll try to find some time to borrow the device and test your patch)
> There's also a report that suggests a C200 may be affected as well.
> (https://marc.info/?l=openbsd-misc=127952275021290=2)

Aaah, very interesting data point. Thanks for the pointer.. maybe a
'generic fix' would be to ignore size checks for all logitech webcams,
or to print a warning but not error out in this case. Is there some kind
of 'usbdevs -v/lsusb -v' database somewhere ?

Landry



Re: uaudio: fix logitech c310 integrated mic

2018-06-28 Thread Landry Breuil
On Mon, Jun 25, 2018 at 11:59:46AM +0200, Martin Pieuchot wrote:
> On 24/06/18(Sun) 14:15, Landry Breuil wrote:
> > Hi,
> > 
> > the logitech c310 is supported by uvideo, but its uaudio fails to
> > attach properly and fallbacks to ugen.
> > 



> > from that point, i dunno if my analysis is wrong or right, nor how to
> > properly handle it, so here's my take:
> > - add a UAUDIO_FLAG_BAD_ADC_LEN quirk
> > - set it for the c310 vendor/product
> > - if the device matches this quirk, aclen++
> > 
> > feedback or better ideas welcome :)
> 
> Did you look at how other OSes deal with that problem?  Maybe you'll
> find a more generic hack.

I've looked on bxr.su but it seems netbsd and freebsd don't have the
c310 in their usbdevs, and https://wiki.freebsd.org/WebcamCompat
mentions it needing this strange 'usbconfig -d ugenX.Y do_request' hack.

afaict, netbsd uses more or less our logic (cf
http://bxr.su/NetBSD/sys/dev/usb/uaudio.c#1914, iirc our uaudio comes
from it)

freebsd has a 5000+ lines uaudio.c in
http://bxr.su/FreeBSD/sys/dev/sound/usb/uaudio.c#955 and i didnt see
support for such specific case.

> If you don't find any better solution, I'd suggest using a device ID
> check rather than adding a quirk.  Because such quirk cannot be generic.
> In that case you have an off-by-one, but another device might have a
> different problem.

That was in the case we'd encounter other devices where the descriptor
is bogus and has the same issue, but i agree this is unlikely.

So checking for USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310,
maybe the device revision, and eventually checking if the advertised
size is 38 ?

Landry



Re: ldapd: fix regress

2018-06-25 Thread Landry Breuil
On Mon, Jun 25, 2018 at 11:08:45AM -0300, Gleydson Soares wrote:
> On Mon, Jun 25, 2018 at 08:49:31AM +0200, Landry Breuil wrote:
> > On Mon, Jun 25, 2018 at 02:25:40AM -0300, Gleydson Soares wrote:
> > > unbreak ldapd regress,
> > > everything seems to be working fine.
> > 
> > the point of the overly complicated grep line was to handle the case
> > where you have a running production ldapd, and you spawn another one for
> > regress.. that's also why it starts on another tcp port.
> 
> currently it's broken, grep doesn't grab the pid since
> there's no pid there in log file.

bah, maybe the debug output format changed.. whatever, anything would be
better, and if your solution works then it's ok for me.



Re: ldapd: fix regress

2018-06-25 Thread Landry Breuil
On Mon, Jun 25, 2018 at 02:25:40AM -0300, Gleydson Soares wrote:
> unbreak ldapd regress,
> everything seems to be working fine.

the point of the overly complicated grep line was to handle the case
where you have a running production ldapd, and you spawn another one for
regress.. that's also why it starts on another tcp port.

Landry



Re: sndiod.8: document how to start with multiple devices

2018-06-24 Thread Landry Breuil
On Sun, Jun 24, 2018 at 06:26:02PM +0200, Alexandre Ratchov wrote:
> On Sun, Jun 24, 2018 at 05:38:11PM +0200, Landry Breuil wrote:
> > On Sun, Jun 24, 2018 at 05:23:46PM +0200, Landry Breuil wrote:
> > > Hi,
> > > 
> > > here's an attempt at documenting the usecase i always forget when
> > > playing with multiple devices/mics. Feedback/reworking/tweaks welcome.
> > 
> > New version after super quick feedback from jmc@, unsure where to put
> > the comma around 'for example' though.
> > 
> > Landry
> > 
> > Index: sndiod.8
> > ===
> > RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
> > retrieving revision 1.2
> > diff -u -r1.2 sndiod.8
> > --- sndiod.818 Jan 2016 11:38:07 -  1.2
> > +++ sndiod.824 Jun 2018 15:37:17 -
> > @@ -484,6 +484,17 @@
> >  Regardless of which device a stream is connected to,
> >  its playback volume knob is exposed.
> >  .Sh EXAMPLES
> > +Start server at boot using two devices, for example
> > +.Xr audio 4
> > +device on
> > +.Pa snd/0
> > +and an external
> > +.Xr uaudio 4
> > +microphone on
> > +.Pa snd/1 :
> > +.Pp
> > +.Dl # rcctl set sndiod flags '-frsnd/0 -frsnd/1'
> > +.Pp
> >  Start server using default parameters, creating an
> >  additional sub-device for output to channels 2:3 only (rear speakers
> >  on most cards), exposing the
> > 
> 
> I've two remarks: 
> 
> - I'd prefer we make this example look like the other examples,
>   without refence to rcctl. I think people can figure out by
>   themselves how to transpose this to rcctl.

Normal joe user rarely starts sndiod manually... this particular example
allows one to copy/paste it in a term and be done with it forever :)

> - The exemple is valid for any devices, not only uaudio
>   one. Furthermore refs to audio(4) may raise the question "should I
>   use device paths like /dev/audio as specified in audio(4)"? The rest
>   of the manual uses "sndio audio device" or just "audio device".

how about 'for example the internal audio device and an external usb
microphone device' without any specific markup/link ?



Re: sndiod.8: document how to start with multiple devices

2018-06-24 Thread Landry Breuil
On Sun, Jun 24, 2018 at 05:23:46PM +0200, Landry Breuil wrote:
> Hi,
> 
> here's an attempt at documenting the usecase i always forget when
> playing with multiple devices/mics. Feedback/reworking/tweaks welcome.

New version after super quick feedback from jmc@, unsure where to put
the comma around 'for example' though.

Landry

Index: sndiod.8
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
retrieving revision 1.2
diff -u -r1.2 sndiod.8
--- sndiod.818 Jan 2016 11:38:07 -  1.2
+++ sndiod.824 Jun 2018 15:37:17 -
@@ -484,6 +484,17 @@
 Regardless of which device a stream is connected to,
 its playback volume knob is exposed.
 .Sh EXAMPLES
+Start server at boot using two devices, for example
+.Xr audio 4
+device on
+.Pa snd/0
+and an external
+.Xr uaudio 4
+microphone on
+.Pa snd/1 :
+.Pp
+.Dl # rcctl set sndiod flags '-frsnd/0 -frsnd/1'
+.Pp
 Start server using default parameters, creating an
 additional sub-device for output to channels 2:3 only (rear speakers
 on most cards), exposing the



sndiod.8: document how to start with multiple devices

2018-06-24 Thread Landry Breuil
Hi,

here's an attempt at documenting the usecase i always forget when
playing with multiple devices/mics. Feedback/reworking/tweaks welcome.

Landry

Index: sndiod.8
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
retrieving revision 1.2
diff -u -r1.2 sndiod.8
--- sndiod.818 Jan 2016 11:38:07 -  1.2
+++ sndiod.824 Jun 2018 15:14:18 -
@@ -484,6 +484,21 @@
 Regardless of which device a stream is connected to,
 its playback volume knob is exposed.
 .Sh EXAMPLES
+
+Start server at boot using two devices (ex: builtin
+.Xr audio 4
+ device on
+.Pa snd/0
+, and an external
+.Xr uaudio 4
+microphone on
+.Pa snd/1
+):
+.Bd -literal -offset indent
+# rcctl set sndiod flags '-frsnd/0 -frsnd/1'
+.Ed
+.Pp
+
 Start server using default parameters, creating an
 additional sub-device for output to channels 2:3 only (rear speakers
 on most cards), exposing the



uaudio: fix logitech c310 integrated mic

2018-06-24 Thread Landry Breuil
Hi,

the logitech c310 is supported by uvideo, but its uaudio fails to
attach properly and fallbacks to ugen.

uaudio0 at uhub0 port 1 configuration 1 interface 2 "Logitech Webcam C310" rev 
2.00/0.12 addr 2
uaudio_identify_ac: AC interface is 2
uaudio_identify_ac: found AC header, vers=100, len=38
uaudio0: audio descriptors make no sense, error=4
ugen0 at uhub0 port 1 configuration 1 "Logitech Webcam C310" rev 2.00/0.12 addr 
2

after a bit of poking, and looking at lsusb -v output i figured out that
the audio descriptor was lying on its size, saying it was 38:

  AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype  1 (HEADER)
bcdADC   1.00
wTotalLength   38

but the sum of the sub-descriptor lengths is in fact 9+12+9+9=39..

  AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype  1 (HEADER)
  AudioControl Interface Descriptor:
bLength12
bDescriptorSubtype  2 (INPUT_TERMINAL)
wTerminalType  0x0201 Microphone
  AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype  3 (OUTPUT_TERMINAL)
wTerminalType  0x0101 USB Streaming
  AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype  6 (FEATURE_UNIT)

if i 'fix' aclen to be 39 in sys/dev/usb/uaudio.c, line 1806 then the
device attaches and works fine:
uaudio0 at uhub1 port 2 configuration 1 interface 2 "Logitech Webcam C310" rev 
2.00/0.12 addr 5

from that point, i dunno if my analysis is wrong or right, nor how to
properly handle it, so here's my take:
- add a UAUDIO_FLAG_BAD_ADC_LEN quirk
- set it for the c310 vendor/product
- if the device matches this quirk, aclen++

feedback or better ideas welcome :)

Landry
Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.128
diff -u -r1.128 uaudio.c
--- uaudio.c30 Dec 2017 23:08:29 -  1.128
+++ uaudio.c24 Jun 2018 11:10:53 -
@@ -62,10 +62,11 @@
 #include 
 
 /* #define UAUDIO_DEBUG */
+#define UAUDIO_DEBUG
 #ifdef UAUDIO_DEBUG
 #define DPRINTF(x) do { if (uaudiodebug) printf x; } while (0)
 #define DPRINTFN(n,x)  do { if (uaudiodebug>(n)) printf x; } while (0)
-intuaudiodebug = 0;
+intuaudiodebug = 5;
 #else
 #define DPRINTF(x)
 #define DPRINTFN(n,x)
@@ -172,6 +173,7 @@
 #define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but 
works */
 #define UAUDIO_FLAG_DEPENDENT   0x0020 /* play and record params must equal */
 #define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_BAD_ADC_LEN 0x0080 /* bad audio control descriptor 
size */
 
 struct uaudio_devs {
struct usb_devno uv_dev;
@@ -222,6 +224,8 @@
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
+   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310 },
+   UAUDIO_FLAG_BAD_ADC_LEN },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
UAUDIO_FLAG_NO_FRAC }
 };
@@ -1806,6 +1810,9 @@
aclen = UGETW(acdp->wTotalLength);
if (offs + aclen > size)
return (USBD_INVAL);
+
+   if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
+   aclen++;
 
if (!(sc->sc_quirks & UAUDIO_FLAG_BAD_ADC) &&
 UGETW(acdp->bcdADC) != UAUDIO_VERSION)


Add 'video' pledge

2018-05-24 Thread Landry Breuil
Hi,

here's two simple diffs (one for the kernel, one for the pledge.2
manpage) that allow me to use my webcam again within firefox when
pledged,, adding 'video' to the main process pledges.

The kernel changes are similar to what was done for 'audio' pledge, and
i took the ioctl list from the ones found in the mozilla codebase:
https://dxr.mozilla.org/mozilla-central/search?q=vidioc=false

Comments and feedback welcome. Of course, to test it using firefox, you
still need to grant your user access to /dev/video*.

On a sidenote, i noticed the 'disklabel' pledge is not documented in the
manpage, there might be others..

Landry
Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.230
diff -u -r1.230 kern_pledge.c
--- kern_pledge.c   28 Apr 2018 12:49:21 -  1.230
+++ kern_pledge.c   24 May 2018 17:00:16 -
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -391,6 +392,7 @@
{ "tmppath",PLEDGE_TMPPATH },
{ "tty",PLEDGE_TTY },
{ "unix",   PLEDGE_UNIX },
+   { "video",  PLEDGE_VIDEO },
{ "vminfo", PLEDGE_VMINFO },
{ "vmm",PLEDGE_VMM },
{ "wpath",  PLEDGE_WPATH },
@@ -1087,6 +1089,30 @@
break;
}
}
+
+   if ((p->p_p->ps_pledge & PLEDGE_VIDEO)) {
+   switch (com) {
+   case VIDIOC_QUERYCAP:
+   case VIDIOC_ENUM_FMT:
+   case VIDIOC_S_FMT:
+   case VIDIOC_G_PARM:
+   case VIDIOC_S_PARM:
+   case VIDIOC_REQBUFS:
+   case VIDIOC_QBUF:
+   case VIDIOC_DQBUF:
+   case VIDIOC_QUERYBUF:
+   case VIDIOC_STREAMON:
+   case VIDIOC_STREAMOFF:
+   case VIDIOC_ENUM_FRAMESIZES:
+   case VIDIOC_ENUM_FRAMEINTERVALS:
+   if (fp->f_type == DTYPE_VNODE &&
+   vp->v_type == VCHR &&
+   cdevsw[major(vp->v_rdev)].d_open == videoopen)
+   return (0);
+   break;
+   }
+   }
+
 
 #if NPF > 0
if ((p->p_p->ps_pledge & PLEDGE_PF)) {
Index: pledge.2
===
RCS file: /cvs/src/lib/libc/sys/pledge.2,v
retrieving revision 1.52
diff -u -r1.52 pledge.2
--- pledge.216 Mar 2018 07:11:03 -  1.52
+++ pledge.224 May 2018 17:08:42 -
@@ -116,6 +116,7 @@
 .Va route ,
 .Va tape ,
 .Va tty ,
+.Va video ,
 and
 .Va vmm .
 .It Xo
@@ -547,6 +548,26 @@
 .Dv AUDIO_SETPAR ,
 .Dv AUDIO_START ,
 .Dv AUDIO_STOP
+.It Va video
+Allows a subset of
+.Xr ioctl 2
+operations on
+.Xr video 4
+devices:
+.Pp
+.Dv VIDIOC_DQBUF ,
+.Dv VIDIOC_ENUM_FMT ,
+.Dv VIDIOC_ENUM_FRAMEINTERVALS ,
+.Dv VIDIOC_ENUM_FRAMESIZES ,
+.Dv VIDIOC_G_PARM ,
+.Dv VIDIOC_QBUF ,
+.Dv VIDIOC_QUERYBUF ,
+.Dv VIDIOC_QUERYCAP ,
+.Dv VIDIOC_S_FMT ,
+.Dv VIDIOC_S_PARM ,
+.Dv VIDIOC_STREAMOFF ,
+.Dv VIDIOC_STREAMON ,
+.Dv VIDIOC_REQBUFS
 .It Va bpf
 Allow
 .Dv BIOCGSTATS


Re: Faster msdosfs read

2018-04-30 Thread Landry Breuil
On Sat, Apr 28, 2018 at 04:55:29PM +0200, Martin Pieuchot wrote:
> On 26/04/18(Thu) 23:06, Landry Breuil wrote:
> > On Thu, Apr 26, 2018 at 10:01:25PM +0200, Martin Pieuchot wrote:
> > > This is the 4th attempt to implement clustering for msdosfs.  See [0]
> > > for the previous story.
> > > 
> > > This version implements the strict necessary to read a maximum of 64k
> > > per read.  Unlike the previous version we no longer mix cluster and
> > > block numbers.
> > > 
> > > This speeds up msdosfs reads significantly.  Regress tests are passing.
> > > 
> > > I'd appreciate more tests and reviews.
> > 
> > Well, i see a 2.5x dropout in perfs.. rsyncing a 100mb dir from the same
> > usb key:
> > 
> > sd1 at scsibus4 targ 1 lun 0: <Kingston, DataTraveler G2, 1.00> SCSI2 
> > 0/direct removable serial.09511624F0417921125B
> > sd1: 3689MB, 512 bytes/sector, 728 sectors
> > 
> > without the diff:
> > sent 101,634,811 bytes  received 153 bytes  2,540,870.35 bytes/sec
> > 
> > with the diff:
> > sent 101,634,811 bytes  received 153 bytes  936,727.78 bytes/sec
> > sent 101,634,811 bytes  received 153 bytes  928,173.19 bytes/sec
> > 
> > To be analysed ...
> 
> Thanks to landry I now understand why we cannot keep using "block numbers"
> as logical value to index buffers.
> 
> The VFS clustering code assume that the next logical block number correspond
> to the next block on disk.  In the case of FAT it is true for cluster.
> 
> So I believe we should commit the diff below.

With this diff, using a generic 4gb usb disk with a ubuntu iso, i get a
nice 5x improvement operating on a single 1.2G file, after a cold boot
and no prior access to the actual file:

$time md5 /mnt/sd2/CASPER/filesystem.squashfs
MD5 (/mnt/sd2/CASPER/filesystem.squashfs) = 341d91956976a601de687b80a23a4d3c
1m05.26s real 0m04.35s user 0m04.20s system

$time rsync --stats /mnt/sd2/CASPER/filesystem.squashfs /tmp
sent 1,237,978,281 bytes  received 35 bytes  15,770,424.41 bytes/sec
total size is 1,237,676,032  speedup is 1.00
1m17.82s real 0m06.91s user 0m18.78s system

Same tests without the diff:

$time rsync --stats /mnt/sd2/CASPER/filesystem.squashfs /tmp
sent 1,237,978,281 bytes  received 35 bytes  3,583,149.97 bytes/sec
total size is 1,237,676,032  speedup is 1.00
5m45.39s real 0m07.38s user 0m29.90s system

$time md5 /mnt/sd2/CASPER/filesystem.squashfs
MD5 (/mnt/sd2/CASPER/filesystem.squashfs) = 341d91956976a601de687b80a23a4d3c
5m38.83s real 0m04.79s user 0m13.41s system

Landry



Re: Faster msdosfs read

2018-04-26 Thread Landry Breuil
On Thu, Apr 26, 2018 at 10:01:25PM +0200, Martin Pieuchot wrote:
> This is the 4th attempt to implement clustering for msdosfs.  See [0]
> for the previous story.
> 
> This version implements the strict necessary to read a maximum of 64k
> per read.  Unlike the previous version we no longer mix cluster and
> block numbers.
> 
> This speeds up msdosfs reads significantly.  Regress tests are passing.
> 
> I'd appreciate more tests and reviews.

Well, i see a 2.5x dropout in perfs.. rsyncing a 100mb dir from the same
usb key:

sd1 at scsibus4 targ 1 lun 0:  SCSI2 0/direct 
removable serial.09511624F0417921125B
sd1: 3689MB, 512 bytes/sector, 728 sectors

without the diff:
sent 101,634,811 bytes  received 153 bytes  2,540,870.35 bytes/sec

with the diff:
sent 101,634,811 bytes  received 153 bytes  936,727.78 bytes/sec
sent 101,634,811 bytes  received 153 bytes  928,173.19 bytes/sec

To be analysed ...

Landry



Re: net80211: stub SIOCS80211SCAN, make ifconfig scan instant

2018-04-26 Thread Landry Breuil
On Wed, Apr 25, 2018 at 09:14:42PM +0300, Paul Irofti wrote:
> On Wed, Apr 25, 2018 at 08:55:26PM +0300, Paul Irofti wrote:
> > Hi,
> > 
> > The following diff removes the functionality of the SIOCS80211SCAN ioctl.
> > After long discussions with stps@, mpi@, and deraadt@ we decided that
> > this was the correct way of fixing ifconfig scan from blocking the
> > network stack.
> > 
> > The kernel will continue scanning in the background and filling the
> > nodes array, but ifconfig scan commands will now basically do just a
> > SIOCG80211ALLNODES and pretty print the array. So the output stays the
> > same but is instant.
> > 
> > In fact, when the interface is freshly brought up, if you type fast
> > enough, you can see the array being filled by running multiple ifconfig
> > scans in sequence.
> > 
> > Another change that this introduces is the fact that ifconfig scan no
> > longer plays with UP and DOWN. If the interface is down it complains and
> > exits. This is needed in order to maintain the nodes list.
> > 
> > Currently tested on iwm(4), urtwn(4), run(4) and athn(4). We are
> > interested in wider testing, specially on iwn(4) and athn(4) in client
> > mode.
> > 
> > Thoughts? OKs?
> > 
> > Paul
> 
> Updated diff that fixes scan on hostap mode. This removes the hostap
> check from SIOCG80211ALLNODES that I added by mistake.

Works as advertised on x200s w/
iwn0 at pci2 dev 0 function 0 "Intel WiFi Link 5100" rev 0x00: msi, MIMO 1T2R, 
MoW



query usb device info at device creation

2018-04-25 Thread Landry Breuil
Hi,

prompted by mpi@ i tried simplifying the way we get the
vendor/product/serial info from the device. Currently this is done
lazily, and sometimes several times. Instead of this, let's query the
device in usbd_new_device(), fallback to the global known lists then to
the USB IDs, cache the info in struct usbd_device and reuse it where
needed instead of pollin the device again.

The comment about vendor/product being possibly NULL in usbdivar.h might
need to be amended, since this is no longer the case.

As a bonus it allows us to remove the usedev parameter that was being
passed around.

Looking for testers, working fine here on my x1, with devices providing
the information directly, devices gettting the info from the global
usb_known_product/usb_known_vendor lists, and devices defaulting to
print the USB IDs as a fallback (which is the case within bsd.rd)

To look for regressions, compare usbdevs -vd and lsusb -v (from usbutils
pkg) outputs with and without the diff.

Landry
? usb_subr.c.dbg
Index: ugen.c
===
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.97
diff -u -r1.97 ugen.c
--- ugen.c  30 Dec 2017 23:08:29 -  1.97
+++ ugen.c  25 Apr 2018 18:32:36 -
@@ -1202,7 +1202,7 @@
}
case USB_GET_DEVICEINFO:
usbd_fill_deviceinfo(sc->sc_udev,
-(struct usb_device_info *)addr, 1);
+(struct usb_device_info *)addr);
break;
default:
return (EINVAL);
Index: uhid.c
===
RCS file: /cvs/src/sys/dev/usb/uhid.c,v
retrieving revision 1.70
diff -u -r1.70 uhid.c
--- uhid.c  30 Dec 2017 20:46:59 -  1.70
+++ uhid.c  25 Apr 2018 18:32:36 -
@@ -364,7 +364,7 @@
 
case USB_GET_DEVICEINFO:
usbd_fill_deviceinfo(sc->sc_hdev.sc_udev,
-(struct usb_device_info *)addr, 1);
+(struct usb_device_info *)addr);
break;
 
case USB_GET_REPORT_DESC:
Index: umass_scsi.c
===
RCS file: /cvs/src/sys/dev/usb/umass_scsi.c,v
retrieving revision 1.45
diff -u -r1.45 umass_scsi.c
--- umass_scsi.c3 Aug 2016 13:44:49 -   1.45
+++ umass_scsi.c25 Apr 2018 18:32:36 -
@@ -160,7 +160,7 @@
if (sc->maxlun > 0)
return (0);
 
-   usbd_fill_deviceinfo(sc->sc_udev, , 1);
+   usbd_fill_deviceinfo(sc->sc_udev, );
 
/*
 * Create a fake devid using the vendor and product ids and the last
Index: uoak_subr.c
===
RCS file: /cvs/src/sys/dev/usb/uoak_subr.c,v
retrieving revision 1.8
diff -u -r1.8 uoak_subr.c
--- uoak_subr.c 9 Jan 2017 14:44:28 -   1.8
+++ uoak_subr.c 25 Apr 2018 18:32:36 -
@@ -231,7 +231,7 @@
 uoak_get_devinfo(struct uoak_softc *sc)
 {
/* get device serial# */
-   usbd_fill_deviceinfo(sc->sc_udev, >sc_udi, 1);
+   usbd_fill_deviceinfo(sc->sc_udev, >sc_udi);
 }
 
 void
Index: usb.c
===
RCS file: /cvs/src/sys/dev/usb/usb.c,v
retrieving revision 1.118
diff -u -r1.118 usb.c
--- usb.c   26 Feb 2018 13:06:49 -  1.118
+++ usb.c   25 Apr 2018 18:32:36 -
@@ -527,7 +527,7 @@
if (dev == NULL)
return;
 
-   usbd_fill_deviceinfo(dev, di, 0);
+   usbd_fill_deviceinfo(dev, di);
 }
 
 void
Index: usb_subr.c
===
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.135
diff -u -r1.135 usb_subr.c
--- usb_subr.c  24 Apr 2018 17:22:33 -  1.135
+++ usb_subr.c  25 Apr 2018 18:32:37 -
@@ -66,6 +66,7 @@
 intusbd_getnewaddr(struct usbd_bus *);
 intusbd_print(void *, const char *);
 void   usbd_free_iface_data(struct usbd_device *, int);
+intusbd_cache_devinfo(struct usbd_device *);
 usbd_statususbd_probe_and_attach(struct device *,
struct usbd_device *, int, int);
 
@@ -230,70 +231,70 @@
return (buf);
 }
 
-void
-usbd_devinfo_vp(struct usbd_device *dev, char *v, size_t vl,
-char *p, size_t pl, int usedev)
+int
+usbd_cache_devinfo(struct usbd_device *dev)
 {
usb_device_descriptor_t *udd = >ddesc;
-   char *vendor = NULL, *product = NULL;
-#ifdef USBVERBOSE
-   const struct usb_known_vendor *ukv;
-   const struct usb_known_product *ukp;
-#endif
 
-   if (dev == NULL) {
-   v[0] = p[0] = '\0';
-   return;
-   }
+   dev->serial = malloc(USB_MAX_STRING_LEN, M_USB, M_NOWAIT);
+   if (dev->serial == NULL)
+   return (ENOMEM);
 
-   if (usedev) {
-   vendor = 

Re: fill device description with product name in uvideo(4)

2018-04-24 Thread Landry Breuil
On Tue, Apr 24, 2018 at 04:04:34PM +0100, Stuart Henderson wrote:
> On 2018/04/24 16:34, Landry Breuil wrote:
> > Hi,
> > 
> > here's a diff to reuse usbd_devinfo_vp() (exposed in usbdivar.h) in
> > VIDIOC_QUERYCAP ioctl callback, this way we can fill v4l2_capability
> > card struct member with the actual usb product name instead of a dummy
> > "Generic USB video class device".
> 
> Generally I think this is a good idea, but some devices have crap in
> v/p, is that likely to cause any problems?

Well, the exact same function is called in usbd_devinfo() during boot to
generate the dmesg lines for the devices.. so i dont really see how it makes
things worse.

uvideo0 at uhub1 port 1 configuration 1 interface 0 "Logitech QuickCam Orbit 
AF" rev 2.00/0.08 addr 3
uvideo1 at uhub1 port 8 configuration 1 interface 0 "Chicony Electronics 
Co.,Ltd. Integrated Camera" rev 2.00/0.29 addr 5

Landry



fill device description with product name in uvideo(4)

2018-04-24 Thread Landry Breuil
Hi,

here's a diff to reuse usbd_devinfo_vp() (exposed in usbdivar.h) in
VIDIOC_QUERYCAP ioctl callback, this way we can fill v4l2_capability
card struct member with the actual usb product name instead of a dummy
"Generic USB video class device".

Firefox uses that ioctl to get the user-facing device name in
https://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/linux/device_info_linux.cc#313
so that's helpful when you have several devices
and the webrtc doorhanger asks you which camera you want to
share.. cf http://i.imgur.com/uLnWw6u.png

USB_MAX_STRING_LEN is 127 and card is 32 in v4l2_capability, so strlcpy
should take care of null-terminating/truncating.

Thx mpi@ for the directions..

Landry
Index: usb_subr.c
===
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.134
diff -u -r1.134 usb_subr.c
--- usb_subr.c  8 Apr 2017 02:57:25 -   1.134
+++ usb_subr.c  24 Apr 2018 14:20:00 -
@@ -61,8 +61,6 @@
 
 usbd_statususbd_set_config(struct usbd_device *, int);
 void   usbd_devinfo(struct usbd_device *, int, char *, size_t);
-void   usbd_devinfo_vp(struct usbd_device *, char *, size_t,
-   char *, size_t, int);
 char   *usbd_get_device_string(struct usbd_device *, uByte);
 char   *usbd_get_string(struct usbd_device *, int, char *, size_t);
 intusbd_getnewaddr(struct usbd_bus *);
Index: usbdivar.h
===
RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.73
diff -u -r1.73 usbdivar.h
--- usbdivar.h  3 Feb 2018 13:37:37 -   1.73
+++ usbdivar.h  24 Apr 2018 14:20:00 -
@@ -257,6 +257,8 @@
 usbd_statususb_insert_transfer(struct usbd_xfer *);
 void   usb_transfer_complete(struct usbd_xfer *);
 intusbd_detach(struct usbd_device *, struct device *);
+void   usbd_devinfo_vp(struct usbd_device *, char *, size_t,
+   char *, size_t, int);
 
 /* Routines from usb.c */
 void   usb_needs_explore(struct usbd_device *, int);
Index: uvideo.c
===
RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
retrieving revision 1.196
diff -u -r1.196 uvideo.c
--- uvideo.c30 Dec 2017 23:08:29 -  1.196
+++ uvideo.c24 Apr 2018 14:20:00 -
@@ -2781,11 +2781,14 @@
 uvideo_querycap(void *v, struct v4l2_capability *caps)
 {
struct uvideo_softc *sc = v;
+   char vendor[USB_MAX_STRING_LEN];
+   char product[USB_MAX_STRING_LEN];
 
bzero(caps, sizeof(*caps));
strlcpy(caps->driver, DEVNAME(sc), sizeof(caps->driver));
-   strlcpy(caps->card, "Generic USB video class device",
-   sizeof(caps->card));
+   usbd_devinfo_vp(sc->sc_udev, vendor, sizeof (vendor), product,
+   sizeof (product), 1);
+   strlcpy(caps->card, product, sizeof(caps->card));
strlcpy(caps->bus_info, "usb", sizeof(caps->bus_info));
 
caps->version = 1;


Re: [patch] Remove redundant quotes in sys/sys/resource.h and sys/sys/sysctl.h

2018-04-20 Thread Landry Breuil
On Fri, Apr 20, 2018 at 06:53:44AM -0500, Jimmy Hess wrote:
> On Fri, Apr 20, 2018 at 3:57 AM, Theo Buehler  wrote:
> > I don't think these are "redundant quotes" but rather ditto marks:
> 
> The  ditto mark and the quote character are  not the same character,
> and the comments are nonsensical since they used the wrong character.
> 
> If the writer means Ditto mark, then you type the  Ditto character: 〃 (U+3003)
> and not the  double quotation mark symbol:  "  (U+0022)

UTF-8 in source code ? please kill me now.



document netstart's -n flag

2018-03-02 Thread Landry Breuil
Hi,

crude attempt at documenting -n in netstart, the wording can probably be
improved.

Landry

Index: netstart.8
===
RCS file: /cvs/src/share/man/man8/netstart.8,v
retrieving revision 1.21
diff -u -r1.21 netstart.8
--- netstart.8  5 Sep 2016 12:58:17 -   1.21
+++ netstart.8  2 Mar 2018 11:05:24 -
@@ -33,6 +33,7 @@
 .Nd command script for network startup
 .Sh SYNOPSIS
 .Nm /etc/netstart
+.Op Fl n
 .Op Ar interface ...
 .Sh DESCRIPTION
 .Nm
@@ -49,6 +50,17 @@
 extent by variables defined in
 .Xr rc.conf 8 ,
 which specifies which daemons and services are to be run.
+.Pp
+If
+option
+.Fl n
+is used
+.Po
+along an interface argument
+.Pc ,
+.Nm
+will print the commands that it would run to configure the given
+interface.
 .Pp
 During the system boot,
 .Nm



Re: Fixed sys_socket() plumbing

2018-02-12 Thread Landry Breuil
On Mon, Feb 12, 2018 at 09:22:54AM +0100, Martin Pieuchot wrote:
> I found my mistake in the previous diff.  `SS_NBIO' was never set on
> non blocking sockets.  Diff below fixes that by checking `nonblock'
> like it is done in sys_socketpair().
> 
> Tests & oks welcome :)

Running with it now on my $work desktop (where i had also seen the
"firefox waiting forever on dns resolution" issue).

Landry



Re: fixing kvm lapic hangs

2018-02-07 Thread Landry Breuil
On Thu, Feb 08, 2018 at 04:32:29PM +1300, Jonathan Matthew wrote:
> This diff (most of which has been around for a while) changes delay_func
> when running in KVM to use pvclock (effectively the TSC) to determine when to
> stop spinning.  Since this is done in a KVM-specific driver, it won't have
> any effect anywhere other than in KVM guests.
> 
> Using pvclock rather than the lapic avoids the hangs caused by KVM's use of
> the VMX preemption timer to emulate a lapic.  To summarise that problem:
> occasionally KVM fails to restart the lapic timer until it gets an exit from
> the guest, and if we're busy polling the lapic, we'll never generate such an
> exit, and the lapic counter will never reach the value we're waiting for, at
> which point we're stuck.

>From my understanding of your explanation, that fixes the hangs i was
seeing with KVM vms, hangs which were worked around by disabling the
host 'preemption timer' via putting kvm-intel.preemption_timer=0 on the
host kernel commandline.. so if i get it right, once this diff is in, no
need for this workaround on the host ? Correct ?

> A couple of testers have confirmed that this does *not* fix the time slowdowns
> seen on KVM guests.  I believe fixing that will require more invasive changes.

Sign me up for testing those changes then !
Thanks for your work on it :)

Landry



Re: syslog.conf(5): example about logging by sender

2018-02-01 Thread Landry Breuil
On Thu, Feb 01, 2018 at 02:26:16PM +0100, Alexander Bluhm wrote:
> On Thu, Feb 01, 2018 at 09:08:04AM +0100, Landry Breuil wrote:
> > > # Log everything coming from host bastion to a separate file
> > > ++bastion /var/log/bastion
> > > *.*
> > > +*
> > 
> > well maybe that's clearer, but the version without *.* works here.. i
> > dont have a preference, so i'll defer to experts :)
> 
> I wonder how that works.  I have tried it and it does not, my
> understanding of the code is that everything after ++bastion on
> that line is not parsed.
> 
> The log file name must be in the same line as the severity and
> facility.  The hostname just starts the block.  And the block should
> be closed.  I put that block at the beginning of my syslog.conf.
> 
> ++bastion
> *.*   /var/log/bastion
> +*

Dammit, you're right again :) I had a closer look, and indeed the
version without the facility.severity *doesn't* work.

So indeed this version should be better:

Index: syslog.conf.5
===
RCS file: /cvs/src/usr.sbin/syslogd/syslog.conf.5,v
retrieving revision 1.34
diff -u -r1.34 syslog.conf.5
--- syslog.conf.5   6 Jul 2016 19:29:13 -   1.34
+++ syslog.conf.5   1 Feb 2018 14:05:06 -
@@ -306,6 +306,11 @@
 # Root and Eric get alert and higher messages.
 *.alertroot,eric
 
+# Log everything coming from host bastion to a separate file
+++bastion
+*.*/var/log/bastion
++*
+
 # Save mail and news errors of level err and higher in a
 # special file.
 mail,news.err  /var/log/spoolerr



Re: syslog.conf(5): example about logging by sender

2018-02-01 Thread Landry Breuil
On Wed, Jan 31, 2018 at 03:06:13PM -0700, Todd C. Miller wrote:
> On Wed, 31 Jan 2018 20:44:10 +0100, Landry Breuil wrote:
> 
> > the default etc/syslog.conf has a commented out example for by-prog
> > logging, but nothing for by-host logging. I fighted a bit with it; so
> > why not providing an example in the EXAMPLES section of the manpage ?
> 
> I recently did the same so an example would be nice.  I do have one
> question though.
> 
> > Index: syslog.conf.5
> > ===
> > RCS file: /cvs/src/usr.sbin/syslogd/syslog.conf.5,v
> > retrieving revision 1.34
> > diff -u -r1.34 syslog.conf.5
> > --- syslog.conf.5   6 Jul 2016 19:29:13 -   1.34
> > +++ syslog.conf.5   31 Jan 2018 19:40:59 -
> > @@ -306,6 +306,10 @@
> >  # Root and Eric get alert and higher messages.
> >  *.alertroot,eric
> >  
> > +# Log everything coming from host bastion to a separate file
> > +++bastion  /var/log/bastion
> > ++*
> 
> Shouldn't this be:
> 
> # Log everything coming from host bastion to a separate file
> ++bastion /var/log/bastion
> *.*
> +*

well maybe that's clearer, but the version without *.* works here.. i
dont have a preference, so i'll defer to experts :)

Landry



syslog.conf(5): example about logging by sender

2018-01-31 Thread Landry Breuil
Hi,

the default etc/syslog.conf has a commented out example for by-prog
logging, but nothing for by-host logging. I fighted a bit with it; so
why not providing an example in the EXAMPLES section of the manpage ?

Landry

Index: syslog.conf.5
===
RCS file: /cvs/src/usr.sbin/syslogd/syslog.conf.5,v
retrieving revision 1.34
diff -u -r1.34 syslog.conf.5
--- syslog.conf.5   6 Jul 2016 19:29:13 -   1.34
+++ syslog.conf.5   31 Jan 2018 19:40:59 -
@@ -306,6 +306,10 @@
 # Root and Eric get alert and higher messages.
 *.alertroot,eric
 
+# Log everything coming from host bastion to a separate file
+++bastion  /var/log/bastion
++*
+
 # Save mail and news errors of level err and higher in a
 # special file.
 mail,news.err  /var/log/spoolerr



Re: [patch] Remove superfluous seek_filesize() from less

2017-09-25 Thread Landry Breuil
On Mon, Sep 25, 2017 at 10:19:16AM +0200, Jesper Wallin wrote:
> Shameless bump, now when we're out of beta. :-)

being out of beta means we're in release mode..



Re: caesar(6) documents incorrect frequencies

2017-08-03 Thread Landry Breuil
On Thu, Aug 03, 2017 at 11:20:15AM +0200, Daniel Hartmeier wrote:
> Maybe you mean Etaoin Shrdlu, it has a fascinating story
> 
>   https://archive.org/details/FarewellEtaoinShrdlu

Wow, just wow. Thanks for this piece of history :)



Re: make pkg_info -Q work with other flags

2017-08-03 Thread Landry Breuil
On Wed, Aug 02, 2017 at 12:11:13PM -0600, Aaron Bieber wrote:
> On Sat, Jul 29, 2017 at 10:40:37AM -0600, Aaron Bieber wrote:
> > Hola,
> > 
> > Currently "pkg_info -Q" doesn't respect other flags and the way
> > pkg_info(1) reads, it implies that they will work with it.
> > 
> > This diff makes pkg_info function as expected when other flags are
> > passed when using -Q.
> 
> Ping. Clue sticks? OKs?

Makes sense to me, allows for example to do:

$pkg_info -Q firefox -s
Information for 
https://packages.rhaalovely.net/pub/OpenBSD/snapshots/packages/amd64/firefox-55.0rc1.tgz

Size: 147593632

or
$pkg_info -Q xfwm4 -f (which shows the packing-list for xfwm4 and
xfwm4-themes)

ok



Re: introduction of an additional non-POSIX function in libpthread

2017-07-18 Thread Landry Breuil
On Tue, Jul 18, 2017 at 01:43:42PM +0100, David CARLIER wrote:
> Ah I recognise you you re Mozilla contributor right ? You re probably right
> but that was just an example eventhough I admit it s not extremely widely
> used ...

Note that you'll have to be careful in your arguments for this function:

- _np prefix is for 'nonportable' so nothing standardises it
- some oses have getname_np, freebsd has get_name_np
- linux and netbsd share the function name but not the prototype for
  setname_mp ?
- macos enforces the running thread to set its own name

(cf https://stackoverflow.com/a/7989973)

so this looks like a shitshow, and a pretty good reason for not
having it :)



Re: introduction of an additional non-POSIX function in libpthread

2017-07-18 Thread Landry Breuil
On Tue, Jul 18, 2017 at 01:24:21PM +0100, David CARLIER wrote:
> Hi.
> 
> I sent a diff originally to a smaller audience but finally decided to post
> it in the mailing list.
> So it is to introduce the pthread_set_name_np's counterpart so
> pthread_get_name_np.
> Some softwares use it (NSPR for example) and anyway internally in my
> workplace we have multiplatform needing this feature would be nice having
> this for OpenBSD as well :-)

the js engine uses it:
https://bugzilla.mozilla.org/show_bug.cgi?id=1298451

but nspr stopped using it or never used it, there's only a reference to
pthread_set_name_np:
https://hg.mozilla.org/projects/nspr/file/tip/pr/src/pthreads/ptthread.c#l1759

PR_GetThreadName (just below) returns the 'local' name value, and doesnt
get it from the system.

Landry



Re: cwm: remove ssh auto-completion

2017-07-10 Thread Landry Breuil
On Mon, Jul 10, 2017 at 04:04:47PM +0200, Ingo Schwarze wrote:
> Hi,
> 
> Bryan Steele wrote on Mon, Jul 10, 2017 at 09:38:32AM -0400:
> 
> > This was a feature added last year by nicm@, not touching emacs.c
> > at all..
> 
> edit.c is a helper file containing common utilities for emacs.c
> and vi.c.  So it is also misdocumented.  It is only documented
> for emacs mode, but vi mode does it, too.
> 
> > http://marc.info/?l=openbsd-cvs=147300974112400=2
> > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/bin/ksh/edit.c.diff?r1=1.57=1.54
> 
> I must have missed it.
> I probably would have opposed it if i had noticed it.

Ingo,

i agree that sometimes you need to keep stuff clean and readable and
etc, but if you are at a point that your shell doesn't offer you
'basic' features (present in almost every other shell) that *improves*
your productivity and you need to use another shell for that, then
you've lost.

I, for one, welcome the ability to easily define such completions,
especially for ssh hosts when you have to manage dozens of them.

Landry



Re: dhcpd: don't reject DHCPINFORM from behind relay

2017-07-07 Thread Landry Breuil
On Thu, Jul 06, 2017 at 11:28:11AM +0200, Landry Breuil wrote:
> On Wed, Jul 05, 2017 at 04:37:39PM +0200, Reyk Floeter wrote:
> > Hi,
> > 
> > landry@ sees many log messages 'DHCPINFORM from xx but ciaddr yy is
> > not consistent with actual address' in a setup where dhcpd runs behind
> > dhcrelay.
> > 
> > The code in dhcpd's dhcpinform() seems wrong - it assumes that ciaddr
> > (the client IP) is identical to the packet source address and it
> > doesn't consider a relay, indicated by giaddr (gateway).
> > 
> > I looked at isc-dhcpd code and, omg my eyes are bleeding, it seems to
> > handle DHCPINFORM from relayed clients with giaddr set.
> > 
> > So it seems that dhcpd never accepted DHCPINFORM from clients behind a
> > relay, and the diff below changes it and stops the log spam.  But it
> > also changes behavior, so it needs some testing and maybe feedback
> > from DHCP experts (prodding krw).
> 
> I've put this on a 6.1 vm behind a dhcrelay at work, previously the
> w7 clients were apparently sending DHCPINFORM every ~10mn, and in bursts:
> 
> Jul  5 14:52:42 oberalp dhcpd[92834]: DHCPINFORM from 172.20.85.254 but 
> ciaddr 172.20.85.124 is not consistent with actual address
> Jul  5 14:52:45 oberalp dhcpd[92834]: DHCPINFORM from 172.20.85.254 but 
> ciaddr 172.20.85.124 is not consistent with actual address
> Jul  5 14:54:11 oberalp last message repeated 2 times
> Jul  5 14:55:23 oberalp dhcpd[92834]: DHCPINFORM from 172.20.85.254 but 
> ciaddr 172.20.85.107 is not consistent with actual address
> Jul  5 14:55:26 oberalp dhcpd[92834]: DHCPINFORM from 172.20.85.254 but 
> ciaddr 172.20.85.107 is not consistent with actual address
> Jul  5 14:56:50 oberalp last message repeated 2 times
> 
> With this diff, now the clients still send DHCPINFORM, some every ~10mn,
> some every ~30m, still in bursts, and the DHCPACK is sent back:
> 
> Jul  6 11:08:06 oberalp dhcpd[42317]: DHCPACK to 172.20.85.107 
> () via 172.20.85.254
> Jul  6 11:08:09 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
> Jul  6 11:08:09 oberalp dhcpd[42317]: DHCPACK to 172.20.85.107 
> () via 172.20.85.254
> Jul  6 11:18:13 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
> Jul  6 11:18:13 oberalp dhcpd[42317]: DHCPACK to 172.20.85.107 
> () via 172.20.85.254
> Jul  6 11:18:16 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
> Jul  6 11:18:16 oberalp dhcpd[42317]: DHCPACK to 172.20.85.107 
> () via 172.20.85.254
> Jul  6 11:19:13 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
> Jul  6 11:19:13 oberalp dhcpd[42317]: DHCPACK to 172.20.85.122 
> () via 172.20.85.254
> Jul  6 11:19:16 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
> Jul  6 11:19:16 oberalp dhcpd[42317]: DHCPACK to 172.20.85.122 
> () via 172.20.85.254
> Jul  6 11:20:58 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
> Jul  6 11:20:58 oberalp dhcpd[42317]: DHCPACK to 172.20.85.122 
> () via 172.20.85.254
> Jul  6 11:21:01 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
> Jul  6 11:21:01 oberalp dhcpd[42317]: DHCPACK to 172.20.85.122 
> () via 172.20.85.254

And it seems after a while the win7 clients *finally* stop spamming the
server with DHCPINFORM requests. Yay. Maybe that's because of the hack
put in dhcpd.conf

 option autoproxy-script "\\n";


Something else, which i dunno is related to this diff or not - before, a dhcpd
was receiving a single DHCPREQUEST (ie i ran dhclient on an openbsd client) and
replying to it:

Jul  5 14:22:15 oberalp dhcpd[92834]: DHCPREQUEST for 172.20.85.109 from 
00:24:e8:3d:46:fe via 172.20.85.254
Jul  5 14:22:15 oberalp dhcpd[92834]: DHCPACK on 172.20.85.109 to 
00:24:e8:3d:46:fe via 172.20.85.254

Now, for some linux clients which are re-requesting/renewing a lease, i see two
REQUESTS in the log, one logged as coming directly on vio0, and one coming from
the dhcrelay gateway:

Jul  7 06:16:14 oberalp dhcpd[91347]: DHCPREQUEST for 172.20.85.110 from 
00:14:fd:12:96:8e via vio0
Jul  7 06:16:14 oberalp dhcpd[91347]: DHCPACK on 172.20.85.110 to 
00:14:fd:12:96:8e via vio0
Jul  7 06:16:14 oberalp dhcpd[91347]: DHCPREQUEST for 172.20.85.110 from 
00:14:fd:12:96:8e via 172.20.85.254
Jul  7 06:16:14 oberalp dhcpd[91347]: DHCPACK on 172.20.85.110 to 
00:14:fd:12:96:8e via 172.20.85.254

Jul  7 06:51:10 oberalp dhcpd[91347]: DHCPREQUEST for 172.20.85.111 from 
00:19:b9:fe:b2:10 via vio0
Jul  7 06:51:10 oberalp dhcpd[91347]: DHCPACK on 172.20.85.111 to 
00:19:b9:fe:b2:10 via vio0
Jul  7 06:51:10 oberalp dhcpd[91347]: DHCPREQUEST for 172.20.85.111 from 
00:19:b9:fe:b2:10 via 172.20.85.254
Jul  7 06:51:10 oberalp dhcpd[91347]: DHCPACK on 172.20.85.111 to 
00:19:b9:fe:b2:10 via 172.20.85.254

Dunno if that's an expected behaviour - and it seems it was already
this way before the diff.

F

Re: dhcpd: don't reject DHCPINFORM from behind relay

2017-07-06 Thread Landry Breuil
On Wed, Jul 05, 2017 at 04:37:39PM +0200, Reyk Floeter wrote:
> Hi,
> 
> landry@ sees many log messages 'DHCPINFORM from xx but ciaddr yy is
> not consistent with actual address' in a setup where dhcpd runs behind
> dhcrelay.
> 
> The code in dhcpd's dhcpinform() seems wrong - it assumes that ciaddr
> (the client IP) is identical to the packet source address and it
> doesn't consider a relay, indicated by giaddr (gateway).
> 
> I looked at isc-dhcpd code and, omg my eyes are bleeding, it seems to
> handle DHCPINFORM from relayed clients with giaddr set.
> 
> So it seems that dhcpd never accepted DHCPINFORM from clients behind a
> relay, and the diff below changes it and stops the log spam.  But it
> also changes behavior, so it needs some testing and maybe feedback
> from DHCP experts (prodding krw).

I've put this on a 6.1 vm behind a dhcrelay at work, previously the
w7 clients were apparently sending DHCPINFORM every ~10mn, and in bursts:

Jul  5 14:52:42 oberalp dhcpd[92834]: DHCPINFORM from 172.20.85.254 but ciaddr 
172.20.85.124 is not consistent with actual address
Jul  5 14:52:45 oberalp dhcpd[92834]: DHCPINFORM from 172.20.85.254 but ciaddr 
172.20.85.124 is not consistent with actual address
Jul  5 14:54:11 oberalp last message repeated 2 times
Jul  5 14:55:23 oberalp dhcpd[92834]: DHCPINFORM from 172.20.85.254 but ciaddr 
172.20.85.107 is not consistent with actual address
Jul  5 14:55:26 oberalp dhcpd[92834]: DHCPINFORM from 172.20.85.254 but ciaddr 
172.20.85.107 is not consistent with actual address
Jul  5 14:56:50 oberalp last message repeated 2 times

With this diff, now the clients still send DHCPINFORM, some every ~10mn,
some every ~30m, still in bursts, and the DHCPACK is sent back:

Jul  6 11:08:06 oberalp dhcpd[42317]: DHCPACK to 172.20.85.107 
() via 172.20.85.254
Jul  6 11:08:09 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
Jul  6 11:08:09 oberalp dhcpd[42317]: DHCPACK to 172.20.85.107 
() via 172.20.85.254
Jul  6 11:18:13 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
Jul  6 11:18:13 oberalp dhcpd[42317]: DHCPACK to 172.20.85.107 
() via 172.20.85.254
Jul  6 11:18:16 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
Jul  6 11:18:16 oberalp dhcpd[42317]: DHCPACK to 172.20.85.107 
() via 172.20.85.254
Jul  6 11:19:13 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
Jul  6 11:19:13 oberalp dhcpd[42317]: DHCPACK to 172.20.85.122 
() via 172.20.85.254
Jul  6 11:19:16 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
Jul  6 11:19:16 oberalp dhcpd[42317]: DHCPACK to 172.20.85.122 
() via 172.20.85.254
Jul  6 11:20:58 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
Jul  6 11:20:58 oberalp dhcpd[42317]: DHCPACK to 172.20.85.122 
() via 172.20.85.254
Jul  6 11:21:01 oberalp dhcpd[42317]: DHCPINFORM from 172.20.85.254
Jul  6 11:21:01 oberalp dhcpd[42317]: DHCPACK to 172.20.85.122 
() via 172.20.85.254

dhcpdump of the DHCPINFORM request:

OPTION:  53 (  1) DHCP message type 8 (DHCPINFORM)
OPTION:  61 (  7) Client-identifier 
OPTION:  12 (  7) Host name 
OPTION:  60 (  8) Vendor class identifier   MSFT 5.0
OPTION:  55 ( 13) Parameter Request List  1 (Subnet mask)
 15 (Domainname)
  3 (Routers)
  6 (DNS server)
 44 (NetBIOS name server)
 46 (NetBIOS node type)
 47 (NetBIOS scope)
 31 (Perform router discovery)
 33 (Static route)
121 (Classless Static Route)
249 (MSFT - Classless route)
 43 (Vendor specific info)
252 (MSFT - WinSock Proxy Auto 
Detect)

So i dunno if the clients behavious will adapt over time (searching for
'windows7 dhcpinform' on the interwebs leads to tons of pages..), or if it's
even worse in terms of syslog spam, but at least it seems more 'correct'
dhcp-wise, and apparently the client still has a working network.

Can't really okay it as i dont have all the big picture details,
but at least it doesnt break this setup.

Landry



Re: ports framework change: readme and rc generation

2017-06-26 Thread Landry Breuil
On Mon, Jun 26, 2017 at 12:36:16PM +0200, Marc Espie wrote:
> I need to get this thru my next bulk.
> This should work around several existing issues.
> 
> First, PKGDIR must exist.  Creating it during fake is no longer possible,
> because this doesn't work with dpb in privsep mode.
> 
> Having PKGDIR not be a directory/pointing in the wrong location means you
> might miss MESSAGE/UNMESSAGE (which are not even written down in the PLIST)
> or miss MESSAGE/RCscripts which must exist in the PLIST.
> 
> The more ambitious change involves changing the flow of creating new ports
> to make it slightly faster.
> 
> Mainly, READMEs and RC installation is not really tweakable, since it happens
> at the end of fake. So instead of doing it during fake, do it prior to
> update-plist and package itself and (that's the cool part) do NOT generate
> a cookie.   So it will happen each and every time. Accordingly, wipe existing
> readmes and rc (this assumes RCDIR in the fake area is initially empty,
> which should always be the case).
> 
> That way, a porter should be able to tweak the readmes/rc, add variables, 
> add variable values, and quickly regen packages without having to wipe 
> fake and redo it each time, which is a win.

Oooh yea that's a win ! I suppose rc scripts and README paths will survive make
update-plist once the latter is redone from scratch ?



Re: [PATCH] cp(1): add -v option for verbosity

2017-06-25 Thread Landry Breuil
On Sun, Jun 25, 2017 at 09:59:35PM +0200, Paul de Weerd wrote:
> On Sun, Jun 25, 2017 at 06:22:05PM +0200, Job Snijders wrote:
> | Dear Alexander,
> | 
> | On Sun, Jun 25, 2017 at 06:13:40PM +0200, Alexander Hall wrote:
> | > On June 25, 2017 2:06:20 PM GMT+02:00, Job Snijders  
> wrote:
> | > >This patch adds a -v option to cp(1) for more verbose output.
> | > >
> | > > $ touch a b; mkdir c
> | > > $ cp -v a b c
> | > > 'a' -> 'c/a'
> | > > 'b' -> 'c/b'
> | > > $ cp -rv c d
> | > > 'c' -> 'd/'
> | > > 'c/a' -> 'd/a'
> | > > 'c/b' -> 'd/b'
> | > 
> | > Pardon my ignorance, but why?
> | 
> | A fair question.  I myself have two use cases, but others may have their
> | own to add.
> | 
> | When a glob pattern is used, it can be beneficial to immediately observe
> | (during the execution of the command) which files have been copied.
> | 
> | When copying very large trees, the -v option provides some insight as to
> | what progress the cp operation has made so far.
> 
> I like the -v option for the above reason most (and have missed it on
> several occassions): copy, move or remove a whole bunch of files; it
> takes a while.  Is it working?  Or is NFS stalling / IO to the storage
> device otherwise acting up?
> 
> Also, when using these tools in crons, it can be very useful to have
> cron send out reports of the files copied/moved/deleted.  Note that
> directories can be altered outside of the control of said script: it
> is impossible to deterministically figure out what cp/mv/rm did after
> (or before, as the 'study `find *`' hint suggests) they are run.
> 
> | Alternatively one can use rsync(1), but that is not part of the base.
> 
> That may work for cp(1), but it's hard to replicate mv(1) behavior
> with rsync (only metadata changes when on the same fs) or even
> impossible to replciate rm(1) behavior.

Technically it could be possible to replicate mv with rsync
--remove-source-files ... :)



Re: [PATCH] mv(1): add -v option for verbosity

2017-06-25 Thread Landry Breuil
On Sun, Jun 25, 2017 at 02:41:28PM +0200, Mark Kettenis wrote:
> > Date: Sun, 25 Jun 2017 14:06:11 +0200
> > From: Job Snijders 
> > 
> > Hi all,
> > 
> > This patch adds a -v option to mv(1) for more verbose output.
> > 
> > $ touch a
> > $ mv -v a b
> > 'a' -> 'b'
> > $ mkdir c
> > $ mv -v b c
> > 'b' -> 'c/b'
> > $ mv -v c d
> > 'e' -> 'd'
> > 
> > And here is an example of the output of the situation mentioned in the
> > 'caveats' section in the manpage:
> > 
> > $ touch f g; mkdir -p d/f
> > $ mv -v f g d
> > mv: rename f to d/f: Is a directory
> > 'g' -> 'd/g'
> > $ echo $?
> > 1
> > 
> > Kind regards,
> 
> This is not something we want for OpenBSD.

I beg to differ. Personally, i've sometimes had to use rsync -P (or -i,
or -v, or other equivalents) to have some kind of listing/progress of a
large copy/move operation. Granted, it's not in POSIX or other
standards, and it's in GNU mv/coreutils that everyone loves to hate for
the kitchensink approach, but that shouldnt be a reason. If there's a
sensible usecase, why not ?

Landry



Re: start building clang alongside gcc on amd64

2017-04-17 Thread Landry Breuil
On Mon, Apr 17, 2017 at 09:20:21AM +0200, Mark Kettenis wrote:
> After further discussion it Theo, here is a new version of the diff.
> It explicitly lists the gcc4 architectures now, introduces BUILD_GCC3
> and BUILD_GCC4 variables and fixes installation of the c++ headers.
> 
> Index: gnu/lib/Makefile
> ===
> RCS file: /cvs/src/gnu/lib/Makefile,v
> retrieving revision 1.20
> diff -u -p -r1.20 Makefile
> --- gnu/lib/Makefile  21 Jan 2017 12:40:49 -  1.20
> +++ gnu/lib/Makefile  16 Apr 2017 18:30:53 -
> @@ -6,9 +6,10 @@ SUBDIR+=libiberty libreadline
>  .if make(obj)
>  SUBDIR+=libobjc libstdc++ libstdc++-v3 libsupc++-v3 ../usr.bin/cc/libobjc
>  .else
> -.  if ${COMPILER_VERSION:L} == "gcc3"
> +.  if ${BUILD_GCC3:L} == "yes"
>  SUBDIR+=libobjc libstdc++
> -.  elif ${COMPILER_VERSION:L} == "gcc4"
> +.  endif
> +.  if ${BUILD_GCC4:L} == "gcc4"

Typo here ? Shouldnt this be .  if ${BUILD_GCC4:L} == "yes" ?



Re: Xorg stipple

2017-02-26 Thread Landry Breuil
On Wed, Mar 09, 2016 at 05:09:13PM -0600, joshua stein wrote:
> Is anyone seriously finding video/Xorg bugs through the default X
> stipple pattern anymore?  Xorg changed the default to draw a black
> background a while ago (with stipple enabled using the -retro flag),
> but we have this local change that reverted it while adding a silly
> -retard flag in order to show the black background.
> 
> I think we can finally stop partying like it's 1989 (vax is dead,
> after all) and have X show a solid black background by default.

Reviving this thread because everyone likes to party like it's
1989^W^W^W^Wthe smell of a sunday morning bikeshed From what i
understand, we default to the -retro mode, that option is #ifndef'ed
out, but Xserver(1) still mentions it. We instead provide (since xserver
1.6.4, 8 years ago) an undocumented -retard option which is supposed to
have the default 'black background' behaviour. And then, there's -br
flag.

So.. should we fix the code (from what i understood in the thread,
there was opposition) or the manpage ?
Or document (where?) that one can use xsetroot -solid in
/etc/X11/xdm/Xsetup_0 to paint the default xdm/X background to its
own bikeshed color ?



Re: add empty /root/.ssh/authorized_keys to mtree/sets ?

2017-02-10 Thread Landry Breuil
On Fri, Feb 10, 2017 at 09:36:16AM +0100, Antoine Jacoutot wrote:
> On Thu, Feb 09, 2017 at 06:19:54PM +0100, Landry Breuil wrote:
> > On Sun, Feb 05, 2017 at 08:37:31PM +, Stuart Henderson wrote:
> > > On 2017/02/05 09:53, Robert Peichaer wrote:
> > > > On Sun, Feb 05, 2017 at 10:46:41AM +0100, Landry Breuil wrote:
> > > > > Hi,
> > > > > 
> > > > > when installing 'throwaway' VMs (manually, not always using 
> > > > > autoinstall for
> > > > > $REASONS) i've often found myself having to do right after the 
> > > > > install:
> > > > > install -d -m 700 /root/.ssh
> > > > > install -m 600 /dev/null /root/.ssh/authorized_keys
> > > > > (or touch /root/.ssh/authorized_keys && chmod 600
> > > > > /root/.ssh/authorized_keys, ymmv)
> > > > > 
> > > > > those are present in /etc/skel for "real" users, so why not creating
> > > > > them for the root account ? install.sub also creates /mnt/root/.ssh 
> > > > > when
> > > > > using autoinstall and giving an ssh pubkey, so that'll be one less 
> > > > > step
> > > > > to do there.
> > > > > 
> > > > > We advise ppl to set prohibit-password for PermitRootLogin, so why 
> > > > > not make it
> > > > > easier to use it ? This ways, the correct modes are set.. i often 
> > > > > fat-fingered
> > > > > this, to see sshd complaining (rightly!) about bad modes on 
> > > > > .ssh/authorized_keys.
> > > > 
> > > > Conceptually I'd like this going in.
> > > 
> > > +1. (On "managed" systems I use root-owned authorized_keys in a system 
> > > directory,
> > > but this doesn't get in the way, and it makes things easier on ad-hoc 
> > > installed
> > > systems).
> > 
> > Finally built a release with this, the empty file is created in
> > /var/sysmerge/etc.tgz, and sysmerge didnt overwrite my own
> > /root/.ssh/authorized_keys - so i think i can now explicitely ask for okays.
> > dtucker@ mentioned that in ${INSTALL} -c idiom the -c was a noop, but i 
> > kept it
> > for consistency.
> > Hopefully more ppl can chime in and think of potential drawbacks this
> > diff exposes...
> > 
> > Sets diff added too, modeled after what's done for
> > /etc/skel/.ssh/authorized_keys - dunno if it should be commited along the 
> > etc/
> > change.
> 
> Can you add it to mtree/special please?

Sure ! Here's a new fuller diff touching files all around..

Index: etc/Makefile
===
RCS file: /cvs/src/etc/Makefile,v
retrieving revision 1.449
diff -u -r1.449 Makefile
--- etc/Makefile2 Feb 2017 21:35:05 -   1.449
+++ etc/Makefile10 Feb 2017 08:59:27 -
@@ -110,6 +110,8 @@
${DESTDIR}/root/.Xdefaults; \
${INSTALL} -c -o root -g wheel -m 644 dot.cvsrc \
${DESTDIR}/root/.cvsrc; \
+   ${INSTALL} -c -o root -g wheel -m 600 /dev/null \
+   ${DESTDIR}/root/.ssh/authorized_keys; \
rm -f ${DESTDIR}/.cshrc ${DESTDIR}/.profile; \
${INSTALL} -c -o root -g wheel -m 644 dot.cshrc \
${DESTDIR}/.cshrc; \
Index: etc/mtree/4.4BSD.dist
===
RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
retrieving revision 1.293
diff -u -r1.293 4.4BSD.dist
--- etc/mtree/4.4BSD.dist   27 Dec 2016 09:17:52 -  1.293
+++ etc/mtree/4.4BSD.dist   10 Feb 2017 08:59:27 -
@@ -118,6 +118,8 @@
 mnt
 ..
 root   mode=0700
+.ssh   uname=root mode=0700
+..
 ..
 sbin
 ..
Index: etc/mtree/special
===
RCS file: /cvs/src/etc/mtree/special,v
retrieving revision 1.122
diff -u -r1.122 special
--- etc/mtree/special   27 Dec 2016 09:17:52 -  1.122
+++ etc/mtree/special   10 Feb 2017 08:59:27 -
@@ -121,6 +121,9 @@
 .login type=file mode=0644 uname=root gname=wheel
 .profile   type=file mode=0644 uname=root gname=wheel
 .rhoststype=file mode=0600 uname=root gname=wheel optional
+.ssh   type=dir mode=0700 uname=root gname=wheel
+.. #.ssh
+authorized_keystype=file mode=0600 uname=root gname=wheel
 .. #root
 
 sbin   type=dir mode=0755 uname=root gname=wheel ignore
Index: distrib/miniroot/install.sub
=

Re: add empty /root/.ssh/authorized_keys to mtree/sets ?

2017-02-09 Thread Landry Breuil
On Thu, Feb 09, 2017 at 06:19:54PM +0100, Landry Breuil wrote:
> On Sun, Feb 05, 2017 at 08:37:31PM +, Stuart Henderson wrote:
> > On 2017/02/05 09:53, Robert Peichaer wrote:
> > > On Sun, Feb 05, 2017 at 10:46:41AM +0100, Landry Breuil wrote:
> > > > Hi,
> > > > 
> > > > when installing 'throwaway' VMs (manually, not always using autoinstall 
> > > > for
> > > > $REASONS) i've often found myself having to do right after the install:
> > > > install -d -m 700 /root/.ssh
> > > > install -m 600 /dev/null /root/.ssh/authorized_keys
> > > > (or touch /root/.ssh/authorized_keys && chmod 600
> > > > /root/.ssh/authorized_keys, ymmv)
> > > > 
> > > > those are present in /etc/skel for "real" users, so why not creating
> > > > them for the root account ? install.sub also creates /mnt/root/.ssh when
> > > > using autoinstall and giving an ssh pubkey, so that'll be one less step
> > > > to do there.
> > > > 
> > > > We advise ppl to set prohibit-password for PermitRootLogin, so why not 
> > > > make it
> > > > easier to use it ? This ways, the correct modes are set.. i often 
> > > > fat-fingered
> > > > this, to see sshd complaining (rightly!) about bad modes on 
> > > > .ssh/authorized_keys.
> > > 
> > > Conceptually I'd like this going in.
> > 
> > +1. (On "managed" systems I use root-owned authorized_keys in a system 
> > directory,
> > but this doesn't get in the way, and it makes things easier on ad-hoc 
> > installed
> > systems).
> 
> Finally built a release with this, the empty file is created in
> /var/sysmerge/etc.tgz, and sysmerge didnt overwrite my own
> /root/.ssh/authorized_keys - so i think i can now explicitely ask for okays.
> dtucker@ mentioned that in ${INSTALL} -c idiom the -c was a noop, but i kept 
> it
> for consistency.
> Hopefully more ppl can chime in and think of potential drawbacks this
> diff exposes...

One of the drawbacks i see is that ppl *might* get a security alert from
changelist if the (empty) file suddenly appears after an upgrade... but
i think we can/should live with that ?

Landry



Re: add empty /root/.ssh/authorized_keys to mtree/sets ?

2017-02-09 Thread Landry Breuil
On Sun, Feb 05, 2017 at 08:37:31PM +, Stuart Henderson wrote:
> On 2017/02/05 09:53, Robert Peichaer wrote:
> > On Sun, Feb 05, 2017 at 10:46:41AM +0100, Landry Breuil wrote:
> > > Hi,
> > > 
> > > when installing 'throwaway' VMs (manually, not always using autoinstall 
> > > for
> > > $REASONS) i've often found myself having to do right after the install:
> > > install -d -m 700 /root/.ssh
> > > install -m 600 /dev/null /root/.ssh/authorized_keys
> > > (or touch /root/.ssh/authorized_keys && chmod 600
> > > /root/.ssh/authorized_keys, ymmv)
> > > 
> > > those are present in /etc/skel for "real" users, so why not creating
> > > them for the root account ? install.sub also creates /mnt/root/.ssh when
> > > using autoinstall and giving an ssh pubkey, so that'll be one less step
> > > to do there.
> > > 
> > > We advise ppl to set prohibit-password for PermitRootLogin, so why not 
> > > make it
> > > easier to use it ? This ways, the correct modes are set.. i often 
> > > fat-fingered
> > > this, to see sshd complaining (rightly!) about bad modes on 
> > > .ssh/authorized_keys.
> > 
> > Conceptually I'd like this going in.
> 
> +1. (On "managed" systems I use root-owned authorized_keys in a system 
> directory,
> but this doesn't get in the way, and it makes things easier on ad-hoc 
> installed
> systems).

Finally built a release with this, the empty file is created in
/var/sysmerge/etc.tgz, and sysmerge didnt overwrite my own
/root/.ssh/authorized_keys - so i think i can now explicitely ask for okays.
dtucker@ mentioned that in ${INSTALL} -c idiom the -c was a noop, but i kept it
for consistency.
Hopefully more ppl can chime in and think of potential drawbacks this
diff exposes...

Sets diff added too, modeled after what's done for
/etc/skel/.ssh/authorized_keys - dunno if it should be commited along the etc/
change.

Landry

Index: etc/Makefile
===
RCS file: /cvs/src/etc/Makefile,v
retrieving revision 1.449
diff -u -r1.449 Makefile
--- etc/Makefile2 Feb 2017 21:35:05 -   1.449
+++ etc/Makefile9 Feb 2017 17:13:00 -
@@ -110,6 +110,8 @@
${DESTDIR}/root/.Xdefaults; \
${INSTALL} -c -o root -g wheel -m 644 dot.cvsrc \
${DESTDIR}/root/.cvsrc; \
+   ${INSTALL} -c -o root -g wheel -m 600 /dev/null \
+   ${DESTDIR}/root/.ssh/authorized_keys; \
rm -f ${DESTDIR}/.cshrc ${DESTDIR}/.profile; \
${INSTALL} -c -o root -g wheel -m 644 dot.cshrc \
${DESTDIR}/.cshrc; \
Index: etc/mtree/4.4BSD.dist
===
RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
retrieving revision 1.293
diff -u -r1.293 4.4BSD.dist
--- etc/mtree/4.4BSD.dist   27 Dec 2016 09:17:52 -  1.293
+++ etc/mtree/4.4BSD.dist   9 Feb 2017 17:13:00 -
@@ -118,6 +118,8 @@
 mnt
 ..
 root   mode=0700
+.ssh   uname=root mode=0700
+..
 ..
 sbin
 ..


Index: distrib/sets/lists/base/mi
===
RCS file: /cvs/src/distrib/sets/lists/base/mi,v
retrieving revision 1.820
diff -u -r1.820 mi
--- distrib/sets/lists/base/mi  7 Feb 2017 21:32:48 -   1.820
+++ distrib/sets/lists/base/mi  9 Feb 2017 17:12:42 -
@@ -232,6 +232,7 @@
 ./home
 ./mnt
 ./root
+./root/.ssh
 ./sbin
 ./sbin/atactl
 ./sbin/badsect
Index: distrib/sets/lists/etc/mi
===
RCS file: /cvs/src/distrib/sets/lists/etc/mi,v
retrieving revision 1.211
diff -u -r1.211 mi
--- distrib/sets/lists/etc/mi   1 Oct 2016 16:58:29 -   1.211
+++ distrib/sets/lists/etc/mi   9 Feb 2017 17:12:42 -
@@ -50,6 +50,7 @@
 ./root/.cvsrc
 ./root/.login
 ./root/.profile
+./root/.ssh/authorized_keys
 ./var/crash/minfree
 ./var/cron/at.deny
 ./var/cron/cron.deny



add empty /root/.ssh/authorized_keys to mtree/sets ?

2017-02-05 Thread Landry Breuil
Hi,

when installing 'throwaway' VMs (manually, not always using autoinstall for
$REASONS) i've often found myself having to do right after the install:
install -d -m 700 /root/.ssh
install -m 600 /dev/null /root/.ssh/authorized_keys
(or touch /root/.ssh/authorized_keys && chmod 600
/root/.ssh/authorized_keys, ymmv)

those are present in /etc/skel for "real" users, so why not creating
them for the root account ? install.sub also creates /mnt/root/.ssh when
using autoinstall and giving an ssh pubkey, so that'll be one less step
to do there.

We advise ppl to set prohibit-password for PermitRootLogin, so why not make it
easier to use it ? This ways, the correct modes are set.. i often fat-fingered
this, to see sshd complaining (rightly!) about bad modes on 
.ssh/authorized_keys.

Conceptual (untested) diff below for discussion, i'll build a release with it
depending on the feedback/opinions..

Landry

Index: Makefile
===
RCS file: /cvs/src/etc/Makefile,v
retrieving revision 1.449
diff -u -r1.449 Makefile
--- Makefile2 Feb 2017 21:35:05 -   1.449
+++ Makefile5 Feb 2017 09:34:58 -
@@ -110,6 +110,8 @@
${DESTDIR}/root/.Xdefaults; \
${INSTALL} -c -o root -g wheel -m 644 dot.cvsrc \
${DESTDIR}/root/.cvsrc; \
+   ${INSTALL} -c -o root -g wheel -m 600 /dev/null \
+   ${DESTDIR}/root/.ssh/authorized_keys
rm -f ${DESTDIR}/.cshrc ${DESTDIR}/.profile; \
${INSTALL} -c -o root -g wheel -m 644 dot.cshrc \
${DESTDIR}/.cshrc; \
Index: mtree/4.4BSD.dist
===
RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
retrieving revision 1.293
diff -u -r1.293 4.4BSD.dist
--- mtree/4.4BSD.dist   27 Dec 2016 09:17:52 -  1.293
+++ mtree/4.4BSD.dist   5 Feb 2017 09:34:58 -
@@ -118,6 +118,8 @@
 mnt
 ..
 root   mode=0700
+.ssh   uname=root mode=0700
+..
 ..
 sbin
 ..



Re: Allow install from https server w/ self signed cert

2017-01-06 Thread Landry Breuil
On Fri, Jan 06, 2017 at 11:28:34AM +, Stuart Henderson wrote:
> Related to this (and particularly thinking about autoinstalls),
> would it make sense to allow explicit protocols in the hostname?
> 
> some.host -> https with http fallback
> http://some.host/ -> http only
> https://some.host/ -> https only, no fallback

Definitely in favor of less implicit/magic behaviour..



Re: add commented out spamd config to default syslog.conf

2016-12-08 Thread Landry Breuil
On Thu, Dec 08, 2016 at 01:16:43PM +, Stuart Henderson wrote:
> On 2016/12/08 09:42, Landry Breuil wrote:
> > Hi,
> > 
> > i know this is in the examples section of syslog.conf(5), but i got
> > bitten by this again, so what do ppl thing about adding those 3 lines to
> > the default syslog.conf ?
> > 
> > wont push for this, was just taunted by someone "i would OK a diff to
> > add an example to the default syslog.conf" :)
> > 
> > Landry
> > 
> > Index: etc/syslog.conf
> > ===
> > RCS file: /cvs/src/etc/syslog.conf,v
> > retrieving revision 1.19
> > diff -u -r1.19 syslog.conf
> > --- etc/syslog.conf 26 Nov 2015 15:25:14 -  1.19
> > +++ etc/syslog.conf 8 Dec 2016 08:39:21 -
> > @@ -1,6 +1,13 @@
> >  #  $OpenBSD: syslog.conf,v 1.19 2015/11/26 15:25:14 deraadt Exp $
> >  #
> >  
> > +# Uncomment these three lines to log messages from spamd(8) to its own
> > +# logfile, without being also logged in /var/log/daemon.
> > +# Note that the logfile has to be created beforehands.
> > +#!!spamd
> > +#daemon.info   /var/log/spamd
> > +#!*
> > +
> >  *.notice;auth,authpriv,cron,ftp,kern,lpr,mail,user.none
> > /var/log/messages
> >  kern.debug;syslog,user.info
> > /var/log/messages
> >  auth.info  /var/log/authlog
> > 
> 
> I'm not sure if we need to tell people to create the file; at least
> we don't tell them to create files for other commented-out examples
> (uucp, doas).
> 
> Slight tweaks to the english, plus I tweaked the doas example to
> try and make the difference clear between it and the first example
> ("instead of" vs "as well as").

Much betterer than my initial brokenglish version :)



add commented out spamd config to default syslog.conf

2016-12-08 Thread Landry Breuil
Hi,

i know this is in the examples section of syslog.conf(5), but i got
bitten by this again, so what do ppl thing about adding those 3 lines to
the default syslog.conf ?

wont push for this, was just taunted by someone "i would OK a diff to
add an example to the default syslog.conf" :)

Landry

Index: etc/syslog.conf
===
RCS file: /cvs/src/etc/syslog.conf,v
retrieving revision 1.19
diff -u -r1.19 syslog.conf
--- etc/syslog.conf 26 Nov 2015 15:25:14 -  1.19
+++ etc/syslog.conf 8 Dec 2016 08:39:21 -
@@ -1,6 +1,13 @@
 #  $OpenBSD: syslog.conf,v 1.19 2015/11/26 15:25:14 deraadt Exp $
 #
 
+# Uncomment these three lines to log messages from spamd(8) to its own
+# logfile, without being also logged in /var/log/daemon.
+# Note that the logfile has to be created beforehands.
+#!!spamd
+#daemon.info   /var/log/spamd
+#!*
+
 *.notice;auth,authpriv,cron,ftp,kern,lpr,mail,user.none
/var/log/messages
 kern.debug;syslog,user.info/var/log/messages
 auth.info  /var/log/authlog



Re: reloading pf through ansible easy hook

2016-11-22 Thread Landry Breuil
On Tue, Nov 22, 2016 at 11:15:01AM +, BARDOU Pierre wrote:
> Hello,
> 
> - name: "Loading pf.conf"
>   template: src=pf.conf dest=/etc/ validate="pfctl -f %s"

Fwiw, i find it nicer to validate with 'pfctl -nf' ..

Landry



Re: Append system start up messages to new /var/log/dmesg in /etc/rc

2016-10-19 Thread Landry Breuil
On Wed, Oct 19, 2016 at 02:19:22PM +0100, Craig Skinner wrote:
> Hi,
> 
> With dmesg's new -s flag, append each boot's full log to a new
> /var/log/dmesg semi-private log file.
> 
> Inspired by Alexander Hall's recent post:
> http://marc.info/?l=openbsd-misc=147674181621645
> 
> This works for me (on 5.9) & rotates correctly:

Note that dmesg is already saved in var/run/dmesg.boot but not dmesg
-s.

Landry



Re: missing includes in games/

2016-09-11 Thread Landry Breuil
On Sun, Sep 11, 2016 at 11:17:25AM +0200, Theo Buehler wrote:
> These all call time(3), so should include  directly.
> 
> ok?
> 
> Index: bcd/bcd.c
> ===
> RCS file: /var/cvs/src/games/bcd/bcd.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 bcd.c
> --- bcd/bcd.c 7 Mar 2016 12:07:55 -   1.25
> +++ bcd/bcd.c 11 Sep 2016 08:53:40 -
> @@ -63,6 +63,8 @@
>   * Nov 5, 1993
>   */
>  
> +#include 
> +
>  #include 
>  #include 
>  #include 

This one is unrelated ?



  1   2   >