Re: [PATCH] Reduce case duplication in kern_sysctl
On Sat, 09 Jan 2021 14:39:53 -0800, Greg Steuck wrote: > How's this instead? OK? > > Tested with the usual diff of before/after sysctl outputs and a random > sysctl -w poke. > > Subject: [PATCH] Split hierarchical calls into kern_sysctl_dirs > > Removed a rash of +/-1 and made both functions shorter and more focused. OK millert@ - todd
Re: hid_is_collection() sync with NetBSD
On Sat, 9 Jan 2021 21:51:03 +0100 (CET) Mark Kettenis wrote: > > Date: Sat, 9 Jan 2021 10:38:20 +0100 > > From: Marcus Glocker > > > > I have not much clue about HID, but when we did some testing for the > > new ujoy(4) driver it turned out that the PS4 controller doesn't get > > handled correctly by hid.c:hid_is_collection(). This made me peek > > in to the NetBSD code where I could find an update in this function. > > > > Syncing it up makes the PS4 controller attachment work correctly, > > while not breaking my other HID devices. > > > > The change was introduced in NetBSD with hid.c revision 1.25 on the > > 19.06.2006, obviously as part of a bigger change, not saying > > something about the hid_is_collection() update itself: > > > > -- > > > > Initial import of bluetooth stack on behalf of Iain Hibbert. > > (plunky@, NetBSD Foundation Membership still pending.) This stack > > was written by Iain under sponsorship from Itronix Inc. > > > > The stack includes support for rfcomm networking (networking via > > your bluetooth enabled cell phone), hid devices (keyboards/mice), > > and headsets. > > > > Drivers for both PCMCIA and USB bluetooth controllers are included. > > > > -- > > > > http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/usb/Attic/hid.c.diff?r1=1.24=1.25_with_tag=MAIN > > > > More testers? Comments? OKs? > > Probably needs some testing with touchpad and touchscreen devices. Hmm, yes. I just quickly tested this on my X1 with ums(4) touchscreen. Beside that it still works, the diff seems to fix something in the ums attach. Before the diff: uhidev0 at uhub0 port 10 configuration 1 interface 0 "Raydium Corporation Raydium Touch System" rev 2.01/1.08 addr 4 uhidev0: iclass 3/0, 68 report ids uhid0 at uhidev0 reportid 1: input=0, output=63, feature=0 uhid1 at uhidev0 reportid 2: input=63, output=0, feature=0 uhid2 at uhidev0 reportid 3: input=0, output=63, feature=0 uhid3 at uhidev0 reportid 4: input=0, output=63, feature=0 uhid4 at uhidev0 reportid 5: input=0, output=63, feature=0 uhid5 at uhidev0 reportid 6: input=63, output=0, feature=0 uhid6 at uhidev0 reportid 7: input=0, output=63, feature=0 uhid7 at uhidev0 reportid 8: input=0, output=63, feature=0 uhid8 at uhidev0 reportid 9: input=63, output=0, feature=0 ums0 at uhidev0 reportid 10: 1 button, tip wsmouse2 at ums0 mux 0 uhid9 at uhidev0 reportid 11: input=0, output=0, feature=1 uhid10 at uhidev0 reportid 12: input=0, output=0, feature=255 uhid11 at uhidev0 reportid 13: input=0, output=0, feature=255 ums1 at uhidev0 reportid 68 ums1: mouse has no X report I didn't notice before, but ums1 seems not to make sense, since there is no second ums device attached here. After the diff the reportid 68 just turns in another uhid of uhidev0 which makes more sense I guess: uhidev0 at uhub0 port 10 configuration 1 interface 0 "Raydium Corporation Raydium Touch System" rev 2.01/1.08 addr 4 uhidev0: iclass 3/0, 68 report ids uhid0 at uhidev0 reportid 1: input=0, output=63, feature=0 uhid1 at uhidev0 reportid 2: input=63, output=0, feature=0 uhid2 at uhidev0 reportid 3: input=0, output=63, feature=0 uhid3 at uhidev0 reportid 4: input=0, output=63, feature=0 uhid4 at uhidev0 reportid 5: input=0, output=63, feature=0 uhid5 at uhidev0 reportid 6: input=63, output=0, feature=0 uhid6 at uhidev0 reportid 7: input=0, output=63, feature=0 uhid7 at uhidev0 reportid 8: input=0, output=63, feature=0 uhid8 at uhidev0 reportid 9: input=63, output=0, feature=0 ums0 at uhidev0 reportid 10: 1 button, tip wsmouse2 at ums0 mux 0 uhid9 at uhidev0 reportid 11: input=0, output=0, feature=1 uhid10 at uhidev0 reportid 12: input=0, output=0, feature=255 uhid11 at uhidev0 reportid 13: input=0, output=0, feature=255 uhid12 at uhidev0 reportid 68: input=0, output=0, feature=255 > > Index: dev/hid/hid.c > > === > > RCS file: /cvs/src/sys/dev/hid/hid.c,v > > retrieving revision 1.3 > > diff -u -p -u -p -r1.3 hid.c > > --- dev/hid/hid.c 4 Jun 2020 23:03:43 - 1.3 > > +++ dev/hid/hid.c 9 Jan 2021 08:43:30 - > > @@ -630,6 +630,25 @@ hid_get_udata(const uint8_t *buf, int le > > return (hid_get_data_sub(buf, len, loc, 0)); > > } > > > > +/* > > + * hid_is_collection(desc, size, id, usage) > > + * > > + * This function is broken in the following way. > > + * > > + * It is used to discover if the given 'id' is part of 'usage' > > collection > > + * in the descriptor in order to match report id against device > > type. > > + * > > + * The semantics of hid_start_parse() means though, that only a > > single > > + * kind of report is considered. The current HID code that uses > > this for > > + * matching is actually only looking for input reports, so this > > works > > + * for now. > > + * > > + * This function could try all report kinds (input, output and > > feature) > > + * consecutively if necessary, but it may be better to integrate > > the > > + * libusbhid code which
Re: [PATCH] Reduce case duplication in kern_sysctl
Thanks for the reviews!
Marcus Glocker writes:
> On Sat, 9 Jan 2021 22:09:06 +0100
> Marcus Glocker wrote:
> If you could fix the switch() indentation in a separate commit (as you
> already mentioned in one of your previous e-mails), that would be nice.
How's this instead? OK?
Tested with the usual diff of before/after sysctl outputs and a random
sysctl -w poke.
Subject: [PATCH] Split hierarchical calls into kern_sysctl_dirs
Removed a rash of +/-1 and made both functions shorter and more focused.
---
sys/kern/kern_sysctl.c | 86 ++
1 file changed, 45 insertions(+), 41 deletions(-)
diff --git sys/kern/kern_sysctl.c sys/kern/kern_sysctl.c
index 7b164d3d32a..383b2a7c074 100644
--- sys/kern/kern_sysctl.c
+++ sys/kern/kern_sysctl.c
@@ -352,106 +352,110 @@ const struct sysctl_bounded_args kern_vars[] = {
#endif
};
-/*
- * kernel related system variables.
- */
int
-kern_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
-size_t newlen, struct proc *p)
+kern_sysctl_dirs(int top_name, int *name, u_int namelen,
+void *oldp, size_t *oldlenp, void *newp, size_t newlen, struct proc *p)
{
- int error, level, inthostid, stackgap;
- dev_t dev;
- extern int pool_debug;
-
- /* dispatch the non-terminal nodes first */
- if (namelen != 1) {
- switch (name[0]) {
+ switch (top_name) {
#ifndef SMALL_KERNEL
case KERN_PROC:
- return (sysctl_doproc(name + 1, namelen - 1, oldp, oldlenp));
+ return (sysctl_doproc(name, namelen, oldp, oldlenp));
case KERN_PROC_ARGS:
- return (sysctl_proc_args(name + 1, namelen - 1, oldp, oldlenp,
-p));
+ return (sysctl_proc_args(name, namelen, oldp, oldlenp, p));
case KERN_PROC_CWD:
- return (sysctl_proc_cwd(name + 1, namelen - 1, oldp, oldlenp,
-p));
+ return (sysctl_proc_cwd(name, namelen, oldp, oldlenp, p));
case KERN_PROC_NOBROADCASTKILL:
- return (sysctl_proc_nobroadcastkill(name + 1, namelen - 1,
+ return (sysctl_proc_nobroadcastkill(name, namelen,
newp, newlen, oldp, oldlenp, p));
case KERN_PROC_VMMAP:
- return (sysctl_proc_vmmap(name + 1, namelen - 1, oldp, oldlenp,
-p));
+ return (sysctl_proc_vmmap(name, namelen, oldp, oldlenp, p));
case KERN_FILE:
- return (sysctl_file(name + 1, namelen - 1, oldp, oldlenp, p));
+ return (sysctl_file(name, namelen, oldp, oldlenp, p));
#endif
#if defined(GPROF) || defined(DDBPROF)
case KERN_PROF:
- return (sysctl_doprof(name + 1, namelen - 1, oldp, oldlenp,
+ return (sysctl_doprof(name, namelen, oldp, oldlenp,
newp, newlen));
#endif
case KERN_MALLOCSTATS:
- return (sysctl_malloc(name + 1, namelen - 1, oldp, oldlenp,
+ return (sysctl_malloc(name, namelen, oldp, oldlenp,
newp, newlen, p));
case KERN_TTY:
- return (sysctl_tty(name + 1, namelen - 1, oldp, oldlenp,
+ return (sysctl_tty(name, namelen, oldp, oldlenp,
newp, newlen));
case KERN_POOL:
- return (sysctl_dopool(name + 1, namelen - 1, oldp, oldlenp));
+ return (sysctl_dopool(name, namelen, oldp, oldlenp));
#if defined(SYSVMSG) || defined(SYSVSEM) || defined(SYSVSHM)
case KERN_SYSVIPC_INFO:
- return (sysctl_sysvipc(name + 1, namelen - 1, oldp, oldlenp));
+ return (sysctl_sysvipc(name, namelen, oldp, oldlenp));
#endif
#ifdef SYSVSEM
case KERN_SEMINFO:
- return (sysctl_sysvsem(name + 1, namelen - 1, oldp, oldlenp,
+ return (sysctl_sysvsem(name, namelen, oldp, oldlenp,
newp, newlen));
#endif
#ifdef SYSVSHM
case KERN_SHMINFO:
- return (sysctl_sysvshm(name + 1, namelen - 1, oldp, oldlenp,
+ return (sysctl_sysvshm(name, namelen, oldp, oldlenp,
newp, newlen));
#endif
#ifndef SMALL_KERNEL
case KERN_INTRCNT:
- return (sysctl_intrcnt(name + 1, namelen - 1, oldp, oldlenp));
+ return (sysctl_intrcnt(name, namelen, oldp, oldlenp));
case KERN_WATCHDOG:
- return (sysctl_wdog(name + 1, namelen - 1, oldp, oldlenp,
+ return (sysctl_wdog(name, namelen, oldp, oldlenp,
newp, newlen));
#endif
#ifndef SMALL_KERNEL
case KERN_EVCOUNT:
- return (evcount_sysctl(name + 1, namelen - 1, oldp, oldlenp,
+ return (evcount_sysctl(name, namelen, oldp, oldlenp,
newp, newlen));
#endif
case KERN_TIMECOUNTER:
- return (sysctl_tc(name + 1, namelen - 1, oldp, oldlenp,
- newp,
Re: all platforms: isolate hardclock(9) from statclock()
> Date: Fri, 8 Jan 2021 10:18:27 -0600
> From: Scott Cheloha
>
> On Thu, Jan 07, 2021 at 08:12:10PM -0600, Scott Cheloha wrote:
> > On Thu, Jan 07, 2021 at 09:37:58PM +0100, Mark Kettenis wrote:
> > > > Date: Thu, 7 Jan 2021 11:15:41 -0600
> > > > From: Scott Cheloha
> > > >
> > > > Hi,
> > > >
> > > > I want to isolate statclock() from hardclock(9). This will simplify
> > > > the logic in my WIP dynamic clock interrupt framework.
> > > >
> > > > Currently, if stathz is zero, we call statclock() from within
> > > > hardclock(9). It looks like this (see sys/kern/kern_clock.c):
> > > >
> > > > void
> > > > hardclock(struct clockframe *frame)
> > > > {
> > > > /* [...] */
> > > >
> > > > if (stathz == 0)
> > > > statclock(frame);
> > > >
> > > > /* [...] */
> > > >
> > > > This is the case on alpha, amd64 (w/ lapic), hppa, i386 (w/ lapic),
> > > > loongson, luna88k, mips64, and sh.
> > > >
> > > > (We seem to do it on sgi, too. I was under the impression that sgi
> > > > *was* a mips64 platform, yet sgi seems to it have its own clock
> > > > interrupt code distinct from mips64's general clock interrupt code
> > > > which is used by e.g. octeon).
> > > >
> > > > However, if stathz is not zero we call statclock() separately. This
> > > > is the case on armv7, arm, arm64, macppc, powerpc64, and sparc64.
> > > >
> > > > (The situation for the general powerpc code and socppc in particular
> > > > is a mystery to me.)
> > > >
> > > > If we could remove this MD distinction it would make my MI framework
> > > > simpler. Instead of checking stathz and conditionally starting a
> > > > statclock event I will be able to unconditionally start a statclock
> > > > event on all platforms on every CPU.
> > > >
> > > > In general I don't think the "is stathz zero?" variance between
> > > > platforms is useful:
> > > >
> > > > - The difference is invisible to userspace, as we hide the fact that
> > > > stathz is zero from e.g. the kern.clockrate sysctl.
> > > >
> > > > - We run statclock() at some point, regardless of whether stathz is
> > > > zero. If statclock() is run from hardclock(9) then isn't stathz
> > > > effectively equal to hz?
> > > >
> > > > - Because stathz might be zero we need to add a bunch of safety checks
> > > > to our MI code to ensure we don't accidentally divide by zero.
> > > >
> > > > Maybe we can ensure stathz is non-zero in a later diff...
> > > >
> > > > --
> > > >
> > > > Anyway, I don't think I have missed any platforms. However, if
> > > > platform experts could weigh in here to verify my changes (and test
> > > > them!) I'd really appreciate it.
> > > >
> > > > In particular, I'm confused about how clock interrupts work on
> > > > powerpc, socppc, and sgi.
> > > >
> > > > --
> > > >
> > > > Thoughts? Platform-specific OKs?
> > >
> > > I wouldn't be opposed to doing this. It is less magic!
> > >
> > > But yes, this needs to be tested on the platforms that you change.
> >
> > I guess I'll CC all the platform-specific people I'm aware of.
> >
> > > Note that many platforms don't have have separate schedclock and
> > > statclock. But on many platforms where we use a one-shot timer as the
> > > clock we have a randomized statclock. I'm sure Theo would love to
> > > tell you about the cpuhog story...
> >
> > I am familiar with cpuhog. It's the one thing everybody mentions when
> > I talk about clock interrupts and/or statclock().
> >
> > Related:
> >
> > I wonder if we could avoid the cpuhog problem entirely by implementing
> > some kind of MI cycle counting clock API that we use to timestamp
> > whenever we cross the syscall boundary, or enter an interrupt, etc.,
> > to determine the time a thread spends using the CPU without any
> > sampling error.
> >
> > Instead of a process accumulating ticks from a sampling clock
> > interrupt you would accumulate, say, a 64-bit count of cycles, or
> > something like that.
> >
> > Sampling with a regular clock interrupt is prone to error and trickery
> > like cpuhog. The classic BSD solution to the cpuhog exploit was to
> > randomize the statclock/schedclock to make it harder to fool the
> > sampler. But if we used cycle counts or instruction counts at each
> > state transition it would be impossible to fool because we wouldn't be
> > sampling at all.
> >
> > Unsure what the performance implications would be, but in general I
> > would guess that most platforms have a way to count instructions or
> > cycles and that reading this data is fast enough for us to use it in
> > e.g. syscall() or the interrupt handler without a huge performance
> > hit.
> >
> > > Anyway, we probably want that on amd64 as well.
> >
> > My WIP dynamic clock interrupt system can run a randomized statclock()
> > on amd64 boxes with a lapic. I imagine we will be able to do the same
> > on i386 systems that have a lapic, too, though it will be slower
> > because all the i386 timecounters are
Re: [PATCH] Reduce case duplication in kern_sysctl
On Sat, 9 Jan 2021 22:09:06 +0100 Marcus Glocker wrote: > On Sat, 09 Jan 2021 13:06:36 -0800 > Greg Steuck wrote: > > > Thanks Todd for reviewing these boring patches! > > > > Todd C. Miller writes: > > > > > Updated diff looks good to me. OK millert@ but it would be good > > > to get feedback from Marcus too. > > > > Sure thing, I'll wait till tomorrow then? > > > > Thanks > > Greg > > Sorry, I totally missed that. > I'm just checking it now, give me a bit. If you could fix the switch() indentation in a separate commit (as you already mentioned in one of your previous e-mails), that would be nice. Otherwise the diff is fine for me as well. OK mglocker@
Re: [PATCH] Reduce case duplication in kern_sysctl
On Sat, 09 Jan 2021 13:06:36 -0800 Greg Steuck wrote: > Thanks Todd for reviewing these boring patches! > > Todd C. Miller writes: > > > Updated diff looks good to me. OK millert@ but it would be good > > to get feedback from Marcus too. > > Sure thing, I'll wait till tomorrow then? > > Thanks > Greg Sorry, I totally missed that. I'm just checking it now, give me a bit.
Re: [PATCH] Reduce case duplication in kern_sysctl
Thanks Todd for reviewing these boring patches! Todd C. Miller writes: > Updated diff looks good to me. OK millert@ but it would be good > to get feedback from Marcus too. Sure thing, I'll wait till tomorrow then? Thanks Greg
Re: hid_is_collection() sync with NetBSD
> Date: Sat, 9 Jan 2021 10:38:20 +0100
> From: Marcus Glocker
>
> I have not much clue about HID, but when we did some testing for the
> new ujoy(4) driver it turned out that the PS4 controller doesn't get
> handled correctly by hid.c:hid_is_collection(). This made me peek in
> to the NetBSD code where I could find an update in this function.
>
> Syncing it up makes the PS4 controller attachment work correctly,
> while not breaking my other HID devices.
>
> The change was introduced in NetBSD with hid.c revision 1.25 on the
> 19.06.2006, obviously as part of a bigger change, not saying something
> about the hid_is_collection() update itself:
>
> --
>
> Initial import of bluetooth stack on behalf of Iain Hibbert. (plunky@,
> NetBSD Foundation Membership still pending.) This stack was written by
> Iain under sponsorship from Itronix Inc.
>
> The stack includes support for rfcomm networking (networking via your
> bluetooth enabled cell phone), hid devices (keyboards/mice), and
> headsets.
>
> Drivers for both PCMCIA and USB bluetooth controllers are included.
>
> --
>
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/usb/Attic/hid.c.diff?r1=1.24=1.25_with_tag=MAIN
>
> More testers? Comments? OKs?
Probably needs some testing with touchpad and touchscreen devices.
> Index: dev/hid/hid.c
> ===
> RCS file: /cvs/src/sys/dev/hid/hid.c,v
> retrieving revision 1.3
> diff -u -p -u -p -r1.3 hid.c
> --- dev/hid/hid.c 4 Jun 2020 23:03:43 - 1.3
> +++ dev/hid/hid.c 9 Jan 2021 08:43:30 -
> @@ -630,6 +630,25 @@ hid_get_udata(const uint8_t *buf, int le
> return (hid_get_data_sub(buf, len, loc, 0));
> }
>
> +/*
> + * hid_is_collection(desc, size, id, usage)
> + *
> + * This function is broken in the following way.
> + *
> + * It is used to discover if the given 'id' is part of 'usage' collection
> + * in the descriptor in order to match report id against device type.
> + *
> + * The semantics of hid_start_parse() means though, that only a single
> + * kind of report is considered. The current HID code that uses this for
> + * matching is actually only looking for input reports, so this works
> + * for now.
> + *
> + * This function could try all report kinds (input, output and feature)
> + * consecutively if necessary, but it may be better to integrate the
> + * libusbhid code which can consider multiple report kinds simultaneously
> + *
> + * Needs some thought.
> + */
> int
> hid_is_collection(const void *desc, int size, uint8_t id, int32_t usage)
> {
> @@ -637,17 +656,25 @@ hid_is_collection(const void *desc, int
> struct hid_item hi;
> uint32_t coll_usage = ~0;
>
> - hd = hid_start_parse(desc, size, hid_none);
> + hd = hid_start_parse(desc, size, hid_input);
> + if (hd == NULL)
> + return (0);
>
> DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage);
> while (hid_get_item(hd, )) {
> DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__,
> hi.kind, hi.report_ID, hi.usage, coll_usage);
> +
> if (hi.kind == hid_collection &&
> hi.collection == HCOLL_APPLICATION)
> coll_usage = hi.usage;
> - if (hi.kind == hid_endcollection &&
> - coll_usage == usage && hi.report_ID == id) {
> +
> + if (hi.kind == hid_endcollection)
> + coll_usage = ~0;
> +
> + if (hi.kind == hid_input &&
> + coll_usage == usage &&
> + hi.report_ID == id) {
> DPRINTF("%s: found\n", __func__);
> hid_end_parse(hd);
> return (1);
>
>
Re: [PATCH] Reduce case duplication in kern_sysctl
On Mon, 28 Dec 2020 22:00:55 -0800, Greg Steuck wrote: > Here's an updated version to account from an intervening change since I > posted this first. Updated diff looks good to me. OK millert@ but it would be good to get feedback from Marcus too. - todd
Re: [PATCH] Reduce case duplication in kern_sysctl
On Sun, 20 Dec 2020 15:52:07 -0800, Greg Steuck wrote: > I tested this by diff'ing sysctl output before/after on amd64. Since > there's a bunch of ifdef'ness I verified RAMDISK still builds. > > I deliberately didn't fix the indentation to keep this diff a pure line > motion (would run over 80 chars otherwise). I can either fix that it in > a separate commit or in this one before submitting. Can you re-send a version of this that applies on top of -current? - todd
Re: [PATCH]es sysctl_int_bounded goodness
On Fri, 18 Dec 2020 12:31:54 -0800, Greg Steuck wrote: > I'm scraping the bottom of the barrel with these, so dumped them all > together for ease of review. Will submit piecemeal as reviews happen. > > All verified manually with sysctl -w. Even bothered to find an i386 > machine with watchdog and build a WITNESS kernel to run all code paths. OK millert@ - todd
Re: all platforms: isolate hardclock(9) from statclock()
The 'magic' here was that MD code could choose to implement statclock (and
set stathz appropriately), or MD code could not care about the multiple
statclock/hardclock interfaces into the scheduler. Also some clock drivers
on a platform could enable split hardclock/statclock where others did not.
Back near the beginning of OpenBSD platforms existed that had no higher
precision clock than that timer interrupt, and nothing existed to even
approximate higher precision (eg cycle counters or instruction counters).
Some clock drivers have a separate timer/interrupt or separate 'event'
tracked to schedule the stat() event. These platforms may also
(dynamically) change the stathz clock when profiling is enabled/disabled.
This is implemented in arm64 in agtimer_setstatclockrate()
Any clock driver that directly calls statclock() should make certain to
stathz (and profhz) appropriately, as no assumptions to it's rate/frequency
should be assumed.
This isn't to say that removing the stathz== 0 magic should not be done,
but if done, make certain that stathz and profhz are properly
updated/configured.
Dale
On Sat, Jan 9, 2021 at 5:57 AM Visa Hankala wrote:
> On Fri, Jan 08, 2021 at 10:18:27AM -0600, Scott Cheloha wrote:
> > On Thu, Jan 07, 2021 at 08:12:10PM -0600, Scott Cheloha wrote:
> > > On Thu, Jan 07, 2021 at 09:37:58PM +0100, Mark Kettenis wrote:
> > > > > Date: Thu, 7 Jan 2021 11:15:41 -0600
> > > > > From: Scott Cheloha
> > > > >
> > > > > Hi,
> > > > >
> > > > > I want to isolate statclock() from hardclock(9). This will
> simplify
> > > > > the logic in my WIP dynamic clock interrupt framework.
> > > > >
> > > > > Currently, if stathz is zero, we call statclock() from within
> > > > > hardclock(9). It looks like this (see sys/kern/kern_clock.c):
> > > > >
> > > > > void
> > > > > hardclock(struct clockframe *frame)
> > > > > {
> > > > > /* [...] */
> > > > >
> > > > > if (stathz == 0)
> > > > > statclock(frame);
> > > > >
> > > > > /* [...] */
> > > > >
> > > > > This is the case on alpha, amd64 (w/ lapic), hppa, i386 (w/ lapic),
> > > > > loongson, luna88k, mips64, and sh.
> > > > >
> > > > > (We seem to do it on sgi, too. I was under the impression that sgi
> > > > > *was* a mips64 platform, yet sgi seems to it have its own clock
> > > > > interrupt code distinct from mips64's general clock interrupt code
> > > > > which is used by e.g. octeon).
> > > > >
> > > > > However, if stathz is not zero we call statclock() separately.
> This
> > > > > is the case on armv7, arm, arm64, macppc, powerpc64, and sparc64.
> > > > >
> > > > > (The situation for the general powerpc code and socppc in
> particular
> > > > > is a mystery to me.)
> > > > >
> > > > > If we could remove this MD distinction it would make my MI
> framework
> > > > > simpler. Instead of checking stathz and conditionally starting a
> > > > > statclock event I will be able to unconditionally start a statclock
> > > > > event on all platforms on every CPU.
> > > > >
> > > > > In general I don't think the "is stathz zero?" variance between
> > > > > platforms is useful:
> > > > >
> > > > > - The difference is invisible to userspace, as we hide the fact
> that
> > > > > stathz is zero from e.g. the kern.clockrate sysctl.
> > > > >
> > > > > - We run statclock() at some point, regardless of whether stathz is
> > > > > zero. If statclock() is run from hardclock(9) then isn't stathz
> > > > > effectively equal to hz?
> > > > >
> > > > > - Because stathz might be zero we need to add a bunch of safety
> checks
> > > > > to our MI code to ensure we don't accidentally divide by zero.
> > > > >
> > > > > Maybe we can ensure stathz is non-zero in a later diff...
> > > > >
> > > > > --
> > > > >
> > > > > Anyway, I don't think I have missed any platforms. However, if
> > > > > platform experts could weigh in here to verify my changes (and test
> > > > > them!) I'd really appreciate it.
> > > > >
> > > > > In particular, I'm confused about how clock interrupts work on
> > > > > powerpc, socppc, and sgi.
> > > > >
> > > > > --
> > > > >
> > > > > Thoughts? Platform-specific OKs?
> > > >
> > > > I wouldn't be opposed to doing this. It is less magic!
> > > >
> > > > But yes, this needs to be tested on the platforms that you change.
> > >
> > > I guess I'll CC all the platform-specific people I'm aware of.
> > >
> > > > Note that many platforms don't have have separate schedclock and
> > > > statclock. But on many platforms where we use a one-shot timer as
> the
> > > > clock we have a randomized statclock. I'm sure Theo would love to
> > > > tell you about the cpuhog story...
> > >
> > > I am familiar with cpuhog. It's the one thing everybody mentions when
> > > I talk about clock interrupts and/or statclock().
> > >
> > > Related:
> > >
> > > I wonder if we could avoid the cpuhog problem entirely by implementing
> > > some kind of MI cycle counting clock API that we use to timestamp
> > > whenever we
Re: New ujoy(4) device for USB gamecontrollers
On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: [...] > So the problem doesn't seem to be in your new ujoy(4) code, but how the > dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4 > controller. > > I'm not much in to HID, but when I sync up the hid_is_collection() > function with NetBSD, the PS4 controller attaches to ujoy(4) as > expected, and also can be accessed by usbhidctl(1), while my other > HID devices continue to work as before: Nice find. [...] > Once this has been fixed I would continue to do some more ujoy(4) > testing, and check about a possible import. We also should agree on > who/how to fix the ports part. While I can't speak for all 11,000 packages, I am yet to encounter a port that uses gamecontrollers/joysticks without going through sdl or sdl2. And for those 2 ports it's literally a 1-line diff each. I'm not sure if looking at all the ports with 'usbhid' in WANTLIB would be helpful. I have a list of 55 of those from sqlports, excluding anything with 'sdl' in its name.
route sourceaddr: simplify code & get out of ART
This diff place the user-set source address outside of struct art_root and make
the code more readable (to me).
Based on a concept by mpi@
Index: net/art.h
===
RCS file: /cvs/src/sys/net/art.h,v
retrieving revision 1.20
diff -u -p -r1.20 art.h
--- net/art.h 12 Nov 2020 15:25:28 - 1.20
+++ net/art.h 9 Jan 2021 16:04:02 -
@@ -42,7 +42,6 @@ struct art_root {
uint8_t ar_nlvl; /* [I] Number of levels */
uint8_t ar_alen; /* [I] Address length in bits */
uint8_t ar_off;/* [I] Offset of key in bytes */
- struct sockaddr *source;/* [K] optional src addr to use
*/
};
#define ISLEAF(e) (((unsigned long)(e) & 1) == 0)
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.397
diff -u -p -r1.397 route.c
--- net/route.c 29 Oct 2020 21:15:27 - 1.397
+++ net/route.c 9 Jan 2021 16:04:02 -
@@ -1192,9 +1192,9 @@ rt_ifa_del(struct ifaddr *ifa, int flags
if (flags & RTF_CONNECTED)
prio = ifp->if_priority + RTP_CONNECTED;
- rtable_clearsource(rdomain, ifa->ifa_addr);
error = rtrequest_delete(, prio, ifp, , rdomain);
if (error == 0) {
+ rt_sourceclear(rt, rdomain);
rtm_send(rt, RTM_DELETE, 0, rdomain);
if (flags & RTF_LOCAL)
rtm_addr(RTM_DELADDR, ifa);
Index: net/route.h
===
RCS file: /cvs/src/sys/net/route.h,v
retrieving revision 1.183
diff -u -p -r1.183 route.h
--- net/route.h 29 Oct 2020 21:15:27 - 1.183
+++ net/route.h 9 Jan 2021 16:04:02 -
@@ -478,6 +478,9 @@ int rtrequest_delete(struct rt_addrinfo
int rt_if_track(struct ifnet *);
int rt_if_linkstate_change(struct rtentry *, void *, u_int);
int rtdeletemsg(struct rtentry *, struct ifnet *, u_int);
+
+struct ifaddr *rt_get_ifa(struct rtentry *, unsigned int);
+voidrt_sourceclear(struct rtentry *, unsigned int);
#endif /* _KERNEL */
#endif /* _NET_ROUTE_H_ */
Index: net/rtable.c
===
RCS file: /cvs/src/sys/net/rtable.c,v
retrieving revision 1.72
diff -u -p -r1.72 rtable.c
--- net/rtable.c7 Nov 2020 09:51:40 - 1.72
+++ net/rtable.c9 Jan 2021 16:04:02 -
@@ -365,44 +365,6 @@ rtable_alloc(unsigned int rtableid, unsi
return (art_alloc(rtableid, alen, off));
}
-int
-rtable_setsource(unsigned int rtableid, int af, struct sockaddr *src)
-{
- struct art_root *ar;
-
- if ((ar = rtable_get(rtableid, af)) == NULL)
- return (EAFNOSUPPORT);
-
- ar->source = src;
-
- return (0);
-}
-
-struct sockaddr *
-rtable_getsource(unsigned int rtableid, int af)
-{
- struct art_root *ar;
-
- ar = rtable_get(rtableid, af);
- if (ar == NULL)
- return (NULL);
-
- return (ar->source);
-}
-
-void
-rtable_clearsource(unsigned int rtableid, struct sockaddr *src)
-{
- struct sockaddr *addr;
-
- addr = rtable_getsource(rtableid, src->sa_family);
- if (addr && (addr->sa_len == src->sa_len)) {
- if (memcmp(src, addr, addr->sa_len) == 0) {
- rtable_setsource(rtableid, src->sa_family, NULL);
- }
- }
-}
-
struct rtentry *
rtable_lookup(unsigned int rtableid, struct sockaddr *dst,
struct sockaddr *mask, struct sockaddr *gateway, uint8_t prio)
Index: net/rtable.h
===
RCS file: /cvs/src/sys/net/rtable.h,v
retrieving revision 1.26
diff -u -p -r1.26 rtable.h
--- net/rtable.h7 Nov 2020 09:51:40 - 1.26
+++ net/rtable.h9 Jan 2021 16:04:02 -
@@ -39,9 +39,6 @@ unsigned int rtable_l2(unsigned int);
unsigned intrtable_loindex(unsigned int);
voidrtable_l2set(unsigned int, unsigned int, unsigned int);
-int rtable_setsource(unsigned int, int, struct sockaddr *);
-struct sockaddr *rtable_getsource(unsigned int, int);
-voidrtable_clearsource(unsigned int, struct sockaddr *);
struct rtentry *rtable_lookup(unsigned int, struct sockaddr *,
struct sockaddr *, struct sockaddr *, uint8_t);
struct rtentry *rtable_match(unsigned int, struct sockaddr *, uint32_t *);
Index: net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.304
diff -u -p -r1.304 rtsock.c
--- net/rtsock.c7 Nov 2020 09:51:40 - 1.304
+++ net/rtsock.c9 Jan 2021 16:04:02 -
@@ -138,7 +138,8 @@ int sysctl_iflist(int, struct walkarg
int sysctl_ifnames(struct walkarg *);
int
XBox One gamecontroller support
Hi,
This diff adds support for the XBox One gamecontroller in a similar way
to what we have for the (older) XBox 360 controller [1][2]. This diff
is based on the pertinent code in NetBSD's uhidev.c.
Similarities include that the device doesn't provide a report
descriptor, so this diff adds one to uhid_rdesc.h.
The biggest difference (that held this up for a while) is that the
controller needs an init byte sequence to "wake up."
The original report descriptor from NetBSD just maps the buttons and
axes in the order that they appear in the report. I modified the report
descriptor to assign the buttons/axes in the most logical order for
compatibility, which results in the same layout that we already have
for the XBox 360 controller.
The addition of the member sc_flags to struct uhidev_softc follows the
NetBSD example.
I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
package), and a couple of SDL2 games (Owlboy, Cryptark).
If you have an XBox One controller, the easiest way to test is with:
$ usbhidctl -f /dev/uhidX -l
Make sure to pick the right uhid device and that you have read
permissions for it.
Of course, this works only with the controller connected via USB, and
doesn't magically add wireless support.
If you test actual SDL2 applications, the button layout will likely
default to an odd configuration. I am simultaneously preparing a diff
for sdl2-2.0.14p0 that improves recognition and automatic mapping and
solves any issues with XBox One default button layout.
comments? ok?
[1] https://marc.info/?l=openbsd-cvs=138267062532046=mbox
[2] https://marc.info/?l=openbsd-cvs=144956819605939=mbox
Index: uhid_rdesc.h
===
RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
retrieving revision 1.1
diff -u -p -r1.1 uhid_rdesc.h
--- uhid_rdesc.h25 Oct 2013 03:09:59 - 1.1
+++ uhid_rdesc.h9 Jan 2021 15:11:30 -
@@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
0x95, 0x01,/* REPORT COUNT (1)*/
0x81, 0x01,/* INPUT (Constant)*/
0xc0, /* END COLLECTION */
+};
+
+static const uByte uhid_xbonegp_report_descr[] = {
+0x05, 0x01,/* USAGE PAGE (Generic Desktop)
*/
+0x09, 0x05,/* USAGE (Gamepad)
*/
+0xa1, 0x01,/* COLLECTION (Application)
*/
+/* Button packet */
+0xa1, 0x00,/* COLLECTION (Physical)
*/
+0x85, 0x20,/* REPORT ID (0x20)
*/
+/* Skip unknown field and counter */
+0x09, 0x00,/* USAGE (Undefined)
*/
+0x75, 0x08,/* REPORT SIZE (8)
*/
+0x95, 0x02,/* REPORT COUNT (2)
*/
+0x81, 0x03,/* INPUT (Constant, Variable,
Absolute) */
+/* Payload size */
+0x09, 0x3b,/* USAGE (Byte Count)
*/
+0x95, 0x01,/* REPORT COUNT (1)
*/
+0x81, 0x02,/* INPUT (Data, Variable, Absolute)
*/
+/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
+ *9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
+ */
+/* Skip the first 2 as they are not used */
+0x75, 0x01,/* REPORT SIZE (1)
*/
+0x95, 0x02,/* REPORT COUNT (2)
*/
+0x81, 0x01,/* INPUT (Constant)
*/
+/* Assign buttons Start(7), Back(8), ABXY(1-4) */
+0x15, 0x00,/* LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/* LOGICAL MAXIMUM (1)
*/
+0x95, 0x06,/* REPORT COUNT (6)
*/
+0x05, 0x09,/* USAGE PAGE (Button)
*/
+0x09, 0x08,/* USAGE (Button 8)
*/
+0x09, 0x07,/* USAGE (Button 7)
*/
+0x09, 0x01,/* USAGE (Button 1)
*/
+0x09, 0x02,/* USAGE (Button 2)
*/
+0x09, 0x03,/* USAGE (Button 3)
*/
+0x09, 0x04,/* USAGE (Button 4)
*/
+0x81, 0x02,/* INPUT (Data, Variable,
Re: getopt.3 bugs section
On Jan 9, 2021 7:07 AM, Christian Weisgerber wrote: Edgar Pettijohn: > In the BUGS section for the getopt(3) manual it mentions not using > single digits for options. I know spamd uses -4 and -6 there are > probably others. Should they be changed? Or is the manual mistaken? You misunderstand. The manual warns against the use of digits to pass numerical arguments. This usage exists in some historical cases, e.g. "nice -10" where <10> is the number 10. The use of digits as flags characters, like -4 or -6 to indicate IPv4 or IPv6, is perfectly fine. -- Christian "naddy" Weisgerber na...@mips.inka.de Makes sense. I think it would have been clearer if it just had an example of what not to do instead of how to do the wrong thing right. Thanks, Edgar
Re: slimblade support
On 2021 Jan 09 (Sat) at 07:00:29 -0700 (-0700), Thomas Frohwein wrote:
:On Sat, Nov 21, 2020 at 08:10:03AM +0200, Timo Myyrä wrote:
:> Hi,
:>
:> The last attempt at adding Kensington Slimblade trackball support seems
:> to have stalled:
:> https://marc.info/?l=openbsd-tech=147444999319756=2
:>
:> I tested the diff and it still seems apply with little fuzz and works
:> with my slimblade. It would be nice to have this included so I can paste
:> with mouse.
:>
:> Here's cleaned up patch for reference.
:>
:> timo
:
:I've been running with this diff through several snapshot upgrades
:without issues. Would be interested to hear if there are concerns about
:this diff or if I can get another ok? As far as quirks go, it's not
:much code and relatively simple...
:
this makes sense to have a quirk for, OK
:>
:>
:> Index: sys/dev/usb/ums.c
:> ===
:> RCS file: /cvs/src/sys/dev/usb/ums.c,v
:> retrieving revision 1.45
:> diff -u -p -u -p -r1.45 ums.c
:> --- sys/dev/usb/ums.c23 Aug 2020 11:08:02 - 1.45
:> +++ sys/dev/usb/ums.c20 Nov 2020 20:22:11 -
:> @@ -150,6 +150,8 @@ ums_attach(struct device *parent, struct
:> qflags |= HIDMS_MS_BAD_CLASS;
:> if (quirks & UQ_MS_LEADING_BYTE)
:> qflags |= HIDMS_LEADINGBYTE;
:> +if (quirks & UQ_MS_VENDOR_BUTTONS)
:> +qflags |= HIDMS_VENDOR_BUTTONS;
:>
:> if (hidms_setup(self, ms, qflags, uha->reportid, desc, size) != 0)
:> return;
:> Index: sys/dev/usb/usb_quirks.c
:> ===
:> RCS file: /cvs/src/sys/dev/usb/usb_quirks.c,v
:> retrieving revision 1.76
:> diff -u -p -u -p -r1.76 usb_quirks.c
:> --- sys/dev/usb/usb_quirks.c 5 Jan 2020 00:54:13 - 1.76
:> +++ sys/dev/usb/usb_quirks.c 20 Nov 2020 20:22:11 -
:> @@ -150,6 +150,9 @@ const struct usbd_quirk_entry {
:> { USB_VENDOR_MICROSOFT, USB_PRODUCT_MICROSOFT_WLNOTEBOOK2,
:> ANY, { UQ_MS_BAD_CLASS | UQ_MS_LEADING_BYTE }},
:>
:> + { USB_VENDOR_KENSINGTON, USB_PRODUCT_KENSINGTON_SLIMBLADE,
:> +ANY, { UQ_MS_VENDOR_BUTTONS }},
:> +
:> { 0, 0, 0, { 0 } }
:> };
:>
:> Index: sys/dev/usb/usb_quirks.h
:> ===
:> RCS file: /cvs/src/sys/dev/usb/usb_quirks.h,v
:> retrieving revision 1.16
:> diff -u -p -u -p -r1.16 usb_quirks.h
:> --- sys/dev/usb/usb_quirks.h 19 Jul 2010 05:08:37 - 1.16
:> +++ sys/dev/usb/usb_quirks.h 20 Nov 2020 20:22:11 -
:> @@ -49,6 +49,8 @@ struct usbd_quirks {
:> #define UQ_MS_LEADING_BYTE 0x0001 /* mouse sends unknown leading byte
*/
:> #define UQ_EHCI_NEEDTO_DISOWN 0x0002 /* must hand device over to
USB 1.1
:> if attached to EHCI */
:> +#define UQ_MS_VENDOR_BUTTONS0x0004 /* mouse reports extra
buttons in
:> +vendor page */
:> };
:>
:> extern const struct usbd_quirks usbd_no_quirk;
:> Index: sys/dev/usb/usbdevs
:> ===
:> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
:> retrieving revision 1.728
:> diff -u -p -u -p -r1.728 usbdevs
:> --- sys/dev/usb/usbdevs 16 Nov 2020 09:49:10 - 1.728
:> +++ sys/dev/usb/usbdevs 20 Nov 2020 20:22:11 -
:> @@ -2491,6 +2491,7 @@ product KENSINGTON TURBOBALL 0x1005 Turb
:> product KENSINGTON ORBIT_MAC0x1009 Orbit trackball for Mac
:> product KENSINGTON BT_EDR 0x105e Bluetooth
:> product KENSINGTON VIDEOCAM_VGA 0x5002 VideoCAM VGA
:> +product KENSINGTON SLIMBLADE0x2041 Slimblade Trackball
:>
:> /* Keyspan products */
:> product KEYSPAN USA28_NF0x0101 USA-28 serial
:> Index: sys/dev/usb/usbdevs.h
:> ===
:> RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v
:> retrieving revision 1.740
:> diff -u -p -u -p -r1.740 usbdevs.h
:> --- sys/dev/usb/usbdevs.h16 Nov 2020 09:49:40 - 1.740
:> +++ sys/dev/usb/usbdevs.h20 Nov 2020 20:22:11 -
:> @@ -2498,6 +2498,7 @@
:> #define USB_PRODUCT_KENSINGTON_ORBIT_MAC0x1009 /*
Orbit trackball for Mac */
:> #define USB_PRODUCT_KENSINGTON_BT_EDR 0x105e /* Bluetooth */
:> #define USB_PRODUCT_KENSINGTON_VIDEOCAM_VGA 0x5002 /*
VideoCAM VGA */
:> +#define USB_PRODUCT_KENSINGTON_SLIMBLADE0x2041 /*
Slimblade Trackball */
:>
:> /* Keyspan products */
:> #define USB_PRODUCT_KEYSPAN_USA28_NF0x0101 /* USA-28
serial */
:> Index: sys/dev/usb/usbdevs_data.h
:> ===
:> RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v
:> retrieving revision 1.734
:> diff -u -p -u -p -r1.734 usbdevs_data.h
:> --- sys/dev/usb/usbdevs_data.h 16 Nov 2020 09:49:40 - 1.734
:> +++
Re: getopt.3 bugs section
On Sat, Jan 09, 2021 at 02:07:32PM +0100, Christian Weisgerber wrote: > Edgar Pettijohn: > > > In the BUGS section for the getopt(3) manual it mentions not using > > single digits for options. I know spamd uses -4 and -6 there are > > probably others. Should they be changed? Or is the manual mistaken? > > You misunderstand. The manual warns against the use of digits to > pass numerical arguments. This usage exists in some historical > cases, e.g. "nice -10" where <10> is the number 10. > > The use of digits as flags characters, like -4 or -6 to indicate > IPv4 or IPv6, is perfectly fine. If you want to change this, it seems best to point to existing good and bad examples. Good: - using digits as flags, e.g. for IPv4 vs IPv6 support Bad: - using digits as numbers, e.g. nice(1), tail(1) historic use. Recommendation to avoid bad use: -n Joerg
Re: slimblade support
On Sat, Nov 21, 2020 at 08:10:03AM +0200, Timo Myyrä wrote:
> Hi,
>
> The last attempt at adding Kensington Slimblade trackball support seems
> to have stalled:
> https://marc.info/?l=openbsd-tech=147444999319756=2
>
> I tested the diff and it still seems apply with little fuzz and works
> with my slimblade. It would be nice to have this included so I can paste
> with mouse.
>
> Here's cleaned up patch for reference.
>
> timo
I've been running with this diff through several snapshot upgrades
without issues. Would be interested to hear if there are concerns about
this diff or if I can get another ok? As far as quirks go, it's not
much code and relatively simple...
>
>
> Index: sys/dev/usb/ums.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ums.c,v
> retrieving revision 1.45
> diff -u -p -u -p -r1.45 ums.c
> --- sys/dev/usb/ums.c 23 Aug 2020 11:08:02 - 1.45
> +++ sys/dev/usb/ums.c 20 Nov 2020 20:22:11 -
> @@ -150,6 +150,8 @@ ums_attach(struct device *parent, struct
> qflags |= HIDMS_MS_BAD_CLASS;
> if (quirks & UQ_MS_LEADING_BYTE)
> qflags |= HIDMS_LEADINGBYTE;
> + if (quirks & UQ_MS_VENDOR_BUTTONS)
> + qflags |= HIDMS_VENDOR_BUTTONS;
>
> if (hidms_setup(self, ms, qflags, uha->reportid, desc, size) != 0)
> return;
> Index: sys/dev/usb/usb_quirks.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_quirks.c,v
> retrieving revision 1.76
> diff -u -p -u -p -r1.76 usb_quirks.c
> --- sys/dev/usb/usb_quirks.c 5 Jan 2020 00:54:13 - 1.76
> +++ sys/dev/usb/usb_quirks.c 20 Nov 2020 20:22:11 -
> @@ -150,6 +150,9 @@ const struct usbd_quirk_entry {
> { USB_VENDOR_MICROSOFT, USB_PRODUCT_MICROSOFT_WLNOTEBOOK2,
> ANY, { UQ_MS_BAD_CLASS | UQ_MS_LEADING_BYTE }},
>
> + { USB_VENDOR_KENSINGTON, USB_PRODUCT_KENSINGTON_SLIMBLADE,
> + ANY, { UQ_MS_VENDOR_BUTTONS }},
> +
> { 0, 0, 0, { 0 } }
> };
>
> Index: sys/dev/usb/usb_quirks.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_quirks.h,v
> retrieving revision 1.16
> diff -u -p -u -p -r1.16 usb_quirks.h
> --- sys/dev/usb/usb_quirks.h 19 Jul 2010 05:08:37 - 1.16
> +++ sys/dev/usb/usb_quirks.h 20 Nov 2020 20:22:11 -
> @@ -49,6 +49,8 @@ struct usbd_quirks {
> #define UQ_MS_LEADING_BYTE 0x0001 /* mouse sends unknown leading byte
> */
> #define UQ_EHCI_NEEDTO_DISOWN0x0002 /* must hand device over to
> USB 1.1
> if attached to EHCI */
> +#define UQ_MS_VENDOR_BUTTONS 0x0004 /* mouse reports extra buttons in
> + vendor page */
> };
>
> extern const struct usbd_quirks usbd_no_quirk;
> Index: sys/dev/usb/usbdevs
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> retrieving revision 1.728
> diff -u -p -u -p -r1.728 usbdevs
> --- sys/dev/usb/usbdevs 16 Nov 2020 09:49:10 - 1.728
> +++ sys/dev/usb/usbdevs 20 Nov 2020 20:22:11 -
> @@ -2491,6 +2491,7 @@ product KENSINGTON TURBOBALL0x1005 Turb
> product KENSINGTON ORBIT_MAC 0x1009 Orbit trackball for Mac
> product KENSINGTON BT_EDR0x105e Bluetooth
> product KENSINGTON VIDEOCAM_VGA 0x5002 VideoCAM VGA
> +product KENSINGTON SLIMBLADE 0x2041 Slimblade Trackball
>
> /* Keyspan products */
> product KEYSPAN USA28_NF 0x0101 USA-28 serial
> Index: sys/dev/usb/usbdevs.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v
> retrieving revision 1.740
> diff -u -p -u -p -r1.740 usbdevs.h
> --- sys/dev/usb/usbdevs.h 16 Nov 2020 09:49:40 - 1.740
> +++ sys/dev/usb/usbdevs.h 20 Nov 2020 20:22:11 -
> @@ -2498,6 +2498,7 @@
> #define USB_PRODUCT_KENSINGTON_ORBIT_MAC0x1009 /*
> Orbit trackball for Mac */
> #define USB_PRODUCT_KENSINGTON_BT_EDR 0x105e /* Bluetooth */
> #define USB_PRODUCT_KENSINGTON_VIDEOCAM_VGA 0x5002 /*
> VideoCAM VGA */
> +#define USB_PRODUCT_KENSINGTON_SLIMBLADE0x2041 /*
> Slimblade Trackball */
>
> /* Keyspan products */
> #define USB_PRODUCT_KEYSPAN_USA28_NF0x0101 /* USA-28
> serial */
> Index: sys/dev/usb/usbdevs_data.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v
> retrieving revision 1.734
> diff -u -p -u -p -r1.734 usbdevs_data.h
> --- sys/dev/usb/usbdevs_data.h16 Nov 2020 09:49:40 - 1.734
> +++ sys/dev/usb/usbdevs_data.h20 Nov 2020 20:22:11 -
> @@ -5402,6 +5402,10 @@ const struct usb_known_product usb_known
> "VideoCAM VGA",
> },
> {
> + USB_VENDOR_KENSINGTON,
Re: getopt.3 bugs section
On Sat, Jan 09 2021, Christian Weisgerber wrote: > Edgar Pettijohn: > >> In the BUGS section for the getopt(3) manual it mentions not using >> single digits for options. I know spamd uses -4 and -6 there are >> probably others. Should they be changed? Or is the manual mistaken? > > You misunderstand. The manual warns against the use of digits to > pass numerical arguments. This usage exists in some historical > cases, e.g. "nice -10" where <10> is the number 10. IMO giving a *number* as an example would make things clearer. Index: getopt.3 === RCS file: /d/cvs/src/lib/libc/stdlib/getopt.3,v retrieving revision 1.46 diff -u -p -p -u -r1.46 getopt.3 --- getopt.34 Jan 2016 19:43:13 - 1.46 +++ getopt.39 Jan 2021 13:39:06 - @@ -326,7 +326,7 @@ It is possible to handle digits as optio This allows .Fn getopt to be used with programs that expect a number -.Pq Dq Li \-3 +.Pq Dq Li \-389 as an option. This practice is wrong, and should not be used in any current development. It is provided for backward compatibility Other ideas for improvement: - give a real life example Index: getopt.3 === RCS file: /d/cvs/src/lib/libc/stdlib/getopt.3,v retrieving revision 1.46 diff -u -p -p -u -r1.46 getopt.3 --- getopt.34 Jan 2016 19:43:13 - 1.46 +++ getopt.39 Jan 2021 13:44:29 - @@ -326,7 +326,7 @@ It is possible to handle digits as optio This allows .Fn getopt to be used with programs that expect a number -.Pq Dq Li \-3 +.Pq Dq Li nice \-10 program as an option. This practice is wrong, and should not be used in any current development. It is provided for backward compatibility - move this warning to CAVEATS, since it's not a bug Index: getopt.3 === RCS file: /d/cvs/src/lib/libc/stdlib/getopt.3,v retrieving revision 1.46 diff -u -p -p -u -r1.46 getopt.3 --- getopt.34 Jan 2016 19:43:13 - 1.46 +++ getopt.39 Jan 2021 13:47:08 - @@ -309,24 +309,12 @@ The .Fn getopt function appeared in .Bx 4.3 . -.Sh BUGS -The -.Fn getopt -function was once specified to return -.Dv EOF -instead of \-1. -This was changed by -.St -p1003.2-92 -to decouple -.Fn getopt -from -.In stdio.h . -.Pp +.Sh CAVEATS It is possible to handle digits as option letters. This allows .Fn getopt to be used with programs that expect a number -.Pq Dq Li \-3 +.Pq Dq Li nice \-10 program as an option. This practice is wrong, and should not be used in any current development. It is provided for backward compatibility @@ -361,3 +349,15 @@ while ((ch = getopt(argc, argv, "0123456 prevoptind = optind; } .Ed +.Sh BUGS +The +.Fn getopt +function was once specified to return +.Dv EOF +instead of \-1. +This was changed by +.St -p1003.2-92 +to decouple +.Fn getopt +from +.In stdio.h . Would the mention of EOF vs -1 fit better in HISTORY or CAVEATS too? -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: getopt.3 bugs section
Edgar Pettijohn: > In the BUGS section for the getopt(3) manual it mentions not using > single digits for options. I know spamd uses -4 and -6 there are > probably others. Should they be changed? Or is the manual mistaken? You misunderstand. The manual warns against the use of digits to pass numerical arguments. This usage exists in some historical cases, e.g. "nice -10" where <10> is the number 10. The use of digits as flags characters, like -4 or -6 to indicate IPv4 or IPv6, is perfectly fine. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: all platforms: isolate hardclock(9) from statclock()
On Fri, Jan 08, 2021 at 10:18:27AM -0600, Scott Cheloha wrote:
> On Thu, Jan 07, 2021 at 08:12:10PM -0600, Scott Cheloha wrote:
> > On Thu, Jan 07, 2021 at 09:37:58PM +0100, Mark Kettenis wrote:
> > > > Date: Thu, 7 Jan 2021 11:15:41 -0600
> > > > From: Scott Cheloha
> > > >
> > > > Hi,
> > > >
> > > > I want to isolate statclock() from hardclock(9). This will simplify
> > > > the logic in my WIP dynamic clock interrupt framework.
> > > >
> > > > Currently, if stathz is zero, we call statclock() from within
> > > > hardclock(9). It looks like this (see sys/kern/kern_clock.c):
> > > >
> > > > void
> > > > hardclock(struct clockframe *frame)
> > > > {
> > > > /* [...] */
> > > >
> > > > if (stathz == 0)
> > > > statclock(frame);
> > > >
> > > > /* [...] */
> > > >
> > > > This is the case on alpha, amd64 (w/ lapic), hppa, i386 (w/ lapic),
> > > > loongson, luna88k, mips64, and sh.
> > > >
> > > > (We seem to do it on sgi, too. I was under the impression that sgi
> > > > *was* a mips64 platform, yet sgi seems to it have its own clock
> > > > interrupt code distinct from mips64's general clock interrupt code
> > > > which is used by e.g. octeon).
> > > >
> > > > However, if stathz is not zero we call statclock() separately. This
> > > > is the case on armv7, arm, arm64, macppc, powerpc64, and sparc64.
> > > >
> > > > (The situation for the general powerpc code and socppc in particular
> > > > is a mystery to me.)
> > > >
> > > > If we could remove this MD distinction it would make my MI framework
> > > > simpler. Instead of checking stathz and conditionally starting a
> > > > statclock event I will be able to unconditionally start a statclock
> > > > event on all platforms on every CPU.
> > > >
> > > > In general I don't think the "is stathz zero?" variance between
> > > > platforms is useful:
> > > >
> > > > - The difference is invisible to userspace, as we hide the fact that
> > > > stathz is zero from e.g. the kern.clockrate sysctl.
> > > >
> > > > - We run statclock() at some point, regardless of whether stathz is
> > > > zero. If statclock() is run from hardclock(9) then isn't stathz
> > > > effectively equal to hz?
> > > >
> > > > - Because stathz might be zero we need to add a bunch of safety checks
> > > > to our MI code to ensure we don't accidentally divide by zero.
> > > >
> > > > Maybe we can ensure stathz is non-zero in a later diff...
> > > >
> > > > --
> > > >
> > > > Anyway, I don't think I have missed any platforms. However, if
> > > > platform experts could weigh in here to verify my changes (and test
> > > > them!) I'd really appreciate it.
> > > >
> > > > In particular, I'm confused about how clock interrupts work on
> > > > powerpc, socppc, and sgi.
> > > >
> > > > --
> > > >
> > > > Thoughts? Platform-specific OKs?
> > >
> > > I wouldn't be opposed to doing this. It is less magic!
> > >
> > > But yes, this needs to be tested on the platforms that you change.
> >
> > I guess I'll CC all the platform-specific people I'm aware of.
> >
> > > Note that many platforms don't have have separate schedclock and
> > > statclock. But on many platforms where we use a one-shot timer as the
> > > clock we have a randomized statclock. I'm sure Theo would love to
> > > tell you about the cpuhog story...
> >
> > I am familiar with cpuhog. It's the one thing everybody mentions when
> > I talk about clock interrupts and/or statclock().
> >
> > Related:
> >
> > I wonder if we could avoid the cpuhog problem entirely by implementing
> > some kind of MI cycle counting clock API that we use to timestamp
> > whenever we cross the syscall boundary, or enter an interrupt, etc.,
> > to determine the time a thread spends using the CPU without any
> > sampling error.
> >
> > Instead of a process accumulating ticks from a sampling clock
> > interrupt you would accumulate, say, a 64-bit count of cycles, or
> > something like that.
> >
> > Sampling with a regular clock interrupt is prone to error and trickery
> > like cpuhog. The classic BSD solution to the cpuhog exploit was to
> > randomize the statclock/schedclock to make it harder to fool the
> > sampler. But if we used cycle counts or instruction counts at each
> > state transition it would be impossible to fool because we wouldn't be
> > sampling at all.
I think statclock is run at different frequency than hardclock for a good
reason. If their frequency is the same and there is no variable offset,
the sampling is prone to bias. For example, let's assume that each
hardclock tick makes the system spend the first half of tick interval
in kernel, say in soft interrupt code. Does that kernel time get noticed?
> > Unsure what the performance implications would be, but in general I
> > would guess that most platforms have a way to count instructions or
> > cycles and that reading this data is fast enough for us to use it in
> > e.g. syscall() or the interrupt handler without a
Re: all platforms: isolate hardclock(9) from statclock()
Hello,
On Sat, 09 Jan 2021 01:18:27 +0900,
Scott Cheloha wrote:
> > > > Anyway, I don't think I have missed any platforms. However, if
> > > > platform experts could weigh in here to verify my changes (and test
> > > > them!) I'd really appreciate it.
> Also, sorry if I've CC'd you and you're not the right person for one
> of these platforms/architectures. My thinking is:
>
> miod: loongson (?), sh
> aoyama: luna88k
> visa: mips64, sgi
> deraadt: alpha
> kettenis: hppa
> sthen: i386
Yes, I am the right person for luna88k:-)
I applied this diff and it works well on my luna88k box.
-- aoyama
> Index: sys/kern/kern_clock.c
> ===
> RCS file: /cvs/src/sys/kern/kern_clock.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 kern_clock.c
> --- sys/kern/kern_clock.c 21 Jan 2020 16:16:23 - 1.101
> +++ sys/kern/kern_clock.c 8 Jan 2021 15:56:24 -
> @@ -164,12 +164,6 @@ hardclock(struct clockframe *frame)
> }
> }
>
> - /*
> - * If no separate statistics clock is available, run it from here.
> - */
> - if (stathz == 0)
> - statclock(frame);
> -
> if (--ci->ci_schedstate.spc_rrticks <= 0)
> roundrobin(ci);
>
> Index: sys/arch/alpha/alpha/clock.c
> ===
> RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 clock.c
> --- sys/arch/alpha/alpha/clock.c 6 Jul 2020 13:33:06 - 1.24
> +++ sys/arch/alpha/alpha/clock.c 8 Jan 2021 15:56:24 -
> @@ -136,6 +136,13 @@ clockattach(dev, fns)
> * Machine-dependent clock routines.
> */
>
> +void
> +clockintr(struct clockframe *frame)
> +{
> + hardclock(frame);
> + statclock(frame);
> +}
> +
> /*
> * Start the real-time and statistics clocks. Leave stathz 0 since there
> * are no other timers available.
> @@ -165,7 +172,7 @@ cpu_initclocks(void)
>* hardclock, which would then fall over because the pointer
>* to the virtual timers wasn't set at that time.
>*/
> - platform.clockintr = hardclock;
> + platform.clockintr = clockintr;
> schedhz = 16;
>
> evcount_attach(_count, "clock", _irq);
> Index: sys/arch/amd64/amd64/lapic.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 lapic.c
> --- sys/arch/amd64/amd64/lapic.c 6 Sep 2020 20:50:00 - 1.57
> +++ sys/arch/amd64/amd64/lapic.c 8 Jan 2021 15:56:25 -
> @@ -452,6 +452,7 @@ lapic_clockintr(void *arg, struct intrfr
> floor = ci->ci_handled_intr_level;
> ci->ci_handled_intr_level = ci->ci_ilevel;
> hardclock((struct clockframe *));
> + statclock((struct clockframe *));
> ci->ci_handled_intr_level = floor;
>
> clk_count.ec_count++;
> Index: sys/arch/hppa/dev/clock.c
> ===
> RCS file: /cvs/src/sys/arch/hppa/dev/clock.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 clock.c
> --- sys/arch/hppa/dev/clock.c 6 Jul 2020 13:33:07 - 1.31
> +++ sys/arch/hppa/dev/clock.c 8 Jan 2021 15:56:25 -
> @@ -43,7 +43,7 @@
>
> u_long cpu_hzticks;
>
> -int cpu_hardclock(void *);
> +int cpu_clockintr(void *);
> u_intitmr_get_timecount(struct timecounter *);
>
> struct timecounter itmr_timecounter = {
> @@ -106,7 +106,7 @@ cpu_initclocks(void)
> }
>
> int
> -cpu_hardclock(void *v)
> +cpu_clockintr(void *v)
> {
> struct cpu_info *ci = curcpu();
> u_long __itmr, delta, eta;
> @@ -114,14 +114,15 @@ cpu_hardclock(void *v)
> register_t eiem;
>
> /*
> - * Invoke hardclock as many times as there has been cpu_hzticks
> - * ticks since the last interrupt.
> + * Invoke hardclock and statclock as many times as there has been
> + * cpu_hzticks ticks since the last interrupt.
>*/
> for (;;) {
> mfctl(CR_ITMR, __itmr);
> delta = __itmr - ci->ci_itmr;
> if (delta >= cpu_hzticks) {
> hardclock(v);
> + statclock(v);
> ci->ci_itmr += cpu_hzticks;
> } else
> break;
> Index: sys/arch/hppa/dev/cpu.c
> ===
> RCS file: /cvs/src/sys/arch/hppa/dev/cpu.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 cpu.c
> --- sys/arch/hppa/dev/cpu.c 29 May 2020 04:42:23 - 1.42
> +++ sys/arch/hppa/dev/cpu.c 8 Jan 2021 15:56:25 -
> @@ -89,7 +89,7 @@ cpuattach(struct device *parent, struct
> extern u_int cpu_ticksnum, cpu_ticksdenom;
> extern u_int fpu_enable;
> /* clock.c */
> - extern int cpu_hardclock(void *);
> + extern int cpu_clockintr(void *);
> /*
Re: unwind(8): respect DO flag
On Fri, Jan 08, 2021 at 06:37:35PM +0100, Florian Obser wrote:
> Rewrite query parsing and answer formating using libunbound provided
> functions.
> With this we can filter out DNSSEC RRsets if the client did not ask
> for them.
> We will also be able to send truncated answers to indicate to the
> client to switch to tcp (disabled for now since we have no tcp support
> yet).
>
> Tests, OKs?
>
It's better to leak less memory...
diff --git frontend.c frontend.c
index 9f91396c4c3..60dafd81a79 100644
--- frontend.c
+++ frontend.c
@@ -48,7 +48,11 @@
#include "libunbound/sldns/sbuffer.h"
#include "libunbound/sldns/str2wire.h"
#include "libunbound/sldns/wire2str.h"
+#include "libunbound/util/alloc.h"
+#include "libunbound/util/net_help.h"
+#include "libunbound/util/regional.h"
#include "libunbound/util/data/dname.h"
+#include "libunbound/util/data/msgencode.h"
#include "libunbound/util/data/msgparse.h"
#include "libunbound/util/data/msgreply.h"
@@ -81,14 +85,13 @@ struct pending_query {
TAILQ_ENTRY(pending_query) entry;
struct sockaddr_storage from;
struct sldns_buffer *qbuf;
- ssize_t len;
+ struct sldns_buffer *abuf;
+ struct regional *region;
+ struct query_infoqinfo;
+ struct msg_parse*qmsg;
+ struct edns_data edns;
uint64_t imsg_id;
int fd;
- int bogus;
- int rcode_override;
- int answer_len;
- int received;
- uint8_t *answer;
};
TAILQ_HEAD(, pending_query) pending_queries;
@@ -102,8 +105,12 @@ __dead void frontend_shutdown(void);
voidfrontend_sig_handler(int, short, void *);
voidfrontend_startup(void);
voidudp_receive(int, short, void *);
+voidhandle_query(struct pending_query *);
+voidfree_pending_query(struct pending_query *);
int check_query(sldns_buffer*);
+voidnoerror_answer(struct pending_query *);
voidchaos_answer(struct pending_query *);
+voiderror_answer(struct pending_query *, int rcode);
voidsend_answer(struct pending_query *);
voidroute_receive(int, short, void *);
voidhandle_route_message(struct rt_msghdr *,
@@ -471,37 +478,38 @@ frontend_dispatch_resolver(int fd, short event, void
*bula)
}
if (answer_header->srvfail) {
- free(pq->answer);
- pq->answer_len = 0;
- pq->answer = NULL;
- pq->rcode_override = LDNS_RCODE_SERVFAIL;
+ error_answer(pq, LDNS_RCODE_SERVFAIL);
send_answer(pq);
break;
}
- if (pq->answer == NULL) {
- pq->answer = malloc(answer_header->answer_len);
- if (pq->answer == NULL) {
- pq->answer_len = 0;
- pq->rcode_override =
- LDNS_RCODE_SERVFAIL;
- send_answer(pq);
- break;
- }
- pq->answer_len = answer_header->answer_len;
- pq->received = 0;
- pq->bogus = answer_header->bogus;
+ if (answer_header->bogus && !(pq->qmsg->flags &
+ BIT_CD)) {
+ error_answer(pq, LDNS_RCODE_SERVFAIL);
+ send_answer(pq);
+ break;
+ }
+
+ if (sldns_buffer_position(pq->abuf) == 0 &&
+ !sldns_buffer_set_capacity(pq->abuf,
+ answer_header->answer_len)) {
+ error_answer(pq, LDNS_RCODE_SERVFAIL);
+ send_answer(pq);
+ break;
}
- if (pq->received + data_len > pq->answer_len)
+ if (sldns_buffer_position(pq->abuf) + data_len >
+ sldns_buffer_capacity(pq->abuf))
fatalx("%s: IMSG_ANSWER answer too big: %d",
__func__, data_len);
+
hid_is_collection() sync with NetBSD
I have not much clue about HID, but when we did some testing for the
new ujoy(4) driver it turned out that the PS4 controller doesn't get
handled correctly by hid.c:hid_is_collection(). This made me peek in
to the NetBSD code where I could find an update in this function.
Syncing it up makes the PS4 controller attachment work correctly,
while not breaking my other HID devices.
The change was introduced in NetBSD with hid.c revision 1.25 on the
19.06.2006, obviously as part of a bigger change, not saying something
about the hid_is_collection() update itself:
--
Initial import of bluetooth stack on behalf of Iain Hibbert. (plunky@,
NetBSD Foundation Membership still pending.) This stack was written by
Iain under sponsorship from Itronix Inc.
The stack includes support for rfcomm networking (networking via your
bluetooth enabled cell phone), hid devices (keyboards/mice), and
headsets.
Drivers for both PCMCIA and USB bluetooth controllers are included.
--
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/usb/Attic/hid.c.diff?r1=1.24=1.25_with_tag=MAIN
More testers? Comments? OKs?
Index: dev/hid/hid.c
===
RCS file: /cvs/src/sys/dev/hid/hid.c,v
retrieving revision 1.3
diff -u -p -u -p -r1.3 hid.c
--- dev/hid/hid.c 4 Jun 2020 23:03:43 - 1.3
+++ dev/hid/hid.c 9 Jan 2021 08:43:30 -
@@ -630,6 +630,25 @@ hid_get_udata(const uint8_t *buf, int le
return (hid_get_data_sub(buf, len, loc, 0));
}
+/*
+ * hid_is_collection(desc, size, id, usage)
+ *
+ * This function is broken in the following way.
+ *
+ * It is used to discover if the given 'id' is part of 'usage' collection
+ * in the descriptor in order to match report id against device type.
+ *
+ * The semantics of hid_start_parse() means though, that only a single
+ * kind of report is considered. The current HID code that uses this for
+ * matching is actually only looking for input reports, so this works
+ * for now.
+ *
+ * This function could try all report kinds (input, output and feature)
+ * consecutively if necessary, but it may be better to integrate the
+ * libusbhid code which can consider multiple report kinds simultaneously
+ *
+ * Needs some thought.
+ */
int
hid_is_collection(const void *desc, int size, uint8_t id, int32_t usage)
{
@@ -637,17 +656,25 @@ hid_is_collection(const void *desc, int
struct hid_item hi;
uint32_t coll_usage = ~0;
- hd = hid_start_parse(desc, size, hid_none);
+ hd = hid_start_parse(desc, size, hid_input);
+ if (hd == NULL)
+ return (0);
DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage);
while (hid_get_item(hd, )) {
DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__,
hi.kind, hi.report_ID, hi.usage, coll_usage);
+
if (hi.kind == hid_collection &&
hi.collection == HCOLL_APPLICATION)
coll_usage = hi.usage;
- if (hi.kind == hid_endcollection &&
- coll_usage == usage && hi.report_ID == id) {
+
+ if (hi.kind == hid_endcollection)
+ coll_usage = ~0;
+
+ if (hi.kind == hid_input &&
+ coll_usage == usage &&
+ hi.report_ID == id) {
DPRINTF("%s: found\n", __func__);
hid_end_parse(hd);
return (1);
Re: cmp -s bugfix
On Sat, Jan 09, 2021 at 08:00:42AM +0100, Otto Moerbeek wrote:
> As reported on misc@
>
> https://marc.info/?l=openbsd-misc=161016188503894=2
ok tb
>
> -Otto
>
> Index: regular.c
> ===
> RCS file: /cvs/src/usr.bin/cmp/regular.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 regular.c
> --- regular.c 6 Feb 2015 23:21:59 - 1.12
> +++ regular.c 9 Jan 2021 06:53:20 -
> @@ -51,15 +51,15 @@ c_regular(int fd1, char *file1, off_t sk
> off_t byte, length, line;
> int dfound;
>
> - if (sflag && len1 != len2)
> - exit(1);
> -
> if (skip1 > len1)
> eofmsg(file1);
> len1 -= skip1;
> if (skip2 > len2)
> eofmsg(file2);
> len2 -= skip2;
> +
> + if (sflag && len1 != len2)
> + exit(1);
>
> length = MINIMUM(len1, len2);
> if (length > SIZE_MAX) {
>
Re: New ujoy(4) device for USB gamecontrollers
On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote: > > I have heard from others who tried the diff that the PS4 controller is > > causing problems with the way it attaches. I ordered one to trial-and- > > error this myself at home. Could you share output of lsusb -vv? Thanks > > for giving it a try! > > Sure, here we go. > If I can find anything myself in the meantime I let you know. So the problem doesn't seem to be in your new ujoy(4) code, but how the dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4 controller. I'm not much in to HID, but when I sync up the hid_is_collection() function with NetBSD, the PS4 controller attaches to ujoy(4) as expected, and also can be accessed by usbhidctl(1), while my other HID devices continue to work as before: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 4 ctls audio1 at uaudio0 uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 uhidev6: iclass 3/0, 180 report ids ujoy0 at uhidev6 reportid 1: input=63, output=0, feature=0 <-- uhid6 at uhidev6 reportid 2: input=0, output=0, feature=36 uhid7 at uhidev6 reportid 4: input=0, output=0, feature=36 uhid8 at uhidev6 reportid 5: input=0, output=31, feature=0 uhid9 at uhidev6 reportid 8: input=0, output=0, feature=3 uhid10 at uhidev6 reportid 16: input=0, output=0, feature=4 uhid11 at uhidev6 reportid 17: input=0, output=0, feature=2 uhid12 at uhidev6 reportid 18: input=0, output=0, feature=15 uhid13 at uhidev6 reportid 19: input=0, output=0, feature=22 uhid14 at uhidev6 reportid 20: input=0, output=0, feature=16 uhid15 at uhidev6 reportid 21: input=0, output=0, feature=44 uhid16 at uhidev6 reportid 128: input=0, output=0, feature=6 uhid17 at uhidev6 reportid 129: input=0, output=0, feature=6 uhid18 at uhidev6 reportid 130: input=0, output=0, feature=5 uhid19 at uhidev6 reportid 131: input=0, output=0, feature=1 uhid20 at uhidev6 reportid 132: input=0, output=0, feature=4 uhid21 at uhidev6 reportid 133: input=0, output=0, feature=6 uhid22 at uhidev6 reportid 134: input=0, output=0, feature=6 uhid23 at uhidev6 reportid 135: input=0, output=0, feature=35 uhid24 at uhidev6 reportid 136: input=0, output=0, feature=34 uhid25 at uhidev6 reportid 137: input=0, output=0, feature=2 uhid26 at uhidev6 reportid 144: input=0, output=0, feature=5 uhid27 at uhidev6 reportid 145: input=0, output=0, feature=3 uhid28 at uhidev6 reportid 146: input=0, output=0, feature=3 uhid29 at uhidev6 reportid 147: input=0, output=0, feature=12 uhid30 at uhidev6 reportid 160: input=0, output=0, feature=6 uhid31 at uhidev6 reportid 161: input=0, output=0, feature=1 uhid32 at uhidev6 reportid 162: input=0, output=0, feature=1 uhid33 at uhidev6 reportid 163: input=0, output=0, feature=48 uhid34 at uhidev6 reportid 164: input=0, output=0, feature=13 uhid35 at uhidev6 reportid 165: input=0, output=0, feature=21 uhid36 at uhidev6 reportid 166: input=0, output=0, feature=21 uhid37 at uhidev6 reportid 167: input=0, output=0, feature=1 uhid38 at uhidev6 reportid 168: input=0, output=0, feature=1 uhid39 at uhidev6 reportid 169: input=0, output=0, feature=8 uhid40 at uhidev6 reportid 170: input=0, output=0, feature=1 uhid41 at uhidev6 reportid 171: input=0, output=0, feature=57 uhid42 at uhidev6 reportid 172: input=0, output=0, feature=57 uhid43 at uhidev6 reportid 173: input=0, output=0, feature=11 uhid44 at uhidev6 reportid 174: input=0, output=0, feature=1 uhid45 at uhidev6 reportid 175: input=0, output=0, feature=2 uhid46 at uhidev6 reportid 176: input=0, output=0, feature=63 uhid47 at uhidev6 reportid 177: input=0, output=0, feature=2 uhid48 at uhidev6 reportid 178: input=0, output=0, feature=2 uhid49 at uhidev6 reportid 179: input=0, output=0, feature=63 uhid50 at uhidev6 reportid 180: input=0, output=0, feature=63 $ usbhidctl -f /dev/ujoy/0 -l Game_Pad.X=126 Game_Pad.Y=126 Game_Pad.Z=125 Game_Pad.Rz=129 Game_Pad.Hat_Switch=8 Game_Pad.Button_1=0 Game_Pad.Button_2=0 Game_Pad.Button_3=0 Game_Pad.Button_4=0 Game_Pad.Button_5=0 Game_Pad.Button_6=0 Game_Pad.Button_7=0 Game_Pad.Button_8=0 Game_Pad.Button_9=0 Game_Pad.Button_10=0 Game_Pad.Button_11=0 Game_Pad.Button_12=0 Game_Pad.Button_13=0 Game_Pad.Button_14=0 Game_Pad.0x0020=28 Game_Pad.Rx=0 Game_Pad.Ry=0 [...] If others want to regression test this diff, and somebody wants to OK it. Maybe I should send it out on a separate thread. Once this has been fixed I would continue to do some more ujoy(4) testing, and check about a possible import. We also should agree on who/how to fix the ports part. Index: dev/hid/hid.c
Re: rpki-client check IP and ASnum coverage only on ROAs
On Thu, Jan 07, 2021 at 04:11:47PM +, Job Snijders wrote: > On Fri, Jan 08, 2021 at 03:43:18PM +0100, Claudio Jeker wrote: > > rpki-client is currently very strict about the ip ranges and as ranges in > > certificates. If a child certificate has a uncovered range in its list it > > is considered invalid and is removed from the pool (with it all the ROA > > entries as well). > > > > Now rfc8360 relaxes this a bit and mentions that a ROA for 192.0.2.0/24 > > is valid if that prefix is covered in all certs in the chain. > > RFC 8360 makes a lot of sense Actually after closer inspection RFC 8360 only relaxes this for a new form of certs that include new types of certificate policy, ip address ranges and as number ranges types. So this diff is not correct and I probably need to work on proper RFC 8360 support (even though it seems no CA is using RFC 8360 ids right now). -- :wq Claudio
Re: bgpd simplify update path
On Fri, Jan 08, 2021 at 09:42:57PM +0100, Sebastian Benoit wrote:
> Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.01.07 19:34:23 +0100:
> > When bgpd generates an UPDATE to update or withdraw prefixes it does this
> > from rde_generate_updates() and then decends into up_generate_update().
> > Now there is up_test_update() that checks if a new prefix is actually OK
> > to be distributed. It checks things for route reflectors and the common
> > communities (NO_EXPORT, ...). There are a few more checks that are pure
> > peer config checks and those should be moved up to rde_generate_updates().
> >
> > Last but not least there is this bit about ORIGINATOR_ID which seems
> > sensible but on second thought I think it is actually wrong and an
> > extension on top of the RFC. Since I think this code currently has not the
> > right withdraw behaviour I decided it is the best to just remove it.
>
> I think it should not matter because the receiving router will do the same
> check (against its own id) and ignore the update:
>
> A router [that recognizes the ORIGINATOR_ID attribute] SHOULD
> ignore a route received with its BGP Identifier as the ORIGINATOR_ID.
> (RFC 4456)
>
> However your change is correct because the RFC does say that the receiver
> should make this descision. We do seem to correctly check that when
> receiving updates in rde_reflector().
What I wonder about is that because of return -1 the prefix is actually
not withdrawn from the neighbor. So an old prefix could get stuck on the
peer. This is why I think it is best to remove this.
> > This code simplifies the return of up_test_update() to a pure true / false
> > case and make up_generate_update() simpler. Also I think doing the peer
> > checks early on will improve performance.
>
> ok benno@
> one whitespace error below
Will fix those and then commit. Thanks
> >
> > Please review :)
> > --
> > :wq Claudio
> >
> > Index: rde.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> > retrieving revision 1.510
> > diff -u -p -r1.510 rde.c
> > --- rde.c 30 Dec 2020 07:29:56 - 1.510
> > +++ rde.c 7 Jan 2021 17:04:53 -
> > @@ -2814,7 +2814,8 @@ rde_send_kroute(struct rib *rib, struct
> > void
> > rde_generate_updates(struct rib *rib, struct prefix *new, struct prefix
> > *old)
> > {
> > - struct rde_peer *peer;
> > + struct rde_peer *peer;
> > + u_int8_t aid;
> >
> > /*
> > * If old is != NULL we know it was active and should be removed.
> > @@ -2824,6 +2825,11 @@ rde_generate_updates(struct rib *rib, st
> > if (old == NULL && new == NULL)
> > return;
> >
> > + if (new)
> > + aid = new->pt->aid;
> > + else
> > + aid = old->pt->aid;
> > +
> > LIST_FOREACH(peer, , peer_l) {
> > if (peer->conf.id == 0)
> > continue;
> > @@ -2831,6 +2837,14 @@ rde_generate_updates(struct rib *rib, st
> > continue;
> > if (peer->state != PEER_UP)
> > continue;
> > + /* check if peer actually supports the address family */
> > + if (peer->capa.mp[aid] == 0)
> > + continue;
> > + /* skip peers with special export types */
>
> spaces instead of tabs
>
>
> > + if (peer->conf.export_type == EXPORT_NONE ||
> > + peer->conf.export_type == EXPORT_DEFAULT_ROUTE)
> > + continue;
> > +
> > up_generate_updates(out_rules, peer, new, old);
> > }
> > }
> > Index: rde_update.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
> > retrieving revision 1.123
> > diff -u -p -r1.123 rde_update.c
> > --- rde_update.c24 Jan 2020 05:44:05 - 1.123
> > +++ rde_update.c7 Jan 2021 18:13:45 -
> > @@ -47,11 +47,9 @@ static struct community comm_no_expsubco
> > static int
> > up_test_update(struct rde_peer *peer, struct prefix *p)
> > {
> > - struct bgpd_addr addr;
> > struct rde_aspath *asp;
> > struct rde_community*comm;
> > struct rde_peer *prefp;
> > - struct attr *attr;
> >
> > if (p == NULL)
> > /* no prefix available */
> > @@ -70,10 +68,6 @@ up_test_update(struct rde_peer *peer, st
> > if (asp->flags & F_ATTR_LOOP)
> > fatalx("try to send out a looped path");
> >
> > - pt_getaddr(p->pt, );
> > - if (peer->capa.mp[addr.aid] == 0)
> > - return (-1);
> > -
> > if (!prefp->conf.ebgp && !peer->conf.ebgp) {
> > /*
> > * route reflector redistribution rules:
> > @@ -90,16 +84,6 @@ up_test_update(struct rde_peer *peer, st
> > return (0);
> > }
> >
> > - /* export type handling */
> > - if (peer->conf.export_type == EXPORT_NONE ||
> > -
