Re: timekeep: fixing large skews on amd64 with RDTSCP

2020-08-22 Thread Scott Cheloha
On Tue, Jul 28, 2020 at 10:02:07AM +0300, Paul Irofti wrote:
> 
> [...]
> 
> Is the issue with LFENCE slowing down the network stack settled? That was
> the argument against it last time.

... a month passes.  Nobody says anything.

This "it might slow down the network stack" thing keeps coming up, and
yet nobody can point to (a) who expressed this concern or (b) what the
penalty is in practice.

Note that the alternative is "your timecounter might not be monotonic
between threads".  For me, that's already a dealbreaker.

But for sake of discussion let's look at some data.  For those of you
watching from home, please follow along!  I would like to know what
your results look like.

To start, here is a microbenchmarking program for clock_gettime(2) on
amd64.  If you have the userspace timecounter, then

clock_gettime(CLOCK_MONOTONIC, ...);

is a suitable surrogate for nanouptime(9), so this microbenchmark can
actually tell us about how nanouptime(9) or nanotime(9) would be
impacted by a comparable change in the kernel timecounter.

--

/*
 * clock_gettime-bench.c
 */
#include 
#include 
#include 
#include 
#include 

static uint64_t
rdtsc_lfence(void)
{
uint32_t hi, lo;

__asm volatile("lfence; rdtsc; lfence" : "=d" (hi), "=a" (lo));
return ((uint64_t)hi << 32) | lo;
}

int
main(int argc, char *argv[])
{
struct timespec now;
uint64_t begin, end;
long long count, i;
const char *errstr;

if (argc != 2) {
fprintf(stderr, "usage: %s count\n", getprogname());
return 1;
}
count = strtonum(argv[1], 1, LLONG_MAX, );
if (errstr != NULL)
errx(1, "count is %s: %s", errstr, argv[1]);

begin = rdtsc_lfence();
for (i = 0; i < count; i++)
clock_gettime(CLOCK_MONOTONIC, );
end = rdtsc_lfence();

printf("%lld\t%llu\n", count, end - begin);

return 0;
}

--

Now consider a benchmark of 100K clock_gettime(2) calls against the
userspace timecounter.

$ clock_gettime-bench 10
10  15703664

Let's collect 10K of these benchmarks -- our samples -- atop an
unpatched libc.  Use the shell script below.  Note that we throw out
samples where we hit a context switch.

--

#! /bin/sh

