Giving love to ospf6d

[Denis Fondras]

Hello,

I want to use ospf6d but it is a bit rough around the edge.
A simple "ospf6ctl reload" makes it crash ! Come on, this is far from the
expected OpenBSD quality :p

I made a patch that make it crash less often. I still stumble on some "lost
interface" or "unknown interface" from time to time, lsa addresses are
often duplicated but it is better (from my point of view).

Concerning the duplicated lsa, this is what I get before reload :
orig_link_lsa: interface rl0
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::

And after reload :
orig_link_lsa: interface rl0
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: interface rl0
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: interface rl0
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: link local address fe80::20a:e4ff:fe03:87b4
orig_link_lsa: prefix 2001:7a8:b5ad:2030::
orig_link_lsa: prefix 2001:7a8:b5ad:2030::

The fix would be (I think, I am still investigating) to check for existing LSA
before adding a new one in ospfe_dispatch_main() (ospfe.c) when handling
IMSG_IFADDRNEW. Perhaps use a tree instead of a list ?

Would someone help me to give some love to ospf6d to improve it and make it
usable ?

Denis

Index: interface.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/interface.c,v
retrieving revision 1.22
diff -u -p -r1.22 interface.c
--- interface.c 27 Sep 2015 17:31:50 -  1.22
+++ interface.c 15 Dec 2015 16:40:33 -
@@ -170,8 +170,7 @@ int
 if_init(void)
 {
TAILQ_INIT();
-
-   return (fetchifs(0));
+   return (0);
 }
 
 /* XXX using a linked list should be OK for now */
Index: ospf6d.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
retrieving revision 1.29
diff -u -p -r1.29 ospf6d.c
--- ospf6d.c5 Dec 2015 13:12:41 -   1.29
+++ ospf6d.c15 Dec 2015 16:40:34 -
@@ -597,6 +597,7 @@ ospf_reload(void)
 {
struct area *area;
struct ospfd_conf   *xconf;
+   struct iface*iface;
 
if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL)
return (-1);
@@ -605,10 +606,16 @@ ospf_reload(void)
if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1)
return (-1);
 
-   /* send areas, interfaces happen out of band */
+   /* send areas & interfaces */
LIST_FOREACH(area, >area_list, entry) {
if (ospf_sendboth(IMSG_RECONF_AREA, area, sizeof(*area)) == -1)
return (-1);
+
+   LIST_FOREACH(iface, >iface_list, entry) {
+   if (ospf_sendboth(IMSG_RECONF_IFACE, iface,
+   sizeof(*iface)) == -1)
+   return (-1);
+   }
}
 
if (ospf_sendboth(IMSG_RECONF_END, NULL, 0) == -1)
Index: ospf6d.h
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
retrieving revision 1.29
diff -u -p -r1.29 ospf6d.h
--- ospf6d.h27 Sep 2015 17:31:50 -  1.29
+++ ospf6d.h15 Dec 2015 16:40:34 -
@@ -121,6 +121,7 @@ enum imsg_type {
IMSG_ABR_DOWN,
IMSG_RECONF_CONF,
IMSG_RECONF_AREA,
+   IMSG_RECONF_IFACE,
IMSG_RECONF_END,
IMSG_DEMOTE
 };
