Re: pf: honor quick on anchor rules
* Sebastian Benoit [2018-10-08 10:50]: > The quick in the anchor does not do anything by itself, it should just > "behave like all the rules inside the anchor had the quick keyword". no, that is *not* the intended semantic. the intent is that ruleset evaluation is aborted if any rule inside the anchor matched. note that this is very different from "any rule inside treated like it had quick", since that would abort evaluation *inside* the anchor immediately as well. -- 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: fstat -r flag to display rdomains on sockets
* Stuart Henderson [2018-04-07 12:51]: > How about skipping the -r flag, but printing it after the existing > "internet [...] addr:port", and only printing if it's a non-default > table? This should minimise breakage, and it's not like the format > is cast in stone (spliced sockets were added in the past). agreed > I wonder about using "rtable" instead of "rdomain" in the text, it > would be more accurate (but then I do see "rdomain" in some other > programs like bgpd). rtable is the right term here. an rdomain is a special case of an rtable (and bgpd uses both). -- 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 generic packet delay
* Martin Pieuchot [2018-02-23 10:04]:
> On 23/02/18(Fri) 04:08, Henning Brauer wrote:
> > * Martin Pieuchot [2018-02-21 09:37]:
> > > On 21/02/18(Wed) 02:37, Henning Brauer wrote:
> > > I'd suggest moving the pool allocation and the function in net/pf*.c
> > > and only have a function call under #if NPF > 0.
> > worth discussing, but imo that part doesn't really have all that much
> > to do with pf.
> It is only used by pf(4). All the code is under #if NPF > 0, so it *is*
> related to PF. We have been reducing the #ifdef maze through the years
> for maintainability reasons. I don't see the point for reintroducing
> them for taste.
you're jumping the gun from seeing #ifdef NPF - that is really used
here to keep stuff out of the ramdisks. Unfortunately NPF has become
quite a kitchensink ifdef for that purpose. The delay functionality is
network stack and not pf in my book, with pf being one of the possible
triggers. But anyway, this is a corner case and I don't care too much
and code can easily be moved later.
It actually has the welcome sideeffect that the pool limit can be
adjusted easily.
> If you can easily avoid using ph_cookie, then please do it. Otherwise
> you're putting maintenance burden by writing fragile code that will
> subtly break when somebody will start using ph_cookie for something else.
I don't see that in this case but anyway, not worth splitting
hair over. Not that I had much anyway.
Here's the HND-MUC version.
Index: sys/sys/mbuf.h
===
RCS file: /cvs/src/sys/sys/mbuf.h,v
retrieving revision 1.235
diff -u -p -r1.235 mbuf.h
--- sys/sys/mbuf.h 11 Feb 2018 00:24:13 - 1.235
+++ sys/sys/mbuf.h 11 Feb 2018 04:47:44 -
@@ -100,10 +100,11 @@ struct pkthdr_pf {
struct inpcb*inp; /* connected pcb for outgoing packet */
u_int32_tqid; /* queue id */
u_int16_ttag; /* tag id */
+ u_int16_tdelay; /* delay packet by X ms */
u_int8_t flags;
u_int8_t routed;
u_int8_t prio;
- u_int8_t pad[3];
+ u_int8_t pad;
};
/* pkthdr_pf.flags */
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.549
diff -u -p -r1.549 if.c
--- sys/net/if.c20 Mar 2018 08:58:19 - 1.549
+++ sys/net/if.c1 Apr 2018 13:26:42 -
@@ -681,6 +681,11 @@ if_enqueue(struct ifnet *ifp, struct mbu
struct ifqueue *ifq;
int error;
+#if NPF > 0
+ if (m->m_pkthdr.pf.delay > 0)
+ return (pf_delay_pkt(m, ifp->if_index));
+#endif
+
#if NBRIDGE > 0
if (ifp->if_bridgeport && (m->m_flags & M_PROTO1) == 0) {
KERNEL_LOCK();
Index: sys/net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1063
diff -u -p -r1.1063 pf.c
--- sys/net/pf.c6 Mar 2018 17:35:53 - 1.1063
+++ sys/net/pf.c17 Mar 2018 21:55:18 -
@@ -161,7 +161,7 @@ struct pf_test_ctx {
struct pool pf_src_tree_pl, pf_rule_pl, pf_queue_pl;
struct pool pf_state_pl, pf_state_key_pl, pf_state_item_pl;
-struct pool pf_rule_item_pl, pf_sn_item_pl;
+struct pool pf_rule_item_pl, pf_sn_item_pl, pf_pktdelay_pl;
voidpf_add_threshold(struct pf_threshold *);
int pf_check_threshold(struct pf_threshold *);
@@ -258,6 +258,7 @@ void pf_state_key_link_inpcb(struct
p
struct inpcb *);
voidpf_state_key_unlink_inpcb(struct pf_state_key *);
voidpf_inpcb_unlink_state_key(struct inpcb *);
+voidpf_pktenqueue_delayed(void *);
#if NPFLOG > 0
voidpf_log_matches(struct pf_pdesc *, struct pf_rule *,
@@ -273,7 +274,8 @@ struct pf_pool_limit pf_pool_limits[PF_L
{ &pf_src_tree_pl, PFSNODE_HIWAT, PFSNODE_HIWAT },
{ &pf_frent_pl, PFFRAG_FRENT_HIWAT, PFFRAG_FRENT_HIWAT },
{ &pfr_ktable_pl, PFR_KTABLE_HIWAT, PFR_KTABLE_HIWAT },
- { &pfr_kentry_pl, PFR_KENTRY_HIWAT, PFR_KENTRY_HIWAT }
+ { &pfr_kentry_pl, PFR_KENTRY_HIWAT, PFR_KENTRY_HIWAT },
+ { &pf_pktdelay_pl, PF_PKTDELAY_MAXPKTS, PF_PKTDELAY_MAXPKTS }
};
#define STATE_LOOKUP(i, k, d, s, m)\
@@ -3488,6 +3490,8 @@ pf_rule_to_actions(struct pf_rule *r, st
a->set_prio[0] = r->set_prio[0];
a->set_prio[1] = r->set_prio[1];
}
+ if (r->rule_flag & PFRULE_SETDELAY)
+ a->delay
Re: pf generic packet delay
* Martin Pieuchot [2018-02-21 09:37]: > On 21/02/18(Wed) 02:37, Henning Brauer wrote: > I'd suggest moving the pool allocation and the function in net/pf*.c > and only have a function call under #if NPF > 0. worth discussing, but imo that part doesn't really have all that much to do with pf. > I'd suggest defining your own structure containing a timeout and a mbuf > pointer instead of abusing ph_cookie. Since you're already allocating > something it doesn't matter much and you're code won't be broken by > a future refactoring. dlg pointed me to ph_cookie, I was about to use my own structure. "ph_cookie is for exactly that" > You're leaking a mbuf if the interface has been detached. hah! true. fixed locally (the obvious: else m_freem(m);) -- 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/
pf generic packet delay
Here comes generic delay functionality for pf.
The manpage bits are missing for the moment, but it's really simple to
use:
match in set delay 1
delay is in ms. should I change the parser to explicitely require
"ms", as in "match in set delay 1ms"?
I have a pool_sethardlimit as a "last resort" style upper limit of
delayed packets held, hardcoded to 1 right now - does that need to
be tunable? if not - what value? I'll obviously make it a define at
least.
delay range is 1-65535ms - is 65s too excessive, aka do we need to
impose a lower upper limit?
I would welcome a few tests (and then oks from ppl with the right @) so
that we can go further once the basic functionality is in. And yes,
things like a delay range with random value in between chosen can be
added later. Getting the basic delay machinery, foremost in if.c, right
is the main goal now, the pf side is relatively easy to twiddle with.
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.547
diff -u -p -r1.547 if.c
--- sys/net/if.c20 Feb 2018 03:46:45 - 1.547
+++ sys/net/if.c21 Feb 2018 01:26:37 -
@@ -127,6 +127,10 @@
#if NPF > 0
#include
+#include
+
+#defineIF_TIMEOUTPL_LIMIT 1
+struct pooliftimeopl;
#endif
void if_attachsetup(struct ifnet *);
@@ -164,6 +168,10 @@ void ifa_print_all(void);
void if_qstart_compat(struct ifqueue *);
+#if NPF > 0
+void if_enqueue_delayed(void *);
+#endif
+
/*
* interface index map
*
@@ -261,6 +269,14 @@ ifinit(void)
}
net_tick(&net_tick_to);
+
+#if NPF > 0
+ pool_init(&iftimeopl, sizeof(struct timeout), 0, IPL_SOFTNET, 0,
+ "iftimeout", NULL);
+ pool_sethardlimit(&iftimeopl, IF_TIMEOUTPL_LIMIT, "iftimeout pool "
+ "limit exceeded, dropping packets", 60);
+/* XXX hardlimit as last resort safeguard, limit? */
+#endif
}
static struct if_idxmap if_idxmap = {
@@ -674,12 +690,44 @@ if_qstart_compat(struct ifqueue *ifq)
KERNEL_UNLOCK();
}
+#if NPF > 0
+void
+if_enqueue_delayed(void *arg)
+{
+ struct mbuf *m = arg;
+ struct ifnet*ifp;
+ struct timeout *to = m->m_pkthdr.ph_cookie;
+
+ ifp = if_get(m->m_pkthdr.ph_ifidx);
+ if (ifp != NULL) {
+ if_enqueue(ifp, m);
+ if_put(ifp);
+ }
+ pool_put(&iftimeopl, to);
+}
+#endif
+
int
if_enqueue(struct ifnet *ifp, struct mbuf *m)
{
unsigned int idx;
struct ifqueue *ifq;
int error;
+
+#if NPF > 0
+ if (m->m_pkthdr.pf.delay > 0) {
+ struct timeout *to;
+
+ if ((to = pool_get(&iftimeopl, PR_NOWAIT)) == NULL)
+ return (ENOBUFS);
+ timeout_set(to, if_enqueue_delayed, m);
+ timeout_add_msec(to, m->m_pkthdr.pf.delay);
+ m->m_pkthdr.pf.delay = 0;
+ m->m_pkthdr.ph_cookie = to;
+ m->m_pkthdr.ph_ifidx = ifp->if_index;
+ return (0);
+ }
+#endif
#if NBRIDGE > 0
if (ifp->if_bridgeport && (m->m_flags & M_PROTO1) == 0) {
Index: sys/net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1061
diff -u -p -r1.1061 pf.c
--- sys/net/pf.c18 Feb 2018 21:45:30 - 1.1061
+++ sys/net/pf.c21 Feb 2018 01:16:00 -
@@ -3484,6 +3484,8 @@ pf_rule_to_actions(struct pf_rule *r, st
a->set_prio[0] = r->set_prio[0];
a->set_prio[1] = r->set_prio[1];
}
+ if (r->rule_flag & PFRULE_SETDELAY)
+ a->delay = r->delay;
}
#define PF_TEST_ATTRIB(t, a) \
@@ -3979,6 +3981,7 @@ pf_create_state(struct pf_pdesc *pd, str
#endif /* NPFSYNC > 0 */
s->set_prio[0] = act->set_prio[0];
s->set_prio[1] = act->set_prio[1];
+ s->delay = act->delay;
SLIST_INIT(&s->src_nodes);
switch (pd->proto) {
@@ -7025,6 +7028,7 @@ done:
if (s->state_flags & PFSTATE_SETPRIO)
pd.m->m_pkthdr.pf.prio = s->set_prio[0];
}
+ pd.m->m_pkthdr.pf.delay = s->delay;
} else {
pf_scrub(pd.m, r->scrub_flags, pd.af, r->min_ttl,
r->set_tos);
@@ -7037,6 +7041,7 @@ done:
if (r->scrub_flags & PFSTATE_SETPRIO)
pd.m->m_pkthdr.pf.prio = r->set_prio[0];
}
+ pd.m->m_pkthdr.pf.delay = r->delay;
}
}
Index: sys/net/pfvar.h
===
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.476
diff -u -p -r1.476 pfvar.h
--- sys/net/pfvar.h 9 Feb 2018 09:35:03 - 1.476
+
bridge arpfilter
} else if (strcmp((*argv)[0], "tpa") == 0) {
+ rule->ifbr_arpf.brla_flags |= BRLA_TPA;
+ dia = &rule->ifbr_arpf.brla_tpa;
+ } else
+ return (0);
+
+ (*argc)--; (*argv)++;
+ if (dea != NULL) {
+ if (*argc == 0)
+ return (-1);
+ ea = ether_aton((*argv)[0]);
+ if (ea == NULL) {
+ warnx("invalid address: %s", (*argv)[0]);
+ return (-1);
+ }
+ bcopy(ea, dea, sizeof(*dea));
+ (*argc)--; (*argv)++;
+ }
+ if (dia != NULL) {
+ if (*argc == 0)
+ return (-1);
+ ia.s_addr = inet_addr((*argv)[0]);
+ if (ia.s_addr == INADDR_NONE) {
+ warnx("invalid address: %s", (*argv)[0]);
+ return (-1);
+ }
+ bcopy(&ia, dia, sizeof(*dia));
+ (*argc)--; (*argv)++;
+ }
+ }
+ return (0);
+}
+
+
+#define MAXRULEWORDS 32
void
bridge_rulefile(const char *fname, int d)
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.288
diff -u -p -r1.288 ifconfig.8
--- sbin/ifconfig/ifconfig.814 Sep 2017 13:02:12 - 1.288
+++ sbin/ifconfig/ifconfig.87 Nov 2017 10:09:01 -
@@ -685,9 +685,10 @@ like a hub or a wireless network.
.Cm block Ns | Ns Cm pass
.Op Cm in | out
.Cm on Ar interface
-.Op Cm src Ar address
-.Op Cm dst Ar address
+.Op Cm src Ar lladdr
+.Op Cm dst Ar lladdr
.Op Cm tag Ar tagname
+.Op Cm arp | rarp Ar [ request | reply ] [ Cm sha Ar lladdr ] [ Cm spa Ar
ipaddr ] [ Cm tha Ar lladdr ] [ Cm tpa Ar ipaddr ]
.Xc
Add a filtering rule to an interface.
Rules have a similar syntax to those in
@@ -697,6 +698,25 @@ MAC addresses.
They can also tag packets for
.Xr pf 4
to filter on.
+.Xr arp 4
+packets can be matched with the
+.Cm arp
+keyword for regular and
+.Cm rarp
+for reverse arp packets.
+.Ar request
+and
+.Ar reply
+limit matches to requests or replies.
+The source and destination host addresses can be matched with the
+.Cm sha
+and
+.Cm tha
+keywords,
+the protocol addresses with
+.Cm spa
+and
+.Cm tpa .
Rules are processed in the order in which they were added
to the interface, and the first rule matched takes the action (block or pass)
and, if given, the tag of the 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/
bridge: apply filters outbound, too
bridge doesn't call the filters on outbound.
That only affects locally generated packets. It is debatable whether
that is a bug or a (questionable) design decision. The impact on
existing setups needs to be evaluated; I'm very unsure. But then I'm
not the biggest bridge fan on earth and have never seen anyone using
the bridge filters in a real world scenario until now, afair.
ok?
Index: sys/net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.298
diff -u -p -r1.298 if_bridge.c
--- sys/net/if_bridge.c 17 Aug 2017 10:14:08 - 1.298
+++ sys/net/if_bridge.c 25 Oct 2017 13:32:36 -
@@ -734,6 +734,7 @@ bridge_output(struct ifnet *ifp, struct
struct ether_addr *dst;
struct bridge_softc *sc;
struct bridge_tunneltag *brtag;
+ struct bridge_iflist *ifl;
int error;
/* ifp must be a member interface of the bridge. */
@@ -819,6 +820,12 @@ bridge_output(struct ifnet *ifp, struct
continue;
}
}
+
+ ifl = (struct bridge_iflist *)dst_if->if_bridgeport;
+ KASSERT(ifl != NULL);
+ if (bridge_filterrule(&ifl->bif_brlout, eh, mc) ==
+ BRL_ACTION_BLOCK)
+ continue;
error = bridge_ifenqueue(sc, dst_if, mc);
if (error)
--
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.5 translation option happens immediately only on match rules
* Tony Gong [2017-05-31 10:28]: > Pretty sure pf applies translations immediately only if the rule is a > match rule. > Diff makes this clear in the man page. yup, in, thx -- 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: tcpdump: drop atalk support
* Theo de Raadt [2017-05-30 10:56]: > > How about just dropping support for /etc/appletalk.names, which as far > > as I can tell was never used, and drop the manpage bit, reducing it by > > 10%. Most of the text in the manpage is outdated anyway, talking about > > /etc/atalk.names - support for which was removed in 2004 with the > > privsep work. Something like this: > > Sure sure. > > My main objection to full removal was that you see a numbered packet > flying over your network and don't know what catagory it is in. > Suddenly google search is neccessary because tcpdump is going out > of the way to not help. So it should help, answering the minimum > question of "what type is that packet, should I worry". agreed. can we limit this to just being able to identify appletalk? note that this is ethertype appletalk, not appletalk over ip. afaik that means pre-macosx. -- 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: [PATCH] ntpd: allow to specify a source IP address for outgoing queries
* Sebastian Benoit [2017-05-28 22:52]: > which makes me think: > would a global local-address be good enough? I think so. This is a kinda weird/rare case. -- 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: tcpdump: drop atalk support
* Michal Mazurek [2017-05-28 16:00]: > Remove atalk support. Significantly shortens the manpage. libpcap still > supports it. This diff doesn't include the removal of two files: > appletalk.h and print-atalk.c. afaict atalk is so dead that the corpse is way beyond the point of smelling - so yeah, imo it is time to let that go. -- 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: Problems with rdomain and net/if.c v1.455
* Martin Pieuchot [2016-11-09 11:55]: > On 08/11/16(Tue) 17:23, Claudio Jeker wrote: > > On Tue, Nov 08, 2016 at 03:36:22PM +0100, Martin Pieuchot wrote: > > > I'm not sure to understand the benefit. What's the use case for loop(4)? > > 2 name spaces, so that I don't have a conflict if I use lo1 for my > > loopback IPs and then later on create rdomain 1. > I'm afraid this would confuse newcomers. It seems to me that this is > just a bandage for people already using multiple conflicting lo(4) and > rdomains. indeed. > I'd say just put your loopback IPs on lo1000 or lo42... But maybe this > should be discussed by people using that ;) I fully agree. Why? -the use of lo interfaces except lo0 is a relatively rare exception (heavy ospf/bgp/... users largely I guess) -picking a non-conflicting unit # for these few cases is easy enough -introducing a copy of lo just to split namespaces seems overkill -- 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: rebound quantum entanglement
* Ted Unangst [2016-09-15 16:15]: > The good news is I think we can still bind to > localhost:53 if nsd is on *:53 (right?). right. -- 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: teach BFD how to send route messages
* Peter Hessler [2016-09-15 14:48]: > On 2016 Sep 15 (Thu) at 13:28:10 +0200 (+0200), Martin Pieuchot wrote: > :By the way 'softc' is generally for device drivers, you might prefer > :something like bfdd (d for descriptor). > There's be a whole lot of bikeshedding about this topic, I would prefer > not to change it for now. errm, no. please fix. softc is clear to any developer who's spent time in kernel land, and this is abuse. misleading as f***. -- 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: Bridge broken in 6.0?
* Aaron Riekenberg [2016-09-05 13:04]: > Thanks for the explanation. > > I am curious though - is dhclient really the right place to fix this? I > might use some other dhcp client (dhcpcd in ports for example) or some > other application that uses BPF. Should every userland program using BPF > have to worry whether or not it is breaking bridging? no, the key is the dropping in BPF, which is an OpenBSD extension. [I don't know whether others have similiar schemes or followed our lead, and that's NOT THE TOPIC HERE. point is, it is nonstandard and not in widespread use by 3rd party code if at all] strictly speaking, the bpf filters in the base dhcp programs have been matching (and thus eating) too much forever since we added them, it just didn't show up because it was covered by the behaviour (strictly speaking, I'd say misbehaviour) of the stack with bridge so far. -- 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: tcpbench(4) support for AF_UNIX
* Sebastian Benoit [2016-07-20 21:42]: > Claudio Jeker(cje...@diehard.n-r-g.com) on 2016.07.20 19:08:51 +0200: > > On Wed, Jul 20, 2016 at 04:09:48PM +0200, Claudio Jeker wrote: > > > For testing I want to abuse tcpbench to work over AF_UNIX sockets. > > > This diff does exactly that with minimal extras. Especially the unix > > > socket is not removed from the filesystem when closed. I don't want to > > > add pledge cpath to tcpbench just for that. > > > > > > > New version that documents the -R feature that I sneaked in (without > > realizing) and also fixes the documentation for -U a bit. > > I added -R some time ago to stress test different mbuf sizes. tcpbench is > > a test tool for me :) > > ich habe es kompiliert und getestet. > > ok, jawohl. jawoll! anybody left on tech who doesn't (pretend to) speak german? -- 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 macro with space
* Stefan Sperling [2016-06-21 11:15]: > Generally, I would appreciate more detailed error messages from the pf.conf > parser. I recall several occasions where pfctl threw "syntax error" and more > specific error reporting would have saved me some time with finding the > silly mistake I made. And in this case the ruleset loads successfully even > though, while parsing, we already know it's not going to work as intended... true, that's shared by all yacc-style parsers, if the grammar doesn't match you just get syntax error without much of a hint what's wrong. however, the ruleset in this case does NOT load. $ echo '"a macro with spaces"="foo"\npass from $a\ macro\ with\ spaces"' | pfctl -nvf - a macro with spaces = "foo" stdin:2: macro 'a' not defined stdin:2: syntax error > Only as long as it doesn't make the parser code overly complex, of course. > But currently the balance is tilted too much towards terse error messages > for my taste. So I liked benno's first diff. it's just a tiny check indeed, which swings the "cost" (not in financial terms) vs benefit pendulum towards the benefit side, yes. -- 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 macro with space
* Sebastian Benoit [2016-06-21 10:15]: > same thing without a stupid helper function, pointed out by henning. I'm actually not quite sure we need or want this. From my PoV, making the tools too much of a nanny is against unix spirit. Macros with spaces don't actually cause harm, they just don't work. Not too unexpected apparently given that, afair at least, nobody spoke up on it in more than a decade. So, do we really want this extra check? I'm unsure. If not, short mention in the manpage or just leave things as they are? -- 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: af-to on pass out should be a parser error
* Mike Belopuhov [2016-06-20 00:33]: > rdr-to/nat-to are not checked on purpose. i'm not certain about > route-to/reply-to. indeed, rdr-to/nat-to in the "unnatural" direction DO work, with caveats. route-to and af-to are different. as others already pointed out the check should be != PF_IN and not == PF_OUT, to catch unspecified direction. With that, ok with me. > as far as i'm concerned, af-to should be restricted to "pass in". > but it would be nice to know if "pass out route-to" and "pass in > reply-to" produce working configurations to restrict them as well > if they don't. ack - I dunno either otoh -- 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: Set prio when bypassing pf(4)
* Mike Belopuhov [2016-06-08 22:49]: > We have discussed this here with henning and benno and think that > this diff should go in. OK mikeb heh, ROP (relayed ok protocol)? :) quite surprised that there is equipment blocking traffic based on prio... pretty broken. but fighting windmills isn't too rewarding, either. re default 3, that is nicely in the middle and otoh i was looking at other implementations and their defaults and that was quite common. afaict most switches with just 4 queues map 0+1 / 2+3 / 4+5 / 6+7. so, indeed, ok. -- 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: [ntpd] Simultaneously listen on IPv4 and IPv6
* Martin Pieuchot [2016-05-17 17:05]:
> On 17/05/16(Tue) 16:16, Henning Brauer wrote:
> > * Gilles Chehade [2016-05-17 15:56]:
> > > On Tue, May 17, 2016 at 08:27:42AM -0500, Brent Cook wrote:
> > > > This patch came by way of the openntpd github. Linux (and possibly
> > > > others)
> > > > will attempt to bind to 0.0.0.0 when binding to '::' and return an
> > > > error if
> > > > it can't, unless IPV6_V6ONLY is set. See
> > > > https://github.com/openntpd-portable/openntpd-portable/issues/19
> > > >
> > > > OK as an in-tree patch? OpenBSD seems to adopt a more liberal
> > > > interpretation and not return a failure in the same scenario. (The patch
> > > > against the 5.7 tree, but you get the idea)
> > > >
> > > > +#ifdef IPV6_V6ONLY
> > > > + if (la->sa.ss_family == AF_INET6 && setsockopt(la->fd,
> > > > + IPPROTO_IPV6, IPV6_V6ONLY, &on, sizeof(on)) == -1)
> > > > + log_warn("setsockopt IPV6_V6ONLY");
> > > > +#endif
> > this is exactly what is supposed to live in the portable imho, to not
> > clutter the native sources.
> Are you sure?
no, or I wouldn't have asked about where to draw the line :)
> What about systems with net.inet6.ip6.v6only=0?
Those haven't been taken into consideration by yours truly and might be
the compelling argument to have this code :)
--
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: [ntpd] Simultaneously listen on IPv4 and IPv6
* Gilles Chehade [2016-05-17 15:56]:
> On Tue, May 17, 2016 at 08:27:42AM -0500, Brent Cook wrote:
> > This patch came by way of the openntpd github. Linux (and possibly others)
> > will attempt to bind to 0.0.0.0 when binding to '::' and return an error if
> > it can't, unless IPV6_V6ONLY is set. See
> > https://github.com/openntpd-portable/openntpd-portable/issues/19
> >
> > OK as an in-tree patch? OpenBSD seems to adopt a more liberal
> > interpretation and not return a failure in the same scenario. (The patch
> > against the 5.7 tree, but you get the idea)
> >
>
> I'm ok as far as I'm concerned and would like to do the same with
> OpenSMTPD to reduce delta with portable ;-)
hmm.
> > +#ifdef IPV6_V6ONLY
> > + if (la->sa.ss_family == AF_INET6 && setsockopt(la->fd,
> > + IPPROTO_IPV6, IPV6_V6ONLY, &on, sizeof(on)) == -1)
> > + log_warn("setsockopt IPV6_V6ONLY");
> > +#endif
this is exactly what is supposed to live in the portable imho, to not
clutter the native sources.
Yes, it's small.
Where exactly do you draw the line?
--
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: 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/
Re: [patch] cleaner checksum modification for pf
* Alexandr Nedvedicky [2015-09-29 12:17]: > I have not looked at current checksum handling at PF on OpenBSD, so can't tell > exactly what's going on there. I feel PF does not bother too much with > updating > the checksum, when it changes the packet. It seems to me the > in_proto_cksum_out() gets called as soon as outbound packet gets inspected by > pf_test() to calculate/fix checksums. It looks like in_proto_cksum_out() has > to > recalculate checksum in SW for entire packet, when underlying HW does not > offer > checksum offload. Is that right? Or am I missing some piece? Basically. Packets that are modified by pf or are locally originated get "needs checksumming" flags (there are a few actually). in_proto_cksum_out basically emulates the hw cksum engine if we don't have one. I consider having one the norm these days. -- 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: [patch] cleaner checksum modification for pf
* Martin Pieuchot [2015-09-11 13:54]: > On 11/09/15(Fri) 13:28, Henning Brauer wrote: > > Ryan pointed me to this diff and we briefly discussed it; we remain > > convinced that the in-tree approach is better than this. > Could you elaborate why? Well we've been thru it more than once; the argument presented here was that modifying the cksum instead of verify + recalc is better as it wouldn't hide cksum mismatches if the cksum engines on the NICs we offload to misbehave. After many years with the verify + recalc approach I think it is pretty safe to say that this is of no concern... And given that, the approach that has less and simpler code and makes better use of offloading wins. there's a more elaborate discussion with exactly the same people in teh archives from around the time the cksum rewrite hit the tree, with the same conclusion. -- 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: [patch] cleaner checksum modification for pf
Ryan pointed me to this diff and we briefly discussed it; we remain convinced that the in-tree approach is better than this. -- 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-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: 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/
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: 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/
Re: ospfd announces carp interface with physical link down
* Johan Ymerson [2015-05-19 19:25]: > On Tue, 2015-05-19 at 11:16 +, Johan Ymerson wrote: > > On Tue, 2015-05-19 at 11:24 +0100, Stuart Henderson wrote: > > > On 2015/05/19 10:10, Johan Ymerson wrote: > > > Yes I understand that, but if carp init was counted in "LINK_STATE_DOWN" > > > then the metric would be 65535 which I think would still avoid the > > > problem you're seeing, and would involve less special-casing in ospfd. > > Yes, that would also resolve the issue, but it is a bit illogical to > > announce a network we cannot possibly route traffic to (due to hardware > > problems). > After some more testing I think we can conclude that this is most > definitely a kernel issue. hmm. there's definately more to it. > If the network cable is unplugged while the machine is running, ifconfig > reports the following: > carp2: flags=8803 mtu 1500 > lladdr 00:00:5e:00:01:03 > priority: 0 > carp: INIT carpdev em2 vhid 3 advbase 1 advskew 100 > groups: carp > status: invalid > inet 192.168.254.1 netmask 0xfe00 broadcast 192.168.255.255 > In this case network is announced with the correct metric (65535). > > However, if the machine is started with the cable unplugged, ifconfig > instead reports this: > carp2: flags=8803 mtu 1500 > lladdr 00:00:5e:00:01:03 > priority: 0 > carp: INIT carpdev em2 vhid 3 advbase 1 advskew 100 > groups: carp > inet 192.168.254.1 netmask 0xfe00 broadcast 192.168.255.255 > In this case the network is incorrectly announced as available with low > metric. > > Initializing if_link_state in the kernel carp code does seem to fix the > issue: > Index: sys/netinet/ip_carp.c > === > RCS file: /cvs/src/sys/netinet/ip_carp.c,v > retrieving revision 1.247 > diff -u -p -r1.247 ip_carp.c > --- sys/netinet/ip_carp.c 4 Mar 2015 10:59:52 - 1.247 > +++ sys/netinet/ip_carp.c 19 May 2015 17:22:18 - > @@ -751,6 +751,7 @@ carp_clone_create(ifc, unit) > ifp->if_addrlen = ETHER_ADDR_LEN; > ifp->if_hdrlen = ETHER_HDR_LEN; > ifp->if_mtu = ETHERMTU; > + ifp->if_link_state = LINK_STATE_INVALID; > IFQ_SET_MAXLEN(&ifp->if_snd, IFQ_MAXLEN); > IFQ_SET_READY(&ifp->if_snd); > if_attach(ifp); just for completeness: LINK_STATE_INVALID is 1, and that's what carp_set_state uses for everything but master and backup. so far so good. ifp is part of the sc which in turn is malloc'd with M_ZERO in carp_clone_create, so link state will be 0 aka LINK_STATE_UNKNOWN. however, at the end of carp_clone_create, we call carp_set_state_all(sc, INIT) which should take care of that. Now the question is why that doesn't work, your one-liner above SHOULD not make a difference. Either the fact that you set the link state before if_attach() makes a difference (I don't see how atm), or something isn't working as expected/intended in carp_set_state_all() resp. its sibling carp_set_state(). printf debugging time? -- 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: Small ifconfig output tweak for inet6?
* Florian Obser [2015-03-26 18:36]: > On Thu, Mar 26, 2015 at 05:46:12PM +0100, Henning Brauer wrote: > > * Mike Belopuhov [2015-03-26 14:36]: > > > however I agree that if we do this for ipv6 we should do it for ipv4 as > > > well > > > but then do we care about tons of stuff out there parsing ifconfig output? > > that's the prime question. I would love to move to CIDR notation - are > > we breaking people's scripts with that? The inet side has been the same > > for, what, decades? > Of course this breaks stuff :) uh, now that you mention it, I didn't chose a very obvious way to ask the question - of course the ifconfig output change breaks scripts parsing ifconfig output, the real question being: how common are scripts doing that? -- 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: Small ifconfig output tweak for inet6?
* Mike Belopuhov [2015-03-26 14:36]: > On 26 March 2015 at 14:27, Stuart Henderson wrote: > > seems reasonable. (I'd quite like that for v4 too, though it wouldn't > > cope with non-contiguous netmask ;) > non-contiguous netmasks for IPv4 addresses configured on an interface? > is that possible? what's the use case? > perhaps you're confusing this with non-contiguous netmasks in the radix > tree that are entered by the ipsec flows containing port numbers? I don't think we need to worry about non-contiguous netmasks here. > however I agree that if we do this for ipv6 we should do it for ipv4 as well > but then do we care about tons of stuff out there parsing ifconfig output? that's the prime question. I would love to move to CIDR notation - are we breaking people's scripts with that? The inet side has been the same for, what, decades? -- 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/
A thanks to the donors, and a small request
The OpenBSD foundation has just acquired 4 Dell r210s for my OpenBSD development setup to replace their aging predecessors from 2007. I would like to take the opportunity to thank everybody who has donated to the foundation, you made this possible. To complete the setup, I need at least 2 single and one dual port 10GBaseT ix(4) cards. There is one previously donated on in Australia that I could use, unfortunately we cannot quite figure out right now whether it is single or dual - depending on that, I'll need 2 single or 1 single and 1 dual port one on top. The machines come without the rackmount rails, having them would make it considerably easier for me - for regular 4-post racks. Since the time I have these days for hacking is already scarce, I would really prefer these things to just show up, rather than having to find & order them myself - that leaves more time for actual OpenBSD hacking. If you can help with any of these items, please drop me a mail. I'm located in Hamburg, Germany. Thanks.
Re: Authenticated TLS "contraints" in ntpd(8)
* Henning Brauer [2015-02-10 13:21]: > * Kevin Chadwick [2015-02-10 13:14]: > > On Tue, 10 Feb 2015 10:55:53 +0100 > > Reyk Floeter wrote: > > > The standardized attempts to add authentication to NTP are a) fairly > > > horrible (ASN.1 etc.) and b) rarely deployed. > > When ntpd acts as a server, could the package signing code be of use > > with ntpd keys? > getting the signature into the ntp packets in a way that doesn't break > compatibility is the challenge. let me elaborate slightly: even if we came up with a überauth mechanism that doesn't suck and doesn't break compat, it wouldn't be of much use unless the servers you sync from support it - one of the pools for most. even if you could completely trust them and whatever model of key distribution, for them to support this, you have to get it standarized. and even if we managed to get it pushed through the standards corpses^Wbodies, it would take ages until it gets deployed, IF it ever gets widely deployed. That's a lot of ifs, I leave the judgement on likeliness to you. constraints from https, however - that works today. -- 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 congestion handling
I already talked to dlg here, but that obviously cuts you out which isn't good :/ * Alexander Bluhm [2015-02-10 23:12]: > We do not use the pf congestion feature, we have disabled it with > an #ifdef. Prefering states over rules means that you cannot login > into a congested box. There are cases where this policy makes > sense, in our use case it does not. well, you can over the console, and preferring existing states is the right thing to do imo: -(D)DoS-Traffic won't match a state, prefering states means that your legit traffic has a much higher chance -ruleset evaluation is MUCH more expensive than state matching not sure whether your product is the extreme outlier here; if it is then the "you have to ifdef it out" is perfectly acceptable imo; if it isn't we might need a button (shrug). the fact that nobody asked for a button or the like in a decade makes me tend towards "not needed". > I can't see cases where different congestion states for each input > queue are useful. me neither. things are significantly different now than they were 10+ years ago when bob & I chose "ipintrq full" as trigger. the congestion trick isn't as effective any more as it used to be since the arrival of MCLGETI - which overall is much more effective, and less selective at the same time. the two should cooperate.
Re: Authenticated TLS "contraints" in ntpd(8)
* Kevin Chadwick [2015-02-10 13:14]: > On Tue, 10 Feb 2015 10:55:53 +0100 > Reyk Floeter wrote: > > The standardized attempts to add authentication to NTP are a) fairly > > horrible (ASN.1 etc.) and b) rarely deployed. > When ntpd acts as a server, could the package signing code be of use > with ntpd keys? getting the signature into the ntp packets in a way that doesn't break compatibility is the challenge. -- 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/
pfsync: include set prio
what $subject says.
no real compat issue since we're using spare bytes.
old -> new ends up with set prio (0, 0) equivalent
new -> old is entirely harmless, old ignores the prios.
acceptable imo, since the effects of set prio aren't all THAT big and
the other option, bumping the pfsync version, is much more intrusive
and leads to no syncing between old and new at all.
Index: net/if_pfsync.c
===
RCS file: /cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.216
diff -u -p -r1.216 if_pfsync.c
--- net/if_pfsync.c 24 Jan 2015 00:29:06 - 1.216
+++ net/if_pfsync.c 9 Feb 2015 13:19:58 -
@@ -594,6 +594,8 @@ pfsync_state_import(struct pfsync_state
st->max_mss = ntohs(sp->max_mss);
st->min_ttl = sp->min_ttl;
st->set_tos = sp->set_tos;
+ st->set_prio[0] = sp->set_prio[0];
+ st->set_prio[1] = sp->set_prio[1];
st->id = sp->id;
st->creatorid = sp->creatorid;
Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.901
diff -u -p -r1.901 pf.c
--- net/pf.c7 Feb 2015 09:15:25 - 1.901
+++ net/pf.c9 Feb 2015 13:20:55 -
@@ -1142,6 +1142,8 @@ pf_state_export(struct pfsync_state *sp,
sp->max_mss = htons(st->max_mss);
sp->min_ttl = st->min_ttl;
sp->set_tos = st->set_tos;
+ sp->set_prio[0] = st->set_prio[0];
+ sp->set_prio[1] = st->set_prio[1];
}
/* END state table stuff */
Index: net/pfvar.h
===
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.409
diff -u -p -r1.409 pfvar.h
--- net/pfvar.h 7 Feb 2015 06:27:46 - 1.409
+++ net/pfvar.h 9 Feb 2015 13:18:34 -
@@ -914,7 +914,7 @@ struct pfsync_state {
u_int8_t min_ttl;
u_int8_t set_tos;
u_int16_tstate_flags;
- u_int8_t pad[2];
+ u_int8_t set_prio[2];
} __packed;
#define PFSYNC_FLAG_SRCNODE0x04
--
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/
pf log(matches to pflog42)
LQ_NEXT(r, entries);
@@ -3293,8 +3296,10 @@ pf_test_rule(struct pf_pdesc *pd, struct
}
REASON_SET(&reason, PFRES_MATCH);
- if (r->log || act.log & PF_LOG_MATCHES)
- PFLOG_PACKET(pd, reason, r, a, ruleset);
+ if (r->log)
+ PFLOG_PACKET(pd, reason, r, a, ruleset, NULL);
+ if (act.log & PF_LOG_MATCHES)
+ pf_log_matches(pd, r, a, ruleset, &rules);
if (pd->virtual_proto != PF_VPROTO_FRAGMENT &&
(r->action == PF_DROP) &&
@@ -6543,12 +6548,12 @@ done:
struct pf_rule_item *ri;
if (pd.pflog & PF_LOG_FORCE || r->log & PF_LOG_ALL)
- PFLOG_PACKET(&pd, reason, r, a, ruleset);
+ PFLOG_PACKET(&pd, reason, r, a, ruleset, NULL);
if (s) {
SLIST_FOREACH(ri, &s->match_rules, entry)
if (ri->r->log & PF_LOG_ALL)
PFLOG_PACKET(&pd, reason, ri->r, a,
- ruleset);
+ ruleset, NULL);
}
}
@@ -6677,4 +6682,19 @@ void
pf_pkt_addr_changed(struct mbuf *m)
{
m->m_pkthdr.pf.statekey = NULL;
+}
+
+void
+pf_log_matches(struct pf_pdesc *pd, struct pf_rule *rm, struct pf_rule *am,
+struct pf_ruleset *ruleset, struct pf_rule_slist *matchrules)
+{
+ struct pf_rule_item *ri;
+
+ /* if this is the log(matches) rule, packet has been logged already */
+ if (rm->log & PF_LOG_MATCHES)
+ return;
+
+ SLIST_FOREACH(ri, matchrules, entry)
+ if (ri->r->log & PF_LOG_MATCHES)
+ PFLOG_PACKET(pd, PFRES_MATCH, rm, am, ruleset, ri->r);
}
Index: net/pfvar.h
===
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.409
diff -u -p -r1.409 pfvar.h
--- net/pfvar.h 7 Feb 2015 06:27:46 - 1.409
+++ net/pfvar.h 8 Feb 2015 05:10:02 -
@@ -1814,7 +1814,7 @@ void pf_change_a(struct pf_pdesc *, void
intpf_check_proto_cksum(struct pf_pdesc *, int, int, u_int8_t,
sa_family_t);
intpflog_packet(struct pf_pdesc *, u_int8_t, struct pf_rule *,
- struct pf_rule *, struct pf_ruleset *);
+ struct pf_rule *, struct pf_ruleset *, struct pf_rule *);
void pf_send_deferred_syn(struct pf_state *);
intpf_match_addr(u_int8_t, struct pf_addr *, struct pf_addr *,
struct pf_addr *, sa_family_t);
--
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/
pf match on prio
"rtable" number | "probability" number"%" |
+ "rtable" number | "probability" number"%" | "prio" number |
"af-to" af "from" ( redirhost | "{" redirhost-list "}" )
[ "to" ( redirhost | "{" redirhost-list "}" ) ] |
"binat-to" ( redirhost | "{" redirhost-list "}" )
Index: sys/net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.901
diff -u -p -r1.901 pf.c
--- sys/net/pf.c7 Feb 2015 09:15:25 - 1.901
+++ sys/net/pf.c7 Feb 2015 23:59:41 -
@@ -3228,6 +3228,9 @@ pf_test_rule(struct pf_pdesc *pd, struct
PF_TEST_ATTRIB((r->rcv_kif && pf_match_rcvif(pd->m, r) ==
r->rcvifnot),
TAILQ_NEXT(r, entries));
+ PF_TEST_ATTRIB((r->prio != 0xff &&
+ r->prio != pd->m->m_pkthdr.pf.prio),
+ TAILQ_NEXT(r, entries));
/* FALLTHROUGH */
if (r->tag)
Index: sys/net/pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.281
diff -u -p -r1.281 pf_ioctl.c
--- sys/net/pf_ioctl.c 24 Jan 2015 00:29:06 - 1.281
+++ sys/net/pf_ioctl.c 7 Feb 2015 23:57:51 -
@@ -2459,6 +2459,7 @@ pf_rule_copyin(struct pf_rule *from, str
to->divert.port = from->divert.port;
to->divert_packet.addr = from->divert_packet.addr;
to->divert_packet.port = from->divert_packet.port;
+ to->prio = from->prio;
to->set_prio[0] = from->set_prio[0];
to->set_prio[1] = from->set_prio[1];
Index: sys/net/pfvar.h
===
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.409
diff -u -p -r1.409 pfvar.h
--- sys/net/pfvar.h 7 Feb 2015 06:27:46 - 1.409
+++ sys/net/pfvar.h 7 Feb 2015 23:37:57 -
@@ -644,10 +644,11 @@ struct pf_rule {
#define PF_FLUSH 0x01
#define PF_FLUSH_GLOBAL0x02
u_int8_t flush;
+ u_int8_t prio;
u_int8_t set_prio[2];
sa_family_t naf;
u_int8_t rcvifnot;
- u_int8_t pad[3];
+ u_int8_t pad[2];
struct {
struct pf_addr addr;
--
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/
wrong mac address used with carp and unnumbered carpdevs
so, carp interface with underlaying unnumbered carpdev, i. e.
ifconfig em1 up
ifconfig carp0 carpdev em1 vhid 0 ... 10.0.0.1/24
carp announcements and some stuff like arp goes out with the carp
interface mac address, fine.
however, IP traffic goes out with the carpdev's mac, which is wrong
and leads to problems in places with a strict mac address regime -
exchange points are a typical case.
the culprit is sys/net/if_ethersubr.c ether_output().
The ifp passed to ether_output is (usually) determined by looking up
the route to the destination and grabbing the ifp from it. So in the
numbered carpdev case (em1 10.0.0.x/24, carp 10.0.0.y/32) it'll be the
carpdev (em1 here) right away. In the unnumbered carpdev case, it'll be
the carp interface itself. ether_output has a hack to exchange the carp
ifp with the carpdev's one, to send out the frame on the carpdev and
not the carp if. This little hack is before the src mac address is
determined tho, and that is the bug.
ok?
Index: if_ethersubr.c
===
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.175
diff -u -p -r1.175 if_ethersubr.c
--- if_ethersubr.c 7 Oct 2014 20:23:32 - 1.175
+++ if_ethersubr.c 28 Oct 2014 12:18:36 -
@@ -270,6 +270,8 @@ ether_output(struct ifnet *ifp0, struct
senderr(EBUSY);
#endif
+ esrc = ac->ac_enaddr;
+
#if NCARP > 0
if (ifp->if_type == IFT_CARP) {
ifp = ifp->if_carpdev;
@@ -310,7 +312,6 @@ ether_output(struct ifnet *ifp0, struct
time_second < rt->rt_rmx.rmx_expire)
senderr(rt == rt0 ? EHOSTDOWN : EHOSTUNREACH);
}
- esrc = ac->ac_enaddr;
switch (dst->sa_family) {
#ifdef INET
--
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: IPv6 packet refragmentation in pf(4)
* Ian Grant [2014-10-25 18:15]:
> #ifdef INET6
> /* if reassembled packet passed, create new fragments */
> if (pf_status.reass && action == PF_PASS && *m0 && fwdir == PF_FWD) {
> struct m_tag*mtag;
>
> if ((mtag = m_tag_find(*m0, PACKET_TAG_PF_REASSEMBLED, NULL)))
> action = pf_refragment6(m0, mtag, fwdir);
> }
> #endif
>
> But from pf.c, the fn pf_test is only ever called with fwdir == PF_OUT
> and the only other places from which it is called are in if_bridge.c,
> and they set fwdir to either PF_OUT or PF_IN (aliased as BRIDGE_OUT
> and BRIDGE_IN respectively, when PF is enabled). Therefore fwdir ==
> PF_FWD never holds, and so reassembled IPv6 packets are never
> refragmented, contradicting the manual page pf.conf(5).
you need to improve your grepping skills :)
netinet6/ip6_forward.c:348: pf_test(AF_INET6, PF_FWD, encif,
&m, NULL) != PF_PASS) {
netinet6/ip6_forward.c:459: if (pf_test(AF_INET6, PF_FWD, rt->rt_ifp, &m,
NULL) != PF_PASS) {
--
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: pppoe(4), add example for ipv6
* Andres Perera [2014-10-23 17:14]:
> [What I'm pointing out below looks like a mistake regardless of the
> variables in scope.]
> > void
> > +addaf(const char *vname, int value)
> > +{
>
> ^ *v*name
>
> > + struct if_afreq ifar;
> > +
> > + strlcpy(ifar.ifar_name, name, sizeof(ifar.ifar_name));
>
> ^ name
you're absolutely right; it works correctly nontheless because of the
global "name" var that happens to carry the ifname, too... oh
ifconfig.
fixed, thx.
--
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: pppoe(4), add example for ipv6
* Chris Cappuccio [2014-10-22 01:11]:
> Stuart Henderson [st...@openbsd.org] wrote:
> > Any comments on the diff in this?
> >
> > > +#ifdef INET6
> > > + sc->sc_sppp.pp_if.if_xflags &= ~IFXF_NOINET6;
> > > +#endif
> Aside from what Stefan said, isn't this flag going to be removed
> in favor of a flag that explicitly enables INET6 for interfaces?
remove yes, no need for a new one.
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.287
diff -u -p -r1.287 ifconfig.c
--- sbin/ifconfig/ifconfig.c12 Jul 2014 19:58:17 - 1.287
+++ sbin/ifconfig/ifconfig.c3 Oct 2014 12:58:22 -
@@ -148,6 +148,7 @@ voidsetiflladdr(const char *, int);
void setifdstaddr(const char *, int);
void setifflags(const char *, int);
void setifxflags(const char *, int);
+void addaf(const char *, int);
void removeaf(const char *, int);
void setifbroadaddr(const char *, int);
void setifmtu(const char *, int);
@@ -682,7 +683,7 @@ main(int argc, char *argv[])
}
#ifdef INET6
if (argc != 0 && af == AF_INET6)
- setifxflags("inet6", -IFXF_NOINET6);
+ addaf(name, AF_INET6);
#endif
while (argc > 0) {
const struct cmd *p;
@@ -1258,18 +1259,25 @@ setifxflags(const char *vname, int value
}
void
+addaf(const char *vname, int value)
+{
+ struct if_afreq ifar;
+
+ strlcpy(ifar.ifar_name, name, sizeof(ifar.ifar_name));
+ ifar.ifar_af = value;
+ if (ioctl(s, SIOCIFAFATTACH, (caddr_t)&ifar) < 0)
+ warn("SIOCIFAFATTACH");
+}
+
+void
removeaf(const char *vname, int value)
{
- switch (value) {
-#ifdef INET6
- case AF_INET6:
- setifxflags(vname, IFXF_NOINET6);
- setifxflags(vname, -IFXF_AUTOCONF6);
- break;
-#endif
- default:
- errx(1, "removeaf not implemented for this AF");
- }
+ struct if_afreq ifar;
+
+ strlcpy(ifar.ifar_name, name, sizeof(ifar.ifar_name));
+ ifar.ifar_af = value;
+ if (ioctl(s, SIOCIFAFDETACH, (caddr_t)&ifar) < 0)
+ warn("SIOCIFAFDETACH");
}
#ifdef INET6
@@ -1331,7 +1339,9 @@ setia6eui64(const char *cmd, int val)
if (afp->af_af != AF_INET6)
errx(1, "%s not allowed for the AF", cmd);
- setifxflags("inet6", -IFXF_NOINET6);
+#ifdef INET6
+ addaf(name, AF_INET6);
+#endif
in6 = (struct in6_addr *)&in6_addreq.ifra_addr.sin6_addr;
if (memcmp(&in6addr_any.s6_addr[8], &in6->s6_addr[8], 8) != 0)
errx(1, "interface index is already filled");
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.301
diff -u -p -r1.301 if.c
--- sys/net/if.c30 Sep 2014 08:27:57 - 1.301
+++ sys/net/if.c3 Oct 2014 12:59:29 -
@@ -428,10 +428,6 @@ if_attach(struct ifnet *ifp)
#else
TAILQ_INSERT_TAIL(&ifnet, ifp, if_list);
#endif
-#ifdef INET6
- ifp->if_xflags |= IFXF_NOINET6;
-#endif
-
if_attachsetup(ifp);
}
@@ -1142,11 +1138,6 @@ if_up(struct ifnet *ifp)
bstp_ifstate(ifp);
#endif
rt_ifmsg(ifp);
-#ifdef INET6
- if (!(ifp->if_xflags & IFXF_NOINET6))
- in6_if_up(ifp);
-#endif
-
#ifndef SMALL_KERNEL
rt_if_track(ifp);
#endif
@@ -1246,6 +1237,7 @@ ifioctl(struct socket *so, u_long cmd, c
struct ifaddr *ifa;
struct sockaddr_dl *sdl;
struct ifgroupreq *ifgr;
+ struct if_afreq *ifar;
char ifdescrbuf[IFDESCRSIZE];
char ifrtlabelbuf[RTLABEL_LEN];
int s, error = 0;
@@ -1280,6 +1272,28 @@ ifioctl(struct socket *so, u_long cmd, c
if ((error = suser(p, 0)) != 0)
return (error);
return (if_setgroupattribs(data));
+ case SIOCIFAFATTACH:
+ case SIOCIFAFDETACH:
+ if ((error = suser(p, 0)) != 0)
+ return (error);
+ ifar = (struct if_afreq *)data;
+ if ((ifp = ifunit(ifar->ifar_name)) == NULL)
+ return (ENXIO);
+ switch (ifar->ifar_af) {
+#ifdef INET6
+ case AF_INET6:
+ s = splnet();
+ if (cmd == SIOCIFAFATTACH) {
+ if (in6ifa_ifpforlinklocal(ifp, 0) == NULL)
+ in6_if_up(ifp);
+ } else
+ in6_ifdetach(ifp);
+ splx(s);
+ return (0);
+#endif /* INET6 */
+ default:
+ return (EAFNOSUPPORT);
+ }
}
ifp = ifunit(ifr->ifr_name);
@@ -1335,25 +1349,26 @@ ifioctl(struct socket *so, u_long cmd, c
case SIOCSIFXFLAGS:
if ((e
Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.
* Adam [2014-09-19 10:37]: > On Fri, 19 Sep 2014 00:14:47 -0700 > Doug Hogan wrote: > > On Thu, Sep 18, 2014 at 09:28:41AM +0100, bytevolc...@safe-mail.net > > wrote: > > > + strlcpy(mountpoint, src, sizeof(mountpoint)); > > > > The strlcpy call should check for truncation. There's no guarantee > > that src is less than sizeof(mountpoint) since src = template = > > optarg. > > According to man strlcpy(3): > > "strlcpy() copies up to dstsize - 1 characters from the string src to > dst, NUL-terminating the result if dstsize is not 0." > > Thus, such a check here would be redundant. HUH? Doug is entirely right. src is user controlled and can be larger than mountpoint. In that case, we want to bail and whine at the user instead of silently truncating and going on. -- 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: arp(8) output and expire timer
* Martin Pieuchot [2014-08-18 11:03]: > On 15/08/14(Fri) 10:43, Henning Brauer wrote: > > * Stuart Henderson [2014-08-15 10:29]: > > > On 2014/08/12 15:46, Martin Pieuchot wrote: > > > > I find arp(8) output really difficult to read, but more importantly it > > > > does not print the expire time of non permanent entries like ndp(8). > > > > So the diff below change arp(8)'s output to be more similar to ndp(8)'s > > > > one. > > > Personally I like the extra information from the timer, > > same here > > > but not the big change of format (I find the extra whitespace makes > > > it harder to see which MAC address goes with each IP address) > > I actually like the proposed new format there better. > > > or loss of IP addresses where a name exists. > > here I agree with stuart. > Well I couldn't came with a better trade-off. The actual output does > not fit in 80 columns as soon as a FQDN is a bit long and adding the > timer information does not help. So instead of reinventing an output, > I tried to match what ndp does. > At least with this diff the "-n" flag is coherent with what route(8), > netsat(8) and ndp(8) do. > So I hear what you say but I don't see which output can address the > points you raised. > So unless somebody has a better idea, I'd like to commit this so that > we can get use to the new output 8) fair enough. ok. -- 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: arp(8) output and expire timer
* Stuart Henderson [2014-08-15 10:29]: > On 2014/08/12 15:46, Martin Pieuchot wrote: > > I find arp(8) output really difficult to read, but more importantly it > > does not print the expire time of non permanent entries like ndp(8). > > > > So the diff below change arp(8)'s output to be more similar to ndp(8)'s > > one. > Personally I like the extra information from the timer, same here > but not the big change of format (I find the extra whitespace makes > it harder to see which MAC address goes with each IP address) I actually like the proposed new format there better. > or loss of IP addresses where a name exists. here I agree with stuart. -- 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/
trunk on RAMDISK_CD
naddy's mpbios RAMDISK_CD mail reminded me that I have put trunk into RAMDISK_CD here for some time. Without, upgrading of machines with trunk is unecessarily hard, since, well, you won't get your trunk interface which typically has the IP(s) on it and you need to resort to manual network config w/ one of the underlaying interfaces. I can't for the sake of it remember whether I actually used the resulting bsd.rd on trunk'd machines (that's what you get for not mailing out the diff rigth away, sigh). Anybody has a chance to give this a spin on such a system? Would be really nice to have this in before release to make upgrades easier for trunk users. Index: sys/arch/amd64/conf/RAMDISK_CD === RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK_CD,v retrieving revision 1.142 diff -u -p -r1.142 RAMDISK_CD --- sys/arch/amd64/conf/RAMDISK_CD 14 Jul 2014 09:51:16 - 1.142 +++ sys/arch/amd64/conf/RAMDISK_CD 15 Jul 2014 10:41:29 - @@ -344,6 +344,7 @@ pseudo-device loop1 # network loopback #pseudo-device sl 1 # CSLIP #pseudo-device ppp 1 # PPP pseudo-device vlan# IEEE 802.1Q VLAN +pseudo-device trunk # Trunking support pseudo-device bpfilter 1 # packet filter pseudo-device rd 1 # ramdisk pseudo-device wsmux 2 Index: sys/arch/i386/conf/RAMDISK_CD === RCS file: /cvs/src/sys/arch/i386/conf/RAMDISK_CD,v retrieving revision 1.216 diff -u -p -r1.216 RAMDISK_CD --- sys/arch/i386/conf/RAMDISK_CD 12 Jul 2014 21:56:56 - 1.216 +++ sys/arch/i386/conf/RAMDISK_CD 15 Jul 2014 10:41:37 - @@ -414,6 +414,7 @@ pseudo-device loop1 # network loopback #pseudo-device sl 1 # CSLIP #pseudo-device ppp 1 # PPP pseudo-device vlan# IEEE 802.1Q VLAN +pseudo-device trunk # Trunking support pseudo-device bpfilter 1 # packet filter pseudo-device rd 1 # ramdisk pseudo-device wsmux 2 -- 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: IFXF_NOINET doesn't make sense any more
* Stefan Sperling [2014-07-15 12:35]: > On Tue, Jul 15, 2014 at 12:15:12PM +0200, Henning Brauer wrote: > > I'm slightly undecided on whether this should make this release or > > not... > In that situation, I usually decide that the risk won't outweigh > the benefits of just waiting for a while. No change means nobody > can get hurt. let me reformulate that as "haven't made my mind up". first step of coirse is some form of risk assessment, and that risk seems to be rather small. i can perfectly live with postponing this to after release as well, not pushing. > > kernel-rtsol should make release imo. > I haven't seen a working diff yet. I don't think he mailed it out yet. Sitting next to each otehr in Ljubljana had some benefits :) > Last time I talked to florian there were still some open questions. tiny nits > Generally, I think it's non-trivial to > move rtsold into the kernel so we should give such a change a soak period > of at least a few weeks (I don't know if there is enough time left for > that, but perhaps not). The kernel environment has much more potential > for hidden side-effects. oh I'm NOT suggesting to remove rtsol/d yet. It's not like they would conflict with the kernel-based rtsol handling, at worst you send a solicitation too much, which really is no big deal. but again, I wouldn't have a problem with postponing this to after release either. -- 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: IFXF_NOINET doesn't make sense any more
* Stefan Sperling [2014-07-15 11:06]: > On Sun, Jul 13, 2014 at 03:48:47PM +0200, Henning Brauer wrote: > > now that we have an uncontaminated, err, inet6-free system by default, > > IFXF_NOINET6 just doesn't make sense any more. > > fully go for no inet6 by default, get rid of the IFXF_NOINET6 guarded > > attachments etc. > > introduce IFAFATTACH and IFAFDETACH ioctls. note that they are NOT > > inet6 specific; the kernel only has code for inet6 so far and will > > properly return EAFNOSUPPORT for all others. > > > > there should be no user visible changes from this. > > I like this direction. > It's a lot better than the AF-specific "kill switch". oh absolutely. and yes i plan to add "ifconfig -inet" as a convenient way to get rid of all v4 addrs at once; inet doesn't really need that since it just doesn't have sth like the dreaded link-locals and the rtadv magic, but symmetry is good. and who knows whether we'll need it for inet7 anyway :) > However, since we're heading towards release, I believe we should wait > and do this flag removal + ioctl addition later. We've already got the > no-link-local-by-default change, and the new autoconf6 flag. > I don't see much harm in leaving it at that for this release cycle and > moving further in the next one. I'm looking forward to florian's in-kernel > rtsol as well, in the next cycle (it's not done yet anyway AFAIK). florian is done with that really, we're at the "last tiny nits" stage, polishing that could as well happen in-tree. I'm slightly undecided on whether this should make this release or not... it is actually kinda mechanic, i. e. we already know the relevant places since they are the ones that had the NONINET6 flag check before. I do think the risk is not big, so if we get some solid tests we should be golden. I for one haven't been able to make it misbehave no matter how hard I tried. kernel-rtsol should make release imo. > Perhaps we can also sort out the autoconf vs. ip6forward conundrum then, > and ship working IPv6 for SOHO routers in 5.7. yeah that limitation is bizarre - straight from the spec and not added by us, tho. > I'll be testing this regardless and will let you know if I run into issues. coolio, thx -- 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/
network autoconfig
before I forget half of what we talked about here and to share it, here's how I think the network autoconfig stuff should work in the future. from the user's PoV, there shouldn't be more needed than ifconfig inet autoconf ifconfig inet6 autoconf aka inet/inet6 autoconf in hostname.if. for inet6, we're almost there with the work done by florian and me from this week. not all of that is in yet. now for inet, I'm pretty damn certain we don't want to do DHCP in the kernel. turning on autoconf should lead to a new message on the routing socket, and a little userland daemon, let's call it autoconfd for the moment, reacts to that message insofar that it sends out the dhcp request(s) and deals with the answers. using the dhclient code of course. open question is what to do if inet autoconf is on on more than one interface - gotta select the default route somehow. inet6 does that and does it very different. we'll figure sth out it. if needed/useful dhcp6 could be integrated into that scheme kinda easily, too. i won't judge on whether that is the case; it isn't relevant yet. for the nameservers: in the inet case, autoconfd would know them from dhcp. i've been told the inet6 RAs can contain nameserver information, too. we can pass those up from the kernel using a message on the routing socket as well (this is inspired by the kernel-ipsec <-> iked/isakmpd model). the resolv.conf writing races / fights would be gone since autoconfd is the only one dealing with it. of course i don't insist on implementing all that myself, not remotely. -- 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/
IFXF_NOINET doesn't make sense any more
now that we have an uncontaminated, err, inet6-free system by default,
IFXF_NOINET6 just doesn't make sense any more.
fully go for no inet6 by default, get rid of the IFXF_NOINET6 guarded
attachments etc.
introduce IFAFATTACH and IFAFDETACH ioctls. note that they are NOT
inet6 specific; the kernel only has code for inet6 so far and will
properly return EAFNOSUPPORT for all others.
there should be no user visible changes from this.
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.287
diff -u -p -r1.287 ifconfig.c
--- sbin/ifconfig/ifconfig.c12 Jul 2014 19:58:17 - 1.287
+++ sbin/ifconfig/ifconfig.c13 Jul 2014 13:05:15 -
@@ -148,6 +148,7 @@ voidsetiflladdr(const char *, int);
void setifdstaddr(const char *, int);
void setifflags(const char *, int);
void setifxflags(const char *, int);
+void addaf(const char *, int);
void removeaf(const char *, int);
void setifbroadaddr(const char *, int);
void setifmtu(const char *, int);
@@ -682,7 +683,7 @@ main(int argc, char *argv[])
}
#ifdef INET6
if (argc != 0 && af == AF_INET6)
- setifxflags("inet6", -IFXF_NOINET6);
+ addaf(name, AF_INET6);
#endif
while (argc > 0) {
const struct cmd *p;
@@ -1258,18 +1259,25 @@ setifxflags(const char *vname, int value
}
void
+addaf(const char *vname, int value)
+{
+ struct if_afreq ifar;
+
+ strlcpy(ifar.ifar_name, name, sizeof(ifar.ifar_name));
+ ifar.ifar_af = value;
+ if (ioctl(s, SIOCIFAFATTACH, (caddr_t)&ifar) < 0)
+ warn("SIOCIFAFATTACH");
+}
+
+void
removeaf(const char *vname, int value)
{
- switch (value) {
-#ifdef INET6
- case AF_INET6:
- setifxflags(vname, IFXF_NOINET6);
- setifxflags(vname, -IFXF_AUTOCONF6);
- break;
-#endif
- default:
- errx(1, "removeaf not implemented for this AF");
- }
+ struct if_afreq ifar;
+
+ strlcpy(ifar.ifar_name, name, sizeof(ifar.ifar_name));
+ ifar.ifar_af = value;
+ if (ioctl(s, SIOCIFAFDETACH, (caddr_t)&ifar) < 0)
+ warn("SIOCIFAFDETACH");
}
#ifdef INET6
@@ -1331,7 +1339,9 @@ setia6eui64(const char *cmd, int val)
if (afp->af_af != AF_INET6)
errx(1, "%s not allowed for the AF", cmd);
- setifxflags("inet6", -IFXF_NOINET6);
+#ifdef INET6
+ addaf(name, AF_INET6);
+#endif
in6 = (struct in6_addr *)&in6_addreq.ifra_addr.sin6_addr;
if (memcmp(&in6addr_any.s6_addr[8], &in6->s6_addr[8], 8) != 0)
errx(1, "interface index is already filled");
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.297
diff -u -p -r1.297 if.c
--- sys/net/if.c12 Jul 2014 18:44:22 - 1.297
+++ sys/net/if.c13 Jul 2014 13:15:09 -
@@ -428,10 +428,6 @@ if_attach(struct ifnet *ifp)
#else
TAILQ_INSERT_TAIL(&ifnet, ifp, if_list);
#endif
-#ifdef INET6
- ifp->if_xflags |= IFXF_NOINET6;
-#endif
-
if_attachsetup(ifp);
}
@@ -1133,11 +1129,6 @@ if_up(struct ifnet *ifp)
bstp_ifstate(ifp);
#endif
rt_ifmsg(ifp);
-#ifdef INET6
- if (!(ifp->if_xflags & IFXF_NOINET6))
- in6_if_up(ifp);
-#endif
-
#ifndef SMALL_KERNEL
rt_if_track(ifp);
#endif
@@ -1237,6 +1228,7 @@ ifioctl(struct socket *so, u_long cmd, c
struct ifaddr *ifa;
struct sockaddr_dl *sdl;
struct ifgroupreq *ifgr;
+ struct if_afreq *ifar;
char ifdescrbuf[IFDESCRSIZE];
char ifrtlabelbuf[RTLABEL_LEN];
int s, error = 0;
@@ -1271,6 +1263,26 @@ ifioctl(struct socket *so, u_long cmd, c
if ((error = suser(p, 0)) != 0)
return (error);
return (if_setgroupattribs(data));
+ case SIOCIFAFATTACH:
+ case SIOCIFAFDETACH:
+ if ((error = suser(p, 0)) != 0)
+ return (error);
+ ifar = (struct if_afreq *)data;
+ if ((ifp = ifunit(ifar->ifar_name)) == NULL)
+ return (ENXIO);
+ switch (ifar->ifar_af) {
+ case AF_INET6:
+ s = splnet();
+ if (cmd == SIOCIFAFATTACH) {
+ if (in6ifa_ifpforlinklocal(ifp, 0) == NULL)
+ in6_if_up(ifp);
+ } else
+ in6_ifdetach(ifp);
+ splx(s);
+ return (0);
+ default:
+ return (EAFNOSUPPORT);
+ }
}
ifp = ifunit(ifr->ifr_name);
@@ -1327,25 +1339,6 @@ ifioctl(struct socket *so, u_long cmd, c
if ((error = suse
Re: sshd add back hmac-sha1
* Ted Unangst [2014-07-11 11:32]: > I think the proposal rampaging went one algorithm too far. sha1 is the > best algorithm supported by many clients and it's still pretty secure. > without it, a lot of clients have stopped working. temporarily alieve > the pain? yes, please. -- 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: lynx: disable old protocols
* Paul Irofti [2014-07-11 11:40]: > No, gopher can't go! just do pkg_gyp gopher to get over it. -- 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: lynx: disable old protocols
* Stuart Henderson [2014-07-11 10:49]: > Should we just move lynx to packages? hmm. having a simple text browser in base is worthwile imo. and if it is just to download sth where i don't know the exact URL. personally, I haven't used lynx for anything but http and https in... what, a decade? -- 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: tun TUNDOIOVEC ioctl
* Matthew Dempsky [2014-07-10 22:56]: > On Thu, Jul 10, 2014 at 1:20 PM, Ted Unangst wrote: > > Thoughts? > > Seems kind of hacky to me, but if it results in significant > performance improvements in real world uses, then I could be swayed > since it's not very intrusive either. indeed. -- 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: unify some bpf code
* Kent R. Spillner [2014-07-10 20:47]: > I saw this was already committed, but one tiny consistency nit inline below. I'd argue it's not consistency, rather the opposite, since: > > - mh.mh_len = 4; > > + bpf_mtap_hdr(arg, (caddr_t)&afh, 4, m, direction, NULL); you see this was very mechanic. however: > I realize this is kind of obvious because afh is declared immediately before > this, but to me that 4 looks out of place. Perhaps sizeof(afh) would be > better? I could not agree more. i'll commit it as soon as I find a tree in my forest that has a clean bpf.c :o -- 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: bpf_mtap_stripvlan
* Paul de Weerd [2014-07-10 14:33]: > On Thu, Jul 10, 2014 at 01:30:29PM +0100, Stuart Henderson wrote: > | On 2014/07/10 13:11, Henning Brauer wrote: > | > I committed the bpf chunk, but nothing is using it yet. pls give the > | > if_vlan.c chunk a spin. > | I think weerd@ might need something similar for bridge for his tv... > I'm testing this diff later today or tomorrow... can't help the bridge case, not using bpf. -- 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: bpf_mtap_stripvlan
* Stuart Henderson [2014-07-10 14:30]: > On 2014/07/10 13:11, Henning Brauer wrote: > > I committed the bpf chunk, but nothing is using it yet. pls give the > > if_vlan.c chunk a spin. > I think weerd@ might need something similar for bridge for his tv... the f&^(*$@&)($#@ bridge needs to die. I was told about some bridging-between-vlans issue yesterday (have no details tho), peeked into the bridge code and... found vlan tagging code. Sigh. whatever else does the job cannot be worse. -- 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: bpf_mtap_stripvlan
I committed the bpf chunk, but nothing is using it yet. pls give the
if_vlan.c chunk a spin.
Index: net/if_vlan.c
===
RCS file: /cvs/src/sys/net/if_vlan.c,v
retrieving revision 1.106
diff -u -p -r1.106 if_vlan.c
--- net/if_vlan.c 9 Jul 2014 09:30:49 - 1.106
+++ net/if_vlan.c 9 Jul 2014 14:09:43 -
@@ -213,7 +213,7 @@ vlan_start(struct ifnet *ifp)
#if NBPFILTER > 0
if (ifp->if_bpf)
- bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_OUT);
+ bpf_mtap_stripvlan(ifp->if_bpf, m, BPF_DIRECTION_OUT);
#endif
/*
* Henning Brauer [2014-07-09 23:46]:
> so dlg noticed that tcpdump on vlan is now somewhat busted,
> specifically dhc* don't work on the any more. the reason is that bpf
> now sees the ether_vlan_header instead of the ether_header. only
> visible if your NIC does NOT have hw vlan tagging.
> reason: while we previously would prepend an ethernet header in
> ether_output and way later in vlan_start throw the ethernet header
> away again, replacing it by an ether_vlan_header, we now add the
> ether_vlan_header in ether_output already. the mtap is in vlan_start,
> aka after.
> now removing the ether_vlan_header and either prepending a new
> ether_header or calling bpf_mtap_ether which adds a fake one didn't
> seem too smart. so I made a bpf_mcopy_stripvlan which, well, cuts
> those extra 4 bytes out.
>
> the if_ethersubr.c chunk eases testing, it'll make us hit the right
> codepath wether the hw has tagging or not. that chunk not to be
> committed of course.
--
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: divert(4) checksum offload
* Lawrence Teo [2014-07-10 06:14]: > This diff does the following for reinjected packets: > > 1. Zero the protocol checksum. that should not be needed. at least afair. > 2. Set the checksum flag in pkthdr. > 3a. For outbound packets, let the stack take care of the checksum. i like. might be biased :) > 3b. For inbound packets, calculate the checksum immediately with > in_proto_cksum_out(m, NULL). > > I'm not sure if it's all right to use in_proto_cksum_out() for inbound > packets (its name ends with "_out" after all :)) but using it really > helps to simplify things and avoid redundant code. well, could argue it goes out to divert... -- 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/
bpf_mtap_stripvlan
mp; IFCAP_VLAN_HWTAGGING) &&
+ if (0 && (p->if_capabilities & IFCAP_VLAN_HWTAGGING) &&
(ifv->ifv_type == ETHERTYPE_VLAN)) {
(*m)->m_pkthdr.ether_vtag = ifv->ifv_tag +
((*m)->m_pkthdr.pf.prio << EVL_PRIO_BITS);
Index: net/if_vlan.c
===
RCS file: /cvs/src/sys/net/if_vlan.c,v
retrieving revision 1.106
diff -u -p -r1.106 if_vlan.c
--- net/if_vlan.c 9 Jul 2014 09:30:49 - 1.106
+++ net/if_vlan.c 9 Jul 2014 14:09:43 -
@@ -213,7 +213,7 @@ vlan_start(struct ifnet *ifp)
#if NBPFILTER > 0
if (ifp->if_bpf)
- bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_OUT);
+ bpf_mtap_stripvlan(ifp->if_bpf, m, BPF_DIRECTION_OUT);
#endif
/*
--
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: divert(4) without mbuf tags
* Reyk Floeter [2014-07-09 11:21]: > Nice one. indeed. > Does anyone have an idea why the mbuf tag was added in the first > place? Maybe henning's PF shuffling removed the need for it. while not impossible, I doubt it. looks like a copy & paste issue. ok -- 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/
unify some bpf code
I'll need this for some upcoming changes, at least to do it WITHOUT
adding the 3rd or 4th or 5th copy of the bpf_mtap loop. most of these
bpf_mtap_* are almost identical, minor differences in what to prepend,
and foremost: passing custom copy functions. since bpf_mtap is all
over the place I made bpf_mtap_hdr the flexible one: takes the copy
function as parameter (if NULL, use default), and don't prepend
anything if dlen == 0 (so can be used like bpf_mtap).
lightly tested.
Index: arch/sparc/dev/if_ie.c
===
RCS file: /cvs/src/sys/arch/sparc/dev/if_ie.c,v
retrieving revision 1.45
diff -u -p -r1.45 if_ie.c
--- arch/sparc/dev/if_ie.c 28 Nov 2013 22:18:52 - 1.45
+++ arch/sparc/dev/if_ie.c 8 Jul 2014 14:37:17 -
@@ -1335,7 +1335,7 @@ ie_readframe(sc, num)
if (bpf_gets_it) {
/* Pass it up. */
bpf_mtap_hdr(sc->sc_arpcom.ac_if.if_bpf, (caddr_t)&eh,
- sizeof(eh), m, BPF_DIRECTION_IN);
+ sizeof(eh), m, BPF_DIRECTION_IN, NULL);
}
/*
* A signal passed up from the filtering code indicating that the
Index: net/bpf.c
===
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.93
diff -u -p -r1.93 bpf.c
--- net/bpf.c 23 Apr 2014 10:50:18 - 1.93
+++ net/bpf.c 8 Jul 2014 15:09:10 -
@@ -92,6 +92,8 @@ LIST_HEAD(, bpf_d) bpf_d_list;
void bpf_allocbufs(struct bpf_d *);
void bpf_freed(struct bpf_d *);
void bpf_ifname(struct ifnet *, struct ifreq *);
+void _bpf_mtap(caddr_t, struct mbuf *, u_int,
+ void (*)(const void *, void *, size_t));
void bpf_mcopy(const void *, void *, size_t);
intbpf_movein(struct uio *, u_int, struct mbuf **,
struct sockaddr *, struct bpf_insn *);
@@ -1159,10 +1161,11 @@ bpf_mcopy(const void *src_arg, void *dst
}
/*
- * Incoming linkage from device drivers, when packet is in an mbuf chain.
+ * like bpf_mtap, but copy fn can be given. used by various bpf_mtap*
*/
void
-bpf_mtap(caddr_t arg, struct mbuf *m, u_int direction)
+_bpf_mtap(caddr_t arg, struct mbuf *m, u_int direction,
+void (*cpfn)(const void *, void *, size_t))
{
struct bpf_if *bp = (struct bpf_if *)arg;
struct bpf_d *d;
@@ -1174,6 +1177,9 @@ bpf_mtap(caddr_t arg, struct mbuf *m, u_
if (m == NULL)
return;
+ if (cpfn == NULL)
+ cpfn = bpf_mcopy;
+
pktlen = 0;
for (m0 = m; m0 != 0; m0 = m0->m_next)
pktlen += m0->m_len;
@@ -1191,13 +1197,22 @@ bpf_mtap(caddr_t arg, struct mbuf *m, u_
if (!gottime++)
microtime(&tv);
- bpf_catchpacket(d, (u_char *)m, pktlen, slen, bpf_mcopy, &tv);
+ bpf_catchpacket(d, (u_char *)m, pktlen, slen, cpfn, &tv);
if (d->bd_fildrop)
m->m_flags |= M_FILDROP;
}
}
/*
+ * Incoming linkage from device drivers, when packet is in an mbuf chain.
+ */
+void
+bpf_mtap(caddr_t arg, struct mbuf *m, u_int direction)
+{
+ _bpf_mtap(arg, m, direction, NULL);
+}
+
+/*
* Incoming linkage from device drivers, where we have a mbuf chain
* but need to prepend some arbitrary header from a linear buffer.
*
@@ -1208,17 +1223,23 @@ bpf_mtap(caddr_t arg, struct mbuf *m, u_
*/
void
bpf_mtap_hdr(caddr_t arg, caddr_t data, u_int dlen, struct mbuf *m,
-u_int direction)
+u_int direction, void (*cpfn)(const void *, void *, size_t))
{
- struct m_hdr mh;
-
- mh.mh_flags = 0;
- mh.mh_next = m;
- mh.mh_len = dlen;
- mh.mh_data = data;
+ struct m_hdr mh;
+ struct mbuf *m0;
- bpf_mtap(arg, (struct mbuf *) &mh, direction);
- m->m_flags |= mh.mh_flags & M_FILDROP;
+ if (dlen > 0) {
+ mh.mh_flags = 0;
+ mh.mh_next = m;
+ mh.mh_len = dlen;
+ mh.mh_data = data;
+ m0 = (struct mbuf *)&mh;
+ } else
+ m0 = m;
+
+ _bpf_mtap(arg, (struct mbuf *) m0, direction, cpfn);
+ if (m0 != m)
+ m->m_flags |= m0->m_flags & M_FILDROP;
}
/*
@@ -1233,17 +1254,10 @@ bpf_mtap_hdr(caddr_t arg, caddr_t data,
void
bpf_mtap_af(caddr_t arg, u_int32_t af, struct mbuf *m, u_int direction)
{
- struct m_hdr mh;
u_int32_tafh;
- mh.mh_flags = 0;
- mh.mh_next = m;
- mh.mh_len = 4;
afh = htonl(af);
- mh.mh_data = (caddr_t)&afh;
-
- bpf_mtap(arg, (struct mbuf *) &mh, direction);
- m->m_flags |= mh.mh_flags & M_FILDROP;
+ bpf_mtap_hdr(arg, (caddr_t)&afh, 4, m, direction, NULL);
}
/*
@@ -1259,7 +1273,6 @@ void
bpf_mtap_ether(caddr_t arg, struct mbuf *m, u_int direction)
{
#if NVLAN > 0
- struct m_hdr mh;
struct ether_vlan_header evh;
if ((m->m_flags & M_VLANT
Re: idea to block some scanners
* Leclerc, Sebastien [2014-06-27 16:40]: > + if (ioctl(pfdev, DIOCNATLOOK, &pnl) == -1) no DIOCNATLOOK is stupid. I'll celebrate the day when I can kill it. Please look at less ancient ftp-proxy/*-proxy code for inspiration. -- 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: NOINET6 by default
since no consensus could be found yet for a new command line option to
ifconfig, heck, not even about wether it is needed, I propose this for
now.
1) make "ifconfig inet6 eui64" reset the NOINET6 flag
unconditionally, so a link-local will be assigned if there isn't one
yet.
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.283
diff -u -p -r1.283 ifconfig.c
--- sbin/ifconfig/ifconfig.c12 May 2014 08:47:37 - 1.283
+++ sbin/ifconfig/ifconfig.c19 May 2014 00:27:07 -
@@ -411,7 +411,7 @@ const structcmd {
{ "flowdst",NEXTARG,0, setpflow_receiver },
{ "-flowdst", 1,0, unsetpflow_receiver },
{ "pflowproto", NEXTARG,0, setpflowproto },
- { "-inet6", IFXF_NOINET6, 0, setifxflags } ,
+ { "-inet6", IFXF_NOINET6, 0, setifxflags },
{ "keepalive", NEXTARG2, 0, NULL, setkeepalive },
{ "-keepalive", 1, 0, unsetkeepalive },
{ "add",NEXTARG,0, bridge_add },
@@ -1312,6 +1312,7 @@ setia6eui64(const char *cmd, int val)
if (afp->af_af != AF_INET6)
errx(1, "%s not allowed for the AF", cmd);
+ setifxflags("inet6", -IFXF_NOINET6);
in6 = (struct in6_addr *)&in6_addreq.ifra_addr.sin6_addr;
if (memcmp(&in6addr_any.s6_addr[8], &in6->s6_addr[8], 8) != 0)
errx(1, "interface index is already filled");
2) turn the NOINET6 flag on by default.
As said previously, it will be reset and thus a link-local assigned
transparently if either
-rtsol(d) is run
-an inet6 address is assigned
-ifconfig inet6 eui64 is run
and thus should be entirely transparent for the vast majority of inet6
users.
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.289
diff -u -p -r1.289 if.c
--- sys/net/if.c16 May 2014 08:21:54 - 1.289
+++ sys/net/if.c16 May 2014 14:15:24 -
@@ -423,6 +423,9 @@ if_attach(struct ifnet *ifp)
#else
TAILQ_INSERT_TAIL(&ifnet, ifp, if_list);
#endif
+#ifdef INET6
+ ifp->if_xflags |= IFXF_NOINET6;
+#endif
m_clinitifp(ifp);
wether we need a less obscure ifconfig command than eui64 can be
discussed after.
oks?
--
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 anchor references
* Mike Belopuhov [2014-06-02 17:52]: > Any opinions on this? Does anyone make any sensible use of it? > Should we try to simplify this by removing ambiguous functionality? I've been thinking that anchors as of now are a bit overengineered for a long time. When they were written, we had no clear idea where anchors would go and how people use them. That explains some functionality that is there today. But heck: now we DO know how they're being used, so let's get rid of the other parts where appropriate. -- 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: Create a default local route for every IPv4 address
* Kenneth Westerback [2014-05-26 14:05]: > dhclient used to create such routes but that was removed as useless so > I'm not sure why we want/need to add them back. I'm not a routing > table guru so perhaps this is different in some way. there is a broad difference between "the kernel does it always" and "in some cases, some userland app does it". in the former case, the existance of the local route can be used e. g. for the local/remote decision, in the latter case that is utterly unreliable. -- 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: NOINET6 by default
* Claudio Jeker [2014-05-16 08:06]: > On Fri, May 16, 2014 at 12:43:52AM -0500, Todd T. Fries wrote: > > When I travel between networks.. at home with rtsol capable networks .. > > and at e.g. a library that does not have native IPv6 .. I find it invaluable > > to 'zzz' then upon resume 'ifconfig wpi0 -inet6' for the library and then > > 'rtsol wpi0' at home. -inet6 stays no matter what, wether that does setifxflags IFXF_NOINET6 in the back or just removes all inet6 addrs is transparent to the user. how to add linklocal back without the flag is another question, need to come up with something in that case (that is definately not a hard problem tho), since eui64 effectively does nothing but whine if it cannot find a link-local... > I did not talk about -inet6 but about the kernel IFXF_NOINET6 flag. > It is not needed to use a flag on the interface for this. Instead we > handle it all when creating / removing IPv6 addresses on the interface. yup. > In the long run I would like to have a -inet as well so that you can > remove all IPv4 addresses of an interface in a easy way. yeah, that makes sense. probably comes for free when implementing -inet6 without IFXF_NOINET6. -- 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: NOINET6 by default
* Claudio Jeker [2014-05-15 09:33]: > On Wed, May 14, 2014 at 11:29:20PM +0200, Henning Brauer wrote: > > so as discussed recently having the inet6 link-local addrs on every > > interface by default is stupid and a security risk. > > > > this diff fixes that. well, really two independent parts. > > one: set the NOINET6 flag by default on each and every interface. > > two: implement "ifconfig +inet6" to turn inet6 on and assign > > the link-local addr. > > > > this should be transparent for almost all real use cases of inet6 > > since assigning any inet6 address also resets the flag (and ll is > > assigned then as well). > > lo0 still gets it's ::1 and fe80::1%lo0 by default. > > > > the only use case that needs config adoption: people ONLY using > > link-local, they will need to put +inet6 in the corresponding > > hostname.if file. > > > > ok? > > To be honest the right fix would be to get rid of IFXF_NOINET6 and > just make it the default. There is no need for such a flag anymore. very valid point, I'll happily clean that up right after - one thing at a time. -- 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: NOINET6 by default
* Claudio Jeker [2014-05-15 09:42]: > On Thu, May 15, 2014 at 05:48:16AM +0200, Henning Brauer wrote: > > * Reyk Flöter [2014-05-15 01:04]: > > > > On 15.05.2014, at 00:46, Henning Brauer > > > > wrote: > > > > * Mark Kettenis [2014-05-15 00:15]: > > > >> I don't think this is a good idea; didn't we establish the other day > > > >> that "ifconfig eui64" already did what your +inet6 does? > > > > almost, it's ifconfig inet6 eui64 - but that isn't all THAT > > > > intuitive. I like +inet6 as the opposite of -inet6. > > > We don't have "+" something. It is foo or -foo but not +foo. I know that > > > inet6 is already used for the regular addresses, but +inet6 sounds like > > > an inconsistent workaround for a workaround. I don't like it. > > > > just inet6 doesn't work, since that is already used to show all inet6 > > addrs. > > i find +inet6 very intuitive... > > This should just die. Did you ever do ifconfig em0 inet or ifconfig em0 inet6? > I never did and I have a few interfaces with a lot of IPs on them. > It is a useless gimmick of ifconfig. changing semantics of an existing interface like this is of course much more intrusive than adding a new one. if the concensus is that the current inet/inet6 to show the addreses of that af only is bollocks and we'd rather use inet6 to turn it on, that's simple to do as well. We just need to take a decision here instead of bikeshedding forever... -- 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: NOINET6 by default
* Todd T. Fries [2014-05-15 06:29]:
> Penned by Henning Brauer on 20140514 22:48.16, we have:
> | * Reyk Flöter [2014-05-15 01:04]:
> | > > On 15.05.2014, at 00:46, Henning Brauer
> wrote:
> | > > * Mark Kettenis [2014-05-15 00:15]:
> | > >> I don't think this is a good idea; didn't we establish the other day
> | > >> that "ifconfig eui64" already did what your +inet6 does?
> | > > almost, it's ifconfig inet6 eui64 - but that isn't all THAT
> | > > intuitive. I like +inet6 as the opposite of -inet6.
> | > We don't have "+" something. It is foo or -foo but not +foo. I know that
> inet6 is already used for the regular addresses, but +inet6 sounds like an
> inconsistent workaround for a workaround. I don't like it.
> |
> | just inet6 doesn't work, since that is already used to show all inet6
> | addrs.
> | i find +inet6 very intuitive...
> |
> | > To "enable IPv6" link-local I would rather prefer two options to put
> | > either "inet6 eui64" (or an alias like "inet6 link-local") or an actual
> | > inet6 address in your hostname.if. The latter should automatically
> | > remove the flag and enable the link-local address - does it work this
> | > way?
> |
> | as said many times, yes it does.
>
> I ack that it is a security risk to auto address interfaces without some admin
> action.
>
> The proposed solution seems sound, 'inet6 eui64' seems sane. In theory it
> should work, but I must be doing something wrong:
>
> # ifconfig vether0 create
> # ifconfig vether0 -inet6
> # ifconfig vether0 inet6 eui64
> ifconfig: could not determine link local address
eui64 by itself is NOT enough, this is why I have the 2 line change to
the eui64 handler in the diff for the +inet6 case. Making that
unconditional is trivial, I just don't think "inet6 eui64" is very
intuitive. see, I even think about the inet6 users.
> Once that works properly, I say we let the diff in and bikeshed if we
> truly need to invent more syntax ('+inet6') that is unlike anything else
> vs let the few of us that want this apparently obsecure case add 'inet6
> eui64' and be done with it.
>
> Aka, lets not hold up the rest of the functionality just because we
> can't agree if we need a further diff to make 'inet6 eui64'
> "better/faster/easier/another way to skin the cat"...
i couldn't agree more
> IMHO, its time to polish in the tree. This is, afterall, a _security_
> related diff, no?
i'd say so.
--
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: NOINET6 by default
* Reyk Flöter [2014-05-15 01:04]: > > On 15.05.2014, at 00:46, Henning Brauer wrote: > > * Mark Kettenis [2014-05-15 00:15]: > >> I don't think this is a good idea; didn't we establish the other day > >> that "ifconfig eui64" already did what your +inet6 does? > > almost, it's ifconfig inet6 eui64 - but that isn't all THAT > > intuitive. I like +inet6 as the opposite of -inet6. > We don't have "+" something. It is foo or -foo but not +foo. I know that > inet6 is already used for the regular addresses, but +inet6 sounds like an > inconsistent workaround for a workaround. I don't like it. just inet6 doesn't work, since that is already used to show all inet6 addrs. i find +inet6 very intuitive... > To "enable IPv6" link-local I would rather prefer two options to put > either "inet6 eui64" (or an alias like "inet6 link-local") or an actual > inet6 address in your hostname.if. The latter should automatically > remove the flag and enable the link-local address - does it work this > way? as said many times, yes it does. -- 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: NOINET6 by default
* Alexander Bluhm [2014-05-15 00:15]: > On Wed, May 14, 2014 at 11:29:20PM +0200, Henning Brauer wrote: > > so as discussed recently having the inet6 link-local addrs on every > > interface by default is stupid and a security risk. > Connecting a computer to the internet is a security risk. > IPv4 is on by default, and so IPv6 should be on by default. > I want both to be handled the same way. WITH my diff they finally become the same, IPv4 does NOT assign some special address to the interface by default. The analogy really breaks here since v4 just doesn't have link local. > > the only use case that needs config adoption: people ONLY using > > link-local, they will need to put +inet6 in the corresponding > > hostname.if file. > There is a use case for running IPv6 over an interface without > setting an address. Configure a global IPv6 address on lo0, run > ospf6d on any physical interface and it will provide connection. > IPv6 autoconfiguration with link-local addresses is useful. so you put +inet6 in the corresponding hostname.if file and everything works like you want it to. -- 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: NOINET6 by default
* Mark Kettenis [2014-05-15 00:15]: > I don't think this is a good idea; didn't we establish the other day > that "ifconfig eui64" already did what your +inet6 does? almost, it's ifconfig inet6 eui64 - but that isn't all THAT intuitive. I like +inet6 as the opposite of -inet6. -- 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/
IFT_L2VLAN is unused
don't kill the define, since this is userland visible, but there is
never ever an interface in our kernel with if_type == IFT_L2VLAN - see
my commit from 2 weeks ago or so. To clarify this once again, I didn't
remove the L2VLAN use, it was never really used.
ok?
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.288
diff -u -p -r1.288 if.c
--- net/if.c13 May 2014 14:33:25 - 1.288
+++ net/if.c14 May 2014 21:41:01 -
@@ -1625,7 +1625,6 @@ ifioctl(struct socket *so, u_long cmd, c
case IFT_CARP:
case IFT_XETHER:
case IFT_ISO88025:
- case IFT_L2VLAN:
bcopy((caddr_t)ifr->ifr_addr.sa_data,
(caddr_t)((struct arpcom *)ifp)->ac_enaddr,
ETHER_ADDR_LEN);
Index: net/if_pppoe.c
===
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.38
diff -u -p -r1.38 if_pppoe.c
--- net/if_pppoe.c 14 Apr 2014 09:06:42 - 1.38
+++ net/if_pppoe.c 24 Apr 2014 23:38:10 -
@@ -924,9 +924,7 @@ pppoe_ioctl(struct ifnet *ifp, unsigned
struct ifnet*eth_if;
eth_if = ifunit(parms->eth_ifname);
- if (eth_if == NULL ||
- (eth_if->if_type != IFT_ETHER &&
-eth_if->if_type != IFT_L2VLAN)) {
+ if (eth_if == NULL || eth_if->if_type != IFT_ETHER) {
sc->sc_eth_if = NULL;
return (ENXIO);
}
Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.116
diff -u -p -r1.116 nd6.c
--- netinet6/nd6.c 7 May 2014 08:14:59 - 1.116
+++ netinet6/nd6.c 13 May 2014 14:38:16 -
@@ -1830,7 +1830,6 @@ nd6_need_cache(struct ifnet *ifp)
case IFT_ETHER:
case IFT_IEEE1394:
case IFT_PROPVIRTUAL:
- case IFT_L2VLAN:
case IFT_IEEE80211:
case IFT_CARP:
case IFT_GIF: /* XXX need more cases? */
Index: netinet6/nd6_nbr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.78
diff -u -p -r1.78 nd6_nbr.c
--- netinet6/nd6_nbr.c 18 Apr 2014 10:48:30 - 1.78
+++ netinet6/nd6_nbr.c 24 Apr 2014 23:38:10 -
@@ -1064,7 +1064,6 @@ nd6_ifptomac(struct ifnet *ifp)
case IFT_IEEE1394:
case IFT_PROPVIRTUAL:
case IFT_CARP:
- case IFT_L2VLAN:
case IFT_IEEE80211:
return ((caddr_t)(ifp + 1));
default:
--
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/
NOINET6 by default
so as discussed recently having the inet6 link-local addrs on every
interface by default is stupid and a security risk.
this diff fixes that. well, really two independent parts.
one: set the NOINET6 flag by default on each and every interface.
two: implement "ifconfig +inet6" to turn inet6 on and assign
the link-local addr.
this should be transparent for almost all real use cases of inet6
since assigning any inet6 address also resets the flag (and ll is
assigned then as well).
lo0 still gets it's ::1 and fe80::1%lo0 by default.
the only use case that needs config adoption: people ONLY using
link-local, they will need to put +inet6 in the corresponding
hostname.if file.
ok?
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.288
diff -u -p -r1.288 if.c
--- sys/net/if.c13 May 2014 14:33:25 - 1.288
+++ sys/net/if.c14 May 2014 21:03:45 -
@@ -429,6 +429,9 @@ if_attach(struct ifnet *ifp)
#else
TAILQ_INSERT_TAIL(&ifnet, ifp, if_list);
#endif
+#ifdef INET6
+ ifp->if_xflags |= IFXF_NOINET6;
+#endif
m_clinitifp(ifp);
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.241
diff -u -p -r1.241 ifconfig.8
--- sbin/ifconfig/ifconfig.822 Apr 2014 10:11:32 - 1.241
+++ sbin/ifconfig/ifconfig.813 May 2014 14:58:58 -
@@ -283,8 +283,12 @@ Disable
on the given interface and remove all configured
.Xr inet6 4
addresses, including the link-local ones.
-To turn it on again, assign any inet6 address or run
+To turn it on again, use +inet6, assign any inet6 address or run
.Xr rtsol 8 .
+.It +inet6
+Enable
+.Xr inet6 4
+and assign a link local address if the interface doesn't have one yet.
.It Cm instance Ar minst
Set the media instance to
.Ar minst .
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.283
diff -u -p -r1.283 ifconfig.c
--- sbin/ifconfig/ifconfig.c12 May 2014 08:47:37 - 1.283
+++ sbin/ifconfig/ifconfig.c13 May 2014 14:56:25 -
@@ -412,6 +412,7 @@ const structcmd {
{ "-flowdst", 1,0, unsetpflow_receiver },
{ "pflowproto", NEXTARG,0, setpflowproto },
{ "-inet6", IFXF_NOINET6, 0, setifxflags } ,
+ { "+inet6", 0, 0, setia6eui64 },
{ "keepalive", NEXTARG2, 0, NULL, setkeepalive },
{ "-keepalive", 1, 0, unsetkeepalive },
{ "add",NEXTARG,0, bridge_add },
@@ -1310,7 +1311,9 @@ setia6eui64(const char *cmd, int val)
const struct in6_addr *lladdr = NULL;
struct in6_addr *in6;
- if (afp->af_af != AF_INET6)
+ if (!strcmp(cmd, "+inet6"))
+ setifxflags("inet6", -IFXF_NOINET6);
+ else if (afp->af_af != AF_INET6)
errx(1, "%s not allowed for the AF", cmd);
in6 = (struct in6_addr *)&in6_addreq.ifra_addr.sin6_addr;
if (memcmp(&in6addr_any.s6_addr[8], &in6->s6_addr[8], 8) != 0)
--
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: libc: #define to remove support for %n from printf(3)?
* Marc Espie [2014-05-03 11:50]: > On Sat, May 03, 2014 at 11:28:16AM +0200, Reyk Floeter wrote: > > On Fri, May 02, 2014 at 06:50:04PM -0600, Bob Beck wrote: > > > What's their hangup with %n? We normally don't like polluting the world > > > with #ifdef OPENSSL_NO_PERCENT_N... We normally nuke stuff like that > > Well, it is an evil thing that is rarely used and well-known for some > > format string vulnerabilities. So either adding this #define or > > removing it from our libc wouldn't be a bad idea. But it is in POSIX, > > right? > > At a first look, I only found it in a few places of our tree where it > > could be removed: > > > > gcc, texinfo, curses, libform, sqlite, less, tmux, unbound, nsd > > > > Not couting ports, but some software already seems to check if %n is > > available to do a fallback otherwise. > And you do expect the fallback to be of good quality ? > > I think that the same rationale that lead to including some crypto code > in openssl applies there: if we don't provide it, people will roll their > own posixy-printf, and we know how well that turns out in general... > > (including stuff that peeks and duplicates the internals of FILE, like > I'll be happy if I *never* get to fix this kind of code again). both of you are kinda missing the point. there is no argument here to remove %n support from our libc, the point is wether we can add a #define to allow people compiling themselves (probably not as part of OpenBSD) to remove it without having to change the code. And since that's not intrusive and doesn't create a portability mess like the one we're dealing with in libssl right now, I don't see a problem with that. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: [RFC] Ai_ADDRCONFIG^WAIAIAIAIAIAIAEEEEEEEEE tweaks?
* Kenneth Westerback [2014-05-02 22:14]: > On 2 May 2014 16:08, Paul de Weerd wrote: > > Well, I think -inet6 would be a good default, but I think there's more > > to it. Enabling net.inet6.ip6.accept_rtadv should still get me a > > link-local address (and, if router advertisements are present on the > > local network, an autoconfigured (autoconfprivacy) address too). But > > if I have multiple interfaces and configure my system for SLAAC, what > > should happen? To me, it seems that accept_rtadv should be a > > per-interface thing. > > > > Anyway, I believe at least -inet6 is a better default than the current > > situation. > -inet6 as the default seems more OpenBSD'ish to me. Everything off > that can be off, but not more. there is way more to it than "the default". there is no easy way to get rid of ipvshit completely, short of recompiling w/o option INET6. every interface you take up has that linklocal shit, unless you give -inet6 for each and every one every time, which is very easy to miss. thus I do think we want a net.inet6.ip.enable sysctl or the like, which, if not set to 1, enforces -inet6 on all ifs. what the default of such a sysctl would be is another discussion - any value is fine with me as long as it is 0. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: [RFC] Ai_ADDRCONFIG^WAIAIAIAIAIAIAEEEEEEEEE tweaks?
* Paul de Weerd [2014-05-02 21:20]: > On Fri, May 02, 2014 at 06:53:08PM +0200, Jérémie Courrèges-Anglas wrote: [connectivity via link-local] > | Not really, I'm puzzled by your question. It works and has always > | worked but I shouldn't expect them to work... > I'm puzzled by the fact it has always been this way in OpenBSD. It > goes against the OpenBSD philosophy. see where the v6 zealots got us? > I'll try to rephrase the question: > > Why do you expect that you are accessible on IPv6 > when you configure an interface with IPv4? You > don't expect to get IPv4 connectivity when you > configure IPv6, do you? a very good question to ask. i wish -inet6 was default. i'll probably add a sysctl to globally nuke v6 from all interfaces soon. somebody pls remind me at the next hackathon. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: Annoying emacs variable in if_spppsubr.c
I don't think that kind of stuff has any place in our tree whatsoever.
everybody inserting such "hints" for his/her favorite editor...?
* Jérémie Courrèges-Anglas [2014-05-02 12:11]:
>
> This one is bugging me each time I start my Emacs session (because Emacs
> now asks confirmation for most variables). This one would be useful
> only with hilit19.el (obsolete) from editors/emacs21... if the size of
> the file was actually under 12 bytes (it's not anymore).
>
> ok to kill it?
>
> Index: if_spppsubr.c
> ===
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.121
> diff -u -p -p -u -r1.121 if_spppsubr.c
> --- if_spppsubr.c 19 Apr 2014 12:12:02 - 1.121
> +++ if_spppsubr.c 30 Apr 2014 00:53:56 -
> @@ -5236,13 +5236,6 @@ sppp_null(struct sppp *unused)
> {
> /* do just nothing */
> }
> -/*
> - * This file is large. Tell emacs to highlight it nevertheless.
> - *
> - * Local Variables:
> - * hilit-auto-highlight-maxout: 12
> - * End:
> - */
>
> void
> sppp_set_phase(struct sppp *sp)
>
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>
--
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/
vlan: stop if_type wankery
so, vlan: -calls ether_ifattach, which, last not least, sets if_type to IFT_ETHER -right after set if_type to IFT_L2VLAN -just to set it to if_ether again as soon as it gets configured (parents can only be IFT_ETHER) that is... pointless. fwiw, this was the one and only use of IFT_L2VLAN in the entire kernel. the question in the comment has been answered by the last 13 years. ok? Index: net/if_vlan.c === RCS file: /cvs/src/sys/net/if_vlan.c,v retrieving revision 1.103 diff -u -p -r1.103 if_vlan.c --- net/if_vlan.c 22 Apr 2014 11:43:07 - 1.103 +++ net/if_vlan.c 1 May 2014 19:49:50 - @@ -151,8 +151,6 @@ vlan_clone_create(struct if_clone *ifc, IFQ_SET_READY(&ifp->if_snd); if_attach(ifp); ether_ifattach(ifp); - /* Now undo some of the damage... */ - ifp->if_type = IFT_L2VLAN; ifp->if_hdrlen = EVL_ENCAPLEN; ifp->if_output = vlan_output; @@ -374,12 +372,6 @@ vlan_config(struct ifvlan *ifv, struct i } /* -* Inherit the if_type from the parent. This allows us to -* participate in bridges of that type. -*/ - ifv->ifv_if.if_type = p->if_type; - - /* * Inherit baudrate from the parent. An SNMP agent would use this * information. */ @@ -392,9 +384,6 @@ vlan_config(struct ifvlan *ifv, struct i * * If the card cannot handle hardware tagging, it cannot * possibly compute the correct checksums for tagged packets. -* -* This brings up another possibility, do cards exist which -* have all of these capabilities but cannot utilize them together? */ if (p->if_capabilities & IFCAP_VLAN_HWTAGGING) ifv->ifv_if.if_capabilities = p->if_capabilities & -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: IPv6 by default
* Simon Perreault [2014-04-29 16:05]: > Le 2014-04-29 09:55, Henning Brauer a écrit : > >> Wouldn't it be better if libasr would run A and requests in > >> parallel? Whichever response arrives first "wins". > > no, since that gives extremely unpredictable results. > > How about this then: > > - Run both requests in parallel. > - When one response is received, start a short timer (e.g. 200ms or so). > - If the second response is received before the timer expires, sort and > return the results as usual. > - Otherwise, kill the second request and return what you have. that could work, of course. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: IPv6 by default
* Simon Perreault [2014-04-29 14:41]: > Le 2014-04-28 18:53, Chris Cappuccio a écrit : > >> Why is the burden on everyone to provide 'valid' objections? Should > >> not the burden be on you to at least hint at a point to this change? > >> Given the miniscule IPv6 usage out there, why should IPv6 come first? > > > > I like how IPv6 support turns primary and secondary DNS caches from > > a redundancy feature for clients to dual points of failure (for some > > resolver implementations.) No response from either server for the first > > AF you try? Just wait for a full time out before you try the second AF! > > This is a valid point IMHO. > > Wouldn't it be better if libasr would run A and requests in > parallel? Whichever response arrives first "wins". no, since that gives extremely unpredictable results. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: IPv6 by default
* Simon Perreault [2014-04-29 14:58]: > I don't see how "usage" is relevant. If IPv6 provided 1000% performance > improvement with no downsides, we would want to use it even if global > usage was low. however, it provides far worse performance with shitloads of downsides... -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: IPv6 by default
* Adam Thompson [2014-04-29 04:37]: > On April 28, 2014 5:43:34 PM CDT, Kenneth Westerback > wrote: > >On 28 April 2014 18:05, Simon Perreault wrote: > >> Now that my AI_ADDRCONFIG diff is in, it's time to reveal my evil > >master plan: > >> make getaddrinfo() return IPv6 results first by default. no way. > >Why is the burden on everyone to provide 'valid' objections? Should > >not the burden be on you to at least hint at a point to this change? > >Given the miniscule IPv6 usage out there, why should IPv6 come first? that is the right question, and there is no good answer... > Someone has to take the first/next step except that it is a step towards the drain. > Sent from my Android device with K-9 Mail. Please excuse my brevity. Sent from a computer using a keyboard and software. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: Remove rti_ifp from "struct rt_addrinfo"
* Ryan McBride [2014-04-25 10:31]: > On Fri, Apr 25, 2014 at 02:40:57AM +0200, Alexander Bluhm wrote: > > On Fri, Apr 25, 2014 at 09:09:03AM +0900, Ryan McBride wrote: > > > Part of the reason it's there is to make carp work properly for services > > > listening on the carp interface, in particular so that hosts in the > > > BACKUP state will reach the MASTER rather than trying and failing to > > > connect to their own carp interface. Maybe not needed in all setups, but > > > likely to break things if we simply remove it. > > > > Why do you want to connect from the BACKUP machine to the MASTER > > using CARP addresses? Just add another fixed address and you can > > do that. > > Two reasons that come to mind are: > > 1) For troubleshooting, so I can ping or otherwise monitor the MASTER > host. > > 2) In some cases it's undisireable (or even not possible) to run > services on other IP addresses. For example, services that only allow > you to configure 1 listening IP, or services where you wish to avoid > users connecting to anything but the MASTER server. > > > The current implementation may change the routing table in subtile > > ways until nothing works. In IPv6 the routes are fixed and there > > are less problems. > > In my opinion the current (intended) behaviour is correct; my preference > would be to see this fixed rather than removed. given that -it is done for v4 only -it has been demonstrated to cause problems, namely screwed up routing tables -it, afair, not working in the unnumbered case at all the only conclusion I can come to is "nuke it!". especially due to the 2nd point. I causes more harm than good in its current state. if this is desired (I can't really see the need to be honest) it must be done properly doing route priorities and marking routes down. This functionaity didn't exist when we did carp. Going that route (haha), the code for that wouldn't have much in common with what is currently there, so... I'm in favor of nuking. coincidently, I have a diff which does that :) -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: Remove rti_ifp from "struct rt_addrinfo"
* Alexander Bluhm [2014-04-24 18:18]: > The carp_setroute() function starts with the encouraging comment > /* XXX this mess needs fixing */. I didn't change my mind at least :) 1.136(henning 04-May-07): /* XXX this mess needs fixing */ > When switching carp states fast, > the routing table gets messed up. In our product we removed all > calls to carp_setroute(sc, RTM_DELETE). There is one carp_setroute(sc, > RTM_ADD) left. I don't know wether it is needed. I would recommend > to try to delete the whole function and fix fallout if any. I tend towards that. ryan, marco? -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: Remove rti_ifp from "struct rt_addrinfo"
* Martin Pieuchot [2014-04-24 13:24]: > This ifp pointer is only needed by rt_getifa() to find an address, so > make it a local variable. > > The rtrequest1(9) change might introduce a negligible slowdown since > I remove the already known ifp pointer. But this only happens in the > case described in the comment just before and I would bet because of > carp_setroute(), still nobody to fix this? It's not better than > OpenSSL... > > In the rtsock chunk, the two pointers are equivalent. > > Ok? yup. the carp route fiddling is pretty damn mad, and with the route priorities and the ability to mark routes as down there should be a much cleaner way to do this these days. heck, the entire carp route fiddling needs to be reassesed. things changed, i can\t even fully remeber why it is there (i think it was about backup nodes still being able to reach a network only present on the carp if or the like), and i seem to remember it doesn't quite work as expected anyway, but don't take my word for it, memory REALLY fuzzy on that front. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: [patch] cvs some values never read
* Fritjof Bornebusch [2014-04-23 20:15]: > >* Fritjof Bornebusch [2014-04-23 19:30]: >> there are some set operations, which are never read. > >> RCS file: /cvs/src/usr.bin/cvs/rcsparse.c,v > >guess we need to decide what to do with opencvs really. either there >is someone who cares and picks it up, or we can straight delete it. it >hasn't moved forward in years, and I have a hard time seeing it going >anywhere (except Attic). But that's just me, of course. > > If opencvs is going to be deleted, what is the alternative? gnucvs? err, that's what we've been using all the time. It has never become ready. revision 1.114 date: 2010/06/26 03:59:34; author: deraadt; state: Exp; lines: +2 -2; disable opencvs; maintainers went bye bye -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: [patch] cvs some values never read
* Fritjof Bornebusch [2014-04-23 19:30]: > there are some set operations, which are never read. > RCS file: /cvs/src/usr.bin/cvs/rcsparse.c,v guess we need to decide what to do with opencvs really. either there is someone who cares and picks it up, or we can straight delete it. it hasn't moved forward in years, and I have a hard time seeing it going anywhere (except Attic). But that's just me, of course. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: bpf(4) obsolete data-link levels
* Jérémie Courrèges-Anglas [2014-04-23 02:05]: > If I'm not mistaken, we had no drivers left that use those types? correct, swing the burning axe. ok. > - case DLT_FDDI: > - case DLT_ATM_RFC1483: -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: typo security.8
* Fritjof Bornebusch [2014-04-22 18:29]: > it's Trojan horse not Trojan horsed, right? yup. a trojan horse. the binary has been trojan horsed. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: vlan tagging surgery
ON_OUT);
#endif
-
- /*
-* If the IFCAP_VLAN_HWTAGGING capability is set on the parent,
-* it can do VLAN tag insertion itself and doesn't require us
-* to create a special header for it. In this case, we just pass
-* the packet along.
-*/
- if ((p->if_capabilities & IFCAP_VLAN_HWTAGGING) &&
- (ifv->ifv_type == ETHERTYPE_VLAN)) {
- m->m_pkthdr.ether_vtag = ifv->ifv_tag +
- (m->m_pkthdr.pf.prio << EVL_PRIO_BITS);
- m->m_flags |= M_VLANTAG;
- } else {
- struct ether_vlan_header evh;
-
- m_copydata(m, 0, ETHER_HDR_LEN, (caddr_t)&evh);
- evh.evl_proto = evh.evl_encap_proto;
- evh.evl_encap_proto = htons(ifv->ifv_type);
- evh.evl_tag = htons(ifv->ifv_tag +
- (m->m_pkthdr.pf.prio << EVL_PRIO_BITS));
-
- m_adj(m, ETHER_HDR_LEN);
- M_PREPEND(m, sizeof(evh), M_DONTWAIT);
- if (m == NULL) {
- ifp->if_oerrors++;
- continue;
- }
-
- m_copyback(m, 0, sizeof(evh), &evh, M_NOWAIT);
- }
/*
* Send it, precisely as ether_output() would have.
--
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/
Re: vlan tagging surgery
this bikeshedding is annoying as hell and only makes sure we're not proceeding. makes me almost want to reconsider the open development process using tech@ instead of a private list. and I want to proceed here, so I am looking for oks unless someone has real, non-bikeshedding issues. and certainly not dreamed up layering violations that don't exist here. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: vlan tagging surgery
* Alexey Suslikov [2014-04-21 13:56]: > Henning Brauer bsws.de> writes: > > > I must admit I am getting tired of all these "good proposals/ideas". > > don't you think we've gone thru this before? > > Look, I haven't called them good or bad. > > > what you propose would require a custom vlan_output function which > > does nothing but setting the flag and then calls ether_output. > > what exactly is won with that? except making things less obvious? > > preparing for the highly likely case that something but a vlan > > interface (as in, IFT_L2VLAN) needs to add a L2VLAN ethernet header? > > (I understand you want code - not theoretical speculations). > > I assume, there is an input and output of a stack. > > And lot of (possible) encapsulation subsystems in the middle: vlan, > vlan-in-vlan, ipsec, you name it. VLAN IS NOT AN ENCAPSULATION. > And if I understood your cksum plan right, being in the middle, given > packet doesn't know its destiny, but different subsystems may assign > tags so on the output packet may assemble itself right (by calling > necessary methods) where's cksum in this game??? if you're referring to "same model, emulate hw support": it is pointless here, since the point where we could set the flag is the very same point where we actually add the header! > Given a number of subsystems, delayed processing (promise pattern > variation, actually) is way to go, imo, because stack will have > homogeneous approach for entire packet assembly logic. you cannot delay this reasonably, it IS far down the road, basically right before sending the frame out. > In terms of above pattern, right: vlan_output will only set a flag > and call ether_output - this is what you already did with cksums. no, not even remotely. sigh. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: vlan tagging surgery
* Alexey Suslikov [2014-04-21 13:13]: > Henning Brauer bsws.de> writes: > > congratulations, that is close to unauditable. > > i put the vlan and the !vlan case next to each other ON PURPOSE. both > > cases add an ethernet header, one with a few extra fields, one > > without. Having that next to each other makes it f***ing obvious both > > cases are identical, except the vlan case filling the 3 extra fields. > Another approach is to use delayed pattern (like you did for cksums): > compute flags, a set of header assembly conditions, and, later on, keep > method call logic obvious using that flags. I must admit I am getting tired of all these "good proposals/ideas". don't you think we've gone thru this before? what you propose would require a custom vlan_output function which does nothing but setting the flag and then calls ether_output. what exactly is won with that? except making things less obvious? preparing for the highly likely case that something but a vlan interface (as in, IFT_L2VLAN) needs to add a L2VLAN ethernet header? I'm even going so far as claiming this isn't even a layering violation (of which we have MANY, usually for very good reasons) since vlan is an integral part of ethernet and not stacked on top. it's not like it'd be vlan header - ethernet header - payload it actually is slightly extended ethernet header - payload or if you wish, could be expressed as ethernet header w/ etype vlan - vlan tag, prio, inside etype - payload but the latter is not how it is implemented. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: vlan tagging surgery
* Alexey Suslikov [2014-04-21 12:38]:
> Henning Brauer bsws.de> writes:
>
> > > #if NVLAN > 0
> > > if (ifp->if_type == IFT_L2VLAN)
> > > return vlan_encap(ifp, m);
> > > #endif
> >
> > I don't think so, really. We're talking about 25 lines of code, in a
> > function that either prepends a regular ethernet header or a
> > vlan-ethernet header. Having both cases next to each other makes it
> > much more obvious what's going on imho.
>
> If I would be writing such a code, I wouldn't be breaking Single
> Responsibility Pattern and obfuscate ether_addheader with VLAN
> logic, but add small dedicated methods instead.
>
> Something like:
>
> ...
>
> #if NVLAN > 0
> //void ether_addvlanhdr1...
> //void ether_addvlanhdr2...
> #endif
>
> ...
>
> #if NVLAN > 0
> if (ifp->if_type == IFT_L2VLAN) {
> if ((p->if_capabilities & IFCAP_VLAN_HWTAGGING) &&
> (ifv->ifv_type == ETHERTYPE_VLAN)) {
> //ether_addvlanhdr1();
> } else {
> //ether_addvlanhdr2();
> return (0);
> }
> }
> #endif
congratulations, that is close to unauditable.
i put the vlan and the !vlan case next to each other ON PURPOSE. both
cases add an ethernet header, one with a few extra fields, one
without. Having that next to each other makes it f***ing obvious both
cases are identical, except the vlan case filling the 3 extra fields.
--
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/
Re: vlan tagging surgery
* Claudio Jeker [2014-04-21 11:14]: > __inline is dead long live inline. oups, that crept back in. > Would it make sense to put this into a vlan_encap function so that we can > reduce the amount of layer violation here? > > #if NVLAN > 0 > if (ifp->if_type == IFT_L2VLAN) > return vlan_encap(ifp, m); > #endif I don't think so, really. We're talking about 25 lines of code, in a function that either prepends a regular ethernet header or a vlan-ethernet header. Having both cases next to each other makes it much more obvious what's going on imho. > We could also add a ifp->if_encap function pointer but if it is just for > vlan(4) I see no point in it. indeed. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