[ $# -ne 1 ] && exit 1
RESULTS=$1
shift

TIME=$(mktemp) || exit 1
TMP=$(mktemp) || exit 1

# Collect 10K samples.
i=0
while [ $i -lt 1 ]; do
# Call clock_gettime(2) 100K times.
/usr/bin/time -l ~/scratch/clock_gettime-bench 10 > $TMP 2> $TIME
# Ignore this sample if a context switch occurred.
if egrep -q '[1-9][0-9]* +(in)?voluntary context' $TIME; then
continue
fi
cat $TMP >> $RESULTS
i=$((i + 1))
done

rm $TMP $TIME

--

Run it like this:

$ ksh bench.sh unpatched.out

That will take ~5-10 minutes at most.

Next, we'll patch libc to add the LFENCE to the userspace timecounter.

Index: usertc.c
===
RCS file: /cvs/src/lib/libc/arch/amd64/gen/usertc.c,v
retrieving revision 1.2
diff -u -p -r1.2 usertc.c
--- usertc.c8 Jul 2020 09:17:48 -   1.2
+++ usertc.c22 Aug 2020 22:18:47 -
@@ -19,10 +19,10 @@
 #include 
 
 static inline u_int
-rdtsc(void)
+rdtsc_lfence(void)
 {
uint32_t hi, lo;
-   asm volatile("rdtsc" : "=a"(lo), "=d"(hi));
+   asm volatile("lfence; rdtsc" : "=a"(lo), "=d"(hi));
return ((uint64_t)lo)|(((uint64_t)hi)<<32);
 }
 
@@ -31,7 +31,7 @@ tc_get_timecount(struct timekeep *tk, u_
 {
switch (tk->tk_user) {
case TC_TSC:
-   *tc = rdtsc();
+   *tc = rdtsc_lfence();
return 0;
}
 
--

Recompile and reinstall libc.

Then rerun the benchmark.  Be careful not to overwrite our results
from the unpatched libc:

$ ksh bench.sh patched.out

--

Alright, now let's compare the results.  I'm not a mathemagician so I
use ministat and trust it implicitly.  A stat jock could probably do
this in R or with some python, but I am not that clever, so I will
stick with ministat.

There is no ministat port for OpenBSD, but it is pretty trivial to
clone this github repo and build it on -current:

https://github.com/thorduri/ministat

--

Okay, you have ministat?

Let's compare the results.  We want the 2nd column in the output
(-C2).  I'm not interested in the graph (-q), given our population
size.  We have N=1, so let's push the CI up (-c 99.5).

$ ~/repo/ministat/ministat -C2 -q -c99.5 unpatched.out patched.out
x unpatched.out
+ patched.out
N   Min   MaxMedian   AvgStddev
x 1  13752102  18019218  14442398  14431918 237842.31
+ 1  15196970  16992030  15721390  15779178 181623.5
Difference at 99.5% confidence
1.34726e+06 +/- 9247.11
9.33528% +/- 0.064074%
(Student's t, pooled s = 211608)

So, in 

Re: top: filter by routing table

2020-08-22 Thread Todd C . Miller
This looks good to me but I've refrained from commenting simply
because I don't use rtables at all myself.  Can we get some feedback
from people who actually use rtables?

 - todd



Improve semantics and punctuation in ifconfig.8

2020-08-22 Thread Evan Silberman
Not to provoke a deep philosophical debate about the difference between
Ql, Cm, and Sy, but surely Em isn't the best choice here. A couple other
nits that bothered me in the same general region, I guess your taste
might vary.

Evan Silberman

---
commit 233b62ce4e6cd3388e68f407e50758111b3eeffc (ifconfig.8)
from: Evan Silberman 
date: Sat Aug 22 21:14:25 2020 UTC
 
 Improve markup and punctuation concerning groups
 
 - They're groups, not scare-quotes-"groups"
 - The markup for the default groups is debatable, but Cm seems closest.
   You might also like: Ql, Sy
 - Parenthesized plural(s) is (are) distracting
 
diff cb34809c69289319bd78d14b4f5ed7d4e93b080f 
c3c1549c9f1e94ec03820de542eaf10eaf0dc3f0
blob - 9e6ad912b385de338033598de04f0184d015224e
blob + 968886c1a6d4849c4362ab2d759cc72c71d125ed
--- sbin/ifconfig/ifconfig.8
+++ sbin/ifconfig/ifconfig.8
@@ -238,8 +238,8 @@ transmit messages through that interface.
 If possible, the interface will be reset to disable reception as well.
 This action automatically disables routes using the interface.
 .It Cm group Ar group-name
-Assign the interface to a
-.Dq group .
+Assign the interface to a group.
+The
 .Ar group-name
 may not be longer than 15 characters and must not end with a digit.
 Any interface can be in multiple groups.
@@ -254,36 +254,35 @@ Some interfaces belong to specific groups by default:
 .Bl -dash -width Ds -compact
 .It
 All interfaces are members of the
-.Em all
+.Cm all
 interface group.
 .It
 Cloned interfaces are members of their interface family group.
 For example, a PPP interface such as
-.Em ppp0
+.Ql ppp0
 is a member of the
-.Em ppp
+.Cm ppp
 interface family group.
 .It
 .Xr pppx 4
 interfaces are members of the
-.Em pppx
+.Cm pppx
 interface group.
 .It
-The interface(s) the default route(s) point to are members of the
-.Em egress
+The interfaces the default routes point to are members of the
+.Cm egress
 interface group.
 .It
 IEEE 802.11 wireless interfaces are members of the
-.Em wlan
+.Cm wlan
 interface group.
 .It
 Any interfaces used for network booting are members of the
-.Em netboot
+.Cm netboot
 interface group.
 .El
 .It Cm -group Ar group-name
-Remove the interface from the given
-.Dq group .
+Remove the interface from the given group.
 .It Cm hwfeatures
 Display the interface hardware features:
 .Pp



Re: top: filter by routing table

2020-08-22 Thread Klemens Nanni
On Sun, Aug 09, 2020 at 09:02:14PM +0200, Klemens Nanni wrote:
> Sometimes I want to see processes outside the default routing table with
> `-T -0', sometimes those in in a specific one with `-T 3' (for testing).
> 
> Since others have poked around with routing tables and/or domains as of
> late, perhaps this deemed useful enough?
> 
> Semantically, filtering is identical to that of users: pick or hide one;
> this makes the code pretty much copy/paste within top(1), manual wording
> and command line flag is taken from pgrep(1).
> 
> pgrep is currently the only way to identify processes by routing table.
> After netstat(1)'s relatively recent addition of `-R', filtering in top
> makes for a quite handy set of tooling around rtable(4).
> 
> Feedback? OK?
Anyone?

One thing is that routing tables are now the only values you can filter
for which are never visible in top - I thought about adding another
RTABLE column, replacing one as `H' does with USERNAME -> TID or so,
but did not deem it necessary/worthwile in the end.

What do you think?

Same diff below.


Index: display.c
===
RCS file: /cvs/src/usr.bin/top/display.c,v
retrieving revision 1.63
diff -u -p -r1.63 display.c
--- display.c   26 Jul 2020 21:59:16 -  1.63
+++ display.c   9 Aug 2020 17:55:56 -
@@ -824,6 +824,8 @@ show_help(void)
"r count pid  - renice process `pid' to nice value `count'\n"
"S- toggle the display of system processes\n"
"s time   - change delay between displays to `time' seconds\n"
+   "T [-]rtable  - show processes associated with routing table 
`rtable'\n"
+   "   (T+ shows all, T -rtable hides rtable)\n"
"u [-]user- show processes for `user' (u+ shows all, u -user 
hides user)\n"
"\n");
 
Index: machine.c
===
RCS file: /cvs/src/usr.bin/top/machine.c,v
retrieving revision 1.107
diff -u -p -r1.107 machine.c
--- machine.c   6 Jul 2020 16:27:59 -   1.107
+++ machine.c   9 Aug 2020 18:06:36 -
@@ -414,7 +414,7 @@ get_process_info(struct system_info *si,
 int (*compare) (const void *, const void *))
 {
int show_idle, show_system, show_threads, show_uid, show_pid, show_cmd;
-   int hide_uid;
+   int show_rtable, hide_rtable, hide_uid;
int total_procs, active_procs;
struct kinfo_proc **prefp, *pp;
int what = KERN_PROC_ALL;
@@ -446,6 +446,8 @@ get_process_info(struct system_info *si,
show_uid = sel->uid != (uid_t)-1;
hide_uid = sel->huid != (uid_t)-1;
show_pid = sel->pid != (pid_t)-1;
+   show_rtable = sel->rtableid != -1;
+   hide_rtable = sel->hrtableid != -1;
show_cmd = sel->command != NULL;
 
/* count up process states and get pointers to interesting procs */
@@ -474,6 +476,8 @@ get_process_info(struct system_info *si,
(!hide_uid || pp->p_ruid != sel->huid) &&
(!show_uid || pp->p_ruid == sel->uid) &&
(!show_pid || pp->p_pid == sel->pid) &&
+   (!hide_rtable || pp->p_rtableid != sel->hrtableid) 
&&
+   (!show_rtable || pp->p_rtableid == sel->rtableid) &&
(!show_cmd || cmd_matches(pp, sel->command))) {
*prefp++ = pp;
active_procs++;
Index: machine.h
===
RCS file: /cvs/src/usr.bin/top/machine.h,v
retrieving revision 1.29
diff -u -p -r1.29 machine.h
--- machine.h   25 Jun 2020 20:38:41 -  1.29
+++ machine.h   9 Aug 2020 17:50:14 -
@@ -77,6 +77,8 @@ struct process_select {
uid_t   uid;/* only this uid (unless uid == -1) */
uid_t   huid;   /* hide this uid (unless huid == -1) */
pid_t   pid;/* only this pid (unless pid == -1) */
+   int rtableid;   /* only this rtable (unless rtableid == 
-1) */
+   int hrtableid;  /* hide this rtable (unless hrtableid 
== -1) */
char   *command;/* only this command (unless == NULL) */
 };
 
Index: top.1
===
RCS file: /cvs/src/usr.bin/top/top.1,v
retrieving revision 1.75
diff -u -p -r1.75 top.1
--- top.1   26 Jul 2020 21:59:16 -  1.75
+++ top.1   9 Aug 2020 18:16:12 -
@@ -38,6 +38,7 @@
 .Op Fl o Oo - Oc Ns Ar field
 .Op Fl p Ar pid
 .Op Fl s Ar time
+.Op Fl T Oo - Oc Ns Ar rtable
 .Op Fl U Oo - Oc Ns Ar user
 .Op Ar number
 .Ek
@@ -179,6 +180,14 @@ Set the delay between screen updates to
 seconds.
 The value may be fractional, to permit delays of less than 1 second.
 The default delay between updates is 5 seconds.
+.It Fl T Oo - Oc Ns Ar rtable
+Display only processes 

Re: sppp: add size to free() calls

2020-08-22 Thread Vitaliy Makkoveev
Yeah, we override both of 'auth->name' and 'auth->secret’.

Since there is the only difference against your previous diff and
the only place where you touch them I have no objections.

> On 22 Aug 2020, at 18:00, Klemens Nanni  wrote:
> 
> On Sat, Aug 22, 2020 at 02:32:17PM +0200, Klemens Nanni wrote:
>> Another round, this time obvious sizes which are in immediate scope of
>> the free() call, e.g. right below the malloc() call.
>> 
>> This leaves only a few selected free() calls with size zero in
>> if_spppsubr.c due to the fact that there is currently no variable to
>> keep track of username and password string lengths.
>> 
>> Feedback? OK?
> Sorry, here's the correct version of the diff omitting sizes for the
> very string buffers mentioned above.
> 
> 
> Index: if_spppsubr.c
> ===
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.185
> diff -u -p -r1.185 if_spppsubr.c
> --- if_spppsubr.c 14 Aug 2020 12:17:34 -  1.185
> +++ if_spppsubr.c 22 Aug 2020 14:55:49 -
> @@ -1737,7 +1737,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
> 
>   len -= 4;
>   origlen = len;
> - buf = r = malloc (len, M_TEMP, M_NOWAIT);
> + buf = r = malloc (origlen, M_TEMP, M_NOWAIT);
>   if (! buf)
>   return (0);
> 
> @@ -1749,7 +1749,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
>   p = (void*) (h+1);
>   for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
>   if (p[1] < 2 || p[1] > len) {
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, origlen);
>   return (-1);
>   }
>   if (debug)
> @@ -1926,7 +1926,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
>   }
> 
>  end:
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, origlen);
>   return (rlen == 0);
> }
> 
> @@ -2312,7 +2312,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
> {
>   u_char *buf, *r, *p;
>   struct ifnet *ifp = >pp_if;
> - int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
> + int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
>   u_int32_t hisaddr, desiredaddr;
> 
>   len -= 4;
> @@ -2321,7 +2321,8 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>* Make sure to allocate a buf that can at least hold a
>* conf-nak with an `address' option.  We might need it below.
>*/
> - buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
> + buflen = len < 6? 6: len;
> + buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
>   if (! buf)
>   return (0);
> 
> @@ -2332,7 +2333,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>   p = (void*) (h+1);
>   for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
>   if (p[1] < 2 || p[1] > len) {
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, buflen);
>   return (-1);
>   }
>   if (debug)
> @@ -2476,7 +2477,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>   }
> 
>  end:
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, buflen);
>   return (rlen == 0);
> }
> 
> @@ -2773,7 +2774,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
> {
>   u_char *buf, *r, *p;
>   struct ifnet *ifp = >pp_if;
> - int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
> + int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
>   struct in6_addr myaddr, desiredaddr, suggestaddr;
>   int ifidcount;
>   int type;
> @@ -2786,7 +2787,8 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>* Make sure to allocate a buf that can at least hold a
>* conf-nak with an `address' option.  We might need it below.
>*/
> - buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
> + buflen = len < 6? 6: len;
> + buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
>   if (! buf)
>   return (0);
> 
> @@ -2799,7 +2801,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>   for (rlen=0; len>1 && p[1]; len-=p[1], p+=p[1]) {
>   /* Sanity check option length */
>   if (p[1] < 2 || p[1] > len) {
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, buflen);
>   return (-1);
>   }
>   if (debug)
> @@ -2933,7 +2935,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>   }
> 
> end:
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, buflen);
>   return (rlen == 0);
> }
> 
> @@ -4475,10 +4477,10 @@ sppp_get_params(struct sppp *sp, struct 
>   spr->phase = sp->pp_phase;
> 
>   if (copyout(spr, (caddr_t)ifr->ifr_data, sizeof(*spr)) != 0) {
> - free(spr, M_DEVBUF, 0);
> + free(spr, M_DEVBUF, sizeof(*spr));
>   return EFAULT;
>   }
> - free(spr, M_DEVBUF, 0);
> + free(spr, 

Re: sppp: add size to free() calls

2020-08-22 Thread Klemens Nanni
On Sat, Aug 22, 2020 at 02:32:17PM +0200, Klemens Nanni wrote:
> Another round, this time obvious sizes which are in immediate scope of
> the free() call, e.g. right below the malloc() call.
> 
> This leaves only a few selected free() calls with size zero in
> if_spppsubr.c due to the fact that there is currently no variable to
> keep track of username and password string lengths.
> 
> Feedback? OK?
Sorry, here's the correct version of the diff omitting sizes for the
very string buffers mentioned above.


Index: if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.185
diff -u -p -r1.185 if_spppsubr.c
--- if_spppsubr.c   14 Aug 2020 12:17:34 -  1.185
+++ if_spppsubr.c   22 Aug 2020 14:55:49 -
@@ -1737,7 +1737,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
 
len -= 4;
origlen = len;
-   buf = r = malloc (len, M_TEMP, M_NOWAIT);
+   buf = r = malloc (origlen, M_TEMP, M_NOWAIT);
if (! buf)
return (0);
 
@@ -1749,7 +1749,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
p = (void*) (h+1);
for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
if (p[1] < 2 || p[1] > len) {
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, origlen);
return (-1);
}
if (debug)
@@ -1926,7 +1926,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
}
 
  end:
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, origlen);
return (rlen == 0);
 }
 
