Re: ospf6d: rework rde_lsdb.c
On Sat, Feb 15, 2020 at 11:37:12AM +0100, Denis Fondras wrote:
> 3 changes in rde_lsdb.c
> - lsa_find_lsid() has redondant parameters
> - call to lsa_self() can be simplified (== ospfd)
> - update debug messages to be more suitable
>
ok remi@
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/rde.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 rde.c
> --- rde.c 21 Jan 2020 15:17:12 - 1.83
> +++ rde.c 27 Jan 2020 17:11:52 -
> @@ -455,17 +455,10 @@ rde_dispatch_imsg(int fd, short event, v
>
> rde_req_list_del(nbr, &lsa->hdr);
>
> - self = lsa_self(lsa);
> - if (self) {
> - if (v == NULL)
> - /* LSA is no longer announced,
> - * remove by premature aging. */
> - lsa_flush(nbr, lsa);
> - else
> - lsa_reflood(v, lsa);
> - } else if (lsa_add(nbr, lsa))
> - /* delayed lsa, don't flood yet */
> - break;
> + if (!(self = lsa_self(nbr, lsa, v)))
> + if (lsa_add(nbr, lsa))
> + /* delayed lsa */
> + break;
>
> /* flood and perhaps ack LSA */
> imsg_compose_event(iev_ospfe, IMSG_LS_FLOOD,
> @@ -1683,8 +1676,7 @@ orig_asext_lsa(struct kroute *kr, u_int1
> memcpy((char *)lsa + sizeof(struct lsa_hdr) + sizeof(struct lsa_asext),
> &kr->prefix, LSA_PREFIXSIZE(kr->prefixlen));
>
> - lsa->hdr.ls_id = lsa_find_lsid(&asext_tree, lsa->hdr.type,
> - lsa->hdr.adv_rtr, comp_asext, lsa);
> + lsa->hdr.ls_id = lsa_find_lsid(&asext_tree, comp_asext, lsa);
>
> if (age == MAX_AGE) {
> /* inherit metric and ext_tag from the current LSA,
> Index: rde.h
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/rde.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 rde.h
> --- rde.h 21 Jan 2020 15:17:12 - 1.24
> +++ rde.h 27 Jan 2020 17:11:52 -
> @@ -145,9 +145,7 @@ void vertex_nexthop_add(struct vertex
> const struct in6_addr *, u_int32_t);
> int lsa_newer(struct lsa_hdr *, struct lsa_hdr *);
> int lsa_check(struct rde_nbr *, struct lsa *, u_int16_t);
> -int lsa_self(struct lsa *);
> -void lsa_flush(struct rde_nbr *, struct lsa *);
> -void lsa_reflood(struct vertex *, struct lsa*);
> +int lsa_self(struct rde_nbr *, struct lsa *, struct vertex *);
> int lsa_add(struct rde_nbr *, struct lsa *);
> void lsa_del(struct rde_nbr *, struct lsa_hdr *);
> void lsa_age(struct vertex *);
> @@ -156,7 +154,7 @@ struct vertex *lsa_find_rtr(struct area
> struct vertex*lsa_find_rtr_frag(struct area *, u_int32_t, unsigned
> int);
> struct vertex*lsa_find_tree(struct lsa_tree *, u_int16_t, u_int32_t,
> u_int32_t);
> -u_int32_t lsa_find_lsid(struct lsa_tree *, u_int16_t, u_int32_t,
> +u_int32_t lsa_find_lsid(struct lsa_tree *,
> int (*)(struct lsa *, struct lsa *), struct lsa *);
> u_int16_t lsa_num_links(struct vertex *);
> void lsa_snap(struct rde_nbr *);
> Index: rde_lsdb.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/rde_lsdb.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 rde_lsdb.c
> --- rde_lsdb.c21 Jan 2020 15:17:13 - 1.42
> +++ rde_lsdb.c27 Jan 2020 17:11:52 -
> @@ -192,7 +192,7 @@ lsa_check(struct rde_nbr *nbr, struct ls
> return (0);
> }
> if (ntohs(lsa->hdr.len) != len) {
> - log_warnx("lsa_check: bad packet size");
> + log_warnx("lsa_check: bad packet length");
> return (0);
> }
>
> @@ -244,7 +244,7 @@ lsa_check(struct rde_nbr *nbr, struct ls
> }
> metric = ntohl(lsa->data.pref_sum.metric);
> if (metric & ~LSA_METRIC_MASK) {
> - log_warnx("lsa_check: bad LSA summary metric");
> + log_warnx("lsa_check: bad LSA prefix summary metric");
> return (0);
> }
> if (lsa_get_prefix(((char *)lsa) + sizeof(lsa->hdr) +
> @@ -263,7 +263,7 @@ lsa_check(struct rde_nbr *nbr, struct ls
> }
> metric = ntohl(lsa->data.rtr_sum.metric);
> if (metric & ~LSA_METRIC_MASK) {
>
Re: em(4) towards multiqueues
On 14.2.2020. 18:28, Martin Pieuchot wrote: > I'm running this on: > > em0 at pci1 dev 0 function 0 "Intel I210" rev 0x03: msi > em0 at pci0 dev 20 function 0 "Intel I354 SGMII" rev 0x03: msi > > More tests are always welcome ;) em0 at pci0 dev 25 function 0 "Intel 82579LM" rev 0x04: msi em1 at pci1 dev 0 function 1 "Intel 82571EB" rev 0x06: apic 0 int 17 em4 at pci2 dev 0 function 1 "Intel 82576" rev 0x01: msi em5 at pci3 dev 0 function 0 "Intel 82572EI" rev 0x06: apic 0 int 16 em0 at pci9 dev 0 function 0 "Intel I350" rev 0x01: msi it just works .. :)
Re: snmp(1) fix parse agent
On 2/17/20 8:12 AM, Martijn van Duren wrote:
> jan@ found some issues with this diff during u2k20.
> Here's an updated diff. Changes since previous diff are:
> - If we don't know the protocol yet, we can't assume a port is
> specified.
> - If we can create a ip6? socket don't wait to connect until we're out
> of the loop.
>
> It passes everything I've thrown at it, but it's still extremely
> finicky, so more scrutiny/testing is welcome.
>
> OK?
now with 100% more diff...
>
> martijn@
>
> On 1/13/20 12:48 PM, Martijn van Duren wrote:
>> So apparently I use IPv6 more often than the end-users.
>> This diff allows us to actually skip the {ud,tc}p6 prefix if we actually
>> present an IPv6 address as described in the manpage and done by
>> net-snmp.
>>
>> While here I also made it possible possible to do a retry if
>> IPv6-address:port fails by pasting the port back to the hostname.
>> This allows us to do snmp walk ::1, similar to what net-snmp does.
>> Not that with this we actually comform better to what snmpcmd(1)
>> describes in their manpage, which apparently doesn't support ::1:161,
>> but only [::1]:161.
>>
>> OK?
>>
>> martijn@
>>
>>
>
Index: snmpc.c
===
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.21
diff -u -p -r1.21 snmpc.c
--- snmpc.c 25 Jan 2020 17:17:31 - 1.21
+++ snmpc.c 17 Feb 2020 07:18:54 -
@@ -1192,7 +1192,6 @@ snmpc_parseagent(char *agent, char *defa
hints.ai_socktype = SOCK_STREAM;
} else if (strcasecmp(specifier, "unix") == 0) {
hints.ai_family = AF_UNIX;
- hints.ai_socktype = SOCK_STREAM;
hints.ai_addr = (struct sockaddr *)&saddr;
hints.ai_addrlen = sizeof(saddr);
saddr.sun_len = sizeof(saddr);
@@ -1202,60 +1201,67 @@ snmpc_parseagent(char *agent, char *defa
errx(1, "Hostname path too long");
ai = &hints;
} else {
- port = hostname;
+ *--hostname = ':';
hostname = specifier;
- specifier = NULL;
- hints.ai_family = AF_INET;
- hints.ai_socktype = SOCK_DGRAM;
- }
- if (port == NULL) {
- if (hints.ai_family == AF_INET) {
- if ((port = strchr(hostname, ':')) != NULL)
- *port++ = '\0';
- } else if (hints.ai_family == AF_INET6) {
- if (hostname[0] == '[') {
- hostname++;
- if ((port = strchr(hostname, ']')) ==
NULL)
- errx(1, "invalid agent");
- *port++ = '\0';
- if (port[0] == ':')
- *port++ = '\0';
- else
- port = NULL;
- } else {
- if ((port = strrchr(hostname, ':')) ==
NULL)
- errx(1, "invalid agent");
- *port++ = '\0';
- }
- }
}
} else {
hostname = specifier;
- hints.ai_family = AF_INET;
- hints.ai_socktype = SOCK_DGRAM;
+ }
+
+ if (hints.ai_family == AF_INET) {
+ if ((port = strchr(hostname, ':')) != NULL)
+ *port++ = '\0';
+ } else if (hints.ai_family == AF_INET6 || hints.ai_family == 0) {
+ if (hostname[0] == '[') {
+ hints.ai_family = AF_INET6;
+ hostname++;
+ if ((port = strchr(hostname, ']')) == NULL)
+ errx(1, "invalid agent");
+ *port++ = '\0';
+ if (port[0] == ':')
+ *port++ = '\0';
+ else if (port[0] == '\0')
+ port = NULL;
+ else
+ errx(1, "invalid agent");
+ } else {
+ if ((port = strrchr(hostname, ':')) != NULL)
+ *port++ = '\0';
+ }
}
if (hints.ai_family != AF_UNIX) {
+ if (hints.ai_socktype == 0)
+ hints.ai_socktype = SOCK_DGRAM;
if (port == NULL)
port = defaultport;
error = getaddrinfo(hostname, port, &hints, &ai0);
-
Re: snmp(1) fix parse agent
jan@ found some issues with this diff during u2k20.
Here's an updated diff. Changes since previous diff are:
- If we don't know the protocol yet, we can't assume a port is
specified.
- If we can create a ip6? socket don't wait to connect until we're out
of the loop.
It passes everything I've thrown at it, but it's still extremely
finicky, so more scrutiny/testing is welcome.
OK?
martijn@
On 1/13/20 12:48 PM, Martijn van Duren wrote:
> So apparently I use IPv6 more often than the end-users.
> This diff allows us to actually skip the {ud,tc}p6 prefix if we actually
> present an IPv6 address as described in the manpage and done by
> net-snmp.
>
> While here I also made it possible possible to do a retry if
> IPv6-address:port fails by pasting the port back to the hostname.
> This allows us to do snmp walk ::1, similar to what net-snmp does.
> Not that with this we actually comform better to what snmpcmd(1)
> describes in their manpage, which apparently doesn't support ::1:161,
> but only [::1]:161.
>
> OK?
>
> martijn@
>
>
Re: bgpd.conf.5: Tag groups
On Sun, Feb 16, 2020 at 09:47:43PM +0100, Ingo Schwarze wrote: > Sure. I'm not convinced you even need OKs for such simple tagging > corrections. > > By the way, just > > .Tg > .Ic neighbor > > .Tg > .Ic group > > is sufficient, the .Tg macro defaults to the first argument of the first > macro on the following line, if any. That's why I didn't commit without feedback. I'm a) new to BGP and b) wasn't sure whether that markup was 100% correct. Thanks.
Re: bgpd.conf.5: Tag groups
Hi Klemens,
Klemens Nanni wrote on Sun, Feb 16, 2020 at 09:32:52PM +0100:
> On Sun, Feb 16, 2020 at 09:25:51PM +0100, Klemens Nanni wrote:
>> Going through the example with bgpd.conf(5) side by side, jumping to the
>> "group" tag shows
>>
>> group descr Neighbors in this group will be matched.
>> AS as-number
>> Neighbors with this AS will be matched.
>> ...
>>
>> which is somewhere in the manual that assumes knowing what groups are.
>> mdoc's automatic tagging failed here, so markup `Ic group' in proper
>> section:
>>
>> NEIGHBORS AND GROUPS
>> bgpd(8) establishes TCP connections to other BGP speakers called
>> neighbors. A neighbor and its properties are specified by a
>> neighbor
>> section:
>>
>> neighbor 10.0.0.2 {
>> remote-as 65002
>> descr "a neighbor"
>> }
>>
>> Neighbors placed within a group section inherit the properties
>> common to
>> that group:
>> ...
>>
>> This makes typing ":tgroup" jump to that very last sentence.
> Same for `neighbor' in the very same section as well.
>
> OK?
Sure. I'm not convinced you even need OKs for such simple tagging
corrections.
By the way, just
.Tg
.Ic neighbor
.Tg
.Ic group
is sufficient, the .Tg macro defaults to the first argument of the first
macro on the following line, if any.
Yours,
Ingo
> Index: bgpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
> retrieving revision 1.200
> diff -u -p -r1.200 bgpd.conf.5
> --- bgpd.conf.5 9 Feb 2020 14:21:26 - 1.200
> +++ bgpd.conf.5 16 Feb 2020 20:31:46 -
> @@ -683,6 +683,7 @@ can be chosen by the local operator.
> establishes TCP connections to other BGP speakers called
> .Em neighbors .
> A neighbor and its properties are specified by a
> +.Tg neighbor
> .Ic neighbor
> section:
> .Bd -literal -offset indent
> @@ -693,6 +694,7 @@ neighbor 10.0.0.2 {
> .Ed
> .Pp
> Neighbors placed within a
> +.Tg group
> .Ic group
> section inherit the properties common to that group:
> .Bd -literal -offset indent
Re: bgpd.conf.5: Tag groups
On Sun, Feb 16, 2020 at 09:25:51PM +0100, Klemens Nanni wrote:
> Going through the example with bgpd.conf(5) side by side, jumping to the
> "group" tag shows
>
> group descr Neighbors in this group will be matched.
> AS as-number
>Neighbors with this AS will be matched.
> ...
>
> which is somewhere in the manual that assumes knowing what groups are.
> mdoc's automatic tagging failed here, so markup `Ic group' in proper
> section:
>
> NEIGHBORS AND GROUPS
>bgpd(8) establishes TCP connections to other BGP speakers called
>neighbors. A neighbor and its properties are specified by a
> neighbor
>section:
>
>neighbor 10.0.0.2 {
>remote-as 65002
>descr "a neighbor"
>}
>
>Neighbors placed within a group section inherit the properties
> common to
>that group:
>...
>
> This makes typing ":tgroup" jump to that very last sentence.
Same for `neighbor' in the very same section as well.
OK?
Index: bgpd.conf.5
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.200
diff -u -p -r1.200 bgpd.conf.5
--- bgpd.conf.5 9 Feb 2020 14:21:26 - 1.200
+++ bgpd.conf.5 16 Feb 2020 20:31:46 -
@@ -683,6 +683,7 @@ can be chosen by the local operator.
establishes TCP connections to other BGP speakers called
.Em neighbors .
A neighbor and its properties are specified by a
+.Tg neighbor
.Ic neighbor
section:
.Bd -literal -offset indent
@@ -693,6 +694,7 @@ neighbor 10.0.0.2 {
.Ed
.Pp
Neighbors placed within a
+.Tg group
.Ic group
section inherit the properties common to that group:
.Bd -literal -offset indent
bgpd.conf.5: Tag groups
Going through the example with bgpd.conf(5) side by side, jumping to the
"group" tag shows
group descr Neighbors in this group will be matched.
AS as-number
Neighbors with this AS will be matched.
...
which is somewhere in the manual that assumes knowing what groups are.
mdoc's automatic tagging failed here, so markup `Ic group' in proper
section:
NEIGHBORS AND GROUPS
bgpd(8) establishes TCP connections to other BGP speakers called
neighbors. A neighbor and its properties are specified by a
neighbor
section:
neighbor 10.0.0.2 {
remote-as 65002
descr "a neighbor"
}
Neighbors placed within a group section inherit the properties
common to
that group:
...
This makes typing ":tgroup" jump to that very last sentence.
Feedback? OK?
Index: bgpd.conf.5
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.200
diff -u -p -r1.200 bgpd.conf.5
--- bgpd.conf.5 9 Feb 2020 14:21:26 - 1.200
+++ bgpd.conf.5 16 Feb 2020 20:15:50 -
@@ -693,6 +693,7 @@ neighbor 10.0.0.2 {
.Ed
.Pp
Neighbors placed within a
+.Tg group
.Ic group
section inherit the properties common to that group:
.Bd -literal -offset indent
examples/bgpd.conf: Remove trailing backslashes
Reading it for the first time, I thought `mynetworks' is special and
that's why it has these line continuations, but it's not and the example
looks much nicer without.
The `bogons' block directly following doesn't use it, either.
Feedback? OK?
Index: etc/examples/bgpd.conf
===
RCS file: /cvs/src/etc/examples/bgpd.conf,v
retrieving revision 1.17
diff -u -p -r1.17 bgpd.conf
--- etc/examples/bgpd.conf 30 Nov 2019 02:31:12 - 1.17
+++ etc/examples/bgpd.conf 16 Feb 2020 19:04:41 -
@@ -9,9 +9,9 @@ AS $ASN
router-id 192.0.2.1
# list of networks that may be originated by our ASN
-prefix-set mynetworks {\
- 192.0.2.0/24\
- 2001:db8:abcd::/48 \
+prefix-set mynetworks {
+ 192.0.2.0/24
+ 2001:db8:abcd::/48
}
include "/var/db/rpki-client/openbgpd"
Set UF_EXCLOSE inside finishdup()
The closing of a file can be a complex operation, and hence it would be
good to hold as few locks as possible when calling closef(). In
particular, the code behind close(2) already releases the file
descriptor table lock before closef(). This might not be strictly
necessary when the type of the file is known beforehand. However,
general file descriptor operations should be careful.
The following diff moves the setting of the UF_EXCLOSE fd flag inside
finishdup(). This makes possible to release fdplock before closef() in
finishdup() in a subsequent patch.
This changes the order of setting UF_EXCLOSE relative to
knote_fdclose(). However, that should not be a problem because the flag
concerns file descriptor handling and not kqueue.
OK?
Index: kern/kern_descrip.c
===
RCS file: src/sys/kern/kern_descrip.c,v
retrieving revision 1.198
diff -u -p -r1.198 kern_descrip.c
--- kern/kern_descrip.c 5 Feb 2020 17:03:13 - 1.198
+++ kern/kern_descrip.c 16 Feb 2020 16:56:14 -
@@ -82,6 +82,9 @@ int finishdup(struct proc *, struct file
int find_last_set(struct filedesc *, int);
int dodup3(struct proc *, int, int, int, register_t *);
+#define DUPF_CLOEXEC 0x01
+#define DUPF_DUP2 0x02
+
struct pool file_pool;
struct pool fdesc_pool;
@@ -350,7 +353,7 @@ dodup3(struct proc *p, int old, int new,
{
struct filedesc *fdp = p->p_fd;
struct file *fp;
- int i, error;
+ int dupflags, error, i;
restart:
if ((fp = fd_getfile(fdp, old)) == NULL)
@@ -385,10 +388,13 @@ restart:
panic("dup2: fdalloc");
fd_unused(fdp, new);
}
+
+ dupflags = DUPF_DUP2;
+ if (flags & O_CLOEXEC)
+ dupflags |= DUPF_CLOEXEC;
+
/* No need for FRELE(), finishdup() uses current ref. */
- error = finishdup(p, fp, old, new, retval, 1);
- if (!error && flags & O_CLOEXEC)
- fdp->fd_ofileflags[new] |= UF_EXCLOSE;
+ error = finishdup(p, fp, old, new, retval, dupflags);
out:
fdpunlock(fdp);
@@ -440,11 +446,13 @@ restart:
goto restart;
}
} else {
- /* No need for FRELE(), finishdup() uses current ref. */
- error = finishdup(p, fp, fd, i, retval, 0);
+ int dupflags = 0;
+
+ if (SCARG(uap, cmd) == F_DUPFD_CLOEXEC)
+ dupflags |= DUPF_CLOEXEC;
- if (!error && SCARG(uap, cmd) == F_DUPFD_CLOEXEC)
- fdp->fd_ofileflags[i] |= UF_EXCLOSE;
+ /* No need for FRELE(), finishdup() uses current ref. */
+ error = finishdup(p, fp, fd, i, retval, dupflags);
}
fdpunlock(fdp);
@@ -645,7 +653,7 @@ out:
*/
int
finishdup(struct proc *p, struct file *fp, int old, int new,
-register_t *retval, int dup2)
+register_t *retval, int dupflags)
{
struct file *oldfp;
struct filedesc *fdp = p->p_fd;
@@ -659,7 +667,7 @@ finishdup(struct proc *p, struct file *f
}
oldfp = fd_getfile(fdp, new);
- if (dup2 && oldfp == NULL) {
+ if ((dupflags & DUPF_DUP2) && oldfp == NULL) {
if (fd_inuse(fdp, new)) {
FRELE(fp, p);
return (EBUSY);
@@ -676,6 +684,8 @@ finishdup(struct proc *p, struct file *f
mtx_leave(&fdp->fd_fplock);
fdp->fd_ofileflags[new] = fdp->fd_ofileflags[old] & ~UF_EXCLOSE;
+ if (dupflags & DUPF_CLOEXEC)
+ fdp->fd_ofileflags[new] |= UF_EXCLOSE;
*retval = new;
if (oldfp != NULL) {
Re: Raise spl for updating kn_status
On Sat, Feb 15, 2020 at 09:42:53PM +0100, Martin Pieuchot wrote: > On 15/02/20(Sat) 16:56, Visa Hankala wrote: > > When I added the knote_acquire() call in kqueue_register(), I overlooked > > the fact that the knote could be modified by a (soft) interrupt. > > Interrupts have to be blocked when changing kn_status. Otherwise the > > state could become confused. > > Can the same knote be modified by different interrupt handlers or are we > just considering a race between process and interrupt contexts? In theory, the same knote could be modified by different interrupt handlers if the associated event source used more than one interrupt. The kernel lock already ensures that only one CPU at at time can access a knote. This diff fixes a situation where the interrupt context is not blocked properly. The particular case in my mind is timer events. However, the same issue applies to all situations where activation can happen from interrupt context. > > The diff below adds splhigh() where kn_status is modified. This also > > ensures that the steps of knote_activate() run as an uninterrupted > > sequence. As a consequence, knote_enqueue() and knote_dequeue() can rely > > on the caller to raise the spl. > > Do you think it would make sense to document which fields required a SPL > protection just like we started doing with locks? Maybe, though I am somewhat hesitant to start adding more formalism. With mutexes the SPL requirement is implied, though not necessarily fully accurate. For example, the same mutex can protect some fields that are accessed from process context only and some other fields that are accessed from both process and interrupt contexts. With tricky code, instead of SPL it might be good to indicate somehow if multiple contexts are involved though.
dumpfs: don't pick alternate superblock
Hi, If the block size is 64k, the first alternate ffs1 superblock ends up in a location first looked at by dumpfs. fsck_ffs(8) (see setup.c) and ffs_mountfs() in sys/ufs/ffs/ffs_vfsops.c have protection against that case, since we really want the primary superblock, that's the one that is actually updated when a fs is used. So also do that in dumpfs(8). OK? -Otto Index: dumpfs.c === RCS file: /cvs/src/sbin/dumpfs/dumpfs.c,v retrieving revision 1.34 diff -u -p -r1.34 dumpfs.c --- dumpfs.c28 Jun 2019 13:32:43 - 1.34 +++ dumpfs.c16 Feb 2020 12:24:31 - @@ -139,6 +139,8 @@ open_disk(const char *name) if (n == SBLOCKSIZE && (afs.fs_magic == FS_UFS1_MAGIC || (afs.fs_magic == FS_UFS2_MAGIC && afs.fs_sblockloc == sbtry[i])) && + !(afs.fs_magic == FS_UFS1_MAGIC && + sbtry[i] == SBLOCK_UFS2) && afs.fs_bsize <= MAXBSIZE && afs.fs_bsize >= sizeof(struct fs)) break;
Re: iked.conf.5: Add BUGS section
On Fri, Feb 14, 2020 at 08:56:25PM +0100, Klemens Nanni wrote: > I'm experiencing some issues with iked(8) and the first one took > me much longer than appreciated: order of commands matters. iked/iked.conf is not special, ipsecctl/ipsec.conf obviously has it, too. So while the problem of undocumented order does exist, I no longer think BUGS is the right place for it. Either ways, all effected tools ought to be fixed, not just iked. pfctl/pf.conf for example does expect a certain order of keywords as well, yet it provides syntax in BNF which rules out any ambiguity. Thus, I drop this diff (but keep the lesson).
Re: iked.conf.5, ipsec.conf.5: Quote $domain in tag string
On Sun, Feb 16, 2020 at 12:23:40AM +0100, Klemens Nanni wrote: > On Sat, Feb 15, 2020 at 10:30:52PM +, Jason McIntyre wrote: > > from a practical point of view, is there a reason to say when expansion > > happens? by this i mean, what (if any) difference does it have for the > > user - they will specify this in the conf file anyway, no? > Macros are expanded by the parser at parse time, whereas variables are > read as ordinary strings and left unmodified; hence, quoted `"$domain"' > gets passed to the daemon as is, which substitutes proper values before > passing it to the kernel. `$domain' without quotes never makes it to > the daemon, that is with `domain = foo' somewhere else "foo" is being > eventually passed unmodified to the kernel. > > Macro: > > $ echo 'ike esp from ::1 to ::2 tag $domain' | ipsecctl -vnf- | grep > PF-Tag > stdin: 1: macro 'domain' not defined > stdin: 1: syntax error > ipsecctl: Syntax error in config file: ipsec rules not loaded > $ echo 'ike esp from ::1 to ::2 tag $domain' | ipsecctl -Ddomain=foo > -vnf- | grep PF-Tag > C set [from-::1-to-::2]:PF-Tag=foo force > > Variable: > > $ echo 'ike esp from ::1 to ::2 tag "$domain"' | ipsecctl -vnf- | grep > PF-Tag > C set [from-::1-to-::2]:PF-Tag=$domain force > $ echo 'ike esp from ::1 to ::2 tag "$domain"' | ipsecctl -Ddomain=foo > -vnf- | grep PF-Tag > C set [from-::1-to-::2]:PF-Tag=$domain force > > > > if it doesn;t have to be said, we could knock out the whole runtime > > sentence. > > > > if it does have to be said (i realise i may be overlooking something > > obvious here) could we be smarter about making the text shorter? > It briefly outlines the above mentioned, so I'd like to retain it. > > Strictly speaking, it must only be quoted if the tag string _starts_ > with a dollar sign, but that is parser specific and I explicitly want > to advise general quoting of variables: > > $ echo 'ike esp from ::1 to ::2 tag ipsec-$domain' | ipsecctl -vnf- | > grep PF-Tag > C set [from-::1-to-::2]:PF-Tag=ipsec-$domain force > > > The variable expansion for the > > .Ar tag > > directive only occurs at runtime (not when the file is parsed) > > and must be quoted, or it will be interpreted as a macro. > That reads fine, I incorporated your wording, thanks. > > OK? > yep, ok by me. jmc > > Index: sbin/iked/iked.conf.5 > === > RCS file: /cvs/src/sbin/iked/iked.conf.5,v > retrieving revision 1.61 > diff -u -p -r1.61 iked.conf.5 > --- sbin/iked/iked.conf.5 10 Feb 2020 13:18:20 - 1.61 > +++ sbin/iked/iked.conf.5 15 Feb 2020 23:19:20 - > @@ -64,7 +64,7 @@ for more information about manual keying > is divided into three main sections: > .Bl -tag -width > .It Sy Macros > -User-defined variables may be defined and used later, simplifying the > +User-defined macros may be defined and used later, simplifying the > configuration file. > .It Sy Global Configuration > Global settings for > @@ -643,7 +643,8 @@ expands to > .Dq ipsec-example.com . > The variable expansion for the > .Ar tag > -directive occurs only at runtime, not during configuration file parse time. > +directive occurs only at runtime (not when the file is parsed) > +and must be quoted, or it will be interpreted as a macro. > .It Ic tap Ar interface > Send the decapsulated IPsec traffic to the specified > .Xr enc 4 @@ -766,7 +767,7 @@ configuration and also sets an alternati > device: > .Bd -literal -offset indent > ikev2 esp from 10.1.1.0/24 to 10.1.2.0/24 peer 192.168.3.2 \e > - tag ipsec-$domain tap "enc1" > + tag "ipsec-$domain" tap "enc1" > .Ed > .Sh OUTGOING NETWORK ADDRESS TRANSLATION > In some network topologies it is desirable to perform NAT on traffic leaving > Index: sbin/ipsecctl/ipsec.conf.5 > === > RCS file: /cvs/src/sbin/ipsecctl/ipsec.conf.5,v > retrieving revision 1.158 > diff -u -p -r1.158 ipsec.conf.5 > --- sbin/ipsecctl/ipsec.conf.510 Feb 2020 13:18:20 - 1.158 > +++ sbin/ipsecctl/ipsec.conf.515 Feb 2020 23:19:43 - > @@ -466,7 +466,8 @@ expands to > .Dq ipsec-bar.org . > The variable expansion for the > .Ar tag > -directive occurs only at runtime, not during configuration file parse time. > +directive occurs only at runtime (not when the file is parsed) > +and must be quoted, or it will be interpreted as a macro. > .El > .Sh PACKET FILTERING > IPsec traffic appears unencrypted on the > @@ -575,7 +576,7 @@ The tags will be assigned by the followi > example: > .Bd -literal -offset indent > ike esp from 10.1.1.0/24 to 10.1.2.0/24 peer 192.168.3.2 \e > - tag ipsec-$domain > + tag "ipsec-$domain" > .Ed > .Sh OUTGOING NETWORK ADDRESS TRANSLATION > In some network topologies it is desirable to perform NAT on traffic leaving >
Re: vmctl.8, vm.conf.5: DHCP is configured on the first interface only
On Sun, Feb 16, 2020 at 06:35:10AM +0100, Theo Buehler wrote: > I don't think this patch is correct. > > The vmctl part contradicts the "LOCAL INTERFACES" section, which > explains how the addresses are calculated and also states: > > Multiple -L options can be provided to the 'vmctl start' command, > if more than one interface is desired. > > # rcctl -f start vmd > vmd(ok) > # vmctl start -b /bsd.rd -c -L -L test > [...] > Welcome to the OpenBSD/amd64 6.6 installation program. > (I)nstall, (U)pgrade, (A)utoinstall or (S)hell? s > # dhclient vio0 > vio0: 100.64.1.3 lease accepted from 100.64.1.2 (fe:e1:bb:d1:bc:46) > # dhclient vio1 > vio1: 100.64.1.5 lease accepted from 100.64.1.4 (fe:e1:bb:d2:3a:8e) > > In vm.conf, the 'local' keyword applies per interface. It's perfectly > valid to have several 'local interface' lines for the same VM in vm.conf > and the corresponding vio interfaces will get leases from the built-in > dhcp server. Oh, you are correct. I completely missed that part from vmctl.5's "LOCAL INTERFACES" section. Reading `-L's description itself and the fact that it functions as a boolean switch contrary to how `-i' expects a number, I made the wrong assumption that it can only work for the first interface. -L Add a local network interface. vmd(8) will auto- generate an IPv4 subnet for the interface, configure a gateway address on the VM host side, and run a simple DHCP/BOOTP server for the VM. See LOCAL INTERFACES "a local network interface" and "for the interface" supported my false believes, so I dismissed the referenced section which goes into all the details about most but apparently not all details I already know. My second mistake was to imply analogue behaviour for the configuration. Now that you stated the obvious about `local' being per `interface' line, it makes absoloutely no sense to above mentioned behaviour for static VM definitions. I will revert my change and think about whether the vmctl(8) bits can be improved, vm.conf(5) is totally clear. Thank you Theo and sorry for such sloppiness.
arm64 syscall ABI change
In order to fix a speculative execution issue on various ARM CPUs, the OpenBSD/arm64 system call ABI has been changed. System calls now skip the two instructions immediately following the system call instruction. This allows us to insert a barrier that blocks the CPU from speculating further without a significant performance penalty. The speculative execution issue was originally brought to our attention by Anthony Steinhauser. This changed was rolled out in a way such that a smooth transition over the ABI bump is possible. As usual, we recommend updating your systems using snapshots. But if you want to upgrade from source, make sure you have a userland built from sources dated after January 25th before booting a new kernel. Note that old static binaries (built on a userland from before January 25th) will break. Code that rolls its own system calls (a practice we strongly discourage) will need to be adjusted. Fixes for go (which rolls its own system calls) have been committed already, and the latest arm64 package snapshot sould work fine. Cheers, Mark
