Re: [ping] dump -U by default
On Tue, May 12, 2015 at 2:00 AM, Manuel Giraud wrote:
> Philip Guenther writes:
>
>> Can we suppress the device form if there's a matching DUID entry?
>
> Okay. The DUID/device dance is not that easy (at least for me). So here
> is a new patch that should work.
>
> For your issue, I choose to convert dumpdates entries to DUID (when
> possible) at read time so dump now has the following features/drawbacks:
>
> - All dumpdates entries (of present devices) will be converted
> to DUID at the next dump (even those that are not being dumped).
> - Even a dump -w/W tries to opendev the device (in order to find
> its UID).
Semantics were good, thanks! There were a few resource leaks I took
care of before committing:
> +char *
> +getduid(char *path)
> +{
> + int fd;
> + struct disklabel lab;
> + u_int64_t zero_uid = 0;
> + char *duid;
> +
> + if ((fd = opendev(path, O_RDONLY | O_NOFOLLOW, 0, NULL)) >= 0) {
> + if (ioctl(fd, DIOCGDINFO, (char *)&lab) < 0) {
> + close(fd);
> + warn("ioctl(DIOCGDINFO)");
> + return (NULL);
> + }
> +
> + if (memcmp(lab.d_uid, &zero_uid, sizeof(lab.d_uid)) != 0) {
> + if (asprintf(&duid,
> +
> "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx.%c",
> +lab.d_uid[0], lab.d_uid[1], lab.d_uid[2],
> +lab.d_uid[3], lab.d_uid[4], lab.d_uid[5],
> +lab.d_uid[6], lab.d_uid[7],
> +path[strlen(path)-1]) == -1) {
> + close(fd);
> + warn("Cannot malloc duid");
> + return (NULL);
> + }
> + return (duid);
> + }
> + }
> +
> + return (NULL);
> }
This failed to close fd in the success path; I moved the second
close() up to before the memcmp().
> @@ -347,6 +347,13 @@ fstabsearch(char *key)
> rn = rawname(fs->fs_spec);
> if (rn != NULL && strcmp(rn, key) == 0)
> return (fs);
> + if (rn != NULL) {
> + uid = getduid(rn);
> + } else {
> + uid = getduid(fs->fs_spec);
> + }
> + if (uid != NULL && strcmp(uid, key) == 0)
> + return (fs);
uid is malloced by getduid(), so it needed to be freed here.
Nothing big, thanks for doing the hard part!
Philip Guenther
Re: Fix for connect race in relayd
This breaks relayd with TLS inspection. Moving down the "called
once" check after the F_TLSINSPECT block fixes the plain SSL case.
But HTTPS still hangs. I have just commited a test. Try
cd /usr/src/regress/usr.sbin/relayd && make run-regress-args-https-inspect.pl
bluhm
On Fri, May 22, 2015 at 03:55:16PM +0200, Claudio Jeker wrote:
> On our production systems we did hit the "relay_connect: no connection in
> flight" on a so regular bases that I had to make it non-fatal with the
> result of leaking sockets.
>
> After more investigation I found the problem to be a race against
> connecting to the backend servers. In short:
> - relay_read_http() will open a connection if following conditions are met
> cre->dir == RELAY_DIR_REQUEST && cre->toread <= 0 && cre->dst->bev == NULL
>
> - relay_connect() does not initialize con->se_out.bev (which is also
> cre->dst->bev). Instead this is deferred to relay_connected()
>
> - if a event happens that calls relay_read_http() while connecting to the
> backend then relay_connect() will be called again. Result is the panic
> since the count gets out of sync.
>
> The following diff solves this issue by adding an extra flag to
> ctl_relay_event to know if a relay is already connected (or the connect is
> pending). relay_close() will then clean the flag when closing the session.
> I decided to use a flag since the EMFILE || ENFILE case is hard to detect
> otherwise.
>
> Running with this on production with no visible issues at the moment.
> I think it would make sense to restructure the http proxy code more and
> introduce a proper state machine but that is a much bigger and complex
> issue, so lets fix the bug first.
>
> OK?
> --
> :wq Claudio
>
> Index: relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.194
> diff -u -p -r1.194 relay.c
> --- relay.c 18 May 2015 16:57:20 - 1.194
> +++ relay.c 21 May 2015 08:15:05 -
> @@ -1496,6 +1498,12 @@ relay_connect(struct rsession *con)
> struct timeval evtpause = { 1, 0 };
> int bnds = -1, ret;
>
> + /* relay_connect should only be called once per relay */
> + if (con->se_out.connected) {
> + log_debug("%s: connect already called once", __func__);
> + return (0);
> + }
> +
> /* Connection is already established but session not active */
> if ((rlay->rl_conf.flags & F_TLSINSPECT) && con->se_out.s != -1) {
> if (con->se_out.ssl == NULL) {
> @@ -1555,6 +1565,8 @@ relay_connect(struct rsession *con)
> event_del(&rlay->rl_ev);
> evtimer_add(&con->se_inflightevt, &evtpause);
> evtimer_add(&rlay->rl_evt, &evtpause);
> + /* this connect is pending */
> + con->se_out.connected = 1;
> return (0);
> } else {
> if (con->se_retry) {
> @@ -1572,6 +1584,7 @@ relay_connect(struct rsession *con)
> }
> }
>
> + con->se_out.connected = 1;
> relay_inflight--;
> DPRINTF("%s: inflight decremented, now %d",__func__,
> relay_inflight);
> @@ -1673,6 +1686,7 @@ relay_close(struct rsession *con, const
> event_add(&rlay->rl_ev, NULL);
> }
> }
> + con->se_out.connected = 0;
>
> if (con->se_out.buf != NULL)
> free(con->se_out.buf);
> Index: relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 relay_http.c
> --- relay_http.c 22 May 2015 01:34:13 - 1.47
> +++ relay_http.c 22 May 2015 13:40:38 -
> @@ -419,7 +419,7 @@ relay_read_http(struct bufferevent *bev,
> relay_reset_http(cre);
> done:
> if (cre->dir == RELAY_DIR_REQUEST && cre->toread <= 0 &&
> - cre->dst->bev == NULL) {
> + !cre->dst->connected) {
> if (rlay->rl_conf.fwdmode == FWD_TRANS) {
> relay_bindanyreq(con, 0, IPPROTO_TCP);
> return;
> Index: relayd.h
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
> retrieving revision 1.209
> diff -u -p -r1.209 relayd.h
> --- relayd.h 2 May 2015 13:15:24 - 1.209
> +++ relayd.h 21 May 2015 06:55:44 -
> @@ -200,6 +200,7 @@ struct ctl_relay_event {
> int line;
> int done;
> int timedout;
> + int connected;
> enum direction dir;
>
> u_int8_t*buf;
Re: pf.conf from/to negation homogeneous behavior
On Fri, May 22, 2015 at 1:13 PM, Henning Brauer
wrote:
> * sven falempin [2015-05-22 16:33]:
> > But it does not explain the output i have.
>
> otoh I'd say your diff is incomplete and misses a bit in expand_rule.
>
>
Ok i get it now, log is not like pass, the is compute
through the ruleset and use,
at the end so the ! was clearing the mistake.
So no ! { } for the moment, as it is an miss-leading expansion, even doing
it like this :
Index: parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.648
diff -u -p -r1.648 parse.y
--- parse.y 21 Apr 2015 16:34:59 - 1.648
+++ parse.y 22 May 2015 18:56:35 -
@@ -2563,7 +2563,12 @@ optnl: '\n' optnl
ipspec : ANY { $$ = NULL; }
| xhost { $$ = $1; }
- | '{' optnl host_list '}' { $$ = $3; }
+ | not '{' optnl host_list '}' {
+ struct node_host*n;
+ for (n = $4; n != NULL; n = n->next)
+ n->not = $1;
+ $$ = $4;
+ }
;
Creating a anonymous table would be a bad idea because it would be hard to
flush during a reload ?
--
-
() ascii ribbon campaign - against html e-mail
/\
Re: vmxnet3 panic
Not sure if I’ll be able to reproduce this at all.
Never seen this before.
But diff is applied.
//mxb
> On 22 maj 2015, at 19:53, Mike Belopuhov wrote:
>
> On Fri, May 22, 2015 at 19:35 +0200, mxb wrote:
>>
>> Hey,
>> got a panic as of todays ‘cvs up’
>> trace below
>>
>> panic: vmxnet3_rxintr: NULL ring->m[44]
>> Stopped at Debugger+0x9: leave
>> RUN AT LEAST 'trace' AND 'ps' AND INCLUDE OUTPUT WHEN REPORTING THIS PANIC!
>> IF RUNNING SMP, USE 'mach ddbcpu <#>' AND 'trace' ON OTHER PROCESSORS, TOO.
>> DO NOT EVEN BOTHER REPORTING THIS WITHOUT INCLUDING THAT INFORMATION!
>> ddb{0}> Debugger() at Debugger+0x9
>> panic() at panic+0xfe
>> vmxnet3_rxintr() at vmxnet3_rxintr+0x284
>> vmxnet3_intr() at vmxnet3_intr+0x4a
>> intr_handler() at intr_handler+0x67
>> Xintr_ioapic_level10() at Xintr_ioapic_level10+0xcd
>> --- interrupt ---
>> Xspllower() at Xspllower+0xe
>> if_downall() at if_downall+0x9b
>> boot() at boot+0xe4
>> reboot() at reboot+0x26
>> sys_reboot() at sys_reboot+0x5e
>> syscall() at syscall+0x297
>> --- syscall (number 55) ---
>> end of kernel
>> end trace frame: 0x7f7d5858, count: -12
>> 0x184f48704eda:
>> ddb{0}> rebooting...
>> OpenBSD 5.7-current (GENERIC.MP) #0: Fri May 22 16:30:54 CEST 2015
>>
>> //mxb
>>
>
> vmx doesn't check if it's wasn't stopped before calling rx/tx
> interrupt routines... the patch below should fix it up. not
> entirely sure why do they need to re-enable the interrupt
> every time, but i'm pretty positive you don't want that if
> you're !IFF_RUNNING.
>
> diff --git sys/dev/pci/if_vmx.c sys/dev/pci/if_vmx.c
> index 925a977..989cae1 100644
> --- sys/dev/pci/if_vmx.c
> +++ sys/dev/pci/if_vmx.c
> @@ -588,21 +588,24 @@ vmxnet3_disable_all_intrs(struct vmxnet3_softc *sc)
>
> int
> vmxnet3_intr(void *arg)
> {
> struct vmxnet3_softc *sc = arg;
> + struct ifnet *ifp = &sc->sc_arpcom.ac_if;
>
> if (READ_BAR1(sc, VMXNET3_BAR1_INTR) == 0)
> return 0;
> if (sc->sc_ds->event)
> vmxnet3_evintr(sc);
> - vmxnet3_rxintr(sc, &sc->sc_rxq[0]);
> - vmxnet3_txintr(sc, &sc->sc_txq[0]);
> #ifdef VMXNET3_STAT
> vmxstat.intr++;
> #endif
> - vmxnet3_enable_intr(sc, 0);
> + if (ifp->if_flags & IFF_RUNNING) {
> + vmxnet3_rxintr(sc, &sc->sc_rxq[0]);
> + vmxnet3_txintr(sc, &sc->sc_txq[0]);
> + vmxnet3_enable_intr(sc, 0);
> + }
> return 1;
> }
>
> void
> vmxnet3_evintr(struct vmxnet3_softc *sc)
Re: vmxnet3 panic
On Fri, May 22, 2015 at 19:35 +0200, mxb wrote:
>
> Hey,
> got a panic as of todays ‘cvs up’
> trace below
>
> panic: vmxnet3_rxintr: NULL ring->m[44]
> Stopped at Debugger+0x9: leave
> RUN AT LEAST 'trace' AND 'ps' AND INCLUDE OUTPUT WHEN REPORTING THIS PANIC!
> IF RUNNING SMP, USE 'mach ddbcpu <#>' AND 'trace' ON OTHER PROCESSORS, TOO.
> DO NOT EVEN BOTHER REPORTING THIS WITHOUT INCLUDING THAT INFORMATION!
> ddb{0}> Debugger() at Debugger+0x9
> panic() at panic+0xfe
> vmxnet3_rxintr() at vmxnet3_rxintr+0x284
> vmxnet3_intr() at vmxnet3_intr+0x4a
> intr_handler() at intr_handler+0x67
> Xintr_ioapic_level10() at Xintr_ioapic_level10+0xcd
> --- interrupt ---
> Xspllower() at Xspllower+0xe
> if_downall() at if_downall+0x9b
> boot() at boot+0xe4
> reboot() at reboot+0x26
> sys_reboot() at sys_reboot+0x5e
> syscall() at syscall+0x297
> --- syscall (number 55) ---
> end of kernel
> end trace frame: 0x7f7d5858, count: -12
> 0x184f48704eda:
> ddb{0}> rebooting...
> OpenBSD 5.7-current (GENERIC.MP) #0: Fri May 22 16:30:54 CEST 2015
>
> //mxb
>
vmx doesn't check if it's wasn't stopped before calling rx/tx
interrupt routines... the patch below should fix it up. not
entirely sure why do they need to re-enable the interrupt
every time, but i'm pretty positive you don't want that if
you're !IFF_RUNNING.
diff --git sys/dev/pci/if_vmx.c sys/dev/pci/if_vmx.c
index 925a977..989cae1 100644
--- sys/dev/pci/if_vmx.c
+++ sys/dev/pci/if_vmx.c
@@ -588,21 +588,24 @@ vmxnet3_disable_all_intrs(struct vmxnet3_softc *sc)
int
vmxnet3_intr(void *arg)
{
struct vmxnet3_softc *sc = arg;
+ struct ifnet *ifp = &sc->sc_arpcom.ac_if;
if (READ_BAR1(sc, VMXNET3_BAR1_INTR) == 0)
return 0;
if (sc->sc_ds->event)
vmxnet3_evintr(sc);
- vmxnet3_rxintr(sc, &sc->sc_rxq[0]);
- vmxnet3_txintr(sc, &sc->sc_txq[0]);
#ifdef VMXNET3_STAT
vmxstat.intr++;
#endif
- vmxnet3_enable_intr(sc, 0);
+ if (ifp->if_flags & IFF_RUNNING) {
+ vmxnet3_rxintr(sc, &sc->sc_rxq[0]);
+ vmxnet3_txintr(sc, &sc->sc_txq[0]);
+ vmxnet3_enable_intr(sc, 0);
+ }
return 1;
}
void
vmxnet3_evintr(struct vmxnet3_softc *sc)
vmxnet3 panic
Hey,
got a panic as of todays ‘cvs up’
trace below
panic: vmxnet3_rxintr: NULL ring->m[44]
Stopped at Debugger+0x9: leave
RUN AT LEAST 'trace' AND 'ps' AND INCLUDE OUTPUT WHEN REPORTING THIS PANIC!
IF RUNNING SMP, USE 'mach ddbcpu <#>' AND 'trace' ON OTHER PROCESSORS, TOO.
DO NOT EVEN BOTHER REPORTING THIS WITHOUT INCLUDING THAT INFORMATION!
ddb{0}> Debugger() at Debugger+0x9
panic() at panic+0xfe
vmxnet3_rxintr() at vmxnet3_rxintr+0x284
vmxnet3_intr() at vmxnet3_intr+0x4a
intr_handler() at intr_handler+0x67
Xintr_ioapic_level10() at Xintr_ioapic_level10+0xcd
--- interrupt ---
Xspllower() at Xspllower+0xe
if_downall() at if_downall+0x9b
boot() at boot+0xe4
reboot() at reboot+0x26
sys_reboot() at sys_reboot+0x5e
syscall() at syscall+0x297
--- syscall (number 55) ---
end of kernel
end trace frame: 0x7f7d5858, count: -12
0x184f48704eda:
ddb{0}> rebooting...
OpenBSD 5.7-current (GENERIC.MP) #0: Fri May 22 16:30:54 CEST 2015
//mxb
Re: pf.conf from/to negation homogeneous behavior
* sven falempin [2015-05-22 16:33]: > But it does not explain the output i have. otoh I'd say your diff is incomplete and misses a bit in expand_rule. -- 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/
Re: mismatch for ICMP state created by inound response
On Thu, May 21, 2015 at 21:08 +0200, Alexandr Nedvedicky wrote: > Hello, > > > > > Well, not entirely (: I did it while exploring the code and sent > > out to provoke further discussion. Today I've talked to reyk@ and > > we think that it's better to go down a different road: make sure we > > don't create states on reply packets in the first place. > > > that's actually very wise approach as replies can be spoofed... > > > I've tested this with ICMP, ICMPv6 and NAT64 (slightly). Any OKs? > > Objections? > > I have no objections, just a small wish, can you set icmp_dir to -1, > if we are not dealing with ICMP? there is a tool we use in Solaris, > which yells on us because of uninitialized variable. I know it's > false positive, but I've gave up on explaining... > > patch below combines your fix with my 'wish'. > if you don't mind, i'd rather set it to 0 (PF_IN is 1) right where it's declared than add an additional case, like so: diff --git sys/net/pf.c sys/net/pf.c index 39d5cb6..5d44f43 100644 --- sys/net/pf.c +++ sys/net/pf.c @@ -3075,11 +3075,11 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm, u_short reason; int rewrite = 0; int tag = -1; int asd = 0; int match = 0; - int state_icmp = 0, icmp_dir; + int state_icmp = 0, icmp_dir = 0; u_int16_tvirtual_type, virtual_id; u_int8_t icmptype = 0, icmpcode = 0; bzero(&act, sizeof(act)); bzero(sns, sizeof(sns)); @@ -3201,10 +3201,15 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm, PF_TEST_ATTRIB((r->type && r->type != icmptype + 1), TAILQ_NEXT(r, entries)); /* icmp only. type always 0 in other cases */ PF_TEST_ATTRIB((r->code && r->code != icmpcode + 1), TAILQ_NEXT(r, entries)); + /* icmp only. don't create states on replies */ + PF_TEST_ATTRIB((r->keep_state && !state_icmp && + (r->rule_flag & PFRULE_STATESLOPPY) == 0 && + icmp_dir != PF_IN), + TAILQ_NEXT(r, entries)); break; default: break; }
Re: pf.conf from/to negation homogeneous behavior
On Fri, May 22, 2015 at 10:15 AM, Henning Brauer wrote: > * sven falempin [2015-05-22 14:18]: > > looking the rule actually show and unexpected result : > > > match log on vic0 inet proto icmp from any to ! 8.8.8.8 > > match log on vic0 inet proto icmp from any to 8.8.4.4 > > so it's even worse, you lose the negation on expansion for subsequent > rules. > > > This result are really puzzling for me, > > when i first test the table negation i was really glad that list negation > > was possible, > > the (block) alternative is often ridiculous to write. > > so use a table - since lists are expanded at load time, negation there > just can't work that way. > > I certainly could do that, and I understand this table behavior while looking for list negation. But it does not explain the output i have. -- - () ascii ribbon campaign - against html e-mail /\
Re: pf.conf from/to negation homogeneous behavior
* sven falempin [2015-05-22 14:18]: > looking the rule actually show and unexpected result : > match log on vic0 inet proto icmp from any to ! 8.8.8.8 > match log on vic0 inet proto icmp from any to 8.8.4.4 so it's even worse, you lose the negation on expansion for subsequent rules. > This result are really puzzling for me, > when i first test the table negation i was really glad that list negation > was possible, > the (block) alternative is often ridiculous to write. so use a table - since lists are expanded at load time, negation there just can't work that way. -- 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/
carp(4) is out
Let's take carp(4) out of ether_input(). This is quite similar to what
happened to trunk(4) and vlan(4).
I appreciate tests of any kind, reviews and oks.
Index: net/if_ethersubr.c
===
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.199
diff -u -p -r1.199 if_ethersubr.c
--- net/if_ethersubr.c 19 May 2015 11:09:24 - 1.199
+++ net/if_ethersubr.c 22 May 2015 13:39:44 -
@@ -502,18 +502,6 @@ ether_input(struct mbuf *m, void *hdr)
}
#endif
-#if NCARP > 0
- if (ifp->if_carp) {
- if (ifp->if_type != IFT_CARP && (carp_input(ifp, eh, m) == 0))
- return (1);
- /* clear mcast if received on a carp IP balanced address */
- else if (ifp->if_type == IFT_CARP &&
- m->m_flags & (M_BCAST|M_MCAST) &&
- carp_our_mcastaddr(ifp, (u_int8_t *)&eh->ether_dhost))
- m->m_flags &= ~(M_BCAST|M_MCAST);
- }
-#endif /* NCARP > 0 */
-
ac = (struct arpcom *)ifp;
/*
Index: netinet/ip_carp.c
===
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.257
diff -u -p -r1.257 ip_carp.c
--- netinet/ip_carp.c 21 May 2015 09:17:53 - 1.257
+++ netinet/ip_carp.c 22 May 2015 13:54:30 -
@@ -120,6 +120,7 @@ struct carp_softc {
#definesc_carpdev sc_ac.ac_if.if_carpdev
void *ah_cookie;
void *lh_cookie;
+ struct ifih *sc_ifih;
struct ip_moptions sc_imo;
#ifdef INET6
struct ip6_moptions sc_im6o;
@@ -193,6 +194,7 @@ voidcarp_hmac_generate(struct carp_vhos
unsigned char *, u_int8_t);
intcarp_hmac_verify(struct carp_vhost_entry *, u_int32_t *,
unsigned char *);
+intcarp_input(struct mbuf *, void *);
void carp_proto_input_c(struct mbuf *, struct carp_header *, int,
sa_family_t);
void carpattach(int);
@@ -824,6 +826,7 @@ carp_del_all_timeouts(struct carp_softc
void
carpdetach(struct carp_softc *sc)
{
+ struct ifnet *ifp;
struct carp_if *cif;
int s;
@@ -839,20 +842,29 @@ carpdetach(struct carp_softc *sc)
carp_setrun_all(sc, 0);
carp_multicast_cleanup(sc);
- s = splnet();
if (sc->ah_cookie != NULL)
hook_disestablish(sc->sc_if.if_addrhooks, sc->ah_cookie);
- if (sc->sc_carpdev != NULL) {
- if (sc->lh_cookie != NULL)
- hook_disestablish(sc->sc_carpdev->if_linkstatehooks,
- sc->lh_cookie);
- cif = (struct carp_if *)sc->sc_carpdev->if_carp;
- TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
- if (!--cif->vhif_nvrs) {
- ifpromisc(sc->sc_carpdev, 0);
- sc->sc_carpdev->if_carp = NULL;
- free(cif, M_IFADDR, sizeof(*cif));
- }
+
+ ifp = sc->sc_carpdev;
+ if (ifp == NULL)
+ return;
+
+ s = splnet();
+ /* Restore previous input handler. */
+ if (--sc->sc_ifih->ifih_refcnt == 0) {
+ SLIST_REMOVE(&ifp->if_inputs, sc->sc_ifih, ifih, ifih_next);
+ free(sc->sc_ifih, M_DEVBUF, sizeof(*sc->sc_ifih));
+ }
+
+ if (sc->lh_cookie != NULL)
+ hook_disestablish(ifp->if_linkstatehooks,
+ sc->lh_cookie);
+ cif = (struct carp_if *)ifp->if_carp;
+ TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
+ if (!--cif->vhif_nvrs) {
+ ifpromisc(ifp, 0);
+ ifp->if_carp = NULL;
+ free(cif, M_IFADDR, sizeof(*cif));
}
sc->sc_carpdev = NULL;
splx(s);
@@ -1403,27 +1415,21 @@ carp_get_srclladdr(struct ifnet *ifp, u_
}
int
-carp_our_mcastaddr(struct ifnet *ifp, u_int8_t *d_enaddr)
-{
- struct carp_softc *sc = ifp->if_softc;
-
- if (sc->sc_balancing != CARP_BAL_IP)
- return (0);
-
- return (!memcmp(sc->sc_ac.ac_enaddr, d_enaddr, ETHER_ADDR_LEN));
-}
-
-
-int
-carp_input(struct ifnet *ifp0, struct ether_header *eh0, struct mbuf *m)
+carp_input(struct mbuf *m, void *hdr)
{
+ struct carp_softc *sc;
struct ether_header *eh;
- struct carp_if *cif = (struct carp_if *)ifp0->if_carp;
- struct ifnet *ifp;
+ struct mbuf_list ml = MBUF_LIST_INITIALIZER();
+ struct carp_if *cif;
+ struct ifnet *ifp0, *ifp;
- ifp = carp_ourether(cif, eh0->ether_dhost);
- if (ifp == NULL && (m->m_flags & (M_BCAST|M_MCAST)) == 0)
- return (1);
+ ifp0 = m->m_pkthdr.rcvif;
+ eh = mtod(m, struct ether_header *);
+ cif = (struct carp_if *)ifp0->if_carp;
+
+ ifp = carp_ourether(cif, eh->ether_dhost);
+ if (ifp == NULL && !ETHER_IS_MULTICAST(eh->ether_dhost))
+ return (0);
if (ifp == NUL
Re: tcpdump'ing on vether(4)
On 22/05/15(Fri) 15:23, Martin Pieuchot wrote:
> I find it really useful to be able see output packets when tcpdump'ing
> a vether(4) interface.
>
> ok?
Please use this diff instead, the previous one relies on a non-committed
diff that changes carp_input(mbuf *, void *) -> carp_input(mbuf *).
Index: net/if_ethersubr.c
===
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.199
diff -u -p -r1.199 if_ethersubr.c
--- net/if_ethersubr.c 19 May 2015 11:09:24 - 1.199
+++ net/if_ethersubr.c 22 May 2015 13:39:44 -
@@ -502,18 +502,6 @@ ether_input(struct mbuf *m, void *hdr)
}
#endif
-#if NCARP > 0
- if (ifp->if_carp) {
- if (ifp->if_type != IFT_CARP && (carp_input(ifp, eh, m) == 0))
- return (1);
- /* clear mcast if received on a carp IP balanced address */
- else if (ifp->if_type == IFT_CARP &&
- m->m_flags & (M_BCAST|M_MCAST) &&
- carp_our_mcastaddr(ifp, (u_int8_t *)&eh->ether_dhost))
- m->m_flags &= ~(M_BCAST|M_MCAST);
- }
-#endif /* NCARP > 0 */
-
ac = (struct arpcom *)ifp;
/*
Index: netinet/ip_carp.c
===
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.257
diff -u -p -r1.257 ip_carp.c
--- netinet/ip_carp.c 21 May 2015 09:17:53 - 1.257
+++ netinet/ip_carp.c 22 May 2015 13:54:30 -
@@ -120,6 +120,7 @@ struct carp_softc {
#definesc_carpdev sc_ac.ac_if.if_carpdev
void *ah_cookie;
void *lh_cookie;
+ struct ifih *sc_ifih;
struct ip_moptions sc_imo;
#ifdef INET6
struct ip6_moptions sc_im6o;
@@ -193,6 +194,7 @@ voidcarp_hmac_generate(struct carp_vhos
unsigned char *, u_int8_t);
intcarp_hmac_verify(struct carp_vhost_entry *, u_int32_t *,
unsigned char *);
+intcarp_input(struct mbuf *, void *);
void carp_proto_input_c(struct mbuf *, struct carp_header *, int,
sa_family_t);
void carpattach(int);
@@ -824,6 +826,7 @@ carp_del_all_timeouts(struct carp_softc
void
carpdetach(struct carp_softc *sc)
{
+ struct ifnet *ifp;
struct carp_if *cif;
int s;
@@ -839,20 +842,29 @@ carpdetach(struct carp_softc *sc)
carp_setrun_all(sc, 0);
carp_multicast_cleanup(sc);
- s = splnet();
if (sc->ah_cookie != NULL)
hook_disestablish(sc->sc_if.if_addrhooks, sc->ah_cookie);
- if (sc->sc_carpdev != NULL) {
- if (sc->lh_cookie != NULL)
- hook_disestablish(sc->sc_carpdev->if_linkstatehooks,
- sc->lh_cookie);
- cif = (struct carp_if *)sc->sc_carpdev->if_carp;
- TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
- if (!--cif->vhif_nvrs) {
- ifpromisc(sc->sc_carpdev, 0);
- sc->sc_carpdev->if_carp = NULL;
- free(cif, M_IFADDR, sizeof(*cif));
- }
+
+ ifp = sc->sc_carpdev;
+ if (ifp == NULL)
+ return;
+
+ s = splnet();
+ /* Restore previous input handler. */
+ if (--sc->sc_ifih->ifih_refcnt == 0) {
+ SLIST_REMOVE(&ifp->if_inputs, sc->sc_ifih, ifih, ifih_next);
+ free(sc->sc_ifih, M_DEVBUF, sizeof(*sc->sc_ifih));
+ }
+
+ if (sc->lh_cookie != NULL)
+ hook_disestablish(ifp->if_linkstatehooks,
+ sc->lh_cookie);
+ cif = (struct carp_if *)ifp->if_carp;
+ TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
+ if (!--cif->vhif_nvrs) {
+ ifpromisc(ifp, 0);
+ ifp->if_carp = NULL;
+ free(cif, M_IFADDR, sizeof(*cif));
}
sc->sc_carpdev = NULL;
splx(s);
@@ -1403,27 +1415,21 @@ carp_get_srclladdr(struct ifnet *ifp, u_
}
int
-carp_our_mcastaddr(struct ifnet *ifp, u_int8_t *d_enaddr)
-{
- struct carp_softc *sc = ifp->if_softc;
-
- if (sc->sc_balancing != CARP_BAL_IP)
- return (0);
-
- return (!memcmp(sc->sc_ac.ac_enaddr, d_enaddr, ETHER_ADDR_LEN));
-}
-
-
-int
-carp_input(struct ifnet *ifp0, struct ether_header *eh0, struct mbuf *m)
+carp_input(struct mbuf *m, void *hdr)
{
+ struct carp_softc *sc;
struct ether_header *eh;
- struct carp_if *cif = (struct carp_if *)ifp0->if_carp;
- struct ifnet *ifp;
+ struct mbuf_list ml = MBUF_LIST_INITIALIZER();
+ struct carp_if *cif;
+ struct ifnet *ifp0, *ifp;
- ifp = carp_ourether(cif, eh0->ether_dhost);
- if (ifp == NULL && (m->m_flags & (M_BCAST|M_MCAST)) == 0)
- return (1);
+ ifp0 = m->m_pkthdr.rcvif;
+ eh = mtod(m, struct ether_header *);
+ cif = (struct carp_if *)ifp0->if_carp;
+
+ ifp = carp_ourether(c
Fix for connect race in relayd
On our production systems we did hit the "relay_connect: no connection in
flight" on a so regular bases that I had to make it non-fatal with the
result of leaking sockets.
After more investigation I found the problem to be a race against
connecting to the backend servers. In short:
- relay_read_http() will open a connection if following conditions are met
cre->dir == RELAY_DIR_REQUEST && cre->toread <= 0 && cre->dst->bev == NULL
- relay_connect() does not initialize con->se_out.bev (which is also
cre->dst->bev). Instead this is deferred to relay_connected()
- if a event happens that calls relay_read_http() while connecting to the
backend then relay_connect() will be called again. Result is the panic
since the count gets out of sync.
The following diff solves this issue by adding an extra flag to
ctl_relay_event to know if a relay is already connected (or the connect is
pending). relay_close() will then clean the flag when closing the session.
I decided to use a flag since the EMFILE || ENFILE case is hard to detect
otherwise.
Running with this on production with no visible issues at the moment.
I think it would make sense to restructure the http proxy code more and
introduce a proper state machine but that is a much bigger and complex
issue, so lets fix the bug first.
OK?
--
:wq Claudio
Index: relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.194
diff -u -p -r1.194 relay.c
--- relay.c 18 May 2015 16:57:20 - 1.194
+++ relay.c 21 May 2015 08:15:05 -
@@ -1496,6 +1498,12 @@ relay_connect(struct rsession *con)
struct timeval evtpause = { 1, 0 };
int bnds = -1, ret;
+ /* relay_connect should only be called once per relay */
+ if (con->se_out.connected) {
+ log_debug("%s: connect already called once", __func__);
+ return (0);
+ }
+
/* Connection is already established but session not active */
if ((rlay->rl_conf.flags & F_TLSINSPECT) && con->se_out.s != -1) {
if (con->se_out.ssl == NULL) {
@@ -1555,6 +1565,8 @@ relay_connect(struct rsession *con)
event_del(&rlay->rl_ev);
evtimer_add(&con->se_inflightevt, &evtpause);
evtimer_add(&rlay->rl_evt, &evtpause);
+ /* this connect is pending */
+ con->se_out.connected = 1;
return (0);
} else {
if (con->se_retry) {
@@ -1572,6 +1584,7 @@ relay_connect(struct rsession *con)
}
}
+ con->se_out.connected = 1;
relay_inflight--;
DPRINTF("%s: inflight decremented, now %d",__func__,
relay_inflight);
@@ -1673,6 +1686,7 @@ relay_close(struct rsession *con, const
event_add(&rlay->rl_ev, NULL);
}
}
+ con->se_out.connected = 0;
if (con->se_out.buf != NULL)
free(con->se_out.buf);
Index: relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.47
diff -u -p -r1.47 relay_http.c
--- relay_http.c22 May 2015 01:34:13 - 1.47
+++ relay_http.c22 May 2015 13:40:38 -
@@ -419,7 +419,7 @@ relay_read_http(struct bufferevent *bev,
relay_reset_http(cre);
done:
if (cre->dir == RELAY_DIR_REQUEST && cre->toread <= 0 &&
- cre->dst->bev == NULL) {
+ !cre->dst->connected) {
if (rlay->rl_conf.fwdmode == FWD_TRANS) {
relay_bindanyreq(con, 0, IPPROTO_TCP);
return;
Index: relayd.h
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.209
diff -u -p -r1.209 relayd.h
--- relayd.h2 May 2015 13:15:24 - 1.209
+++ relayd.h21 May 2015 06:55:44 -
@@ -200,6 +200,7 @@ struct ctl_relay_event {
int line;
int done;
int timedout;
+ int connected;
enum direction dir;
u_int8_t*buf;
carp(4) is out
Let's take carp(4) out of ether_input(). This is quite similar to what
happened to trunk(4) and vlan(4).
I appreciate tests of any kind, reviews and oks.
Index: net/if_ethersubr.c
===
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.199
diff -u -p -r1.199 if_ethersubr.c
--- net/if_ethersubr.c 19 May 2015 11:09:24 - 1.199
+++ net/if_ethersubr.c 22 May 2015 13:39:44 -
@@ -502,18 +502,6 @@ ether_input(struct mbuf *m, void *hdr)
}
#endif
-#if NCARP > 0
- if (ifp->if_carp) {
- if (ifp->if_type != IFT_CARP && (carp_input(ifp, eh, m) == 0))
- return (1);
- /* clear mcast if received on a carp IP balanced address */
- else if (ifp->if_type == IFT_CARP &&
- m->m_flags & (M_BCAST|M_MCAST) &&
- carp_our_mcastaddr(ifp, (u_int8_t *)&eh->ether_dhost))
- m->m_flags &= ~(M_BCAST|M_MCAST);
- }
-#endif /* NCARP > 0 */
-
ac = (struct arpcom *)ifp;
/*
Index: netinet/ip_carp.c
===
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.257
diff -u -p -r1.257 ip_carp.c
--- netinet/ip_carp.c 21 May 2015 09:17:53 - 1.257
+++ netinet/ip_carp.c 22 May 2015 13:39:44 -
@@ -120,6 +120,7 @@ struct carp_softc {
#definesc_carpdev sc_ac.ac_if.if_carpdev
void *ah_cookie;
void *lh_cookie;
+ struct ifih *sc_ifih;
struct ip_moptions sc_imo;
#ifdef INET6
struct ip6_moptions sc_im6o;
@@ -193,6 +194,7 @@ voidcarp_hmac_generate(struct carp_vhos
unsigned char *, u_int8_t);
intcarp_hmac_verify(struct carp_vhost_entry *, u_int32_t *,
unsigned char *);
+intcarp_input(struct mbuf *);
void carp_proto_input_c(struct mbuf *, struct carp_header *, int,
sa_family_t);
void carpattach(int);
@@ -824,6 +826,7 @@ carp_del_all_timeouts(struct carp_softc
void
carpdetach(struct carp_softc *sc)
{
+ struct ifnet *ifp;
struct carp_if *cif;
int s;
@@ -839,20 +842,29 @@ carpdetach(struct carp_softc *sc)
carp_setrun_all(sc, 0);
carp_multicast_cleanup(sc);
- s = splnet();
if (sc->ah_cookie != NULL)
hook_disestablish(sc->sc_if.if_addrhooks, sc->ah_cookie);
- if (sc->sc_carpdev != NULL) {
- if (sc->lh_cookie != NULL)
- hook_disestablish(sc->sc_carpdev->if_linkstatehooks,
- sc->lh_cookie);
- cif = (struct carp_if *)sc->sc_carpdev->if_carp;
- TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
- if (!--cif->vhif_nvrs) {
- ifpromisc(sc->sc_carpdev, 0);
- sc->sc_carpdev->if_carp = NULL;
- free(cif, M_IFADDR, sizeof(*cif));
- }
+
+ ifp = sc->sc_carpdev;
+ if (ifp == NULL)
+ return;
+
+ s = splnet();
+ /* Restore previous input handler. */
+ if (--sc->sc_ifih->ifih_refcnt == 0) {
+ SLIST_REMOVE(&ifp->if_inputs, sc->sc_ifih, ifih, ifih_next);
+ free(sc->sc_ifih, M_DEVBUF, sizeof(*sc->sc_ifih));
+ }
+
+ if (sc->lh_cookie != NULL)
+ hook_disestablish(ifp->if_linkstatehooks,
+ sc->lh_cookie);
+ cif = (struct carp_if *)ifp->if_carp;
+ TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
+ if (!--cif->vhif_nvrs) {
+ ifpromisc(ifp, 0);
+ ifp->if_carp = NULL;
+ free(cif, M_IFADDR, sizeof(*cif));
}
sc->sc_carpdev = NULL;
splx(s);
@@ -1403,27 +1415,21 @@ carp_get_srclladdr(struct ifnet *ifp, u_
}
int
-carp_our_mcastaddr(struct ifnet *ifp, u_int8_t *d_enaddr)
-{
- struct carp_softc *sc = ifp->if_softc;
-
- if (sc->sc_balancing != CARP_BAL_IP)
- return (0);
-
- return (!memcmp(sc->sc_ac.ac_enaddr, d_enaddr, ETHER_ADDR_LEN));
-}
-
-
-int
-carp_input(struct ifnet *ifp0, struct ether_header *eh0, struct mbuf *m)
+carp_input(struct mbuf *m)
{
+ struct carp_softc *sc;
struct ether_header *eh;
- struct carp_if *cif = (struct carp_if *)ifp0->if_carp;
- struct ifnet *ifp;
+ struct mbuf_list ml = MBUF_LIST_INITIALIZER();
+ struct carp_if *cif;
+ struct ifnet *ifp0, *ifp;
- ifp = carp_ourether(cif, eh0->ether_dhost);
- if (ifp == NULL && (m->m_flags & (M_BCAST|M_MCAST)) == 0)
- return (1);
+ ifp0 = m->m_pkthdr.rcvif;
+ eh = mtod(m, struct ether_header *);
+ cif = (struct carp_if *)ifp0->if_carp;
+
+ ifp = carp_ourether(cif, eh->ether_dhost);
+ if (ifp == NULL && !ETHER_IS_MULTICAST(eh->ether_dhost))
+ return (0);
if (ifp == NULL) {
tcpdump'ing on vether(4)
I find it really useful to be able see output packets when tcpdump'ing
a vether(4) interface.
ok?
Index: net/if_vether.c
===
RCS file: /cvs/src/sys/net/if_vether.c,v
retrieving revision 1.23
diff -u -p -r1.23 if_vether.c
--- net/if_vether.c 14 Mar 2015 03:38:51 - 1.23
+++ net/if_vether.c 22 May 2015 13:19:30 -
@@ -29,6 +29,11 @@
#include
#include
+#include "bpfilter.h"
+#if NBPFILTER > 0
+#include
+#endif
+
void vetherattach(int);
intvetherioctl(struct ifnet *, u_long, caddr_t);
void vetherstart(struct ifnet *);
@@ -117,15 +122,17 @@ void
vetherstart(struct ifnet *ifp)
{
struct mbuf *m;
- int s;
for (;;) {
- s = splnet();
IFQ_DEQUEUE(&ifp->if_snd, m);
- splx(s);
-
if (m == NULL)
return;
+
+#if NBPFILTER > 0
+ if (ifp->if_bpf)
+ bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_OUT);
+#endif /* NBPFILTER > 0 */
+
ifp->if_opackets++;
m_freem(m);
}
Re: pf.conf from/to negation homogeneous behavior
On Fri, May 22, 2015 at 5:09 AM, Henning Brauer
wrote:
> * sven falempin [2015-05-21 17:29]:
> > I propose
> >
> > Index: pfctl/parse.y
> > ===
> > RCS file: /cvs/src/sbin/pfctl/parse.y,v
> > retrieving revision 1.648
> > diff -u -p -r1.648 parse.y
> > --- pfctl/parse.y 21 Apr 2015 16:34:59 - 1.648
> > +++ pfctl/parse.y 21 May 2015 15:21:54 -
> > @@ -2563,7 +2563,7 @@ optnl : '\n' optnl
> >
> > ipspec : ANY { $$ = NULL; }
> > | xhost { $$ = $1; }
> > - | '{' optnl host_list '}' { $$ = $3; }
> > + | not '{' optnl host_list '}' { $$ = $4; $$->not = $1;
> }
> >
> >
> this doesn't do what you think it does. You think it matches
> everything but 8.8.8.8 and 8.8.4.4, while in reality, it matches
> everything. Feed that rule through
> pfctl -nvf -
> and you'll see it expanded to
>
> match log on vic0 proto icmp from any to ! 8.8.8.8
> match log on vic0 proto icmp from any to ! 8.8.4,4
>
> the list negation discussion is as old as pf.
>
>
>
Sir,
looking the rule actually show and unexpected result :
[0]-[sn386.localdomain]-[/root]
# pfctl -s rules
block return all
match log on vic0 inet proto icmp from any to ! 8.8.8.8
match log on vic0 inet proto icmp from any to 8.8.4.4
match log on vic0 proto icmp from any to !
pass all flags S/SA
block return in on ! lo0 proto tcp from any to any port 6000:6010
I did my small test of yesterday again , i can imagine things but not the
same twice ;-)
[0]-[sn386.localdomain]-[/root]
# tcpdump -tteni pflog0 icmp&
[1] 25796
[0]-[sn386.localdomain]-[/root]
# tcpdump: WARNING: snaplen raised from 116 to 160
tcpdump: listening on pflog0, link-type PFLOG
ping -c 4 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: icmp_seq=0 ttl=51 time=26.397 ms
64 bytes from 8.8.8.8: icmp_seq=1 ttl=51 time=24.652 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=51 time=28.601 ms
64 bytes from 8.8.8.8: icmp_seq=3 ttl=51 time=23.564 ms
--- 8.8.8.8 ping statistics ---
4 packets transmitted, 4 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 23.564/25.803/28.601/1.911 ms
[0]-[sn386.localdomain]-[/root]
# ping -c 4 8.8.4.4
PING 8.8.4.4 (8.8.4.4): 56 data bytes
64 bytes from 8.8.4.4: icmp_seq=0 ttl=51 time=30.802 ms
64 bytes from 8.8.4.4: icmp_seq=1 ttl=51 time=21.942 ms
64 bytes from 8.8.4.4: icmp_seq=2 ttl=51 time=28.501 ms
64 bytes from 8.8.4.4: icmp_seq=3 ttl=51 time=28.315 ms
--- 8.8.4.4 ping statistics ---
4 packets transmitted, 4 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 21.942/27.390/30.802/3.294 ms
[0]-[sn386.localdomain]-[/root]
# ping -c 4 192.168.238.1
PING 192.168.238.1 (192.168.238.1): 56 data bytes
64 bytes from 192.168.238.1: icmp_seq=0 ttl=128 time=0.452 ms
1432296447.095596 rule 1/(match) match out on vic0: 192.168.238.133 >
192.168.238.1: icmp: echo request
1432296447.095604 rule 3/(match) match out on vic0: 192.168.238.133 >
192.168.238.1: icmp: echo request
64 bytes from 192.168.238.1: icmp_seq=1 ttl=128 time=0.321 ms
64 bytes from 192.168.238.1: icmp_seq=2 ttl=128 time=0.379 ms
64 bytes from 192.168.238.1: icmp_seq=3 ttl=128 time=0.406 ms
--- 192.168.238.1 ping statistics ---
4 packets transmitted, 4 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 0.321/0.389/0.452/0.051 ms
[0]-[sn386.localdomain]-[/root]
#
Only 192.168.238.1 show result in pflog0,
This result are really puzzling for me,
when i first test the table negation i was really glad that list negation
was possible,
the (block) alternative is often ridiculous to write.
--
-
() ascii ribbon campaign - against html e-mail
/\
Re: pf_create_state() is sometimes better to use pf_unlink_state()
* Alexandr Nedvedicky [2015-05-21 21:31]:
> On Thu, May 21, 2015 at 07:43:51PM +0200, Mike Belopuhov wrote:
> > On Thu, May 21, 2015 at 17:34 +0200, Alexandr Nedvedicky wrote:
> > > snippet below comes from pf_create_state():
> > >
> > >3559 if (pf_state_insert(BOUND_IFACE(r, pd->kif), skw, sks,
> > > s)) {
> > >3560 pf_state_key_detach(s, PF_SK_STACK);
> > >3561 pf_state_key_detach(s, PF_SK_WIRE);
> > >3562 *sks = *skw = NULL;
> > >3563 REASON_SET(&reason, PFRES_STATEINS);
> > >3564 goto csfailed;
> > >3565 } else
> > >3566 *sm = s;
> > >3567
> > >3568 /* attach src nodes late, otherwise cleanup on error
> > > nontrivial */
> > >3569 for (i = 0; i < PF_SN_MAX; i++)
> > >3570 if (sns[i] != NULL) {
> > >3571 struct pf_sn_item *sni;
> > >3572
> > >3573 sni = pool_get(&pf_sn_item_pl, PR_NOWAIT);
> > >3574 if (sni == NULL) {
> > >3575 REASON_SET(&reason, PFRES_MEMORY);
> > >3576 pf_src_tree_remove_state(s);
> > >3577 STATE_DEC_COUNTERS(s);
> > >3578 pool_put(&pf_state_pl, s);
> > >3579 return (PF_DROP);
> > >3580 }
> > >3581 sni->sn = sns[i];
> > >3582 SLIST_INSERT_HEAD(&s->src_nodes, sni,
> > > next);
> > >3583 sni->sn->states++;
> > >3584 }
> > >
> > > at line 3559 PF inserts state to table. If insert operation succeeds, then
> > > state can no longer be killed using simple pool_put() as it currently
> > > happens at line 3578. I think PF should go for pf_unlink_state() instead.
> > > patch below should kill the bug.
> > Indeed. But I don't like the comment stating that we're attaching
> > src nodes late because the "cleanup on error nontrivial". Perhaps
> > we should do a pf_state_insert afterwards? This might simplify
> > locking later on.
> perhaps swapping the for loop block with pf_state_insert() will work.
> We can then bail out using goto csfailed then (see patch below...)
makes sense, I like it.
> > > would you be interested in SMP patch for PF?
> > > it basically introduces fine locking and reference counting
> > > on PF data objects, so firewall can handle more packets at
> > > single instance of time.
I'd certainly like to see it.
As Mike points out, there's more to do before it can be useful for us
tho :/
> > Thanks for your quality diffs btw, help is always much appreciated.
absolutely!
--
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/
Re: pf.conf from/to negation homogeneous behavior
* sven falempin [2015-05-21 17:29]:
> I propose
>
> Index: pfctl/parse.y
> ===
> RCS file: /cvs/src/sbin/pfctl/parse.y,v
> retrieving revision 1.648
> diff -u -p -r1.648 parse.y
> --- pfctl/parse.y 21 Apr 2015 16:34:59 - 1.648
> +++ pfctl/parse.y 21 May 2015 15:21:54 -
> @@ -2563,7 +2563,7 @@ optnl : '\n' optnl
>
> ipspec : ANY { $$ = NULL; }
> | xhost { $$ = $1; }
> - | '{' optnl host_list '}' { $$ = $3; }
> + | not '{' optnl host_list '}' { $$ = $4; $$->not = $1; }
>
>
> I tested it on i386 current with a small ruleset ! table and ! {} got now
> same behavior,
huh?
> match log on vic0 proto icmp from any to !{ 8.8.8.8, 8.8.4.4 }
this doesn't do what you think it does. You think it matches
everything but 8.8.8.8 and 8.8.4.4, while in reality, it matches
everything. Feed that rule through
pfctl -nvf -
and you'll see it expanded to
match log on vic0 proto icmp from any to ! 8.8.8.8
match log on vic0 proto icmp from any to ! 8.8.4,4
the list negation discussion is as old as pf.
--
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/
Re: luarexlib patch
On Sun, May 10, 2015 at 10:20:10PM +0200, Mark Kettenis wrote: > [ Sending this to tech@ since I don't just want the ports people to > know about this ] > > Invoking the linker directly to build executables or shared libraries > is strongly discouraged. It really doesn't work properly unless you > also link in the appropriate /usr/lib/crt*.o files. We rely on code > in these files for: > > * The private stack protector cookie. > > * For atexit(3) to work in shared libraries that can be dlclose()ed. > > * For pthread_atfork(3) to work in shared libraries that can be dlclose()ed. > > With binutils 2.15 linking will succeed, but you might see random > crashes if executable and shared libraries happen to be loaded more > than 2G apart from eachother. With binutils 2.17 you'll see the > R_X86_64_PC32 relocation failure, and linking will fail. > > In general, you should just invoke cc(1) instead of ld(1), since it > will take care of all the magic. > > Diff below fixes things for devel/luarexlib. > > ok? OK aja (with a rev bump) > > > Index: patches/patch-src_common_mak > === > RCS file: /cvs/ports/devel/luarexlib/patches/patch-src_common_mak,v > retrieving revision 1.1 > diff -u -p -r1.1 patch-src_common_mak > --- patches/patch-src_common_mak 9 Sep 2009 18:02:01 - 1.1 > +++ patches/patch-src_common_mak 10 May 2015 20:05:23 - > @@ -1,6 +1,6 @@ > $OpenBSD: patch-src_common_mak,v 1.1 2009/09/09 18:02:01 jolan Exp $ > --- src/common.mak.orig Sat Jun 14 20:11:08 2008 > -+++ src/common.mak Wed Sep 9 12:13:00 2009 > src/common.mak Sun May 10 22:02:08 2015 > @@ -3,11 +3,11 @@ > V = 2.4 > > @@ -9,8 +9,9 @@ $OpenBSD: patch-src_common_mak,v 1.1 200 > +CFLAGS+= $(MYCFLAGS) $(DEFS) $(INC) > TRG_AR = lib$(TRG).a > TRG_SO = $(TRG).so > - LD = ld > +-LD = ld > -LDFLAGS= -shared > ++LD = cc > +LDFLAGS= -fPIC -shared > > all: $(TRG_AR) $(TRG_SO) > -- Antoine
Re: mismatch for ICMP state created by inound response
* Alexandr Nedvedicky [2015-05-21 21:29]: > > Well, not entirely (: I did it while exploring the code and sent > > out to provoke further discussion. Today I've talked to reyk@ and > > we think that it's better to go down a different road: make sure we > > don't create states on reply packets in the first place. > that's actually very wise approach as replies can be spoofed... agreed. > > I've tested this with ICMP, ICMPv6 and NAT64 (slightly). Any OKs? > > Objections? > I have no objections, just a small wish, can you set icmp_dir to -1, > if we are not dealing with ICMP? there is a tool we use in Solaris, > which yells on us because of uninitialized variable. I know it's > false positive, but I've gave up on explaining... I don't see any harm done by this on our side, so yeah, why not. having a default case there is better style anyway. -- 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/
