Re: Return ESRCH instead of zero result for KERN_FILE sysctl
Alexander Bluhm wrote: > > Can we have the same logic for KERN_FILE_BYPID and KERN_FILE_BYUID? I think this should be BYPID only. I'm not sure why. Maybe because a user exists even when the user isn't running any processes. But a process doesn't exist when it doesn't exist.
Re: pledge: telnet should not verify if hostname is a fully qualified domain
Adam Wolk [adam.w...@tintagel.pl] wrote: > > I would like to just drop that part of code. Any OK's, comments? > Please do. It's utterly useless. ok chris@ > Index: commands.c > === > RCS file: /cvs/src/usr.bin/telnet/commands.c,v > retrieving revision 1.83 > diff -u -p -r1.83 commands.c > --- commands.c16 Mar 2016 15:41:11 - 1.83 > +++ commands.c3 May 2016 00:24:51 - > @@ -1445,14 +1445,6 @@ env_init(void) > > gethostname(hbuf, sizeof hbuf); > > - /* If this is not the full name, try to get it via DNS */ > - if (strchr(hbuf, '.') == 0) { > - struct hostent *he = gethostbyname(hbuf); > - if (he != 0) > - strncpy(hbuf, he->h_name, sizeof hbuf-1); > - hbuf[sizeof hbuf-1] = '\0'; > - } > - > if (asprintf (&cp, "%s%s", hbuf, cp2) == -1) > err(1, "asprintf"); >
pledge: telnet should not verify if hostname is a fully qualified domain
Hi tech@, I have been noticing coredumps from telnet on my laptop for some time now and finally found an evening to investigate it. The typical use case: $ telnet localhost 22 Trying 127.0.0.1... Connected to localhost. Escape character is '^]'. SSH-2.0-OpenSSH_7.2 ^] telnet> quit Connection closed. Abort trap (core dumped) $ Plus the following in dmesg: telnet(67078): syscall 97 "dns" The bug was reproducible by me both by calling quit or close in the telnet> prompt but no one else I asked was able to reproduce it. Rebuilding the code with debug symbols and grabbing the backtrace revealed this fine piece of code: /* If this is not the full name, try to get it via DNS */ if (strchr(hbuf, '.') == 0) { struct hostent *he = gethostbyname(hbuf); if (he != 0) strncpy(hbuf, he->h_name, sizeof hbuf-1); hbuf[sizeof hbuf-1] = '\0'; } Full backtrace: https://gist.github.com/mulander/392bce616de89830f64aaf72b9cab56d Which was added in 12-March-98 by art@ while adding encryption support from kth-krb (kerberos only) plus doing some tweaks for better binary/8-bit support (http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/telnet/commands.c#rev1.10). The reason for entering that code path is me having a not fully qualified name for my host. Setting up a proper name (napalm.local instead of napalm) makes telnet happy again. Regardless I don't see a reason why telnet should be doing this check. Here is the rationale: - It's not performed and required on initial run (either by running telnet + telnet> open host port or by running telnet host port directly) - It breaks the pledge assumption of not needing DNS after the connection is established I would like to just drop that part of code. Any OK's, comments? Index: commands.c === RCS file: /cvs/src/usr.bin/telnet/commands.c,v retrieving revision 1.83 diff -u -p -r1.83 commands.c --- commands.c 16 Mar 2016 15:41:11 - 1.83 +++ commands.c 3 May 2016 00:24:51 - @@ -1445,14 +1445,6 @@ env_init(void) gethostname(hbuf, sizeof hbuf); - /* If this is not the full name, try to get it via DNS */ - if (strchr(hbuf, '.') == 0) { - struct hostent *he = gethostbyname(hbuf); - if (he != 0) - strncpy(hbuf, he->h_name, sizeof hbuf-1); - hbuf[sizeof hbuf-1] = '\0'; - } - if (asprintf (&cp, "%s%s", hbuf, cp2) == -1) err(1, "asprintf");
Re: Return ESRCH instead of zero result for KERN_FILE sysctl
2016-05-03 1:19 GMT+03:00 Mark Kettenis : >> Date: Tue, 3 May 2016 00:04:13 +0200 >> From: Alexander Bluhm >> >> On Sun, May 01, 2016 at 01:24:19AM +0300, Vadim Zhukov wrote: >> > When matching by PID, we'd better return ESRCH expicitly rather >> > than zero entries. This makes, for example, fstat(1) behaviour >> > more consistent and makes it easier to replace "if lsof -p ..." >> > checks in third-party code with "if fstat -p ...". >> > >> > For libkvm, I explicitly test for ESRCH to avoid going inside >> > kmem without real need. >> > >> > Tested on amd64. >> > >> > Comments? Okays? >> >> Your use case makes sense. So I think this should go in. > > The code change is abit convoluted though though. It's enough to > initialize matched to 0 before the LIST_FOREACH and set it > unconditionally to after the > > if (arg > 0 && pr->ps_pid != (pid_t)arg) > > check. So: > > matched = 0 > LIST_FOREACH(pr, &allprocess, ps_list) { > /* > * skip system, exiting, embryonic and undead > * processes > */ > if (pr->ps_flags & (PS_SYSTEM | PS_EMBRYO | > PS_EXITING)) > continue; > if (arg > 0 && pr->ps_pid != (pid_t)arg) { > /* not the pid we are looking for */ > continue; > } > matched = 1; I think you're right. I've modelled the kernel code after libkvm, and your suggestion makes it much easier to read, thanks! I'll send updated diff a bit later, have to close lid right now. Thanks! >> > Index: sys/kern/kern_sysctl.c >> > === >> > RCS file: /cvs/src/sys/kern/kern_sysctl.c,v >> > retrieving revision 1.300 >> > diff -u -p -r1.300 kern_sysctl.c >> > --- sys/kern/kern_sysctl.c 29 Feb 2016 19:44:07 - 1.300 >> > +++ sys/kern/kern_sysctl.c 30 Apr 2016 22:26:06 - >> > @@ -1225,7 +1225,7 @@ sysctl_file(int *name, u_int namelen, ch >> > struct process *pr; >> > size_t buflen, elem_size, elem_count, outsize; >> > char *dp = where; >> > - int arg, i, error = 0, needed = 0; >> > + int arg, i, error = 0, needed = 0, matched; >> > u_int op; >> > int show_pointers; >> > >> > @@ -1325,6 +1325,7 @@ sysctl_file(int *name, u_int namelen, ch >> > error = EINVAL; >> > break; >> > } >> > + matched = (arg <= 0); >> > LIST_FOREACH(pr, &allprocess, ps_list) { >> > /* >> > * skip system, exiting, embryonic and undead >> > @@ -1332,9 +1333,12 @@ sysctl_file(int *name, u_int namelen, ch >> > */ >> > if (pr->ps_flags & (PS_SYSTEM | PS_EMBRYO | >> > PS_EXITING)) >> > continue; >> > - if (arg > 0 && pr->ps_pid != (pid_t)arg) { >> > - /* not the pid we are looking for */ >> > - continue; >> > + if (arg > 0) { >> > + /* is this the pid we are looking for? */ >> > + if (pr->ps_pid == (pid_t)arg) >> > + matched = 1; >> > + else >> > + continue; >> > } >> > fdp = pr->ps_fd; >> > if (pr->ps_textvp) >> > @@ -1353,6 +1357,8 @@ sysctl_file(int *name, u_int namelen, ch >> > FILLIT(fp, fdp, i, NULL, pr); >> > } >> > } >> > + if (!matched) >> > + error = ESRCH; >> > break; >> > case KERN_FILE_BYUID: >> > LIST_FOREACH(pr, &allprocess, ps_list) { >> > Index: lib/libkvm/kvm_file2.c >> > === >> > RCS file: /cvs/src/lib/libkvm/kvm_file2.c,v >> > retrieving revision 1.47 >> > diff -u -p -r1.47 kvm_file2.c >> > --- lib/libkvm/kvm_file2.c 4 Sep 2015 02:58:14 - 1.47 >> > +++ lib/libkvm/kvm_file2.c 30 Apr 2016 22:26:06 - >> > @@ -149,7 +149,7 @@ kvm_getfiles(kvm_t *kd, int op, int arg, >> > /* find size and alloc buffer */ >> > rv = sysctl(mib, 6, NULL, &size, NULL, 0); >> > if (rv == -1) { >> > - if (kd->vmfd != -1) >> > + if (errno != ESRCH && kd->vmfd != -1) >> > goto deadway; >> > _kvm_syserr(kd, kd->program, "kvm_getfiles"); >> > return (NULL); >> > @@ -266,7 +266,7 @@ kvm_deadfile_byid(kvm_t *kd, int op, int >> > { >> > size_t buflen; >> > struct nlist nl[4], *np; >> >
Re: Return ESRCH instead of zero result for KERN_FILE sysctl
2016-05-03 1:04 GMT+03:00 Alexander Bluhm : > On Sun, May 01, 2016 at 01:24:19AM +0300, Vadim Zhukov wrote: >> When matching by PID, we'd better return ESRCH expicitly rather >> than zero entries. This makes, for example, fstat(1) behaviour >> more consistent and makes it easier to replace "if lsof -p ..." >> checks in third-party code with "if fstat -p ...". >> >> For libkvm, I explicitly test for ESRCH to avoid going inside >> kmem without real need. >> >> Tested on amd64. >> >> Comments? Okays? > > Your use case makes sense. So I think this should go in. > > Can we have the same logic for KERN_FILE_BYPID and KERN_FILE_BYUID? I don't have strong opinion here. On one side, while PID matching is a single match by definition, UID matching is not. On the other side, if you're looking for particular UID, you do know something about already... If you want it so, then let it be so. I'll send adjusted patch a bit later, having to close laptop ASAP. > man kvm_getfiles says (-1 for all processes), but code does arg > 0. > I think it should be arg => 0 like in KERN_FILE_BYUID. > > The (arg < -1) EINVAL check is missing in KERN_FILE_BYUID. Does > anyone know why? I was stumbled by those differences as well, but preferred to keep the current logic; it could be fixed separately anyway. > errno ESRCH should be documented in kvm_getfiles(3). At the moment, there is not errors section in kvm_getfiles(3) at all. I'm not sure what else should go there: EINVAL? > exit code 1 should be documented in fstat(1). Should it behave > like grep? >0 One or more lines were selected. >1 No lines were selected. >>1 An error occurred. I think that's good idea. Thank you for review!
xclock patch
I saw this in /var/log/messages May 2 17:19:07 thinkpad /bsd: xclock(81091): syscall 5 "rpath" Does it mean xclock needs rpath? If so here you go. Index: xclock.c === RCS file: /cvs/xenocara/app/xclock/xclock.c,v retrieving revision 1.6 diff -u -p -u -r1.6 xclock.c --- xclock.c11 Nov 2015 21:12:19 -1.6 +++ xclock.c2 May 2016 22:37:28 - @@ -228,7 +228,7 @@ main(int argc, char *argv[]) #endif #ifdef HAVE_PLEDGE -if (pledge("stdio", NULL) == -1) +if (pledge("stdio rpath", NULL) == -1) err(1, "pledge"); #endif
Re: Moving away from softnet interrupts
On Mon, May 02, 2016 at 11:27:36AM +0200, Martin Pieuchot wrote: > On 18/04/16(Mon) 10:50, Martin Pieuchot wrote: > > The current goal of the Network SMP effort is to have a single CPU > > process the IP forwarding path in a process context without holding > > the KERNEL_LOCK(). To achieve this goal we're progressively moving > > code from the softnet interrupt context to the if_input_task. In > > the end we'll completely get rid of this soft-interrupt. > > > > So now would be a good time to know if moving all the code currently > > run in a soft-interrupt context to a task uncovers any bug. I'm > > happily running the diff below on amd64 and macppc, it even gives me > > a small performance boost. > > > > I'd appreciate more tests especially on exotic archs. > > So far this has been tested on amd64, i386, powerpc, mips64 and sparc. > > So I'm looking for oks. OK bluhm@ > > > Index: net/if.c > > === > > RCS file: /cvs/src/sys/net/if.c,v > > retrieving revision 1.429 > > diff -u -p -r1.429 if.c > > --- net/if.c16 Mar 2016 12:08:09 - 1.429 > > +++ net/if.c18 Apr 2016 08:00:08 - > > @@ -64,9 +64,12 @@ > > #include "bpfilter.h" > > #include "bridge.h" > > #include "carp.h" > > +#include "ether.h" > > #include "pf.h" > > +#include "pfsync.h" > > +#include "ppp.h" > > +#include "pppoe.h" > > #include "trunk.h" > > -#include "ether.h" > > > > #include > > #include > > @@ -148,6 +151,7 @@ int if_group_egress_build(void); > > void if_watchdog_task(void *); > > > > void if_input_process(void *); > > +void if_netisr(void *); > > > > #ifdef DDB > > void ifa_print_all(void); > > @@ -216,13 +220,15 @@ void net_tick(void *); > > intnet_livelocked(void); > > intifq_congestion; > > > > -struct taskq *softnettq; > > +int netisr; > > +struct taskq *softnettq; > > + > > +struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET); > > +struct task if_input_task = TASK_INITIALIZER(if_input_process, > > &if_input_queue); > > +struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL); > > > > /* > > * Network interface utility routines. > > - * > > - * Routines with ifa_ifwith* names take sockaddr *'s as > > - * parameters. > > */ > > void > > ifinit(void) > > @@ -590,9 +596,6 @@ if_enqueue(struct ifnet *ifp, struct mbu > > return (0); > > } > > > > -struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET); > > -struct task if_input_task = TASK_INITIALIZER(if_input_process, > > &if_input_queue); > > - > > void > > if_input(struct ifnet *ifp, struct mbuf_list *ml) > > { > > @@ -803,6 +806,50 @@ if_input_process(void *xmq) > > if_put(ifp); > > } > > splx(s); > > +} > > + > > +void > > +if_netisr(void *unused) > > +{ > > + int n, t = 0; > > + int s; > > + > > + KERNEL_LOCK(); > > + s = splsoftnet(); > > + > > + while ((n = netisr) != 0) { > > + sched_pause(); > > + > > + atomic_clearbits_int(&netisr, n); > > + > > + if (n & (1 << NETISR_IP)) > > + ipintr(); > > +#ifdef INET6 > > + if (n & (1 << NETISR_IPV6)) > > + ip6intr(); > > +#endif > > +#if NPPP > 0 > > + if (n & (1 << NETISR_PPP)) > > + pppintr(); > > +#endif > > +#if NBRIDGE > 0 > > + if (n & (1 << NETISR_BRIDGE)) > > + bridgeintr(); > > +#endif > > +#if NPPPOE > 0 > > + if (n & (1 << NETISR_PPPOE)) > > + pppoeintr(); > > +#endif > > + t |= n; > > + } > > + > > +#if NPFSYNC > 0 > > + if (t & (1 << NETISR_PFSYNC)) > > + pfsyncintr(); > > +#endif > > + > > + splx(s); > > + KERNEL_UNLOCK(); > > } > > > > void > > Index: net/netisr.c > > === > > RCS file: net/netisr.c > > diff -N net/netisr.c > > --- net/netisr.c8 Jan 2016 13:53:24 - 1.10 > > +++ /dev/null 1 Jan 1970 00:00:00 - > > @@ -1,74 +0,0 @@ > > -/* > > - * Copyright (c) 2010 Owain G. Ainsworth > > - * > > - * Permission to use, copy, modify, and distribute this software for any > > - * purpose with or without fee is hereby granted, provided that the above > > - * copyright notice and this permission notice appear in all copies. > > - * > > - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > > - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > > - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > > - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > > - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > > - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > > - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
tcpdump yacc reference
Hi. Not sure if yacc is needed here. Index: src/usr.sbin/tcpdump/tcpdump.c === RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.c,v retrieving revision 1.78 diff -u -p -u -r1.78 tcpdump.c --- src/usr.sbin/tcpdump/tcpdump.c 22 Dec 2015 21:01:07 - 1.78 +++ src/usr.sbin/tcpdump/tcpdump.c 2 May 2016 22:12:18 - @@ -224,7 +224,7 @@ main(int argc, char **argv) opterr = 0; while ((op = getopt(argc, argv, - "Aac:D:deE:fF:i:IlLnNOopqr:s:StT:vw:xXy:Y")) != -1) + "Aac:D:deE:fF:i:IlLnNOopqr:s:StT:vw:xXy:")) != -1) switch (op) { case 'A': @@ -357,15 +357,6 @@ main(int argc, char **argv) case 'w': WFileName = optarg; break; -#ifdef YYDEBUG - case 'Y': - { - /* Undocumented flag */ - extern int yydebug; - yydebug = 1; - } - break; -#endif case 'y': i = pcap_datalink_name_to_val(optarg); if (i < 0)
Re: Return ESRCH instead of zero result for KERN_FILE sysctl
> Date: Tue, 3 May 2016 00:04:13 +0200 > From: Alexander Bluhm > > On Sun, May 01, 2016 at 01:24:19AM +0300, Vadim Zhukov wrote: > > When matching by PID, we'd better return ESRCH expicitly rather > > than zero entries. This makes, for example, fstat(1) behaviour > > more consistent and makes it easier to replace "if lsof -p ..." > > checks in third-party code with "if fstat -p ...". > > > > For libkvm, I explicitly test for ESRCH to avoid going inside > > kmem without real need. > > > > Tested on amd64. > > > > Comments? Okays? > > Your use case makes sense. So I think this should go in. The code change is abit convoluted though though. It's enough to initialize matched to 0 before the LIST_FOREACH and set it unconditionally to after the if (arg > 0 && pr->ps_pid != (pid_t)arg) check. So: matched = 0 LIST_FOREACH(pr, &allprocess, ps_list) { /* * skip system, exiting, embryonic and undead * processes */ if (pr->ps_flags & (PS_SYSTEM | PS_EMBRYO | PS_EXITING)) continue; if (arg > 0 && pr->ps_pid != (pid_t)arg) { /* not the pid we are looking for */ continue; } matched = 1; > > Index: sys/kern/kern_sysctl.c > > === > > RCS file: /cvs/src/sys/kern/kern_sysctl.c,v > > retrieving revision 1.300 > > diff -u -p -r1.300 kern_sysctl.c > > --- sys/kern/kern_sysctl.c 29 Feb 2016 19:44:07 - 1.300 > > +++ sys/kern/kern_sysctl.c 30 Apr 2016 22:26:06 - > > @@ -1225,7 +1225,7 @@ sysctl_file(int *name, u_int namelen, ch > > struct process *pr; > > size_t buflen, elem_size, elem_count, outsize; > > char *dp = where; > > - int arg, i, error = 0, needed = 0; > > + int arg, i, error = 0, needed = 0, matched; > > u_int op; > > int show_pointers; > > > > @@ -1325,6 +1325,7 @@ sysctl_file(int *name, u_int namelen, ch > > error = EINVAL; > > break; > > } > > + matched = (arg <= 0); > > LIST_FOREACH(pr, &allprocess, ps_list) { > > /* > > * skip system, exiting, embryonic and undead > > @@ -1332,9 +1333,12 @@ sysctl_file(int *name, u_int namelen, ch > > */ > > if (pr->ps_flags & (PS_SYSTEM | PS_EMBRYO | PS_EXITING)) > > continue; > > - if (arg > 0 && pr->ps_pid != (pid_t)arg) { > > - /* not the pid we are looking for */ > > - continue; > > + if (arg > 0) { > > + /* is this the pid we are looking for? */ > > + if (pr->ps_pid == (pid_t)arg) > > + matched = 1; > > + else > > + continue; > > } > > fdp = pr->ps_fd; > > if (pr->ps_textvp) > > @@ -1353,6 +1357,8 @@ sysctl_file(int *name, u_int namelen, ch > > FILLIT(fp, fdp, i, NULL, pr); > > } > > } > > + if (!matched) > > + error = ESRCH; > > break; > > case KERN_FILE_BYUID: > > LIST_FOREACH(pr, &allprocess, ps_list) { > > Index: lib/libkvm/kvm_file2.c > > === > > RCS file: /cvs/src/lib/libkvm/kvm_file2.c,v > > retrieving revision 1.47 > > diff -u -p -r1.47 kvm_file2.c > > --- lib/libkvm/kvm_file2.c 4 Sep 2015 02:58:14 - 1.47 > > +++ lib/libkvm/kvm_file2.c 30 Apr 2016 22:26:06 - > > @@ -149,7 +149,7 @@ kvm_getfiles(kvm_t *kd, int op, int arg, > > /* find size and alloc buffer */ > > rv = sysctl(mib, 6, NULL, &size, NULL, 0); > > if (rv == -1) { > > - if (kd->vmfd != -1) > > + if (errno != ESRCH && kd->vmfd != -1) > > goto deadway; > > _kvm_syserr(kd, kd->program, "kvm_getfiles"); > > return (NULL); > > @@ -266,7 +266,7 @@ kvm_deadfile_byid(kvm_t *kd, int op, int > > { > > size_t buflen; > > struct nlist nl[4], *np; > > - int n = 0; > > + int n = 0, matched = 0; > > char *where; > > struct kinfo_file kf; > > struct file *fp, file; > > @@ -312,6 +312,9 @@ kvm_deadfile_byid(kvm_t *kd, int op, int > > kd->filebase = (void *)where; > > buflen = (nfiles + 10) * esize; > > > > + if (op != KERN_FILE_BYPID || arg <= 0) > > + matched = 1; > > + > > f
Re: Single route lookup when forwarding an IPv4 packet
On Mon, Apr 25, 2016 at 02:40:41PM +0200, Martin Pieuchot wrote: > Diff below makes ip_forward() use the route entry fetched in in_ouraddr(). > > As you can imagine a proper caching could be done for forwarding using > PF statekey. > > This diff has been tested by Hrvoje Popovski who confirmed that the > benchmark performances are similar to the ones using a single cache > entry. > > ok? OK bluhm@ > > > Index: netinet/ip_input.c > === > RCS file: /cvs/src/sys/netinet/ip_input.c,v > retrieving revision 1.274 > diff -u -p -r1.274 ip_input.c > --- netinet/ip_input.c25 Apr 2016 12:33:48 - 1.274 > +++ netinet/ip_input.c25 Apr 2016 12:34:46 - > @@ -126,8 +126,8 @@ static struct mbuf_queue ipsend_mq; > > void ip_ours(struct mbuf *); > int ip_dooptions(struct mbuf *, struct ifnet *); > -int in_ouraddr(struct mbuf *, struct ifnet *, struct in_addr); > -void ip_forward(struct mbuf *, struct ifnet *, int); > +int in_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **); > +void ip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int); > #ifdef IPSEC > int ip_input_ipsec_fwd_check(struct mbuf *, int); > int ip_input_ipsec_ours_check(struct mbuf *, int); > @@ -223,8 +223,9 @@ ipintr(void) > void > ipv4_input(struct mbuf *m) > { > - struct ifnet *ifp; > - struct ip *ip; > + struct ifnet*ifp; > + struct rtentry *rt = NULL; > + struct ip *ip; > int hlen, len; > #if defined(MROUTING) || defined(IPSEC) > int rv; > @@ -341,7 +342,7 @@ ipv4_input(struct mbuf *m) > goto out; > } > > - if (in_ouraddr(m, ifp, ip->ip_dst)) { > + if (in_ouraddr(m, ifp, &rt)) { > ip_ours(m); > goto out; > } > @@ -443,12 +444,13 @@ ipv4_input(struct mbuf *m) > } > #endif /* IPSEC */ > > - ip_forward(m, ifp, pfrdr); > + ip_forward(m, ifp, rt, pfrdr); > if_put(ifp); > return; > bad: > m_freem(m); > out: > + rtfree(rt); > if_put(ifp); > } > > @@ -575,9 +577,10 @@ bad: > } > > int > -in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct in_addr ina) > +in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct rtentry **prt) > { > struct rtentry *rt; > + struct ip *ip; > struct sockaddr_in sin; > int match = 0; > #if NPF > 0 > @@ -597,10 +600,12 @@ in_ouraddr(struct mbuf *m, struct ifnet > } > #endif > > + ip = mtod(m, struct ip *); > + > memset(&sin, 0, sizeof(sin)); > sin.sin_len = sizeof(sin); > sin.sin_family = AF_INET; > - sin.sin_addr = ina; > + sin.sin_addr = ip->ip_dst; > rt = rtalloc(sintosa(&sin), 0, m->m_pkthdr.ph_rtableid); > if (rtisvalid(rt)) { > if (ISSET(rt->rt_flags, RTF_LOCAL)) > @@ -618,7 +623,7 @@ in_ouraddr(struct mbuf *m, struct ifnet > m->m_flags |= M_BCAST; > } > } > - rtfree(rt); > + *prt = rt; > > if (!match) { > struct ifaddr *ifa; > @@ -630,7 +635,7 @@ in_ouraddr(struct mbuf *m, struct ifnet >* address on the interface it was received on. >*/ > if (!ISSET(m->m_flags, M_BCAST) || > - !IN_CLASSFULBROADCAST(ina.s_addr, ina.s_addr)) > + !IN_CLASSFULBROADCAST(ip->ip_dst.s_addr, ip->ip_dst.s_addr)) > return (0); > > if (ifp->if_rdomain != rtable_l2(m->m_pkthdr.ph_rtableid)) > @@ -645,7 +650,7 @@ in_ouraddr(struct mbuf *m, struct ifnet > if (ifa->ifa_addr->sa_family != AF_INET) > continue; > > - if (IN_CLASSFULBROADCAST(ina.s_addr, > + if (IN_CLASSFULBROADCAST(ip->ip_dst.s_addr, > ifatoia(ifa)->ia_addr.sin_addr.s_addr)) { > match = 1; > break; > @@ -1241,7 +1246,7 @@ ip_dooptions(struct mbuf *m, struct ifne > } > KERNEL_UNLOCK(); > if (forward && ipforwarding) { > - ip_forward(m, ifp, 1); > + ip_forward(m, ifp, NULL, 1); > return (1); > } > return (0); > @@ -1387,12 +1392,11 @@ int inetctlerrmap[PRC_NCMDS] = { > * via a source route. > */ > void > -ip_forward(struct mbuf *m, struct ifnet *ifp, int srcrt) > +ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt) > { > struct mbuf mfake, *mcopy = NULL; > struct ip *ip = mtod(m, struct ip *); > struct sockaddr_in *sin; > - struct rtentry *rt; > struct route ro; > int error, type = 0, code = 0, destmtu = 0, fake = 0, len; > u_int32_t dest; > @@ -1401,25 +1405,27 @@ ip_forward(struct mbuf *m, struct ifnet > if (m->m_flags & (M_BCAST|M_MCAST) || in_canforward(ip->ip_dst
Re: Return ESRCH instead of zero result for KERN_FILE sysctl
On Sun, May 01, 2016 at 01:24:19AM +0300, Vadim Zhukov wrote: > When matching by PID, we'd better return ESRCH expicitly rather > than zero entries. This makes, for example, fstat(1) behaviour > more consistent and makes it easier to replace "if lsof -p ..." > checks in third-party code with "if fstat -p ...". > > For libkvm, I explicitly test for ESRCH to avoid going inside > kmem without real need. > > Tested on amd64. > > Comments? Okays? Your use case makes sense. So I think this should go in. Can we have the same logic for KERN_FILE_BYPID and KERN_FILE_BYUID? man kvm_getfiles says (-1 for all processes), but code does arg > 0. I think it should be arg => 0 like in KERN_FILE_BYUID. The (arg < -1) EINVAL check is missing in KERN_FILE_BYUID. Does anyone know why? errno ESRCH should be documented in kvm_getfiles(3). exit code 1 should be documented in fstat(1). Should it behave like grep? 0 One or more lines were selected. 1 No lines were selected. >1 An error occurred. bluhm > > -- > WBR, > Vadim Zhukov > > > Index: sys/kern/kern_sysctl.c > === > RCS file: /cvs/src/sys/kern/kern_sysctl.c,v > retrieving revision 1.300 > diff -u -p -r1.300 kern_sysctl.c > --- sys/kern/kern_sysctl.c29 Feb 2016 19:44:07 - 1.300 > +++ sys/kern/kern_sysctl.c30 Apr 2016 22:26:06 - > @@ -1225,7 +1225,7 @@ sysctl_file(int *name, u_int namelen, ch > struct process *pr; > size_t buflen, elem_size, elem_count, outsize; > char *dp = where; > - int arg, i, error = 0, needed = 0; > + int arg, i, error = 0, needed = 0, matched; > u_int op; > int show_pointers; > > @@ -1325,6 +1325,7 @@ sysctl_file(int *name, u_int namelen, ch > error = EINVAL; > break; > } > + matched = (arg <= 0); > LIST_FOREACH(pr, &allprocess, ps_list) { > /* >* skip system, exiting, embryonic and undead > @@ -1332,9 +1333,12 @@ sysctl_file(int *name, u_int namelen, ch >*/ > if (pr->ps_flags & (PS_SYSTEM | PS_EMBRYO | PS_EXITING)) > continue; > - if (arg > 0 && pr->ps_pid != (pid_t)arg) { > - /* not the pid we are looking for */ > - continue; > + if (arg > 0) { > + /* is this the pid we are looking for? */ > + if (pr->ps_pid == (pid_t)arg) > + matched = 1; > + else > + continue; > } > fdp = pr->ps_fd; > if (pr->ps_textvp) > @@ -1353,6 +1357,8 @@ sysctl_file(int *name, u_int namelen, ch > FILLIT(fp, fdp, i, NULL, pr); > } > } > + if (!matched) > + error = ESRCH; > break; > case KERN_FILE_BYUID: > LIST_FOREACH(pr, &allprocess, ps_list) { > Index: lib/libkvm/kvm_file2.c > === > RCS file: /cvs/src/lib/libkvm/kvm_file2.c,v > retrieving revision 1.47 > diff -u -p -r1.47 kvm_file2.c > --- lib/libkvm/kvm_file2.c4 Sep 2015 02:58:14 - 1.47 > +++ lib/libkvm/kvm_file2.c30 Apr 2016 22:26:06 - > @@ -149,7 +149,7 @@ kvm_getfiles(kvm_t *kd, int op, int arg, > /* find size and alloc buffer */ > rv = sysctl(mib, 6, NULL, &size, NULL, 0); > if (rv == -1) { > - if (kd->vmfd != -1) > + if (errno != ESRCH && kd->vmfd != -1) > goto deadway; > _kvm_syserr(kd, kd->program, "kvm_getfiles"); > return (NULL); > @@ -266,7 +266,7 @@ kvm_deadfile_byid(kvm_t *kd, int op, int > { > size_t buflen; > struct nlist nl[4], *np; > - int n = 0; > + int n = 0, matched = 0; > char *where; > struct kinfo_file kf; > struct file *fp, file; > @@ -312,6 +312,9 @@ kvm_deadfile_byid(kvm_t *kd, int op, int > kd->filebase = (void *)where; > buflen = (nfiles + 10) * esize; > > + if (op != KERN_FILE_BYPID || arg <= 0) > + matched = 1; > + > for (pr = LIST_FIRST(&allprocess); > pr != NULL; > pr = LIST_NEXT(&process, ps_list)) { > @@ -333,10 +336,12 @@ kvm_deadfile_byid(kvm_t *kd, int op, int > goto cleanup; > } > > - if (op == KERN_FILE_BYPID && arg > 0 && > - proc.p_pid != (pid_t)arg) { > - /* not t
Re: Speedup sdhc(4)
On 5/1/16 3:09 PM, Mark Kettenis wrote: Date: Sat, 30 Apr 2016 13:31:21 +0200 (CEST) From: Mark Kettenis From: John Troy Date: Fri, 29 Apr 2016 11:56:24 -0400 On 4/28/16 2:30 PM, Mark Kettenis wrote: So here are just the bits that add DMA support. Since Theo likes this so much, I'd like to move forward with this. ok? Hi Mark, This diff seems to break things on my Lenovo Ideapad 100s. The 100s has an internal eMMC and a microSD card slot. As far as I can tell, reading from a microSD card breaks both eMMC and microSD. Reading from the eMMC, twice for good measure: # dd if=/dev/rsd0c of=/dev/null bs=1M count=1 1+0 records in 1+0 records out 1048576 bytes transferred in 0.191 secs (5486853 bytes/sec) # dd if=/dev/rsd0c of=/dev/null bs=1M count=1 1+0 records in 1+0 records out 1048576 bytes transferred in 0.190 secs (5506851 bytes/sec) Reading from the microSD: # dd if=/dev/rsd1c of=/dev/null bs=1M count=1 dd: /dev/rsd1c: Input/output error 0+0 records in 0+0 records out 0 bytes transferred in 3.019 secs (0 bytes/sec) Reading from the eMMC again: # dd if=/dev/rsd0c of=/dev/null bs=1M count=1 dd: /dev/rsd0c: Input/output error 0+0 records in 0+0 records out 0 bytes transferred in 0.004 secs (0 bytes/sec) At this point the system is unusable, and there's nothing else interesting in dmesg. Can reproduce this on the Lenovo stick, which is in many ways very similar to the 100s. So far I've not found a solution. Since the diff gives significant improvements and seems to be completely stable if I leave the SD card slot empty, I've committed it anyway. You may want to revert the changes to dev/acpi/sdhc_acpi.c for now if you intend to use the SD card slot on the 100s. Hopefully I'll figure out what the problem is soon. Otherwise I might selectively disable DMA support on this hardware. Found the problem. Should be fixed with: CVSROOT:/cvs Module name:src Changes by: kette...@cvs.openbsd.org2016/05/01 11:13:55 Modified files: sys/dev/sdmmc : sdhc.c Log message: Always write block count. This fixes the DMA issues on Bay Trail. Works like a charm. Raw eMMC reads went from ~3.5MB/s to ~35MB/s and microSD reads went from ~3MB/s to ~10MB/s. Writes to filesystems on these devices are a good deal quicker as well, about 4x and 2x, respectively. These changes make a big difference in terms of usability. Thanks for your work on this, Mark. -John
Re: sqlite c api manpages?
>> If you're on OpenBSD, you started with "apropos -s3 sqlite3", were >> shocked that there's nothing there, then moved on to Google with a >> wounding confusion in your heart. > > Indeed. I see this didn't get much traction, but it sounds good to me. We are > shipping sqlite as a system interface. We should provide documentation for at > least the programmatic aspects of it. (The full extent of SQL language > documentation may be beyond the scope of base, but C APIs are not.) Ted, Yes, the sqlite folks haven't said anything since an initial few exchanges on their list. Not sure what else I can do there, but I've been using the sqlite(3) manpages and now can no longer imagine not having them. Just drop a note if you'd like features or something similar---the only thing I can think of is a check to verify that the markup is sane. (I think there's one or two places where some s are unmatched?) Best, Kristaps
Re: mklocale(1): TODIGIT > 255 clobbers type and width data
Hi, Andrew Fresh wrote on Mon, May 02, 2016 at 11:55:52AM -0700: > On Mon, May 02, 2016 at 08:34:43PM +0200, Ingo Schwarze wrote: >> The following patch fixes the bug by limiting digit values to >> the range 0x00 to 0xff that our LC_CTYPE file format can actually >> store. > Does this fix mean that these numbers (like the super cool Aegean > numerals) and the more likely (I guess?) used Ancient Greek numbers > and roman numerals can still not be used in calculations? Well, how would you convert Unicode non-arabic numbers to a numeric variable in the first place? The wcsto*(3) don't handle that, look at libc/locale/_wcstol.h and libc/locale/wctoint.h. Nor do the *wscanf(3) %d, %i, %o, %u, and %x conversions, look at CT_INT in libc/stdio/vfwscanf.c. Grepping for "re_map" and "re_rune_types" across libc/locale shows that the C library provides no read access to the lower byte storing the numerical value. > As you probably noticed, there are several languages that seem to have a > character for 1000, but I don't know how likely those are to be used > in OpenBSD and if so, whether they will use those characters and expect > them to work as numbers. > > (I, for one, think this is almost as nice as scientific notation and way > cooler) > https://en.wikipedia.org/wiki/Aegean_numerals > > Would it be "better" to comment out these TODIGIT entries in > en_US.UTF-8.src noting that our tools currently don't support it? Well, we could delete TODIGIT support outright, we don't use it for anything in the first place, the only effect it had during the first ten years of its life was causing this long-lived and elusive bug. Should i send a patch instead to kill it completely? It doesn't really look that urgent to me, and later cleanup is likely to get round to it anyway. > Possibly making it a validation failure instead of forcing to 0xff > in mklocale? Well, gen_ctype_utf8.pl and en_US.UTF-8.src aren't really incorrect, these characters really have these meanings. So why should these tools complain? The point is really that we don't care all that much about advanced Unicode semantics, and that mklocale(1) shouldn't screw up. > This might make for better errors. Who will look at errors from gen_ctype_utf8.pl and mklocale(1) except you and me, and what are people supposed to do about warning messages from these tools? I don't really see the point. Why warn about edge cases for something that is completely unimplemented in the first place? Todd Miller wrote: > Wouldn't it be better to just ignore values > 0xff instead of > clamping to 0xff? It doesn't really matter, the value is unused in the first place. Putting 0xff at least signals "big digit" to people looking at it with gdb(1). Ignoring it would put 0x00 instead, which really makes no functional difference. A nice bikeshed, so colourful. The point that matters is that this irrelevant information must not clobber character type data and width data. In the worst case, this could cause vulnerabilities, and it is quite hard to evalute what the effects of corrupt type date may be. Yours, Ingo
Re: mklocale(1): TODIGIT > 255 clobbers type and width data
On Mon, 02 May 2016 20:34:43 +0200, Ingo Schwarze wrote: > The following patch fixes the bug by limiting digit values to > the range 0x00 to 0xff that our LC_CTYPE file format can actually > store. Wouldn't it be better to just ignore values > 0xff instead of clamping to 0xff? - todd
Re: mklocale(1): TODIGIT > 255 clobbers type and width data
On Mon, May 02, 2016 at 08:34:43PM +0200, Ingo Schwarze wrote: > The following patch fixes the bug by limiting digit values to > the range 0x00 to 0xff that our LC_CTYPE file format can actually > store. Does this fix mean that these numbers (like the super cool Aegean numerals) and the more likely (I guess?) used Ancient Greek numbers and roman numerals can still not be used in calculations? As you probably noticed, there are several languages that seem to have a character for 1000, but I don't know how likely those are to be used in OpenBSD and if so, whether they will use those characters and expect them to work as numbers. (I, for one, think this is almost as nice as scientific notation and way cooler) https://en.wikipedia.org/wiki/Aegean_numerals Would it be "better" to comment out these TODIGIT entries in en_US.UTF-8.src noting that our tools currently don't support it? Possibly making it a validation failure instead of forcing to 0xff in mklocale? This might make for better errors. l8rZ, -- andrew - http://afresh1.com Computer Science: solving today's problems tomorrow.
Re: sqlite c api manpages?
Kristaps Dzonsons wrote: > If you're on OpenBSD, you started with "apropos -s3 sqlite3", were > shocked that there's nothing there, then moved on to Google with a > wounding confusion in your heart. Indeed. I see this didn't get much traction, but it sounds good to me. We are shipping sqlite as a system interface. We should provide documentation for at least the programmatic aspects of it. (The full extent of SQL language documentation may be beyond the scope of base, but C APIs are not.)
Re: DMA overruns and iommu's
Mark Kettenis wrote: > It is pretty clear that the DMA engine on the Davicom dc(4) hardware > is broken and will read beyond the end of the buffer that we pass it. > This is bad news for hardware that uses an IOMMU, as it will detect > the DMA overrun and (at least on sparc64) signal an unrecoverable > error. > Index: arch/sparc64/dev/iommu.c > === > RCS file: /cvs/src/sys/arch/sparc64/dev/iommu.c,v > retrieving revision 1.72 > diff -u -p -r1.72 iommu.c > --- arch/sparc64/dev/iommu.c 9 Jan 2015 14:23:25 - 1.72 > +++ arch/sparc64/dev/iommu.c 2 Apr 2016 15:42:17 - > @@ -195,6 +195,13 @@ iommu_init(char *name, struct iommu_stat > pmap_update(pmap_kernel()); > memset(is->is_tsb, 0, size); > > + TAILQ_INIT(&mlist); > + if (uvm_pglistalloc(PAGE_SIZE, (paddr_t)0, (paddr_t)-1, > + (paddr_t)PAGE_SIZE, (paddr_t)0, &mlist, 1, UVM_PLA_NOWAIT) != 0) > + panic("iommu_init: no memory"); > + m = TAILQ_FIRST(&mlist); > + is->is_scratch = VM_PAGE_TO_PHYS(m); You want to zero this page I think? I don't like the idea that the device could be reading old data it should not be (even if we don't think it's actually doing anything).
mklocale(1): TODIGIT > 255 clobbers type and width data
Hi, when mklocale(1) input - the only relevant input file for us being /usr/src/share/locale/ctype/en_US.UTF-8.src - contains a line TODIGIT < character(s) digit > where digit is greater than 0xff, mklocale(1) clobbers character type and character width data for the specified character(s). Our input file does indeed contain several such lines. One consequence is that on -current, wcwidth(U+5146) == 3 Oops. That's what made me find the issue, by the way, but it breaks many other characters in various ways, too. The following patch fixes the bug by limiting digit values to the range 0x00 to 0xff that our LC_CTYPE file format can actually store. The only person who ever did a non-janitorial commit to mklocale(1) is espie@, rev. 1.1, more than 10 years ago, so i'm Cc:ing him. OK? Ingo P.S. Et ceterum censeo yaccem lexemque esse delendam, because they are code obfuscation tools striving to make debugging a royal PITA. Index: yacc.y === RCS file: /cvs/src/usr.bin/mklocale/yacc.y,v retrieving revision 1.9 diff -u -p -r1.9 yacc.y --- yacc.y 11 Nov 2015 02:52:46 - 1.9 +++ yacc.y 2 May 2016 18:05:57 - @@ -350,16 +350,22 @@ set_map(rune_map *map, rune_list *list, void set_digitmap(rune_map *map, rune_list *list) { -rune_t i; +rune_t i, digit; while (list) { rune_list *nlist = list->next; for (i = list->min; i <= list->max; ++i) { - if (list->map + (i - list->min)) { + digit = list->map + (i - list->min); + if (digit) { rune_list *tmp = xmalloc(sizeof(rune_list)); tmp->min = i; tmp->max = i; - add_map(map, tmp, list->map + (i - list->min)); + /* +* Limit digit values to 8 bits +* in order to not clobber type and width flags; +* see _RUNETYPE_* in runetype.h. +*/ + add_map(map, tmp, digit > 0xff ? 0xff : digit); } } free(list);
Re: armv7: only attach platform busses if explicitly asked for
On Mon, May 02, 2016 at 10:25:13AM +0200, Patrick Wildt wrote: > Hi, > > currently the armv7 platform busses always match if the board id is set > to something they know. This means that even with a device tree the > platforms will always try to match, even though they should not. > > By properly checking for the ma_name member, we can find out if we're > being asked to run because we're in legacy mode and attach only in that > case. > > ok? ok jsg@ We'll want that (ma->ma_name == NULL) test in arm/cortex/cortex.c as well though. > > Patrick > > diff --git sys/arch/armv7/exynos/exynos.c sys/arch/armv7/exynos/exynos.c > index 16b7a1e..8e8c7dd 100644 > --- sys/arch/armv7/exynos/exynos.c > +++ sys/arch/armv7/exynos/exynos.c > @@ -20,10 +20,8 @@ > #include > > #include > -#if NFDT > 0 > -#include > -#endif > > +#include > #include > > int exynos_match(struct device *, void *, void *); > @@ -171,12 +169,14 @@ exynos_board_name(void) > int > exynos_match(struct device *parent, void *cfdata, void *aux) > { > -#if NFDT > 0 > - /* If we're running with fdt, do not attach. */ > - /* XXX: Find a better way. */ > - if (fdt_next_node(0)) > + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; > + struct cfdata *cf = (struct cfdata *)cfdata; > + > + if (ma->ma_name == NULL) > + return (0); > + > + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) > return (0); > -#endif > > return (exynos_board_devs() != NULL); > } > diff --git sys/arch/armv7/imx/imx.c sys/arch/armv7/imx/imx.c > index 0471a21..f6102e0 100644 > --- sys/arch/armv7/imx/imx.c > +++ sys/arch/armv7/imx/imx.c > @@ -21,6 +21,7 @@ > > #include > > +#include > #include > > int imx_match(struct device *, void *, void *); > @@ -301,5 +302,14 @@ imx_board_name(void) > int > imx_match(struct device *parent, void *cfdata, void *aux) > { > + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; > + struct cfdata *cf = (struct cfdata *)cfdata; > + > + if (ma->ma_name == NULL) > + return (0); > + > + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) > + return (0); > + > return (imx_board_devs() != NULL); > } > diff --git sys/arch/armv7/omap/omap.c sys/arch/armv7/omap/omap.c > index 837ebef..c195360 100644 > --- sys/arch/armv7/omap/omap.c > +++ sys/arch/armv7/omap/omap.c > @@ -20,6 +20,7 @@ > > #include > > +#include > #include > > int omap_match(struct device *, void *, void *); > @@ -175,5 +176,14 @@ omap_board_name(void) > int > omap_match(struct device *parent, void *cfdata, void *aux) > { > + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; > + struct cfdata *cf = (struct cfdata *)cfdata; > + > + if (ma->ma_name == NULL) > + return (0); > + > + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) > + return (0); > + > return (omap_board_devs() != NULL); > } > diff --git sys/arch/armv7/sunxi/sunxi.c sys/arch/armv7/sunxi/sunxi.c > index dac0348..380f9be 100644 > --- sys/arch/armv7/sunxi/sunxi.c > +++ sys/arch/armv7/sunxi/sunxi.c > @@ -21,6 +21,7 @@ > #include > > #include > +#include > #include > #include > > @@ -155,5 +156,14 @@ sunxi_board_name(void) > int > sunxi_match(struct device *parent, void *cfdata, void *aux) > { > + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; > + struct cfdata *cf = (struct cfdata *)cfdata; > + > + if (ma->ma_name == NULL) > + return (0); > + > + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) > + return (0); > + > return (sunxi_board_devs() != NULL); > } > diff --git sys/arch/armv7/vexpress/vexpress.c > sys/arch/armv7/vexpress/vexpress.c > index fcf33f8..1724ddc 100644 > --- sys/arch/armv7/vexpress/vexpress.c > +++ sys/arch/armv7/vexpress/vexpress.c > @@ -22,6 +22,7 @@ > #include > > #include > +#include > #include > > int vexpress_match(struct device *, void *, void *); > @@ -105,5 +106,14 @@ vexpress_board_name(void) > int > vexpress_match(struct device *parent, void *cfdata, void *aux) > { > + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; > + struct cfdata *cf = (struct cfdata *)cfdata; > + > + if (ma->ma_name == NULL) > + return (0); > + > + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) > + return (0); > + > return (vexpress_board_devs() != NULL); > } >
Re: Document inet4/prefix in hostname.if(5)
On Mon, May 02, 2016 at 01:25:22PM +0100, Jason McIntyre wrote: > On Mon, May 02, 2016 at 01:27:50PM +0200, Henning Brauer wrote: > > [...] > > technically, hostname.if doesn't support ip/len notation. It is a > > notation that the hostname parser doesn't grok and just passes on to > > ifconfig. That is the modus operandi for almost everything actually > > - except the classic "inet [addr] [mask] [bcast]" notation. This > > "dual" approach, parsing by netstart vs just passing on to ifconfig, > > is the source of this slightly confusing behaviour. > > > > yep. and someone went to the trouble of trying to make that all nice > and clear, at the top of hostname.if(5). so i don;t see that anything > needs to be done, doc-fix-wise. > [...] Fair enough, I retract my patch then :) -- Gregor
64-bit time_t for user(1)
Whilst reading through Ricardo Mestre's user.c diff I noticed that user(1) still treats time_t as long for password aging. - todd Index: user.c === RCS file: /cvs/src/usr.sbin/user/user.c,v retrieving revision 1.109 diff -u -p -u -r1.109 user.c --- user.c 26 Apr 2016 13:30:12 - 1.109 +++ user.c 2 May 2016 12:55:38 - @@ -1140,14 +1140,14 @@ adduser(char *login_name, user_t *up) up->u_password, password); } } - cc = snprintf(buf, sizeof(buf), "%s:%s:%u:%u:%s:%ld:%ld:%s:%s:%s\n", + cc = snprintf(buf, sizeof(buf), "%s:%s:%u:%u:%s:%lld:%lld:%s:%s:%s\n", login_name, password, up->u_uid, gid, up->u_class, - (long) inactive, - (long) expire, + (long long) inactive, + (long long) expire, up->u_comment, home, up->u_shell); @@ -1601,14 +1601,14 @@ moduser(char *login_name, char *newlogin if (strncmp(login_name, buf, loginc) == 0 && loginc == colonc) { if (up != NULL) { if ((len = snprintf(buf, sizeof(buf), - "%s:%s:%u:%u:%s:%ld:%ld:%s:%s:%s\n", + "%s:%s:%u:%u:%s:%lld:%lld:%s:%s:%s\n", newlogin, pwp->pw_passwd, pwp->pw_uid, pwp->pw_gid, pwp->pw_class, - (long)pwp->pw_change, - (long)pwp->pw_expire, + (long long)pwp->pw_change, + (long long)pwp->pw_expire, pwp->pw_gecos, pwp->pw_dir, pwp->pw_shell)) >= sizeof(buf) || len < 0 ||
Re: Document inet4/prefix in hostname.if(5)
On Mon, May 02, 2016 at 01:27:50PM +0200, Henning Brauer wrote: > * Gregor Best [2016-05-01 19:22]: > > /etc/hostname.if supports IPv4 addresses with a CIDR prefix length: > > > > inet 10.0.0.1/16 > > > > which is not documented in hostname.if(5). The attached patch fixes > > that. I'm not sure whether describing '/prefixlen' before 'netmask' is a > > good idea, but it matches the order things have to be specified in and > > 'netmask' is the next paragraph after '/prefixlen' > > technically, hostname.if doesn't support ip/len notation. It is a > notation that the hostname parser doesn't grok and just passes on to > ifconfig. That is the modus operandi for almost everything actually - > except the classic "inet [addr] [mask] [bcast]" notation. This "dual" > approach, parsing by netstart vs just passing on to ifconfig, is the > source of this slightly confusing behaviour. > yep. and someone went to the trouble of trying to make that all nice and clear, at the top of hostname.if(5). so i don;t see that anything needs to be done, doc-fix-wise. jmc
Re: Document inet4/prefix in hostname.if(5)
* Gregor Best [2016-05-01 19:22]: > /etc/hostname.if supports IPv4 addresses with a CIDR prefix length: > > inet 10.0.0.1/16 > > which is not documented in hostname.if(5). The attached patch fixes > that. I'm not sure whether describing '/prefixlen' before 'netmask' is a > good idea, but it matches the order things have to be specified in and > 'netmask' is the next paragraph after '/prefixlen' technically, hostname.if doesn't support ip/len notation. It is a notation that the hostname parser doesn't grok and just passes on to ifconfig. That is the modus operandi for almost everything actually - except the classic "inet [addr] [mask] [bcast]" notation. This "dual" approach, parsing by netstart vs just passing on to ifconfig, is the source of this slightly confusing behaviour. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
ndp(8) CPPFLAGS
ndp.c doesn't have any #ifdef INET6 preprocessor directive, I can't see how keeping that in CPPFLAGS changes anything. While here, -I${.CURDIR} isn't needed either. Verified with sha256(1). ok? Index: Makefile === RCS file: /cvs/src/usr.sbin/ndp/Makefile,v retrieving revision 1.3 diff -u -p -r1.3 Makefile --- Makefile9 Aug 2013 17:52:12 - 1.3 +++ Makefile2 May 2016 11:15:58 - @@ -4,6 +4,4 @@ PROG= ndp SRCS= ndp.c gmt2local.c MAN= ndp.8 -CPPFLAGS+=-DINET6 -I${.CURDIR} - .include -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: document NOLOCK in syscalls.master
On 10:30:02, 7.04.16, Martin Natano wrote: > > * mention INDIR and NOLOCK in 'Fields'. > > How about something like "one of the types listed below, or one of the > compatibility options defined in syscalls.conf" instead? That's a better idea, included below but s/listed/described/. > > * sort the list of types in 'Fields'. > > * sort the list of types in 'types'. > > I think the order was more logical before (most used type first). I wanted to verify this statement: perl -ne 's/^(\d+\s+)// && s/\s+[{a-z4].*$// && print' kern/syscalls.master | perl -pse 's/\s+/\n/g' | sort | uniq -c | sort -nr 211 STD 59 OBSOL 31 UNIMPL 19 NOLOCK 2 INDIR It seems NODEF and NOARGS are never used, is this right? > > * place a dot ('.') at the end of each sentence. > > Bike-shedding: Those are not full sentences, but only fragments. There > doesn't have to be a dot at the end. I think it's better to have consistency, dot or not. Attached is a version without the unfortunate dots. On 14:57:02, 6.04.16, Mike Belopuhov wrote: > "NOLOCK the syscall doesn't grab any locks whatsoever" > is a poor description IMO. It should say it doesn't grab the > KERNEL_LOCK or biglock or something like that. "Any > locks whatsoever" is incorrect as there might be mutexes, > rwlocks, etc. How about this: NOLOCK don't acquire the kernel lock when calling this syscall. Index: sys/kern/syscalls.master === RCS file: /cvs/src/sys/kern/syscalls.master,v retrieving revision 1.169 diff -u -p -r1.169 syscalls.master --- sys/kern/syscalls.master30 Mar 2016 07:49:11 - 1.169 +++ sys/kern/syscalls.master2 May 2016 10:31:12 - @@ -8,17 +8,18 @@ ; ; Fields: number type [type-dependent ...] ; number system call number, must be in order -; typeone of STD, OBSOL, UNIMPL, NODEF, NOARGS, or one of -; the compatibility options defined in syscalls.conf. +; typeone of the types described below, or one of the compatibility +; options defined in syscalls.conf ; ; types: -; STD always included +; INDIR included, but don't define the syscall args structure, +; and allow it to be "really" varargs +; NOARGS included, but don't define the syscall args structure +; NODEF included, but don't define the syscall number +; NOLOCK don't acquire the kernel lock when calling this syscall ; OBSOL obsolete, not included in system +; STD always included ; UNIMPL unimplemented, not included in system -; NODEF included, but don't define the syscall number -; NOARGS included, but don't define the syscall args structure -; INDIR included, but don't define the syscall args structure, -; and allow it to be "really" varargs. ; ; The compat options are defined in the syscalls.conf file, and the ; compat option name is prefixed to the syscall name. Other than -- Michal Mazurek
Re: remove MPSAFE from makesyscalls.sh
On 10:22:58, 7.04.16, Martin Natano wrote: > On Wed, Apr 06, 2016 at 03:42:20PM +0200, Michal Mazurek wrote: > > MPSAFE is never used, and doesn't look like it's even supported (no > > matching SY_MPSAFE anywhere). > > SY_MPSAFE seems to be unused since it's introduction in 2007 and it > doesn't have any effect; ok natano@, if no one comes up with a reason to > keep this. Nearly a month later, nobody did come up with a reason to keep it. > > Index: sys/kern/makesyscalls.sh > > === > > RCS file: /cvs/src/sys/kern/makesyscalls.sh,v > > retrieving revision 1.11 > > diff -u -p -r1.11 makesyscalls.sh > > --- sys/kern/makesyscalls.sh27 Nov 2007 18:04:01 - 1.11 > > +++ sys/kern/makesyscalls.sh6 Apr 2016 12:53:28 - > > @@ -255,10 +255,6 @@ function parseline() { > > funcalias="" > > end=NF > > } > > - if ($f == "MPSAFE") { # allow MP-safe syscalls > > - sycall_flags = sprintf("SY_MPSAFE | %s", sycall_flags) > > - f++ > > - } > > if ($f == "NOLOCK") { # syscall does not need locks > > sycall_flags = sprintf("SY_NOLOCK | %s", sycall_flags) > > f++ > > Index: sys/sys/systm.h > > === > > RCS file: /cvs/src/sys/sys/systm.h,v > > retrieving revision 1.111 > > diff -u -p -r1.111 systm.h > > --- sys/sys/systm.h 24 Mar 2016 08:57:51 - 1.111 > > +++ sys/sys/systm.h 6 Apr 2016 12:53:28 - > > @@ -112,8 +112,7 @@ extern struct sysent { /* system call t > > sy_call_t *sy_call; /* implementing function */ > > } sysent[]; > > > > -#define SY_MPSAFE 0x01 > > -#define SY_NOLOCK 0x02 > > +#define SY_NOLOCK 0x01 > > > > #if_BYTE_ORDER == _BIG_ENDIAN > > #define SCARG(p, k)((p)->k.be.datum) /* get arg from args > > pointer */ > > > > -- > > Michal Mazurek > > > -- Michal Mazurek
Re: Moving away from softnet interrupts
> Date: Mon, 2 May 2016 11:27:36 +0200 > From: Martin Pieuchot > > On 18/04/16(Mon) 10:50, Martin Pieuchot wrote: > > The current goal of the Network SMP effort is to have a single CPU > > process the IP forwarding path in a process context without holding > > the KERNEL_LOCK(). To achieve this goal we're progressively moving > > code from the softnet interrupt context to the if_input_task. In > > the end we'll completely get rid of this soft-interrupt. > > > > So now would be a good time to know if moving all the code currently > > run in a soft-interrupt context to a task uncovers any bug. I'm > > happily running the diff below on amd64 and macppc, it even gives me > > a small performance boost. > > > > I'd appreciate more tests especially on exotic archs. > > So far this has been tested on amd64, i386, powerpc, mips64 and sparc. And sparc64 (with em, vnet and bridge). No real performance testing done, but I noticed no issues. > So I'm looking for oks. ok kettenis@ > > Index: net/if.c > > === > > RCS file: /cvs/src/sys/net/if.c,v > > retrieving revision 1.429 > > diff -u -p -r1.429 if.c > > --- net/if.c16 Mar 2016 12:08:09 - 1.429 > > +++ net/if.c18 Apr 2016 08:00:08 - > > @@ -64,9 +64,12 @@ > > #include "bpfilter.h" > > #include "bridge.h" > > #include "carp.h" > > +#include "ether.h" > > #include "pf.h" > > +#include "pfsync.h" > > +#include "ppp.h" > > +#include "pppoe.h" > > #include "trunk.h" > > -#include "ether.h" > > > > #include > > #include > > @@ -148,6 +151,7 @@ int if_group_egress_build(void); > > void if_watchdog_task(void *); > > > > void if_input_process(void *); > > +void if_netisr(void *); > > > > #ifdef DDB > > void ifa_print_all(void); > > @@ -216,13 +220,15 @@ void net_tick(void *); > > intnet_livelocked(void); > > intifq_congestion; > > > > -struct taskq *softnettq; > > +int netisr; > > +struct taskq *softnettq; > > + > > +struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET); > > +struct task if_input_task = TASK_INITIALIZER(if_input_process, > > &if_input_queue); > > +struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL); > > > > /* > > * Network interface utility routines. > > - * > > - * Routines with ifa_ifwith* names take sockaddr *'s as > > - * parameters. > > */ > > void > > ifinit(void) > > @@ -590,9 +596,6 @@ if_enqueue(struct ifnet *ifp, struct mbu > > return (0); > > } > > > > -struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET); > > -struct task if_input_task = TASK_INITIALIZER(if_input_process, > > &if_input_queue); > > - > > void > > if_input(struct ifnet *ifp, struct mbuf_list *ml) > > { > > @@ -803,6 +806,50 @@ if_input_process(void *xmq) > > if_put(ifp); > > } > > splx(s); > > +} > > + > > +void > > +if_netisr(void *unused) > > +{ > > + int n, t = 0; > > + int s; > > + > > + KERNEL_LOCK(); > > + s = splsoftnet(); > > + > > + while ((n = netisr) != 0) { > > + sched_pause(); > > + > > + atomic_clearbits_int(&netisr, n); > > + > > + if (n & (1 << NETISR_IP)) > > + ipintr(); > > +#ifdef INET6 > > + if (n & (1 << NETISR_IPV6)) > > + ip6intr(); > > +#endif > > +#if NPPP > 0 > > + if (n & (1 << NETISR_PPP)) > > + pppintr(); > > +#endif > > +#if NBRIDGE > 0 > > + if (n & (1 << NETISR_BRIDGE)) > > + bridgeintr(); > > +#endif > > +#if NPPPOE > 0 > > + if (n & (1 << NETISR_PPPOE)) > > + pppoeintr(); > > +#endif > > + t |= n; > > + } > > + > > +#if NPFSYNC > 0 > > + if (t & (1 << NETISR_PFSYNC)) > > + pfsyncintr(); > > +#endif > > + > > + splx(s); > > + KERNEL_UNLOCK(); > > } > > > > void > > Index: net/netisr.c > > === > > RCS file: net/netisr.c > > diff -N net/netisr.c > > --- net/netisr.c8 Jan 2016 13:53:24 - 1.10 > > +++ /dev/null 1 Jan 1970 00:00:00 - > > @@ -1,74 +0,0 @@ > > -/* > > - * Copyright (c) 2010 Owain G. Ainsworth > > - * > > - * Permission to use, copy, modify, and distribute this software for any > > - * purpose with or without fee is hereby granted, provided that the above > > - * copyright notice and this permission notice appear in all copies. > > - * > > - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > > - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > > - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > > - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > > - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > > - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TO
Re: Moving away from softnet interrupts
On 18/04/16(Mon) 10:50, Martin Pieuchot wrote: > The current goal of the Network SMP effort is to have a single CPU > process the IP forwarding path in a process context without holding > the KERNEL_LOCK(). To achieve this goal we're progressively moving > code from the softnet interrupt context to the if_input_task. In > the end we'll completely get rid of this soft-interrupt. > > So now would be a good time to know if moving all the code currently > run in a soft-interrupt context to a task uncovers any bug. I'm > happily running the diff below on amd64 and macppc, it even gives me > a small performance boost. > > I'd appreciate more tests especially on exotic archs. So far this has been tested on amd64, i386, powerpc, mips64 and sparc. So I'm looking for oks. > Index: net/if.c > === > RCS file: /cvs/src/sys/net/if.c,v > retrieving revision 1.429 > diff -u -p -r1.429 if.c > --- net/if.c 16 Mar 2016 12:08:09 - 1.429 > +++ net/if.c 18 Apr 2016 08:00:08 - > @@ -64,9 +64,12 @@ > #include "bpfilter.h" > #include "bridge.h" > #include "carp.h" > +#include "ether.h" > #include "pf.h" > +#include "pfsync.h" > +#include "ppp.h" > +#include "pppoe.h" > #include "trunk.h" > -#include "ether.h" > > #include > #include > @@ -148,6 +151,7 @@ int if_group_egress_build(void); > void if_watchdog_task(void *); > > void if_input_process(void *); > +void if_netisr(void *); > > #ifdef DDB > void ifa_print_all(void); > @@ -216,13 +220,15 @@ voidnet_tick(void *); > int net_livelocked(void); > int ifq_congestion; > > -struct taskq *softnettq; > +int netisr; > +struct taskq *softnettq; > + > +struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET); > +struct task if_input_task = TASK_INITIALIZER(if_input_process, > &if_input_queue); > +struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL); > > /* > * Network interface utility routines. > - * > - * Routines with ifa_ifwith* names take sockaddr *'s as > - * parameters. > */ > void > ifinit(void) > @@ -590,9 +596,6 @@ if_enqueue(struct ifnet *ifp, struct mbu > return (0); > } > > -struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET); > -struct task if_input_task = TASK_INITIALIZER(if_input_process, > &if_input_queue); > - > void > if_input(struct ifnet *ifp, struct mbuf_list *ml) > { > @@ -803,6 +806,50 @@ if_input_process(void *xmq) > if_put(ifp); > } > splx(s); > +} > + > +void > +if_netisr(void *unused) > +{ > + int n, t = 0; > + int s; > + > + KERNEL_LOCK(); > + s = splsoftnet(); > + > + while ((n = netisr) != 0) { > + sched_pause(); > + > + atomic_clearbits_int(&netisr, n); > + > + if (n & (1 << NETISR_IP)) > + ipintr(); > +#ifdef INET6 > + if (n & (1 << NETISR_IPV6)) > + ip6intr(); > +#endif > +#if NPPP > 0 > + if (n & (1 << NETISR_PPP)) > + pppintr(); > +#endif > +#if NBRIDGE > 0 > + if (n & (1 << NETISR_BRIDGE)) > + bridgeintr(); > +#endif > +#if NPPPOE > 0 > + if (n & (1 << NETISR_PPPOE)) > + pppoeintr(); > +#endif > + t |= n; > + } > + > +#if NPFSYNC > 0 > + if (t & (1 << NETISR_PFSYNC)) > + pfsyncintr(); > +#endif > + > + splx(s); > + KERNEL_UNLOCK(); > } > > void > Index: net/netisr.c > === > RCS file: net/netisr.c > diff -N net/netisr.c > --- net/netisr.c 8 Jan 2016 13:53:24 - 1.10 > +++ /dev/null 1 Jan 1970 00:00:00 - > @@ -1,74 +0,0 @@ > -/* > - * Copyright (c) 2010 Owain G. Ainsworth > - * > - * Permission to use, copy, modify, and distribute this software for any > - * purpose with or without fee is hereby granted, provided that the above > - * copyright notice and this permission notice appear in all copies. > - * > - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > - */ > -#include > -#include > - > -#include > - > -#include > - > -#include "ppp.h" > -#include "bridge.h" > -#include "pppoe.h" > -#include "pfsync.h" > - > -void netintr(void *); > - > -int netisr; > -void *netisr_intr; > - > -void > -netintr(void *unused) > -{ > - int n, t = 0; > - > - while ((n = netisr) != 0) { > - atomic_clearbits_int(&netisr, n);
Re: Single route lookup when forwarding an IPv4 packet
On 25/04/16(Mon) 14:40, Martin Pieuchot wrote: > Diff below makes ip_forward() use the route entry fetched in in_ouraddr(). > > As you can imagine a proper caching could be done for forwarding using > PF statekey. > > This diff has been tested by Hrvoje Popovski who confirmed that the > benchmark performances are similar to the ones using a single cache > entry. > > ok? Anyone? > Index: netinet/ip_input.c > === > RCS file: /cvs/src/sys/netinet/ip_input.c,v > retrieving revision 1.274 > diff -u -p -r1.274 ip_input.c > --- netinet/ip_input.c25 Apr 2016 12:33:48 - 1.274 > +++ netinet/ip_input.c25 Apr 2016 12:34:46 - > @@ -126,8 +126,8 @@ static struct mbuf_queue ipsend_mq; > > void ip_ours(struct mbuf *); > int ip_dooptions(struct mbuf *, struct ifnet *); > -int in_ouraddr(struct mbuf *, struct ifnet *, struct in_addr); > -void ip_forward(struct mbuf *, struct ifnet *, int); > +int in_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **); > +void ip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int); > #ifdef IPSEC > int ip_input_ipsec_fwd_check(struct mbuf *, int); > int ip_input_ipsec_ours_check(struct mbuf *, int); > @@ -223,8 +223,9 @@ ipintr(void) > void > ipv4_input(struct mbuf *m) > { > - struct ifnet *ifp; > - struct ip *ip; > + struct ifnet*ifp; > + struct rtentry *rt = NULL; > + struct ip *ip; > int hlen, len; > #if defined(MROUTING) || defined(IPSEC) > int rv; > @@ -341,7 +342,7 @@ ipv4_input(struct mbuf *m) > goto out; > } > > - if (in_ouraddr(m, ifp, ip->ip_dst)) { > + if (in_ouraddr(m, ifp, &rt)) { > ip_ours(m); > goto out; > } > @@ -443,12 +444,13 @@ ipv4_input(struct mbuf *m) > } > #endif /* IPSEC */ > > - ip_forward(m, ifp, pfrdr); > + ip_forward(m, ifp, rt, pfrdr); > if_put(ifp); > return; > bad: > m_freem(m); > out: > + rtfree(rt); > if_put(ifp); > } > > @@ -575,9 +577,10 @@ bad: > } > > int > -in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct in_addr ina) > +in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct rtentry **prt) > { > struct rtentry *rt; > + struct ip *ip; > struct sockaddr_in sin; > int match = 0; > #if NPF > 0 > @@ -597,10 +600,12 @@ in_ouraddr(struct mbuf *m, struct ifnet > } > #endif > > + ip = mtod(m, struct ip *); > + > memset(&sin, 0, sizeof(sin)); > sin.sin_len = sizeof(sin); > sin.sin_family = AF_INET; > - sin.sin_addr = ina; > + sin.sin_addr = ip->ip_dst; > rt = rtalloc(sintosa(&sin), 0, m->m_pkthdr.ph_rtableid); > if (rtisvalid(rt)) { > if (ISSET(rt->rt_flags, RTF_LOCAL)) > @@ -618,7 +623,7 @@ in_ouraddr(struct mbuf *m, struct ifnet > m->m_flags |= M_BCAST; > } > } > - rtfree(rt); > + *prt = rt; > > if (!match) { > struct ifaddr *ifa; > @@ -630,7 +635,7 @@ in_ouraddr(struct mbuf *m, struct ifnet >* address on the interface it was received on. >*/ > if (!ISSET(m->m_flags, M_BCAST) || > - !IN_CLASSFULBROADCAST(ina.s_addr, ina.s_addr)) > + !IN_CLASSFULBROADCAST(ip->ip_dst.s_addr, ip->ip_dst.s_addr)) > return (0); > > if (ifp->if_rdomain != rtable_l2(m->m_pkthdr.ph_rtableid)) > @@ -645,7 +650,7 @@ in_ouraddr(struct mbuf *m, struct ifnet > if (ifa->ifa_addr->sa_family != AF_INET) > continue; > > - if (IN_CLASSFULBROADCAST(ina.s_addr, > + if (IN_CLASSFULBROADCAST(ip->ip_dst.s_addr, > ifatoia(ifa)->ia_addr.sin_addr.s_addr)) { > match = 1; > break; > @@ -1241,7 +1246,7 @@ ip_dooptions(struct mbuf *m, struct ifne > } > KERNEL_UNLOCK(); > if (forward && ipforwarding) { > - ip_forward(m, ifp, 1); > + ip_forward(m, ifp, NULL, 1); > return (1); > } > return (0); > @@ -1387,12 +1392,11 @@ int inetctlerrmap[PRC_NCMDS] = { > * via a source route. > */ > void > -ip_forward(struct mbuf *m, struct ifnet *ifp, int srcrt) > +ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt) > { > struct mbuf mfake, *mcopy = NULL; > struct ip *ip = mtod(m, struct ip *); > struct sockaddr_in *sin; > - struct rtentry *rt; > struct route ro; > int error, type = 0, code = 0, destmtu = 0, fake = 0, len; > u_int32_t dest; > @@ -1401,25 +1405,27 @@ ip_forward(struct mbuf *m, struct ifnet > if (m->m_flags & (M_BCAST|M_MCAST) || in_canforward(ip->ip_dst) == 0) { >
libpcap: use /dev/bpf
And again: /dev/bpf instead of /dev/bpf*. Ok? natano Index: pcap-bpf.c === RCS file: /cvs/src/lib/libpcap/pcap-bpf.c,v retrieving revision 1.32 diff -u -p -r1.32 pcap-bpf.c --- pcap-bpf.c 22 Dec 2015 19:51:04 - 1.32 +++ pcap-bpf.c 2 May 2016 08:44:39 - @@ -215,69 +215,20 @@ static __inline int bpf_open(pcap_t *p) { int fd; - int n = 0; - char device[sizeof "/dev/bpf00"]; - /* -* Go through all the minors and find one that isn't in use. -*/ - do { - (void)snprintf(device, sizeof device, "/dev/bpf%d", n++); - fd = open(device, O_RDWR); - if (fd < 0 && errno == EACCES) - fd = open(device, O_RDONLY); - } while (fd < 0 && errno == EBUSY); + fd = open("/dev/bpf", O_RDWR); + if (fd == -1 && errno == EACCES) + fd = open("/dev/bpf", O_RDONLY); - /* -* XXX better message for all minors used -*/ - if (fd < 0) { - switch (errno) { - - case ENOENT: - fd = PCAP_ERROR; - if (n == 1) { - /* -* /dev/bpf0 doesn't exist, which -* means we probably have no BPF -* devices. -*/ - snprintf(p->errbuf, PCAP_ERRBUF_SIZE, - "(there are no BPF devices)"); - } else { - /* -* We got EBUSY on at least one -* BPF device, so we have BPF -* devices, but all the ones -* that exist are busy. -*/ - snprintf(p->errbuf, PCAP_ERRBUF_SIZE, - "(all BPF devices are busy)"); - } - break; - - case EACCES: - /* -* Got EACCES on the last device we tried, -* and EBUSY on all devices before that, -* if any. -*/ + if (fd == -1) { + if (errno == EACCES) fd = PCAP_ERROR_PERM_DENIED; - snprintf(p->errbuf, PCAP_ERRBUF_SIZE, - "(cannot open BPF device) %s: %s", device, - pcap_strerror(errno)); - break; - - default: - /* -* Some other problem. -*/ + else fd = PCAP_ERROR; - snprintf(p->errbuf, PCAP_ERRBUF_SIZE, - "(cannot open BPF device) %s: %s", device, - pcap_strerror(errno)); - break; - } + + snprintf(p->errbuf, PCAP_ERRBUF_SIZE, + "(cannot open BPF device): %s", + pcap_strerror(errno)); } return (fd);
armv7: only attach platform busses if explicitly asked for
Hi, currently the armv7 platform busses always match if the board id is set to something they know. This means that even with a device tree the platforms will always try to match, even though they should not. By properly checking for the ma_name member, we can find out if we're being asked to run because we're in legacy mode and attach only in that case. ok? Patrick diff --git sys/arch/armv7/exynos/exynos.c sys/arch/armv7/exynos/exynos.c index 16b7a1e..8e8c7dd 100644 --- sys/arch/armv7/exynos/exynos.c +++ sys/arch/armv7/exynos/exynos.c @@ -20,10 +20,8 @@ #include #include -#if NFDT > 0 -#include -#endif +#include #include intexynos_match(struct device *, void *, void *); @@ -171,12 +169,14 @@ exynos_board_name(void) int exynos_match(struct device *parent, void *cfdata, void *aux) { -#if NFDT > 0 - /* If we're running with fdt, do not attach. */ - /* XXX: Find a better way. */ - if (fdt_next_node(0)) + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; + struct cfdata *cf = (struct cfdata *)cfdata; + + if (ma->ma_name == NULL) + return (0); + + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) return (0); -#endif return (exynos_board_devs() != NULL); } diff --git sys/arch/armv7/imx/imx.c sys/arch/armv7/imx/imx.c index 0471a21..f6102e0 100644 --- sys/arch/armv7/imx/imx.c +++ sys/arch/armv7/imx/imx.c @@ -21,6 +21,7 @@ #include +#include #include intimx_match(struct device *, void *, void *); @@ -301,5 +302,14 @@ imx_board_name(void) int imx_match(struct device *parent, void *cfdata, void *aux) { + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; + struct cfdata *cf = (struct cfdata *)cfdata; + + if (ma->ma_name == NULL) + return (0); + + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) + return (0); + return (imx_board_devs() != NULL); } diff --git sys/arch/armv7/omap/omap.c sys/arch/armv7/omap/omap.c index 837ebef..c195360 100644 --- sys/arch/armv7/omap/omap.c +++ sys/arch/armv7/omap/omap.c @@ -20,6 +20,7 @@ #include +#include #include intomap_match(struct device *, void *, void *); @@ -175,5 +176,14 @@ omap_board_name(void) int omap_match(struct device *parent, void *cfdata, void *aux) { + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; + struct cfdata *cf = (struct cfdata *)cfdata; + + if (ma->ma_name == NULL) + return (0); + + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) + return (0); + return (omap_board_devs() != NULL); } diff --git sys/arch/armv7/sunxi/sunxi.c sys/arch/armv7/sunxi/sunxi.c index dac0348..380f9be 100644 --- sys/arch/armv7/sunxi/sunxi.c +++ sys/arch/armv7/sunxi/sunxi.c @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -155,5 +156,14 @@ sunxi_board_name(void) int sunxi_match(struct device *parent, void *cfdata, void *aux) { + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; + struct cfdata *cf = (struct cfdata *)cfdata; + + if (ma->ma_name == NULL) + return (0); + + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) + return (0); + return (sunxi_board_devs() != NULL); } diff --git sys/arch/armv7/vexpress/vexpress.c sys/arch/armv7/vexpress/vexpress.c index fcf33f8..1724ddc 100644 --- sys/arch/armv7/vexpress/vexpress.c +++ sys/arch/armv7/vexpress/vexpress.c @@ -22,6 +22,7 @@ #include #include +#include #include intvexpress_match(struct device *, void *, void *); @@ -105,5 +106,14 @@ vexpress_board_name(void) int vexpress_match(struct device *parent, void *cfdata, void *aux) { + union mainbus_attach_args *ma = (union mainbus_attach_args *)aux; + struct cfdata *cf = (struct cfdata *)cfdata; + + if (ma->ma_name == NULL) + return (0); + + if (strcmp(cf->cf_driver->cd_name, ma->ma_name) != 0) + return (0); + return (vexpress_board_devs() != NULL); }
Re: arm: new FDT-enabled mainbus
On Sun, May 01, 2016 at 04:55:02PM +0200, Mark Kettenis wrote: > > Date: Sun, 1 May 2016 13:27:29 +0200 > > From: Patrick Wildt > > > > Hi, > > > > I updated the diff with the feedback received. This basically adds > > a tree-like topology by making mainbus FDT aware and implementing > > a simplebus that can span the tree's roots into more branches. > > > > Next steps (and diffs) are implementing an FDT platform for armv7, > > similar to imx/omap/... and having the generic interrupt controller > > and timer attach to a simplebus/fdt bus. > > > > Comments? > > Looks good to me. If jsg@ agrees, let's get this in and work on it > further in the tree. Yes, I agree. > > > diff --git sys/arch/arm/conf/files.arm sys/arch/arm/conf/files.arm > > index cb11960..c70f9ab 100644 > > --- sys/arch/arm/conf/files.arm > > +++ sys/arch/arm/conf/files.arm > > @@ -16,15 +16,24 @@ filearch/arm/arm/disassem.c ddb > > file arch/arm/arm/fiq.c fiq > > file arch/arm/arm/fiq_subr.S fiq > > > > +define fdt {} > > + > > # mainbus files > > -device mainbus {} > > +device mainbus: fdt > > attach mainbus at root > > file arch/arm/mainbus/mainbus.c mainbus > > > > +device simplebus: fdt > > +attach simplebus at fdt > > +file arch/arm/simplebus/simplebus.c simplebus > > + > > +# FDT support > > +file dev/ofw/fdt.c > > + > > include "arch/arm/cortex/files.cortex" > > > > device cpu {} > > -attach cpu at mainbus with cpu_mainbus > > +attach cpu at fdt with cpu_mainbus > > file arch/arm/mainbus/cpu_mainbus.c cpu_mainbus > > > > > > diff --git sys/arch/arm/cortex/cortex.c sys/arch/arm/cortex/cortex.c > > index 913feb7..06a7823 100644 > > --- sys/arch/arm/cortex/cortex.c > > +++ sys/arch/arm/cortex/cortex.c > > @@ -97,7 +97,7 @@ struct cfdriver cortex_cd = { > > int > > cortexmatch(struct device *parent, void *cfdata, void *aux) > > { > > - struct mainbus_attach_args *ma = aux; > > + union mainbus_attach_args *ma = aux; > > struct cfdata *cf = (struct cfdata *)cfdata; > > int cputype = cpufunc_id(); > > > > diff --git sys/arch/arm/cortex/files.cortex sys/arch/arm/cortex/files.cortex > > index c0f4359..052acfd 100644 > > --- sys/arch/arm/cortex/files.cortex > > +++ sys/arch/arm/cortex/files.cortex > > @@ -2,7 +2,7 @@ > > > > # ARM core > > device cortex {} > > -attach cortex at mainbus > > +attach cortex at fdt > > file arch/arm/cortex/cortex.ccortex > > > > device ampintc > > diff --git sys/arch/arm/include/fdt.h sys/arch/arm/include/fdt.h > > new file mode 100644 > > index 000..0eec567 > > --- /dev/null > > +++ sys/arch/arm/include/fdt.h > > @@ -0,0 +1,31 @@ > > +/* $OpenBSD$ */ > > +/* > > + * Copyright (c) 2016 Patrick Wildt > > + * > > + * Permission to use, copy, modify, and distribute this software for any > > + * purpose with or without fee is hereby granted, provided that the above > > + * copyright notice and this permission notice appear in all copies. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > > + */ > > + > > +#ifndef __ARM_FDT_H__ > > +#define __ARM_FDT_H__ > > + > > +#define _ARM32_BUS_DMA_PRIVATE > > +#include > > + > > +struct fdt_attach_args { > > + const char *fa_name; > > + int fa_node; > > + bus_space_tag_t fa_iot; > > + bus_dma_tag_tfa_dmat; > > +}; > > + > > +#endif /* __ARM_FDT_H__ */ > > diff --git sys/arch/arm/mainbus/cpu_mainbus.c > > sys/arch/arm/mainbus/cpu_mainbus.c > > index 63de209..88410bf 100644 > > --- sys/arch/arm/mainbus/cpu_mainbus.c > > +++ sys/arch/arm/mainbus/cpu_mainbus.c > > @@ -72,7 +72,7 @@ static void cpu_mainbus_attach (struct device *, struct > > device *, void *); > > static int > > cpu_mainbus_match(struct device *parent, void *vcf, void *aux) > > { > > - struct mainbus_attach_args *ma = aux; > > + union mainbus_attach_args *ma = aux; > > struct cfdata *cf = (struct cfdata *)vcf; > > > > return (strcmp(cf->cf_driver->cd_name, ma->ma_name) == 0); > > diff --git sys/arch/arm/mainbus/mainbus.c sys/arch/arm/mainbus/mainbus.c > > index 6ad3e8f..7f72161 100644 > > --- sys/arch/arm/mainbus/mainbus.c > > +++ sys/arch/arm/mainbus/mainbus.c > > @@ -1,45 +1,18 @@ > > -/* $OpenBSD: mainbus.c,v 1.7 2013/05/30 16:15:01 deraadt Exp $ */ > > -/* $NetBSD: mainbus.c,v 1.3 2001/
dhclient: use /dev/bpf
Diff below simplifies the device open path and removes an explanation about bpf device nodes from the manpage. Ok? natano Index: bpf.c === RCS file: /cvs/src/sbin/dhclient/bpf.c,v retrieving revision 1.38 diff -u -p -r1.38 bpf.c --- bpf.c 6 Feb 2016 19:30:52 - 1.38 +++ bpf.c 2 May 2016 07:27:46 - @@ -64,8 +64,6 @@ #include "dhcp.h" #include "dhcpd.h" -#define BPF_FORMAT "/dev/bpf%d" - int if_register_bpf(void); /* @@ -76,29 +74,17 @@ int if_register_bpf(void); int if_register_bpf(void) { - char filename[50]; struct ifreq ifr; - int sock, b; + int sock; - /* Open a BPF device */ - for (b = 0; 1; b++) { - snprintf(filename, sizeof(filename), BPF_FORMAT, b); - sock = open(filename, O_RDWR | O_CLOEXEC, 0); - if (sock < 0) { - if (errno == EBUSY) - continue; - else - error("Can't find free bpf: %s", - strerror(errno)); - } else - break; - } + if ((sock = open("/dev/bpf", O_RDWR | O_CLOEXEC)) == -1) + error("Can't open bpf: %s", strerror(errno)); /* Set the BPF device to point at this interface. */ strlcpy(ifr.ifr_name, ifi->name, IFNAMSIZ); if (ioctl(sock, BIOCSETIF, &ifr) < 0) - error("Can't attach interface %s to %s: %s", - ifi->name, filename, strerror(errno)); + error("Can't attach interface %s to /dev/bpf: %s", + ifi->name, strerror(errno)); return (sock); } Index: dhclient.8 === RCS file: /cvs/src/sbin/dhclient/dhclient.8,v retrieving revision 1.26 diff -u -p -r1.26 dhclient.8 --- dhclient.8 15 Nov 2014 00:12:52 - 1.26 +++ dhclient.8 2 May 2016 07:27:46 - @@ -249,14 +249,6 @@ In that case, it may be advantageous to arrange with the network administrator for an entry on the BOOTP database, so that the host can boot quickly on that network rather than cycling through the list of old leases. -.Pp -.Nm -requires at least one -.Pa /dev/bpf* -file for each broadcast network interface. -See -.Xr bpf 4 -for more information. .Sh SIGNALS While running, .Nm @@ -306,7 +298,6 @@ interface-specific configuration files database of acquired leases .El .Sh SEE ALSO -.Xr bpf 4 , .Xr dhclient.conf 5 , .Xr dhclient.leases 5 , .Xr hostname.if 5 ,