@@ -2312,7 +2312,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
 {
u_char *buf, *r, *p;
struct ifnet *ifp = >pp_if;
-   int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
+   int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
u_int32_t hisaddr, desiredaddr;
 
len -= 4;
@@ -2321,7 +2321,8 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
 * Make sure to allocate a buf that can at least hold a
 * conf-nak with an `address' option.  We might need it below.
 */
-   buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
+   buflen = len < 6? 6: len;
+   buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
if (! buf)
return (0);
 
@@ -2332,7 +2333,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
p = (void*) (h+1);
for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
if (p[1] < 2 || p[1] > len) {
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, buflen);
return (-1);
}
if (debug)
@@ -2476,7 +2477,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
}
 
  end:
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, buflen);
return (rlen == 0);
 }
 
@@ -2773,7 +2774,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
 {
u_char *buf, *r, *p;
struct ifnet *ifp = >pp_if;
-   int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
+   int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
struct in6_addr myaddr, desiredaddr, suggestaddr;
int ifidcount;
int type;
@@ -2786,7 +2787,8 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
 * Make sure to allocate a buf that can at least hold a
 * conf-nak with an `address' option.  We might need it below.
 */
-   buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
+   buflen = len < 6? 6: len;
+   buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
if (! buf)
return (0);
 
@@ -2799,7 +2801,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
for (rlen=0; len>1 && p[1]; len-=p[1], p+=p[1]) {
/* Sanity check option length */
if (p[1] < 2 || p[1] > len) {
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, buflen);
return (-1);
}
if (debug)
@@ -2933,7 +2935,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
}
 
 end:
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, buflen);
return (rlen == 0);
 }
 
