Re: [PATCH] Pluggable disk formats for vmd (qcow2 preparation)
On Sun, Aug 19, 2018 at 11:46:12PM -0700, Ori Bernstein wrote:
> One more minor update to this patch:
>
> - Remove unused enum
> - Remove unused function prototype
> - Move some qcow2-specific headers into the qcow2 patch.
Ori, it's nice seeing good progress on this. I have a couple of
questions inline below.
+--+
Carlos
>
> +/*
> + * Initializes a raw disk image backing file from an fd.
> + * Stores the number of 512 byte sectors in *szp,
> + * returning -1 for error, 0 for success.
> + */
> +int
> +virtio_init_raw(struct virtio_backing *file, off_t *szp, int fd)
> +{
> + off_t sz;
> + int *fdp;
> +
> + sz = lseek(fd, 0, SEEK_END);
> + if (sz == -1)
> + return -1;
> +
> + fdp = malloc(sizeof(int));
> + *fdp = fd;
> + file->p = fdp;
> + file->pread = raw_pread;
> + file->pwrite = raw_pwrite;
> + file->close = raw_close;
> + *szp = sz / 512;
> + return 0;
> +}
Why are we making sz represent the number of 512 byte sectors? This may
be true for hd disks but it needs to be *bytes* for ISO/cdrom.
[snip]
> @@ -1944,12 +1957,12 @@ virtio_init(struct vmd_vm *vm, int child_cdrom, int
> *child_disks,
> + sizeof(uint16_t) * (2 + VIOSCSI_QUEUE_SIZE));
> vioscsi->vq[i].last_avail = 0;
> }
> - sz = lseek(child_cdrom, 0, SEEK_END);
> - vioscsi->fd = child_cdrom;
> + if (virtio_init_disk(&vioscsi->file, &vioscsi->sz,
> + child_cdrom) == -1)
> + return;
> vioscsi->locked = 0;
> vioscsi->lba = 0;
> - vioscsi->sz = sz;
> - vioscsi->n_blocks = sz >> 11; /* num of 2048 blocks in file */
> + vioscsi->n_blocks = vioscsi->sz >> 11; /* num of 2048 blocks in
> file */
This has to be the number of bytes shifted to get the number of 2K
blocks. As the patch is, this yields the incorrect number of blocks an
ISO has. This breaks ISO/cdrom support.
> vioscsi->max_xfer = VIOSCSI_BLOCK_SIZE_CDROM;
> vioscsi->pci_id = id;
> vioscsi->vm_id = vcp->vcp_id;
[snip]
> +struct virtio_backing {
> + void *p;
> + char *path;
> + ssize_t (*pread)(void *p, char *buf, size_t len, off_t off);
> + ssize_t (*pwrite)(void *p, char *buf, size_t len, off_t off);
> + void (*close)(void *p);
> +};
What is path used for? I don't see it used anywhere in this patch or in
the qcow2 patch.
Re: vmctl: add unveil
On Tue, Aug 21, 2018 at 09:51:52PM -0700, Carlos Cardenas wrote:
> Patch to unveil vmctl.
>
> Comments/OK?
>
> +--+
> Carlos
reads ok to me
> Index: main.c
> ===
> RCS file: /home/los/cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 main.c
> --- main.c12 Jul 2018 14:53:37 - 1.39
> +++ main.c18 Aug 2018 23:22:39 -
> @@ -160,7 +160,7 @@ parse(int argc, char *argv[])
>
> if (!ctl->has_pledge) {
> /* pledge(2) default if command doesn't have its own pledge */
> - if (pledge("stdio rpath exec unix getpw", NULL) == -1)
> + if (pledge("stdio rpath exec unix getpw unveil", NULL) == -1)
> err(1, "pledge");
> }
> if (ctl->main(&res, argc, argv) != 0)
> @@ -185,6 +185,8 @@ vmmaction(struct parse_result *res)
> unsigned int flags;
>
> if (ctl_sock == -1) {
> + if (unveil(SOCKET_NAME, "r") == -1)
> + err(1, "unveil");
> if ((ctl_sock = socket(AF_UNIX,
> SOCK_STREAM|SOCK_CLOEXEC, 0)) == -1)
> err(1, "socket");
> @@ -477,6 +479,10 @@ ctl_create(struct parse_result *res, int
>
> paths[0] = argv[1];
> paths[1] = NULL;
> +
> + if (unveil(paths[0], "rwc") == -1)
> + err(1, "unveil");
> +
> if (pledge("stdio rpath wpath cpath", NULL) == -1)
> err(1, "pledge");
> argc--;
> @@ -759,6 +765,8 @@ __dead void
> ctl_openconsole(const char *name)
> {
> closefrom(STDERR_FILENO + 1);
> + if (unveil(VMCTL_CU, "x") == -1)
> + err(1, "unveil");
> execl(VMCTL_CU, VMCTL_CU, "-l", name, "-s", "115200", (char *)NULL);
> err(1, "failed to open the console");
> }
vmctl: add unveil
Patch to unveil vmctl.
Comments/OK?
+--+
Carlos
Index: main.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmctl/main.c,v
retrieving revision 1.39
diff -u -p -r1.39 main.c
--- main.c 12 Jul 2018 14:53:37 - 1.39
+++ main.c 18 Aug 2018 23:22:39 -
@@ -160,7 +160,7 @@ parse(int argc, char *argv[])
if (!ctl->has_pledge) {
/* pledge(2) default if command doesn't have its own pledge */
- if (pledge("stdio rpath exec unix getpw", NULL) == -1)
+ if (pledge("stdio rpath exec unix getpw unveil", NULL) == -1)
err(1, "pledge");
}
if (ctl->main(&res, argc, argv) != 0)
@@ -185,6 +185,8 @@ vmmaction(struct parse_result *res)
unsigned int flags;
if (ctl_sock == -1) {
+ if (unveil(SOCKET_NAME, "r") == -1)
+ err(1, "unveil");
if ((ctl_sock = socket(AF_UNIX,
SOCK_STREAM|SOCK_CLOEXEC, 0)) == -1)
err(1, "socket");
@@ -477,6 +479,10 @@ ctl_create(struct parse_result *res, int
paths[0] = argv[1];
paths[1] = NULL;
+
+ if (unveil(paths[0], "rwc") == -1)
+ err(1, "unveil");
+
if (pledge("stdio rpath wpath cpath", NULL) == -1)
err(1, "pledge");
argc--;
@@ -759,6 +765,8 @@ __dead void
ctl_openconsole(const char *name)
{
closefrom(STDERR_FILENO + 1);
+ if (unveil(VMCTL_CU, "x") == -1)
+ err(1, "unveil");
execl(VMCTL_CU, VMCTL_CU, "-l", name, "-s", "115200", (char *)NULL);
err(1, "failed to open the console");
}
Re: ospfd: prevent additional ospfd from starting
On Tue, Aug 21, 2018 at 05:54:18PM +0100, Stuart Henderson wrote:
> On 2018/08/21 17:16, Remi Locherer wrote:
> > Hi tech,
> >
> > recently we had a short outage in our network. A script started an
> > additional
> > ospfd instance because the -n flag for config test was missing.
>
> This is a problem with bgpd as well, last time I did this it killed one of the
> *other* routers on the network (i.e. not just the one where I accidentally ran
> 2x bgpd...).
>
> > What then happend was not nice:
> > - The new ospfd unlinked the control socket of the first ospfd
> > - The new ospfd removed all routes from the first ospfd
> > - The new ospfd was not able to build up an adjacency and therefore could
> > not install the routes needed for a recovery.
> > - Both ospfd instances were running but non-functional.
> >
> > Of course the faulty script is fixed by now. ;-)
> >
> > It would be nice if ospfd could prevent such a situation.
> >
> > Below diff does these things:
> > - Detect a running ospfd by first doing a connect on the control socket.
> > - Do not delete the control socket on exit.
> > - This could delete the socket of another instance.
> > - Unlinking the socket on shutdown will be in the way once we add pledge
> > to the main process. It was removed recently from various daemons.
>
> This all sounds very sensible.
>
> > - Do not delete routes added by another process even if they have
> > prio RTP_OSPF. Without this the new ospfd will remove all the routes
> > of the first one.
>
> I'm unsure about this, the above changes stop the new ospfd from running
> don't they, so that shouldn't be a problem?
It stops to late. kr_init happens before and kill all existing routes with
priority 32. And again in the shutdown function of ospfd.
>
> If an ospfd blows up for whatever reason, it would be quite inconvenient
> if it needs manual route tweaks rather than just 'rcctl start ospfd' to fix
> it ..
Yes, this is not optimal.
The new diff below defers kr_init until the ospf engine notifies the parent
that the control socket is ready. In case the ospf engine exits because the
control socket is already in use no routes are known that could be removed.
With this ospfd keeps the behaviour of removing foreign routes with
priority 32.
Better?
Index: control.c
===
RCS file: /cvs/src/usr.sbin/ospfd/control.c,v
retrieving revision 1.44
diff -u -p -r1.44 control.c
--- control.c 24 Jan 2017 04:24:25 - 1.44
+++ control.c 17 Aug 2018 22:41:43 -
@@ -42,19 +42,29 @@ int
control_init(char *path)
{
struct sockaddr_un sun;
- int fd;
+ int fd, fd_check;
mode_t old_umask;
+ bzero(&sun, sizeof(sun));
+ sun.sun_family = AF_UNIX;
+ strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
+
+ if ((fd_check = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+ log_warn("control_init: socket check");
+ return (-1);
+ }
+ if (connect(fd_check, (struct sockaddr *)&sun, sizeof(sun)) == 0) {
+ log_warnx("control_init: socket in use");
+ return (-1);
+ }
+ close(fd_check);
+
if ((fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
0)) == -1) {
log_warn("control_init: socket");
return (-1);
}
- bzero(&sun, sizeof(sun));
- sun.sun_family = AF_UNIX;
- strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
-
if (unlink(path) == -1)
if (errno != ENOENT) {
log_warn("control_init: unlink %s", path);
@@ -98,16 +108,6 @@ control_listen(void)
evtimer_set(&control_state.evt, control_accept, NULL);
return (0);
-}
-
-void
-control_cleanup(char *path)
-{
- if (path == NULL)
- return;
- event_del(&control_state.ev);
- event_del(&control_state.evt);
- unlink(path);
}
/* ARGSUSED */
Index: control.h
===
RCS file: /cvs/src/usr.sbin/ospfd/control.h,v
retrieving revision 1.6
diff -u -p -r1.6 control.h
--- control.h 10 Feb 2015 05:24:48 - 1.6
+++ control.h 17 Aug 2018 21:02:36 -
@@ -39,6 +39,5 @@ int control_listen(void);
void control_accept(int, short, void *);
void control_dispatch_imsg(int, short, void *);
intcontrol_imsg_relay(struct imsg *);
-void control_cleanup(char *);
#endif /* _CONTROL_H_ */
Index: ospfd.c
===
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
retrieving revision 1.99
diff -u -p -r1.99 ospfd.c
--- ospfd.c 11 Jul 2018 12:09:34 - 1.99
+++ ospfd.c 21 Aug 2018 21:39:23 -
@@ -270,10 +270,6 @@ main(int argc, char *argv[])
iev_rde->handler, iev_rde);
event_add(&iev
Allocate PCI BAR for radeondrm(4)
If firmware doesn't initialize the device, the BAR will be zero and
mapping that address leads to "interesting" issues on many machines.
This BAR is a bit special since we don't want to immediately map it.
So we have to explicitly allocate the address space for the BAR. Bail
out if we fail. Don't do this on sparc64 because there the address
can actually be zero.
ok?
Index: dev/pci/drm/radeon/radeon_kms.c
===
RCS file: /cvs/src/sys/dev/pci/drm/radeon/radeon_kms.c,v
retrieving revision 1.56
diff -u -p -r1.56 radeon_kms.c
--- dev/pci/drm/radeon/radeon_kms.c 3 May 2018 10:09:51 - 1.56
+++ dev/pci/drm/radeon/radeon_kms.c 21 Aug 2018 21:30:45 -
@@ -485,6 +485,26 @@ radeondrm_attach_kms(struct device *pare
printf(": can't get frambuffer info\n");
return;
}
+#if !defined(__sparc64__)
+ if (rdev->fb_aper_offset == 0) {
+ bus_size_t start, end;
+ bus_addr_t base;
+
+ start = max(PCI_MEM_START, pa->pa_memex->ex_start);
+ end = min(PCI_MEM_END, pa->pa_memex->ex_end);
+ if (pa->pa_memex == NULL ||
+ extent_alloc_subregion(pa->pa_memex, start, end,
+ rdev->fb_aper_size, rdev->fb_aper_size, 0, 0, 0, &base)) {
+ printf(": can't reserve framebuffer space\n");
+ return;
+ }
+ pci_conf_write(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM, base);
+ if (PCI_MAPREG_MEM_TYPE(type) == PCI_MAPREG_MEM_TYPE_64BIT)
+ pci_conf_write(pa->pa_pc, pa->pa_tag,
+ RADEON_PCI_MEM + 4, (uint64_t)base >> 32);
+ rdev->fb_aper_offset = base;
+ }
+#endif
for (i = PCI_MAPREG_START; i < PCI_MAPREG_END ; i+= 4) {
type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, i);
Re: ospfd: prevent additional ospfd from starting
On Tue, Aug 21, 2018 at 05:16:47PM +0200, Remi Locherer wrote: > Hi tech, > > recently we had a short outage in our network. A script started an additional > ospfd instance because the -n flag for config test was missing. > > What then happend was not nice: > - The new ospfd unlinked the control socket of the first ospfd > - The new ospfd removed all routes from the first ospfd > - The new ospfd was not able to build up an adjacency and therefore could > not install the routes needed for a recovery. > - Both ospfd instances were running but non-functional. > > Of course the faulty script is fixed by now. ;-) > > It would be nice if ospfd could prevent such a situation. > > Below diff does these things: > - Detect a running ospfd by first doing a connect on the control socket. > - Do not delete the control socket on exit. > - This could delete the socket of another instance. > - Unlinking the socket on shutdown will be in the way once we add pledge > to the main process. It was removed recently from various daemons. > - Do not delete routes added by another process even if they have > prio RTP_OSPF. Without this the new ospfd will remove all the routes > of the first one. > > A side effect of this is that alien OSPF routes are now only logged but > not removed anymore. Should a crashed ospfd leave some routes behind the > next ospfd does not clean them up anymore. The admin would need to check > the logs and remove them manually with the route command. > > Does this make sense? > Manually removing routes does not :)
Re: rasops(9) cursor drawing code
> Date: Mon, 20 Aug 2018 20:55:16 +0200 (CEST)
> From: Mark Kettenis
>
> > Date: Mon, 20 Aug 2018 15:46:57 +0200
> > From: Frederic Cambus
> >
> > There is an issue when moving the cursor backwards with the left arrow
> > key, characters disappear and only reappear when moving back forward
> > with the right arrow key.
> >
> > I also noticed some similar artefacts when using interactive programs
> > with wsmoused enabled.
>
> Ah, I didn't take the scrollback offset into account. Here is a
> better diff.
As fcambus@ noticed, I attached the old diff again. Here is the right
one.
Index: dev/rasops/rasops.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops.c,v
retrieving revision 1.54
diff -u -p -r1.54 rasops.c
--- dev/rasops/rasops.c 3 May 2018 10:05:47 - 1.54
+++ dev/rasops/rasops.c 21 Aug 2018 18:04:48 -
@@ -130,6 +130,22 @@ const u_char rasops_isgray[16] = {
0, 0, 0, 1
};
+struct rasops_screen {
+ LIST_ENTRY(rasops_screen) rs_next;
+ struct rasops_info *rs_ri;
+
+ struct wsdisplay_charcell *rs_bs;
+ int rs_visible;
+ int rs_crow;
+ int rs_ccol;
+ long rs_defattr;
+
+ int rs_sbscreens;
+#define RS_SCROLLBACK_SCREENS 5
+ int rs_dispoffset; /* rs_bs index, start of our actual screen */
+ int rs_visibleoffset; /* rs_bs index, current scrollback screen */
+};
+
/* Generic functions */
intrasops_copycols(void *, int, int, int, int);
intrasops_copyrows(void *, int, int, int);
@@ -179,6 +195,7 @@ int rasops_wronly_copycols(void *, int,
intrasops_wronly_erasecols(void *, int, int, int, long);
intrasops_wronly_copyrows(void *, int, int, int);
intrasops_wronly_eraserows(void *, int, int, long);
+intrasops_wronly_do_cursor(struct rasops_info *);
intrasops_add_font(struct rasops_info *, struct wsdisplay_font *);
intrasops_use_font(struct rasops_info *, struct wsdisplay_font *);
@@ -268,6 +285,8 @@ rasops_init(struct rasops_info *ri, int
return (-1);
ri->ri_active = cookie;
+ ri->ri_bs =
+ &ri->ri_active->rs_bs[ri->ri_active->rs_visibleoffset];
ri->ri_ops.cursor = rasops_vcons_cursor;
ri->ri_ops.mapchar = rasops_vcons_mapchar;
@@ -278,6 +297,7 @@ rasops_init(struct rasops_info *ri, int
ri->ri_ops.eraserows = rasops_vcons_eraserows;
ri->ri_ops.alloc_attr = rasops_vcons_alloc_attr;
ri->ri_ops.unpack_attr = rasops_vcons_unpack_attr;
+ ri->ri_do_cursor = rasops_wronly_do_cursor;
} else if ((ri->ri_flg & RI_WRONLY) && ri->ri_bs != NULL) {
long attr;
int i;
@@ -287,6 +307,7 @@ rasops_init(struct rasops_info *ri, int
ri->ri_ops.erasecols = rasops_wronly_erasecols;
ri->ri_ops.copyrows = rasops_wronly_copyrows;
ri->ri_ops.eraserows = rasops_wronly_eraserows;
+ ri->ri_do_cursor = rasops_wronly_do_cursor;
ri->ri_alloc_attr(ri, 0, 0, 0, &attr);
for (i = 0; i < ri->ri_rows * ri->ri_cols; i++) {
@@ -1365,22 +1386,6 @@ slow_bcopy(void *s, void *d, size_t len)
}
#endif /* NRASOPS_BSWAP */
-struct rasops_screen {
- LIST_ENTRY(rasops_screen) rs_next;
- struct rasops_info *rs_ri;
-
- struct wsdisplay_charcell *rs_bs;
- int rs_visible;
- int rs_crow;
- int rs_ccol;
- long rs_defattr;
-
- int rs_sbscreens;
-#define RS_SCROLLBACK_SCREENS 5
- int rs_dispoffset; /* rs_bs index, start of our actual screen */
- int rs_visibleoffset; /* rs_bs index, current scrollback screen */
-};
-
int
rasops_alloc_screen(void *v, void **cookiep,
int *curxp, int *curyp, long *attrp)
@@ -1482,6 +1487,7 @@ rasops_doswitch(void *v)
ri->ri_active->rs_visible = 0;
ri->ri_eraserows(ri, 0, ri->ri_rows, scr->rs_defattr);
ri->ri_active = scr;
+ ri->ri_bs = &ri->ri_active->rs_bs[ri->ri_active->rs_dispoffset];
ri->ri_active->rs_visible = 1;
ri->ri_active->rs_visibleoffset = ri->ri_active->rs_dispoffset;
for (row = 0; row < ri->ri_rows; row++) {
@@ -1767,6 +1773,27 @@ rasops_wronly_eraserows(void *cookie, in
}
return ri->ri_eraserows(ri, row, num, attr);
+}
+
+int
+rasops_wronly_do_cursor(struct rasops_info *ri)
+{
+ int off = ri->ri_crow * ri->ri_cols + ri->ri_ccol;
+ u_int uc;
+ long attr;
+ int fg, bg;
+
+ uc = ri->ri_bs[off].uc;
+ attr = ri->ri_bs[off].attr;
+
+ if ((ri->ri_flg & RI_CURSOR) == 0) {
+ fg = ((u_int)attr >> 24) & 0xf;
+ bg = ((u_int)attr >> 16) & 0xf;
+ attr &= ~0x0;
+ attr |= (fg << 16) | (bg << 24);
+ }
+
+ return ri->ri_putchar(ri, ri->ri_crow, ri->ri_ccol, uc, attr);
}
/*
Re: ospfd: prevent additional ospfd from starting
On 2018/08/21 17:16, Remi Locherer wrote:
> Hi tech,
>
> recently we had a short outage in our network. A script started an additional
> ospfd instance because the -n flag for config test was missing.
This is a problem with bgpd as well, last time I did this it killed one of the
*other* routers on the network (i.e. not just the one where I accidentally ran
2x bgpd...).
> What then happend was not nice:
> - The new ospfd unlinked the control socket of the first ospfd
> - The new ospfd removed all routes from the first ospfd
> - The new ospfd was not able to build up an adjacency and therefore could
> not install the routes needed for a recovery.
> - Both ospfd instances were running but non-functional.
>
> Of course the faulty script is fixed by now. ;-)
>
> It would be nice if ospfd could prevent such a situation.
>
> Below diff does these things:
> - Detect a running ospfd by first doing a connect on the control socket.
> - Do not delete the control socket on exit.
> - This could delete the socket of another instance.
> - Unlinking the socket on shutdown will be in the way once we add pledge
> to the main process. It was removed recently from various daemons.
This all sounds very sensible.
> - Do not delete routes added by another process even if they have
> prio RTP_OSPF. Without this the new ospfd will remove all the routes
> of the first one.
I'm unsure about this, the above changes stop the new ospfd from running
don't they, so that shouldn't be a problem?
If an ospfd blows up for whatever reason, it would be quite inconvenient
if it needs manual route tweaks rather than just 'rcctl start ospfd' to fix it
..
> A side effect of this is that alien OSPF routes are now only logged but
> not removed anymore. Should a crashed ospfd leave some routes behind the
> next ospfd does not clean them up anymore. The admin would need to check
> the logs and remove them manually with the route command.
>
> Does this make sense?
>
> Comments? OK?
>
> Remi
>
>
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/control.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 control.c
> --- control.c 24 Jan 2017 04:24:25 - 1.44
> +++ control.c 17 Aug 2018 22:41:43 -
> @@ -42,19 +42,29 @@ int
> control_init(char *path)
> {
> struct sockaddr_un sun;
> - int fd;
> + int fd, fd_check;
> mode_t old_umask;
>
> + bzero(&sun, sizeof(sun));
> + sun.sun_family = AF_UNIX;
> + strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
> +
> + if ((fd_check = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
> + log_warn("control_init: socket check");
> + return (-1);
> + }
> + if (connect(fd_check, (struct sockaddr *)&sun, sizeof(sun)) == 0) {
> + log_warnx("control_init: socket in use");
> + return (-1);
> + }
> + close(fd_check);
> +
> if ((fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
> 0)) == -1) {
> log_warn("control_init: socket");
> return (-1);
> }
>
> - bzero(&sun, sizeof(sun));
> - sun.sun_family = AF_UNIX;
> - strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
> -
> if (unlink(path) == -1)
> if (errno != ENOENT) {
> log_warn("control_init: unlink %s", path);
> @@ -98,16 +108,6 @@ control_listen(void)
> evtimer_set(&control_state.evt, control_accept, NULL);
>
> return (0);
> -}
> -
> -void
> -control_cleanup(char *path)
> -{
> - if (path == NULL)
> - return;
> - event_del(&control_state.ev);
> - event_del(&control_state.evt);
> - unlink(path);
> }
>
> /* ARGSUSED */
> Index: control.h
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/control.h,v
> retrieving revision 1.6
> diff -u -p -r1.6 control.h
> --- control.h 10 Feb 2015 05:24:48 - 1.6
> +++ control.h 17 Aug 2018 21:02:36 -
> @@ -39,6 +39,5 @@ int control_listen(void);
> void control_accept(int, short, void *);
> void control_dispatch_imsg(int, short, void *);
> int control_imsg_relay(struct imsg *);
> -void control_cleanup(char *);
>
> #endif /* _CONTROL_H_ */
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v
> retrieving revision 1.111
> diff -u -p -r1.111 kroute.c
> --- kroute.c 10 Jul 2018 11:49:04 - 1.111
> +++ kroute.c 21 Aug 2018 14:13:27 -
> @@ -263,6 +263,7 @@ kr_change_fib(struct kroute_node *kr, st
> kn->r.nexthop.s_addr = kroute[i].nexthop.s_addr;
> kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED;
> kn->r.priority = RTP_OSPF;
> + kn->r.pid = kr_state.pid;
> kn->r.ext_tag = kroute
openssl s_time: use monotonic timer for test timeout
The timeout should be based on a monotonic clock.
The wrapper function for the app_timer_* interfaces obfuscates
things, so delete it.
totalTimer should be set, not added to: the code doesn't
increment it every iteration.
ok?
P.S. huh, there's an FPE if nConn == 0.
Index: s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.28
diff -u -p -r1.28 s_time.c
--- s_time.c21 Aug 2018 15:56:39 - 1.28
+++ s_time.c21 Aug 2018 16:22:52 -
@@ -227,18 +227,6 @@ s_time_usage(void)
}
/***
- * TIME - time functions
- */
-#define START TM_RESET
-#define STOP TM_GET
-
-static double
-tm_Time_F(int op)
-{
- return app_timer_user(op);
-}
-
-/***
* MAIN - main processing area for client
* real name depends on MONOLITH
*/
@@ -407,10 +395,9 @@ run_test(SSL *scon)
static int
benchmark(int reuse_session)
{
- double totalTime = 0.0;
+ double elapsed, totalTime;
int nConn = 0;
SSL *scon = NULL;
- time_t finishtime;
int ret = 1;
int ver;
@@ -426,15 +413,13 @@ benchmark(int reuse_session)
}
nConn = 0;
- totalTime = 0.0;
-
- finishtime = time(NULL) + s_time_config.maxtime;
-
bytes_read = 0;
- tm_Time_F(START);
+ app_timer_real(TM_RESET);
+ app_timer_user(TM_RESET);
for (;;) {
- if (finishtime < time(NULL))
+ elapsed = app_timer_real(TM_GET);
+ if (elapsed > s_time_config.maxtime)
break;
if (scon == NULL) {
if ((scon = SSL_new(tm_ctx)) == NULL)
@@ -464,13 +449,13 @@ benchmark(int reuse_session)
scon = NULL;
}
}
- totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
+ totalTime = app_timer_user(TM_GET);
printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes
read %ld\n",
nConn, totalTime, ((double) nConn / totalTime), bytes_read);
- printf("%d connections in %lld real seconds, %ld bytes read per
connection\n",
+ printf("%d connections in %.0f real seconds, %ld bytes read per
connection\n",
nConn,
- (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+ elapsed,
bytes_read / nConn);
ret = 0;
Re: [patch] uvm_mmap / little change proposal
I knew it was not documented for a reason but why is it ? Just curious ... but it s ok if we drop this change ... On Tue, 21 Aug 2018 at 16:38, Mark Kettenis wrote: > > > From: "Todd C. Miller" > > Date: Tue, 21 Aug 2018 09:19:05 -0600 > > > > On Tue, 21 Aug 2018 08:57:06 +0100, David CARLIER wrote: > > > > > For portability sake and to make it more "visible" I changed the > > > __MAP_NOREPLACE to MAP_EXCL, documented it in the man page and also > > > changed the errno to EEXIST instead (I thought it would reflect > > > better the error in case the addressed mapped already exists but > > > can be changed back in case ...). > > > > Does anyone other than FreeBSD define MAP_EXCL? A quick google > > search didn't turn up any other systems with it. > > The symbol starts with a double underscore for a reason. We really > didn't want this to be used outside of the very specific code in base > that needs this.
Re: [patch] uvm_mmap / little change proposal
> From: "Todd C. Miller" > Date: Tue, 21 Aug 2018 09:19:05 -0600 > > On Tue, 21 Aug 2018 08:57:06 +0100, David CARLIER wrote: > > > For portability sake and to make it more "visible" I changed the > > __MAP_NOREPLACE to MAP_EXCL, documented it in the man page and also > > changed the errno to EEXIST instead (I thought it would reflect > > better the error in case the addressed mapped already exists but > > can be changed back in case ...). > > Does anyone other than FreeBSD define MAP_EXCL? A quick google > search didn't turn up any other systems with it. The symbol starts with a double underscore for a reason. We really didn't want this to be used outside of the very specific code in base that needs this.
Re: [patch] uvm_mmap / little change proposal
No one else .. Linux has MAP_FIXED_NOREPLACE which is MAP_FIXED|MAP_EXCL equivalent... On Tue, 21 Aug 2018 at 16:19, Todd C. Miller wrote: > > On Tue, 21 Aug 2018 08:57:06 +0100, David CARLIER wrote: > > > For portability sake and to make it more "visible" I changed the > > __MAP_NOREPLACE to MAP_EXCL, documented it in the man page and also > > changed the errno to EEXIST instead (I thought it would reflect > > better the error in case the addressed mapped already exists but > > can be changed back in case ...). > > Does anyone other than FreeBSD define MAP_EXCL? A quick google > search didn't turn up any other systems with it. > > - todd
Re: [patch] uvm_mmap / little change proposal
On Tue, 21 Aug 2018 08:57:06 +0100, David CARLIER wrote: > For portability sake and to make it more "visible" I changed the > __MAP_NOREPLACE to MAP_EXCL, documented it in the man page and also > changed the errno to EEXIST instead (I thought it would reflect > better the error in case the addressed mapped already exists but > can be changed back in case ...). Does anyone other than FreeBSD define MAP_EXCL? A quick google search didn't turn up any other systems with it. - todd
ospfd: prevent additional ospfd from starting
Hi tech,
recently we had a short outage in our network. A script started an additional
ospfd instance because the -n flag for config test was missing.
What then happend was not nice:
- The new ospfd unlinked the control socket of the first ospfd
- The new ospfd removed all routes from the first ospfd
- The new ospfd was not able to build up an adjacency and therefore could
not install the routes needed for a recovery.
- Both ospfd instances were running but non-functional.
Of course the faulty script is fixed by now. ;-)
It would be nice if ospfd could prevent such a situation.
Below diff does these things:
- Detect a running ospfd by first doing a connect on the control socket.
- Do not delete the control socket on exit.
- This could delete the socket of another instance.
- Unlinking the socket on shutdown will be in the way once we add pledge
to the main process. It was removed recently from various daemons.
- Do not delete routes added by another process even if they have
prio RTP_OSPF. Without this the new ospfd will remove all the routes
of the first one.
A side effect of this is that alien OSPF routes are now only logged but
not removed anymore. Should a crashed ospfd leave some routes behind the
next ospfd does not clean them up anymore. The admin would need to check
the logs and remove them manually with the route command.
Does this make sense?
Comments? OK?
Remi
Index: control.c
===
RCS file: /cvs/src/usr.sbin/ospfd/control.c,v
retrieving revision 1.44
diff -u -p -r1.44 control.c
--- control.c 24 Jan 2017 04:24:25 - 1.44
+++ control.c 17 Aug 2018 22:41:43 -
@@ -42,19 +42,29 @@ int
control_init(char *path)
{
struct sockaddr_un sun;
- int fd;
+ int fd, fd_check;
mode_t old_umask;
+ bzero(&sun, sizeof(sun));
+ sun.sun_family = AF_UNIX;
+ strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
+
+ if ((fd_check = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+ log_warn("control_init: socket check");
+ return (-1);
+ }
+ if (connect(fd_check, (struct sockaddr *)&sun, sizeof(sun)) == 0) {
+ log_warnx("control_init: socket in use");
+ return (-1);
+ }
+ close(fd_check);
+
if ((fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
0)) == -1) {
log_warn("control_init: socket");
return (-1);
}
- bzero(&sun, sizeof(sun));
- sun.sun_family = AF_UNIX;
- strlcpy(sun.sun_path, path, sizeof(sun.sun_path));
-
if (unlink(path) == -1)
if (errno != ENOENT) {
log_warn("control_init: unlink %s", path);
@@ -98,16 +108,6 @@ control_listen(void)
evtimer_set(&control_state.evt, control_accept, NULL);
return (0);
-}
-
-void
-control_cleanup(char *path)
-{
- if (path == NULL)
- return;
- event_del(&control_state.ev);
- event_del(&control_state.evt);
- unlink(path);
}
/* ARGSUSED */
Index: control.h
===
RCS file: /cvs/src/usr.sbin/ospfd/control.h,v
retrieving revision 1.6
diff -u -p -r1.6 control.h
--- control.h 10 Feb 2015 05:24:48 - 1.6
+++ control.h 17 Aug 2018 21:02:36 -
@@ -39,6 +39,5 @@ int control_listen(void);
void control_accept(int, short, void *);
void control_dispatch_imsg(int, short, void *);
intcontrol_imsg_relay(struct imsg *);
-void control_cleanup(char *);
#endif /* _CONTROL_H_ */
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v
retrieving revision 1.111
diff -u -p -r1.111 kroute.c
--- kroute.c10 Jul 2018 11:49:04 - 1.111
+++ kroute.c21 Aug 2018 14:13:27 -
@@ -263,6 +263,7 @@ kr_change_fib(struct kroute_node *kr, st
kn->r.nexthop.s_addr = kroute[i].nexthop.s_addr;
kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED;
kn->r.priority = RTP_OSPF;
+ kn->r.pid = kr_state.pid;
kn->r.ext_tag = kroute[i].ext_tag;
rtlabel_unref(kn->r.rtlabel); /* for RTM_CHANGE */
kn->r.rtlabel = kroute[i].rtlabel;
@@ -365,7 +366,7 @@ kr_fib_decouple(void)
return;
RB_FOREACH(kr, kroute_tree, &krt)
- if (kr->r.priority == RTP_OSPF)
+ if (kr->r.priority == RTP_OSPF && kr->r.pid == kr_state.pid)
for (kn = kr; kn != NULL; kn = kn->next)
send_rtmsg(kr_state.fd, RTM_DELETE, &kn->r);
@@ -1365,7 +1366,7 @@ rtmsg_process(char *buf, size_t len)
u_int8_t prefixlen, prio;
int flags, mpath;
Re: IP_SENDSRCADDR cmsg_len and dnsmasq
On Mon, Jul 16, 2018 at 11:11:58AM +0200, Vincent Gross wrote: > I have a regression test for this based on Alexander Markert code + > rework by mpi@, do you want me to commit it right now ? Finally I have commited the fix. Could you take care of the regression test? bluhm
[patch] uvm_mmap / little change proposal
Hi, For portability sake and to make it more "visible" I changed the __MAP_NOREPLACE to MAP_EXCL, documented it in the man page and also changed the errno to EEXIST instead (I thought it would reflect better the error in case the addressed mapped already exists but can be changed back in case ...). Thanks. Regards. uvm_mmap.diff Description: Binary data
