ip local delivery

[Alexander Bluhm]

Hi,

I want merge my diff that avoids queueing in the IP protocol input
path and mpi@'s diff that queues local packets to unlock the
forwarding path.

One step is to rename ip_local() to ip_deliver() and give it the
same parameters as the pr_input functions.

I also added an assert that IPv4 delivery ends in IP proto done.
This is necessary to check that IPv4 protocol functions work like
IPv6.

ok?

bluhm

Index: netinet/ip_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.304
diff -u -p -r1.304 ip_input.c
--- netinet/ip_input.c  22 May 2017 22:23:11 -  1.304
+++ netinet/ip_input.c  28 May 2017 00:58:14 -
@@ -564,26 +564,25 @@ found:
ip_freef(fp);
}
 
-   ip_local(m, hlen, ip->ip_p);
+   ip_deliver(, , ip->ip_p, AF_INET);
return;
 bad:
m_freem(m);
 }
 
 void
-ip_local(struct mbuf *m, int off, int nxt)
+ip_deliver(struct mbuf **mp, int *offp, int nxt, int af)
 {
KERNEL_ASSERT_LOCKED();
 
/* pf might have modified stuff, might have to chksum */
-   in_proto_cksum_out(m, NULL);
+   in_proto_cksum_out(*mp, NULL);
 
 #ifdef IPSEC
if (ipsec_in_use) {
-   if (ipsec_local_check(m, off, nxt, AF_INET) != 0) {
+   if (ipsec_local_check(*mp, *offp, nxt, af) != 0) {
ipstat_inc(ips_cantforward);
-   m_freem(m);
-   return;
+   goto bad;
}
}
/* Otherwise, just fall through and deliver the packet */
@@ -593,7 +592,11 @@ ip_local(struct mbuf *m, int off, int nx
 * Switch out to protocol's input routine.
 */
ipstat_inc(ips_delivered);
-   (*inetsw[ip_protox[nxt]].pr_input)(, , nxt, AF_INET);
+   nxt = (*inetsw[ip_protox[nxt]].pr_input)(mp, offp, nxt, af);
+   KASSERT(nxt == IPPROTO_DONE);
+   return;
+ bad:
+   m_freem(*mp);
 }
 
 int
Index: netinet/ip_var.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.75
diff -u -p -r1.75 ip_var.h
--- netinet/ip_var.h22 May 2017 22:23:11 -  1.75
+++ netinet/ip_var.h28 May 2017 00:47:44 -
@@ -249,7 +249,7 @@ void ip_savecontrol(struct inpcb *, str
struct mbuf *);
 voidipintr(void);
 voidipv4_input(struct mbuf *);
-voidip_local(struct mbuf *, int, int);
+voidip_deliver(struct mbuf **, int *, int, int);
 voidip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int);
 int rip_ctloutput(int, struct socket *, int, int, struct mbuf *);
 voidrip_init(void);
Index: netinet/ipsec_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_input.c,v
retrieving revision 1.153
diff -u -p -r1.153 ipsec_input.c
--- netinet/ipsec_input.c   22 May 2017 22:23:11 -  1.153
+++ netinet/ipsec_input.c   28 May 2017 00:49:10 -
@@ -583,7 +583,7 @@ ipsec_common_input_cb(struct mbuf *m, st
 
 #if NPF > 0
/*
-* The ip_local() shortcut avoids running through ip_input() with the
+* The ip_deliver() shortcut avoids running through ip_input() with the
 * same IP header twice.  Packets in transport mode have to be be
 * passed to pf explicitly.  In tunnel mode the inner IP header will
 * run through ip_input() and pf anyway.
@@ -609,11 +609,11 @@ ipsec_common_input_cb(struct mbuf *m, st
/* Call the appropriate IPsec transform callback. */
switch (af) {
case AF_INET:
-   ip_local(m, skip, prot);
+   ip_deliver(, , prot, af);
return;
 #ifdef INET6
case AF_INET6:
-   ip6_local(m, skip, prot);
+   ip6_deliver(, , prot, af);
return;
 #endif /* INET6 */
default:
Index: netinet6/ip6_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.189
diff -u -p -r1.189 ip6_input.c
--- netinet6/ip6_input.c23 May 2017 08:13:10 -  1.189
+++ netinet6/ip6_input.c28 May 2017 00:47:44 -
@@ -387,7 +387,7 @@ ip6_input(struct mbuf *m)
ip6stat_inc(ip6s_cantforward);
m_freem(m);
} else if (ours) {
-   ip6_local(m, off, nxt);
+   ip6_deliver(, , nxt, AF_INET6);
} else {
m_freem(m);
}
@@ -465,7 +465,7 @@ ip6_input(struct mbuf *m)
 
if (ours) {
KERNEL_LOCK();
-   ip6_local(m, off, nxt);
+   ip6_deliver(, , nxt, 

Re: vmd read/write pfunctions

[Mike Larkin]

On Sat, May 27, 2017 at 07:24:30PM -0400, Ted Unangst wrote:
> using pread and pwrite is a little shorter and half the syscalls.
> 

ok mlarkin. thanks!

> Index: virtio.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 virtio.c
> --- virtio.c  11 May 2017 07:31:20 -  1.46
> +++ virtio.c  27 May 2017 23:20:23 -
> @@ -335,14 +335,7 @@ vioblk_do_read(struct vioblk_dev *dev, o
>   return (NULL);
>   }
>  
> - if (lseek(dev->fd, sector * VIRTIO_BLK_SECTOR_SIZE,
> - SEEK_SET) == -1) {
> - log_warn("seek error in vioblk read");
> - free(buf);
> - return (NULL);
> - }
> -
> - if (read(dev->fd, buf, sz) != sz) {
> + if (pread(dev->fd, buf, sz, sector * VIRTIO_BLK_SECTOR_SIZE) != sz) {
>   log_warn("vioblk read error");
>   free(buf);
>   return (NULL);
> @@ -354,13 +347,7 @@ vioblk_do_read(struct vioblk_dev *dev, o
>  static int
>  vioblk_do_write(struct vioblk_dev *dev, off_t sector, char *buf, ssize_t sz)
>  {
> - if (lseek(dev->fd, sector * VIRTIO_BLK_SECTOR_SIZE,
> - SEEK_SET) == -1) {
> - log_warn("seek error in vioblk write");
> - return (1);
> - }
> -
> - if (write(dev->fd, buf, sz) != sz) {
> + if (pwrite(dev->fd, buf, sz, sector * VIRTIO_BLK_SECTOR_SIZE) != sz) {
>   log_warn("vioblk write error");
>   return (1);
>   }
> 

vmd read/write pfunctions

[Ted Unangst]

using pread and pwrite is a little shorter and half the syscalls.

Index: virtio.c
===
RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
retrieving revision 1.46
diff -u -p -r1.46 virtio.c
--- virtio.c11 May 2017 07:31:20 -  1.46
+++ virtio.c27 May 2017 23:20:23 -
@@ -335,14 +335,7 @@ vioblk_do_read(struct vioblk_dev *dev, o
return (NULL);
}
 
-   if (lseek(dev->fd, sector * VIRTIO_BLK_SECTOR_SIZE,
-   SEEK_SET) == -1) {
-   log_warn("seek error in vioblk read");
-   free(buf);
-   return (NULL);
-   }
-
-   if (read(dev->fd, buf, sz) != sz) {
+   if (pread(dev->fd, buf, sz, sector * VIRTIO_BLK_SECTOR_SIZE) != sz) {
log_warn("vioblk read error");
free(buf);
return (NULL);
@@ -354,13 +347,7 @@ vioblk_do_read(struct vioblk_dev *dev, o
 static int
 vioblk_do_write(struct vioblk_dev *dev, off_t sector, char *buf, ssize_t sz)
 {
-   if (lseek(dev->fd, sector * VIRTIO_BLK_SECTOR_SIZE,
-   SEEK_SET) == -1) {
-   log_warn("seek error in vioblk write");
-   return (1);
-   }
-
-   if (write(dev->fd, buf, sz) != sz) {
+   if (pwrite(dev->fd, buf, sz, sector * VIRTIO_BLK_SECTOR_SIZE) != sz) {
log_warn("vioblk write error");
return (1);
}

Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Adam Wolk]

On Sat, May 27, 2017 at 10:58:40PM +0100, Jason McIntyre wrote:
> On Sat, May 27, 2017 at 11:45:43PM +0200, Adam Wolk wrote:
> > Index: chown.8
> > ===
> > RCS file: /cvs/src/bin/chmod/chown.8,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 chown.8
> > --- chown.8 31 Dec 2015 23:38:16 -  1.20
> > +++ chown.8 27 May 2017 21:37:48 -
> > @@ -166,7 +166,12 @@ Previous versions of the
> >  utility used the dot
> >  .Pq Sq \&.
> >  character to distinguish the group name.
> > -This has been changed to be a colon
> > +This has been changed when the utility was first
> 
> s/has been/was/
> 
> > +standardised in
> > +.St -p1003.2-92
> > +to be a colon
> >  .Pq Sq \&:
> > -character so that user and
> > -group names may contain the dot character.
> > +character so that user and group names may contain the dot
> 
> s/may/could/
> or
> s/so that user and group names may/to allow user and group names to/
> 
> > +character, however the dot separator still remains supported
> 
> s/however/though/
> 
> > +due to widely required backwards compatibility.
> > +
> 
> jmc
> 

Thanks!

Included updated diffs with suggested changes applied.

Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 -  1.8
+++ Makefile27 May 2017 22:04:37 -
@@ -1,7 +1,6 @@
 #  $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
 
 PROG=  chmod
-CFLAGS+=-DSUPPORT_DOT
 MAN=   chmod.1 chgrp.1 chown.8 chflags.1
 LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 -  1.41
+++ chmod.c 27 May 2017 22:04:37 -
@@ -197,14 +197,16 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
-   /* UID and GID are separated by a dot and UID exists. */
+   /*
+* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever
+*/
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 -  1.20
+++ chown.8 27 May 2017 22:04:37 -
@@ -166,7 +166,11 @@ Previous versions of the
 utility used the dot
 .Pq Sq \&.
 character to distinguish the group name.
-This has been changed to be a colon
+This was changed when the utility was first standardised in
+.St -p1003.2-92
+to be a colon
 .Pq Sq \&:
-character so that user and
-group names may contain the dot character.
+character to allow user and group names to contain the dot
+character, though the dot separator still remains supported
+due to widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 -   1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
-   chown -LR root.wheel $_file
+   chown -LR root:wheel $_file
fi
 
# Check for ifconfig'able interface, except if -n option is specified.

Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Jason McIntyre]

On Sat, May 27, 2017 at 11:45:43PM +0200, Adam Wolk wrote:
> Index: chown.8
> ===
> RCS file: /cvs/src/bin/chmod/chown.8,v
> retrieving revision 1.20
> diff -u -p -r1.20 chown.8
> --- chown.8   31 Dec 2015 23:38:16 -  1.20
> +++ chown.8   27 May 2017 21:37:48 -
> @@ -166,7 +166,12 @@ Previous versions of the
>  utility used the dot
>  .Pq Sq \&.
>  character to distinguish the group name.
> -This has been changed to be a colon
> +This has been changed when the utility was first

s/has been/was/

> +standardised in
> +.St -p1003.2-92
> +to be a colon
>  .Pq Sq \&:
> -character so that user and
> -group names may contain the dot character.
> +character so that user and group names may contain the dot

s/may/could/
or
s/so that user and group names may/to allow user and group names to/

> +character, however the dot separator still remains supported

s/however/though/

> +due to widely required backwards compatibility.
> +

jmc

Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Adam Wolk]

On Sat, May 27, 2017 at 11:01:29PM +0200, Adam Wolk wrote:
> On Sat, May 27, 2017 at 01:42:45PM -0600, Theo de Raadt wrote:
> > I agree with you.  Maybe change the comment
> > 
> > /* UID and GID are separated by a dot and UID exists. */
> > 
> > to say a bit more on the matter, to prevent a zealot from arriving 2-3
> > years from now and proposing removal. Just a few words to hint . support
> > will stay forever.
> > 
> > It seems the sentences in the man page could be changed a bit.  Rather
> > than speaking about Previous versions, it could say POSIX (rev?)
> > deprecated '.' and introduced ':' as the default seperator, however '.'
> > seperator support remains for widely required backwards compat.  The current
> > sentences speak a bit too strongly about '.' actually being gone.
> > 
> > 
> 
> Updated the man page and expanded the comment in code.
> 
> Attaching updated diffs, OK?
> 

- style(9) the chmod.c comment
- use .St syntax to mark the standard in the man page instead of manually
  hard coding the name

both issues pointed out by brynet@, thanks!

Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 -  1.8
+++ Makefile27 May 2017 21:37:48 -
@@ -1,7 +1,6 @@
 #  $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
 
 PROG=  chmod
-CFLAGS+=-DSUPPORT_DOT
 MAN=   chmod.1 chgrp.1 chown.8 chflags.1
 LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 -  1.41
+++ chmod.c 27 May 2017 21:37:48 -
@@ -197,14 +197,16 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
-   /* UID and GID are separated by a dot and UID exists. */
+   /*
+* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever
+*/
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 -  1.20
+++ chown.8 27 May 2017 21:37:48 -
@@ -166,7 +166,12 @@ Previous versions of the
 utility used the dot
 .Pq Sq \&.
 character to distinguish the group name.
-This has been changed to be a colon
+This has been changed when the utility was first
+standardised in
+.St -p1003.2-92
+to be a colon
 .Pq Sq \&:
-character so that user and
-group names may contain the dot character.
+character so that user and group names may contain the dot
+character, however the dot separator still remains supported
+due to widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 -   1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
-   chown -LR root.wheel $_file
+   chown -LR root:wheel $_file
fi
 
# Check for ifconfig'able interface, except if -n option is specified.

Re: mcast-proxy daemon

[Paul de Weerd]

Hi tech@,

I've been talking to Rafael about this piece of code over the last
week a bit, and have tried it on my home TV setup.  My ISP has a
multicast setup for watching television, with their set-top-box
connected to my TV.  Up until now, I was using net/igmpproxy on my
OpenBSD gateway to get the streams from ISP to STB.

At first, Rafael's code didn't work for me, but after some back and
forth with him we got things going with the below diff (to be applied
on top of Rafael's diff).  Obviously some things are still missing:

- runs as user _dhcp
- make install gives you /mcast-proxy
- lacks an rc.d init script
- no pledge(2) for the important part of the code

However, this is already an incredible jump forward in terms of
usability (sane defaults!) and stability, let alone safe coding
practices over igmpproxy: runs as root, doesn't drop privs, doesn't
chroot, etc.

Compare my old /etc/igmpproxy.conf with my new /etc/mcast-proxy.conf:

--- /etc/igmpproxy.conf --
quickleave
phyint vlan4 upstream ratelimit 0 threshold 1 altnet 10.0.0.0/8
phyint vlan10 downstream ratelimit 0 threshold 1
phyint lo0 disabled
phyint em0 disabled
phyint em1 disabled
phyint em2 disabled
phyint em3 disabled
phyint em4 disabled
phyint em5 disabled
phyint bridge0 disabled
phyint bridge1 disabled
phyint bridge20 disabled
phyint vlan20 disabled
phyint vlan21 disabled
--

--- /etc/mcast-proxy.conf 
interface vlan4 {
upstream
source 10.0.0.0/8
}

interface vlan10 {
downstream
}
--

It would be really awesome to have something like this in base.  Your
networking kit really isn't complete without a multicast proxying
solution!  (plus, it's nice to have something that exercises these
codepaths in base).

Cheers,

Paul 'WEiRD' de Weerd

PS: Please note that any mistakes in the below diff are mine; Rafael
guided me through writing this over e-mail.  Sending this out so
others can help test, as Rafael mentioned he'd be AFK for a few days.

diff -ur a/mcast-proxy.c b/mcast-proxy.c
--- a/mcast-proxy.c Sat May 27 22:20:02 2017
+++ b/mcast-proxy.c Sat May 27 18:25:53 2017
@@ -475,24 +475,6 @@
return 0;
}
 
-   /* IP header validations. */
-   if (ip->ip_v != IPVERSION) {
-   log_debug("%s: wrong IP version", __func__);
-   return 0;
-   }
-   hlen = ip->ip_hl << 2;
-   if (hlen < sizeof(*ip)) {
-   log_debug("%s: wrong IP header length", __func__);
-   return 0;
-   }
-   if ((ip->ip_off & IP_OFFMASK) != 0) {
-   log_debug("%s: fragmented packet", __func__);
-   return 0;
-   }
-   if (ip->ip_ttl == 0) {
-   log_debug("%s: invalid TTL", __func__);
-   return 0;
-   }
if (ip->ip_src.s_addr == INADDR_ANY ||
ip->ip_dst.s_addr == INADDR_ANY) {
log_debug("%s: invalid packet addresses", __func__);
@@ -525,6 +507,25 @@
log_debug("%s: expected IGMP message, got %d",
__func__, ip->ip_p);
return NULL;
+   }
+   /* IP header validations. */
+   if (ip->ip_v != IPVERSION) {
+   log_debug("%s: wrong IP version", __func__);
+   return 0;
+   }
+   hlen = ip->ip_hl << 2;
+   if (hlen < sizeof(*ip)) {
+   log_debug("%s: wrong IP header length", __func__);
+   return 0;
+   }
+/* Disable check to see if it makes a difference */
+/* if ((ip->ip_off & IP_OFFMASK) != 0) {
+   log_debug("%s: fragmented packet", __func__);
+   return 0;
+   }*/
+   if (ip->ip_ttl == 0) {
+   log_debug("%s: invalid TTL", __func__);
+   return 0;
}
 
hlen = ip->ip_hl << 2;
diff -ur a/mrt.c b/mrt.c
--- a/mrt.c Sat May 27 22:20:02 2017
+++ b/mrt.c Sat May 27 22:10:01 2017
@@ -514,7 +514,7 @@
 {
struct sockaddr_storage ss;
 
-   if (mr->mr_upstream == NULL) {
+   if (upstreamif == NULL) {
log_debug("%s: no upstream interface", __func__);
return;
}
@@ -538,7 +538,7 @@
if (LIST_EMPTY(>mr_molist))
return;
 
-   mcast_join(mr->mr_upstream, );
+   mcast_join(upstreamif, );
mr->mr_state = MS_JOINED;
break;
 
@@ -547,7 +547,7 @@
if (!LIST_EMPTY(>mr_molist))
return;
 
-   mcast_leave(mr->mr_upstream, );
+   mcast_leave(upstreamif, );
mr->mr_state = MS_NOTJOINED;
break;
 


On Fri, May 19, 2017 at 06:25:01PM +0200, Rafael Zalamena wrote:
| Hello tech@,
| 
| I 

Re: remove waf from port-modules(5)

[Vadim Zhukov]

2017-05-27 22:45 GMT+03:00 Joerg Jung :
> Hi,
>
> I think devel/waf is gone since two years and may not come back, so no
> need to mention in port-modules(5).
>
> OK?

Yes, and don't forget to bring some gas for nice burning!

Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Adam Wolk]

On Sat, May 27, 2017 at 01:42:45PM -0600, Theo de Raadt wrote:
> I agree with you.  Maybe change the comment
> 
> /* UID and GID are separated by a dot and UID exists. */
> 
> to say a bit more on the matter, to prevent a zealot from arriving 2-3
> years from now and proposing removal. Just a few words to hint . support
> will stay forever.
> 
> It seems the sentences in the man page could be changed a bit.  Rather
> than speaking about Previous versions, it could say POSIX (rev?)
> deprecated '.' and introduced ':' as the default seperator, however '.'
> seperator support remains for widely required backwards compat.  The current
> sentences speak a bit too strongly about '.' actually being gone.
> 
> 

Updated the man page and expanded the comment in code.

Attaching updated diffs, OK?

Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 -  1.8
+++ Makefile27 May 2017 20:53:36 -
@@ -1,7 +1,6 @@
 #  $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
 
 PROG=  chmod
-CFLAGS+=-DSUPPORT_DOT
 MAN=   chmod.1 chgrp.1 chown.8 chflags.1
 LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 -  1.41
+++ chmod.c 27 May 2017 20:53:36 -
@@ -197,14 +197,14 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
-   /* UID and GID are separated by a dot and UID exists. */
+   /* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever */
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 -  1.20
+++ chown.8 27 May 2017 20:53:36 -
@@ -166,7 +166,12 @@ Previous versions of the
 utility used the dot
 .Pq Sq \&.
 character to distinguish the group name.
-This has been changed to be a colon
+This has been changed when the utility was first
+standardised in POSIX.2 (IEEE Std 1003.2-1992)
+to be a colon
 .Pq Sq \&:
 character so that user and
-group names may contain the dot character.
+group names may contain the dot character, however
+the dot separator still remains supported due to
+widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 -   1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
-   chown -LR root.wheel $_file
+   chown -LR root:wheel $_file
fi
 
# Check for ifconfig'able interface, except if -n option is specified.

Re: ext2fs: Validate file size when lengthening

[Ted Unangst]

Steven McDonald wrote:
> Hi there,
> 
> ext2fs currently has #if 0 around the size check for lengthening a
> file. This allows ftruncate(2) calls for very large files to succeed,
> but result in a smaller file than was requested. I noticed the problem
> while trying to create vmd(8) images on an ext2fs filesystem.
> 
> Below is a patch that fixes the problem, followed by a program that
> demonstrates the problem.
> 
> I am not certain this patch is correct. A bit further down in
> ext2fs_inode.c, we find:

The code has been like this since prehistory, so we don't know why the check
was disabled. But obviously there's a problem without the check.

If any other ext2fs users would like to test and verify, I'll give them some
time to do so, but otherwise I guess we take the patch and see what happens.
Thanks.

remove waf from port-modules(5)

[Joerg Jung]

Hi,

I think devel/waf is gone since two years and may not come back, so no
need to mention in port-modules(5).

OK?

Regards,
Joerg



Index: share/man/man5/port-modules.5
===
RCS file: /cvs/src/share/man/man5/port-modules.5,v
retrieving revision 1.217
diff -u -p -r1.217 port-modules.5
--- share/man/man5/port-modules.5   22 Apr 2017 14:00:30 -  1.217
+++ share/man/man5/port-modules.5   27 May 2017 19:37:38 -
@@ -795,26 +795,6 @@ It provides a
 and
 .Cm do-install
 targets that can be overridden in the port Makefile.
-.It devel/waf
-Adds
-.Pa devel/waf
-to
-.Ev BUILD_DEPENDS ,
-.Pa lang/python
-to
-.Ev MODULES ,
-and provides
-.Cm do-configure ,
-.Cm do-build ,
-.Cm do-install
-and
-.Cm post-install
-targets.
-.Cm do-build ,
-.Cm do-install
-and
-.Cm post-install
-can be overridden in the port Makefile.
 .It font
 .It fortran
 Sets

Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Theo de Raadt]

I agree with you.  Maybe change the comment

/* UID and GID are separated by a dot and UID exists. */

to say a bit more on the matter, to prevent a zealot from arriving 2-3
years from now and proposing removal. Just a few words to hint . support
will stay forever.

It seems the sentences in the man page could be changed a bit.  Rather
than speaking about Previous versions, it could say POSIX (rev?)
deprecated '.' and introduced ':' as the default seperator, however '.'
seperator support remains for widely required backwards compat.  The current
sentences speak a bit too strongly about '.' actually being gone.

tcpdump/pfctl_osfp.c: catch up

[Michal Mazurek]

Catch up with pfctl/pfctl_osfp.c, no binary change.

Index: usr.sbin/tcpdump/pfctl_osfp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/pfctl_osfp.c,v
retrieving revision 1.11
diff -u -p -r1.11 pfctl_osfp.c
--- usr.sbin/tcpdump/pfctl_osfp.c   5 Dec 2015 19:27:17 -   1.11
+++ usr.sbin/tcpdump/pfctl_osfp.c   27 May 2017 19:22:33 -
@@ -33,17 +33,10 @@
 #include 
 #include 
 
-
 #include "privsep.h"
 #include "pfctl_parser.h"
 
-#ifndef MIN
-# define MIN(a,b)  (((a) < (b)) ? (a) : (b))
-#endif /* MIN */
-#ifndef MAX
-# define MAX(a,b)  (((a) > (b)) ? (a) : (b))
-#endif /* MAX */
-
+#define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
 
 #if 0
 # define DEBUG(fp, str, v...) \
@@ -647,7 +640,7 @@ import_fingerprint(struct pf_osfp_ioctl 
nm_class = fingerprint_name_entry(, fp->fp_os.fp_class_nm);
if (nm_class->nm_num == 0) {
nm_class->nm_num = class;
-   class_count = MAX(class_count, class);
+   class_count = MAXIMUM(class_count, class);
}
 
nm_version = fingerprint_name_entry(_class->nm_sublist,
@@ -655,7 +648,7 @@ import_fingerprint(struct pf_osfp_ioctl 
if (nm_version) {
if (nm_version->nm_num == 0) {
nm_version->nm_num = version;
-   nm_class->nm_sublist_num = MAX(nm_class->nm_sublist_num,
+   nm_class->nm_sublist_num = 
MAXIMUM(nm_class->nm_sublist_num,
version);
}
nm_subtype = fingerprint_name_entry(_version->nm_sublist,
@@ -664,7 +657,7 @@ import_fingerprint(struct pf_osfp_ioctl 
if (nm_subtype->nm_num == 0) {
nm_subtype->nm_num = subtype;
nm_version->nm_sublist_num =
-   MAX(nm_version->nm_sublist_num, subtype);
+   MAXIMUM(nm_version->nm_sublist_num, 
subtype);
}
}
}
@@ -754,7 +747,6 @@ sort_name_list(int opts, struct name_lis
LIST_INSERT_AFTER(nmlast, nm, nm_entry);
nmlast = nm;
}
-   return;
 }
 
 /* parse the next integer in a formatted config file line */
@@ -823,7 +815,7 @@ get_int(u_char **line, size_t *len, int 
}
 
for (; i < fieldlen; i++) {
-   if (field[i] < '0'  || field[i] > '9') {
+   if (field[i] < '0' || field[i] > '9') {
fprintf(stderr, "%s:%d non-digit character in %s\n",
filename, lineno, name);
return (1);
@@ -959,7 +951,7 @@ get_tcpopts(const char *filename, int li
return (0);
 }
 
-/* rip the next field ouf of a formatted config file line */
+/* rip the next field out of a formatted config file line */
 char *
 get_field(u_char **line, size_t *len, int *fieldlen)
 {


-- 
Michal Mazurek

chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years

[Adam Wolk]

Hi tech@,

I stumbled on SUPPORT_DOT while reading /usr/src/bin/chmod.c, got curious
and started doing some research.

POSIX changed the separator from . to : to make the utility properly work with
usernames containing a dot. The standard doesn't forbid keeping the dot handling
for backwards compatiblity.

The code is currently #ifdef'ed in. I assume the reason was to phase it out
sometime in the future.

The code was there and enabled with CFLAGS back in 1995
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/chown/Attic/Makefile?rev=1.1=text/x-cvsweb-markup

There were some attempts to weed it out but as far as I see they were abandonned
or stopped.

Back in 2001, by disabling the compat and trying to build the base system, no
followup email (that I can find): 
https://marc.info/?l=openbsd-tech=99647882113533=2

Discussion that brought SUPPORT_DOT into the topic. Mostly people argumenting if
man pages shold be altered (sorry, can't find the thread on marc.info):

http://misc.openbsd.narkive.com/4ejjhI6O/in-username

I think it's unlikely at this point that this backwards support will go away.
Linux, Mac, NetBSD and FreeBSD all support the compat, people seem to be using a
mix of both (including in our base where ie. /etc/netstart uses the dot
notation).

I suggest dropping the ifdef and define. It's been built enabled by default for
22 years. I'm also adding a diff for /etc/netstart to switch it to the :
separator. It's one less strchr call, though that obviously doesn't make
much difference performance wise in this case.

Feedback? OK's?
? chmod
? support_dot.diff
Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 -  1.8
+++ Makefile27 May 2017 18:39:17 -
@@ -1,7 +1,6 @@
 #  $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
 
 PROG=  chmod
-CFLAGS+=-DSUPPORT_DOT
 MAN=   chmod.1 chgrp.1 chown.8 chflags.1
 LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 -  1.41
+++ chmod.c 27 May 2017 18:39:17 -
@@ -197,14 +197,12 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
/* UID and GID are separated by a dot and UID exists. */
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 -   1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
-   chown -LR root.wheel $_file
+   chown -LR root:wheel $_file
fi
 
# Check for ifconfig'able interface, except if -n option is specified.

Re: trunk(4) vs splnet

[Alexandr Nedvedicky]

Hello,

On Sat, May 27, 2017 at 08:45:31PM +0200, Martin Pieuchot wrote:
> On 27/05/17(Sat) 17:33, Alexandr Nedvedicky wrote:
> > Hello,
> > 
> > On Fri, May 26, 2017 at 04:54:57PM +0200, Martin Pieuchot wrote:
> > > The global list of softc is *not* accessed in the input path, so it
> > > doesn't need splnet().
> > > 
> > > ioctl(2) handlers are already executed with the NET_LOCK() held, so
> > > splnet() is superfluous.
> > 
> > changes look good to me, but still have a question:
> > 
> > what is the plan for splnet()/splx() in trunk_init() and trunk_stop()
> > functions. It seems to me they are executed through ioctl(2) only,
> > so we can also let them go. or you want to leave it for another day?
> 
> You're right!  Plus these functions are only present for lacp and just
> schedule a timeout.
> 
> So we can get rid of them as well.  Updated diff below.
> 

it looks good to me

OK sashan@

Re: pflow 1/2

[Martin Pieuchot]

On 27/05/17(Sat) 12:58, Sebastian Benoit wrote:
> (benno_pflow_try4_task.diff)
> 
> ok?

Two nits below with that ok mpi@

> diff --git sys/net/if_pflow.c sys/net/if_pflow.c
> index a40fe23862b..20ab4e0e88d 100644
> --- sys/net/if_pflow.c
> +++ sys/net/if_pflow.c
> @@ -67,6 +67,7 @@ struct pflowstatspflowstats;
>  void pflowattach(int);
>  int  pflow_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
>   struct rtentry *rt);
> +void pflow_output_process(void *);
>  int  pflow_clone_create(struct if_clone *, int);
>  int  pflow_clone_destroy(struct ifnet *);
>  int  pflow_set(struct pflow_softc *, struct pflowreq *);
> @@ -124,11 +125,25 @@ pflow_output(struct ifnet *ifp, struct mbuf *m, struct 
> sockaddr *dst,
>   return (EAFNOSUPPORT);
>  }
>  
> +void
> +pflow_output_process(void *arg)
> +{
> +struct pflow_softc *sc = arg;

Too many spaces.

> + struct mbuf *m;
> +
> + KERNEL_LOCK();
> + while ((m = ml_dequeue(>sc_outputqueue)) != NULL) {
> + pflow_sendout_mbuf(sc, m);
> + }
> + KERNEL_UNLOCK();
> +}
> +
>  int
>  pflow_clone_create(struct if_clone *ifc, int unit)
>  {
>   struct ifnet*ifp;
>   struct pflow_softc  *pflowif;
> + int  s;
>  
>   if ((pflowif = malloc(sizeof(*pflowif),
>   M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
> @@ -241,13 +256,18 @@ pflow_clone_create(struct if_clone *ifc, int unit)
>   ifp->if_hdrlen = PFLOW_HDRLEN;
>   ifp->if_flags = IFF_UP;
>   ifp->if_flags &= ~IFF_RUNNING;  /* not running, need receiver */
> + ml_init(>sc_outputqueue);
>   pflow_setmtu(pflowif, ETHERMTU);
>   pflow_init_timeouts(pflowif);
>   if_attach(ifp);
>   if_alloc_sadl(ifp);
>  
> + task_set(>sc_outputtask, pflow_output_process, pflowif);
> +
>   /* Insert into list of pflows */
> + NET_LOCK(s);
>   SLIST_INSERT_HEAD(_list, pflowif, sc_next);
> + NET_UNLOCK(s);
>   return (0);
>  }
>  
> @@ -267,6 +287,7 @@ pflow_clone_destroy(struct ifnet *ifp)
>   if (timeout_initialized(>sc_tmo_tmpl))
>   timeout_del(>sc_tmo_tmpl);
>   pflow_flush(sc);
> + task_del(softnettq, >sc_outputtask);

You also need to call ml_purge() on your queue here.

>   m_freem(sc->send_nam);
>   if (sc->so != NULL) {
>   error = soclose(sc->so);
> @@ -462,14 +483,8 @@ pflowioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
>   ifp->if_flags |= IFF_RUNNING;
>   sc->sc_gcounter=pflowstats.pflow_flows;
>   /* send templates on startup */
> - if (sc->sc_version == PFLOW_PROTO_10) {
> - /* XXXSMP breaks atomicity */
> - rw_exit_write();
> - s = splnet();
> + if (sc->sc_version == PFLOW_PROTO_10)
>   pflow_sendout_ipfix_tmpl(sc);
> - splx(s);
> - rw_enter_write();
> - }
>   } else
>   ifp->if_flags &= ~IFF_RUNNING;
>   break;
> @@ -513,17 +528,16 @@ pflowioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
>   s = splnet();
>   error = pflow_set(sc, );
>   splx(s);
> - if (error != 0)
> + if (error != 0) {
> + rw_enter_write();
>   return (error);
> + }
>  
>   if ((ifp->if_flags & IFF_UP) && sc->so != NULL) {
>   ifp->if_flags |= IFF_RUNNING;
>   sc->sc_gcounter=pflowstats.pflow_flows;
> - if (sc->sc_version == PFLOW_PROTO_10) {
> - s = splnet();
> + if (sc->sc_version == PFLOW_PROTO_10)
>   pflow_sendout_ipfix_tmpl(sc);
> - splx(s);
> - }
>   } else
>   ifp->if_flags &= ~IFF_RUNNING;
>  
> @@ -852,14 +866,11 @@ export_pflow_if(struct pf_state *st, struct 
> pf_state_key *sk,
>  int
>  copy_flow_to_m(struct pflow_flow *flow, struct pflow_softc *sc)
>  {
> - int s, ret = 0;
> + int ret = 0;
>  
> - s = splnet();
>   if (sc->sc_mbuf == NULL) {
> - if ((sc->sc_mbuf = pflow_get_mbuf(sc, 0)) == NULL) {
> - splx(s);
> + if ((sc->sc_mbuf = pflow_get_mbuf(sc, 0)) == NULL)
>   return (ENOBUFS);
> - }
>   }
>   m_copyback(sc->sc_mbuf, PFLOW_HDRLEN +
>   (sc->sc_count * sizeof(struct pflow_flow)),
> @@ -873,20 +884,17 @@ copy_flow_to_m(struct pflow_flow *flow, struct 
> pflow_softc *sc)
>   if (sc->sc_count >= sc->sc_maxcount)
>   ret = pflow_sendout_v5(sc);
>  
> - splx(s);
>   return(ret);
>  }
>  
>  int

Re: trunk(4) vs splnet

[Martin Pieuchot]

On 27/05/17(Sat) 17:33, Alexandr Nedvedicky wrote:
> Hello,
> 
> On Fri, May 26, 2017 at 04:54:57PM +0200, Martin Pieuchot wrote:
> > The global list of softc is *not* accessed in the input path, so it
> > doesn't need splnet().
> > 
> > ioctl(2) handlers are already executed with the NET_LOCK() held, so
> > splnet() is superfluous.
> 
> changes look good to me, but still have a question:
> 
> what is the plan for splnet()/splx() in trunk_init() and trunk_stop()
> functions. It seems to me they are executed through ioctl(2) only,
> so we can also let them go. or you want to leave it for another day?

You're right!  Plus these functions are only present for lacp and just
schedule a timeout.

So we can get rid of them as well.  Updated diff below.

Index: net/if_trunk.c
===
RCS file: /cvs/src/sys/net/if_trunk.c,v
retrieving revision 1.129
diff -u -p -r1.129 if_trunk.c
--- net/if_trunk.c  22 Jan 2017 10:17:39 -  1.129
+++ net/if_trunk.c  27 May 2017 18:43:57 -
@@ -210,19 +210,15 @@ trunk_clone_destroy(struct ifnet *ifp)
 {
struct trunk_softc *tr = (struct trunk_softc *)ifp->if_softc;
struct trunk_port *tp;
-   int error, s;
+   int error;
 
/* Remove any multicast groups that we may have joined. */
trunk_ether_purgemulti(tr);
 
-   s = splnet();
-
/* Shutdown and remove trunk ports, return on error */
while ((tp = SLIST_FIRST(>tr_ports)) != NULL) {
-   if ((error = trunk_port_destroy(tp)) != 0) {
-   splx(s);
+   if ((error = trunk_port_destroy(tp)) != 0)
return (error);
-   }
}
 
ifmedia_delete_instance(>tr_media, IFM_INST_ANY);
@@ -232,8 +228,6 @@ trunk_clone_destroy(struct ifnet *ifp)
SLIST_REMOVE(_list, tr, trunk_softc, tr_entries);
free(tr, M_DEVBUF, sizeof *tr);
 
-   splx(s);
-
return (0);
 }
 
@@ -474,9 +468,7 @@ trunk_port_ioctl(struct ifnet *ifp, u_lo
struct trunk_reqport *rp = (struct trunk_reqport *)data;
struct trunk_softc *tr;
struct trunk_port *tp = NULL;
-   int s, error = 0;
-
-   s = splnet();
+   int error = 0;
 
/* Should be checked by the caller */
if (ifp->if_type != IFT_IEEE8023ADLAG ||
@@ -512,12 +504,9 @@ trunk_port_ioctl(struct ifnet *ifp, u_lo
goto fallback;
}
 
-   splx(s);
return (error);
 
  fallback:
-   splx(s);
-
if (tp != NULL)
error = (*tp->tp_ioctl)(ifp, cmd, data);
 
@@ -616,9 +605,7 @@ trunk_ioctl(struct ifnet *ifp, u_long cm
struct ifreq *ifr = (struct ifreq *)data;
struct trunk_port *tp;
struct ifnet *tpif;
-   int s, i, error = 0;
-
-   s = splnet();
+   int i, error = 0;
 
bzero(, sizeof(rpbuf));
 
@@ -768,7 +755,6 @@ trunk_ioctl(struct ifnet *ifp, u_long cm
}
 
  out:
-   splx(s);
return (error);
 }
 
@@ -1014,32 +1000,22 @@ void
 trunk_init(struct ifnet *ifp)
 {
struct trunk_softc *tr = (struct trunk_softc *)ifp->if_softc;
-   int s;
-
-   s = splnet();
 
ifp->if_flags |= IFF_RUNNING;
 
if (tr->tr_init != NULL)
(*tr->tr_init)(tr);
-
-   splx(s);
 }
 
 void
 trunk_stop(struct ifnet *ifp)
 {
struct trunk_softc *tr = (struct trunk_softc *)ifp->if_softc;
-   int s;
-
-   s = splnet();
 
ifp->if_flags &= ~IFF_RUNNING;
 
if (tr->tr_stop != NULL)
(*tr->tr_stop)(tr);
-
-   splx(s);
 }
 
 int

Re: exclude PF_KEY from solock like PF_ROUTE

[Alexander Bluhm]

On Sat, May 27, 2017 at 08:30:01PM +0200, Claudio Jeker wrote:
> Agreed, new diff

OK bluhm@

> 
> -- 
> :wq Claudio
> 
> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.184
> diff -u -p -r1.184 uipc_socket.c
> --- kern/uipc_socket.c15 May 2017 13:00:10 -  1.184
> +++ kern/uipc_socket.c27 May 2017 18:27:05 -
> @@ -1043,7 +1043,8 @@ sorflush(struct socket *so)
>  
>   sb->sb_flags |= SB_NOINTR;
>   sblock(sb, M_WAITOK,
> - (af != PF_LOCAL && af != PF_ROUTE) ?  : NULL);
> + (af != PF_LOCAL && af != PF_ROUTE && af != PF_KEY) ?
> +  : NULL);
>   socantrcvmore(so);
>   sbunlock(sb);
>   asb = *sb;
> Index: kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 uipc_socket2.c
> --- kern/uipc_socket2.c   15 May 2017 12:26:00 -  1.76
> +++ kern/uipc_socket2.c   27 May 2017 14:04:34 -
> @@ -273,7 +273,8 @@ solock(struct socket *so)
>   int s;
>  
>   if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
> - (so->so_proto->pr_domain->dom_family != PF_ROUTE))
> + (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
> + (so->so_proto->pr_domain->dom_family != PF_KEY))
>   NET_LOCK(s);
>   else
>   s = -42;
> @@ -292,7 +293,8 @@ void
>  soassertlocked(struct socket *so)
>  {
>   if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
> - (so->so_proto->pr_domain->dom_family != PF_ROUTE))
> + (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
> + (so->so_proto->pr_domain->dom_family != PF_KEY))
>   NET_ASSERT_LOCKED();
>  }
>  
> @@ -300,7 +302,8 @@ int
>  sosleep(struct socket *so, void *ident, int prio, const char *wmesg, int 
> timo)
>  {
>   if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
> - (so->so_proto->pr_domain->dom_family != PF_ROUTE)) {
> + (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
> + (so->so_proto->pr_domain->dom_family != PF_KEY)) {
>   return rwsleep(ident, , prio, wmesg, timo);
>   } else
>   return tsleep(ident, prio, wmesg, timo);
> Index: net/pfkeyv2.c
> ===
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.156
> diff -u -p -r1.156 pfkeyv2.c
> --- net/pfkeyv2.c 26 May 2017 19:11:20 -  1.156
> +++ net/pfkeyv2.c 27 May 2017 16:24:37 -
> @@ -795,7 +795,7 @@ pfkeyv2_get_proto_alg(u_int8_t satype, u
>  int
>  pfkeyv2_send(struct socket *socket, void *message, int len)
>  {
> - int i, j, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
> + int i, j, s, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
>   int delflag = 0;
>   struct sockaddr_encap encapdst, encapnetmask;
>   struct ipsec_policy *ipo, *tmpipo;
> @@ -820,7 +820,7 @@ pfkeyv2_send(struct socket *socket, void
>  
>   u_int rdomain;
>  
> - NET_ASSERT_LOCKED();
> + NET_LOCK(s);
>  
>   /* Verify that we received this over a legitimate pfkeyv2 socket */
>   bzero(headers, sizeof(headers));
> @@ -1356,7 +1356,6 @@ pfkeyv2_send(struct socket *socket, void
>   rval = tdb_walk(rdomain, pfkeyv2_dump_walker, _state);
>   if (!rval)
>   goto realret;
> -
>   if ((rval == ENOMEM) || (rval == ENOBUFS))
>   rval = 0;
>   }
> @@ -1690,6 +1689,8 @@ ret:
>   rval = pfkeyv2_sendmessage(headers, mode, socket, 0, 0, rdomain);
>  
>  realret:
> + NET_UNLOCK(s);
> +
>   if (freeme)
>   free(freeme, M_PFKEY, 0);
>  

Re: pfctl/pfctl_osfp.c: print_ioctl() used only in debug mode

[Alexander Bluhm]

On Sat, May 27, 2017 at 08:14:37PM +0200, Michal Mazurek wrote:
> print_ioctl() is unused if not debugging. Found with clang, after
> marking the function "static". Use OSFP_DEBUG, in a similar
> fashion to OPT_DEBUG (pfctl_optimize.c).
> 
> OK?

OK bluhm@

> 
> Index: sbin/pfctl/pfctl_osfp.c
> ===
> RCS file: /cvs/src/sbin/pfctl/pfctl_osfp.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 pfctl_osfp.c
> --- sbin/pfctl/pfctl_osfp.c   10 Dec 2015 17:27:00 -  1.23
> +++ sbin/pfctl/pfctl_osfp.c   27 May 2017 18:08:57 -
> @@ -39,7 +39,8 @@
>  
>  #define MAXIMUM(a, b)(((a) > (b)) ? (a) : (b))
>  
> -#if 0
> +/* #define OSFP_DEBUG1 */
> +#ifdef OSFP_DEBUG
>  # define DEBUG(fp, str, v...) \
>   fprintf(stderr, "%s:%s:%s " str "\n", (fp)->fp_os.fp_class_nm, \
>   (fp)->fp_os.fp_version_nm, (fp)->fp_os.fp_subtype_nm , ## v);
> @@ -74,7 +75,9 @@ int  get_tcpopts(const char *, int, co
>   pf_tcpopts_t *, int *, int *, int *, int *, int *,
>   int *);
>  void  import_fingerprint(struct pf_osfp_ioctl *);
> +#ifdef OSFP_DEBUG
>  const char   *print_ioctl(struct pf_osfp_ioctl *);
> +#endif
>  void  print_name_list(int, struct name_list *, const char *);
>  void  sort_name_list(int, struct name_list *);
>  struct name_entry*lookup_name_list(struct name_list *, const char *);
> @@ -990,6 +993,7 @@ get_field(char **line, size_t *len, int 
>  }
>  
>  
> +#ifdef OSFP_DEBUG
>  const char *
>  print_ioctl(struct pf_osfp_ioctl *fp)
>  {
> @@ -1091,3 +1095,4 @@ print_ioctl(struct pf_osfp_ioctl *fp)
>  
>   return (buf);
>  }
> +#endif
> 
> -- 
> Michal Mazurek

Re: memory barriers and atomic instructions

[Martin Pieuchot]

On 23/05/17(Tue) 17:41, Mark Kettenis wrote:
> So here is a diff that implements what I proposed recently.  This
> recognizes that atomic instructions on amd64 already include an
> implicit memory barrier and allows us to write optimized code that
> avoids a redundant memory barrier.
> 
> Note that I don't have a use-case for membar_exit_before_atomic() yet;
> I merely added it for symmetry reasons.  I can leave it out if that's
> what people prefer.
> 
> This should allow us to use a generic mutex implementation written in
> C without a significant penalty.

This will be used in userland futex-based mutexes as well.

ok mpi@

> Index: sys/atomic.h
> ===
> RCS file: /cvs/src/sys/sys/atomic.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 atomic.h
> --- sys/atomic.h  24 Jan 2017 22:22:20 -  1.4
> +++ sys/atomic.h  23 May 2017 15:01:34 -
> @@ -219,4 +219,12 @@ atomic_sub_long_nv(volatile unsigned lon
>  #define membar_sync() __sync_synchronize()
>  #endif
>  
> +#ifndef membar_enter_after_atomic
> +#define membar_enter_after_atomic() membar_enter()
> +#endif
> +
> +#ifndef membar_exit_before_atomic
> +#define membar_exit_before_atomic() membar_exit()
> +#endif
> +
>  #endif /* _SYS_ATOMIC_H_ */
> Index: arch/amd64/include/atomic.h
> ===
> RCS file: /cvs/src/sys/arch/amd64/include/atomic.h,v
> retrieving revision 1.19
> diff -u -p -r1.19 atomic.h
> --- arch/amd64/include/atomic.h   12 May 2017 08:47:03 -  1.19
> +++ arch/amd64/include/atomic.h   23 May 2017 15:02:23 -
> @@ -276,6 +276,9 @@ _atomic_sub_long_nv(volatile unsigned lo
>  #define membar_sync()__membar("")
>  #endif
>  
> +#define membar_enter_after_atomic()  __membar("")
> +#define membar_exit_before_atomic()  __membar("")
> +
>  #ifdef _KERNEL
>  
>  /* virtio needs MP membars even on SP kernels */
> 

Re: exclude PF_KEY from solock like PF_ROUTE

[Claudio Jeker]

On Sat, May 27, 2017 at 07:45:49PM +0200, Alexander Bluhm wrote:
> On Sat, May 27, 2017 at 06:37:48PM +0200, Claudio Jeker wrote:
> > Similar to the routing socket the pfkey socket only needs the
> > netlock further down (in pfkeyv2_send()). With this all users of
> > raw_usrreq are now not using the NET_LOCK which should simplify to push
> > the NET_LOCK down.
> 
> In sorflush() we have
> sa_family_t af = pr->pr_domain->dom_family;
> sblock(sb, M_WAITOK,
> (af != PF_LOCAL && af != PF_ROUTE) ?  : NULL);
> 
> I think you should keep that in sync and add PF_KEY here.
> 

Agreed, new diff

-- 
:wq Claudio

Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.184
diff -u -p -r1.184 uipc_socket.c
--- kern/uipc_socket.c  15 May 2017 13:00:10 -  1.184
+++ kern/uipc_socket.c  27 May 2017 18:27:05 -
@@ -1043,7 +1043,8 @@ sorflush(struct socket *so)
 
sb->sb_flags |= SB_NOINTR;
sblock(sb, M_WAITOK,
-   (af != PF_LOCAL && af != PF_ROUTE) ?  : NULL);
+   (af != PF_LOCAL && af != PF_ROUTE && af != PF_KEY) ?
+: NULL);
socantrcvmore(so);
sbunlock(sb);
asb = *sb;
Index: kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.76
diff -u -p -r1.76 uipc_socket2.c
--- kern/uipc_socket2.c 15 May 2017 12:26:00 -  1.76
+++ kern/uipc_socket2.c 27 May 2017 14:04:34 -
@@ -273,7 +273,8 @@ solock(struct socket *so)
int s;
 
if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
-   (so->so_proto->pr_domain->dom_family != PF_ROUTE))
+   (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
+   (so->so_proto->pr_domain->dom_family != PF_KEY))
NET_LOCK(s);
else
s = -42;
@@ -292,7 +293,8 @@ void
 soassertlocked(struct socket *so)
 {
if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
-   (so->so_proto->pr_domain->dom_family != PF_ROUTE))
+   (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
+   (so->so_proto->pr_domain->dom_family != PF_KEY))
NET_ASSERT_LOCKED();
 }
 
@@ -300,7 +302,8 @@ int
 sosleep(struct socket *so, void *ident, int prio, const char *wmesg, int timo)
 {
if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
-   (so->so_proto->pr_domain->dom_family != PF_ROUTE)) {
+   (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
+   (so->so_proto->pr_domain->dom_family != PF_KEY)) {
return rwsleep(ident, , prio, wmesg, timo);
} else
return tsleep(ident, prio, wmesg, timo);
Index: net/pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.156
diff -u -p -r1.156 pfkeyv2.c
--- net/pfkeyv2.c   26 May 2017 19:11:20 -  1.156
+++ net/pfkeyv2.c   27 May 2017 16:24:37 -
@@ -795,7 +795,7 @@ pfkeyv2_get_proto_alg(u_int8_t satype, u
 int
 pfkeyv2_send(struct socket *socket, void *message, int len)
 {
-   int i, j, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
+   int i, j, s, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
int delflag = 0;
struct sockaddr_encap encapdst, encapnetmask;
struct ipsec_policy *ipo, *tmpipo;
@@ -820,7 +820,7 @@ pfkeyv2_send(struct socket *socket, void
 
u_int rdomain;
 
-   NET_ASSERT_LOCKED();
+   NET_LOCK(s);
 
/* Verify that we received this over a legitimate pfkeyv2 socket */
bzero(headers, sizeof(headers));
@@ -1356,7 +1356,6 @@ pfkeyv2_send(struct socket *socket, void
rval = tdb_walk(rdomain, pfkeyv2_dump_walker, _state);
if (!rval)
goto realret;
-
if ((rval == ENOMEM) || (rval == ENOBUFS))
rval = 0;
}
@@ -1690,6 +1689,8 @@ ret:
rval = pfkeyv2_sendmessage(headers, mode, socket, 0, 0, rdomain);
 
 realret:
+   NET_UNLOCK(s);
+
if (freeme)
free(freeme, M_PFKEY, 0);
 

Re: Throttle bgpd's RDE if the client is slow

[Peter Hessler]

This improves our AMS-IX border router (200 configured sessions, plus
Route Servers) starting performance.

>From 45 minutes to bring everyone up, to 3 minutes, and bgpctl stays
acceptably responsive during the thundering herd.  Memory pressure is
also far more relaxed.

OK


On 2017 May 26 (Fri) at 21:27:46 +0200 (+0200), Claudio Jeker wrote:
:Both bgpctl and bgp neighbors are often not fast enough to keep up with
:the RDE. The result is quite a bit of memory bloat or some ugly
:workarounds for bgpctl which can result in starving other bgpctl calls to
:death.
:
:This implements a simple XON / XOFF protocol for peers and control
:sessions and helps reducing the pain on busy boxes. It is a first step.
:There is still some major changes needed to reduce the update overhead
:seen when many session start up at the same time.
:
:I would love to hear from people with larger setups if there are any
:problems.
:-- 
::wq Claudio
:
:
:Index: bgpd.h
:===
:RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
:retrieving revision 1.300
:diff -u -p -r1.300 bgpd.h
:--- bgpd.h 25 Jan 2017 00:11:07 -  1.300
:+++ bgpd.h 25 Jan 2017 04:22:34 -
:@@ -87,13 +87,17 @@
: #define   F_RTLABEL   0x1
: 
: /*
:- * Limit the number of control messages generated by the RDE and queued in
:- * session engine. The RDE limit defines how many imsg are generated in
:- * one poll round. Then if the SE limit is hit the RDE control socket will no
:- * longer be polled.
:+ * Limit the number of messages queued in the session engine.
:+ * The SE will send an IMSG_XOFF messages to the RDE if the high water mark
:+ * is reached. The RDE should then throttle this peer or control connection.
:+ * Once the message queue in the SE drops below the low water mark an
:+ * IMSG_XON message will be sent and the RDE will produce more messages again.
:  */
: #define RDE_RUNNER_ROUNDS 100
:-#define SESSION_CTL_QUEUE_MAX 1
:+#define SESS_MSG_HIGH_MARK300
:+#define SESS_MSG_LOW_MARK 50
:+#define CTL_MSG_HIGH_MARK 500
:+#define CTL_MSG_LOW_MARK  100
: 
: enum bgpd_process {
:   PROC_MAIN,
:@@ -425,7 +429,9 @@ enum imsg_type {
:   IMSG_PFTABLE_COMMIT,
:   IMSG_REFRESH,
:   IMSG_IFINFO,
:-  IMSG_DEMOTE
:+  IMSG_DEMOTE,
:+  IMSG_XON,
:+  IMSG_XOFF
: };
: 
: struct demote_msg {
:Index: control.c
:===
:RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
:retrieving revision 1.87
:diff -u -p -r1.87 control.c
:--- control.c  13 Feb 2017 14:48:44 -  1.87
:+++ control.c  16 Feb 2017 19:20:23 -
:@@ -213,11 +213,16 @@ control_dispatch_msg(struct pollfd *pfd,
:   return (0);
:   }
: 
:-  if (pfd->revents & POLLOUT)
:+  if (pfd->revents & POLLOUT) {
:   if (msgbuf_write(>ibuf.w) <= 0 && errno != EAGAIN) {
:   *ctl_cnt -= control_close(pfd->fd);
:   return (1);
:   }
:+  if (c->throttled && c->ibuf.w.queued < CTL_MSG_LOW_MARK) {
:+  if (imsg_ctl_rde(IMSG_XON, c->ibuf.pid, NULL, 0) != -1)
:+  c->throttled = 0;
:+  }
:+  }
: 
:   if (!(pfd->revents & POLLIN))
:   return (0);
:@@ -521,6 +526,11 @@ control_imsg_relay(struct imsg *imsg)
: 
:   if ((c = control_connbypid(imsg->hdr.pid)) == NULL)
:   return (0);
:+
:+  if (!c->throttled && c->ibuf.w.queued > CTL_MSG_HIGH_MARK) {
:+  if (imsg_ctl_rde(IMSG_XOFF, imsg->hdr.pid, NULL, 0) != -1)
:+  c->throttled = 1;
:+  }
: 
:   return (imsg_compose(>ibuf, imsg->hdr.type, 0, imsg->hdr.pid, -1,
:   imsg->data, imsg->hdr.len - IMSG_HEADER_SIZE));
:Index: rde.c
:===
:RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
:retrieving revision 1.361
:diff -u -p -r1.361 rde.c
:--- rde.c  25 Jan 2017 03:21:55 -  1.361
:+++ rde.c  26 May 2017 18:57:51 -
:@@ -76,7 +76,7 @@ void  rde_update_log(const char *, u_in
: void   rde_as4byte_fixup(struct rde_peer *, struct rde_aspath *);
: void   rde_reflector(struct rde_peer *, struct rde_aspath *);
: 
:-void   rde_dump_rib_as(struct prefix *, struct rde_aspath *,pid_t,
:+void   rde_dump_rib_as(struct prefix *, struct rde_aspath *, pid_t,
:int);
: void   rde_dump_filter(struct prefix *,
:struct ctl_show_rib_request *);
:@@ -86,8 +86,14 @@ void rde_dump_upcall(struct rib_entry 
: void   rde_dump_prefix_upcall(struct rib_entry *, void *);
: void   rde_dump_ctx_new(struct ctl_show_rib_request *, pid_t,
:enum imsg_type);
:-void   rde_dump_mrt_new(struct mrt *, pid_t, int);
:+void   rde_dump_ctx_throttle(pid_t pid, int throttle);

Re: warn on duplicate network statements

[Claudio Jeker]

On Sat, May 27, 2017 at 06:55:52PM +0200, Sebastian Benoit wrote:
> Sebastian Benoit(be...@openbsd.org) on 2017.05.27 18:35:06 +0200:
> > 
> > in a bgpd.conf config with
> > 
> > network 2a00:15a8:6:100::/56 set ext-community soo 23:42
> > network 2a00:15a8:6:100::/56
> > 
> > the second line "overwrites" the first line.
> > 
> > this prints a warning, because i wasted 30 minutes wondering why i could not
> > see the community on the wire...
> 
> as noted by flrian and claudio, now with yyerror:
> 
> $ bgpd -n
> /etc/bgpd.conf:46: duplicate prefix in network statement
> $ bgpctl reload
> reload request sent.
> config file has errors, reload failed
> 
> ok?

OK
 
> diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y
> index 64a257d9882..a7285256d01 100644
> --- usr.sbin/bgpd/parse.y
> +++ usr.sbin/bgpd/parse.y
> @@ -678,7 +678,7 @@ mrtdump   : DUMP STRING inout STRING optnumber
> {
>   ;
>  
>  network  : NETWORK prefix filter_set {
> - struct network  *n;
> + struct network  *n, *m;
>  
>   if ((n = calloc(1, sizeof(struct network))) == NULL)
>   fatal("new_network");
> @@ -687,6 +687,13 @@ network  : NETWORK prefix filter_set {
>   n->net.prefixlen = $2.len;
>   filterset_move($3, >net.attrset);
>   free($3);
> + TAILQ_FOREACH(m, netconf, entry) {
> + if (n->net.prefixlen == m->net.prefixlen &&
> + !prefix_compare(>net.prefix,
> + >net.prefix, n->net.prefixlen))
> + yyerror("duplicate prefix "
> + "in network statement");
> + }
>  
>   TAILQ_INSERT_TAIL(netconf, n, entry);
>   }
> 

-- 
:wq Claudio

pfctl/pfctl_osfp.c: print_ioctl() used only in debug mode

[Michal Mazurek]

print_ioctl() is unused if not debugging. Found with clang, after
marking the function "static". Use OSFP_DEBUG, in a similar
fashion to OPT_DEBUG (pfctl_optimize.c).

OK?

Index: sbin/pfctl/pfctl_osfp.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_osfp.c,v
retrieving revision 1.23
diff -u -p -r1.23 pfctl_osfp.c
--- sbin/pfctl/pfctl_osfp.c 10 Dec 2015 17:27:00 -  1.23
+++ sbin/pfctl/pfctl_osfp.c 27 May 2017 18:08:57 -
@@ -39,7 +39,8 @@
 
 #define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
 
-#if 0
+/* #define OSFP_DEBUG  1 */
+#ifdef OSFP_DEBUG
 # define DEBUG(fp, str, v...) \
fprintf(stderr, "%s:%s:%s " str "\n", (fp)->fp_os.fp_class_nm, \
(fp)->fp_os.fp_version_nm, (fp)->fp_os.fp_subtype_nm , ## v);
@@ -74,7 +75,9 @@ intget_tcpopts(const char *, int, co
pf_tcpopts_t *, int *, int *, int *, int *, int *,
int *);
 voidimport_fingerprint(struct pf_osfp_ioctl *);
+#ifdef OSFP_DEBUG
 const char *print_ioctl(struct pf_osfp_ioctl *);
+#endif
 voidprint_name_list(int, struct name_list *, const char *);
 voidsort_name_list(int, struct name_list *);
 struct name_entry  *lookup_name_list(struct name_list *, const char *);
@@ -990,6 +993,7 @@ get_field(char **line, size_t *len, int 
 }
 
 
+#ifdef OSFP_DEBUG
 const char *
 print_ioctl(struct pf_osfp_ioctl *fp)
 {
@@ -1091,3 +1095,4 @@ print_ioctl(struct pf_osfp_ioctl *fp)
 
return (buf);
 }
+#endif

-- 
Michal Mazurek

Re: exclude PF_KEY from solock like PF_ROUTE

[Alexander Bluhm]

On Sat, May 27, 2017 at 06:37:48PM +0200, Claudio Jeker wrote:
> Similar to the routing socket the pfkey socket only needs the
> netlock further down (in pfkeyv2_send()). With this all users of
> raw_usrreq are now not using the NET_LOCK which should simplify to push
> the NET_LOCK down.

In sorflush() we have
sa_family_t af = pr->pr_domain->dom_family;
sblock(sb, M_WAITOK,
(af != PF_LOCAL && af != PF_ROUTE) ?  : NULL);

I think you should keep that in sync and add PF_KEY here.

bluhm

> 
> OK?
> -- 
> :wq Claudio
> 
> Index: kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 uipc_socket2.c
> --- kern/uipc_socket2.c   15 May 2017 12:26:00 -  1.76
> +++ kern/uipc_socket2.c   27 May 2017 14:04:34 -
> @@ -273,7 +273,8 @@ solock(struct socket *so)
>   int s;
>  
>   if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
> - (so->so_proto->pr_domain->dom_family != PF_ROUTE))
> + (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
> + (so->so_proto->pr_domain->dom_family != PF_KEY))
>   NET_LOCK(s);
>   else
>   s = -42;
> @@ -292,7 +293,8 @@ void
>  soassertlocked(struct socket *so)
>  {
>   if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
> - (so->so_proto->pr_domain->dom_family != PF_ROUTE))
> + (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
> + (so->so_proto->pr_domain->dom_family != PF_KEY))
>   NET_ASSERT_LOCKED();
>  }
>  
> @@ -300,7 +302,8 @@ int
>  sosleep(struct socket *so, void *ident, int prio, const char *wmesg, int 
> timo)
>  {
>   if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
> - (so->so_proto->pr_domain->dom_family != PF_ROUTE)) {
> + (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
> + (so->so_proto->pr_domain->dom_family != PF_KEY)) {
>   return rwsleep(ident, , prio, wmesg, timo);
>   } else
>   return tsleep(ident, prio, wmesg, timo);
> Index: net/pfkeyv2.c
> ===
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.156
> diff -u -p -r1.156 pfkeyv2.c
> --- net/pfkeyv2.c 26 May 2017 19:11:20 -  1.156
> +++ net/pfkeyv2.c 27 May 2017 16:24:37 -
> @@ -795,7 +795,7 @@ pfkeyv2_get_proto_alg(u_int8_t satype, u
>  int
>  pfkeyv2_send(struct socket *socket, void *message, int len)
>  {
> - int i, j, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
> + int i, j, s, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
>   int delflag = 0;
>   struct sockaddr_encap encapdst, encapnetmask;
>   struct ipsec_policy *ipo, *tmpipo;
> @@ -820,7 +820,7 @@ pfkeyv2_send(struct socket *socket, void
>  
>   u_int rdomain;
>  
> - NET_ASSERT_LOCKED();
> + NET_LOCK(s);
>  
>   /* Verify that we received this over a legitimate pfkeyv2 socket */
>   bzero(headers, sizeof(headers));
> @@ -1356,7 +1356,6 @@ pfkeyv2_send(struct socket *socket, void
>   rval = tdb_walk(rdomain, pfkeyv2_dump_walker, _state);
>   if (!rval)
>   goto realret;
> -
>   if ((rval == ENOMEM) || (rval == ENOBUFS))
>   rval = 0;
>   }
> @@ -1690,6 +1689,8 @@ ret:
>   rval = pfkeyv2_sendmessage(headers, mode, socket, 0, 0, rdomain);
>  
>  realret:
> + NET_UNLOCK(s);
> +
>   if (freeme)
>   free(freeme, M_PFKEY, 0);
>  

pfctl/pfctl_osfp.c: mark functions as static

[Michal Mazurek]

Mark local functions as static. Also remove some double newlines between
function definitions.


Index: sbin/pfctl/pfctl_osfp.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_osfp.c,v
retrieving revision 1.23
diff -u -p -r1.23 pfctl_osfp.c
--- sbin/pfctl/pfctl_osfp.c 10 Dec 2015 17:27:00 -  1.23
+++ sbin/pfctl/pfctl_osfp.c 27 May 2017 17:35:04 -
@@ -62,22 +62,22 @@ struct name_list classes = LIST_HEAD_INI
 int class_count;
 int fingerprint_count;
 
-voidadd_fingerprint(int, int, struct pf_osfp_ioctl *);
-struct name_entry  *fingerprint_name_entry(struct name_list *, char *);
-voidpfctl_flush_my_fingerprints(struct name_list *);
-char   *get_field(char **, size_t *, int *);
-int get_int(char **, size_t *, int *, int *, const char *,
+static void add_fingerprint(int, int, struct pf_osfp_ioctl *);
+static struct name_entry *fingerprint_name_entry(struct name_list *, char *);
+static void pfctl_flush_my_fingerprints(struct name_list *);
+static char*get_field(char **, size_t *, int *);
+static int  get_int(char **, size_t *, int *, int *, const char *,
 int, int, const char *, int);
-int get_str(char **, size_t *, char **, const char *, int,
+static int  get_str(char **, size_t *, char **, const char *, int,
 const char *, int);
-int get_tcpopts(const char *, int, const char *,
+static int  get_tcpopts(const char *, int, const char *,
pf_tcpopts_t *, int *, int *, int *, int *, int *,
int *);
-voidimport_fingerprint(struct pf_osfp_ioctl *);
-const char *print_ioctl(struct pf_osfp_ioctl *);
-voidprint_name_list(int, struct name_list *, const char *);
-voidsort_name_list(int, struct name_list *);
-struct name_entry  *lookup_name_list(struct name_list *, const char *);
+static void import_fingerprint(struct pf_osfp_ioctl *);
+static const char  *print_ioctl(struct pf_osfp_ioctl *);
+static void print_name_list(int, struct name_list *, const char *);
+static void sort_name_list(int, struct name_list *);
+static struct name_entry *lookup_name_list(struct name_list *, const char *);
 
 /* Load fingerprints from a file */
 int
@@ -262,7 +262,7 @@ pfctl_clear_fingerprints(int dev, int op
 }
 
 /* flush pfctl's view of the fingerprints */
-void
+static void
 pfctl_flush_my_fingerprints(struct name_list *list)
 {
struct name_entry *nm;
@@ -516,7 +516,7 @@ found:
 }
 
 /* lookup a name in a list */
-struct name_entry *
+static struct name_entry *
 lookup_name_list(struct name_list *list, const char *name)
 {
struct name_entry *nm;
@@ -527,8 +527,7 @@ lookup_name_list(struct name_list *list,
return (NULL);
 }
 
-
-void
+static void
 add_fingerprint(int dev, int opts, struct pf_osfp_ioctl *fp)
 {
struct pf_osfp_ioctl fptmp;
@@ -639,7 +638,7 @@ add_fingerprint(int dev, int opts, struc
 }
 
 /* import a fingerprint from the kernel */
-void
+static void
 import_fingerprint(struct pf_osfp_ioctl *fp)
 {
struct name_entry *nm_class, *nm_version, *nm_subtype;
@@ -678,7 +677,7 @@ import_fingerprint(struct pf_osfp_ioctl 
 }
 
 /* Find an entry for a fingerprints class/version/subtype */
-struct name_entry *
+static struct name_entry *
 fingerprint_name_entry(struct name_list *list, char *name)
 {
struct name_entry *nm_entry;
@@ -704,8 +703,7 @@ fingerprint_name_entry(struct name_list 
return (nm_entry);
 }
 
-
-void
+static void
 print_name_list(int opts, struct name_list *nml, const char *prefix)
 {
char newprefix[32];
@@ -719,7 +717,7 @@ print_name_list(int opts, struct name_li
}
 }
 
-void
+static void
 sort_name_list(int opts, struct name_list *nml)
 {
struct name_list new;
@@ -760,7 +758,7 @@ sort_name_list(int opts, struct name_lis
 }
 
 /* parse the next integer in a formatted config file line */
-int
+static int
 get_int(char **line, size_t *len, int *var, int *mod,
 const char *name, int flags, int max, const char *filename, int lineno)
 {
@@ -849,7 +847,7 @@ get_int(char **line, size_t *len, int *v
 }
 
 /* parse the next string in a formatted config file line */
-int
+static int
 get_str(char **line, size_t *len, char **v, const char *name, int minlen,
 const char *filename, int lineno)
 {
@@ -874,7 +872,7 @@ get_str(char **line, size_t *len, char *
 }
 
 /* Parse out the TCP opts */
-int
+static int
 get_tcpopts(const char *filename, int lineno, const char *tcpopts,
 pf_tcpopts_t *packed, int *optcnt, int *mss, int *mss_mod, int *wscale,
 int *wscale_mod, int *ts0)
@@ -962,7 +960,7 @@ get_tcpopts(const char *filename, int li
 }
 
 /* rip 

Re: warn on duplicate network statements

[Sebastian Benoit]

Sebastian Benoit(be...@openbsd.org) on 2017.05.27 18:35:06 +0200:
> 
> in a bgpd.conf config with
> 
> network 2a00:15a8:6:100::/56 set ext-community soo 23:42
> network 2a00:15a8:6:100::/56
> 
> the second line "overwrites" the first line.
> 
> this prints a warning, because i wasted 30 minutes wondering why i could not
> see the community on the wire...

as noted by flrian and claudio, now with yyerror:

$ bgpd -n
/etc/bgpd.conf:46: duplicate prefix in network statement
$ bgpctl reload
reload request sent.
config file has errors, reload failed

ok?

diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y
index 64a257d9882..a7285256d01 100644
--- usr.sbin/bgpd/parse.y
+++ usr.sbin/bgpd/parse.y
@@ -678,7 +678,7 @@ mrtdump : DUMP STRING inout STRING optnumber
{
;
 
 network: NETWORK prefix filter_set {
-   struct network  *n;
+   struct network  *n, *m;
 
if ((n = calloc(1, sizeof(struct network))) == NULL)
fatal("new_network");
@@ -687,6 +687,13 @@ network: NETWORK prefix filter_set {
n->net.prefixlen = $2.len;
filterset_move($3, >net.attrset);
free($3);
+   TAILQ_FOREACH(m, netconf, entry) {
+   if (n->net.prefixlen == m->net.prefixlen &&
+   !prefix_compare(>net.prefix,
+   >net.prefix, n->net.prefixlen))
+   yyerror("duplicate prefix "
+   "in network statement");
+   }
 
TAILQ_INSERT_TAIL(netconf, n, entry);
}

exclude PF_KEY from solock like PF_ROUTE

[Claudio Jeker]

Similar to the routing socket the pfkey socket only needs the
netlock further down (in pfkeyv2_send()). With this all users of
raw_usrreq are now not using the NET_LOCK which should simplify to push
the NET_LOCK down.

OK?
-- 
:wq Claudio

Index: kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.76
diff -u -p -r1.76 uipc_socket2.c
--- kern/uipc_socket2.c 15 May 2017 12:26:00 -  1.76
+++ kern/uipc_socket2.c 27 May 2017 14:04:34 -
@@ -273,7 +273,8 @@ solock(struct socket *so)
int s;
 
if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
-   (so->so_proto->pr_domain->dom_family != PF_ROUTE))
+   (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
+   (so->so_proto->pr_domain->dom_family != PF_KEY))
NET_LOCK(s);
else
s = -42;
@@ -292,7 +293,8 @@ void
 soassertlocked(struct socket *so)
 {
if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
-   (so->so_proto->pr_domain->dom_family != PF_ROUTE))
+   (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
+   (so->so_proto->pr_domain->dom_family != PF_KEY))
NET_ASSERT_LOCKED();
 }
 
@@ -300,7 +302,8 @@ int
 sosleep(struct socket *so, void *ident, int prio, const char *wmesg, int timo)
 {
if ((so->so_proto->pr_domain->dom_family != PF_LOCAL) &&
-   (so->so_proto->pr_domain->dom_family != PF_ROUTE)) {
+   (so->so_proto->pr_domain->dom_family != PF_ROUTE) &&
+   (so->so_proto->pr_domain->dom_family != PF_KEY)) {
return rwsleep(ident, , prio, wmesg, timo);
} else
return tsleep(ident, prio, wmesg, timo);
Index: net/pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.156
diff -u -p -r1.156 pfkeyv2.c
--- net/pfkeyv2.c   26 May 2017 19:11:20 -  1.156
+++ net/pfkeyv2.c   27 May 2017 16:24:37 -
@@ -795,7 +795,7 @@ pfkeyv2_get_proto_alg(u_int8_t satype, u
 int
 pfkeyv2_send(struct socket *socket, void *message, int len)
 {
-   int i, j, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
+   int i, j, s, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
int delflag = 0;
struct sockaddr_encap encapdst, encapnetmask;
struct ipsec_policy *ipo, *tmpipo;
@@ -820,7 +820,7 @@ pfkeyv2_send(struct socket *socket, void
 
u_int rdomain;
 
-   NET_ASSERT_LOCKED();
+   NET_LOCK(s);
 
/* Verify that we received this over a legitimate pfkeyv2 socket */
bzero(headers, sizeof(headers));
@@ -1356,7 +1356,6 @@ pfkeyv2_send(struct socket *socket, void
rval = tdb_walk(rdomain, pfkeyv2_dump_walker, _state);
if (!rval)
goto realret;
-
if ((rval == ENOMEM) || (rval == ENOBUFS))
rval = 0;
}
@@ -1690,6 +1689,8 @@ ret:
rval = pfkeyv2_sendmessage(headers, mode, socket, 0, 0, rdomain);
 
 realret:
+   NET_UNLOCK(s);
+
if (freeme)
free(freeme, M_PFKEY, 0);
 

warn on duplicate network statements

[Sebastian Benoit]

in a bgpd.conf config with

network 2a00:15a8:6:100::/56 set ext-community soo 23:42
network 2a00:15a8:6:100::/56

the second line "overwrites" the first line.

this prints a warning, because i wasted 30 minutes wondering why i could not
see the community on the wire...

ok?

(benno_bgpd_parse_duplicate_net.diff)

diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y
index 64a257d9882..2ad3010309c 100644
--- usr.sbin/bgpd/parse.y
+++ usr.sbin/bgpd/parse.y
@@ -678,7 +678,7 @@ mrtdump : DUMP STRING inout STRING optnumber
{
;
 
 network: NETWORK prefix filter_set {
-   struct network  *n;
+   struct network  *n, *m;
 
if ((n = calloc(1, sizeof(struct network))) == NULL)
fatal("new_network");
@@ -687,6 +687,13 @@ network: NETWORK prefix filter_set {
n->net.prefixlen = $2.len;
filterset_move($3, >net.attrset);
free($3);
+   TAILQ_FOREACH(m, netconf, entry) {
+   if (n->net.prefixlen == m->net.prefixlen &&
+   !prefix_compare(>net.prefix,
+   >net.prefix, n->net.prefixlen))
+   logit(LOG_NOTICE, "duplicate prefix "
+   "in network statement\n");
+   }
 
TAILQ_INSERT_TAIL(netconf, n, entry);
}

Re: sha2.c: move sha224_initial_hash_value[] under !SHA2_SMALL

[Ted Unangst]

Christian Weisgerber wrote:
> Move sha224_initial_hash_value[] under !SHA2_SMALL, else it is unused
> if sha2.c is built with SHA2_SMALL, as pointed out by clang.
> 
> This applies to the sha2.c copies in libc and libsa.  The one in
> sys/crypto doesn't have SHA224.
> 
> ok?

does anything anywhere use sha224? i'd be ok just removing it, but maybe
that's the next conversation.

Re: enc(4) vs splnet

[Alexandr Nedvedicky]

Hello,

On Fri, May 26, 2017 at 05:11:22PM +0200, Martin Pieuchot wrote:
> The global array of interfaces is accessed in the input path and need
> the NET_LOCK().
> 
> Ok?
> 

looks good to me.

OK sashan@

Re: Fix carp balancing ip-stealth

[Florian Obser]

OK florian@

On Sat, May 27, 2017 at 10:15:45AM +0200, Florian Riehm wrote:
> Hi,
> 
> This patch fixes the carp mode 'balancing ip-stealth'.
> 
> Problem:
> System A
> carp1: flags=8843 mtu 1500
> lladdr 00:00:5e:00:01:01
> description: Carp-intern
> index 7 priority 15 llprio 3
> carp: carpdev vio2 advbase 1 balancing ip-stealth
> state MASTER vhid 1 advskew 0
> state BACKUP vhid 2 advskew 100
> 
> System B
> carp1: flags=8843 mtu 1500
> lladdr 00:00:5e:00:01:01
> description: Carp-intern
> index 7 priority 15 llprio 3
> carp: carpdev vio2 advbase 1 balancing ip-stealth
> state BACKUP vhid 1 advskew 100
> state MASTER vhid 2 advskew 0
> 
> System B was setting the if_link_state to LINK_STATE_DOWN because
> vhid 1 was in state BACKUP. The cloning routes were missing the
> RTF_UP flag then.
> 
> We musst set the link state UP if at least one vhid is in state MASTER.
> 
> Please note that carp 'balancing ip' (non-stealth-mode) is still broken.
> My next patch will address this problem.
> 
> Regards
> 
> friehm
> 
> Index: netinet/ip_carp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.309
> diff -u -p -r1.309 ip_carp.c
> --- netinet/ip_carp.c 4 May 2017 17:58:46 -   1.309
> +++ netinet/ip_carp.c 27 May 2017 08:11:22 -
> @@ -2362,6 +2362,7 @@ carp_set_state(struct carp_vhost_entry *
>   struct carp_softc *sc = vhe->parent_sc;
>   static const char *carp_states[] = { CARP_STATES };
>   int loglevel;
> + struct carp_vhost_entry *vhe0;
>  
>   KASSERT(vhe->state != state);
>  
> @@ -2382,20 +2383,20 @@ carp_set_state(struct carp_vhost_entry *
>   vhe->state = state;
>   carp_update_lsmask(sc);
>  
> - /* only the master vhe creates link state messages */
> - if (!vhe->vhe_leader)
> - return;
> -
> - switch (state) {
> - case BACKUP:
> - sc->sc_if.if_link_state = LINK_STATE_DOWN;
> - break;
> - case MASTER:
> - sc->sc_if.if_link_state = LINK_STATE_UP;
> - break;
> - default:
> - sc->sc_if.if_link_state = LINK_STATE_INVALID;
> - break;
> + KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
> +
> + sc->sc_if.if_link_state = LINK_STATE_INVALID;
> + SRPL_FOREACH_LOCKED(vhe0, >carp_vhosts, vhost_entries) {
> + /*
> +  * Link must be up if at least one vhe is in state MASTER to
> +  * bring or keep route up.
> +  */
> + if (vhe0->state == MASTER) {
> + sc->sc_if.if_link_state = LINK_STATE_UP;
> + break;
> + } else if (vhe0->state == BACKUP) {
> + sc->sc_if.if_link_state = LINK_STATE_DOWN;
> + }
>   }
>   if_link_state_change(>sc_if);
>  }
> 

-- 
I'm not entirely sure you are real.

Re: Fix carp balancing ip-stealth

[Alexander Bluhm]

On Sat, May 27, 2017 at 10:15:45AM +0200, Florian Riehm wrote:
> We musst set the link state UP if at least one vhid is in state MASTER.

OK bluhm@

> 
> friehm
> 
> Index: netinet/ip_carp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.309
> diff -u -p -r1.309 ip_carp.c
> --- netinet/ip_carp.c 4 May 2017 17:58:46 -   1.309
> +++ netinet/ip_carp.c 27 May 2017 08:11:22 -
> @@ -2362,6 +2362,7 @@ carp_set_state(struct carp_vhost_entry *
>   struct carp_softc *sc = vhe->parent_sc;
>   static const char *carp_states[] = { CARP_STATES };
>   int loglevel;
> + struct carp_vhost_entry *vhe0;
>  
>   KASSERT(vhe->state != state);
>  
> @@ -2382,20 +2383,20 @@ carp_set_state(struct carp_vhost_entry *
>   vhe->state = state;
>   carp_update_lsmask(sc);
>  
> - /* only the master vhe creates link state messages */
> - if (!vhe->vhe_leader)
> - return;
> -
> - switch (state) {
> - case BACKUP:
> - sc->sc_if.if_link_state = LINK_STATE_DOWN;
> - break;
> - case MASTER:
> - sc->sc_if.if_link_state = LINK_STATE_UP;
> - break;
> - default:
> - sc->sc_if.if_link_state = LINK_STATE_INVALID;
> - break;
> + KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
> +
> + sc->sc_if.if_link_state = LINK_STATE_INVALID;
> + SRPL_FOREACH_LOCKED(vhe0, >carp_vhosts, vhost_entries) {
> + /*
> +  * Link must be up if at least one vhe is in state MASTER to
> +  * bring or keep route up.
> +  */
> + if (vhe0->state == MASTER) {
> + sc->sc_if.if_link_state = LINK_STATE_UP;
> + break;
> + } else if (vhe0->state == BACKUP) {
> + sc->sc_if.if_link_state = LINK_STATE_DOWN;
> + }
>   }
>   if_link_state_change(>sc_if);
>  }

Re: trunk(4) vs splnet

[Alexandr Nedvedicky]

Hello,

On Fri, May 26, 2017 at 04:54:57PM +0200, Martin Pieuchot wrote:
> The global list of softc is *not* accessed in the input path, so it
> doesn't need splnet().
> 
> ioctl(2) handlers are already executed with the NET_LOCK() held, so
> splnet() is superfluous.

changes look good to me, but still have a question:

what is the plan for splnet()/splx() in trunk_init() and trunk_stop()
functions. It seems to me they are executed through ioctl(2) only,
so we can also let them go. or you want to leave it for another day?

thanks and
regards
sasha

amd64: Build boot code with -ffreestanding?

[Christian Weisgerber]

We already build the amd64 kernel with -ffreestanding.  Should we
also do this for the boot code?  It would make clang happier, and
conceptually it makes sense.

I've successfully tested the resulting gcc-built pxeboot(8) as well
as boot(8) with root on crypto softraid.

Index: arch/amd64/stand/Makefile.inc
===
RCS file: /cvs/src/sys/arch/amd64/stand/Makefile.inc,v
retrieving revision 1.15
diff -u -p -r1.15 Makefile.inc
--- arch/amd64/stand/Makefile.inc   6 Nov 2016 16:04:20 -   1.15
+++ arch/amd64/stand/Makefile.inc   27 May 2017 14:55:59 -
@@ -1,6 +1,7 @@
 #  $OpenBSD: Makefile.inc,v 1.15 2016/11/06 16:04:20 tb Exp $
 
 CFLAGS=${DEBUG} ${COPTS} -Os -Wall -Werror
+CFLAGS+=   -ffreestanding
 CFLAGS+=   -fno-stack-protector -DMDRANDOM
 CPPFLAGS+=-I${S} -I${SADIR}/libsa -I. -I${.CURDIR}
 SACFLAGS=-D_STANDALONE
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Use copyin32(9) to implement futex(2)

[Mark Kettenis]

This makes MULTIPROCESSOR kernels use copyin32(9) to guarantee
atomicity.  This will break m88k GENERIC.MP; shouldn't be too
difficult to fix for someone whu understands m88k assembly.

Long-term goal is to implement copyin32(9) on all our architectures
and remove the #define.

ok?


Index: kern/sys_futex.c
===
RCS file: /cvs/src/sys/kern/sys_futex.c,v
retrieving revision 1.2
diff -u -p -r1.2 sys_futex.c
--- kern/sys_futex.c30 Apr 2017 10:10:21 -  1.2
+++ kern/sys_futex.c27 May 2017 15:10:50 -
@@ -32,6 +32,15 @@
 #endif
 
 /*
+ * Atomicity is only needed on MULTIPROCESSOR kernels.  Fall back on
+ * copyin(9) until non-MULTIPROCESSOR architectures have a copyin32(9)
+ * implementation.
+ */
+#ifndef MULTIPROCESSOR
+#define copyin32(uaddr, kaddr) copyin((uaddr), (kaddr), sizeof(uint32_t))
+#endif
+
+/*
  * Kernel representation of a futex.
  */
 struct futex {
@@ -187,10 +196,8 @@ futex_wait(uint32_t *uaddr, uint32_t val
 
/*
 * Read user space futex value
-*
-* XXX copyin(9) is not guaranteed to be atomic.
 */
-   if ((error = copyin(uaddr, , sizeof(cval
+   if ((error = copyin32(uaddr, )))
return error;
 
/* If the value changed, stop here. */

sha2.c: move sha224_initial_hash_value[] under !SHA2_SMALL

[Christian Weisgerber]

Move sha224_initial_hash_value[] under !SHA2_SMALL, else it is unused
if sha2.c is built with SHA2_SMALL, as pointed out by clang.

This applies to the sha2.c copies in libc and libsa.  The one in
sys/crypto doesn't have SHA224.

ok?

Index: lib/libc/hash/sha2.c
===
RCS file: /cvs/src/lib/libc/hash/sha2.c,v
retrieving revision 1.25
diff -u -p -r1.25 sha2.c
--- lib/libc/hash/sha2.c3 Sep 2016 16:25:03 -   1.25
+++ lib/libc/hash/sha2.c27 May 2017 14:11:27 -
@@ -195,18 +195,6 @@ static const u_int32_t K256[64] = {
0x90befffaUL, 0xa4506cebUL, 0xbef9a3f7UL, 0xc67178f2UL
 };
 
-/* Initial hash value H for SHA-224: */
-static const u_int32_t sha224_initial_hash_value[8] = {
-   0xc1059ed8UL,
-   0x367cd507UL,
-   0x3070dd17UL,
-   0xf70e5939UL,
-   0xffc00b31UL,
-   0x68581511UL,
-   0x64f98fa7UL,
-   0xbefa4fa4UL
-};
-
 /* Initial hash value H for SHA-256: */
 static const u_int32_t sha256_initial_hash_value[8] = {
0x6a09e667UL,
@@ -276,6 +264,18 @@ static const u_int64_t sha512_initial_ha
 };
 
 #if !defined(SHA2_SMALL)
+/* Initial hash value H for SHA-224: */
+static const u_int32_t sha224_initial_hash_value[8] = {
+   0xc1059ed8UL,
+   0x367cd507UL,
+   0x3070dd17UL,
+   0xf70e5939UL,
+   0xffc00b31UL,
+   0x68581511UL,
+   0x64f98fa7UL,
+   0xbefa4fa4UL
+};
+
 /* Initial hash value H for SHA-384 */
 static const u_int64_t sha384_initial_hash_value[8] = {
0xcbbb9d5dc1059ed8ULL,
Index: sys/lib/libsa/sha2.c
===
RCS file: /cvs/src/sys/lib/libsa/sha2.c,v
retrieving revision 1.1
diff -u -p -r1.1 sha2.c
--- sys/lib/libsa/sha2.c10 Sep 2016 18:22:34 -  1.1
+++ sys/lib/libsa/sha2.c27 May 2017 14:12:35 -
@@ -196,18 +196,6 @@ static const u_int32_t K256[64] = {
0x90befffaUL, 0xa4506cebUL, 0xbef9a3f7UL, 0xc67178f2UL
 };
 
-/* Initial hash value H for SHA-224: */
-static const u_int32_t sha224_initial_hash_value[8] = {
-   0xc1059ed8UL,
-   0x367cd507UL,
-   0x3070dd17UL,
-   0xf70e5939UL,
-   0xffc00b31UL,
-   0x68581511UL,
-   0x64f98fa7UL,
-   0xbefa4fa4UL
-};
-
 /* Initial hash value H for SHA-256: */
 static const u_int32_t sha256_initial_hash_value[8] = {
0x6a09e667UL,
@@ -277,6 +265,18 @@ static const u_int64_t sha512_initial_ha
 };
 
 #if !defined(SHA2_SMALL)
+/* Initial hash value H for SHA-224: */
+static const u_int32_t sha224_initial_hash_value[8] = {
+   0xc1059ed8UL,
+   0x367cd507UL,
+   0x3070dd17UL,
+   0xf70e5939UL,
+   0xffc00b31UL,
+   0x68581511UL,
+   0x64f98fa7UL,
+   0xbefa4fa4UL
+};
+
 /* Initial hash value H for SHA-384 */
 static const u_int64_t sha384_initial_hash_value[8] = {
0xcbbb9d5dc1059ed8ULL,
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: [bgpd] enforce local-as no

[Sebastian Benoit]

reads ok

and yes on sthens suggestion

Peter Hessler(phess...@openbsd.org) on 2017.05.27 14:50:25 +0200:
> Allow us to receive our own AS paths from a neighbor.
> 
> Like several of the related diffs, this also invites dragons and grues
> into your network.
> 
> Probably needs the most love in the man page, as usual ;).
> 
> OK?
> 
> 
> Index: bgpd.conf.5
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.conf.5,v
> retrieving revision 1.154
> diff -u -p -u -p -r1.154 bgpd.conf.5
> --- bgpd.conf.5   27 May 2017 10:33:15 -  1.154
> +++ bgpd.conf.5   27 May 2017 12:49:42 -
> @@ -767,6 +767,19 @@ section in
>  .Sx GLOBAL CONFIGURATION .
>  .Pp
>  .It Xo
> +.Ic enforce local-as
> +.Pq Ic yes Ns | Ns Ic no
> +.Xc
> +If set to
> +.Ic no ,
> +.Em AS paths
> +will not be checked for AS loop detection.
> +Since there is no AS path loop check, this option is dangerous, and
> +requires you to add filters to prevent receiving your own prefixes.
> +The default value is
> +.Ic yes .
> +.Pp
> +.It Xo
>  .Ic enforce neighbor-as
>  .Pq Ic yes Ns | Ns Ic no
>  .Xc
> Index: bgpd.h
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.303
> diff -u -p -u -p -r1.303 bgpd.h
> --- bgpd.h27 May 2017 12:09:27 -  1.303
> +++ bgpd.h27 May 2017 12:34:57 -
> @@ -309,6 +309,7 @@ struct peer_config {
>   u_int32_tmax_prefix;
>   enum announce_type   announce_type;
>   enum enforce_as  enforce_as;
> + enum enforce_as  enforce_local_as;
>   enum reconf_action   reconf_action;
>   u_int16_tmax_prefix_restart;
>   u_int16_tholdtime;
> Index: parse.y
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.303
> diff -u -p -u -p -r1.303 parse.y
> --- parse.y   27 May 2017 10:33:15 -  1.303
> +++ parse.y   27 May 2017 12:35:33 -
> @@ -1183,6 +1183,12 @@ peeropts   : REMOTEAS as4number{
>   else
>   curpeer->conf.enforce_as = ENFORCE_AS_OFF;
>   }
> + | ENFORCE LOCALAS yesno {
> + if ($3)
> + curpeer->conf.enforce_local_as = ENFORCE_AS_ON;
> + else
> + curpeer->conf.enforce_local_as = ENFORCE_AS_OFF;
> + }
>   | MAXPREFIX NUMBER restart {
>   if ($2 < 0 || $2 > UINT_MAX) {
>   yyerror("bad maximum number of prefixes");
> @@ -3690,6 +3696,8 @@ neighbor_consistent(struct peer *p)
>   if (p->conf.enforce_as == ENFORCE_AS_UNDEF)
>   p->conf.enforce_as = p->conf.ebgp ?
>   ENFORCE_AS_ON : ENFORCE_AS_OFF;
> + if (p->conf.enforce_local_as == ENFORCE_AS_UNDEF)
> + p->conf.enforce_local_as = ENFORCE_AS_ON;
>  
>   /* EBGP neighbors are not allowed in route reflector clusters */
>   if (p->conf.reflector_client && p->conf.ebgp) {
> Index: printconf.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/printconf.c,v
> retrieving revision 1.102
> diff -u -p -u -p -r1.102 printconf.c
> --- printconf.c   27 May 2017 10:33:15 -  1.102
> +++ printconf.c   27 May 2017 12:38:13 -
> @@ -470,6 +470,10 @@ print_peer(struct peer_config *p, struct
>   printf("%s\tenforce neighbor-as yes\n", c);
>   else
>   printf("%s\tenforce neighbor-as no\n", c);
> + if (p->enforce_local_as == ENFORCE_AS_ON)
> + printf("%s\tenforce local-as yes\n", c);
> + else
> + printf("%s\tenforce local-as no\n", c);
>   if (p->reflector_client) {
>   if (conf->clusterid == 0)
>   printf("%s\troute-reflector\n", c);
> Index: rde.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.362
> diff -u -p -u -p -r1.362 rde.c
> --- rde.c 27 May 2017 10:33:15 -  1.362
> +++ rde.c 27 May 2017 12:41:06 -
> @@ -1104,6 +1104,7 @@ rde_update_dispatch(struct imsg *imsg)
>  
>   /* aspath needs to be loop free nota bene this is not a hard error */
>   if (peer->conf.ebgp &&
> + peer->conf.enforce_local_as == ENFORCE_AS_ON &&
>   !aspath_loopfree(asp->aspath, peer->conf.local_as))
>   asp->flags |= F_ATTR_LOOP;
>  
> 
> 
> 
> -- 
> The porcupine with the sharpest quills gets stuck on a tree more often.
> 

Re: [bgpd] enforce local-as no

[Stuart Henderson]

On 2017/05/27 14:50, Peter Hessler wrote:
> Allow us to receive our own AS paths from a neighbor.
> 
> Like several of the related diffs, this also invites dragons and grues
> into your network.
> 
> Probably needs the most love in the man page, as usual ;).
> 
> OK?

> Index: bgpd.conf.5
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.conf.5,v
> retrieving revision 1.154
> diff -u -p -u -p -r1.154 bgpd.conf.5
> --- bgpd.conf.5   27 May 2017 10:33:15 -  1.154
> +++ bgpd.conf.5   27 May 2017 12:49:42 -
> @@ -767,6 +767,19 @@ section in
>  .Sx GLOBAL CONFIGURATION .
>  .Pp
>  .It Xo
> +.Ic enforce local-as
> +.Pq Ic yes Ns | Ns Ic no
> +.Xc
> +If set to
> +.Ic no ,
> +.Em AS paths
> +will not be checked for AS loop detection.
> +Since there is no AS path loop check, this option is dangerous, and
> +requires you to add filters to prevent receiving your own prefixes.

That seems fine.

Is it worth adding something like "Similar to allowas-in in some other
BGP implementations" to help users find it if they already know the
term used by cisco/brocade?

> +.Pp
> +.It Xo

>  .Ic enforce neighbor-as
>  .Pq Ic yes Ns | Ns Ic no
>  .Xc
> Index: bgpd.h
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.303
> diff -u -p -u -p -r1.303 bgpd.h
> --- bgpd.h27 May 2017 12:09:27 -  1.303
> +++ bgpd.h27 May 2017 12:34:57 -
> @@ -309,6 +309,7 @@ struct peer_config {
>   u_int32_tmax_prefix;
>   enum announce_type   announce_type;
>   enum enforce_as  enforce_as;
> + enum enforce_as  enforce_local_as;
>   enum reconf_action   reconf_action;
>   u_int16_tmax_prefix_restart;
>   u_int16_tholdtime;
> Index: parse.y
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.303
> diff -u -p -u -p -r1.303 parse.y
> --- parse.y   27 May 2017 10:33:15 -  1.303
> +++ parse.y   27 May 2017 12:35:33 -
> @@ -1183,6 +1183,12 @@ peeropts   : REMOTEAS as4number{
>   else
>   curpeer->conf.enforce_as = ENFORCE_AS_OFF;
>   }
> + | ENFORCE LOCALAS yesno {
> + if ($3)
> + curpeer->conf.enforce_local_as = ENFORCE_AS_ON;
> + else
> + curpeer->conf.enforce_local_as = ENFORCE_AS_OFF;
> + }
>   | MAXPREFIX NUMBER restart {
>   if ($2 < 0 || $2 > UINT_MAX) {
>   yyerror("bad maximum number of prefixes");
> @@ -3690,6 +3696,8 @@ neighbor_consistent(struct peer *p)
>   if (p->conf.enforce_as == ENFORCE_AS_UNDEF)
>   p->conf.enforce_as = p->conf.ebgp ?
>   ENFORCE_AS_ON : ENFORCE_AS_OFF;
> + if (p->conf.enforce_local_as == ENFORCE_AS_UNDEF)
> + p->conf.enforce_local_as = ENFORCE_AS_ON;
>  
>   /* EBGP neighbors are not allowed in route reflector clusters */
>   if (p->conf.reflector_client && p->conf.ebgp) {
> Index: printconf.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/printconf.c,v
> retrieving revision 1.102
> diff -u -p -u -p -r1.102 printconf.c
> --- printconf.c   27 May 2017 10:33:15 -  1.102
> +++ printconf.c   27 May 2017 12:38:13 -
> @@ -470,6 +470,10 @@ print_peer(struct peer_config *p, struct
>   printf("%s\tenforce neighbor-as yes\n", c);
>   else
>   printf("%s\tenforce neighbor-as no\n", c);
> + if (p->enforce_local_as == ENFORCE_AS_ON)
> + printf("%s\tenforce local-as yes\n", c);
> + else
> + printf("%s\tenforce local-as no\n", c);
>   if (p->reflector_client) {
>   if (conf->clusterid == 0)
>   printf("%s\troute-reflector\n", c);
> Index: rde.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.362
> diff -u -p -u -p -r1.362 rde.c
> --- rde.c 27 May 2017 10:33:15 -  1.362
> +++ rde.c 27 May 2017 12:41:06 -
> @@ -1104,6 +1104,7 @@ rde_update_dispatch(struct imsg *imsg)
>  
>   /* aspath needs to be loop free nota bene this is not a hard error */
>   if (peer->conf.ebgp &&
> + peer->conf.enforce_local_as == ENFORCE_AS_ON &&
>   !aspath_loopfree(asp->aspath, peer->conf.local_as))
>   asp->flags |= F_ATTR_LOOP;
>  

ok.

[bgpd] enforce local-as no

[Peter Hessler]

Allow us to receive our own AS paths from a neighbor.

Like several of the related diffs, this also invites dragons and grues
into your network.

Probably needs the most love in the man page, as usual ;).

OK?


Index: bgpd.conf.5
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.154
diff -u -p -u -p -r1.154 bgpd.conf.5
--- bgpd.conf.5 27 May 2017 10:33:15 -  1.154
+++ bgpd.conf.5 27 May 2017 12:49:42 -
@@ -767,6 +767,19 @@ section in
 .Sx GLOBAL CONFIGURATION .
 .Pp
 .It Xo
+.Ic enforce local-as
+.Pq Ic yes Ns | Ns Ic no
+.Xc
+If set to
+.Ic no ,
+.Em AS paths
+will not be checked for AS loop detection.
+Since there is no AS path loop check, this option is dangerous, and
+requires you to add filters to prevent receiving your own prefixes.
+The default value is
+.Ic yes .
+.Pp
+.It Xo
 .Ic enforce neighbor-as
 .Pq Ic yes Ns | Ns Ic no
 .Xc
Index: bgpd.h
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.303
diff -u -p -u -p -r1.303 bgpd.h
--- bgpd.h  27 May 2017 12:09:27 -  1.303
+++ bgpd.h  27 May 2017 12:34:57 -
@@ -309,6 +309,7 @@ struct peer_config {
u_int32_tmax_prefix;
enum announce_type   announce_type;
enum enforce_as  enforce_as;
+   enum enforce_as  enforce_local_as;
enum reconf_action   reconf_action;
u_int16_tmax_prefix_restart;
u_int16_tholdtime;
Index: parse.y
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.303
diff -u -p -u -p -r1.303 parse.y
--- parse.y 27 May 2017 10:33:15 -  1.303
+++ parse.y 27 May 2017 12:35:33 -
@@ -1183,6 +1183,12 @@ peeropts : REMOTEAS as4number{
else
curpeer->conf.enforce_as = ENFORCE_AS_OFF;
}
+   | ENFORCE LOCALAS yesno {
+   if ($3)
+   curpeer->conf.enforce_local_as = ENFORCE_AS_ON;
+   else
+   curpeer->conf.enforce_local_as = ENFORCE_AS_OFF;
+   }
| MAXPREFIX NUMBER restart {
if ($2 < 0 || $2 > UINT_MAX) {
yyerror("bad maximum number of prefixes");
@@ -3690,6 +3696,8 @@ neighbor_consistent(struct peer *p)
if (p->conf.enforce_as == ENFORCE_AS_UNDEF)
p->conf.enforce_as = p->conf.ebgp ?
ENFORCE_AS_ON : ENFORCE_AS_OFF;
+   if (p->conf.enforce_local_as == ENFORCE_AS_UNDEF)
+   p->conf.enforce_local_as = ENFORCE_AS_ON;
 
/* EBGP neighbors are not allowed in route reflector clusters */
if (p->conf.reflector_client && p->conf.ebgp) {
Index: printconf.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/printconf.c,v
retrieving revision 1.102
diff -u -p -u -p -r1.102 printconf.c
--- printconf.c 27 May 2017 10:33:15 -  1.102
+++ printconf.c 27 May 2017 12:38:13 -
@@ -470,6 +470,10 @@ print_peer(struct peer_config *p, struct
printf("%s\tenforce neighbor-as yes\n", c);
else
printf("%s\tenforce neighbor-as no\n", c);
+   if (p->enforce_local_as == ENFORCE_AS_ON)
+   printf("%s\tenforce local-as yes\n", c);
+   else
+   printf("%s\tenforce local-as no\n", c);
if (p->reflector_client) {
if (conf->clusterid == 0)
printf("%s\troute-reflector\n", c);
Index: rde.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.362
diff -u -p -u -p -r1.362 rde.c
--- rde.c   27 May 2017 10:33:15 -  1.362
+++ rde.c   27 May 2017 12:41:06 -
@@ -1104,6 +1104,7 @@ rde_update_dispatch(struct imsg *imsg)
 
/* aspath needs to be loop free nota bene this is not a hard error */
if (peer->conf.ebgp &&
+   peer->conf.enforce_local_as == ENFORCE_AS_ON &&
!aspath_loopfree(asp->aspath, peer->conf.local_as))
asp->flags |= F_ATTR_LOOP;
 



-- 
The porcupine with the sharpest quills gets stuck on a tree more often.

Re: wsconsctl.8: mention the display.font variable in EXAMPLES

[Jason McIntyre]

On Sat, May 27, 2017 at 11:55:48AM +0200, Frederic Cambus wrote:
> On Fri, May 26, 2017 at 05:37:25PM +0100, Jason McIntyre wrote:
> 
> > the examples are expected to be typed as shown. so i think "name" should
> > be changed to an actual font name, and the description adjusted to show 
> > that.
> > 
> > otherwise it may be neccessary to document it more fully if it's not so
> > simple.
> 
> Makes sense, here is a revised diff:
> 
> Comments? OK?
> 

yes, ok.
jmc

> Index: sbin/wsconsctl/wsconsctl.8
> ===
> RCS file: /cvs/src/sbin/wsconsctl/wsconsctl.8,v
> retrieving revision 1.24
> diff -u -p -r1.24 wsconsctl.8
> --- sbin/wsconsctl/wsconsctl.86 Apr 2017 17:33:39 -   1.24
> +++ sbin/wsconsctl/wsconsctl.827 May 2017 09:49:07 -
> @@ -184,6 +184,10 @@ Set the bell pitch to be 1200:
>  Add 200 to the current pitch of the bell:
>  .Pp
>  .Dl # wsconsctl keyboard.bell.pitch+=200
> +.Pp
> +Set the display font to Gallant:
> +.Pp
> +.Dl # wsconsctl display.font=Gallant
>  .Sh SEE ALSO
>  .Xr pckbd 4 ,
>  .Xr wscons 4 ,
> 

Re: pflow 1/2

[Sebastian Benoit]

(benno_pflow_try4_task.diff)

ok?

diff --git sys/net/if_pflow.c sys/net/if_pflow.c
index a40fe23862b..20ab4e0e88d 100644
--- sys/net/if_pflow.c
+++ sys/net/if_pflow.c
@@ -67,6 +67,7 @@ struct pflowstats  pflowstats;
 void   pflowattach(int);
 intpflow_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
struct rtentry *rt);
+void   pflow_output_process(void *);
 intpflow_clone_create(struct if_clone *, int);
 intpflow_clone_destroy(struct ifnet *);
 intpflow_set(struct pflow_softc *, struct pflowreq *);
@@ -124,11 +125,25 @@ pflow_output(struct ifnet *ifp, struct mbuf *m, struct 
sockaddr *dst,
return (EAFNOSUPPORT);
 }
 
+void
+pflow_output_process(void *arg)
+{
+struct pflow_softc *sc = arg;
+   struct mbuf *m;
+
+   KERNEL_LOCK();
+   while ((m = ml_dequeue(>sc_outputqueue)) != NULL) {
+   pflow_sendout_mbuf(sc, m);
+   }
+   KERNEL_UNLOCK();
+}
+
 int
 pflow_clone_create(struct if_clone *ifc, int unit)
 {
struct ifnet*ifp;
struct pflow_softc  *pflowif;
+   int  s;
 
if ((pflowif = malloc(sizeof(*pflowif),
M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
@@ -241,13 +256,18 @@ pflow_clone_create(struct if_clone *ifc, int unit)
ifp->if_hdrlen = PFLOW_HDRLEN;
ifp->if_flags = IFF_UP;
ifp->if_flags &= ~IFF_RUNNING;  /* not running, need receiver */
+   ml_init(>sc_outputqueue);
pflow_setmtu(pflowif, ETHERMTU);
pflow_init_timeouts(pflowif);
if_attach(ifp);
if_alloc_sadl(ifp);
 
+   task_set(>sc_outputtask, pflow_output_process, pflowif);
+
/* Insert into list of pflows */
+   NET_LOCK(s);
SLIST_INSERT_HEAD(_list, pflowif, sc_next);
+   NET_UNLOCK(s);
return (0);
 }
 
@@ -267,6 +287,7 @@ pflow_clone_destroy(struct ifnet *ifp)
if (timeout_initialized(>sc_tmo_tmpl))
timeout_del(>sc_tmo_tmpl);
pflow_flush(sc);
+   task_del(softnettq, >sc_outputtask);
m_freem(sc->send_nam);
if (sc->so != NULL) {
error = soclose(sc->so);
@@ -462,14 +483,8 @@ pflowioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
ifp->if_flags |= IFF_RUNNING;
sc->sc_gcounter=pflowstats.pflow_flows;
/* send templates on startup */
-   if (sc->sc_version == PFLOW_PROTO_10) {
-   /* XXXSMP breaks atomicity */
-   rw_exit_write();
-   s = splnet();
+   if (sc->sc_version == PFLOW_PROTO_10)
pflow_sendout_ipfix_tmpl(sc);
-   splx(s);
-   rw_enter_write();
-   }
} else
ifp->if_flags &= ~IFF_RUNNING;
break;
@@ -513,17 +528,16 @@ pflowioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
s = splnet();
error = pflow_set(sc, );
splx(s);
-   if (error != 0)
+   if (error != 0) {
+   rw_enter_write();
return (error);
+   }
 
if ((ifp->if_flags & IFF_UP) && sc->so != NULL) {
ifp->if_flags |= IFF_RUNNING;
sc->sc_gcounter=pflowstats.pflow_flows;
-   if (sc->sc_version == PFLOW_PROTO_10) {
-   s = splnet();
+   if (sc->sc_version == PFLOW_PROTO_10)
pflow_sendout_ipfix_tmpl(sc);
-   splx(s);
-   }
} else
ifp->if_flags &= ~IFF_RUNNING;
 
@@ -852,14 +866,11 @@ export_pflow_if(struct pf_state *st, struct pf_state_key 
*sk,
 int
 copy_flow_to_m(struct pflow_flow *flow, struct pflow_softc *sc)
 {
-   int s, ret = 0;
+   int ret = 0;
 
-   s = splnet();
if (sc->sc_mbuf == NULL) {
-   if ((sc->sc_mbuf = pflow_get_mbuf(sc, 0)) == NULL) {
-   splx(s);
+   if ((sc->sc_mbuf = pflow_get_mbuf(sc, 0)) == NULL)
return (ENOBUFS);
-   }
}
m_copyback(sc->sc_mbuf, PFLOW_HDRLEN +
(sc->sc_count * sizeof(struct pflow_flow)),
@@ -873,20 +884,17 @@ copy_flow_to_m(struct pflow_flow *flow, struct 
pflow_softc *sc)
if (sc->sc_count >= sc->sc_maxcount)
ret = pflow_sendout_v5(sc);
 
-   splx(s);
return(ret);
 }
 
 int
 copy_flow_ipfix_4_to_m(struct pflow_ipfix_flow4 *flow, struct pflow_softc *sc)
 {
-   int s, ret = 0;
+   int ret = 0;
 
-   s = splnet();
if (sc->sc_mbuf == NULL) {
if ((sc->sc_mbuf =
  

wscons: add missing comments in ascii.h

[Frederic Cambus]

Hi tech@,

Add missing comments for CAN and SUB, for consistency.

Comments? OK?

Index: sys/dev/wscons/ascii.h
===
RCS file: /cvs/src/sys/dev/wscons/ascii.h,v
retrieving revision 1.5
diff -u -p -r1.5 ascii.h
--- sys/dev/wscons/ascii.h  12 Jan 2009 20:43:53 -  1.5
+++ sys/dev/wscons/ascii.h  26 May 2017 19:18:58 -
@@ -11,6 +11,6 @@
 #define ASCII_CR   0x0d/* carriage return */
 #define ASCII_SO   0x0e/* shift out */
 #define ASCII_SI   0x0f/* shift in */
-#defineASCII_CAN   0x18
-#defineASCII_SUB   0x1a
+#defineASCII_CAN   0x18/* cancel */
+#defineASCII_SUB   0x1a/* substitute */
 #define ASCII_ESC  0x1b/* escape */

Re: nvme: Don't set prp1 for DEL_IOCQ

[Claudio Jeker]

On Sat, May 27, 2017 at 11:07:00AM +0200, Stefan Fritsch wrote:
> NVM_ADMIN_DEL_IOCQ does not need prp1 (just as NVM_ADMIN_DEL_IOSQ).
> Remove what is likely a cut'n'paste error from the *_ADD_* code.
> 
> ok?

Tested on X270. No regression noticed.

> 
> --- sys/dev/ic/nvme.c
> +++ sys/dev/ic/nvme.c
> @@ -1120,7 +1120,6 @@ nvme_q_delete(struct nvme_softc *sc, struct nvme_queue 
> *q)
>  
>   memset(, 0, sizeof(sqe));
>   sqe.opcode = NVM_ADMIN_DEL_IOCQ;
> - htolem64(, NVME_DMA_DVA(q->q_sq_dmamem));
>   htolem16(, q->q_id);
>  
>   rv = nvme_poll(sc, sc->sc_admin_q, ccb, nvme_sqe_fill);
> 

-- 
:wq Claudio

Re: Suspend/Resume for nvme

[Claudio Jeker]

On Sat, May 27, 2017 at 11:05:42AM +0200, Stefan Fritsch wrote:
> On Fri, 26 May 2017, Claudio Jeker wrote:
> 
> > Testing it on my X270. I get:
> > nvme0: unable to delete q, disabling
> > 
> > Apart from that it seems to work (eventhough without inteldrm not very
> > helpful since I lose the display).
> 
> Thanks for testing.
> 
> We get called twice on suspend, once with DVACT_SUSPEND and once with 
> DVACT_POWERDOWN. So, here is a patch that does it like in ahci.c an does 
> everything in the DVACT_POWERDOWN path and nothing in the DVACT_SUSPEND 
> path.
> 
> ok ?
> 

Tested on my X270 and this seems to work.

> 
> --- sys/dev/ic/nvme.c
> +++ sys/dev/ic/nvme.c
> @@ -45,6 +45,7 @@ int nvme_ready(struct nvme_softc *, u_int32_t);
>  int  nvme_enable(struct nvme_softc *, u_int);
>  int  nvme_disable(struct nvme_softc *);
>  int  nvme_shutdown(struct nvme_softc *);
> +int  nvme_resume(struct nvme_softc *);
>  
>  void nvme_dumpregs(struct nvme_softc *);
>  int  nvme_identify(struct nvme_softc *, u_int);
> @@ -68,6 +69,7 @@ voidnvme_empty_done(struct nvme_softc *, struct 
> nvme_ccb *,
>  struct nvme_queue *
>   nvme_q_alloc(struct nvme_softc *, u_int16_t, u_int, u_int);
>  int  nvme_q_create(struct nvme_softc *, struct nvme_queue *);
> +int  nvme_q_reset(struct nvme_softc *, struct nvme_queue *);
>  int  nvme_q_delete(struct nvme_softc *, struct nvme_queue *);
>  void nvme_q_submit(struct nvme_softc *,
>   struct nvme_queue *, struct nvme_ccb *,
> @@ -264,7 +266,6 @@ nvme_attach(struct nvme_softc *sc)
>   struct scsibus_attach_args saa;
>   u_int64_t cap;
>   u_int32_t reg;
> - u_int dstrd;
>   u_int mps = PAGE_SHIFT;
>  
>   mtx_init(>sc_ccb_mtx, IPL_BIO);
> @@ -280,7 +281,7 @@ nvme_attach(struct nvme_softc *sc)
>   printf(", NVMe %d.%d\n", NVME_VS_MJR(reg), NVME_VS_MNR(reg));
>  
>   cap = nvme_read8(sc, NVME_CAP);
> - dstrd = NVME_CAP_DSTRD(cap);
> + sc->sc_dstrd = NVME_CAP_DSTRD(cap);
>   if (NVME_CAP_MPSMIN(cap) > PAGE_SHIFT) {
>   printf("%s: NVMe minimum page size %u "
>   "is greater than CPU page size %u\n", DEVNAME(sc),
> @@ -292,6 +293,7 @@ nvme_attach(struct nvme_softc *sc)
>  
>   sc->sc_rdy_to = NVME_CAP_TO(cap);
>   sc->sc_mps = 1 << mps;
> + sc->sc_mps_bits = mps;
>   sc->sc_mdts = MAXPHYS;
>   sc->sc_max_sgl = 2;
>  
> @@ -300,7 +302,7 @@ nvme_attach(struct nvme_softc *sc)
>   return (1);
>   }
>  
> - sc->sc_admin_q = nvme_q_alloc(sc, NVME_ADMIN_Q, 128, dstrd);
> + sc->sc_admin_q = nvme_q_alloc(sc, NVME_ADMIN_Q, 128, sc->sc_dstrd);
>   if (sc->sc_admin_q == NULL) {
>   printf("%s: unable to allocate admin queue\n", DEVNAME(sc));
>   return (1);
> @@ -330,7 +332,7 @@ nvme_attach(struct nvme_softc *sc)
>   goto free_admin_q;
>   }
>  
> - sc->sc_q = nvme_q_alloc(sc, 1, 128, dstrd);
> + sc->sc_q = nvme_q_alloc(sc, 1, 128, sc->sc_dstrd);
>   if (sc->sc_q == NULL) {
>   printf("%s: unable to allocate io q\n", DEVNAME(sc));
>   goto disable;
> @@ -375,6 +377,47 @@ free_admin_q:
>  }
>  
>  int
> +nvme_resume(struct nvme_softc *sc)
> +{
> + if (nvme_disable(sc) != 0) {
> + printf("%s: unable to disable controller\n", DEVNAME(sc));
> + return (1);
> + }
> +
> + if (nvme_q_reset(sc, sc->sc_admin_q) != 0) {
> + printf("%s: unable to reset admin queue\n", DEVNAME(sc));
> + return (1);
> + }
> +
> + if (nvme_enable(sc, sc->sc_mps_bits) != 0) {
> + printf("%s: unable to enable controller\n", DEVNAME(sc));
> + return (1);
> + }
> +
> + sc->sc_q = nvme_q_alloc(sc, 1, 128, sc->sc_dstrd);
> + if (sc->sc_q == NULL) {
> + printf("%s: unable to allocate io q\n", DEVNAME(sc));
> + goto disable;
> + }
> +
> + if (nvme_q_create(sc, sc->sc_q) != 0) {
> + printf("%s: unable to create io q\n", DEVNAME(sc));
> + goto free_q;
> + }
> +
> + nvme_write4(sc, NVME_INTMC, 1);
> +
> + return (0);
> +
> +free_q:
> + nvme_q_free(sc, sc->sc_q);
> +disable:
> + nvme_disable(sc);
> +
> + return (1);
> +}
> +
> +int
>  nvme_scsi_probe(struct scsi_link *link)
>  {
>   struct nvme_softc *sc = link->adapter_softc;
> @@ -469,6 +512,11 @@ nvme_activate(struct nvme_softc *sc, int act)
>   rv = config_activate_children(>sc_dev, act);
>   nvme_shutdown(sc);
>   break;
> + case DVACT_RESUME:
> + rv = nvme_resume(sc);
> + if (rv == 0)
> + rv = config_activate_children(>sc_dev, act);
> + break;
>   default:
>   rv = config_activate_children(>sc_dev, act);
>   break;
> @@ -1079,6 +1127,8 @@ nvme_q_delete(struct nvme_softc *sc, struct nvme_queue 
> *q)
>   if (rv != 0)
>   goto fail;
>  
> + 

Re: refactor m_makespace() to use MCLGETI

[Alexander Bluhm]

On Sat, May 27, 2017 at 09:38:28AM +0200, Claudio Jeker wrote:
> The MLEN check is for n which never has m_type MT_HEADER. n is allocated
> with MGET() and we want to put remain bytes into it. If that is bigger
> than MLEN it will not fit and we allocate a cluster.

You are right.  I was confused by MT_HEADER and M_PKTHDR.

I wonder whether we should add a KASSERT(m0->m_flags & M_PKTHDR)
as we access m0->m_pkthdr.len unconditionally.

> Updated version attached

OK bluhm@

> -- 
> :wq Claudio
> 
> Index: kern/uipc_mbuf.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
> retrieving revision 1.246
> diff -u -p -r1.246 uipc_mbuf.c
> --- kern/uipc_mbuf.c  8 May 2017 15:47:49 -   1.246
> +++ kern/uipc_mbuf.c  27 May 2017 07:36:12 -
> @@ -1078,7 +1110,12 @@ m_makespace(struct mbuf *m0, int skip, i
>   struct mbuf *m;
>   unsigned remain;
>  
> - KASSERT(m0 != NULL);
> + /*
> +  * Limit the size of the new header to MHLEN. In case
> +  * skip = 0 and the first buffer is not a cluster this
> +  * is the maximum space available in that mbuf.
> +  * In other words this code never prepends a mbuf.
> +  */
>   KASSERT(hlen < MHLEN);
>  
>   for (m = m0; m && skip > m->m_len; m = m->m_next)
> @@ -1089,7 +1126,7 @@ m_makespace(struct mbuf *m0, int skip, i
>* At this point skip is the offset into the mbuf m
>* where the new header should be placed.  Figure out
>* if there's space to insert the new header.  If so,
> -  * and copying the remainder makese sense then do so.
> +  * and copying the remainder makes sense then do so.
>* Otherwise insert a new mbuf in the chain, splitting
>* the contents of m as needed.
>*/
> @@ -1099,69 +1136,45 @@ m_makespace(struct mbuf *m0, int skip, i
>   memmove(m->m_data-hlen, m->m_data, skip);
>   m->m_data -= hlen;
>   m->m_len += hlen;
> - (*off) = skip;
> + *off = skip;
>   } else if (hlen > M_TRAILINGSPACE(m)) {
> - struct mbuf *n0, *n, **np;
> - int todo, len, done, alloc;
> + struct mbuf *n;
>  
> - n0 = NULL;
> - np = 
> - alloc = 0;
> - done = 0;
> - todo = remain;
> - while (todo > 0) {
> + if (remain > 0) {
>   MGET(n, M_DONTWAIT, m->m_type);
> - len = MHLEN;
> - if (n && todo > MHLEN) {
> - MCLGET(n, M_DONTWAIT);
> - len = MCLBYTES;
> + if (n && remain > MLEN) {
> + MCLGETI(n, M_DONTWAIT, NULL, remain);
>   if ((n->m_flags & M_EXT) == 0) {
>   m_free(n);
>   n = NULL;
>   }
>   }
> - if (n == NULL) {
> - m_freem(n0);
> - return NULL;
> - }
> - *np = n;
> - np = >m_next;
> - alloc++;
> - len = min(todo, len);
> - memcpy(n->m_data, mtod(m, char *) + skip + done, len);
> - n->m_len = len;
> - done += len;
> - todo -= len;
> + if (n == NULL)
> + return (NULL);
> +
> + memcpy(n->m_data, mtod(m, char *) + skip, remain);
> + n->m_len = remain;
> + m->m_len -= remain;
> +
> + n->m_next = m->m_next;
> + m->m_next = n;
>   }
>  
> - if (hlen <= M_TRAILINGSPACE(m) + remain) {
> - m->m_len = skip + hlen;
> + if (hlen <= M_TRAILINGSPACE(m)) {
> + m->m_len += hlen;
>   *off = skip;
> - if (n0 != NULL) {
> - *np = m->m_next;
> - m->m_next = n0;
> - }
> - }
> - else {
> + } else {
>   n = m_get(M_DONTWAIT, m->m_type);
> - if (n == NULL) {
> - m_freem(n0);
> + if (n == NULL)
>   return NULL;
> - }
> - alloc++;
> -
> - if ((n->m_next = n0) == NULL)
> - np = >m_next;
> - n0 = n;
> -
> - *np = m->m_next;
> - m->m_next = n0;
>  
>   n->m_len = hlen;
> - m->m_len = skip;
>  
> - m = n;

Re: [bgpd] local-as for use in filters

[Sebastian Benoit]

Peter Hessler(phess...@openbsd.org) on 2017.05.27 12:17:56 +0200:
> Right now, we have 'neighbor-as' that can be used as magic syntax sugar
> for filter rules.  I want to have magic syntax sugar for 'local-as',
> which expands to our ASN.  It already plays nicely with the local-as
> diff I sent, and can be committed in either order.
> 
> match in from $neighbor set community local-as:neighbor-as
>   
> OK?
> 
> 
> Index: bgpd.conf.5
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.conf.5,v
> retrieving revision 1.152
> diff -u -p -u -p -r1.152 bgpd.conf.5
> --- bgpd.conf.5   13 Jan 2017 18:59:12 -  1.152
> +++ bgpd.conf.5   27 May 2017 10:12:23 -
> @@ -1079,7 +1079,9 @@ is an AS number as explained above under
>  .Sx GLOBAL CONFIGURATION .
>  It may be set to
>  .Ic neighbor-as ,
> -which is expanded to the current neighbor remote AS number.
> +which is expanded to the current neighbor remote AS number, or
> +.Ic self-as ,

local-as

> +which is expanded to the locally assigned AS number.
>  .Pp
>  The
>  .Ar operator
> @@ -1149,7 +1151,9 @@ and
>  .Ar local
>  may be set to
>  .Ic neighbor-as ,
> -which is expanded to the current neighbor remote AS number.
> +which is expanded to the current neighbor remote AS number, or
> +.Ic self-as ,

local-as

> +which is expanded to the locally assigned AS number.
>  .Pp
>  .It Xo
>  .Ic large-community
> @@ -1181,7 +1185,9 @@ and
>  .Ar local
>  may be set to
>  .Ic neighbor-as ,
> -which is expanded to the current neighbor remote AS number.
> +which is expanded to the current neighbor remote AS number,
> +.Ic self-as ,

local-as

otherwise ok

> +which is expanded to the locally assigned AS number.
>  .Pp
>  .It Xo
>  .Ic ext-community
> Index: bgpd.h
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.300
> diff -u -p -u -p -r1.300 bgpd.h
> --- bgpd.h25 Jan 2017 00:11:07 -  1.300
> +++ bgpd.h27 May 2017 09:31:25 -
> @@ -739,7 +739,8 @@ struct filter_peers {
>  #define  COMMUNITY_ERROR -1
>  #define  COMMUNITY_ANY   -2
>  #define  COMMUNITY_NEIGHBOR_AS   -3
> -#define  COMMUNITY_UNSET -4
> +#define  COMMUNITY_LOCAL_AS  -4
> +#define  COMMUNITY_UNSET -5
>  #define  COMMUNITY_WELLKNOWN 0x
>  #define  COMMUNITY_BLACKHOLE 0x029A  /* 
> draft-ymbk-grow-blackholing-01 */
>  #define  COMMUNITY_NO_EXPORT 0xff01
> Index: parse.y
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.301
> diff -u -p -u -p -r1.301 parse.y
> --- parse.y   26 May 2017 20:55:30 -  1.301
> +++ parse.y   27 May 2017 09:30:58 -
> @@ -2953,6 +2953,8 @@ getcommunity(char *s)
>   return (COMMUNITY_ANY);
>   if (strcmp(s, "neighbor-as") == 0)
>   return (COMMUNITY_NEIGHBOR_AS);
> + if (strcmp(s, "local-as") == 0)
> + return (COMMUNITY_LOCAL_AS);
>   val = strtonum(s, 0, USHRT_MAX, );
>   if (errstr) {
>   yyerror("Community %s is %s (max: %u)", s, errstr, USHRT_MAX);
> @@ -3022,6 +3024,8 @@ getlargecommunity(char *s)
>   return (COMMUNITY_ANY);
>   if (strcmp(s, "neighbor-as") == 0)
>   return (COMMUNITY_NEIGHBOR_AS);
> + if (strcmp(s, "local-as") == 0)
> + return (COMMUNITY_LOCAL_AS);
>   val = strtonum(s, 0, UINT_MAX, );
>   if (errstr) {
>   yyerror("Large Community %s is %s (max: %u)",
> Index: printconf.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/printconf.c,v
> retrieving revision 1.100
> diff -u -p -u -p -r1.100 printconf.c
> --- printconf.c   24 Jan 2017 04:22:42 -  1.100
> +++ printconf.c   27 May 2017 10:10:42 -
> @@ -94,6 +94,8 @@ print_community(int as, int type)
>   printf("*:");
>   else if (as == COMMUNITY_NEIGHBOR_AS)
>   printf("neighbor-as:");
> + else if (as == COMMUNITY_LOCAL_AS)
> + printf("local-as:");
>   else
>   printf("%u:", (unsigned int)as);
>  
> @@ -101,6 +103,8 @@ print_community(int as, int type)
>   printf("* ");
>   else if (type == COMMUNITY_NEIGHBOR_AS)
>   printf("neighbor-as ");
> + else if (type == COMMUNITY_LOCAL_AS)
> + printf("local-as");
>   else
>   printf("%d ", type);
>  }
> @@ -112,6 +116,8 @@ print_largecommunity(int64_t as, int64_t
>   printf("*:");
>   else if (as == COMMUNITY_NEIGHBOR_AS)
>   printf("neighbor-as:");
> + else if (as == COMMUNITY_LOCAL_AS)
> + 

[bgpd] local-as for use in filters

[Peter Hessler]

Right now, we have 'neighbor-as' that can be used as magic syntax sugar
for filter rules.  I want to have magic syntax sugar for 'local-as',
which expands to our ASN.  It already plays nicely with the local-as
diff I sent, and can be committed in either order.

match in from $neighbor set community local-as:neighbor-as
  
OK?


Index: bgpd.conf.5
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.152
diff -u -p -u -p -r1.152 bgpd.conf.5
--- bgpd.conf.5 13 Jan 2017 18:59:12 -  1.152
+++ bgpd.conf.5 27 May 2017 10:12:23 -
@@ -1079,7 +1079,9 @@ is an AS number as explained above under
 .Sx GLOBAL CONFIGURATION .
 It may be set to
 .Ic neighbor-as ,
-which is expanded to the current neighbor remote AS number.
+which is expanded to the current neighbor remote AS number, or
+.Ic self-as ,
+which is expanded to the locally assigned AS number.
 .Pp
 The
 .Ar operator
@@ -1149,7 +1151,9 @@ and
 .Ar local
 may be set to
 .Ic neighbor-as ,
-which is expanded to the current neighbor remote AS number.
+which is expanded to the current neighbor remote AS number, or
+.Ic self-as ,
+which is expanded to the locally assigned AS number.
 .Pp
 .It Xo
 .Ic large-community
@@ -1181,7 +1185,9 @@ and
 .Ar local
 may be set to
 .Ic neighbor-as ,
-which is expanded to the current neighbor remote AS number.
+which is expanded to the current neighbor remote AS number,
+.Ic self-as ,
+which is expanded to the locally assigned AS number.
 .Pp
 .It Xo
 .Ic ext-community
Index: bgpd.h
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.300
diff -u -p -u -p -r1.300 bgpd.h
--- bgpd.h  25 Jan 2017 00:11:07 -  1.300
+++ bgpd.h  27 May 2017 09:31:25 -
@@ -739,7 +739,8 @@ struct filter_peers {
 #defineCOMMUNITY_ERROR -1
 #defineCOMMUNITY_ANY   -2
 #defineCOMMUNITY_NEIGHBOR_AS   -3
-#defineCOMMUNITY_UNSET -4
+#defineCOMMUNITY_LOCAL_AS  -4
+#defineCOMMUNITY_UNSET -5
 #defineCOMMUNITY_WELLKNOWN 0x
 #defineCOMMUNITY_BLACKHOLE 0x029A  /* 
draft-ymbk-grow-blackholing-01 */
 #defineCOMMUNITY_NO_EXPORT 0xff01
Index: parse.y
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.301
diff -u -p -u -p -r1.301 parse.y
--- parse.y 26 May 2017 20:55:30 -  1.301
+++ parse.y 27 May 2017 09:30:58 -
@@ -2953,6 +2953,8 @@ getcommunity(char *s)
return (COMMUNITY_ANY);
if (strcmp(s, "neighbor-as") == 0)
return (COMMUNITY_NEIGHBOR_AS);
+   if (strcmp(s, "local-as") == 0)
+   return (COMMUNITY_LOCAL_AS);
val = strtonum(s, 0, USHRT_MAX, );
if (errstr) {
yyerror("Community %s is %s (max: %u)", s, errstr, USHRT_MAX);
@@ -3022,6 +3024,8 @@ getlargecommunity(char *s)
return (COMMUNITY_ANY);
if (strcmp(s, "neighbor-as") == 0)
return (COMMUNITY_NEIGHBOR_AS);
+   if (strcmp(s, "local-as") == 0)
+   return (COMMUNITY_LOCAL_AS);
val = strtonum(s, 0, UINT_MAX, );
if (errstr) {
yyerror("Large Community %s is %s (max: %u)",
Index: printconf.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/printconf.c,v
retrieving revision 1.100
diff -u -p -u -p -r1.100 printconf.c
--- printconf.c 24 Jan 2017 04:22:42 -  1.100
+++ printconf.c 27 May 2017 10:10:42 -
@@ -94,6 +94,8 @@ print_community(int as, int type)
printf("*:");
else if (as == COMMUNITY_NEIGHBOR_AS)
printf("neighbor-as:");
+   else if (as == COMMUNITY_LOCAL_AS)
+   printf("local-as:");
else
printf("%u:", (unsigned int)as);
 
@@ -101,6 +103,8 @@ print_community(int as, int type)
printf("* ");
else if (type == COMMUNITY_NEIGHBOR_AS)
printf("neighbor-as ");
+   else if (type == COMMUNITY_LOCAL_AS)
+   printf("local-as");
else
printf("%d ", type);
 }
@@ -112,6 +116,8 @@ print_largecommunity(int64_t as, int64_t
printf("*:");
else if (as == COMMUNITY_NEIGHBOR_AS)
printf("neighbor-as:");
+   else if (as == COMMUNITY_LOCAL_AS)
+   printf("local-as:");
else
printf("%lld:", as);
 
@@ -119,6 +125,8 @@ print_largecommunity(int64_t as, int64_t
printf("*:");
else if (ld1 == COMMUNITY_NEIGHBOR_AS)
printf("neighbor-as:");
+   else if (ld1 == COMMUNITY_LOCAL_AS)

Re: bgpd: local-as

[Peter Hessler]

On 2017 May 27 (Sat) at 11:05:30 +0200 (+0200), Peter Hessler wrote:
:On 2016 Sep 26 (Mon) at 20:09:13 +0200 (+0200), Peter Hessler wrote:
::We already have a local AS saved per peer.  Let's use it.  This is very
::useful when one needs to change their local AS.
::
::"
::neighbor 192.0.2.1 {
::  remote-as 65530
::  local-as 131000
::}
::"
::
::OK?
::
:
:Updated!
:
:Now it works far better, it supports "local-as 131000 65000", works
:better with prepend-self.
:
:You MUST use filters to protect yourself from receiving your own routes
:over the local-as peer.  There be dragons and grues.
:
:OK?
:

Only an updated man page bit:


Index: bgpd.conf.5
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.152
diff -u -p -u -p -r1.152 bgpd.conf.5
--- bgpd.conf.5 13 Jan 2017 18:59:12 -  1.152
+++ bgpd.conf.5 27 May 2017 10:04:03 -
@@ -874,6 +874,17 @@ is given,
 .Xr bgpd 8
 binds to this address first.
 .Pp
+.It Ic local-as Ar as-number Op Ar as-number
+Set the AS number sent to the remote system.
+Used as described above under
+.Sx GLOBAL CONFIGURATION
+option
+.Ic AS .
+.Pp
+Since there is no AS path loop check, this option is dangerous, and
+requires you to add filters to prevent receiving your ASNs.
+Intended to be used temporarily, for migrations to another AS.
+.Pp
 .It Ic log no
 Disable neighbor specific logging.
 .Pp



-- 
Procrastinators do it tomorrow.

Re: wsconsctl.8: mention the display.font variable in EXAMPLES

[Frederic Cambus]

On Fri, May 26, 2017 at 05:37:25PM +0100, Jason McIntyre wrote:

> the examples are expected to be typed as shown. so i think "name" should
> be changed to an actual font name, and the description adjusted to show that.
> 
> otherwise it may be neccessary to document it more fully if it's not so
> simple.

Makes sense, here is a revised diff:

Comments? OK?

Index: sbin/wsconsctl/wsconsctl.8
===
RCS file: /cvs/src/sbin/wsconsctl/wsconsctl.8,v
retrieving revision 1.24
diff -u -p -r1.24 wsconsctl.8
--- sbin/wsconsctl/wsconsctl.8  6 Apr 2017 17:33:39 -   1.24
+++ sbin/wsconsctl/wsconsctl.8  27 May 2017 09:49:07 -
@@ -184,6 +184,10 @@ Set the bell pitch to be 1200:
 Add 200 to the current pitch of the bell:
 .Pp
 .Dl # wsconsctl keyboard.bell.pitch+=200
+.Pp
+Set the display font to Gallant:
+.Pp
+.Dl # wsconsctl display.font=Gallant
 .Sh SEE ALSO
 .Xr pckbd 4 ,
 .Xr wscons 4 ,

Re: pflow 2/2

[Martin Pieuchot]

On 27/05/17(Sat) 11:34, Sebastian Benoit wrote:
> (benno_pflow_try3_2_locked_arg.diff)
> 
> Introduce a 'locked' argument to sobind(), socreate() and soclose() to
> indicate if its called with a lock held.
> In pflow in the ioctl path, these functions can now be used without
> giving up the netlock first.

I don't see a reason why we should do this.

> diff --git sys/kern/sys_socket.c sys/kern/sys_socket.c
> index 0a225cb95ec..4c6d9bcf516 100644
> --- sys/kern/sys_socket.c
> +++ sys/kern/sys_socket.c
> @@ -200,7 +200,7 @@ soo_close(struct file *fp, struct proc *p)
>   int error = 0;
>  
>   if (fp->f_data)
> - error = soclose(fp->f_data);
> + error = soclose(fp->f_data, 0);
>   fp->f_data = 0;
>   return (error);
>  }
> diff --git sys/kern/uipc_socket.c sys/kern/uipc_socket.c
> index 29323e8a41f..d2973ccf053 100644
> --- sys/kern/uipc_socket.c
> +++ sys/kern/uipc_socket.c
> @@ -108,7 +108,7 @@ soinit(void)
>   * switching out to the protocol specific routines.
>   */
>  int
> -socreate(int dom, struct socket **aso, int type, int proto)
> +socreate(int dom, struct socket **aso, int type, int proto, int locked)
>  {
>   struct proc *p = curproc;   /* XXX */
>   struct protosw *prp;
> @@ -136,27 +136,32 @@ socreate(int dom, struct socket **aso, int type, int 
> proto)
>   so->so_cpid = p->p_p->ps_pid;
>   so->so_proto = prp;
>  
> - s = solock(so);
> + if (!locked)
> + s = solock(so);
>   error = (*prp->pr_attach)(so, proto);
>   if (error) {
>   so->so_state |= SS_NOFDREF;
>   sofree(so);
> - sounlock(s);
> + if (!locked)
> + sounlock(s);
>   return (error);
>   }
> - sounlock(s);
> + if (!locked)
> + sounlock(s);
>   *aso = so;
>   return (0);
>  }
>  
>  int
> -sobind(struct socket *so, struct mbuf *nam, struct proc *p)
> +sobind(struct socket *so, struct mbuf *nam, struct proc *p, int locked)
>  {
>   int s, error;
>  
> - s = solock(so);
> + if (!locked)
> + s = solock(so);
>   error = (*so->so_proto->pr_usrreq)(so, PRU_BIND, NULL, nam, NULL, p);
> - sounlock(s);
> + if (!locked)
> + sounlock(s);
>   return (error);
>  }
>  
> @@ -227,12 +232,13 @@ sofree(struct socket *so)
>   * Free socket when disconnect complete.
>   */
>  int
> -soclose(struct socket *so)
> +soclose(struct socket *so, int locked)
>  {
>   struct socket *so2;
>   int s, error = 0;
>  
> - s = solock(so);
> + if (!locked)
> + s = solock(so);
>   if (so->so_options & SO_ACCEPTCONN) {
>   while ((so2 = TAILQ_FIRST(>so_q0)) != NULL) {
>   (void) soqremque(so2, 0);
> @@ -273,10 +279,12 @@ drop:
>   }
>  discard:
>   if (so->so_state & SS_NOFDREF)
> - panic("soclose NOFDREF: so %p, so_type %d", so, so->so_type);
> + panic("soclose NOFDREF: so %p, so_type %d locked %d", so,
> + so->so_type, locked);
>   so->so_state |= SS_NOFDREF;
>   sofree(so);
> - sounlock(s);
> + if (!locked)
> + sounlock(s);
>   return (error);
>  }
>  
> diff --git sys/kern/uipc_syscalls.c sys/kern/uipc_syscalls.c
> index acf671a65f3..587da137dff 100644
> --- sys/kern/uipc_syscalls.c
> +++ sys/kern/uipc_syscalls.c
> @@ -103,7 +103,8 @@ sys_socket(struct proc *p, void *v, register_t *retval)
>   fp->f_type = DTYPE_SOCKET;
>   fp->f_ops = 
>   error = socreate(SCARG(uap, domain), ,
> - type & ~(SOCK_CLOEXEC | SOCK_NONBLOCK | SOCK_DNS), SCARG(uap, 
> protocol));
> + type & ~(SOCK_CLOEXEC | SOCK_NONBLOCK | SOCK_DNS),
> + SCARG(uap, protocol), 0);
>   if (error) {
>   fdplock(fdp);
>   fdremove(fdp, fd);
> @@ -201,7 +202,7 @@ sys_bind(struct proc *p, void *v, register_t *retval)
>   if (KTRPOINT(p, KTR_STRUCT))
>   ktrsockaddr(p, mtod(nam, caddr_t), SCARG(uap, namelen));
>  #endif
> - error = sobind(so, nam, p);
> + error = sobind(so, nam, p, 0);
>   m_freem(nam);
>  out:
>   FRELE(fp, p);
> @@ -450,10 +451,12 @@ sys_socketpair(struct proc *p, void *v, register_t 
> *retval)
>   nonblock = SCARG(uap, type) &  SOCK_NONBLOCK;
>   fflag = FREAD | FWRITE | (nonblock ? FNONBLOCK : 0);
>  
> - error = socreate(SCARG(uap, domain), , type, SCARG(uap, protocol));
> + error = socreate(SCARG(uap, domain), , type,
> + SCARG(uap, protocol), 0);
>   if (error)
>   return (error);
> - error = socreate(SCARG(uap, domain), , type, SCARG(uap, protocol));
> + error = socreate(SCARG(uap, domain), , type,
> + SCARG(uap, protocol), 0);
>   if (error)
>   goto free1;
>  
> @@ -508,10 +511,10 @@ free3:
>   fdpunlock(fdp);
>  free2:
>   if (so2 != NULL)
> - (void)soclose(so2);
> + 

Re: pflow 1/2

[Martin Pieuchot]

On 27/05/17(Sat) 11:33, Sebastian Benoit wrote:
> (benno_pflow_try3_1_task.diff)
> 
> move sending of pflow packet into a task, seperated from the data
> collection by a mbuf queue.

Comments inline.

> diff --git sys/net/if_pflow.c sys/net/if_pflow.c
> index a40fe23862b..8cfffa1e4e7 100644
> --- sys/net/if_pflow.c
> +++ sys/net/if_pflow.c
> @@ -67,6 +67,7 @@ struct pflowstatspflowstats;
>  void pflowattach(int);
>  int  pflow_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
>   struct rtentry *rt);
> +void pflow_output_process(void *);
>  int  pflow_clone_create(struct if_clone *, int);
>  int  pflow_clone_destroy(struct ifnet *);
>  int  pflow_set(struct pflow_softc *, struct pflowreq *);
> @@ -124,11 +125,33 @@ pflow_output(struct ifnet *ifp, struct mbuf *m, struct 
> sockaddr *dst,
>   return (EAFNOSUPPORT);
>  }
>  
> +void
> +pflow_output_process(void *xifidx)
> +{

You can pass the softc pointer as argument because the task has the same
lifetime as its softc.

What protects you is the (missing) task_del() in clone_destroy().

> + unsigned int ifidx = (unsigned long)xifidx;
> + struct mbuf *m;
> + struct ifnet *ifp;
> + struct pflow_softc  *sc;
> +
> + ifp = if_get(ifidx);
> + if (ifp == NULL)
> + return;
> + sc = ifp->if_softc;
> +
> + KERNEL_LOCK();
> + while ((m = mq_dequeue(>sc_outputqueue)) != NULL) {

You want to use a mbuf_list, alas, ml_dequeue() because you don't need a
mutex protection.

> + pflow_sendout_mbuf(sc, m);
> + }
> + KERNEL_UNLOCK();
> + if_put(ifp);
> +}
> +
>  int
>  pflow_clone_create(struct if_clone *ifc, int unit)
>  {
>   struct ifnet*ifp;
>   struct pflow_softc  *pflowif;
> + unsigned longifidx;
>  
>   if ((pflowif = malloc(sizeof(*pflowif),
>   M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
> @@ -241,11 +264,15 @@ pflow_clone_create(struct if_clone *ifc, int unit)
>   ifp->if_hdrlen = PFLOW_HDRLEN;
>   ifp->if_flags = IFF_UP;
>   ifp->if_flags &= ~IFF_RUNNING;  /* not running, need receiver */
> + mq_init(>sc_outputqueue, IFQ_MAXLEN, IPL_NET);

ml_init(9) :)


>   pflow_setmtu(pflowif, ETHERMTU);
>   pflow_init_timeouts(pflowif);
>   if_attach(ifp);
>   if_alloc_sadl(ifp);
>  
> + ifidx = ifp->if_index;
> + task_set(>sc_outputtask, pflow_output_process, (void *)ifidx);
> +
>   /* Insert into list of pflows */
>   SLIST_INSERT_HEAD(_list, pflowif, sc_next);

Insertion in pflowif_list needs the NET_LOCK().

>   return (0);
> @@ -255,11 +282,10 @@ int
>  pflow_clone_destroy(struct ifnet *ifp)
>  {
>   struct pflow_softc  *sc = ifp->if_softc;
> - int  s, error;
> + int  error;
>  
>   error = 0;
>  
> - s = splnet();
>   if (timeout_initialized(>sc_tmo))
>   timeout_del(>sc_tmo);
>   if (timeout_initialized(>sc_tmo6))
> @@ -279,7 +305,6 @@ pflow_clone_destroy(struct ifnet *ifp)
>   if_detach(ifp);
>   SLIST_REMOVE(_list, sc, pflow_softc, sc_next);

Deletion as well.

>   free(sc, M_DEVBUF, sizeof(*sc));
> - splx(s);
>   return (error);
>  }
>  
> @@ -311,7 +336,7 @@ pflow_set(struct pflow_softc *sc, struct pflowreq *pflowr)
>   struct proc *p = curproc;
>   struct socket   *so;
>   struct sockaddr *sa;
> - int  error = 0;
> + int  s, error = 0;
>  
>   if (pflowr->addrmask & PFLOW_MASK_VERSION) {
>   switch(pflowr->version) {
> @@ -407,6 +432,8 @@ pflow_set(struct pflow_softc *sc, struct pflowreq *pflowr)
>   }
>  
>   if (sc->so == NULL) {

Please leave the unlock/lock dance around pflow_set().  It is not a
problem.

The rest if fine!  Thanks for taking care of this.

Re: ppp vs splnet()

[Alexandr Nedvedicky]

Hello,

this looks good to me too.

OK sashan@

On Fri, May 26, 2017 at 04:22:29PM +0200, Martin Pieuchot wrote:
> The global list of softc is used in the input path and need to be
> protected by the NET_LOCK().
> 
> ok?
> 

pflow 2/2

[Sebastian Benoit]

(benno_pflow_try3_2_locked_arg.diff)

Introduce a 'locked' argument to sobind(), socreate() and soclose() to
indicate if its called with a lock held.
In pflow in the ioctl path, these functions can now be used without
giving up the netlock first.

diff --git sys/kern/sys_socket.c sys/kern/sys_socket.c
index 0a225cb95ec..4c6d9bcf516 100644
--- sys/kern/sys_socket.c
+++ sys/kern/sys_socket.c
@@ -200,7 +200,7 @@ soo_close(struct file *fp, struct proc *p)
int error = 0;
 
if (fp->f_data)
-   error = soclose(fp->f_data);
+   error = soclose(fp->f_data, 0);
fp->f_data = 0;
return (error);
 }
diff --git sys/kern/uipc_socket.c sys/kern/uipc_socket.c
index 29323e8a41f..d2973ccf053 100644
--- sys/kern/uipc_socket.c
+++ sys/kern/uipc_socket.c
@@ -108,7 +108,7 @@ soinit(void)
  * switching out to the protocol specific routines.
  */
 int
-socreate(int dom, struct socket **aso, int type, int proto)
+socreate(int dom, struct socket **aso, int type, int proto, int locked)
 {
struct proc *p = curproc;   /* XXX */
struct protosw *prp;
@@ -136,27 +136,32 @@ socreate(int dom, struct socket **aso, int type, int 
proto)
so->so_cpid = p->p_p->ps_pid;
so->so_proto = prp;
 
-   s = solock(so);
+   if (!locked)
+   s = solock(so);
error = (*prp->pr_attach)(so, proto);
if (error) {
so->so_state |= SS_NOFDREF;
sofree(so);
-   sounlock(s);
+   if (!locked)
+   sounlock(s);
return (error);
}
-   sounlock(s);
+   if (!locked)
+   sounlock(s);
*aso = so;
return (0);
 }
 
 int
-sobind(struct socket *so, struct mbuf *nam, struct proc *p)
+sobind(struct socket *so, struct mbuf *nam, struct proc *p, int locked)
 {
int s, error;
 
-   s = solock(so);
+   if (!locked)
+   s = solock(so);
error = (*so->so_proto->pr_usrreq)(so, PRU_BIND, NULL, nam, NULL, p);
-   sounlock(s);
+   if (!locked)
+   sounlock(s);
return (error);
 }
 
@@ -227,12 +232,13 @@ sofree(struct socket *so)
  * Free socket when disconnect complete.
  */
 int
-soclose(struct socket *so)
+soclose(struct socket *so, int locked)
 {
struct socket *so2;
int s, error = 0;
 
-   s = solock(so);
+   if (!locked)
+   s = solock(so);
if (so->so_options & SO_ACCEPTCONN) {
while ((so2 = TAILQ_FIRST(>so_q0)) != NULL) {
(void) soqremque(so2, 0);
@@ -273,10 +279,12 @@ drop:
}
 discard:
if (so->so_state & SS_NOFDREF)
-   panic("soclose NOFDREF: so %p, so_type %d", so, so->so_type);
+   panic("soclose NOFDREF: so %p, so_type %d locked %d", so,
+   so->so_type, locked);
so->so_state |= SS_NOFDREF;
sofree(so);
-   sounlock(s);
+   if (!locked)
+   sounlock(s);
return (error);
 }
 
diff --git sys/kern/uipc_syscalls.c sys/kern/uipc_syscalls.c
index acf671a65f3..587da137dff 100644
--- sys/kern/uipc_syscalls.c
+++ sys/kern/uipc_syscalls.c
@@ -103,7 +103,8 @@ sys_socket(struct proc *p, void *v, register_t *retval)
fp->f_type = DTYPE_SOCKET;
fp->f_ops = 
error = socreate(SCARG(uap, domain), ,
-   type & ~(SOCK_CLOEXEC | SOCK_NONBLOCK | SOCK_DNS), SCARG(uap, 
protocol));
+   type & ~(SOCK_CLOEXEC | SOCK_NONBLOCK | SOCK_DNS),
+   SCARG(uap, protocol), 0);
if (error) {
fdplock(fdp);
fdremove(fdp, fd);
@@ -201,7 +202,7 @@ sys_bind(struct proc *p, void *v, register_t *retval)
if (KTRPOINT(p, KTR_STRUCT))
ktrsockaddr(p, mtod(nam, caddr_t), SCARG(uap, namelen));
 #endif
-   error = sobind(so, nam, p);
+   error = sobind(so, nam, p, 0);
m_freem(nam);
 out:
FRELE(fp, p);
@@ -450,10 +451,12 @@ sys_socketpair(struct proc *p, void *v, register_t 
*retval)
nonblock = SCARG(uap, type) &  SOCK_NONBLOCK;
fflag = FREAD | FWRITE | (nonblock ? FNONBLOCK : 0);
 
-   error = socreate(SCARG(uap, domain), , type, SCARG(uap, protocol));
+   error = socreate(SCARG(uap, domain), , type,
+   SCARG(uap, protocol), 0);
if (error)
return (error);
-   error = socreate(SCARG(uap, domain), , type, SCARG(uap, protocol));
+   error = socreate(SCARG(uap, domain), , type,
+   SCARG(uap, protocol), 0);
if (error)
goto free1;
 
@@ -508,10 +511,10 @@ free3:
fdpunlock(fdp);
 free2:
if (so2 != NULL)
-   (void)soclose(so2);
+   (void)soclose(so2, 0);
 free1:
if (so1 != NULL)
-   (void)soclose(so1);
+   (void)soclose(so1, 0);
return (error);
 }
 
diff --git sys/miscfs/fifofs/fifo_vnops.c 

pflow 1/2

[Sebastian Benoit]

(benno_pflow_try3_1_task.diff)

move sending of pflow packet into a task, seperated from the data
collection by a mbuf queue.

diff --git sys/net/if_pflow.c sys/net/if_pflow.c
index a40fe23862b..8cfffa1e4e7 100644
--- sys/net/if_pflow.c
+++ sys/net/if_pflow.c
@@ -67,6 +67,7 @@ struct pflowstats  pflowstats;
 void   pflowattach(int);
 intpflow_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
struct rtentry *rt);
+void   pflow_output_process(void *);
 intpflow_clone_create(struct if_clone *, int);
 intpflow_clone_destroy(struct ifnet *);
 intpflow_set(struct pflow_softc *, struct pflowreq *);
@@ -124,11 +125,33 @@ pflow_output(struct ifnet *ifp, struct mbuf *m, struct 
sockaddr *dst,
return (EAFNOSUPPORT);
 }
 
+void
+pflow_output_process(void *xifidx)
+{
+   unsigned int ifidx = (unsigned long)xifidx;
+   struct mbuf *m;
+   struct ifnet *ifp;
+   struct pflow_softc  *sc;
+
+   ifp = if_get(ifidx);
+   if (ifp == NULL)
+   return;
+   sc = ifp->if_softc;
+
+   KERNEL_LOCK();
+   while ((m = mq_dequeue(>sc_outputqueue)) != NULL) {
+   pflow_sendout_mbuf(sc, m);
+   }
+   KERNEL_UNLOCK();
+   if_put(ifp);
+}
+
 int
 pflow_clone_create(struct if_clone *ifc, int unit)
 {
struct ifnet*ifp;
struct pflow_softc  *pflowif;
+   unsigned longifidx;
 
if ((pflowif = malloc(sizeof(*pflowif),
M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
@@ -241,11 +264,15 @@ pflow_clone_create(struct if_clone *ifc, int unit)
ifp->if_hdrlen = PFLOW_HDRLEN;
ifp->if_flags = IFF_UP;
ifp->if_flags &= ~IFF_RUNNING;  /* not running, need receiver */
+   mq_init(>sc_outputqueue, IFQ_MAXLEN, IPL_NET);
pflow_setmtu(pflowif, ETHERMTU);
pflow_init_timeouts(pflowif);
if_attach(ifp);
if_alloc_sadl(ifp);
 
+   ifidx = ifp->if_index;
+   task_set(>sc_outputtask, pflow_output_process, (void *)ifidx);
+
/* Insert into list of pflows */
SLIST_INSERT_HEAD(_list, pflowif, sc_next);
return (0);
@@ -255,11 +282,10 @@ int
 pflow_clone_destroy(struct ifnet *ifp)
 {
struct pflow_softc  *sc = ifp->if_softc;
-   int  s, error;
+   int  error;
 
error = 0;
 
-   s = splnet();
if (timeout_initialized(>sc_tmo))
timeout_del(>sc_tmo);
if (timeout_initialized(>sc_tmo6))
@@ -279,7 +305,6 @@ pflow_clone_destroy(struct ifnet *ifp)
if_detach(ifp);
SLIST_REMOVE(_list, sc, pflow_softc, sc_next);
free(sc, M_DEVBUF, sizeof(*sc));
-   splx(s);
return (error);
 }
 
@@ -311,7 +336,7 @@ pflow_set(struct pflow_softc *sc, struct pflowreq *pflowr)
struct proc *p = curproc;
struct socket   *so;
struct sockaddr *sa;
-   int  error = 0;
+   int  s, error = 0;
 
if (pflowr->addrmask & PFLOW_MASK_VERSION) {
switch(pflowr->version) {
@@ -407,6 +432,8 @@ pflow_set(struct pflow_softc *sc, struct pflowreq *pflowr)
}
 
if (sc->so == NULL) {
+   rw_exit_write();
+   s = splnet();
if (pflowvalidsockaddr(sc->sc_flowdst, 0)) {
error = socreate(sc->sc_flowdst->sa_family,
, SOCK_DGRAM, 0);
@@ -430,9 +457,15 @@ pflow_set(struct pflow_softc *sc, struct pflowreq *pflowr)
}
sc->so = so;
}
+   splx(s);
+   rw_enter_write();
} else if (!pflowvalidsockaddr(sc->sc_flowdst, 0)) {
+   rw_exit_write();
+   s = splnet();
soclose(sc->so);
sc->so = NULL;
+   splx(s);
+   rw_enter_write();
}
 
/* error check is above */
@@ -452,7 +485,7 @@ pflowioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
struct pflow_softc  *sc = ifp->if_softc;
struct ifreq*ifr = (struct ifreq *)data;
struct pflowreq  pflowr;
-   int  s, error;
+   int  error;
 
switch (cmd) {
case SIOCSIFADDR:
@@ -462,14 +495,8 @@ pflowioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
ifp->if_flags |= IFF_RUNNING;
sc->sc_gcounter=pflowstats.pflow_flows;
/* send templates on startup */
-   if (sc->sc_version == PFLOW_PROTO_10) {
-   /* XXXSMP breaks atomicity */
-   rw_exit_write();
-   s = splnet();
+   if (sc->sc_version == PFLOW_PROTO_10)
pflow_sendout_ipfix_tmpl(sc);
-  

Re: Suspend/Resume for nvme

[Theo de Raadt]

> We get called twice on suspend, once with DVACT_SUSPEND and once with 
> DVACT_POWERDOWN. So, here is a patch that does it like in ahci.c an does 
> everything in the DVACT_POWERDOWN path and nothing in the DVACT_SUSPEND 
> path.

I suppose so.

> @@ -469,6 +512,11 @@ nvme_activate(struct nvme_softc *sc, int act)
>   rv = config_activate_children(>sc_dev, act);
>   nvme_shutdown(sc);
>   break;
> + case DVACT_RESUME:
> + rv = nvme_resume(sc);
> + if (rv == 0)
> + rv = config_activate_children(>sc_dev, act);
> + break;
>   default:
>   rv = config_activate_children(>sc_dev, act);
>   break;

However, note that nvme_resume() cannot fail.  If it fails, and you
don't resume the children, all sorts of stuff goes wrong.

Also your nvme_resume() function performs diagnostic printf's.  During
a resume, those might not work, fact is they may make the situation worse.
Imagine a console screen.  Sure it is just printing characters.. unless
it has to scroll, now you are running a tremendous amount of code in another
driver during a resume function.

So the trend here should be to write code which silently just works.
Most of the resume code is written this way.  Try to get it right in a
minimal fashion, and create as few side effects as possible.  So
perhaps remove the diagnostics checking for failure later.  Such
diagnostics could make a failure to resume act very strange, leading someone
astray quite far.

Re: Suspend/Resume for nvme

[Stefan Fritsch]

On Fri, 26 May 2017, Claudio Jeker wrote:

> Testing it on my X270. I get:
> nvme0: unable to delete q, disabling
> 
> Apart from that it seems to work (eventhough without inteldrm not very
> helpful since I lose the display).

Thanks for testing.

We get called twice on suspend, once with DVACT_SUSPEND and once with 
DVACT_POWERDOWN. So, here is a patch that does it like in ahci.c an does 
everything in the DVACT_POWERDOWN path and nothing in the DVACT_SUSPEND 
path.

ok ?


--- sys/dev/ic/nvme.c
+++ sys/dev/ic/nvme.c
@@ -45,6 +45,7 @@ int   nvme_ready(struct nvme_softc *, u_int32_t);
 intnvme_enable(struct nvme_softc *, u_int);
 intnvme_disable(struct nvme_softc *);
 intnvme_shutdown(struct nvme_softc *);
+intnvme_resume(struct nvme_softc *);
 
 void   nvme_dumpregs(struct nvme_softc *);
 intnvme_identify(struct nvme_softc *, u_int);
@@ -68,6 +69,7 @@ void  nvme_empty_done(struct nvme_softc *, struct nvme_ccb *,
 struct nvme_queue *
nvme_q_alloc(struct nvme_softc *, u_int16_t, u_int, u_int);
 intnvme_q_create(struct nvme_softc *, struct nvme_queue *);
+intnvme_q_reset(struct nvme_softc *, struct nvme_queue *);
 intnvme_q_delete(struct nvme_softc *, struct nvme_queue *);
 void   nvme_q_submit(struct nvme_softc *,
struct nvme_queue *, struct nvme_ccb *,
@@ -264,7 +266,6 @@ nvme_attach(struct nvme_softc *sc)
struct scsibus_attach_args saa;
u_int64_t cap;
u_int32_t reg;
-   u_int dstrd;
u_int mps = PAGE_SHIFT;
 
mtx_init(>sc_ccb_mtx, IPL_BIO);
@@ -280,7 +281,7 @@ nvme_attach(struct nvme_softc *sc)
printf(", NVMe %d.%d\n", NVME_VS_MJR(reg), NVME_VS_MNR(reg));
 
cap = nvme_read8(sc, NVME_CAP);
-   dstrd = NVME_CAP_DSTRD(cap);
+   sc->sc_dstrd = NVME_CAP_DSTRD(cap);
if (NVME_CAP_MPSMIN(cap) > PAGE_SHIFT) {
printf("%s: NVMe minimum page size %u "
"is greater than CPU page size %u\n", DEVNAME(sc),
@@ -292,6 +293,7 @@ nvme_attach(struct nvme_softc *sc)
 
sc->sc_rdy_to = NVME_CAP_TO(cap);
sc->sc_mps = 1 << mps;
+   sc->sc_mps_bits = mps;
sc->sc_mdts = MAXPHYS;
sc->sc_max_sgl = 2;
 
@@ -300,7 +302,7 @@ nvme_attach(struct nvme_softc *sc)
return (1);
}
 
-   sc->sc_admin_q = nvme_q_alloc(sc, NVME_ADMIN_Q, 128, dstrd);
+   sc->sc_admin_q = nvme_q_alloc(sc, NVME_ADMIN_Q, 128, sc->sc_dstrd);
if (sc->sc_admin_q == NULL) {
printf("%s: unable to allocate admin queue\n", DEVNAME(sc));
return (1);
@@ -330,7 +332,7 @@ nvme_attach(struct nvme_softc *sc)
goto free_admin_q;
}
 
-   sc->sc_q = nvme_q_alloc(sc, 1, 128, dstrd);
+   sc->sc_q = nvme_q_alloc(sc, 1, 128, sc->sc_dstrd);
if (sc->sc_q == NULL) {
printf("%s: unable to allocate io q\n", DEVNAME(sc));
goto disable;
@@ -375,6 +377,47 @@ free_admin_q:
 }
 
 int
+nvme_resume(struct nvme_softc *sc)
+{
+   if (nvme_disable(sc) != 0) {
+   printf("%s: unable to disable controller\n", DEVNAME(sc));
+   return (1);
+   }
+
+   if (nvme_q_reset(sc, sc->sc_admin_q) != 0) {
+   printf("%s: unable to reset admin queue\n", DEVNAME(sc));
+   return (1);
+   }
+
+   if (nvme_enable(sc, sc->sc_mps_bits) != 0) {
+   printf("%s: unable to enable controller\n", DEVNAME(sc));
+   return (1);
+   }
+
+   sc->sc_q = nvme_q_alloc(sc, 1, 128, sc->sc_dstrd);
+   if (sc->sc_q == NULL) {
+   printf("%s: unable to allocate io q\n", DEVNAME(sc));
+   goto disable;
+   }
+
+   if (nvme_q_create(sc, sc->sc_q) != 0) {
+   printf("%s: unable to create io q\n", DEVNAME(sc));
+   goto free_q;
+   }
+
+   nvme_write4(sc, NVME_INTMC, 1);
+
+   return (0);
+
+free_q:
+   nvme_q_free(sc, sc->sc_q);
+disable:
+   nvme_disable(sc);
+
+   return (1);
+}
+
+int
 nvme_scsi_probe(struct scsi_link *link)
 {
struct nvme_softc *sc = link->adapter_softc;
@@ -469,6 +512,11 @@ nvme_activate(struct nvme_softc *sc, int act)
rv = config_activate_children(>sc_dev, act);
nvme_shutdown(sc);
break;
+   case DVACT_RESUME:
+   rv = nvme_resume(sc);
+   if (rv == 0)
+   rv = config_activate_children(>sc_dev, act);
+   break;
default:
rv = config_activate_children(>sc_dev, act);
break;
@@ -1079,6 +1127,8 @@ nvme_q_delete(struct nvme_softc *sc, struct nvme_queue *q)
if (rv != 0)
goto fail;
 
+   nvme_q_free(sc, q);
+
 fail:
scsi_io_put(>sc_iopool, ccb);
return (rv);
@@ -1208,6 +1258,7 @@ nvme_q_alloc(struct nvme_softc *sc, u_int16_t id, u_int 
entries, u_int dstrd)
mtx_init(>q_cq_mtx, IPL_BIO);
q->q_sqtdbl 

Re: bgpd: local-as

[Peter Hessler]

On 2016 Sep 26 (Mon) at 20:09:13 +0200 (+0200), Peter Hessler wrote:
:We already have a local AS saved per peer.  Let's use it.  This is very
:useful when one needs to change their local AS.
:
:"
:neighbor 192.0.2.1 {
:   remote-as 65530
:   local-as 131000
:}
:"
:
:OK?
:

Updated!

Now it works far better, it supports "local-as 131000 65000", works
better with prepend-self.

You MUST use filters to protect yourself from receiving your own routes
over the local-as peer.  There be dragons and grues.

OK?


Index: bgpd.conf.5
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.152
diff -u -p -u -p -r1.152 bgpd.conf.5
--- bgpd.conf.5 13 Jan 2017 18:59:12 -  1.152
+++ bgpd.conf.5 27 May 2017 09:04:28 -
@@ -874,6 +874,17 @@ is given,
 .Xr bgpd 8
 binds to this address first.
 .Pp
+.It Ic local-as Ar as-number Op Ar as-number
+Set the AS number sent to the remote system.
+If the first AS number is a 4-byte AS it is possible to specify a secondary
+2-byte AS number which is used for neighbors which do not support 4-byte AS
+numbers.
+The default for the secondary AS is 23456.
+.Pp
+This option is dangerous, and requires you to add filters to prevent
+receiving your global ASN from this peer.
+Intended to be used temporarily, for migrations to another AS.
+.Pp
 .It Ic log no
 Disable neighbor specific logging.
 .Pp
Index: parse.y
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.300
diff -u -p -u -p -r1.300 parse.y
--- parse.y 26 May 2017 14:08:51 -  1.300
+++ parse.y 27 May 2017 08:55:02 -
@@ -188,7 +188,7 @@ typedef struct {
 %token RDOMAIN RD EXPORTTRGT IMPORTTRGT
 %token RDE RIB EVALUATE IGNORE COMPARE
 %token GROUP NEIGHBOR NETWORK
-%token REMOTEAS DESCR LOCALADDR MULTIHOP PASSIVE MAXPREFIX RESTART
+%token LOCALAS REMOTEAS DESCR LOCALADDR MULTIHOP PASSIVE MAXPREFIX RESTART
 %token ANNOUNCE CAPABILITIES REFRESH AS4BYTE CONNECTRETRY
 %token DEMOTE ENFORCE NEIGHBORAS REFLECTOR DEPEND DOWN SOFTRECONFIG
 %token DUMP IN OUT SOCKET RESTRICTED
@@ -1038,6 +1038,17 @@ peeroptsl: peeropts nl
 peeropts   : REMOTEAS as4number{
curpeer->conf.remote_as = $2;
}
+   | LOCALAS as4number {
+   curpeer->conf.local_as = $2;
+   if ($2 > USHRT_MAX)
+   curpeer->conf.local_short_as = AS_TRANS;
+   else
+   curpeer->conf.local_short_as = $2;
+   }
+   | LOCALAS as4number asnumber {
+   curpeer->conf.local_as = $2;
+   curpeer->conf.local_short_as = $3;
+   }
| DESCR string  {
if (strlcpy(curpeer->conf.descr, $2,
sizeof(curpeer->conf.descr)) >=
@@ -2369,6 +2380,7 @@ lookup(char *s)
{ "large-community",LARGECOMMUNITY},
{ "listen", LISTEN},
{ "local-address",  LOCALADDR},
+   { "local-as",   LOCALAS},
{ "localpref",  LOCALPREF},
{ "log",LOG},
{ "match",  MATCH},
Index: printconf.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/printconf.c,v
retrieving revision 1.100
diff -u -p -u -p -r1.100 printconf.c
--- printconf.c 24 Jan 2017 04:22:42 -  1.100
+++ printconf.c 26 May 2017 18:01:32 -
@@ -414,6 +414,8 @@ print_peer(struct peer_config *p, struct
printf("%s\trib \"%s\"\n", c, p->rib);
if (p->remote_as)
printf("%s\tremote-as %s\n", c, log_as(p->remote_as));
+   if (p->local_as != conf->as)
+   printf("%s\tlocal-as %s\n", c, log_as(p->local_as));
if (p->down)
printf("%s\tdown\n", c);
if (p->distance > 1)
Index: rde.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.361
diff -u -p -u -p -r1.361 rde.c
--- rde.c   25 Jan 2017 03:21:55 -  1.361
+++ rde.c   27 May 2017 08:26:56 -
@@ -1103,7 +1103,8 @@ rde_update_dispatch(struct imsg *imsg)
p += 2 + attrpath_len;
 
/* aspath needs to be loop free nota bene this is not a hard error */
-   if (peer->conf.ebgp && !aspath_loopfree(asp->aspath, conf->as))
+   if (peer->conf.ebgp &&
+   !aspath_loopfree(asp->aspath, peer->conf.local_as))
asp->flags |= F_ATTR_LOOP;
 
/* parse nlri prefix */
Index: rde_filter.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde_filter.c,v
retrieving revision 1.80

Re: acme-client(1) -vD output - double slash ('//') in PEM file paths

[Sebastian Benoit]

ok

Florian Obser(flor...@openbsd.org) on 2017.05.27 08:06:04 +:
> It's probably best to use basename(3) here like for the cert file
> instead of incorrectly handrolling the functionality ;)
> 
> OK?
> 
> diff --git main.c main.c
> index f8dc22c8756..9a7c3c5568c 100644
> --- main.c
> +++ main.c
> @@ -128,23 +128,19 @@ main(int argc, char *argv[])
>   }
>  
>   if(domain->chain != NULL) {
> - if ((chainfile = strstr(domain->chain, certdir)) != NULL)
> - chainfile = domain->chain + strlen(certdir);
> - else
> - chainfile = domain->chain;
> -
> - if ((chainfile = strdup(chainfile)) == NULL)
> - err(EXIT_FAILURE, "strdup");
> + if ((chainfile = basename(domain->chain)) != NULL) {
> + if ((chainfile = strdup(chainfile)) == NULL)
> + err(EXIT_FAILURE, "strdup");
> + } else
> + err(EXIT_FAILURE, "basename");
>   }
>  
>   if(domain->fullchain != NULL) {
> - if ((fullchainfile = strstr(domain->fullchain, certdir)) != 
> NULL)
> - fullchainfile = domain->fullchain + strlen(certdir);
> - else
> - fullchainfile = domain->fullchain;
> -
> - if ((fullchainfile = strdup(fullchainfile)) == NULL)
> - err(EXIT_FAILURE, "strdup");
> + if ((fullchainfile = basename(domain->fullchain)) != NULL) {
> + if ((fullchainfile = strdup(fullchainfile)) == NULL)
> + err(EXIT_FAILURE, "strdup");
> + } else
> + err(EXIT_FAILURE, "basename");
>   }
>  
>   if ((auth = domain->auth) == NULL) {
> 
> 
> On Sat, May 20, 2017 at 06:56:34AM +0100, Raf Czlonka wrote:
> > Hi all,
> > 
> > A minor thing - I'm not sure it warrants an email to bugs@
> > 
> > Something I had noticed only today:
> > 
> > # acme-client -vD domain.org
> > [...]
> > acme-client: /etc/ssl//domain.org.chain.pem: created
> > acme-client: /etc/ssl/domain.org.crt: created
> > acme-client: /etc/ssl//domain.org.fullchain.pem: created
> > 
> > Please notice the double slash ('//') in chain and fullchain.
> > 
> > Before you ask, yes my config file does *not* contain those :^)
> > 
> > Regards,
> > 
> > Raf
> > 
> 
> -- 
> I'm not entirely sure you are real.
> 

Fix carp balancing ip-stealth

[Florian Riehm]

Hi,

This patch fixes the carp mode 'balancing ip-stealth'.

Problem:
System A
carp1: flags=8843 mtu 1500
lladdr 00:00:5e:00:01:01
description: Carp-intern
index 7 priority 15 llprio 3
carp: carpdev vio2 advbase 1 balancing ip-stealth
state MASTER vhid 1 advskew 0
state BACKUP vhid 2 advskew 100

System B
carp1: flags=8843 mtu 1500
lladdr 00:00:5e:00:01:01
description: Carp-intern
index 7 priority 15 llprio 3
carp: carpdev vio2 advbase 1 balancing ip-stealth
state BACKUP vhid 1 advskew 100
state MASTER vhid 2 advskew 0

System B was setting the if_link_state to LINK_STATE_DOWN because
vhid 1 was in state BACKUP. The cloning routes were missing the
RTF_UP flag then.

We musst set the link state UP if at least one vhid is in state MASTER.

Please note that carp 'balancing ip' (non-stealth-mode) is still broken.
My next patch will address this problem.

Regards

friehm

Index: netinet/ip_carp.c
===
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.309
diff -u -p -r1.309 ip_carp.c
--- netinet/ip_carp.c   4 May 2017 17:58:46 -   1.309
+++ netinet/ip_carp.c   27 May 2017 08:11:22 -
@@ -2362,6 +2362,7 @@ carp_set_state(struct carp_vhost_entry *
struct carp_softc *sc = vhe->parent_sc;
static const char *carp_states[] = { CARP_STATES };
int loglevel;
+   struct carp_vhost_entry *vhe0;
 
KASSERT(vhe->state != state);
 
@@ -2382,20 +2383,20 @@ carp_set_state(struct carp_vhost_entry *
vhe->state = state;
carp_update_lsmask(sc);
 
-   /* only the master vhe creates link state messages */
-   if (!vhe->vhe_leader)
-   return;
-
-   switch (state) {
-   case BACKUP:
-   sc->sc_if.if_link_state = LINK_STATE_DOWN;
-   break;
-   case MASTER:
-   sc->sc_if.if_link_state = LINK_STATE_UP;
-   break;
-   default:
-   sc->sc_if.if_link_state = LINK_STATE_INVALID;
-   break;
+   KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
+
+   sc->sc_if.if_link_state = LINK_STATE_INVALID;
+   SRPL_FOREACH_LOCKED(vhe0, >carp_vhosts, vhost_entries) {
+   /*
+* Link must be up if at least one vhe is in state MASTER to
+* bring or keep route up.
+*/
+   if (vhe0->state == MASTER) {
+   sc->sc_if.if_link_state = LINK_STATE_UP;
+   break;
+   } else if (vhe0->state == BACKUP) {
+   sc->sc_if.if_link_state = LINK_STATE_DOWN;
+   }
}
if_link_state_change(>sc_if);
 }

Re: acme-client(1) -vD output - double slash ('//') in PEM file paths

[Florian Obser]

It's probably best to use basename(3) here like for the cert file
instead of incorrectly handrolling the functionality ;)

OK?

diff --git main.c main.c
index f8dc22c8756..9a7c3c5568c 100644
--- main.c
+++ main.c
@@ -128,23 +128,19 @@ main(int argc, char *argv[])
}
 
if(domain->chain != NULL) {
-   if ((chainfile = strstr(domain->chain, certdir)) != NULL)
-   chainfile = domain->chain + strlen(certdir);
-   else
-   chainfile = domain->chain;
-
-   if ((chainfile = strdup(chainfile)) == NULL)
-   err(EXIT_FAILURE, "strdup");
+   if ((chainfile = basename(domain->chain)) != NULL) {
+   if ((chainfile = strdup(chainfile)) == NULL)
+   err(EXIT_FAILURE, "strdup");
+   } else
+   err(EXIT_FAILURE, "basename");
}
 
if(domain->fullchain != NULL) {
-   if ((fullchainfile = strstr(domain->fullchain, certdir)) != 
NULL)
-   fullchainfile = domain->fullchain + strlen(certdir);
-   else
-   fullchainfile = domain->fullchain;
-
-   if ((fullchainfile = strdup(fullchainfile)) == NULL)
-   err(EXIT_FAILURE, "strdup");
+   if ((fullchainfile = basename(domain->fullchain)) != NULL) {
+   if ((fullchainfile = strdup(fullchainfile)) == NULL)
+   err(EXIT_FAILURE, "strdup");
+   } else
+   err(EXIT_FAILURE, "basename");
}
 
if ((auth = domain->auth) == NULL) {


On Sat, May 20, 2017 at 06:56:34AM +0100, Raf Czlonka wrote:
> Hi all,
> 
> A minor thing - I'm not sure it warrants an email to bugs@
> 
> Something I had noticed only today:
> 
>   # acme-client -vD domain.org
>   [...]
>   acme-client: /etc/ssl//domain.org.chain.pem: created
>   acme-client: /etc/ssl/domain.org.crt: created
>   acme-client: /etc/ssl//domain.org.fullchain.pem: created
> 
> Please notice the double slash ('//') in chain and fullchain.
> 
> Before you ask, yes my config file does *not* contain those :^)
> 
> Regards,
> 
> Raf
> 

-- 
I'm not entirely sure you are real.

place disconnected ahci ports in listen mode

[Jonathan Matthew]

While looking at implementing disk hotplug for ahci, I saw that the ahci spec
suggests putting disconnected ports in listen mode, which allows the controller
to put the phy in a low power state while still allowing it to generate 
interrupts
when a device is attached.  As a first step towards hotplug, the diff below
implements this, and also stops freeing disconnected ports, since they may 
become
active later on.

In listen mode, ahci_ata_probe will always return ATA_T_PORT_NONE, so no 
commands
will be issued to the port.  Resetting the port exits listen mode.

Some rough testing here shows that placing an otherwise active port in listen
mode reduces power draw by around 0.1W (assuming we can believe acpibat 
sensors),
so if your laptop has an extra sata phy for some reason, it might get you a 
couple
more minutes of battery life, for the cost of a few kb of memory.  I've tested 
this
on intel, amd and asmedia ahcis and haven't seen any ill effects.


Index: ahci.c
===
RCS file: /cvs/src/sys/dev/ic/ahci.c,v
retrieving revision 1.29
diff -u -p -r1.29 ahci.c
--- ahci.c  5 Mar 2017 09:55:16 -   1.29
+++ ahci.c  27 May 2017 06:42:36 -
@@ -81,6 +81,7 @@ int   ahci_port_clo(struct ahci_port *);
 intahci_port_softreset(struct ahci_port *);
 intahci_port_portreset(struct ahci_port *, int);
 intahci_port_signature(struct ahci_port *);
+intahci_port_listenmode(struct ahci_port *);
 intahci_pmp_port_softreset(struct ahci_port *, int);
 intahci_pmp_port_portreset(struct ahci_port *, int);
 intahci_pmp_port_probe(struct ahci_port *ap, int pmp_port);
@@ -186,6 +187,7 @@ ahci_attach(struct ahci_softc *sc)
printf("\n");
 
sc->sc_cap = ahci_read(sc, AHCI_REG_CAP);
+   sc->sc_cap2 = ahci_read(sc, AHCI_REG_CAP2);
sc->sc_ncmds = AHCI_REG_CAP_NCS(sc->sc_cap);
 #ifdef AHCI_DEBUG
if (ahcidebug & AHCI_D_VERBOSE) {
@@ -210,7 +212,7 @@ ahci_attach(struct ahci_softc *sc)
DEVNAME(sc), sc->sc_cap, AHCI_FMT_CAP,
AHCI_REG_CAP_NP(sc->sc_cap), sc->sc_ncmds, gen);
printf("%s: extended capabilities 0x%b\n", DEVNAME(sc),
-   ahci_read(sc, AHCI_REG_CAP2), AHCI_FMT_CAP2);
+   sc->sc_cap2, AHCI_FMT_CAP2);
}
 #endif
 
@@ -628,7 +631,11 @@ nomem:
"on port %d\n", DEVNAME(sc), port);
break;
}
-   goto freeport;
+
+   rc = ahci_port_listenmode(ap);
+   if (rc != 0)
+   goto freeport;
+   goto flushport;
 
case EBUSY:
printf("%s: device on port %d didn't come ready, "
@@ -676,6 +689,7 @@ nomem:
rc = ENXIO; /* couldn't start port */
}
 
+flushport:
/* Flush interrupts for port */
ahci_pwrite(ap, AHCI_PREG_IS, ahci_pread(ap, AHCI_PREG_IS));
ahci_write(sc, AHCI_REG_IS, 1 << port);
@@ -807,7 +821,10 @@ ahci_port_init(struct ahci_softc *sc, u_
"on port %d\n", DEVNAME(sc), port);
break;
}
-   goto reterr;
+   rc = ahci_port_listenmode(ap);
+   if (rc == 0)
+   goto reterr;
+   goto flushport;
 
case EBUSY:
printf("%s: device on port %d didn't come ready, "
@@ -857,6 +874,7 @@ ahci_port_init(struct ahci_softc *sc, u_
rc = ENXIO; /* couldn't start port */
}
 
+flushport:
/* Flush interrupts for port */
ahci_pwrite(ap, AHCI_PREG_IS, ahci_pread(ap, AHCI_PREG_IS));
ahci_write(sc, AHCI_REG_IS, 1 << port);
@@ -933,6 +951,39 @@ ahci_port_stop(struct ahci_port *ap, int
return (0);
 }
 
+/* place port in listen mode (10.10.1) */
+int
+ahci_port_listenmode(struct ahci_port *ap)
+{
+   int rc;
+   uint32_t cmd, ipm;
+
+   rc = ahci_port_stop(ap, 1);
+   if (rc != 0)
+   return (rc);
+
+   cmd = ahci_pread(ap, AHCI_PREG_CMD);
+   cmd &= ~(AHCI_PREG_CMD_CLO | AHCI_PREG_CMD_PMA | AHCI_PREG_CMD_ICC);
+   ahci_pwrite(ap, AHCI_PREG_CMD, cmd);
+
+   /* disable IPM and DET */
+   ipm = AHCI_PREG_SCTL_IPM_DISABLED;
+   if (ap->ap_sc->sc_cap2 & AHCI_REG_CAP2_SDS)
+   ipm |= AHCI_PREG_SCTL_IPM_NODEVSLP;
+   ahci_pwrite(ap, AHCI_PREG_SCTL, ipm);
+
+   delay(1);
+
+   /* disable SUD */
+   cmd = ahci_pread(ap, AHCI_PREG_CMD);
+   cmd &= ~AHCI_PREG_CMD_SUD;
+   ahci_pwrite(ap, AHCI_PREG_CMD, cmd);
+
+   ap->ap_state = AP_S_LISTEN;
+
+   return (0);
+}
+
 /* AHCI command list override -> forcibly clear TFD.STS.{BSY,DRQ} */
 int
 ahci_port_clo(struct ahci_port *ap)
@@ -1407,6 +1458,18 @@ 

Re: ksh(1): vi mode UTF-8 bug

[Theo Buehler]

On Fri, May 19, 2017 at 04:19:57PM +0200, Ingo Schwarze wrote:
> Hi Anton,
> 
> Anton Lindqvist wrote on Fri, May 19, 2017 at 02:11:37PM +0200:
> 
> > Hi,
> > Another UTF-8 related bug reported by tb@. How to re-produce:
> > 
> > 1. Enable vi mode:
> > 
> >$ set -o vi
> > 
> > 2. Input the following characters: öa
> > 
> > 3. Press escape and then x twice.
> > 
> > 4. An invalid UTF-8 character is displayed.
> > 
> > Similar to one of my previous diffs, looks like the column counter is
> > wrong. The diff below fixes the problem and includes a regression test.
> > I'm not running vi mode myself so further testing would be appreciated.
> 
> Yes, that is correct, from both testing and code inspection.
> 
> I'd prefer to also delete the condition that becomes obsolete,
> see below.
> 
> OK?

What's the status of this diff? I sent my ok to Ingo, but it seems to
have been forgotten. Ingo or Anton, are you going to commit it?

>   Ingo
> 
> 
> Index: vi.c
> ===
> RCS file: /cvs/src/bin/ksh/vi.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 vi.c
> --- vi.c  17 Oct 2016 18:39:43 -  1.44
> +++ vi.c  19 May 2017 14:15:27 -
> @@ -1154,8 +1154,10 @@ vi_cmd(int argcnt, const char *cmd)
>   expand_word(1);
>   break;
>   }
> - if (insert == 0 && es->cursor != 0 && es->cursor >= es->linelen)
> - es->cursor--;
> + if (insert == 0 && es->cursor >= es->linelen)
> + while (es->cursor > 0)
> + if (!isu8cont(es->cbuf[--es->cursor]))
> + break;
>   }
>   return 0;
>  }
> 
> > Index: regress/bin/ksh/vi/vi.sh
> > ===
> > RCS file: /cvs/src/regress/bin/ksh/vi/vi.sh,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 vi.sh
> > --- regress/bin/ksh/vi/vi.sh12 Jan 2016 09:00:39 -  1.1
> > +++ regress/bin/ksh/vi/vi.sh19 May 2017 10:47:44 -
> > @@ -168,6 +168,7 @@ testseq "abcd\00332X" " $ abcd\b\b\bd  \
> >  # x: Delete character.
> >  # |: Move to column.
> >  testseq "abcd\00332|2x" " $ abcd\b\b\bd  \b\b\b\r\nad"
> > +testseq "\0303\0266a\0033xx" " $ \0303\0266a\b \b\b  \b\b\r"
> >  
> >  # Y: Yank to end of line.
> >  testseq "abcd\0033hYp" " $ abcd\b\bccdd\b\b\r\nabccdd"
> > 

Kill option KEY

[Claudio Jeker]

There is no need to have 3 buttons to enable pfkey protocol.
Especially a button that is only enabling the protocol.

Found by jmc@ because of the manpage bits
-- 
:wq Claudio

Index: share/man/man4/options.4
===
RCS file: /cvs/src/share/man/man4/options.4,v
retrieving revision 1.249
diff -u -p -r1.249 options.4
--- share/man/man4/options.430 Apr 2017 20:09:28 -  1.249
+++ share/man/man4/options.426 May 2017 19:57:21 -
@@ -408,10 +408,6 @@ This option enables IP security protocol
 See
 .Xr ipsec 4
 for more details.
-.It Cd option KEY
-Enables PFKEYv2 (RFC 2367) support.
-While not IP specific, this option is usually used in conjunction with option
-.Em IPSEC .
 .It Cd option MROUTING
 Includes support for IP multicast routers.
 Multicast routing is controlled by the
Index: sys/conf/GENERIC
===
RCS file: /cvs/src/sys/conf/GENERIC,v
retrieving revision 1.243
diff -u -p -r1.243 GENERIC
--- sys/conf/GENERIC20 Apr 2017 14:23:53 -  1.243
+++ sys/conf/GENERIC26 May 2017 19:54:40 -
@@ -54,7 +54,6 @@ optionTCP_SIGNATURE   # TCP MD5 Signatur
 option ART # Allotment Routing Table
 option INET6   # IPv6
 option IPSEC   # IPsec
-#optionKEY # PF_KEY (implied by IPSEC)
 option PPP_BSDCOMP # PPP BSD compression
 option PPP_DEFLATE
 option PIPEX   # Ppp IP EXtension, for npppd
Index: sys/kern/uipc_domain.c
===
RCS file: /cvs/src/sys/kern/uipc_domain.c,v
retrieving revision 1.50
diff -u -p -r1.50 uipc_domain.c
--- sys/kern/uipc_domain.c  9 May 2017 09:37:25 -   1.50
+++ sys/kern/uipc_domain.c  26 May 2017 19:55:03 -
@@ -56,7 +56,7 @@ struct domain *domains[] = {
 #ifdef MPLS
,
 #endif
-#if defined (KEY) || defined (IPSEC) || defined (TCP_SIGNATURE)
+#if defined (IPSEC) || defined (TCP_SIGNATURE)
,
 #endif
 #ifdef INET6

Re: refactor m_makespace() to use MCLGETI

[Claudio Jeker]

On Fri, May 26, 2017 at 10:24:23PM +0200, Alexander Bluhm wrote:
> On Fri, May 26, 2017 at 05:38:07PM +0200, Claudio Jeker wrote:
> > m_makespace() is using an old way to build up the mbuf chain. By using
> > MCLGETI() a lot of the code can be simplified and results at least in my
> > opinion in better readable code. In some cases this may reduce the number
> 
> Yes, the new code is easier to understand.
> 
> > of MGET and MCLGET calls so it may be a bit quicker...
> 
> I wonder whether we can avoid the final m_get() for the new header
> if we allocate a bigger cluster for the new header plus the remainder.
> But not in this diff.
> 
> > MGET(n, M_DONTWAIT, m->m_type);
> > -   len = MHLEN;
> > -   if (n && todo > MHLEN) {
> > -   MCLGET(n, M_DONTWAIT);
> > -   len = MCLBYTES;
> > +   if (n && remain > MLEN) {
> 
> Depending on m->m_type == MT_HEADER you have compare with MHLEN or MLEN.
> 

The MLEN check is for n which never has m_type MT_HEADER. n is allocated
with MGET() and we want to put remain bytes into it. If that is bigger
than MLEN it will not fit and we allocate a cluster.

> > +   memcpy(n->m_data, mtod(m, char *) + skip, remain);
> > +   n->m_len = remain;
> > +   m->m_len -= remain;
> > +
> > +   n->m_next = m->m_next;
> > +   m->m_next = n;
> > }
> >  
> > if (hlen <= M_TRAILINGSPACE(m) + remain) {
> > -   m->m_len = skip + hlen;
> > +   m->m_len += hlen;
> > *off = skip;
> 
> You update the m->m_len before calculation M_TRAILINGSPACE(m) now.
> So either check that (hlen <= M_TRAILINGSPACE(m)) or move the
> m->m_len -= remain somehow.
> 

True. The '+ remain' is no longer needed. Since at that point we just try
to figure out if the header fits in the original mbuf or if we need to
allocate a new one. Good catch.

Updated version attached
-- 
:wq Claudio

Index: kern/uipc_mbuf.c
===
RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.246
diff -u -p -r1.246 uipc_mbuf.c
--- kern/uipc_mbuf.c8 May 2017 15:47:49 -   1.246
+++ kern/uipc_mbuf.c27 May 2017 07:36:12 -
@@ -1078,7 +1110,12 @@ m_makespace(struct mbuf *m0, int skip, i
struct mbuf *m;
unsigned remain;
 
-   KASSERT(m0 != NULL);
+   /*
+* Limit the size of the new header to MHLEN. In case
+* skip = 0 and the first buffer is not a cluster this
+* is the maximum space available in that mbuf.
+* In other words this code never prepends a mbuf.
+*/
KASSERT(hlen < MHLEN);
 
for (m = m0; m && skip > m->m_len; m = m->m_next)
@@ -1089,7 +1126,7 @@ m_makespace(struct mbuf *m0, int skip, i
 * At this point skip is the offset into the mbuf m
 * where the new header should be placed.  Figure out
 * if there's space to insert the new header.  If so,
-* and copying the remainder makese sense then do so.
+* and copying the remainder makes sense then do so.
 * Otherwise insert a new mbuf in the chain, splitting
 * the contents of m as needed.
 */
@@ -1099,69 +1136,45 @@ m_makespace(struct mbuf *m0, int skip, i
memmove(m->m_data-hlen, m->m_data, skip);
m->m_data -= hlen;
m->m_len += hlen;
-   (*off) = skip;
+   *off = skip;
} else if (hlen > M_TRAILINGSPACE(m)) {
-   struct mbuf *n0, *n, **np;
-   int todo, len, done, alloc;
+   struct mbuf *n;
 
-   n0 = NULL;
-   np = 
-   alloc = 0;
-   done = 0;
-   todo = remain;
-   while (todo > 0) {
+   if (remain > 0) {
MGET(n, M_DONTWAIT, m->m_type);
-   len = MHLEN;
-   if (n && todo > MHLEN) {
-   MCLGET(n, M_DONTWAIT);
-   len = MCLBYTES;
+   if (n && remain > MLEN) {
+   MCLGETI(n, M_DONTWAIT, NULL, remain);
if ((n->m_flags & M_EXT) == 0) {
m_free(n);
n = NULL;
}
}
-   if (n == NULL) {
-   m_freem(n0);
-   return NULL;
-   }
-   *np = n;
-   np = >m_next;
-   alloc++;
-   len = min(todo, len);
-   memcpy(n->m_data, mtod(m, char *) + skip + done, len);
-   n->m_len 

Re: httpd: proposed patch to add TLS client certificate support

[Jack Burton]

On Thu, 30 Mar 2017 22:17:46 +1030
Jack Burton  wrote:
> One of our sites has a need to require/verify TLS client certs,
> without the overhead & complexity of apache / nginx, etc.
> 
> OpenBSD's httpd seemed the obvious candidate, and I figured that the
> feature would be useful to others too -- see attached diff for an
> initial implementation.

Here's an updated diff.

Changes since my first diff are:

* passes through the whole client cert chain to fastcgi (as suggested
  by William Ahern) as TLS_PEER_CHAIN (since beck@'s 5 Apr libtls patch
  now lets us do that easily)

* uses chunked imsg transfers for setting/updating the client CA
  cert chain, as suggested by Jan Klemkow (but I couldn't get his
  implementation of that from last year to work reliably, so I rewrote
  it), to support long CA chains

* adds a regression test, also as suggested by Jan Klemkow (but keeps
  it separate from existing tls regression tests).

In & of itself, my diff still only adds support to httpd for verifying
client certificate authenticity (not for checking certificate
revocation status).

But at least it's now sufficiently complete (which my first diff
wasn't) to allow fastcgi responders to implement certificate revocation
status checking.

So I figured that this would be a good place to stop and ask for review
& approval or criticism of this diff (before returning to the question
of whether CRL [or whatever other form of client cert revocation
status] checking most "belongs" in libtls, in httpd or in the fastcgi
responders).


Index: usr.sbin/httpd/config.c
===
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.51
diff -u -p -r1.51 config.c
--- usr.sbin/httpd/config.c 25 Mar 2017 17:25:34 -  1.51
+++ usr.sbin/httpd/config.c 27 May 2017 06:14:27 -
@@ -326,6 +326,57 @@ config_settls(struct httpd *env, struct 
}
}
 
+   if (srv_conf->tls_ca_len != 0)
+   return config_settls_ca(env, srv);
+
+   return (0);
+}
+
+int
+config_settls_ca(struct httpd *env, struct server *srv)
+{
+   struct privsep  *ps = env->sc_ps;
+   struct server_config*srv_conf = >srv_conf;
+   struct tls_configtls;
+   struct iovec iov[2];
+   size_t   c, chunk_len, i, sent;
+
+   if ((srv_conf->flags & SRVFLAG_TLS) == 0)
+   return (0);
+
+   log_debug("%s: configuring tls ca for %s", __func__, srv_conf->name);
+
+   if (srv_conf->tls_ca_len != 0) {
+   DPRINTF("%s: sending ca cert(s) for \"%s[%u]\" to %s fd %d",
+   __func__, srv_conf->name, srv_conf->id,
+   ps->ps_title[PROC_SERVER], srv->srv_s);
+
+   memset(, 0, sizeof(tls));
+   tls.id = srv_conf->id;
+   tls.tls_ca_len = srv_conf->tls_ca_len;
+
+   chunk_len = MAX_IMSGSIZE - (ssize_t)IMSG_HEADER_SIZE - 
sizeof(tls);
+   for (sent = 0, i = 1; sent < tls.tls_ca_len;
+   sent += chunk_len, i++) {
+   if (tls.tls_ca_len - sent < chunk_len)
+   chunk_len = tls.tls_ca_len - sent;
+   tls.tls_ca_off = sent;
+   c = 0;
+   iov[c].iov_base = 
+   iov[c++].iov_len = sizeof(tls);
+   iov[c].iov_base = srv_conf->tls_ca + sent;
+   iov[c++].iov_len = chunk_len;
+
+   if (proc_composev(ps, PROC_SERVER, IMSG_CFG_TLS_CA,
+   iov, c) != 0) {
+   log_warn("%s: failed to compose chunk %lu of "
+   "IMSG_CFG_TLS_CA imsg for `%s'", __func__,
+   i, srv_conf->name);
+   return (-1);
+   }
+   }
+   }
+
return (0);
 }
 
@@ -644,6 +695,60 @@ config_gettls(struct httpd *env, struct 
tls_conf.tls_ocsp_staple_len)) == NULL)
goto fail;
s += tls_conf.tls_ocsp_staple_len;
+   }
+
+   return (0);
+
+ fail:
+   return (-1);
+}
+
+int
+config_gettls_ca(struct httpd *env, struct imsg *imsg)
+{
+#ifdef DEBUG
+   struct privsep  *ps = env->sc_ps;
+#endif
+   struct server_config*srv_conf = NULL;
+   struct tls_configtls_conf;
+   uint8_t *p = imsg->data;
+   size_t   s, chunk_len, min_chunk_len;
+
+   IMSG_SIZE_CHECK(imsg, _conf);
+   memcpy(_conf, p, sizeof(tls_conf));
+   s = sizeof(tls_conf);
+   chunk_len = MAX_IMSGSIZE - IMSG_HEADER_SIZE - s;
+   min_chunk_len = (tls_conf.tls_ca_len - tls_conf.tls_ca_off) > chunk_len
+   ? chunk_len : tls_conf.tls_ca_len - tls_conf.tls_ca_off;
+
+   if ((IMSG_DATA_SIZE(imsg) - s) 

Fix addition of routes to a rtable

[Claudio Jeker]

Found the other day:
`route -T 10 add default 127.0.0.1` fails with:
add net default: gateway 127.0.0.1: Network is unreachable
The workaround to make it work is to add -ifp lo0 

The problem is in ifa_ifwithroute() which does an rtalloc() call to get
the gateway route and it fails for rtables (not rdomains). Because the
gateway routes for rtables live in the rdomain.
The following diff fixes this problem by using rtable_l2().

OK?
-- 
:wq Claudio

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.356
diff -u -p -r1.356 route.c
--- net/route.c 16 May 2017 12:24:01 -  1.356
+++ net/route.c 24 May 2017 07:25:06 -
@@ -750,7 +750,8 @@ ifa_ifwithroute(int flags, struct sockad
} else {
struct rtentry *rt;
 
-   rt = rtalloc(gateway, RT_RESOLVE, rtableid);
+   /* need to do the rtalloc on the rdomain */
+   rt = rtalloc(gateway, RT_RESOLVE, rtable_l2(rtableid));
if (rt != NULL)
ifa = rt->rt_ifa;
rtfree(rt);

Re: httpd/libtls: TLS client certificate revocation checking

[Jack Burton]

On Sun, 2 Apr 2017 06:27:45 +0930
Jack Burton  wrote:
> On Sat, 01 Apr 2017 18:22:17 +
> Bob Beck  wrote:
> > There will be some libtls api additions post 6.1 to get the peer
> > cert in PEM format  
> 
> Thanks Bob. That sounds like exactly what's needed. Happy to wait.

...and your tls_peer_cert_chain_pem() solves things nicely.

Many thanks for that.

Here's a little diff to document it.

Index: lib/libtls/man/tls_conn_version.3
===
RCS file: /cvs/src/lib/libtls/man/tls_conn_version.3,v
retrieving revision 1.4
diff -u -p -r1.4 tls_conn_version.3
--- lib/libtls/man/tls_conn_version.3   28 Jan 2017 00:59:36 -  1.4
+++ lib/libtls/man/tls_conn_version.3   27 May 2017 06:55:41 -
@@ -24,6 +24,7 @@
 .Nm tls_conn_alpn_selected ,
 .Nm tls_conn_servername ,
 .Nm tls_peer_cert_provided ,
+.Nm tls_peer_cert_chain_pem ,
 .Nm tls_peer_cert_contains_name ,
 .Nm tls_peer_cert_issuer ,
 .Nm tls_peer_cert_subject ,
@@ -43,6 +44,11 @@
 .Fn tls_conn_servername "struct tls *ctx"
 .Ft int
 .Fn tls_peer_cert_provided "struct tls *ctx"
+.Ft const uint8_t *
+.Fo tls_peer_cert_chain_pem
+.Fa "struct tls *ctx"
+.Fa "size_t *size"
+.Fc
 .Ft int
 .Fo tls_peer_cert_contains_name
 .Fa "struct tls *ctx"
@@ -89,6 +95,14 @@ checks if the peer of
 .Ar ctx
 has provided a certificate.
 .Pp
+.Fn tls_peer_cert_chain_pem
+returns a string consisting of the PEM encoded certificate chain of the peer
+from
+.Ar ctx .
+The value of
+.Ar size
+is set to the length of the string.
+.Pp
 .Fn tls_peer_cert_contains_name
 checks if the peer of a TLS
 .Ar ctx
@@ -172,6 +186,10 @@ and
 .Fn tls_conn_alpn_selected
 appeared in
 .Ox 6.1 .
+.Pp
+.Fn tls_peer_cert_chain_pem
+appeared in
+.Ox 6.2 .
 .Sh AUTHORS
 .An Bob Beck Aq Mt b...@openbsd.org
 .An Joel Sing Aq Mt js...@openbsd.org

Re: pppoe(4) vs splnet

[Martin Pieuchot]

On 26/05/17(Fri) 23:02, Alexandr Nedvedicky wrote:
> Hello,
> 
> I hope my question does not sound dumb...
> 
> The function pppoe_timeout() grabs NET_LOCK() at line 1059 and then
> it goes after splnet() at line 1076, is that intentional?

It's rather historical than intentional.  It was already like that, I
did not spent any time to figure out if the splnet() are still really
needed or not.