Re: interfaces and priorities for relayd routers
On 2015/06/22 10:20, Reyk Floeter wrote: The router code was mostly done for something like link balancing, you have two or more uplink gateways and want to select the route based on their availability. I've never really understood how this is meant to work, in real life the usual case is where the gateway is up but the attached circuit is down so there's no routing beyond it, am I missing a non-obvious way where relayd can check for this?
Re: interfaces and priorities for relayd routers
On 24 Jun 2015, at 10:38 am, David Gwynne da...@gwynne.id.au wrote: On 22 Jun 2015, at 18:20, Reyk Floeter r...@openbsd.org wrote: Hi, On Thu, May 14, 2015 at 09:44:22PM +1000, David Gwynne wrote: i want relayd to check teh availability of some services and inject routes when the service is available. if it is available, i want to advertise the routes using ospfd, but i also want the local machine to be able to contact the service even if it isnt the carp master. to do that i need to inject the routes twice, once on my real interface and again on my carp interfaces. the route on the real interface needs to be a higher priority than the carp route. this shuffles relayd to accomodate this. it mostly adds stuff to the route statements in routers, but i also take priorities away from hosts in tables so i can specify them on routes. I'm generally ok with extending the use cases of router, but it seems that you're replacing one narrow use case with another one. The router code was mostly done for something like link balancing, you have two or more uplink gateways and want to select the route based on their availability. I'm afraid that moving the priority from hosts to routes, and changing the semantics, breaks this original use case somehow. table gateways { $gw1 ip ttl 1 priority 8, $gw2 ip ttl 1 priority 52 } router uplinks { route 0.0.0.0/0 forward to gateways check icmp } But maybe even multiple routes for something internal: router srvlan { route 10.10.0.0/16 route 10.40.0.0/16 forward to srvgateways check icmp } Sometimes different gateways can even be connected via the same interface! (e.g. with an intermediate switch and a single cable to the OpenBSD box) this is an example config: rns_nogal=130.102.71.227 rns_neem=130.102.71.229 table rns { $rns_nogal ip ttl 1, $rns_neem ip ttl 1 } router rns { route 130.102.71.160/31 interface vlan888 priority 8 route 130.102.71.160/31 interface carp40888 priority 16 forward to rns check icmp } I'm wondering, how would you translate my examples above? hrm. we could allow priorities on both the route statements and on the host statements in the table and combine them somehow when the actual routes are inserted into the kernel. or allow a relative priorities to be specified with a + or - prefix. eg: table gateways { $gw1 priority +1, $gw2 } router uplinks { route 0.0.0.0/0 priority 16 forward to gateways check icmp } if you add increase the priority of a route, should its numeric value get higher or lower? anyway, allowing priorities in both tables and router blocks would cover both use cases i think. another option could be table gw1 { $gw1 } table gw2 { $gw2 } router hipri-uplink { route 0.0.0.0/0 priority 8 forward to gw1 check icmp } router lopri-uplink { route 0.0.0.0/0 priority 52 forward to gw2 check icmp } dlg redirect dns { listen on 130.102.71.160 tcp port 53 listen on 130.102.71.160 udp port 53 listen on 130.102.71.161 tcp port 53 listen on 130.102.71.161 udp port 53 match pftag rns forward to rns port 53 check icmp } i wish redirects took a prefix. maybe thats a diff for another day. Would make sense. anyway, here's the diff. thoughts? tweaks? ok? In addition to my general concern, see comments inline below. Reyk Index: parse.y === RCS file: /cvs/src/usr.sbin/relayd/parse.y,v retrieving revision 1.204 diff -u -p -r1.204 parse.y --- parse.y 2 May 2015 13:15:24 - 1.204 +++ parse.y 14 May 2015 11:30:28 - @@ -173,6 +173,7 @@ typedef struct { %token ROUTER RTLABEL TRANSPARENT TRAP UPDATES URL VIRTUAL WITH TTL RTABLE %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD ECDH %token EDH CURVE +%token NONE LOCAL CONNECTED STATIC OSPF ISIS RIP BGP DEFAULT A lot of keywords for basically an enum. See below. %token v.string STRING %token v.number NUMBER %type v.string hostname interface table value optstring @@ -182,6 +183,7 @@ typedef struct { %type v.number redirect_proto relay_proto match %type v.number action ruleaf key_option %type v.number tlsdhparams tlsecdhcurve +%type v.number rtprio %type v.portport %type v.hosthost %type v.addraddress @@ -1864,8 +1866,8 @@ router: ROUTER STRING { router = rt; tableport = -1; - } '{' optnl routeopts_l '}' { - if (!router-rt_conf.nroutes) {
Re: pkg_info: print used repos
On Tue, Jun 23, 2015 at 08:53:15PM -0600, Theo de Raadt wrote: - $state-handle_options('cCdfF:hIKLmPQ:qr:RsSUe:E:Ml:aAt', + $state-handle_options('cCdfF:hIKLmpPQ:qr:RsSUe:E:Ml:aAt', Starting to look a lot like ls. What a coincidence. It's used to list things. So is ls.
Re: interfaces and priorities for relayd routers
On Wed, Jun 24, 2015 at 10:35:26AM +0100, Stuart Henderson wrote: On 2015/06/22 10:20, Reyk Floeter wrote: The router code was mostly done for something like link balancing, you have two or more uplink gateways and want to select the route based on their availability. I've never really understood how this is meant to work, in real life the usual case is where the gateway is up but the attached circuit is down so there's no routing beyond it, am I missing a non-obvious way where relayd can check for this? What do you mean with real life? Is mine not real? That reminds me of fobser's signature: I'm not entirely sure you are real. ;-) So it is basically a cheap alternative for sites that want uplink redundancy without speaking BGP. You have to assume that you are the next hop that is connected to some layer 2 uplink, without a local 3rd party router in between: +-+- Line 1 - [ gateway @ISP-A ] | OpenBSD | +-+- Line 2 - [ gateway @ISP-B ] The ttl 1 makes sure that you're not accidentally testing ISP-A's gateway address all the way over ISP-B. table gateways { $ispgwa ip ttl 1 $ispgwb ip ttl 1 } I never tried, but you could theoretically use the parent option to inherit the status from a different hop behind your local gateway. +-+-[ local 1 ]- Line 1 - [ gateway @ISP-A ] | OpenBSD | +-+-[ local 2 ]- Line 2 - [ gateway @ISP-B ] table nexthops { $ispgwa ip ttl 2 $ispgwb ip ttl 2 } table gateways { $local1 parent 1 $local2 parent 2 } You'd have to define a fake check somewhere to run health checks on the gateway IPs behind the nexthops (eg. an unused relay or redirect). But, as I said, router can be extened, just don't break it ;) Reyk
Remove a #if NCARP hack
Time goes by and things must be cleaned. Thanks to claudio@'s work to support multiple connected routes carp(4) now have its own default priority. So I audited the remaining iterations on finet and I couldn't find any good reason to force carp(4) interfaces at a special position in the list of interfaces. OpenBSD's network stack changed in the past years and ifa_ifwithnet() is now only used in a few places that should not matter. Maybe a brave soul (anyone?) will even get rid of this function completely. The other place where this order could matter is in6_ifawithscope(). But stsp@ added an explicit check for carp(4) interfaces last October. Remember also that since 5.7 the ``carpdev'' argument is mandatory. So you're rather unlikely to have a carp(4) interface inserted in ifnet before its parent interface. Finally I sleep better with fewer #if PSEUDOFROG in the stack :) Ok? Index: net/if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.333 diff -u -p -r1.333 if.c --- net/if.c20 May 2015 08:28:54 - 1.333 +++ net/if.c26 May 2015 08:45:51 - @@ -373,24 +373,8 @@ if_attachhead(struct ifnet *ifp) void if_attach(struct ifnet *ifp) { -#if NCARP 0 - struct ifnet *before = NULL; -#endif - if_attach_common(ifp); - -#if NCARP 0 - if (ifp-if_type != IFT_CARP) - TAILQ_FOREACH(before, ifnet, if_list) - if (before-if_type == IFT_CARP) - break; - if (before == NULL) - TAILQ_INSERT_TAIL(ifnet, ifp, if_list); - else - TAILQ_INSERT_BEFORE(before, ifp, if_list); -#else TAILQ_INSERT_TAIL(ifnet, ifp, if_list); -#endif if_attachsetup(ifp); }
Re: [diff] trivia: change xorl = xorq
On Wed, Jun 24, 2015 at 12:33 AM, Philip Guenther guent...@gmail.com wrote: On Tue, Jun 23, 2015 at 1:18 PM, Mike Larkin mlar...@azathoth.net wrote: On Tue, Jun 23, 2015 at 09:49:26AM +0300, Alexey Dobriyan wrote: Clearing 32-bit register clears whole register, save REX prefix. I see nothing documented in the Intel SDM that says this. Can you cite a reference to support this claim? It's true. Volume 1, section 3.4.1 says this: ... 9.2.1 Use Legacy 32-Bit Instructions When Data Size Is 32 Bits XOR r32, r32 is also documented to break register dependencies while XOR r64, r64 is not (15.3.2.5 zeroing idioms). Just noticed, subject line is bogus, it should be xorq = xorl obviously.
Re: pkg_info: print used repos
On Wed, Jun 24, 2015 at 11:59:41AM +0200, ludovic coues wrote: 2015-06-24 11:40 GMT+02:00 Marc Espie es...@nerim.net: On Tue, Jun 23, 2015 at 08:53:15PM -0600, Theo de Raadt wrote: - $state-handle_options('cCdfF:hIKLmPQ:qr:RsSUe:E:Ml:aAt', + $state-handle_options('cCdfF:hIKLmpPQ:qr:RsSUe:E:Ml:aAt', Starting to look a lot like ls. What a coincidence. It's used to list things. So is ls. Maybe Theo was hoping someone would trim ls of all the unused option. Not surprisingly. But getting pkg_info cleaned up is not very simple, and also it's definitely not my main priority now. I would very much prefer someone to track down the sometimes issue where pkg_add burps with /var/db/pkg out of synch. I also need to rewrite Vstat.pm to handle some things it can. I have a race condition in dpb that I want to fix, and my chroot script isn't finished yet. There are many things which are not perfect in pkg_add nor dpb. Getting things better is happening but it's a slow process.
Re: bridge_output() without m_buf_tag
On 17/06/15(Wed) 14:07, Martin Pieuchot wrote: On 08/06/15(Mon) 15:58, Martin Pieuchot wrote: Diff below moves bridge_output() to if_output(). It fixes the case I already described some weeks ago where you have a physical interface in a bridge and a vlan on top of it which is not in the bridge. It also change the loop prevention code to use M_PROTO1 like in the input path. Tests, comments and oks welcome. Updated diff to match the recent if_get() change. I've got one positive report so far, any ok? I'm still looking for oks. Index: net/if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.340 diff -u -p -r1.340 if.c --- net/if.c 16 Jun 2015 11:09:39 - 1.340 +++ net/if.c 17 Jun 2015 12:01:12 - @@ -449,6 +449,19 @@ if_output(struct ifnet *ifp, struct mbuf int s, length, error = 0; unsigned short mflags; +#ifdef DIAGNOSTIC + if (ifp-if_rdomain != rtable_l2(m-m_pkthdr.ph_rtableid)) { + printf(%s: trying to send packet on wrong domain. + if %d vs. mbuf %d\n, ifp-if_xname, ifp-if_rdomain, + rtable_l2(m-m_pkthdr.ph_rtableid)); + } +#endif + +#if NBRIDGE 0 + if (ifp-if_bridgeport (m-m_flags M_PROTO1) == 0) + return (bridge_output(ifp, m, NULL, NULL)); +#endif + length = m-m_pkthdr.len; mflags = m-m_flags; Index: net/if_bridge.c === RCS file: /cvs/src/sys/net/if_bridge.c,v retrieving revision 1.244 diff -u -p -r1.244 if_bridge.c --- net/if_bridge.c 16 Jun 2015 11:09:39 - 1.244 +++ net/if_bridge.c 17 Jun 2015 12:01:12 - @@ -2635,10 +2635,12 @@ bridge_ifenqueue(struct bridge_softc *sc { int error, len; + /* Loop prevention. */ + m-m_flags |= M_PROTO1; + #if NGIF 0 /* Packet needs etherip encapsulation. */ if (ifp-if_type == IFT_GIF) { - m-m_flags |= M_PROTO1; /* Count packets input into the gif from outside */ ifp-if_ipackets++; Index: net/if_ethersubr.c === RCS file: /cvs/src/sys/net/if_ethersubr.c,v retrieving revision 1.205 diff -u -p -r1.205 if_ethersubr.c --- net/if_ethersubr.c16 Jun 2015 11:09:39 - 1.205 +++ net/if_ethersubr.c17 Jun 2015 12:01:12 - @@ -181,15 +181,6 @@ ether_output(struct ifnet *ifp, struct m struct arpcom *ac = (struct arpcom *)ifp; int error = 0; -#ifdef DIAGNOSTIC - if (ifp-if_rdomain != rtable_l2(m-m_pkthdr.ph_rtableid)) { - printf(%s: trying to send packet on wrong domain. - if %d vs. mbuf %d, AF %d\n, ifp-if_xname, - ifp-if_rdomain, rtable_l2(m-m_pkthdr.ph_rtableid), - dst-sa_family); - } -#endif - esrc = ac-ac_enaddr; if ((ifp-if_flags (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING)) @@ -276,47 +267,6 @@ ether_output(struct ifnet *ifp, struct m eh-ether_type = etype; memcpy(eh-ether_dhost, edst, sizeof(eh-ether_dhost)); memcpy(eh-ether_shost, esrc, sizeof(eh-ether_shost)); - -#if NBRIDGE 0 - /* - * Interfaces that are bridgeports need special handling for output. - */ - if (ifp-if_bridgeport) { - struct m_tag *mtag; - - /* - * Check if this packet has already been sent out through - * this bridgeport, in which case we simply send it out - * without further bridge processing. - */ - for (mtag = m_tag_find(m, PACKET_TAG_BRIDGE, NULL); mtag; - mtag = m_tag_find(m, PACKET_TAG_BRIDGE, mtag)) { -#ifdef DEBUG - /* Check that the information is there */ - if (mtag-m_tag_len != sizeof(caddr_t)) { - error = EINVAL; - goto bad; - } -#endif - if (!memcmp(ifp-if_bridgeport, mtag + 1, - sizeof(caddr_t))) - break; - } - if (mtag == NULL) { - /* Attach a tag so we can detect loops */ - mtag = m_tag_get(PACKET_TAG_BRIDGE, sizeof(caddr_t), - M_NOWAIT); - if (mtag == NULL) { - error = ENOBUFS; - goto bad; - } - memcpy(mtag + 1, ifp-if_bridgeport, sizeof(caddr_t)); - m_tag_prepend(m, mtag); - error = bridge_output(ifp, m, NULL, NULL); - return (error); - } - } -#endif return (if_output(ifp, m)); bad: Index: sys/mbuf.h
Re: pkg_info: print used repos
2015-06-24 11:40 GMT+02:00 Marc Espie es...@nerim.net: On Tue, Jun 23, 2015 at 08:53:15PM -0600, Theo de Raadt wrote: - $state-handle_options('cCdfF:hIKLmPQ:qr:RsSUe:E:Ml:aAt', + $state-handle_options('cCdfF:hIKLmpPQ:qr:RsSUe:E:Ml:aAt', Starting to look a lot like ls. What a coincidence. It's used to list things. So is ls. Maybe Theo was hoping someone would trim ls of all the unused option.
Re: Machine hangs on boot with snapshots 5.8-beta
On Wed, 24 Jun 2015, Pedro Caetano wrote: If case gmail wraps the lines output from dmesg and sendbug is attached Thanks. I believe the issue is that the ACPI APIC table has extra LAPCI/processor entries which are marked as disabled...and which we let overwrite the enabled entry for cpu0. Can you apply this patch, rebuild, and see if things are happier? Philip Guenther Index: acpimadt.c === RCS file: /data/src/openbsd/src/sys/dev/acpi/acpimadt.c,v retrieving revision 1.31 diff -u -p -r1.31 acpimadt.c --- acpimadt.c 9 Feb 2015 08:15:19 - 1.31 +++ acpimadt.c 24 Jun 2015 22:33:04 - @@ -239,13 +239,13 @@ acpimadt_attach(struct device *parent, s entry-madt_lapic.apic_id, entry-madt_lapic.flags); + if ((entry-madt_lapic.flags ACPI_PROC_ENABLE) == 0) + break; + lapic_map[entry-madt_lapic.acpi_proc_id] = entry-madt_lapic.apic_id; acpi_lapic_flags[entry-madt_lapic.acpi_proc_id] = entry-madt_lapic.flags; - - if ((entry-madt_lapic.flags ACPI_PROC_ENABLE) == 0) - break; memset(caa, 0, sizeof(struct cpu_attach_args)); if (lapic_cpu_number() == entry-madt_lapic.apic_id)
Re: macppc IPI counter
Date: Wed, 24 Jun 2015 16:11:08 +0200 From: Martin Pieuchot m...@openbsd.org Use only one ipi counter just like other archs do. ok? Problem is that the event counters aren't really MP safe. By keeping them per-CPU you circumvent any problems with that. Index: dev/openpic.c === RCS file: /cvs/src/sys/arch/macppc/dev/openpic.c,v retrieving revision 1.81 diff -u -p -r1.81 openpic.c --- dev/openpic.c 24 Jun 2015 11:58:06 - 1.81 +++ dev/openpic.c 24 Jun 2015 13:47:34 - @@ -130,11 +130,9 @@ void openpic_ipi_ddb(void); #define IPI_VECTOR_NOP 64 #define IPI_VECTOR_DDB 65 -static struct evcount ipi_ddb[PPC_MAXPROCS]; -static struct evcount ipi_nop[PPC_MAXPROCS]; +static struct evcount ipi_count; -static int ipi_nopirq = IPI_VECTOR_NOP; -static int ipi_ddbirq = IPI_VECTOR_DDB; +static int ipi_irq = IPI_VECTOR_NOP; intr_send_ipi_t openpic_send_ipi; #endif /* MULTIPROCESSOR */ @@ -288,11 +286,7 @@ openpic_attach(struct device *parent, st x |= 15 OPENPIC_PRIORITY_SHIFT; openpic_write(OPENPIC_IPI_VECTOR(1), x); - /* XXX - ncpus */ - evcount_attach(ipi_nop[0], ipi_nop0, ipi_nopirq); - evcount_attach(ipi_nop[1], ipi_nop1, ipi_nopirq); - evcount_attach(ipi_ddb[0], ipi_ddb0, ipi_ddbirq); - evcount_attach(ipi_ddb[1], ipi_ddb1, ipi_ddbirq); + evcount_attach(ipi_count, ipi, ipi_irq); #endif /* clear all pending interrunts */ @@ -638,16 +632,11 @@ openpic_ext_intr(void) return; } #ifdef MULTIPROCESSOR - if (irq == IPI_VECTOR_NOP) { - ipi_nop[ci-ci_cpuid].ec_count++; + if (irq == IPI_VECTOR_NOP || irq == IPI_VECTOR_DDB) { + ipi_count.ec_count++; openpic_eoi(ci-ci_cpuid); - irq = openpic_read_irq(ci-ci_cpuid); - continue; - } - if (irq == IPI_VECTOR_DDB) { - ipi_ddb[ci-ci_cpuid].ec_count++; - openpic_eoi(ci-ci_cpuid); - openpic_ipi_ddb(); + if (irq == IPI_VECTOR_DDB) + openpic_ipi_ddb(); irq = openpic_read_irq(ci-ci_cpuid); continue; } @@ -758,7 +747,6 @@ openpic_send_ipi(struct cpu_info *ci, in void openpic_ipi_ddb(void) { - DPRINTF(ipi_ddb() called\n); #ifdef DDB Debugger(); #endif
Re: macppc IPI counter
On 24/06/15(Wed) 16:45, Mark Kettenis wrote: Date: Wed, 24 Jun 2015 16:11:08 +0200 From: Martin Pieuchot m...@openbsd.org Use only one ipi counter just like other archs do. ok? Problem is that the event counters aren't really MP safe. By keeping them per-CPU you circumvent any problems with that. Do you suggest that I add another 4 counters for my CPUs #2 and #3? Honestly the trashing is much more noticeable on the clock counter which also not protected. But as you know counters are 64bit long so we can't easily use atomic operations like on sparc64. Plus if I'm not mistaken i386 and amd64 are already doing that... If you have a suggestion for a correct fix, I'm interested, but I believe this should be done anyway.
macppc IPI counter
Use only one ipi counter just like other archs do. ok? Index: dev/openpic.c === RCS file: /cvs/src/sys/arch/macppc/dev/openpic.c,v retrieving revision 1.81 diff -u -p -r1.81 openpic.c --- dev/openpic.c 24 Jun 2015 11:58:06 - 1.81 +++ dev/openpic.c 24 Jun 2015 13:47:34 - @@ -130,11 +130,9 @@ void openpic_ipi_ddb(void); #define IPI_VECTOR_NOP 64 #define IPI_VECTOR_DDB 65 -static struct evcount ipi_ddb[PPC_MAXPROCS]; -static struct evcount ipi_nop[PPC_MAXPROCS]; +static struct evcount ipi_count; -static int ipi_nopirq = IPI_VECTOR_NOP; -static int ipi_ddbirq = IPI_VECTOR_DDB; +static int ipi_irq = IPI_VECTOR_NOP; intr_send_ipi_t openpic_send_ipi; #endif /* MULTIPROCESSOR */ @@ -288,11 +286,7 @@ openpic_attach(struct device *parent, st x |= 15 OPENPIC_PRIORITY_SHIFT; openpic_write(OPENPIC_IPI_VECTOR(1), x); - /* XXX - ncpus */ - evcount_attach(ipi_nop[0], ipi_nop0, ipi_nopirq); - evcount_attach(ipi_nop[1], ipi_nop1, ipi_nopirq); - evcount_attach(ipi_ddb[0], ipi_ddb0, ipi_ddbirq); - evcount_attach(ipi_ddb[1], ipi_ddb1, ipi_ddbirq); + evcount_attach(ipi_count, ipi, ipi_irq); #endif /* clear all pending interrunts */ @@ -638,16 +632,11 @@ openpic_ext_intr(void) return; } #ifdef MULTIPROCESSOR - if (irq == IPI_VECTOR_NOP) { - ipi_nop[ci-ci_cpuid].ec_count++; + if (irq == IPI_VECTOR_NOP || irq == IPI_VECTOR_DDB) { + ipi_count.ec_count++; openpic_eoi(ci-ci_cpuid); - irq = openpic_read_irq(ci-ci_cpuid); - continue; - } - if (irq == IPI_VECTOR_DDB) { - ipi_ddb[ci-ci_cpuid].ec_count++; - openpic_eoi(ci-ci_cpuid); - openpic_ipi_ddb(); + if (irq == IPI_VECTOR_DDB) + openpic_ipi_ddb(); irq = openpic_read_irq(ci-ci_cpuid); continue; } @@ -758,7 +747,6 @@ openpic_send_ipi(struct cpu_info *ci, in void openpic_ipi_ddb(void) { - DPRINTF(ipi_ddb() called\n); #ifdef DDB Debugger(); #endif