Re: urndis issues
On Wed, Jul 12, 2017 at 21:04 +, Jonathan Armani wrote: > Hi, > > Thanks I was cooking the same diff. > > Ok armani@ > Hi, thanks! I want to get rid of printfs though and return errors (or unhandled status codes) so that we don't paper over them. In theory, all error codes that I've seen in the ndis.h from the DDK have the MSB set, i.e. (rval & 0x8000) != 0 for those, but doing it would be a bit too hackish IMO. However, if we go down this road the rval check can be rewritten as: /* Not an error */ if ((rval & 0x8000) == 0) rval = RNDIS_STATUS_SUCCESS; else printf("%s: status 0x%x\n", DEVNAME(sc), rval); Is this something we want to do? I was also going to add sc_link toggled by those status codes and check it in the urndis_start like other USB Ethernet drivers do, but decided against it cause this would break other devices that don't send this message. diff --git sys/dev/usb/if_urndis.c sys/dev/usb/if_urndis.c index 4af6b55cf05..956207f73a8 100644 --- sys/dev/usb/if_urndis.c +++ sys/dev/usb/if_urndis.c @@ -88,10 +88,12 @@ u_int32_t urndis_ctrl_handle_init(struct urndis_softc *, const struct rndis_comp_hdr *); u_int32_t urndis_ctrl_handle_query(struct urndis_softc *, const struct rndis_comp_hdr *, void **, size_t *); u_int32_t urndis_ctrl_handle_reset(struct urndis_softc *, const struct rndis_comp_hdr *); +u_int32_t urndis_ctrl_handle_status(struct urndis_softc *, +const struct rndis_comp_hdr *); u_int32_t urndis_ctrl_init(struct urndis_softc *); u_int32_t urndis_ctrl_halt(struct urndis_softc *); u_int32_t urndis_ctrl_query(struct urndis_softc *, u_int32_t, void *, size_t, void **, size_t *); @@ -233,10 +235,14 @@ urndis_ctrl_handle(struct urndis_softc *sc, struct rndis_comp_hdr *hdr, case REMOTE_NDIS_KEEPALIVE_CMPLT: case REMOTE_NDIS_SET_CMPLT: rval = letoh32(hdr->rm_status); break; + case REMOTE_NDIS_INDICATE_STATUS_MSG: + rval = urndis_ctrl_handle_status(sc, hdr); + break; + default: printf("%s: ctrl message error: unknown event 0x%x\n", DEVNAME(sc), letoh32(hdr->rm_type)); rval = RNDIS_STATUS_FAILURE; } @@ -400,10 +406,42 @@ urndis_ctrl_handle_reset(struct urndis_softc *sc, return rval; } u_int32_t +urndis_ctrl_handle_status(struct urndis_softc *sc, +const struct rndis_comp_hdr *hdr) +{ + const struct rndis_status_msg *msg; + u_int32_trval; + + msg = (struct rndis_status_msg *)hdr; + + rval = letoh32(msg->rm_status); + + DPRINTF(("%s: urndis_ctrl_handle_status: len %u status 0x%x " + "stbuflen %u\n", + DEVNAME(sc), + letoh32(msg->rm_len), + rval, + letoh32(msg->rm_stbuflen))); + + switch (rval) { + case RNDIS_STATUS_MEDIA_CONNECT: + case RNDIS_STATUS_MEDIA_DISCONNECT: + case RNDIS_STATUS_OFFLOAD_CURRENT_CONFIG: + rval = RNDIS_STATUS_SUCCESS; + break; + + default: + printf("%s: status 0x%x\n", DEVNAME(sc), rval); + } + + return rval; +} + +u_int32_t urndis_ctrl_init(struct urndis_softc *sc) { struct rndis_init_req *msg; u_int32_trval; struct rndis_comp_hdr *hdr;
Re: time(1): use monotonic clock for computing elapsed time
On Thu, Jul 13, 2017 at 13:44 +1000, David Gwynne wrote: > > > On 13 Jul 2017, at 11:16 am, Scott Cheloha wrote: > > > > Hi, > > > > The "real" elapsed time for time(1) and the ksh/csh time builtins is > > currently computed with gettimeofday(2), so it's subject to changes > > by adjtime(2) and, if you're really unlucky, clock_settime(2) or > > settimeofday(2). In pathological cases you can get negative values > > in the output. > > > > This seems wrong to me. I personally use these tools like a stopwatch, > > and I was surprised to see that the elapsed difference wasn't (more) > > immune to changes to the system clock. > > > > The attached patches change the "real" listing for time(1), ksh's time > > builtin, and csh's time builtin to use a monotonic clock, which I think > > more closely matches what the typical user and programmer expects. This > > interpretation is, near as I can tell, also compatible with the POSIX.1 > > 2008 description of the time(1) utility. In particular, the use of > > "elapsed," implying a scalar value, makes me think that this is the > > intended behavior. [1] > > > > NetBSD did this in 2011 without much fanfare, though for some reason they > > did it for time(1) and csh's builtin but not for ksh's builtin. [2] > > > > I've tested pathological cases in each of the three and these patches > > correct the result in said cases without (perceptibly) changing the > > result in the typical case. > > > > Thoughts? Feedback? > > this makes sense to me, id like to see it go in. > Same here. I'm surprised to learn time(1) is not using CLOCK_MONOTONIC.
Re: armv7 small XXX fix
On Wed, Jul 12, 2017 at 04:21:11PM -0400, Dale Rahn wrote: > On Wed, Jul 12, 2017 at 11:06:23PM +0300, Artturi Alm wrote: > > On Wed, Jul 12, 2017 at 06:12:34PM +0200, Mark Kettenis wrote: > > > > Date: Mon, 10 Jul 2017 23:18:59 +0300 > > > > From: Artturi Alm > > > > > > > > Hi, > > > > > > > > this does clutter my diffs, and the XXX comment is correct, > > > > > > It probably isn't. None of the other architectures have those macros > > > in their . > > > > > > > Ok, didn't consider that, you're right and the diff withdrawn, but anyway, > > what i was imprecisely after was that i'd prefer > > having something like RODATA(name) found from sparc64/include/asm.h, > > instead of the useless _C_LABEL() to use in armv7 md .S assembly, and > > just because of the weird name i didn't find intuitive enough, i didn't > > even suggest the arm64 EENTRY().. > > Don't get me wrong, i'm more than happy to drop all the labeling > > macros out of my diffs, and choose my self what is global and what is not, > > while it's against the minimalism i'm now aiming at to even get the diffs > > read&commented when freetime&interest does meet:) > > _C_LABEL() is a holdover from the pre-ELF days. At one time (in NetBSD) > this code still compiled using an a.out toolchain. In that era, it was > defined to be _C_LABEL(x) _/**/x (this was also pre-ansi) > (or _ ## x post ansi). On ELF _C_LABEL evaluates to just the symbol. It > cannot have any association to either text or data because the proper use > of it can refer to either text or data eg: bl _C_LABEL(c_func). > > Basically it is a relic from the past, it might make sense to just remove > all references to _C_LABEL() at this point and just the symbol itself. > > Dale Rahn dr...@dalerahn.com Oh, that explains what i was missing about 'why?', thank you for the info/ details. At this point, given how harmless it is, i think cleaning up all at once would be just churn, so i will consider it deprecated myself, and go w/o it in my future diffs, which would touch any of them for other reasons. -Artturi
Re: arm/sysreg.h use in C
On Sat, Jul 01, 2017 at 10:53:14AM +0300, Artturi Alm wrote: > Hi, > > just in case i didn't make it clear what it is for, here's diff "fixing" > current uses below, compile-tested. > > -Artturi > Hi, ping? Noone up for bikeshedding, or seen useless/worse than handcrafting? I think this would alleviate from some of the complementary commenting, regarding the CP15 reg usage, that is currently somewhat of necessity. -Artturi > > diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c > index c91108e7066..fcb56627af7 100644 > --- a/sys/arch/arm/arm/cpufunc.c > +++ b/sys/arch/arm/arm/cpufunc.c > @@ -55,6 +55,7 @@ > #include > #include > #include > +#include > > #if defined(PERFCTRS) > struct arm_pmc_funcs *arm_pmc; > @@ -176,8 +177,7 @@ arm_get_cachetype_cp15v7(void) > uint32_t sel, level; > > /* CTR - Cache Type Register */ > - __asm volatile("mrc p15, 0, %0, c0, c0, 1" > - : "=r" (ctype)); > + __asm volatile("mrc " SR_STR(CP15_CTR(%0)) "\n" : "=r"(ctype)); > > arm_dcache_min_line_size = 1 << (CPU_CT_DMINLINE(ctype) + 2); > arm_icache_min_line_size = 1 << (CPU_CT_IMINLINE(ctype) + 2); > @@ -185,8 +185,8 @@ arm_get_cachetype_cp15v7(void) > min(arm_icache_min_line_size, arm_dcache_min_line_size); > > /* CLIDR - Cache Level ID Register */ > - __asm volatile("mrc p15, 1, %0, c0, c0, 1" > - : "=r" (cache_level_id) :); > + __asm volatile("mrc " SR_STR(CP15_CLIDR(%0)) > + : "=r"(cache_level_id)); > cpu_drain_writebuf(); > > /* L1 Cache available. */ > @@ -201,17 +201,18 @@ arm_get_cachetype_cp15v7(void) > cache_level_id & (0x2 << level)) { > sel = level << 1 | 0 << 0; /* L1 | unified/data cache */ > /* CSSELR - Cache Size Selection Register */ > - __asm volatile("mcr p15, 2, %0, c0, c0, 0" > - :: "r" (sel)); > + __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n" > + :: "r"(sel)); > cpu_drain_writebuf(); > /* CCSIDR - Cache Size Identification Register */ > - __asm volatile("mrc p15, 1, %0, c0, c0, 0" > - : "=r" (cachereg) :); > + __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n" > + : "=r"(cachereg)); > cpu_drain_writebuf(); > sets = ((cachereg >> 13) & 0x7fff) + 1; > arm_pdcache_line_size = 1 << ((cachereg & 0x7) + 4); > arm_pdcache_ways = ((cachereg >> 3) & 0x3ff) + 1; > - arm_pdcache_size = arm_pdcache_line_size * > arm_pdcache_ways * sets; > + arm_pdcache_size = > + arm_pdcache_line_size * arm_pdcache_ways * sets; > switch (cachereg & 0xc000) { > case 0x: > arm_pcache_type = 0; > @@ -230,24 +231,26 @@ arm_get_cachetype_cp15v7(void) > if (cache_level_id & (0x1 << level)) { > sel = level << 1 | 1 << 0; /* L1 | instruction cache */ > /* CSSELR - Cache Size Selection Register */ > - __asm volatile("mcr p15, 2, %0, c0, c0, 0" > - :: "r" (sel)); > + __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n" > + :: "r"(sel)); > cpu_drain_writebuf(); > /* CCSIDR - Cache Size Identification Register */ > - __asm volatile("mrc p15, 1, %0, c0, c0, 0" > - : "=r" (cachereg) :); > + __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n" > + : "=r"(cachereg)); > cpu_drain_writebuf(); > sets = ((cachereg >> 13) & 0x7fff) + 1; > arm_picache_line_size = 1 << ((cachereg & 0x7) + 4); > arm_picache_ways = ((cachereg >> 3) & 0x3ff) + 1; > - arm_picache_size = arm_picache_line_size * > arm_picache_ways * sets; > + arm_picache_size = > + arm_picache_line_size * arm_picache_ways * sets; > } > } > > arm_dcache_align = arm_pdcache_line_size; > arm_dcache_align_mask = arm_dcache_align - 1; > > - arm_dcache_l2_nsets = > arm_pdcache_size/arm_pdcache_ways/arm_pdcache_line_size; > + arm_dcache_l2_nsets = > + arm_pdcache_size / arm_pdcache_ways / arm_pdcache_line_size; > arm_dcache_l2_assoc = log2(arm_pdcache_ways); > arm_dcache_l2_linesize = log2(arm_pdcache_line_size); > } > @@ -255,17 +258,16 @@ arm_get_cachetype_cp15v7(void) > /* > */ > void > -armv7_idcache_wbinv_all() > +armv7_
signal info code SEGV_ACCERR
Hi, The regress test src/regress/sys/kern/siginfo-fault checks wether the si_code is set to SEGV_ACCERR after memory access with wrong permissions has triggert a SIGSEGV. Relevant commit message of the test is: According to POSIX, SIGSEGV should specify SEGV_ACCERR if the memory pages are mapped, but the protections don't match the user's access attempts, while SEGV_MAPERR should only be specified for pages that are unmapped. Some platforms currently handle this correctly, but not all. This diff changes the behavior of i366 and amd64. hppa seems to do something similar. Changing error form EACCES to EFAULT in amd64 affects only a printf, so I removed the useless code. Also change rv to errno in i386 to make it look like amd64. ok? bluhm Index: arch/amd64/amd64/trap.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/trap.c,v retrieving revision 1.54 diff -u -p -r1.54 trap.c --- arch/amd64/amd64/trap.c 30 Apr 2017 13:04:49 - 1.54 +++ arch/amd64/amd64/trap.c 13 Jul 2017 12:31:28 - @@ -362,9 +362,6 @@ faultcommon: KERNEL_UNLOCK(); goto out; } - if (error == EACCES) { - error = EFAULT; - } if (type == T_PAGEFLT) { if (pcb->pcb_onfault != 0) { @@ -389,7 +386,8 @@ faultcommon: frame_dump(frame); #endif sv.sival_ptr = (void *)fa; - trapsignal(p, SIGSEGV, T_PAGEFLT, SEGV_MAPERR, sv); + trapsignal(p, SIGSEGV, T_PAGEFLT, + error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv); } KERNEL_UNLOCK(); break; Index: arch/i386/i386/trap.c === RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/trap.c,v retrieving revision 1.130 diff -u -p -r1.130 trap.c --- arch/i386/i386/trap.c 30 Apr 2017 13:04:49 - 1.130 +++ arch/i386/i386/trap.c 13 Jul 2017 12:33:30 - @@ -373,7 +373,7 @@ trap(struct trapframe *frame) vaddr_t va, fa; struct vmspace *vm; struct vm_map *map; - int rv; + int error; cr2 = rcr2(); KERNEL_LOCK(); @@ -406,12 +406,12 @@ trap(struct trapframe *frame) if (curcpu()->ci_inatomic == 0 || map == kernel_map) { onfault = p->p_addr->u_pcb.pcb_onfault; p->p_addr->u_pcb.pcb_onfault = NULL; - rv = uvm_fault(map, va, 0, ftype); + error = uvm_fault(map, va, 0, ftype); p->p_addr->u_pcb.pcb_onfault = onfault; } else - rv = EFAULT; + error = EFAULT; - if (rv == 0) { + if (error == 0) { if (map != kernel_map) uvm_grow(p, va); if (type == T_PAGEFLT) { @@ -428,11 +428,12 @@ trap(struct trapframe *frame) goto copyfault; } printf("uvm_fault(%p, 0x%lx, 0, %d) -> %x\n", - map, va, ftype, rv); + map, va, ftype, error); goto we_re_toast; } sv.sival_int = fa; - trapsignal(p, SIGSEGV, vftype, SEGV_MAPERR, sv); + trapsignal(p, SIGSEGV, vftype, + error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv); KERNEL_UNLOCK(); break; }
Re: arm/sysreg.h use in C
> Date: Thu, 13 Jul 2017 15:49:03 +0300 > From: Artturi Alm > > On Sat, Jul 01, 2017 at 10:53:14AM +0300, Artturi Alm wrote: > > Hi, > > > > just in case i didn't make it clear what it is for, here's diff "fixing" > > current uses below, compile-tested. > > > > -Artturi > > > > Hi, > > ping? > Noone up for bikeshedding, or seen useless/worse than handcrafting? > I think this would alleviate from some of the complementary commenting, > regarding the CP15 reg usage, that is currently somewhat of necessity. I'm not sure myself if doing something like this is actually an improvement. > > diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c > > index c91108e7066..fcb56627af7 100644 > > --- a/sys/arch/arm/arm/cpufunc.c > > +++ b/sys/arch/arm/arm/cpufunc.c > > @@ -55,6 +55,7 @@ > > #include > > #include > > #include > > +#include > > > > #if defined(PERFCTRS) > > struct arm_pmc_funcs *arm_pmc; > > @@ -176,8 +177,7 @@ arm_get_cachetype_cp15v7(void) > > uint32_t sel, level; > > > > /* CTR - Cache Type Register */ > > - __asm volatile("mrc p15, 0, %0, c0, c0, 1" > > - : "=r" (ctype)); > > + __asm volatile("mrc " SR_STR(CP15_CTR(%0)) "\n" : "=r"(ctype)); > > > > arm_dcache_min_line_size = 1 << (CPU_CT_DMINLINE(ctype) + 2); > > arm_icache_min_line_size = 1 << (CPU_CT_IMINLINE(ctype) + 2); > > @@ -185,8 +185,8 @@ arm_get_cachetype_cp15v7(void) > > min(arm_icache_min_line_size, arm_dcache_min_line_size); > > > > /* CLIDR - Cache Level ID Register */ > > - __asm volatile("mrc p15, 1, %0, c0, c0, 1" > > - : "=r" (cache_level_id) :); > > + __asm volatile("mrc " SR_STR(CP15_CLIDR(%0)) > > + : "=r"(cache_level_id)); > > cpu_drain_writebuf(); > > > > /* L1 Cache available. */ > > @@ -201,17 +201,18 @@ arm_get_cachetype_cp15v7(void) > > cache_level_id & (0x2 << level)) { > > sel = level << 1 | 0 << 0; /* L1 | unified/data cache */ > > /* CSSELR - Cache Size Selection Register */ > > - __asm volatile("mcr p15, 2, %0, c0, c0, 0" > > - :: "r" (sel)); > > + __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n" > > + :: "r"(sel)); > > cpu_drain_writebuf(); > > /* CCSIDR - Cache Size Identification Register */ > > - __asm volatile("mrc p15, 1, %0, c0, c0, 0" > > - : "=r" (cachereg) :); > > + __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n" > > + : "=r"(cachereg)); > > cpu_drain_writebuf(); > > sets = ((cachereg >> 13) & 0x7fff) + 1; > > arm_pdcache_line_size = 1 << ((cachereg & 0x7) + 4); > > arm_pdcache_ways = ((cachereg >> 3) & 0x3ff) + 1; > > - arm_pdcache_size = arm_pdcache_line_size * > > arm_pdcache_ways * sets; > > + arm_pdcache_size = > > + arm_pdcache_line_size * arm_pdcache_ways * sets; > > switch (cachereg & 0xc000) { > > case 0x: > > arm_pcache_type = 0; > > @@ -230,24 +231,26 @@ arm_get_cachetype_cp15v7(void) > > if (cache_level_id & (0x1 << level)) { > > sel = level << 1 | 1 << 0; /* L1 | instruction cache */ > > /* CSSELR - Cache Size Selection Register */ > > - __asm volatile("mcr p15, 2, %0, c0, c0, 0" > > - :: "r" (sel)); > > + __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n" > > + :: "r"(sel)); > > cpu_drain_writebuf(); > > /* CCSIDR - Cache Size Identification Register */ > > - __asm volatile("mrc p15, 1, %0, c0, c0, 0" > > - : "=r" (cachereg) :); > > + __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n" > > + : "=r"(cachereg)); > > cpu_drain_writebuf(); > > sets = ((cachereg >> 13) & 0x7fff) + 1; > > arm_picache_line_size = 1 << ((cachereg & 0x7) + 4); > > arm_picache_ways = ((cachereg >> 3) & 0x3ff) + 1; > > - arm_picache_size = arm_picache_line_size * > > arm_picache_ways * sets; > > + arm_picache_size = > > + arm_picache_line_size * arm_picache_ways * sets; > > } > > } > > > > arm_dcache_align = arm_pdcache_line_size; > > arm_dcache_align_mask = arm_dcache_align - 1; > > > > - arm_dcache_l2_nsets = > > arm_pdcache_size/arm_pdcache_ways/arm_pdcache_line_size; > > + arm_dcache_l2_nsets = > > + arm_pdcache_size / arm_pdcache_ways / arm_pdcache_line_size; > > arm_dcache_l2_assoc
Re: signal info code SEGV_ACCERR
> Date: Thu, 13 Jul 2017 14:54:41 +0200 > From: Alexander Bluhm > > Hi, > > The regress test src/regress/sys/kern/siginfo-fault checks wether > the si_code is set to SEGV_ACCERR after memory access with wrong > permissions has triggert a SIGSEGV. > > Relevant commit message of the test is: > According to POSIX, SIGSEGV should specify SEGV_ACCERR if the memory > pages are mapped, but the protections don't match the user's access > attempts, while SEGV_MAPERR should only be specified for pages that > are unmapped. Some platforms currently handle this correctly, but not > all. > > This diff changes the behavior of i366 and amd64. hppa seems to > do something similar. Changing error form EACCES to EFAULT in amd64 > affects only a printf, so I removed the useless code. Also change > rv to errno in i386 to make it look like amd64. > > ok? I like this. ok kettenis@ > Index: arch/amd64/amd64/trap.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/trap.c,v > retrieving revision 1.54 > diff -u -p -r1.54 trap.c > --- arch/amd64/amd64/trap.c 30 Apr 2017 13:04:49 - 1.54 > +++ arch/amd64/amd64/trap.c 13 Jul 2017 12:31:28 - > @@ -362,9 +362,6 @@ faultcommon: > KERNEL_UNLOCK(); > goto out; > } > - if (error == EACCES) { > - error = EFAULT; > - } > > if (type == T_PAGEFLT) { > if (pcb->pcb_onfault != 0) { > @@ -389,7 +386,8 @@ faultcommon: > frame_dump(frame); > #endif > sv.sival_ptr = (void *)fa; > - trapsignal(p, SIGSEGV, T_PAGEFLT, SEGV_MAPERR, sv); > + trapsignal(p, SIGSEGV, T_PAGEFLT, > + error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv); > } > KERNEL_UNLOCK(); > break; > Index: arch/i386/i386/trap.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/trap.c,v > retrieving revision 1.130 > diff -u -p -r1.130 trap.c > --- arch/i386/i386/trap.c 30 Apr 2017 13:04:49 - 1.130 > +++ arch/i386/i386/trap.c 13 Jul 2017 12:33:30 - > @@ -373,7 +373,7 @@ trap(struct trapframe *frame) > vaddr_t va, fa; > struct vmspace *vm; > struct vm_map *map; > - int rv; > + int error; > > cr2 = rcr2(); > KERNEL_LOCK(); > @@ -406,12 +406,12 @@ trap(struct trapframe *frame) > if (curcpu()->ci_inatomic == 0 || map == kernel_map) { > onfault = p->p_addr->u_pcb.pcb_onfault; > p->p_addr->u_pcb.pcb_onfault = NULL; > - rv = uvm_fault(map, va, 0, ftype); > + error = uvm_fault(map, va, 0, ftype); > p->p_addr->u_pcb.pcb_onfault = onfault; > } else > - rv = EFAULT; > + error = EFAULT; > > - if (rv == 0) { > + if (error == 0) { > if (map != kernel_map) > uvm_grow(p, va); > if (type == T_PAGEFLT) { > @@ -428,11 +428,12 @@ trap(struct trapframe *frame) > goto copyfault; > } > printf("uvm_fault(%p, 0x%lx, 0, %d) -> %x\n", > - map, va, ftype, rv); > + map, va, ftype, error); > goto we_re_toast; > } > sv.sival_int = fa; > - trapsignal(p, SIGSEGV, vftype, SEGV_MAPERR, sv); > + trapsignal(p, SIGSEGV, vftype, > + error == EACCES ? SEGV_ACCERR : SEGV_MAPERR, sv); > KERNEL_UNLOCK(); > break; > } > >
remove net.inet6.ip6.maxifdefrouters and net.inet6.ip6.maxifprefixes sysctls
pointed out by jmc, these sysctls are useless now, too, the kernel no longer tracks prefixes or default routers from router advertisements. OK? diff --git lib/libc/gen/sysctl.3 lib/libc/gen/sysctl.3 index 0ac92a5c079..e6d3d092b2f 100644 --- lib/libc/gen/sysctl.3 +++ lib/libc/gen/sysctl.3 @@ -1654,8 +1654,6 @@ The currently defined protocols and names are: .It ip6 Ta maxdynroutes Ta integer Ta yes .It ip6 Ta maxfragpackets Ta integer Ta yes .It ip6 Ta maxfrags Ta integer Ta yes -.It ip6 Ta maxifprefixes Ta integer Ta yes -.It ip6 Ta maxifdefrouters Ta integer Ta yes .It ip6 Ta mforwarding Ta integer Ta yes .It ip6 Ta mtudisctimeout Ta integer Ta yes .It ip6 Ta multicast_mtudisc Ta integer Ta yes @@ -1814,17 +1812,6 @@ The maximum number of fragments the node will accept. \-1 means that the node will accept as many fragments as it receives. The flag is provided basically for avoiding possible DoS attacks. .Pp -.It Li ip6.maxifprefixes Pq Va net.inet6.ip6.maxifprefixes -Maximum number of prefixes created by route advertisements per interface. -Set to negative to disable. -The default value is 16. -.Pp -.It Li ip6.maxifdefrouters Pq Va net.inet6.ip6.maxifdefrouters -Maximum number of default routers created by route advertisements per -interface. -Set to negative to disable. -The default value is 16. -.Pp .It Li ip6.mforwarding Pq Va net.inet6.ip6.mforwarding If set to 1, then multicast forwarding is enabled for the host. The default is 0. diff --git sys/netinet6/in6.h sys/netinet6/in6.h index 1aac6581916..2af7d987335 100644 --- sys/netinet6/in6.h +++ sys/netinet6/in6.h @@ -584,8 +584,6 @@ ifatoia6(struct ifaddr *ifa) #define IPV6CTL_MULTIPATH 43 #define IPV6CTL_MCAST_PMTU 44 /* path MTU discovery for multicast */ #define IPV6CTL_NEIGHBORGCTHRESH 45 -#define IPV6CTL_MAXIFPREFIXES 46 -#define IPV6CTL_MAXIFDEFROUTERS 47 #define IPV6CTL_MAXDYNROUTES 48 #define IPV6CTL_DAD_PENDING49 #define IPV6CTL_MTUDISCTIMEOUT 50 @@ -644,8 +642,8 @@ ifatoia6(struct ifaddr *ifa) { "multipath", CTLTYPE_INT }, \ { "multicast_mtudisc", CTLTYPE_INT }, \ { "neighborgcthresh", CTLTYPE_INT }, \ - { "maxifprefixes", CTLTYPE_INT }, \ - { "maxifdefrouters", CTLTYPE_INT }, \ + { 0, 0 }, \ + { 0, 0 }, \ { "maxdynroutes", CTLTYPE_INT }, \ { "dad_pending", CTLTYPE_INT }, \ { "mtudisctimeout", CTLTYPE_INT }, \ @@ -701,8 +699,8 @@ ifatoia6(struct ifaddr *ifa) &ip6_multipath, \ &ip6_mcast_pmtu, \ &ip6_neighborgcthresh, \ - &ip6_maxifprefixes, \ - &ip6_maxifdefrouters, \ + NULL, \ + NULL, \ &ip6_maxdynroutes, \ NULL, \ NULL, \ diff --git sys/netinet6/in6_proto.c sys/netinet6/in6_proto.c index e2294ed1cb4..2353571afd6 100644 --- sys/netinet6/in6_proto.c +++ sys/netinet6/in6_proto.c @@ -360,8 +360,6 @@ int ip6_auto_flowlabel = 1; intip6_use_deprecated = 1; /* allow deprecated addr (RFC2462 5.5.4) */ intip6_mcast_pmtu = 0; /* enable pMTU discovery for multicast? */ intip6_neighborgcthresh = 2048; /* Threshold # of NDP entries for GC */ -intip6_maxifprefixes = 16; /* Max acceptable prefixes via RA per IF */ -intip6_maxifdefrouters = 16; /* Max acceptable def routers via RA */ intip6_maxdynroutes = 4096; /* Max # of routes created via redirect */ time_t ip6_log_time = (time_t)0L; diff --git sys/netinet6/ip6_var.h sys/netinet6/ip6_var.h index 9b58d9ee0af..45225f272fa 100644 --- sys/netinet6/ip6_var.h +++ sys/netinet6/ip6_var.h @@ -279,8 +279,6 @@ extern int ip6_sendredirect; /* send ICMPv6 redirect? */ extern int ip6_use_deprecated; /* allow deprecated addr as source */ extern int ip6_mcast_pmtu; /* path MTU discovery for multicast */ extern int ip6_neighborgcthresh; /* Threshold # of NDP entries for GC */ -extern int ip6_maxifprefixes; /* Max acceptable prefixes via RA per IF */ -extern int ip6_maxifdefrouters; /* Max acceptable def routers via RA */ extern int ip6_maxdynroutes; /* Max # of routes created via redirect */ extern struct socket *ip6_mrouter[RT_TABLEID_MAX]; /* multicast routing daemon */ -- I'm not entirely sure you are real.
Re: arm/sysreg.h use in C
13.7.2017 17.50 "Mark Kettenis" kirjoitti: > Date: Thu, 13 Jul 2017 15:49:03 +0300 > From: Artturi Alm > > On Sat, Jul 01, 2017 at 10:53:14AM +0300, Artturi Alm wrote: > > Hi, > > > > just in case i didn't make it clear what it is for, here's diff "fixing" > > current uses below, compile-tested. > > > > -Artturi > > > > Hi, > > ping? > Noone up for bikeshedding, or seen useless/worse than handcrafting? > I think this would alleviate from some of the complementary commenting, > regarding the CP15 reg usage, that is currently somewhat of necessity. I'm not sure myself if doing something like this is actually an improvement. Ok, i'll try to get some fbsd dev to comment why they never went for this, just for my own curiousity, so not pushing the diff any further. -Artturi > > diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c > > index c91108e7066..fcb56627af7 100644 > > --- a/sys/arch/arm/arm/cpufunc.c > > +++ b/sys/arch/arm/arm/cpufunc.c > > @@ -55,6 +55,7 @@ > > #include > > #include > > #include > > +#include > > > > #if defined(PERFCTRS) > > struct arm_pmc_funcs *arm_pmc; > > @@ -176,8 +177,7 @@ arm_get_cachetype_cp15v7(void) > > uint32_t sel, level; > > > > /* CTR - Cache Type Register */ > > - __asm volatile("mrc p15, 0, %0, c0, c0, 1" > > - : "=r" (ctype)); > > + __asm volatile("mrc " SR_STR(CP15_CTR(%0)) "\n" : "=r"(ctype)); > > > > arm_dcache_min_line_size = 1 << (CPU_CT_DMINLINE(ctype) + 2); > > arm_icache_min_line_size = 1 << (CPU_CT_IMINLINE(ctype) + 2); > > @@ -185,8 +185,8 @@ arm_get_cachetype_cp15v7(void) > > min(arm_icache_min_line_size, arm_dcache_min_line_size); > > > > /* CLIDR - Cache Level ID Register */ > > - __asm volatile("mrc p15, 1, %0, c0, c0, 1" > > - : "=r" (cache_level_id) :); > > + __asm volatile("mrc " SR_STR(CP15_CLIDR(%0)) > > + : "=r"(cache_level_id)); > > cpu_drain_writebuf(); > > > > /* L1 Cache available. */ > > @@ -201,17 +201,18 @@ arm_get_cachetype_cp15v7(void) > > cache_level_id & (0x2 << level)) { > > sel = level << 1 | 0 << 0; /* L1 | unified/data cache */ > > /* CSSELR - Cache Size Selection Register */ > > - __asm volatile("mcr p15, 2, %0, c0, c0, 0" > > - :: "r" (sel)); > > + __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n" > > + :: "r"(sel)); > > cpu_drain_writebuf(); > > /* CCSIDR - Cache Size Identification Register */ > > - __asm volatile("mrc p15, 1, %0, c0, c0, 0" > > - : "=r" (cachereg) :); > > + __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n" > > + : "=r"(cachereg)); > > cpu_drain_writebuf(); > > sets = ((cachereg >> 13) & 0x7fff) + 1; > > arm_pdcache_line_size = 1 << ((cachereg & 0x7) + 4); > > arm_pdcache_ways = ((cachereg >> 3) & 0x3ff) + 1; > > - arm_pdcache_size = arm_pdcache_line_size * arm_pdcache_ways * sets; > > + arm_pdcache_size = > > + arm_pdcache_line_size * arm_pdcache_ways * sets; > > switch (cachereg & 0xc000) { > > case 0x: > > arm_pcache_type = 0; > > @@ -230,24 +231,26 @@ arm_get_cachetype_cp15v7(void) > > if (cache_level_id & (0x1 << level)) { > > sel = level << 1 | 1 << 0; /* L1 | instruction cache */ > > /* CSSELR - Cache Size Selection Register */ > > - __asm volatile("mcr p15, 2, %0, c0, c0, 0" > > - :: "r" (sel)); > > + __asm volatile("mcr " SR_STR(CP15_CSSELR(%0)) "\n" > > + :: "r"(sel)); > > cpu_drain_writebuf(); > > /* CCSIDR - Cache Size Identification Register */ > > - __asm volatile("mrc p15, 1, %0, c0, c0, 0" > > - : "=r" (cachereg) :); > > + __asm volatile("mcr " SR_STR(CP15_CCSIDR(%0)) "\n" > > + : "=r"(cachereg)); > > cpu_drain_writebuf(); > > sets = ((cachereg >> 13) & 0x7fff) + 1; > > arm_picache_line_size = 1 << ((cachereg & 0x7) + 4); > > arm_picache_ways = ((cachereg >> 3) & 0x3ff) + 1; > > - arm_picache_size = arm_picache_line_size * arm_picache_ways * sets; > > + arm_picache_size = > > + arm_picache_line_size * arm_picache_ways * sets; > > } > > } > > > > arm_dcache_align = arm_pdcache_line_size; > > arm_dcache_align_mask = arm_dcache_align - 1; > > > > - arm_dcache_l2_nsets = arm_pdcache_size/arm_pdc
Re: CVS: cvs.openbsd.org: src
On 07/03/17 08:30, Martijn van Duren wrote: > On 07/01/17 18:14, Mark Kettenis wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: kette...@cvs.openbsd.org2017/07/01 10:14:10 >> >> Modified files: >> sys/dev/pci/drm: drm_irq.c drm_linux.c drm_linux.h >> drm_linux_list.h drm_mm.c drm_mm.h drm_mode.h >> drm_modes.c drm_rect.c drm_rect.h >> drm_vma_manager.c files.drm i915_drm.h >> i915_pciids.h linux_hdmi.h >> sys/dev/pci/drm/i915: dvo.h dvo_ch7017.c dvo_ch7xxx.c dvo_ivch.c >>dvo_ns2501.c dvo_sil164.c dvo_tfp410.c >>i915_dma.c i915_drv.c i915_drv.h >>i915_gem.c i915_gem_context.c >>i915_gem_evict.c i915_gem_execbuffer.c >>i915_gem_gtt.c i915_gem_stolen.c >>i915_gem_tiling.c i915_gpu_error.c >>i915_irq.c i915_reg.h i915_suspend.c >>i915_trace.h intel_bios.c intel_bios.h >>intel_crt.c intel_ddi.c intel_display.c >>intel_dp.c intel_drv.h intel_dvo.c >>intel_fbdev.c intel_hdmi.c intel_i2c.c >>intel_lvds.c intel_modes.c >>intel_opregion.c intel_overlay.c >>intel_panel.c intel_pm.c >>intel_ringbuffer.c intel_ringbuffer.h >>intel_sdvo.c intel_sdvo_regs.h >>intel_sideband.c intel_sprite.c intel_tv.c >>intel_uncore.c >> sys/dev/pci/drm/radeon: atombios_crtc.c atombios_dp.c >> atombios_i2c.c nid.h r100.c radeon.h >> radeon_connectors.c radeon_device.c >> radeon_display.c radeon_fb.c >> radeon_i2c.c radeon_irq_kms.c >> radeon_kms.c radeon_legacy_crtc.c >> radeon_legacy_encoders.c radeon_mode.h >> radeon_pm.c >> sys/dev/pci/drm/ttm: ttm_bo_manager.c >> Added files: >> sys/dev/pci/drm: drm_linux_atomic.h drm_mipi_dsi.h drm_modes.h >> drm_modeset_lock.c drm_modeset_lock.h >> drm_panel.c drm_panel.h drm_plane_helper.c >> drm_plane_helper.h drm_probe_helper.c >> linux_list_sort.c linux_types.h >> linux_ww_mutex.h >> sys/dev/pci/drm/i915: i915_cmd_parser.c i915_gem_batch_pool.c >>i915_gem_batch_pool.h i915_gem_fence.c >>i915_gem_gtt.h i915_gem_render_state.c >>i915_gem_render_state.h i915_gem_userptr.c >>i915_guc_reg.h i915_guc_submission.c >>i915_params.c i915_vgpu.c i915_vgpu.h >>intel_atomic.c intel_atomic_plane.c >>intel_audio.c intel_csr.c intel_dp_mst.c >>intel_dsi.c intel_dsi.h >>intel_dsi_panel_vbt.c intel_dsi_pll.c >>intel_fbc.c intel_fifo_underrun.c >>intel_frontbuffer.c intel_gtt.c >>intel_guc.h intel_guc_fwif.h >>intel_guc_loader.c intel_hotplug.c >>intel_lrc.c intel_lrc.h intel_mocs.c >>intel_mocs.h intel_psr.c >>intel_renderstate.h >>intel_renderstate_gen6.c >>intel_renderstate_gen7.c >>intel_renderstate_gen8.c >>intel_renderstate_gen9.c >>intel_runtime_pm.c >> sys/dev/pci/drm/radeon: radeon_dp_auxch.c >> >> Log message: >> Update inteldrm(4) to code based on Linux 4.4.70. This brings us support for >> Skylake and Cherryview and better support for Broadwell and Valleyview. Also >> adds MST support. Some tweaks to the TTM code and radeondrm(4) to keep it >> working with the updated generic DRM code needed for inteldrm(4). >> >> Tested by many. >> > This change *STILL* breaks my $DAYJOB machine. > After a couple of updates where I had to revert back to an old kernel I managed to lay claim to a "new" system. This one is intel only and doesn't have any issues. If there's still interest in fixing this issue, the hardware is still available, but there's no direct need for me any more. OpenBSD 6.1-current (GENERIC.MP) #95: Wed Jul 12 19:23:28 MDT 2017 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP RTC BIOS d
sosend netlock assertion crash
Hi, My laptop just crashed while running some php ports regress tests. The kernel complained that it did not hold the netlock in sounlock() after the out label in sosend(). The assert is correct, let's fix the obvious offender. ok? bluhm Index: kern/uipc_socket.c === RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.193 diff -u -p -r1.193 uipc_socket.c --- kern/uipc_socket.c 8 Jul 2017 09:19:02 - 1.193 +++ kern/uipc_socket.c 13 Jul 2017 15:34:52 - @@ -396,8 +396,9 @@ sosend(struct socket *so, struct mbuf *a resid = top->m_pkthdr.len; /* MSG_EOR on a SOCK_STREAM socket is invalid. */ if (so->so_type == SOCK_STREAM && (flags & MSG_EOR)) { - error = EINVAL; - goto out; + m_freem(top); + m_freem(control); + return (EINVAL); } if (uio && uio->uio_procp) uio->uio_procp->p_ru.ru_msgsnd++;
RFC 7217: random but stable addresses (take 2)
this has all the bells and whistles notably the installer and documentation for the net.inet6.ip6.soiikey sysctl are missing. the sysctl implementation is from dlg, all the mistakes are probably tweaks by me ;) It switches the hash function to SipHash24 from sha512 as suggested by dlg Comments, tests, improvments, OKs? diff --git etc/rc etc/rc index 48e5671335f..fda8a620bbb 100644 --- etc/rc +++ etc/rc @@ -47,6 +47,14 @@ update_limit() { done } +# Apply soiikey.conf settings. +soiikey_conf() { + stripcom /etc/soiikey.conf | + while read _line; do + sysctl -q "net.inet6.ip6.soiikey=$_line" + done +} + # Apply sysctl.conf(5) settings. sysctl_conf() { stripcom /etc/sysctl.conf | @@ -60,6 +68,7 @@ sysctl_conf() { update_limit -n openfiles;; esac done + soiikey_conf } # Apply mixerctl.conf(5) settings. @@ -154,6 +163,11 @@ make_keys() { fi ssh-keygen -A + + if [[ ! -f /etc/soiikey.conf ]]; then + openssl rand -hex 16 > /etc/soiikey.conf && \ + chmod 600 /etc/soiikey.conf && soiikey_conf + fi } # Re-link libraries, placing the objects in a random order. diff --git sbin/ifconfig/brconfig.h sbin/ifconfig/brconfig.h index ee68feb411b..09c871b352b 100644 --- sbin/ifconfig/brconfig.h +++ sbin/ifconfig/brconfig.h @@ -73,7 +73,7 @@ void switch_portno(const char *, const char *); "\024\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6NOTRAILERS" \ "\7RUNNING\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX"\ "\15LINK0\16LINK1\17LINK2\20MULTICAST" \ - "\23INET6_NOPRIVACY\24MPLS\25WOL\26AUTOCONF6" + "\23INET6_NOPRIVACY\24MPLS\25WOL\26AUTOCONF6\27INET6_NOSOII" void printb(char *, unsigned int, unsigned char *); diff --git sbin/ifconfig/ifconfig.8 sbin/ifconfig/ifconfig.8 index cac8eafc2cb..30721e41ad4 100644 --- sbin/ifconfig/ifconfig.8 +++ sbin/ifconfig/ifconfig.8 @@ -1070,6 +1070,7 @@ protocol when supported by the access point. .Op Oo Fl Oc Ns Cm autoconfprivacy .Op Cm eui64 .Op Cm pltime Ar n +.Op Oo Fl Oc Ns Cm soii .Op Oo Fl Oc Ns Cm tentative .Op Cm vltime Ar n .Ek @@ -1127,6 +1128,21 @@ Fill the interface index automatically. .It Cm pltime Ar n Set preferred lifetime for the address. +.It Cm soii +Enable persistent Semantically Opaque Interface Identifiers (SOIIs), +as per RFC 7217, for link local and SLAAC addresses on the interface. +These identifiers are enabled by default. +The purpose of these identifiers is to make discovery of hosts by +scanning a whole prefix more difficult. +SOOIs use the whole 64 bit of the host part while SLAAC addresses are +formed from MAC addresses and have 48 bits of entropy at most. +In reality entropy can be as low as 24 bit. +See RFC 8064 for details. +.It Cm -soii +Disable IPv6 persistent Semantically Opaque Interface Identifiers on the +interface. +Currently configured addresses will not be removed until they become +invalid. .It Cm tentative Set the IPv6 tentative address bit. .It Cm -tentative diff --git sbin/ifconfig/ifconfig.c sbin/ifconfig/ifconfig.c index d99bcb34871..bd7ce506398 100644 --- sbin/ifconfig/ifconfig.c +++ sbin/ifconfig/ifconfig.c @@ -388,6 +388,8 @@ const structcmd { { "eui64", 0, 0, setia6eui64 }, { "autoconfprivacy",-IFXF_INET6_NOPRIVACY, 0, setifxflags }, { "-autoconfprivacy", IFXF_INET6_NOPRIVACY, 0, setifxflags }, + { "soii", -IFXF_INET6_NOSOII, 0, setifxflags }, + { "-soii", IFXF_INET6_NOSOII, 0, setifxflags }, #ifndef SMALL { "hwfeatures", NEXTARG0, 0, printifhwfeatures }, { "metric", NEXTARG,0, setifmetric }, diff --git sbin/slaacd/control.c sbin/slaacd/control.c index 76b0f3b15ea..4f38d26331c 100644 --- sbin/slaacd/control.c +++ sbin/slaacd/control.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include diff --git sbin/slaacd/engine.c sbin/slaacd/engine.c index e02cb6ec3d0..cc7abd68512 100644 --- sbin/slaacd/engine.c +++ sbin/slaacd/engine.c @@ -70,6 +70,7 @@ #include #include #include +#include #include #include #include @@ -180,6 +181,7 @@ struct address_proposal { uint8_t prefix_len; uint32_t vltime; uint32_t pltime; + SIPHASH_KEY soiikey; }; struct dfr_proposal { @@ -205,8 +207,10 @@ struct slaacd_iface { uint32_t if_index; int running; int autoconfprivacy; + int soii; struct ether_addrhw_address; struct sockaddr_in6 ll_address; + SIPHASH_KEY so
OpenNTPD 6.1p1, 6.2p1 released
announce at openbsd.org Thu, 13 Jul 2017 10:41:48 -0500
OpenNTPD 6.1p1, 6.2p1 released
We have made two new portable OpenNTPD releases today. These should be arriving soon in the OpenNTPD directory of an OpenBSD mirror near you. OpenNTPD 6.1p1 represents the version shipped with OpenBSD 6.1. It provides a number of new features and reliability improvements. OpenNTPD 6.2p1 is the first development snapshot from what will become OpenBSD 6.2. Changes since OpenNTPD 6.0p1 * Quieted warnings about constraint connection retries. * Implemented fork+exec for ntpd child processes. * Added imsg inter-process reliability fixes. * Fixed memory leaks and reduced heap memory usage. * Numerous logging improvements and additions. * Added macOS 10.12 getentropy support. * Fixed arc4random blacklist use native implementations where possible. Changes since OpenNTPD 6.1p1 * Added option "query from " to ntpd.conf, to specify a local IP address for outgoing NTP queries. The libtls library, as shipped with LibreSSL 2.5.0 or later, is required to use the HTTPS constraint feature, though it is not required to use OpenNTPD.
Re: RFC 7217: random but stable addresses (take 2)
On Thu, Jul 13, 2017 at 03:43:50PM +, Florian Obser wrote: > It switches the hash function to SipHash24 from sha512 as suggested by dlg Is is performance critical? Then siphash would be better. Is is a security concern? Is is a problem that someone could try to calculate our secret when he knows a bunch of our IP addresses? Then sha512 would be better. I don't know wether the algorithm is relevant here. So I would have chosen sha512. bluhm
Re: sosend netlock assertion crash
On Thu, 13 Jul 2017 17:41:19 +0200, Alexander Bluhm wrote: > My laptop just crashed while running some php ports regress tests. > The kernel complained that it did not hold the netlock in sounlock() > after the out label in sosend(). The assert is correct, let's fix > the obvious offender. Alternately, we could just initialize s to -42. - todd
Re: sosend netlock assertion crash
On Thu, Jul 13, 2017 at 10:01:09AM -0600, Todd C. Miller wrote: > On Thu, 13 Jul 2017 17:41:19 +0200, Alexander Bluhm wrote: > > > My laptop just crashed while running some php ports regress tests. > > The kernel complained that it did not hold the netlock in sounlock() > > after the out label in sosend(). The assert is correct, let's fix > > the obvious offender. > > Alternately, we could just initialize s to -42. I still hope that this -42 hack will go away eventually. bluhm
Re: remove net.inet6.ip6.maxifdefrouters and net.inet6.ip6.maxifprefixes sysctls
On 2017/07/13 15:23, Florian Obser wrote: > pointed out by jmc, these sysctls are useless now, too, the kernel no > longer tracks prefixes or default routers from router advertisements. > OK? ports/shells/nsh will need patching, not sure about others yet, X crashed when I tried to search unpacked ports source..trying that one again now!
Re: sosend netlock assertion crash
On 13/07/17(Thu) 18:10, Alexander Bluhm wrote: > On Thu, Jul 13, 2017 at 10:01:09AM -0600, Todd C. Miller wrote: > > On Thu, 13 Jul 2017 17:41:19 +0200, Alexander Bluhm wrote: > > > > > My laptop just crashed while running some php ports regress tests. > > > The kernel complained that it did not hold the netlock in sounlock() > > > after the out label in sosend(). The assert is correct, let's fix > > > the obvious offender. > > > > Alternately, we could just initialize s to -42. > > I still hope that this -42 hack will go away eventually. It will. The first step is to run the diff I sent to get rid of the splsotnet()/splx() in the NET_LOCK().
pledge ifstated
The following diff is loosely based on the approach that was taken for pledging mountd. Other code/approaches leveraged from various networking daemons. This first step moves the ioctl with SIOCGIFDATA call to a privileged child so we can at least pledge "stdio rpath dns inet proc exec" without (intentionally) changing existing behaviour. Comments and suggestions welcome. Thanks! Rob Index: Makefile === RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v retrieving revision 1.10 diff -u -p -r1.10 Makefile --- Makefile20 Jul 2010 02:05:15 - 1.10 +++ Makefile13 Jul 2017 16:17:20 - @@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual YFLAGS= MAN= ifstated.8 ifstated.conf.5 -LDADD+=-levent +LDADD+=-levent -lutil DPADD+= ${LIBEVENT} .include Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.50 diff -u -p -r1.50 ifstated.c --- ifstated.c 4 Jul 2017 21:09:52 - 1.50 +++ ifstated.c 13 Jul 2017 16:17:20 - @@ -25,13 +25,16 @@ #include #include #include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -39,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +51,9 @@ #include "log.h" struct ifsd_config *conf = NULL, *newconf = NULL; +struct imsgbuf ibuf; +pid_t privchild_pid; int opts = 0; int opt_inhibit = 0; char *configfile = "/etc/ifstated.conf"; @@ -74,6 +80,7 @@ void do_action(struct ifsd_action *); void remove_action(struct ifsd_action *, struct ifsd_state *); void remove_expression(struct ifsd_expression *, struct ifsd_state *); +void privchild(int); __dead void usage(void) @@ -89,6 +96,7 @@ int main(int argc, char *argv[]) { struct timeval tv; + int socks[2]; int ch, rt_fd; int debug = 0; unsigned int rtfilter; @@ -143,6 +151,22 @@ main(int argc, char *argv[]) if (!debug) daemon(1, 0); + if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) { + fatal("%s: socketpair", __func__); + } + + switch (privchild_pid = fork()) { + case -1: + fatal("%s: fork", __func__); + case 0: + close(socks[0]); + privchild(socks[1]); + } + close(socks[1]); + + imsg_init(&ibuf, socks[0]); + setproctitle("parent"); + event_init(); log_init(debug, LOG_DAEMON); log_setverbose(opts & IFSD_OPT_VERBOSE); @@ -160,6 +184,9 @@ main(int argc, char *argv[]) &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); + if (pledge("stdio rpath dns inet proc exec", NULL) == -1) + fatal("pledge"); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); @@ -252,6 +279,16 @@ rt_msg_handler(int fd, short event, void void sigchld_handler(int fd, short event, void *arg) { + pid_t pid; + int status; + + do { + pid = waitpid(privchild_pid, &status, WNOHANG); + if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status))) + fatal("privchild %i exited due to receipt of signal %i", + privchild_pid, WTERMSIG(status)); + } while (pid == -1 && errno == EINTR); + check_external_status(&conf->initstate); if (conf->curstate != NULL) check_external_status(conf->curstate); @@ -599,29 +636,75 @@ do_action(struct ifsd_action *action) void fetch_ifstate(void) { + struct imsg imsg; struct ifaddrs *ifap, *ifa; + struct pollfd pfd[1]; char *oname = NULL; int sock = socket(AF_INET, SOCK_DGRAM, 0); + int done = 0, state, rv; + ssize_t n; if (getifaddrs(&ifap) != 0) err(1, "getifaddrs"); for (ifa = ifap; ifa; ifa = ifa->ifa_next) { struct ifreq ifr; - struct if_data ifrdat; if (oname && !strcmp(oname, ifa->ifa_name)) continue; oname = ifa->ifa_name; strlcpy(ifr.ifr_name, ifa->ifa_name, sizeof(ifr.ifr_name)); - ifr.ifr_data = (caddr_t)&ifrdat; - - if (ioctl(sock, SIOCGIFDATA, (caddr_t)&ifr) == -1) - continue; - scan_ifstate(if_nametoindex(ifa->ifa_name), - ifrdat.ifi_link_state, 0); + /* +* synchronous imsg request/response to privchild +*/ + if (imsg_compose(&ibuf, IMSG_IFSTATE_REQ, 0, 0, -1, &ifr, +
Re: [patch] Remove binc from vi(1)
No one? On 07/02/17 19:58, Martijn van Duren wrote: > Any takers? > > On 06/22/17 21:32, Martijn van Duren wrote: >> Hello tech@, >> >> Attached a patch to remove the binc function from vi and replace it with >> recallocarray. The functions effectively do the same thing since >> BINC_{GOTO,RET} already do the nlen > llen comparison. I've run >> this without any issues, but since recallocarray does extra checks and >> binc ALWAYS allocates A LOT more than requested there might be some >> bugs lurking. >> >> I haven't changed the name since the size component is always one and >> hence they don't expose a similar interface. >> >> OK? >> >> martijn@ >> >> Index: common/mem.h >> === >> RCS file: /cvs/src/usr.bin/vi/common/mem.h,v >> retrieving revision 1.9 >> diff -u -p -r1.9 mem.h >> --- common/mem.h 7 May 2016 14:03:01 - 1.9 >> +++ common/mem.h 22 Jun 2017 19:02:28 - >> @@ -14,30 +14,30 @@ >> /* Increase the size of a malloc'd buffer. Two versions, one that >> * returns, one that jumps to an error label. >> */ >> -#define BINC_GOTO(sp, lp, llen, nlen) { >> \ >> -void *L__bincp; \ >> +#define BINC_GOTO(sp, p, llen, nlen) { >> \ >> +void *tmpp; \ >> if ((nlen) > (llen)) { \ >> -if ((L__bincp = binc((sp), (lp), &(llen), (nlen))) \ >> -== NULL)\ >> +if (((tmpp) = recallocarray((p), (llen), (nlen), 1))\ >> +== NULL) { \ >> +msgq((sp), M_SYSERR, NULL); \ >> +free(p);\ >> goto alloc_err; \ >> -/* \ >> - * !!! \ >> - * Possible pointer conversion. \ >> - */ \ >> -(lp) = L__bincp;\ >> +} \ >> +llen = nlen;\ >> +(p) = tmpp; \ >> } \ >> } >> -#define BINC_RET(sp, lp, llen, nlen) { >> \ >> -void *L__bincp; \ >> +#define BINC_RET(sp, p, llen, nlen) { >> \ >> +void *tmpp; \ >> if ((nlen) > (llen)) { \ >> -if ((L__bincp = binc((sp), (lp), &(llen), (nlen))) \ >> -== NULL)\ >> +if (((tmpp) = recallocarray((p), (llen), (nlen), 1))\ >> +== NULL) { \ >> +msgq((sp), M_SYSERR, NULL); \ >> +free(p);\ >> return (1); \ >> -/* \ >> - * !!! \ >> - * Possible pointer conversion. \ >> - */ \ >> -(lp) = L__bincp;\ >> +} \ >> +llen = nlen;\ >> +(p) = tmpp; \ >> } \ >> } >> >> Index: common/util.c >> === >> RCS file: /cvs/src/usr.bin/vi/common/util.c,v >> retrieving revision 1.15 >> diff -u -p -r1.15 util.c >> --- common/util.c27 May 2016 09:18:11 - 1.15 >> +++ common/util.c22 Jun 2017 19:02:28 - >> @@ -24,43 +24,6 @@ >> >> #include "common.h" >> >> -#define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b)) >> - >> -/* >> - * binc -- >> - * Increase the size of a buffer. >> - * >> - * PUBLIC: void *binc(SCR *, void *, size_t *, size_t); >> - */ >> -void * >> -binc(SCR *sp, void *bp, size_t *bsizep, size_t min) >> -{ >> -size_t csize; >> - >> -/* If already larger than the min
fsck_ext2fs: Remove always false comparison
src/sbin/fsck_ext2fs/dir.c: In function 'dircheck': src/sbin/fsck_ext2fs/dir.c:241: warning: comparison is always false due to limited range of data type And indeed in /usr/include/ufs/ext2fs/ext2fs_dir.h e2d_namelen is a u_int8_t and a few lines above #define EXT2FS_MAXNAMLEN255 diff --git dir.c dir.c index 9864bacd0d3..105f5153997 100644 --- dir.c +++ dir.c @@ -237,8 +237,7 @@ dircheck(struct inodesc *idesc, struct ext2fs_direct *dp) return (1); size = EXT2FS_DIRSIZ(dp->e2d_namlen); if (reclen < size || - idesc->id_filesize < size || - dp->e2d_namlen > EXT2FS_MAXNAMLEN) + idesc->id_filesize < size) return (0); for (cp = dp->e2d_name, size = 0; size < dp->e2d_namlen; size++) if (*cp == '\0' || (*cp++ == '/'))
dhcpd - pf table handler child not cleaned up
Hi tech@, sthen@ pointed out to me that dhcpd doesn't properly terminate the pf table handler. I reproduced the issue both on 6.1 and -current. Minimal config I used on my server: /etc/dhcpd.conf subnet 45.63.9.186 netmask 255.255.255.224 { range 45.63.9.186 45.63.9.186; } enabled dhcpd and used the -A flag to trigger the pf handler being spawned: rcctl enable dhcpd rcctl set dhcpd flags -A test and performed the test # rcctl start dhcpd; rcctl stop dhcpd; pgrep -lf dhcpd dhcpd(ok) dhcpd(ok) 96584 dhcpd: pf table handler which demonstrates that indeed the pf table handler is left running. Both the main program and the child spin in a for(;;) loop, there is no code anticipating the need to signal a child to quit, there is also no attempt to detect the parent dying by the child. The child also uses a notreached exit(3) after the loop. When the parent is not around the unattended child proceeds to puke all over the logfiles: Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe brynet@ noticed the exit(3) instead of _exit(2) and he also pointed out that the pf table handler is a bit optimistic on warnings instead of bailing out with an error and terminating. Examples: 55if ((fd = open(_PATH_DEV_PF, O_RDWR|O_NOFOLLOW, 0660)) == -1) 56log_warn("can't open pf device"); this child is useless if it can't pf 57if (chroot(_PATH_VAREMPTY) == -1) 58log_warn("chroot %s", _PATH_VAREMPTY); 59if (chdir("/") == -1) 60log_warn("chdir(\"/\")"); 61if (setgroups(1, &pw->pw_gid) || 62setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) || 63setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid)) 64log_warn("can't drop privileges"); not able to chroot and drop privileges seems like good enough reason to die. 76if (nfds > 0 && (pfd[0].revents & POLLHUP)) 77log_warnx("pf pipe closed"); we won't get any work to do as the pipe is closed, should we die? Now, I did try a quick diff changing log_warn on those to fatal() https://junk.tintagel.pl/dhcpd-pf-handler.diff This of course results in the child dying when the parent is gone but we believe it's not commitable as the following needs to be decided: - if the handler fails (ie. not able to open pf) should it signal the main process to die or just stop working (like it did now)? - fatal() can't be used as it uses exit(3) instead of _exit(2) - anything else that we might have missed looking for pointers/suggestions on how to move forward with this. regards, Adam
Re: RFC 7217: random but stable addresses (take 2)
On Thu, Jul 13, 2017 at 05:59:24PM +0200, Alexander Bluhm wrote: > On Thu, Jul 13, 2017 at 03:43:50PM +, Florian Obser wrote: > > It switches the hash function to SipHash24 from sha512 as suggested by dlg > > Is is performance critical? Then siphash would be better. no > > Is is a security concern? Is is a problem that someone could try maybe.. > to calculate our secret when he knows a bunch of our IP addresses? > Then sha512 would be better. if you know the key and the mac you can track a host when it moves to a different prefix. > > I don't know wether the algorithm is relevant here. So I would > have chosen sha512. sha512 is certainly the conservative choice. note that we are only use 64 bit so the digest is a wee bit to big ;) I'm happy to bikeshed this for a bit since it kinda defeats the purpose if we need to change the hash function later. dlg? > > bluhm > -- I'm not entirely sure you are real.
Re: RFC 7217: random but stable addresses (take 2)
On 2017-07-13, Florian Obser wrote: > It switches the hash function to SipHash24 from sha512 as suggested by dlg It's for from clear to me whether SipHash is suitable for crypto operations, and which ones, other than the hash tables it was designed for. We went with SHA-512 for RFC 1948 TCP initial sequence number generation, which is arguably performance-sensitive, while RFC 7217 addresses are certainly not. I think we should use a SHA-2-family function such as SHA-512 here. > +# Apply soiikey.conf settings. > +soiikey_conf() { > + stripcom /etc/soiikey.conf | > + while read _line; do > + sysctl -q "net.inet6.ip6.soiikey=$_line" > + done > +} I think .conf is a strange choice of name for what is not a configuration file but effectively a private key, cf. /etc/{iked,isakmpd}/private/local.key /etc/ssh/ssh_host__key > +SOOIs use the whole 64 bit of the host part while SLAAC addresses are > +formed from MAC addresses and have 48 bits of entropy at most. 46 bits. (The first bit of a MAC address is 0 for unicast addresses, the second is 0 for "universally administered" addresses, i.e., those that are uniquely assigned to a device by its manufacturer.) -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: Add Diffie-Hellman group negotiation to iked
On 17-06-25 21:44:24, Tim Stewart wrote: > Hi, > > In this message I've tried to encode everything I've done to allow > strongSwan on Android to connect with iked, including the latest patch. > I have also verified that it breaks neither initial negotiation nor > Child SA rekeying for OpenBSD, Windows, and strongSwan (on Android) > clients. This patch gets my android phone much closer to being able to negotiate a connection, but there are still issues. Paraphrasing analysis mikeb performed on IRC: android sends incorrect (for us) group, and with this patch we now send a failure message and android retries. But, we don't increment msgid "because we did sa_free and restarted, so we can assume that android thinks that negotiation continues, that's why it re-sends the IKE_SA_INIT" > Stuart Henderson writes: > > > On 2017/05/22 01:52, Tim Stewart wrote: > >> Hello again, > >> > >> Tim Stewart writes: > >> > >> > Tim Stewart writes: > >> > > >> >> This patch teaches iked to reject a KE with a Notify payload of type > >> >> INVALID_KE_PAYLOAD when the KE uses a different Diffie-Hellman group > >> >> than is configured locally. The rejection indicates the desired > >> >> group. > >> >> > >> >> In my environment, this patch allows stock strongSwan on Android from > >> >> the Google Play store to interop with iked. strongSwan's logs show > >> >> the following once iked is patched: > >> >> > >> >> [IKE] initiating IKE_SA android[7] to 192.0.2.1 > >> >> [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) > >> >> N(NATD_D_IP) N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ] > >> >> [ENC] parsed IKE_SA_INIT response 0 [ N(INVAL_KE) ] > >> >> [IKE] peer didn't accept DH group ECP_256, it requested MODP_2048 > >> >> [IKE] initiating IKE_SA android[7] to 192.0.2.1 > >> >> [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) > >> >> N(NATD_D_IP) N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ] > >> >> [ENC] parsed IKE_SA_INIT response 0 [ SA KE No N(NATD_S_IP) > >> >> N(NATD_D_IP) CERTREQ N(HASH_ALG) ] > >> >> > >> >> I'm happy to iterate on this patch to get it into proper shape for > >> >> inclusion. > >> > > >> > I discovered a bug in the previous patch that broke renegotiation of > >> > CHILD SAs. I was ignoring "other than NONE" in the following sentence > >> > from RFC 5996 section 3.4: > >> > > >> > If the selected proposal uses a different Diffie-Hellman group > >> > (other than NONE), the message MUST be rejected with a Notify > >> > payload of type INVALID_KE_PAYLOAD. > >> > > >> > The new patch below repairs the flaw. > >> > >> After re-reading relevant parts of the RFC I'm not convinced that my fix > >> (rejecting with INVALID_KE_PAYLOAD unless msg->msg_dhgroup is > >> IKEV2_XFORMDH_NONE) is correct. It happens to resolve my local issue > >> but I think it may accidentally work due to a side effect of the code > >> path for rekeying a child SA. > >> > >> I will look at it more closely this week. > > My first patch did, in fact, break Child SAs rekeying. I have a new > patch at the end of this message that simply restricts DH group > negotiation to IKE SAs (I *think* that DH group guessing only applies to > IKE SAs, and perhaps only the IKE_SA_INIT exchange, but I'm still > working through the RFC). This may not be the ultimate solution, but it > does allow us to move forward. > > > Hi, I'm interested in this, but wasn't able to get strongswan to connect > > with either of your patches (and had iked exiting on one attempt, though > > I haven't been able to repeat that). > > After making changes I usually rebuild all of /usr/src/sbin/iked/ (make > clean && make). I started doing this after experiencing similar exits > and the problem went away. That could just be a coincidence, though! > > > If you have any updates please do send them here, it can be a bit slow > > getting feedback on iked diffs at times but it definitely is worth sending > > them out. > > I'll summarize what I know about strongSwan (on Android) and iked > interop. strongSwan chooses ECP_256 for its DH group guess when it > starts the IKE_SA_INIT exchange. The negotiation gets pretty far if I > specify `group ecp256' in the ikesa directive, but there there is some > incompatibility between iked and strongSwan that causes the following: > > ... > ikev2_recv: IKE_AUTH request from initiator 5.6.7.8:49317 to 1.2.3.4:4500 > policy 'default' id 1, 2040 bytes > ikev2_recv: ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8 > ikev2_recv: updated SA to peer 5.6.7.8:49317 local 1.2.3.4:4500 > ikev2_pld_parse: header ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8 > nextpayload SK version 0x20 exchange IKE_AUTH flags 0x08 msgid 1 length 2040 > response 0 > ikev2_pld_payloads: payload SK nextpayload IDi critical 0x00 length 2012 > ikev2_msg_decrypt: IV length 16 > ikev2_msg_decrypt: encrypted payload length 1968 > ikev2_msg_decrypt: integrity checksum length 24 > ikev2_msg_decrypt: integrity check faile
Re: add simple ifstated regression test script
Hi, i wanted to commit this, but saw that it does not have a licence yet. Can i add /usr/share/misc/license.template with your name and email-Adress? /Benno Rob Pierce(r...@2keys.ca) on 2017.07.06 13:12:26 -0400: > On Sun, Jul 02, 2017 at 06:29:07PM +0200, Sebastian Benoit wrote: > > Rob Pierce(r...@2keys.ca) on 2017.07.02 12:06:25 -0400: > > > I am currently using this regression script for basic ifstated sanity > > > testing. > > > > > > Still a work in progress. Requesting commit for safe keeping. > > > > > > Hi, > > > > this should go into /usr/src/regress/usr.sbin/ifstated > > (which does not esist yet). > > > > Also, it should hook into the regress framework (bsd.regress.mk(5)). > > > > As it needs some network configuration, maybe it should be similar to > > relayd regress tests. > > > > Happy to work with you on that. > > > > /B. > > I have updated the ifstated regression scripts based on your feedback. > > I also added a script to test drive the state machine. > > Both should be more systematic in coverage, but hopefully it is a good start. > > Regards, > > Rob > > Index: regress/usr.sbin/ifstated/Makefile > === > RCS file: regress/usr.sbin/ifstated/Makefile > diff -N regress/usr.sbin/ifstated/Makefile > --- /dev/null 1 Jan 1970 00:00:00 - > +++ regress/usr.sbin/ifstated/Makefile6 Jul 2017 16:55:57 - > @@ -0,0 +1,13 @@ > +# $OpenBSD$ > + > +# Regress tests for ifstated > + > +REGRESS_TARGETS =run-regress-statemachine run-regress-ifstated > + > +run-regress-statemachine: > + sh ${.CURDIR}/statemachine > + > +run-regress-ifstated: > + sh ${.CURDIR}/ifstated > + > +.include > > Index: regress/usr.sbin/ifstated/ifstated > === > RCS file: regress/usr.sbin/ifstated/ifstated > diff -N regress/usr.sbin/ifstated/ifstated > --- /dev/null 1 Jan 1970 00:00:00 - > +++ regress/usr.sbin/ifstated/ifstated6 Jul 2017 16:55:57 - > @@ -0,0 +1,148 @@ > +# $OpenBSD$ > + > +# Basic ifstated regression script to test interface changes. > + > +# Golbal variables > +VHIDA=252 > +VHIDB=253 > +PREFIX=172.16.0 > +DEMOTE=ifconfig > +PROMOTE=ifconfig > +EVERY=5 > +SLEEP=10 > + > +cleanup() { > + ifconfig carp${VHIDA} destroy > /dev/null 2>&1 > + ifconfig carp${VHIDB} destroy > /dev/null 2>&1 > + rm working/ifstated.conf >/dev/null 2>&1 > + rm working/ifstated.log >/dev/null 2>&1 > + rm working/output.test >/dev/null 2>&1 > + rm working/output.new >/dev/null 2>&1 > + rm working/nohup.out >/dev/null 2>&1 > + rmdir working >/dev/null 2>&1 > +} > + > +fail() { > + echo FAILED > + cleanup > + exit 1 > +} > + > +skip() { > + echo SKIPPED > + cleanup > + exit 0 > +} > + > +trap 'skip' INT > + > +# look for a suitable physical interface for carp > +NIC="$(netstat -rn -finet | grep ^default | awk '{ print $8 }')" > +STATUS="$(ifconfig | grep -A5 ^${NIC} | grep status: | awk '{ print $2 }')" > + > +if [ "$STATUS" != "active" ] > +then > + echo "No suitable physical interface found." > + echo SKIPPED > + exit 0 > +fi > + > +if [ "$(pgrep ifstated)" ] > +then > + echo "The ifstated daemon is already running." > + echo SKIPPED > + exit 0 > +fi > + > +for interface in carp${VHIDA} carp${VHIDB} > +do > + ifconfig ${interface} > /dev/null 2>&1 > + if [ $? -eq 0 ] > + then > + echo "Interface $interface already exists." > + echo SKIPPED > + exit 0 > + fi > +done > + > +mkdir -p working > + > +cat > working/ifstated.conf < +# This is a config template for ifstated regression testing > +carp = "carp${VHIDA}.link.up" > +init-state primary > +net = '( "ping -q -c 1 -w 1 ${PREFIX}.${VHIDB} > /dev/null" every ${EVERY})' > +state primary { > + init { > + run "ifconfig" > + } > + if ! \$net > + set-state demoted > + if ! \$carp > + set-state demoted > +} > +state demoted { > + init { > + run "ifconfig" > + } > + if \$net && \$carp > + set-state primary > +} > +EOF > + > +ifconfig carp${VHIDA} inet ${PREFIX}.${VHIDA} netmask 255.255.255.0 > broadcast \ > + ${PREFIX}.255 vhid ${VHIDA} carpdev ${NIC} > +ifconfig carp${VHIDB} inet ${PREFIX}.${VHIDB} netmask 255.255.255.0 > broadcast \ > + ${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC} > + > +# give the carp interface time to come up as MASTER > +sleep 5 > + > +cat > working/output.test < +changing state to primary > +changing state to demoted > +changing state to primary > +changing state to demoted > +changing state to primary > +changing state to demoted > +changing state to primary > +changing state to primary > +EOF > + > +(cd working && nohup ifstated -dvf ./ifstated.conf > ifstated.log 2>&1) & > + > +sleep ${SLEEP} > +ifconfig carp${VHIDA} down > +sleep ${SLEEP} > +ifconfig carp${VHIDA} up > +
Re: add simple ifstated regression test script
Sure, no problem. Thank you. Rob From: "Sebastian Benoit" To: "Rob Pierce" Cc: "tech" Sent: Thursday, July 13, 2017 6:12:14 PM Subject: Re: add simple ifstated regression test script BQ_BEGIN Hi, i wanted to commit this, but saw that it does not have a licence yet. Can i add /usr/share/misc/license.template with your name and email-Adress? /Benno Rob Pierce(r...@2keys.ca) on 2017.07.06 13:12:26 -0400: > On Sun, Jul 02, 2017 at 06:29:07PM +0200, Sebastian Benoit wrote: > > Rob Pierce(r...@2keys.ca) on 2017.07.02 12:06:25 -0400: > > > I am currently using this regression script for basic ifstated sanity > > > testing. > > > > > > Still a work in progress. Requesting commit for safe keeping. > > > > > > Hi, > > > > this should go into /usr/src/regress/usr.sbin/ifstated > > (which does not esist yet). > > > > Also, it should hook into the regress framework (bsd.regress.mk(5)). > > > > As it needs some network configuration, maybe it should be similar to > > relayd regress tests. > > > > Happy to work with you on that. > > > > /B. > > I have updated the ifstated regression scripts based on your feedback. > > I also added a script to test drive the state machine. > > Both should be more systematic in coverage, but hopefully it is a good start. > > Regards, > > Rob > > Index: regress/usr.sbin/ifstated/Makefile > === > RCS file: regress/usr.sbin/ifstated/Makefile > diff -N regress/usr.sbin/ifstated/Makefile > --- /dev/null 1 Jan 1970 00:00:00 - > +++ regress/usr.sbin/ifstated/Makefile 6 Jul 2017 16:55:57 - > @@ -0,0 +1,13 @@ > +# $OpenBSD$ > + > +# Regress tests for ifstated > + > +REGRESS_TARGETS = run-regress-statemachine run-regress-ifstated > + > +run-regress-statemachine: > + sh ${.CURDIR}/statemachine > + > +run-regress-ifstated: > + sh ${.CURDIR}/ifstated > + > +.include > > Index: regress/usr.sbin/ifstated/ifstated > === > RCS file: regress/usr.sbin/ifstated/ifstated > diff -N regress/usr.sbin/ifstated/ifstated > --- /dev/null 1 Jan 1970 00:00:00 - > +++ regress/usr.sbin/ifstated/ifstated 6 Jul 2017 16:55:57 - > @@ -0,0 +1,148 @@ > +# $OpenBSD$ > + > +# Basic ifstated regression script to test interface changes. > + > +# Golbal variables > +VHIDA=252 > +VHIDB=253 > +PREFIX=172.16.0 > +DEMOTE=ifconfig > +PROMOTE=ifconfig > +EVERY=5 > +SLEEP=10 > + > +cleanup() { > + ifconfig carp${VHIDA} destroy > /dev/null 2>&1 > + ifconfig carp${VHIDB} destroy > /dev/null 2>&1 > + rm working/ifstated.conf >/dev/null 2>&1 > + rm working/ifstated.log >/dev/null 2>&1 > + rm working/output.test >/dev/null 2>&1 > + rm working/output.new >/dev/null 2>&1 > + rm working/nohup.out >/dev/null 2>&1 > + rmdir working >/dev/null 2>&1 > +} > + > +fail() { > + echo FAILED > + cleanup > + exit 1 > +} > + > +skip() { > + echo SKIPPED > + cleanup > + exit 0 > +} > + > +trap 'skip' INT > + > +# look for a suitable physical interface for carp > +NIC="$(netstat -rn -finet | grep ^default | awk '{ print $8 }')" > +STATUS="$(ifconfig | grep -A5 ^${NIC} | grep status: | awk '{ print $2 }')" > + > +if [ "$STATUS" != "active" ] > +then > + echo "No suitable physical interface found." > + echo SKIPPED > + exit 0 > +fi > + > +if [ "$(pgrep ifstated)" ] > +then > + echo "The ifstated daemon is already running." > + echo SKIPPED > + exit 0 > +fi > + > +for interface in carp${VHIDA} carp${VHIDB} > +do > + ifconfig ${interface} > /dev/null 2>&1 > + if [ $? -eq 0 ] > + then > + echo "Interface $interface already exists." > + echo SKIPPED > + exit 0 > + fi > +done > + > +mkdir -p working > + > +cat > working/ifstated.conf < +# This is a config template for ifstated regression testing > +carp = "carp${VHIDA}.link.up" > +init-state primary > +net = '( "ping -q -c 1 -w 1 ${PREFIX}.${VHIDB} > /dev/null" every ${EVERY})' > +state primary { > + init { > + run "ifconfig" > + } > + if ! \$net > + set-state demoted > + if ! \$carp > + set-state demoted > +} > +state demoted { > + init { > + run "ifconfig" > + } > + if \$net && \$carp > + set-state primary > +} > +EOF > + > +ifconfig carp${VHIDA} inet ${PREFIX}.${VHIDA} netmask 255.255.255.0 > broadcast \ > + ${PREFIX}.255 vhid ${VHIDA} carpdev ${NIC} > +ifconfig carp${VHIDB} inet ${PREFIX}.${VHIDB} netmask 255.255.255.0 > broadcast \ > + ${PREFIX}.255 vhid ${VHIDB} carpdev ${NIC} > + > +# give the carp interface time to come up as MASTER > +sleep 5 > + > +cat > working/output.test < +changing state to primary > +changing state to demoted > +changing state to primary > +changing state to demoted > +changing state to primary > +changing state to demoted > +changing state to primary > +changing state to primary > +EOF > + > +(cd working && nohup ifstated -dvf ./ifstated.conf > if
time(1): make global flags local
The flags don't need to be global, and there are more obvious ways to zero a variable. While here, order the stack structures and variables by size. -- Scott Cheloha Index: usr.bin/time/time.c === RCS file: /cvs/src/usr.bin/time/time.c,v retrieving revision 1.22 diff -u -p -r1.22 time.c --- usr.bin/time/time.c 13 Jul 2017 06:39:54 - 1.22 +++ usr.bin/time/time.c 14 Jul 2017 00:10:09 - @@ -42,19 +42,17 @@ #include #include -int lflag; -int portableflag; - __dead void usage(void); int main(int argc, char *argv[]) { - pid_t pid; - int ch, status; - struct timespec before, after, during; struct rusage ru; - int exitonsig = 0; + struct timespec before, after, during; + pid_t pid; + int ch, exitonsig, lflag, portableflag, status; + + exitonsig = lflag = portableflag = 0; if (pledge("stdio proc exec", NULL) == -1) err(1, "pledge");
time(1): kill some NOTREACHEDs
style(9) says these can go. -- Scott Cheloha Index: usr.bin/time/time.c === RCS file: /cvs/src/usr.bin/time/time.c,v retrieving revision 1.22 diff -u -p -r1.22 time.c --- usr.bin/time/time.c 13 Jul 2017 06:39:54 - 1.22 +++ usr.bin/time/time.c 14 Jul 2017 00:16:39 - @@ -69,10 +69,8 @@ main(int argc, char *argv[]) break; default: usage(); - /* NOTREACHED */ } } - argc -= optind; argv += optind; @@ -84,12 +82,10 @@ main(int argc, char *argv[]) case -1:/* error */ perror("time"); exit(1); - /* NOTREACHED */ case 0: /* child */ execvp(*argv, argv); perror(*argv); _exit((errno == ENOENT) ? 127 : 126); - /* NOTREACHED */ } /* parent */
armv7 _dcache_wbinv_all, or _dcache_wb_all ?
Hi, i'm having hard time choosing, between this diff, and the one that renames it to for what it does, which would likely incur cleanup elsewhere. current is just wrong.. but i'm open to be taught on this matter anyway. -Artturi diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c index c91108e7066..0631ba8fa78 100644 --- a/sys/arch/arm/arm/cpufunc.c +++ b/sys/arch/arm/arm/cpufunc.c @@ -291,8 +291,8 @@ armv7_dcache_wbinv_all() for (ways = 0; ways < nways; ways++) { word = wayval | setval | lvl; - /* Clean D cache SE with Set/Index */ - __asm volatile("mcr p15, 0, %0, c7, c10, 2" + /* Data cache clean and invalidate by set/way */ + __asm volatile("mcr p15, 0, rr, c7, c14, 2\n" : : "r" (word)); wayval += wayincr; }
time(1): perror(3) -> err(3) and friends
We currently use a mix of perror(3) and err(3). In one case you can merge perror + exit into err, which is nice. The warns, though, are not equivalent (you get a "time: " prefix), so maybe this is too risky. Putting it out here anyway. -- Scott Cheloha Index: usr.bin/time/time.c === RCS file: /cvs/src/usr.bin/time/time.c,v retrieving revision 1.22 diff -u -p -r1.22 time.c --- usr.bin/time/time.c 13 Jul 2017 06:39:54 - 1.22 +++ usr.bin/time/time.c 14 Jul 2017 00:24:59 - @@ -82,12 +82,11 @@ main(int argc, char *argv[]) clock_gettime(CLOCK_MONOTONIC, &before); switch(pid = vfork()) { case -1:/* error */ - perror("time"); - exit(1); + err(1, NULL); /* NOTREACHED */ case 0: /* child */ execvp(*argv, argv); - perror(*argv); + warn("%s", *argv); _exit((errno == ENOENT) ? 127 : 126); /* NOTREACHED */ } @@ -170,7 +169,7 @@ main(int argc, char *argv[]) if (exitonsig) { if (signal(exitonsig, SIG_DFL) == SIG_ERR) - perror("signal"); + warn("signal"); else kill(getpid(), exitonsig); }
Re: armv7 _dcache_wbinv_all, or _dcache_wb_all ?
On Fri, Jul 14, 2017 at 03:28:25AM +0300, Artturi Alm wrote: > Hi, > > i'm having hard time choosing, between this diff, and the one that > renames it to for what it does, which would likely incur cleanup > elsewhere. current is just wrong.. but i'm open to be taught on this > matter anyway. > > -Artturi > Wonders of manually recreating a quick diff by hand above.. fixed: diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c index c91108e7066..fd5385043fc 100644 --- a/sys/arch/arm/arm/cpufunc.c +++ b/sys/arch/arm/arm/cpufunc.c @@ -291,8 +291,8 @@ armv7_dcache_wbinv_all() for (ways = 0; ways < nways; ways++) { word = wayval | setval | lvl; - /* Clean D cache SE with Set/Index */ - __asm volatile("mcr p15, 0, %0, c7, c10, 2" + /* Data cache clean and invalidate by set/way */ + __asm volatile("mcr p15, 0, %0, c7, c14, 2\n" : : "r" (word)); wayval += wayincr; }
time(1): kill some lint-era voids, switch to getprogname(3)
The (void) casts are going out of style. While here, switch from __progname to getprogname(3) -- Scott Cheloha Index: usr.bin/time/time.c === RCS file: /cvs/src/usr.bin/time/time.c,v retrieving revision 1.22 diff -u -p -r1.22 time.c --- usr.bin/time/time.c 13 Jul 2017 06:39:54 - 1.22 +++ usr.bin/time/time.c 14 Jul 2017 00:34:33 - @@ -93,8 +93,8 @@ main(int argc, char *argv[]) } /* parent */ - (void)signal(SIGINT, SIG_IGN); - (void)signal(SIGQUIT, SIG_IGN); + signal(SIGINT, SIG_IGN); + signal(SIGQUIT, SIG_IGN); while (wait3(&status, 0, &ru) != pid) ; clock_gettime(CLOCK_MONOTONIC, &after); @@ -180,9 +180,7 @@ main(int argc, char *argv[]) __dead void usage(void) { - extern char *__progname; - - (void)fprintf(stderr, "usage: %s [-lp] utility [argument ...]\n", - __progname); + fprintf(stderr, "usage: %s [-lp] utility [argument ...]\n", + getprogname()); exit(1); }
Re: RFC 7217: random but stable addresses (take 2)
> On 14 Jul 2017, at 06:30, Christian Weisgerber wrote: > > On 2017-07-13, Florian Obser wrote: > >> It switches the hash function to SipHash24 from sha512 as suggested by dlg > > It's for from clear to me whether SipHash is suitable for crypto > operations, and which ones, other than the hash tables it was > designed for. the abstract on the siphash paper says: "SipHash is a family of pseudorandom functions optimized for short inputs. Target applications include network traffic authentication and hash-table lookups protected against hash-flooding denial-of-service attacks." i somehow have the impression that it's use for hash tables was a convenient bonus rather than a specific design target of the algorithm. it was handy that it is good at spreading bits from a small input into its output. > > We went with SHA-512 for RFC 1948 TCP initial sequence number > generation, which is arguably performance-sensitive, while RFC 7217 > addresses are certainly not. I think we should use a SHA-2-family > function such as SHA-512 here. i dont mind which way we go, but i would like to make two (possibly invalid) arguments. firstly, we're generating public information. if anyone wants to "collide" with us, the can just copy and use the same ip, regardless of how we got there. as long as the key is strong enough we should be able to prevent prediction of other soii based addresses. if we're trying to protect against recovery of information, specifically the mac address, we could just not feed that into the hash. use the name of the interface instead, which is what netstart uses to assign addresses too. secondly, im always wary of truncating hash output in case it throws away some of the guarantees it's supposed to provide. if you cut sha512 output down to an 8th of its size, is it 8 times easier to calculate a collision, or more than 8 times easier? sha384 being a truncation of sha512 kind of argues against this though. with regard to the code, siphash implied a length for soiikey which makes the sysctl handling a lot easier. if (when?) we go back to sha512 id like to see a fixed key length remain. we can make it bigger, but just keep it one size. > >> +# Apply soiikey.conf settings. >> +soiikey_conf() { >> +stripcom /etc/soiikey.conf | >> +while read _line; do >> +sysctl -q "net.inet6.ip6.soiikey=$_line" >> +done >> +} > > I think .conf is a strange choice of name for what is not a > configuration file but effectively a private key, cf. > > /etc/{iked,isakmpd}/private/local.key > /etc/ssh/ssh_host__key id also like to see key in the name, not conf. > >> +SOOIs use the whole 64 bit of the host part while SLAAC addresses are >> +formed from MAC addresses and have 48 bits of entropy at most. > > 46 bits. > (The first bit of a MAC address is 0 for unicast addresses, the > second is 0 for "universally administered" addresses, i.e., those > that are uniquely assigned to a device by its manufacturer.) > > -- > Christian "naddy" Weisgerber na...@mips.inka.de >
Re: pledge ifstated
On Mon, Jul 10, 2017 at 01:21:58PM -0400, Rob Pierce wrote: > The following diff is loosely based on the approach that was taken for > pledging mountd. Other code/approaches leveraged from various networking > daemons. > > This first step moves the ioctl with SIOCGIFDATA call to a privileged > child so we can at least pledge "stdio rpath dns inet proc exec" without > (intentionally) changing existing behaviour. > > Comments and suggestions welcome. > > Thanks! > > Rob An unnecessary call to log_info snuck in. Here is a clean diff. Rob Index: Makefile === RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v retrieving revision 1.10 diff -u -p -r1.10 Makefile --- Makefile20 Jul 2010 02:05:15 - 1.10 +++ Makefile14 Jul 2017 01:12:58 - @@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual YFLAGS= MAN= ifstated.8 ifstated.conf.5 -LDADD+=-levent +LDADD+=-levent -lutil DPADD+= ${LIBEVENT} .include Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.50 diff -u -p -r1.50 ifstated.c --- ifstated.c 4 Jul 2017 21:09:52 - 1.50 +++ ifstated.c 14 Jul 2017 01:12:58 - @@ -25,13 +25,16 @@ #include #include #include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -39,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +51,9 @@ #include "log.h" struct ifsd_config *conf = NULL, *newconf = NULL; +struct imsgbuf ibuf; +pid_t privchild_pid; int opts = 0; int opt_inhibit = 0; char *configfile = "/etc/ifstated.conf"; @@ -74,6 +80,7 @@ void do_action(struct ifsd_action *); void remove_action(struct ifsd_action *, struct ifsd_state *); void remove_expression(struct ifsd_expression *, struct ifsd_state *); +void privchild(int); __dead void usage(void) @@ -89,6 +96,7 @@ int main(int argc, char *argv[]) { struct timeval tv; + int socks[2]; int ch, rt_fd; int debug = 0; unsigned int rtfilter; @@ -143,6 +151,22 @@ main(int argc, char *argv[]) if (!debug) daemon(1, 0); + if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) { + fatal("%s: socketpair", __func__); + } + + switch (privchild_pid = fork()) { + case -1: + fatal("%s: fork", __func__); + case 0: + close(socks[0]); + privchild(socks[1]); + } + close(socks[1]); + + imsg_init(&ibuf, socks[0]); + setproctitle("parent"); + event_init(); log_init(debug, LOG_DAEMON); log_setverbose(opts & IFSD_OPT_VERBOSE); @@ -160,6 +184,9 @@ main(int argc, char *argv[]) &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */ log_warn("%s: setsockopt tablefilter", __func__); + if (pledge("stdio rpath dns inet proc exec", NULL) == -1) + fatal("pledge"); + signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL); signal_add(&sigchld_ev, NULL); @@ -252,6 +279,16 @@ rt_msg_handler(int fd, short event, void void sigchld_handler(int fd, short event, void *arg) { + pid_t pid; + int status; + + do { + pid = waitpid(privchild_pid, &status, WNOHANG); + if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status))) + fatal("privchild %i exited due to receipt of signal %i", + privchild_pid, WTERMSIG(status)); + } while (pid == -1 && errno == EINTR); + check_external_status(&conf->initstate); if (conf->curstate != NULL) check_external_status(conf->curstate); @@ -599,29 +636,74 @@ do_action(struct ifsd_action *action) void fetch_ifstate(void) { + struct imsg imsg; struct ifaddrs *ifap, *ifa; + struct pollfd pfd[1]; char *oname = NULL; int sock = socket(AF_INET, SOCK_DGRAM, 0); + int done = 0, state, rv; + ssize_t n; if (getifaddrs(&ifap) != 0) err(1, "getifaddrs"); for (ifa = ifap; ifa; ifa = ifa->ifa_next) { struct ifreq ifr; - struct if_data ifrdat; if (oname && !strcmp(oname, ifa->ifa_name)) continue; oname = ifa->ifa_name; strlcpy(ifr.ifr_name, ifa->ifa_name, sizeof(ifr.ifr_name)); - ifr.ifr_data = (caddr_t)&ifrdat; - - if (ioctl(sock, SIOCGIFDATA, (caddr_t)&ifr) == -1) - continue; - scan_ifstate(if_nametoindex(ifa->ifa_name), - ifrdat.ifi_link_state, 0); + /* +
Re: RFC 7217: random but stable addresses (take 2)
David Gwynne wrote: > secondly, im always wary of truncating hash output in case it throws away > some of the guarantees it's supposed to provide. if you cut sha512 output > down to an 8th of its size, is it 8 times easier to calculate a collision, or > more than 8 times easier? sha384 being a truncation of sha512 kind of argues > against this though. on this point, all the bits are supposed to be equally good. you can take as many as you want, and just like that, you have an X bit hash.
Re: RFC 7217: random but stable addresses (take 2)
On Thu, Jul 13, 2017 at 08:30:55PM -, Christian Weisgerber wrote: > On 2017-07-13, Florian Obser wrote: [...] > > +# Apply soiikey.conf settings. > > +soiikey_conf() { > > + stripcom /etc/soiikey.conf | > > + while read _line; do > > + sysctl -q "net.inet6.ip6.soiikey=$_line" > > + done > > +} > > I think .conf is a strange choice of name for what is not a > configuration file but effectively a private key, cf. > > /etc/{iked,isakmpd}/private/local.key > /etc/ssh/ssh_host__key > Ugh, yes, brain fart > > +SOOIs use the whole 64 bit of the host part while SLAAC addresses are > > +formed from MAC addresses and have 48 bits of entropy at most. > > 46 bits. > (The first bit of a MAC address is 0 for unicast addresses, the > second is 0 for "universally administered" addresses, i.e., those > that are uniquely assigned to a device by its manufacturer.) Yes, I know, that's why I used "at most", and yes, you cannot actually reach it. The point is to give the casual reader ball park figures: 128 bit: Yeah, I'm not going to try 64 bit: Probably fine, doesn't cost me any sleep 48 bit: Hmm... 24 bit: Hold my beer and watch this, I can send a million packets with my phone! I'm afraid if we put in 46 people will start to wonder why 46 while they might have heard that a mac is 48 bits. Anyway, I'll try to come up with wording to avoid exact numbers. > > -- > Christian "naddy" Weisgerber na...@mips.inka.de > -- I'm not entirely sure you are real.
Re: RFC 7217: random but stable addresses (take 2)
On Fri, Jul 14, 2017 at 11:06:49AM +1000, David Gwynne wrote: > > > On 14 Jul 2017, at 06:30, Christian Weisgerber wrote: > > > > On 2017-07-13, Florian Obser wrote: > > > >> It switches the hash function to SipHash24 from sha512 as suggested by dlg > > > > It's for from clear to me whether SipHash is suitable for crypto > > operations, and which ones, other than the hash tables it was > > designed for. > > the abstract on the siphash paper says: > > "SipHash is a family of pseudorandom functions optimized for short inputs. > Target applications include network traffic authentication and hash-table > lookups protected against hash-flooding denial-of-service attacks." > > i somehow have the impression that it's use for hash tables was a convenient > bonus rather than a specific design target of the algorithm. it was handy > that it is good at spreading bits from a small input into its output. > > > > > We went with SHA-512 for RFC 1948 TCP initial sequence number > > generation, which is arguably performance-sensitive, while RFC 7217 > > addresses are certainly not. I think we should use a SHA-2-family > > function such as SHA-512 here. > > i dont mind which way we go, but i would like to make two (possibly invalid) > arguments. > > firstly, we're generating public information. if anyone wants to "collide" > with us, the can just copy and use the same ip, regardless of how we got > there. as long as the key is strong enough we should be able to prevent > prediction of other soii based addresses. if we're trying to protect against > recovery of information, specifically the mac address, we could just not feed > that into the hash. use the name of the interface instead, which is what > netstart uses to assign addresses too. > > secondly, im always wary of truncating hash output in case it throws away > some of the guarantees it's supposed to provide. if you cut sha512 output > down to an 8th of its size, is it 8 times easier to calculate a collision, or > more than 8 times easier? sha384 being a truncation of sha512 kind of argues > against this though. My understanding of siphash is, too, that we are probably fine with using it, but then what do I know about hash functions... And since we are arguing about it and this is not performance critical at all we shouldn't half-arse it and just go with sha512. > > with regard to the code, siphash implied a length for soiikey which makes the > sysctl handling a lot easier. if (when?) we go back to sha512 id like to see > a fixed key length remain. we can make it bigger, but just keep it one size. Yes, there is no point for a user to set a key of a specific length. It's a randomly generated opaque blob. I'll send an updated diff shortly addressing all the input so far, thanks! -- I'm not entirely sure you are real.