Re: mg: fgetln -> getline
> On Oct 11, 2017, at 2:36 AM, Florian Obser wrote: > > On Sun, Sep 17, 2017 at 02:56:32AM +, Scott Cheloha wrote: >> >> >> if (buf[len - 1] == '\n') >> buf[len - 1] = '\0'; > > the diff reads fine to me, but code tends to get copied around, > can you send a diff with those lines added please? That's the > canonical way of using getline I believe and there is no good > reason for mg to be different, thanks! here you go -- Scott Cheloha Index: usr.bin/mg/cscope.c === RCS file: /cvs/src/usr.bin/mg/cscope.c,v retrieving revision 1.16 diff -u -p -r1.16 cscope.c --- usr.bin/mg/cscope.c 19 Jan 2016 14:51:00 - 1.16 +++ usr.bin/mg/cscope.c 12 Oct 2017 04:11:36 - @@ -166,9 +166,13 @@ cscreatelist(int f, int n) struct stat sb; FILE *fpipe; char dir[NFILEN], cmd[BUFSIZ], title[BUFSIZ], *line, *bufp; - size_t len; + size_t sz; + ssize_t len; int clen; + line = NULL; + sz = 0; + if (getbufcwd(dir, sizeof(dir)) == FALSE) dir[0] = '\0'; @@ -221,11 +225,14 @@ cscreatelist(int f, int n) } addline(bp, title); addline(bp, ""); - /* All lines are NUL terminated */ - while ((line = fgetln(fpipe, &len)) != NULL) { - line[len - 1] = '\0'; + while ((len = getline(&line, &sz, fpipe)) != -1) { + if (line[len - 1] == '\n') + line[len - 1] = '\0'; addline(bp, line); } + free(line); + if (ferror(fpipe)) + ewprintf("Problem reading pipe"); pclose(fpipe); return (popbuftop(bp, WNONE)); } @@ -397,7 +404,11 @@ do_cscope(int i) char pattern[MAX_TOKEN], cmd[BUFSIZ], title[BUFSIZ]; char *p, *buf; int clen, nores = 0; - size_t len; + size_t sz; + ssize_t len; + + buf = NULL; + sz = 0; /* If current buffer isn't a source file just return */ if (fnmatch("*.[chy]", curbp->b_fname, 0) != 0) { @@ -447,13 +458,18 @@ do_cscope(int i) addline(bp, title); addline(bp, ""); addline(bp, "---"); - /* All lines are NUL terminated */ - while ((buf = fgetln(fpipe, &len)) != NULL) { - buf[len - 1] = '\0'; - if (addentry(bp, buf) != TRUE) + while ((len = getline(&buf, &sz, fpipe)) != -1) { + if (buf[len - 1] == '\n') + buf[len - 1] = '\0'; + if (addentry(bp, buf) != TRUE) { + free(buf); return (FALSE); + } nores = 1; - }; + } + free(buf); + if (ferror(fpipe)) + ewprintf("Problem reading pipe"); pclose(fpipe); addline(bp, "---"); if (nores == 0) Index: usr.bin/mg/grep.c === RCS file: /cvs/src/usr.bin/mg/grep.c,v retrieving revision 1.44 diff -u -p -r1.44 grep.c --- usr.bin/mg/grep.c 19 Mar 2015 21:48:05 - 1.44 +++ usr.bin/mg/grep.c 12 Oct 2017 04:11:36 - @@ -178,12 +178,16 @@ compile_mode(const char *name, const cha struct buffer *bp; FILE*fpipe; char*buf; - size_t len; + size_t sz; + ssize_t len; int ret, n; char cwd[NFILEN], qcmd[NFILEN]; char timestr[NTIME]; time_t t; + buf = NULL; + sz = 0; + n = snprintf(qcmd, sizeof(qcmd), "%s 2>&1", command); if (n < 0 || n >= sizeof(qcmd)) return (NULL); @@ -210,15 +214,14 @@ compile_mode(const char *name, const cha ewprintf("Problem opening pipe"); return (NULL); } - /* -* We know that our commands are nice and the last line will end with -* a \n, so we don't need to try to deal with the last line problem -* in fgetln. -*/ - while ((buf = fgetln(fpipe, &len)) != NULL) { - buf[len - 1] = '\0'; + while ((len = getline(&buf, &sz, fpipe)) != -1) { + if (buf[len - 1] == '\n') + buf[len - 1] = '\0'; addline(bp, buf); } + free(buf); + if (ferror(fpipe)) + ewprintf("Problem reading pipe"); ret = pclose(fpipe); t = time(NULL); strftime(timestr, sizeof(timestr), "%a %b %e %T %Y", localtime(&t));
Re: [patch] hostname.if5 additional info on point to point addressing
Hello Stuart, all, Thanks for the corrections Stuart, I have corrected the patch to take into account your suggestions and I hope this proposed patch is more correct and useful Index: src/share/man/man5/hostname.if.5 === RCS file: /cvs/src/share/man/man5/hostname.if.5,v retrieving revision 1.65 diff -u -p -u -r1.65 hostname.if.5 --- src/share/man/man5/hostname.if.510 Mar 2017 18:28:11 - 1.65 +++ src/share/man/man5/hostname.if.512 Oct 2017 00:06:15 - @@ -91,6 +91,16 @@ Regular IPv4 network setup: .Va dest_addr .Ed .Pp +Point to Point IPv4 network setup: +.Bd -ragged -offset indent +.Li inet +.Op Li alias +.Va addr +.Va netmask +.Va network_addr +.Va options +.Ed +.Pp Regular IPv6 network setup: .Bd -ragged -offset indent .Li inet6 @@ -122,6 +132,15 @@ inet6 alias fec0::1 64 inet6 alias fec0::2 64 anycast !route add 65.65.65.65 10.0.1.13 up +.Ed +.Pp +Point to point IP addresses or IP unnumbered addresses +can also be applied to an interface iff it is a tunnel or serial interface +such as; gif(4), gre(4), pppoe(4), ppp(4), sppp(4). +For example: +.Bd -literal -offset 1n +inet 10.64.100.2 0x 10.64.80.25 +#local_addr /32_netmask remote_addr .Ed .Pp The above formats have the following field values: On 2 October 2017 at 11:33, Stuart Henderson wrote: > On 2017/10/02 03:04, Tom Smyth wrote: >> Hello, >> >> But the Ip configuration syntax in hostname.if is the same. > > For a /31 you just use e.g. "inet 192.0.2.100/31" (and it works properly > in other parts of the system, e.g. ospfd). > >> Is there anything specifically wrong in the proposed patch ? > > This configuration only works on actual point-to-point interfaces (gif, gre, > tun). Without further explanation people might expect it to work on ethernet > like interfaces, and the "endpoint" address (10.64.80.25 in your example) > doesn't do anything there. > -- Kindest regards, Tom Smyth Mobile: +353 87 6193172 The information contained in this E-mail is intended only for the confidential use of the named recipient. If the reader of this message is not the intended recipient or the person responsible for delivering it to the recipient, you are hereby notified that you have received this communication in error and that any review, dissemination or copying of this communication is strictly prohibited. If you have received this in error, please notify the sender immediately by telephone at the number above and erase the message You are requested to carry out your own virus check before opening any attachment.
route warning
Hi, when a default gateway is not set : # route get 4.4.4.4 route: writing to routing socket: No such process this small patch uses oerrno translation : # route get 4.4.4.4 get host 4.4.4.4: not in table Index: route.c === RCS file: /cvs/src/sbin/route/route.c,v retrieving revision 1.203 diff -u -p -r1.203 route.c --- route.c 6 Sep 2017 20:21:22 - 1.203 +++ route.c 11 Oct 2017 22:05:22 - @@ -676,8 +676,7 @@ newroute(int argc, char **argv) } if (*cmd == 'g') { if (ret != 0 && qflag == 0) - warn("writing to routing socket"); - exit(0); + oerrno = ESRCH; } oerrno = errno; if (!qflag) {
Re: IPsec w/o KERNEL_LOCK()
On Wed, Oct 11, 2017 at 05:01:46PM +0200, Martin Pieuchot wrote: > Tests and comments welcome. All regress tests passed. Code looks reasonable. OK bluhm@ > Index: net/if_enc.c > === > RCS file: /cvs/src/sys/net/if_enc.c,v > retrieving revision 1.69 > diff -u -p -r1.69 if_enc.c > --- net/if_enc.c 11 Aug 2017 21:24:19 - 1.69 > +++ net/if_enc.c 11 Oct 2017 14:44:48 - > @@ -214,6 +214,8 @@ enc_getif(u_int id, u_int unit) > { > struct ifnet*ifp; > > + NET_ASSERT_LOCKED(); > + > /* Check if the caller wants to get a non-default enc interface */ > if (unit > 0) { > if (unit > enc_max_unit) > Index: net/pfkeyv2.c > === > RCS file: /cvs/src/sys/net/pfkeyv2.c,v > retrieving revision 1.167 > diff -u -p -r1.167 pfkeyv2.c > --- net/pfkeyv2.c 9 Oct 2017 08:35:38 - 1.167 > +++ net/pfkeyv2.c 11 Oct 2017 14:44:49 - > @@ -80,6 +80,8 @@ > #include > #include > #include > +#include > + > #include > #include > #include > @@ -148,6 +150,8 @@ struct dump_state { > > /* Static globals */ > static LIST_HEAD(, keycb) pfkeyv2_sockets = LIST_HEAD_INITIALIZER(keycb); > + > +struct mutex pfkeyv2_mtx = MUTEX_INITIALIZER(IPL_NONE); > static uint32_t pfkeyv2_seq = 1; > static int nregistered = 0; > static int npromisc = 0; > @@ -260,11 +264,16 @@ pfkeyv2_detach(struct socket *so, struct > > LIST_REMOVE(kp, kcb_list); > > - if (kp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED) > - nregistered--; > - > - if (kp->flags & PFKEYV2_SOCKETFLAGS_PROMISC) > - npromisc--; > + if (kp->flags & > + (PFKEYV2_SOCKETFLAGS_REGISTERED|PFKEYV2_SOCKETFLAGS_PROMISC)) { > + mtx_enter(&pfkeyv2_mtx); > + if (kp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED) > + nregistered--; > + > + if (kp->flags & PFKEYV2_SOCKETFLAGS_PROMISC) > + npromisc--; > + mtx_leave(&pfkeyv2_mtx); > + } > > raw_detach(&kp->rcb); > return (0); > @@ -940,23 +949,22 @@ pfkeyv2_send(struct socket *so, void *me > struct ipsec_acquire *ipa; > struct radix_node_head *rnh; > struct radix_node *rn = NULL; > - > struct keycb *kp, *bkp; > - > void *freeme = NULL, *bckptr = NULL; > void *headers[SADB_EXT_MAX + 1]; > - > union sockaddr_union *sunionp; > - > struct tdb *sa1 = NULL, *sa2 = NULL; > - > struct sadb_msg *smsg; > struct sadb_spirange *sprng; > struct sadb_sa *ssa; > struct sadb_supported *ssup; > struct sadb_ident *sid, *did; > - > u_int rdomain; > + int promisc; > + > + mtx_enter(&pfkeyv2_mtx); > + promisc = npromisc; > + mtx_leave(&pfkeyv2_mtx); > > NET_LOCK(); > > @@ -972,7 +980,7 @@ pfkeyv2_send(struct socket *so, void *me > rdomain = kp->rdomain; > > /* If we have any promiscuous listeners, send them a copy of the > message */ > - if (npromisc) { > + if (promisc) { > struct mbuf *packet; > > if (!(freeme = malloc(sizeof(struct sadb_msg) + len, M_PFKEY, > @@ -1392,7 +1400,9 @@ pfkeyv2_send(struct socket *so, void *me > case SADB_REGISTER: > if (!(kp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)) { > kp->flags |= PFKEYV2_SOCKETFLAGS_REGISTERED; > + mtx_enter(&pfkeyv2_mtx); > nregistered++; > + mtx_leave(&pfkeyv2_mtx); > } > > i = sizeof(struct sadb_supported) + sizeof(ealgs); > @@ -1788,11 +1798,15 @@ pfkeyv2_send(struct socket *so, void *me > if (j) { > kp->flags |= > PFKEYV2_SOCKETFLAGS_PROMISC; > + mtx_enter(&pfkeyv2_mtx); > npromisc++; > + mtx_leave(&pfkeyv2_mtx); > } else { > kp->flags &= > ~PFKEYV2_SOCKETFLAGS_PROMISC; > + mtx_enter(&pfkeyv2_mtx); > npromisc--; > + mtx_leave(&pfkeyv2_mtx); > } > } > } > @@ -1859,11 +1873,15 @@ pfkeyv2_acquire(struct ipsec_policy *ipo > struct sadb_prop *sa_prop; > struct sadb_msg *smsg; > int rval = 0; > - int i, j; > + int i, j, registered; > > + mtx_enter(&pfkeyv2_mtx); > *seq = pfkeyv2_seq++; > > - if (!nregistered) { > + registered = nregistered; > + mtx_leave(&pfkeyv2_mtx); > + > + if (!registered) { > rval = E
Re: rw locks vs memory barriers and adaptive spinning
On Wed, Oct 11, 2017 at 03:15:42PM +0200, Martin Pieuchot wrote: > On 10/10/17(Tue) 20:19, Mateusz Guzik wrote: > > On Tue, Oct 10, 2017 at 10:15:48AM +0200, Martin Pieuchot wrote: > > > Hello Mateusz, > > > > > > On 09/10/17(Mon) 21:43, Mateusz Guzik wrote: > > > > I was looking at rw lock code out of curiosity and noticed you always do > > > > membar_enter which on MP-enabled amd64 kernel translates to mfence. > > > > This makes the entire business a little bit slower. > > > > > > > > Interestingly you already have relevant macros for amd64: > > > > #define membar_enter_after_atomic() __membar("") > > > > #define membar_exit_before_atomic() __membar("") > > > > > > If you look at the history, you'll see that theses macro didn't exist > > > when membar_* where added to rw locks. > > > > > > > I know. Addition of such a macro sounds like an accelent opportunity to > > examine existing users. I figured rw locks were ommitted by accident > > as the original posting explicitly mentions mutexes. > > > > https://marc.info/?l=openbsd-tech&m=149555411827749&w=2 > > > > > > And there is even a default variant for archs which don't provide one. > > > > I guess the switch should be easy. > > > > > > Then please do it :) At lot of stuff is easy on OpenBSD but we are not > > > enough. > > > > > > Do it, test it, explain it, get oks and commit it 8) > > > > > > > As noted earlier the patch is trivial. I have no easy means to even > > compile-test it though as I'm not using OpenBSD. Only had a look out of > > curiosity. > > Why don't you start using it? Testing is what makes the difference. > Many of us have ideas of how to improve the kernel but we're lacking > the time to test & debug them all. > I'm playing with concurrency as a hobby and smpifying single-threaded code is above the pain threshold I'm willing to accept in my spare time. > Sometimes a "correct" change exposes a bug and we try not to break > -current, so we cannot afford to commit a non-tested diff. > I definitely don't advocate for just committing stuff. I do state though that there should be no binary change for archs other than i386 and amd64 (== nothing to test there). Given the nature of the change for these two it really seemed it just fell through the cracks earlier and I just wanted to point it out as perhaps someone will find cycles to pick it up and test on relevant platforms. > > Nonetheless, here is the diff which can be split in 2. One part deals > > with barriers, another one cleans up rw_status. > > > > [...] > > > > Read from the lock only once in rw_status. The lock word is marked as > > volatile which forces re-read on each access. There is no correctness > > issue here, but the assembly is worse than it needs to be and in case > > of contention this adds avoidable cache traffic. > > Here's the diff to cleans rw_status. The difference on amd64 with > clang 4.0 is the following: > > 48 8b 0fmov(%rdi),%rcx > f6 c1 04test $0x4,%cl > 75 0c jne738 > 31 c0 xor%eax,%eax > - 48 83 3f 00 cmpq $0x0,(%rdi) > + 48 85 c9test %rcx,%rcx > > ok? There should be another change past rrw_status+0x18: 812557e8: 65 48 8b 04 25 08 00mov%gs:0x8,%rax 812557ef: 00 00 812557f1: 48 8b 80 90 02 00 00mov0x290(%rax),%rax 812557f8: 48 33 07xor(%rdi),%rax > > Index: kern/kern_rwlock.c > === > RCS file: /cvs/src/sys/kern/kern_rwlock.c,v > retrieving revision 1.30 > diff -u -p -r1.30 kern_rwlock.c > --- kern/kern_rwlock.c12 Aug 2017 23:27:44 - 1.30 > +++ kern/kern_rwlock.c11 Oct 2017 12:59:19 - > @@ -312,13 +312,15 @@ _rw_exit(struct rwlock *rwl LOCK_FL_VARS > int > rw_status(struct rwlock *rwl) > { > - if (rwl->rwl_owner & RWLOCK_WRLOCK) { > - if (RW_PROC(curproc) == RW_PROC(rwl->rwl_owner)) > + unsigned long owner = rwl->rwl_owner; > + > + if (owner & RWLOCK_WRLOCK) { > + if (RW_PROC(curproc) == RW_PROC(owner)) > return RW_WRITE; > else > return RW_WRITE_OTHER; > } > - if (rwl->rwl_owner) > + if (owner) > return RW_READ; > return (0); > } -- Mateusz Guzik
Re: perl 5.24.3 update (OK?)
On Fri, Sep 22, 2017 at 06:56:27PM -0700, Andrew Fresh wrote: > Hoping this will be able to go in before the lock, I've heard good Updating Perl after unlock is better. Running with it for a while without problems. No library bump needed. OK bluhm@ for Perl v5.24.3
IPsec w/o KERNEL_LOCK()
OpenBSD 6.2 includes nice performance and latency improvements due to the work done in the Network Stack in the previous years. However as soon as IPsec is enabled, all network related processing are affected. In other words you cannot profit from the last MP work in the Network stack if you use IPsec. During the last 6 months I hoped that somebody else would look at the IPsec stack and tell us what needs to be done. This didn't happen. Now that 6.2 is released, we cannot afford to continue to parallelize the Network Stack if some of our users and testers still run it under KERNEL_LOCK(). So I did an audit of the IPsec stack and came with the small diff below. This diff doesn't remove the KERNEL_LOCK() (yet), but add some asserts to make sure that the global data structure are all accessed while holding the NET_LOCK(). I introduced a mutex for protecting some globals in the PF_KEY sockets. If you want to improve it and try to unlock the PF_KEY socket layer, that'd make me really happy. In the whole IPsec stack, a single timeout wasn't running in process context, I changed that and made it grab the NET_LOCK(). I tried to document which data structures are protected by the NET_LOCK(), to make it short it's "all of them". Tests and comments welcome. Index: net/if_enc.c === RCS file: /cvs/src/sys/net/if_enc.c,v retrieving revision 1.69 diff -u -p -r1.69 if_enc.c --- net/if_enc.c11 Aug 2017 21:24:19 - 1.69 +++ net/if_enc.c11 Oct 2017 14:44:48 - @@ -214,6 +214,8 @@ enc_getif(u_int id, u_int unit) { struct ifnet*ifp; + NET_ASSERT_LOCKED(); + /* Check if the caller wants to get a non-default enc interface */ if (unit > 0) { if (unit > enc_max_unit) Index: net/pfkeyv2.c === RCS file: /cvs/src/sys/net/pfkeyv2.c,v retrieving revision 1.167 diff -u -p -r1.167 pfkeyv2.c --- net/pfkeyv2.c 9 Oct 2017 08:35:38 - 1.167 +++ net/pfkeyv2.c 11 Oct 2017 14:44:49 - @@ -80,6 +80,8 @@ #include #include #include +#include + #include #include #include @@ -148,6 +150,8 @@ struct dump_state { /* Static globals */ static LIST_HEAD(, keycb) pfkeyv2_sockets = LIST_HEAD_INITIALIZER(keycb); + +struct mutex pfkeyv2_mtx = MUTEX_INITIALIZER(IPL_NONE); static uint32_t pfkeyv2_seq = 1; static int nregistered = 0; static int npromisc = 0; @@ -260,11 +264,16 @@ pfkeyv2_detach(struct socket *so, struct LIST_REMOVE(kp, kcb_list); - if (kp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED) - nregistered--; - - if (kp->flags & PFKEYV2_SOCKETFLAGS_PROMISC) - npromisc--; + if (kp->flags & + (PFKEYV2_SOCKETFLAGS_REGISTERED|PFKEYV2_SOCKETFLAGS_PROMISC)) { + mtx_enter(&pfkeyv2_mtx); + if (kp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED) + nregistered--; + + if (kp->flags & PFKEYV2_SOCKETFLAGS_PROMISC) + npromisc--; + mtx_leave(&pfkeyv2_mtx); + } raw_detach(&kp->rcb); return (0); @@ -940,23 +949,22 @@ pfkeyv2_send(struct socket *so, void *me struct ipsec_acquire *ipa; struct radix_node_head *rnh; struct radix_node *rn = NULL; - struct keycb *kp, *bkp; - void *freeme = NULL, *bckptr = NULL; void *headers[SADB_EXT_MAX + 1]; - union sockaddr_union *sunionp; - struct tdb *sa1 = NULL, *sa2 = NULL; - struct sadb_msg *smsg; struct sadb_spirange *sprng; struct sadb_sa *ssa; struct sadb_supported *ssup; struct sadb_ident *sid, *did; - u_int rdomain; + int promisc; + + mtx_enter(&pfkeyv2_mtx); + promisc = npromisc; + mtx_leave(&pfkeyv2_mtx); NET_LOCK(); @@ -972,7 +980,7 @@ pfkeyv2_send(struct socket *so, void *me rdomain = kp->rdomain; /* If we have any promiscuous listeners, send them a copy of the message */ - if (npromisc) { + if (promisc) { struct mbuf *packet; if (!(freeme = malloc(sizeof(struct sadb_msg) + len, M_PFKEY, @@ -1392,7 +1400,9 @@ pfkeyv2_send(struct socket *so, void *me case SADB_REGISTER: if (!(kp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)) { kp->flags |= PFKEYV2_SOCKETFLAGS_REGISTERED; + mtx_enter(&pfkeyv2_mtx); nregistered++; + mtx_leave(&pfkeyv2_mtx); } i = sizeof(struct sadb_supported) + sizeof(ealgs); @@ -1788,11 +1798,15 @@ pfkeyv2_send(struct socket *so, void *me if (j) { kp->flags |= PFKEYV2_SOCKETFLAGS_PROMISC; +
Re: Send sysctl_mq() where it belongs
On Wed, Oct 11, 2017 at 02:58:21PM +0200, Martin Pieuchot wrote: > sysctl_mq() messes with 'struct mbuf_queue' internals, so keep it in > kern/uipc_mbuf.c with the rest of the mq* functions. I wonder whether it should be called mq_sysctl() as it will be located in the mq_ namespace. A grep for _sysctl and sysctl_ shows that we are rather inconsistent in naming, so commit it either way. > ok? OK bluhm@ > Index: net/if.c > === > RCS file: /cvs/src/sys/net/if.c,v > retrieving revision 1.514 > diff -u -p -r1.514 if.c > --- net/if.c 11 Oct 2017 07:57:27 - 1.514 > +++ net/if.c 11 Oct 2017 12:56:18 - > @@ -82,7 +82,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -2654,38 +2653,6 @@ ifpromisc(struct ifnet *ifp, int pswitch > } > ifr.ifr_flags = ifp->if_flags; > return ((*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifr)); > -} > - > -/* XXX move to kern/uipc_mbuf.c */ > -int > -sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp, > -void *newp, size_t newlen, struct mbuf_queue *mq) > -{ > - unsigned int maxlen; > - int error; > - > - /* All sysctl names at this level are terminal. */ > - if (namelen != 1) > - return (ENOTDIR); > - > - switch (name[0]) { > - case IFQCTL_LEN: > - return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq))); > - case IFQCTL_MAXLEN: > - maxlen = mq->mq_maxlen; > - error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen); > - if (!error && maxlen != mq->mq_maxlen) { > - mtx_enter(&mq->mq_mtx); > - mq->mq_maxlen = maxlen; > - mtx_leave(&mq->mq_mtx); > - } > - return (error); > - case IFQCTL_DROPS: > - return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq))); > - default: > - return (EOPNOTSUPP); > - } > - /* NOTREACHED */ > } > > void > Index: net/if_var.h > === > RCS file: /cvs/src/sys/net/if_var.h,v > retrieving revision 1.81 > diff -u -p -r1.81 if_var.h > --- net/if_var.h 8 May 2017 08:46:39 - 1.81 > +++ net/if_var.h 11 Oct 2017 12:56:18 - > @@ -324,9 +324,6 @@ int if_clone_destroy(const char *); > struct if_clone * > if_clone_lookup(const char *, int *); > > -int sysctl_mq(int *, u_int, void *, size_t *, void *, size_t, > - struct mbuf_queue *); > - > void ifa_add(struct ifnet *, struct ifaddr *); > void ifa_del(struct ifnet *, struct ifaddr *); > void ifa_update_broadaddr(struct ifnet *, struct ifaddr *, > Index: kern/uipc_mbuf.c > === > RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v > retrieving revision 1.249 > diff -u -p -r1.249 uipc_mbuf.c > --- kern/uipc_mbuf.c 15 Sep 2017 18:13:05 - 1.249 > +++ kern/uipc_mbuf.c 11 Oct 2017 12:56:19 - > @@ -84,6 +84,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -1648,4 +1649,35 @@ mq_purge(struct mbuf_queue *mq) > mq_delist(mq, &ml); > > return (ml_purge(&ml)); > +} > + > +int > +sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp, > +void *newp, size_t newlen, struct mbuf_queue *mq) > +{ > + unsigned int maxlen; > + int error; > + > + /* All sysctl names at this level are terminal. */ > + if (namelen != 1) > + return (ENOTDIR); > + > + switch (name[0]) { > + case IFQCTL_LEN: > + return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq))); > + case IFQCTL_MAXLEN: > + maxlen = mq->mq_maxlen; > + error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen); > + if (!error && maxlen != mq->mq_maxlen) { > + mtx_enter(&mq->mq_mtx); > + mq->mq_maxlen = maxlen; > + mtx_leave(&mq->mq_mtx); > + } > + return (error); > + case IFQCTL_DROPS: > + return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq))); > + default: > + return (EOPNOTSUPP); > + } > + /* NOTREACHED */ > } > Index: sys/sysctl.h > === > RCS file: /cvs/src/sys/sys/sysctl.h,v > retrieving revision 1.174 > diff -u -p -r1.174 sysctl.h > --- sys/sysctl.h 14 Jun 2017 03:00:40 - 1.174 > +++ sys/sysctl.h 11 Oct 2017 12:56:21 - > @@ -936,6 +936,9 @@ int sysctl_rdstruct(void *, size_t *, vo > int sysctl_struct(void *, size_t *, void *, size_t, void *, size_t); > int sysctl_file(int *, u_int, char *, size_t *, struct proc *); > int sysctl_doproc(int *, u_int, char *, size_t *); > +struct mbuf_queue; > +int sysctl_mq(int *, u_int, void *, size_t *, void *, size_t, > +struct mb
Re: ifioctl() cleanup, step 2
On Wed, Oct 11, 2017 at 10:44:22AM +0200, Martin Pieuchot wrote: > Moar cleanup to be able to selectively take the NET_LOCK() around some > ioctls. > > This diff change many "return (error)" into "break". It looks a bit inconsistent, where you replace the return and where not. But I am sure your next diff will resolve this. > It adds error checks for SIOC{A,D}IFGROUP. The only driver handling > these ioctl(2)s is... carp(4)! This (error == ENOTTY) is strange. I hides something in the kernel where the user land requests impossible things and cannot cope with an error. But as we do not want to break things, I think this approach makes sense. > ok? OK bluhm@ > Index: net/if.c > === > RCS file: /cvs/src/sys/net/if.c,v > retrieving revision 1.514 > diff -u -p -r1.514 if.c > --- net/if.c 11 Oct 2017 07:57:27 - 1.514 > +++ net/if.c 11 Oct 2017 08:37:31 - > @@ -1817,7 +1817,7 @@ ifioctl(struct socket *so, u_long cmd, c > { > struct ifnet *ifp; > struct ifreq *ifr = (struct ifreq *)data; > - struct ifgroupreq *ifgr; > + struct ifgroupreq *ifgr = (struct ifgroupreq *)data; > struct if_afreq *ifar = (struct if_afreq *)data; > char ifdescrbuf[IFDESCRSIZE]; > char ifrtlabelbuf[RTLABEL_LEN]; > @@ -1858,7 +1858,7 @@ ifioctl(struct socket *so, u_long cmd, c > case SIOCIFAFATTACH: > case SIOCIFAFDETACH: > if ((error = suser(p, 0)) != 0) > - return (error); > + break; > switch (ifar->ifar_af) { > case AF_INET: > /* attach is a noop for AF_INET */ > @@ -1874,7 +1874,7 @@ ifioctl(struct socket *so, u_long cmd, c > break; > #endif /* INET6 */ > default: > - return (EAFNOSUPPORT); > + error = EAFNOSUPPORT; > } > break; > > @@ -1909,7 +1909,7 @@ ifioctl(struct socket *so, u_long cmd, c > > case SIOCSIFFLAGS: > if ((error = suser(p, 0)) != 0) > - return (error); > + break; > > ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) | > (ifr->ifr_flags & ~IFF_CANTCHANGE); > @@ -1934,13 +1934,13 @@ ifioctl(struct socket *so, u_long cmd, c > > case SIOCSIFXFLAGS: > if ((error = suser(p, 0)) != 0) > - return (error); > + break; > > #ifdef INET6 > if (ISSET(ifr->ifr_flags, IFXF_AUTOCONF6)) { > error = in6_ifattach(ifp); > if (error != 0) > - return (error); > + break; > } > #endif /* INET6 */ > > @@ -1972,7 +1972,7 @@ ifioctl(struct socket *so, u_long cmd, c > error = ifp->if_wol(ifp, 1); > splx(s); > if (error) > - return (error); > + break; > } > if (ISSET(ifp->if_xflags, IFXF_WOL) && > !ISSET(ifr->ifr_flags, IFXF_WOL)) { > @@ -1981,7 +1981,7 @@ ifioctl(struct socket *so, u_long cmd, c > error = ifp->if_wol(ifp, 0); > splx(s); > if (error) > - return (error); > + break; > } > } else if (ISSET(ifr->ifr_flags, IFXF_WOL)) { > ifr->ifr_flags &= ~IFXF_WOL; > @@ -1995,13 +1995,13 @@ ifioctl(struct socket *so, u_long cmd, c > > case SIOCSIFMETRIC: > if ((error = suser(p, 0)) != 0) > - return (error); > + break; > ifp->if_metric = ifr->ifr_metric; > break; > > case SIOCSIFMTU: > if ((error = suser(p, 0)) != 0) > - return (error); > + break; > if (ifp->if_ioctl == NULL) > return (EOPNOTSUPP); > error = (*ifp->if_ioctl)(ifp, cmd, data); > @@ -2049,7 +2049,7 @@ ifioctl(struct socket *so, u_long cmd, c > > case SIOCSIFDESCR: > if ((error = suser(p, 0)) != 0) > - return (error); > + break; > error = copyinstr(ifr->ifr_data, ifdescrbuf, > IFDESCRSIZE, &bytesdone); > if (error == 0) { > @@ -2070,7 +2070,7 @@ ifioctl(struct socket *so, u_long cmd, c > > case SIOCSIFRTLABEL: > if ((error = suser(p, 0)) != 0) > - return (error); > + break; > error = copyinstr(ifr->ifr_data, ifrtlabelbuf, >
Re: rw locks vs memory barriers and adaptive spinning
On 10/10/17(Tue) 20:19, Mateusz Guzik wrote: > On Tue, Oct 10, 2017 at 10:15:48AM +0200, Martin Pieuchot wrote: > > Hello Mateusz, > > > > On 09/10/17(Mon) 21:43, Mateusz Guzik wrote: > > > I was looking at rw lock code out of curiosity and noticed you always do > > > membar_enter which on MP-enabled amd64 kernel translates to mfence. > > > This makes the entire business a little bit slower. > > > > > > Interestingly you already have relevant macros for amd64: > > > #define membar_enter_after_atomic() __membar("") > > > #define membar_exit_before_atomic() __membar("") > > > > If you look at the history, you'll see that theses macro didn't exist > > when membar_* where added to rw locks. > > > > I know. Addition of such a macro sounds like an accelent opportunity to > examine existing users. I figured rw locks were ommitted by accident > as the original posting explicitly mentions mutexes. > > https://marc.info/?l=openbsd-tech&m=149555411827749&w=2 > > > > And there is even a default variant for archs which don't provide one. > > > I guess the switch should be easy. > > > > Then please do it :) At lot of stuff is easy on OpenBSD but we are not > > enough. > > > > Do it, test it, explain it, get oks and commit it 8) > > > > As noted earlier the patch is trivial. I have no easy means to even > compile-test it though as I'm not using OpenBSD. Only had a look out of > curiosity. Why don't you start using it? Testing is what makes the difference. Many of us have ideas of how to improve the kernel but we're lacking the time to test & debug them all. Sometimes a "correct" change exposes a bug and we try not to break -current, so we cannot afford to commit a non-tested diff. > Nonetheless, here is the diff which can be split in 2. One part deals > with barriers, another one cleans up rw_status. > > [...] > > Read from the lock only once in rw_status. The lock word is marked as > volatile which forces re-read on each access. There is no correctness > issue here, but the assembly is worse than it needs to be and in case > of contention this adds avoidable cache traffic. Here's the diff to cleans rw_status. The difference on amd64 with clang 4.0 is the following: 48 8b 0fmov(%rdi),%rcx f6 c1 04test $0x4,%cl 75 0c jne738 31 c0 xor%eax,%eax - 48 83 3f 00 cmpq $0x0,(%rdi) + 48 85 c9test %rcx,%rcx ok? Index: kern/kern_rwlock.c === RCS file: /cvs/src/sys/kern/kern_rwlock.c,v retrieving revision 1.30 diff -u -p -r1.30 kern_rwlock.c --- kern/kern_rwlock.c 12 Aug 2017 23:27:44 - 1.30 +++ kern/kern_rwlock.c 11 Oct 2017 12:59:19 - @@ -312,13 +312,15 @@ _rw_exit(struct rwlock *rwl LOCK_FL_VARS int rw_status(struct rwlock *rwl) { - if (rwl->rwl_owner & RWLOCK_WRLOCK) { - if (RW_PROC(curproc) == RW_PROC(rwl->rwl_owner)) + unsigned long owner = rwl->rwl_owner; + + if (owner & RWLOCK_WRLOCK) { + if (RW_PROC(curproc) == RW_PROC(owner)) return RW_WRITE; else return RW_WRITE_OTHER; } - if (rwl->rwl_owner) + if (owner) return RW_READ; return (0); }
Send sysctl_mq() where it belongs
sysctl_mq() messes with 'struct mbuf_queue' internals, so keep it in kern/uipc_mbuf.c with the rest of the mq* functions. ok? Index: net/if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.514 diff -u -p -r1.514 if.c --- net/if.c11 Oct 2017 07:57:27 - 1.514 +++ net/if.c11 Oct 2017 12:56:18 - @@ -82,7 +82,6 @@ #include #include #include -#include #include #include #include @@ -2654,38 +2653,6 @@ ifpromisc(struct ifnet *ifp, int pswitch } ifr.ifr_flags = ifp->if_flags; return ((*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifr)); -} - -/* XXX move to kern/uipc_mbuf.c */ -int -sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp, -void *newp, size_t newlen, struct mbuf_queue *mq) -{ - unsigned int maxlen; - int error; - - /* All sysctl names at this level are terminal. */ - if (namelen != 1) - return (ENOTDIR); - - switch (name[0]) { - case IFQCTL_LEN: - return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq))); - case IFQCTL_MAXLEN: - maxlen = mq->mq_maxlen; - error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen); - if (!error && maxlen != mq->mq_maxlen) { - mtx_enter(&mq->mq_mtx); - mq->mq_maxlen = maxlen; - mtx_leave(&mq->mq_mtx); - } - return (error); - case IFQCTL_DROPS: - return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq))); - default: - return (EOPNOTSUPP); - } - /* NOTREACHED */ } void Index: net/if_var.h === RCS file: /cvs/src/sys/net/if_var.h,v retrieving revision 1.81 diff -u -p -r1.81 if_var.h --- net/if_var.h8 May 2017 08:46:39 - 1.81 +++ net/if_var.h11 Oct 2017 12:56:18 - @@ -324,9 +324,6 @@ int if_clone_destroy(const char *); struct if_clone * if_clone_lookup(const char *, int *); -int sysctl_mq(int *, u_int, void *, size_t *, void *, size_t, - struct mbuf_queue *); - void ifa_add(struct ifnet *, struct ifaddr *); void ifa_del(struct ifnet *, struct ifaddr *); void ifa_update_broadaddr(struct ifnet *, struct ifaddr *, Index: kern/uipc_mbuf.c === RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v retrieving revision 1.249 diff -u -p -r1.249 uipc_mbuf.c --- kern/uipc_mbuf.c15 Sep 2017 18:13:05 - 1.249 +++ kern/uipc_mbuf.c11 Oct 2017 12:56:19 - @@ -84,6 +84,7 @@ #include #include #include +#include #include #include @@ -1648,4 +1649,35 @@ mq_purge(struct mbuf_queue *mq) mq_delist(mq, &ml); return (ml_purge(&ml)); +} + +int +sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp, +void *newp, size_t newlen, struct mbuf_queue *mq) +{ + unsigned int maxlen; + int error; + + /* All sysctl names at this level are terminal. */ + if (namelen != 1) + return (ENOTDIR); + + switch (name[0]) { + case IFQCTL_LEN: + return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq))); + case IFQCTL_MAXLEN: + maxlen = mq->mq_maxlen; + error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen); + if (!error && maxlen != mq->mq_maxlen) { + mtx_enter(&mq->mq_mtx); + mq->mq_maxlen = maxlen; + mtx_leave(&mq->mq_mtx); + } + return (error); + case IFQCTL_DROPS: + return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq))); + default: + return (EOPNOTSUPP); + } + /* NOTREACHED */ } Index: sys/sysctl.h === RCS file: /cvs/src/sys/sys/sysctl.h,v retrieving revision 1.174 diff -u -p -r1.174 sysctl.h --- sys/sysctl.h14 Jun 2017 03:00:40 - 1.174 +++ sys/sysctl.h11 Oct 2017 12:56:21 - @@ -936,6 +936,9 @@ int sysctl_rdstruct(void *, size_t *, vo int sysctl_struct(void *, size_t *, void *, size_t, void *, size_t); int sysctl_file(int *, u_int, char *, size_t *, struct proc *); int sysctl_doproc(int *, u_int, char *, size_t *); +struct mbuf_queue; +int sysctl_mq(int *, u_int, void *, size_t *, void *, size_t, +struct mbuf_queue *); struct rtentry; struct walkarg; int sysctl_dumpentry(struct rtentry *, void *, unsigned int);
Re: LLVM 5.0 on armv7
> Date: Wed, 11 Oct 2017 14:21:17 +0200 > From: Patrick Wildt > > On Wed, Oct 11, 2017 at 12:19:47PM +0200, Mark Kettenis wrote: > > Diff below makes clang and lld build again on armv7. Sorted the files > > such that they match the order in the CMakeLists.txt file. > > The lists are explicitly sorted alphabetically and not like in the > CMakeLists. I would actually like to keep it that way. > > If there are unsorted lists, then that might be because I didn't have > to touch them since the last update. :) Makes it hard to compare the lists though. We can fight over that another time though. > > Index: gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile > > === > > RCS file: /cvs/src/gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile,v > > retrieving revision 1.3 > > diff -u -p -r1.3 Makefile > > --- gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile9 Jul 2017 15:28:34 > > - 1.3 > > +++ gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile11 Oct 2017 10:17:13 > > - > > @@ -11,7 +11,6 @@ SRCS= A15SDOptimizer.cpp \ > > ARMAsmPrinter.cpp \ > > ARMBaseInstrInfo.cpp \ > > ARMBaseRegisterInfo.cpp \ > > - ARMComputeBlockSize.cpp \ > > ARMConstantIslandPass.cpp \ > > ARMConstantPoolValue.cpp \ > > ARMExpandPseudoInsts.cpp \ > > @@ -24,6 +23,7 @@ SRCS= A15SDOptimizer.cpp \ > > ARMLoadStoreOptimizer.cpp \ > > ARMMCInstLower.cpp \ > > ARMMachineFunctionInfo.cpp \ > > + ARMMacroFusion.cpp \ > > ARMRegisterInfo.cpp \ > > ARMOptimizeBarriersPass.cpp \ > > ARMSelectionDAGInfo.cpp \ > > @@ -37,7 +37,8 @@ SRCS= A15SDOptimizer.cpp \ > > ThumbRegisterInfo.cpp \ > > Thumb2ITBlockPass.cpp \ > > Thumb2InstrInfo.cpp \ > > - Thumb2SizeReduction.cpp > > + Thumb2SizeReduction.cpp \ > > + ARMComputeBlockSize.cpp > > > > .PATH: ${.CURDIR}/../../../llvm/lib/Target/ARM > > > > >
additions
To make our CTF tools (and any ELF-related tool) easier to port to other OSes, I'd like to follow the Solaris/FreeBSD/OSX lead and use a header instead of our current mix of & . However devel/libelf will use instead of its own, if it is available. But with the current content of our two ports fail to build: devel/libdwarf and devel/valgrind. The diff below adds the necessary defines for these ports to build. Then I'd like to add and bump devel/libelf. Once I've dealt with runtime fallouts, if any, I'll convert our base tools to use . ok? Index: sys/exec_elf.h === RCS file: /cvs/src/sys/sys/exec_elf.h,v retrieving revision 1.75 diff -u -p -r1.75 exec_elf.h --- sys/exec_elf.h 5 Sep 2017 06:35:19 - 1.75 +++ sys/exec_elf.h 11 Oct 2017 12:34:35 - @@ -187,12 +187,14 @@ typedef struct { #define EM_PARISC 15 /* HPPA */ #define EM_SPARC32PLUS 18 /* Enhanced instruction set SPARC */ #define EM_PPC 20 /* PowerPC */ +#define EM_PPC64 21 /* PowerPC 64 */ #define EM_ARM 40 /* Advanced RISC Machines ARM */ #define EM_ALPHA 41 /* DEC ALPHA */ -#defineEM_SH 42 /* Hitachi/Renesas Super-H */ +#define EM_SH 42 /* Hitachi/Renesas Super-H */ #define EM_SPARCV9 43 /* SPARC version 9 */ #define EM_IA_64 50 /* Intel IA-64 Processor */ #define EM_AMD64 62 /* AMD64 architecture */ +#define EM_X86_64 EM_AMD64 #define EM_VAX 75 /* DEC VAX */ #define EM_AARCH64 183 /* ARM 64-bit architecture (AArch64) */ @@ -288,10 +290,18 @@ typedef struct { /* Section Attribute Flags - sh_flags */ -#define SHF_WRITE 0x1 /* Writable */ -#define SHF_ALLOC 0x2 /* occupies memory */ -#define SHF_EXECINSTR 0x4 /* executable */ -#define SHF_TLS0x400 /* thread local storage */ +#define SHF_WRITE 0x1 /* Writable */ +#define SHF_ALLOC 0x2 /* occupies memory */ +#define SHF_EXECINSTR 0x4 /* executable */ +#define SHF_MERGE 0x10/* may be merged */ +#define SHF_STRINGS0x20/* contains strings */ +#define SHF_INFO_LINK 0x40/* sh_info holds section index */ +#define SHF_LINK_ORDER 0x80/* ordering requirements */ +#define SHF_OS_NONCONFORMING 0x100 /* OS-specific processing required */ +#define SHF_GROUP 0x200 /* member of section group */ +#define SHF_TLS0x400 /* thread local storage */ +#define SHF_COMPRESSED 0x800 /* contains compressed data */ +#define SHF_MASKOS 0x0ff0 /* OS-specific semantics */ #define SHF_MASKPROC 0xf000 /* reserved bits for processor */ /* specific section attributes */ @@ -557,6 +567,11 @@ typedef struct { Elf64_Half descsz; Elf64_Half type; } Elf64_Note; + +/* Values for n_type. */ +#define NT_PRSTATUS1 /* Process status. */ +#define NT_FPREGSET2 /* Floating point registers. */ +#define NT_PRPSINFO3 /* Process state info. */ /* * OpenBSD-specific core file information.
Re: [PATCH] tests for vmd config parsing
On Tue, Oct 10, 2017 at 10:57:22PM -0700, Mike Larkin wrote: > On Tue, Oct 10, 2017 at 07:54:20PM -0700, Carlos Cardenas wrote: > > This patch adds a set of tests for vmd config parsing. > > > > Comments? Ok? > > > > ok by me. I think bluhm@ also looked at this. > > bluhm, ok to commit? > > -ml Committed, thanks for the test. Passes on amd64 and i386. Added to my daily test list. bluhm
Re: [PATCH] VMD: Remove switch on reload prior to re-creating
On 10/10/17 23:31, Claudio Jeker wrote: > On Tue, Oct 10, 2017 at 10:52:42PM -0700, Mike Larkin wrote: >> On Wed, Oct 11, 2017 at 06:46:22AM +0200, Claudio Jeker wrote: >>> On Tue, Oct 10, 2017 at 03:57:25PM -0700, Carlos Cardenas wrote: Destroy switch on `vmctl reload` to allow SIOCBRDGADD to succeed when creating new bridge and attaching interfaces to it. Comments? Ok? >>> >>> I don't think it is a good idea to destroy and recreate bridge interfaces >>> on every reload. This changes the underlying network and may result in >>> broken connections and network hickups. I would suggest that vmctl is >>> instead checking which interfaces are on the bridge and calls SIOCBRDDEL >>> if needed and skips the SIOCBRDGADD if not needed. >>> >> >> you mean vmd, right? >> >> vmctl has no special privilege to do this. > > yeah, sorry, was not fully awake yet. vmd needs to be smarter on config > reload IMO. Destroying interfaces for no reason is a bad practice. > We can definitely make things smarter. For my sake, I'm going to go through a couple current scenarios to make sure I'm picking up what you're putting down. 1) vm.conf has the following switch definition switch "a" { add vether0 } On vmd startup, a bridge(4) is created (bridge0 at first) and vether0 is added to it. On reset/reload, any running vms are stopped and the switch is removed prior to re-reading the config file and creating new bridges/vms. This means, the new bridge is now bridge1 and vether0 will be added. Today, this scenario fails since vether0 was never removed from bridge0 to be added to bridge1. (what the patch addresses). 2) vm.conf has the following definition switch "a" { interface "bridge0" add vether0 } Just like 1) except this time after any reset/reload, the switch name is always bridge0. This seems to be way a switch(4) is used in vmd. In scenario 1, are you proposing we keep bridge0 (and other predecessors around) and only remove/add the bridge ports (vether0 in this case) to the current bridge{n}? If so, should we use the bridge ports defined in vm.conf or from SIOCBRDGIFS since they could be different? How should we handle scenario 2 since my patch is kinda heavy handed for it? Another option would be to make "interface NAME" mandatory in switch definitions which collapses our scenarios. What are y'all's thoughts? +--+ Carlos >>> diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c index ef42549d105..0190c049837 100644 --- usr.sbin/vmd/priv.c +++ usr.sbin/vmd/priv.c @@ -88,6 +88,7 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg) switch (imsg->hdr.type) { case IMSG_VMDOP_PRIV_IFDESCR: case IMSG_VMDOP_PRIV_IFCREATE: + case IMSG_VMDOP_PRIV_IFDESTROY: case IMSG_VMDOP_PRIV_IFRDOMAIN: case IMSG_VMDOP_PRIV_IFADD: case IMSG_VMDOP_PRIV_IFUP: @@ -125,6 +126,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg) errno != EEXIST) log_warn("SIOCIFCREATE"); break; + case IMSG_VMDOP_PRIV_IFDESTROY: + /* Destroy the bridge */ + strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name)); + if (ioctl(env->vmd_fd, SIOCIFDESTROY, &ifr) < 0 && + errno != EEXIST) + log_warn("SIOCIFDESTROY"); + break; case IMSG_VMDOP_PRIV_IFRDOMAIN: strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name)); ifr.ifr_rdomainid = vfr.vfr_id; @@ -444,6 +452,23 @@ vm_priv_brconfig(struct privsep *ps, struct vmd_switch *vsw) return (0); } +int +vm_priv_brdestroy(struct privsep *ps, struct vmd_switch *vsw) +{ + struct vmop_ifreqvfr; + + memset(&vfr, 0, sizeof(vfr)); + + if (strlcpy(vfr.vfr_name, vsw->sw_ifname, + sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name)) + return (-1); + + proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESTROY, + &vfr, sizeof(vfr)); + + return (0); +} + uint32_t vm_priv_addr(struct address *h, uint32_t vmid, int idx, int isvm) { diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c index f1abc54d9a3..834654a76de 100644 --- usr.sbin/vmd/vmd.c +++ usr.sbin/vmd/vmd.c @@ -852,7 +852,7 @@ int vmd_reload(unsigned int reset, const char *filename) { struct vmd_vm *vm, *next_vm; - struct vmd_switch *vsw; + struct vmd_switch *vsw, *next_vsw; int reload = 0; /* Switch back to the default config file */ @@ -885,6 +885,19 @@ vmd_reload(unsigned int reset, const char *filename) } } + TAILQ_FOREACH_SAFE(vsw, env->v
Re: LLVM 5.0 on armv7
On Wed, Oct 11, 2017 at 12:19:47PM +0200, Mark Kettenis wrote: > Diff below makes clang and lld build again on armv7. Sorted the files > such that they match the order in the CMakeLists.txt file. The lists are explicitly sorted alphabetically and not like in the CMakeLists. I would actually like to keep it that way. If there are unsorted lists, then that might be because I didn't have to touch them since the last update. :) > ok? > > > Index: gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile > === > RCS file: /cvs/src/gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile,v > retrieving revision 1.3 > diff -u -p -r1.3 Makefile > --- gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile 9 Jul 2017 15:28:34 > - 1.3 > +++ gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile 11 Oct 2017 10:17:13 > - > @@ -11,7 +11,6 @@ SRCS= A15SDOptimizer.cpp \ > ARMAsmPrinter.cpp \ > ARMBaseInstrInfo.cpp \ > ARMBaseRegisterInfo.cpp \ > - ARMComputeBlockSize.cpp \ > ARMConstantIslandPass.cpp \ > ARMConstantPoolValue.cpp \ > ARMExpandPseudoInsts.cpp \ > @@ -24,6 +23,7 @@ SRCS= A15SDOptimizer.cpp \ > ARMLoadStoreOptimizer.cpp \ > ARMMCInstLower.cpp \ > ARMMachineFunctionInfo.cpp \ > + ARMMacroFusion.cpp \ > ARMRegisterInfo.cpp \ > ARMOptimizeBarriersPass.cpp \ > ARMSelectionDAGInfo.cpp \ > @@ -37,7 +37,8 @@ SRCS= A15SDOptimizer.cpp \ > ThumbRegisterInfo.cpp \ > Thumb2ITBlockPass.cpp \ > Thumb2InstrInfo.cpp \ > - Thumb2SizeReduction.cpp > + Thumb2SizeReduction.cpp \ > + ARMComputeBlockSize.cpp > > .PATH: ${.CURDIR}/../../../llvm/lib/Target/ARM > >
Re: ifioctl() cleanup, step 2
On Wed, Oct 11, 2017 at 12:43:45PM +0200, Martin Pieuchot wrote: > On 11/10/17(Wed) 12:28, Alexandr Nedvedicky wrote: > > Hello, > > > > just for my curiosity: > > > > why do you leave some returns untreated? is that intentional? > > Yes it is intentional to make the diff shorter and easier to review. > O.K. understood. thanks and regards sasha
Re: ifioctl() cleanup, step 2
On 11/10/17(Wed) 12:28, Alexandr Nedvedicky wrote: > Hello, > > just for my curiosity: > > why do you leave some returns untreated? is that intentional? Yes it is intentional to make the diff shorter and easier to review. > just like here: > @@ -2048,8 +2048,11 @@ ifioctl(struct socket *so, u_long cmd, caddr_t > data, struct proc *p) > case SIOCGVNETID: > case SIOCGIFPAIR: > case SIOCGIFPARENT: > - if (ifp->if_ioctl == 0) > - return (EOPNOTSUPP); > + if (ifp->if_ioctl == 0) { > + /* ??? sashan@ */ > + error = EOPNOTSUPP; > + break; > + } > error = (*ifp->if_ioctl)(ifp, cmd, data); > break; This will be addressed in the next diff. We can simply remove the if () block because all interface MUST have an if_ioctl handler.
Re: ifioctl() cleanup, step 2
Hello, just for my curiosity: why do you leave some returns untreated? is that intentional? just like here: @@ -2048,8 +2048,11 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCGVNETID: case SIOCGIFPAIR: case SIOCGIFPARENT: - if (ifp->if_ioctl == 0) - return (EOPNOTSUPP); + if (ifp->if_ioctl == 0) { + /* ??? sashan@ */ + error = EOPNOTSUPP; + break; + } error = (*ifp->if_ioctl)(ifp, cmd, data); break; below is patch, which makes switch in ifioctl() consistent by converting all return (error) to breaks; thanks and regards sasha 8<---8<---8<--8< diff --git a/sys/net/if.c b/sys/net/if.c index 60020606df5..40f6fe1b776 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -1876,7 +1876,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) break; #endif /* INET6 */ default: - return (EAFNOSUPPORT); + error = EAFNOSUPPORT; } if (oif_flags != ifp->if_flags || oif_xflags != ifp->if_xflags) rtm_ifchg(ifp); @@ -1920,7 +1920,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCSIFFLAGS: if ((error = suser(p, 0)) != 0) - return (error); + break; ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) | (ifr->ifr_flags & ~IFF_CANTCHANGE); @@ -1945,13 +1945,13 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCSIFXFLAGS: if ((error = suser(p, 0)) != 0) - return (error); + break; #ifdef INET6 if (ISSET(ifr->ifr_flags, IFXF_AUTOCONF6)) { error = in6_ifattach(ifp); if (error != 0) - return (error); + break; } #endif /* INET6 */ @@ -1983,7 +1983,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) error = ifp->if_wol(ifp, 1); splx(s); if (error) - return (error); + break; } if (ISSET(ifp->if_xflags, IFXF_WOL) && !ISSET(ifr->ifr_flags, IFXF_WOL)) { @@ -1992,7 +1992,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) error = ifp->if_wol(ifp, 0); splx(s); if (error) - return (error); + break; } } else if (ISSET(ifr->ifr_flags, IFXF_WOL)) { ifr->ifr_flags &= ~IFXF_WOL; @@ -2007,13 +2007,13 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCSIFMETRIC: if ((error = suser(p, 0)) != 0) - return (error); + break; ifp->if_metric = ifr->ifr_metric; break; case SIOCSIFMTU: if ((error = suser(p, 0)) != 0) - return (error); + break; if (ifp->if_ioctl == NULL) return (EOPNOTSUPP); error = (*ifp->if_ioctl)(ifp, cmd, data); @@ -2037,7 +2037,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCSIFPARENT: case SIOCDIFPARENT: if ((error = suser(p, 0)) != 0) - return (error); + break; /* FALLTHROUGH */ case SIOCGIFPSRCADDR: case SIOCGIFPDSTADDR: @@ -2048,8 +2048,10 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCGVNETID: case SIOCGIFPAIR: case SIOCGIFPARENT: - if (ifp->if_ioctl == 0) - return (EOPNOTSUPP); + if (ifp->if_ioctl == 0) { + error = EOPNOTSUPP; + break; + } error = (*ifp->if_ioctl)(ifp, cmd, data); break; @@ -2061,7 +2063,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCSIFDESCR: if ((error = suser(p, 0)) != 0) - return (error); + break; error = c
LLVM 5.0 on armv7
Diff below makes clang and lld build again on armv7. Sorted the files such that they match the order in the CMakeLists.txt file. ok? Index: gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile === RCS file: /cvs/src/gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile,v retrieving revision 1.3 diff -u -p -r1.3 Makefile --- gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile9 Jul 2017 15:28:34 - 1.3 +++ gnu/usr.bin/clang/libLLVMARMCodeGen/Makefile11 Oct 2017 10:17:13 - @@ -11,7 +11,6 @@ SRCS= A15SDOptimizer.cpp \ ARMAsmPrinter.cpp \ ARMBaseInstrInfo.cpp \ ARMBaseRegisterInfo.cpp \ - ARMComputeBlockSize.cpp \ ARMConstantIslandPass.cpp \ ARMConstantPoolValue.cpp \ ARMExpandPseudoInsts.cpp \ @@ -24,6 +23,7 @@ SRCS= A15SDOptimizer.cpp \ ARMLoadStoreOptimizer.cpp \ ARMMCInstLower.cpp \ ARMMachineFunctionInfo.cpp \ + ARMMacroFusion.cpp \ ARMRegisterInfo.cpp \ ARMOptimizeBarriersPass.cpp \ ARMSelectionDAGInfo.cpp \ @@ -37,7 +37,8 @@ SRCS= A15SDOptimizer.cpp \ ThumbRegisterInfo.cpp \ Thumb2ITBlockPass.cpp \ Thumb2InstrInfo.cpp \ - Thumb2SizeReduction.cpp + Thumb2SizeReduction.cpp \ + ARMComputeBlockSize.cpp .PATH: ${.CURDIR}/../../../llvm/lib/Target/ARM
Re: inteldrm: reduce i2c busy-wait to 100us
> Date: Tue, 10 Oct 2017 15:42:26 -0500 > From: joshua stein > > On my Kaby Lake laptop, running xbacklight or xrandr causes the > audio and mouse cursor to pause briefly. This is because those > codepaths do DRM operations that have to probe all of the available > connectors, even disconnected ones. For HDMI, it does i2c > operations that have to timeout. > > Reducing the DELAY() calls to 100us avoids this, while still making > HDMI output work when it's actually connected. > > intel_gmbus_exec appears to be an OpenBSD-specific function. This will need careful testing on older hardware, especially with multi-screen desktop setups. > Index: sys/dev/pci/drm/i915/intel_i2c.c > === > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_i2c.c,v > retrieving revision 1.12 > diff -u -p -u -p -r1.12 intel_i2c.c > --- sys/dev/pci/drm/i915/intel_i2c.c 30 Sep 2017 07:36:56 - 1.12 > +++ sys/dev/pci/drm/i915/intel_i2c.c 10 Oct 2017 20:32:59 - > @@ -231,7 +231,7 @@ intel_gmbus_exec(void *cookie, i2c_op_t > st = I915_READ(GMBUS2); > if (st & (GMBUS_SATOER | GMBUS_HW_RDY)) > break; > - DELAY(1000); > + DELAY(100); > } > if (st & GMBUS_SATOER) { > bus_err = 1; > @@ -254,7 +254,7 @@ intel_gmbus_exec(void *cookie, i2c_op_t > st = I915_READ(GMBUS2); > if (st & (GMBUS_SATOER | GMBUS_HW_RDY)) > break; > - DELAY(1000); > + DELAY(100); > } > if (st & GMBUS_SATOER) { > bus_err = 1; > @@ -279,7 +279,7 @@ out: > bus_err = 1; > if ((st & GMBUS_ACTIVE) == 0) > break; > - DELAY(1000); > + DELAY(100); > } > if (st & GMBUS_ACTIVE) > return (ETIMEDOUT); > >
ifioctl() cleanup, step 2
Moar cleanup to be able to selectively take the NET_LOCK() around some ioctls. This diff change many "return (error)" into "break". It adds error checks for SIOC{A,D}IFGROUP. The only driver handling these ioctl(2)s is... carp(4)! ok? Index: net/if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.514 diff -u -p -r1.514 if.c --- net/if.c11 Oct 2017 07:57:27 - 1.514 +++ net/if.c11 Oct 2017 08:37:31 - @@ -1817,7 +1817,7 @@ ifioctl(struct socket *so, u_long cmd, c { struct ifnet *ifp; struct ifreq *ifr = (struct ifreq *)data; - struct ifgroupreq *ifgr; + struct ifgroupreq *ifgr = (struct ifgroupreq *)data; struct if_afreq *ifar = (struct if_afreq *)data; char ifdescrbuf[IFDESCRSIZE]; char ifrtlabelbuf[RTLABEL_LEN]; @@ -1858,7 +1858,7 @@ ifioctl(struct socket *so, u_long cmd, c case SIOCIFAFATTACH: case SIOCIFAFDETACH: if ((error = suser(p, 0)) != 0) - return (error); + break; switch (ifar->ifar_af) { case AF_INET: /* attach is a noop for AF_INET */ @@ -1874,7 +1874,7 @@ ifioctl(struct socket *so, u_long cmd, c break; #endif /* INET6 */ default: - return (EAFNOSUPPORT); + error = EAFNOSUPPORT; } break; @@ -1909,7 +1909,7 @@ ifioctl(struct socket *so, u_long cmd, c case SIOCSIFFLAGS: if ((error = suser(p, 0)) != 0) - return (error); + break; ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) | (ifr->ifr_flags & ~IFF_CANTCHANGE); @@ -1934,13 +1934,13 @@ ifioctl(struct socket *so, u_long cmd, c case SIOCSIFXFLAGS: if ((error = suser(p, 0)) != 0) - return (error); + break; #ifdef INET6 if (ISSET(ifr->ifr_flags, IFXF_AUTOCONF6)) { error = in6_ifattach(ifp); if (error != 0) - return (error); + break; } #endif /* INET6 */ @@ -1972,7 +1972,7 @@ ifioctl(struct socket *so, u_long cmd, c error = ifp->if_wol(ifp, 1); splx(s); if (error) - return (error); + break; } if (ISSET(ifp->if_xflags, IFXF_WOL) && !ISSET(ifr->ifr_flags, IFXF_WOL)) { @@ -1981,7 +1981,7 @@ ifioctl(struct socket *so, u_long cmd, c error = ifp->if_wol(ifp, 0); splx(s); if (error) - return (error); + break; } } else if (ISSET(ifr->ifr_flags, IFXF_WOL)) { ifr->ifr_flags &= ~IFXF_WOL; @@ -1995,13 +1995,13 @@ ifioctl(struct socket *so, u_long cmd, c case SIOCSIFMETRIC: if ((error = suser(p, 0)) != 0) - return (error); + break; ifp->if_metric = ifr->ifr_metric; break; case SIOCSIFMTU: if ((error = suser(p, 0)) != 0) - return (error); + break; if (ifp->if_ioctl == NULL) return (EOPNOTSUPP); error = (*ifp->if_ioctl)(ifp, cmd, data); @@ -2049,7 +2049,7 @@ ifioctl(struct socket *so, u_long cmd, c case SIOCSIFDESCR: if ((error = suser(p, 0)) != 0) - return (error); + break; error = copyinstr(ifr->ifr_data, ifdescrbuf, IFDESCRSIZE, &bytesdone); if (error == 0) { @@ -2070,7 +2070,7 @@ ifioctl(struct socket *so, u_long cmd, c case SIOCSIFRTLABEL: if ((error = suser(p, 0)) != 0) - return (error); + break; error = copyinstr(ifr->ifr_data, ifrtlabelbuf, RTLABEL_LEN, &bytesdone); if (error == 0) { @@ -2085,7 +2085,7 @@ ifioctl(struct socket *so, u_long cmd, c case SIOCSIFPRIORITY: if ((error = suser(p, 0)) != 0) - return (error); + break; if (ifr->ifr_metric < 0 || ifr->ifr_metric > 15) return (EINVAL); ifp->if_priority = ifr->ifr_metric; @@ -2097,32 +2097,32 @@ ifioctl(struct socket *so, u_
acpi: free() size
Hi, Add missing size to free(), tested on amd64. Comments? OK? Index: dsdt.c === RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v retrieving revision 1.234 diff -u -p -r1.234 dsdt.c --- dsdt.c 28 May 2017 15:36:45 - 1.234 +++ dsdt.c 11 Oct 2017 07:51:57 - @@ -452,7 +452,7 @@ _acpi_os_free(void *ptr, const char *fn, #endif dnprintf(99, "free: %p %s:%d\n", sptr, fn, line); - free(sptr, M_ACPI, 0); + free(sptr, M_ACPI, sptr->size + sizeof(*sptr)); } }
Re: mg: fgetln -> getline
On Sun, Sep 17, 2017 at 02:56:32AM +, Scott Cheloha wrote: > Hi, > > This will make mg(1) ever so slightly more portable: downstream > will appreciate it. And fgetln(3) was recently deprecated. > > Misc. comments: > > The comments about NUL and "the last line problem" aren't per se > applicable with getline(3) so I removed them. > > I will say that if ever a command does *not* emit a trailing > newline that we will stub out the last byte, but I decided to take > the comments' authors at their word and continued to assume the > presence of a newline. This can be rectified with: > > if (buf[len - 1] == '\n') > buf[len - 1] = '\0'; the diff reads fine to me, but code tends to get copied around, can you send a diff with those lines added please? That's the canonical way of using getline I believe and there is no good reason for mg to be different, thanks! > > The '};' in do_cscope() looked like a typo so I deleted the > semicolon. > > I also noticed that there were no error checks after the read loops > so I added an echo print on ferror(). I don't know if this is > sufficient, but we weren't doing anything before, so it's a start. > > Thoughts? > > -- > Scott Cheloha > > Index: usr.bin/mg/cscope.c > === > RCS file: /cvs/src/usr.bin/mg/cscope.c,v > retrieving revision 1.16 > diff -u -p -r1.16 cscope.c > --- usr.bin/mg/cscope.c 19 Jan 2016 14:51:00 - 1.16 > +++ usr.bin/mg/cscope.c 16 Sep 2017 22:44:50 - > @@ -166,9 +166,13 @@ cscreatelist(int f, int n) > struct stat sb; > FILE *fpipe; > char dir[NFILEN], cmd[BUFSIZ], title[BUFSIZ], *line, *bufp; > - size_t len; > + size_t sz; > + ssize_t len; > int clen; > > + line = NULL; > + sz = 0; > + > if (getbufcwd(dir, sizeof(dir)) == FALSE) > dir[0] = '\0'; > > @@ -221,11 +225,13 @@ cscreatelist(int f, int n) > } > addline(bp, title); > addline(bp, ""); > - /* All lines are NUL terminated */ > - while ((line = fgetln(fpipe, &len)) != NULL) { > + while ((len = getline(&line, &sz, fpipe)) != -1) { > line[len - 1] = '\0'; > addline(bp, line); > } > + free(line); > + if (ferror(fpipe)) > + ewprintf("Problem reading pipe"); > pclose(fpipe); > return (popbuftop(bp, WNONE)); > } > @@ -397,7 +403,11 @@ do_cscope(int i) > char pattern[MAX_TOKEN], cmd[BUFSIZ], title[BUFSIZ]; > char *p, *buf; > int clen, nores = 0; > - size_t len; > + size_t sz; > + ssize_t len; > + > + buf = NULL; > + sz = 0; > > /* If current buffer isn't a source file just return */ > if (fnmatch("*.[chy]", curbp->b_fname, 0) != 0) { > @@ -447,13 +457,17 @@ do_cscope(int i) > addline(bp, title); > addline(bp, ""); > addline(bp, > "---"); > - /* All lines are NUL terminated */ > - while ((buf = fgetln(fpipe, &len)) != NULL) { > + while ((len = getline(&buf, &sz, fpipe)) != -1) { > buf[len - 1] = '\0'; > - if (addentry(bp, buf) != TRUE) > + if (addentry(bp, buf) != TRUE) { > + free(buf); > return (FALSE); > + } > nores = 1; > - }; > + } > + free(buf); > + if (ferror(fpipe)) > + ewprintf("Problem reading pipe"); > pclose(fpipe); > addline(bp, > "---"); > if (nores == 0) > Index: usr.bin/mg/grep.c > === > RCS file: /cvs/src/usr.bin/mg/grep.c,v > retrieving revision 1.44 > diff -u -p -r1.44 grep.c > --- usr.bin/mg/grep.c 19 Mar 2015 21:48:05 - 1.44 > +++ usr.bin/mg/grep.c 16 Sep 2017 22:44:50 - > @@ -178,12 +178,16 @@ compile_mode(const char *name, const cha > struct buffer *bp; > FILE*fpipe; > char*buf; > - size_t len; > + size_t sz; > + ssize_t len; > int ret, n; > char cwd[NFILEN], qcmd[NFILEN]; > char timestr[NTIME]; > time_t t; > > + buf = NULL; > + sz = 0; > + > n = snprintf(qcmd, sizeof(qcmd), "%s 2>&1", command); > if (n < 0 || n >= sizeof(qcmd)) > return (NULL); > @@ -210,15 +214,13 @@ compile_mode(const char *name, const cha > ewprintf("Problem opening pipe"); > return (NULL); > } > - /* > - * We know that our commands are nice and the last line will end with > - * a \n, so we don't need to try to deal with the last line problem > - * in fgetln. > - */ > - while ((buf = fgetln(fpipe, &len)) != NULL) { > + while ((len = getline(&buf, &sz, fpipe)) != -1) { > buf[len -