@@ -4475,10 +4477,10 @@ sppp_get_params(struct sppp *sp, struct 
spr->phase = sp->pp_phase;
 
if (copyout(spr, (caddr_t)ifr->ifr_data, sizeof(*spr)) != 0) {
-   free(spr, M_DEVBUF, 0);
+   free(spr, M_DEVBUF, sizeof(*spr));
return EFAULT;
}
-   free(spr, M_DEVBUF, 0);
+   free(spr, M_DEVBUF, sizeof(*spr));
break;
}
case SPPPIOGMAUTH:
@@ -4498,10 +4500,10 @@ sppp_get_params(struct sppp *sp, struct 
strlcpy(spa->name, auth->name, sizeof(spa->name));
 
if (copyout(spa, (caddr_t)ifr->ifr_data, sizeof(*spa)) != 0) {
-

Re: sppp: add size to free() calls

2020-08-22 Thread Vitaliy Makkoveev
ok mvs@

> On 22 Aug 2020, at 15:32, Klemens Nanni  wrote:
> 
> Another round, this time obvious sizes which are in immediate scope of
> the free() call, e.g. right below the malloc() call.
> 
> This leaves only a few selected free() calls with size zero in
> if_spppsubr.c due to the fact that there is currently no variable to
> keep track of username and password string lengths.
> 
> Feedback? OK?
> 
> 
> Index: if_spppsubr.c
> ===
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.185
> diff -u -p -r1.185 if_spppsubr.c
> --- if_spppsubr.c 14 Aug 2020 12:17:34 -  1.185
> +++ if_spppsubr.c 22 Aug 2020 12:25:37 -
> @@ -1737,7 +1737,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
> 
>   len -= 4;
>   origlen = len;
> - buf = r = malloc (len, M_TEMP, M_NOWAIT);
> + buf = r = malloc (origlen, M_TEMP, M_NOWAIT);
>   if (! buf)
>   return (0);
> 
> @@ -1749,7 +1749,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
>   p = (void*) (h+1);
>   for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
>   if (p[1] < 2 || p[1] > len) {
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, origlen);
>   return (-1);
>   }
>   if (debug)
> @@ -1926,7 +1926,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
>   }
> 
>  end:
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, origlen);
>   return (rlen == 0);
> }
> 
> @@ -2312,7 +2312,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
> {
>   u_char *buf, *r, *p;
>   struct ifnet *ifp = >pp_if;
> - int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
> + int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
>   u_int32_t hisaddr, desiredaddr;
> 
>   len -= 4;
> @@ -2321,7 +2321,8 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>* Make sure to allocate a buf that can at least hold a
>* conf-nak with an `address' option.  We might need it below.
>*/
> - buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
> + buflen = len < 6? 6: len;
> + buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
>   if (! buf)
>   return (0);
> 
> @@ -2332,7 +2333,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>   p = (void*) (h+1);
>   for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
>   if (p[1] < 2 || p[1] > len) {
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, buflen);
>   return (-1);
>   }
>   if (debug)
> @@ -2476,7 +2477,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>   }
> 
>  end:
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, buflen);
>   return (rlen == 0);
> }
> 
> @@ -2773,7 +2774,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
> {
>   u_char *buf, *r, *p;
>   struct ifnet *ifp = >pp_if;
> - int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
> + int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
>   struct in6_addr myaddr, desiredaddr, suggestaddr;
>   int ifidcount;
>   int type;
> @@ -2786,7 +2787,8 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>* Make sure to allocate a buf that can at least hold a
>* conf-nak with an `address' option.  We might need it below.
>*/
> - buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
> + buflen = len < 6? 6: len;
> + buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
>   if (! buf)
>   return (0);
> 
> @@ -2799,7 +2801,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>   for (rlen=0; len>1 && p[1]; len-=p[1], p+=p[1]) {
>   /* Sanity check option length */
>   if (p[1] < 2 || p[1] > len) {
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, buflen);
>   return (-1);
>   }
>   if (debug)
> @@ -2933,7 +2935,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>   }
> 
> end:
> - free(buf, M_TEMP, 0);
> + free(buf, M_TEMP, buflen);
>   return (rlen == 0);
> }
> 
> @@ -4475,10 +4477,10 @@ sppp_get_params(struct sppp *sp, struct 
>   spr->phase = sp->pp_phase;
> 
>   if (copyout(spr, (caddr_t)ifr->ifr_data, sizeof(*spr)) != 0) {
> - free(spr, M_DEVBUF, 0);
> + free(spr, M_DEVBUF, sizeof(*spr));
>   return EFAULT;
>   }
> - free(spr, M_DEVBUF, 0);
> + free(spr, M_DEVBUF, sizeof(*spr));
>   break;
>   }
>   case SPPPIOGMAUTH:
> @@ -4498,10 +4500,10 @@ sppp_get_params(struct sppp *sp, struct 
>   strlcpy(spa->name, auth->name, sizeof(spa->name));
> 
>   if (copyout(spa, (caddr_t)ifr->ifr_data, sizeof(*spa)) != 0) {
> - free(spa, M_DEVBUF, 0);
> +

ntpd: go into unsynced mode

2020-08-22 Thread Otto Moerbeek
Hi,

At the moment ntpd never goes into unsynced mode if network
connectivity is lost. The code to do that is only triggered when a
pakcet is received, which does not happen. 

This diff fixes that by going into unsynced mode if no time data was
processed for a while. 

An earlier version of this diff was tested by naddy@. Compared to that
version, the needed period of inactivity is now three times as large
and I set scale to 1, so recovery goes faster.

Please test and review,

-Otto

Index: ntp.c
===
RCS file: /cvs/src/usr.sbin/ntpd/ntp.c,v
retrieving revision 1.165
diff -u -p -r1.165 ntp.c
--- ntp.c   22 Jun 2020 06:11:34 -  1.165
+++ ntp.c   22 Aug 2020 13:48:34 -
@@ -89,6 +89,7 @@ ntp_main(struct ntpd_conf *nconf, struct
struct stat  stb;
struct ctl_conn *cc;
time_t   nextaction, last_sensor_scan = 0, now;
+   time_t   last_action = 0, interval;
void*newp;
 
if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNSPEC,
@@ -402,6 +403,7 @@ ntp_main(struct ntpd_conf *nconf, struct
for (; nfds > 0 && j < idx_clients; j++) {
if (pfd[j].revents & (POLLIN|POLLERR)) {
nfds--;
+   last_action = now;
if (client_dispatch(idx2peer[j - idx_peers],
conf->settime, conf->automatic) == -1) {
log_warn("pipe write error (settime)");
@@ -417,8 +419,24 @@ ntp_main(struct ntpd_conf *nconf, struct
for (s = TAILQ_FIRST(>ntp_sensors); s != NULL;
s = next_s) {
next_s = TAILQ_NEXT(s, entry);
-   if (s->next <= getmonotime())
+   if (s->next <= now) {
+   last_action = now;
sensor_query(s);
+   }
+   }
+
+   /*
+* Compute maximum of scale_interval(INTERVAL_QUERY_NORMAL),
+* if we did not process a time message for three times that
+* interval, stop advertising we're synced.
+*/
+   interval = INTERVAL_QUERY_NORMAL * conf->scale;
+   interval += MAXIMUM(5, interval / 10) - 1;
+   if (conf->status.synced && last_action + 3 * interval < now) {
+   log_info("clock is now unsynced");
+   conf->status.synced = 0;
+   conf->scale = 1;
+   priv_dns(IMSG_UNSYNCED, NULL, 0);
}
}
 



Re: sdmmc(4): add UHS-I support

2020-08-22 Thread Marcus Glocker
On Sat, 22 Aug 2020 14:09:53 +0200 (CEST)
Mark Kettenis  wrote:

> > Date: Mon, 17 Aug 2020 12:57:58 +0200 (CEST)
> > From: Mark Kettenis 
> >   
> > > Date: Sun, 16 Aug 2020 19:32:03 +0200 (CEST)
> > > From: Mark Kettenis 
> > > 
> > > The diff below adds support for higher speeds as supported by
> > > UHS-I SD cards to the generic sdmmc(4) layer.  The diff in itself
> > > does not enable the use of those modes.  That needs separate
> > > changes to the SD/MMC controller drivers.  I have such a diff for
> > > amlmmc(4) that allows me to use SDR50 mode.
> > > 
> > > However, to make sure this diff doesn't break existing lower speed
> > > modes I'd appreciate tests on a variety of hardware.  So if
> > > sdmmc(4) shows up in your dmesg, please test this by exercising
> > > your (u)SD or (e)MMC cards.
> > > 
> > > Thanks,
> > > 
> > > Mark  
> > 
> > Previous diff didn't build properly on amd64.  Here is a new diff.  
> 
> I did not receive a lot of test reports, but the diff has been in
> snaps for about a week.
> 
> ok?

No issues found using it on my boot device:

sdmmc0 at sximmc0: 4-bit, sd high-speed, mmc high-speed, dma
scsibus0 at sdmmc0: 2 targets, initiator 0

ok mglocker@

> 
> > Index: dev/sdmmc/sdmmc.c
> > ===
> > RCS file: /cvs/src/sys/dev/sdmmc/sdmmc.c,v
> > retrieving revision 1.57
> > diff -u -p -r1.57 sdmmc.c
> > --- dev/sdmmc/sdmmc.c   15 Aug 2020 13:21:02 -  1.57
> > +++ dev/sdmmc/sdmmc.c   17 Aug 2020 10:38:11 -
> > @@ -111,6 +111,10 @@ sdmmc_attach(struct device *parent, stru
> > printf(": 1-bit");
> > if (ISSET(saa->caps, SMC_CAPS_SD_HIGHSPEED))
> > printf(", sd high-speed");
> > +   if (ISSET(saa->caps, SMC_CAPS_UHS_SDR50))
> > +   printf(", sdr50");
> > +   if (ISSET(saa->caps, SMC_CAPS_UHS_SDR104))
> > +   printf(", sdr104");
> > if (ISSET(saa->caps, SMC_CAPS_MMC_HIGHSPEED))
> > printf(", mmc high-speed");
> > if (ISSET(saa->caps, SMC_CAPS_MMC_DDR52))
> > Index: dev/sdmmc/sdmmc_mem.c
> > ===
> > RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_mem.c,v
> > retrieving revision 1.34
> > diff -u -p -r1.34 sdmmc_mem.c
> > --- dev/sdmmc/sdmmc_mem.c   14 Aug 2020 14:49:04 -
> > 1.34 +++ dev/sdmmc/sdmmc_mem.c  17 Aug 2020 10:38:11 -
> > @@ -52,6 +52,7 @@ int   sdmmc_mem_decode_scr(struct sdmmc_so
> >  intsdmmc_mem_send_cxd_data(struct sdmmc_softc *, int, void
> > *, size_t); int sdmmc_mem_set_bus_width(struct
> > sdmmc_function *, int); int sdmmc_mem_mmc_switch(struct
> > sdmmc_function *, uint8_t, uint8_t, uint8_t); +int
> > sdmmc_mem_signal_voltage(struct sdmmc_softc *, int); 
> >  intsdmmc_mem_sd_init(struct sdmmc_softc *, struct
> > sdmmc_function *); int  sdmmc_mem_mmc_init(struct sdmmc_softc
> > *, struct sdmmc_function *); @@ -104,12 +105,16 @@ const int
> > sdmmc_mmc_timings[] = { int
> >  sdmmc_mem_enable(struct sdmmc_softc *sc)
> >  {
> > -   u_int32_t host_ocr;
> > -   u_int32_t card_ocr;
> > +   uint32_t host_ocr;
> > +   uint32_t card_ocr;
> > +   uint32_t new_ocr;
> > +   uint32_t ocr = 0;
> > +   int error;
> >  
> > rw_assert_wrlock(>sc_lock);
> >  
> > /* Set host mode to SD "combo" card or SD memory-only. */
> > +   CLR(sc->sc_flags, SMF_UHS_MODE);
> > SET(sc->sc_flags, SMF_SD_MODE|SMF_MEM_MODE);
> >  
> > /* Reset memory (*must* do that before CMD55 or CMD1). */
> > @@ -153,14 +158,86 @@ sdmmc_mem_enable(struct sdmmc_softc *sc)
> >  
> > host_ocr &= card_ocr; /* only allow the common voltages */
> >  
> > -   if (sdmmc_send_if_cond(sc, card_ocr) == 0)
> > -   host_ocr |= SD_OCR_SDHC_CAP;
> > +   if (ISSET(sc->sc_flags, SMF_SD_MODE)) {
> > +   if (sdmmc_send_if_cond(sc, card_ocr) == 0)
> > +   SET(ocr, MMC_OCR_HCS);
> > +
> > +   if (sdmmc_chip_host_ocr(sc->sct, sc->sch) &
> > MMC_OCR_S18A)
> > +   SET(ocr, MMC_OCR_S18A);
> > +   }
> > +   host_ocr |= ocr;
> >  
> > /* Send the new OCR value until all cards are ready. */
> > -   if (sdmmc_mem_send_op_cond(sc, host_ocr, NULL) != 0) {
> > +   if (sdmmc_mem_send_op_cond(sc, host_ocr, _ocr) != 0) {
> > DPRINTF(("%s: can't send memory OCR\n",
> > DEVNAME(sc))); return 1;
> > }
> > +
> > +   if (ISSET(sc->sc_flags, SMF_SD_MODE) && ISSET(new_ocr,
> > MMC_OCR_S18A)) {
> > +   /*
> > +* Card and host support low voltage mode, begin
> > switch
> > +* sequence.
> > +*/
> > +   struct sdmmc_command cmd;
> > +
> > +   memset(, 0, sizeof(cmd));
> > +   cmd.c_arg = 0;
> > +   cmd.c_flags = SCF_CMD_AC | SCF_RSP_R1;
> > +   cmd.c_opcode = SD_VOLTAGE_SWITCH;
> > +   DPRINTF(("%s: switching card to 1.8V\n",
> > DEVNAME(sc)));
> > +   error = sdmmc_mmc_command(sc, );
> > +   if (error) {
> > + 

sppp: add size to free() calls

2020-08-22 Thread Klemens Nanni
Another round, this time obvious sizes which are in immediate scope of
the free() call, e.g. right below the malloc() call.

This leaves only a few selected free() calls with size zero in
if_spppsubr.c due to the fact that there is currently no variable to
keep track of username and password string lengths.

Feedback? OK?


Index: if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.185
diff -u -p -r1.185 if_spppsubr.c
--- if_spppsubr.c   14 Aug 2020 12:17:34 -  1.185
+++ if_spppsubr.c   22 Aug 2020 12:25:37 -
@@ -1737,7 +1737,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
 
len -= 4;
origlen = len;
-   buf = r = malloc (len, M_TEMP, M_NOWAIT);
+   buf = r = malloc (origlen, M_TEMP, M_NOWAIT);
if (! buf)
return (0);
 
@@ -1749,7 +1749,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
p = (void*) (h+1);
for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
if (p[1] < 2 || p[1] > len) {
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, origlen);
return (-1);
}
if (debug)
@@ -1926,7 +1926,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
}
 
  end:
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, origlen);
return (rlen == 0);
 }
 
@@ -2312,7 +2312,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
 {
u_char *buf, *r, *p;
struct ifnet *ifp = >pp_if;
-   int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
+   int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
u_int32_t hisaddr, desiredaddr;
 
len -= 4;
@@ -2321,7 +2321,8 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
 * Make sure to allocate a buf that can at least hold a
 * conf-nak with an `address' option.  We might need it below.
 */
-   buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
+   buflen = len < 6? 6: len;
+   buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
if (! buf)
return (0);
 
@@ -2332,7 +2333,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
p = (void*) (h+1);
for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
if (p[1] < 2 || p[1] > len) {
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, buflen);
return (-1);
}
if (debug)
@@ -2476,7 +2477,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
}
 
  end:
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, buflen);
return (rlen == 0);
 }
 
@@ -2773,7 +2774,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
 {
u_char *buf, *r, *p;
struct ifnet *ifp = >pp_if;
-   int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
+   int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
struct in6_addr myaddr, desiredaddr, suggestaddr;
int ifidcount;
int type;
@@ -2786,7 +2787,8 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
 * Make sure to allocate a buf that can at least hold a
 * conf-nak with an `address' option.  We might need it below.
 */
-   buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
+   buflen = len < 6? 6: len;
+   buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
if (! buf)
return (0);
 
@@ -2799,7 +2801,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
for (rlen=0; len>1 && p[1]; len-=p[1], p+=p[1]) {
/* Sanity check option length */
if (p[1] < 2 || p[1] > len) {
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, buflen);
return (-1);
}
if (debug)
@@ -2933,7 +2935,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
}
 
 end:
-   free(buf, M_TEMP, 0);
+   free(buf, M_TEMP, buflen);
return (rlen == 0);
 }
 
@@ -4475,10 +4477,10 @@ sppp_get_params(struct sppp *sp, struct 
spr->phase = sp->pp_phase;
 
if (copyout(spr, (caddr_t)ifr->ifr_data, sizeof(*spr)) != 0) {
-   free(spr, M_DEVBUF, 0);
+   free(spr, M_DEVBUF, sizeof(*spr));
return EFAULT;
}
-   free(spr, M_DEVBUF, 0);
+   free(spr, M_DEVBUF, sizeof(*spr));
break;
}
case SPPPIOGMAUTH:
@@ -4498,10 +4500,10 @@ sppp_get_params(struct sppp *sp, struct 
strlcpy(spa->name, auth->name, sizeof(spa->name));
 
if (copyout(spa, (caddr_t)ifr->ifr_data, sizeof(*spa)) != 0) {
-   free(spa, M_DEVBUF, 0);
+   free(spa, M_DEVBUF, sizeof(*spa));
return EFAULT;
}
-   free(spa, 

Re: sdmmc(4): add UHS-I support

2020-08-22 Thread Mark Kettenis
> Date: Mon, 17 Aug 2020 12:57:58 +0200 (CEST)
> From: Mark Kettenis 
> 
> > Date: Sun, 16 Aug 2020 19:32:03 +0200 (CEST)
> > From: Mark Kettenis 
> > 
> > The diff below adds support for higher speeds as supported by UHS-I SD
> > cards to the generic sdmmc(4) layer.  The diff in itself does not
> > enable the use of those modes.  That needs separate changes to the
> > SD/MMC controller drivers.  I have such a diff for amlmmc(4) that
> > allows me to use SDR50 mode.
> > 
> > However, to make sure this diff doesn't break existing lower speed
> > modes I'd appreciate tests on a variety of hardware.  So if sdmmc(4)
> > shows up in your dmesg, please test this by exercising your (u)SD or
> > (e)MMC cards.
> > 
> > Thanks,
> > 
> > Mark
> 
> Previous diff didn't build properly on amd64.  Here is a new diff.

I did not receive a lot of test reports, but the diff has been in
snaps for about a week.

ok?


> Index: dev/sdmmc/sdmmc.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmc.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 sdmmc.c
> --- dev/sdmmc/sdmmc.c 15 Aug 2020 13:21:02 -  1.57
> +++ dev/sdmmc/sdmmc.c 17 Aug 2020 10:38:11 -
> @@ -111,6 +111,10 @@ sdmmc_attach(struct device *parent, stru
>   printf(": 1-bit");
>   if (ISSET(saa->caps, SMC_CAPS_SD_HIGHSPEED))
>   printf(", sd high-speed");
> + if (ISSET(saa->caps, SMC_CAPS_UHS_SDR50))
> + printf(", sdr50");
> + if (ISSET(saa->caps, SMC_CAPS_UHS_SDR104))
> + printf(", sdr104");
>   if (ISSET(saa->caps, SMC_CAPS_MMC_HIGHSPEED))
>   printf(", mmc high-speed");
>   if (ISSET(saa->caps, SMC_CAPS_MMC_DDR52))
> Index: dev/sdmmc/sdmmc_mem.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_mem.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 sdmmc_mem.c
> --- dev/sdmmc/sdmmc_mem.c 14 Aug 2020 14:49:04 -  1.34
> +++ dev/sdmmc/sdmmc_mem.c 17 Aug 2020 10:38:11 -
> @@ -52,6 +52,7 @@ int sdmmc_mem_decode_scr(struct sdmmc_so
>  int  sdmmc_mem_send_cxd_data(struct sdmmc_softc *, int, void *, size_t);
>  int  sdmmc_mem_set_bus_width(struct sdmmc_function *, int);
>  int  sdmmc_mem_mmc_switch(struct sdmmc_function *, uint8_t, uint8_t, 
> uint8_t);
> +int  sdmmc_mem_signal_voltage(struct sdmmc_softc *, int);
>  
>  int  sdmmc_mem_sd_init(struct sdmmc_softc *, struct sdmmc_function *);
>  int  sdmmc_mem_mmc_init(struct sdmmc_softc *, struct sdmmc_function *);
> @@ -104,12 +105,16 @@ const int sdmmc_mmc_timings[] = {
>  int
>  sdmmc_mem_enable(struct sdmmc_softc *sc)
>  {
> - u_int32_t host_ocr;
> - u_int32_t card_ocr;
> + uint32_t host_ocr;
> + uint32_t card_ocr;
> + uint32_t new_ocr;
> + uint32_t ocr = 0;
> + int error;
>  
>   rw_assert_wrlock(>sc_lock);
>  
>   /* Set host mode to SD "combo" card or SD memory-only. */
> + CLR(sc->sc_flags, SMF_UHS_MODE);
>   SET(sc->sc_flags, SMF_SD_MODE|SMF_MEM_MODE);
>  
>   /* Reset memory (*must* do that before CMD55 or CMD1). */
> @@ -153,14 +158,86 @@ sdmmc_mem_enable(struct sdmmc_softc *sc)
>  
>   host_ocr &= card_ocr; /* only allow the common voltages */
>  
> - if (sdmmc_send_if_cond(sc, card_ocr) == 0)
> - host_ocr |= SD_OCR_SDHC_CAP;
> + if (ISSET(sc->sc_flags, SMF_SD_MODE)) {
> + if (sdmmc_send_if_cond(sc, card_ocr) == 0)
> + SET(ocr, MMC_OCR_HCS);
> +
> + if (sdmmc_chip_host_ocr(sc->sct, sc->sch) & MMC_OCR_S18A)
> + SET(ocr, MMC_OCR_S18A);
> + }
> + host_ocr |= ocr;
>  
>   /* Send the new OCR value until all cards are ready. */
> - if (sdmmc_mem_send_op_cond(sc, host_ocr, NULL) != 0) {
> + if (sdmmc_mem_send_op_cond(sc, host_ocr, _ocr) != 0) {
>   DPRINTF(("%s: can't send memory OCR\n", DEVNAME(sc)));
>   return 1;
>   }
> +
> + if (ISSET(sc->sc_flags, SMF_SD_MODE) && ISSET(new_ocr, MMC_OCR_S18A)) {
> + /*
> +  * Card and host support low voltage mode, begin switch
> +  * sequence.
> +  */
> + struct sdmmc_command cmd;
> +
> + memset(, 0, sizeof(cmd));
> + cmd.c_arg = 0;
> + cmd.c_flags = SCF_CMD_AC | SCF_RSP_R1;
> + cmd.c_opcode = SD_VOLTAGE_SWITCH;
> + DPRINTF(("%s: switching card to 1.8V\n", DEVNAME(sc)));
> + error = sdmmc_mmc_command(sc, );
> + if (error) {
> + DPRINTF(("%s: voltage switch command failed\n",
> + SDMMCDEVNAME(sc)));
> + return error;
> + }
> +
> + error = sdmmc_mem_signal_voltage(sc, SDMMC_SIGNAL_VOLTAGE_180);
> + if (error)
> + return error;
> +
> + SET(sc->sc_flags, SMF_UHS_MODE);
> + }
> +
> + return 

Re: snmpd(8) Remove struct listen_sock

2020-08-22 Thread Jan Klemkow
On Sun, Aug 09, 2020 at 04:10:13PM +0200, Martijn van Duren wrote:
> I still want to move udp/tcp handling out of snmpe, so that there's 
> better layering, but my previous diff never got any response and might  
> do with some more polishing.
> 
> For now, lets remove listen_sock from snmpd, since there's a 1:1
> correlation with struct address. This saves a couple of callocs during
> startup and 18 LoC.
> 
> OK?

OK jan@

> martijn@
> 
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v
> retrieving revision 1.58
> diff -u -p -r1.58 parse.y
> --- parse.y   30 Jun 2020 17:11:49 -  1.58
> +++ parse.y   9 Aug 2020 14:08:18 -
> @@ -997,7 +997,6 @@ parse_config(const char *filename, u_int
>   conf->sc_flags = flags;
>   conf->sc_confpath = filename;
>   TAILQ_INIT(>sc_addresses);
> - TAILQ_INIT(>sc_sockets);
>   strlcpy(conf->sc_rdcommunity, "public", SNMPD_MAXCOMMUNITYLEN);
>   strlcpy(conf->sc_rwcommunity, "private", SNMPD_MAXCOMMUNITYLEN);
>   strlcpy(conf->sc_trcommunity, "public", SNMPD_MAXCOMMUNITYLEN);
> Index: snmpd.h
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
> retrieving revision 1.88
> diff -u -p -r1.88 snmpd.h
> --- snmpd.h   8 Aug 2020 13:39:57 -   1.88
> +++ snmpd.h   9 Aug 2020 14:08:18 -
> @@ -482,6 +482,9 @@ struct address {
>   struct sockaddr_storage  ss;
>   in_port_tport;
>   int  ipproto;
> + int  fd;
> + struct event ev;
> + struct event evt;
>  
>   TAILQ_ENTRY(address) entry;
>  
> @@ -492,15 +495,6 @@ struct address {
>  };
>  TAILQ_HEAD(addresslist, address);
>  
> -struct listen_sock {
> - int s_fd;
> - int s_ipproto;
> - struct events_ev;
> - struct events_evt;
> - TAILQ_ENTRY(listen_sock)entry;
> -};
> -TAILQ_HEAD(socklist, listen_sock);
> -
>  enum usmauth {
>   AUTH_NONE = 0,
>   AUTH_MD5,   /* HMAC-MD5-96, RFC3414 */
> @@ -545,7 +539,6 @@ struct snmpd {
>  
>   const char  *sc_confpath;
>   struct addresslist   sc_addresses;
> - struct socklist  sc_sockets;
>   struct timeval   sc_starttime;
>   u_int32_tsc_engine_boots;
>  
> Index: snmpe.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 snmpe.c
> --- snmpe.c   8 Aug 2020 13:39:57 -   1.63
> +++ snmpe.c   9 Aug 2020 14:08:18 -
> @@ -68,7 +68,6 @@ snmpe(struct privsep *ps, struct privsep
>  {
>   struct snmpd*env = ps->ps_env;
>   struct address  *h;
> - struct listen_sock  *so;
>  #ifdef DEBUG
>   char buf[BUFSIZ];
>   struct oid  *oid;
> @@ -82,14 +81,9 @@ snmpe(struct privsep *ps, struct privsep
>  #endif
>  
>   /* bind SNMP UDP/TCP sockets */
> - TAILQ_FOREACH(h, >sc_addresses, entry) {
> - if ((so = calloc(1, sizeof(*so))) == NULL)
> - fatal("snmpe: %s", __func__);
> - if ((so->s_fd = snmpe_bind(h)) == -1)
> + TAILQ_FOREACH(h, >sc_addresses, entry)
> + if ((h->fd = snmpe_bind(h)) == -1)
>   fatal("snmpe: failed to bind SNMP socket");
> - so->s_ipproto = h->ipproto;
> - TAILQ_INSERT_TAIL(>sc_sockets, so, entry);
> - }
>  
>   proc_run(ps, p, procs, nitems(procs), snmpe_init, NULL);
>  }
> @@ -99,7 +93,7 @@ void
>  snmpe_init(struct privsep *ps, struct privsep_proc *p, void *arg)
>  {
>   struct snmpd*env = ps->ps_env;
> - struct listen_sock  *so;
> + struct address  *h;
>  
>   kr_init();
>   trap_init();
> @@ -107,17 +101,17 @@ snmpe_init(struct privsep *ps, struct pr
>   usm_generate_keys();
>  
>   /* listen for incoming SNMP UDP/TCP messages */
> - TAILQ_FOREACH(so, >sc_sockets, entry) {
> - if (so->s_ipproto == IPPROTO_TCP) {
> - if (listen(so->s_fd, 5) < 0)
> + TAILQ_FOREACH(h, >sc_addresses, entry) {
> + if (h->ipproto == IPPROTO_TCP) {
> + if (listen(h->fd, 5) < 0)
>   fatalx("snmpe: failed to listen on socket");
> - event_set(>s_ev, so->s_fd, EV_READ, snmpe_acceptcb, 
> so);
> - evtimer_set(>s_evt, snmpe_acceptcb, so);
> + event_set(>ev, h->fd, EV_READ, snmpe_acceptcb, h);
> + evtimer_set(>evt, snmpe_acceptcb, h);
>   } else {
> - event_set(>s_ev, so->s_fd, EV_READ|EV_PERSIST,
> + event_set(>ev, h->fd,