vmd(8): teach it to exec vm's after fork
tech@,
vmd currently uses a fork-only approach to spawning new vm's, resulting
in each vm having the same address space and inheriting some global
state from the vmm process (like the list of running vm's). This diff
introduces an execvp(2) call after fork, making the vm process re-exec.
Things to note:
1. the vmm process switches to unveil(2) instead of chroot(2) as it now
needs access to the vmd binary for exec. As a result, I adapted the
design from sshd(8) requiring an an absolute path for execution. You
will receive a "vmd: re-exec requires execution with an absolute
path" warning and vmd will terminate if not launched with an absolute
path to vmd.
2. vm process similarly adopts unveil(2) as it is now re-exec'ing and
not root, so it cannot call chroot(2).
3. vmm process adopts "exec" pledge(2) promise.
If testing the diff and you want to use send/receive, you'll also need
the diff [1] I previously shared on tech@.
I'd prefer some testing before asking for commit as I'd like to land [1]
first before this change.
-dv
[1] https://marc.info/?l=openbsd-tech=168166549712160=mbox
diffstat refs/heads/master refs/heads/vmd-fork+exec
M usr.sbin/vmd/vm.c | 139+ 82-
M usr.sbin/vmd/vmd.c | 28+ 5-
M usr.sbin/vmd/vmd.h |5+ 0-
M usr.sbin/vmd/vmm.c | 100+ 12-
4 files changed, 272 insertions(+), 99 deletions(-)
diff refs/heads/master refs/heads/vmd-fork+exec
commit - 291811d84b76a57a1ac3f9885ec1482a600a81cd
commit + 89b4ed10ffc7ed48c6f1334497272e071f7c7a88
blob - 5b9a1831f5b5b896589f7e8c79309d370ace400d
blob + 50d59fbcfb8a7c63dfb8c3f9316b7cc52e8be514
--- usr.sbin/vmd/vm.c
+++ usr.sbin/vmd/vm.c
@@ -74,8 +74,7 @@ int run_vm(int, int[][VM_MAX_BASE_PER_DISK], int *,
io_fn_t ioports_map[MAX_PORTS];
-int run_vm(int, int[][VM_MAX_BASE_PER_DISK], int *,
-struct vmop_create_params *, struct vcpu_reg_state *);
+static int run_vm(struct vmop_create_params *, struct vcpu_reg_state *);
void vm_dispatch_vmm(int, short, void *);
void *event_thread(void *);
void *vcpu_run_loop(void *);
@@ -214,6 +213,72 @@ static const struct vcpu_reg_state vcpu_init_flat16 =
};
/*
+ * vm_main
+ *
+ * Primary entrypoint for launching a vm. Does not return.
+ *
+ * fd: file descriptor for communicating with vmm process.
+ */
+void
+vm_main(int fd)
+{
+ struct vm_create_params *vcp = NULL;
+ struct vmd_vmvm;
+ size_t sz = 0;
+ int ret = 0;
+
+ /*
+* We aren't root, so we can't chroot(2). Use unveil(2) instead.
+*/
+ if (unveil("/var/empty", "") == -1)
+ fatal("unveil /var/empty");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil lock");
+
+ /*
+* pledge in the vm processes:
+* stdio - for malloc and basic I/O including events.
+* vmm - for the vmm ioctls and operations.
+* recvfd - for vm send/recv and sending fd to devices.
+* proc - required for vmm(4) VMM_IOC_CREATE ioctl
+*/
+ if (pledge("stdio vmm recvfd proc", NULL) == -1)
+ fatal("pledge");
+
+ /* Receive our vm configuration. */
+ memset(, 0, sizeof(vm));
+ sz = atomicio(read, fd, , sizeof(vm));
+ if (sz != sizeof(vm)) {
+ log_warnx("failed to receive start message");
+ _exit(EIO);
+ }
+
+ /* Receive the /dev/vmm fd number. */
+ sz = atomicio(read, fd, >vmd_fd, sizeof(env->vmd_fd));
+ if (sz != sizeof(env->vmd_fd)) {
+ log_warnx("failed to receive /dev/vmm fd");
+ _exit(EIO);
+ }
+
+ /* Update process with the vm name. */
+ vcp = _params.vmc_params;
+ setproctitle("%s", vcp->vcp_name);
+ log_procinit(vcp->vcp_name);
+
+ /*
+* We need, at minimum, a vm_kernel fd to boot a vm. This is either a
+* kernel or a BIOS image.
+*/
+ if (vm.vm_kernel < 0 && !(vm.vm_state & VM_STATE_RECEIVED)) {
+ log_warnx("%s: failed to receive boot fd", vcp->vcp_name);
+ _exit(EINVAL);
+ }
+
+ ret = start_vm(, fd);
+ _exit(ret);
+}
+
+/*
* loadfile_bios
*
* Alternatively to loadfile_elf, this function loads a non-ELF BIOS image
@@ -300,15 +365,14 @@ start_vm(struct vmd_vm *vm, int fd)
struct vm_rwregs_params vrp;
struct stat sb;
- /* Child */
- setproctitle("%s", vcp->vcp_name);
- log_procinit(vcp->vcp_name);
-
+ /*
+* We first try to initialize and allocate memory before bothering
+* vmm(4) with a request to create a new vm.
+*/
if (!(vm->vm_state & VM_STATE_RECEIVED))
create_memory_map(vcp);
- ret = alloc_guest_mem(vcp);
-
+ ret = alloc_guest_mem(>vm_params.vmc_params);
if (ret) {
struct rlimit lim;
char buf[FMT_SCALED_STRSIZE];
@@ -318,31 +382,44 @@ start_vm(struct
Re: bgpd: reverse output of flowspec_cmp
On Mon, Apr 17, 2023 at 06:56:53PM +0200, Claudio Jeker wrote: > I noticed that the order generated in an RB tree using flowspec_cmp() is > reversed. The problem is that for addresses preferred means smaller. > I think it is best to change the flowspec_cmp function to sort data so > that RB_FOREACH will print them most-preferred to least-preferred. > > I had not caught this oversight until I started to really test all the > bits an pieces. Oh yes. ok
mg: allow to change the tab width
This makes the tab width customizable per-buffer. There's a new
function called `set-tab-width' that changes the tab width for the
current buffer or the default one for new buffers if called with a
prefix argument (or from the startup file.)
The default tab width is still 8 column if not changed by the user,
but together with the newly resurrected no-tab-mode this allows mg to
be used for a variety of programming languages and coding styles.
Want to hack on lua? easy:
$ cat <> ~/.mg
set-tab-width 3
auto-execute *.lua no-tab-mode
EOF
You might be tempted to say
auto-execute *.lua set-tab-width 3 # wrong!
but it's currently a syntax error since auto-execute takes a function
without arguments. For the time being at least.
Most of the changes are to replace the common idiom
col |= 0x07;
col++;
with the new helper function ntabstop().
display.c has quite some churn since I'm basically breaking an
abstraction: update() deals with the line to render but knows very
little of the state of the terminal, vtputc() and friends instead
knows nothing of the buffer but know the current physical column. I'm
pushing the mgwin struct down into the vt*() layer so that it knows
how wide a tab is. Previously I did it with a global variable and
while it needed less changes, it was uglier.
util.c may not be the perfect place for ntabstop(). c-mode won't be
happy with a tab width different from eight, still have to think what
to do there. 1 and 16 as min/max value for the tab width are
arbirtrary.
All in all I'm quite happy with how it works. Comments, reviews and
tests appreciated :)
Index: basic.c
===
RCS file: /cvs/src/usr.bin/mg/basic.c,v
retrieving revision 1.53
diff -u -p -r1.53 basic.c
--- basic.c 17 Apr 2023 09:49:04 - 1.53
+++ basic.c 17 Apr 2023 16:32:21 -
@@ -277,8 +277,7 @@ getgoal(struct line *dlp)
for (i = 0; i < llength(dlp); i++) {
c = lgetc(dlp, i);
if (c == '\t') {
- col |= 0x07;
- col++;
+ col = ntabstop(col, curbp->b_tabw);
} else if (ISCTRL(c) != FALSE) {
col += 2;
} else if (isprint(c))
Index: buffer.c
===
RCS file: /cvs/src/usr.bin/mg/buffer.c,v
retrieving revision 1.113
diff -u -p -r1.113 buffer.c
--- buffer.c8 Mar 2023 04:43:11 - 1.113
+++ buffer.c17 Apr 2023 16:32:21 -
@@ -26,9 +26,42 @@ static struct buffer *bnew(const char *)
static int usebufname(const char *);
+/* Default tab width */
+int defb_tabw = 8;
+
/* Flag for global working dir */
extern int globalwd;
+/*
+ * Set the tab width for the current buffer, or the default for new
+ * buffers if called with a prefix argument.
+ */
+int
+settabw(int f, int n)
+{
+ charbuf[8], *bufp;
+ const char *errstr;
+
+ if (f & FFARG) {
+ if (n <= 0 || n > 16)
+ return (FALSE);
+ defb_tabw = n;
+ return (TRUE);
+ }
+
+ if ((bufp = eread("Tab Width: ", buf, sizeof(buf),
+ EFNUL | EFNEW | EFCR)) == NULL)
+ return (ABORT);
+ if (bufp[0] == '\0')
+ return (ABORT);
+ n = strtonum(buf, 1, 16, );
+ if (errstr)
+ return (dobeep_msgs("Tab width", errstr));
+ curbp->b_tabw = n;
+ curwp->w_rflag |= WFFRAME;
+ return (TRUE);
+}
+
int
togglereadonlyall(int f, int n)
{
@@ -588,6 +621,7 @@ bnew(const char *bname)
bp->b_lines = 1;
bp->b_nlseq = "\n"; /* use unix default */
bp->b_nlchr = bp->b_nlseq;
+ bp->b_tabw = defb_tabw;
if ((bp->b_bname = strdup(bname)) == NULL) {
dobeep();
ewprintf("Can't get %d bytes", strlen(bname) + 1);
Index: cmode.c
===
RCS file: /cvs/src/usr.bin/mg/cmode.c,v
retrieving revision 1.21
diff -u -p -r1.21 cmode.c
--- cmode.c 17 Apr 2023 09:49:04 - 1.21
+++ cmode.c 17 Apr 2023 16:32:21 -
@@ -245,10 +245,10 @@ getindent(const struct line *lp, int *cu
for (lo = 0; lo < llength(lp); lo++) {
if (!isspace(c = lgetc(lp, lo)))
break;
- if (c == '\t') {
- nicol |= 0x07;
- }
- nicol++;
+ if (c == '\t')
+ nicol = ntabstop(nicol, curbp->b_tabw);
+ else
+ nicol++;
}
/* If last line was blank, choose 0 */
@@ -411,8 +411,7 @@ findcolpos(const struct buffer *bp, cons
for (i = 0; i < lo; ++i) {
c = lgetc(lp, i);
if (c == '\t') {
- col |= 0x07;
-
bgpd: reverse output of flowspec_cmp
I noticed that the order generated in an RB tree using flowspec_cmp() is
reversed. The problem is that for addresses preferred means smaller.
I think it is best to change the flowspec_cmp function to sort data so
that RB_FOREACH will print them most-preferred to least-preferred.
I had not caught this oversight until I started to really test all the
bits an pieces.
--
:wq Claudio
Index: flowspec.c
===
RCS file: /cvs/src/usr.sbin/bgpd/flowspec.c,v
retrieving revision 1.1
diff -u -p -r1.1 flowspec.c
--- flowspec.c 17 Apr 2023 08:02:21 - 1.1
+++ flowspec.c 17 Apr 2023 16:50:59 -
@@ -91,7 +91,7 @@ flowspec_next_component(const uint8_t *b
/*
* Compare two IPv4 flowspec prefix components.
- * Returns 1 if first prefix is preferred, -1 if second, 0 if equal.
+ * Returns -1 if first prefix is preferred, 1 if second, 0 if equal.
*/
static int
flowspec_cmp_prefix4(const uint8_t *abuf, int ablen, const uint8_t *bbuf,
@@ -113,15 +113,15 @@ flowspec_cmp_prefix4(const uint8_t *abuf
/* lowest IP value has precedence */
cmp = memcmp(a, b, sizeof(a));
if (cmp < 0)
- return 1;
- if (cmp > 0)
return -1;
+ if (cmp > 0)
+ return 1;
/* if common prefix, more specific route has precedence */
if (alen > blen)
- return 1;
- if (alen < blen)
return -1;
+ if (alen < blen)
+ return 1;
return 0;
}
@@ -139,9 +139,9 @@ flowspec_cmp_prefix6(const uint8_t *abuf
/* lowest offset has precedence */
if (abuf[2] < bbuf[2])
- return 1;
- if (abuf[2] > bbuf[2])
return -1;
+ if (abuf[2] > bbuf[2])
+ return 1;
/* calculate actual pattern size (len - offset) */
alen = abuf[1] - abuf[2];
@@ -157,15 +157,15 @@ flowspec_cmp_prefix6(const uint8_t *abuf
/* lowest IP value has precedence */
cmp = memcmp(a, b, sizeof(a));
if (cmp < 0)
- return 1;
- if (cmp > 0)
return -1;
+ if (cmp > 0)
+ return 1;
/* if common prefix, more specific route has precedence */
if (alen > blen)
- return 1;
- if (alen < blen)
return -1;
+ if (alen < blen)
+ return 1;
return 0;
}
@@ -199,7 +199,7 @@ flowspec_valid(const uint8_t *buf, int l
/*
* Compare two valid flowspec NLRI objects according to RFC 8955 & 8956.
- * Returns 1 if the first object has preference, -1 if not, and 0 if the
+ * Returns -1 if the first object has preference, 1 if not, and 0 if the
* two objects are equal.
*/
int
@@ -219,9 +219,9 @@ flowspec_cmp(const uint8_t *a, int alen,
/* If types differ, lowest type wins. */
if (atype < btype)
- return 1;
- if (atype > btype)
return -1;
+ if (atype > btype)
+ return 1;
switch (atype) {
case FLOWSPEC_TYPE_DEST:
@@ -244,9 +244,9 @@ flowspec_cmp(const uint8_t *a, int alen,
* string has precedence.
*/
if (cmp < 0)
- return 1;
- if (cmp > 0)
return -1;
+ if (cmp > 0)
+ return 1;
/*
* Longest component wins when common prefix is equal.
* This is not really possible because EOL encoding will
@@ -254,9 +254,9 @@ flowspec_cmp(const uint8_t *a, int alen,
* it (and it is cheap).
*/
if (acomplen > bcomplen)
- return 1;
- if (acomplen < bcomplen)
return -1;
+ if (acomplen < bcomplen)
+ return 1;
break;
}
a += acomplen;
@@ -266,9 +266,9 @@ flowspec_cmp(const uint8_t *a, int alen,
/* Rule with more components wins */
if (alen > 0 && blen <= 0)
- return 1;
- if (alen <= 0 && blen > 0)
return -1;
+ if (alen <= 0 && blen > 0)
+ return 1;
}
return 0;
}
Re: Call sysctl_source() with shared netlock
On Mon, Apr 17, 2023 at 06:33:50PM +0300, Vitaliy Makkoveev wrote:
> However, the renaming of `source' to `ar_source' diff is pretty small.
> We use 'art_root' structure in regress/sys/net/rtable/delete and in
> usr.bin/netstat/ but we don't touch `source'.
OK bluhm@
> Index: sys/net/art.h
> ===
> RCS file: /cvs/src/sys/net/art.h,v
> retrieving revision 1.21
> diff -u -p -r1.21 art.h
> --- sys/net/art.h 2 Mar 2021 17:50:41 - 1.21
> +++ sys/net/art.h 17 Apr 2023 15:32:43 -
> @@ -41,7 +41,7 @@ struct art_root {
> uint8_t ar_nlvl; /* [I] Number of levels */
> uint8_t ar_alen; /* [I] Address length in bits */
> uint8_t ar_off;/* [I] Offset of key in bytes */
> - struct sockaddr *source;/* [K] optional src addr to use
> */
> + struct sockaddr *ar_source; /* [K] optional src addr to use
> */
> };
>
> #define ISLEAF(e)(((unsigned long)(e) & 1) == 0)
> Index: sys/net/rtable.c
> ===
> RCS file: /cvs/src/sys/net/rtable.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 rtable.c
> --- sys/net/rtable.c 29 Jun 2022 22:20:47 - 1.80
> +++ sys/net/rtable.c 17 Apr 2023 15:32:43 -
> @@ -379,7 +379,7 @@ rtable_setsource(unsigned int rtableid,
> if ((ar = rtable_get(rtableid, af)) == NULL)
> return (EAFNOSUPPORT);
>
> - ar->source = src;
> + ar->ar_source = src;
>
> return (0);
> }
> @@ -393,7 +393,7 @@ rtable_getsource(unsigned int rtableid,
> if (ar == NULL)
> return (NULL);
>
> - return (ar->source);
> + return (ar->ar_source);
> }
>
> void
Re: Call sysctl_source() with shared netlock
On Mon, Apr 17, 2023 at 04:32:13PM +0200, Alexander Bluhm wrote:
> On Mon, Apr 17, 2023 at 02:36:57AM +0300, Vitaliy Makkoveev wrote:
> > It seems rt_setsource() needs some attention, but sysctl_source() could
> > be called with shared netlock just now.
>
> I think rtable_setsource() is not MP safe. It is documented as [K]
> kernel lock. But that is not true and makes no sense. It should
> be exclusive netlock. in_pcbselsrc() calls rtable_getsource() with
> netlock. We should rename source to ar_source so we can grep for
> its users.
Perhaps something like this. I will run it through regress.
Index: net/art.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/art.h,v
retrieving revision 1.21
diff -u -p -r1.21 art.h
--- net/art.h 2 Mar 2021 17:50:41 - 1.21
+++ net/art.h 17 Apr 2023 14:48:46 -
@@ -41,7 +41,7 @@ struct art_root {
uint8_t ar_nlvl; /* [I] Number of levels */
uint8_t ar_alen; /* [I] Address length in bits */
uint8_t ar_off;/* [I] Offset of key in bytes */
- struct sockaddr *source;/* [K] optional src addr to use
*/
+ struct sockaddr *ar_source; /* [N] use optional src addr */
};
#define ISLEAF(e) (((unsigned long)(e) & 1) == 0)
Index: net/rtable.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtable.c,v
retrieving revision 1.80
diff -u -p -r1.80 rtable.c
--- net/rtable.c29 Jun 2022 22:20:47 - 1.80
+++ net/rtable.c17 Apr 2023 14:54:52 -
@@ -376,10 +376,12 @@ rtable_setsource(unsigned int rtableid,
{
struct art_root *ar;
+ NET_ASSERT_LOCKED_EXCLUSIVE();
+
if ((ar = rtable_get(rtableid, af)) == NULL)
return (EAFNOSUPPORT);
- ar->source = src;
+ ar->ar_source = src;
return (0);
}
@@ -389,11 +391,13 @@ rtable_getsource(unsigned int rtableid,
{
struct art_root *ar;
+ NET_ASSERT_LOCKED();
+
ar = rtable_get(rtableid, af);
if (ar == NULL)
return (NULL);
- return (ar->source);
+ return (ar->ar_source);
}
void
Index: net/rtsock.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtsock.c,v
retrieving revision 1.359
diff -u -p -r1.359 rtsock.c
--- net/rtsock.c22 Jan 2023 12:05:44 - 1.359
+++ net/rtsock.c17 Apr 2023 15:16:48 -
@@ -853,8 +853,10 @@ route_output(struct mbuf *m, struct sock
error = EINVAL;
goto fail;
}
- if ((error =
- rt_setsource(tableid, info.rti_info[RTAX_IFA])) != 0)
+ NET_LOCK();
+ error = rt_setsource(tableid, info.rti_info[RTAX_IFA]);
+ NET_UNLOCK();
+ if (error)
goto fail;
} else {
error = rtm_output(rtm, , , prio, tableid);
@@ -862,7 +864,9 @@ route_output(struct mbuf *m, struct sock
type = rtm->rtm_type;
seq = rtm->rtm_seq;
free(rtm, M_RTABLE, len);
+ NET_LOCK_SHARED();
rtm = rtm_report(rt, type, seq, tableid);
+ NET_UNLOCK_SHARED();
len = rtm->rtm_msglen;
}
}
Re: Call sysctl_source() with shared netlock
On Mon, Apr 17, 2023 at 04:32:13PM +0200, Alexander Bluhm wrote:
> On Mon, Apr 17, 2023 at 02:36:57AM +0300, Vitaliy Makkoveev wrote:
> > It seems rt_setsource() needs some attention, but sysctl_source() could
> > be called with shared netlock just now.
>
> I think rtable_setsource() is not MP safe. It is documented as [K]
> kernel lock. But that is not true and makes no sense. It should
> be exclusive netlock. in_pcbselsrc() calls rtable_getsource() with
> netlock. We should rename source to ar_source so we can grep for
> its users.
>
rtable_*source() locking should be reworked. The data pointed by
ar->source actually belong to `ifa'. So we need to use netlock, as I
propose in the "Remove kernel lock from ifa_ifwithaddr" diff.
However, the renaming of `source' to `ar_source' diff is pretty small.
We use 'art_root' structure in regress/sys/net/rtable/delete and in
usr.bin/netstat/ but we don't touch `source'.
Index: sys/net/art.h
===
RCS file: /cvs/src/sys/net/art.h,v
retrieving revision 1.21
diff -u -p -r1.21 art.h
--- sys/net/art.h 2 Mar 2021 17:50:41 - 1.21
+++ sys/net/art.h 17 Apr 2023 15:32:43 -
@@ -41,7 +41,7 @@ struct art_root {
uint8_t ar_nlvl; /* [I] Number of levels */
uint8_t ar_alen; /* [I] Address length in bits */
uint8_t ar_off;/* [I] Offset of key in bytes */
- struct sockaddr *source;/* [K] optional src addr to use
*/
+ struct sockaddr *ar_source; /* [K] optional src addr to use
*/
};
#define ISLEAF(e) (((unsigned long)(e) & 1) == 0)
Index: sys/net/rtable.c
===
RCS file: /cvs/src/sys/net/rtable.c,v
retrieving revision 1.80
diff -u -p -r1.80 rtable.c
--- sys/net/rtable.c29 Jun 2022 22:20:47 - 1.80
+++ sys/net/rtable.c17 Apr 2023 15:32:43 -
@@ -379,7 +379,7 @@ rtable_setsource(unsigned int rtableid,
if ((ar = rtable_get(rtableid, af)) == NULL)
return (EAFNOSUPPORT);
- ar->source = src;
+ ar->ar_source = src;
return (0);
}
@@ -393,7 +393,7 @@ rtable_getsource(unsigned int rtableid,
if (ar == NULL)
return (NULL);
- return (ar->source);
+ return (ar->ar_source);
}
void
Re: Call sysctl_ifnames() with shared netlock
On Mon, Apr 17, 2023 at 02:58:59PM +0200, Alexander Bluhm wrote:
> On Mon, Apr 17, 2023 at 01:20:28AM +0300, Vitaliy Makkoveev wrote:
> > It performs read-only access to netlock protected data.
>
> OK bluhm@
>
> Could you somewhere document that ifnetlist is protected by netlock?
>
We use both kernel and net lock for protect `ifnetlist'. This is the
exception, so I propose to document this like below.
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.688
diff -u -p -r1.688 if.c
--- sys/net/if.c8 Apr 2023 13:49:38 - 1.688
+++ sys/net/if.c17 Apr 2023 14:42:10 -
@@ -272,6 +272,10 @@ ifinit(void)
static struct if_idxmap if_idxmap;
+/*
+ * XXXSMP: For `ifnetlist' modification both kernel and net locks
+ * should be taken. For read-only access only one lock of them required.
+ */
struct ifnet_head ifnetlist = TAILQ_HEAD_INITIALIZER(ifnetlist);
static inline unsigned int
Index: sys/net/if_var.h
===
RCS file: /cvs/src/sys/net/if_var.h,v
retrieving revision 1.123
diff -u -p -r1.123 if_var.h
--- sys/net/if_var.h5 Apr 2023 19:35:23 - 1.123
+++ sys/net/if_var.h17 Apr 2023 14:42:10 -
@@ -121,7 +121,7 @@ TAILQ_HEAD(ifnet_head, ifnet); /* the a
struct ifnet { /* and the entries */
void*if_softc; /* [I] lower-level data for this if */
struct refcnt if_refcnt;
- TAILQ_ENTRY(ifnet) if_list; /* [K] all struct ifnets are chained */
+ TAILQ_ENTRY(ifnet) if_list; /* [NK] all struct ifnets are chained */
TAILQ_HEAD(, ifaddr) if_addrlist; /* [N] list of addresses per if */
TAILQ_HEAD(, ifmaddr) if_maddrlist; /* [N] list of multicast records */
TAILQ_HEAD(, ifg_list) if_groups; /* [N] list of groups per if */
Re: Call sysctl_dumpentry() with shared netlock
On Mon, Apr 17, 2023 at 02:16:14AM +0300, Vitaliy Makkoveev wrote:
> Also performs read-only access to netlock protected data.
rtable_getsource() needs kernel lock. But that problem does not
get worse by this diff.
OK bluhm@
> Index: sys/net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.359
> diff -u -p -r1.359 rtsock.c
> --- sys/net/rtsock.c 22 Jan 2023 12:05:44 - 1.359
> +++ sys/net/rtsock.c 16 Apr 2023 23:12:25 -
> @@ -2159,7 +2159,7 @@ sysctl_rtable(int *name, u_int namelen,
> switch (w.w_op) {
> case NET_RT_DUMP:
> case NET_RT_FLAGS:
> - NET_LOCK();
> + NET_LOCK_SHARED();
> for (i = 1; i <= AF_MAX; i++) {
> if (af != 0 && af != i)
> continue;
> @@ -2171,7 +2171,7 @@ sysctl_rtable(int *name, u_int namelen,
> if (error)
> break;
> }
> - NET_UNLOCK();
> + NET_UNLOCK_SHARED();
> break;
>
> case NET_RT_IFLIST:
Re: Call sysctl_source() with shared netlock
On Mon, Apr 17, 2023 at 02:36:57AM +0300, Vitaliy Makkoveev wrote:
> It seems rt_setsource() needs some attention, but sysctl_source() could
> be called with shared netlock just now.
I think rtable_setsource() is not MP safe. It is documented as [K]
kernel lock. But that is not true and makes no sense. It should
be exclusive netlock. in_pcbselsrc() calls rtable_getsource() with
netlock. We should rename source to ar_source so we can grep for
its users.
> Index: sys/net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.359
> diff -u -p -r1.359 rtsock.c
> --- sys/net/rtsock.c 22 Jan 2023 12:05:44 - 1.359
> +++ sys/net/rtsock.c 16 Apr 2023 23:14:11 -
> @@ -2201,7 +2201,7 @@ sysctl_rtable(int *name, u_int namelen,
> tableid = w.w_arg;
> if (!rtable_exists(tableid))
> return (ENOENT);
> - NET_LOCK();
> + NET_LOCK_SHARED();
> for (i = 1; i <= AF_MAX; i++) {
> if (af != 0 && af != i)
> continue;
> @@ -2212,7 +2212,7 @@ sysctl_rtable(int *name, u_int namelen,
> if (error)
> break;
> }
> - NET_UNLOCK();
> + NET_UNLOCK_SHARED();
> break;
> }
> free(w.w_tmem, M_RTABLE, w.w_tmemsize);
mg: fix buffer overflow in displaymatch()
When the match for a character is not in the visible part of the
window, mg shows a copy of that line in the echo area. To do so, it
copies the line to a local buffer, so that tabs and control characters
can be rendered properly, but doesn't check for overflow.
NLINE (the size of `buf') is 256, so if the matching parentesis is on
a line longer than 256 characters that's not visible on the screen, we
start writing out-of-bounds and likely crashing.
I've reproduced the crash by editing (a copy of) /bsd which has very
long lines and verified that diff below fixes it. The kernel is nice
for this because it contains a lot of bytes that mg interprets as
controls and so stresses the rendering of control characters.
ok?
Index: match.c
===
RCS file: /home/cvs/src/usr.bin/mg/match.c,v
retrieving revision 1.23
diff -u -p -r1.23 match.c
--- match.c 17 Apr 2023 09:49:04 - 1.23
+++ match.c 17 Apr 2023 13:59:57 -
@@ -168,17 +168,23 @@ displaymatch(struct line *clp, int cbo)
/* match is not in this window, so display line in echo area */
bufo = 0;
for (cp = 0; cp < llength(clp); cp++) {
+ if (bufo >= sizeof(buf) - 1)
+ break;
+
c = lgetc(clp, cp);
- if (c != '\t')
+ if (c != '\t') {
if (ISCTRL(c)) {
+ if (bufo + 2 >= sizeof(buf) - 1)
+ break;
buf[bufo++] = '^';
buf[bufo++] = CCHR(c);
} else
buf[bufo++] = c;
- else
+ } else {
do {
buf[bufo++] = ' ';
- } while (bufo & 7);
+ } while ((bufo & 7) && bufo < sizeof(buf) - 1);
+ }
}
buf[bufo++] = '\0';
ewprintf("Matches %s", buf);
Re: Call sysctl_iflist() with shared netlock
On Mon, Apr 17, 2023 at 01:22:07AM +0300, Vitaliy Makkoveev wrote: > As sysctl_ifnames(), it performs read-only access to netlock protected > data. OK bluhm@ > Index: sys/net/rtsock.c > === > RCS file: /cvs/src/sys/net/rtsock.c,v > retrieving revision 1.359 > diff -u -p -r1.359 rtsock.c > --- sys/net/rtsock.c 22 Jan 2023 12:05:44 - 1.359 > +++ sys/net/rtsock.c 16 Apr 2023 22:16:21 - > @@ -2175,9 +2175,9 @@ sysctl_rtable(int *name, u_int namelen, > break; > > case NET_RT_IFLIST: > - NET_LOCK(); > + NET_LOCK_SHARED(); > error = sysctl_iflist(af, ); > - NET_UNLOCK(); > + NET_UNLOCK_SHARED(); > break; > > case NET_RT_STATS:
Re: bgpctl parse numbers
On Mon, Apr 17, 2023 at 02:39:42PM +0200, Claudio Jeker wrote: > This does the same trick as with communities of matching both the keyword > and parsing the next argument in one go. Again a few helper tables go > away. Lovely. ok tb
Re: Call sysctl_ifnames() with shared netlock
On Mon, Apr 17, 2023 at 01:20:28AM +0300, Vitaliy Makkoveev wrote: > It performs read-only access to netlock protected data. OK bluhm@ Could you somewhere document that ifnetlist is protected by netlock? > Index: sys/net/rtsock.c > === > RCS file: /cvs/src/sys/net/rtsock.c,v > retrieving revision 1.359 > diff -u -p -r1.359 rtsock.c > --- sys/net/rtsock.c 22 Jan 2023 12:05:44 - 1.359 > +++ sys/net/rtsock.c 16 Apr 2023 22:15:13 - > @@ -2193,9 +2193,9 @@ sysctl_rtable(int *name, u_int namelen, > , sizeof(tableinfo)); > return (error); > case NET_RT_IFNAMES: > - NET_LOCK(); > + NET_LOCK_SHARED(); > error = sysctl_ifnames(); > - NET_UNLOCK(); > + NET_UNLOCK_SHARED(); > break; > case NET_RT_SOURCE: > tableid = w.w_arg;
Re: bgpctl show rib 192.0.2.1 detail
On Mon, Apr 17, 2023 at 02:30:40PM +0200, Claudio Jeker wrote:
> Forgot this bit in the 'bgpctl show rib 192.0.2.1 detail' support I
> commited this weekend.
>
> The problem is that parse_prefix() is entered with 'detail' as argument
> and clears the previously set address. So be more careful and only modify
> the addr pointer if parse_prefix() and parse_addr() are successful.
Ugh, sorry for missing this.
ok tb
>
> --
> :wq Claudio
>
> Index: parser.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
> retrieving revision 1.127
> diff -u -p -r1.127 parser.c
> --- parser.c 17 Apr 2023 11:02:40 - 1.127
> +++ parser.c 17 Apr 2023 12:29:29 -
> @@ -876,10 +876,10 @@ parse_addr(const char *word, struct bgpd
> if (word == NULL)
> return (0);
>
> - memset(addr, 0, sizeof(struct bgpd_addr));
> memset(, 0, sizeof(ina));
>
> if (inet_net_pton(AF_INET, word, , sizeof(ina)) != -1) {
> + memset(addr, 0, sizeof(*addr));
> addr->aid = AID_INET;
> addr->v4 = ina;
> return (1);
> @@ -902,6 +902,7 @@ int
> parse_prefix(const char *word, size_t wordlen, struct bgpd_addr *addr,
> uint8_t *prefixlen)
> {
> + struct bgpd_addr tmp;
> char*p, *ps;
> const char *errstr;
> int mask = -1;
> @@ -909,7 +910,7 @@ parse_prefix(const char *word, size_t wo
> if (word == NULL)
> return (0);
>
> - memset(addr, 0, sizeof(struct bgpd_addr));
> + memset(, 0, sizeof(tmp));
>
> if ((p = strrchr(word, '/')) != NULL) {
> size_t plen = strlen(p);
> @@ -921,17 +922,17 @@ parse_prefix(const char *word, size_t wo
> err(1, "parse_prefix: malloc");
> strlcpy(ps, word, wordlen - plen + 1);
>
> - if (parse_addr(ps, addr) == 0) {
> + if (parse_addr(ps, ) == 0) {
> free(ps);
> return (0);
> }
>
> free(ps);
> } else
> - if (parse_addr(word, addr) == 0)
> + if (parse_addr(word, ) == 0)
> return (0);
>
> - switch (addr->aid) {
> + switch (tmp.aid) {
> case AID_INET:
> if (mask == -1)
> mask = 32;
> @@ -946,7 +947,7 @@ parse_prefix(const char *word, size_t wo
> return (0);
> }
>
> - applymask(addr, addr, mask);
> + applymask(addr, , mask);
> *prefixlen = mask;
> return (1);
> }
>
bgpctl parse numbers
This does the same trick as with communities of matching both the keyword
and parsing the next argument in one go. Again a few helper tables go
away.
--
:wq Claudio
Index: parser.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
retrieving revision 1.127
diff -u -p -r1.127 parser.c
--- parser.c17 Apr 2023 11:02:40 - 1.127
+++ parser.c17 Apr 2023 12:03:35 -
@@ -100,18 +100,10 @@ static const struct token t_bulk[];
static const struct token t_network_show[];
static const struct token t_prefix[];
static const struct token t_set[];
-static const struct token t_localpref[];
-static const struct token t_med[];
static const struct token t_nexthop[];
static const struct token t_pftable[];
-static const struct token t_prepnbr[];
-static const struct token t_prepself[];
-static const struct token t_weight[];
static const struct token t_log[];
-static const struct token t_fib_table[];
-static const struct token t_show_fib_table[];
static const struct token t_communication[];
-static const struct token t_show_rib_path[];
static const struct token t_main[] = {
{ KEYWORD, "fib", FIB,t_fib},
@@ -153,7 +145,7 @@ static const struct token t_show_fib[] =
{ FLAG, "connected",F_CONNECTED,t_show_fib},
{ FLAG, "nexthop", F_NEXTHOP, t_show_fib},
{ FLAG, "static", F_STATIC, t_show_fib},
- { KEYWORD, "table",NONE, t_show_fib_table},
+ { RTABLE, "table",NONE, t_show_fib},
{ FAMILY, "", NONE, t_show_fib},
{ ADDRESS, "", NONE, NULL},
{ ENDTOKEN, "", NONE, NULL}
@@ -177,7 +169,7 @@ static const struct token t_show_rib[] =
{ KEYWORD, "neighbor", NONE, t_show_rib_neigh},
{ FLAG, "out", F_CTL_ADJ_OUT, t_show_rib},
{ KEYWORD, "ovs", NONE, t_show_ovs},
- { KEYWORD, "path-id", NONE, t_show_rib_path},
+ { PATHID, "path-id", NONE, t_show_rib},
{ ASTYPE, "peer-as", AS_PEER,t_show_rib_as},
{ FLAG, "selected", F_CTL_BEST, t_show_rib},
{ ASTYPE, "source-as",AS_SOURCE, t_show_rib_as},
@@ -272,7 +264,7 @@ static const struct token t_show_neighbo
static const struct token t_fib[] = {
{ KEYWORD, "couple", FIB_COUPLE, NULL},
{ KEYWORD, "decouple", FIB_DECOUPLE, NULL},
- { KEYWORD, "table",NONE, t_fib_table},
+ { RTABLE, "table",NONE, t_fib},
{ ENDTOKEN, "", NONE, NULL}
};
@@ -364,25 +356,15 @@ static const struct token t_set[] = {
{ COMMUNITY,"community",NONE, t_set},
{ EXTCOMMUNITY, "ext-community",NONE, t_set},
{ LRGCOMMUNITY, "large-community", NONE, t_set},
- { KEYWORD, "localpref",NONE, t_localpref},
- { KEYWORD, "med", NONE, t_med},
- { KEYWORD, "metric", NONE, t_med},
+ { LOCALPREF,"localpref",NONE, t_set},
+ { MED, "med", NONE, t_set},
+ { MED, "metric", NONE, t_set},
{ KEYWORD, "nexthop", NONE, t_nexthop},
{ KEYWORD, "pftable", NONE, t_pftable},
- { KEYWORD, "prepend-neighbor", NONE, t_prepnbr},
- { KEYWORD, "prepend-self", NONE, t_prepself},
+ { PREPNBR, "prepend-neighbor", NONE, t_set},
+ { PREPSELF, "prepend-self", NONE, t_set},
{ KEYWORD, "rd", NONE, t_rd},
- { KEYWORD, "weight", NONE, t_weight},
- { ENDTOKEN, "", NONE, NULL}
-};
-
-static const struct token t_localpref[] = {
- { LOCALPREF,"", NONE, t_set},
- { ENDTOKEN, "", NONE, NULL}
-};
-
-static const struct token t_med[] = {
- { MED, "", NONE, t_set},
+ { WEIGHT, "weight", NONE, t_set},
{ ENDTOKEN, "", NONE, NULL}
};
@@ -396,42 +378,12 @@ static const struct token t_pftable[] =
{ ENDTOKEN, "", NONE, NULL}
};
-static const struct token t_prepnbr[] = {
- { PREPNBR, "", NONE, t_set},
- { ENDTOKEN, "", NONE, NULL}
-};
-
-static const struct token t_prepself[] = {
- { PREPSELF, "", NONE, t_set},
- { ENDTOKEN, "",
bgpctl show rib 192.0.2.1 detail
Forgot this bit in the 'bgpctl show rib 192.0.2.1 detail' support I
commited this weekend.
The problem is that parse_prefix() is entered with 'detail' as argument
and clears the previously set address. So be more careful and only modify
the addr pointer if parse_prefix() and parse_addr() are successful.
--
:wq Claudio
Index: parser.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
retrieving revision 1.127
diff -u -p -r1.127 parser.c
--- parser.c17 Apr 2023 11:02:40 - 1.127
+++ parser.c17 Apr 2023 12:29:29 -
@@ -876,10 +876,10 @@ parse_addr(const char *word, struct bgpd
if (word == NULL)
return (0);
- memset(addr, 0, sizeof(struct bgpd_addr));
memset(, 0, sizeof(ina));
if (inet_net_pton(AF_INET, word, , sizeof(ina)) != -1) {
+ memset(addr, 0, sizeof(*addr));
addr->aid = AID_INET;
addr->v4 = ina;
return (1);
@@ -902,6 +902,7 @@ int
parse_prefix(const char *word, size_t wordlen, struct bgpd_addr *addr,
uint8_t *prefixlen)
{
+ struct bgpd_addr tmp;
char*p, *ps;
const char *errstr;
int mask = -1;
@@ -909,7 +910,7 @@ parse_prefix(const char *word, size_t wo
if (word == NULL)
return (0);
- memset(addr, 0, sizeof(struct bgpd_addr));
+ memset(, 0, sizeof(tmp));
if ((p = strrchr(word, '/')) != NULL) {
size_t plen = strlen(p);
@@ -921,17 +922,17 @@ parse_prefix(const char *word, size_t wo
err(1, "parse_prefix: malloc");
strlcpy(ps, word, wordlen - plen + 1);
- if (parse_addr(ps, addr) == 0) {
+ if (parse_addr(ps, ) == 0) {
free(ps);
return (0);
}
free(ps);
} else
- if (parse_addr(word, addr) == 0)
+ if (parse_addr(word, ) == 0)
return (0);
- switch (addr->aid) {
+ switch (tmp.aid) {
case AID_INET:
if (mask == -1)
mask = 32;
@@ -946,7 +947,7 @@ parse_prefix(const char *word, size_t wo
return (0);
}
- applymask(addr, addr, mask);
+ applymask(addr, , mask);
*prefixlen = mask;
return (1);
}
diff: www/faq/pf/carp.html
Hi, carpdemote is increased/decreased when the link state of the carpdev is down/up. This behavior is not related to net.inet.carp.preempt since "carpdemote" is introduced. But the faq still says the "net.inet.carp.preempt" variable enables it. I'd like to commit the diff. ok or any comment is welcome. Index: faq/pf/carp.html === RCS file: /cvs/www/faq/pf/carp.html,v retrieving revision 1.65 diff -u -p -r1.65 carp.html --- faq/pf/carp.html5 May 2021 21:49:29 - 1.65 +++ faq/pf/carp.html17 Apr 2023 11:32:46 - @@ -191,8 +191,9 @@ As such, CARP is configured using By default, all carp interfaces are added to the carp group. Each group has a carpdemote counter affecting all carp interfaces belonging to that group. -As described below, it can be useful to group certain interfaces together -for failover purposes. +If one physical CARP-enabled interface goes down, CARP will increase +the demotion counter by 1 on interface groups that the carp(4) interface is +a member of, in effect causing all group members to fail-over together. ipaddress This is the shared IP address assigned to the redundancy group. @@ -216,12 +217,6 @@ Further CARP behavior can be controlled net.inet.carp.preempt Allow hosts within a redundancy group that have a better advbase and advskew to preempt the master. -In addition, this option also enables failing over a group of interfaces -together in the event that one interface goes down. -If one physical CARP-enabled interface goes down, CARP will increase -the demotion counter, carpdemote, by 1 on interface groups -that the carp(4) interface is a member of, in effect causing all group -members to fail-over together. net.inet.carp.preempt is 0 (disabled) by default. net.inet.carp.log
Re: bgpctl change parser for communities
On Mon, Apr 17, 2023 at 12:45:43PM +0200, Claudio Jeker wrote: > On Mon, Apr 17, 2023 at 12:12:47PM +0200, Theo Buehler wrote: > > On Mon, Apr 17, 2023 at 11:28:37AM +0200, Claudio Jeker wrote: > > > I want to extend the parser to support lists in a few places. > > > One of them is for communities. This is the first step towards this goal. > > > The change uses the fact that match_token() has access to argc and argv > > > and changes the community parsers to parse the next token for communities. > > > As a nice side-effect this reduces the amount of tables and removes the > > > ext-community subtype tables which are probably never in sync with the one > > > from bgpd.h. > > > > I like the direction and it looks like a good intermediate step. Go > > ahead > > > > ok > > > > > The way argv is passed to match_token() is not great, tripple pointers are > > > just strange. I plan to fix this in an upcomming diff. > > > > That would be nice. > > How about this? I think this is the simplest way to handle this. Yes, that's much better and agreed, I don't see a simpler way. > -const struct token *match_token(int *argc, char **argv[], > - const struct token []); > +const struct token *match_token(int argc, char *argv[], > + const struct token [], int *); This is not new, but I'd either name all arguments or none of them. Other than that, ok
Re: bgpctl change parser for communities
On Mon, Apr 17, 2023 at 12:12:47PM +0200, Theo Buehler wrote:
> On Mon, Apr 17, 2023 at 11:28:37AM +0200, Claudio Jeker wrote:
> > I want to extend the parser to support lists in a few places.
> > One of them is for communities. This is the first step towards this goal.
> > The change uses the fact that match_token() has access to argc and argv
> > and changes the community parsers to parse the next token for communities.
> > As a nice side-effect this reduces the amount of tables and removes the
> > ext-community subtype tables which are probably never in sync with the one
> > from bgpd.h.
>
> I like the direction and it looks like a good intermediate step. Go
> ahead
>
> ok
>
> > The way argv is passed to match_token() is not great, tripple pointers are
> > just strange. I plan to fix this in an upcomming diff.
>
> That would be nice.
How about this? I think this is the simplest way to handle this.
--
:wq Claudio
Index: parser.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
retrieving revision 1.126
diff -u -p -r1.126 parser.c
--- parser.c17 Apr 2023 10:23:32 - 1.126
+++ parser.c17 Apr 2023 10:43:16 -
@@ -434,8 +434,8 @@ static const struct token t_show_rib_pat
static struct parse_result res;
-const struct token *match_token(int *argc, char **argv[],
- const struct token []);
+const struct token *match_token(int argc, char *argv[],
+ const struct token [], int *);
voidshow_valid_args(const struct token []);
intparse_addr(const char *, struct bgpd_addr *);
@@ -451,13 +451,14 @@ parse(int argc, char *argv[])
{
const struct token *table = t_main;
const struct token *match;
+ int used;
memset(, 0, sizeof(res));
res.rtableid = getrtable();
TAILQ_INIT();
while (argc >= 0) {
- if ((match = match_token(, , table)) == NULL) {
+ if ((match = match_token(argc, argv, table, )) == NULL) {
fprintf(stderr, "valid commands/args:\n");
show_valid_args(table);
return (NULL);
@@ -469,12 +470,11 @@ parse(int argc, char *argv[])
continue;
}
- argc--;
- argv++;
+ argc -= used;
+ argv += used;
if (match->type == NOTOKEN || match->next == NULL)
break;
-
table = match->next;
}
@@ -487,14 +487,15 @@ parse(int argc, char *argv[])
}
const struct token *
-match_token(int *argc, char **argv[], const struct token table[])
+match_token(int argc, char *argv[], const struct token table[], int *argsused)
{
u_inti, match;
const struct token *t = NULL;
struct filter_set *fs;
- const char *word = (*argv)[0];
- size_t wordlen = 0;
+ const char *word = argv[0];
+ size_t wordlen = 0;
+ *argsused = 1;
match = 0;
if (word != NULL)
wordlen = strlen(word);
@@ -625,10 +626,9 @@ match_token(int *argc, char **argv[], co
break;
case COMMUNITY:
if (word != NULL && strncmp(word, table[i].keyword,
- wordlen) == 0 && *argc > 1) {
- parsecommunity(, (*argv)[1]);
- *argc -= 1;
- *argv += 1;
+ wordlen) == 0 && argc > 1) {
+ parsecommunity(, argv[1]);
+ *argsused += 1;
if ((fs = calloc(1, sizeof(*fs))) == NULL)
err(1, NULL);
@@ -642,10 +642,9 @@ match_token(int *argc, char **argv[], co
break;
case LRGCOMMUNITY:
if (word != NULL && strncmp(word, table[i].keyword,
- wordlen) == 0 && *argc > 1) {
- parselargecommunity(, (*argv)[1]);
- *argc -= 1;
- *argv += 1;
+ wordlen) == 0 && argc > 1) {
+ parselargecommunity(, argv[1]);
+ *argsused += 1;
if ((fs = calloc(1, sizeof(*fs))) == NULL)
err(1, NULL);
@@ -659,11 +658,10 @@ match_token(int *argc, char **argv[], co
break;
case EXTCOMMUNITY:
if (word != NULL && strncmp(word, table[i].keyword,
- wordlen) == 0 && *argc > 2) {
+
Re: bgpctl change parser for communities
On Mon, Apr 17, 2023 at 11:28:37AM +0200, Claudio Jeker wrote: > I want to extend the parser to support lists in a few places. > One of them is for communities. This is the first step towards this goal. > The change uses the fact that match_token() has access to argc and argv > and changes the community parsers to parse the next token for communities. > As a nice side-effect this reduces the amount of tables and removes the > ext-community subtype tables which are probably never in sync with the one > from bgpd.h. I like the direction and it looks like a good intermediate step. Go ahead ok > The way argv is passed to match_token() is not great, tripple pointers are > just strange. I plan to fix this in an upcomming diff. That would be nice.
mg: fix spacing in a few dobeep_msgs() calls
as per subject, when dobeep_msgs() was introduced some places weren't
converted correctly: it already adds a space between the two
arguments, so no need to duplicate.
Index: extend.c
===
RCS file: /cvs/src/usr.bin/mg/extend.c,v
retrieving revision 1.79
diff -u -p -r1.79 extend.c
--- extend.c30 Mar 2023 22:56:47 - 1.79
+++ extend.c17 Apr 2023 10:00:55 -
@@ -493,7 +493,7 @@ redefine_key(int f, int n)
return (FALSE);
(void)strlcat(buf, tmp, sizeof(buf));
if ((mp = name_map(tmp)) == NULL)
- return (dobeep_msgs("Unknown map ", tmp));
+ return (dobeep_msgs("Unknown map", tmp));
if (strlcat(buf, "key: ", sizeof(buf)) >= sizeof(buf))
return (FALSE);
@@ -720,7 +720,7 @@ excline(char *line, int llen, int lnum)
n = (int)nl;
}
if ((fp = name_function(funcp)) == NULL)
- return (dobeep_msgs("Unknown function: ", funcp));
+ return (dobeep_msgs("Unknown function:", funcp));
if (fp == bindtokey || fp == unbindtokey) {
bind = BINDARG;
@@ -847,7 +847,7 @@ excline(char *line, int llen, int lnum)
case BINDNEXT:
lp->l_text[lp->l_used] = '\0';
if ((curmap = name_map(lp->l_text)) == NULL) {
- (void)dobeep_msgs("No such mode: ", lp->l_text);
+ (void)dobeep_msgs("No such mode:", lp->l_text);
status = FALSE;
free(lp);
goto cleanup;
Index: interpreter.c
===
RCS file: /cvs/src/usr.bin/mg/interpreter.c,v
retrieving revision 1.34
diff -u -p -r1.34 interpreter.c
--- interpreter.c 28 Jan 2022 06:18:41 - 1.34
+++ interpreter.c 17 Apr 2023 10:00:55 -
@@ -406,7 +406,7 @@ parsexp(char *begp, const char *par1, co
* If no extant mg command found, just return.
*/
if ((funcp = name_function(cmdp)) == NULL)
- return (dobeep_msgs("Unknown command: ", cmdp));
+ return (dobeep_msgs("Unknown command:", cmdp));
numparams = numparams_function(funcp);
if (numparams == 0)
bgpctl change parser for communities
I want to extend the parser to support lists in a few places.
One of them is for communities. This is the first step towards this goal.
The change uses the fact that match_token() has access to argc and argv
and changes the community parsers to parse the next token for communities.
As a nice side-effect this reduces the amount of tables and removes the
ext-community subtype tables which are probably never in sync with the one
from bgpd.h.
The way argv is passed to match_token() is not great, tripple pointers are
just strange. I plan to fix this in an upcomming diff.
--
:wq Claudio
Index: parser.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
retrieving revision 1.125
diff -u -p -r1.125 parser.c
--- parser.c15 Apr 2023 10:36:59 - 1.125
+++ parser.c17 Apr 2023 09:15:55 -
@@ -50,8 +50,7 @@ enum token_type {
COMMUNICATION,
COMMUNITY,
EXTCOMMUNITY,
- EXTCOM_SUBTYPE,
- LARGE_COMMUNITY,
+ LRGCOMMUNITY,
LOCALPREF,
MED,
NEXTHOP,
@@ -96,19 +95,11 @@ static const struct token t_show_rib_as[
static const struct token t_show_mrt_as[];
static const struct token t_show_prefix[];
static const struct token t_show_ip[];
-static const struct token t_show_community[];
-static const struct token t_show_extcommunity[];
-static const struct token t_show_ext_subtype[];
-static const struct token t_show_largecommunity[];
static const struct token t_network[];
static const struct token t_bulk[];
static const struct token t_network_show[];
static const struct token t_prefix[];
static const struct token t_set[];
-static const struct token t_community[];
-static const struct token t_extcommunity[];
-static const struct token t_ext_subtype[];
-static const struct token t_largecommunity[];
static const struct token t_localpref[];
static const struct token t_med[];
static const struct token t_nexthop[];
@@ -173,14 +164,14 @@ static const struct token t_show_rib[] =
{ ASTYPE, "as", AS_ALL, t_show_rib_as},
{ KEYWORD, "avs", NONE, t_show_avs},
{ FLAG, "best", F_CTL_BEST, t_show_rib},
- { KEYWORD, "community",NONE, t_show_community},
+ { COMMUNITY,"community",NONE, t_show_rib},
{ FLAG, "detail", F_CTL_DETAIL, t_show_rib},
{ ASTYPE, "empty-as", AS_EMPTY, t_show_rib},
{ FLAG, "error",F_CTL_INVALID, t_show_rib},
- { KEYWORD, "ext-community", NONE, t_show_extcommunity},
+ { EXTCOMMUNITY, "ext-community", NONE, t_show_rib},
{ FLAG, "in", F_CTL_ADJ_IN, t_show_rib},
{ FLAG, "invalid", F_CTL_INELIGIBLE, t_show_rib},
- { KEYWORD, "large-community", NONE,t_show_largecommunity},
+ { LRGCOMMUNITY, "large-community", NONE,t_show_rib},
{ FLAG, "leaked", F_CTL_LEAKED, t_show_rib},
{ KEYWORD, "memory", SHOW_RIB_MEM, NULL},
{ KEYWORD, "neighbor", NONE, t_show_rib_neigh},
@@ -336,39 +327,6 @@ static const struct token t_show_ip[] =
{ ENDTOKEN, "", NONE, NULL}
};
-static const struct token t_show_community[] = {
- { COMMUNITY,"", NONE, t_show_rib},
- { ENDTOKEN, "", NONE, NULL}
-};
-
-static const struct token t_show_extcommunity[] = {
- { EXTCOM_SUBTYPE, "bdc", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "defgw",NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "esi-lab", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "esi-rt", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "l2vid",NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "mac-mob", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "odi", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "ori", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "ort", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "ovs", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "rt", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "soo", NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "srcas",NONE, t_show_ext_subtype},
- { EXTCOM_SUBTYPE, "vrfri",NONE, t_show_ext_subtype},
- { ENDTOKEN, "", NONE, NULL}
-};
-
-static const struct token t_show_ext_subtype[] = {
- { EXTCOMMUNITY, "", NONE, t_show_rib},
- { ENDTOKEN, "", NONE, NULL}
-};
-
-static const struct token t_show_largecommunity[] = {
- { LARGE_COMMUNITY, "", NONE,
Re: bgpd first bunch of flowspec code
On Thu, Apr 13, 2023 at 03:35:33PM +0200, Claudio Jeker wrote: > If you have better suggestions on how to work with such a blob I'm happy > to hear. I tried to keep the madness in one place and have a mostly sane > API on top. I see and understand that and you did that wrapping probably as nicely as these RFCs' madness allows. I do have some vague ideas that I would like to discuss with you when we meet soon. > > #define FLOWSPEC_OP_NUM_FALSE 0x00 > > #define FLOWSPEC_OP_NUM_TRUE0x07 > > > > and use them in the switch in flowspec_fmt_num_op(). > > Torn about that. The concept of TRUE and FALSE is so rediculously > stupid that I'm not sure it is something we should promote out of dark > basement it should remain in. Understood. The real reason I suggested it is that it would have made the true/false mistake more immediately obvious. I'm fine with 0 and 0x7 if you prefer that. > Updated version of the diff below. It includes now a few more formatting > functions. This looks good to me. I spent quite a bit of time on the code now and couldn't spot anything anymore. Let's land it and improve in tree as needed. ok tb
malloc leak detection available in -current
Hi, OpenBSD current now has built-in malloc leak detection. Make sure you run current and have debug symbols (OpenBSD base libraries have debug symbols, compile your own program with -g). To record the leak report: $ MALLOC_OPTIONS=D ktrace -tu a.out To view the leak report: $ kdump -u malloc Example output: Start dump a.out *** M=8 I=1 F=0 U=0 J=1 R=0 X=0 C=0 cache=64 G=0 Leak report: f sum #avg 0x0 1088864 9722112 addr2line -e '?' 0x0 0xf4b73093c 31136278112 addr2line -e a.out 0x1093c End dump a.out *** $ addr2line -e a.out 0x1093c /home/otto/x.c:6 Some additional info: The null "f" values (call sites) are due to the sampling nature of small allocations. Recording all call sites of all potential leaks introduces too much overhead. Note that aggresssive optimizations might confuse the line numbers reported. For -static programs, compile with -nopie to make addr2line work. In some cases will want to use the packaged version of addr2line (gaddr2line, in the binutils package) as the base addr2line does not grok all debug info formats. -Otto
