monotonic clock going backwards?
Hello -
I noticed some negative roundtrip times when running traceroute, which
uses the monotonic clock to calculate the RTT.
If I run the following code, it eventually bombs. It bombs quick if I
launch Firefox.
timespeccmp failed
tp1 s:103780 n:63101042
tp2 s:103779 n:761117849
#include
#include
#include
#include
int
main(void)
{
int r;
struct timespec tp1, tp2, tout;
tout.tv_sec = 0;
tout.tv_nsec = 10;
for (;;) {
r = clock_gettime(CLOCK_MONOTONIC, );
if (r == -1) {
perror("clock_gettime");
exit(-1);
}
nanosleep(, NULL);
r = clock_gettime(CLOCK_MONOTONIC, );
if (r == -1) {
perror("clock_gettime");
exit(-1);
}
// tp1 should never be larger than tp2
r = timespeccmp(, , >);
if (r == 1) {
printf("timespeccmp failed\n");
printf("tp1 s:%lld n:%ld\n", tp1.tv_sec,
tp1.tv_nsec);
printf("tp2 s:%lld n:%ld\n", tp2.tv_sec,
tp2.tv_nsec);
exit(-1);
}
}
return 0;
}
Re: if_netisr(): trade NET_LOCK() for NET_RLOCK()
Hello, On Wed, Jun 05, 2019 at 10:50:18AM -0300, Martin Pieuchot wrote: > On 04/06/19(Tue) 23:57, Alexandr Nedvedicky wrote: > > Hello, > > > > I see, I owe some clarification to share wider context of this change. > > > > On Tue, Jun 04, 2019 at 10:32:57AM -0300, Martin Pieuchot wrote: > > > On 04/06/19(Tue) 01:37, Alexandr Nedvedicky wrote: > > > > Hello, > > > > > > > > diff below is just cosmetic change, which has no impact on current > > > > functionality, because there is just single network task to deliver > > > > packets to > > > > IP stack. I just want to push this small change to tree to minimize > > > > delta > > > > between current and my experimental branch. > > > > > > But why is it fine? > > > > currently all packets reaching IP stack are processed by if_netisr(), > > which > > runs as a task. The task pops up a packet from its queue (a.k.a. taskq) > > and > > passes the packet to corresponding xintr() handler for further > > processing. > > Only for packets that needs to be delivered/processed locally. In the > case of forwarding if_input_process() is the task that process packets. > > thanks for pointing that out. I keep forgetting this important detail. regards sashan
Re: [patch] rsync: fix free() on uninitialized pointer with -rx and same device
Christian Weisgerber:
> > To reproduce:
> >
> > mkdir -p /tmp/test /tmp/plop
> > openrsync -rx /tmp/test/ /tmp/plop/
> >
> > Result:
> >
> > openrsync(3470) in free(): bogus pointer (double free?) 0x7f7dcdc8
> > Abort trap (core dumped)
> >
> > The patch below fixes it and simplifies the logic:
>
> I agree. However, the (re)allocation of xdev also looks bogus.
>
> How about this?
>
> (Also, the realloc idiom is exactly the one the man page warns
> against. Do we care here?)
Well, let's not set a bad example.
Index: flist.c
===
RCS file: /cvs/src/usr.bin/rsync/flist.c,v
retrieving revision 1.27
diff -u -p -r1.27 flist.c
--- flist.c 2 Jun 2019 14:29:58 - 1.27
+++ flist.c 5 Jun 2019 22:36:28 -
@@ -808,7 +808,7 @@ flist_gen_dirent(struct sess *sess, char
FTSENT *ent;
struct flist*f;
size_t flsz = 0, stripdir;
- dev_t *xdev;
+ dev_t *newxdev, *xdev = NULL;
struct stat st;
cargv[0] = root;
@@ -931,7 +931,8 @@ flist_gen_dirent(struct sess *sess, char
!S_ISDIR(ent->fts_statp->st_mode))
continue;
- if ((xdev = malloc(sizeof(dev_t))) == NULL) {
+ if (xdev == NULL &&
+ (xdev = malloc(sizeof(dev_t))) == NULL) {
ERRX1("malloc");
goto out;
}
@@ -945,12 +946,14 @@ flist_gen_dirent(struct sess *sess, char
if (flag)
continue;
- if (nxdev)
- if ((xdev = realloc(xdev, sizeof(dev_t))) ==
- NULL) {
+ if (nxdev) {
+ if ((newxdev = reallocarray(xdev, nxdev + 1,
+ sizeof(dev_t))) == NULL) {
ERRX1("realloc");
goto out;
}
+ xdev = newxdev;
+ }
xdev[nxdev] = ent->fts_statp->st_dev;
nxdev++;
}
@@ -1008,8 +1011,7 @@ flist_gen_dirent(struct sess *sess, char
rc = 1;
out:
fts_close(fts);
- if (sess->opts->one_file_system)
- free(xdev);
+ free(xdev);
return rc;
}
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: [patch] rsync: fix free() on uninitialized pointer with -rx and same device
Hiltjo Posthuma:
> I noticed a free() issue on an uninitialized pointer on a certain condition.
>
> To reproduce:
>
> mkdir -p /tmp/test /tmp/plop
> openrsync -rx /tmp/test/ /tmp/plop/
>
> Result:
>
> openrsync(3470) in free(): bogus pointer (double free?) 0x7f7dcdc8
> Abort trap (core dumped)
>
> The patch below fixes it and simplifies the logic:
I agree. However, the (re)allocation of xdev also looks bogus.
How about this?
(Also, the realloc idiom is exactly the one the man page warns
against. Do we care here?)
Index: flist.c
===
RCS file: /cvs/src/usr.bin/rsync/flist.c,v
retrieving revision 1.27
diff -u -p -r1.27 flist.c
--- flist.c 2 Jun 2019 14:29:58 - 1.27
+++ flist.c 5 Jun 2019 22:15:10 -
@@ -808,7 +808,7 @@ flist_gen_dirent(struct sess *sess, char
FTSENT *ent;
struct flist*f;
size_t flsz = 0, stripdir;
- dev_t *xdev;
+ dev_t *xdev = NULL;
struct stat st;
cargv[0] = root;
@@ -931,7 +931,8 @@ flist_gen_dirent(struct sess *sess, char
!S_ISDIR(ent->fts_statp->st_mode))
continue;
- if ((xdev = malloc(sizeof(dev_t))) == NULL) {
+ if (xdev == NULL &&
+ (xdev = malloc(sizeof(dev_t))) == NULL) {
ERRX1("malloc");
goto out;
}
@@ -946,8 +947,8 @@ flist_gen_dirent(struct sess *sess, char
continue;
if (nxdev)
- if ((xdev = realloc(xdev, sizeof(dev_t))) ==
- NULL) {
+ if ((xdev = reallocarray(xdev, nxdev + 1,
+ sizeof(dev_t))) == NULL) {
ERRX1("realloc");
goto out;
}
@@ -1008,8 +1009,7 @@ flist_gen_dirent(struct sess *sess, char
rc = 1;
out:
fts_close(fts);
- if (sess->opts->one_file_system)
- free(xdev);
+ free(xdev);
return rc;
}
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: vmd(8) i8042 device implementation questions
On Sat, Jun 01, 2019 at 06:12:16PM -0500, Katherine Rohl wrote: > Couple questions: > > > This means no interrupt will be injected. I'm not sure if that's what you > > want. > > See vm.c: vcpu_exit_inout(..). It looks like you may have manually asserted > > the > > IRQ in this file, which is a bit different than what we do in other > > devices. That > > may be okay, though. > > The device can assert zero, one, or two IRQs depending on the state of the > input ports. Are we capable of asserting two IRQs at once through > vcpu_exit_i8042? > Yes, just assert/deassert what you want and it should drive the PIC state machine accordingly. LMK if that doesn't work. Assert followed by immediate deassert for edge triggered, and assert followed by deassert after ack for level triggered. I think all these are edge triggered though. > > For this IRQ, if it's edge triggered, please assert then deassert the line. > > The i8259 code should handle that properly. What you have here is a level > > triggered interrupt (eg, the line will stay asserted until someone > > does a 1 -> 0 transition below). Same goes for the next few cases. > > Would asserting the IRQs through the exit function handle this for me if > that’s possible? The return-to-vmm path can assert one single interrupt via returning something other than 0xFF from the exit handler in vmd. But if you need two, you'll need to either rework that logic (and fix all the other devices) or just assert/deassert one in the exit handler function and have the return-to-vmm path handle the other. Your call. Note - it is probably a good idea to rework the logic at some point because the current model can only handle "inject this edge triggered interrupt", and not other interesting scenarios we may need later like "assert this level triggered interrupt" or being able to flexibly configure a number of different interrupts (edge or level triggered). There is also no provision to handle deasserting something in the return-to-vmm path presently (that has to be handled in each device's I/O routine in vmd - yuck). > > > Also, please bump the revision in the vcpu struct for send/receive > > as we will be sending a new struct layout now. > > Where exactly? The file revision?
Re: vmd(8) i8042 device implementation questions
On Sun, Jun 02, 2019 at 03:21:34PM +0200, Jasper Lievisse Adriaanse wrote: > On Sat, Jun 01, 2019 at 06:12:16PM -0500, Katherine Rohl wrote: > > Couple questions: > > > > > This means no interrupt will be injected. I'm not sure if that's what you > > > want. > > > See vm.c: vcpu_exit_inout(..). It looks like you may have manually > > > asserted the > > > IRQ in this file, which is a bit different than what we do in other > > > devices. That > > > may be okay, though. > > > > The device can assert zero, one, or two IRQs depending on the state of the > > input ports. Are we capable of asserting two IRQs at once through > > vcpu_exit_i8042? > > > > > For this IRQ, if it's edge triggered, please assert then deassert the > > > line. > > > The i8259 code should handle that properly. What you have here is a level > > > triggered interrupt (eg, the line will stay asserted until someone > > > does a 1 -> 0 transition below). Same goes for the next few cases. > > > > Would asserting the IRQs through the exit function handle this for me if > > that???s possible? > > > > > Also, please bump the revision in the vcpu struct for send/receive > > > as we will be sending a new struct layout now. > > > > Where exactly? The file revision? > That would be VM_DUMP_VERSION in vmd.h I reckon. > Yes, that's what I meant. > -- > jasper >
Re: Spleen kernel fonts improvements
Frederic Cambus: > Here is a diff to sync kernel fonts with the latest released Spleen > version, bringing the following improvements: [...] > - Add some artefacts on each side of the lower case 'i' characters Yes, please! Actually, I still think the 'i' should have a wide foot like the '1'... but I'll try to stay out of this. > Comments? OK? ok -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: [patch] use acme-client to sign certificated with ecdsa keys
On Wed, Jun 05, 2019 at 08:39:51AM +0200, Renaud Allard wrote:
>
>
> On 6/5/19 8:20 AM, Gilles Chehade wrote:
> > On Tue, Jun 04, 2019 at 03:54:11PM +0200, Renaud Allard wrote:
> > >
> > >
> > > On 6/3/19 11:53 AM, Renaud Allard wrote:
> > > > > >
> > > > > > On 5/29/19 9:58 AM, Florian Obser wrote:
> > > > > > > why not let acme-client generate the key?
> > > > > >
> > > > >
> > > > > Here is a more complete diff where you can use the -E switch to
> > > > > generate a ECDSA key instead of the RSA one.
> > >
> > > I refined a little bit the patch to not put ecdsa functions into rsa.c.
> > > So I
> > > renamed rsa.c to key.c and removed the rsa references to functions which
> > > apply to both rsa and ecdsa.
> > >
> >
> > reads, builds and works fine for me
> >
> > a couple comments inlined
> >
>
> I removed the parenthesis and used another wording, removed the RSA from a
> "Load RSA key" as it might not be RSA and added E to the SYNOPSYS.
>
ok gilles@
> 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 5 Jun 2019 06:37:00 -
> @@ -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.12
> diff -u -p -r1.12 acctproc.c
> --- acctproc.c28 Jul 2018 15:25:23 - 1.12
> +++ acctproc.c5 Jun 2019 06:37:00 -
> @@ -82,8 +82,8 @@ op_thumb_rsa(EVP_PKEY *pkey)
> warnx("bn2string");
> else if ((exp = bn2string(r->e)) == NULL)
> warnx("bn2string");
> - else if ((json = json_fmt_thumb_rsa(exp, mod)) == NULL)
> - warnx("json_fmt_thumb_rsa");
> + else if ((json = json_fmt_thumb_key(exp, mod)) == NULL)
> + warnx("json_fmt_thumb_key");
>
> free(exp);
> free(mod);
> @@ -175,10 +175,10 @@ op_sign_rsa(char **head, char **prot, EV
> warnx("bn2string");
> else if ((exp = bn2string(r->e)) == NULL)
> warnx("bn2string");
> - else if ((*head = json_fmt_header_rsa(exp, mod)) == NULL)
> - warnx("json_fmt_header_rsa");
> - else if ((*prot = json_fmt_protected_rsa(exp, mod, nonce)) == NULL)
> - warnx("json_fmt_protected_rsa");
> + else if ((*head = json_fmt_header_key(exp, mod)) == NULL)
> + warnx("json_fmt_header_key");
> + else if ((*prot = json_fmt_protected_key(exp, mod, nonce)) == NULL)
> + warnx("json_fmt_protected_key");
> else
> rc = 1;
>
> @@ -338,7 +338,7 @@ 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;
> doddbg("%s: loaded RSA account key", acctkey);
> }
> Index: acme-client.1
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
> retrieving revision 1.29
> diff -u -p -r1.29 acme-client.1
> --- acme-client.1 3 Feb 2019 20:39:35 - 1.29
> +++ acme-client.1 5 Jun 2019 06:37:00 -
> @@ -22,7 +22,7 @@
> .Nd ACME client
> .Sh SYNOPSIS
> .Nm acme-client
> -.Op Fl ADFnrv
> +.Op Fl ADEFnrv
> .Op Fl f Ar configfile
> .Ar domain
> .Sh DESCRIPTION
> @@ -79,7 +79,9 @@ The options are as follows:
> .It Fl A
> Create a new RSA account key if one does not already exist.
> .It Fl D
> -Create a new RSA domain key if one does not already exist.
> +Create a new domain key if one does not already exist. Defaults to RSA.
> +.It Fl E
> +Switch the new domain key algorithm to ECDSA instead of RSA.
> .It Fl F
> Force certificate renewal, even if it's too soon.
> .It Fl f Ar configfile
> Index: ecdsa.h
> ===
> RCS file: ecdsa.h
> diff -N ecdsa.h
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ ecdsa.h 5 Jun 2019 06:37:00 -
> @@ -0,0 +1,22 @@
> +/* $Id: rsa.h,v 1.1 2016/08/31 22:01:42 florian Exp $ */
> +/*
> + * Copyright (c) 2019 Renaud Allard
> + *
> + * 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
Re: Pump my sched: fewer SCHED_LOCK() & kill p_priority
On 02/06/19(Sun) 22:03, Mark Kettenis wrote: > > Date: Sat, 1 Jun 2019 18:55:20 -0300 > > From: Martin Pieuchot > [...] > > - First change is to stop calling tsleep(9) at PUSER. That makes > > it clear that all "sleeping priorities" are smaller than PUSER. > > That's important to understand for the diff below. `p_priority' > > is currently a placeholder for the "sleeping priority" and the > > "runnqueue priority". Both fields are separated by this diff. > > Separating out the fields is a good idea. The current way priorities > are recorded is just confusing. The use of PUSER vs. PWAIT seems to > be fairly arbitrary, so that is probably not a big issue. Except > maybe for the single-threded signal stuff. Would be good to get > guenther@'s thoughts on this bit. > > The PUSER -> PWAIT change isn't really necessary is it? It just makes > it easier for you to understand what;s going on when looking at the > queues. The problem becomes easier to understand with this change. They is currently two places where `p_priority' is updated iff it was previously >= PUSER. These two places are schedclock() and schedcpu() they both "duplicate" the same logic to update `p_priority' just after having recalculated `p_usrpri'. So with it we can say that this code do no apply to the new `p_slpprio' because it is always < PUSER. Now the question is why does `p_priority' exist and why it some times reflect `p_usrpri'? The only places where mi_switch() is called without updating `p_priority' nor putting the caller on a queue are related to signals. To exit the SSTOP state, setrunnable() is called on a thread. Since setrunnable() is shared between SSLEEP and SSTOP threads it is not obvious that `p_priority' is the "sleeping priority" in the first case and 'p_usrpri' in the second case. The only exception to this logic are stopped threads with nice(1) value that have a `p_priority' < PUSER. In this specific case my diff, that passes `p_usrpri' as argument to setrunnable(), introduces a change in behavior. However I doubt it matters since such threads are generally exceptions and the only ones with a priority < PUSER. This can be observed by stopping sndiod(8). > > [...] > > - `p_estcpu' `p_usrpri' and `p_slptime' which represent the "priority" > > of a thread are now updated while holding a per-thread mutex. As a > > result schedclock() and donice() no longer takes the SCHED_LOCK(), > > and schedcpu() almost never take it. > > Need to look closer at how this works. `p_slptime' could be removed if we want to simplify the logic. I don't see the point of deferring the calculation of sleeping/stopped threads. > > > - With this diff top(1) and ps(1) will report the "real" `p_usrpi' value > > when displaying priorities. This is helpful to understand what's > > happening: > > Do you intend to remove that bit before committing this? I don't know. Which "priority" should we export then? The sleep/run priority? This would be the most backward-compatible change. However the priority that really matters *is* `p_usrpri'. With this diff it becomes so much easier to understand what's happening...
Re: if_netisr(): trade NET_LOCK() for NET_RLOCK()
On 04/06/19(Tue) 23:57, Alexandr Nedvedicky wrote: > Hello, > > I see, I owe some clarification to share wider context of this change. > > On Tue, Jun 04, 2019 at 10:32:57AM -0300, Martin Pieuchot wrote: > > On 04/06/19(Tue) 01:37, Alexandr Nedvedicky wrote: > > > Hello, > > > > > > diff below is just cosmetic change, which has no impact on current > > > functionality, because there is just single network task to deliver > > > packets to > > > IP stack. I just want to push this small change to tree to minimize delta > > > between current and my experimental branch. > > > > But why is it fine? > > currently all packets reaching IP stack are processed by if_netisr(), > which > runs as a task. The task pops up a packet from its queue (a.k.a. taskq) > and > passes the packet to corresponding xintr() handler for further processing. Only for packets that needs to be delivered/processed locally. In the case of forwarding if_input_process() is the task that process packets. > We still have a single taskq (see NET_TASKQ defined as 1 in net/if.c), > hence there is just one instance of if_netisr() running. Therefore I say > 'the change has no impact'. > > We still need to make sure the network stack is ready to deal with > multiple > tasks for local bound packets. Nice plan! > > Is it because the read version of the netlock is > > enough to protect the packet processing path? > > from network stack point of view the packets acquire netlock as > readers as they do not perform any changes to network subsystem > configuration. Route entries might be interested, cached routes might be updated. But theoretically that should be fine. That's something to keep in mind when adding more tasks. > > Or is it because it > > guarantees that no ioctl will be executed in the meantime? > > yes, the purpose of netlock is to synchronize packets with > ioctl() operations, which change configuration of networks stack > (e.g. adding/removing interface). > > > And why do > > we need that? To parallelise the packet processing path? > > correct. the plan is to have more netisr tasks to process > packets. those will run in parallel/concurrently as netlock readers. > we still have to make sure the network stack is able to run on multiple > tasks for local bound packets. I all for it (o:
Re: Spleen kernel fonts improvements
> Date: Wed, 5 Jun 2019 12:57:27 +0200
> From: Frederic Cambus
>
> Hi tech@,
>
> Here is a diff to sync kernel fonts with the latest released Spleen
> version, bringing the following improvements:
>
> - Shift the middle bar of the upper case 'G' one pixel down in the 12x24
> version
> - Shift lower case 'k' character right, for better alignment in the 12x24,
> 16x32, and 32x64 versions
> - Make upper case 'X' thicker in the 16x32 and 32x64 versions
> - Make upper case 'V' thicker in the 32x64 version
> - Make lower case 'g' character smoother in the 16x32 and 32x64 versions
> - Add some artefacts on each side of the lower case 'i' characters
>
> Comments? OK?
ok kettenis@
> Index: sys/dev/wsfont/spleen12x24.h
> ===
> RCS file: /cvs/src/sys/dev/wsfont/spleen12x24.h,v
> retrieving revision 1.2
> diff -u -p -r1.2 spleen12x24.h
> --- sys/dev/wsfont/spleen12x24.h 8 Mar 2019 10:53:59 - 1.2
> +++ sys/dev/wsfont/spleen12x24.h 5 Jun 2019 10:15:21 -
> @@ -1029,13 +1029,13 @@ static u_char spleen12x24_data[] = {
> 0x60, 0x00, /* .**. */
> 0x60, 0x00, /* .**. */
> 0x60, 0x00, /* .**. */
> + 0x60, 0x00, /* .**. */
> 0x63, 0xe0, /* .**...*. */
> 0x60, 0x60, /* .**..**. */
> 0x60, 0x60, /* .**..**. */
> 0x60, 0x60, /* .**..**. */
> 0x60, 0x60, /* .**..**. */
> 0x60, 0x60, /* .**..**. */
> - 0x60, 0x60, /* .**..**. */
> 0x30, 0x60, /* ..**.**. */
> 0x1f, 0xe0, /* .... */
> 0x00, 0x00, /* */
> @@ -1877,6 +1877,7 @@ static u_char spleen12x24_data[] = {
> 0x06, 0x00, /* .**. */
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> + 0x1e, 0x00, /* .... */
> 0x06, 0x00, /* .**. */
> 0x06, 0x00, /* .**. */
> 0x06, 0x00, /* .**. */
> @@ -1886,8 +1887,7 @@ static u_char spleen12x24_data[] = {
> 0x06, 0x00, /* .**. */
> 0x06, 0x00, /* .**. */
> 0x06, 0x00, /* .**. */
> - 0x06, 0x00, /* .**. */
> - 0x06, 0x00, /* .**. */
> + 0x07, 0x80, /* .... */
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> @@ -1923,21 +1923,21 @@ static u_char spleen12x24_data[] = {
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> - 0x60, 0x00, /* .**. */
> - 0x60, 0x00, /* .**. */
> - 0x60, 0x00, /* .**. */
> - 0x60, 0x00, /* .**. */
> - 0x61, 0x80, /* .****... */
> - 0x61, 0x80, /* .****... */
> - 0x63, 0x00, /* .**...** */
> - 0x66, 0x00, /* .**..**. */
> - 0x7c, 0x00, /* .*.. */
> - 0x6c, 0x00, /* .**.**.. */
> - 0x66, 0x00, /* .**..**. */
> - 0x63, 0x00, /* .**...** */
> - 0x61, 0x80, /* .****... */
> - 0x60, 0xc0, /* .**.**.. */
> - 0x60, 0xc0, /* .**.**.. */
> + 0x30, 0x00, /* ..** */
> + 0x30, 0x00, /* ..** */
> + 0x30, 0x00, /* ..** */
> + 0x30, 0x00, /* ..** */
> + 0x30, 0xc0, /* ..****.. */
> + 0x30, 0xc0, /* ..****.. */
> + 0x31, 0x80, /* ..**...**... */
> + 0x33, 0x00, /* ..**..** */
> + 0x3e, 0x00, /* ..*. */
> + 0x36, 0x00, /* ..**.**. */
> + 0x33, 0x00, /* ..**..** */
> + 0x31, 0x80, /* ..**...**... */
> + 0x30, 0xc0, /* ..****.. */
> + 0x30, 0x60, /* ..**.**. */
> + 0x30, 0x60, /* ..**.**. */
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> @@ -5152,6 +5152,7 @@ static u_char spleen12x24_data[] = {
> 0x03, 0x00, /* ..** */
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> + 0x1e, 0x00, /* .... */
> 0x06, 0x00, /* .**. */
> 0x06, 0x00, /* .**. */
> 0x06, 0x00, /* .**. */
> @@ -5161,8 +5162,7 @@ static u_char spleen12x24_data[] = {
> 0x06, 0x00, /* .**. */
> 0x06, 0x00, /* .**. */
> 0x06, 0x00, /* .**. */
> -
Re: [patch] use acme-client to sign certificated with ecdsa keys
On 6/5/19 8:39 AM, Renaud Allard wrote: On 6/5/19 8:20 AM, Gilles Chehade wrote: On Tue, Jun 04, 2019 at 03:54:11PM +0200, Renaud Allard wrote: On 6/3/19 11:53 AM, Renaud Allard wrote: On 5/29/19 9:58 AM, Florian Obser wrote: why not let acme-client generate the key? Here is a more complete diff where you can use the -E switch to generate a ECDSA key instead of the RSA one. I refined a little bit the patch to not put ecdsa functions into rsa.c. So I renamed rsa.c to key.c and removed the rsa references to functions which apply to both rsa and ecdsa. reads, builds and works fine for me a couple comments inlined I removed the parenthesis and used another wording, removed the RSA from a "Load RSA key" as it might not be RSA and added E to the SYNOPSYS. For completion, just in case you are wondering why I choose secp384r1 instead of secp521r1. It is because letsencrypt doesn't allow 521 bits keys. They return "Invalid key in certificate request :: ECDSA curve P-521 not allowed" smime.p7s Description: S/MIME Cryptographic Signature
Spleen kernel fonts improvements
Hi tech@,
Here is a diff to sync kernel fonts with the latest released Spleen
version, bringing the following improvements:
- Shift the middle bar of the upper case 'G' one pixel down in the 12x24
version
- Shift lower case 'k' character right, for better alignment in the 12x24,
16x32, and 32x64 versions
- Make upper case 'X' thicker in the 16x32 and 32x64 versions
- Make upper case 'V' thicker in the 32x64 version
- Make lower case 'g' character smoother in the 16x32 and 32x64 versions
- Add some artefacts on each side of the lower case 'i' characters
Comments? OK?
Index: sys/dev/wsfont/spleen12x24.h
===
RCS file: /cvs/src/sys/dev/wsfont/spleen12x24.h,v
retrieving revision 1.2
diff -u -p -r1.2 spleen12x24.h
--- sys/dev/wsfont/spleen12x24.h8 Mar 2019 10:53:59 - 1.2
+++ sys/dev/wsfont/spleen12x24.h5 Jun 2019 10:15:21 -
@@ -1029,13 +1029,13 @@ static u_char spleen12x24_data[] = {
0x60, 0x00, /* .**. */
0x60, 0x00, /* .**. */
0x60, 0x00, /* .**. */
+ 0x60, 0x00, /* .**. */
0x63, 0xe0, /* .**...*. */
0x60, 0x60, /* .**..**. */
0x60, 0x60, /* .**..**. */
0x60, 0x60, /* .**..**. */
0x60, 0x60, /* .**..**. */
0x60, 0x60, /* .**..**. */
- 0x60, 0x60, /* .**..**. */
0x30, 0x60, /* ..**.**. */
0x1f, 0xe0, /* .... */
0x00, 0x00, /* */
@@ -1877,6 +1877,7 @@ static u_char spleen12x24_data[] = {
0x06, 0x00, /* .**. */
0x00, 0x00, /* */
0x00, 0x00, /* */
+ 0x1e, 0x00, /* .... */
0x06, 0x00, /* .**. */
0x06, 0x00, /* .**. */
0x06, 0x00, /* .**. */
@@ -1886,8 +1887,7 @@ static u_char spleen12x24_data[] = {
0x06, 0x00, /* .**. */
0x06, 0x00, /* .**. */
0x06, 0x00, /* .**. */
- 0x06, 0x00, /* .**. */
- 0x06, 0x00, /* .**. */
+ 0x07, 0x80, /* .... */
0x00, 0x00, /* */
0x00, 0x00, /* */
0x00, 0x00, /* */
@@ -1923,21 +1923,21 @@ static u_char spleen12x24_data[] = {
0x00, 0x00, /* */
0x00, 0x00, /* */
0x00, 0x00, /* */
- 0x60, 0x00, /* .**. */
- 0x60, 0x00, /* .**. */
- 0x60, 0x00, /* .**. */
- 0x60, 0x00, /* .**. */
- 0x61, 0x80, /* .****... */
- 0x61, 0x80, /* .****... */
- 0x63, 0x00, /* .**...** */
- 0x66, 0x00, /* .**..**. */
- 0x7c, 0x00, /* .*.. */
- 0x6c, 0x00, /* .**.**.. */
- 0x66, 0x00, /* .**..**. */
- 0x63, 0x00, /* .**...** */
- 0x61, 0x80, /* .****... */
- 0x60, 0xc0, /* .**.**.. */
- 0x60, 0xc0, /* .**.**.. */
+ 0x30, 0x00, /* ..** */
+ 0x30, 0x00, /* ..** */
+ 0x30, 0x00, /* ..** */
+ 0x30, 0x00, /* ..** */
+ 0x30, 0xc0, /* ..****.. */
+ 0x30, 0xc0, /* ..****.. */
+ 0x31, 0x80, /* ..**...**... */
+ 0x33, 0x00, /* ..**..** */
+ 0x3e, 0x00, /* ..*. */
+ 0x36, 0x00, /* ..**.**. */
+ 0x33, 0x00, /* ..**..** */
+ 0x31, 0x80, /* ..**...**... */
+ 0x30, 0xc0, /* ..****.. */
+ 0x30, 0x60, /* ..**.**. */
+ 0x30, 0x60, /* ..**.**. */
0x00, 0x00, /* */
0x00, 0x00, /* */
0x00, 0x00, /* */
@@ -5152,6 +5152,7 @@ static u_char spleen12x24_data[] = {
0x03, 0x00, /* ..** */
0x00, 0x00, /* */
0x00, 0x00, /* */
+ 0x1e, 0x00, /* .... */
0x06, 0x00, /* .**. */
0x06, 0x00, /* .**. */
0x06, 0x00, /* .**. */
@@ -5161,8 +5162,7 @@ static u_char spleen12x24_data[] = {
0x06, 0x00, /* .**. */
0x06, 0x00, /* .**. */
0x06, 0x00, /* .**. */
- 0x06, 0x00, /* .**. */
- 0x06, 0x00, /* .**. */
+ 0x07, 0x80, /*
tcpdump: send_fd: sendmsg(3): Broken pipe
Hi all, when closing tcpdump while sending high rate traffic over box i'm getting log below: x3550m4# tcpdump -ni ix1 ^C x3550m4# tcpdump: send_fd: sendmsg(3): Broken pipe tcpdump: send_fd: sendmsg: expected sent 1 got -1 OpenBSD 6.5-current (GENERIC.MP) #6: Tue Jun 4 15:05:10 MDT 2019 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
Re: net80211: fix ifconfig mode command
On Tue, Jun 04, 2019 at 09:56:27AM +0200, Stefan Sperling wrote:
> That problem was not quite fixed yet so the previous diff was backed out.
> The problem should be fixed with this version.
Please ignore this.
Ken found that it triggers the same panic yet again.
> diff 108b98dc66011292b9ce4c54c4d31191422de773 /usr/src
> blob - 6c4cbc0a4eb31dc30a7dcac705e5c79bdee3320b
> file + sys/net80211/ieee80211.c
> --- sys/net80211/ieee80211.c
> +++ sys/net80211/ieee80211.c
> @@ -1007,6 +1007,7 @@ enum ieee80211_phymode
> ieee80211_next_mode(struct ifnet *ifp)
> {
> struct ieee80211com *ic = (void *)ifp;
> + uint16_t mode;
>
> if (IFM_MODE(ic->ic_media.ifm_cur->ifm_media) != IFM_AUTO) {
> /*
> @@ -1018,36 +1019,54 @@ ieee80211_next_mode(struct ifnet *ifp)
> }
>
> /*
> - * Get the next supported mode
> + * Always scan in AUTO mode if the driver scans all bands, and
> + * leave the current mode as it is.
>*/
> - for (++ic->ic_curmode;
> - ic->ic_curmode <= IEEE80211_MODE_MAX;
> - ic->ic_curmode++) {
> + if (ic->ic_caps & IEEE80211_C_SCANALLBAND)
> + return (IEEE80211_MODE_AUTO);
> +
> + /*
> + * Get the next supported mode; effectively, this alternates between
> + * the 11a (5GHz) and 11b/g (2GHz) modes. What matters is that each
> + * supported channel gets scanned.
> + */
> + for (mode = ic->ic_curmode + 1; mode <= IEEE80211_MODE_MAX; mode++) {
> /*
>* Skip over 11n mode. Its set of channels is the superset
>* of all channels supported by the other modes.
>*/
> - if (ic->ic_curmode == IEEE80211_MODE_11N)
> + if (mode == IEEE80211_MODE_11N)
> continue;
> /*
>* Skip over 11ac mode. Its set of channels is the set
>* of all channels supported by 11a.
>*/
> - if (ic->ic_curmode == IEEE80211_MODE_11AC)
> + if (mode == IEEE80211_MODE_11AC)
> continue;
>
> - /* Always scan in AUTO mode if the driver scans all bands. */
> - if (ic->ic_curmode >= IEEE80211_MODE_MAX ||
> - (ic->ic_caps & IEEE80211_C_SCANALLBAND)) {
> - ic->ic_curmode = IEEE80211_MODE_AUTO;
> + if (ic->ic_modecaps & (1 << IEEE80211_MODE_11G)) {
> + /*
> + * Skip over 11b mode. Its set of channels is
> + * the set of all channels supported by 11g.
> + */
> + if (mode == IEEE80211_MODE_11B)
> + continue;
> + }
> +
> + /* Start over if we have already tried all modes. */
> + if (mode == IEEE80211_MODE_MAX) {
> + mode = IEEE80211_MODE_AUTO;
> break;
> }
>
> - if (ic->ic_modecaps & (1 << ic->ic_curmode))
> + if (ic->ic_modecaps & (1 << mode))
> break;
> }
>
> - ieee80211_setmode(ic, ic->ic_curmode);
> + if (mode != ic->ic_curmode)
> + ieee80211_setmode(ic, mode);
> + else
> + ieee80211_reset_scan(ifp);
>
> return (ic->ic_curmode);
> }
> @@ -1058,26 +1077,41 @@ ieee80211_next_mode(struct ifnet *ifp)
> * work here assumes how things work elsewhere in this code.
> *
> * Because the result of this function is ultimately used to select a
> - * rate from the rate set of the returned mode, it must not return
> - * IEEE80211_MODE_11N, which uses MCS instead of rates for unicast frames.
> + * rate from the rate set of the returned mode, it must return one of the
> + * legacy 11a/b/g modes; 11n and 11ac modes use MCS instead of rate sets.
> */
> enum ieee80211_phymode
> ieee80211_chan2mode(struct ieee80211com *ic,
> const struct ieee80211_channel *chan)
> {
> /*
> + * Are we fixed in 11a/b/g mode?
>* NB: this assumes the channel would not be supplied to us
>* unless it was already compatible with the current mode.
>*/
> - if (ic->ic_curmode != IEEE80211_MODE_11N &&
> - ic->ic_curmode != IEEE80211_MODE_11AC &&
> - (ic->ic_curmode != IEEE80211_MODE_AUTO ||
> - chan == IEEE80211_CHAN_ANYC))
> + if (ic->ic_curmode == IEEE80211_MODE_11A ||
> + ic->ic_curmode == IEEE80211_MODE_11B ||
> + ic->ic_curmode == IEEE80211_MODE_11G)
> return ic->ic_curmode;
> - /*
> - * In autoselect or 11n mode; deduce a mode based on the channel
> - * characteristics.
> - */
> +
> + /* If no channel was provided, return the most suitable legacy mode. */
> + if (chan == IEEE80211_CHAN_ANYC) {
> + switch (ic->ic_curmode) {
> + case IEEE80211_MODE_AUTO:
> + case IEEE80211_MODE_11N:
> +
Re: [patch] push the KERNEL_LOCK deeper on read(2) and write(2)
> Date: Wed, 5 Jun 2019 10:16:25 +0200
> From: Sebastien Marie
>
> Hi,
>
> I would like to have feedback and testing on this diff. The initial work
> was done by ian@.
Don't forget to run "make syscalls" in sys/kern when building your own
kernel with this diff!
> It unlocks read(2) and write(2) families, and push the KERNEL_LOCK
> deeper in the code path. With it, read(2) and write(2) on socket will be
> KERNEL_LOCK-free.
>
> read(2) and write(2) are file type agnostics by using `struct fileops'
> (see sys/file.h) which define effective implementation per file type.
>
> Currently, we have the following fileops defined:
> - dmabuf dev/pci/drm/drm_linux.c
> - kqueue kern/kern_event.c
> - pipekern/sys_pipe.c
> - socket kern/sys_socket.c
> - vnode kern/vfs_vnops.c
>
>
> read(2) family uses dofilereadv(), which calls:
> - fileops::fo_read()
> - FRELE() -> fdrop() -> fileops::fo_close()
>
> write(2) family uses dofilewritev(), which calls:
> - fileops::fo_write()
> - FRELE() -> fdrop() -> fileops::fo_close()
>
>
> The diff pushs the KERNEL_LOCK inside each function, where it is
> effectively needed. But as some file types doesn't need the lock (socket
> for example), it makes read(2) and write(2) lock-free for them, while
> still grabbing the lock for others (vnode for example).
The socket calls will still grab the kernel lock for selwakeup and
sending signals of course. But some of them will indeed be lock free.
It should be fairly easy to also unlock pipes, but that can come
later.
> fileops::fo_read
> - dmabuf_read fine lock-free (just return ENXIO)
> - kqueue_read fine lock-free (just return ENXIO)
> - pipe_read KERNEL_LOCK/UNLOCK added (tsleep, wakeup, selwakeup)
> - soo_readcalls soreceive() which is already called without KERNEL_LOCK
> by recvit() (via recvmsg(2))
> - vn_read KERNEL_LOCK/UNLOCK added
>
> fileops::fo_write
> - dmabuf_writefine lock-free (just return ENXIO)
> - kqueue_writefine lock-free (just return ENXIO)
> - pipe_write KERNEL_LOCK/UNLOCK added (tsleep, wakeup, selwakeup)
> - soo_write calls sosend() which is already called without KERNEL_LOCK by
> sendit() (via sendmsg(2))
> - vn_writeKERNEL_LOCK/UNLOCK added
>
> fileops::fo_close
> - dmabuf_closealready take care of it
> - kqueue_closealready take care of it
> - pipe_close already take care of it
> - soo_close call soclose() which is already called without KERNEL_LOCK by
> socketpair(2)
> - vn_closefilealready take care of it
>
>
> In dofilewritev(), I take care of calling ptsignal() with the
> KERNEL_LOCK() too, as it requires the lock (it is asserted).
>
> Others functions should be fine to be called without the KERNEL_LOCK, as
> they are already used in such context.
Looks good to me. Certainly needs some testing especially on setups
that do a lot of socket communication. But if testing doesn't reveal
any problems this is ok kettenis@
> Index: kern/sys_generic.c
> ===
> RCS file: /cvs/src/sys/kern/sys_generic.c,v
> retrieving revision 1.123
> diff -u -p -r1.123 sys_generic.c
> --- kern/sys_generic.c21 Jan 2019 23:41:26 - 1.123
> +++ kern/sys_generic.c4 Jun 2019 06:19:23 -
> @@ -366,8 +366,11 @@ dofilewritev(struct proc *p, int fd, str
> if (uio->uio_resid != cnt && (error == ERESTART ||
> error == EINTR || error == EWOULDBLOCK))
> error = 0;
> - if (error == EPIPE)
> + if (error == EPIPE) {
> + KERNEL_LOCK();
> ptsignal(p, SIGPIPE, STHREAD);
> + KERNEL_UNLOCK();
> + }
> }
> cnt -= uio->uio_resid;
>
> Index: kern/sys_pipe.c
> ===
> RCS file: /cvs/src/sys/kern/sys_pipe.c,v
> retrieving revision 1.87
> diff -u -p -r1.87 sys_pipe.c
> --- kern/sys_pipe.c 13 Nov 2018 13:02:20 - 1.87
> +++ kern/sys_pipe.c 5 Jun 2019 08:06:19 -
> @@ -314,9 +314,11 @@ pipe_read(struct file *fp, struct uio *u
> int error;
> size_t size, nread = 0;
>
> + KERNEL_LOCK();
> +
> error = pipelock(rpipe);
> if (error)
> - return (error);
> + goto done;
>
> ++rpipe->pipe_busy;
>
> @@ -420,6 +422,8 @@ unlocked_error:
> if ((rpipe->pipe_buffer.size - rpipe->pipe_buffer.cnt) >= PIPE_BUF)
> pipeselwakeup(rpipe);
>
> +done:
> + KERNEL_UNLOCK();
> return (error);
> }
>
> @@ -430,6 +434,8 @@ pipe_write(struct file *fp, struct uio *
> size_t orig_resid;
> struct pipe *wpipe, *rpipe;
>
> + KERNEL_LOCK();
> +
> rpipe = fp->f_data;
> wpipe = rpipe->pipe_peer;
>
> @@ -437,7 +443,8 @@ pipe_write(struct file *fp, struct uio *
>* detect loss of pipe read side, issue SIGPIPE if
Re: bfd: respond to poll sequence from peer
Hi Mitchell
Thanks a lot for the work you are putting into BFD, I'll be able to
review this properly over the weekend.
-peter
On 2019 Jun 03 (Mon) at 20:37:17 +1000 (+1000), Mitchell Krome wrote:
:Hi,
:
:Testing bfd against frr on linux, their bfd implementation sends polls
:as soon as the session is up even if the session timers haven't
:changed from what it was advertising while it was in the down state.
:Currently openbsd bfd doesn't respond to polls, so this diff adds that
:support. tcpdump output during session setup (.9 is openbsd):
:
:14:56:31.225339 10.10.20.9.58974 > 10.10.20.10.bfd-control: [udp sum ok] BFD
v1 length 24 state down flags [] diag none my-discrim 3396727743 your-discrim 0
mintx 100 minrx 100 minecho 0 multiplier 3 [to s 0xc0] (ttl 255, id
48014, len 52)
:14:56:31.533645 10.10.20.10.49143 > 10.10.20.9.bfd-control: [udp sum ok] BFD
v1 length 24 state init flags [] diag neighbor-down my-discrim 2 your-discrim
3396727743 mintx 150 minrx 150 minecho 5 mul tiplier 3 (DF) [tos
0xc0] (ttl 255, id 36838, len 52)
:14:56:32.022601 10.10.20.9.58974 > 10.10.20.10.bfd-control: [udp sum ok] BFD
v1 length 24 state up flags [] diag none my-discrim 3396727743 your-discrim 2
mintx 100 minrx 150 minecho 0 multiplier 3 [tos 0xc0] (ttl 255, id
21474, len 52)
:14:56:32.023134 10.10.20.10.49143 > 10.10.20.9.bfd-control: [udp sum ok] BFD
v1 length 24 state up flags [P] diag none my-discrim 2 your-discrim 3396727743
mintx 150 minrx 150 minecho 5 multiplier 3 (DF) [tos 0xc0] (ttl
255, id 36952, len 52)
:14:56:32.023207 10.10.20.9.58974 > 10.10.20.10.bfd-control: [udp sum ok] BFD
v1 length 24 state up flags [F] diag none my-discrim 3396727743 your-discrim 2
mintx 150 minrx 150 minecho 0 multiplier 3 [tos 0xc0] (ttl 255, id
23805, len 52)
:14:56:32.997091 10.10.20.10.49143 > 10.10.20.9.bfd-control: [udp sum ok] BFD
v1 length 24 state up flags [] diag none my-discrim 2 your-discrim 3396727743
mintx 150 minrx 150 minecho 5 multiplier 3 ( DF) [tos 0xc0] (ttl
255, id 36991, len 52)
:
:I also added some handling for generating polls and receiving finals while
:in there, but there isn't any code to actually start our own poll
:sequence just yet.
:
:I only have frr and openbsd peers to test with - if anybody has
:something else hooked up would be good to check what they do.
:
:Mitchell
:
:
:diff --git sys/net/bfd.c sys/net/bfd.c
:index 42995531a8a..2ae287a15bb 100644
:--- sys/net/bfd.c
:+++ sys/net/bfd.c
:@@ -741,6 +741,8 @@ bfd_reset(struct bfd_config *bfd)
:
: bfd->bc_mode = BFD_MODE_ASYNC;
: bfd->bc_state = BFD_STATE_DOWN;
:+ bfd->bc_poll_seq = 0;
:+ bfd->bc_poll_rcvd = 0;
:
: /* rfc5880 6.8.18 */
: bfd->bc_neighbor->bn_lstate = BFD_STATE_DOWN;
:@@ -825,7 +827,10 @@ bfd_input(struct bfd_config *bfd, struct mbuf *m)
: bfd->bc_neighbor->bn_rdiscr = ntohl(peer->bfd_my_discriminator);
: bfd->bc_neighbor->bn_rstate = state;
: bfd->bc_neighbor->bn_rdemand = (flags & BFD_FLAG_D);
:- bfd->bc_poll = (flags & BFD_FLAG_F);
:+
:+ if (flags & BFD_FLAG_F && bfd->bc_poll_seq) {
:+ bfd->bc_poll_seq = 0;
:+ }
:
: /* Local change to the algorithm, we don't accept below 50ms */
: if (ntohl(peer->bfd_required_min_rx_interval) < BFD_MINIMUM)
:@@ -891,6 +896,12 @@ bfd_input(struct bfd_config *bfd, struct mbuf *m)
:
: bfd->bc_error = 0;
:
:+ /* Reply to poll if we aren't down */
:+ if (flags & BFD_FLAG_P && bfd->bc_state > BFD_STATE_DOWN) {
:+ bfd->bc_poll_rcvd = 1;
:+ bfd_send_control(bfd);
:+ }
:+
: discard:
: bfd->bc_neighbor->bn_rdiag = diag;
: m_free(m);
:@@ -979,6 +990,13 @@ bfd_send_control(void *x)
:
: h->bfd_ver_diag = ((BFD_VERSION << 5) | (bfd->bc_neighbor->bn_ldiag));
: h->bfd_sta_flags = (bfd->bc_state << 6);
:+ /* Can't send a poll and a final in the same packet. */
:+ if (bfd->bc_poll_rcvd) {
:+ h->bfd_sta_flags |= BFD_FLAG_F;
:+ bfd->bc_poll_rcvd = 0;
:+ } else if (bfd->bc_poll_seq) {
:+ h->bfd_sta_flags |= BFD_FLAG_P;
:+ }
: h->bfd_detect_multi = bfd->bc_neighbor->bn_mult;
: h->bfd_length = BFD_HDRLEN;
: h->bfd_my_discriminator = htonl(bfd->bc_neighbor->bn_ldiscr);
:diff --git sys/net/bfd.h sys/net/bfd.h
:index 3e8da45086f..8ee372faa5d 100644
:--- sys/net/bfd.h
:+++ sys/net/bfd.h
:@@ -143,7 +143,8 @@ struct bfd_config {
: time_t bc_lastuptime;
: unsigned int bc_laststate;
: unsigned int bc_state;
:- unsigned int bc_poll;
:+ unsigned int bc_poll_seq;
:+ unsigned int bc_poll_rcvd;
: unsigned int bc_error;
: uint32_t bc_minrx;
: uint32_t bc_mintx;
:
--
Philosophy will clip an angel's wings.
-- John Keats
[patch] push the KERNEL_LOCK deeper on read(2) and write(2)
Hi,
I would like to have feedback and testing on this diff. The initial work
was done by ian@.
It unlocks read(2) and write(2) families, and push the KERNEL_LOCK
deeper in the code path. With it, read(2) and write(2) on socket will be
KERNEL_LOCK-free.
read(2) and write(2) are file type agnostics by using `struct fileops'
(see sys/file.h) which define effective implementation per file type.
Currently, we have the following fileops defined:
- dmabufdev/pci/drm/drm_linux.c
- kqueuekern/kern_event.c
- pipe kern/sys_pipe.c
- socketkern/sys_socket.c
- vnode kern/vfs_vnops.c
read(2) family uses dofilereadv(), which calls:
- fileops::fo_read()
- FRELE() -> fdrop() -> fileops::fo_close()
write(2) family uses dofilewritev(), which calls:
- fileops::fo_write()
- FRELE() -> fdrop() -> fileops::fo_close()
The diff pushs the KERNEL_LOCK inside each function, where it is
effectively needed. But as some file types doesn't need the lock (socket
for example), it makes read(2) and write(2) lock-free for them, while
still grabbing the lock for others (vnode for example).
fileops::fo_read
- dmabuf_read fine lock-free (just return ENXIO)
- kqueue_read fine lock-free (just return ENXIO)
- pipe_read KERNEL_LOCK/UNLOCK added (tsleep, wakeup, selwakeup)
- soo_read calls soreceive() which is already called without KERNEL_LOCK
by recvit() (via recvmsg(2))
- vn_read KERNEL_LOCK/UNLOCK added
fileops::fo_write
- dmabuf_write fine lock-free (just return ENXIO)
- kqueue_write fine lock-free (just return ENXIO)
- pipe_writeKERNEL_LOCK/UNLOCK added (tsleep, wakeup, selwakeup)
- soo_write calls sosend() which is already called without KERNEL_LOCK by
sendit() (via sendmsg(2))
- vn_write KERNEL_LOCK/UNLOCK added
fileops::fo_close
- dmabuf_close already take care of it
- kqueue_close already take care of it
- pipe_closealready take care of it
- soo_close call soclose() which is already called without KERNEL_LOCK by
socketpair(2)
- vn_closefile already take care of it
In dofilewritev(), I take care of calling ptsignal() with the
KERNEL_LOCK() too, as it requires the lock (it is asserted).
Others functions should be fine to be called without the KERNEL_LOCK, as
they are already used in such context.
Thanks.
--
Sebastien Marie
Index: kern/sys_generic.c
===
RCS file: /cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.123
diff -u -p -r1.123 sys_generic.c
--- kern/sys_generic.c 21 Jan 2019 23:41:26 - 1.123
+++ kern/sys_generic.c 4 Jun 2019 06:19:23 -
@@ -366,8 +366,11 @@ dofilewritev(struct proc *p, int fd, str
if (uio->uio_resid != cnt && (error == ERESTART ||
error == EINTR || error == EWOULDBLOCK))
error = 0;
- if (error == EPIPE)
+ if (error == EPIPE) {
+ KERNEL_LOCK();
ptsignal(p, SIGPIPE, STHREAD);
+ KERNEL_UNLOCK();
+ }
}
cnt -= uio->uio_resid;
Index: kern/sys_pipe.c
===
RCS file: /cvs/src/sys/kern/sys_pipe.c,v
retrieving revision 1.87
diff -u -p -r1.87 sys_pipe.c
--- kern/sys_pipe.c 13 Nov 2018 13:02:20 - 1.87
+++ kern/sys_pipe.c 5 Jun 2019 08:06:19 -
@@ -314,9 +314,11 @@ pipe_read(struct file *fp, struct uio *u
int error;
size_t size, nread = 0;
+ KERNEL_LOCK();
+
error = pipelock(rpipe);
if (error)
- return (error);
+ goto done;
++rpipe->pipe_busy;
@@ -420,6 +422,8 @@ unlocked_error:
if ((rpipe->pipe_buffer.size - rpipe->pipe_buffer.cnt) >= PIPE_BUF)
pipeselwakeup(rpipe);
+done:
+ KERNEL_UNLOCK();
return (error);
}
@@ -430,6 +434,8 @@ pipe_write(struct file *fp, struct uio *
size_t orig_resid;
struct pipe *wpipe, *rpipe;
+ KERNEL_LOCK();
+
rpipe = fp->f_data;
wpipe = rpipe->pipe_peer;
@@ -437,7 +443,8 @@ pipe_write(struct file *fp, struct uio *
* detect loss of pipe read side, issue SIGPIPE if lost.
*/
if ((wpipe == NULL) || (wpipe->pipe_state & PIPE_EOF)) {
- return (EPIPE);
+ error = EPIPE;
+ goto done;
}
++wpipe->pipe_busy;
@@ -471,7 +478,7 @@ pipe_write(struct file *fp, struct uio *
wpipe->pipe_state &= ~(PIPE_WANT | PIPE_WANTR);
wakeup(wpipe);
}
- return (error);
+ goto done;
}
orig_resid = uio->uio_resid;
@@ -642,6 +649,8 @@ retrywrite:
if (wpipe->pipe_buffer.cnt)
pipeselwakeup(wpipe);
+done:
+ KERNEL_UNLOCK();
return (error);
}
Index: kern/syscalls.master
Re: [patch] use acme-client to sign certificated with ecdsa keys
On 6/5/19 8:20 AM, Gilles Chehade wrote:
On Tue, Jun 04, 2019 at 03:54:11PM +0200, Renaud Allard wrote:
On 6/3/19 11:53 AM, Renaud Allard wrote:
On 5/29/19 9:58 AM, Florian Obser wrote:
why not let acme-client generate the key?
Here is a more complete diff where you can use the -E switch to
generate a ECDSA key instead of the RSA one.
I refined a little bit the patch to not put ecdsa functions into rsa.c. So I
renamed rsa.c to key.c and removed the rsa references to functions which
apply to both rsa and ecdsa.
reads, builds and works fine for me
a couple comments inlined
I removed the parenthesis and used another wording, removed the RSA from
a "Load RSA key" as it might not be RSA and added E to the SYNOPSYS.
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 5 Jun 2019 06:37:00 -
@@ -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.12
diff -u -p -r1.12 acctproc.c
--- acctproc.c 28 Jul 2018 15:25:23 - 1.12
+++ acctproc.c 5 Jun 2019 06:37:00 -
@@ -82,8 +82,8 @@ op_thumb_rsa(EVP_PKEY *pkey)
warnx("bn2string");
else if ((exp = bn2string(r->e)) == NULL)
warnx("bn2string");
- else if ((json = json_fmt_thumb_rsa(exp, mod)) == NULL)
- warnx("json_fmt_thumb_rsa");
+ else if ((json = json_fmt_thumb_key(exp, mod)) == NULL)
+ warnx("json_fmt_thumb_key");
free(exp);
free(mod);
@@ -175,10 +175,10 @@ op_sign_rsa(char **head, char **prot, EV
warnx("bn2string");
else if ((exp = bn2string(r->e)) == NULL)
warnx("bn2string");
- else if ((*head = json_fmt_header_rsa(exp, mod)) == NULL)
- warnx("json_fmt_header_rsa");
- else if ((*prot = json_fmt_protected_rsa(exp, mod, nonce)) == NULL)
- warnx("json_fmt_protected_rsa");
+ else if ((*head = json_fmt_header_key(exp, mod)) == NULL)
+ warnx("json_fmt_header_key");
+ else if ((*prot = json_fmt_protected_key(exp, mod, nonce)) == NULL)
+ warnx("json_fmt_protected_key");
else
rc = 1;
@@ -338,7 +338,7 @@ 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;
doddbg("%s: loaded RSA account key", acctkey);
}
Index: acme-client.1
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
retrieving revision 1.29
diff -u -p -r1.29 acme-client.1
--- acme-client.1 3 Feb 2019 20:39:35 - 1.29
+++ acme-client.1 5 Jun 2019 06:37:00 -
@@ -22,7 +22,7 @@
.Nd ACME client
.Sh SYNOPSIS
.Nm acme-client
-.Op Fl ADFnrv
+.Op Fl ADEFnrv
.Op Fl f Ar configfile
.Ar domain
.Sh DESCRIPTION
@@ -79,7 +79,9 @@ The options are as follows:
.It Fl A
Create a new RSA account key if one does not already exist.
.It Fl D
-Create a new RSA domain key if one does not already exist.
+Create a new domain key if one does not already exist. Defaults to RSA.
+.It Fl E
+Switch the new domain key algorithm to ECDSA instead of RSA.
.It Fl F
Force certificate renewal, even if it's too soon.
.It Fl f Ar configfile
Index: ecdsa.h
===
RCS file: ecdsa.h
diff -N ecdsa.h
--- /dev/null 1 Jan 1970 00:00:00 -
+++ ecdsa.h 5 Jun 2019 06:37:00 -
@@ -0,0 +1,22 @@
+/* $Id: rsa.h,v 1.1 2016/08/31 22:01:42 florian Exp $ */
+/*
+ * Copyright (c) 2019 Renaud Allard
+ *
+ * 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.
+ */
+#ifndef ECDSA_H
+#define ECDSA_H
+
+EVP_PKEY *ec_key_create(FILE *, const char *);
+
+#endif /* ! ECDSA_H */
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/acme-client/extern.h,v
retrieving revision
Re: [patch] use acme-client to sign certificated with ecdsa keys
On Tue, Jun 04, 2019 at 03:54:11PM +0200, Renaud Allard wrote:
>
>
> On 6/3/19 11:53 AM, Renaud Allard wrote:
> > > >
> > > > On 5/29/19 9:58 AM, Florian Obser wrote:
> > > > > On Wed, May 22, 2019 at 01:33:11PM +0200, Renaud Allard wrote:
> > > > > > The key needs to be generated manually
> > > > > > i.e.: openssl ecparam -genkey -name secp384r1 -out privkey.pem
> > > > >
> > > > > why not let acme-client generate the key?
> > > >
> > >
> > > Here is a more complete diff where you can use the -E switch to
> > > generate a ECDSA key instead of the RSA one.
>
> I refined a little bit the patch to not put ecdsa functions into rsa.c. So I
> renamed rsa.c to key.c and removed the rsa references to functions which
> apply to both rsa and ecdsa.
>
reads, builds and works fine for me
a couple comments inlined
> 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 4 Jun 2019 13:50:28 -
> @@ -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.12
> diff -u -p -r1.12 acctproc.c
> --- acctproc.c28 Jul 2018 15:25:23 - 1.12
> +++ acctproc.c4 Jun 2019 13:50:28 -
> @@ -82,8 +82,8 @@ op_thumb_rsa(EVP_PKEY *pkey)
> warnx("bn2string");
> else if ((exp = bn2string(r->e)) == NULL)
> warnx("bn2string");
> - else if ((json = json_fmt_thumb_rsa(exp, mod)) == NULL)
> - warnx("json_fmt_thumb_rsa");
> + else if ((json = json_fmt_thumb_key(exp, mod)) == NULL)
> + warnx("json_fmt_thumb_key");
>
> free(exp);
> free(mod);
> @@ -175,10 +175,10 @@ op_sign_rsa(char **head, char **prot, EV
> warnx("bn2string");
> else if ((exp = bn2string(r->e)) == NULL)
> warnx("bn2string");
> - else if ((*head = json_fmt_header_rsa(exp, mod)) == NULL)
> - warnx("json_fmt_header_rsa");
> - else if ((*prot = json_fmt_protected_rsa(exp, mod, nonce)) == NULL)
> - warnx("json_fmt_protected_rsa");
> + else if ((*head = json_fmt_header_key(exp, mod)) == NULL)
> + warnx("json_fmt_header_key");
> + else if ((*prot = json_fmt_protected_key(exp, mod, nonce)) == NULL)
> + warnx("json_fmt_protected_key");
> else
> rc = 1;
>
> @@ -338,7 +338,7 @@ 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;
> doddbg("%s: loaded RSA account key", acctkey);
> }
> Index: acme-client.1
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
> retrieving revision 1.29
> diff -u -p -r1.29 acme-client.1
> --- acme-client.1 3 Feb 2019 20:39:35 - 1.29
> +++ acme-client.1 4 Jun 2019 13:50:28 -
> @@ -79,7 +79,9 @@ The options are as follows:
> .It Fl A
> Create a new RSA account key if one does not already exist.
> .It Fl D
> -Create a new RSA domain key if one does not already exist.
> +Create a new (RSA) domain key if one does not already exist.
why is RSA in parenthesis here ?
> +.It Fl E
> +Switch the new domain key algorithm to ECDSA instead of RSA.
the E option should be added to the SYNOPSIS too
> .It Fl F
> Force certificate renewal, even if it's too soon.
> .It Fl f Ar configfile
> Index: ecdsa.h
> ===
> RCS file: ecdsa.h
> diff -N ecdsa.h
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ ecdsa.h 4 Jun 2019 13:50:28 -
> @@ -0,0 +1,22 @@
> +/* $Id: rsa.h,v 1.1 2016/08/31 22:01:42 florian Exp $ */
> +/*
> + * Copyright (c) 2019 Renaud Allard
> + *
> + * 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
