vmm: load vmcs before reading vcpu registers
tech@,
Continuing my vmm/vmd bug hunt, the following diff adapts
vcpu_readregs_vmx to optionally load the vmcs on the current cpu. This
has gone unnoticed as the ioctl isn't used in typical vmd usage and the
usage of vcpu_readregs_vmx in the run ioctl is after the vmcs is already
loaded on the current cpu.
This fixes `vmctl send` on Intel hosts. (A fix for `vmctl receive` comes
next.)
Currently, `vmctl send` tries to serialize the vcpu registers as part of
serializing the vm state. On an MP machine, it's highly probable that
the vmread instructions will fail as they'll be executed on a cpu that
doesn't have the vmcs loaded.
While here, I noticed the vcpu_writeregs_vmx function doesn't set the
vcpu's vmcs state variable to VMCS_CLEARED after running vmclear. This
can cause failure to vm-enter as vmm uses that state to determine which
of the two Intel instructions to call (vmlaunch or vmresume).
ok?
-dv
Index: sys/arch/amd64/amd64/vmm.c
===
RCS file: /opt/cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.308
diff -u -p -r1.308 vmm.c
--- sys/arch/amd64/amd64/vmm.c 4 May 2022 02:24:26 - 1.308
+++ sys/arch/amd64/amd64/vmm.c 8 May 2022 18:37:42 -
@@ -140,7 +140,7 @@ int vm_rwregs(struct vm_rwregs_params *,
int vm_mprotect_ept(struct vm_mprotect_ept_params *);
int vm_rwvmparams(struct vm_rwvmparams_params *, int);
int vm_find(uint32_t, struct vm **);
-int vcpu_readregs_vmx(struct vcpu *, uint64_t, struct vcpu_reg_state *);
+int vcpu_readregs_vmx(struct vcpu *, uint64_t, int, struct vcpu_reg_state *);
int vcpu_readregs_svm(struct vcpu *, uint64_t, struct vcpu_reg_state *);
int vcpu_writeregs_vmx(struct vcpu *, uint64_t, int, struct vcpu_reg_state *);
int vcpu_writeregs_svm(struct vcpu *, uint64_t, struct vcpu_reg_state *);
@@ -978,7 +978,7 @@ vm_rwregs(struct vm_rwregs_params *vrwp,
if (vmm_softc->mode == VMM_MODE_VMX ||
vmm_softc->mode == VMM_MODE_EPT)
ret = (dir == 0) ?
- vcpu_readregs_vmx(vcpu, vrwp->vrwp_mask, vrs) :
+ vcpu_readregs_vmx(vcpu, vrwp->vrwp_mask, 1, vrs) :
vcpu_writeregs_vmx(vcpu, vrwp->vrwp_mask, 1, vrs);
else if (vmm_softc->mode == VMM_MODE_SVM ||
vmm_softc->mode == VMM_MODE_RVI)
@@ -1986,6 +1986,7 @@ vcpu_reload_vmcs_vmx(struct vcpu *vcpu)
* Parameters:
* vcpu: the vcpu to read register values from
* regmask: the types of registers to read
+ * loadvmcs: bit to indicate whether the VMCS has to be loaded first
* vrs: output parameter where register values are stored
*
* Return values:
@@ -1993,7 +1994,7 @@ vcpu_reload_vmcs_vmx(struct vcpu *vcpu)
* EINVAL: an error reading registers occurred
*/
int
-vcpu_readregs_vmx(struct vcpu *vcpu, uint64_t regmask,
+vcpu_readregs_vmx(struct vcpu *vcpu, uint64_t regmask, int loadvmcs,
struct vcpu_reg_state *vrs)
{
int i, ret = 0;
@@ -2005,6 +2006,11 @@ vcpu_readregs_vmx(struct vcpu *vcpu, uin
struct vcpu_segment_info *sregs = vrs->vrs_sregs;
struct vmx_msr_store *msr_store;
+ if (loadvmcs) {
+ if (vcpu_reload_vmcs_vmx(vcpu))
+ return (EINVAL);
+ }
+
#ifdef VMM_DEBUG
/* VMCS should be loaded... */
paddr_t pa = 0ULL;
@@ -2393,6 +2399,7 @@ out:
if (loadvmcs) {
if (vmclear(&vcpu->vc_control_pa))
ret = EINVAL;
+ atomic_swap_uint(&vcpu->vc_vmx_vmcs_state, VMCS_CLEARED);
}
return (ret);
}
@@ -4631,7 +4638,7 @@ vmm_translate_gva(struct vcpu *vcpu, uin
if (vmm_softc->mode == VMM_MODE_EPT ||
vmm_softc->mode == VMM_MODE_VMX) {
- if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, &vrs))
+ if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, 1, &vrs))
return (EINVAL);
} else if (vmm_softc->mode == VMM_MODE_RVI ||
vmm_softc->mode == VMM_MODE_SVM) {
@@ -5111,7 +5118,7 @@ vcpu_run_vmx(struct vcpu *vcpu, struct v
vcpu->vc_last_pcpu = curcpu();
/* Copy the VCPU register state to the exit structure */
- if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, &vcpu->vc_exit.vrs))
+ if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, 0, &vcpu->vc_exit.vrs))
ret = EINVAL;
vcpu->vc_exit.cpl = vmm_get_guest_cpu_cpl(vcpu);
Re: [External] : net lock priority
Hello,
On Sun, May 08, 2022 at 10:13:20PM +0200, Alexander Bluhm wrote:
> On Sun, May 08, 2022 at 09:19:23PM +0200, Alexander Bluhm wrote:
> > I will run my tests with the diff below.
>
> With the third chunk reboot hangs during reordering libraries in
> vmmaplk. So this needs more thought.
rw_do_exit() looks odd:
335 /* membar_exit_before_atomic() has to precede call of this function. */
336 void
337 rw_do_exit(struct rwlock *rwl, unsigned long wrlock)
338 {
339 unsigned long owner, set;
340
341 do {
342 owner = rwl->rwl_owner;
343 if (wrlock)
344 set = 0;
345 else
346 set = (owner - RWLOCK_READ_INCR) & ~RWLOCK_WAIT;
347 } while (__predict_false(rw_cas(&rwl->rwl_owner, owner, set)));
348
349 if (owner & RWLOCK_WAIT)
350 wakeup(rwl);
351 }
what bothers me is the situation where there are
more than one reader. The line 350 is executed by
the first reader which drops the lock. So the process
woken up by wakeup(rwl) are going to find out the
lock is still occupied by remaining readers.
regards
sashan
Re: [patch] CPU frequency scheduler change proposal
> Date: Sun, 20 Mar 2022 18:13:16 +0100
> From: Solene Rapenne
>
> I'm proposing a very simple change to the automatic policy of the CPU
> frequency scheduler.
>
> Currently, every 100ms the scheduler is doing this:
>
> - when the CPU load exceeds the threshold, CPU frequency is set to the
> maximum and the variable downbeats is set to 5.
> - when the CPU load is below the threshold, downbeats is decremented, if
> it's == 0 then the CPU load is reduced to 0
>
> my proposal is to change the downbeat to be adaptive to the load, instead
> of setting it to 5 all the time, I propose to increment it with a limit
> of 5. Instead of having the frequency set at max for 500ms (5 cycles)
> every time the CPU usage is above the treshold, we will keep the
> frequency high for a number of cycles depending how long it was high
> (up to 5). So, in case of short CPU usage burst like opening a new MP3
> file for decoding or a click on a GUI, we have a frequency burst of
> 100ms instead of 500ms.
>
> I've been using it for a few days, I noticed a huge battery life
> improvement with no responsiveness change.
I've been using variants of this diff on several machines now, and I
think it is an imrpovement and should go in.
I'm still experimenting with diffs that bring back cpu frequency
scaling on machines that are not running on battery and what I have
builds on top of this diff.
So, ok kettenis@
> Index: sched_bsd.c
> ===
> RCS file: /home/reposync/src/sys/kern/sched_bsd.c,v
> retrieving revision 1.70
> diff -u -r1.70 sched_bsd.c
> --- sched_bsd.c 30 Oct 2021 23:24:48 - 1.70
> +++ sched_bsd.c 20 Mar 2022 16:30:22 -
> @@ -579,8 +579,8 @@
> }
> if (allidle < alltotal / 2)
> speedup = 1;
> - if (speedup)
> - downbeats = 5;
> + if (speedup && downbeats < 5)
> + downbeats++;
>
> if (speedup && perflevel != 100) {
> faster:
>
>
Re: [External] : net lock priority
On Sun, May 08, 2022 at 09:19:23PM +0200, Alexander Bluhm wrote:
> I will run my tests with the diff below.
With the third chunk reboot hangs during reordering libraries in
vmmaplk. So this needs more thought.
> Index: kern/kern_rwlock.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_rwlock.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 kern_rwlock.c
> --- kern/kern_rwlock.c8 Feb 2021 08:18:45 - 1.47
> +++ kern/kern_rwlock.c8 May 2022 18:55:52 -
> @@ -81,7 +81,7 @@ static const struct rwlock_op {
> },
> { /* RW_READ */
> RWLOCK_READ_INCR,
> - RWLOCK_WRLOCK,
> + RWLOCK_WRLOCK | RWLOCK_WRWANT,
> RWLOCK_WAIT,
> 0,
> PLOCK
> @@ -103,7 +103,7 @@ rw_enter_read(struct rwlock *rwl)
> {
> unsigned long owner = rwl->rwl_owner;
>
> - if (__predict_false((owner & RWLOCK_WRLOCK) ||
> + if (__predict_false((owner & (RWLOCK_WRLOCK | RWLOCK_WRWANT)) ||
> rw_cas(&rwl->rwl_owner, owner, owner + RWLOCK_READ_INCR)))
> rw_enter(rwl, RW_READ);
> else {
> @@ -343,8 +343,7 @@ rw_do_exit(struct rwlock *rwl, unsigned
> if (wrlock)
> set = 0;
> else
> - set = (owner - RWLOCK_READ_INCR) &
> - ~(RWLOCK_WAIT|RWLOCK_WRWANT);
> + set = (owner - RWLOCK_READ_INCR) & ~RWLOCK_WAIT;
> } while (__predict_false(rw_cas(&rwl->rwl_owner, owner, set)));
>
> if (owner & RWLOCK_WAIT)
Re: [External] : Re: pf.conf(5) clarify ICMP sloppy state handling
On Sun, May 08, 2022 at 09:58:47PM +0200, Alexandr Nedvedicky wrote: > Hello, > > On Sun, May 08, 2022 at 08:06:57PM +0200, Alexander Bluhm wrote: > > On Sun, May 08, 2022 at 06:37:57PM +0200, Alexandr Nedvedicky wrote: > > > this tiny update to pf.conf(5) has been prompted here [1] on > > > pf mailing list. By default only ICMP queries are allowed > > > to create state in pf(4). The sloppy option relaxes that > > > so also ICMP replies can create a state. I think this should > > > be also mentioned in pf.conf(5) > > > > > > OK to my suggestion below? > > > > I would make it a bit shorter. pf.conf(5) is very long already. > > > > With this option ICMP replies can create states. > > > > Does this describe everything? > > yes, it does. I Like it. Updated diff below. OK bluhm@ > 8<---8<---8<--8< > diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 > index fe4b117994a..e4af2a37c5e 100644 > --- a/share/man/man5/pf.conf.5 > +++ b/share/man/man5/pf.conf.5 > @@ -2186,6 +2186,7 @@ It cannot be used with > .Cm modulate state > or > .Cm synproxy state . > +With this option ICMP replies can create states. > .It Ar timeout seconds > Changes the > .Ar timeout
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
Dear Theo, fellow developers,
Many thanks for the first review pass, much appreciated.
> This is a good first step. I have a few initial comments inline. Once you fix
> those, review of the rest will be easier.
>
> The main thing is: please remove all the copy-pasted functions that had no
> changes whatsoever or only very minor changes.
As the various same-named-but-different 'parse' structs are not easily
interchangeable without more refactoring, I marked them "XXX:". Perhaps
we can work on that in tree?
> [snip]
The following is a list of changes compared to previous changeset
proposal:
* removed the __func__ references
* added copyright lines
* use d2i_X509_ALGOR() instead of deserializing by hand
* removed all 'else if'
* fixed spaces vs tabs
* renamed elemz to elemsz, and made the check more readable
* added default case in switch (aor->type)
* removed superfluous parenthesis
Kind regards,
Job
Index: usr.sbin/rpki-client/Makefile
===
RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
retrieving revision 1.24
diff -u -p -r1.24 Makefile
--- usr.sbin/rpki-client/Makefile 21 Apr 2022 09:53:07 - 1.24
+++ usr.sbin/rpki-client/Makefile 8 May 2022 20:01:25 -
@@ -5,7 +5,7 @@ SRCS= as.c cert.c cms.c crl.c encoding.c
log.c main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
output-csv.c output-json.c parser.c print.c repo.c roa.c rrdp.c \
rrdp_delta.c rrdp_notification.c rrdp_snapshot.c rrdp_util.c \
- rsync.c tal.c validate.c x509.c
+ rsc.c rsync.c tal.c validate.c x509.c
MAN= rpki-client.8
LDADD+= -lexpat -ltls -lssl -lcrypto -lutil
Index: usr.sbin/rpki-client/extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.132
diff -u -p -r1.132 extern.h
--- usr.sbin/rpki-client/extern.h 21 Apr 2022 12:59:03 - 1.132
+++ usr.sbin/rpki-client/extern.h 8 May 2022 20:01:25 -
@@ -24,6 +24,14 @@
#include
#include
+/*
+ * Enumeration for ASN.1 explicit tags in RSC eContent
+ */
+enum rsc_resourceblock_tag {
+ RSRCBLK_TYPE_ASID,
+ RSRCBLK_TYPE_IPADDRBLK,
+};
+
enum cert_as_type {
CERT_AS_ID, /* single identifier */
CERT_AS_INHERIT, /* inherit from parent */
@@ -164,6 +172,7 @@ enum rtype {
RTYPE_ASPA,
RTYPE_REPO,
RTYPE_FILE,
+ RTYPE_RSC,
};
enum location {
@@ -232,6 +241,29 @@ struct roa {
time_t expires; /* do not use after */
};
+struct rscfile {
+ char*filename; /* an optional filename on the checklist */
+ unsigned charhash[SHA256_DIGEST_LENGTH]; /* the digest */
+};
+
+/*
+ * A Signed Checklist (RSC)
+ */
+struct rsc {
+ int talid; /* RSC covered by what TAL */
+ int valid; /* eContent resources covered by EE's 3779? */
+ struct cert_ip *ips; /* IP prefixes */
+ size_t ipsz; /* number of IP prefixes */
+ struct cert_as *as; /* AS resources */
+ size_t asz; /* number of AS resources */
+ struct rscfile *files; /* FileAndHashes in the RSC */
+ size_t filesz; /* number of FileAndHashes */
+ char*aia; /* AIA */
+ char*aki; /* AKI */
+ char*ski; /* SKI */
+ time_t expires; /* Not After of the RSC EE */
+};
+
/*
* A single Ghostbuster record
*/
@@ -450,6 +482,12 @@ voidgbr_free(struct gbr *);
struct gbr *gbr_parse(X509 **, const char *, const unsigned char *,
size_t);
+voidrsc_buffer(struct ibuf *, const struct rsc *);
+voidrsc_free(struct rsc *);
+struct rsc *rsc_parse(X509 **, const char *, const unsigned char *,
+ size_t);
+struct rsc *rsc_read(struct ibuf *);
+
/* crl.c */
struct crl *crl_parse(const char *, const unsigned char *, size_t);
struct crl *crl_get(struct crl_tree *, const struct auth *);
@@ -470,6 +508,7 @@ int valid_uri(const char *, size_t, co
int valid_origin(const char *, const char *);
int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
struct crl *, int);
+int valid_rsc(const char *, struct auth *, struct rsc *);
/* Working with CMS. */
unsigned char *cms_parse_validate(X509 **, const char *,
@@ -608,6 +647,7 @@ void crl_print(const struct crl *);
voidmft_print(const X509 *, const struct mft *);
voidroa_print(const X509 *, const struct roa *);
voidgbr_print(const X509 *, const struct gbr *);
+voidrsc_print(const X509 *, const struct rsc *);
/* Output! */
Index: usr.sbin/rpki-client/filemode.c
===
Re: [External] : Re: pf.conf(5) clarify ICMP sloppy state handling
Hello, On Sun, May 08, 2022 at 08:06:57PM +0200, Alexander Bluhm wrote: > On Sun, May 08, 2022 at 06:37:57PM +0200, Alexandr Nedvedicky wrote: > > this tiny update to pf.conf(5) has been prompted here [1] on > > pf mailing list. By default only ICMP queries are allowed > > to create state in pf(4). The sloppy option relaxes that > > so also ICMP replies can create a state. I think this should > > be also mentioned in pf.conf(5) > > > > OK to my suggestion below? > > I would make it a bit shorter. pf.conf(5) is very long already. > > With this option ICMP replies can create states. > > Does this describe everything? yes, it does. I Like it. Updated diff below. thanks and regards sashan 8<---8<---8<--8< diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 index fe4b117994a..e4af2a37c5e 100644 --- a/share/man/man5/pf.conf.5 +++ b/share/man/man5/pf.conf.5 @@ -2186,6 +2186,7 @@ It cannot be used with .Cm modulate state or .Cm synproxy state . +With this option ICMP replies can create states. .It Ar timeout seconds Changes the .Ar timeout
Re: [External] : net lock priority
On Sun, May 08, 2022 at 07:55:44PM +0200, Alexandr Nedvedicky wrote:
> my question is why do we reset RWLOCK_WAIT and RWLOCK_WRWANT flags?
This is a very good question.
> I think those flags should be reset the last reader gone. Perhaps
> the else branch for reader requires this:
>
> else {
> set = (owner - RWLOCK_READ_INCR) &
> ~(RWLOCK_WAIT|RWLOCK_WRWANT)
> if (set != 0)
> set |= (owner & RWLOCK_MASK);
> }
Why should a reader change RWLOCK_WRWANT at all? The writer sets
and clears it. This code was moved to this place in rev 1.8.
Before rw_exit_read() had this comment:
/*
* Potential MP race here. If the owner had WRWANT set we cleared
* it and a reader can sneak in before a writer. Do we care?
*/
I will run my tests with the diff below.
bluhm
Index: kern/kern_rwlock.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.47
diff -u -p -r1.47 kern_rwlock.c
--- kern/kern_rwlock.c 8 Feb 2021 08:18:45 - 1.47
+++ kern/kern_rwlock.c 8 May 2022 18:55:52 -
@@ -81,7 +81,7 @@ static const struct rwlock_op {
},
{ /* RW_READ */
RWLOCK_READ_INCR,
- RWLOCK_WRLOCK,
+ RWLOCK_WRLOCK | RWLOCK_WRWANT,
RWLOCK_WAIT,
0,
PLOCK
@@ -103,7 +103,7 @@ rw_enter_read(struct rwlock *rwl)
{
unsigned long owner = rwl->rwl_owner;
- if (__predict_false((owner & RWLOCK_WRLOCK) ||
+ if (__predict_false((owner & (RWLOCK_WRLOCK | RWLOCK_WRWANT)) ||
rw_cas(&rwl->rwl_owner, owner, owner + RWLOCK_READ_INCR)))
rw_enter(rwl, RW_READ);
else {
@@ -343,8 +343,7 @@ rw_do_exit(struct rwlock *rwl, unsigned
if (wrlock)
set = 0;
else
- set = (owner - RWLOCK_READ_INCR) &
- ~(RWLOCK_WAIT|RWLOCK_WRWANT);
+ set = (owner - RWLOCK_READ_INCR) & ~RWLOCK_WAIT;
} while (__predict_false(rw_cas(&rwl->rwl_owner, owner, set)));
if (owner & RWLOCK_WAIT)
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
On Sun, May 08, 2022 at 04:18:13PM +, Job Snijders wrote:
> Dear all,
>
> This changeset adds support for decoding and verifying the cryptographic
> validity of RPKI Signed Checklists (RSC) files. The draft [1] is in
> Working Group Last Call, the wire image is considered stable.
>
> A RSC is a Cryptographic Message Syntax (CMS) profile for a general
> purpose listing of checksums (a 'checklist'), for use with the
> Resource Public Key Infrastructure (RPKI). The objective is to
> allow an attestation, in the form of a listing of one or more
> checksums of arbitrary digital objects (files), to be signed "with
> resources", and for validation to provide a means to confirm a
> specific Internet Resource Holder produced the Signed Checklist.
> The profile is intended to provide for the signing of an arbitrary
> checksum listing with a specific set of Internet Number Resources.
>
> https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-rpki-rsc
>
> Example:
>
> $ rpki-client -j -f checklist.sig
> {
> "file": "checklist.sig",
> "hash_id": "GcuTYA9a2aXFt19jA8uLKaA7WAnr6pTeg1GkMoo8Pcg=",
> "ski": "B7:D4:EF:59:EE:7C:0E:60:DA:55:97:E3:71:31:90:92:20:C2:8A:A1",
> "aki": "38:E1:4F:92:FD:C7:CC:FB:FC:18:23:61:52:3A:E2:7D:69:7E:95:2F",
> "cert_serial": "0",
> "aia":
> "rsync://rpki.ripe.net/repository/DEFAULT/OOFPkv3HzPv8GCNhUjrifWl-lS8.cer",
> "valid_until": 1653730307,
> "signed_with_resources": [
> { "ip_prefix": "2001:67c:208c::/48" }
> ],
> "filenamesandhashes": [
> { "filename": "b42_ipv6_loa.png", "hash_digest":
> "lRbdZL58FyW5/KEXEg5Y6NhCpSBoczmbPd/8kcS2rPA=" },
> { "filename": "", "hash_digest":
> "CuE5RyIAXNkvTGqgJNXWs+LmfWKfEXINlHimM6EXocc=" }
> ],
> "validation": "OK"
> }
>
> The example checklist.sig file can be obtained at
> https://sobornost.net/~job/checklist.sig and validates via the RIPE NCC
> RPKI Trust Anchor.
>
> Feedback? OK?
This is a good first step. I have a few initial comments inline. Once you fix
those, review of the rest will be easier.
The main thing is: please remove all the copy-pasted functions that had no
changes whatsoever or only very minor changes.
If you feel the need to adapt some error messages in some of the
functions, you can pass a string in, so that you don't have to duplicate
tricky code.
Also, you sometimes use __func__ in your error messages, and sometimes
you don't. Please do this consistently within each function - I'd rather
drop all __funcs__.
>
> Kind regards,
>
> Job
>
> Index: usr.sbin/rpki-client/Makefile
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
> retrieving revision 1.24
> diff -u -p -r1.24 Makefile
> --- usr.sbin/rpki-client/Makefile 21 Apr 2022 09:53:07 - 1.24
> +++ usr.sbin/rpki-client/Makefile 8 May 2022 16:00:41 -
> @@ -5,7 +5,7 @@ SRCS= as.c cert.c cms.c crl.c encoding.c
> log.c main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
> output-csv.c output-json.c parser.c print.c repo.c roa.c rrdp.c \
> rrdp_delta.c rrdp_notification.c rrdp_snapshot.c rrdp_util.c \
> - rsync.c tal.c validate.c x509.c
> + rsc.c rsync.c tal.c validate.c x509.c
> MAN= rpki-client.8
>
> LDADD+= -lexpat -ltls -lssl -lcrypto -lutil
> Index: usr.sbin/rpki-client/extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.132
> diff -u -p -r1.132 extern.h
> --- usr.sbin/rpki-client/extern.h 21 Apr 2022 12:59:03 - 1.132
> +++ usr.sbin/rpki-client/extern.h 8 May 2022 16:00:41 -
> @@ -24,6 +24,14 @@
> #include
> #include
>
> +/*
> + * Enumeration for ASN.1 explicit tags in RSC eContent
> + */
> +enum rsc_resourceblock_tag {
> + RSRCBLK_TYPE_ASID,
> + RSRCBLK_TYPE_IPADDRBLK,
> +};
> +
> enum cert_as_type {
> CERT_AS_ID, /* single identifier */
> CERT_AS_INHERIT, /* inherit from parent */
> @@ -164,6 +172,7 @@ enum rtype {
> RTYPE_ASPA,
> RTYPE_REPO,
> RTYPE_FILE,
> + RTYPE_RSC,
> };
>
> enum location {
> @@ -232,6 +241,29 @@ struct roa {
> time_t expires; /* do not use after */
> };
>
> +struct rscfile {
> + char*filename; /* an optional filename on the checklist */
> + unsigned charhash[SHA256_DIGEST_LENGTH]; /* the digest */
> +};
> +
> +/*
> + * A Signed Checklist (RSC)
> + */
> +struct rsc {
> + int talid; /* RSC covered by what TAL */
> + int valid; /* eContent resources covered by EE's 3779? */
> + struct cert_ip *ips; /* IP prefixes */
> + size_t ipsz; /* number of IP prefixes */
> + struct cert_as *as; /* AS resources */
> + size_t asz; /* number of AS resources
Re: pf.conf(5) clarify ICMP sloppy state handling
On Sun, May 08, 2022 at 06:37:57PM +0200, Alexandr Nedvedicky wrote: > this tiny update to pf.conf(5) has been prompted here [1] on > pf mailing list. By default only ICMP queries are allowed > to create state in pf(4). The sloppy option relaxes that > so also ICMP replies can create a state. I think this should > be also mentioned in pf.conf(5) > > OK to my suggestion below? I would make it a bit shorter. pf.conf(5) is very long already. With this option ICMP replies can create states. Does this describe everything? > 8<---8<---8<--8< > diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 > index fe4b117994a..7389d231fe2 100644 > --- a/share/man/man5/pf.conf.5 > +++ b/share/man/man5/pf.conf.5 > @@ -2186,6 +2186,9 @@ It cannot be used with > .Cm modulate state > or > .Cm synproxy state . > +The option also relaxes handling of ICMP such that also ICMP replies > +are allowed to create state. > +By default ICMP queries only are allowed to create state. > .It Ar timeout seconds > Changes the > .Ar timeout
Re: [External] : net lock priority
Hello,
On Sun, May 08, 2022 at 07:31:46PM +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> On Fri, May 06, 2022 at 09:50:30PM +0200, Alexander Bluhm wrote:
> > Hi,
> >
> > When creating network load by forwarding packets, SSH gets unusable
> > and ping time gets above 10 seconds.
> >
> > Problem is that while multiple forwarding threads are running with
> > shared net lock, the exclusive lock cannot be acquired. This is
> > unfair.
> >
> > Diff below prevents that a read lock is granted when another thread
> > is waiting for the exclusive lock. With that ping time stays under
> > 300 ms.
> >
> > Does this read write lock prio change make sense?
>
> I can confirm this diff helps. And if I understand things right,
> then we basically let all readers to take a slow path via a sleep,
> if there is a writer waiting (think of like putting readers
> behind the waiting writer in queue).
>
> I was afraid the change can trade writer starvation for reader
> starvation. However I think it is not the case after seeing
> rw_do_exit(), where we zero out lock after writer does rw_exit().
> this basically gives equal chance to all readers/writers to acquire
> the lock.
>
> not my department, however I would say change looks good to me.
>
I was perhaps too fast in my judgement. I think we should also
look more closely at rw_do_exit():
335 /* membar_exit_before_atomic() has to precede call of this function. */
336 void
337 rw_do_exit(struct rwlock *rwl, unsigned long wrlock)
338 {
339 unsigned long owner, set;
340
341 do {
342 owner = rwl->rwl_owner;
343 if (wrlock)
344 set = 0;
345 else
346 set = (owner - RWLOCK_READ_INCR) &
347 ~(RWLOCK_WAIT|RWLOCK_WRWANT);
348 } while (__predict_false(rw_cas(&rwl->rwl_owner, owner, set)));
349
350 if (owner & RWLOCK_WAIT)
351 wakeup(rwl);
352
my question is why do we reset RWLOCK_WAIT and RWLOCK_WRWANT flags?
I think those flags should be reset the last reader gone. Perhaps
the else branch for reader requires this:
else {
set = (owner - RWLOCK_READ_INCR) &
~(RWLOCK_WAIT|RWLOCK_WRWANT)
if (set != 0)
set |= (owner & RWLOCK_MASK);
}
thanks and
regards
sashan
Re: [External] : net lock priority
Hello,
On Fri, May 06, 2022 at 09:50:30PM +0200, Alexander Bluhm wrote:
> Hi,
>
> When creating network load by forwarding packets, SSH gets unusable
> and ping time gets above 10 seconds.
>
> Problem is that while multiple forwarding threads are running with
> shared net lock, the exclusive lock cannot be acquired. This is
> unfair.
>
> Diff below prevents that a read lock is granted when another thread
> is waiting for the exclusive lock. With that ping time stays under
> 300 ms.
>
> Does this read write lock prio change make sense?
I can confirm this diff helps. And if I understand things right,
then we basically let all readers to take a slow path via a sleep,
if there is a writer waiting (think of like putting readers
behind the waiting writer in queue).
I was afraid the change can trade writer starvation for reader
starvation. However I think it is not the case after seeing
rw_do_exit(), where we zero out lock after writer does rw_exit().
this basically gives equal chance to all readers/writers to acquire
the lock.
not my department, however I would say change looks good to me.
OK sashan@ (given my OK counts in this area)
>
> bluhm
>
> Index: kern/kern_rwlock.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_rwlock.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 kern_rwlock.c
> --- kern/kern_rwlock.c8 Feb 2021 08:18:45 - 1.47
> +++ kern/kern_rwlock.c6 May 2022 12:08:01 -
> @@ -81,7 +81,7 @@ static const struct rwlock_op {
> },
> { /* RW_READ */
> RWLOCK_READ_INCR,
> - RWLOCK_WRLOCK,
> + RWLOCK_WRLOCK | RWLOCK_WRWANT,
> RWLOCK_WAIT,
> 0,
> PLOCK
> @@ -103,7 +103,7 @@ rw_enter_read(struct rwlock *rwl)
> {
> unsigned long owner = rwl->rwl_owner;
>
> - if (__predict_false((owner & RWLOCK_WRLOCK) ||
> + if (__predict_false((owner & (RWLOCK_WRLOCK | RWLOCK_WRWANT)) ||
> rw_cas(&rwl->rwl_owner, owner, owner + RWLOCK_READ_INCR)))
> rw_enter(rwl, RW_READ);
> else {
>
pf.conf(5) clarify ICMP sloppy state handling
Hello, this tiny update to pf.conf(5) has been prompted here [1] on pf mailing list. By default only ICMP queries are allowed to create state in pf(4). The sloppy option relaxes that so also ICMP replies can create a state. I think this should be also mentioned in pf.conf(5) OK to my suggestion below? thanks and regards sashan [1] https://marc.info/?l=openbsd-pf&m=165160086423472&w=2 8<---8<---8<--8< diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 index fe4b117994a..7389d231fe2 100644 --- a/share/man/man5/pf.conf.5 +++ b/share/man/man5/pf.conf.5 @@ -2186,6 +2186,9 @@ It cannot be used with .Cm modulate state or .Cm synproxy state . +The option also relaxes handling of ICMP such that also ICMP replies +are allowed to create state. +By default ICMP queries only are allowed to create state. .It Ar timeout seconds Changes the .Ar timeout
rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
Dear all,
This changeset adds support for decoding and verifying the cryptographic
validity of RPKI Signed Checklists (RSC) files. The draft [1] is in
Working Group Last Call, the wire image is considered stable.
A RSC is a Cryptographic Message Syntax (CMS) profile for a general
purpose listing of checksums (a 'checklist'), for use with the
Resource Public Key Infrastructure (RPKI). The objective is to
allow an attestation, in the form of a listing of one or more
checksums of arbitrary digital objects (files), to be signed "with
resources", and for validation to provide a means to confirm a
specific Internet Resource Holder produced the Signed Checklist.
The profile is intended to provide for the signing of an arbitrary
checksum listing with a specific set of Internet Number Resources.
https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-rpki-rsc
Example:
$ rpki-client -j -f checklist.sig
{
"file": "checklist.sig",
"hash_id": "GcuTYA9a2aXFt19jA8uLKaA7WAnr6pTeg1GkMoo8Pcg=",
"ski": "B7:D4:EF:59:EE:7C:0E:60:DA:55:97:E3:71:31:90:92:20:C2:8A:A1",
"aki": "38:E1:4F:92:FD:C7:CC:FB:FC:18:23:61:52:3A:E2:7D:69:7E:95:2F",
"cert_serial": "0",
"aia":
"rsync://rpki.ripe.net/repository/DEFAULT/OOFPkv3HzPv8GCNhUjrifWl-lS8.cer",
"valid_until": 1653730307,
"signed_with_resources": [
{ "ip_prefix": "2001:67c:208c::/48" }
],
"filenamesandhashes": [
{ "filename": "b42_ipv6_loa.png", "hash_digest":
"lRbdZL58FyW5/KEXEg5Y6NhCpSBoczmbPd/8kcS2rPA=" },
{ "filename": "", "hash_digest":
"CuE5RyIAXNkvTGqgJNXWs+LmfWKfEXINlHimM6EXocc=" }
],
"validation": "OK"
}
The example checklist.sig file can be obtained at
https://sobornost.net/~job/checklist.sig and validates via the RIPE NCC
RPKI Trust Anchor.
Feedback? OK?
Kind regards,
Job
Index: usr.sbin/rpki-client/Makefile
===
RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
retrieving revision 1.24
diff -u -p -r1.24 Makefile
--- usr.sbin/rpki-client/Makefile 21 Apr 2022 09:53:07 - 1.24
+++ usr.sbin/rpki-client/Makefile 8 May 2022 16:00:41 -
@@ -5,7 +5,7 @@ SRCS= as.c cert.c cms.c crl.c encoding.c
log.c main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
output-csv.c output-json.c parser.c print.c repo.c roa.c rrdp.c \
rrdp_delta.c rrdp_notification.c rrdp_snapshot.c rrdp_util.c \
- rsync.c tal.c validate.c x509.c
+ rsc.c rsync.c tal.c validate.c x509.c
MAN= rpki-client.8
LDADD+= -lexpat -ltls -lssl -lcrypto -lutil
Index: usr.sbin/rpki-client/extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.132
diff -u -p -r1.132 extern.h
--- usr.sbin/rpki-client/extern.h 21 Apr 2022 12:59:03 - 1.132
+++ usr.sbin/rpki-client/extern.h 8 May 2022 16:00:41 -
@@ -24,6 +24,14 @@
#include
#include
+/*
+ * Enumeration for ASN.1 explicit tags in RSC eContent
+ */
+enum rsc_resourceblock_tag {
+ RSRCBLK_TYPE_ASID,
+ RSRCBLK_TYPE_IPADDRBLK,
+};
+
enum cert_as_type {
CERT_AS_ID, /* single identifier */
CERT_AS_INHERIT, /* inherit from parent */
@@ -164,6 +172,7 @@ enum rtype {
RTYPE_ASPA,
RTYPE_REPO,
RTYPE_FILE,
+ RTYPE_RSC,
};
enum location {
@@ -232,6 +241,29 @@ struct roa {
time_t expires; /* do not use after */
};
+struct rscfile {
+ char*filename; /* an optional filename on the checklist */
+ unsigned charhash[SHA256_DIGEST_LENGTH]; /* the digest */
+};
+
+/*
+ * A Signed Checklist (RSC)
+ */
+struct rsc {
+ int talid; /* RSC covered by what TAL */
+ int valid; /* eContent resources covered by EE's 3779? */
+ struct cert_ip *ips; /* IP prefixes */
+ size_t ipsz; /* number of IP prefixes */
+ struct cert_as *as; /* AS resources */
+ size_t asz; /* number of AS resources */
+ struct rscfile *files; /* FileAndHashes in the RSC */
+ size_t filesz; /* number of FileAndHashes */
+ char*aia; /* AIA */
+ char*aki; /* AKI */
+ char*ski; /* SKI */
+ time_t expires; /* Not After of the RSC EE */
+};
+
/*
* A single Ghostbuster record
*/
@@ -450,6 +482,12 @@ voidgbr_free(struct gbr *);
struct gbr *gbr_parse(X509 **, const char *, const unsigned char *,
size_t);
+voidrsc_buffer(struct ibuf *, const struct rsc *);
+voidrsc_free(struct rsc *);
+struct rsc *rsc_parse(X509 **, const char *, const unsigned char *,
+ size_t);
+struct rsc *rsc_read(struct ibuf *);
+
/* crl.c */
struct c
move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()
Hello,
diff below reshuffles pfr_create_ktable() so all memory allocations happens
outside of NET_LOCK()/PF_LOCK(). pf(4) currently allocates for new table with
PR_WAITOK flag, when function is invoked on behalf of ioctl (PFR_FLAG_USERIOCTL
flag is set). PR_WAITOK for memory allocation is OK if we don't hold NET_LOCK()
of PF_LOCK() while waiting for memory. Unfortunately code in current is not OK
in this respect. Diff below fixes that.
The change pushes locks acquisition from pf_ioctl.c further down to
pfr_create_ktable(). The proposed change splits current giant loop
in pfr_create_ktable() into three loops:
the first loop allocates auxiliary list of tables we copied in.
the loop also checks for duplicity in input.
we grab NET_LOCK()/PF_LOCK() before entering second loop.
second loop moves tables instances from auxiliary list
into addq, when table does not exist already. If table
does exist it is left in auxq and we put existing table
we've just found into changeq.
If no PFR_FLAG_DUMMY is set the code will execute the
third loop, which processes addq we've constructed earlier
in second loop.
The most complicated part is the third loop, which processes
addq. The third loop must handle those situations:
- table which is being inserted is attached to main ruleset,
in this case we must move '->pfrkt_root' table we've
pre-allocated in the first to auxq
- the root table ('->pfrkt_root') for newly introduced table
exists already. In that case use existing one and move
pre-allocated table to auxq
- if root table does not exist yet, then initialize it and
insert it into addq.
OK ?
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 8315b115474..1f036e1368f 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -2217,12 +2217,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
error = ENODEV;
goto fail;
}
- NET_LOCK();
- PF_LOCK();
error = pfr_add_tables(io->pfrio_buffer, io->pfrio_size,
&io->pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
- NET_UNLOCK();
break;
}
diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
index fb23bcabe04..79fd3e0447b 100644
--- a/sys/net/pf_table.c
+++ b/sys/net/pf_table.c
@@ -189,6 +189,7 @@ void pfr_clstats_ktable(struct
pfr_ktable *, time_t, int);
struct pfr_ktable *pfr_create_ktable(struct pfr_table *, time_t, int,
int);
voidpfr_destroy_ktables(struct pfr_ktableworkq *, int);
+voidpfr_destroy_ktables_aux(struct pfr_ktableworkq *);
voidpfr_destroy_ktable(struct pfr_ktable *, int);
int pfr_ktable_compare(struct pfr_ktable *,
struct pfr_ktable *);
@@ -1493,14 +1494,16 @@ pfr_clr_tables(struct pfr_table *filter, int *ndel, int
flags)
int
pfr_add_tables(struct pfr_table *tbl, int size, int *nadd, int flags)
{
- struct pfr_ktableworkq addq, changeq;
- struct pfr_ktable *p, *q, *r, key;
+ struct pfr_ktableworkq addq, changeq, auxq;
+ struct pfr_ktable *p, *q, *r, *n, *w, key;
int i, rv, xadd = 0;
time_t tzero = gettime();
ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
SLIST_INIT(&addq);
SLIST_INIT(&changeq);
+ SLIST_INIT(&auxq);
+ /* pre-allocate all memory outside of locks */
for (i = 0; i < size; i++) {
YIELD(flags & PFR_FLAG_USERIOCTL);
if (COPYIN(tbl+i, &key.pfrkt_t, sizeof(key.pfrkt_t), flags))
@@ -1509,65 +1512,142 @@ pfr_add_tables(struct pfr_table *tbl, int size, int
*nadd, int flags)
flags & PFR_FLAG_USERIOCTL))
senderr(EINVAL);
key.pfrkt_flags |= PFR_TFLAG_ACTIVE;
- p = RB_FIND(pfr_ktablehead, &pfr_ktables, &key);
+ p = pfr_create_ktable(&key.pfrkt_t, tzero, 0,
+ !(flags & PFR_FLAG_USERIOCTL));
+ if (p == NULL)
+ senderr(ENOMEM);
+
+ /*
+* Note: we also pre-allocate a root table here. We keep it
+* at ->pfrkt_root, which we must not forget about.
+*/
+ key.pfrkt_flags = 0;
+ memset(key.pfrkt_anchor, 0, sizeof(key.pfrkt_anchor));
+ p->pfrkt_root = pfr_create_ktable(&key.pfrkt_t, 0, 0,
+ !(flags & PFR_FLAG_USERIOCTL));
+ if (p->pfrkt_root == NULL) {
+ pfr_destroy_ktable(p, 0);
+ senderr(ENOMEM);
+
