Re: iked(8): get rid of IPv6 flow and -6 flag?
We use the -6 option and I agree with deprecating it for one OpenBSD release instead. Especially now with sysupgrade(8), after upgrading our remote servers, our site-to-site VPN wouldn't come back up after upgrade. On Mon, Jan 13, 2020 at 12:58 PM Klemens Nanni wrote: > On Mon, Jan 13, 2020 at 05:55:06PM +0100, Tobias Heider wrote: > > iked by default blocks all IPv6 traffic on a host unless any > > of the configured policies use v6. This was originally meant > > as a measure to prevent VPN leakage for people who did not > > think of IPv6 when configuring IPsec. With the -6 flag > > set, iked does not install this IPv6 blocking flow. > It it still considered a leakage prevention, altough I doubt its > usefulness. > > > I think we should discuss whether we can remove the flow > > (and the -6 flag) as I constantly hear people complaining > > that it broke their setups and I don't think anyone > > expects some seemingly unrelated program breaking IPv6. > iked(8) is the only tool I know going completely counter-intuitive with > it's `-6' option; I expect those to behave like in nc(1). > > I'm in favour of removing the option and OK with your diff, but simply > removing it is probably a bad idea given its nature. > > What about printing a deprecation warning so that users can safely > adjust their rcctl flags instead of running into "iked(failed)" on the > next snapshot. > >
Re: diff: simplify globbing in ftpd(8)
On Mon, Jan 13, 2020 at 10:30:11AM -0700, Todd C. Miller wrote: > On Mon, 13 Jan 2020 13:45:55 +0100, Jan Klemkow wrote: > > This diff simplifies globbing for the ftpd(8) ls and stat command. And > > it avoids to glob for the parameters "-lgA". > > > > Due, we just pass a single string to the ftpd_ls() function, we don't > > need to provide infrastructure for a dynamic list of arguments. > > This conflicts with the diff to support NLIST arguments from SASANO > Takayoshi. I think we can support this by adding a flag to ftpd_ls() > that indicates whether it is a long list or not. Oh, I missed that. And will it checkout first. Thanks.
Re: iked(8): get rid of IPv6 flow and -6 flag?
On Mon, Jan 13, 2020 at 05:55:06PM +0100, Tobias Heider wrote: > iked by default blocks all IPv6 traffic on a host unless any > of the configured policies use v6. This was originally meant > as a measure to prevent VPN leakage for people who did not > think of IPv6 when configuring IPsec. With the -6 flag > set, iked does not install this IPv6 blocking flow. It it still considered a leakage prevention, altough I doubt its usefulness. > I think we should discuss whether we can remove the flow > (and the -6 flag) as I constantly hear people complaining > that it broke their setups and I don't think anyone > expects some seemingly unrelated program breaking IPv6. iked(8) is the only tool I know going completely counter-intuitive with it's `-6' option; I expect those to behave like in nc(1). I'm in favour of removing the option and OK with your diff, but simply removing it is probably a bad idea given its nature. What about printing a deprecation warning so that users can safely adjust their rcctl flags instead of running into "iked(failed)" on the next snapshot.
Re: pfctl - allow recursive flush operations [ 3/5 ]
On Mon, Jan 13, 2020 at 08:04:20PM +0100, Alexandr Nedvedicky wrote: > yes, final 5/5 diff already contains the change you propose here. > As I've said earlier the partial diffs are just code review. Those > were created artificially, when I tried to split big change to > smaller changes, just to show the evolution of idea. Ah, ok. I assumed that the split up diffs were still equivalent to the bigger one, so I tested the 5/5 but reviewed [1-4]/5.
Re: pfctl - allow recursive flush operations [ 3/5 ]
Hello,
> > @@ -2109,7 +2112,20 @@ pfctl_debug(int dev, u_int32_t level, int opts)
> > }
> >
> > int
> > -pfctl_show_anchors(int dev, int opts, char *anchorname)
> > +pfctl_walk_show(int opts, struct pfioc_ruleset *pr, void *warg)
> > +{
> > + if (pr->path[0]) {
> > + if (pr->path[0] != '_' || (opts & PF_OPT_VERBOSE))
> The latter is always true because you only call this function inside
>
> if (opts & PF_OPT_VERBOSE) {
> ...
> }
>
we should keep this test here in place, and fix pfctl_walk_show()
caller (see further below).
> > + printf(" %s/%s\n", pr->path, pr->name);
> > + } else if (pr->name[0] != '_' || (opts & PF_OPT_VERBOSE))
> > + printf(" %s\n", pr->name);
> > +
> > + return (0);
> Do you have plans for other callback functions in the future? If so,
> returning `int' makes sense if those might possibly fail.
sure. The plans materialize in diff 4/5, which introduces pfctl_call_...()
functions. I really need return value.
> > +pfctl_walk_anchors(int dev, int opts, char *anchorname, void *warg,
> > +int(walkf)(int, struct pfioc_ruleset *, void *))
> "warg" and "walkf" reads weird: Naturally function comes before
> argument(s) but then you're also abbreviating them inconsistently.
> How aobut "walkfunc" and "walkarg" in that order?
>
makes sense. I'll fix it in diff 5/5, which I want to commit.
> > + if (opts & PF_OPT_VERBOSE) {
> > + charsub[PATH_MAX];
> Although the same, please use MAXPATHLEN since that is used to define
> PF_ANCHOR_MAXPATH in pfvar.h.
I see. Can we leave it for follow up change? PATH_MAX is currently
consistent with what's being used in PF.
>
> > +
> > + if (pr.path[0])
> > + snprintf(sub, sizeof(sub), "%s/%s",
> > + pr.path, pr.name);
> > + else
> > + snprintf(sub, sizeof(sub), "%s",
> > + pr.name);
> > + if (pfctl_walk_anchors(dev, opts, sub, warg, walkf))
> > + return (-1);
> I'm probbaly missing something but why does recursion depend on
> PF_OPT_VERBOSE? This also omits printing of internal anchors (prefixed
> with "_") unless PF_OPT_VERBOSE is set, whereas the previous code did
> this regardless of verbosity.
>
the code is odd. It might be a result of copy'n'paste/fat finger.
I've delete the code in 5/5 diff.
> > +pfctl_show_anchors(int dev, int opts, char *anchorname)
> I know it's named "anchorname" already and this is nitpicking, but we're
> passing the full anchor path and name here, so both "anchorname" and
> "anchorpath" suggest either of the two... components; how about just
> "anchor" like in `pfctl -a anchor'? Then I don't have to double check
> whether this parameters actually contains the path, name or both.
yes, I agree. I'll rename it in 5/5 diff.
>
> > +{
> > + int rv;
> > +
> > + rv = pfctl_walk_anchors(dev, opts, anchorname, NULL, pfctl_walk_show);
> > + return (rv);
> How about simply
>
> return (pfctl_walk_anchors(dev, opts, anchorname, NULL,
> pfctl_walk_show));
>
yes, final 5/5 diff already contains the change you propose here.
As I've said earlier the partial diffs are just code review. Those
were created artificially, when I tried to split big change to
smaller changes, just to show the evolution of idea.
thanks and
regards
sashan
Re: diff: simplify globbing in ftpd(8)
On Mon, 13 Jan 2020 13:45:55 +0100, Jan Klemkow wrote: > This diff simplifies globbing for the ftpd(8) ls and stat command. And > it avoids to glob for the parameters "-lgA". > > Due, we just pass a single string to the ftpd_ls() function, we don't > need to provide infrastructure for a dynamic list of arguments. This conflicts with the diff to support NLIST arguments from SASANO Takayoshi. I think we can support this by adding a flag to ftpd_ls() that indicates whether it is a long list or not. - todd
Re: iked(8): get rid of IPv6 flow and -6 flag?
On Mon, Jan 13, 2020 at 05:55:06PM +0100, Tobias Heider wrote: > I think we should discuss whether we can remove the flow > (and the -6 flag) as I constantly hear people complaining > that it broke their setups and I don't think anyone > expects some seemingly unrelated program breaking IPv6. A missing -6 flag on the iked command line, is a very unexpected way to break your IPv6 setup. So we should remove that. OK bluhm@ If there is demand for such a feature, we could create an option in the example/iked.conf that shows how to disable IPv6. And perhaps one to disable IPv4 for the IPv6 hipser :-) bluhm
iked(8): get rid of IPv6 flow and -6 flag?
Hi,
iked by default blocks all IPv6 traffic on a host unless any
of the configured policies use v6. This was originally meant
as a measure to prevent VPN leakage for people who did not
think of IPv6 when configuring IPsec. With the -6 flag
set, iked does not install this IPv6 blocking flow.
I think we should discuss whether we can remove the flow
(and the -6 flag) as I constantly hear people complaining
that it broke their setups and I don't think anyone
expects some seemingly unrelated program breaking IPv6.
diff --git a/sbin/iked/iked.8 b/sbin/iked/iked.8
index f715db47afd..c7682500414 100644
--- a/sbin/iked/iked.8
+++ b/sbin/iked/iked.8
@@ -22,7 +22,7 @@
.Nd Internet Key Exchange version 2 (IKEv2) daemon
.Sh SYNOPSIS
.Nm iked
-.Op Fl 6dnSTtv
+.Op Fl dnSTtv
.Op Fl D Ar macro Ns = Ns Ar value
.Op Fl f Ar file
.Sh DESCRIPTION
@@ -55,14 +55,6 @@ infrastructure.
.Pp
The options are as follows:
.Bl -tag -width Ds
-.It Fl 6
-Disable automatic blocking of IPv6 traffic.
-By default,
-.Nm
-blocks any IPv6 traffic unless a flow for this address family has been
-negotiated.
-This option disables VPN traffic leakage prevention on dual stack hosts
-(RFC 7359).
.It Fl D Ar macro Ns = Ns Ar value
Define
.Ar macro
diff --git a/sbin/iked/iked.c b/sbin/iked/iked.c
index 6714e0b2088..bc0b8109651 100644
--- a/sbin/iked/iked.c
+++ b/sbin/iked/iked.c
@@ -56,7 +56,7 @@ usage(void)
{
extern char *__progname;
- fprintf(stderr, "usage: %s [-6dnSTtv] [-D macro=value] "
+ fprintf(stderr, "usage: %s [-dnSTtv] [-D macro=value] "
"[-f file]\n", __progname);
exit(1);
}
@@ -73,11 +73,8 @@ main(int argc, char *argv[])
log_init(1, LOG_DAEMON);
- while ((c = getopt(argc, argv, "6dD:nf:vSTt")) != -1) {
+ while ((c = getopt(argc, argv, "dD:nf:vSTt")) != -1) {
switch (c) {
- case '6':
- opts |= IKED_OPT_NOIPV6BLOCKING;
- break;
case 'd':
debug++;
break;
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index 897669ac625..5a071a43f75 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -950,7 +950,6 @@ int eap_parse(struct iked *, struct iked_sa *, void *,
int);
int pfkey_couple(int, struct iked_sas *, int);
int pfkey_flow_add(int fd, struct iked_flow *);
int pfkey_flow_delete(int fd, struct iked_flow *);
-int pfkey_block(int, int, unsigned int);
int pfkey_sa_init(int, struct iked_childsa *, uint32_t *);
int pfkey_sa_add(int, struct iked_childsa *, struct iked_childsa *);
int pfkey_sa_update_addresses(int, struct iked_childsa *);
diff --git a/sbin/iked/pfkey.c b/sbin/iked/pfkey.c
index b9f90687784..de8055c6863 100644
--- a/sbin/iked/pfkey.c
+++ b/sbin/iked/pfkey.c
@@ -50,9 +50,7 @@
static uint32_t sadb_msg_seq = 0;
static unsigned int sadb_decoupled = 0;
-static unsigned int sadb_ipv6refcnt = 0;
-static int pfkey_blockipv6 = 0;
static struct event pfkey_timer_ev;
static struct timeval pfkey_timer_tv;
@@ -1259,12 +1257,6 @@ pfkey_flow_add(int fd, struct iked_flow *flow)
flow->flow_loaded = 1;
- if (flow->flow_dst.addr_af == AF_INET6) {
- sadb_ipv6refcnt++;
- if (sadb_ipv6refcnt == 1)
- return (pfkey_block(fd, AF_INET6, SADB_X_DELFLOW));
- }
-
return (0);
}
@@ -1284,42 +1276,6 @@ pfkey_flow_delete(int fd, struct iked_flow *flow)
flow->flow_loaded = 0;
- if (flow->flow_dst.addr_af == AF_INET6) {
- sadb_ipv6refcnt--;
- if (sadb_ipv6refcnt == 0)
- return (pfkey_block(fd, AF_INET6, SADB_X_ADDFLOW));
- }
-
- return (0);
-}
-
-int
-pfkey_block(int fd, int af, unsigned int action)
-{
- struct iked_flow flow;
-
- if (!pfkey_blockipv6)
- return (0);
-
- /*
-* Prevent VPN traffic leakages in dual-stack hosts/networks.
-* https://tools.ietf.org/html/draft-gont-opsec-vpn-leakages.
-* We forcibly block IPv6 traffic unless it is used in any of
-* the flows by tracking a sadb_ipv6refcnt reference counter.
-*/
- bzero(&flow, sizeof(flow));
- flow.flow_src.addr_af = flow.flow_src.addr.ss_family = af;
- flow.flow_src.addr_net = 1;
- socket_af((struct sockaddr *)&flow.flow_src.addr, 0);
- flow.flow_dst.addr_af = flow.flow_dst.addr.ss_family = af;
- flow.flow_dst.addr_net = 1;
- socket_af((struct sockaddr *)&flow.flow_dst.addr, 0);
- flow.flow_type = SADB_X_FLOW_TYPE_DENY;
- flow.flow_dir = IPSP_DIRECTION_OUT;
-
- if (pfkey_flow(fd, 0, action, &flow) == -1)
- return (-1);
-
return (0);
}
@@ -1550,14 +1506,6 @@ pfkey_init(struct iked *env, int fd)
if (pfkey_write(fd, &smsg, &iov, 1, NULL, NULL))
fatal("pfkey_init: failed to set up AH acquires");
-
Re: Towards splitting SCHED_LOCK()
> Date: Mon, 13 Jan 2020 17:02:12 +0100
> From: Martin Pieuchot
>
> I'm facing a lock ordering issue while profiling the scheduler which
> cannot be solved without using a different lock for the global sleepqueue
> and the runqueues.
>
> The SCHED_LOCK() currently protects both data structures as well as the
> related fields in 'struct proc'. One of this fields is `p_wchan' which
> is obviously related to the global sleepqueue.
>
> The cleaning diff below introduces a new function, awake(), that unify
> the multiple uses of the following idiom:
>
> if (p->p_stat == SSLEEP)
> setrunnable(p);
> else
> unsleep(p);
>
> This is generally done after checking if the thread `p' is on the
> sleepqueue.
>
> This diff introduces a change in behavior in the Linux compat:
> wake_up_process() will now return 1 if the thread was stopped and not
> sleeping. This should be fine since the only place this value is
> checked is with the combination of task_asleep().
I think that is actually ok. Can't be 100% sure.
> While here I removed two unnecessary `p_wchan' check before unsleep().
>
> ok?
I think so.
> Index: dev/pci/drm/drm_linux.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 drm_linux.c
> --- dev/pci/drm/drm_linux.c 5 Jan 2020 13:46:02 - 1.55
> +++ dev/pci/drm/drm_linux.c 13 Jan 2020 15:34:44 -
> @@ -115,20 +115,8 @@ schedule_timeout(long timeout)
> int
> wake_up_process(struct proc *p)
> {
> - int s, r = 0;
> -
> - SCHED_LOCK(s);
> atomic_cas_ptr(&sch_proc, p, NULL);
> - if (p->p_wchan) {
> - if (p->p_stat == SSLEEP) {
> - setrunnable(p);
> - r = 1;
> - } else
> - unsleep(p);
> - }
> - SCHED_UNLOCK(s);
> -
> - return r;
> + return awake(p, NULL);
> }
>
> void
> Index: kern/sys_generic.c
> ===
> RCS file: /cvs/src/sys/kern/sys_generic.c,v
> retrieving revision 1.127
> diff -u -p -r1.127 sys_generic.c
> --- kern/sys_generic.c8 Jan 2020 16:27:41 - 1.127
> +++ kern/sys_generic.c13 Jan 2020 15:35:22 -
> @@ -767,7 +767,6 @@ void
> selwakeup(struct selinfo *sip)
> {
> struct proc *p;
> - int s;
>
> KNOTE(&sip->si_note, NOTE_SUBMIT);
> if (sip->si_seltid == 0)
> @@ -780,15 +779,10 @@ selwakeup(struct selinfo *sip)
> p = tfind(sip->si_seltid);
> sip->si_seltid = 0;
> if (p != NULL) {
> - SCHED_LOCK(s);
> - if (p->p_wchan == (caddr_t)&selwait) {
> - if (p->p_stat == SSLEEP)
> - setrunnable(p);
> - else
> - unsleep(p);
> + if (awake(p, &selwait)) {
> + /* nothing else to do */
> } else if (p->p_flag & P_SELECT)
> atomic_clearbits_int(&p->p_flag, P_SELECT);
> - SCHED_UNLOCK(s);
> }
> }
>
> 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 13 Jan 2020 15:41:00 -
> @@ -469,8 +469,7 @@ sleep_setup_signal(struct sleep_state *s
>*/
> atomic_setbits_int(&p->p_flag, P_SINTR);
> if (p->p_p->ps_single != NULL || (sls->sls_sig = CURSIG(p)) != 0) {
> - if (p->p_wchan)
> - unsleep(p);
> + unsleep(p);
> p->p_stat = SONPROC;
> sls->sls_do_sleep = 0;
> } else if (p->p_wchan == 0) {
> @@ -505,6 +504,25 @@ sleep_finish_signal(struct sleep_state *
> return (error);
> }
>
> +int
> +awake(struct proc *p, const volatile void *chan)
> +{
> + int s, awakened = 0;
> +
> + SCHED_LOCK(s);
> + if (p->p_wchan != NULL &&
> +((chan == NULL) || (p->p_wchan == chan))) {
> + awakened = 1;
> + if (p->p_stat == SSLEEP)
> + setrunnable(p);
> + else
> + unsleep(p);
> + }
> + SCHED_UNLOCK(s);
> +
> + return awakened;
> +}
> +
> /*
> * Implement timeout for tsleep.
> * If process hasn't been awakened (wchan non-zero),
> @@ -515,17 +533,9 @@ void
> endtsleep(void *arg)
> {
> struct proc *p = arg;
> - int s;
>
> - SCHED_LOCK(s);
> - if (p->p_wchan) {
> - if (p->p_stat == SSLEEP)
> - setrunnable(p);
> - else
> - unsleep(p);
> + if (awake(p, NULL))
> atomic_setbits_int(&p->p_flag, P_TIMEOUT);
> - }
> - SCHED_UNLOCK(s);
> }
>
> /*
> @@ -536,7 +546,7 @
Re: pfctl - allow recursive flush operations [ 3/5 ]
On Mon, Jan 13, 2020 at 03:53:41PM +0100, Alexandr Nedvedicky wrote:
> change below splits 'pfctl_show_anchors()' to two functions:
> - pfctl_walk_anchors() function, which can traverse all anchor
> tree loaded to PF driver and
>
> - pfctl_walk_show(), which is a call back for pfctl_walk_anchors()
> it just prints anchor name
>
> this change is a foundation for a recursive flush.
This approach looks good to me.
A few comments/nits inline.
> diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
> index d1a309d919f..ba3610a6a7b 100644
> --- a/sbin/pfctl/pfctl.c
> +++ b/sbin/pfctl/pfctl.c
> @@ -106,6 +106,9 @@ const char*pfctl_lookup_option(char *, const char
> **);
> void pfctl_state_store(int, const char *);
> void pfctl_state_load(int, const char *);
> void pfctl_reset(int, int);
> +int pfctl_walk_show(int, struct pfioc_ruleset *, void *);
> +int pfctl_walk_anchors(int, int, char *, void *,
> +int(*)(int, struct pfioc_ruleset *, void *));
>
> const char *clearopt;
> char *rulesopt;
> @@ -2109,7 +2112,20 @@ pfctl_debug(int dev, u_int32_t level, int opts)
> }
>
> int
> -pfctl_show_anchors(int dev, int opts, char *anchorname)
> +pfctl_walk_show(int opts, struct pfioc_ruleset *pr, void *warg)
> +{
> + if (pr->path[0]) {
> + if (pr->path[0] != '_' || (opts & PF_OPT_VERBOSE))
The latter is always true because you only call this function inside
if (opts & PF_OPT_VERBOSE) {
...
}
> + printf(" %s/%s\n", pr->path, pr->name);
> + } else if (pr->name[0] != '_' || (opts & PF_OPT_VERBOSE))
> + printf(" %s\n", pr->name);
> +
> + return (0);
Do you have plans for other callback functions in the future? If so,
returning `int' makes sense if those might possibly fail.
Otherwise let's just make the callback function signature void.
> +}
> +
> +int
> +pfctl_walk_anchors(int dev, int opts, char *anchorname, void *warg,
> +int(walkf)(int, struct pfioc_ruleset *, void *))
"warg" and "walkf" reads weird: Naturally function comes before
argument(s) but then you're also abbreviating them inconsistently.
How aobut "walkfunc" and "walkarg" in that order?
> {
> struct pfioc_ruleset pr;
> u_int32_tmnr, nr;
> @@ -2134,19 +2150,35 @@ pfctl_show_anchors(int dev, int opts, char
> *anchorname)
> if (!strcmp(pr.name, PF_RESERVED_ANCHOR))
> continue;
> sub[0] = '\0';
> - if (pr.path[0]) {
> - strlcat(sub, pr.path, sizeof(sub));
> - strlcat(sub, "/", sizeof(sub));
> - }
> - strlcat(sub, pr.name, sizeof(sub));
> - if (sub[0] != '_' || (opts & PF_OPT_VERBOSE))
> - printf(" %s\n", sub);
> - if ((opts & PF_OPT_VERBOSE) && pfctl_show_anchors(dev, opts,
> sub))
> +
> + if (walkf(opts, &pr, warg))
> return (-1);
> +
> + if (opts & PF_OPT_VERBOSE) {
> + charsub[PATH_MAX];
Although the same, please use MAXPATHLEN since that is used to define
PF_ANCHOR_MAXPATH in pfvar.h.
> +
> + if (pr.path[0])
> + snprintf(sub, sizeof(sub), "%s/%s",
> + pr.path, pr.name);
> + else
> + snprintf(sub, sizeof(sub), "%s",
> + pr.name);
> + if (pfctl_walk_anchors(dev, opts, sub, warg, walkf))
> + return (-1);
I'm probbaly missing something but why does recursion depend on
PF_OPT_VERBOSE? This also omits printing of internal anchors (prefixed
with "_") unless PF_OPT_VERBOSE is set, whereas the previous code did
this regardless of verbosity.
> + }
> }
> return (0);
> }
>
> +int
> +pfctl_show_anchors(int dev, int opts, char *anchorname)
I know it's named "anchorname" already and this is nitpicking, but we're
passing the full anchor path and name here, so both "anchorname" and
"anchorpath" suggest either of the two... components; how about just
"anchor" like in `pfctl -a anchor'? Then I don't have to double check
whether this parameters actually contains the path, name or both.
> +{
> + int rv;
> +
> + rv = pfctl_walk_anchors(dev, opts, anchorname, NULL, pfctl_walk_show);
> + return (rv);
How about simply
return (pfctl_walk_anchors(dev, opts, anchorname, NULL,
pfctl_walk_show));
> +}
> +
> const char *
> pfctl_lookup_option(char *cmd, const char **list)
> {
>
Towards splitting SCHED_LOCK()
I'm facing a lock ordering issue while profiling the scheduler which
cannot be solved without using a different lock for the global sleepqueue
and the runqueues.
The SCHED_LOCK() currently protects both data structures as well as the
related fields in 'struct proc'. One of this fields is `p_wchan' which
is obviously related to the global sleepqueue.
The cleaning diff below introduces a new function, awake(), that unify
the multiple uses of the following idiom:
if (p->p_stat == SSLEEP)
setrunnable(p);
else
unsleep(p);
This is generally done after checking if the thread `p' is on the
sleepqueue.
This diff introduces a change in behavior in the Linux compat:
wake_up_process() will now return 1 if the thread was stopped and not
sleeping. This should be fine since the only place this value is
checked is with the combination of task_asleep().
While here I removed two unnecessary `p_wchan' check before unsleep().
ok?
Index: dev/pci/drm/drm_linux.c
===
RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
retrieving revision 1.55
diff -u -p -r1.55 drm_linux.c
--- dev/pci/drm/drm_linux.c 5 Jan 2020 13:46:02 - 1.55
+++ dev/pci/drm/drm_linux.c 13 Jan 2020 15:34:44 -
@@ -115,20 +115,8 @@ schedule_timeout(long timeout)
int
wake_up_process(struct proc *p)
{
- int s, r = 0;
-
- SCHED_LOCK(s);
atomic_cas_ptr(&sch_proc, p, NULL);
- if (p->p_wchan) {
- if (p->p_stat == SSLEEP) {
- setrunnable(p);
- r = 1;
- } else
- unsleep(p);
- }
- SCHED_UNLOCK(s);
-
- return r;
+ return awake(p, NULL);
}
void
Index: kern/sys_generic.c
===
RCS file: /cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.127
diff -u -p -r1.127 sys_generic.c
--- kern/sys_generic.c 8 Jan 2020 16:27:41 - 1.127
+++ kern/sys_generic.c 13 Jan 2020 15:35:22 -
@@ -767,7 +767,6 @@ void
selwakeup(struct selinfo *sip)
{
struct proc *p;
- int s;
KNOTE(&sip->si_note, NOTE_SUBMIT);
if (sip->si_seltid == 0)
@@ -780,15 +779,10 @@ selwakeup(struct selinfo *sip)
p = tfind(sip->si_seltid);
sip->si_seltid = 0;
if (p != NULL) {
- SCHED_LOCK(s);
- if (p->p_wchan == (caddr_t)&selwait) {
- if (p->p_stat == SSLEEP)
- setrunnable(p);
- else
- unsleep(p);
+ if (awake(p, &selwait)) {
+ /* nothing else to do */
} else if (p->p_flag & P_SELECT)
atomic_clearbits_int(&p->p_flag, P_SELECT);
- SCHED_UNLOCK(s);
}
}
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 13 Jan 2020 15:41:00 -
@@ -469,8 +469,7 @@ sleep_setup_signal(struct sleep_state *s
*/
atomic_setbits_int(&p->p_flag, P_SINTR);
if (p->p_p->ps_single != NULL || (sls->sls_sig = CURSIG(p)) != 0) {
- if (p->p_wchan)
- unsleep(p);
+ unsleep(p);
p->p_stat = SONPROC;
sls->sls_do_sleep = 0;
} else if (p->p_wchan == 0) {
@@ -505,6 +504,25 @@ sleep_finish_signal(struct sleep_state *
return (error);
}
+int
+awake(struct proc *p, const volatile void *chan)
+{
+ int s, awakened = 0;
+
+ SCHED_LOCK(s);
+ if (p->p_wchan != NULL &&
+ ((chan == NULL) || (p->p_wchan == chan))) {
+ awakened = 1;
+ if (p->p_stat == SSLEEP)
+ setrunnable(p);
+ else
+ unsleep(p);
+ }
+ SCHED_UNLOCK(s);
+
+ return awakened;
+}
+
/*
* Implement timeout for tsleep.
* If process hasn't been awakened (wchan non-zero),
@@ -515,17 +533,9 @@ void
endtsleep(void *arg)
{
struct proc *p = arg;
- int s;
- SCHED_LOCK(s);
- if (p->p_wchan) {
- if (p->p_stat == SSLEEP)
- setrunnable(p);
- else
- unsleep(p);
+ if (awake(p, NULL))
atomic_setbits_int(&p->p_flag, P_TIMEOUT);
- }
- SCHED_UNLOCK(s);
}
/*
@@ -536,7 +546,7 @@ unsleep(struct proc *p)
{
SCHED_ASSERT_LOCKED();
- if (p->p_wchan) {
+ if (p->p_wchan != NULL) {
TAILQ_REMOVE(&slpque[LOOKUP(p->p_wchan)], p, p_runq);
p->p_wchan = NULL;
}
@@ -570,13 +580,8 @@ wakeup_n(const volatile void *ident, int
Re: umb(4) authentication
On Mon, Jan 13, 2020 at 04:42:22PM +0100, Martijn van Duren wrote: > On 1/13/20 4:30 PM, Anders Andersson wrote: > > On Mon, Jan 13, 2020 at 3:00 PM leeb wrote: > >> > >> Hello, > >> > >> > >> Thanks, > >> > >> Lee. > > > > This email should be gold plated and moved to a permanent location in > > the FAQ on how to help OpenBSD and how to request new features. > > > Lee is obviously new to the community and doesn't know the workflow well > enough. He tries to be polite and make sure he doesn't clutter the list. > If you actually read his mail you would've seen that he has a diff and > it works for him, so it's obviously not a dumb feature request. > > Please refrain from ridiculing people with good intentions. > I originally read Anders' mail as complimentary, but now I'm not so sure? Lee.
Re: umb(4) authentication
On Mon, Jan 13, 2020 at 4:42 PM Martijn van Duren wrote: > > On 1/13/20 4:30 PM, Anders Andersson wrote: > > On Mon, Jan 13, 2020 at 3:00 PM leeb wrote: > >> > >> Hello, > >> > >> I had an itch, so I scratched it. > >> > >> The umb(4) driver has a FIXME in if_umb.c for user name > >> and passphrase support. My LTE provider happens to require > >> this so I thought I'd have a go. > >> > >> The provider seems happy with my umb changes, and I've > >> added some bits in ifconfig(8) to allow setting of the > >> authentication information (user/pass/protocol): > >> > >> umb0: flags=8851 mtu 1500 > >> index 5 priority 6 llprio 3 > >> roaming disabled registration home network > >> state up cell-class LTE rssi -67dBm speed 47.7Mps up 95.4Mps down > >> SIM initialized PIN valid (3 attempts left) > >> subscriber-id 999 ICC-id 999 provider > >> xx > >> device MC7700 IMEI 999 firmware SWI9200X_03.05.19.00Aap > >> phone# 999 APN xx User Pass x > >> Auth CHAP > >> dns x.x.x.x x.x.x.x > >> groups: egress > >> status: active > >> inet x.x.x.x --> x.x.x.x netmask 0xfffc > >> > >> (please excuse line wraps, and I've redacted possibly > >> sensitive info) > >> > >> Now, this is my first attempt at OpenBSD development, and > >> writing my changes raised a few questions. So, does someone > >> want to spare a few minutes off-list, or should I just > >> clutter up tech@ ? > >> > >> (I've read the top of the Makefile, I've been through the > >> web pages on AnonCVS and release(8)) > >> > >> Thanks, > >> > >> Lee. > > > > This email should be gold plated and moved to a permanent location in > > the FAQ on how to help OpenBSD and how to request new features. > > > Lee is obviously new to the community and doesn't know the workflow well > enough. He tries to be polite and make sure he doesn't clutter the list. > If you actually read his mail you would've seen that he has a diff and > it works for him, so it's obviously not a dumb feature request. > > Please refrain from ridiculing people with good intentions. I did read the whole email. Now I'm wondering if you read my reply? It's not very long, just one sentence. Did you add a few "not" in there that I didn't write? Or if you are meta-trolling me somehow, I apologize if I missed the wooshing noise.
Re: pfctl - allow recursive flush operations [ 4/5 ]
On Mon, Jan 13, 2020 at 03:52:57PM +0100, Alexandr Nedvedicky wrote: > pfctl diff bellow enables me to relax err(3) and errx(3) to warnings > where needed. The idea is to introduce PF_OPT_IGNFAIL and pfctl_err() > and pfctl_errx() wrappers around err(3) and errx(3) functions. > > the wrappers, when acting in warning mode (PF_OPT_IGNFAIL set), would > also set exit_val global variable so main() can report error. Makes sense. OK kn Contrary to other options, this should be a strictly internal one and not be settable in any way.
Re: umb(4) authentication
On 1/13/20 4:30 PM, Anders Andersson wrote: > On Mon, Jan 13, 2020 at 3:00 PM leeb wrote: >> >> Hello, >> >> I had an itch, so I scratched it. >> >> The umb(4) driver has a FIXME in if_umb.c for user name >> and passphrase support. My LTE provider happens to require >> this so I thought I'd have a go. >> >> The provider seems happy with my umb changes, and I've >> added some bits in ifconfig(8) to allow setting of the >> authentication information (user/pass/protocol): >> >> umb0: flags=8851 mtu 1500 >> index 5 priority 6 llprio 3 >> roaming disabled registration home network >> state up cell-class LTE rssi -67dBm speed 47.7Mps up 95.4Mps down >> SIM initialized PIN valid (3 attempts left) >> subscriber-id 999 ICC-id 999 provider >> xx >> device MC7700 IMEI 999 firmware SWI9200X_03.05.19.00Aap >> phone# 999 APN xx User Pass x Auth >> CHAP >> dns x.x.x.x x.x.x.x >> groups: egress >> status: active >> inet x.x.x.x --> x.x.x.x netmask 0xfffc >> >> (please excuse line wraps, and I've redacted possibly >> sensitive info) >> >> Now, this is my first attempt at OpenBSD development, and >> writing my changes raised a few questions. So, does someone >> want to spare a few minutes off-list, or should I just >> clutter up tech@ ? >> >> (I've read the top of the Makefile, I've been through the >> web pages on AnonCVS and release(8)) >> >> Thanks, >> >> Lee. > > This email should be gold plated and moved to a permanent location in > the FAQ on how to help OpenBSD and how to request new features. > Lee is obviously new to the community and doesn't know the workflow well enough. He tries to be polite and make sure he doesn't clutter the list. If you actually read his mail you would've seen that he has a diff and it works for him, so it's obviously not a dumb feature request. Please refrain from ridiculing people with good intentions.
Re: pfctl - allow recursive flush operations [ 5/5 ]
On Mon, Jan 13, 2020 at 03:47:51PM +0100, Alexandr Nedvedicky wrote: > Also kn@ was testing that change back in 2019 and found some glitches [2]. > IMO the issues pointed out by kn@ are present in code already, diff below > just uncovers them. We can investigate/fix them once the change below will > be in. # cd /usr/src/regress/sbin/pfctl # make # cd /usr/src/sbin/pfctl # pfctl -Fa -a regress\* Removing: regress/relative regress/one/two regress/one regress/foo/bar regress/foo/_4 regress/foo regress/_1/foo/_3 regress/_1/foo regress/_1/_3/_4/_5/_6/_7/_8/_9/_10 regress/_1/_3/_4/_5/_6/_7/_8/_9 regress/_1/_3/_4/_5/_6/_7/_8 regress/_1/_3/_4/_5/_6/_7 regress/_1/_3/_4/_5/_6 regress/_1/_3/_4/_5 regress/_1/_3/_4 regress/_1/_3 regress/_1/_2 regress/_1 regress Perfect, everything else in my ruleset (anchors, tables, states) is left untouched; the previous diff[1] would ignore `-a anchor' and wipe *everything* -- that was my biggest issue. > I'll send partial diffs as a reply to this email to keep things organized. Thanks, much appreciated. I'd also be happy with committing diffs separately so the log keeps clear and separated. > [1] https://marc.info/?l=openbsd-tech&m=155545759616085&w=2 > > [2] https://marc.info/?l=openbsd-tech&m=155769978704563&w=2
Re: umb(4) authentication
On Mon, Jan 13, 2020 at 3:00 PM leeb wrote: > > Hello, > > I had an itch, so I scratched it. > > The umb(4) driver has a FIXME in if_umb.c for user name > and passphrase support. My LTE provider happens to require > this so I thought I'd have a go. > > The provider seems happy with my umb changes, and I've > added some bits in ifconfig(8) to allow setting of the > authentication information (user/pass/protocol): > > umb0: flags=8851 mtu 1500 > index 5 priority 6 llprio 3 > roaming disabled registration home network > state up cell-class LTE rssi -67dBm speed 47.7Mps up 95.4Mps down > SIM initialized PIN valid (3 attempts left) > subscriber-id 999 ICC-id 999 provider > xx > device MC7700 IMEI 999 firmware SWI9200X_03.05.19.00Aap > phone# 999 APN xx User Pass x Auth > CHAP > dns x.x.x.x x.x.x.x > groups: egress > status: active > inet x.x.x.x --> x.x.x.x netmask 0xfffc > > (please excuse line wraps, and I've redacted possibly > sensitive info) > > Now, this is my first attempt at OpenBSD development, and > writing my changes raised a few questions. So, does someone > want to spare a few minutes off-list, or should I just > clutter up tech@ ? > > (I've read the top of the Makefile, I've been through the > web pages on AnonCVS and release(8)) > > Thanks, > > Lee. This email should be gold plated and moved to a permanent location in the FAQ on how to help OpenBSD and how to request new features.
Re: pfctl - allow recursive flush operations [ 1/5 ]
Hello,
this is the last piece, which makes everything fit together. The
change introduces three 'nuke()' functions:
- pfctl_call_clearrules()
- pfctl_call_cleartables()
- pfctl_call_clearanchors()
Those are callbacks for pfctl_recurse() we've introduced earlier. Those
functions just call existing pfctl_clear_...() function. We also must turn
pfctl_clear_...() function from void to int so error can be reported. Also all
pfctl_clear_...() function must adhere a PF_OPT_IGNFAIL flag now.
Not diff below also includes a change, which does a right thing
in case we ask pfctl to remove just subtree:
pfctl -a 'foo/bar/*' -Fr
However keep in mind all those changes are uncovering glitches,
which exit in PF kernel driver already.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 054da006e69..9fe661e8c89 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -51,10 +51,9 @@
#include
#include
#include
-
#include
-
#include
+#include
#include "pfctl_parser.h"
#include "pfctl.h"
@@ -65,7 +64,7 @@ intpfctl_disable(int, int);
voidpfctl_clear_queues(struct pf_qihead *);
voidpfctl_clear_stats(int, const char *, int);
voidpfctl_clear_interface_flags(int, int);
-voidpfctl_clear_rules(int, int, char *);
+int pfctl_clear_rules(int, int, char *);
voidpfctl_clear_src_nodes(int, int);
voidpfctl_clear_states(int, const char *, int);
struct addrinfo *
@@ -110,11 +109,15 @@ void pfctl_state_load(int, const char *);
void pfctl_reset(int, int);
intpfctl_walk_show(int, struct pfioc_ruleset *, void *);
intpfctl_walk_get(int, struct pfioc_ruleset *, void *);
-intpfctl_walk_anchors(int, int, char *, void *,
+intpfctl_walk_anchors(int, int, const char *, void *,
int(*)(int, struct pfioc_ruleset *, void *));
struct pfr_anchors *
- pfctl_get_anchors(int, int);
-intpfctl_recurse(int, int, int(*nuke)(int, int, struct pfr_anchoritem *));
+ pfctl_get_anchors(int, const char *, int);
+intpfctl_recurse(int, int, const char *,
+ int(*)(int, int, struct pfr_anchoritem *));
+intpfctl_call_clearrules(int, int, struct pfr_anchoritem *);
+intpfctl_call_cleartables(int, int, struct pfr_anchoritem *);
+intpfctl_call_clearanchors(int, int, struct pfr_anchoritem *);
const char *clearopt;
char *rulesopt;
@@ -244,7 +247,6 @@ static const char *optiopt_list[] = {
struct pf_qihead qspecs = TAILQ_HEAD_INITIALIZER(qspecs);
struct pf_qihead rootqs = TAILQ_HEAD_INITIALIZER(rootqs);
-
__dead void
usage(void)
{
@@ -361,11 +363,10 @@ pfctl_clear_interface_flags(int dev, int opts)
}
}
-void
+int
pfctl_clear_rules(int dev, int opts, char *anchorname)
{
- struct pfr_buffer t;
- int rv = 0;
+ struct pfr_buffer t;
memset(&t, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
@@ -373,9 +374,11 @@ pfctl_clear_rules(int dev, int opts, char *anchorname)
pfctl_trans(dev, &t, DIOCXBEGIN, 0) ||
pfctl_trans(dev, &t, DIOCXCOMMIT, 0)) {
pfctl_err(opts, 1, "%s", __func__);
- rv = 1;
+ return (1);
} else if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "rules cleared\n");
+
+ return (0);
}
void
@@ -2171,9 +2174,9 @@ pfctl_walk_get(int opts, struct pfioc_ruleset *pr, void
*warg)
{
struct pfr_anchoritem *pfra;
unsigned int len;
- struct {
- struct pfr_anchoritem *pfra;
- } *tail_pfra = warg;
+ struct pfr_anchors *anchors;
+
+ anchors = (struct pfr_anchors *) warg;
pfra = malloc(sizeof(*pfra));
if (pfra == NULL)
@@ -2191,16 +2194,13 @@ pfctl_walk_get(int opts, struct pfioc_ruleset *pr, void
*warg)
else
snprintf(pfra->pfra_anchorname, len, "%s", pr->name);
- if (tail_pfra->pfra != NULL)
- SLIST_INSERT_AFTER(tail_pfra->pfra, pfra, pfra_sle);
-
- tail_pfra->pfra = pfra;
+ SLIST_INSERT_HEAD(anchors, pfra, pfra_sle);
return (0);
}
int
-pfctl_walk_anchors(int dev, int opts, char *anchorname, void *warg,
+pfctl_walk_anchors(int dev, int opts, const char *anchorname, void *warg,
int(walkf)(int, struct pfioc_ruleset *, void *))
{
struct pfioc_ruleset pr;
@@ -2249,54 +2249,82 @@ pfctl_walk_anchors(int dev, int opts, char *anchorname,
void *warg,
int
pfctl_show_anchors(int dev, int opts, char *anchorname)
{
- int rv;
-
- rv = pfctl_walk_anchors(dev, opts, anchorname, NULL, pfctl_walk_show);
- return (rv);
+ return (
+ pfctl_walk_anchors(dev, opts, anchorname, NULL, pfctl_walk_show));
}
struct pfr_anchors *
-pfctl_get_anchors(int dev, int opts)
+pfctl_get_anchors(int dev, const char *anchorname, int opts)
{
struct pfioc_rulesetpr;
Re: pfctl - allow recursive flush operations [ 2/5 ]
Hello,
change introduces a pfctl_recurse() function we use to implement
recursive flush operation. The pfctl_recurse() is built from
two pieces:
- pfctl_walk_get(), which itself is a new callback for pfctl_walk()
we introduced in earlier patch. The pfctl_walk_get() puts every
anchor found in tree to single linked list
- pfctl_get_anchors() uses pfctl_walk() to collect all anchors
found in PF driver to list
- once there is a list of anchors pfctl_recurse() consumes
the list of anchors applying a nuke() function. The nuke()
function is an argument to pfctl_recurse(). The actual
implementation is subject of follow up diff.
I've done some expirements with making pfctl_recurse() to pass
a nuke() callback directly to pfctl_walk() function, but the results
where 'disappointing'. We soon were tripping a panics on dangling
pointers. I think we need to revisit pf_ruleset.c before we will
be able to get rid of the extra list here. I'd like to stay pragmatic
and go ahead with list and makes things nice and tidy later.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index ba3610a6a7b..2c5835df7c6 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -107,8 +107,12 @@ void pfctl_state_store(int, const char *);
void pfctl_state_load(int, const char *);
void pfctl_reset(int, int);
intpfctl_walk_show(int, struct pfioc_ruleset *, void *);
+intpfctl_walk_get(int, struct pfioc_ruleset *, void *);
intpfctl_walk_anchors(int, int, char *, void *,
int(*)(int, struct pfioc_ruleset *, void *));
+struct pfr_anchors *
+ pfctl_get_anchors(int, int);
+intpfctl_recurse(int, int, int(*nuke)(int, int, struct pfr_anchoritem *));
const char *clearopt;
char *rulesopt;
@@ -2123,6 +2127,39 @@ pfctl_walk_show(int opts, struct pfioc_ruleset *pr, void
*warg)
return (0);
}
+int
+pfctl_walk_get(int opts, struct pfioc_ruleset *pr, void *warg)
+{
+ struct pfr_anchoritem *pfra;
+ unsigned int len;
+ struct {
+ struct pfr_anchoritem *pfra;
+ } *tail_pfra = warg;
+
+ pfra = malloc(sizeof(*pfra));
+ if (pfra == NULL)
+ err(1, "%s", __func__);
+
+ len = strlen(pr->path) + 1;
+ len += strlen(pr->name) + 1;
+ pfra->pfra_anchorname = malloc(len);
+ if (pfra->pfra_anchorname == NULL)
+ err(1, "%s", __func__);
+
+ if (pr->path[0])
+ snprintf(pfra->pfra_anchorname, len, "%s/%s",
+ pr->path, pr->name);
+ else
+ snprintf(pfra->pfra_anchorname, len, "%s", pr->name);
+
+ if (tail_pfra->pfra != NULL)
+ SLIST_INSERT_AFTER(tail_pfra->pfra, pfra, pfra_sle);
+
+ tail_pfra->pfra = pfra;
+
+ return (0);
+}
+
int
pfctl_walk_anchors(int dev, int opts, char *anchorname, void *warg,
int(walkf)(int, struct pfioc_ruleset *, void *))
@@ -2131,7 +2168,7 @@ pfctl_walk_anchors(int dev, int opts, char *anchorname,
void *warg,
u_int32_tmnr, nr;
memset(&pr, 0, sizeof(pr));
- memcpy(pr.path, anchorname, sizeof(pr.path));
+ strlcpy(pr.path, anchorname, sizeof(pr.path));
if (ioctl(dev, DIOCGETRULESETS, &pr) == -1) {
if (errno == EINVAL)
fprintf(stderr, "Anchor '%s' not found.\n",
@@ -2179,6 +2216,56 @@ pfctl_show_anchors(int dev, int opts, char *anchorname)
return (rv);
}
+struct pfr_anchors *
+pfctl_get_anchors(int dev, int opts)
+{
+ struct pfioc_rulesetpr;
+ struct {
+ struct pfr_anchoritem *pfra;
+ } pfra;
+ static struct pfr_anchors anchors;
+
+ SLIST_INIT(&anchors);
+
+ memset(&pr, 0, sizeof(pr));
+ pfra.pfra = NULL;
+ pfctl_walk_get(opts, &pr, &pfra);
+ if (pfra.pfra == NULL) {
+ fprintf(stderr, "%s failed to allocate main anchor\n",
+ __func__);
+ exit(1);
+ }
+ SLIST_INSERT_HEAD(&anchors, pfra.pfra, pfra_sle);
+
+ opts |= PF_OPT_VERBOSE;
+ if (pfctl_walk_anchors(dev, opts, "", &pfra, pfctl_walk_get)) {
+ fprintf(stderr,
+ "%s failed to retreive list of anchors, can't continue\n",
+ __func__);
+ exit(1);
+ }
+
+ return (&anchors);
+}
+
+int
+pfctl_recurse(int dev, int opts, int(*nuke)(int, int, struct pfr_anchoritem *))
+{
+ int rv = 0;
+ struct pfr_anchors *anchors;
+ struct pfr_anchoritem *pfra, *pfra_save;
+
+ anchors = pfctl_get_anchors(dev, opts);
+ SLIST_FOREACH_SAFE(pfra, anchors, pfra_sle, pfra_save) {
+ rv |= nuke(dev, opts, pfra);
+ SLIST_REMOVE(anchors, pfra, pfr_anchoritem, pfra_sle);
+ free(pfra->pfra_anchorname);
+
Re: pfctl - allow recursive flush operations [ 3/5 ]
Hello,
change below splits 'pfctl_show_anchors()' to two functions:
- pfctl_walk_anchors() function, which can traverse all anchor
tree loaded to PF driver and
- pfctl_walk_show(), which is a call back for pfctl_walk_anchors()
it just prints anchor name
this change is a foundation for a recursive flush.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index d1a309d919f..ba3610a6a7b 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -106,6 +106,9 @@ const char *pfctl_lookup_option(char *, const char **);
void pfctl_state_store(int, const char *);
void pfctl_state_load(int, const char *);
void pfctl_reset(int, int);
+intpfctl_walk_show(int, struct pfioc_ruleset *, void *);
+intpfctl_walk_anchors(int, int, char *, void *,
+int(*)(int, struct pfioc_ruleset *, void *));
const char *clearopt;
char *rulesopt;
@@ -2109,7 +2112,20 @@ pfctl_debug(int dev, u_int32_t level, int opts)
}
int
-pfctl_show_anchors(int dev, int opts, char *anchorname)
+pfctl_walk_show(int opts, struct pfioc_ruleset *pr, void *warg)
+{
+ if (pr->path[0]) {
+ if (pr->path[0] != '_' || (opts & PF_OPT_VERBOSE))
+ printf(" %s/%s\n", pr->path, pr->name);
+ } else if (pr->name[0] != '_' || (opts & PF_OPT_VERBOSE))
+ printf(" %s\n", pr->name);
+
+ return (0);
+}
+
+int
+pfctl_walk_anchors(int dev, int opts, char *anchorname, void *warg,
+int(walkf)(int, struct pfioc_ruleset *, void *))
{
struct pfioc_ruleset pr;
u_int32_tmnr, nr;
@@ -2134,19 +2150,35 @@ pfctl_show_anchors(int dev, int opts, char *anchorname)
if (!strcmp(pr.name, PF_RESERVED_ANCHOR))
continue;
sub[0] = '\0';
- if (pr.path[0]) {
- strlcat(sub, pr.path, sizeof(sub));
- strlcat(sub, "/", sizeof(sub));
- }
- strlcat(sub, pr.name, sizeof(sub));
- if (sub[0] != '_' || (opts & PF_OPT_VERBOSE))
- printf(" %s\n", sub);
- if ((opts & PF_OPT_VERBOSE) && pfctl_show_anchors(dev, opts,
sub))
+
+ if (walkf(opts, &pr, warg))
return (-1);
+
+ if (opts & PF_OPT_VERBOSE) {
+ charsub[PATH_MAX];
+
+ if (pr.path[0])
+ snprintf(sub, sizeof(sub), "%s/%s",
+ pr.path, pr.name);
+ else
+ snprintf(sub, sizeof(sub), "%s",
+ pr.name);
+ if (pfctl_walk_anchors(dev, opts, sub, warg, walkf))
+ return (-1);
+ }
}
return (0);
}
+int
+pfctl_show_anchors(int dev, int opts, char *anchorname)
+{
+ int rv;
+
+ rv = pfctl_walk_anchors(dev, opts, anchorname, NULL, pfctl_walk_show);
+ return (rv);
+}
+
const char *
pfctl_lookup_option(char *cmd, const char **list)
{
Re: pfctl - allow recursive flush operations [ 4/5 ]
Hello,
pfctl diff bellow enables me to relax err(3) and errx(3) to warnings
where needed. The idea is to introduce PF_OPT_IGNFAIL and pfctl_err()
and pfctl_errx() wrappers around err(3) and errx(3) functions.
the wrappers, when acting in warning mode (PF_OPT_IGNFAIL set), would
also set exit_val global variable so main() can report error.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index d1a309d919f..cf91b435121 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -54,6 +54,8 @@
#include
+#include
+
#include "pfctl_parser.h"
#include "pfctl.h"
@@ -125,6 +127,7 @@ char*state_kill[2];
int dev = -1;
int first_title = 1;
int labels = 0;
+int exit_val = 0;
#define INDENT(d, o) do {\
if (o) {\
@@ -251,6 +254,40 @@ usage(void)
exit(1);
}
+void
+pfctl_err(int opts, int eval, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+
+ if ((opts & PF_OPT_IGNFAIL) == 0)
+ verr(eval, fmt, ap);
+ else
+ vwarn(fmt, ap);
+
+ va_end(ap);
+
+ exit_val = eval;
+}
+
+void
+pfctl_errx(int opts, int eval, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+
+ if ((opts & PF_OPT_IGNFAIL) == 0)
+ verrx(eval, fmt, ap);
+ else
+ vwarnx(fmt, ap);
+
+ va_end(ap);
+
+ exit_val = eval;
+}
+
int
pfctl_enable(int dev, int opts)
{
@@ -289,10 +326,10 @@ pfctl_clear_stats(int dev, const char *iface, int opts)
memset(&pi, 0, sizeof(pi));
if (iface != NULL && strlcpy(pi.pfiio_name, iface,
sizeof(pi.pfiio_name)) >= sizeof(pi.pfiio_name))
- errx(1, "invalid interface: %s", iface);
+ pfctl_errx(opts, 1, "invalid interface: %s", iface);
if (ioctl(dev, DIOCCLRSTATUS, &pi) == -1)
- err(1, "DIOCCLRSTATUS");
+ pfctl_err(opts, 1, "DIOCCLRSTATUS");
if ((opts & PF_OPT_QUIET) == 0) {
fprintf(stderr, "pf: statistics cleared");
if (iface != NULL)
@@ -311,32 +348,36 @@ pfctl_clear_interface_flags(int dev, int opts)
pi.pfiio_flags = PFI_IFLAG_SKIP;
if (ioctl(dev, DIOCCLRIFFLAG, &pi) == -1)
- err(1, "DIOCCLRIFFLAG");
+ pfctl_err(opts, 1, "DIOCCLRIFFLAG");
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "pf: interface flags reset\n");
}
}
-void
+int
pfctl_clear_rules(int dev, int opts, char *anchorname)
{
struct pfr_buffer t;
+ int rv = 0;
memset(&t, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
if (pfctl_add_trans(&t, PF_TRANS_RULESET, anchorname) ||
pfctl_trans(dev, &t, DIOCXBEGIN, 0) ||
- pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
- err(1, "pfctl_clear_rules");
- if ((opts & PF_OPT_QUIET) == 0)
+ pfctl_trans(dev, &t, DIOCXCOMMIT, 0)) {
+ pfctl_err(opts, 1, "%s", __func__);
+ rv = 1;
+ } else if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "rules cleared\n");
+
+ return (rv);
}
void
pfctl_clear_src_nodes(int dev, int opts)
{
if (ioctl(dev, DIOCCLRSRCNODES) == -1)
- err(1, "DIOCCLRSRCNODES");
+ pfctl_err(opts, 1, "DIOCCLRSRCNODES");
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "source tracking entries cleared\n");
}
@@ -349,10 +390,10 @@ pfctl_clear_states(int dev, const char *iface, int opts)
memset(&psk, 0, sizeof(psk));
if (iface != NULL && strlcpy(psk.psk_ifname, iface,
sizeof(psk.psk_ifname)) >= sizeof(psk.psk_ifname))
- errx(1, "invalid interface: %s", iface);
+ pfctl_errx(opts, 1, "invalid interface: %s", iface);
if (ioctl(dev, DIOCCLRSTATES, &psk) == -1)
- err(1, "DIOCCLRSTATES");
+ pfctl_err(opts, 1, "DIOCCLRSTATES");
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "%d states cleared\n", psk.psk_killed);
}
diff --git a/sbin/pfctl/pfctl.h b/sbin/pfctl/pfctl.h
index 7981cf66fdb..669ffcb3784 100644
--- a/sbin/pfctl/pfctl.h
+++ b/sbin/pfctl/pfctl.h
@@ -96,5 +96,7 @@ u_int32_t
int pfctl_trans(int, struct pfr_buffer *, u_long, int);
int pfctl_show_queues(int, const char *, int, int);
+voidpfctl_err(int, int, const char *, ...);
+voidpfctl_errx(int, int, const char *, ...);
#endif /* _PFCTL_H_ */
diff --git a/sbin/pfctl/pfctl_osfp.c b/sbin/pfctl/pfctl_osfp.c
index 79abfd1a7ab..8c0ea24171b 100644
--- a/sbin/pfctl/pfctl_osfp.c
+++ b/sbin/pfctl/pfctl_osfp.c
@@ -260,7 +260,7 @
pfctl - allow recursive flush operations [ 5/5 ]
Hello,
below is a complete change I'd like to commit. Diff below enables pfctl to
recursively flush objects (tables, rules, anchors) from PF. The same change
has been discussed last spring [1]. But the discussion died and all effort
dropped down on the floor. I'd like to restart the effort now.
The idea is to enable "pfctl -a '*' -F[art]" flush all objects recursively.
Change below does not update manpage yet. I'd like to fine tune manpage changes
in follow up commit. The idea is to make current implementation of "pfctl -a
'*' -sr" more generic. My change turns that to generic function, which can walk
tree of anchors and apply desired operation (show/flush) on every node found.
I'm going to change partial diffs just for reference, to make review bit
easier. However I'd like to commit a big diff in one go, because partial
diffs were never tested.
Also kn@ was testing that change back in 2019 and found some glitches [2].
IMO the issues pointed out by kn@ are present in code already, diff below
just uncovers them. We can investigate/fix them once the change below will
be in.
I'll send partial diffs as a reply to this email to keep things organized.
thanks and
regards
sashan
[1] https://marc.info/?l=openbsd-tech&m=155545759616085&w=2
[2] https://marc.info/?l=openbsd-tech&m=155769978704563&w=2
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index d1a309d919f..4a6bdc588a4 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -51,8 +51,9 @@
#include
#include
#include
-
#include
+#include
+#include
#include "pfctl_parser.h"
#include "pfctl.h"
@@ -63,7 +64,7 @@ intpfctl_disable(int, int);
voidpfctl_clear_queues(struct pf_qihead *);
voidpfctl_clear_stats(int, const char *, int);
voidpfctl_clear_interface_flags(int, int);
-voidpfctl_clear_rules(int, int, char *);
+int pfctl_clear_rules(int, int, char *);
voidpfctl_clear_src_nodes(int, int);
voidpfctl_clear_states(int, const char *, int);
struct addrinfo *
@@ -106,6 +107,17 @@ const char *pfctl_lookup_option(char *, const char **);
void pfctl_state_store(int, const char *);
void pfctl_state_load(int, const char *);
void pfctl_reset(int, int);
+intpfctl_walk_show(int, struct pfioc_ruleset *, void *);
+intpfctl_walk_get(int, struct pfioc_ruleset *, void *);
+intpfctl_walk_anchors(int, int, const char *, void *,
+int(*)(int, struct pfioc_ruleset *, void *));
+struct pfr_anchors *
+ pfctl_get_anchors(int, const char *, int);
+intpfctl_recurse(int, int, const char *,
+ int(*)(int, int, struct pfr_anchoritem *));
+intpfctl_call_clearrules(int, int, struct pfr_anchoritem *);
+intpfctl_call_cleartables(int, int, struct pfr_anchoritem *);
+intpfctl_call_clearanchors(int, int, struct pfr_anchoritem *);
const char *clearopt;
char *rulesopt;
@@ -125,6 +137,7 @@ char*state_kill[2];
int dev = -1;
int first_title = 1;
int labels = 0;
+int exit_val = 0;
#define INDENT(d, o) do {\
if (o) {\
@@ -234,7 +247,6 @@ static const char *optiopt_list[] = {
struct pf_qihead qspecs = TAILQ_HEAD_INITIALIZER(qspecs);
struct pf_qihead rootqs = TAILQ_HEAD_INITIALIZER(rootqs);
-
__dead void
usage(void)
{
@@ -251,6 +263,40 @@ usage(void)
exit(1);
}
+void
+pfctl_err(int opts, int eval, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+
+ if ((opts & PF_OPT_IGNFAIL) == 0)
+ verr(eval, fmt, ap);
+ else
+ vwarn(fmt, ap);
+
+ va_end(ap);
+
+ exit_val = eval;
+}
+
+void
+pfctl_errx(int opts, int eval, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+
+ if ((opts & PF_OPT_IGNFAIL) == 0)
+ verrx(eval, fmt, ap);
+ else
+ vwarnx(fmt, ap);
+
+ va_end(ap);
+
+ exit_val = eval;
+}
+
int
pfctl_enable(int dev, int opts)
{
@@ -289,10 +335,10 @@ pfctl_clear_stats(int dev, const char *iface, int opts)
memset(&pi, 0, sizeof(pi));
if (iface != NULL && strlcpy(pi.pfiio_name, iface,
sizeof(pi.pfiio_name)) >= sizeof(pi.pfiio_name))
- errx(1, "invalid interface: %s", iface);
+ pfctl_errx(opts, 1, "invalid interface: %s", iface);
if (ioctl(dev, DIOCCLRSTATUS, &pi) == -1)
- err(1, "DIOCCLRSTATUS");
+ pfctl_err(opts, 1, "DIOCCLRSTATUS");
if ((opts & PF_OPT_QUIET) == 0) {
fprintf(stderr, "pf: statistics cleared");
if (iface != NULL)
@@ -311,32 +357,35 @@ pfctl_clear_interface_flags(int dev, int opts)
pi.pfiio_flags = PFI_IFLAG_SKIP;
if (ioctl(dev, DIOCCLRIFFLAG, &pi) == -1)
-
umb(4) authentication
Hello, I had an itch, so I scratched it. The umb(4) driver has a FIXME in if_umb.c for user name and passphrase support. My LTE provider happens to require this so I thought I'd have a go. The provider seems happy with my umb changes, and I've added some bits in ifconfig(8) to allow setting of the authentication information (user/pass/protocol): umb0: flags=8851 mtu 1500 index 5 priority 6 llprio 3 roaming disabled registration home network state up cell-class LTE rssi -67dBm speed 47.7Mps up 95.4Mps down SIM initialized PIN valid (3 attempts left) subscriber-id 999 ICC-id 999 provider xx device MC7700 IMEI 999 firmware SWI9200X_03.05.19.00Aap phone# 999 APN xx User Pass x Auth CHAP dns x.x.x.x x.x.x.x groups: egress status: active inet x.x.x.x --> x.x.x.x netmask 0xfffc (please excuse line wraps, and I've redacted possibly sensitive info) Now, this is my first attempt at OpenBSD development, and writing my changes raised a few questions. So, does someone want to spare a few minutes off-list, or should I just clutter up tech@ ? (I've read the top of the Makefile, I've been through the web pages on AnonCVS and release(8)) Thanks, Lee.
snmp(1) Better varbind exception support
Currently we only support noSuchObject by overloading BER_TYPE_EOC.
This is wrong.
Diff below adds support for all 3 exception cases.
OK?
martijn@
Index: smi.c
===
RCS file: /cvs/src/usr.bin/snmp/smi.c,v
retrieving revision 1.6
diff -u -p -r1.6 smi.c
--- smi.c 24 Oct 2019 12:39:26 - 1.6
+++ smi.c 13 Jan 2020 13:48:18 -
@@ -365,9 +365,19 @@ smi_print_element(struct ber_element *ro
print_hint ? "IpAddress: " : "",
inet_ntoa(*(struct in_addr *)buf)) == -1)
goto fail;
- } else if (root->be_class == BER_CLASS_CONTEXT &&
- root->be_type == BER_TYPE_EOC) {
- str = strdup("No Such Object available on this agent at
this OID");
+ } else if (root->be_class == BER_CLASS_CONTEXT) {
+ if (root->be_type == SNMP_E_NOSUCHOBJECT)
+ str = strdup("No Such Object available on this "
+ "agent at this OID");
+ else if (root->be_type == SNMP_E_NOSUCHINSTANCE)
+ str = strdup("No Such Instance currently "
+ "exists at this OID");
+ else if (root->be_type == SNMP_E_ENDOFMIB)
+ str = strdup("No more variables left in this "
+ "MIB View (It is past the end of the MIB "
+ "tree)");
+ else
+ str = strdup("Unknown status at this OID");
} else {
for (i = 0; i < root->be_len; i++) {
if (!isprint(buf[i])) {
Index: snmp.h
===
RCS file: /cvs/src/usr.bin/snmp/snmp.h,v
retrieving revision 1.6
diff -u -p -r1.6 snmp.h
--- snmp.h 3 Oct 2019 11:02:26 - 1.6
+++ snmp.h 13 Jan 2020 13:48:18 -
@@ -108,6 +108,12 @@ enum snmp_security_model {
SNMP_SEC_TSM= 4
};
+enum snmp_application_exception {
+ SNMP_E_NOSUCHOBJECT = 0,
+ SNMP_E_NOSUCHINSTANCE = 1,
+ SNMP_E_ENDOFMIB = 2
+};
+
struct snmp_agent;
struct snmp_sec {
Re: vdsp(4) & tsleep_nsec(9)
On 13/01/20(Mon) 13:35, Mark Kettenis wrote: > > Date: Mon, 13 Jan 2020 13:20:45 +0100 > > From: Martin Pieuchot > > > > vdsp(4) uses a workaround to not block forever in case the hypervisor > > doesn't generate an interrupt. The current behavior is to wait the > > smallest possible interval: one tick. > > The hypervisor is known not to generate an interrupt. > > > Since this is a documented workaround the value doesn't matter much, so > > convert the driver to tsleep_nsec(9) with a conservative value. This > > value could be reduced later on. > > > > Ok? > > Please, make this 10ms; that is what we have now and 100ms would > defenitely be too long. Sure, updated diff below. Index: arch/sparc64/dev/vdsp.c === RCS file: /cvs/src/sys/arch/sparc64/dev/vdsp.c,v retrieving revision 1.46 diff -u -p -r1.46 vdsp.c --- arch/sparc64/dev/vdsp.c 6 Oct 2019 16:24:14 - 1.46 +++ arch/sparc64/dev/vdsp.c 13 Jan 2020 13:01:34 - @@ -894,7 +894,8 @@ vdsp_sendmsg(struct vdsp_softc *sc, void * we specify a timeout such that we don't * block forever. */ - err = tsleep(lc->lc_txq, PWAIT, "vdsp", 1); + err = tsleep_nsec(lc->lc_txq, PWAIT, "vdsp", + MSEC_TO_NSEC(10)); } } while (dowait && err == EWOULDBLOCK); }
Re: softdep semaphores & tsleep_nsec(9)
On Sat, Jan 11, 2020 at 03:43:45PM +0100, Martin Pieuchot wrote:
> The custom semaphores used by softdep always use infinite tsleep(9). So
> remove the `timo' argument from sema_init() and switch the implementation
> to tsleep_nsec(9) and INFSLP.
>
> ok?
OK bluhm@
> Index: ufs/ffs/ffs_softdep.c
> ===
> RCS file: /cvs/src/sys/ufs/ffs/ffs_softdep.c,v
> retrieving revision 1.146
> diff -u -p -r1.146 ffs_softdep.c
> --- ufs/ffs/ffs_softdep.c 4 Jan 2020 16:22:36 - 1.146
> +++ ufs/ffs/ffs_softdep.c 11 Jan 2020 14:40:55 -
> @@ -289,21 +289,19 @@ struct sema {
> pid_t holder;
> char*name;
> int prio;
> - int timo;
> };
> -STATIC void sema_init(struct sema *, char *, int, int);
> +STATIC void sema_init(struct sema *, char *, int);
> STATIC int sema_get(struct sema *, struct lockit *);
> STATIC void sema_release(struct sema *);
>
> STATIC void
> -sema_init(struct sema *semap, char *name, int prio, int timo)
> +sema_init(struct sema *semap, char *name, int prio)
> {
>
> semap->holder = -1;
> semap->value = 0;
> semap->name = name;
> semap->prio = prio;
> - semap->timo = timo;
> }
>
> STATIC int
> @@ -314,7 +312,7 @@ sema_get(struct sema *semap, struct lock
> if (semap->value++ > 0) {
> if (interlock != NULL)
> s = FREE_LOCK_INTERLOCKED(interlock);
> - tsleep((caddr_t)semap, semap->prio, semap->name, semap->timo);
> + tsleep_nsec(semap, semap->prio, semap->name, INFSLP);
> if (interlock != NULL) {
> ACQUIRE_LOCK_INTERLOCKED(interlock, s);
> FREE_LOCK(interlock);
> @@ -1176,12 +1174,12 @@ softdep_initialize(void)
> arc4random_buf(&softdep_hashkey, sizeof(softdep_hashkey));
> pagedep_hashtbl = hashinit(initialvnodes / 5, M_PAGEDEP, M_WAITOK,
> &pagedep_hash);
> - sema_init(&pagedep_in_progress, "pagedep", PRIBIO, 0);
> + sema_init(&pagedep_in_progress, "pagedep", PRIBIO);
> inodedep_hashtbl = hashinit(initialvnodes, M_INODEDEP, M_WAITOK,
> &inodedep_hash);
> - sema_init(&inodedep_in_progress, "inodedep", PRIBIO, 0);
> + sema_init(&inodedep_in_progress, "inodedep", PRIBIO);
> newblk_hashtbl = hashinit(64, M_NEWBLK, M_WAITOK, &newblk_hash);
> - sema_init(&newblk_in_progress, "newblk", PRIBIO, 0);
> + sema_init(&newblk_in_progress, "newblk", PRIBIO);
> timeout_set(&proc_waiting_timeout, pause_timer, NULL);
> pool_init(&pagedep_pool, sizeof(struct pagedep), 0, IPL_NONE,
> PR_WAITOK, "pagedep", NULL);
diff: simplify globbing in ftpd(8)
Hi,
This diff simplifies globbing for the ftpd(8) ls and stat command. And
it avoids to glob for the parameters "-lgA".
Due, we just pass a single string to the ftpd_ls() function, we don't
need to provide infrastructure for a dynamic list of arguments.
Tested it manually and by regression test.
OK?
Bye,
Jan
Index: extern.h
===
RCS file: /cvs/src/libexec/ftpd/extern.h,v
retrieving revision 1.20
diff -u -p -r1.20 extern.h
--- extern.h8 May 2019 23:56:48 - 1.20
+++ extern.h13 Jan 2020 11:03:39 -
@@ -68,7 +68,7 @@ void delete(char *);
void dologout(int);
void fatal(char *);
intftpd_pclose(FILE *, pid_t);
-FILE *ftpd_ls(char *, char *, pid_t *);
+FILE *ftpd_ls(const char *, pid_t *);
int get_line(char *, int, FILE *);
void ftpdlogwtmp(char *, char *, char *);
void lreply(int, const char *, ...);
Index: ftpd.c
===
RCS file: /cvs/src/libexec/ftpd/ftpd.c,v
retrieving revision 1.228
diff -u -p -r1.228 ftpd.c
--- ftpd.c 3 Jul 2019 03:24:04 - 1.228
+++ ftpd.c 13 Jan 2020 11:15:31 -
@@ -1124,7 +1124,7 @@ retrieve(enum ret_cmd cmd, char *name)
fin = fopen(name, "r");
st.st_size = 0;
} else {
- fin = ftpd_ls("-lgA", name, &pid);
+ fin = ftpd_ls(name, &pid);
st.st_size = -1;
st.st_blksize = BUFSIZ;
}
@@ -1730,7 +1730,7 @@ statfilecmd(char *filename)
int c;
int atstart;
pid_t pid;
- fin = ftpd_ls("-lgA", filename, &pid);
+ fin = ftpd_ls(filename, &pid);
if (fin == NULL) {
reply(451, "Local resource failure");
return;
Index: popen.c
===
RCS file: /cvs/src/libexec/ftpd/popen.c,v
retrieving revision 1.28
diff -u -p -r1.28 popen.c
--- popen.c 28 Jun 2019 13:32:53 - 1.28
+++ popen.c 13 Jan 2020 12:07:14 -
@@ -60,51 +60,44 @@
#define MAX_GARGV 1000
FILE *
-ftpd_ls(char *arg, char *path, pid_t *pidptr)
+ftpd_ls(const char *path, pid_t *pidptr)
{
char *cp;
FILE *iop;
- int argc = 0, gargc, pdes[2];
+ int argc = 0, pdes[2];
pid_t pid;
- char **pop, *argv[MAX_ARGV], *gargv[MAX_GARGV];
+ char **pop, *argv[MAX_ARGV];
if (pipe(pdes) == -1)
return (NULL);
/* break up string into pieces */
argv[argc++] = "/bin/ls";
- if (arg != NULL)
- argv[argc++] = arg;
- if (path != NULL)
- argv[argc++] = path;
- argv[argc] = NULL;
- argv[MAX_ARGV-1] = NULL;
+ argv[argc++] = "-lgA";
- /* glob each piece */
- gargv[0] = argv[0];
- for (gargc = argc = 1; argv[argc]; argc++) {
+ /* glob that path */
+ if (path != NULL) {
glob_t gl;
memset(&gl, 0, sizeof(gl));
- if (glob(argv[argc],
+ if (glob(path,
GLOB_BRACE|GLOB_NOCHECK|GLOB_QUOTE|GLOB_TILDE|GLOB_LIMIT,
NULL, &gl)) {
- if (gargc < MAX_GARGV-1) {
- gargv[gargc++] = strdup(argv[argc]);
- if (gargv[gargc -1] == NULL)
- fatal ("Out of memory.");
- }
+ argv[argc++] = strdup(path);
+ if (argv[argc -1] == NULL)
+ fatal ("Out of memory.");
} else if (gl.gl_pathc > 0) {
- for (pop = gl.gl_pathv; *pop && gargc < MAX_GARGV-1;
pop++) {
- gargv[gargc++] = strdup(*pop);
- if (gargv[gargc - 1] == NULL)
+ for (pop = gl.gl_pathv; *pop && argc < MAX_GARGV-1;
pop++) {
+ argv[argc++] = strdup(*pop);
+ if (argv[argc - 1] == NULL)
fatal ("Out of memory.");
}
}
globfree(&gl);
}
- gargv[gargc] = NULL;
+ argv[argc] = NULL;
+ argv[MAX_ARGV-1] = NULL;
iop = NULL;
@@ -128,15 +121,15 @@ ftpd_ls(char *arg, char *path, pid_t *pi
/* reset getopt for ls_main */
optreset = optind = 1;
- exit(ls_main(gargc, gargv));
+ exit(ls_main(argc, argv));
}
/* parent; assume fdopen can't fail... */
iop = fdopen(pdes[0], "r");
(void)close(pdes[1]);
*pidptr = pid;
-pfree: for (argc = 1; gargv[argc] != NULL; argc++)
- free(gargv[argc]);
+pfree: for (argc = 2; argv[argc] != NULL; argc++)
+ free(argv[argc]);
return (iop);
}
sig
Re: sosleep(), SO_RCVTIMEO and TIMEVAL_TO_NSEC()
OK bluhm@
On Mon, Jan 13, 2020 at 12:27:12PM +0100, Martin Pieuchot wrote:
> 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.c13 Jan 2020 11:24:04 -
> @@ -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);
> @@ -265,6 +267,15 @@ sofree(struct socket *so, int s)
> }
> }
>
> +static inline uint64_t
> +solinger_nsec(struct socket *so)
> +{
> + if (so->so_linger == 0)
> + return INFSLP;
> +
> + return SEC_TO_NSEC(so->so_linger);
> +}
> +
> /*
> * Close a socket on last file table reference removal.
> * Initiate disconnect if connected.
> @@ -304,7 +315,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);
> + solinger_nsec(so));
> if (error)
> break;
> }
> @@ -1761,22 +1772,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 (EINVAL);
> + 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 +1924,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 +2144,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;
> + so->so_snd.sb_timeo_nsecs = head->so_snd.sb_timeo_nsecs;
> so->so_rcv.sb_wat = head->so_rcv.sb_wat;
> so->so_rcv.sb_lowat = head->so_rcv.sb_lowat;
> - so->so_rcv.sb_timeo = head->so_rcv.sb_timeo;
Re: vdsp(4) & tsleep_nsec(9)
> Date: Mon, 13 Jan 2020 13:20:45 +0100 > From: Martin Pieuchot > > vdsp(4) uses a workaround to not block forever in case the hypervisor > doesn't generate an interrupt. The current behavior is to wait the > smallest possible interval: one tick. The hypervisor is known not to generate an interrupt. > Since this is a documented workaround the value doesn't matter much, so > convert the driver to tsleep_nsec(9) with a conservative value. This > value could be reduced later on. > > Ok? Please, make this 10ms; that is what we have now and 100ms would defenitely be too long. > Index: arch/sparc64/dev/vdsp.c > === > RCS file: /cvs/src/sys/arch/sparc64/dev/vdsp.c,v > retrieving revision 1.46 > diff -u -p -r1.46 vdsp.c > --- arch/sparc64/dev/vdsp.c 6 Oct 2019 16:24:14 - 1.46 > +++ arch/sparc64/dev/vdsp.c 13 Jan 2020 12:18:48 - > @@ -894,7 +894,8 @@ vdsp_sendmsg(struct vdsp_softc *sc, void >* we specify a timeout such that we don't >* block forever. >*/ > - err = tsleep(lc->lc_txq, PWAIT, "vdsp", 1); > + err = tsleep_nsec(lc->lc_txq, PWAIT, "vdsp", > + MSEC_TO_NSEC(100)); > } > } while (dowait && err == EWOULDBLOCK); > } >
snmp(1) fix parse agent
So apparently I use IPv6 more often than the end-users.
This diff allows us to actually skip the {ud,tc}p6 prefix if we actually
present an IPv6 address as described in the manpage and done by
net-snmp.
While here I also made it possible possible to do a retry if
IPv6-address:port fails by pasting the port back to the hostname.
This allows us to do snmp walk ::1, similar to what net-snmp does.
Not that with this we actually comform better to what snmpcmd(1)
describes in their manpage, which apparently doesn't support ::1:161,
but only [::1]:161.
OK?
martijn@
Index: snmpc.c
===
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.17
diff -u -p -r1.17 snmpc.c
--- snmpc.c 26 Oct 2019 19:34:15 - 1.17
+++ snmpc.c 9 Jan 2020 16:30:54 -
@@ -918,7 +918,6 @@ snmpc_parseagent(char *agent, char *defa
hints.ai_socktype = SOCK_STREAM;
} else if (strcasecmp(specifier, "unix") == 0) {
hints.ai_family = AF_UNIX;
- hints.ai_socktype = SOCK_STREAM;
hints.ai_addr = (struct sockaddr *)&saddr;
hints.ai_addrlen = sizeof(saddr);
saddr.sun_len = sizeof(saddr);
@@ -928,45 +927,51 @@ snmpc_parseagent(char *agent, char *defa
errx(1, "Hostname path too long");
ai = &hints;
} else {
- port = hostname;
+ *--hostname = ':';
hostname = specifier;
- specifier = NULL;
- hints.ai_family = AF_INET;
- hints.ai_socktype = SOCK_DGRAM;
- }
- if (port == NULL) {
- if (hints.ai_family == AF_INET) {
- if ((port = strchr(hostname, ':')) != NULL)
- *port++ = '\0';
- } else if (hints.ai_family == AF_INET6) {
- if (hostname[0] == '[') {
- hostname++;
- if ((port = strchr(hostname, ']')) ==
NULL)
- errx(1, "invalid agent");
- *port++ = '\0';
- if (port[0] == ':')
- *port++ = '\0';
- else
- port = NULL;
- } else {
- if ((port = strrchr(hostname, ':')) ==
NULL)
- errx(1, "invalid agent");
- *port++ = '\0';
- }
- }
}
} else {
hostname = specifier;
- hints.ai_family = AF_INET;
- hints.ai_socktype = SOCK_DGRAM;
+ }
+
+ if (hints.ai_family == AF_INET) {
+ if ((port = strchr(hostname, ':')) != NULL)
+ *port++ = '\0';
+ } else if (hints.ai_family == AF_INET6 || hints.ai_family == 0) {
+ if (hostname[0] == '[') {
+ hints.ai_family = AF_INET6;
+ hostname++;
+ if ((port = strchr(hostname, ']')) == NULL)
+ errx(1, "invalid agent");
+ *port++ = '\0';
+ if (port[0] == ':')
+ *port++ = '\0';
+ else if (port[0] == '\0')
+ port = NULL;
+ else
+ errx(1, "invalid agent");
+ } else {
+ if ((port = strrchr(hostname, ':')) == NULL)
+ errx(1, "invalid agent");
+ *port++ = '\0';
+ }
}
if (hints.ai_family != AF_UNIX) {
+ if (hints.ai_socktype == 0)
+ hints.ai_socktype = SOCK_DGRAM;
if (port == NULL)
port = defaultport;
error = getaddrinfo(hostname, port, &hints, &ai0);
- if (error)
- errx(1, "%s", gai_strerror(error));
+ if (error) {
+ if (error != EAI_NODATA && port != defaultport)
+ errx(1, "%s", gai_strerror(error));
+ *--port = ':';
+ error = getaddrinfo(hostname, defaultport, &hints,
+ &ai0);
+ if (error)
+ errx(1, "%s", gai_strerror(error));
+ }
s = -1;
vdsp(4) & tsleep_nsec(9)
vdsp(4) uses a workaround to not block forever in case the hypervisor doesn't generate an interrupt. The current behavior is to wait the smallest possible interval: one tick. Since this is a documented workaround the value doesn't matter much, so convert the driver to tsleep_nsec(9) with a conservative value. This value could be reduced later on. Ok? Index: arch/sparc64/dev/vdsp.c === RCS file: /cvs/src/sys/arch/sparc64/dev/vdsp.c,v retrieving revision 1.46 diff -u -p -r1.46 vdsp.c --- arch/sparc64/dev/vdsp.c 6 Oct 2019 16:24:14 - 1.46 +++ arch/sparc64/dev/vdsp.c 13 Jan 2020 12:18:48 - @@ -894,7 +894,8 @@ vdsp_sendmsg(struct vdsp_softc *sc, void * we specify a timeout such that we don't * block forever. */ - err = tsleep(lc->lc_txq, PWAIT, "vdsp", 1); + err = tsleep_nsec(lc->lc_txq, PWAIT, "vdsp", + MSEC_TO_NSEC(100)); } } while (dowait && err == EWOULDBLOCK); }
Re: TIMESPEC_TO_NSEC(): futex(2), __thrsigdivert(2) & __thrsleep(2)
On Sun, Jan 12, 2020 at 01:42:44PM +0100, 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.
>
> Diff below introduces the previously discussed TIMESPEC_TO_NSEC(9) macro
> and makes use of it in 3 syscalls.
>
> Comments? Oks?
OK bluhm@
> 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.c 10 Jul 2019 15:52:17 - 1.13
> +++ kern/sys_futex.c 31 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, "thrs
Re: ldomctl: Fail on duplicate vcpu and memory parameters
On Sun, Jan 05, 2020 at 07:09:46PM +0100, Klemens Nanni wrote:
> Domains get to define their cores and memory only once unlike the other
> parameters of which it makes sense to have more than one.
>
> $ cat dup.conf
> domain primary {
> vcpu 2
> vcpu 2
> }
> $ ldomctl init-system -n dup.conf ; echo $?
> 0
> $ ./obj/ldomctl init-system -n dup.conf
> dup.conf:3 duplicate vcpu option
>
> OK?
Now that checks for mandatory options are in, this somewhat completes
it at the other end.
Here's the same diff with an additional check for duplicate iodevices,
only that those must be globally unique (just like domain names).
OK?
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/ldomctl/parse.y,v
retrieving revision 1.15
diff -u -p -r1.15 parse.y
--- parse.y 9 Jan 2020 22:06:23 - 1.15
+++ parse.y 13 Jan 2020 11:44:45 -
@@ -156,10 +156,18 @@ domainoptsl : domainopts nl
;
domainopts : VCPU vcpu {
+ if (domain->vcpu) {
+ yyerror("duplicate vcpu option");
+ YYERROR;
+ }
domain->vcpu = $2.count;
domain->vcpu_stride = $2.stride;
}
| MEMORY memory {
+ if (domain->memory) {
+ yyerror("duplicate memory option");
+ YYERROR;
+ }
domain->memory = $2;
}
| VDISK STRING {
@@ -180,10 +188,19 @@ domainopts: VCPU vcpu {
SIMPLEQ_INSERT_TAIL(&domain->var_list, var, entry);
}
| IODEVICE STRING {
- struct iodev *iodev = xmalloc(sizeof(struct iodev));
+ struct domain *odomain;
+ struct iodev *iodev;
+ SIMPLEQ_FOREACH(odomain, &conf->domain_list, entry)
+ SIMPLEQ_FOREACH(iodev, &odomain->iodev_list,
entry)
+ if (strcmp(iodev->path, $2) == 0) {
+ yyerror("iodevice assigned"
+ " twice: %s", $2);
+ YYERROR;
+ }
+ iodev = xmalloc(sizeof(struct iodev));
iodev->path = $2;
SIMPLEQ_INSERT_TAIL(&domain->iodev_list, iodev, entry);
- }
+ }
;
vnet_opts : { vnet_opts_default(); }
Re: sosleep(), SO_RCVTIMEO and TIMEVAL_TO_NSEC()
On 13/01/20(Mon) 12:27, Martin Pieuchot wrote:
> On 13/01/20(Mon) 12:16, Alexander Bluhm wrote:
> > On Sun, Jan 12, 2020 at 01:33:43PM +0100, Martin Pieuchot wrote:
> > > @@ -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;
> > > }
> >
> > In sosleep() the sleep forever semantics has been converted from 0
> > to INFSLP. We have to pay attention at every caller.
>
> Fixed in the diff below.
>
> > > @@ -333,11 +333,11 @@ nfs_connect(struct nfsmount *nmp, struct
> >
> > In this function is a call that has been forgotten in the conversion.
> > sosleep(so, &so->so_timeo, PSOCK, "nfscon", 2 * hz);
> >
> > Should we rename sosleep() to sosleep_nsec() and force to have all
> > the critical places in the diff? Its semantics has changed, maybe
> > we should change the name.
>
> I'm not fan of such churn, hopefully all tsleep(9) will be soon
> converted and there won't be a need for the "_nsec" suffix.
bluhm@ insisted on the _nsec naming, here's the diff.
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 13 Jan 2020 11:54:58 -
@@ -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);
@@ -265,6 +267,15 @@ sofree(struct socket *so, int s)
}
}
+static inline uint64_t
+solinger_nsec(struct socket *so)
+{
+ if (so->so_linger == 0)
+ return INFSLP;
+
+ return SEC_TO_NSEC(so->so_linger);
+}
+
/*
* Close a socket on last file table reference removal.
* Initiate disconnect if connected.
@@ -302,9 +313,9 @@ soclose(struct socket *so, int flags)
(flags & MSG_DONTWAIT))
goto drop;
while (so->so_state & SS_ISCONNECTED) {
- error = sosleep(so, &so->so_timeo,
+ error = sosleep_nsec(so, &so->so_timeo,
PSOCK | PCATCH, "netcls",
- so->so_linger * hz);
+ solinger_nsec(so));
if (error)
break;
}
@@ -1761,22 +1772,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 (EINVAL);
+ 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 +1924,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
Re: iked(8): better cryptographic defaults
Dropping modp1024 will cause some pain for Windows users, unfortunately you
need to either set a registry key or use a PowerShell applet to have it use
modp2048 or better.
I'm wondering if we should have some simple way to choose from various
default settings, similar to tls_config_set_ciphers() with its
secure/compat/legacy/all.
--
Sent from a phone, apologies for poor formatting.
On 12 January 2020 18:48:47 Tobias Heider wrote:
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: sosleep(), SO_RCVTIMEO and TIMEVAL_TO_NSEC()
On 13/01/20(Mon) 12:16, Alexander Bluhm wrote:
> On Sun, Jan 12, 2020 at 01:33:43PM +0100, Martin Pieuchot wrote:
> > @@ -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;
> > }
>
> In sosleep() the sleep forever semantics has been converted from 0
> to INFSLP. We have to pay attention at every caller.
Fixed in the diff below.
> > @@ -333,11 +333,11 @@ nfs_connect(struct nfsmount *nmp, struct
>
> In this function is a call that has been forgotten in the conversion.
> sosleep(so, &so->so_timeo, PSOCK, "nfscon", 2 * hz);
>
> Should we rename sosleep() to sosleep_nsec() and force to have all
> the critical places in the diff? Its semantics has changed, maybe
> we should change the name.
I'm not fan of such churn, hopefully all tsleep(9) will be soon
converted and there won't be a need for the "_nsec" suffix.
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 13 Jan 2020 11:24:04 -
@@ -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);
@@ -265,6 +267,15 @@ sofree(struct socket *so, int s)
}
}
+static inline uint64_t
+solinger_nsec(struct socket *so)
+{
+ if (so->so_linger == 0)
+ return INFSLP;
+
+ return SEC_TO_NSEC(so->so_linger);
+}
+
/*
* Close a socket on last file table reference removal.
* Initiate disconnect if connected.
@@ -304,7 +315,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);
+ solinger_nsec(so));
if (error)
break;
}
@@ -1761,22 +1772,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 (EINVAL);
+ 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 +1924,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);
Re: acpivout: try to consistently adjust brightness by 5%
On Mon, Jan 13, 2020 at 11:58:11AM +0100, Klemens Nanni wrote: > On Sun, Oct 13, 2019 at 09:28:26PM -0500, joshua stein wrote: > > When responding to hardware keys to increment or decrement screen > > brightness, don't just adjust by 1 BCL level as there may be 100 > > levels. Find the next brightness level that is at least 5% up or > > down, and use that. > revision 1.14 > date: 2019/10/21 16:32:51; author: jcs; state: Exp; lines: +20 -33; > When incrementing or decrementing screen brightness, don't just > adjust by 1 BCL level as there may be 100 levels. Find the next > brightness level that is at least 5% up or down and use that. > > ok kettenis > > This diff broke backlight adjustment on my X230: With 100% screen > brightness, pressing the function keys to *de*crease it won't do > anything. Booting a kernel with ACPIVIDEO_DEBUG suggests that > acpivout_find_brightness() is unable to find the next level below 100% > that is at least 5% less than the full brightness, hence it returns > something above 95% such that acpivout_brightness_step() gets stuck and > will set brightness to 100% again. > > `xbacklight -50' will properly decrease brightness to 50%, then the > function keys also work and I can decrease to 44% as the immediate lower > level, however increasing it again never works and is stuck at 44%. > > In general, setting a lower brightness level with xbacklight(1) makes > the function keys work but only ever in the range of [0, n) -- I can > never reach a higher level than set with xbacklight. > > `xbacklight +100' works and reverting avpivout.c revision 1.14 makes > everything work again for me. > > tobhe has the exact same issues on his T420. > > Below is diff to show my X230 brightness levels, dmesg as well. The problem is that the last two values are 67 and 100. If you go 5% down, it's 95. The nearest will still be 100. The code then realizes that it's the same level as before, and does nlevel--. But nlevel-- is 99, and not 67, because nlevel is the value and not the index of the bcl array. So in essence the change needed is to decrease the index, not the value, and then look up the value. Patrick
Re: sosleep(), SO_RCVTIMEO and TIMEVAL_TO_NSEC()
On Sun, Jan 12, 2020 at 01:33:43PM +0100, Martin Pieuchot wrote:
> @@ -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;
> }
In sosleep() the sleep forever semantics has been converted from 0
to INFSLP. We have to pay attention at every caller.
> @@ -333,11 +333,11 @@ nfs_connect(struct nfsmount *nmp, struct
In this function is a call that has been forgotten in the conversion.
sosleep(so, &so->so_timeo, PSOCK, "nfscon", 2 * hz);
Should we rename sosleep() to sosleep_nsec() and force to have all
the critical places in the diff? Its semantics has changed, maybe
we should change the name.
bluhm
Re: acpicpu log in dmesg
> From: Hrvoje Popovski > Date: Mon, 13 Jan 2020 11:53:36 +0100 > > Hi all, > > with today's snapshot i'm seeing acpipci log below in dmesg. is this ok? > do i need to report it ? Not if everything on these machines works ;) The messages are debug information that will help diagnosing problems with a diff that is in snaps. The diff itself should help with making modern server machines (such as recent Dell servers) work. > acpipci0 at acpi0 PC00: 0x 0x0011 0x0001 > extent `acpipci0 pcibus' (0x0 - 0xff), flags=0 > 0x40 - 0xff > extent `acpipci0 pciio' (0x0 - 0x), flags=0 > 0x3b0 - 0x3df > 0xcf8 - 0xcff > 0x4000 - 0x > extent `acpipci0 pcimem' (0x0 - 0x), flags=0 > 0x0 - 0xb > 0x10 - 0xe0ff > 0xfec0 - 0x63dbfff > 0x1 - 0x > acpicmos0 at acpi0 > "IPI0001" at acpi0 not configured > acpipci1 at acpi0 PC01: 0x 0x0011 0x0001 > extent `acpipci1 pcibus' (0x0 - 0xff), flags=0 > 0x0 - 0x3f > 0x80 - 0xff > extent `acpipci1 pciio' (0x0 - 0x), flags=0 > 0x0 - 0x3fff > 0x7000 - 0x > extent `acpipci1 pcimem' (0x0 - 0x), flags=0 > 0x0 - 0x8fff > 0xab00 - 0x47e7fff > 0x63dc000 - 0x > acpipci2 at acpi0 PC02: 0x 0x0011 0x0001 > extent `acpipci2 pcibus' (0x0 - 0xff), flags=0 > 0x0 - 0x7f > 0xc0 - 0xff > extent `acpipci2 pciio' (0x0 - 0x), flags=0 > 0x0 - 0x6fff > 0xa000 - 0x > extent `acpipci2 pcimem' (0x0 - 0x), flags=0 > 0x0 - 0xaaff > 0xc600 - 0x2bf3fff > 0x47e8000 - 0x > acpipci3 at acpi0 PC03: 0x 0x0011 0x0001 > extent `acpipci3 pcibus' (0x0 - 0xff), flags=0 > 0x0 - 0xbf > extent `acpipci3 pciio' (0x0 - 0x), flags=0 > 0x0 - 0x3af > 0x3e0 - 0x9fff > 0x1 - 0x > extent `acpipci3 pcimem' (0x0 - 0x), flags=0 > 0x0 - 0x9 > 0xc - 0xc5ff > 0xe100 - 0xff > 0x2bf4000 - 0x > >
Re: acpivout: try to consistently adjust brightness by 5%
On Sun, Oct 13, 2019 at 09:28:26PM -0500, joshua stein wrote:
> When responding to hardware keys to increment or decrement screen
> brightness, don't just adjust by 1 BCL level as there may be 100
> levels. Find the next brightness level that is at least 5% up or
> down, and use that.
revision 1.14
date: 2019/10/21 16:32:51; author: jcs; state: Exp; lines: +20 -33;
When incrementing or decrementing screen brightness, don't just
adjust by 1 BCL level as there may be 100 levels. Find the next
brightness level that is at least 5% up or down and use that.
ok kettenis
This diff broke backlight adjustment on my X230: With 100% screen
brightness, pressing the function keys to *de*crease it won't do
anything. Booting a kernel with ACPIVIDEO_DEBUG suggests that
acpivout_find_brightness() is unable to find the next level below 100%
that is at least 5% less than the full brightness, hence it returns
something above 95% such that acpivout_brightness_step() gets stuck and
will set brightness to 100% again.
`xbacklight -50' will properly decrease brightness to 50%, then the
function keys also work and I can decrease to 44% as the immediate lower
level, however increasing it again never works and is stuck at 44%.
In general, setting a lower brightness level with xbacklight(1) makes
the function keys work but only ever in the range of [0, n) -- I can
never reach a higher level than set with xbacklight.
`xbacklight +100' works and reverting avpivout.c revision 1.14 makes
everything work again for me.
tobhe has the exact same issues on his T420.
Below is diff to show my X230 brightness levels, dmesg as well.
Index: dev/acpi/acpivout.c
===
RCS file: /cvs/src/sys/dev/acpi/acpivout.c,v
retrieving revision 1.16
diff -u -p -r1.16 acpivout.c
--- dev/acpi/acpivout.c 14 Dec 2019 10:57:48 - 1.16
+++ dev/acpi/acpivout.c 13 Jan 2020 10:47:13 -
@@ -34,6 +34,7 @@ int acpivout_match(struct device *, void
void acpivout_attach(struct device *, struct device *, void *);
intacpivout_notify(struct aml_node *, int, void *);
+#define ACPIVIDEO_DEBUG
#ifdef ACPIVIDEO_DEBUG
#define DPRINTF(x) printf x
#else
@@ -215,6 +216,9 @@ int
acpivout_find_brightness(struct acpivout_softc *sc, int level)
{
int i, mid;
+
+ for (i = 0; i < sc->sc_bcl_len; i++)
+ printf("%s: sc->sc_bcl[%d]: %d\n", __func__, i, sc->sc_bcl[i]);
for (i = 0; i < sc->sc_bcl_len - 1; i++) {
mid = sc->sc_bcl[i] + (sc->sc_bcl[i + 1] - sc->sc_bcl[i]) / 2;
OpenBSD 6.6-current (GENERIC.MP) #3: Mon Jan 13 11:47:31 CET 2020
k...@eru.my.domain:/sys/arch/amd64/compile/GENERIC.MP
real mem = 17118126080 (16325MB)
avail mem = 16586883072 (15818MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xbff31020 (17 entries)
bios0: vendor coreboot version "CBET4000 x230-seabios" date 01/07/2020
bios0: LENOVO 2325A95
acpi0 at bios0: ACPI 4.0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP SSDT MCFG TCPA APIC DMAR HPET
acpi0: wakeup devices HDEF(S4) EHC1(S4) EHC2(S4) XHC_(S4) SLPB(S3) LID_(S3)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xf000, bus 0-63
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i5-3320M CPU @ 2.60GHz, 2594.47 MHz, 06-3a-09
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM) i5-3320M CPU @ 2.60GHz, 2594.11 MHz, 06-3a-09
cpu1:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: Intel(R) Core(TM) i5-3320M CPU @ 2.60GHz, 2594.12 MHz, 06-3a-09
cpu2:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCN
acpicpu log in dmesg
Hi all, with today's snapshot i'm seeing acpipci log below in dmesg. is this ok? do i need to report it ? acpipci0 at acpi0 PC00: 0x 0x0011 0x0001 extent `acpipci0 pcibus' (0x0 - 0xff), flags=0 0x40 - 0xff extent `acpipci0 pciio' (0x0 - 0x), flags=0 0x3b0 - 0x3df 0xcf8 - 0xcff 0x4000 - 0x extent `acpipci0 pcimem' (0x0 - 0x), flags=0 0x0 - 0xb 0x10 - 0xe0ff 0xfec0 - 0x63dbfff 0x1 - 0x acpicmos0 at acpi0 "IPI0001" at acpi0 not configured acpipci1 at acpi0 PC01: 0x 0x0011 0x0001 extent `acpipci1 pcibus' (0x0 - 0xff), flags=0 0x0 - 0x3f 0x80 - 0xff extent `acpipci1 pciio' (0x0 - 0x), flags=0 0x0 - 0x3fff 0x7000 - 0x extent `acpipci1 pcimem' (0x0 - 0x), flags=0 0x0 - 0x8fff 0xab00 - 0x47e7fff 0x63dc000 - 0x acpipci2 at acpi0 PC02: 0x 0x0011 0x0001 extent `acpipci2 pcibus' (0x0 - 0xff), flags=0 0x0 - 0x7f 0xc0 - 0xff extent `acpipci2 pciio' (0x0 - 0x), flags=0 0x0 - 0x6fff 0xa000 - 0x extent `acpipci2 pcimem' (0x0 - 0x), flags=0 0x0 - 0xaaff 0xc600 - 0x2bf3fff 0x47e8000 - 0x acpipci3 at acpi0 PC03: 0x 0x0011 0x0001 extent `acpipci3 pcibus' (0x0 - 0xff), flags=0 0x0 - 0xbf extent `acpipci3 pciio' (0x0 - 0x), flags=0 0x0 - 0x3af 0x3e0 - 0x9fff 0x1 - 0x extent `acpipci3 pcimem' (0x0 - 0x), flags=0 0x0 - 0x9 0xc - 0xc5ff 0xe100 - 0xff 0x2bf4000 - 0x
Re: sparc64: find root device on hardware RAID
> Date: Mon, 13 Jan 2020 10:59:30 +0100 > From: Klemens Nanni > > On Fri, Jan 03, 2020 at 08:30:42PM +0100, Mark Kettenis wrote: > > Can we leave out the #ifdef __sparc64__? Unless somebody can come up > > with a really good reason for it... > The code should be safe on all platforms but I put it in to ensure I'm > not breaking stuff I cannot test, e.g. anything else than sparc64/OBP. > > deraadt expressed the same concerns. > > With the two of you arguing for removal, I'm confident enough to remove > it and make mpii(4) MI again; unless I hear arguments against it, I'll > commit this soon. Great, ok kettenis@ > Index: mpii.c > === > RCS file: /cvs/src/sys/dev/pci/mpii.c,v > retrieving revision 1.125 > diff -u -p -r1.125 mpii.c > --- mpii.c3 Jan 2020 08:39:31 - 1.125 > +++ mpii.c13 Jan 2020 09:59:01 - > @@ -930,15 +930,13 @@ mpii_scsi_probe(struct scsi_link *link) > return (EINVAL); > > link->port_wwn = letoh64(vpg.wwid); > -#ifdef __sparc64__ > /* >* WWIDs generated by LSI firmware are not IEEE NAA compliant > - * so historical practise in OBP is to set the top nibble to 3 > - * to indicate that this is a RAID volume. > + * and historical practise in OBP on sparc64 is to set the top > + * nibble to 3 to indicate that this is a RAID volume. >*/ > link->port_wwn &= 0x0fff; > link->port_wwn |= 0x3000; > -#endif > > return (0); > } >
Re: sparc64: find root device on hardware RAID
On Fri, Jan 03, 2020 at 08:30:42PM +0100, Mark Kettenis wrote: > Can we leave out the #ifdef __sparc64__? Unless somebody can come up > with a really good reason for it... The code should be safe on all platforms but I put it in to ensure I'm not breaking stuff I cannot test, e.g. anything else than sparc64/OBP. deraadt expressed the same concerns. With the two of you arguing for removal, I'm confident enough to remove it and make mpii(4) MI again; unless I hear arguments against it, I'll commit this soon. Index: mpii.c === RCS file: /cvs/src/sys/dev/pci/mpii.c,v retrieving revision 1.125 diff -u -p -r1.125 mpii.c --- mpii.c 3 Jan 2020 08:39:31 - 1.125 +++ mpii.c 13 Jan 2020 09:59:01 - @@ -930,15 +930,13 @@ mpii_scsi_probe(struct scsi_link *link) return (EINVAL); link->port_wwn = letoh64(vpg.wwid); -#ifdef __sparc64__ /* * WWIDs generated by LSI firmware are not IEEE NAA compliant -* so historical practise in OBP is to set the top nibble to 3 -* to indicate that this is a RAID volume. +* and historical practise in OBP on sparc64 is to set the top +* nibble to 3 to indicate that this is a RAID volume. */ link->port_wwn &= 0x0fff; link->port_wwn |= 0x3000; -#endif return (0); }
Re: TIMESPEC_TO_NSEC(): futex(2), __thrsigdivert(2) & __thrsleep(2)
On 12/01/20(Sun) 18:37, Philip Guenther wrote: > 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'm not interested about making this turd shine myself. I won't oppose to such change. I'd just be pleased if we could finish the hz rampage. There are many uses of `hz' in base that would need some clever brain cell to be converted to other stuff. I would make me very happy if some of these algorithms, like pool cache, profiling, RX pressure, scheduling could receive some love :o)
Re: IPL of timeout_set_proc(9)
On 12/01/20(Sun) 16:29, Scott Cheloha wrote: > On Sun, Jan 12, 2020 at 12:52:53PM +0100, Martin Pieuchot wrote: > [...] > > > > 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. I understand the current behavior differently. Without this change the soft-interrupt might run in-between 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. The soft-interrupt always run first because it schedules the thread, see the wakeup() at the end of softclock(). > > > 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. Why? You give conclusion but don't explain why they would be needed. What code is broken? Isn't the current behavior fine? I don't understand why you want to change something here.
