Re: TIMESPEC_TO_NSEC(): futex(2), __thrsigdivert(2) & __thrsleep(2)
On Sun, Jan 12, 2020 at 4:44 AM Martin Pieuchot wrote: > The consensus is now to switch syscall doing sleeps to use tsleep_nsec(9). > Our direct goal is to stop expressing time as ticks, more might come > later. > The API futex(2) has a bug: it doesn't take a clockid_t and doesn't have a way to take an absolute timeout, which means userspace has to do non-ideal conversions to work around those limitations. While it might(?) make sense to keep futex(2) the way it is to support ports that use that exact API**, it feels like we should support a richer API to make libc/libpthread happier...and perhaps not have the insane argument unuse/reuse/overloading that futex(2) has (e.g., "pass uint32_t val2 as the fourth argument instead of timeout"). Let's say a diff magically appeared splitting out three syscalls for the three ops, with correct types and args and where timeouts were specified with timespec+clockid_t+"is absolute" flag, can that be slotted into all this without wailing and gnashing of teeth? ** I have no idea if any ports actually do that Philip Guenther
Re: sosleep(), SO_RCVTIMEO and TIMEVAL_TO_NSEC()
On Sun, Jan 12, 2020 at 01:33:43PM +0100, Martin Pieuchot wrote:
> On 09/01/20(Thu) 16:10, Martin Pieuchot wrote:
> > SO_RCVTIMEO and SO_SNDTIMEO allow userland to specify a timeout value
> > via a 'struct timeval'. Internally the kernel keeps this time
> > representation in ticks. Diff below changes that to nanoseconds which
> > allows us to use tsleep_nsec(9) in sosleep().
> >
> > That means we need a new conversion routine: TIMEVAL_TO_NSEC(). Here
> > again I screwed up the overflow check. If somebody could fix my C, I'd
> > be glad.
> >
> > The rest of the diff is trivial. Note that SO_RCVTIMEO & SO_SNDTIMEO no
> > longer return EDOM since we now have enough space for storing the
> > interval. We could limit that if desired.
>
> Updated diff addressing previous comments, any more inputs?
One more buglet below, otherwise ok cheloha@.
> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.238
> diff -u -p -r1.238 uipc_socket.c
> --- kern/uipc_socket.c31 Dec 2019 13:48:32 - 1.238
> +++ kern/uipc_socket.c12 Jan 2020 12:29:30 -
> @@ -155,6 +155,8 @@ socreate(int dom, struct socket **aso, i
> so->so_egid = p->p_ucred->cr_gid;
> so->so_cpid = p->p_p->ps_pid;
> so->so_proto = prp;
> + so->so_snd.sb_timeo_nsecs = INFSLP;
> + so->so_rcv.sb_timeo_nsecs = INFSLP;
>
> s = solock(so);
> error = (*prp->pr_attach)(so, proto);
> @@ -304,7 +306,7 @@ soclose(struct socket *so, int flags)
> while (so->so_state & SS_ISCONNECTED) {
> error = sosleep(so, &so->so_timeo,
> PSOCK | PCATCH, "netcls",
> - so->so_linger * hz);
> + SEC_TO_NSEC(so->so_linger));
> if (error)
> break;
> }
> @@ -1761,22 +1763,25 @@ sosetopt(struct socket *so, int level, i
> case SO_RCVTIMEO:
> {
> struct timeval tv;
> - int val;
> + uint64_t nsecs;
>
> if (m == NULL || m->m_len < sizeof (tv))
> return (EINVAL);
> memcpy(&tv, mtod(m, struct timeval *), sizeof tv);
> - val = tvtohz(&tv);
> - if (val > USHRT_MAX)
> + if (!timerisvalid(&tv))
> return (EDOM);
This should be EINVAL, not EDOM.
> -
> + nsecs = TIMEVAL_TO_NSEC(&tv);
> + if (nsecs == UINT64_MAX)
> + return (EDOM);
> + if (nsecs == 0)
> + nsecs = INFSLP;
> switch (optname) {
>
> case SO_SNDTIMEO:
> - so->so_snd.sb_timeo = val;
> + so->so_snd.sb_timeo_nsecs = nsecs;
> break;
> case SO_RCVTIMEO:
> - so->so_rcv.sb_timeo = val;
> + so->so_rcv.sb_timeo_nsecs = nsecs;
> break;
> }
> break;
> @@ -1910,13 +1915,14 @@ sogetopt(struct socket *so, int level, i
> case SO_RCVTIMEO:
> {
> struct timeval tv;
> - int val = (optname == SO_SNDTIMEO ?
> - so->so_snd.sb_timeo : so->so_rcv.sb_timeo);
> + uint64_t nsecs = (optname == SO_SNDTIMEO ?
> + so->so_snd.sb_timeo_nsecs :
> + so->so_rcv.sb_timeo_nsecs);
>
> m->m_len = sizeof(struct timeval);
> memset(&tv, 0, sizeof(tv));
> - tv.tv_sec = val / hz;
> - tv.tv_usec = (val % hz) * tick;
> + if (nsecs != INFSLP)
> + NSEC_TO_TIMEVAL(nsecs, &tv);
> memcpy(mtod(m, struct timeval *), &tv, sizeof tv);
> break;
> }
> @@ -2129,7 +2135,7 @@ sobuf_print(struct sockbuf *sb,
> (*pr)("\tsb_sel: ...\n");
> (*pr)("\tsb_flagsintr: %d\n", sb->sb_flagsintr);
> (*pr)("\tsb_flags: %i\n", sb->sb_flags);
> - (*pr)("\tsb_timeo: %i\n", sb->sb_timeo);
> + (*pr)("\tsb_timeo_nsecs: %llu\n", sb->sb_timeo_nsecs);
> }
>
> void
> Index: kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 uipc_socket2.c
> --- kern/uipc_socket2.c 16 Apr 2019 13:15:32 - 1.101
> +++ kern/uipc_socket2.c 12 J
Re: IPL of timeout_set_proc(9)
On Sun, Jan 12, 2020 at 12:52:53PM +0100, Martin Pieuchot wrote: > On 11/01/20(Sat) 16:12, Scott Cheloha wrote: > > On Sat, Jan 11, 2020 at 05:41:00PM +0100, Martin Pieuchot wrote: > > > Before converting network timeouts to run in a thread context they were > > > executed in a soft-interrupt handler. This design implied that timeouts > > > were serialized. > > > > Yep. > > > > > The current "softclock" thread runs on CPU0 to limit border effects due > > > to the conversion from soft-interrupt to thread context. > > > > Define "border effects". Do you mean limiting the delay between when > > softclock() yields and softclock_thread() runs? > > timeout_hardclock_update() is just executed by CPU0, right? Yes, only on the primary CPU. > So aren't softclock handlers also executed on CPU0? If so, executing the > softclock only on CPU0 is a conservative approach. > > Whatever bug we could expose by running the handler on another CPU isn't > interesting for the moment. That's what I meant with "border effects". Gotcha, alright, sure. Makes sense. > > > However we should raise the IPL level of this thread to ensure no other > > > timeout can run in the middle of another one. > > > > This makes sense, though I have a few questions. Sorry in advance if > > some of this is obvious, my understanding of the "rules" governing the > > interrupt context is still limited. > > > > 1. If softclock() takes more than a full tick to complete and is > >interrupted by hardclock(9), which schedules another softclock(), > >but *before* softclock() returns it wakes up softclock_thread()... > >which of those will run first? > > > >The softclock() interrupt or the softclock_thread()? > > Why do you ask? Without this change we have a precedence: in the scenario I described the interrupt will always run before the thread. Now that they both run with the same IPL I am unsure as to which would run first. I assume the interrupt but I want to make sure because if the order is undefined we might be breaking stuff that depends upon it. > > 2. If both process and vanilla timeouts run at IPL_SOFTCLOCK, what > >really is the difference between them? Is there still a reason to > >distinguish between them? Would it make sense to run *all* timeouts > >from the softclock thread once we have a chance to fork it from > >process 0? > > As always is a matter of exposing and debugging the unknown problems > related to any change. So why would you like to change the existing? There was work a while ago to move all MI interrupts to dedicated threads. Near as I can tell the only one that wasn't moved was softclock(). So I look at this change and the shrinking difference between running timeouts from the interrupt context and running them from the thread and I have to ask: why not just eliminate the interrupt and run them all from the thread? > > 3. Is there a way to prioritize the softclock thread over other > >threads on the primary CPU so that the scheduler makes it runnable > >as soon as possible after softclock() returns to reduce latency? > > Why do you ask? If we could prioritize the timeout thread to run before others we could guarantee that it runs "soon after" the hardclock(9), removing one possible barrier to moving them to run from the thread. Again, I don't know whether this is possible, but I look at the move from interrupts to threads and now the change in IPL levels and question the need for an interrupt if we could do an equivalent job with a thread.
Re: ospf(6)d.conf: define interface parameters per area or globally
Theo de Raadt(dera...@openbsd.org) on 2020.01.12 12:03:40 -0700:
> Remi Locherer wrote:
>
> > On Sun, Jan 12, 2020 at 04:18:26PM +0100, Claudio Jeker wrote:
> > > On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:
> > > > On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > > > > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > > > > I have a diff to allow parameters after interface or area
> > > > > > > definition.
> > > > > > > Not sure if we want to do that though.
> > > > > >
> > > > > > I would appreciate that! ;-)
> > > > > >
> > > > >
> > > > > The ospfd diff needs some more work. Crypt authentication handling is
> > > > > not
> > > > > perfect.
> > > >
> > > > This works fine for me and the diff reads good. I tested ospfd and
> > > > ospf6d.
> > > > Also the crypt options for ospfd worked fine.
> > > >
> > > > ok remi@
> > >
> > > Currently all daemons behave the same way and inherit at the moment of
> > > creation. Having this behave different between daemons is confusing.
> > > At least ospfd and bgpd should behave the same. Not saying that the
> > > current behaviour is great.
> > > I think in the case of ospfd the way auth-md is handled by this diff is
> > > not comparable with the behaviour of the other settings.
> >
> > I agree. But that should not stop us improving one program before the
> > other ones.
> >
> > >
> > > area 0.0.0.0 {
> > > hello-interval 10
> > > auth-md 1 foo
> > >
> > > interface em0
> > >
> > > hello-interval 20
> > > auth-md 1 bar
> > > auth-md 2 foofoo
> > >
> > > interface em1 {
> > > auth-md 3 barbar
> > > }
> > >
> > > hello-interval 30
> > > auth-md 1 bay
> > > auth-md 2 foobar
> > > }
> > >
> > > What values for hello-interval and auth-md should be set on em0 and em1?
> > >
> >
> > To me it looks natural if the latest value per level is used. With your
> > example that would be:
> >
> > em0:
> > - auth-md 1 "bay"
> > - auth-md 2 "foobar"
> > - hello-interval 30
> >
> > em1:
> > - auth-md 1 "bay"
> > - auth-md 2 "foobar"
> > - auth-md 3 "barbar"
> > - hello-interval 30
> >
> > In my testing this is the result of the diff from Denis. (I modified
> > printconf.c to print the keys to see the results).
>
> I think that is very dangerous. In some other daemons it could be
> disastrous.
>
>
> > Another option would be to make it an error specifying the same option
> > more than once at the same level.
>
> I think that will be the easier solution.
I too think that should be an error.
Why should you specify it twice?
> The approach of "collect all the root info first, then apply to the
> children aftwards" will be difficult to apply to all our
> domain-specific-grammer-daemons.
We should keep it the same in the routing daemons. I think the rest is
different here and there already.
/Benno
Re: apply changes immediately to join'd essids
On Sun, Jan 12, 2020 at 09:39:19PM +0100, Peter Hessler wrote:
> When we change attributes for a join essid, we should apply the change
> immediately instead of waiting to (randomly) switch away and switch
> back.
>
> Found by martijn@
>
> OK?
>
> Index: net80211/ieee80211_ioctl.c
> ===
> RCS file: /home/cvs/openbsd/src/sys/net80211/ieee80211_ioctl.c,v
> retrieving revision 1.77
> diff -u -p -u -p -r1.77 ieee80211_ioctl.c
> --- net80211/ieee80211_ioctl.c11 Nov 2019 18:07:21 - 1.77
> +++ net80211/ieee80211_ioctl.c12 Jan 2020 18:38:44 -
> @@ -540,6 +540,17 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
> error = ENETRESET;
> }
> } else {
> + /*
> + * We are reconfiguring the active essid,
Perhaps say "If we are reconfiguring..., reset the interface." instead?
Whether we're reconfiguring depends on the result of the check you're
adding below, while the comment makes it sound as if this decision had
been made further up. Alternatively we could just remove the comment.
> + * so reset the interface.
> + */
> + if (ic->ic_des_esslen == join.i_len &&
> + memcmp(join.i_nwid, ic->ic_des_essid,
> + join.i_len) == 0) {
> + ieee80211_deselect_ess(ic);
> + error = ENETRESET;
> + }
> +
> /* save nwid for auto-join */
> if (ieee80211_add_ess(ic, &join) == 0)
> ic->ic_flags |= IEEE80211_F_AUTO_JOIN;
>
apply changes immediately to join'd essids
When we change attributes for a join essid, we should apply the change
immediately instead of waiting to (randomly) switch away and switch
back.
Found by martijn@
OK?
Index: net80211/ieee80211_ioctl.c
===
RCS file: /home/cvs/openbsd/src/sys/net80211/ieee80211_ioctl.c,v
retrieving revision 1.77
diff -u -p -u -p -r1.77 ieee80211_ioctl.c
--- net80211/ieee80211_ioctl.c 11 Nov 2019 18:07:21 - 1.77
+++ net80211/ieee80211_ioctl.c 12 Jan 2020 18:38:44 -
@@ -540,6 +540,17 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
error = ENETRESET;
}
} else {
+ /*
+* We are reconfiguring the active essid,
+* so reset the interface.
+*/
+ if (ic->ic_des_esslen == join.i_len &&
+ memcmp(join.i_nwid, ic->ic_des_essid,
+ join.i_len) == 0) {
+ ieee80211_deselect_ess(ic);
+ error = ENETRESET;
+ }
+
/* save nwid for auto-join */
if (ieee80211_add_ess(ic, &join) == 0)
ic->ic_flags |= IEEE80211_F_AUTO_JOIN;
--
If the American dream is for Americans only, it will remain our dream
and never be our destiny.
-- René de Visme Williamson
Re: ospf(6)d.conf: define interface parameters per area or globally
Remi Locherer wrote:
> On Sun, Jan 12, 2020 at 04:18:26PM +0100, Claudio Jeker wrote:
> > On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:
> > > On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > > > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > > > I have a diff to allow parameters after interface or area
> > > > > > definition.
> > > > > > Not sure if we want to do that though.
> > > > >
> > > > > I would appreciate that! ;-)
> > > > >
> > > >
> > > > The ospfd diff needs some more work. Crypt authentication handling is
> > > > not
> > > > perfect.
> > >
> > > This works fine for me and the diff reads good. I tested ospfd and ospf6d.
> > > Also the crypt options for ospfd worked fine.
> > >
> > > ok remi@
> >
> > Currently all daemons behave the same way and inherit at the moment of
> > creation. Having this behave different between daemons is confusing.
> > At least ospfd and bgpd should behave the same. Not saying that the
> > current behaviour is great.
> > I think in the case of ospfd the way auth-md is handled by this diff is
> > not comparable with the behaviour of the other settings.
>
> I agree. But that should not stop us improving one program before the
> other ones.
>
> >
> > area 0.0.0.0 {
> > hello-interval 10
> > auth-md 1 foo
> >
> > interface em0
> >
> > hello-interval 20
> > auth-md 1 bar
> > auth-md 2 foofoo
> >
> > interface em1 {
> > auth-md 3 barbar
> > }
> >
> > hello-interval 30
> > auth-md 1 bay
> > auth-md 2 foobar
> > }
> >
> > What values for hello-interval and auth-md should be set on em0 and em1?
> >
>
> To me it looks natural if the latest value per level is used. With your
> example that would be:
>
> em0:
> - auth-md 1 "bay"
> - auth-md 2 "foobar"
> - hello-interval 30
>
> em1:
> - auth-md 1 "bay"
> - auth-md 2 "foobar"
> - auth-md 3 "barbar"
> - hello-interval 30
>
> In my testing this is the result of the diff from Denis. (I modified
> printconf.c to print the keys to see the results).
I think that is very dangerous. In some other daemons it could be
disastrous.
> Another option would be to make it an error specifying the same option
> more than once at the same level.
I think that will be the easier solution.
The approach of "collect all the root info first, then apply to the
children aftwards" will be difficult to apply to all our
domain-specific-grammer-daemons.
Re: ospf(6)d.conf: define interface parameters per area or globally
On Sun, Jan 12, 2020 at 04:18:26PM +0100, Claudio Jeker wrote:
> On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:
> > On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > > I have a diff to allow parameters after interface or area definition.
> > > > > Not sure if we want to do that though.
> > > >
> > > > I would appreciate that! ;-)
> > > >
> > >
> > > The ospfd diff needs some more work. Crypt authentication handling is not
> > > perfect.
> >
> > This works fine for me and the diff reads good. I tested ospfd and ospf6d.
> > Also the crypt options for ospfd worked fine.
> >
> > ok remi@
>
> Currently all daemons behave the same way and inherit at the moment of
> creation. Having this behave different between daemons is confusing.
> At least ospfd and bgpd should behave the same. Not saying that the
> current behaviour is great.
> I think in the case of ospfd the way auth-md is handled by this diff is
> not comparable with the behaviour of the other settings.
I agree. But that should not stop us improving one program before the
other ones.
>
> area 0.0.0.0 {
> hello-interval 10
> auth-md 1 foo
>
> interface em0
>
> hello-interval 20
> auth-md 1 bar
> auth-md 2 foofoo
>
> interface em1 {
> auth-md 3 barbar
> }
>
> hello-interval 30
> auth-md 1 bay
> auth-md 2 foobar
> }
>
> What values for hello-interval and auth-md should be set on em0 and em1?
>
To me it looks natural if the latest value per level is used. With your
example that would be:
em0:
- auth-md 1 "bay"
- auth-md 2 "foobar"
- hello-interval 30
em1:
- auth-md 1 "bay"
- auth-md 2 "foobar"
- auth-md 3 "barbar"
- hello-interval 30
In my testing this is the result of the diff from Denis. (I modified
printconf.c to print the keys to see the results).
Another option would be to make it an error specifying the same option
more than once at the same level.
While looking closer I noticed, that the default value for auth-md-keyid
is set to 0 while the manual says it is 1. But that is not a change
introduced by this diff.
> > >
> > > Index: ospf6d/ospf6d.h
> > > ===
> > > RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
> > > retrieving revision 1.44
> > > diff -u -p -r1.44 ospf6d.h
> > > --- ospf6d/ospf6d.h 3 Jan 2020 17:45:02 - 1.44
> > > +++ ospf6d/ospf6d.h 8 Jan 2020 12:11:20 -
> > > @@ -328,7 +328,7 @@ struct iface {
> > > enum iface_type type;
> > > u_int8_t if_type;
> > > u_int8_t linkstate;
> > > - u_int8_t priority;
> > > + int16_t priority;
> > > u_int8_t p2p;
> > > u_int8_t cflags;
> > > #define F_IFACE_PASSIVE 0x01
> > > Index: ospf6d/parse.y
> > > ===
> > > RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
> > > retrieving revision 1.48
> > > diff -u -p -r1.48 parse.y
> > > --- ospf6d/parse.y26 Dec 2019 10:24:18 - 1.48
> > > +++ ospf6d/parse.y8 Jan 2020 12:11:20 -
> > > @@ -101,7 +101,7 @@ struct config_defaults {
> > > u_int16_t hello_interval;
> > > u_int16_t rxmt_interval;
> > > u_int16_t metric;
> > > - u_int8_tpriority;
> > > + int16_t priority;
> > > };
> > >
> > > struct config_defaultsglobaldefs;
> > > @@ -111,6 +111,7 @@ struct config_defaults*defs;
> > >
> > > struct area *conf_get_area(struct in_addr);
> > > int conf_check_rdomain(u_int);
> > > +void iface_settings(struct iface *, struct config_defaults
> > > *);
> > >
> > > typedef struct {
> > > union {
> > > @@ -465,9 +466,14 @@ comma: ','
> > > area : AREA areaid {
> > > area = conf_get_area($2);
> > >
> > > - memcpy(&areadefs, defs, sizeof(areadefs));
> > > + memset(&areadefs, 0, sizeof(areadefs));
> > > + areadefs.priority = -1;
> > > defs = &areadefs;
> > > } '{' optnl areaopts_l '}' {
> > > + struct iface*i;
> > > + LIST_FOREACH(i, &area->iface_list, entry) {
> > > + iface_settings(i, &areadefs);
> > > + }
> > > area = NULL;
> > > defs = &globaldefs;
> > > }
> > > @@ -540,15 +546,12 @@ interface : INTERFACE STRING {
> > > iface->area = area;
> > > LIST_INSERT_HEAD(&area->iface_list, iface, entry);
> > >
> > > - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> > > + memset(&ifacedefs, 0, sizeof(ifacedefs));
> > > + ifacedefs.priority = -1;
> > >
iked(8): better cryptographic defaults
Hi,
I was looking at iked's cryptographic defaults and noticed
that there's some weak/deprecated primitives while we do not
propose some of the newer (more secure/faster) algorithms.
3DES is considered weak since https://sweet32.info/ and was removed
from OpenSSL in 2016. Logjam and https://weakdh.org/ broke some of the
classical DH groups. The researchers who found it recommend
2048-bit or larger MODP groups or switching to ECDH.
AES-GCM and CHACHA20 can be considerably faster than AES-CBC+HMAC-SHA1
and are well established. The only downside is that GCM heavily
depends on CPU support, so newer Intel/AMD CPUs will be much faster with
GCM. For everything else CHACHA20 might actually be faster (compare
`openssl speed aes-256-gcm/chacha20-poly1305`).
I would also like to add all DH groups in ikev2_default_ike_transforms
to the ikev2_default_ipsec_transforms as perfect forward secrecy for ESP
is generally considered best practice.
SHA1 can stay as it is only used in a HMAC construction which is still
considered secure (see https://sha-mbles.github.io/).
Any strong opinions against any of those changes?
diff --git a/sbin/iked/parse.y b/sbin/iked/parse.y
index fe052068922..7d4158d2242 100644
--- a/sbin/iked/parse.y
+++ b/sbin/iked/parse.y
@@ -140,25 +140,45 @@ struct iked_transform ikev2_default_ike_transforms[] = {
{ IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 256 },
{ IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 },
{ IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 },
- { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_3DES },
+ { IKEV2_XFORMTYPE_PRF, IKEV2_XFORMPRF_HMAC_SHA2_512 },
+ { IKEV2_XFORMTYPE_PRF, IKEV2_XFORMPRF_HMAC_SHA2_384 },
{ IKEV2_XFORMTYPE_PRF, IKEV2_XFORMPRF_HMAC_SHA2_256 },
{ IKEV2_XFORMTYPE_PRF, IKEV2_XFORMPRF_HMAC_SHA1 },
+ { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_512_256 },
+ { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_384_192 },
{ IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
{ IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_CURVE25519 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_521 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_384 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_256 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_4096 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_3072 },
{ IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_2048 },
- { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_1536 },
- { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_1024 },
{ 0 }
};
size_t ikev2_default_nike_transforms = ((sizeof(ikev2_default_ike_transforms) /
sizeof(ikev2_default_ike_transforms[0])) - 1);
struct iked_transform ikev2_default_esp_transforms[] = {
+ { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_GCM_16, 256 },
+ { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_GCM_16, 192 },
+ { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_GCM_16, 128 },
+ { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_CHACHA20_POLY1305 },
{ IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 256 },
{ IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 },
{ IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 },
+ { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_512_256 },
+ { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_384_192 },
{ IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 },
{ IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_CURVE25519 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_521 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_384 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_256 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_4096 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_3072 },
+ { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_2048 },
{ IKEV2_XFORMTYPE_ESN, IKEV2_XFORMESN_ESN },
{ IKEV2_XFORMTYPE_ESN, IKEV2_XFORMESN_NONE },
{ 0 }
Re: ospf(6)d.conf: define interface parameters per area or globally
On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:
> On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > I have a diff to allow parameters after interface or area definition.
> > > > Not sure if we want to do that though.
> > >
> > > I would appreciate that! ;-)
> > >
> >
> > The ospfd diff needs some more work. Crypt authentication handling is not
> > perfect.
>
> This works fine for me and the diff reads good. I tested ospfd and ospf6d.
> Also the crypt options for ospfd worked fine.
>
> ok remi@
Currently all daemons behave the same way and inherit at the moment of
creation. Having this behave different between daemons is confusing.
At least ospfd and bgpd should behave the same. Not saying that the
current behaviour is great.
I think in the case of ospfd the way auth-md is handled by this diff is
not comparable with the behaviour of the other settings.
area 0.0.0.0 {
hello-interval 10
auth-md 1 foo
interface em0
hello-interval 20
auth-md 1 bar
auth-md 2 foofoo
interface em1 {
auth-md 3 barbar
}
hello-interval 30
auth-md 1 bay
auth-md 2 foobar
}
What values for hello-interval and auth-md should be set on em0 and em1?
> >
> > Index: ospf6d/ospf6d.h
> > ===
> > RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
> > retrieving revision 1.44
> > diff -u -p -r1.44 ospf6d.h
> > --- ospf6d/ospf6d.h 3 Jan 2020 17:45:02 - 1.44
> > +++ ospf6d/ospf6d.h 8 Jan 2020 12:11:20 -
> > @@ -328,7 +328,7 @@ struct iface {
> > enum iface_type type;
> > u_int8_t if_type;
> > u_int8_t linkstate;
> > - u_int8_t priority;
> > + int16_t priority;
> > u_int8_t p2p;
> > u_int8_t cflags;
> > #define F_IFACE_PASSIVE0x01
> > Index: ospf6d/parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
> > retrieving revision 1.48
> > diff -u -p -r1.48 parse.y
> > --- ospf6d/parse.y 26 Dec 2019 10:24:18 - 1.48
> > +++ ospf6d/parse.y 8 Jan 2020 12:11:20 -
> > @@ -101,7 +101,7 @@ struct config_defaults {
> > u_int16_t hello_interval;
> > u_int16_t rxmt_interval;
> > u_int16_t metric;
> > - u_int8_tpriority;
> > + int16_t priority;
> > };
> >
> > struct config_defaults globaldefs;
> > @@ -111,6 +111,7 @@ struct config_defaults *defs;
> >
> > struct area*conf_get_area(struct in_addr);
> > int conf_check_rdomain(u_int);
> > +voidiface_settings(struct iface *, struct config_defaults
> > *);
> >
> > typedef struct {
> > union {
> > @@ -465,9 +466,14 @@ comma : ','
> > area : AREA areaid {
> > area = conf_get_area($2);
> >
> > - memcpy(&areadefs, defs, sizeof(areadefs));
> > + memset(&areadefs, 0, sizeof(areadefs));
> > + areadefs.priority = -1;
> > defs = &areadefs;
> > } '{' optnl areaopts_l '}' {
> > + struct iface*i;
> > + LIST_FOREACH(i, &area->iface_list, entry) {
> > + iface_settings(i, &areadefs);
> > + }
> > area = NULL;
> > defs = &globaldefs;
> > }
> > @@ -540,15 +546,12 @@ interface : INTERFACE STRING {
> > iface->area = area;
> > LIST_INSERT_HEAD(&area->iface_list, iface, entry);
> >
> > - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> > + memset(&ifacedefs, 0, sizeof(ifacedefs));
> > + ifacedefs.priority = -1;
> > defs = &ifacedefs;
> > } interface_block {
> > - iface->dead_interval = defs->dead_interval;
> > - iface->transmit_delay = defs->transmit_delay;
> > - iface->hello_interval = defs->hello_interval;
> > - iface->rxmt_interval = defs->rxmt_interval;
> > - iface->metric = defs->metric;
> > - iface->priority = defs->priority;
> > + iface->priority = -1;
> > + iface_settings(iface, defs);
> > iface->cflags |= F_IFACE_CONFIGURED;
> > iface = NULL;
> > /* interface is always part of an area */
> > @@ -1018,6 +1021,8 @@ popfile(void)
> > struct ospfd_conf *
> > parse_config(char *filename, int opts)
> > {
> > + struct area *a;
> > + struct iface*i;
> > struct sym *sym, *next;
> >
> > if ((
Re: ospf(6)d.conf: define interface parameters per area or globally
On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > I have a diff to allow parameters after interface or area definition.
> > > Not sure if we want to do that though.
> >
> > I would appreciate that! ;-)
> >
>
> The ospfd diff needs some more work. Crypt authentication handling is not
> perfect.
This works fine for me and the diff reads good. I tested ospfd and ospf6d.
Also the crypt options for ospfd worked fine.
ok remi@
>
> Index: ospf6d/ospf6d.h
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
> retrieving revision 1.44
> diff -u -p -r1.44 ospf6d.h
> --- ospf6d/ospf6d.h 3 Jan 2020 17:45:02 - 1.44
> +++ ospf6d/ospf6d.h 8 Jan 2020 12:11:20 -
> @@ -328,7 +328,7 @@ struct iface {
> enum iface_type type;
> u_int8_t if_type;
> u_int8_t linkstate;
> - u_int8_t priority;
> + int16_t priority;
> u_int8_t p2p;
> u_int8_t cflags;
> #define F_IFACE_PASSIVE 0x01
> Index: ospf6d/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
> retrieving revision 1.48
> diff -u -p -r1.48 parse.y
> --- ospf6d/parse.y26 Dec 2019 10:24:18 - 1.48
> +++ ospf6d/parse.y8 Jan 2020 12:11:20 -
> @@ -101,7 +101,7 @@ struct config_defaults {
> u_int16_t hello_interval;
> u_int16_t rxmt_interval;
> u_int16_t metric;
> - u_int8_tpriority;
> + int16_t priority;
> };
>
> struct config_defaultsglobaldefs;
> @@ -111,6 +111,7 @@ struct config_defaults*defs;
>
> struct area *conf_get_area(struct in_addr);
> int conf_check_rdomain(u_int);
> +void iface_settings(struct iface *, struct config_defaults *);
>
> typedef struct {
> union {
> @@ -465,9 +466,14 @@ comma: ','
> area : AREA areaid {
> area = conf_get_area($2);
>
> - memcpy(&areadefs, defs, sizeof(areadefs));
> + memset(&areadefs, 0, sizeof(areadefs));
> + areadefs.priority = -1;
> defs = &areadefs;
> } '{' optnl areaopts_l '}' {
> + struct iface*i;
> + LIST_FOREACH(i, &area->iface_list, entry) {
> + iface_settings(i, &areadefs);
> + }
> area = NULL;
> defs = &globaldefs;
> }
> @@ -540,15 +546,12 @@ interface : INTERFACE STRING {
> iface->area = area;
> LIST_INSERT_HEAD(&area->iface_list, iface, entry);
>
> - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> + memset(&ifacedefs, 0, sizeof(ifacedefs));
> + ifacedefs.priority = -1;
> defs = &ifacedefs;
> } interface_block {
> - iface->dead_interval = defs->dead_interval;
> - iface->transmit_delay = defs->transmit_delay;
> - iface->hello_interval = defs->hello_interval;
> - iface->rxmt_interval = defs->rxmt_interval;
> - iface->metric = defs->metric;
> - iface->priority = defs->priority;
> + iface->priority = -1;
> + iface_settings(iface, defs);
> iface->cflags |= F_IFACE_CONFIGURED;
> iface = NULL;
> /* interface is always part of an area */
> @@ -1018,6 +1021,8 @@ popfile(void)
> struct ospfd_conf *
> parse_config(char *filename, int opts)
> {
> + struct area *a;
> + struct iface*i;
> struct sym *sym, *next;
>
> if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
> @@ -1068,6 +1073,10 @@ parse_config(char *filename, int opts)
> }
> }
>
> + LIST_FOREACH(a, &conf->area_list, entry)
> + LIST_FOREACH(i, &a->iface_list, entry)
> + iface_settings(i, defs);
> +
> /* check that all interfaces belong to the configured rdomain */
> errors += conf_check_rdomain(conf->rdomain);
>
> @@ -1319,4 +1328,21 @@ prefix(const char *s, struct in6_addr *a
> }
> *plen = 128;
> return (host(s, addr));
> +}
> +
> +void
> +iface_settings(struct iface *i, struct config_defaults *cfg)
> +{
> + if (i->dead_interval == 0)
> + i->dead_interval = cfg->dead_interval;
> + if (i->transmit_delay == 0)
> + i->transmit_delay = cfg->transmit_delay;
> + if (i->hello_interval == 0)
> + i->hello_interval = cfg->hello_int
Re: MAKE: redux patch
On Sat, 11 Jan 2020 12:34:23 +0100, Marc Espie wrote: > Oh, the test is wrong, and it's now enough to have make complain about it. > > Before the patch, errors in .END and .BEGIN were not properly looked at. > > See, that -f ... && will be *false* if the file doesn't exist. > The way to this kind of test without failing is > > if [ -f ... ]; then ... fi Right, this is a common Makefile issue. OK millert@ - todd
TIMESPEC_TO_NSEC(): futex(2), __thrsigdivert(2) & __thrsleep(2)
The consensus is now to switch syscall doing sleeps to use tsleep_nsec(9).
Our direct goal is to stop expressing time as ticks, more might come
later.
Diff below introduces the previously discussed TIMESPEC_TO_NSEC(9) macro
and makes use of it in 3 syscalls.
Comments? Oks?
Index: kern/kern_sig.c
===
RCS file: /cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.240
diff -u -p -r1.240 kern_sig.c
--- kern/kern_sig.c 8 Jan 2020 16:27:41 - 1.240
+++ kern/kern_sig.c 12 Jan 2020 12:36:40 -
@@ -1699,7 +1699,7 @@ sys___thrsigdivert(struct proc *p, void
sigset_t *m;
sigset_t mask = SCARG(uap, sigmask) &~ sigcantmask;
siginfo_t si;
- uint64_t to_ticks = 0;
+ uint64_t nsecs = INFSLP;
int timeinvalid = 0;
int error = 0;
@@ -1715,14 +1715,8 @@ sys___thrsigdivert(struct proc *p, void
#endif
if (!timespecisvalid(&ts))
timeinvalid = 1;
- else {
- to_ticks = (uint64_t)hz * ts.tv_sec +
- ts.tv_nsec / (tick * 1000);
- if (to_ticks > INT_MAX)
- to_ticks = INT_MAX;
- if (to_ticks == 0 && ts.tv_nsec)
- to_ticks = 1;
- }
+ else
+ nsecs = TIMESPEC_TO_NSEC(&ts);
}
dosigsuspend(p, p->p_sigmask &~ mask);
@@ -1749,14 +1743,14 @@ sys___thrsigdivert(struct proc *p, void
if (timeinvalid)
error = EINVAL;
- if (SCARG(uap, timeout) != NULL && to_ticks == 0)
+ if (SCARG(uap, timeout) != NULL && nsecs == INFSLP)
error = EAGAIN;
if (error != 0)
break;
- error = tsleep(&sigwaitsleep, PPAUSE|PCATCH, "sigwait",
- (int)to_ticks);
+ error = tsleep_nsec(&sigwaitsleep, PPAUSE|PCATCH, "sigwait",
+ nsecs);
}
if (error == 0) {
Index: kern/sys_futex.c
===
RCS file: /cvs/src/sys/kern/sys_futex.c,v
retrieving revision 1.13
diff -u -p -r1.13 sys_futex.c
--- kern/sys_futex.c10 Jul 2019 15:52:17 - 1.13
+++ kern/sys_futex.c31 Dec 2019 10:19:49 -
@@ -213,7 +213,7 @@ futex_wait(uint32_t *uaddr, uint32_t val
{
struct proc *p = curproc;
struct futex *f;
- uint64_t to_ticks = 0;
+ uint64_t nsecs = INFSLP;
uint32_t cval;
int error;
@@ -244,17 +244,14 @@ futex_wait(uint32_t *uaddr, uint32_t val
#endif
if (ts.tv_sec < 0 || !timespecisvalid(&ts))
return EINVAL;
- to_ticks = (uint64_t)hz * ts.tv_sec +
- (ts.tv_nsec + tick * 1000 - 1) / (tick * 1000) + 1;
- if (to_ticks > INT_MAX)
- to_ticks = INT_MAX;
+ nsecs = TIMESPEC_TO_NSEC(&ts);
}
f = futex_get(uaddr, flags | FT_CREATE);
TAILQ_INSERT_TAIL(&f->ft_threads, p, p_fut_link);
p->p_futex = f;
- error = rwsleep(p, &ftlock, PWAIT|PCATCH, "fsleep", (int)to_ticks);
+ error = rwsleep_nsec(p, &ftlock, PWAIT|PCATCH, "fsleep", nsecs);
if (error == ERESTART)
error = ECANCELED;
else if (error == EWOULDBLOCK) {
Index: kern/kern_synch.c
===
RCS file: /cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.156
diff -u -p -r1.156 kern_synch.c
--- kern/kern_synch.c 12 Jan 2020 00:01:12 - 1.156
+++ kern/kern_synch.c 12 Jan 2020 11:42:20 -
@@ -641,7 +641,7 @@ thrsleep(struct proc *p, struct sys___th
long ident = (long)SCARG(uap, ident);
struct timespec *tsp = (struct timespec *)SCARG(uap, tp);
void *lock = SCARG(uap, lock);
- uint64_t to_ticks = 0;
+ uint64_t nsecs = INFSLP;
int abort, error;
clockid_t clock_id = SCARG(uap, clock_id);
@@ -665,10 +665,7 @@ thrsleep(struct proc *p, struct sys___th
}
timespecsub(tsp, &now, tsp);
- to_ticks = (uint64_t)hz * tsp->tv_sec +
- (tsp->tv_nsec + tick * 1000 - 1) / (tick * 1000) + 1;
- if (to_ticks > INT_MAX)
- to_ticks = INT_MAX;
+ nsecs = TIMESPEC_TO_NSEC(tsp);
}
p->p_thrslpid = ident;
@@ -692,8 +689,7 @@ thrsleep(struct proc *p, struct sys___th
void *sleepaddr = &p->p_thrslpid;
if (ident == -1)
sleepaddr = &globalsleepaddr;
- error = tsleep(sleepaddr, PWAIT|PCATCH, "thrsleep",
- (int)to_ticks);
+ error = tsleep_nsec(sleepaddr, PWAIT|PCATCH, "thrsleep", nsecs);
}
ou
Re: Fix manual description in SSL_CTX_add_extra_chain_cert.3
Hello Ingo-san, Thanks for your answer. Today, I had read through this manual, and I just thought that the newly added function was missed in the description section at last update. But now I had understood the status of manual maintenance by your explanation. I just read only one manual page and had not took consider about others. I should have thought about this from comprehensive perspective of view. I would like to abandon this patch for now. Regards, Kinichiro Inoguchi On Sun, Jan 12, 2020 at 10:46:26AM +0100, Ingo Schwarze wrote: > Hello Kinichiro-san, > > Kinichiro Inoguchi wrote on Sun, Jan 12, 2020 at 05:09:52PM +0900: > > > I think both SSL_CTX_get_extra_chain_certs and > > SSL_CTX_get_extra_chain_certs_only should be described here. > > I think the text describing what to do with internal pointers > returned from LibreSSL functions is still quite a mess in general. > In some cases (like this one) something is said explicitly, > but there are many different wordings. In other cases, nothing > is said explicitly. > > In the case at hand, the sentence you are adding to feels somewhat > redundant because the main description already says "retrieves an > internal pointer". The only reason i didn't delete the sentence > when last editing the page is because we didn't come round to a > general cleanup of such statements yet, and adding or deleting such > statements piece-meal without a comprehensive plan didn't feel like > a big improvement. > > Regarding the particular change you propose, on top of the above, > i think what you are adding is already clear from the text above, > which explains the the pointers potentially returned from > SSL_CTX_get_extra_chain_certs_only(3) are a subset of those potentially > returned from SSL_CTX_get_extra_chain_certs(3). So if the latter > mustn't be freed, that includes the former. > > > ok? > > I don't consider the patch a real improvement, but i don't strongly > object to it either. What you are proposing to say is certainly > not wrong. > > > More generally, what do people think to switching to the following > concise wording: > > SOMEOBJ_add0(obj, p) "transfers ownership of t to obj" > (or "sets the FOO of obj to p, transfering > ownership", depending on the context) > SOMEOBJ_add1(obj, p) "sets the FOO of obj to a (deep|shallow) > copy of p" > p = SOMEOBJ_get0(obj) "retrieves an internal pointer to the FOO of obj" > p = SOMEOBJ_get1(obj) "returns a (deep|shallow) copy of the FOO of obj" > > And then delete all the repetitive wordings like "you must free" > or "you must not free"? > > Yours, > Ingo > > > > Index: SSL_CTX_add_extra_chain_cert.3 > > === > > RCS file: /cvs/src/lib/libssl/man/SSL_CTX_add_extra_chain_cert.3,v > > retrieving revision 1.7 > > diff -u -p -u -p -r1.7 SSL_CTX_add_extra_chain_cert.3 > > --- SSL_CTX_add_extra_chain_cert.3 2 Jan 2020 09:09:16 - 1.7 > > +++ SSL_CTX_add_extra_chain_cert.3 12 Jan 2020 08:06:49 - > > @@ -115,7 +115,9 @@ An application should not free the > > object, nor the > > .Pf * Fa certs > > object retrieved by > > -.Fn SSL_CTX_get_extra_chain_certs . > > +.Fn SSL_CTX_get_extra_chain_certs > > +and > > +.Fn SSL_CTX_get_extra_chain_certs_only . > > .Sh RETURN VALUES > > These functions return 1 on success or 0 for failure. > > Check out the error stack to find out the reason for failure.
Re: sosleep(), SO_RCVTIMEO and TIMEVAL_TO_NSEC()
On 09/01/20(Thu) 16:10, Martin Pieuchot wrote:
> SO_RCVTIMEO and SO_SNDTIMEO allow userland to specify a timeout value
> via a 'struct timeval'. Internally the kernel keeps this time
> representation in ticks. Diff below changes that to nanoseconds which
> allows us to use tsleep_nsec(9) in sosleep().
>
> That means we need a new conversion routine: TIMEVAL_TO_NSEC(). Here
> again I screwed up the overflow check. If somebody could fix my C, I'd
> be glad.
>
> The rest of the diff is trivial. Note that SO_RCVTIMEO & SO_SNDTIMEO no
> longer return EDOM since we now have enough space for storing the
> interval. We could limit that if desired.
Updated diff addressing previous comments, any more inputs?
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.238
diff -u -p -r1.238 uipc_socket.c
--- kern/uipc_socket.c 31 Dec 2019 13:48:32 - 1.238
+++ kern/uipc_socket.c 12 Jan 2020 12:29:30 -
@@ -155,6 +155,8 @@ socreate(int dom, struct socket **aso, i
so->so_egid = p->p_ucred->cr_gid;
so->so_cpid = p->p_p->ps_pid;
so->so_proto = prp;
+ so->so_snd.sb_timeo_nsecs = INFSLP;
+ so->so_rcv.sb_timeo_nsecs = INFSLP;
s = solock(so);
error = (*prp->pr_attach)(so, proto);
@@ -304,7 +306,7 @@ soclose(struct socket *so, int flags)
while (so->so_state & SS_ISCONNECTED) {
error = sosleep(so, &so->so_timeo,
PSOCK | PCATCH, "netcls",
- so->so_linger * hz);
+ SEC_TO_NSEC(so->so_linger));
if (error)
break;
}
@@ -1761,22 +1763,25 @@ sosetopt(struct socket *so, int level, i
case SO_RCVTIMEO:
{
struct timeval tv;
- int val;
+ uint64_t nsecs;
if (m == NULL || m->m_len < sizeof (tv))
return (EINVAL);
memcpy(&tv, mtod(m, struct timeval *), sizeof tv);
- val = tvtohz(&tv);
- if (val > USHRT_MAX)
+ if (!timerisvalid(&tv))
return (EDOM);
-
+ nsecs = TIMEVAL_TO_NSEC(&tv);
+ if (nsecs == UINT64_MAX)
+ return (EDOM);
+ if (nsecs == 0)
+ nsecs = INFSLP;
switch (optname) {
case SO_SNDTIMEO:
- so->so_snd.sb_timeo = val;
+ so->so_snd.sb_timeo_nsecs = nsecs;
break;
case SO_RCVTIMEO:
- so->so_rcv.sb_timeo = val;
+ so->so_rcv.sb_timeo_nsecs = nsecs;
break;
}
break;
@@ -1910,13 +1915,14 @@ sogetopt(struct socket *so, int level, i
case SO_RCVTIMEO:
{
struct timeval tv;
- int val = (optname == SO_SNDTIMEO ?
- so->so_snd.sb_timeo : so->so_rcv.sb_timeo);
+ uint64_t nsecs = (optname == SO_SNDTIMEO ?
+ so->so_snd.sb_timeo_nsecs :
+ so->so_rcv.sb_timeo_nsecs);
m->m_len = sizeof(struct timeval);
memset(&tv, 0, sizeof(tv));
- tv.tv_sec = val / hz;
- tv.tv_usec = (val % hz) * tick;
+ if (nsecs != INFSLP)
+ NSEC_TO_TIMEVAL(nsecs, &tv);
memcpy(mtod(m, struct timeval *), &tv, sizeof tv);
break;
}
@@ -2129,7 +2135,7 @@ sobuf_print(struct sockbuf *sb,
(*pr)("\tsb_sel: ...\n");
(*pr)("\tsb_flagsintr: %d\n", sb->sb_flagsintr);
(*pr)("\tsb_flags: %i\n", sb->sb_flags);
- (*pr)("\tsb_timeo: %i\n", sb->sb_timeo);
+ (*pr)("\tsb_timeo_nsecs: %llu\n", sb->sb_timeo_nsecs);
}
void
Index: kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.101
diff -u -p -r1.101 uipc_socket2.c
--- kern/uipc_socket2.c 16 Apr 2019 13:15:32 - 1.101
+++ kern/uipc_socket2.c 12 Jan 2020 12:14:38 -
@@ -181,10 +181,10 @@ sonewconn(struct socket *head, int conns
}
so->so_snd.sb_wat = head->so_snd.sb_wat;
so->so_snd.sb_lowat = head->so_snd.sb_lowat;
- so->so_snd.sb_timeo = head->so_snd.sb_timeo;
+
Re: uthum(4) & tsleep
> Date: Sun, 12 Jan 2020 13:09:23 +0100 > From: Martin Pieuchot > > Now that tsleep_nsec(9) has the same behavior as tsleep(9) the > conversion be low should be safe. > > Ok? ok kettenis@ > Index: uthum.c > === > RCS file: /cvs/src/sys/dev/usb/uthum.c,v > retrieving revision 1.32 > diff -u -p -r1.32 uthum.c > --- uthum.c 9 Jan 2017 14:44:28 - 1.32 > +++ uthum.c 1 Jan 2020 14:27:59 - > @@ -311,7 +311,8 @@ uthum_issue_cmd(struct uthum_softc *sc, > > /* wait if required */ > if (delay > 0) > - tsleep(&sc->sc_sensortask, 0, "uthum", (delay*hz+999)/1000 + 1); > + tsleep_nsec(&sc->sc_sensortask, 0, "uthum", > + MSEC_TO_NSEC(delay)); > > return 0; > } > @@ -337,7 +338,8 @@ uthum_read_data(struct uthum_softc *sc, > > /* wait if required */ > if (delay > 0) > - tsleep(&sc->sc_sensortask, 0, "uthum", (delay*hz+999)/1000 + 1); > + tsleep_nsec(&sc->sc_sensortask, 0, "uthum", > + MSEC_TO_NSEC(delay)); > > /* get answer */ > if (uhidev_get_report(sc->sc_hdev.sc_parent, UHID_FEATURE_REPORT, > >
uthum(4) & tsleep
Now that tsleep_nsec(9) has the same behavior as tsleep(9) the conversion be low should be safe. Ok? Index: uthum.c === RCS file: /cvs/src/sys/dev/usb/uthum.c,v retrieving revision 1.32 diff -u -p -r1.32 uthum.c --- uthum.c 9 Jan 2017 14:44:28 - 1.32 +++ uthum.c 1 Jan 2020 14:27:59 - @@ -311,7 +311,8 @@ uthum_issue_cmd(struct uthum_softc *sc, /* wait if required */ if (delay > 0) - tsleep(&sc->sc_sensortask, 0, "uthum", (delay*hz+999)/1000 + 1); + tsleep_nsec(&sc->sc_sensortask, 0, "uthum", + MSEC_TO_NSEC(delay)); return 0; } @@ -337,7 +338,8 @@ uthum_read_data(struct uthum_softc *sc, /* wait if required */ if (delay > 0) - tsleep(&sc->sc_sensortask, 0, "uthum", (delay*hz+999)/1000 + 1); + tsleep_nsec(&sc->sc_sensortask, 0, "uthum", + MSEC_TO_NSEC(delay)); /* get answer */ if (uhidev_get_report(sc->sc_hdev.sc_parent, UHID_FEATURE_REPORT,
Re: IPL of timeout_set_proc(9)
On 11/01/20(Sat) 16:12, Scott Cheloha wrote: > On Sat, Jan 11, 2020 at 05:41:00PM +0100, Martin Pieuchot wrote: > > Before converting network timeouts to run in a thread context they were > > executed in a soft-interrupt handler. This design implied that timeouts > > were serialized. > > Yep. > > > The current "softclock" thread runs on CPU0 to limit border effects due > > to the conversion from soft-interrupt to thread context. > > Define "border effects". Do you mean limiting the delay between when > softclock() yields and softclock_thread() runs? timeout_hardclock_update() is just executed by CPU0, right? So aren't softclock handlers also executed on CPU0? If so, executing the softclock only on CPU0 is a conservative approach. Whatever bug we could expose by running the handler on another CPU isn't interesting for the moment. That's what I meant with "border effects". > > However we should raise the IPL level of this thread to ensure no other > > timeout can run in the middle of another one. > > This makes sense, though I have a few questions. Sorry in advance if > some of this is obvious, my understanding of the "rules" governing the > interrupt context is still limited. > > 1. If softclock() takes more than a full tick to complete and is >interrupted by hardclock(9), which schedules another softclock(), >but *before* softclock() returns it wakes up softclock_thread()... >which of those will run first? > >The softclock() interrupt or the softclock_thread()? Why do you ask? > 2. If both process and vanilla timeouts run at IPL_SOFTCLOCK, what >really is the difference between them? Is there still a reason to >distinguish between them? Would it make sense to run *all* timeouts >from the softclock thread once we have a chance to fork it from >process 0? As always is a matter of exposing and debugging the unknown problems related to any change. So why would you like to change the existing? > 3. Is there a way to prioritize the softclock thread over other >threads on the primary CPU so that the scheduler makes it runnable >as soon as possible after softclock() returns to reduce latency? Why do you ask?
Re: ldom.conf.5: Use stride in EXAMPLES, elaborate on hardware threads
> Date: Sun, 12 Jan 2020 11:40:23 +0100
> From: Klemens Nanni
>
> On Sat, Jan 04, 2020 at 09:27:30PM +0100, Klemens Nanni wrote:
> > CPU strides provide means to effectively bind guests to certain specific
> > phsyical cores by overallocating virtual CPUs (hardware threads) such
> > that the sum of virtual CPUs and strides yields an integer multiple of
> > the CPU dependent threads-per-core factor, e.g. on T2 based machines
> > each CPU has 8 cores and each core has 8 threads. this makes for n times
> > virtual CPUs to be managed in ldom.conf(5).
> >
> > Diff below slightly extends the given example and tries to further
> > clarify what strides do. The example should match a T5120 machine for
> > example which features 1 T2+ CPU.
> >
> > It hopefully makes it clearer how "vcpu"s correspond to hardware threads
> > not physical CPU cores.
> >
> > Feedback? OK?
> Ping.
>
> How do others understand/read the stride parts? Does this diff make it
> any clearer?
This diff doesn't make it clearer.
> Index: ldom.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldom.conf.5,v
> retrieving revision 1.9
> diff -u -p -r1.9 ldom.conf.5
> --- ldom.conf.5 3 Dec 2019 21:07:03 - 1.9
> +++ ldom.conf.5 12 Jan 2020 10:37:17 -
> @@ -78,11 +78,11 @@ Configure the MTU of the interface.
> .El
> .El
> .Sh EXAMPLES
> -Define a domain with 12 virtual cores, 4GB memory, two file based virtual
> disks
> -and one virtual network interface:
> +Define a domain with 12 virtual cores and 4 additionally allocated cores,
> +4GB memory, two file based virtual disks and one virtual network interface:
> .Bd -literal -offset indent
> domain "puffy" {
> - vcpu 12
> + vcpu 12:4
> memory 4G
> vdisk "/home/puffy/vdisk0"
> vdisk "/home/puffy/vdisk1"
> @@ -101,8 +101,10 @@ domain "salmah" {
> }
> .Ed
> .Pp
> -On a machine with 32 cores and 64GB physical memory, this leaves 12 cores and
> -58GB memory to the primary domain.
> +On a machine with 1 CPU comprising 8 cores with 8 threads each and 64GB
> physical
> +memory, this leaves 40 cores and 58GB memory to the primary domain.
> +.Dq puffy
> +will be assigned 16 threads of which 4 are not being used.
> .Sh SEE ALSO
> .Xr eeprom 8 ,
> .Xr ldomctl 8 ,
>
>
Re: ldom.conf.5: Add BUGS section for hypervisor memory
On Sat, Jan 04, 2020 at 11:54:21PM +0100, Klemens Nanni wrote: > The hypervisor requires memory and allocates it transparently, e.g. on > my T4-2 with 128G in factory-default configuration without guests, the > primary domain boots into OBP with 127.5G while the PRI presents 127.62M > of physical memory available. > > Documentation about those machine dependent allocations seems scarse, > I haven't found any so far except for M10/M12 machines of which I don't > even know whether OpenBSD would boot on them. > > ldomctl works with what it gets from the PRI and some configurations > (nearly) exhausting the available memory may fail to boot as the > hypervisor rejects the configuration, sadly with all too generic memory > related warnings. > > Can we highlight these circumstances under BUGS? I think it helps to > shed some light on this until we find proper documentation to fix this > or some way to handle this from within ldomctl. > > Feedback? OK? Opinions on this? I'll probably put this in during the next week unless I hear objections; this information seems valuable for users and also serves as reminder each time I look at the manual. Index: ldom.conf.5 === RCS file: /cvs/src/usr.sbin/ldomctl/ldom.conf.5,v retrieving revision 1.9 diff -u -p -r1.9 ldom.conf.5 --- ldom.conf.5 3 Dec 2019 21:07:03 - 1.9 +++ ldom.conf.5 12 Jan 2020 10:43:35 - @@ -107,3 +107,18 @@ On a machine with 32 cores and 64GB phys .Xr eeprom 8 , .Xr ldomctl 8 , .Xr ldomd 8 +.Sh BUGS +The hypervisor requires a machine dependent amount of physical memory that is +reserved automatically. +Although the Physical Resource Inventory +.Pq Em PRI +seems to account for this by presenting less available memory, using the entire +amount via +.Ic memory +is not always successful, e.g. the hypervisor would reject the configuration and +fallback to +.Dq factory-default +upon resetting the machine. +.Pp +If in doubt, leave enough memory unused for the hypervisor to reserve. +On bigger T4 based machines, 1024 megabytes has proven to suffice.
Re: ldom.conf.5: Use stride in EXAMPLES, elaborate on hardware threads
On Sat, Jan 04, 2020 at 09:27:30PM +0100, Klemens Nanni wrote:
> CPU strides provide means to effectively bind guests to certain specific
> phsyical cores by overallocating virtual CPUs (hardware threads) such
> that the sum of virtual CPUs and strides yields an integer multiple of
> the CPU dependent threads-per-core factor, e.g. on T2 based machines
> each CPU has 8 cores and each core has 8 threads. this makes for n times
> virtual CPUs to be managed in ldom.conf(5).
>
> Diff below slightly extends the given example and tries to further
> clarify what strides do. The example should match a T5120 machine for
> example which features 1 T2+ CPU.
>
> It hopefully makes it clearer how "vcpu"s correspond to hardware threads
> not physical CPU cores.
>
> Feedback? OK?
Ping.
How do others understand/read the stride parts? Does this diff make it
any clearer?
Index: ldom.conf.5
===
RCS file: /cvs/src/usr.sbin/ldomctl/ldom.conf.5,v
retrieving revision 1.9
diff -u -p -r1.9 ldom.conf.5
--- ldom.conf.5 3 Dec 2019 21:07:03 - 1.9
+++ ldom.conf.5 12 Jan 2020 10:37:17 -
@@ -78,11 +78,11 @@ Configure the MTU of the interface.
.El
.El
.Sh EXAMPLES
-Define a domain with 12 virtual cores, 4GB memory, two file based virtual disks
-and one virtual network interface:
+Define a domain with 12 virtual cores and 4 additionally allocated cores,
+4GB memory, two file based virtual disks and one virtual network interface:
.Bd -literal -offset indent
domain "puffy" {
- vcpu 12
+ vcpu 12:4
memory 4G
vdisk "/home/puffy/vdisk0"
vdisk "/home/puffy/vdisk1"
@@ -101,8 +101,10 @@ domain "salmah" {
}
.Ed
.Pp
-On a machine with 32 cores and 64GB physical memory, this leaves 12 cores and
-58GB memory to the primary domain.
+On a machine with 1 CPU comprising 8 cores with 8 threads each and 64GB
physical
+memory, this leaves 40 cores and 58GB memory to the primary domain.
+.Dq puffy
+will be assigned 16 threads of which 4 are not being used.
.Sh SEE ALSO
.Xr eeprom 8 ,
.Xr ldomctl 8 ,
Re: Fix manual description in SSL_CTX_add_extra_chain_cert.3
Hello Kinichiro-san, Kinichiro Inoguchi wrote on Sun, Jan 12, 2020 at 05:09:52PM +0900: > I think both SSL_CTX_get_extra_chain_certs and > SSL_CTX_get_extra_chain_certs_only should be described here. I think the text describing what to do with internal pointers returned from LibreSSL functions is still quite a mess in general. In some cases (like this one) something is said explicitly, but there are many different wordings. In other cases, nothing is said explicitly. In the case at hand, the sentence you are adding to feels somewhat redundant because the main description already says "retrieves an internal pointer". The only reason i didn't delete the sentence when last editing the page is because we didn't come round to a general cleanup of such statements yet, and adding or deleting such statements piece-meal without a comprehensive plan didn't feel like a big improvement. Regarding the particular change you propose, on top of the above, i think what you are adding is already clear from the text above, which explains the the pointers potentially returned from SSL_CTX_get_extra_chain_certs_only(3) are a subset of those potentially returned from SSL_CTX_get_extra_chain_certs(3). So if the latter mustn't be freed, that includes the former. > ok? I don't consider the patch a real improvement, but i don't strongly object to it either. What you are proposing to say is certainly not wrong. More generally, what do people think to switching to the following concise wording: SOMEOBJ_add0(obj, p) "transfers ownership of t to obj" (or "sets the FOO of obj to p, transfering ownership", depending on the context) SOMEOBJ_add1(obj, p) "sets the FOO of obj to a (deep|shallow) copy of p" p = SOMEOBJ_get0(obj) "retrieves an internal pointer to the FOO of obj" p = SOMEOBJ_get1(obj) "returns a (deep|shallow) copy of the FOO of obj" And then delete all the repetitive wordings like "you must free" or "you must not free"? Yours, Ingo > Index: SSL_CTX_add_extra_chain_cert.3 > === > RCS file: /cvs/src/lib/libssl/man/SSL_CTX_add_extra_chain_cert.3,v > retrieving revision 1.7 > diff -u -p -u -p -r1.7 SSL_CTX_add_extra_chain_cert.3 > --- SSL_CTX_add_extra_chain_cert.32 Jan 2020 09:09:16 - 1.7 > +++ SSL_CTX_add_extra_chain_cert.312 Jan 2020 08:06:49 - > @@ -115,7 +115,9 @@ An application should not free the > object, nor the > .Pf * Fa certs > object retrieved by > -.Fn SSL_CTX_get_extra_chain_certs . > +.Fn SSL_CTX_get_extra_chain_certs > +and > +.Fn SSL_CTX_get_extra_chain_certs_only . > .Sh RETURN VALUES > These functions return 1 on success or 0 for failure. > Check out the error stack to find out the reason for failure.
Fix manual description in SSL_CTX_add_extra_chain_cert.3
I think both SSL_CTX_get_extra_chain_certs and SSL_CTX_get_extra_chain_certs_only should be described here. ok? Index: SSL_CTX_add_extra_chain_cert.3 === RCS file: /cvs/src/lib/libssl/man/SSL_CTX_add_extra_chain_cert.3,v retrieving revision 1.7 diff -u -p -u -p -r1.7 SSL_CTX_add_extra_chain_cert.3 --- SSL_CTX_add_extra_chain_cert.3 2 Jan 2020 09:09:16 - 1.7 +++ SSL_CTX_add_extra_chain_cert.3 12 Jan 2020 08:06:49 - @@ -115,7 +115,9 @@ An application should not free the object, nor the .Pf * Fa certs object retrieved by -.Fn SSL_CTX_get_extra_chain_certs . +.Fn SSL_CTX_get_extra_chain_certs +and +.Fn SSL_CTX_get_extra_chain_certs_only . .Sh RETURN VALUES These functions return 1 on success or 0 for failure. Check out the error stack to find out the reason for failure.