Index: ospfe.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospfe.c,v
retrieving revision 1.47
diff -u -p -r1.47 ospfe.c
--- ospfe.c 5 Dec 2015 13:12:41 -   1.47
+++ ospfe.c 15 Dec 2015 16:40:34 -
@@ -250,6 +250,7 @@ ospfe_dispatch_main(int fd, short event,
static struct area  *narea;
struct area *area;
struct iface*iface, *ifp;
+   struct iface*niface;
struct ifaddrchange *ifc;
struct iface_addr   *ia, *nia;
struct imsg  imsg;
@@ -388,6 +389,19 @@ ospfe_dispatch_main(int fd, short event,
RB_INIT(>lsa_tree);
 
LIST_INSERT_HEAD(>area_list, narea, entry);
+   break;
+   case IMSG_RECONF_IFACE:
+   if ((niface = malloc(sizeof(struct iface))) == NULL)
+   fatal(NULL);
+
+   memcpy(niface, imsg.data, 

Re: Reserve space for the L2 header in BPF injected frames

[Mike Belopuhov]

On Tue, Dec 15, 2015 at 14:30 +0100, Mike Belopuhov wrote:
> Hi,
> 
> This reserves max_linkhdr bytes for a link layer header in the newly
> allocated cluster in the bpf injection path like it's done for the
> packets originating on the host itself (cf. tcp_output).  Saves us
> time doing costly pool allocations later on.
> 
> I believe this has been tested by Yasuoka-san.
> 
> OK?
> 

I must withdraw the diff.  I've just taken another look at it and
there's already an L2 header handling in this function.  I'm not
certain what led me to believe that additional code is needed here.

11n ifmedia monitor mode

[Stefan Sperling]

11n ifmedia doesn't pick up monitor mode correctly.

ok?

Index: ieee80211.c
===
RCS file: /cvs/src/sys/net80211/ieee80211.c,v
retrieving revision 1.51
diff -u -p -r1.51 ieee80211.c
--- ieee80211.c 27 Nov 2015 04:03:45 -  1.51
+++ ieee80211.c 15 Dec 2015 17:05:34 -
@@ -366,6 +366,8 @@ ieee80211_media_init(struct ifnet *ifp,
if (ic->ic_caps & IEEE80211_C_HOSTAP)
ADD(ic, IFM_AUTO, mopt | IFM_IEEE80211_HOSTAP);
 #endif
+   if (ic->ic_caps & IEEE80211_C_MONITOR)
+   ADD(ic, IFM_AUTO, mopt | IFM_IEEE80211_MONITOR);
for (i = 0; i < IEEE80211_HT_NUM_MCS; i++) {
if (!isset(ic->ic_sup_mcs, i))
continue;
@@ -378,6 +380,9 @@ ieee80211_media_init(struct ifnet *ifp,
ADD(ic, IFM_IEEE80211_HT_MCS0 + i,
mopt | IFM_IEEE80211_HOSTAP);
 #endif
+   if (ic->ic_caps & IEEE80211_C_MONITOR)
+   ADD(ic, IFM_IEEE80211_HT_MCS0 + i,
+   mopt | IFM_IEEE80211_MONITOR);
}
ic->ic_flags |= IEEE80211_F_HTON; /* enable 11n by default */
}

UTF-8 support for ps(1)

[Ingo Schwarze]

Hi,

here is one program that i'd better not break.

This isn't used in the ramdisks, right?
Is anybody aware of any reacharound?

A few remarks:
 * The important use cases are non-ASCII characters in command line
   arguments, environment variables, and program names.  Even though
   i don't intend to support non-ASCII user and group names in
   general, it costs nothing to handle them gracefully here, the
   code even becomes a few lines shorter.
 * The specific UTF-8 handling function mbswprint() - for "multi-byte
   string width-limited print" - is yet again somewhat different
   from what other utilities needed, both because no other utility
   needed this kind of truncation yet and because ps(1) wants to
   do a peculiar vis(3) encoding.  Anyway, the function is almost
   as short as the others and uses the same interfaces in the same
   ways, plus vis(3).
 * It might be arguable whether the vis(3) encoding done by ps(1)
   is ideal.  But that seems like a different topic.  For now, i'm
   making sure that nothing changes in the C/POSIX locale and that
   even in a UTF-8 locale, nothing changes for invalid bytes.  Even
   in a UTF-8 locale, the new code only lets through printable UTF-8
   characters, doing correct columnation for them.
 * In a UTF-8 locale, i'm replacing valid, but non-printable UTF-8
   characters with the Unicode replacement character U+FFFD.  Letting
   them through would be a bad idea because they might get assigned
   dangerous purposes in the future.  Alternatively, they could be
   treated just like invalid bytes, C-encoding the bytes individually.
   I don't feel strongly about that.
 * For a single function (mbswprint) used from a single file
   (print.c), i don't see the point in polluting header files.
   It's more straightforward and easier on the eye to just put the
   prototype into the one file using it.
 * The width limiting code in fmt.c, used only from print.c, becomes
   completely obsolete; if this gets committed, i'll cvs rm the
   file.  The new file utf8.c includes the functionality in simpler
   form, along with the more complicated multibyte character handling.
   Yet, the new file is shorter and cannot fail in malloc(3).
 * Grand total, the diff is -142 +114 lines.

Comments, concerns, OKs?
  Ingo


Index: Makefile
===
RCS file: /cvs/src/bin/ps/Makefile,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile
--- Makefile16 Jul 2014 19:57:34 -  1.9
+++ Makefile15 Dec 2015 08:12:45 -
@@ -1,7 +1,7 @@
 #  $OpenBSD: Makefile,v 1.9 2014/07/16 19:57:34 okan Exp $
 
 PROG=  ps
-SRCS=  fmt.c keyword.c nlist.c print.c ps.c
+SRCS=  keyword.c nlist.c print.c ps.c utf8.c
 DPADD= ${LIBM} ${LIBKVM}
 LDADD= -lm -lkvm
 
Index: extern.h
===
RCS file: /cvs/src/bin/ps/extern.h,v
retrieving revision 1.17
diff -u -p -r1.17 extern.h
--- extern.h29 Jun 2015 15:03:33 -  1.17
+++ extern.h15 Dec 2015 08:12:45 -
@@ -48,8 +48,6 @@ void   command(const struct kinfo_proc *,
 voidcputime(const struct kinfo_proc *, VARENT *);
 int donlist(void);
 voidemulname(const struct kinfo_proc *, VARENT *);
-voidfmt_puts(const char *, int *);
-voidfmt_putc(int, int *);
 double  getpcpu(const struct kinfo_proc *);
 double  getpmem(const struct kinfo_proc *);
 voidgname(const struct kinfo_proc *, VARENT *);
Index: print.c
===
RCS file: /cvs/src/bin/ps/print.c,v
retrieving revision 1.64
diff -u -p -r1.64 print.c
--- print.c 25 Oct 2015 15:26:53 -  1.64
+++ print.c 15 Dec 2015 08:12:45 -
@@ -55,6 +55,8 @@
 extern kvm_t *kd;
 extern int needenv, needcomm, neednlist, commandonly;
 
+int mbswprint(const char *, int, int);  /* utf8.c */
+
 static char *cmdpart(char *);
 
 #definemin(a,b)((a) < (b) ? (a) : (b))
@@ -97,6 +99,13 @@ command(const struct kinfo_proc *kp, VAR
int left, wantspace = 0;
char **argv, **p;
 
+   /*
+* Determine the available number of display columns.
+* Always decrement and check after writing.
+* No check is needed before mbswprint()
+* and after writing the last data, though.
+*/
+
v = ve->var;
if (ve->next != NULL || termwidth != UNLIMITED) {
if (ve->next == NULL) {
@@ -106,74 +115,76 @@ command(const struct kinfo_proc *kp, VAR
} else
left = v->width;
} else
-   left = -1;
+   left = INT_MAX;
+
if (needenv && kd != NULL) {
argv = kvm_getenvv(kd, kp, termwidth);
if ((p = argv) != NULL) {
while (*p) {
-   fmt_puts(*p, );
+   if (wantspace) {
+   putchar(' ');
+

Re: removing expired once rules in pf_purge_thread()

[Mike Belopuhov]

On 15 December 2015 at 15:36, Alexandr Nedvedicky
 wrote:
> Hello,
>
>> >
>> >Another possibility would be to require 'once' rules to be 'quick'.
>> >This closes the candidacy window and makes its serialisation, to
>> >preclude multiple matches, more feasible.
>> >
>> >Yet another possibility is to drop 'once' rules as too complex to
>> >implement for multiprocessor but I have no idea if this is viable.
>> >
>>
>> It is.  And I have said that before with an authority of the implementor
>> of "once" rules: since we don't have any userland applications that
>> would use this yet, we can ditch them for now and possibly devise a
>> better approach later.
>>
>> Don't make your lives harder than they have to be!
>>
>
> I'm just trying out patch improved by Richard. I like his idea to put anchor
> rule to garbage collector list. I'd like to give it a try on Solaris. There
> might be small changes in details.
>
> IMO once rules are fine, and offloading hard task to service/garbage collector
> thread works just fine.
>
> regards
> sasha

It just occurred to me that another possibility would be a match-only
rule that matches one but doesn't involve any purging machinery.  Right
now we install ftp-proxy rules as having maximum number of states equal
to 1, however there's a time window between the moment the state is no
longer there, but anchor is not gone yet and it can potentially create
more states which is not a problem for an eavesdropper who can sniff
the plaintext protocol.

Re: removing expired once rules in pf_purge_thread()

[Alexandr Nedvedicky]

Hello,

> It just occurred to me that another possibility would be a match-only
> rule that matches one but doesn't involve any purging machinery.  Right
> now we install ftp-proxy rules as having maximum number of states equal
> to 1, however there's a time window between the moment the state is no
> longer there, but anchor is not gone yet and it can potentially create
> more states which is not a problem for an eavesdropper who can sniff
> the plaintext protocol.

I'm not sure I'm following you. IMO the expire flag should solve that problem.
As soon as rule is marked as expired it can not be matched by packet any more.

I'm attaching yet another version of patch. The list of changes to
patch sent by Richard is as follows:

- atomic operations are gone as we don't need them
  right now. those will be introduced as soon as we will get
  there. pf_test()/pf_test_rule() is still processing a single
  packet at a time.

- it's better to use TAILQ_EMPTY() instead of checking link
  members to determine if last rule is being removed from   
  anchor `a`

- I've also realized we have to call pf_purge_expired_rules()
  from pf_commit_rules(). We have to make sure expired rules are gone
  from active lists. I think this is the best approach for pf@Puffy
  currently. It will be changed as soon as ruleset locks and
  reference counting for rules will be merged in. I had to also
  reshuffle a consistency lock a bit in pf_purge_thread().

- last but not least: silly name got renamed (s/myruleset/ruleset)

so what do you think? OK?

regards
sasha

-8<8<8<---8<
Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.960
diff -u -p -r1.960 pf.c
--- pf.c6 Dec 2015 10:03:23 -   1.960
+++ pf.c16 Dec 2015 00:52:22 -
@@ -295,6 +295,9 @@ RB_GENERATE(pf_state_tree, pf_state_key,
 RB_GENERATE(pf_state_tree_id, pf_state,
 entry_id, pf_state_compare_id);
 
+SLIST_HEAD(pf_rule_gcl, pf_rule)   pf_rule_gcl =
+   SLIST_HEAD_INITIALIZER(pf_rule_gcl);
+
 __inline int
 pf_addr_compare(struct pf_addr *a, struct pf_addr *b, sa_family_t af)
 {
@@ -1137,6 +1140,18 @@ pf_state_export(struct pfsync_state *sp,
 /* END state table stuff */
 
 void
+pf_purge_expired_rules(void)
+{
+   struct pf_rule  *r;
+
+   while ((r = SLIST_FIRST(_rule_gcl)) != NULL) {
+   SLIST_REMOVE(_rule_gcl, r, pf_rule, gcle);
+   KASSERT(r->rule_flag & PFRULE_EXPIRED);
+   pf_purge_rule(r);
+   }
+}
+
+void
 pf_purge_thread(void *v)
 {
int nloops = 0, s;
@@ -1154,6 +1169,11 @@ pf_purge_thread(void *v)
if (++nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
pf_purge_expired_fragments();
pf_purge_expired_src_nodes(0);
+   if (!SLIST_EMPTY(_rule_gcl)) {
+   rw_enter_write(_consistency_lock);
+   pf_purge_expired_rules();
+   rw_exit_write(_consistency_lock);
+   }
nloops = 0;
}
 
@@ -3135,6 +3155,10 @@ pf_test_rule(struct pf_pdesc *pd, struct
ruleset = _main_ruleset;
r = TAILQ_FIRST(pf_main_ruleset.rules.active.ptr);
while (r != NULL) {
+   if (r->rule_flag & PFRULE_EXPIRED) {
+   r = TAILQ_NEXT(r, entries);
+   goto nextrule;
+   }
r->evaluations++;
PF_TEST_ATTRIB((pfi_kif_match(r->kif, pd->kif) == r->ifnot),
r->skip[PF_SKIP_IFP].ptr);
@@ -3433,8 +3457,15 @@ pf_test_rule(struct pf_pdesc *pd, struct
}
 #endif /* NPFSYNC > 0 */
 
-   if (r->rule_flag & PFRULE_ONCE)
-   pf_purge_rule(ruleset, r, aruleset, a);
+   if (r->rule_flag & PFRULE_ONCE) {
+   if ((a != NULL) && TAILQ_EMPTY(a->ruleset->rules.active.ptr)) {
+   a->rule_flag |= PFRULE_EXPIRED;
+   SLIST_INSERT_HEAD(_rule_gcl, a, gcle);
+   }
+
+   r->rule_flag |= PFRULE_EXPIRED;
+   SLIST_INSERT_HEAD(_rule_gcl, r, gcle);
+   }
 
 #ifdef INET6
if (rewrite && skw->af != sks->af)
Index: pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.297
diff -u -p -r1.297 pf_ioctl.c
--- pf_ioctl.c  3 Dec 2015 13:30:18 -   1.297
+++ pf_ioctl.c  16 Dec 2015 00:52:25 -
@@ -301,12 +301,13 @@ pf_rm_rule(struct pf_rulequeue *rulequeu
 }
 
 void
-pf_purge_rule(struct pf_ruleset *ruleset, struct pf_rule *rule,
-struct pf_ruleset *aruleset, struct pf_rule *arule)
+pf_purge_rule(struct pf_rule *rule)
 

Re: preparing multitouch support - request for tests

[Ulf Brosziewski]

Ping? No further thoughts on this, no tests? Do I have to conclude that
most people are happy with wsmouse as it is?

I'm aware that the diff isn't small, but anything smaller would make
some of the remaining parts void and possibly blur the concept. The core
of the approach is actually quite simple: wsmouse_input() will be
replaced by a set of functions for reporting state, and by
wsmouse_input_sync(), which generates the wscons events (please note
that it doesn't produce MT events yet; there are no userland drivers
that could use them). Running mice, and tablets and touchpads that can
only track a single contact wouldn't need much more. MT support needs
some bits of configuration: the number of touches that might be tracked
simultaneously, and the information whether a buffer for "MT tracking"
is required.

For use with the synaptics driver, MT input from touchpads must be
converted into a single-touch representation, and this conversion is
based on a mechanism that selects a "pointer-controlling" slot. The
implementation generalizes the method currently applied by the
Elantech-v4 packet handler in pms.

Single-touch representations are also the base for handling
"compat-mode" of touchpads in wsmouse. Currently, each hardware driver
tracks absolute coordinates in its device data, and has a compat-mode
branch in its packet handler that computes relative coordinates, applies
a more or less arbitrary scaling to them, and calls wsmouse_input() with
the "DELTA" flags. This is not only ugly, it would be even more ugly for
touchpad drivers that use the new MT functions: It would still require a
driver-internal representation of the input, a driver-internal selection
of the pointer-controlling touch, etc. That's obviously nonsense, compat
mode should be handled by wsmouse. However, it requires some
configuration.

There is a second point concerning compat mode that I would like to
change: it could be made useful. Because of the arbitrary scaling and
the unpredictable pointer movement, I cannot use it with wsmoused at the
console. Do touchpads exist where it works?  Recently Thierry Deval
posted a diff here which proved that we could easily do something about
that, but that is a different story. In my diff, wsmouseinput hooks its
"touchpad extension" (wstpad) into the compat-mode conversion function,
which works well with ws for all touchpads that are available to me.

The internal configuration of wstpad may cause headaches, it seems to be
easy to make a mess out if it. This is one of the reasons why there are
no ioctl mechanisms yet for a configuration from userland.

Up to now, I had less headaches with the input-processing parts of
wstpad, and I don't believe that a decent driver necessarily looks like
the synaptics driver, or the touchpad module of libinput. Its tapping
handler, for example, seems to be built on a somewhat unlucky
abstraction (I mean the "state machine", where states mirror input
states and sequences of input states; in wstpad, the "states" of the
handler correspond the kind of decision that is to be taken next). Of
course I don't want to suggest that this is children's play; e.g., I
haven't worked seriously on the palm detection part up to now (I cannot
do that, because I don't have "real" problems with accidental touches).
Many things that may sound trivial first may turn out to be intricate
(e.g., how do you distinguish a two-finger click from a click-and-drag
action? This might need a timeout. Perhaps Mac users can help here ...)


On 12/03/2015 12:20 AM, Ulf Brosziewski wrote:

The diffs below contain a complete and extensive rewrite of the
input-processing parts of wsmouse and the interface it provides to
the hardware drivers. It prepares the support for various kinds of
multitouch input, as well as an extended support for touchpads by
wsmouse.

Tests for regression with all kinds of "pointing devices" would be
welcome. Only a small set of touchpads and USB mice is available to
me, which is a somewhat uncomfortable situation when you are working
on things like this.

Please note that the first diff is for the synaptics driver in
xenocara, the rest is for the kernel. Patching that driver will be
necessary if you test with touchpads (and compiling it requires
the modified version of wsconsio.h in /usr/include/dev/wscons/).

In most drivers I have made only short and trivial changes, the
Elantech-v4 part of pms is the only one that makes full use of the
new MT functions.

Unlike the basic input layer, which I hope is already fairly stable,
the in-kernel touchpad support is in a more experimental stage. If
you have a Synaptics, ALPS, or Elantech-v4 touchpad, you could test
it by adding this xorg.conf to /etc:

Section "InputClass"
 Identifier "wstpad"
 Driver "ws"
 MatchIsTouchPad "true"
EndSection

Only a default configuration will be available with this. It enables
two-finger-scrolling and a lower soft-button area for clickpads, and
two-finger- or edge-scrolling for 

Re: removing expired once rules in pf_purge_thread()

[Richard Procter]

On Tue, 15 Dec 2015, Mike Belopuhov wrote:

> >Yet another possibility is to drop 'once' rules as too complex to
> >implement for multiprocessor but I have no idea if this is viable.
> 
> It is.  And I have said that before with an authority of the implementor
> of "once" rules: since we don't have any userland applications that
> would use this yet, we can ditch them for now and possibly devise a
> better approach later.
 
> Don't make your lives harder than they have to be!

I tend to agree! And I can't see a way to reimplement it for a 
multithreaded pf without introducing downsides.

Quoting pf.conf(5) "once - Creates a one shot rule that will remove itself 
from an active ruleset after the first match."

A 'first' match presupposes a total ordering. This comes for free when pf 
is single-threaded but concurrent rule matching must take the trouble to 
re-impose it somewhere. (I assume that pf aims to do concurrent matching 
of packets against the ruleset.)

One approach serialises the candidacy of 'once' rules during rule 
matching. This ensures an unambiguous first match but costs pf a latent 
glass jaw. In the worst case, a 'once' rule heads the ruleset and matches 
any packet. Its candidacy therefore covers the entire rule-matching 
process and so fully serialises it.
 
The other approach serialises the packets matched by 'once' rules, after 
rule processing is done. This doesn't create a glass jaw but does permit 
the 'once' rule to match more than once.

To my taste neither alternative is especially palatable.

That said, a 'quick' 'once' rule has no candidacy window so it can be 
serialised by atomically testing/setting a flag when evaluating it. This 
also both avoids the glass jaw and provides at most one first match. 

(Note that this issue of determining the first match is independent of 
whether a 'once' rule, having matched, is purged.)

On Wed, 16 Dec 2015, Mike Belopuhov wrote:

> It just occurred to me that another possibility would be a match-only 
> rule that matches one but doesn't involve any purging machinery.

That is an interesting idea. Like a 'quick' rule, a match rule has no 
candidacy window. However a 'once' match rule can no longer influence 
pass/block state --- I can't see how it is usefull? 

best, 
Richard. 

vmx(4) incorrect m_pulldown usage

[Mike Belopuhov]

Hi,

This has been in my tree for a while and I believe Yasuoka-san has
tested it in the scenario where it was crashing.

m_pulldown is done here with a zero offset which means that if
there's been no space reserved for the Ethernet header in the mbuf
or the cluster it will allocate a new chunk of memory and return a
new pointer that the vmx code ignores.  This can realistically
happen only during the bpf injection.

The diff below makes sure to keep the modified chain pointer around
and passes it back to the calling code so that it will get properly
accounted for.  m_pulldown with a zero offset is equivalent to a
m_pullup.

OK?


diff --git sys/dev/pci/if_vmx.c sys/dev/pci/if_vmx.c
index 055cfe1..50edb0b 100644
--- sys/dev/pci/if_vmx.c
+++ sys/dev/pci/if_vmx.c
@@ -164,11 +164,11 @@ void vmxnet3_stop(struct ifnet *);
 void vmxnet3_reset(struct vmxnet3_softc *);
 int vmxnet3_init(struct vmxnet3_softc *);
 int vmxnet3_ioctl(struct ifnet *, u_long, caddr_t);
 void vmxnet3_start(struct ifnet *);
 int vmxnet3_load_mbuf(struct vmxnet3_softc *, struct vmxnet3_txring *,
-struct mbuf *);
+struct mbuf **);
 void vmxnet3_watchdog(struct ifnet *);
 void vmxnet3_media_status(struct ifnet *, struct ifmediareq *);
 int vmxnet3_media_change(struct ifnet *);
 void *vmxnet3_dma_allocmem(struct vmxnet3_softc *, u_int, u_int, bus_addr_t *);
 
@@ -1063,11 +1063,11 @@ vmxnet3_start(struct ifnet *ifp)
 
IFQ_DEQUEUE(>if_snd, m);
if (m == NULL)
break;
 
-   n = vmxnet3_load_mbuf(sc, ring, m);
+   n = vmxnet3_load_mbuf(sc, ring, );
if (n == -1) {
ifp->if_oerrors++;
continue;
}
 
@@ -1087,13 +1087,14 @@ vmxnet3_start(struct ifnet *ifp)
}
 }
 
 int
 vmxnet3_load_mbuf(struct vmxnet3_softc *sc, struct vmxnet3_txring *ring,
-struct mbuf *m)
+struct mbuf **mp)
 {
struct vmxnet3_txdesc *txd, *sop;
+   struct mbuf *n, *m = *mp;
bus_dmamap_t map;
u_int hlen = ETHER_HDR_LEN, csum_off;
u_int prod;
int gen, i;
 
@@ -1105,29 +1106,29 @@ vmxnet3_load_mbuf(struct vmxnet3_softc *sc, struct 
vmxnet3_txring *ring,
sc->sc_dev.dv_xname);
return -1;
}
 #endif
if (m->m_pkthdr.csum_flags & (M_TCP_CSUM_OUT|M_UDP_CSUM_OUT)) {
-   struct mbuf *mp;
struct ip *ip;
int offp;
 
if (m->m_pkthdr.csum_flags & M_TCP_CSUM_OUT)
csum_off = offsetof(struct tcphdr, th_sum);
else
csum_off = offsetof(struct udphdr, uh_sum);
 
-   mp = m_pulldown(m, hlen, sizeof(*ip), );
-   if (mp == NULL)
+   n = m_pulldown(m, hlen, sizeof(*ip), );
+   if (n == NULL)
return (-1);
 
-   ip = (struct ip *)(mp->m_data + offp);
+   ip = (struct ip *)(n->m_data + offp);
hlen += ip->ip_hl << 2;
 
-   mp = m_pulldown(m, 0, hlen + csum_off + 2, );
-   if (mp == NULL)
+   *mp = m_pullup(m, hlen + csum_off + 2);
+   if (*mp == NULL)
return (-1);
+   m = *mp;
}
 
switch (bus_dmamap_load_mbuf(sc->sc_dmat, map, m, BUS_DMA_NOWAIT)) {
case 0:
break;

Reserve space for the L2 header in BPF injected frames

[Mike Belopuhov]

Hi,

This reserves max_linkhdr bytes for a link layer header in the newly
allocated cluster in the bpf injection path like it's done for the
packets originating on the host itself (cf. tcp_output).  Saves us
time doing costly pool allocations later on.

I believe this has been tested by Yasuoka-san.

OK?

diff --git sys/net/bpf.c sys/net/bpf.c
index 86f5f6d..6cdf789 100644
--- sys/net/bpf.c
+++ sys/net/bpf.c
@@ -200,17 +200,18 @@ bpf_movein(struct uio *uio, u_int linktype, struct mbuf 
**mp,
 
MGETHDR(m, M_WAIT, MT_DATA);
m->m_pkthdr.ph_ifidx = 0;
m->m_pkthdr.len = len - hlen;
 
-   if (len > MHLEN) {
-   MCLGETI(m, M_WAIT, NULL, len);
+   if (len + max_linkhdr > MHLEN) {
+   MCLGETI(m, M_WAIT, NULL, len + max_linkhdr);
if ((m->m_flags & M_EXT) == 0) {
error = ENOBUFS;
goto bad;
}
}
+   m->m_data += max_linkhdr;
m->m_len = len;
*mp = m;
 
error = uiomovei(mtod(m, caddr_t), len, uio);
if (error)

Re: removing expired once rules in pf_purge_thread()

[Mike Belopuhov]

On 13 December 2015 at 18:56, Richard Procter
 wrote:
>
> If I understand this patch:
>
> Rule removal requires the consistency lock but to acquire it one must be
> able to sleep. pf_test_rule() cannot sleep as it executes in a (soft)
> interrupt handler, so it passes the expired rule to a thread which can.
>
> However, the code to remove a 'once' rule may wish to also remove its
> parent rule, now computed in the other thread.

This is right: "once" rules remove it's parent anchor if it's empty.

> The updated patch below
> avoids passing that state between threads in struct pf_rule by explictly
> passing the parent rule to be purged when necessary.
>
> There also seem to me a couple of issues, one hairier than the other:
>
>  - r->rule_flag should be updated atomically with respect to other
>(potential) updates (included in the attached patch).
>
>  - Multiple rule-matching threads may match the same 'once' rule before
>one of them reaches the code to set PFRULE_EXPIRED. A thread may even
>match a 'once' rule after PFRULE_EXPIRED is set, if that occurs during
>the 'once' rule's candidacy as a potential match. (This issue is not
>addressed by the attached patch.)
>
>This creates two problems:
>
> a) rules may be purged more than once
> b) what to do with packets matched by expired 'once' rules
>
>In general, we do not know whether or not a 'once' rule candidate is
>the match until a new candidate rule replaces it or the rule matching
>process completes. Serialising this window of candicacy would prevent a
>'once' rule becoming a candidate, and possibly the match, for multiple
>packets at once. That's one approach.
>
>A weaker approach instead permits multiple matches of one 'once' rule
>but processes these serially in a critical section afterwards. The
>first thread to reach it is deemed the first match (note this isn't
>necessarily the first matching packet received at the interface!). It
>then atomically marks it PFRULE_EXPIRED and places it on the purge
>queue. Packets for matches of expired 'once' rule, it seems to me, can
>be dropped as if we never saw them. An alternative might be to retry
>them against the updated rules.
>
>Another possibility would be to require 'once' rules to be 'quick'.
>This closes the candidacy window and makes its serialisation, to
>preclude multiple matches, more feasible.
>
>Yet another possibility is to drop 'once' rules as too complex to
>implement for multiprocessor but I have no idea if this is viable.
>

It is.  And I have said that before with an authority of the implementor
of "once" rules: since we don't have any userland applications that
would use this yet, we can ditch them for now and possibly devise a
better approach later.

Don't make your lives harder than they have to be!

Re: ntpd setting date incorrectly

[Ian Mcwilliam]

Disable sensors * in ntpd.conf and time is good again.

I see this on boot up when things go strange.

hw.sensors.vmt0.timedelta0=1450237689.498077 secs, OK, Tue Nov 29 18:36:38.371

I wonder if it's related to this change? Thoughts?

===
RCS file: /cvs/src/sys/dev/pv/vmt.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- src/sys/dev/pv/vmt.c2015/08/27 19:51:36 1.5
+++ src/sys/dev/pv/vmt.c2015/12/11 16:07:01 1.6
@@ -1,4 +1,4 @@
-/* $OpenBSD: vmt.c,v 1.5 2015/08/27 19:51:36 deraadt Exp $ */
+/* $OpenBSD: vmt.c,v 1.6 2015/12/11 16:07:01 mpi Exp $ */
 
 /*
  * Copyright (c) 2007 David Crawshaw 
@@ -365,7 +365,7 @@
sensordev_install(>sc_sensordev);
 
timeout_set(>sc_tick, vmt_tick, sc);
-   if (mountroothook_establish(vmt_tick, sc) == NULL)
+   if (startuphook_establish(vmt_tick, sc) == NULL)
DPRINTF("%s: unable to establish tick\n", DEVNAME(sc));
 
timeout_set(>sc_tclo_tick, vmt_tclo_tick, sc);


Ian McWilliam


From: Ian Mcwilliam
Sent: Tuesday, 15 December 2015 12:04 PM
To: tech@openbsd.org
Subject: ntpd setting date incorrectly

Not sure what has changed in the last couple of days but apparently I need to 
get back from the future.

This is an OpenBSD VM running under VMWare Fusion Professional Version 8.0.2 
(3164312) Mac OS X 10.11.2


Dec 14 17:22:44 ianm-openbsd ntpd[13773]: peer 121.0.0.41 now valid
Dec 14 17:22:46 ianm-openbsd ntpd[13773]: peer 192.189.54.17 now valid
Dec 14 17:22:49 ianm-openbsd ntpd[13773]: peer 54.252.165.245 now valid
Dec 14 17:22:49 ianm-openbsd ntpd[13773]: peer 54.252.129.186 now valid
Dec 14 17:23:44 ianm-openbsd ntpd[19417]: adjusting local clock by 45.252476s

Machine shutdown

Machine rebooted next day

Dec 15 09:26:00 ianm-openbsd ntpd[1991]: ntp engine ready

time correct until

Nov 27 07:52:36 ianm-openbsd ntpd[21778]: set local clock to Sun Nov 27 
07:52:36 AEDT 2061 (offset 1450131996.187864s)
Nov 27 07:52:36 ianm-openbsd savecore: no core dump
Nov 27 07:52:37 ianm-openbsd apmd: battery status: absent. external power 
status: connected. estimated battery life 0%
Nov 27 07:53:56 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131950.806132s
Nov 27 07:55:26 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131950.410569s
Nov 27 07:56:56 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131949.960758s
Nov 27 07:58:26 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131949.511029s
Nov 27 07:59:56 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131949.061268s
Nov 27 08:01:26 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131948.611426s
Nov 27 08:02:56 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131948.161660s
Nov 27 08:04:26 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131947.711849s
Nov 27 08:05:56 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131947.262024s
Nov 27 08:07:26 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131946.812191s
Nov 27 08:07:32 ianm-openbsd dhclient[21622]: DHCPDISCOVER on em0 - interval 1
Nov 27 08:07:33 ianm-openbsd dhclient[21622]: DHCPDISCOVER on em0 - interval 1
Nov 27 08:07:33 ianm-openbsd dhclient[21622]: DHCPOFFER from 172.16.28.254 
(00:50:56:f1:c4:f7)
Nov 27 08:07:33 ianm-openbsd dhclient[21622]: DHCPREQUEST on em0 to 
255.255.255.255
Nov 27 08:07:33 ianm-openbsd dhclient[21622]: DHCPACK from 172.16.28.254 
(00:50:56:f1:c4:f7)
Nov 27 08:07:33 ianm-openbsd dhclient[21622]: bound to 172.16.28.141 -- renewal 
in 900 seconds.
Nov 27 08:08:56 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131946.362273s

manually setting the date/time shows that things appear fine until next reboot.

Nov 27 08:57:27 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131932.766352s
Nov 27 08:58:57 ianm-openbsd ntpd[11805]: adjusting local clock by 
-1450131932.316650s
Dec 15 10:34:21 ianm-openbsd ntpd[11805]: adjusting local clock by 33.548572s
Dec 15 10:34:21 ianm-openbsd ntpd[1991]: clock is now synced
Dec 15 10:35:52 ianm-openbsd ntpd[11805]: adjusting local clock by 33.107247s
Dec 15 10:35:52 ianm-openbsd ntpd[1991]: clock is now unsynced
Dec 15 10:37:21 ianm-openbsd ntpd[11805]: adjusting local clock by 32.662256s
Dec 15 10:38:51 ianm-openbsd ntpd[11805]: adjusting local clock by 32.212314s
Dec 15 10:40:21 ianm-openbsd ntpd[11805]: adjusting local clock by 31.762446s
Dec 15 10:41:38 ianm-openbsd ntpd[1991]: constraint reply from 74.125.237.17: 
offset 30.905949
Dec 15 10:41:38 ianm-openbsd ntpd[1991]: constraint reply from 74.125.237.20: 
offset 30.905572
Dec 15 10:41:38 ianm-openbsd ntpd[1991]: constraint reply from 74.125.237.19: 
offset 30.903128
Dec 15 10:41:38 ianm-openbsd ntpd[1991]: constraint reply from 74.125.237.18: 
offset 30.900721
Dec 15 10:41:38 ianm-openbsd ntpd[1991]: constraint reply from 74.125.237.16: 

Re: remove unused variables in netcat.c

[Bob Beck]

On Thu, Oct 15, 2015 at 10:30:18PM -0400, Rob Pierce wrote:
> It looks like the subject and issuer variables are no longer used in
> report_tls() since the recent libtls api change. Also a few whitespace

You're correct.  while I'm at it how's this.. (I'll hit the whitespace too 
separately)

Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.146
diff -u -p -u -p -r1.146 netcat.c
--- netcat.c8 Dec 2015 15:33:33 -   1.146
+++ netcat.c16 Dec 2015 03:53:39 -
@@ -1466,7 +1466,7 @@ map_tls(char *s, int *val)
 void
 report_tls(struct tls * tls_ctx, char * host, char *tls_expectname)
 {
-   char *subject = NULL, *issuer = NULL;
+   time_t t;
fprintf(stderr, "TLS handshake negotiated %s/%s with host %s\n",
tls_conn_version(tls_ctx), tls_conn_cipher(tls_ctx), host);
fprintf(stderr, "Peer name %s\n",
@@ -1477,11 +1477,13 @@ report_tls(struct tls * tls_ctx, char * 
if (tls_peer_cert_issuer(tls_ctx))
fprintf(stderr, "Issuer: %s\n",
tls_peer_cert_issuer(tls_ctx));
+   if ((t = tls_peer_cert_notbefore(tls_ctx)) != -1)
+   fprintf(stderr, "Valid From: %s", ctime());
+   if ((t = tls_peer_cert_notafter(tls_ctx)) != -1)
+   fprintf(stderr, "Valid Until: %s", ctime());
if (tls_peer_cert_hash(tls_ctx))
fprintf(stderr, "Cert Hash: %s\n",
tls_peer_cert_hash(tls_ctx));
-   free(subject);
-   free(issuer);
 }
 void
 report_connect(const struct sockaddr *sa, socklen_t salen)

Re: removing expired once rules in pf_purge_thread()

[Alexandr Nedvedicky]

Hello,

> >
> >Another possibility would be to require 'once' rules to be 'quick'.
> >This closes the candidacy window and makes its serialisation, to
> >preclude multiple matches, more feasible.
> >
> >Yet another possibility is to drop 'once' rules as too complex to
> >implement for multiprocessor but I have no idea if this is viable.
> >
> 
> It is.  And I have said that before with an authority of the implementor
> of "once" rules: since we don't have any userland applications that
> would use this yet, we can ditch them for now and possibly devise a
> better approach later.
> 
> Don't make your lives harder than they have to be!
> 

I'm just trying out patch improved by Richard. I like his idea to put anchor
rule to garbage collector list. I'd like to give it a try on Solaris. There
might be small changes in details.

IMO once rules are fine, and offloading hard task to service/garbage collector
thread works just fine.

regards
sasha

wire libevent messages up to tftpd logging

[David Gwynne]

if libevent dies it would be nice to have some kind of record of
why it decided that was a good idea. by default those messages go
nowhere.

this has tftp register a log callback for libevent to call. if
something goes wrong it should end up in the logs.

ok?

Index: tftpd.c
===
RCS file: /cvs/src/usr.sbin/tftpd/tftpd.c,v
retrieving revision 1.34
diff -u -p -r1.34 tftpd.c
--- tftpd.c 14 Dec 2015 16:34:55 -  1.34
+++ tftpd.c 16 Dec 2015 01:08:55 -
@@ -189,6 +189,8 @@ int parse_options(struct tftp_client *,
struct opt_client *);
 intvalidate_access(struct tftp_client *, const char *);
 
+void   log_libevent(int, const char *);
+
 struct tftp_client *
client_alloc(void);
 void   client_free(struct tftp_client *client);
@@ -361,6 +363,7 @@ main(int argc, char *argv[])
if (pledge("stdio rpath wpath cpath fattr dns inet", NULL) == -1)
err(1, "pledge");
 
+   event_set_log_callback(log_libevent);
event_init();
 
if (rewrite != NULL)
@@ -1625,3 +1628,30 @@ syslog_info(const char *fmt, ...)
va_end(ap);
 }
 
+void
+log_libevent(int severity, const char *msg)
+{
+   const char *level = "msg";
+
+   switch (severity) {
+   case _EVENT_LOG_DEBUG:
+   if (!verbose)
+   return;
+   level = "debug";
+   break;
+   case _EVENT_LOG_MSG:
+   level = "msg";
+   break;
+   case _EVENT_LOG_WARN:
+   level = "warn";
+   break;
+   case _EVENT_LOG_ERR:
+   level = "err";
+   break;
+   default:
+   lerrx(1, "%s severity %d", __func__, severity);
+   return;
+   }
+
+   lwarnx("event %s: %s", level, msg);
+}

ksh: variable substitution in double quoted string

[Dmitrij D. Czarkoff]

Hi!

I recently came across a shell script that uses idiom

  var1=var1
  var2=var2
  echo "${var1+($var2)}"

ksh(1) doesn't like it:

  ksh: ${var1+($var2)}": bad substitution

Meanwhile bash and dash just print:

  (var2)

Apparently ksh tries to parse parenthesis within substituted word.
According to 2.2.3 Double-Quotes[1] of POSIX Shell Command Language,
parentheses should not be parsed within double quotes, although 2.6.2
Parameter Expansion[2] sets special rules for parameter substitution
within double quotes, and those don't mention whether substituted word
should be considered quoted or not.

Patch below changes ksh's behavior to match that of bash and dash.  I am
not decided on the matter.

-- 
Dmitrij D. Czarkoff

[1] 
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
[2] 
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02

Index: lex.c
===
RCS file: /cvs/src/bin/ksh/lex.c,v
retrieving revision 1.64
diff -u -p -r1.64 lex.c
--- lex.c   18 Nov 2015 15:31:21 -  1.64
+++ lex.c   16 Dec 2015 07:05:42 -
@@ -579,6 +579,15 @@ yylex(int cf)
break;
 
case SBRACEQ:
+   /*{*/
+   if (c == '}') {
+   POP_STATE();
+   *wp++ = CSUBST;
+   *wp++ = /*{*/ '}';
+   } else
+   goto Subst;
+   break;
+
case SBRACE:
/*{*/
if (c == '}') {