vi.beginner diff and analysis (vi.advanced to follow)
I went through vi.beginner. It works both with vi's regular settings,
and with the settings applied via EXINIT in vi.tut.csh.
I have a diff attached. I was mostly light and gentle with my
changes, but I indeed did change some outdated info and incorrect
info.
I seriously doubt vi.tut.csh's usefulness, but I'll ultimately judge
it after going through vi.advanced. I'll reply to my own email with a
vi.advanced diff when I go through it. (not tonight, but some night
soon)
Notably, I wasn't able to attach my diff in Firefox on the OpenBSD
snapshot I used. Neither could Firefox use the normal file: URL type.
(I had to scp my diff to another machine to attach it to this email)
Index: vi.beginner
===
RCS file: /cvs/src/usr.bin/vi/docs/tutorial/vi.beginner,v
retrieving revision 1.2
diff -u -p -r1.2 vi.beginner
--- vi.beginner 8 Nov 2003 19:17:29 - 1.2
+++ vi.beginner 24 May 2020 05:03:29 -
@@ -174,19 +174,19 @@ look at any line in the file we like. U
followed by a character used in an {m} command, we can return to any location
in the file we have marked.
-However, try {m3}, or {mM}. You should hear a beep, or bell. Only lower-case
-letters are acceptable to the {m} and {'} commands: numbers, upper-case
-letters, and special characters are not acceptable.
-
-If you type the {'} command with a character that is lower-case alphabetic but
-that has not been used in an {m} command, or for which the 'marked' text has
-been deleted, you will also get a beep. Try {'i}. You should get a beep
-because the command {mi} has never been issued. (Unless you've been
-experimenting.)
+Try {m3}, {mM}, or {m$}. Not only lower-case letters are acceptable to the
+{m} and {'} commands: numbers, upper-case letters, and special characters are
+also acceptable.
+
+If you type the {'} command with a character that that has not been used in an
+{m} command, or for which the 'marked' text has been deleted, you will get a
+beep. Try {'i}. You should get a beep because the command {mi} has never
+been issued. (Unless you've been experimenting.)
The command {''} attempts to return you to the location at which you last
modified some part of your file. However, my experience has been that it is
difficult to predict exactly where you will end up.
+
Section 10: {^M} {-}
Now do {ma}, marking your position at the top of the screen. Now hit {^M} (or
return) until the cursor is right ...
@@ -235,12 +235,12 @@ The {-} command moves the cursor to the
{-}, which attempts to move the cursor to the previous line in this file.
However, that line is not on the screen. The resulting action will depend on
your terminal. (Do a {^Mz^M} to reposition the file). On intelligent
-terminals (e.g. VT100s, Z19s, Concept 100s), a top line is 'scrolled on' and
-the bottom line is 'scrolled off'. Other terminals, however, may not have
-this 'reverse scrolling' feature. They will simply repaint the screen with
-the cursor line in the middle of the screen. On such terminals it is
-necessary to type {z^M} to get the cursor line back to the top of the screen.
-
+terminals (e.g. VT100s, xterm, most modern terminals), a top line is 'scrolled
+on' and the bottom line is 'scrolled off'. Some very old terminals, however,
+may not have this 'reverse scrolling' feature. They will simply repaint the
+screen with the cursor line in the middle of the screen. On such terminals it
+is necessary to type {z^M} to get the cursor line back to the top of the
+screen.
@@ -358,13 +358,13 @@ sequence shown):
The first command looks for the next occurrence of the string 'Here 2'.
However the second line of commands looks for an occurrence of 'Here 2' that
-is at the beginning of the line. When the up-arrow is the first character of
-a search string it stands for the beginning of the line. When the dollar-sign
-is the last character of the search string it stands for the end of the line.
-Therefore, the third line of commands searches for the string only when it is
-at the end of the line. Since there is only one place the string begins a
-line, and only one place the string ends the line, subsequent {//^M} and
-{??^M} will find those same strings over and over.
+is at the beginning of the line. When the caret (circumflex, up-arrow) is the
+first character of a search string it stands for the beginning of the line.
+When the dollar-sign is the last character of the search string it stands for
+the end of the line. Therefore, the third line of commands searches for the
+string only when it is at the end of the line. Since there is only one place
+the string begins a line, and only one place the string ends the line,
+subsequent {//^M} and {??^M} will find those same strings over and over.
The {n} command will find the next occurrence of the / or ? search
string. Try {/Here 2/^M} followed by several {n} and observe the
@@ -380,7 +380,7 @@ the screen. Remember tha
Re: Remove useless line from daemon class in login.conf
On Sat, 23 May 2020 22:08:11 +0100, Stuart Henderson
wrote:
> I think bumping the minimum to 2^9 would be reasonable, there's a more
> noticeable delay on some machines but I think that's fair enough (any
> cracking is likely to be done on a fast machine, and the user can
> force it lower themselves if they want to take the risk).
I think it's a good move.
Isn't this is needed as well then?
Index: cryptutil.c
===
RCS file: /cvs/src/lib/libc/crypt/cryptutil.c,v
retrieving revision 1.12
diff -u -p -r1.12 cryptutil.c
--- cryptutil.c 13 Sep 2015 15:33:48 - 1.12
+++ cryptutil.c 24 May 2020 01:57:39 -
@@ -54,7 +54,7 @@ int
crypt_newhash(const char *pass, const char *pref, char *hash, size_t
hashlen) {
int rv = -1;
- const char *defaultpref = "blowfish,8";
+ const char *defaultpref = "blowfish,9";
const char *errstr;
const char *choices[] = { "blowfish", "bcrypt" };
size_t maxchoice = sizeof(choices) / sizeof(choices[0]);
Cheers,
Daniel
vmd(8) and thread safety: a quick proof of concept using libevent 2.1 from ports
Hello tech@, Attached is a diff that patches vmd(8) to utilize libevent 2.1 (from ports) in an attempt to test the hypothesis that thread safety will help stabilize Linux guest support. There's some longer detail below about this hypothesis, but let me cut to the chase: ** This is *not* a petition or request to switch vmd(8) to libevent2 or to import libevent2. It's a proof of concept to help identify if the cause of many ghosts in the (virtual) machines is due to thread safety issues in vmd(8)'s vm processes. ** If anyone is willing, you can apply the diff to your own copy of the src tree OR simply clone my github branch (https://github.com/voutilad/openbsd-src/tree/vmd-libevent2) and build just vmd(8). You'll need to pkg_add libevent first, however. To explain a bit further, I got here after back and forth with pd@ and debugging event queues on my Linux guests. I've been hacking a Linux kernel vmm-clock implementation[0] in an effort to solve often reported clock drift due to reliance on refined-jiffies as a clocksource. A few others, including pd@, have done something similar, and the common theme was when we'd get our kvm-clock derivatives attached in the Linux guest, they'd typically die a quick death. They'd keep proper time for once with no clock drift, but they'd live short miserable lives. I spent over a week debugging the crashes and lockups and I came across 3 common scenarios that seemed to have a common theme of race conditions corrupting the libevent event queue: 1. the emulated serial console locks up, host CPU goes to 100% for the vm process 2. the whole guest locks up, host CPU goes to 100% for the vm process 3. the host dies a sudden, instantaneous death In scenario 1, the vm process would exhibit a rapid spin through the event loop, with some event constantly triggering hence the CPU utilization rising. There was speculation and a patch from pd@ to effectively add some buffering to how fast the emulated ns8250 would read data and at least on my system resolved the CPU issue, but didn't fix the read lockups. In scenario 2, the cause is a very tight loop inside libevent's timeout_process function seen in the wild for libevent 1.x [1] and even in cases for libevent 2.x [2] especially when not enabling pthread support [3]. In scenario 3, after some debugging [4], it looks due to libevent calling exit(3) because of the event_queue_insert() function detecting a double queueing [5]. The below diff does 2 things: 1. ports the vmd(8) codebase to use the libevent 2.1 API 2. recreates an event_base in the vm process forked in vmm.c::vmm_start_vm() and initializes pthread support before initializing a new event_base (which apparently enables locking mechanisms in the event_base) So far in my personal testing (Lenovo x270, i7-7500U) simply swapping in libevent 2.1 and booting my custom Linux kernel [6] with my paravirtualized clock results in a stable guest [7]. I was able to run under 100% cpu load (recompiling linux) and maintain stability and clock for multiple hours with a time drift from the host clock close to 0. I've also experienced less serial console lockups even without pd@'s patch, but they seem to be only on the emulated ns8250 reading my input. If I use `vmctl stop`, my vmmci(4) Linux driver [7] catches the request from vmd(8) and shuts down cleanly and you see console output. Going forward, while importing libevent 2.x will probably be too much headache + future care/feeding for little reward, I'm curious where best to focus next and would love some guidance on what is probably most fruitful: - Rework the vm.c event handling to not be multi-threaded? - Backport some thread safety features from libevent 2.x to the 1.x version in base? - Roll a ports-distributed version of vmd(8) that uses libevent from ports to allow increased testing? - Import libevent 2.1 into base? (just kidding :-P) I believe the "Linux clock support" conversation is a separate matter as there are multiple paths if we can improve guest stability (e.g. masquerade as KVM vs. trying to upstream VMM paravirt support into the kernel) and some really have little to do with OpenBSD specifically. [1] https://libevent-users.monkey.narkive.com/h4racbzJ/infinity-loop [2] https://www.mail-archive.com/libevent-users@freehaven.net/msg00983.html [3] https://github.com/libevent/libevent/issues/431 [4] https://gist.github.com/voutilad/9b55e8ed7abfbfec0860a1cf966aa93a [5] https://github.com/openbsd/src/blob/915a5ad8a32fee41cc229a1395f5cd0641ab2a78/lib/libevent/event.c#L874-L881 [6] https://github.com/voutilad/linux (grab branch linux-5.4-obsd, use the "config-obsd" file as the kernel config or enable "OpenBSD VMM Guest" support manually before building) [7] https://github.com/voutilad/virtio_vmmci -- Dave Voutila Index: Makefile === RCS file: /cvs/src/usr.sbin/vmd/Makefile,v retrieving revision 1.22 diff -u -p -u -p -r1.22 Makefile --- Makefile 18 Jan
[patch] httpd remove unused struct
Remove an unused struct from parse.y.
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.114
diff -u -p -u -r1.114 parse.y
--- parse.y 9 Feb 2020 09:44:04 - 1.114
+++ parse.y 24 May 2020 00:46:03 -
@@ -124,10 +124,6 @@ typedef struct {
struct timeval tv;
struct portrange port;
struct auth auth;
- struct {
- struct sockaddr_storage ss;
- char name[HOST_NAME_MAX+1];
- }addr;
} v;
int lineno;
} YYSTYPE;
[patch] smtpd add timeout to filter registration
Following patch adds a timeout to filter registration. Its easy to have
a filter fail to register due to buffering or just experimenting. With
the timeout smtpd will die and let the user know why instead of
remaining in an unresponsive state.
Index: lka_filter.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.62
diff -u -p -u -r1.62 lka_filter.c
--- lka_filter.c24 Apr 2020 11:34:07 - 1.62
+++ lka_filter.c2 May 2020 15:37:56 -
@@ -66,6 +66,7 @@ static void filter_result_disconnect(uin
static voidfilter_session_io(struct io *, int, void *);
void lka_filter_process_response(const char *, const char *);
+static voidlka_proc_timeout(int, short, void *);
struct filter_session {
uint64_tid;
@@ -180,6 +181,7 @@ struct processor_instance {
char*name;
struct io *io;
struct io *errfd;
+ struct event tmo;
int ready;
uint32_t subsystems;
};
@@ -213,10 +215,13 @@ lka_proc_config(struct processor_instanc
io_printf(pi->io, "config
}
+#define TIMEOUT 10
+
void
lka_proc_forked(const char *name, uint32_t subsystems, int fd)
{
struct processor_instance *processor;
+ struct timeval timeout = { TIMEOUT, 0 };
if (!processors_inited) {
dict_init(&processors);
@@ -232,6 +237,10 @@ lka_proc_forked(const char *name, uint32
io_set_fd(processor->io, fd);
io_set_callback(processor->io, processor_io, processor->name);
+
+ evtimer_set(&processor->tmo, lka_proc_timeout, processor);
+ evtimer_add(&processor->tmo, &timeout);
+
dict_xset(&processors, name, processor);
}
@@ -269,6 +278,7 @@ processor_register(const char *name, con
processor = dict_xget(&processors, name);
if (strcmp(line, "register
+ evtimer_del(&processor->tmo);
processor->ready = 1;
return;
}
@@ -1741,4 +1751,12 @@ lka_report_proc(const char *name, const
sp = ep + 1;
lka_report_filter_report(reqid, name, 0, direction, &tv, sp);
+}
+
+static void
+lka_proc_timeout(int fd, short events, void *arg)
+{
+ struct processor_instance *processor = arg;
+
+ fatalx("%s: failed to register", processor->name);
}
tmux(1): fix args_string_percentage error strings
Hey,
In tmux(1) there are two error strings in args_string_percentage that
are backwards. If taking a percentage yields a value that is too small
then an error string saying "too large" is used and vice versa.
One way to see an incorrect message is with
display-popup -h 0%
which complains that the height is too large.
A small diff to fix this is below.
Cheers,
Kris Katterjohn
Index: usr.bin/tmux/arguments.c
===
RCS file: /cvs/src/usr.bin/tmux/arguments.c,v
retrieving revision 1.32
diff -u -p -r1.32 arguments.c
--- usr.bin/tmux/arguments.c16 May 2020 15:40:04 - 1.32
+++ usr.bin/tmux/arguments.c23 May 2020 22:15:01 -
@@ -407,11 +407,11 @@ args_string_percentage(const char *value
}
ll = (curval * ll) / 100;
if (ll < minval) {
- *cause = xstrdup("too large");
+ *cause = xstrdup("too small");
return (0);
}
if (ll > maxval) {
- *cause = xstrdup("too small");
+ *cause = xstrdup("too large");
return (0);
}
} else {
Re: Remove useless line from daemon class in login.conf
On 2020/05/22 16:04, Theo de Raadt wrote:
> Stuart Henderson wrote:
>
> > On 2020/05/22 17:06, Daniel Jakots wrote:
> > > Hi,
> > >
> > > We used to have different numbers of blowfish rounds between the
> > > default and daemon classes in login.conf. On Jun 26, 2016, tedu
> > > committed "upgrade selected login.conf to use auto rounds for bcrypt"
> > > for amd64, sparc64, i386, and maccpc [1].
> > >
> > > Since the class daemon inherits from the default class, the
> > > :localcipher=blowfish,a:\
> > > is a duplicate.
> > >
> > > Here's a diff to remove them.
> >
> > I'm OK with unifying these settings, but FWIW I never switched to auto
> > for these, it doesn't seem all that sensible for somebody with the ability
> > to generate enough load on the machine to be able to reduce the strength
> > of bcrypt down to the 64 (2^6) rounds minimum.
>
> Yes, that is problematic.
>
> The minimum should be probably be raised, we should consider if auto
> should even exist anymore.
>
As long as it doesn't allow weakening things I think auto should still
exist so that machines can have a stronger bcrypt where it's cheap.
When this was introduced, login.conf for amd64/i386/macppc/sparc64
changed from 8 (normal users) and 9 (daemon class i.e. root) to auto.
Since other, mainly slower, arches stayed with hardcoded 8/9 I don't
think the current minimum reachable in the code makes sense at all.
I've gone to a few machines and done:
- 50 runs of "encrypt -b a" to see what setting was chosen by auto
for i in `jot 50`; do echo foo | encrypt -b a; sleep .1; done | cut -d'$' -f3 |
sort | uniq -c
- 50 runs of "encrypt -b 9" or "encrypt -b 10" and averaged, to see
how long those two settings take
time for i in `jot 50`; do echo foo | encrypt -b 10; done
(divided by 50)
Chosen -b 9-b 10
Cortex-A53 1.4GHz (pi3) all 8 0.220.40
GX-412TC 1GHz (APU2)all 8 0.160.31
Cortex-A72 1.5GHz (pi4) all 9 0.070.14
L5520 2.27GHz all 9 0.080.16
E3-1225v3 3.2GHz12x8 3x9 35x10 0.050.10
E3-1240v5 3.5GHzall 10 0.040.08
E3-1270v6 3.8GHzall 11 0.030.05
I think bumping the minimum to 2^9 would be reasonable, there's a more
noticeable delay on some machines but I think that's fair enough (any
cracking is likely to be done on a fast machine, and the user can force
it lower themselves if they want to take the risk).
With a higher minimum than that the delay starts to get very noticeable
in some cases, so I'm not sure we're ready for that yet.
I think it also makes sense to use blowfish,a in login.conf on all
arches, replacing the old 8/9. Actually -b a is already used in the
installer for both root and the standard user on all archs, whatever
they have in login.conf. Resulting in the situation that on some
archs, the bcrypt created during install for root's password is
weaker than it would be if reset after boot.
So maybe this or something like it?
Index: lib/libc/crypt/bcrypt.c
===
RCS file: /cvs/src/lib/libc/crypt/bcrypt.c,v
retrieving revision 1.57
diff -u -p -r1.57 bcrypt.c
--- lib/libc/crypt/bcrypt.c 26 Aug 2016 08:25:02 - 1.57
+++ lib/libc/crypt/bcrypt.c 23 May 2020 20:16:46 -
@@ -237,14 +237,15 @@ bcrypt_checkpass(const char *pass, const
DEF_WEAK(bcrypt_checkpass);
/*
- * Measure this system's performance by measuring the time for 8 rounds.
- * We are aiming for something that takes around 0.1s, but not too much over.
+ * Measure this system's performance by measuring the time for 2^9 rounds.
+ * We are aiming for something that takes around 0.1s, not too much over,
+ * but without allowing it to be too weak.
*/
int
_bcrypt_autorounds(void)
{
struct timespec before, after;
- int r = 8;
+ int r = 9;
char buf[_PASSWORD_LEN];
int duration;
@@ -257,12 +258,12 @@ _bcrypt_autorounds(void)
duration += (after.tv_nsec - before.tv_nsec) / 1000;
/* too quick? slow it down. */
- while (r < 16 && duration <= 6) {
+ while (r < 16 && duration <= 75000) {
r += 1;
duration *= 2;
}
/* too slow? speed it up. */
- while (r > 6 && duration > 12) {
+ while (r > 10 && duration > 12) {
r -= 1;
duration /= 2;
}
Index: etc/etc.alpha/login.conf
===
RCS file: /cvs/src/etc/etc.alpha/login.conf,v
retrieving revision 1.8
diff -u -p -r1.8 login.conf
--- etc/etc.alpha/login.conf5 Nov 2019 19:03:46 - 1.8
+++ etc/etc.alpha/login.conf23 May 2020 20:36:06 -
@@ -48,7 +48,7 @@ default:\
:openfiles-max=1024:\
:openfiles-cur=512:\
:stacksize-cur=4M:\
- :localcipher=blowfish,8:\
+ :localcipher=blowfish,a:\
:tc=auth-de
tmux(1): fix resize-pane -y error message
Hey,
A small diff is below to fix the error message from tmux(1) when an
invalid value is passed to resize-pane -y. It has mentioned "width"
instead of "height".
Cheers,
Kris Katterjohn
Index: usr.bin/tmux/cmd-resize-pane.c
===
RCS file: /cvs/src/usr.bin/tmux/cmd-resize-pane.c,v
retrieving revision 1.47
diff -u -p -r1.47 cmd-resize-pane.c
--- usr.bin/tmux/cmd-resize-pane.c 16 May 2020 15:01:30 - 1.47
+++ usr.bin/tmux/cmd-resize-pane.c 23 May 2020 19:12:45 -
@@ -117,7 +117,7 @@ cmd_resize_pane_exec(struct cmd *self, s
if (args_has(args, 'y')) {
y = args_percentage(args, 'y', 0, INT_MAX, w->sy, &cause);
if (cause != NULL) {
- cmdq_error(item, "width %s", cause);
+ cmdq_error(item, "height %s", cause);
free(cause);
return (CMD_RETURN_ERROR);
}
[patch] fuse_main.3 - fix example to compile without warnings and apply style changes
Index: fuse_main.3
===
RCS file: /cvs/src/lib/libfuse/fuse_main.3,v
retrieving revision 1.6
diff -u -p -u -r1.6 fuse_main.3
--- fuse_main.3 28 Nov 2018 21:19:11 - 1.6
+++ fuse_main.3 23 May 2020 18:11:16 -
@@ -56,39 +56,46 @@ Here is a simple example of a FUSE imple
static int
fs_readdir(const char *path, void *data, fuse_fill_dir_t filler,
-off_t off, struct fuse_file_info *ffi)
+ off_t off, struct fuse_file_info *ffi)
{
if (strcmp(path, "/") != 0)
- return (-ENOENT);
+ return -ENOENT;
filler(data, ".", NULL, 0);
filler(data, "..", NULL, 0);
filler(data, "file", NULL, 0);
- return (0);
+ return 0;
}
static int
fs_read(const char *path, char *buf, size_t size, off_t off,
-struct fuse_file_info *ffi)
+struct fuse_file_info *ffi)
{
- if (off >= 5)
- return (0);
+ size_t len;
+ const char *file_contents = "fuse filesystem example\\n";
- size = 5 - off;
- memcpy(buf, "data." + off, size);
- return (size);
+ len = strlen(file_contents);
+
+ if (off < len) {
+ if (off + size > len)
+ size = len - off;
+ memcpy(buf, file_contents + off, size);
+ } else
+ size = 0;
+
+ return size;
}
static int
fs_open(const char *path, struct fuse_file_info *ffi)
{
if (strncmp(path, "/file", 10) != 0)
- return (-ENOENT);
+ return -ENOENT;
if ((ffi->flags & 3) != O_RDONLY)
- return (-EACCES);
+ return -EACCES;
- return (0);
+ return 0;
}
static int
@@ -104,10 +111,10 @@ fs_getattr(const char *path, struct stat
st->st_nlink = 1;
st->st_size = 5;
} else {
- return (-ENOENT);
+ return -ENOENT;
}
- return (0);
+ return 0;
}
struct fuse_operations fsops = {
@@ -118,9 +125,9 @@ struct fuse_operations fsops = {
};
int
-main(int ac, char **av)
+main(int argc, char **argv)
{
- return (fuse_main(ac, av, &fsops, NULL));
+ return (fuse_main(argc, argv, &fsops, NULL));
}
.Ed
.Sh SEE ALSO
Re: vmm timer for linux guests
Hello Pratik,
Thanks for the patch. I am about to test it out too.
If you do not mind sharing, do you have your patches hosted somewhere?
Thanks
From: Pratik Vyas [m...@pd.io]
Sent: Friday, May 22, 2020 6:15 AM
To: Renato Aguiar
Cc: tech@openbsd.org; Sivaram Gowkanapalli; d...@sisu.io
Subject: Re: vmm timer for linux guests
* Renato Aguiar [2020-05-21 12:55:45 -0700]:
>Hi Sivaram,
>
>I'm the author of the e-mail thread that you mentioned. After feedback
>I got from OpenBSD community, I created a patch for Linux to enable
>kvm-clock when booting on VMM. It managed to keep clock in sync, but I
>experienced random crashes in vmd after some time running the VM.
>
>Multiple fixes have been merged to vmm/vmd since then, so I'm planning
>to give it another try with OpenBSD 6.7.
>
>If you are interested, I can share the patch with you.
>
>Regards,
>
>--
>Renato Aguiar
>
Hi Sivaram and Renato,
I have been looking at this issue this week with Dave. (funnily Dave
and I independently also decided to write a linux patch to attach
pvclock and debugging these crashes).
If you have your linux vm with kvm-clock around, can you try this rather
crude patch and see if it still crashes? This might also fix the '100%
cpu when using alpine via console' problem.
For ref, this is my linux side patch to attach kvm-clock http://ix.io/2lzK
--
Pratik
diff --git a/usr.sbin/vmd/ns8250.c b/usr.sbin/vmd/ns8250.c
index 497e6fad550..33f1a371c16 100644
--- a/usr.sbin/vmd/ns8250.c
+++ b/usr.sbin/vmd/ns8250.c
@@ -36,6 +36,8 @@
extern char *__progname;
struct ns8250_dev com1_dev;
+struct event read_delay_ev;
+struct timeval read_delay_tv;
static void com_rcv_event(int, short, void *);
static void com_rcv(struct ns8250_dev *, uint32_t, uint32_t);
@@ -61,6 +63,11 @@ ratelimit(int fd, short type, void *arg)
vcpu_deassert_pic_irq(com1_dev.vmid, 0, com1_dev.irq);
}
+static void
+arm_read(int fd, short type, void *arg) {
+ event_add(&com1_dev.event, NULL);
+}
+
void
ns8250_init(int fd, uint32_t vmid)
{
@@ -96,7 +103,7 @@ ns8250_init(int fd, uint32_t vmid)
*/
com1_dev.pause_ct = (com1_dev.baudrate / 8) / 1000 * 10;
- event_set(&com1_dev.event, com1_dev.fd, EV_READ | EV_PERSIST,
+ event_set(&com1_dev.event, com1_dev.fd, EV_READ,
com_rcv_event, (void *)(intptr_t)vmid);
/*
@@ -112,6 +119,9 @@ ns8250_init(int fd, uint32_t vmid)
timerclear(&com1_dev.rate_tv);
com1_dev.rate_tv.tv_usec = 1;
evtimer_set(&com1_dev.rate, ratelimit, NULL);
+ read_delay_tv.tv_usec = 1;
+ evtimer_set(&read_delay_ev, arm_read,
+ (void *)(intptr_t)vmid);
}
static void
@@ -131,6 +141,7 @@ com_rcv_event(int fd, short kind, void *arg)
*/
if (com1_dev.rcv_pending) {
mutex_unlock(&com1_dev.mutex);
+ evtimer_add(&read_delay_ev, &read_delay_tv);
return;
}
@@ -146,6 +157,7 @@ com_rcv_event(int fd, short kind, void *arg)
vcpu_deassert_pic_irq((uintptr_t)arg, 0, com1_dev.irq);
}
}
+ event_add(&com1_dev.event, NULL);
mutex_unlock(&com1_dev.mutex);
}
Fix manpage links in upgrade67.html
Hello, following patch fixes two manpage links that point to the wrong section. Regards, Andre Index: faq/upgrade67.html === RCS file: /cvs/www/faq/upgrade67.html,v retrieving revision 1.9 diff -u -p -r1.9 upgrade67.html --- faq/upgrade67.html 20 May 2020 20:23:38 - 1.9 +++ faq/upgrade67.html 23 May 2020 14:00:23 - @@ -163,7 +163,7 @@ any post-release fixes. Regular users must use the https://man.openbsd.org/OpenBSD-6.7/sndioctl.1";>sndioctl(1) utility in place of - https://man.openbsd.org/OpenBSD-6.7/mixerctl.1";>mixerctl(1) + https://man.openbsd.org/OpenBSD-6.7/mixerctl.8";>mixerctl(8) to adjust the volume, for instance: @@ -177,7 +177,7 @@ any post-release fixes. Note that audio devices continue to be configured with - https://man.openbsd.org/OpenBSD-6.7/mixerctl.1";>mixerctl(1) + https://man.openbsd.org/OpenBSD-6.7/mixerctl.8";>mixerctl(8) as https://man.openbsd.org/OpenBSD-6.7/sndioctl.1";>sndioctl(1) doesn't expose all audio device controls.
Re: fix pppx(4) with net/ifq.c rev 1.38
> On 23 May 2020, at 12:54, Martin Pieuchot wrote:
>
> On 22/05/20(Fri) 13:25, Vitaliy Makkoveev wrote:
>> On Fri, May 22, 2020 at 07:57:13AM +1000, David Gwynne wrote:
>>> [...]
>>> can you try the following diff?
>>>
>>
>> I tested this diff and it works for me. But the problem I pointed is
>> about pipex(4) locking.
>>
>> pipex(4) requires NET_LOCK() be grabbed not only for underlaying
>> ip{,6}_output() but for itself too. But since pppac_start() has
>> unpredictable behavior I suggested to make it predictable [1].
>
> What needs the NET_LOCK() in their? We're talking about
> pipex_ppp_output(), right? Does it really need the NET_LOCK() or
> the KERNEL_LOCK() is what protects those data structures?
Yes, about pipex_ppp_output() and pipex_output(). Except
ip{,6}_output() nothing requires NET_LOCK(). As David Gwynne pointed,
they can be replaced by ip{,6}_send().
>
>> With net/ifq.c rev 1.37 and rev 1.39 pppx_if_start() routine will be
>> allways called under NET_LOCK(), but pppac_start() will not. It depends
>> on packet count stored in `if_snd' (look at net/ifq.c:125).
>
> Ideally the *_start() routine should not require the NET_LOCK().
> Ideally the pseudo-drivers should not require the NET_LOCK(). That's
> what we should aim for.
NET_LOCK() is not required. It’s locked while corresponding start routines
were called directly from pppx_if_output() and pppac_output(). In case of
pppac_start() you can't predict NET_LOCK() status.
>
> In case of pipex(4) is isn't clear that the NET_LOCK() is necessary.
I guess, pipex(4) was wrapped by NET_LOCK() to protect it while it’s
accessed through `pr_input’. Is NET_LOCK() required for this case?
Re: [PATCH] pipex(4): rework PPP input
> On 23 May 2020, at 13:11, Sergey Ryazanov wrote: > > Hello, > > On Wed, May 20, 2020 at 10:13 PM Vitaliy Makkoveev > wrote: >> On Wed, May 20, 2020 at 04:08:01AM +0300, Sergey Ryazanov wrote: >>> On Tue, May 19, 2020 at 12:11 PM Vitaliy Makkoveev >>> wrote: On Mon, May 04, 2020 at 10:03:40PM +0300, Sergey Ryazanov wrote: > Split checks from frame accepting with header removing in the common > PPP input function. This should fix packet capture on a PPP interfaces. Can you describe the problem you fix? As mpi@ pointed to me, reviewers are stupid and have no telepathy skills :) >>> >>> When I tried to capture packets on a ppp (4) interface (with pipex >>> activated), I noticed that all the PPP CCP frames were ok, but all the >>> ingress PPP IP frames were mangled, and they did not contain the PPP >>> header at all. >> >> This time only pppx(4) and pppac(4) have pipex(4) support. > > Yes, and as I wrote in the first mail, now I am working on ppp(4) & > pipex(4) integration to speed up client side of L2TP. May be you can share you work? Not for commit, but for feedback. For example, each pipex session should have unique pair of `protocol’ and `session_id’. These values are passed from userland. While the only instance of npppd(8) uses pipex(4) this is not the problem. But you introduce the case while pipex(4) will be used by multiple independent userland programs. At least, I have interest how you handle this. > >> I don't see >> packet capture problems on them. Can you catch and share how to >> reproduce this problem with pppx(4) or pppac(4)? >> >> Also did you test your diff with pppx(4) and pppac(4)? > > I am entirely missed the fact that pppx(4) also have IFT_PPP type. > Thank you for pointing me. > > I will recheck pppx(4) work and return as soon as I will have a better > solution. Not only. Since you modify pipex(4) you should check pppac(4) too.
iwx: fix firmware error when leaving AUTH state
When moving from AUTH state to INIT or SCAN state the iwx(4) driver
performs the following two steps:
1. Remove the formerly chosen access point from the firmware's station table
2. Flush firmware Tx queues
This order of operations was inherited from iwm(4) where it works fine.
But iwx(4) firmware has changed how the flushing step works.
Flushing the Tx path now depends on the station being present in the
firmware's station table. So the driver should flush the Tx path before
removing the station. Otherwise, we get a fatal firmware error when the
driver moves out of AUTH state.
I came across this bug while testing unrelated changes. Perhaps it's been
triggered in the wild, perhaps not. There are worse problems in this driver
which I haven't figured out yet. But this one was easy to figure out and fix.
ok?
diff 17409ae76a1a51c4975a1efea612708ad984b8df
087785340de686b7a85b51f5f1137690e3b88c2f
blob - dd72733d9028d10a3e8dfdce7f1211c72475843f
blob + 7a9e711abbc6814dfbc2bafc8c72ed8244f53d57
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -5681,6 +5681,12 @@ iwx_deauth(struct iwx_softc *sc)
iwx_unprotect_session(sc, in);
if (sc->sc_flags & IWX_FLAG_STA_ACTIVE) {
+ err = iwx_flush_tx_path(sc);
+ if (err) {
+ printf("%s: could not flush Tx path (error %d)\n",
+ DEVNAME(sc), err);
+ return err;
+ }
err = iwx_rm_sta_cmd(sc, in);
if (err) {
printf("%s: could not remove STA (error %d)\n",
@@ -5688,13 +5694,6 @@ iwx_deauth(struct iwx_softc *sc)
return err;
}
sc->sc_flags &= ~IWX_FLAG_STA_ACTIVE;
- }
-
- err = iwx_flush_tx_path(sc);
- if (err) {
- printf("%s: could not flush Tx path (error %d)\n",
- DEVNAME(sc), err);
- return err;
}
if (sc->sc_flags & IWX_FLAG_BINDING_ACTIVE) {
iwx: fix binding command
The iwx(4) binding command can fail with 5 Ghz channels. Some firmware versions don't expect LMAC_5G_INDEX in the binding command. I have no idea what "CDB" stands for, but this check matches what the Linux driver does and makes the command work on -48 firmware. The Linux driver actually checks two capabilities, CAPA_BINDING_CDB_SUPPORT and CAPA_CDB_SUPPORT. Because BINDING_CDB_SUPPORT is set for all firmware versions used by iwx(4) we don't need to check it explicitly. Linux has to check both flags because their code runs with iwm(4) and iwx(4) devices. ok? diff cad9585f8bd2c9a5a5bcdc5cd1fa273652ba3309 17409ae76a1a51c4975a1efea612708ad984b8df blob - b65fa80f56f6dd878b1ac48bbed7ae2d191ffc76 blob + dd72733d9028d10a3e8dfdce7f1211c72475843f --- sys/dev/pci/if_iwx.c +++ sys/dev/pci/if_iwx.c @@ -3765,7 +3765,8 @@ iwx_binding_cmd(struct iwx_softc *sc, struct iwx_node for (i = 1; i < IWX_MAX_MACS_IN_BINDING; i++) cmd.macs[i] = htole32(IWX_FW_CTXT_INVALID); - if (IEEE80211_IS_CHAN_2GHZ(phyctxt->channel)) + if (IEEE80211_IS_CHAN_2GHZ(phyctxt->channel) || + !isset(sc->sc_enabled_capa, IWX_UCODE_TLV_CAPA_CDB_SUPPORT)) cmd.lmac_id = htole32(IWX_LMAC_24G_INDEX); else cmd.lmac_id = htole32(IWX_LMAC_5G_INDEX);
iwx: beacon filter command v4
I have started looking into updating iwx(4) to newer firmware.
This newer firmware is not working yet but I already have a few simple
changes which could be reviewed and committed. This is the first one:
Newer iwx(4) firmware versions will require a larger beacon filter command.
The extra fields fields are initialized to zero even on Linux but newer
firmware will require a size change anyway.
This is a fairly straightforward adaptation of changes made in iwlwifi.
ok?
Tested on -46 and -48 firmware.
diff 83ac15007194e8c54722e3f86d4805975b1b47a8
cad9585f8bd2c9a5a5bcdc5cd1fa273652ba3309
blob - 96350c02d07ea7a54cb27f04257ced45814b15d6
blob + b65fa80f56f6dd878b1ac48bbed7ae2d191ffc76
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -4411,8 +4411,16 @@ int
iwx_beacon_filter_send_cmd(struct iwx_softc *sc,
struct iwx_beacon_filter_cmd *cmd)
{
+ size_t len;
+
+ if (isset(sc->sc_ucode_api, IWX_UCODE_TLV_API_BEACON_FILTER_V4))
+ len = sizeof(struct iwx_beacon_filter_cmd);
+ else
+ len = offsetof(struct iwx_beacon_filter_cmd,
+ bf_threshold_absolute_low);
+
return iwx_send_cmd_pdu(sc, IWX_REPLY_BEACON_FILTERING_CMD,
- 0, sizeof(struct iwx_beacon_filter_cmd), cmd);
+ 0, len, cmd);
}
int
blob - 3ca42ff579a929d5f5d71b80e8afe1943aec60f2
lob + 04653f5d23e199b791eef8421b881a6a1854343a
--- sys/dev/pci/if_iwxreg.h
+++ sys/dev/pci/if_iwxreg.h
@@ -909,6 +909,7 @@ enum msix_ivar_for_cause {
#define IWX_UCODE_TLV_API_ADAPTIVE_DWELL 32
#define IWX_UCODE_TLV_API_NEW_RX_STATS 35
#define IWX_UCODE_TLV_API_ADAPTIVE_DWELL_V242
+#define IWX_UCODE_TLV_API_BEACON_FILTER_V4 47
#define IWX_UCODE_TLV_API_REDUCED_SCAN_CONFIG 56
#define IWX_UCODE_TLV_API_SCAN_EXT_CHAN_VER58
#define IWX_NUM_UCODE_TLV_API 128
@@ -3996,6 +3997,13 @@ struct iwx_uapsd_misbehaving_ap_notif {
* @ba_escape_timer: Fully receive and parse beacon if no beacons were passed
* for a longer period of time then this escape-timeout. Units: Beacons.
* @ba_enable_beacon_abort: 1, beacon abort is enabled; 0, disabled.
+ * @bf_threshold_absolute_low: See below.
+ * @bf_threshold_absolute_high: Send Beacon to driver if Energy value
calculated
+ * for this beacon crossed this absolute threshold. For the 'Increase'
+ * direction the bf_energy_absolute_low[i] is used. For the 'Decrease'
+ * direction the bf_energy_absolute_high[i] is used. Zero value means
+ * that this specific threshold is ignored for beacon filtering, and
+ * beacon will not be forced to be sent to driver due to this setting.
*/
struct iwx_beacon_filter_cmd {
uint32_t bf_energy_delta;
@@ -4009,7 +4017,9 @@ struct iwx_beacon_filter_cmd {
uint32_t bf_escape_timer;
uint32_t ba_escape_timer;
uint32_t ba_enable_beacon_abort;
-} __packed;
+ uint32_t bf_threshold_absolute_low[2];
+ uint32_t bf_threshold_absolute_high[2];
+} __packed; /* BEACON_FILTER_CONFIG_API_S_VER_4 */
/* Beacon filtering and beacon abort */
#define IWX_BF_ENERGY_DELTA_DEFAULT 5
Re: OpenSMTPD: unprivileged mode - now with diff
On Sat, 23 May 2020 15:53:05 +0200, Christopher Zimmermann wrote: > Ok to commit the below change? OK millert@ - todd
Re: OpenSMTPD: unprivileged mode - now with diff
On Sun, Apr 26, 2020 at 08:55:14AM +, gil...@poolp.org wrote:
April 26, 2020 10:34 AM, "Christopher Zimmermann" wrote:
Hi,
I further developed my approach to allow running smtpd with fewer privileges.
This diff does two
things:
- always run lmtp deliveries as SMTPD_USER. The change to mda_unpriv.c is
needed, because otherwise
all mails would be delivered to SMTPD_USER.
- add two internal flags NOPRIV and NEEDPRIV. NOPRIV can be configured by the
simple directive
"no-priv". NEEDPRIV gets set on all delivery methods / options requiring
setuid() to run as the
receipient user.
A configuration error is produced on any conflict betweed NEEDPRIV and NOPRIV.
In case of a NOPRIV run smtpd will drop root privileges.
This will break .forward and alias filters.
The change to the lmtp delivery has benefits even without the second change.
With the second change
my smtpd now runs without root privileges.
The NEEDPRIV/NOPRIV options are meant to allow restricting of the privileges of
other delivery
methods.
I am now looking for OKs on the first change to do unprivileged lmtp deliveries
and feedback on the
general approach of the second change.
The LMTP change seems interesting to me, it means that a broken LMTP delivery
will fail with _smtpd privileges instead of the (unprivileged) recipient user
so I think it's a good move.
Ok to commit the below change?
Christopher
Index: mda_unpriv.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mda_unpriv.c,v
retrieving revision 1.6
diff -u -p -r1.6 mda_unpriv.c
--- mda_unpriv.c2 Feb 2020 22:13:48 - 1.6
+++ mda_unpriv.c23 May 2020 13:43:39 -
@@ -69,8 +69,8 @@ mda_unpriv(struct dispatcher *dsp, struc
xasprintf(&mda_environ[idx++], "RECIPIENT=%s@%s", deliver->dest.user,
deliver->dest.domain);
xasprintf(&mda_environ[idx++], "SHELL=/bin/sh");
xasprintf(&mda_environ[idx++], "LOCAL=%s", deliver->rcpt.user);
- xasprintf(&mda_environ[idx++], "LOGNAME=%s", pw_name);
- xasprintf(&mda_environ[idx++], "USER=%s", pw_name);
+ xasprintf(&mda_environ[idx++], "LOGNAME=%s",
deliver->userinfo.username);
+ xasprintf(&mda_environ[idx++], "USER=%s", deliver->userinfo.username);
if (deliver->sender.user[0])
xasprintf(&mda_environ[idx++], "SENDER=%s@%s",
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.277
diff -u -p -r1.277 parse.y
--- parse.y 24 Feb 2020 23:54:27 - 1.277
+++ parse.y 23 May 2020 13:43:40 -
@@ -690,10 +690,12 @@ MBOX {
| LMTP STRING {
asprintf(&dispatcher->u.local.command,
"/usr/libexec/mail.lmtp -d %s -u", $2);
+ dispatcher->u.local.user = SMTPD_USER;
} dispatcher_local_options
| LMTP STRING RCPT_TO {
asprintf(&dispatcher->u.local.command,
"/usr/libexec/mail.lmtp -d %s -r", $2);
+ dispatcher->u.local.user = SMTPD_USER;
} dispatcher_local_options
| MDA STRING {
asprintf(&dispatcher->u.local.command,
--
http://gmerlin.de
OpenPGP: http://gmerlin.de/christopher.pub
CB07 DA40 B0B6 571D 35E2 0DEF 87E2 92A7 13E5 DEE1
ldomctl: reject vdisk, vnet and iodevice parameters for primary domain
In analogy to guest domains requiring vcpu, memory and at least one
bootable device (vdisk, vnet or iodevice), the primary domain must not
be configured with vdisk, vnet or iodevice parameters; it does make
sense to provide virtual disks or interfaces to it and PCIe devices not
assigned to guest domains automatically end up in the primary domain.
ldom.conf(5) also documents those explicitly for guest domains only.
Feedback? OK?
===
RCS file: /cvs/src/usr.sbin/ldomctl/parse.y,v
retrieving revision 1.19
diff -u -p -r1.19 parse.y
--- parse.y 23 May 2020 13:06:33 - 1.19
+++ parse.y 23 May 2020 13:54:52 -
@@ -181,12 +181,22 @@ domainopts: VCPU vcpu {
domain->memory = $2;
}
| VDISK STRING vdisk_opts {
+ if (strcmp(domain->name, "primary") == 0) {
+ yyerror("vdisk option invalid for primary"
+ " domain");
+ YYERROR;
+ }
struct vdisk *vdisk = xmalloc(sizeof(struct vdisk));
vdisk->path = $2;
vdisk->devalias = $3.devalias;
SIMPLEQ_INSERT_TAIL(&domain->vdisk_list, vdisk, entry);
}
| VNET vnet_opts {
+ if (strcmp(domain->name, "primary") == 0) {
+ yyerror("vnet option invalid for primary"
+ " domain");
+ YYERROR;
+ }
struct vnet *vnet = xmalloc(sizeof(struct vnet));
vnet->mac_addr = $2.mac_addr;
vnet->mtu = $2.mtu;
@@ -200,6 +210,11 @@ domainopts : VCPU vcpu {
SIMPLEQ_INSERT_TAIL(&domain->var_list, var, entry);
}
| IODEVICE STRING {
+ if (strcmp(domain->name, "primary") == 0) {
+ yyerror("iodevice option invalid for primary"
+ " domain");
+ YYERROR;
+ }
struct domain *odomain;
struct iodev *iodev;
SIMPLEQ_FOREACH(odomain, &conf->domain_list, entry)
Re: carp: send only IPv4 carp packets on dual stack interface
On Sat, May 23, 2020 at 01:55:54PM +1000, David Gwynne wrote: On 23 May 2020, at 8:44 am, Christopher Zimmermann wrote: On Sun, Jan 19, 2020 at 01:32:17PM +, Stuart Henderson wrote: On 2020/01/19 00:11, Sebastian Benoit wrote: chr...@openbsd.org(chr...@openbsd.org) on 2020.01.18 06:18:21 +0100: > On Wed, Jan 15, 2020 at 12:47:28PM +0100, Sebastian Benoit wrote: > >Christopher Zimmermann(chr...@openbsd.org) on 2020.01.15 11:55:43 +0100: > >>Hi, > >> > >>as far as I can see a dual stack carp interface does not care whether it > >>receives advertisements addressed to IPv4 or IPv6. Any one will do. > >>So I propose to send IPv6 advertisements only when IPv4 is not possible. > >> > >>Why? > >> > >>- Noise can be reduced by using unicast advertisements. > >> This is only possible for IPv4 by ``ifconfig carppeer``. > >> I don't like flooding the whole network with carp advertisements when > >> I may also unicast them. > > > >Maybe i'm getting confused, but in the problem description you were talking > >about v6 vs v4, and here you argue about unicast (vs multicast?) being > >better. Thats orthogonal, isnt it? > > Yes, kind of. The point is we support ``carppeer`` for IPv4, but not for > IPv6. > > >>- breaking IPv6 connectivity (for example by running iked without -6) > >> will start a preempt-war, because failing ip6_output will cause the > >> demote counter to be increased. That's what hit me. > > > >But the whole point of carp is to notice broken connectivity. If you run v6 > >on an interface, you want to know if its working, no? > > I grant you that much. But what kind of failures do you hope to detect > on the _sending_ carp master, that would not also affect the backup? sure: misconfigured pf. Missing routes. Buggy switch. misconfigured mac address filter on switch. I'm afraid you guys haven't yet got the point I'm trying to make. Current behaviour is that in a dual-stack carp setup failover only happens when advertisements on _both_ AFs fail to reach the backup. A node in backup state will stay in backup state as long as it receives _any_ advertisements. In my mind this is the only sensible way for a backup node to react. If a backup node that fails to receive advertisements of only one AF would transition to master it would in most cases start a preempt war. So why do we even send dual-stack advertisements? The only effect those dual-stack ipv6 advertisements currently have is that they prevent failover when ipv4 connectivity breaks. I would propose to choose one "sentinel" AF (in this case ipv4) and failover whenever advertisements of this AF fail to reach the backup. Monitoring multiple AFs is not helpful, because there is no good way in which to react to a failure that affects only one AF. I don't know if this helps, but at work we use separate carp interfaces for v4 and v6. It ends up looking a bit like this: # cat /etc/hostname.vlan871: parent aggr0 vnetid 871 inet alias 192.0.2.2/24 inet6 alias 2001:db8:871::2/64 up # cat /etc/hostname.carp40871 carpdev vlan871 vhid 47 -inet6 -group carp group ipv4g inet alias 192.0.2.1/24 up # cat /etc/hostname.carp60871 carpdev vlan871 vhid 61 -group carp group ipv6g inet6 alias 2001:db8:871::1/64 up This let's us run a pair of firewalls one active for v4 and the other for v6. We don't do any af-to in PF, so it works pretty well. But yeah, it means v4 and v6 fail separately. Yes that is a sensible and coherent setup where ipv4 and ipv6 are kept completely separate. But what I'm proposing to change is the dual stack mode of carp where there is only one interface. There is no use for advertisements on two address families, because in case of failure of only one address family there is no sensible way to respond to that failure. > >At the very least, this needs some more thought and testing in > >all > >the ways > >carp can be configured. > > Anyway, my main concern indeed is the broadcast noise generated by carp > and I would be equally happy if we had a ``carppeer6`` option. Would > that be considered? of course carppeer should work with v6, and as claudio says without an extra keyword in ifconfig, but thats a trivial detail. Currently carp only handles one address per af, setting carppeer twice changes the current peer address rather than adding another. A trivial implementation that sets the v4 peer address if a v4 address is passed in, and sets the v6 peer address if a v6 address is passed in, that would mean things work differently with ifconfig carp1 carppeer $foo ifconfig carp1 carppeer $bar depending on whether foo/bar are v4 or v6. Also removing a configured carppeer address to reset to multicast is just done with -carppeer with no way to indicate the af. It would work pretty nicely if you could set multiple carppeer addresses (of whatever af) and remove them individually. That's a more complex change (carp would need to keep a list of peers per af rather than a single address) but without something
Re: ldomctl: Fail on duplicate vcpu and memory parameters
On Sat, May 23, 2020 at 02:38:03PM +0200, Mark Kettenis wrote: > Looks reasonable. I'd change the message for iodevice though to "iodevice %s > already assigned". Done, thanks.
Re: ldomctl: Fail on duplicate vcpu and memory parameters
> Date: Sat, 23 May 2020 11:35:46 +0200
> From: Klemens Nanni
>
> On Mon, Jan 13, 2020 at 12:59:23PM +0100, Klemens Nanni wrote:
> > 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?
> This still is still in my tree.
>
> It properly detects duplicate vcpu and memory lines for all domains
> including the primary one; duplicate iodevice lines are only detected
> for guest domains; preventing iodevice, vnet and vdisk lines in the
> primary domain is stuff for a separate diff.
>
> Feedback? OK?
Looks reasonable. I'd change the message for iodevice though to "iodevice %s
already assigned".
ok with that fix.
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/parse.y,v
> retrieving revision 1.18
> diff -u -p -r1.18 parse.y
> --- parse.y 21 Feb 2020 19:39:28 - 1.18
> +++ parse.y 23 May 2020 09:23:58 -
> @@ -166,10 +166,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 vdisk_opts {
> @@ -192,10 +200,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);
> - }
> + }
> ;
>
> vdisk_opts : { vdisk_opts_default(); }
>
>
Re: Kill unused cdev_pc_init()
> Date: Sat, 23 May 2020 13:32:33 +0200
> From: Martin Pieuchot
>
> ok?
sure
> Index: arch/amd64/amd64/conf.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/conf.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 conf.c
> --- arch/amd64/amd64/conf.c 13 May 2020 08:32:43 - 1.69
> +++ arch/amd64/amd64/conf.c 23 May 2020 11:18:21 -
> @@ -75,12 +75,6 @@ struct bdevsw bdevsw[] =
> };
> int nblkdev = nitems(bdevsw);
>
> -/* open, close, read, write, ioctl, tty, mmap */
> -#define cdev_pc_init(c,n) { \
> - dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
> - dev_init(c,n,write), dev_init(c,n,ioctl), dev_init(c,n,stop), \
> - dev_init(c,n,tty), ttselect, dev_init(c,n,mmap), D_TTY }
> -
> /* open, close, read, ioctl */
> #define cdev_joy_init(c,n) { \
> dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
> Index: arch/arm64/arm64/conf.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/arm64/conf.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 conf.c
> --- arch/arm64/arm64/conf.c 13 May 2020 08:10:03 - 1.13
> +++ arch/arm64/arm64/conf.c 23 May 2020 11:18:24 -
> @@ -72,12 +72,6 @@ struct bdevsw bdevsw[] =
> };
> int nblkdev = nitems(bdevsw);
>
> -/* open, close, read, write, ioctl, tty, mmap */
> -#define cdev_pc_init(c,n) { \
> - dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
> - dev_init(c,n,write), dev_init(c,n,ioctl), dev_init(c,n,stop), \
> - dev_init(c,n,tty), ttselect, dev_init(c,n,mmap), D_TTY }
> -
> /* open, close, read, ioctl */
> #define cdev_joy_init(c,n) { \
> dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
> Index: arch/i386/i386/conf.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/conf.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 conf.c
> --- arch/i386/i386/conf.c 13 May 2020 08:32:43 - 1.168
> +++ arch/i386/i386/conf.c 23 May 2020 11:18:27 -
> @@ -77,12 +77,6 @@ struct bdevsw bdevsw[] =
> };
> int nblkdev = nitems(bdevsw);
>
> -/* open, close, read, write, ioctl, tty, mmap */
> -#define cdev_pc_init(c,n) { \
> - dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
> - dev_init(c,n,write), dev_init(c,n,ioctl), dev_init(c,n,stop), \
> - dev_init(c,n,tty), ttpoll, dev_init(c,n,mmap), D_TTY }
> -
> /* open, close, read, ioctl */
> #define cdev_joy_init(c,n) { \
> dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
> Index: arch/i386/include/conf.h
> ===
> RCS file: /cvs/src/sys/arch/i386/include/conf.h,v
> retrieving revision 1.18
> diff -u -p -r1.18 conf.h
> --- arch/i386/include/conf.h 13 May 2020 08:32:43 - 1.18
> +++ arch/i386/include/conf.h 23 May 2020 11:18:42 -
> @@ -40,15 +40,6 @@ cdev_decl(pms);
> bdev_decl(fd);
> cdev_decl(fd);
>
> -/* open, close, read, write, ioctl, tty, mmap */
> -#define cdev_pc_init(c,n) { \
> - dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
> - dev_init(c,n,write), dev_init(c,n,ioctl), dev_init(c,n,stop), \
> - dev_init(c,n,tty), ttpoll, dev_init(c,n,mmap), D_TTY }
> -
> -cdev_decl(pc);
> -
> -
> #define cdev_acpiapm_init(c,n) {\
> dev_init(c,n,open), dev_init(c,n,close), (dev_type_read((*))) enodev, \
> (dev_type_write((*))) enodev, dev_init(c,n,ioctl), \
>
>
Kill unused cdev_pc_init()
ok?
Index: arch/amd64/amd64/conf.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/conf.c,v
retrieving revision 1.69
diff -u -p -r1.69 conf.c
--- arch/amd64/amd64/conf.c 13 May 2020 08:32:43 - 1.69
+++ arch/amd64/amd64/conf.c 23 May 2020 11:18:21 -
@@ -75,12 +75,6 @@ struct bdevswbdevsw[] =
};
intnblkdev = nitems(bdevsw);
-/* open, close, read, write, ioctl, tty, mmap */
-#define cdev_pc_init(c,n) { \
- dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
- dev_init(c,n,write), dev_init(c,n,ioctl), dev_init(c,n,stop), \
- dev_init(c,n,tty), ttselect, dev_init(c,n,mmap), D_TTY }
-
/* open, close, read, ioctl */
#define cdev_joy_init(c,n) { \
dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
Index: arch/arm64/arm64/conf.c
===
RCS file: /cvs/src/sys/arch/arm64/arm64/conf.c,v
retrieving revision 1.13
diff -u -p -r1.13 conf.c
--- arch/arm64/arm64/conf.c 13 May 2020 08:10:03 - 1.13
+++ arch/arm64/arm64/conf.c 23 May 2020 11:18:24 -
@@ -72,12 +72,6 @@ struct bdevswbdevsw[] =
};
intnblkdev = nitems(bdevsw);
-/* open, close, read, write, ioctl, tty, mmap */
-#define cdev_pc_init(c,n) { \
- dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
- dev_init(c,n,write), dev_init(c,n,ioctl), dev_init(c,n,stop), \
- dev_init(c,n,tty), ttselect, dev_init(c,n,mmap), D_TTY }
-
/* open, close, read, ioctl */
#define cdev_joy_init(c,n) { \
dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
Index: arch/i386/i386/conf.c
===
RCS file: /cvs/src/sys/arch/i386/i386/conf.c,v
retrieving revision 1.168
diff -u -p -r1.168 conf.c
--- arch/i386/i386/conf.c 13 May 2020 08:32:43 - 1.168
+++ arch/i386/i386/conf.c 23 May 2020 11:18:27 -
@@ -77,12 +77,6 @@ struct bdevswbdevsw[] =
};
intnblkdev = nitems(bdevsw);
-/* open, close, read, write, ioctl, tty, mmap */
-#define cdev_pc_init(c,n) { \
- dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
- dev_init(c,n,write), dev_init(c,n,ioctl), dev_init(c,n,stop), \
- dev_init(c,n,tty), ttpoll, dev_init(c,n,mmap), D_TTY }
-
/* open, close, read, ioctl */
#define cdev_joy_init(c,n) { \
dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
Index: arch/i386/include/conf.h
===
RCS file: /cvs/src/sys/arch/i386/include/conf.h,v
retrieving revision 1.18
diff -u -p -r1.18 conf.h
--- arch/i386/include/conf.h13 May 2020 08:32:43 - 1.18
+++ arch/i386/include/conf.h23 May 2020 11:18:42 -
@@ -40,15 +40,6 @@ cdev_decl(pms);
bdev_decl(fd);
cdev_decl(fd);
-/* open, close, read, write, ioctl, tty, mmap */
-#define cdev_pc_init(c,n) { \
- dev_init(c,n,open), dev_init(c,n,close), dev_init(c,n,read), \
- dev_init(c,n,write), dev_init(c,n,ioctl), dev_init(c,n,stop), \
- dev_init(c,n,tty), ttpoll, dev_init(c,n,mmap), D_TTY }
-
-cdev_decl(pc);
-
-
#definecdev_acpiapm_init(c,n) {\
dev_init(c,n,open), dev_init(c,n,close), (dev_type_read((*))) enodev, \
(dev_type_write((*))) enodev, dev_init(c,n,ioctl), \
Re: vldcp(4/sparc64), magma(4) and spif(4) kqfilter
> Date: Sat, 23 May 2020 12:42:04 +0200
> From: Martin Pieuchot
>
> On 21/05/20(Thu) 14:44, Mark Kettenis wrote:
> > > Date: Wed, 20 May 2020 14:39:05 +0200
> > > From: Martin Pieuchot
> > > Cc: tech@openbsd.org
> > > [...]
> > > Diff below fixed the cbus_intr_setenabled() line and `avail' calculation.
> > > Is it what you were pointing?
> >
> > Yes. But I think it still isn't right. See below.
> >
> > Are we allowed to make up what these functions return in kn->kn_data?
> > Since this device only allows reading and writing complete packets,
> > counting packets instead of bytes probably makes sense.
>
> What is written in `kn_data' will be available to userland in the `data'
> field of the corresponding "struct kevent" exchanged via kevent(2). It
> is described as a "Filter-specific data value".
Which for EVFILT_READ and EVFILT_WRITE is dependent on the actual
device so it can be packets instead of bytes.
>
> To implement poll(2)/select(2) on top of kqfilter* routines the value
> doesn't matter, these interfaces only need to know if something can be
> read or written, not how much.
>
> So if you believe that counting packets make more sense, could you tell
> which calculation I should use or maybe it's easier for you to modify the
> routines once in tree?
You're already counting packets.
> > > When a kevent is registered to a kqueue, the corresponding filter
> > > function, pointed by `f_event', is called. This is similar to calling
> > > `fo_poll' inside pollscan() before going to sleep.
> >
> > I guess it has to because if there is something to read/write it
> > shouldn't go to sleep ;). Does this mean kn->kn_data should be set to
> > 0 where we enable the interrupt?
>
> The first time a filter is called with a newly allocated descriptor,
> `kn_data' is already cleared. The existing kqfilters are inconsistent
> when it comes to set kn_data to 0 and which value to test in the return
> statement.
ok
> These 2 behaviors matter for events that stay on the queue after having
> being triggered.
>
> I haven't investigated this part of the handlers yet. This question is
> also related to the EV_CLEAR flag that one can set in events in userland.
>
> For the proposed poll(2)/select(2) diff it doesn't matter because events
> are always allocated & freed for every syscall. However this question
> should be answered if we want to implement lazy removal & reuse of
> descriptors.
In general I think we shouldn't add any new filters in this process
that are "wrong". As this will come back to bite us later. That
said, this device is special and not expected to be used by random
ports so fixing things later is much easier.
> Diff below removes the "-1" as suggested. Is it ok to commit it and
> address your other questions, which apply to all kq filters, later?
There is one fix you missed, see below.
With that change, ok kettenis@
> Index: arch/sparc64/dev/vldcp.c
> ===
> RCS file: /cvs/src/sys/arch/sparc64/dev/vldcp.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 vldcp.c
> --- arch/sparc64/dev/vldcp.c 19 Dec 2019 12:04:38 - 1.19
> +++ arch/sparc64/dev/vldcp.c 23 May 2020 10:38:08 -
> @@ -70,6 +70,11 @@ struct vldcp_softc {
>
> int vldcp_match(struct device *, void *, void *);
> void vldcp_attach(struct device *, struct device *, void *);
> +void filt_vldcprdetach(struct knote *);
> +void filt_vldcpwdetach(struct knote *);
> +int filt_vldcpread(struct knote *, long);
> +int filt_vldcpwrite(struct knote *, long);
> +int vldcpkqfilter(dev_t, struct knote *);
>
> struct cfattach vldcp_ca = {
> sizeof(struct vldcp_softc), vldcp_match, vldcp_attach
> @@ -614,4 +619,122 @@ vldcppoll(dev_t dev, int events, struct
> }
> splx(s);
> return revents;
> +}
> +
> +void
> +filt_vldcprdetach(struct knote *kn)
> +{
> + struct vldcp_softc *sc = (void *)kn->kn_hook;
> + int s;
> +
> + s = spltty();
> + klist_remove(&sc->sc_rsel.si_note, kn);
> + splx(s);
> +}
> +
> +void
> +filt_vldcpwdetach(struct knote *kn)
> +{
> + struct vldcp_softc *sc = (void *)kn->kn_hook;
> + int s;
> +
> + s = spltty();
> + klist_remove(&sc->sc_wsel.si_note, kn);
> + splx(s);
> +}
> +
> +int
> +filt_vldcpread(struct knote *kn, long hint)
> +{
> + struct vldcp_softc *sc = (void *)kn->kn_hook;
> + struct ldc_conn *lc = &sc->sc_lc;
> + uint64_t head, tail, avail, state;
> + int s, err;
> +
> + s = spltty();
> + err = hv_ldc_rx_get_state(lc->lc_id, &head, &tail, &state);
> + if (err == 0 && state == LDC_CHANNEL_UP && head != tail) {
> + avail = (head - tail) / sizeof(struct ldc_pkt) +
> + lc->lc_rxq->lq_nentries;
avail = (tail - head) / sizeof(struct ldc_pkt) +
lc->lc_rxq->lq_nentries;
> + avail %= lc->lc_rxq->lq_nentries;
> + kn->kn_data = avail;
> + } else {
>
Re: vldcp(4/sparc64), magma(4) and spif(4) kqfilter
On 21/05/20(Thu) 14:44, Mark Kettenis wrote:
> > Date: Wed, 20 May 2020 14:39:05 +0200
> > From: Martin Pieuchot
> > Cc: tech@openbsd.org
> > [...]
> > Diff below fixed the cbus_intr_setenabled() line and `avail' calculation.
> > Is it what you were pointing?
>
> Yes. But I think it still isn't right. See below.
>
> Are we allowed to make up what these functions return in kn->kn_data?
> Since this device only allows reading and writing complete packets,
> counting packets instead of bytes probably makes sense.
What is written in `kn_data' will be available to userland in the `data'
field of the corresponding "struct kevent" exchanged via kevent(2). It
is described as a "Filter-specific data value".
To implement poll(2)/select(2) on top of kqfilter* routines the value
doesn't matter, these interfaces only need to know if something can be
read or written, not how much.
So if you believe that counting packets make more sense, could you tell
which calculation I should use or maybe it's easier for you to modify the
routines once in tree?
> > When a kevent is registered to a kqueue, the corresponding filter
> > function, pointed by `f_event', is called. This is similar to calling
> > `fo_poll' inside pollscan() before going to sleep.
>
> I guess it has to because if there is something to read/write it
> shouldn't go to sleep ;). Does this mean kn->kn_data should be set to
> 0 where we enable the interrupt?
The first time a filter is called with a newly allocated descriptor,
`kn_data' is already cleared. The existing kqfilters are inconsistent
when it comes to set kn_data to 0 and which value to test in the return
statement.
These 2 behaviors matter for events that stay on the queue after having
being triggered.
I haven't investigated this part of the handlers yet. This question is
also related to the EV_CLEAR flag that one can set in events in userland.
For the proposed poll(2)/select(2) diff it doesn't matter because events
are always allocated & freed for every syscall. However this question
should be answered if we want to implement lazy removal & reuse of
descriptors.
Diff below removes the "-1" as suggested. Is it ok to commit it and
address your other questions, which apply to all kq filters, later?
Index: arch/sparc64/dev/vldcp.c
===
RCS file: /cvs/src/sys/arch/sparc64/dev/vldcp.c,v
retrieving revision 1.19
diff -u -p -r1.19 vldcp.c
--- arch/sparc64/dev/vldcp.c19 Dec 2019 12:04:38 - 1.19
+++ arch/sparc64/dev/vldcp.c23 May 2020 10:38:08 -
@@ -70,6 +70,11 @@ struct vldcp_softc {
intvldcp_match(struct device *, void *, void *);
void vldcp_attach(struct device *, struct device *, void *);
+void filt_vldcprdetach(struct knote *);
+void filt_vldcpwdetach(struct knote *);
+intfilt_vldcpread(struct knote *, long);
+intfilt_vldcpwrite(struct knote *, long);
+intvldcpkqfilter(dev_t, struct knote *);
struct cfattach vldcp_ca = {
sizeof(struct vldcp_softc), vldcp_match, vldcp_attach
@@ -614,4 +619,122 @@ vldcppoll(dev_t dev, int events, struct
}
splx(s);
return revents;
+}
+
+void
+filt_vldcprdetach(struct knote *kn)
+{
+ struct vldcp_softc *sc = (void *)kn->kn_hook;
+ int s;
+
+ s = spltty();
+ klist_remove(&sc->sc_rsel.si_note, kn);
+ splx(s);
+}
+
+void
+filt_vldcpwdetach(struct knote *kn)
+{
+ struct vldcp_softc *sc = (void *)kn->kn_hook;
+ int s;
+
+ s = spltty();
+ klist_remove(&sc->sc_wsel.si_note, kn);
+ splx(s);
+}
+
+int
+filt_vldcpread(struct knote *kn, long hint)
+{
+ struct vldcp_softc *sc = (void *)kn->kn_hook;
+ struct ldc_conn *lc = &sc->sc_lc;
+ uint64_t head, tail, avail, state;
+ int s, err;
+
+ s = spltty();
+ err = hv_ldc_rx_get_state(lc->lc_id, &head, &tail, &state);
+ if (err == 0 && state == LDC_CHANNEL_UP && head != tail) {
+ avail = (head - tail) / sizeof(struct ldc_pkt) +
+ lc->lc_rxq->lq_nentries;
+ avail %= lc->lc_rxq->lq_nentries;
+ kn->kn_data = avail;
+ } else {
+ cbus_intr_setenabled(sc->sc_bustag, sc->sc_rx_ino,
+ INTR_ENABLED);
+ }
+ splx(s);
+
+ return (kn->kn_data > 0);
+}
+
+int
+filt_vldcwrite(struct knote *kn, long hint)
+{
+ struct vldcp_softc *sc = (void *)kn->kn_hook;
+ struct ldc_conn *lc = &sc->sc_lc;
+ uint64_t head, tail, avail, state;
+ int s, err;
+
+ s = spltty();
+ err = hv_ldc_tx_get_state(lc->lc_id, &head, &tail, &state);
+ if (err == 0 && state == LDC_CHANNEL_UP && head != tail) {
+ avail = (head - tail) / sizeof(struct ldc_pkt) +
+ lc->lc_txq->lq_nentries - 1;
+ avail %= lc->lc_txq->lq_nentries;
+ kn->kn_data = avail;
+ } else {
+ cbus_intr_setenabled(sc->sc_bust
Re: fix pppx(4) with net/ifq.c rev 1.38
On 22/05/20(Fri) 13:25, Vitaliy Makkoveev wrote:
> On Fri, May 22, 2020 at 07:57:13AM +1000, David Gwynne wrote:
> > [...]
> > can you try the following diff?
> >
>
> I tested this diff and it works for me. But the problem I pointed is
> about pipex(4) locking.
>
> pipex(4) requires NET_LOCK() be grabbed not only for underlaying
> ip{,6}_output() but for itself too. But since pppac_start() has
> unpredictable behavior I suggested to make it predictable [1].
What needs the NET_LOCK() in their? We're talking about
pipex_ppp_output(), right? Does it really need the NET_LOCK() or
the KERNEL_LOCK() is what protects those data structures?
> With net/ifq.c rev 1.37 and rev 1.39 pppx_if_start() routine will be
> allways called under NET_LOCK(), but pppac_start() will not. It depends
> on packet count stored in `if_snd' (look at net/ifq.c:125).
Ideally the *_start() routine should not require the NET_LOCK().
Ideally the pseudo-drivers should not require the NET_LOCK(). That's
what we should aim for.
In case of pipex(4) is isn't clear that the NET_LOCK() is necessary.
> With net/ifq.c rev 1.38 pppx_if_start() and pppac_start() have fully
> identical behavior and we can simply add NET_LOCK() [2] required by
> pipex(4).
>
> Also I can write my own handler for `ifp->if_enqueue' to be sure about
> `ifp->if_start' locking state, but I still have misunderstanding why
> pppx(4) does "IFQ_SET_MAXLEN(&ifp->if_snd, 1)". Just to make locking
> state of pppx_if_start() predictable or for avoid hidden bug triggering?
>
> Also this time I'am not shure about KERNEL_LOCK() locking while pipex(4)
> is accessed from the stack so I can't say is NET_LOCK() really required
> for the whole pipex(4).
>
> 1. https://marc.info/?l=openbsd-tech&m=159006455720473&w=2
> 2. https://marc.info/?l=openbsd-tech&m=158998597126002&w=2
>
Re: [PATCH] pipex(4): rework PPP input
Hello, On Wed, May 20, 2020 at 10:13 PM Vitaliy Makkoveev wrote: > On Wed, May 20, 2020 at 04:08:01AM +0300, Sergey Ryazanov wrote: > > On Tue, May 19, 2020 at 12:11 PM Vitaliy Makkoveev > > wrote: > > > On Mon, May 04, 2020 at 10:03:40PM +0300, Sergey Ryazanov wrote: > > > > Split checks from frame accepting with header removing in the common > > > > PPP input function. This should fix packet capture on a PPP interfaces. > > > > > > Can you describe the problem you fix? As mpi@ pointed to me, reviewers > > > are stupid and have no telepathy skills :) > > > > When I tried to capture packets on a ppp (4) interface (with pipex > > activated), I noticed that all the PPP CCP frames were ok, but all the > > ingress PPP IP frames were mangled, and they did not contain the PPP > > header at all. > > This time only pppx(4) and pppac(4) have pipex(4) support. Yes, and as I wrote in the first mail, now I am working on ppp(4) & pipex(4) integration to speed up client side of L2TP. > I don't see > packet capture problems on them. Can you catch and share how to > reproduce this problem with pppx(4) or pppac(4)? > > Also did you test your diff with pppx(4) and pppac(4)? I am entirely missed the fact that pppx(4) also have IFT_PPP type. Thank you for pointing me. I will recheck pppx(4) work and return as soon as I will have a better solution. -- Sergey
Retire
The header is not used any longer. Consequently, it should be safe to remove the following files: sys/arch/alpha/include/varargs.h sys/arch/hppa/include/varargs.h sys/arch/i386/include/varargs.h sys/arch/landisk/include/varargs.h sys/arch/loongson/include/varargs.h sys/arch/luna88k/include/varargs.h sys/arch/m88k/include/varargs.h sys/arch/macppc/include/varargs.h sys/arch/mips64/include/varargs.h sys/arch/octeon/include/varargs.h sys/arch/powerpc/include/varargs.h sys/arch/sgi/include/varargs.h sys/arch/sh/include/varargs.h sys/arch/sparc64/include/varargs.h OK to remove them? (As a leftover of gcc 2.95 times, the header should be ripe for removal as well. clang's copy of gives an #error, whereas attempts to use the header with gcc 4 fail because of missing symbol __builtin_varargs_start.)
Re: ldomctl: Fail on duplicate vcpu and memory parameters
On Mon, Jan 13, 2020 at 12:59:23PM +0100, Klemens Nanni wrote:
> 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?
This still is still in my tree.
It properly detects duplicate vcpu and memory lines for all domains
including the primary one; duplicate iodevice lines are only detected
for guest domains; preventing iodevice, vnet and vdisk lines in the
primary domain is stuff for a separate diff.
Feedback? OK?
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/ldomctl/parse.y,v
retrieving revision 1.18
diff -u -p -r1.18 parse.y
--- parse.y 21 Feb 2020 19:39:28 - 1.18
+++ parse.y 23 May 2020 09:23:58 -
@@ -166,10 +166,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 vdisk_opts {
@@ -192,10 +200,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);
- }
+ }
;
vdisk_opts : { vdisk_opts_default(); }
Re: userland clock_gettime proof of concept
> Discussions.
>
> - /sbin/init init_main.c!start_init() map page? (deraadt@)
> -> that is not the problem, the page should be mapped even there
>by the sys_execve() call
Robert found the proper solution to this: move the find_timekeep bits in
_libc_preinit!
This helps with a lot of things:
- removes the need for the find_timekeep() function
- removes the nasty ELF exports
- shrinks the diff
Good job, Robert!
What's left is the TSC discussion and the bikeshedding bits.
Paul
diff --git lib/libc/asr/asr.c lib/libc/asr/asr.c
index cd056c85719..2b25d49f32a 100644
--- lib/libc/asr/asr.c
+++ lib/libc/asr/asr.c
@@ -196,11 +196,11 @@ poll_intrsafe(struct pollfd *fds, nfds_t nfds, int
timeout)
struct timespec pollstart, pollend, elapsed;
int r;
- if (clock_gettime(CLOCK_MONOTONIC, &pollstart))
+ if (WRAP(clock_gettime)(CLOCK_MONOTONIC, &pollstart))
return -1;
while ((r = poll(fds, 1, timeout)) == -1 && errno == EINTR) {
- if (clock_gettime(CLOCK_MONOTONIC, &pollend))
+ if (WRAP(clock_gettime)(CLOCK_MONOTONIC, &pollend))
return -1;
timespecsub(&pollend, &pollstart, &elapsed);
timeout -= elapsed.tv_sec * 1000 + elapsed.tv_nsec / 100;
@@ -418,7 +418,7 @@ asr_check_reload(struct asr *asr)
asr->a_rtime = 0;
}
- if (clock_gettime(CLOCK_MONOTONIC, &ts) == -1)
+ if (WRAP(clock_gettime)(CLOCK_MONOTONIC, &ts) == -1)
return;
if ((ts.tv_sec - asr->a_rtime) < RELOAD_DELAY && asr->a_rtime != 0)
diff --git lib/libc/crypt/bcrypt.c lib/libc/crypt/bcrypt.c
index 82de8fa33b7..02fd3013cc1 100644
--- lib/libc/crypt/bcrypt.c
+++ lib/libc/crypt/bcrypt.c
@@ -248,9 +248,9 @@ _bcrypt_autorounds(void)
char buf[_PASSWORD_LEN];
int duration;
- clock_gettime(CLOCK_THREAD_CPUTIME_ID, &before);
+ WRAP(clock_gettime)(CLOCK_THREAD_CPUTIME_ID, &before);
bcrypt_newhash("testpassword", r, buf, sizeof(buf));
- clock_gettime(CLOCK_THREAD_CPUTIME_ID, &after);
+ WRAP(clock_gettime)(CLOCK_THREAD_CPUTIME_ID, &after);
duration = after.tv_sec - before.tv_sec;
duration *= 100;
diff --git lib/libc/dlfcn/init.c lib/libc/dlfcn/init.c
index 270f54aada5..70b70eb3ea0 100644
--- lib/libc/dlfcn/init.c
+++ lib/libc/dlfcn/init.c
@@ -30,6 +30,7 @@
#include
#include /* atexit */
#include
+#include /* timekeep */
#include
#include "init.h"
@@ -45,8 +46,9 @@
/* XXX should be in an include file shared with csu */
char ***_csu_finish(char **_argv, char **_envp, void (*_cleanup)(void));
-/* provide definition for this */
+/* provide definition for these */
int_pagesize = 0;
+void *_timekeep = NULL;
/*
* In dynamicly linked binaries environ and __progname are overriden by
@@ -105,6 +107,9 @@ _libc_preinit(int argc, char **argv, char **envp, dl_cb_cb
*cb)
phnum = aux->au_v;
break;
#endif /* !PIC */
+ case AUX_openbsd_timekeep:
+ _timekeep = (void *)aux->au_v;
+ break;
}
}
diff --git lib/libc/gen/times.c lib/libc/gen/times.c
index 02e4dd44b5c..36841810d1b 100644
--- lib/libc/gen/times.c
+++ lib/libc/gen/times.c
@@ -52,7 +52,7 @@ times(struct tms *tp)
return ((clock_t)-1);
tp->tms_cutime = CONVTCK(ru.ru_utime);
tp->tms_cstime = CONVTCK(ru.ru_stime);
- if (clock_gettime(CLOCK_MONOTONIC, &ts) == -1)
+ if (WRAP(clock_gettime)(CLOCK_MONOTONIC, &ts) == -1)
return ((clock_t)-1);
return (ts.tv_sec * CLK_TCK + ts.tv_nsec / (10 / CLK_TCK));
}
diff --git lib/libc/gen/timespec_get.c lib/libc/gen/timespec_get.c
index 520a5954025..845cbe80356 100644
--- lib/libc/gen/timespec_get.c
+++ lib/libc/gen/timespec_get.c
@@ -37,7 +37,7 @@ timespec_get(struct timespec *ts, int base)
{
switch (base) {
case TIME_UTC:
- if (clock_gettime(CLOCK_REALTIME, ts) == -1)
+ if (WRAP(clock_gettime)(CLOCK_REALTIME, ts) == -1)
return 0;
break;
default:
diff --git lib/libc/hidden/time.h lib/libc/hidden/time.h
index 18c49f8fcb9..1137dbcd44f 100644
--- lib/libc/hidden/time.h
+++ lib/libc/hidden/time.h
@@ -24,12 +24,16 @@
extern PROTO_NORMAL(tzname);
#endif
+__BEGIN_HIDDEN_DECLS
+extern void*_timekeep;
+__END_HIDDEN_DECLS
+
PROTO_NORMAL(asctime);
PROTO_NORMAL(asctime_r);
PROTO_STD_DEPRECATED(clock);
PROTO_DEPRECATED(clock_getcpuclockid);
PROTO_NORMAL(clock_getres);
-PROTO_NORMAL(clock_gettime);
+PROTO_WRAP(clock_gettime);
PROTO_NORMAL(clock_settime);
PROTO_STD_DEPRECATED(ctime);
PROTO_DEPRECATED(ctime_r);
diff --git lib/libc/net/res_random.c lib/libc/net/res_random.c
index 763e420bb88..9babb28470a 100644
--- lib/libc/net/res_random.c
+++ lib/libc/net
Re: ldomctl: Make init-sytem -n check vcpu and memory constraints
> From: "Theo de Raadt" > Date: Fri, 22 May 2020 17:57:37 -0600 > > There is an internal VCPU #define, but the keyword is vcpu, and there > appears to be nothing coming from the system which is an uppercase VCPU > > We don't have pfctl spitting out messages like: invalid RDOMAIN, because > rdomain is the keyword, there is no reason to arbitrarily change things > from LOWERCASE to UPPERCASE or even CamelCase. > > That's why I asked twice. The error message is not referring to the keyword. It is saying in plain english that the configuration you created isn't correct because it exceeds the available rsources. In plain english we spell CPU with capital letters becuse it is an abreviation. So VCPU gets spelled the same way. > > Index: config.c > === > RCS file: /cvs/src/usr.sbin/ldomctl/config.c,v > retrieving revision 1.38 > diff -u -p -u -r1.38 config.c > --- config.c 22 May 2020 21:54:20 - 1.38 > +++ config.c 22 May 2020 23:53:34 - > @@ -162,7 +162,7 @@ pri_link_core(struct md *md, struct md_n > > cpu = pri_find_cpu(pid); > if (cpu == NULL) > - errx(1, "couldn't determine core for VCPU > %lld\n", pid); > + errx(1, "couldn't determine core for vcpu > %lld\n", pid); > cpu->core = core; > } > } > @@ -2819,7 +2819,7 @@ build_config(const char *filename, int n > if (primary_memory == 0 && total_memory > memory) > primary_memory = total_memory - memory; > if (num_cpus > total_cpus || primary_num_cpus == 0) > - errx(1, "not enough VCPU resources available"); > + errx(1, "not enough vcpu resources available"); > if (memory > total_memory || primary_memory == 0) > errx(1, "not enough memory available"); > > >
ospfctl json support
Hi, I have attached the first iteration of the ospf json support. Sorry about the large commit, but it had to come in one go, if we didn't want a broken implementation. I will go back and update output.c and output_json.c to remove the detail flag and instead pass through the cli parse_result as is done in bgpctl -j sh rib, but thought it made sense to get the main commit out of the way and then circle back to these tweaks next week. In general I think there should be a consideration over how times are output. At present this diff outputs time using the pretty standard approach used in ospfctl all along, which is also what bgpctl -j sh rib does. I question whether for durations and uptime we should perhaps introduce something like uptime_seconds: 30 uptime_seconds: null as well as or rather than: uptime: "6d06h31m" uptime: "00:00:00" I'm not sure if the above is a standard format (6d06h31m) that can be parsed easily in many languages, which is part of what json support adds to the mix, if it is then it is the best of both worlds already! I can take a dig into this, but i'm sure someone will know off hand, if so what is the RFC or equiv? The only aspects I have not tested are: ospfctl -j show database summary ospfctl -j show database opaque These have no results in my configuration. I do question wheter ospfctl -j show database summary just needs removing? I will perhaps have a dig into ospfd and see what generates the relevant imsg. ospfctl_json.diff Description: Binary data
