Re: expose host capabilities relative to usb isochronous xfers

[Artturi Alm]

On Wed, Jun 12, 2019 at 03:55:59PM +0200, Alexandre Ratchov wrote:
> Currenty USB device driver code has no way to obtain how many frames
> can be scheduled on the HC. If it attempts to schedule too many
> frames, usbd_transfer() fails or silently misbehaves.
> 
> For audio this is a big problem because the max frames count
> determines the block size which must be reported to upper layers
> before any transfer starts. This makes impossible to implement uaudio
> properly. Currently there's a temporary hack to workaround this, which
> limits the block size. Now that uaudio is in and works, it's time to
> start removing such hacks.
> 
> Similarly, driver code needs to know the minimum number of frames per
> transfer to get an interrupt (ie the completion callback). Indeed, if
> the transfer frame count is not rounded to it, we silently miss the
> interrupt, the completion call-back is not called and playback
> stutters.
> 
> The diff below adds a new usbd_bus_info() function which reports the
> maximum frames that can be scheduled and the minimum frames per
> transfer to get an interrupt.
> 
> [snip]
> Index: usbdi.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.100
> diff -u -p -u -p -r1.100 usbdi.c
> --- usbdi.c   18 Nov 2018 16:33:26 -  1.100
> +++ usbdi.c   12 Jun 2019 13:33:41 -
> @@ -275,6 +275,12 @@ usbd_close_pipe(struct usbd_pipe *pipe)
>   return (USBD_NORMAL_COMPLETION);
>  }
>  
> +void
> +usbd_bus_info(struct usbd_device *dev, struct usbd_bus_info *info)
> +{
> + dev->bus->methods->info(dev, info);
> +}
> +
>  usbd_status
>  usbd_transfer(struct usbd_xfer *xfer)
>  {
> [snip]

i think above is not good enough with only ehci, ohci, uhci and xhci
being taken care of in [snip]'ed parts of the diff unfortunately:]
just thinking about dwc2.

-Artturi

new env defaults for doas

[Ted Unangst]

This has come up a few times before. For background, the default rule for doas
is to copy a few environment settings from the user and omit the rest. This is
to prevent confusion, and also supposedly for security. However, some of the
alleged safe variables like PATH probably aren't that safe. And things like
USER are confusing? And why even bother with MAIL? The list is kinda adhoc and
mostly copied from what I understood sudo to do at the time, but I believe
sudo has changed defaults as well.

So here's a new model which I think is safer and more consistent.

1. Always add a DOAS_USER with the invoking user's name.

2. When doing keepenv, nothing changes, except addition of above.

3. When starting with a new environment, fill in USER and HOME and such with
the values of the target user, instead of copying from the invoking user. You
run a command as a user, you should have that user's environment.

4. DISPLAY and TERM are still copied, since they represent a description of
the actual environment in which we are running. (Not sure how useful display
is without xauth cookies, but not my problem.)

5. As before, anything can be overridden with setenv in the config.

This is a kinda breaking change, but probably in a good way.
For simple configurations, I expect nothing changes. For more complicated
setups, this new handling is probably closer to what the admin expects or
desires.


Index: doas.c
===
RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.76
diff -u -p -r1.76 doas.c
--- doas.c  12 Jun 2019 02:50:29 -  1.76
+++ doas.c  13 Jun 2019 02:11:07 -
@@ -421,6 +421,7 @@ main(int argc, char **argv)
errx(1, "no passwd entry for target");
 
if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
+   LOGIN_SETPATH |
LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
LOGIN_SETUSER) != 0)
errx(1, "failed to set user context for target");
@@ -439,8 +440,13 @@ main(int argc, char **argv)
syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
mypw->pw_name, cmdline, targpw->pw_name, cwd);
 
-   envp = prepenv(rule);
+   envp = prepenv(rule, mypw, targpw);
 
+   if (rule->cmd) {
+   /* do this again after setusercontext reset it */
+   if (setenv("PATH", safepath, 1) == -1)
+   err(1, "failed to set PATH '%s'", safepath);
+   }
execvpe(cmd, argv, envp);
 fail:
if (errno == ENOENT)
Index: doas.conf.5
===
RCS file: /home/cvs/src/usr.bin/doas/doas.conf.5,v
retrieving revision 1.35
diff -u -p -r1.35 doas.conf.5
--- doas.conf.5 7 Feb 2018 05:13:57 -   1.35
+++ doas.conf.5 13 Jun 2019 01:41:18 -
@@ -51,15 +51,9 @@ again for some time.
 .It Ic keepenv
 The user's environment is maintained.
 The default is to reset the environment, except for the variables
-.Ev DISPLAY ,
-.Ev HOME ,
-.Ev LOGNAME ,
-.Ev MAIL ,
-.Ev PATH ,
-.Ev TERM ,
-.Ev USER
+.Ev DISPLAY
 and
-.Ev USERNAME .
+.Ev TERM .
 .It Ic setenv { Oo Ar variable ... Oc Oo Ar variable=value ... Oc Ic }
 In addition to the variables mentioned above, keep the space-separated
 specified variables.
Index: doas.h
===
RCS file: /home/cvs/src/usr.bin/doas/doas.h,v
retrieving revision 1.13
diff -u -p -r1.13 doas.h
--- doas.h  6 Apr 2017 21:12:06 -   1.13
+++ doas.h  13 Jun 2019 01:10:29 -
@@ -29,7 +29,10 @@ extern struct rule **rules;
 extern int nrules;
 extern int parse_errors;
 
-char **prepenv(const struct rule *);
+struct passwd;
+
+char **prepenv(const struct rule *, const struct passwd *,
+const struct passwd *);
 
 #define PERMIT 1
 #define DENY   2
Index: env.c
===
RCS file: /home/cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.6
diff -u -p -r1.6 env.c
--- env.c   6 Apr 2017 21:12:06 -   1.6
+++ env.c   13 Jun 2019 02:12:57 -
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "doas.h"
 
@@ -38,6 +39,8 @@ struct env {
u_int count;
 };
 
+static void fillenv(struct env *env, const char **envlist);
+
 static int
 envcmp(struct envnode *a, struct envnode *b)
 {
@@ -68,8 +71,19 @@ freenode(struct envnode *node)
free(node);
 }
 
+static void
+addnode(struct env *env, const char *key, const char *value)
+{
+   struct envnode *node;
+
+   node = createnode(key, value);
+   RB_INSERT(envtree, >root, node);
+   env->count++;
+}
+
 static struct env *
-createenv(const struct rule *rule)
+createenv(const struct rule *rule, const struct passwd *mypw,
+const struct passwd *targpw)
 {
struct env *env;
u_int i;
@@ -80,6 +94,8 @@ createenv(const struct rule *rule)

Re: drm(4), multi-threaded task queues and barriers

[Mark Kettenis]

> From: "Sven M. Hallberg" 
> Date: Wed, 12 Jun 2019 23:18:24 +0200
> 
> Mark Kettenis on Tue, Jun 11 2019:
> > The drm(4) codebase really needs multi-threaded task queues [...]
> >
> > The diff also starts 4 threads for each workqueue that gets created by
> > the drm(4) layer.  The number 4 is a bit arbitrary but it is the
> > number of threads that Linux creates per CPU for a so-called "unbound"
> > workqueue which hopefully is enough to always make progress.
> > 
> > Please test.
> 
> Looks good and appears to work fine with two displays (one internal, one
> external). Will test with three at work tomorrow.
> 
> 
> > -   dev_priv->hotplug.poll_init_work.tq = systq;
> 
> Intentional?

Yes.  It removes a local modification that should no longer be necessary.

Unfortunately the diff doesn't work with amdgpu.  Some more thinking
needed...

> > Index: dev/pci/drm/drm_linux.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> > retrieving revision 1.38
> > diff -u -p -r1.38 drm_linux.c
> > --- dev/pci/drm/drm_linux.c 9 Jun 2019 12:58:30 -   1.38
> > +++ dev/pci/drm/drm_linux.c 11 Jun 2019 18:54:38 -
> > @@ -1399,15 +1399,15 @@ drm_linux_init(void)
> >  {
> > if (system_wq == NULL) {
> > system_wq = (struct workqueue_struct *)
> > -   taskq_create("drmwq", 1, IPL_HIGH, 0);
> > +   taskq_create("drmwq", 4, IPL_HIGH, 0);
> > }
> > if (system_unbound_wq == NULL) {
> > system_unbound_wq = (struct workqueue_struct *)
> > -   taskq_create("drmubwq", 1, IPL_HIGH, 0);
> > +   taskq_create("drmubwq", 4, IPL_HIGH, 0);
> > }
> > if (system_long_wq == NULL) {
> > system_long_wq = (struct workqueue_struct *)
> > -   taskq_create("drmlwq", 1, IPL_HIGH, 0);
> > +   taskq_create("drmlwq", 4, IPL_HIGH, 0);
> > }
> >  
> > if (taskletq == NULL)
> > Index: dev/pci/drm/i915/intel_hotplug.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_hotplug.c,v
> > retrieving revision 1.2
> > diff -u -p -r1.2 intel_hotplug.c
> > --- dev/pci/drm/i915/intel_hotplug.c14 Apr 2019 10:14:52 -  
> > 1.2
> > +++ dev/pci/drm/i915/intel_hotplug.c11 Jun 2019 18:54:38 -
> > @@ -619,7 +619,6 @@ void intel_hpd_init_work(struct drm_i915
> > INIT_WORK(_priv->hotplug.hotplug_work, i915_hotplug_work_func);
> > INIT_WORK(_priv->hotplug.dig_port_work, i915_digport_work_func);
> > INIT_WORK(_priv->hotplug.poll_init_work, i915_hpd_poll_init_work);
> > -   dev_priv->hotplug.poll_init_work.tq = systq;
> > INIT_DELAYED_WORK(_priv->hotplug.reenable_work,
> >   intel_hpd_irq_storm_reenable_work);
> >  }
> > Index: kern/kern_task.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_task.c,v
> > retrieving revision 1.25
> > diff -u -p -r1.25 kern_task.c
> > --- kern/kern_task.c28 Apr 2019 04:20:40 -  1.25
> > +++ kern/kern_task.c11 Jun 2019 18:54:39 -
> > @@ -43,6 +43,7 @@ struct taskq {
> > TQ_S_DESTROYED
> > }tq_state;
> > unsigned int tq_running;
> > +   unsigned int tq_barrier;
> > unsigned int tq_nthreads;
> > unsigned int tq_flags;
> > const char  *tq_name;
> > @@ -59,6 +60,7 @@ static const char taskq_sys_name[] = "sy
> >  struct taskq taskq_sys = {
> > TQ_S_CREATED,
> > 0,
> > +   0,
> > 1,
> > 0,
> > taskq_sys_name,
> > @@ -77,6 +79,7 @@ static const char taskq_sys_mp_name[] = 
> >  struct taskq taskq_sys_mp = {
> > TQ_S_CREATED,
> > 0,
> > +   0,
> > 1,
> > TASKQ_MPSAFE,
> > taskq_sys_mp_name,
> > @@ -122,6 +125,7 @@ taskq_create(const char *name, unsigned 
> >  
> > tq->tq_state = TQ_S_CREATED;
> > tq->tq_running = 0;
> > +   tq->tq_barrier = 0;
> > tq->tq_nthreads = nthreads;
> > tq->tq_name = name;
> > tq->tq_flags = flags;
> > @@ -161,6 +165,7 @@ taskq_destroy(struct taskq *tq)
> > panic("unexpected %s tq state %u", tq->tq_name, tq->tq_state);
> > }
> >  
> > +   tq->tq_barrier = 0;
> > while (tq->tq_running > 0) {
> > wakeup(tq);
> > msleep(>tq_running, >tq_mtx, PWAIT, "tqdestroy", 0);
> > @@ -223,6 +228,7 @@ taskq_barrier(struct taskq *tq)
> >  
> > WITNESS_CHECKORDER(>tq_lock_object, LOP_NEWORDER, NULL);
> >  
> > +   SET(t.t_flags, TASK_BARRIER);
> > task_add(tq, );
> > cond_wait(, "tqbar");
> >  }
> > @@ -238,6 +244,7 @@ taskq_del_barrier(struct taskq *tq, stru
> > if (task_del(tq, del))
> > return;
> >  
> > +   SET(t.t_flags, TASK_BARRIER);
> > task_add(tq, );
> > cond_wait(, "tqbar");
> >  }
> > @@ -304,6 +311,7 @@ taskq_next_work(struct taskq *tq, struct
> 

Re: drm(4), multi-threaded task queues and barriers

[Sven M. Hallberg]

Mark Kettenis on Tue, Jun 11 2019:
> The drm(4) codebase really needs multi-threaded task queues [...]
>
> The diff also starts 4 threads for each workqueue that gets created by
> the drm(4) layer.  The number 4 is a bit arbitrary but it is the
> number of threads that Linux creates per CPU for a so-called "unbound"
> workqueue which hopefully is enough to always make progress.
> 
> Please test.

Looks good and appears to work fine with two displays (one internal, one
external). Will test with three at work tomorrow.


> - dev_priv->hotplug.poll_init_work.tq = systq;

Intentional?


-p


> Index: dev/pci/drm/drm_linux.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 drm_linux.c
> --- dev/pci/drm/drm_linux.c   9 Jun 2019 12:58:30 -   1.38
> +++ dev/pci/drm/drm_linux.c   11 Jun 2019 18:54:38 -
> @@ -1399,15 +1399,15 @@ drm_linux_init(void)
>  {
>   if (system_wq == NULL) {
>   system_wq = (struct workqueue_struct *)
> - taskq_create("drmwq", 1, IPL_HIGH, 0);
> + taskq_create("drmwq", 4, IPL_HIGH, 0);
>   }
>   if (system_unbound_wq == NULL) {
>   system_unbound_wq = (struct workqueue_struct *)
> - taskq_create("drmubwq", 1, IPL_HIGH, 0);
> + taskq_create("drmubwq", 4, IPL_HIGH, 0);
>   }
>   if (system_long_wq == NULL) {
>   system_long_wq = (struct workqueue_struct *)
> - taskq_create("drmlwq", 1, IPL_HIGH, 0);
> + taskq_create("drmlwq", 4, IPL_HIGH, 0);
>   }
>  
>   if (taskletq == NULL)
> Index: dev/pci/drm/i915/intel_hotplug.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_hotplug.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 intel_hotplug.c
> --- dev/pci/drm/i915/intel_hotplug.c  14 Apr 2019 10:14:52 -  1.2
> +++ dev/pci/drm/i915/intel_hotplug.c  11 Jun 2019 18:54:38 -
> @@ -619,7 +619,6 @@ void intel_hpd_init_work(struct drm_i915
>   INIT_WORK(_priv->hotplug.hotplug_work, i915_hotplug_work_func);
>   INIT_WORK(_priv->hotplug.dig_port_work, i915_digport_work_func);
>   INIT_WORK(_priv->hotplug.poll_init_work, i915_hpd_poll_init_work);
> - dev_priv->hotplug.poll_init_work.tq = systq;
>   INIT_DELAYED_WORK(_priv->hotplug.reenable_work,
> intel_hpd_irq_storm_reenable_work);
>  }
> Index: kern/kern_task.c
> ===
> RCS file: /cvs/src/sys/kern/kern_task.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 kern_task.c
> --- kern/kern_task.c  28 Apr 2019 04:20:40 -  1.25
> +++ kern/kern_task.c  11 Jun 2019 18:54:39 -
> @@ -43,6 +43,7 @@ struct taskq {
>   TQ_S_DESTROYED
>   }tq_state;
>   unsigned int tq_running;
> + unsigned int tq_barrier;
>   unsigned int tq_nthreads;
>   unsigned int tq_flags;
>   const char  *tq_name;
> @@ -59,6 +60,7 @@ static const char taskq_sys_name[] = "sy
>  struct taskq taskq_sys = {
>   TQ_S_CREATED,
>   0,
> + 0,
>   1,
>   0,
>   taskq_sys_name,
> @@ -77,6 +79,7 @@ static const char taskq_sys_mp_name[] = 
>  struct taskq taskq_sys_mp = {
>   TQ_S_CREATED,
>   0,
> + 0,
>   1,
>   TASKQ_MPSAFE,
>   taskq_sys_mp_name,
> @@ -122,6 +125,7 @@ taskq_create(const char *name, unsigned 
>  
>   tq->tq_state = TQ_S_CREATED;
>   tq->tq_running = 0;
> + tq->tq_barrier = 0;
>   tq->tq_nthreads = nthreads;
>   tq->tq_name = name;
>   tq->tq_flags = flags;
> @@ -161,6 +165,7 @@ taskq_destroy(struct taskq *tq)
>   panic("unexpected %s tq state %u", tq->tq_name, tq->tq_state);
>   }
>  
> + tq->tq_barrier = 0;
>   while (tq->tq_running > 0) {
>   wakeup(tq);
>   msleep(>tq_running, >tq_mtx, PWAIT, "tqdestroy", 0);
> @@ -223,6 +228,7 @@ taskq_barrier(struct taskq *tq)
>  
>   WITNESS_CHECKORDER(>tq_lock_object, LOP_NEWORDER, NULL);
>  
> + SET(t.t_flags, TASK_BARRIER);
>   task_add(tq, );
>   cond_wait(, "tqbar");
>  }
> @@ -238,6 +244,7 @@ taskq_del_barrier(struct taskq *tq, stru
>   if (task_del(tq, del))
>   return;
>  
> + SET(t.t_flags, TASK_BARRIER);
>   task_add(tq, );
>   cond_wait(, "tqbar");
>  }
> @@ -304,6 +311,7 @@ taskq_next_work(struct taskq *tq, struct
>   struct task *next;
>  
>   mtx_enter(>tq_mtx);
> +retry:
>   while ((next = TAILQ_FIRST(>tq_worklist)) == NULL) {
>   if (tq->tq_state != TQ_S_RUNNING) {
>   mtx_leave(>tq_mtx);
> @@ -311,6 +319,17 @@ taskq_next_work(struct taskq *tq, struct
>   }
>  
>   msleep(tq, >tq_mtx, PWAIT, "bored", 

Re: [patch] rsync: fix free() on uninitialized pointer with -rx and same device

[Hiltjo Posthuma]

On Thu, Jun 06, 2019 at 02:14:05PM +0200, Christian Weisgerber wrote:
> Björn Ketelaars:
> 
> > Diff below is based on the latest diff from naddy@. Changes:
> > - reallocarray likes type_t, as such changes type of nxdev and i;
> > - use reallocarray instead of malloc as xdev is initialised as NULL.
> 
> ok naddy@
> 
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 

Looks good to me.

I have 2 or 3 more patches locally. It would be nice if the above patch can get
committed so I can rebase and send a clean patch.

Thanks,

-- 
Kind regards,
Hiltjo

expose host capabilities relative to usb isochronous xfers

[Alexandre Ratchov]

Currenty USB device driver code has no way to obtain how many frames
can be scheduled on the HC. If it attempts to schedule too many
frames, usbd_transfer() fails or silently misbehaves.

For audio this is a big problem because the max frames count
determines the block size which must be reported to upper layers
before any transfer starts. This makes impossible to implement uaudio
properly. Currently there's a temporary hack to workaround this, which
limits the block size. Now that uaudio is in and works, it's time to
start removing such hacks.

Similarly, driver code needs to know the minimum number of frames per
transfer to get an interrupt (ie the completion callback). Indeed, if
the transfer frame count is not rounded to it, we silently miss the
interrupt, the completion call-back is not called and playback
stutters.

The diff below adds a new usbd_bus_info() function which reports the
maximum frames that can be scheduled and the minimum frames per
transfer to get an interrupt.

Index: ehci.c
===
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.204
diff -u -p -u -p -r1.204 ehci.c
--- ehci.c  31 Mar 2019 06:16:38 -  1.204
+++ ehci.c  12 Jun 2019 13:33:38 -
@@ -113,6 +113,7 @@ struct ehci_pipe {
 u_int8_t   ehci_reverse_bits(u_int8_t, int);
 
 usbd_statusehci_open(struct usbd_pipe *);
+void   ehci_info(struct usbd_device *, struct usbd_bus_info *);
 intehci_setaddr(struct usbd_device *, int);
 void   ehci_poll(struct usbd_bus *);
 void   ehci_softintr(void *);
@@ -225,6 +226,7 @@ struct usbd_bus_methods ehci_bus_methods
.do_poll = ehci_poll,
.allocx = ehci_allocx,
.freex = ehci_freex,
+   .info = ehci_info,
 };
 
 struct usbd_pipe_methods ehci_root_ctrl_methods = {
@@ -493,6 +498,21 @@ ehci_init(struct ehci_softc *sc)
return (err);
 }
 
+void
+ehci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+   /*
+* XXX: even if most hosts have 1024 frame lists, only 256
+* frame transfers work reliably. As soon as 256 is exceeded,
+* we start getting zeroed frames if multiple device are
+* running simultaneously. Set this to sc->sc_fl_size, once
+* ehci is fixed. Interrups occur every 1m, despite the
+* EHCI_CMD_ITC_2 setting.
+*/
+   info->nframes_max = 256;
+   info->intr_thres = (dev->speed >= USB_SPEED_HIGH) ? 8 : 1;
+}
+
 int
 ehci_intr(void *v)
 {
Index: ohci.c
===
RCS file: /cvs/src/sys/dev/usb/ohci.c,v
retrieving revision 1.156
diff -u -p -u -p -r1.156 ohci.c
--- ohci.c  11 Mar 2019 17:50:08 -  1.156
+++ ohci.c  12 Jun 2019 13:33:40 -
@@ -87,6 +87,7 @@ usbd_status   ohci_alloc_std_chain(struct 
struct ohci_soft_td **);
 
 usbd_statusohci_open(struct usbd_pipe *);
+void   ohci_info(struct usbd_device *, struct usbd_bus_info *);
 intohci_setaddr(struct usbd_device *, int);
 void   ohci_poll(struct usbd_bus *);
 void   ohci_softintr(void *);
@@ -236,6 +237,7 @@ struct usbd_bus_methods ohci_bus_methods
.do_poll = ohci_poll,
.allocx = ohci_allocx,
.freex = ohci_freex,
+   .info = ohci_info,
 };
 
 struct usbd_pipe_methods ohci_root_ctrl_methods = {
@@ -931,6 +933,13 @@ ohci_init(struct ohci_softc *sc)
  bad1:
usb_freemem(>sc_bus, >sc_hccadma);
return (err);
+}
+
+void
+ohci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+   info->nframes_max = OHCI_SITD_CHUNK;
+   info->intr_thres = 1;
 }
 
 struct usbd_xfer *
Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.144
diff -u -p -u -p -r1.144 uaudio.c
--- uaudio.c9 May 2019 07:09:04 -   1.144
+++ uaudio.c12 Jun 2019 13:33:41 -
@@ -382,6 +382,7 @@ struct uaudio_softc {
unsigned int ufps;  /* USB frames per second */
unsigned int sync_pktsz;/* size of sync packet */
unsigned int host_nframes;  /* max frames we can schedule */
+   unsigned int host_intr_thres;   /* min frames to get an interrupt */
 
int diff_nsamp; /* samples play is ahead of rec */
int diff_nframes;   /* frames play is ahead of rec */
@@ -2820,12 +2821,15 @@ uaudio_stream_open(struct uaudio_softc *
s->data_pipe = NULL;
s->sync_pipe = NULL;
 
-   s->nframes_mask = 0;
-   i = a->fps;
-   while (i > 1000) {
-   s->nframes_mask = (s->nframes_mask << 1) | 1;
-   i >>= 1;
-   }
+   /*
+* Find the smallest transfer size (power of two), larger than
+* the poll interval and the interrupt threshold.
+*/
+   i = 0;
+   while (a->fps < 

Re: [patch] use acme-client to sign certificated with ecdsa keys

[Renaud Allard]

On 6/11/19 2:36 PM, Sebastian Benoit wrote:

Hi,

some feedback below.

Renaud: maybe wait for feedback from florian or gilles until
acting on my comments, sometimes sending diffs to fast creates more work ;)

/Benno
 


As suggested by benno@
removal of the global variable
removal of KEYTYPE which was not used and was a leftover of a former patch
define ECDSA_KEY to be more readable

Index: extern.h
===
RCS file: /cvs/src/usr.sbin/acme-client/extern.h,v
retrieving revision 1.13
diff -u -p -r1.13 extern.h
--- extern.h	12 Jun 2019 11:09:25 -	1.13
+++ extern.h	12 Jun 2019 12:27:03 -
@@ -207,7 +207,7 @@ int		 revokeproc(int, const char *, cons
 			int, int, const char *const *, size_t);
 int		 fileproc(int, const char *, const char *, const char *,
 			const char *);
-int		 keyproc(int, const char *, const char **, size_t);
+int		 keyproc(int, const char *, const char **, size_t, int);
 int		 netproc(int, int, int, int, int, int, int,
 			struct authority_c *, const char *const *,
 			size_t);
@@ -274,11 +274,6 @@ char		*json_fmt_signed(const char *, con
  * Should we print debugging messages?
  */
 int		 verbose;
-
-/*
- * Should we switch to ecdsa?
- */
-int		ecdsa;
 
 /*
  * What component is the process within (COMP__MAX for none)?
Index: keyproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/keyproc.c,v
retrieving revision 1.13
diff -u -p -r1.13 keyproc.c
--- keyproc.c	12 Jun 2019 11:09:25 -	1.13
+++ keyproc.c	12 Jun 2019 12:27:03 -
@@ -75,7 +75,8 @@ add_ext(STACK_OF(X509_EXTENSION) *sk, in
  */
 int
 keyproc(int netsock, const char *keyfile,
-const char **alts, size_t altsz)
+const char **alts, size_t altsz,
+const int keytype)
 {
 	char		*der64 = NULL, *der = NULL, *dercp;
 	char		*sans = NULL, *san = NULL;
@@ -117,7 +118,7 @@ keyproc(int netsock, const char *keyfile
 	}
 
 	if (newkey) {
-		if (ecdsa) {
+		if (keytype == ECDSA_KEY) {
 			if ((pkey = ec_key_create(f, keyfile)) == NULL)
 goto out;
 			dodbg("%s: generated ECDSA domain key", keyfile);
Index: main.c
===
RCS file: /cvs/src/usr.sbin/acme-client/main.c,v
retrieving revision 1.48
diff -u -p -r1.48 main.c
--- main.c	12 Jun 2019 11:09:25 -	1.48
+++ main.c	12 Jun 2019 12:27:04 -
@@ -49,7 +49,6 @@ main(int argc, char *argv[])
 	int		  popts = 0;
 	pid_t		  pids[COMP__MAX];
 	extern int	  verbose;
-	extern int	  ecdsa;
 	extern enum comp  proccomp;
 	size_t		  i, altsz, ne;
 
@@ -148,10 +147,6 @@ main(int argc, char *argv[])
 			errx(EXIT_FAILURE, "authority %s not found", auth);
 	}
 
-	if (domain->keytype == 1) {
-		ecdsa = 1;
-	}
-
 	acctkey = authority->account;
 
 	if ((chngdir = domain->challengedir) == NULL)
@@ -258,7 +253,8 @@ main(int argc, char *argv[])
 		close(file_fds[0]);
 		close(file_fds[1]);
 		c = keyproc(key_fds[0], domain->key,
-		(const char **)alts, altsz);
+		(const char **)alts, altsz,
+		domain->keytype);
 		exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
 	}
 
Index: parse.h
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.h,v
retrieving revision 1.11
diff -u -p -r1.11 parse.h
--- parse.h	12 Jun 2019 11:09:25 -	1.11
+++ parse.h	12 Jun 2019 12:27:04 -
@@ -21,6 +21,7 @@
 
 #define AUTH_MAXLEN	120	/* max length of an authority_c name */
 #define DOMAIN_MAXLEN	255	/* max len of a domain name (rfc2181) */
+#define ECDSA_KEY	1
 
 /*
  * XXX other size limits needed?
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
retrieving revision 1.35
diff -u -p -r1.35 parse.y
--- parse.y	12 Jun 2019 11:09:25 -	1.35
+++ parse.y	12 Jun 2019 12:27:04 -
@@ -100,7 +100,7 @@ typedef struct {
 %}
 
 %token	AUTHORITY URL API ACCOUNT
-%token	DOMAIN ALTERNATIVE NAMES CERT FULL CHAIN KEY SIGN WITH CHALLENGEDIR KEYTYPE
+%token	DOMAIN ALTERNATIVE NAMES CERT FULL CHAIN KEY SIGN WITH CHALLENGEDIR
 %token	YES NO
 %token	INCLUDE
 %token	ERROR
@@ -261,10 +261,9 @@ domain		: DOMAIN STRING {
 		;
 
 keytype		: RSA { 
-			domain->keytype = 0;
 		}
 		| ECDSA {
-			domain->keytype = 1;
+			domain->keytype = ECDSA_KEY;
 		}
 		| /* nothing */
 		;


smime.p7s
Description: S/MIME Cryptographic Signature

Re: ssh_config.5: refer consistently to HostName, not Hostname

[Jason McIntyre]

On Wed, Jun 12, 2019 at 12:55:01PM +0300, Lauri Tirkkonen wrote:
> On Tue, Jun 11 2019 14:40:03 +0100, Jason McIntyre wrote:
> > On Tue, Jun 11, 2019 at 03:11:03PM +0300, Lauri Tirkkonen wrote:
> > > Hi, trivial manpage diff.
> > > 
> > 
> > fixed, thanks.
> > jmc
> 
> I noticed the followup commit that changes the canonical option name to
> 'Hostname'. I also like that capitalization better, but your diff seems
> to have been a bit incomplete; in particular the option should be called
> oHostname in code also, and a couple manpage mentions of HostName still
> remain. X11UseLocalhost is used consistently in comparison.
> 

ouch. thanks for following this up. i've just committed your diff.
jmc

> diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c
> index fdcca0e25ed..fd43ee4cbb4 100644
> --- a/usr.bin/ssh/clientloop.c
> +++ b/usr.bin/ssh/clientloop.c
> @@ -121,7 +121,7 @@ extern int muxserver_sock; /* XXX use 
> mux_client_cleanup() instead */
>  
>  /*
>   * Name of the host we are connecting to.  This is the name given on the
> - * command line, or the HostName specified for the user-supplied name in a
> + * command line, or the Hostname specified for the user-supplied name in a
>   * configuration file.
>   */
>  extern char *host;
> diff --git a/usr.bin/ssh/readconf.c b/usr.bin/ssh/readconf.c
> index 27638663b78..5b9c97b7c8d 100644
> --- a/usr.bin/ssh/readconf.c
> +++ b/usr.bin/ssh/readconf.c
> @@ -71,7 +71,7 @@
>   User foo
>  
> Host fake.com
> - HostName another.host.name.real.org
> + Hostname another.host.name.real.org
>   User blaah
>   Port 34289
>   ForwardX11 no
> @@ -133,7 +133,7 @@ typedef enum {
>   oGatewayPorts, oExitOnForwardFailure,
>   oPasswordAuthentication, oRSAAuthentication,
>   oChallengeResponseAuthentication, oXAuthLocation,
> - oIdentityFile, oHostName, oPort, oCipher, oRemoteForward, oLocalForward,
> + oIdentityFile, oHostname, oPort, oCipher, oRemoteForward, oLocalForward,
>   oCertificateFile, oAddKeysToAgent, oIdentityAgent,
>   oUser, oEscapeChar, oRhostsRSAAuthentication, oProxyCommand,
>   oGlobalKnownHostsFile, oUserKnownHostsFile, oConnectionAttempts,
> @@ -225,7 +225,7 @@ static struct {
>   { "certificatefile", oCertificateFile },
>   { "addkeystoagent", oAddKeysToAgent },
>   { "identityagent", oIdentityAgent },
> - { "hostname", oHostName },
> + { "hostname", oHostname },
>   { "hostkeyalias", oHostKeyAlias },
>   { "proxycommand", oProxyCommand },
>   { "port", oPort },
> @@ -1102,7 +1102,7 @@ parse_char_array:
>   max_entries = SSH_MAX_HOSTS_FILES;
>   goto parse_char_array;
>  
> - case oHostName:
> + case oHostname:
>   charptr = >hostname;
>   goto parse_string;
>  
> @@ -2576,7 +2576,7 @@ dump_client_config(Options *o, const char *host)
>  
>   /* Most interesting options first: user, host, port */
>   dump_cfg_string(oUser, o->user);
> - dump_cfg_string(oHostName, host);
> + dump_cfg_string(oHostname, host);
>   dump_cfg_int(oPort, o->port);
>  
>   /* Flag options */
> diff --git a/usr.bin/ssh/scp.1 b/usr.bin/ssh/scp.1
> index a2833dab041..29e97808ed1 100644
> --- a/usr.bin/ssh/scp.1
> +++ b/usr.bin/ssh/scp.1
> @@ -164,7 +164,7 @@ For full details of the options listed below, and their 
> possible values, see
>  .It HostbasedKeyTypes
>  .It HostKeyAlgorithms
>  .It HostKeyAlias
> -.It HostName
> +.It Hostname
>  .It IdentitiesOnly
>  .It IdentityAgent
>  .It IdentityFile
> diff --git a/usr.bin/ssh/sftp.1 b/usr.bin/ssh/sftp.1
> index 259095885a7..a6757705941 100644
> --- a/usr.bin/ssh/sftp.1
> +++ b/usr.bin/ssh/sftp.1
> @@ -241,7 +241,7 @@ For full details of the options listed below, and their 
> possible values, see
>  .It HostbasedKeyTypes
>  .It HostKeyAlgorithms
>  .It HostKeyAlias
> -.It HostName
> +.It Hostname
>  .It IdentitiesOnly
>  .It IdentityAgent
>  .It IdentityFile
> diff --git a/usr.bin/ssh/ssh.1 b/usr.bin/ssh/ssh.1
> index 9480eba8d3e..4c2c101d765 100644
> --- a/usr.bin/ssh/ssh.1
> +++ b/usr.bin/ssh/ssh.1
> @@ -504,7 +504,7 @@ For full details of the options listed below, and their 
> possible values, see
>  .It HostbasedKeyTypes
>  .It HostKeyAlgorithms
>  .It HostKeyAlias
> -.It HostName
> +.It Hostname
>  .It IdentitiesOnly
>  .It IdentityAgent
>  .It IdentityFile
> diff --git a/usr.bin/ssh/ssh.c b/usr.bin/ssh/ssh.c
> index defa3c1c337..aeefbe4f91c 100644
> --- a/usr.bin/ssh/ssh.c
> +++ b/usr.bin/ssh/ssh.c
> @@ -146,7 +146,7 @@ char *config = NULL;
>  
>  /*
>   * Name of the host we are connecting to.  This is the name given on the
> - * command line, or the HostName specified for the user-supplied name in a
> + * command line, or the Hostname specified for the user-supplied name in a
>   * configuration file.
>   */
>  char *host;
> diff --git a/usr.bin/ssh/ssh_config.5 b/usr.bin/ssh/ssh_config.5
> index 7bb6cde7ae1..719288b03a5 100644
> --- 

net80211: more steady Tx rate with MiRa (please test)

[Stefan Sperling]

While working on Tx aggregation I noticed that TCP streams will start
stalling whenever MiRa decides to start sending frames at Tx rates
which the AP tends to fail to receive. This means we're dropping far
too many frames while trying to find an optimal Tx rate to use.

The problem can be observed with tcpbench falling into 0.0 Mbps
occasionally and taking lots of time to recover.
This problem disappeared when I hacked a fixed working Tx rate into
the driver and disabled MiRa.

So I took a closer look at MiRa again and measured percentage of good/bad
MCS used for transmission. The diff below eliminates a lot of frames being
sent at bad rates and results in more steady overall Tx performance while
testing my WIP Tx aggregation code.

I can my numbers by recording outgoing frame headers in pcap file while
running tcpbench: tcpdump -n -i iwn0 -y IEEE802_11_RADIO -w /tmp/iwn.pcap
Now I filter this pcap file by MCS index in wireshark, with an expression
such as 'radiotap.mcs.index == 7', and look at the 'Displayed:' percentage
in the bottom right corner of wireshark's window.

Since I am knee-deep in Tx aggregation right now, I would like to delegate
testing of the diff below against plain -current to the community.
If some of you could test the diff below and report back to me I would
appreciate it.
You don't need to get numbers from wireshark for this if you don't want to.
Letting me know if Tx is faster or not and whether there are any perceived
regressions is sufficient.

The drivers affected by this change are athn(4), iwn(4), and iwm(4).
Don't bother testing with any other drivers.
I have tested this diff with iwn(4) only so far.

diff refs/heads/master refs/heads/txagg
blob - 0d14daff4d1c8973b1d1180a9885a25c6754940b
blob + 8a0f3b90198e9707ae08ef53aa4e68706ce2fa15
--- sys/net80211/ieee80211_mira.c
+++ sys/net80211/ieee80211_mira.c
@@ -510,6 +510,12 @@ ieee80211_mira_reset_driver_stats(struct ieee80211_mir
 /* Number of bytes which, alternatively, render a probe valid. */
 #define IEEE80211_MIRA_MIN_PROBE_BYTES IEEE80211_MAX_LEN
 
+/* Number of Tx failures which, alternatively, render a probe valid. */
+#define IEEE80211_MIRA_MAX_PROBE_TXFAIL 1
+
+/* Number of Tx retries which, alternatively, render a probe valid. */
+#define IEEE80211_MIRA_MAX_PROBE_RETRIES 4
+
 int
 ieee80211_mira_next_lower_intra_rate(struct ieee80211_mira_node *mn,
 struct ieee80211_node *ni)
@@ -719,7 +725,9 @@ ieee80211_mira_probe_valid(struct ieee80211_mira_node 
struct ieee80211_mira_goodput_stats *g = >g[ni->ni_txmcs];
 
return (g->nprobes >= IEEE80211_MIRA_MIN_PROBE_FRAMES ||
-   g->nprobe_bytes >= IEEE80211_MIRA_MIN_PROBE_BYTES);
+   g->nprobe_bytes >= IEEE80211_MIRA_MIN_PROBE_BYTES ||
+   mn->txfail >= IEEE80211_MIRA_MAX_PROBE_TXFAIL ||
+   mn->retries >= IEEE80211_MIRA_MAX_PROBE_RETRIES);
 }
 
 void

Re: ssh_config.5: refer consistently to HostName, not Hostname

[Lauri Tirkkonen]

On Tue, Jun 11 2019 14:40:03 +0100, Jason McIntyre wrote:
> On Tue, Jun 11, 2019 at 03:11:03PM +0300, Lauri Tirkkonen wrote:
> > Hi, trivial manpage diff.
> > 
> 
> fixed, thanks.
> jmc

I noticed the followup commit that changes the canonical option name to
'Hostname'. I also like that capitalization better, but your diff seems
to have been a bit incomplete; in particular the option should be called
oHostname in code also, and a couple manpage mentions of HostName still
remain. X11UseLocalhost is used consistently in comparison.

diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c
index fdcca0e25ed..fd43ee4cbb4 100644
--- a/usr.bin/ssh/clientloop.c
+++ b/usr.bin/ssh/clientloop.c
@@ -121,7 +121,7 @@ extern int muxserver_sock; /* XXX use mux_client_cleanup() 
instead */
 
 /*
  * Name of the host we are connecting to.  This is the name given on the
- * command line, or the HostName specified for the user-supplied name in a
+ * command line, or the Hostname specified for the user-supplied name in a
  * configuration file.
  */
 extern char *host;
diff --git a/usr.bin/ssh/readconf.c b/usr.bin/ssh/readconf.c
index 27638663b78..5b9c97b7c8d 100644
--- a/usr.bin/ssh/readconf.c
+++ b/usr.bin/ssh/readconf.c
@@ -71,7 +71,7 @@
  User foo
 
Host fake.com
- HostName another.host.name.real.org
+ Hostname another.host.name.real.org
  User blaah
  Port 34289
  ForwardX11 no
@@ -133,7 +133,7 @@ typedef enum {
oGatewayPorts, oExitOnForwardFailure,
oPasswordAuthentication, oRSAAuthentication,
oChallengeResponseAuthentication, oXAuthLocation,
-   oIdentityFile, oHostName, oPort, oCipher, oRemoteForward, oLocalForward,
+   oIdentityFile, oHostname, oPort, oCipher, oRemoteForward, oLocalForward,
oCertificateFile, oAddKeysToAgent, oIdentityAgent,
oUser, oEscapeChar, oRhostsRSAAuthentication, oProxyCommand,
oGlobalKnownHostsFile, oUserKnownHostsFile, oConnectionAttempts,
@@ -225,7 +225,7 @@ static struct {
{ "certificatefile", oCertificateFile },
{ "addkeystoagent", oAddKeysToAgent },
{ "identityagent", oIdentityAgent },
-   { "hostname", oHostName },
+   { "hostname", oHostname },
{ "hostkeyalias", oHostKeyAlias },
{ "proxycommand", oProxyCommand },
{ "port", oPort },
@@ -1102,7 +1102,7 @@ parse_char_array:
max_entries = SSH_MAX_HOSTS_FILES;
goto parse_char_array;
 
-   case oHostName:
+   case oHostname:
charptr = >hostname;
goto parse_string;
 
@@ -2576,7 +2576,7 @@ dump_client_config(Options *o, const char *host)
 
/* Most interesting options first: user, host, port */
dump_cfg_string(oUser, o->user);
-   dump_cfg_string(oHostName, host);
+   dump_cfg_string(oHostname, host);
dump_cfg_int(oPort, o->port);
 
/* Flag options */
diff --git a/usr.bin/ssh/scp.1 b/usr.bin/ssh/scp.1
index a2833dab041..29e97808ed1 100644
--- a/usr.bin/ssh/scp.1
+++ b/usr.bin/ssh/scp.1
@@ -164,7 +164,7 @@ For full details of the options listed below, and their 
possible values, see
 .It HostbasedKeyTypes
 .It HostKeyAlgorithms
 .It HostKeyAlias
-.It HostName
+.It Hostname
 .It IdentitiesOnly
 .It IdentityAgent
 .It IdentityFile
diff --git a/usr.bin/ssh/sftp.1 b/usr.bin/ssh/sftp.1
index 259095885a7..a6757705941 100644
--- a/usr.bin/ssh/sftp.1
+++ b/usr.bin/ssh/sftp.1
@@ -241,7 +241,7 @@ For full details of the options listed below, and their 
possible values, see
 .It HostbasedKeyTypes
 .It HostKeyAlgorithms
 .It HostKeyAlias
-.It HostName
+.It Hostname
 .It IdentitiesOnly
 .It IdentityAgent
 .It IdentityFile
diff --git a/usr.bin/ssh/ssh.1 b/usr.bin/ssh/ssh.1
index 9480eba8d3e..4c2c101d765 100644
--- a/usr.bin/ssh/ssh.1
+++ b/usr.bin/ssh/ssh.1
@@ -504,7 +504,7 @@ For full details of the options listed below, and their 
possible values, see
 .It HostbasedKeyTypes
 .It HostKeyAlgorithms
 .It HostKeyAlias
-.It HostName
+.It Hostname
 .It IdentitiesOnly
 .It IdentityAgent
 .It IdentityFile
diff --git a/usr.bin/ssh/ssh.c b/usr.bin/ssh/ssh.c
index defa3c1c337..aeefbe4f91c 100644
--- a/usr.bin/ssh/ssh.c
+++ b/usr.bin/ssh/ssh.c
@@ -146,7 +146,7 @@ char *config = NULL;
 
 /*
  * Name of the host we are connecting to.  This is the name given on the
- * command line, or the HostName specified for the user-supplied name in a
+ * command line, or the Hostname specified for the user-supplied name in a
  * configuration file.
  */
 char *host;
diff --git a/usr.bin/ssh/ssh_config.5 b/usr.bin/ssh/ssh_config.5
index 7bb6cde7ae1..719288b03a5 100644
--- a/usr.bin/ssh/ssh_config.5
+++ b/usr.bin/ssh/ssh_config.5
@@ -1223,8 +1223,8 @@ server running on some machine, or execute
 .Ic sshd -i
 somewhere.
 Host key management will be done using the
-HostName of the host being connected (defaulting to the name typed by
-the user).
+.Cm Hostname
+of the host being connected (defaulting to the name typed by the user).
 

Re: [patch] use acme-client to sign certificated with ecdsa keys

[Gilles Chehade]

On Wed, Jun 12, 2019 at 08:12:04AM +0200, Florian Obser wrote:
> 
> I had a look yesterday and it looks mostly OK.
> Something came up and I won't be around the next days.
> 
> Someone can put it and and we can tweak it in tree or we wait a few
> days.
> 

okie dokie, will commit tonight when I get home unless someone
beats me to it :-)


> On Tue, Jun 11, 2019 at 01:37:24PM +0200, Renaud Allard wrote:
> > 
> > 
> > On 6/11/19 10:17 AM, Renaud Allard wrote:
> > > 
> > > Hello,
> > > 
> > > Here is a patch with ecdsa and rsa in %token after the domain key name
> > > 
> > > OK? comments?
> > 
> > I just made a small modification in the formatting of acme.conf man page,
> > putting keytype as an arg. And also a cleaner key.h
> > 
> > OK?
> 
> > Index: Makefile
> > ===
> > RCS file: /cvs/src/usr.sbin/acme-client/Makefile,v
> > retrieving revision 1.8
> > diff -u -p -r1.8 Makefile
> > --- Makefile3 Jul 2017 22:21:47 -   1.8
> > +++ Makefile11 Jun 2019 11:35:24 -
> > @@ -2,7 +2,7 @@
> >  PROG=  acme-client
> >  SRCS=  acctproc.c base64.c certproc.c chngproc.c dbg.c 
> > dnsproc.c
> >  SRCS+= fileproc.c http.c jsmn.c json.c keyproc.c main.c 
> > netproc.c
> > -SRCS+= parse.y revokeproc.c rsa.c util.c
> > +SRCS+= parse.y revokeproc.c key.c util.c
> >  
> >  MAN=   acme-client.1 acme-client.conf.5
> >  
> > Index: acctproc.c
> > ===
> > RCS file: /cvs/src/usr.sbin/acme-client/acctproc.c,v
> > retrieving revision 1.14
> > diff -u -p -r1.14 acctproc.c
> > --- acctproc.c  8 Jun 2019 07:52:55 -   1.14
> > +++ acctproc.c  11 Jun 2019 11:35:24 -
> > @@ -29,7 +29,7 @@
> >  #include 
> >  
> >  #include "extern.h"
> > -#include "rsa.h"
> > +#include "key.h"
> >  
> >  /*
> >   * Converts a BIGNUM to the form used in JWK.
> > @@ -352,7 +352,9 @@ acctproc(int netsock, const char *acctke
> > goto out;
> > dodbg("%s: generated RSA account key", acctkey);
> > } else {
> > -   if ((pkey = rsa_key_load(f, acctkey)) == NULL)
> > +   if ((pkey = key_load(f, acctkey)) == NULL)
> > +   goto out;
> > +   if (EVP_PKEY_type(pkey->type) != EVP_PKEY_RSA) 
> > goto out;
> > doddbg("%s: loaded RSA account key", acctkey);
> > }
> > Index: acme-client.conf.5
> > ===
> > RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 acme-client.conf.5
> > --- acme-client.conf.5  8 Jan 2019 06:46:29 -   1.17
> > +++ acme-client.conf.5  11 Jun 2019 11:35:24 -
> > @@ -109,8 +109,10 @@ Specify a list of alternative names for 
> >  The common name is included automatically if this option is present,
> >  but there is no automatic conversion/inclusion between "www." and
> >  plain domain name forms.
> > -.It Ic domain key Ar file
> > +.It Ic domain key Ar file Op Ar keytype
> >  The private key file for which the certificate will be obtained.
> > +.Ar keytype
> > +can be rsa or ecdsa. Defaults to rsa.
> >  .It Ic domain certificate Ar file
> >  The filename of the certificate that will be issued.
> >  This is optional if
> > Index: extern.h
> > ===
> > RCS file: /cvs/src/usr.sbin/acme-client/extern.h,v
> > retrieving revision 1.12
> > diff -u -p -r1.12 extern.h
> > --- extern.h8 Jun 2019 07:52:55 -   1.12
> > +++ extern.h11 Jun 2019 11:35:24 -
> > @@ -276,6 +276,11 @@ char   *json_fmt_signed(const char *, con
> >  int verbose;
> >  
> >  /*
> > + * Should we switch to ecdsa?
> > + */
> > +intecdsa;
> > +
> > +/*
> >   * What component is the process within (COMP__MAX for none)?
> >   */
> >  enum comp   proccomp;
> > Index: key.c
> > ===
> > RCS file: key.c
> > diff -N key.c
> > --- /dev/null   1 Jan 1970 00:00:00 -
> > +++ key.c   11 Jun 2019 11:35:24 -
> > @@ -0,0 +1,149 @@
> > +/* $Id: rsa.c,v 1.7 2018/07/28 15:25:23 tb Exp $ */
> > +/*
> > + * Copyright (c) 2019 Renaud Allard 
> > + * Copyright (c) 2016 Kristaps Dzonsons 
> > + *
> > + * Permission to use, copy, modify, and distribute this software for any
> > + * purpose with or without fee is hereby granted, provided that the above
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES
> > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR
> > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES 

Re: drm(4), multi-threaded task queues and barriers

[Mark Kettenis]

> Date: Wed, 12 Jun 2019 17:04:10 +1000
> From: Jonathan Gray 
> 
> On Tue, Jun 11, 2019 at 09:10:46PM +0200, Mark Kettenis wrote:
> > The drm(4) codebase really needs multi-threaded task queues since the
> > code has taks that wait for the completion of other tasks that are
> > submitted to the same task queue.  Thank you Linux...
> > 
> > Unfortunately the code also needs to wait for the completion of
> > submitted tasks from other threads.  This implemented using
> > task_barrier(9).  But unfortunately our current implementation only
> > works for single-threaded task queues.
> > 
> > The diff below fixes this by marking the barrier task with a flag and
> > making sure that all threads of the task queue are syncchronized.
> > This achived through a TASK_BARRIER flag that simply blocks the
> > threads until the last unblocked thread sees the flag and executes the
> > task.
> > 
> > The diff also starts 4 threads for each workqueue that gets created by
> > the drm(4) layer.  The number 4 is a bit arbitrary but it is the
> > number of threads that Linux creates per CPU for a so-called "unbound"
> > workqueue which hopefully is enough to always make progress.
> 
> Ignoring taskletq and systq use in burner_task/switchtask across
> drivers and the local only backlight_schedule_update_status(), was it
> intentional to exclude:
> 
> workqueue.h: alloc_workqueue()
>   struct taskq *tq = taskq_create(name, 1, IPL_TTY, 0);
> workqueue.h: alloc_ordered_workqueue()
>   struct taskq *tq = taskq_create(name, 1, IPL_TTY, 0);
> workqueue.h:
>   #define system_power_efficient_wq ((struct workqueue_struct *)systq)

Not entirely.  But I don't want to spawn too many threads...  If this
diff seems to work, we might want to tweak the numbers a bit to see
what works best.  Whatever we do, the ordered workqueues need to stay
single-threaded.

> > Please test.  If you experience a "hang" with this diff, please try to
> > log in to the machine remotely over ssh and send me and jsg@ the
> > output of "ps -AHwlk".
> > 
> > Thanks,
> > 
> > Mark
> > 
> > 
> > Index: dev/pci/drm/drm_linux.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> > retrieving revision 1.38
> > diff -u -p -r1.38 drm_linux.c
> > --- dev/pci/drm/drm_linux.c 9 Jun 2019 12:58:30 -   1.38
> > +++ dev/pci/drm/drm_linux.c 11 Jun 2019 18:54:38 -
> > @@ -1399,15 +1399,15 @@ drm_linux_init(void)
> >  {
> > if (system_wq == NULL) {
> > system_wq = (struct workqueue_struct *)
> > -   taskq_create("drmwq", 1, IPL_HIGH, 0);
> > +   taskq_create("drmwq", 4, IPL_HIGH, 0);
> > }
> > if (system_unbound_wq == NULL) {
> > system_unbound_wq = (struct workqueue_struct *)
> > -   taskq_create("drmubwq", 1, IPL_HIGH, 0);
> > +   taskq_create("drmubwq", 4, IPL_HIGH, 0);
> > }
> > if (system_long_wq == NULL) {
> > system_long_wq = (struct workqueue_struct *)
> > -   taskq_create("drmlwq", 1, IPL_HIGH, 0);
> > +   taskq_create("drmlwq", 4, IPL_HIGH, 0);
> > }
> >  
> > if (taskletq == NULL)
> > Index: dev/pci/drm/i915/intel_hotplug.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_hotplug.c,v
> > retrieving revision 1.2
> > diff -u -p -r1.2 intel_hotplug.c
> > --- dev/pci/drm/i915/intel_hotplug.c14 Apr 2019 10:14:52 -  
> > 1.2
> > +++ dev/pci/drm/i915/intel_hotplug.c11 Jun 2019 18:54:38 -
> > @@ -619,7 +619,6 @@ void intel_hpd_init_work(struct drm_i915
> > INIT_WORK(_priv->hotplug.hotplug_work, i915_hotplug_work_func);
> > INIT_WORK(_priv->hotplug.dig_port_work, i915_digport_work_func);
> > INIT_WORK(_priv->hotplug.poll_init_work, i915_hpd_poll_init_work);
> > -   dev_priv->hotplug.poll_init_work.tq = systq;
> > INIT_DELAYED_WORK(_priv->hotplug.reenable_work,
> >   intel_hpd_irq_storm_reenable_work);
> >  }
> > Index: kern/kern_task.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_task.c,v
> > retrieving revision 1.25
> > diff -u -p -r1.25 kern_task.c
> > --- kern/kern_task.c28 Apr 2019 04:20:40 -  1.25
> > +++ kern/kern_task.c11 Jun 2019 18:54:39 -
> > @@ -43,6 +43,7 @@ struct taskq {
> > TQ_S_DESTROYED
> > }tq_state;
> > unsigned int tq_running;
> > +   unsigned int tq_barrier;
> > unsigned int tq_nthreads;
> > unsigned int tq_flags;
> > const char  *tq_name;
> > @@ -59,6 +60,7 @@ static const char taskq_sys_name[] = "sy
> >  struct taskq taskq_sys = {
> > TQ_S_CREATED,
> > 0,
> > +   0,
> > 1,
> > 0,
> > taskq_sys_name,
> > @@ -77,6 +79,7 @@ static const char taskq_sys_mp_name[] = 
> >  

Re: drm(4), multi-threaded task queues and barriers

[Jonathan Gray]

On Tue, Jun 11, 2019 at 09:10:46PM +0200, Mark Kettenis wrote:
> The drm(4) codebase really needs multi-threaded task queues since the
> code has taks that wait for the completion of other tasks that are
> submitted to the same task queue.  Thank you Linux...
> 
> Unfortunately the code also needs to wait for the completion of
> submitted tasks from other threads.  This implemented using
> task_barrier(9).  But unfortunately our current implementation only
> works for single-threaded task queues.
> 
> The diff below fixes this by marking the barrier task with a flag and
> making sure that all threads of the task queue are syncchronized.
> This achived through a TASK_BARRIER flag that simply blocks the
> threads until the last unblocked thread sees the flag and executes the
> task.
> 
> The diff also starts 4 threads for each workqueue that gets created by
> the drm(4) layer.  The number 4 is a bit arbitrary but it is the
> number of threads that Linux creates per CPU for a so-called "unbound"
> workqueue which hopefully is enough to always make progress.

Ignoring taskletq and systq use in burner_task/switchtask across
drivers and the local only backlight_schedule_update_status(), was it
intentional to exclude:

workqueue.h: alloc_workqueue()
struct taskq *tq = taskq_create(name, 1, IPL_TTY, 0);
workqueue.h: alloc_ordered_workqueue()
struct taskq *tq = taskq_create(name, 1, IPL_TTY, 0);
workqueue.h:
#define system_power_efficient_wq ((struct workqueue_struct *)systq)

> 
> Please test.  If you experience a "hang" with this diff, please try to
> log in to the machine remotely over ssh and send me and jsg@ the
> output of "ps -AHwlk".
> 
> Thanks,
> 
> Mark
> 
> 
> Index: dev/pci/drm/drm_linux.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 drm_linux.c
> --- dev/pci/drm/drm_linux.c   9 Jun 2019 12:58:30 -   1.38
> +++ dev/pci/drm/drm_linux.c   11 Jun 2019 18:54:38 -
> @@ -1399,15 +1399,15 @@ drm_linux_init(void)
>  {
>   if (system_wq == NULL) {
>   system_wq = (struct workqueue_struct *)
> - taskq_create("drmwq", 1, IPL_HIGH, 0);
> + taskq_create("drmwq", 4, IPL_HIGH, 0);
>   }
>   if (system_unbound_wq == NULL) {
>   system_unbound_wq = (struct workqueue_struct *)
> - taskq_create("drmubwq", 1, IPL_HIGH, 0);
> + taskq_create("drmubwq", 4, IPL_HIGH, 0);
>   }
>   if (system_long_wq == NULL) {
>   system_long_wq = (struct workqueue_struct *)
> - taskq_create("drmlwq", 1, IPL_HIGH, 0);
> + taskq_create("drmlwq", 4, IPL_HIGH, 0);
>   }
>  
>   if (taskletq == NULL)
> Index: dev/pci/drm/i915/intel_hotplug.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_hotplug.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 intel_hotplug.c
> --- dev/pci/drm/i915/intel_hotplug.c  14 Apr 2019 10:14:52 -  1.2
> +++ dev/pci/drm/i915/intel_hotplug.c  11 Jun 2019 18:54:38 -
> @@ -619,7 +619,6 @@ void intel_hpd_init_work(struct drm_i915
>   INIT_WORK(_priv->hotplug.hotplug_work, i915_hotplug_work_func);
>   INIT_WORK(_priv->hotplug.dig_port_work, i915_digport_work_func);
>   INIT_WORK(_priv->hotplug.poll_init_work, i915_hpd_poll_init_work);
> - dev_priv->hotplug.poll_init_work.tq = systq;
>   INIT_DELAYED_WORK(_priv->hotplug.reenable_work,
> intel_hpd_irq_storm_reenable_work);
>  }
> Index: kern/kern_task.c
> ===
> RCS file: /cvs/src/sys/kern/kern_task.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 kern_task.c
> --- kern/kern_task.c  28 Apr 2019 04:20:40 -  1.25
> +++ kern/kern_task.c  11 Jun 2019 18:54:39 -
> @@ -43,6 +43,7 @@ struct taskq {
>   TQ_S_DESTROYED
>   }tq_state;
>   unsigned int tq_running;
> + unsigned int tq_barrier;
>   unsigned int tq_nthreads;
>   unsigned int tq_flags;
>   const char  *tq_name;
> @@ -59,6 +60,7 @@ static const char taskq_sys_name[] = "sy
>  struct taskq taskq_sys = {
>   TQ_S_CREATED,
>   0,
> + 0,
>   1,
>   0,
>   taskq_sys_name,
> @@ -77,6 +79,7 @@ static const char taskq_sys_mp_name[] = 
>  struct taskq taskq_sys_mp = {
>   TQ_S_CREATED,
>   0,
> + 0,
>   1,
>   TASKQ_MPSAFE,
>   taskq_sys_mp_name,
> @@ -122,6 +125,7 @@ taskq_create(const char *name, unsigned 
>  
>   tq->tq_state = TQ_S_CREATED;
>   tq->tq_running = 0;
> + tq->tq_barrier = 0;
>   tq->tq_nthreads = nthreads;
>   tq->tq_name = name;
>   tq->tq_flags = flags;
> @@ -161,6 +165,7 @@ taskq_destroy(struct taskq *tq)
>   

try interface rx backpressure again

[David Gwynne]

i backed this out before the 6.5 release because of bad interactions
with virtual interfaces like vlan and trunk. those should now be fixed,
so we can try the other backpressure mechanism again.

the summary is that we count the number of attempts to queue packets for
the system to process rather than the number of packets. this should
result in a more responsive system by better limiting the work a network
taskq does per interface.

this interacted badly with things like vlan because they work on a
packet at a time, however, they directly dispatch processing now so
there's no operations that they count and get backpressure for.

ok? even if it's to throw it in the tree to be backed out when we hit a
problem again.

Index: ifq.c
===
RCS file: /cvs/src/sys/net/ifq.c,v
retrieving revision 1.31
diff -u -p -r1.31 ifq.c
--- ifq.c   16 Apr 2019 04:04:19 -  1.31
+++ ifq.c   12 Jun 2019 01:19:51 -
@@ -498,8 +498,8 @@ ifiq_destroy(struct ifiqueue *ifiq)
ml_purge(>ifiq_ml);
 }
 
-unsigned int ifiq_maxlen_drop = 2048 * 5;
-unsigned int ifiq_maxlen_return = 2048 * 3;
+unsigned int ifiq_pressure_drop = 16;
+unsigned int ifiq_pressure_return = 2;
 
 int
 ifiq_input(struct ifiqueue *ifiq, struct mbuf_list *ml)
@@ -508,7 +508,7 @@ ifiq_input(struct ifiqueue *ifiq, struct
struct mbuf *m;
uint64_t packets;
uint64_t bytes = 0;
-   unsigned int len;
+   unsigned int pressure;
 #if NBPFILTER > 0
caddr_t if_bpf;
 #endif
@@ -552,8 +552,8 @@ ifiq_input(struct ifiqueue *ifiq, struct
ifiq->ifiq_packets += packets;
ifiq->ifiq_bytes += bytes;
 
-   len = ml_len(>ifiq_ml);
-   if (len > ifiq_maxlen_drop)
+   pressure = ++ifiq->ifiq_pressure;
+   if (pressure > ifiq_pressure_drop)
ifiq->ifiq_qdrops += ml_len(ml);
else
ml_enlist(>ifiq_ml, ml);
@@ -564,7 +564,7 @@ ifiq_input(struct ifiqueue *ifiq, struct
else
ml_purge(ml);
 
-   return (len > ifiq_maxlen_return);
+   return (pressure > ifiq_pressure_return);
 }
 
 void
@@ -599,6 +599,7 @@ ifiq_process(void *arg)
return;
 
mtx_enter(>ifiq_mtx);
+   ifiq->ifiq_pressure = 0;
ml = ifiq->ifiq_ml;
ml_init(>ifiq_ml);
mtx_leave(>ifiq_mtx);

Re: [patch] use acme-client to sign certificated with ecdsa keys

[Florian Obser]

I had a look yesterday and it looks mostly OK.
Something came up and I won't be around the next days.

Someone can put it and and we can tweak it in tree or we wait a few
days.

On Tue, Jun 11, 2019 at 01:37:24PM +0200, Renaud Allard wrote:
> 
> 
> On 6/11/19 10:17 AM, Renaud Allard wrote:
> > 
> > Hello,
> > 
> > Here is a patch with ecdsa and rsa in %token after the domain key name
> > 
> > OK? comments?
> 
> I just made a small modification in the formatting of acme.conf man page,
> putting keytype as an arg. And also a cleaner key.h
> 
> OK?

> Index: Makefile
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/Makefile,v
> retrieving revision 1.8
> diff -u -p -r1.8 Makefile
> --- Makefile  3 Jul 2017 22:21:47 -   1.8
> +++ Makefile  11 Jun 2019 11:35:24 -
> @@ -2,7 +2,7 @@
>  PROG=acme-client
>  SRCS=acctproc.c base64.c certproc.c chngproc.c dbg.c 
> dnsproc.c
>  SRCS+=   fileproc.c http.c jsmn.c json.c keyproc.c main.c 
> netproc.c
> -SRCS+=   parse.y revokeproc.c rsa.c util.c
> +SRCS+=   parse.y revokeproc.c key.c util.c
>  
>  MAN= acme-client.1 acme-client.conf.5
>  
> Index: acctproc.c
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/acctproc.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 acctproc.c
> --- acctproc.c8 Jun 2019 07:52:55 -   1.14
> +++ acctproc.c11 Jun 2019 11:35:24 -
> @@ -29,7 +29,7 @@
>  #include 
>  
>  #include "extern.h"
> -#include "rsa.h"
> +#include "key.h"
>  
>  /*
>   * Converts a BIGNUM to the form used in JWK.
> @@ -352,7 +352,9 @@ acctproc(int netsock, const char *acctke
>   goto out;
>   dodbg("%s: generated RSA account key", acctkey);
>   } else {
> - if ((pkey = rsa_key_load(f, acctkey)) == NULL)
> + if ((pkey = key_load(f, acctkey)) == NULL)
> + goto out;
> + if (EVP_PKEY_type(pkey->type) != EVP_PKEY_RSA) 
>   goto out;
>   doddbg("%s: loaded RSA account key", acctkey);
>   }
> Index: acme-client.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v
> retrieving revision 1.17
> diff -u -p -r1.17 acme-client.conf.5
> --- acme-client.conf.58 Jan 2019 06:46:29 -   1.17
> +++ acme-client.conf.511 Jun 2019 11:35:24 -
> @@ -109,8 +109,10 @@ Specify a list of alternative names for 
>  The common name is included automatically if this option is present,
>  but there is no automatic conversion/inclusion between "www." and
>  plain domain name forms.
> -.It Ic domain key Ar file
> +.It Ic domain key Ar file Op Ar keytype
>  The private key file for which the certificate will be obtained.
> +.Ar keytype
> +can be rsa or ecdsa. Defaults to rsa.
>  .It Ic domain certificate Ar file
>  The filename of the certificate that will be issued.
>  This is optional if
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/extern.h,v
> retrieving revision 1.12
> diff -u -p -r1.12 extern.h
> --- extern.h  8 Jun 2019 07:52:55 -   1.12
> +++ extern.h  11 Jun 2019 11:35:24 -
> @@ -276,6 +276,11 @@ char *json_fmt_signed(const char *, con
>  int   verbose;
>  
>  /*
> + * Should we switch to ecdsa?
> + */
> +int  ecdsa;
> +
> +/*
>   * What component is the process within (COMP__MAX for none)?
>   */
>  enum comp proccomp;
> Index: key.c
> ===
> RCS file: key.c
> diff -N key.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ key.c 11 Jun 2019 11:35:24 -
> @@ -0,0 +1,149 @@
> +/*   $Id: rsa.c,v 1.7 2018/07/28 15:25:23 tb Exp $ */
> +/*
> + * Copyright (c) 2019 Renaud Allard 
> + * Copyright (c) 2016 Kristaps Dzonsons 
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "key.h"
> +
>