Re: Merge pfkeyv2_socket and rawcb into one struct

[Martin Pieuchot]

On 30/05/17(Tue) 14:05, Claudio Jeker wrote:
> This is more or less the same thing for PF_KEY that we now do in PF_ROUTE.
> Use one PCB LIST on the keycb and embedd the rawcb in that PF_KEY cb.
> Diff also has a few variable renames in it to make this code less alien
> regarding the rest of our kernel. Mainly use so instead of socket and
> pfkeyv2_socket is also replaced with better variable names.
> 
> This needs the previous diff I just sent out for PF_ROUTE.
> After that I can make pfkey use the same SRPL_LIST as PF_ROUTE (from an
> other diff) to unlock them more.
> -- 
> :wq Claudio
> 
> Index: net/pfkeyv2.c
> ===
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.160
> diff -u -p -r1.160 pfkeyv2.c
> --- net/pfkeyv2.c 29 May 2017 20:31:12 -  1.160
> +++ net/pfkeyv2.c 30 May 2017 08:44:05 -
> @@ -212,71 +213,62 @@ pfkey_init(void)
>  int
>  pfkeyv2_attach(struct socket *so, int proto)
>  {
> - struct pfkeyv2_socket *pfkeyv2_socket;
> + struct rawcb *rp;
> + struct keycb *pk;
>   int error;
>  
>   if ((so->so_state & SS_PRIV) == 0)
>   return EACCES;
>  
> - if (!(so->so_pcb = malloc(sizeof(struct rawcb),
> - M_PCB, M_DONTWAIT | M_ZERO)))
> - return (ENOMEM);
> -
> - error = raw_attach(so, so->so_proto->pr_protocol);
> - if (error)
> - goto ret;
> -
> - ((struct rawcb *)so->so_pcb)->rcb_faddr = _addr;
> -
> - if (!(pfkeyv2_socket = malloc(sizeof(struct pfkeyv2_socket),
> - M_PFKEY, M_NOWAIT | M_ZERO)))
> - return (ENOMEM);
> + pk = malloc(sizeof(struct keycb), M_PCB, M_WAITOK | M_ZERO);
> + rp = >rcb;
> + so->so_pcb = rp;
> +
> + error = raw_attach(so, proto);
> + if (error) {
> + free(pk, M_PCB, sizeof(struct keycb));
> + return (error);
> + }
>  
> - LIST_INSERT_HEAD(_sockets, pfkeyv2_socket, kcb_list);
> - pfkeyv2_socket->socket = so;
> - pfkeyv2_socket->pid = curproc->p_p->ps_pid;
> + rp->rcb_faddr = _addr;
> + pk->pid = curproc->p_p->ps_pid;
>  
>   /*
>* XXX we should get this from the socket instead but
>* XXX rawcb doesn't store the rdomain like inpcb does.
>*/
> - pfkeyv2_socket->rdomain = rtable_l2(curproc->p_p->ps_rtableid);
> + pk->rdomain = rtable_l2(curproc->p_p->ps_rtableid);
> +
> + LIST_INSERT_HEAD(_sockets, pk, kcb_list);
>  
>   so->so_options |= SO_USELOOPBACK;
>   soisconnected(so);

Same comment as for PF_ROUTE, I'd do the insertion here.

>   return (0);
> -ret:
> - free(so->so_pcb, M_PCB, sizeof(struct rawcb));
> - return (error);
>  }
>  
>  /*
>   * Close a PF_KEYv2 socket.
>   */
>  int
> -pfkeyv2_detach(struct socket *socket, struct proc *p)
> +pfkeyv2_detach(struct socket *so, struct proc *p)
>  {
> - struct pfkeyv2_socket *pp;
> + struct keycb *pp;

I'd rename 'pp' into 'pk' for coherency since you're changing all the
lines ;)

>   int error;
>  
> - LIST_FOREACH(pp, _sockets, kcb_list)
> - if (pp->socket == socket)
> - break;
> -
> - if (pp) {
> - LIST_REMOVE(pp, kcb_list);
> + pp = sotokeycb(so);
> + if (pp == NULL)
> + return ENOTCONN;
>  
> - if (pp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)
> - nregistered--;
> + LIST_REMOVE(pp, kcb_list);
>  
> - if (pp->flags & PFKEYV2_SOCKETFLAGS_PROMISC)
> - npromisc--;
> + if (pp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)
> + nregistered--;
>  
> - free(pp, M_PFKEY, 0);
> - }
> + if (pp->flags & PFKEYV2_SOCKETFLAGS_PROMISC)
> + npromisc--;
>  
> - error = raw_usrreq(socket, PRU_DETACH, NULL, NULL, NULL, p);
> + error = raw_usrreq(so, PRU_DETACH, NULL, NULL, NULL, p);

Why not call raw_detach() directly and return 0?

>   return (error);
>  }

The rest is ok :)

Re: ksh(1): vi mode UTF-8 bug

[Ingo Schwarze]

Hi Walter,

Walter Alejandro Iglesias wrote on Sun, Jun 04, 2017 at 11:45:04AM +0200:

> I realized the issue I describe in the message below

I don't understand at all what you are trying to talk about.

For a proper bug report, please send the exact byte sequence that
you are passing in to the shell on standard input (preferably
expressed as a printf(1) statement using ASCII-only characters, or
alternatively just the byte sequence encoded with "hexdump -C",
such that it doesn't get mangled in mail transit), the exact byte
sequence that the shell is passing on to the terminal for display
(again, preferably with "hexdump -C"), and state exactly which byte
sequence you would expect the shell to send to the terminal instead.

If you don't know how to obtain this information, wait a bit until
anton@ has committed his new /usr/src/regress/bin/ksh/edit/ suite.
That will make obtaining such information easier.

Yours,
  Ingo

Re: ksh(1): don't output invalid UTF-8 characters

[Ingo Schwarze]

Hi Anton,

Anton Lindqvist wrote on Sun, Jun 04, 2017 at 11:09:35AM +0200:

> Although this discussion hasn't settled,

True.  I think nicm@ has convinced me that the shell *can* try to be
nicer towards terminals, without risking hangs if done very carefully.
Probably that's worth doing, it makes the system more robust.

The goal would be to only send invalid sequences to the terminal if
the user actually typed invalid input, but to not send invalid
intermediate states to the terminal if what the user types is
correct.

> here's a new diff trying to
> address the previously raised issues:
> 
> - The new function x_e_getu8() tries to read a complete UTF-8 character.
>   When a continuation byte is expected but not received, it resets its
>   state and retries.
> 
>   The fix to u8len() from Boudewijn Dijkstra is incorporated in this
>new function, thanks!

No, no, no.  You are thanking him for really bad advice.
That function is still outrageously wrong.

> - In x_emacs(), In order to distinguish between when the line buffer
>   contains more than one character

I assume you mean "byte" here.

>   due to a UTF-8 character being read
>   or caused by calling x_e_getu8() repeatedly the number of calls to
>   x_e_getu8() is stored ntries. This is of importance to preserve the
>   existing behavior where none matched escape sequences should not be
>   inserted but instead ring the bell.

I didn't do full testing and review of your changes yet, but nothing
springs to my eye as being obviously wrong, except the UTF-8 parsing.

> Index: emacs.c
> ===
> RCS file: /cvs/src/bin/ksh/emacs.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 emacs.c
> --- emacs.c   12 May 2017 14:37:52 -  1.67
> +++ emacs.c   4 Jun 2017 09:06:54 -
[...]
> @@ -1891,6 +1894,49 @@ x_e_getc(void)
>   c = x_getc();
>  
>   return c;
> +}
> +
> +static int
> +x_e_getu8(char *buf, int off)
> +{
> + int c, len;
> +
> +again:
> + c = x_e_getc();
> + if (c == -1)
> + return -1;
> + buf[off++] = c;
> +
> + switch (c & 0xF0) {

That is very WRONG.  You cannot identify a UTF-8 start byte by only
looking at its first half.  At the very least, you must also make
sure that bit 5 is not set.

> + case 0xF0:
> + len = 4;
> + break;

That is very WRONG.  The bytes 0xf8 to 0xff must *not* result in
len = 4.  They are not UTF-8 start bytes, and not UTF-8 at all, so
they must result in len = 1.

Also, we do know that the last three bits must be 100.  If bit 6
was zero, the codepoint would have to be encoded in a three-byte
sequence instead.  If bit 7 or 8 would be set in addition to bit
6, that would take us beyond U+10 (specifically, U+1C to
U+1F, which are invalid).  So why not check bits 6 to 8 right
away, too?

> + case 0xE0:
> + len = 3;
> + break;

Hmmm.
Let me think whether we can already catch surrogates at this point.

Valid low three-byte:   U+0800  1110 1010 1000  0xe0a080
U+D000  11101101 1000 1000  0xed8080
U+D7FF  11101101 1001 1011  0xed9fbf
Invalid surrogates: U+D800  11101101 1010 1000  0xeda080
U+DFFF  11101101 1011 1011  0xedbfbf
Valid high three-byte:  U+E000  11101110 1000 1000  0xee8080
U+  1110 1011 1011  0xefbfbf

So the start byte 0xed includes both the valid range U+D000 to U+D7FF
and the invalid surrogate range U+D800 to U+DFFF.  The decisive bit
is the third bit of the *second* byte, so surrogates cannot be
excluded when only the first bit is known.

> + case 0xC0:
> + case 0xD0:
> + len = 2;
> + break;

That is also (somewhat) wrong.  Arguably, it would be better
for 0xc0 and 0xc1 to result in len = 1, because we already know
that there is no valid continuation.

So, here is a better start byte parser (only partially tested):

if (c == 0xf4)
len = 4;
else if ((c & 0xf0) == 0xe0)
len = 3;
else if ((c & 0xe0) == 0xc0 && c > 0xc1)
len = 2;
else
len = 1;

> + default:
> + len = 1;
> + }
> +
> + for (; len > 1; len--) {
> + c = x_e_getc();

Better use a different variable, say cc, such that we retain the
start byte for subsequent validation.

> + if (c == -1)
> + return -1;
> + if (!isu8cont(c)) {
> + off = 0;
> + x_e_ungetc(c);
> + x_error(0);
> + goto again;

That looks wrong.  At "again:", the first thing you do is x_e_getc(),
undoing the effect of the (reasonable) x_e_ungetc() you just did.
That looks like an endless loop to me.

Also, off = 0 and x_error() seem wrong because that 

Re: remove vlan specific ifconfig settings

[Stuart Henderson]

On 2017/06/05 09:49, Reyk Floeter wrote:
> 
> > Am 05.06.2017 um 09:35 schrieb Reyk Floeter :
> > 
> > 
> >> Am 05.06.2017 um 09:26 schrieb David Gwynne :
> >> 
> >> 
> >>> On 5 Jun 2017, at 17:05, Reyk Floeter  wrote:
> >>> 
> >>> Well, not just muscle memory but the fact that some people including me 
> >>> had hostname.vlanX files without an explicit "vlan X" in it.
> >> 
> >> hrm. yes.
> >> 
> >> that means if you change the vnetid on such an interface at runtime, and 
> >> then re-run netstart later on to try and reset the stack, it wont 
> >> configure it back to the state it would be on boot. im not sure we support 
> >> running netstart after boot though, so maybe it doesnt matter.
> >> 
> > 
> > We do and I run it all the time, for example
> > 
> > # sh /etc/netstart vlan5
> > 
> > But I'd usually destroy a cloner before creating it again.

Yes me too.

> Let me rephrase:
> 
> I'm fine with the change as long as we never error out on vnetid/parent 
> transitions. The kernel should just cope with it, eg. running the following 
> commands in sequence should be fine and not throw EBUSY or similar:
> 
> # ifconfig vlan5 create
> # ifconfig vlan5 up
> # ifconfig vlan5 vlandev em0
> # ifconfig vlan5 vlan 5
> # ifconfig vlan5 vlan 6

That is currently supported.

> # ifconfig vlan5 vlandev em1

Support for changing vlandev was stopped last spring with the mpsafe
vlan commit (it was a bit hairy in the first place, and removing it made
the code much simpler).

routing sockets vs KERNEL_LOCK()

[Martin Pieuchot]

Routing sockets are not protected by the NET_LOCK().  That's one of the
boundaries of the network stack.  That's whhy claudio@ sent some diffs
to no longer require the KERNEL_LOCK() to protect them.

But right now some rtm_* functions can be executed without
KERNEL_LOCK().  That's wrong.  Diff below fixes that and move the
KERNEL_LOCK() further down in rtrequest(9) since the NET_LOCK() is
enough to protect the other data structures.

The scariest bits come from default router advertisements, but I guess
that slaacd(8) saved us ;)

ok?

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.357
diff -u -p -r1.357 route.c
--- net/route.c 27 May 2017 09:51:18 -  1.357
+++ net/route.c 5 Jun 2017 10:02:11 -
@@ -253,7 +253,6 @@ rt_match(struct sockaddr *dst, uint32_t 
memset(, 0, sizeof(info));
info.rti_info[RTAX_DST] = dst;
 
-   KERNEL_LOCK();
/*
 * The priority of cloned route should be different
 * to avoid conflict with /32 cloning routes.
@@ -264,14 +263,17 @@ rt_match(struct sockaddr *dst, uint32_t 
error = rtrequest(RTM_RESOLVE, ,
rt->rt_priority - 1, , tableid);
if (error) {
+   KERNEL_LOCK();
rtm_miss(RTM_MISS, , 0, RTP_NONE, 0,
error, tableid);
+   KERNEL_UNLOCK();
} else {
/* Inform listeners of the new route */
+   KERNEL_LOCK();
rtm_send(rt, RTM_ADD, tableid);
+   KERNEL_UNLOCK();
rtfree(rt0);
}
-   KERNEL_UNLOCK();
}
rt->rt_use++;
} else
@@ -624,7 +626,9 @@ out:
info.rti_info[RTAX_DST] = dst;
info.rti_info[RTAX_GATEWAY] = gateway;
info.rti_info[RTAX_AUTHOR] = src;
+   KERNEL_LOCK();
rtm_miss(RTM_REDIRECT, , flags, prio, ifidx, error, rdomain);
+   KERNEL_UNLOCK();
 }
 
 /*
@@ -653,8 +657,10 @@ rtdeletemsg(struct rtentry *rt, struct i
info.rti_flags = rt->rt_flags;
ifidx = rt->rt_ifidx;
error = rtrequest_delete(, rt->rt_priority, ifp, , tableid);
+   KERNEL_LOCK();
rtm_miss(RTM_DELETE, , info.rti_flags, rt->rt_priority, ifidx,
error, tableid);
+   KERNEL_UNLOCK();
if (error == 0)
rtfree(rt);
return (error);
Index: netinet/in_pcb.c
===
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.220
diff -u -p -r1.220 in_pcb.c
--- netinet/in_pcb.c7 Mar 2017 16:59:40 -   1.220
+++ netinet/in_pcb.c5 Jun 2017 10:02:35 -
@@ -716,8 +716,11 @@ in_losing(struct inpcb *inp)
info.rti_info[RTAX_DST] = >inp_route.ro_dst;
info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt, _mask);
+
+   KERNEL_LOCK();
rtm_miss(RTM_LOSING, , rt->rt_flags, rt->rt_priority,
rt->rt_ifidx, 0, inp->inp_rtableid);
+   KERNEL_UNLOCK();
if (rt->rt_flags & RTF_DYNAMIC)
(void)rtrequest(RTM_DELETE, , rt->rt_priority,
NULL, inp->inp_rtableid);
Index: netinet6/nd6_rtr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.159
diff -u -p -r1.159 nd6_rtr.c
--- netinet6/nd6_rtr.c  30 May 2017 08:58:34 -  1.159
+++ netinet6/nd6_rtr.c  5 Jun 2017 10:03:16 -
@@ -613,7 +613,9 @@ defrouter_addreq(struct nd_defrouter *ne
error = rtrequest(RTM_ADD, , RTP_DEFAULT, ,
new->ifp->if_rdomain);
if (error == 0) {
+   KERNEL_LOCK();
rtm_send(rt, RTM_ADD, new->ifp->if_rdomain);
+   KERNEL_UNLOCK();
rtfree(rt);
new->installed = 1;
}
@@ -717,7 +719,9 @@ defrouter_delreq(struct nd_defrouter *dr
error = rtrequest(RTM_DELETE, , RTP_DEFAULT, ,
dr->ifp->if_rdomain);
if (error == 0) {
+   KERNEL_LOCK();
rtm_send(rt, RTM_DELETE, dr->ifp->if_rdomain);
+   KERNEL_UNLOCK();
rtfree(rt);
}
 

Re: let's add PF_LOCK()

[Martin Pieuchot]

On 01/06/17(Thu) 09:32, Alexandr Nedvedicky wrote:
> [...]
> > 
> > Where do you want to define this WITH_PF_LOCK?
> > 
> 
> I currently add 
> 
>   option WITH_PF_LOCK
> 
> to sys/arch/amd64/compile/GENERIC.MP to build PF with PF_LOCK.
> But there should be better place in tree. Perhaps sys/conf/GENERIC?

I don't think it matters.  We still need more diffs to be able to test
that.

Now I'd really like if you could commit this so we continue improving it
in-tree.

ok mpi@

> diff -r ccb9f01e56a7 .hgtags
> --- /dev/null Thu Jan 01 00:00:00 1970 +
> +++ .hgtags   Thu Jun 01 09:29:15 2017 +0200
> @@ -0,0 +1,1 @@
> +d545881e2652dbc0c057691a39a095bce92f441f pf-lock.baseline
> diff -r ccb9f01e56a7 src/sys/conf/GENERIC
> --- src/sys/conf/GENERIC  Thu Jun 01 09:19:56 2017 +0200
> +++ src/sys/conf/GENERIC  Thu Jun 01 09:29:15 2017 +0200
> @@ -16,7 +16,7 @@ option  ACCOUNTING  # acct(2) process acc
>  option   KMEMSTATS   # collect malloc(9) statistics
>  option   PTRACE  # ptrace(2) system call
>  #option  WITNESS # witness(4) lock checker
> -#option  PF_LOCK # build with pf lock support
> +#option  WITH_PF_LOCK# PF lock support
>  
>  #option  KVA_GUARDPAGES  # slow virtual address recycling (+ 
> guarding)
>  option   POOL_DEBUG  # pool corruption detection
> diff -r ccb9f01e56a7 src/sys/net/pf.c
> --- src/sys/net/pf.c  Thu Jun 01 09:19:56 2017 +0200
> +++ src/sys/net/pf.c  Thu Jun 01 09:29:15 2017 +0200
> @@ -923,7 +923,7 @@ int
>  pf_state_insert(struct pfi_kif *kif, struct pf_state_key **skw,
>  struct pf_state_key **sks, struct pf_state *s)
>  {
> - NET_ASSERT_LOCKED();
> + PF_ASSERT_LOCKED();
>  
>   s->kif = kif;
>   if (*skw == *sks) {
> @@ -1186,7 +1186,7 @@ pf_purge_expired_rules(void)
>  {
>   struct pf_rule  *r;
>  
> - NET_ASSERT_LOCKED();
> + PF_ASSERT_LOCKED();
>  
>   if (SLIST_EMPTY(_rule_gcl))
>   return;
> @@ -1208,15 +1208,22 @@ pf_purge_thread(void *v)
>  
>   NET_LOCK(s);
>  
> + PF_LOCK();
>   /* process a fraction of the state table every second */
>   pf_purge_expired_states(1 + (pf_status.states
>   / pf_default_rule.timeout[PFTM_INTERVAL]));
>  
>   /* purge other expired types every PFTM_INTERVAL seconds */
>   if (++nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
> - pf_purge_expired_fragments();
>   pf_purge_expired_src_nodes(0);
>   pf_purge_expired_rules();
> + }
> + PF_UNLOCK();
> + /*
> +  * Fragments don't require PF_LOCK(), they use their own lock.
> +  */
> + if (nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
> + pf_purge_expired_fragments();
>   nloops = 0;
>   }
>  
> @@ -1267,7 +1274,7 @@ pf_purge_expired_src_nodes(void)
>  {
>   struct pf_src_node  *cur, *next;
>  
> - NET_ASSERT_LOCKED();
> + PF_ASSERT_LOCKED();
>  
>   for (cur = RB_MIN(pf_src_tree, _src_tracking); cur; cur = next) {
>   next = RB_NEXT(pf_src_tree, _src_tracking, cur);
> @@ -1303,7 +1310,7 @@ pf_src_tree_remove_state(struct pf_state
>  void
>  pf_remove_state(struct pf_state *cur)
>  {
> - NET_ASSERT_LOCKED();
> + PF_ASSERT_LOCKED();
>  
>   /* handle load balancing related tasks */
>   pf_postprocess_addr(cur);
> @@ -1350,7 +1357,7 @@ pf_free_state(struct pf_state *cur)
>  {
>   struct pf_rule_item *ri;
>  
> - NET_ASSERT_LOCKED();
> + PF_ASSERT_LOCKED();
>  
>  #if NPFSYNC > 0
>   if (pfsync_state_in_use(cur))
> @@ -1386,7 +1393,7 @@ pf_purge_expired_states(u_int32_t maxche
>   static struct pf_state  *cur = NULL;
>   struct pf_state *next;
>  
> - NET_ASSERT_LOCKED();
> + PF_ASSERT_LOCKED();
>  
>   while (maxcheck--) {
>   /* wrap to start of list when we hit the end */
> @@ -3146,13 +3153,13 @@ pf_socket_lookup(struct pf_pdesc *pd)
>   case IPPROTO_TCP:
>   sport = pd->hdr.tcp.th_sport;
>   dport = pd->hdr.tcp.th_dport;
> - NET_ASSERT_LOCKED();
> + PF_ASSERT_LOCKED();
>   tb = 
>   break;
>   case IPPROTO_UDP:
>   sport = pd->hdr.udp.uh_sport;
>   dport = pd->hdr.udp.uh_dport;
> - NET_ASSERT_LOCKED();
> + PF_ASSERT_LOCKED();
>   tb = 
>   break;
>   default:
> @@ -6678,6 +6685,7 @@ pf_test(sa_family_t af, int fwdir, struc
>   /* if packet sits in reassembly queue, return without error */
>   if (pd.m == NULL)
>   return PF_PASS;
> +
>   if (action != PF_PASS) {
>  #if NPFLOG > 0
>   pd.pflog |= PF_LOG_FORCE;
> @@ -6697,6 

Re: remove vlan specific ifconfig settings

[Jason McIntyre]

On Mon, Jun 05, 2017 at 05:26:58PM +1000, David Gwynne wrote:
> 
> > On 5 Jun 2017, at 17:05, Reyk Floeter  wrote:
> > 
> > Well, not just muscle memory but the fact that some people including me had 
> > hostname.vlanX files without an explicit "vlan X" in it.
> 
> hrm. yes.
> 
> that means if you change the vnetid on such an interface at runtime, and then 
> re-run netstart later on to try and reset the stack, it wont configure it 
> back to the state it would be on boot. im not sure we support running 
> netstart after boot though, so maybe it doesnt matter.
> 

according to netstart(8):

The netstart script can also be used to start newly created
bridges or interfaces, or reset existing interfaces to their
default state.

jmc

Re: move pbc list from rawcb to routecb (and later pfkey cb)

[Martin Pieuchot]

On 30/05/17(Tue) 13:59, Claudio Jeker wrote:
> This is a step I need to do to make progress on the PF_KEY cleanup I'm
> doing. Both PF_ROUTE and PF_KEY need to start to take care of their own
> PCB list and so move the LIST_ENTRY out of rawcb into routecb.
> This allows me to do the same in PF_KEY which will be sent as the next
> diff.
> 
> -- 
> :wq Claudio
> 
> Index: net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.237
> diff -u -p -r1.237 rtsock.c
> --- net/rtsock.c  19 Apr 2017 15:21:54 -  1.237
> +++ net/rtsock.c  30 May 2017 10:29:25 -
> @@ -154,45 +157,50 @@ struct route_cb route_cb;
>  
>  #define ROUTE_DESYNC_RESEND_TIMEOUT  (hz / 5)/* In hz */
>  
> +void
> +route_prinit(void)
> +{
> + LIST_INIT(_cb.rcb);
> +}
> +
> +
>  int
>  route_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam,
>  struct mbuf *control, struct proc *p)
>  {
> - struct rawcb*rp;
>   struct routecb  *rop;
>   int  af;
>   int  error = 0;
>  
> - rp = sotorawcb(so);
> + rop = sotoroutecb(so);
> + if (rop == NULL)
> + return ENOTCONN;

Previously raw_usrreq() was returning EINVAL in that case.  Does it
matter?

You should also call m_freem(m), because even if PRU_RCVD and PRU_DETACH
do not take any argument, we cannot be sure all other code paths cannot
be reached.  That's one of the reasons I'm suggesting we split the PRU
switches in multiple functions.

> @@ -240,10 +248,11 @@ route_attach(struct socket *so, int prot
>  #endif
>   rp->rcb_faddr = _src;
>   route_cb.any_count++;
> + LIST_INSERT_HEAD(_cb.rcb, rop, rcb_list);
>   soisconnected(so);
>   so->so_options |= SO_USELOOPBACK;

I'd do the insertion last.  It doesn't matter if all operations are
serialized but it doesn't cost anything and who knows what's coming
next ;)

Other than that I like this diff very much.

Re: ksh(1): don't output invalid UTF-8 characters

[Walter Alejandro Iglesias]

Just to applogize to developers here,

I'm still not skilled enough to make a proper patch or a clear bug
report (I'm on chapter 2 of K :-)).  I wish with time I'll learn how
to do it.  I came to the ksh utf8 discussion because I've been playing
with some mail mime encoder just to learn C and recognizing valid utf-8
was the first challenge I ecountered.

The code pasted below is what I got so far in recognizing valid utf-8.
I'm showing it to make my point, I realize it isn't easy; and from my
poor C I'm not able to figure out how you can do such test byte by byte
while the user is typing at command line.  (Don't bother in explaining
me how, I know this is not the place to take C lessons.)

By the way, something the last paragraph of the new utf8(7) man page
isn't clear enough (I mentioned this to tedu@).

Thanks to all of you for your work.  Now I know how hard it is.


#include 

#define ASCII   0x7f
#define YES 1
#define NO  0

int
main()
{
int c, ch, wd, ln, col, isutf8;

ch = wd = ln = col = 1;
isutf8 = YES;

while ((c = getchar()) != EOF) {
if (c > ASCII) {
if ((ch == 1 && (c < 0xc2 || c > 0xf7)) ||
((ch > 1 && c <= 4) &&
ch <= wd && (c < 0x80 || c > 0xbf)))
isutf8 = NO;
/* 110. */
else if (ch == 1 && c >= 0xc2 && c <= 0xdf) {
wd = 2;
++ch;
/* 1110 */
} else if (ch == 1 && c >= 0xe0 && c <= 0xef) {
wd = 3;
++ch;
/* 0... */
} else if (ch == 1 && c >= 0xf0 && c <= 0xf7) {
wd = 4;
++ch;
} else if (ch > 1 && c <= 4 &&
ch == wd && c >= 0x80 && c <= 0xbf)
ch = 1;
else if (ch > 1 && c <= 4 &&
ch < wd && c >= 0x80 && c <= 0xbf)
++ch;
else
++ch;
} else if (ch > 1 && ch <= 4 && ch <= wd)
isutf8= NO;
else
ch = 1;

if (isutf8 == NO) {
printf("Invalid utf-8 character");
printf(" at line %d col %d.\n", ln, col);
return 1;
}
if (c == '\n') {
col = 1;
++ln;
} else
++col;
}

return 0;
}

Re: sshd: Remove authorized_keys2 file

[Klemens Nanni]

Bump: Feeback? OK?

On Mon, Apr 17, 2017 at 09:28:29PM +0200, Klemens Nanni wrote:
Now that protocol version 1 was finally dropped in sshd(8), get rid of
this file completely. Our default sshd_config(5) overwrites
AuthorizedKeysFile to ignore it anyway and sshd(8)'s FILES section
doesn't mention it either.

Index: etc/changelist
===
RCS file: /cvs/src/etc/changelist,v
retrieving revision 1.116
diff -u -p -r1.116 changelist
--- etc/changelist  27 Feb 2017 21:53:11 -  1.116
+++ etc/changelist  17 Apr 2017 19:26:47 -
@@ -147,7 +147,6 @@
/root/.rhosts
/root/.shosts
/root/.ssh/authorized_keys
-/root/.ssh/authorized_keys2
/var/cron/at.allow
/var/cron/at.deny
/var/cron/cron.allow
Index: usr.bin/ssh/pathnames.h
===
RCS file: /cvs/src/usr.bin/ssh/pathnames.h,v
retrieving revision 1.25
diff -u -p -r1.25 pathnames.h
--- usr.bin/ssh/pathnames.h 31 Mar 2016 05:24:06 -  1.25
+++ usr.bin/ssh/pathnames.h 17 Apr 2017 19:26:47 -
@@ -79,7 +79,7 @@
#define _PATH_SSH_USER_CONFFILE _PATH_SSH_USER_DIR "/config"

/*
- * File containing a list of those rsa keys that permit logging in as this
+ * File containing a list of those keys that permit logging in as this
 * user.  This file need not be readable by anyone but the user him/herself,
 * but does not contain anything particularly secret.  If the user's home
 * directory resides on an NFS volume where root is mapped to nobody, this
@@ -87,9 +87,6 @@
 * running as root.)
 */
#define _PATH_SSH_USER_PERMITTED_KEYS   _PATH_SSH_USER_DIR "/authorized_keys"
-
-/* backward compat for protocol v2 */
-#define _PATH_SSH_USER_PERMITTED_KEYS2 _PATH_SSH_USER_DIR "/authorized_keys2"

/*
 * Per-user and system-wide ssh "rc" files.  These files are executed with
Index: usr.bin/ssh/servconf.c
===
RCS file: /cvs/src/usr.bin/ssh/servconf.c,v
retrieving revision 1.306
diff -u -p -r1.306 servconf.c
--- usr.bin/ssh/servconf.c  14 Mar 2017 07:19:07 -  1.306
+++ usr.bin/ssh/servconf.c  17 Apr 2017 19:26:47 -
@@ -294,12 +294,9 @@ fill_default_server_options(ServerOption
options->client_alive_interval = 0;
if (options->client_alive_count_max == -1)
options->client_alive_count_max = 3;
-   if (options->num_authkeys_files == 0) {
+   if (options->num_authkeys_files == 0)
options->authorized_keys_files[options->num_authkeys_files++] =
xstrdup(_PATH_SSH_USER_PERMITTED_KEYS);
-   options->authorized_keys_files[options->num_authkeys_files++] =
-   xstrdup(_PATH_SSH_USER_PERMITTED_KEYS2);
-   }
if (options->permit_tun == -1)
options->permit_tun = SSH_TUNMODE_NO;
if (options->ip_qos_interactive == -1)
Index: usr.bin/ssh/sshd.8
===
RCS file: /cvs/src/usr.bin/ssh/sshd.8,v
retrieving revision 1.288
diff -u -p -r1.288 sshd.8
--- usr.bin/ssh/sshd.8  30 Jan 2017 23:27:39 -  1.288
+++ usr.bin/ssh/sshd.8  17 Apr 2017 19:26:47 -
@@ -390,9 +390,7 @@ does not exist either, xauth is used to
specifies the files containing public keys for
public key authentication;
if this option is not specified, the default is
-.Pa ~/.ssh/authorized_keys
-and
-.Pa ~/.ssh/authorized_keys2 .
+.Pa ~/.ssh/authorized_keys .
Each line of the file contains one
key (empty lines and lines starting with a
.Ql #
Index: usr.bin/ssh/sshd_config
===
RCS file: /cvs/src/usr.bin/ssh/sshd_config,v
retrieving revision 1.101
diff -u -p -r1.101 sshd_config
--- usr.bin/ssh/sshd_config 14 Mar 2017 07:19:07 -  1.101
+++ usr.bin/ssh/sshd_config 17 Apr 2017 19:26:47 -
@@ -35,9 +35,7 @@

#PubkeyAuthentication yes

-# The default is to check both .ssh/authorized_keys and .ssh/authorized_keys2
-# but this is overridden so installations will only check .ssh/authorized_keys
-AuthorizedKeysFile .ssh/authorized_keys
+#AuthorizedKeysFile.ssh/authorized_keys

#AuthorizedPrincipalsFile none

Index: usr.bin/ssh/sshd_config.5
===
RCS file: /cvs/src/usr.bin/ssh/sshd_config.5,v
retrieving revision 1.243
diff -u -p -r1.243 sshd_config.5
--- usr.bin/ssh/sshd_config.5   14 Mar 2017 07:19:07 -  1.243
+++ usr.bin/ssh/sshd_config.5   17 Apr 2017 19:26:47 -
@@ -283,7 +283,7 @@ Alternately this option may be set to
.Cm none
to skip checking for user keys in files.
The default is
-.Qq .ssh/authorized_keys .ssh/authorized_keys2 .
+.Qq .ssh/authorized_keys .
.It Cm AuthorizedPrincipalsCommand
Specifies a program to be used to generate the list of allowed
certificate principals as per

Re: tweak txp to avoid ifq_deq_begin/commit/rollback

[Mike Belopuhov]

On Wed, May 31, 2017 at 20:40 +0200, Mike Belopuhov wrote:
> According to the FreeBSD driver, txp(4) is not setting up its TX
> ring correctly.  FreeBSD driver uses up to 16 fragments, while we
> use up to 252 which is suspicious.
> 
> This gets us in line with FreeBSD, introduces goodness of m_defrag
> and removes pesky if_deq_* thingies.
> 
> Does anyone still have the hardware (3com 3CR900 Typhoon) to test?
> OK's are welcome.
>

Any OKs? Tests? Should I go ahead with this?

> diff --git sys/dev/pci/if_txp.c sys/dev/pci/if_txp.c
> index deede70e9de..1aed06765c0 100644
> --- sys/dev/pci/if_txp.c
> +++ sys/dev/pci/if_txp.c
> @@ -883,12 +883,12 @@ txp_alloc_rings(struct txp_softc *sc)
>   sc->sc_txhir.r_desc = (struct txp_tx_desc 
> *)sc->sc_txhiring_dma.dma_vaddr;
>   sc->sc_txhir.r_cons = sc->sc_txhir.r_prod = sc->sc_txhir.r_cnt = 0;
>   sc->sc_txhir.r_off = >sc_hostvar->hv_tx_hi_desc_read_idx;
>   for (i = 0; i < TX_ENTRIES; i++) {
>   if (bus_dmamap_create(sc->sc_dmat, TXP_MAX_PKTLEN,
> - TX_ENTRIES - 4, TXP_MAX_SEGLEN, 0,
> - BUS_DMA_NOWAIT, >sc_txd[i].sd_map) != 0) {
> + TXP_MAXTXSEGS, MCLBYTES, 0, BUS_DMA_NOWAIT,
> + >sc_txd[i].sd_map) != 0) {
>   for (j = 0; j < i; j++) {
>   bus_dmamap_destroy(sc->sc_dmat,
>   sc->sc_txd[j].sd_map);
>   sc->sc_txd[j].sd_map = NULL;
>   }
> @@ -1261,57 +1261,48 @@ txp_start(struct ifnet *ifp)
>   struct txp_softc *sc = ifp->if_softc;
>   struct txp_tx_ring *r = >sc_txhir;
>   struct txp_tx_desc *txd;
>   int txdidx;
>   struct txp_frag_desc *fxd;
> - struct mbuf *m, *mnew;
> + struct mbuf *m;
>   struct txp_swdesc *sd;
>   u_int32_t firstprod, firstcnt, prod, cnt, i;
>  
>   if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(>if_snd))
>   return;
>  
>   prod = r->r_prod;
>   cnt = r->r_cnt;
>  
>   while (1) {
> - m = ifq_deq_begin(>if_snd);
> + if (cnt >= TX_ENTRIES - TXP_MAXTXSEGS - 4)
> + goto oactive;
> +
> + m = ifq_dequeue(>if_snd);
>   if (m == NULL)
>   break;
> - mnew = NULL;
>  
>   firstprod = prod;
>   firstcnt = cnt;
>  
>   sd = sc->sc_txd + prod;
>   sd->sd_mbuf = m;
>  
> - if (bus_dmamap_load_mbuf(sc->sc_dmat, sd->sd_map, m,
> + switch (bus_dmamap_load_mbuf(sc->sc_dmat, sd->sd_map, m,
>   BUS_DMA_NOWAIT)) {
> - MGETHDR(mnew, M_DONTWAIT, MT_DATA);
> - if (mnew == NULL)
> - goto oactive1;
> - if (m->m_pkthdr.len > MHLEN) {
> - MCLGET(mnew, M_DONTWAIT);
> - if ((mnew->m_flags & M_EXT) == 0) {
> - m_freem(mnew);
> - goto oactive1;
> - }
> - }
> - m_copydata(m, 0, m->m_pkthdr.len, mtod(mnew, caddr_t));
> - mnew->m_pkthdr.len = mnew->m_len = m->m_pkthdr.len;
> - ifq_deq_commit(>if_snd, m);
> + case 0:
> + break;
> + case EFBIG:
> + if (m_defrag(m, M_DONTWAIT) == 0 &&
> + bus_dmamap_load_mbuf(sc->sc_dmat, sd->sd_map, m,
> + BUS_DMA_NOWAIT) == 0)
> + break;
> + default:
>   m_freem(m);
> - m = mnew;
> - if (bus_dmamap_load_mbuf(sc->sc_dmat, sd->sd_map, m,
> - BUS_DMA_NOWAIT))
> - goto oactive1;
> + continue;
>   }
>  
> - if ((TX_ENTRIES - cnt) < 4)
> - goto oactive;
> -
>   txd = r->r_desc + prod;
>   txdidx = prod;
>   txd->tx_flags = TX_FLAGS_TYPE_DATA;
>   txd->tx_numdesc = 0;
>   txd->tx_addrlo = 0;
> @@ -1321,13 +1312,10 @@ txp_start(struct ifnet *ifp)
>   txd->tx_numdesc = sd->sd_map->dm_nsegs;
>  
>   if (++prod == TX_ENTRIES)
>   prod = 0;
>  
> - if (++cnt >= (TX_ENTRIES - 4))
> - goto oactive;
> -
>  #if NVLAN > 0
>   if (m->m_flags & M_VLANTAG) {
>   txd->tx_pflags = TX_PFLAGS_VLAN |
>   (htons(m->m_pkthdr.ether_vtag) << 
> TX_PFLAGS_VLANTAG_S);
>   }
> @@ -1351,10 +1339,12 @@ txp_start(struct ifnet *ifp)
>   for (i = 0; i < sd->sd_map->dm_nsegs; i++) {
>   if (++cnt >= (TX_ENTRIES - 4)) {
> 

Re: i386 clang: fix binutils build

[Christian Weisgerber]

Todd C. Miller:

> I think you want 0xU, not 0xL.  Otherwise you will
> have the same issue on i386.

???

We need a constant that comes out as the "long" value
0x      on 64-bit platforms
0x  on 32-bit platforms (degenerate case)

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: routing sockets vs KERNEL_LOCK()

[Martin Pieuchot]

On 05/06/17(Mon) 12:12, Martin Pieuchot wrote:
> Routing sockets are not protected by the NET_LOCK().  That's one of the
> boundaries of the network stack.  That's whhy claudio@ sent some diffs
> to no longer require the KERNEL_LOCK() to protect them.
> 
> But right now some rtm_* functions can be executed without
> KERNEL_LOCK().  That's wrong.  Diff below fixes that and move the
> KERNEL_LOCK() further down in rtrequest(9) since the NET_LOCK() is
> enough to protect the other data structures.
> 
> The scariest bits come from default router advertisements, but I guess
> that slaacd(8) saved us ;)

Also change a KERNEL_ASSERT_LOCKED() into a NET_ASSERT_LOCK() in
rt_{set,put}gwroute().  Theses can now be called without KERNEL_LOCK()
when inserting an ARP entry.

Updated diff below.

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.357
diff -u -p -r1.357 route.c
--- net/route.c 27 May 2017 09:51:18 -  1.357
+++ net/route.c 5 Jun 2017 14:48:49 -
@@ -253,7 +253,6 @@ rt_match(struct sockaddr *dst, uint32_t 
memset(, 0, sizeof(info));
info.rti_info[RTAX_DST] = dst;
 
-   KERNEL_LOCK();
/*
 * The priority of cloned route should be different
 * to avoid conflict with /32 cloning routes.
@@ -264,14 +263,17 @@ rt_match(struct sockaddr *dst, uint32_t 
error = rtrequest(RTM_RESOLVE, ,
rt->rt_priority - 1, , tableid);
if (error) {
+   KERNEL_LOCK();
rtm_miss(RTM_MISS, , 0, RTP_NONE, 0,
error, tableid);
+   KERNEL_UNLOCK();
} else {
/* Inform listeners of the new route */
+   KERNEL_LOCK();
rtm_send(rt, RTM_ADD, tableid);
+   KERNEL_UNLOCK();
rtfree(rt0);
}
-   KERNEL_UNLOCK();
}
rt->rt_use++;
} else
@@ -385,7 +387,7 @@ rt_setgwroute(struct rtentry *rt, u_int 
 {
struct rtentry *nhrt;
 
-   KERNEL_ASSERT_LOCKED();
+   NET_ASSERT_LOCKED();
 
KASSERT(ISSET(rt->rt_flags, RTF_GATEWAY));
 
@@ -442,7 +444,7 @@ rt_putgwroute(struct rtentry *rt)
 {
struct rtentry *nhrt = rt->rt_gwroute;
 
-   KERNEL_ASSERT_LOCKED();
+   NET_ASSERT_LOCKED();
 
if (!ISSET(rt->rt_flags, RTF_GATEWAY) || nhrt == NULL)
return;
@@ -624,7 +626,9 @@ out:
info.rti_info[RTAX_DST] = dst;
info.rti_info[RTAX_GATEWAY] = gateway;
info.rti_info[RTAX_AUTHOR] = src;
+   KERNEL_LOCK();
rtm_miss(RTM_REDIRECT, , flags, prio, ifidx, error, rdomain);
+   KERNEL_UNLOCK();
 }
 
 /*
@@ -653,8 +657,10 @@ rtdeletemsg(struct rtentry *rt, struct i
info.rti_flags = rt->rt_flags;
ifidx = rt->rt_ifidx;
error = rtrequest_delete(, rt->rt_priority, ifp, , tableid);
+   KERNEL_LOCK();
rtm_miss(RTM_DELETE, , info.rti_flags, rt->rt_priority, ifidx,
error, tableid);
+   KERNEL_UNLOCK();
if (error == 0)
rtfree(rt);
return (error);
Index: netinet/in_pcb.c
===
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.220
diff -u -p -r1.220 in_pcb.c
--- netinet/in_pcb.c7 Mar 2017 16:59:40 -   1.220
+++ netinet/in_pcb.c5 Jun 2017 10:02:35 -
@@ -716,8 +716,11 @@ in_losing(struct inpcb *inp)
info.rti_info[RTAX_DST] = >inp_route.ro_dst;
info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt, _mask);
+
+   KERNEL_LOCK();
rtm_miss(RTM_LOSING, , rt->rt_flags, rt->rt_priority,
rt->rt_ifidx, 0, inp->inp_rtableid);
+   KERNEL_UNLOCK();
if (rt->rt_flags & RTF_DYNAMIC)
(void)rtrequest(RTM_DELETE, , rt->rt_priority,
NULL, inp->inp_rtableid);
Index: netinet6/nd6_rtr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.159
diff -u -p -r1.159 nd6_rtr.c
--- netinet6/nd6_rtr.c  30 May 2017 08:58:34 -  1.159
+++ netinet6/nd6_rtr.c  5 Jun 2017 10:03:16 -
@@ -613,7 +613,9 @@ defrouter_addreq(struct nd_defrouter *ne
error = rtrequest(RTM_ADD, , RTP_DEFAULT, ,
new->ifp->if_rdomain);
if (error == 0) {
+   KERNEL_LOCK();
rtm_send(rt, RTM_ADD, new->ifp->if_rdomain);
+   

Re: i386 clang: fix binutils build

[Todd C. Miller]

On Mon, 05 Jun 2017 16:32:01 +0200, Christian Weisgerber wrote:

> Todd C. Miller:
> 
> > I think you want 0xU, not 0xL.  Otherwise you will
> > have the same issue on i386.
> 
> ???
> 
> We need a constant that comes out as the "long" value
> 0x      on 64-bit platforms
> 0x  on 32-bit platforms (degenerate case)

If you assign 0x to a long on i386 the compiler should warn
about it since it is too big to fit unless the value is unsigned.
That is why ULONG_MAX is defined as defined as 0xUL on
32-bit platforms.

 - todd

Re: move pbc list from rawcb to routecb (and later pfkey cb)

[Martin Pieuchot]

On 05/06/17(Mon) 17:32, Claudio Jeker wrote:
> On Mon, Jun 05, 2017 at 11:32:06AM +0200, Martin Pieuchot wrote:
> > On 30/05/17(Tue) 13:59, Claudio Jeker wrote:
> > > - struct rawcb*rp;
> > >   struct routecb  *rop;
> > >   int  af;
> > >   int  error = 0;
> > >  
> > > - rp = sotorawcb(so);
> > > + rop = sotoroutecb(so);
> > > + if (rop == NULL)
> > > + return ENOTCONN;
> > 
> > Previously raw_usrreq() was returning EINVAL in that case.  Does it
> > matter?
> > 
> > You should also call m_freem(m), because even if PRU_RCVD and PRU_DETACH
> > do not take any argument, we cannot be sure all other code paths cannot
> > be reached.  That's one of the reasons I'm suggesting we split the PRU
> > switches in multiple functions.
> > 
> 
> What about we make this a KASSERT()? I think it is impossible to get there
> with a NULL pointer for the pcb.

Fine with me.

Re: move pbc list from rawcb to routecb (and later pfkey cb)

[Claudio Jeker]

On Mon, Jun 05, 2017 at 11:32:06AM +0200, Martin Pieuchot wrote:
> On 30/05/17(Tue) 13:59, Claudio Jeker wrote:
> > This is a step I need to do to make progress on the PF_KEY cleanup I'm
> > doing. Both PF_ROUTE and PF_KEY need to start to take care of their own
> > PCB list and so move the LIST_ENTRY out of rawcb into routecb.
> > This allows me to do the same in PF_KEY which will be sent as the next
> > diff.
> > 
> > -- 
> > :wq Claudio
> > 
> > Index: net/rtsock.c
> > ===
> > RCS file: /cvs/src/sys/net/rtsock.c,v
> > retrieving revision 1.237
> > diff -u -p -r1.237 rtsock.c
> > --- net/rtsock.c19 Apr 2017 15:21:54 -  1.237
> > +++ net/rtsock.c30 May 2017 10:29:25 -
> > @@ -154,45 +157,50 @@ struct route_cb route_cb;
> >  
> >  #define ROUTE_DESYNC_RESEND_TIMEOUT(hz / 5)/* In hz */
> >  
> > +void
> > +route_prinit(void)
> > +{
> > +   LIST_INIT(_cb.rcb);
> > +}
> > +
> > +
> >  int
> >  route_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam,
> >  struct mbuf *control, struct proc *p)
> >  {
> > -   struct rawcb*rp;
> > struct routecb  *rop;
> > int  af;
> > int  error = 0;
> >  
> > -   rp = sotorawcb(so);
> > +   rop = sotoroutecb(so);
> > +   if (rop == NULL)
> > +   return ENOTCONN;
> 
> Previously raw_usrreq() was returning EINVAL in that case.  Does it
> matter?
> 
> You should also call m_freem(m), because even if PRU_RCVD and PRU_DETACH
> do not take any argument, we cannot be sure all other code paths cannot
> be reached.  That's one of the reasons I'm suggesting we split the PRU
> switches in multiple functions.
> 

What about we make this a KASSERT()? I think it is impossible to get there
with a NULL pointer for the pcb.

> > @@ -240,10 +248,11 @@ route_attach(struct socket *so, int prot
> >  #endif
> > rp->rcb_faddr = _src;
> > route_cb.any_count++;
> > +   LIST_INSERT_HEAD(_cb.rcb, rop, rcb_list);
> > soisconnected(so);
> > so->so_options |= SO_USELOOPBACK;
> 
> I'd do the insertion last.  It doesn't matter if all operations are
> serialized but it doesn't cost anything and who knows what's coming
> next ;)

Actually that is the reason why I did it there. Doing the socket setup
last to ensure everything is settled before making the socket writeable.
At least the soisconnected() call will move the state of the socket ahead
and therefore other callers may start talking to it. On the other hand
route_input() does a somewhat bad job at figuring out which PCBs are
available.
 
> Other than that I like this diff very much.
> 

-- 
:wq Claudio

Re: Merge pfkeyv2_socket and rawcb into one struct

[Claudio Jeker]

On Mon, Jun 05, 2017 at 11:39:14AM +0200, Martin Pieuchot wrote:
> On 30/05/17(Tue) 14:05, Claudio Jeker wrote:
> > This is more or less the same thing for PF_KEY that we now do in PF_ROUTE.
> > Use one PCB LIST on the keycb and embedd the rawcb in that PF_KEY cb.
> > Diff also has a few variable renames in it to make this code less alien
> > regarding the rest of our kernel. Mainly use so instead of socket and
> > pfkeyv2_socket is also replaced with better variable names.
> > 
> > This needs the previous diff I just sent out for PF_ROUTE.
> > After that I can make pfkey use the same SRPL_LIST as PF_ROUTE (from an
> > other diff) to unlock them more.
> > -- 
> > :wq Claudio
> > 
> > Index: net/pfkeyv2.c
> > ===
> > RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> > retrieving revision 1.160
> > diff -u -p -r1.160 pfkeyv2.c
> > --- net/pfkeyv2.c   29 May 2017 20:31:12 -  1.160
> > +++ net/pfkeyv2.c   30 May 2017 08:44:05 -
> > @@ -212,71 +213,62 @@ pfkey_init(void)
> >  int
> >  pfkeyv2_attach(struct socket *so, int proto)
> >  {
> > -   struct pfkeyv2_socket *pfkeyv2_socket;
> > +   struct rawcb *rp;
> > +   struct keycb *pk;
> > int error;
> >  
> > if ((so->so_state & SS_PRIV) == 0)
> > return EACCES;
> >  
> > -   if (!(so->so_pcb = malloc(sizeof(struct rawcb),
> > -   M_PCB, M_DONTWAIT | M_ZERO)))
> > -   return (ENOMEM);
> > -
> > -   error = raw_attach(so, so->so_proto->pr_protocol);
> > -   if (error)
> > -   goto ret;
> > -
> > -   ((struct rawcb *)so->so_pcb)->rcb_faddr = _addr;
> > -
> > -   if (!(pfkeyv2_socket = malloc(sizeof(struct pfkeyv2_socket),
> > -   M_PFKEY, M_NOWAIT | M_ZERO)))
> > -   return (ENOMEM);
> > +   pk = malloc(sizeof(struct keycb), M_PCB, M_WAITOK | M_ZERO);
> > +   rp = >rcb;
> > +   so->so_pcb = rp;
> > +
> > +   error = raw_attach(so, proto);
> > +   if (error) {
> > +   free(pk, M_PCB, sizeof(struct keycb));
> > +   return (error);
> > +   }
> >  
> > -   LIST_INSERT_HEAD(_sockets, pfkeyv2_socket, kcb_list);
> > -   pfkeyv2_socket->socket = so;
> > -   pfkeyv2_socket->pid = curproc->p_p->ps_pid;
> > +   rp->rcb_faddr = _addr;
> > +   pk->pid = curproc->p_p->ps_pid;
> >  
> > /*
> >  * XXX we should get this from the socket instead but
> >  * XXX rawcb doesn't store the rdomain like inpcb does.
> >  */
> > -   pfkeyv2_socket->rdomain = rtable_l2(curproc->p_p->ps_rtableid);
> > +   pk->rdomain = rtable_l2(curproc->p_p->ps_rtableid);
> > +
> > +   LIST_INSERT_HEAD(_sockets, pk, kcb_list);
> >  
> > so->so_options |= SO_USELOOPBACK;
> > soisconnected(so);
> 
> Same comment as for PF_ROUTE, I'd do the insertion here.
> 

See other mail. Will think about this a bit more.

> > return (0);
> > -ret:
> > -   free(so->so_pcb, M_PCB, sizeof(struct rawcb));
> > -   return (error);
> >  }
> >  
> >  /*
> >   * Close a PF_KEYv2 socket.
> >   */
> >  int
> > -pfkeyv2_detach(struct socket *socket, struct proc *p)
> > +pfkeyv2_detach(struct socket *so, struct proc *p)
> >  {
> > -   struct pfkeyv2_socket *pp;
> > +   struct keycb *pp;
> 
> I'd rename 'pp' into 'pk' for coherency since you're changing all the
> lines ;)
> 

Done.

> > int error;
> >  
> > -   LIST_FOREACH(pp, _sockets, kcb_list)
> > -   if (pp->socket == socket)
> > -   break;
> > -
> > -   if (pp) {
> > -   LIST_REMOVE(pp, kcb_list);
> > +   pp = sotokeycb(so);
> > +   if (pp == NULL)
> > +   return ENOTCONN;
> >  
> > -   if (pp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)
> > -   nregistered--;
> > +   LIST_REMOVE(pp, kcb_list);
> >  
> > -   if (pp->flags & PFKEYV2_SOCKETFLAGS_PROMISC)
> > -   npromisc--;
> > +   if (pp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)
> > +   nregistered--;
> >  
> > -   free(pp, M_PFKEY, 0);
> > -   }
> > +   if (pp->flags & PFKEYV2_SOCKETFLAGS_PROMISC)
> > +   npromisc--;
> >  
> > -   error = raw_usrreq(socket, PRU_DETACH, NULL, NULL, NULL, p);
> > +   error = raw_usrreq(so, PRU_DETACH, NULL, NULL, NULL, p);
> 
> Why not call raw_detach() directly and return 0?
> 

One step at a time. Anyway. changed to code to do so now.

> > return (error);
> >  }
> 
> The rest is ok :)
> 

-- 
:wq Claudio

Re: i386 clang: fix binutils build

[Todd C. Miller]

On Mon, 05 Jun 2017 09:18:31 -0600, "Todd C. Miller" wrote:

> If you assign 0x to a long on i386 the compiler should warn
> about it since it is too big to fit unless the value is unsigned.
> That is why ULONG_MAX is defined as defined as 0xUL on
> 32-bit platforms.

I suppose it doesn't matter as (~(offsetT) 0xL) is going
to be 0 regardless so that part of the conditional will be optimized
away as always true as (0 == 0).

 - todd

Re: ksh(1): don't output invalid UTF-8 characters

[Ingo Schwarze]

Hi Walter,

Walter Alejandro Iglesias wrote on Mon, Jun 05, 2017 at 04:50:21PM +0200:

> I'm still not skilled enough to make a proper patch or a clear bug
> report (I'm on chapter 2 of K :-)).  I wish with time I'll learn how
> to do it.

IIRC, you said you saw some undesirable behaviour with ksh input.

I assume you have a sequence of key presses on your keyboard that
demonstrate the undesirable behaviour.  To capture the sequence,

 1. Type

printf '

(without hitting enter)

 2. Now type the sequence of characters, BUT
 - before any control character, press Ctrl-V
 - before any single quote, press a backslash

 3. Type

' > input.txt

and hit enter.

You can look at the sequence with

  hexdump -C input.txt

There should be at least one byte for each key pressed, sometimes
more.  If some key presses do not show up or the order is mixed up,
you likely forgot the Ctrl-V before it.  In that case, retry.

If you want to save people who look at your report some work,
you can craft a printf(1) statement that generates the same
output as the above, but using only ASCII letters, and send
that instead of the output of hexdump -C input.txt.

Here is a simple example.
Test insertion of a non-ASCII character between two ASCII characters.

What i type on my keyboard:

  x y Ctrl-B e-accent-aigu

What i type to capture the input:

  p r i n t f blank ' x y Ctrl-V Ctrl-B e-accent-aigu ' > i n p u t . t x t

This can be used to create the same input file:

   $ printf 'xy\x02\xc3\xa9' > input2.txt
   $ cmp input.txt input2.txt 
   $ echo $?
  0

For testing, go to the regress directory:

   $ cd /usr/src/regress/bin/ksh
   $ cvs up -dP
   $ cd edit
   $ make obj
   $ make cleandir
   $ make regress
   $ ./obj/edit < input.txt | hexdump -C
    24 20 78 79 08 c3 a9 79  08 0a   |$ xy...y..|
  000a

Note that the above output is already an improved (uncommitted)
version containing a private patch.

Here is what the -current ksh does:

   $ PATH=/obin ./obj/edit < input.txt | hexdump -C
    24 20 78 79 08 c3 79 08  08 c3 a9 79 08 0a   |$ xy..yy..|
  000e

Also describe in words why you are typing that exact sequence,
what you would like to happen, and how the actual events
differ from that.

Should be feasible, right?


> I came to the ksh utf8 discussion because I've been playing
> with some mail mime encoder just to learn C and recognizing
> valid utf-8 was the first challenge I ecountered.

In a normal non-threaded application program, you should probably
use mbtowc(3) rather than coding your own UTF-8 parser.

> The code pasted below is what I got so far in recognizing valid utf-8.

That code looks confusing.  I'm not studying it in detail right now
because we already have at least two working UTF-8 decoders in the
tree.

One is /usr/src/lib/libc/citrus/citrus_utf8.c,
function _citrus_utf8_ctype_mbrtowc().  It is relatively long,
but quite easy to understand.

Another one is /usr/src/usr.bin/mandoc/preconv.c,
function preconv_encode().  It is substantially shorter,
but admittedly harder to understand.

This is OpenBSD.  Read the fantastic source to learn something.

> I'm showing it to make my point, I realize it isn't easy; and from my
> poor C I'm not able to figure out how you can do such test byte by byte
> while the user is typing at command line.  (Don't bother in explaining
> me how, I know this is not the place to take C lessons.)

Well, it seems likely the something doing just that will be committed
to ksh/emacs.c during the next week or so - not a full parser, but
something showing the incremental nature.  Then you can inspect it.

If you report actual bugs or propose fixes, then developers are often
willing to help with C issues related to it as well, even though
teaching C is indeed not the point of this list.

> By the way, something the last paragraph of the new utf8(7) man page
> isn't clear enough (I mentioned this to tedu@).

Which paragraph exactly, and what is unclear?  Maybe we can fix it
quickly.

> #define YES   1
> #define NO0

Either just use 1 and 0 in the code or use .

[...]
>   }

Don't forget checking ferror(3) after falling out of the main loop.

>   return 0;
> }

Yours,
  Ingo

Re: i386: sync trampoline code with amd64 for clang

[Mike Larkin]

On Sun, Jun 04, 2017 at 11:46:00PM +0200, Christian Weisgerber wrote:
> This catches up i386 with the changes to acpi_wakecode.S and mptramp.s
> that were made on amd64 for clang.  Here's kettenis@'s original
> commit message:
> 
> --->
> Generating mixed 16-bit/32-bit/64-bit code with clang's integrated
> assembler is a bit tricky.  It supports the .code16, .code32 and
> .code64 directives.  But it doesn't know about the data16/data32 and
> addr16/addr32 instruction prefixes.  Instead it tries to determine
> those from the instruction opcode.  It mostly succeeds, but there are
> a couple of corner cases where clang will generate the "addr32" form
> where gas generates the "addr16" form in .code16 segments.  That
> should be no problem (and just waste a couple of bytes), but it makes
> comparing the generated code a bit difficult.
> 
> Allow the trampoline code to be compiled with both.  For clang #define
> away the addr32 prefix and avoid using the data32 prefix by using a
> mnemonic that explicitly encodes the size of the operand.  Add a few
> addr32 prefixes in .code16 blocks to reduce the differences between
> code generated by clang and gas.
> <---
> 
> ok?
> 

yep. go for it. ok mlarkin

> Index: acpi_wakecode.S
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/acpi_wakecode.S,v
> retrieving revision 1.27
> diff -u -p -r1.27 acpi_wakecode.S
> --- acpi_wakecode.S   20 May 2016 02:30:41 -  1.27
> +++ acpi_wakecode.S   4 Jun 2017 21:06:47 -
> @@ -52,6 +52,10 @@
>  #include 
>  #include 
>  
> +#ifdef __clang__
> +#define addr32
> +#endif
> +
>  #define _ACPI_TRMP_LABEL(a) a = . - _C_LABEL(acpi_real_mode_resume) + 
> ACPI_TRAMPOLINE
>  #define _ACPI_TRMP_OFFSET(a) a = . - _C_LABEL(acpi_real_mode_resume)
>  #define _ACPI_TRMP_DATA_LABEL(a) a = . - _C_LABEL(acpi_tramp_data_start) + \
> @@ -108,7 +112,7 @@ _ACPI_TRMP_OFFSET(acpi_s3_vector_real)
>   movw%ax, %ss
>   movw%cs, %ax
>   movw%ax, %es
> - lidtl   clean_idt
> + addr32 lidtlclean_idt
>  
>   /*
>* Set up stack to grow down from offset 0x0FFE.
> @@ -140,7 +144,7 @@ _ACPI_TRMP_OFFSET(acpi_s3_vector_real)
>* to sleep (although on i386, the saved GDT will most likely
>* represent something similar based on machine/segment.h).
>*/
> - data32 addr32 lgdt  tmp_gdt
> + addr32 lgdtltmp_gdt
>  
>   /*
>* Enable protected mode by setting the PE bit in CR0
> @@ -342,7 +346,7 @@ _ACPI_TRMP_LABEL(hibernate_resume_vector
>   movw%ax, %fs
>   movw%ax, %gs
>   movl$0x0FFE, %esp
> - lidtl   clean_idt
> + addr32 lidtlclean_idt
>  
>   /* Jump to the S3 resume vector */
>   ljmp$(_ACPI_RM_CODE_SEG), $acpi_s3_vector_real
> Index: mptramp.s
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/mptramp.s,v
> retrieving revision 1.20
> diff -u -p -r1.20 mptramp.s
> --- mptramp.s 6 Feb 2017 01:50:36 -   1.20
> +++ mptramp.s 4 Jun 2017 20:32:04 -
> @@ -84,6 +84,10 @@
>  #include 
>  #include 
>  
> +#ifdef __clang__
> +#define addr32
> +#endif
> +
>  #define GDTE(a,b).byte   0xff,0xff,0x0,0x0,0x0,a,b,0x0
>  #define _RELOC(x)((x) - KERNBASE)
>  #define RELOC(x) _RELOC(_C_LABEL(x))
> @@ -115,7 +119,7 @@ _C_LABEL(cpu_spinup_trampoline):
>   movw%cs, %ax
>   movw%ax, %es
>   movw%ax, %ss
> - data32 addr32 lgdt  (gdt_desc)  # load flat descriptor table
> + addr32 lgdtl (gdt_desc) # load flat descriptor table
>   movl%cr0, %eax  # get cr0
>   orl $0x1, %eax  # enable protected mode
>   movl%eax, %cr0  # doit
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 

Re: remove vlan specific ifconfig settings

[Reyk Floeter]

Well, not just muscle memory but the fact that some people including me had 
hostname.vlanX files without an explicit "vlan X" in it.

And I did like the implicit tags, despite your vlan6000 problem that nobody 
ever had ;-)

But it is time to move on, we have to cope with it.

So no objections anymore, OK reyk

> Am 05.06.2017 um 07:28 schrieb David Gwynne :
> 
> vlan(4) handles the vnetid and ifparent ioctls, so we dont need the
> vlan specific handling. i previously removed the code that requests
> info with a vlan specific ioctl, but this removes the vlan settings
> code.
> 
> there's a couple of semantic changes to note though.
> 
> firstly, this aliases the "vlan", "vlandev" and "-vlandev" config
> options to setvnetid, setifparent, and delifparent respectively.
> this means hostname.vlan and muscle memory will largely keep working.
> 
> secondly, if ifconfig didnt think you'd already configured a vlan
> tag on an interface, it would set one for you based on the unit
> number of the interface. eg, the following:
> 
> # ifconfig vlan7 create
> # ifconfig vlan7 vlandev em0
> 
> ... would cause vlan7 to be configured with a vnetid of 7 too.
> 
> i have proposed this change before, but some people pushed back,
> mostly claiming they didnt want to learn new muscle memory.
> 
> id still like to do this though. the main reasons are: muscle memory
> hasn't been an excuse not to change things. eg, im still learning
> to cope with the removal of the sparc architecture, and i still
> type sudo a few times before remembering it's doas now.
> 
> secondly, the implicit vlan tag generation is inconsistent, both
> with other drivers, and with vlan itself. if you create vlan6000,
> this doesnt kick in. if you create vxlan0, vxlan1, vxlan6000, there's
> no implicit tag there either.
> 
> id rather the config was explicit, rather than conditially implicit.
> 
> ok?
> 
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.342
> diff -u -p -r1.342 ifconfig.c
> --- ifconfig.c5 Jun 2017 05:10:23 -1.342
> +++ ifconfig.c5 Jun 2017 05:16:36 -
> @@ -91,8 +91,6 @@
> 
> #include 
> 
> -#include 
> -
> #include 
> 
> #include 
> @@ -216,9 +214,6 @@ voidsetmpwencap(const char *, int);
> voidsetmpwlabel(const char *, const char *);
> voidsetmpwneighbor(const char *, int);
> voidsetmpwcontrolword(const char *, int);
> -voidsetvlantag(const char *, int);
> -voidsetvlandev(const char *, int);
> -voidunsetvlandev(const char *, int);
> voidmpe_status(void);
> voidmpw_status(void);
> voidsetrdomain(const char *, int);
> @@ -367,9 +362,9 @@ const structcmd {
>{ "scan",NEXTARG0,0,setifscan },
>{ "broadcast",NEXTARG,0,setifbroadaddr },
>{ "prefixlen",  NEXTARG,0,setifprefixlen},
> -{ "vlan",NEXTARG,0,setvlantag },
> -{ "vlandev",NEXTARG,0,setvlandev },
> -{ "-vlandev",1,0,unsetvlandev },
> +{ "vlan",NEXTARG,0,setvnetid },
> +{ "vlandev",NEXTARG,0,setifparent },
> +{ "-vlandev",1,0,delifparent },
>{ "group",NEXTARG,0,setifgroup },
>{ "-group",NEXTARG,0,unsetifgroup },
>{ "autoconf",1,0,setautoconf },
> @@ -3768,83 +3763,6 @@ getencap(void)
>}
> 
>printf("\n");
> -}
> -
> -static int __tag = 0;
> -static int __have_tag = 0;
> -
> -/* ARGSUSED */
> -void
> -setvlantag(const char *val, int d)
> -{
> -u_int16_t tag;
> -struct vlanreq vreq;
> -const char *errmsg = NULL;
> -
> -__tag = tag = strtonum(val, 0, 4095, );
> -if (errmsg)
> -errx(1, "vlan tag %s: %s", val, errmsg);
> -__have_tag = 1;
> -
> -bzero((char *), sizeof(struct vlanreq));
> -ifr.ifr_data = (caddr_t)
> -
> -if (ioctl(s, SIOCGETVLAN, (caddr_t)) == -1)
> -err(1, "SIOCGETVLAN");
> -
> -vreq.vlr_tag = tag;
> -
> -if (ioctl(s, SIOCSETVLAN, (caddr_t)) == -1)
> -err(1, "SIOCSETVLAN");
> -}
> -
> -/* ARGSUSED */
> -void
> -setvlandev(const char *val, int d)
> -{
> -struct vlanreq vreq;
> -int tag;
> -size_t skip;
> -const char*estr;
> -
> -bzero((char *), sizeof(struct vlanreq));
> -ifr.ifr_data = (caddr_t)
> -
> -if (ioctl(s, SIOCGETVLAN, (caddr_t)) == -1)
> -err(1, "SIOCGETVLAN");
> -
> -(void) strlcpy(vreq.vlr_parent, val, sizeof(vreq.vlr_parent));
> -
> -if (!__have_tag && vreq.vlr_tag == 0) {
> -skip = strcspn(ifr.ifr_name, "0123456789");
> -tag = strtonum(ifr.ifr_name + skip, 0, 4095, );
> -if (estr != NULL)
> -errx(1, "invalid vlan tag and device specification");
> -vreq.vlr_tag = tag;
> -} else if (__have_tag)
> -vreq.vlr_tag = __tag;
> -
> -if 

Re: remove vlan specific ifconfig settings

[David Gwynne]

> On 5 Jun 2017, at 17:05, Reyk Floeter  wrote:
> 
> Well, not just muscle memory but the fact that some people including me had 
> hostname.vlanX files without an explicit "vlan X" in it.

hrm. yes.

that means if you change the vnetid on such an interface at runtime, and then 
re-run netstart later on to try and reset the stack, it wont configure it back 
to the state it would be on boot. im not sure we support running netstart after 
boot though, so maybe it doesnt matter.

> 
> And I did like the implicit tags, despite your vlan6000 problem that nobody 
> ever had ;-)

i had the opposite problem where new vlan interfaces would collide with 
existing ones.

> 
> But it is time to move on, we have to cope with it.
> 
> So no objections anymore, OK reyk

thank you.

> 
>> Am 05.06.2017 um 07:28 schrieb David Gwynne :
>> 
>> vlan(4) handles the vnetid and ifparent ioctls, so we dont need the
>> vlan specific handling. i previously removed the code that requests
>> info with a vlan specific ioctl, but this removes the vlan settings
>> code.
>> 
>> there's a couple of semantic changes to note though.
>> 
>> firstly, this aliases the "vlan", "vlandev" and "-vlandev" config
>> options to setvnetid, setifparent, and delifparent respectively.
>> this means hostname.vlan and muscle memory will largely keep working.
>> 
>> secondly, if ifconfig didnt think you'd already configured a vlan
>> tag on an interface, it would set one for you based on the unit
>> number of the interface. eg, the following:
>> 
>> # ifconfig vlan7 create
>> # ifconfig vlan7 vlandev em0
>> 
>> ... would cause vlan7 to be configured with a vnetid of 7 too.
>> 
>> i have proposed this change before, but some people pushed back,
>> mostly claiming they didnt want to learn new muscle memory.
>> 
>> id still like to do this though. the main reasons are: muscle memory
>> hasn't been an excuse not to change things. eg, im still learning
>> to cope with the removal of the sparc architecture, and i still
>> type sudo a few times before remembering it's doas now.
>> 
>> secondly, the implicit vlan tag generation is inconsistent, both
>> with other drivers, and with vlan itself. if you create vlan6000,
>> this doesnt kick in. if you create vxlan0, vxlan1, vxlan6000, there's
>> no implicit tag there either.
>> 
>> id rather the config was explicit, rather than conditially implicit.
>> 
>> ok?
>> 
>> Index: ifconfig.c
>> ===
>> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
>> retrieving revision 1.342
>> diff -u -p -r1.342 ifconfig.c
>> --- ifconfig.c5 Jun 2017 05:10:23 -1.342
>> +++ ifconfig.c5 Jun 2017 05:16:36 -
>> @@ -91,8 +91,6 @@
>> 
>> #include 
>> 
>> -#include 
>> -
>> #include 
>> 
>> #include 
>> @@ -216,9 +214,6 @@ voidsetmpwencap(const char *, int);
>> voidsetmpwlabel(const char *, const char *);
>> voidsetmpwneighbor(const char *, int);
>> voidsetmpwcontrolword(const char *, int);
>> -voidsetvlantag(const char *, int);
>> -voidsetvlandev(const char *, int);
>> -voidunsetvlandev(const char *, int);
>> voidmpe_status(void);
>> voidmpw_status(void);
>> voidsetrdomain(const char *, int);
>> @@ -367,9 +362,9 @@ const structcmd {
>>   { "scan",NEXTARG0,0,setifscan },
>>   { "broadcast",NEXTARG,0,setifbroadaddr },
>>   { "prefixlen",  NEXTARG,0,setifprefixlen},
>> -{ "vlan",NEXTARG,0,setvlantag },
>> -{ "vlandev",NEXTARG,0,setvlandev },
>> -{ "-vlandev",1,0,unsetvlandev },
>> +{ "vlan",NEXTARG,0,setvnetid },
>> +{ "vlandev",NEXTARG,0,setifparent },
>> +{ "-vlandev",1,0,delifparent },
>>   { "group",NEXTARG,0,setifgroup },
>>   { "-group",NEXTARG,0,unsetifgroup },
>>   { "autoconf",1,0,setautoconf },
>> @@ -3768,83 +3763,6 @@ getencap(void)
>>   }
>> 
>>   printf("\n");
>> -}
>> -
>> -static int __tag = 0;
>> -static int __have_tag = 0;
>> -
>> -/* ARGSUSED */
>> -void
>> -setvlantag(const char *val, int d)
>> -{
>> -u_int16_t tag;
>> -struct vlanreq vreq;
>> -const char *errmsg = NULL;
>> -
>> -__tag = tag = strtonum(val, 0, 4095, );
>> -if (errmsg)
>> -errx(1, "vlan tag %s: %s", val, errmsg);
>> -__have_tag = 1;
>> -
>> -bzero((char *), sizeof(struct vlanreq));
>> -ifr.ifr_data = (caddr_t)
>> -
>> -if (ioctl(s, SIOCGETVLAN, (caddr_t)) == -1)
>> -err(1, "SIOCGETVLAN");
>> -
>> -vreq.vlr_tag = tag;
>> -
>> -if (ioctl(s, SIOCSETVLAN, (caddr_t)) == -1)
>> -err(1, "SIOCSETVLAN");
>> -}
>> -
>> -/* ARGSUSED */
>> -void
>> -setvlandev(const char *val, int d)
>> -{
>> -struct vlanreq vreq;
>> -int tag;
>> -size_t skip;
>> -const char*estr;
>> -
>> -

Re: remove vlan specific ifconfig settings

[Reyk Floeter]

> Am 05.06.2017 um 09:26 schrieb David Gwynne :
> 
> 
>> On 5 Jun 2017, at 17:05, Reyk Floeter  wrote:
>> 
>> Well, not just muscle memory but the fact that some people including me had 
>> hostname.vlanX files without an explicit "vlan X" in it.
> 
> hrm. yes.
> 
> that means if you change the vnetid on such an interface at runtime, and then 
> re-run netstart later on to try and reset the stack, it wont configure it 
> back to the state it would be on boot. im not sure we support running 
> netstart after boot though, so maybe it doesnt matter.
> 

We do and I run it all the time, for example

# sh /etc/netstart vlan5

But I'd usually destroy a cloner before creating it again.

>> 
>> And I did like the implicit tags, despite your vlan6000 problem that nobody 
>> ever had ;-)
> 
> i had the opposite problem where new vlan interfaces would collide with 
> existing ones.
> 

Having the same tag on multiple interfaces? Yes, that was a bit clunky with the 
implicit tags.

>> 
>> But it is time to move on, we have to cope with it.
>> 
>> So no objections anymore, OK reyk
> 
> thank you.
> 

Reyk

>> 
>>> Am 05.06.2017 um 07:28 schrieb David Gwynne :
>>> 
>>> vlan(4) handles the vnetid and ifparent ioctls, so we dont need the
>>> vlan specific handling. i previously removed the code that requests
>>> info with a vlan specific ioctl, but this removes the vlan settings
>>> code.
>>> 
>>> there's a couple of semantic changes to note though.
>>> 
>>> firstly, this aliases the "vlan", "vlandev" and "-vlandev" config
>>> options to setvnetid, setifparent, and delifparent respectively.
>>> this means hostname.vlan and muscle memory will largely keep working.
>>> 
>>> secondly, if ifconfig didnt think you'd already configured a vlan
>>> tag on an interface, it would set one for you based on the unit
>>> number of the interface. eg, the following:
>>> 
>>> # ifconfig vlan7 create
>>> # ifconfig vlan7 vlandev em0
>>> 
>>> ... would cause vlan7 to be configured with a vnetid of 7 too.
>>> 
>>> i have proposed this change before, but some people pushed back,
>>> mostly claiming they didnt want to learn new muscle memory.
>>> 
>>> id still like to do this though. the main reasons are: muscle memory
>>> hasn't been an excuse not to change things. eg, im still learning
>>> to cope with the removal of the sparc architecture, and i still
>>> type sudo a few times before remembering it's doas now.
>>> 
>>> secondly, the implicit vlan tag generation is inconsistent, both
>>> with other drivers, and with vlan itself. if you create vlan6000,
>>> this doesnt kick in. if you create vxlan0, vxlan1, vxlan6000, there's
>>> no implicit tag there either.
>>> 
>>> id rather the config was explicit, rather than conditially implicit.
>>> 
>>> ok?
>>> 
>>> Index: ifconfig.c
>>> ===
>>> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
>>> retrieving revision 1.342
>>> diff -u -p -r1.342 ifconfig.c
>>> --- ifconfig.c5 Jun 2017 05:10:23 -1.342
>>> +++ ifconfig.c5 Jun 2017 05:16:36 -
>>> @@ -91,8 +91,6 @@
>>> 
>>> #include 
>>> 
>>> -#include 
>>> -
>>> #include 
>>> 
>>> #include 
>>> @@ -216,9 +214,6 @@ voidsetmpwencap(const char *, int);
>>> voidsetmpwlabel(const char *, const char *);
>>> voidsetmpwneighbor(const char *, int);
>>> voidsetmpwcontrolword(const char *, int);
>>> -voidsetvlantag(const char *, int);
>>> -voidsetvlandev(const char *, int);
>>> -voidunsetvlandev(const char *, int);
>>> voidmpe_status(void);
>>> voidmpw_status(void);
>>> voidsetrdomain(const char *, int);
>>> @@ -367,9 +362,9 @@ const structcmd {
>>>  { "scan",NEXTARG0,0,setifscan },
>>>  { "broadcast",NEXTARG,0,setifbroadaddr },
>>>  { "prefixlen",  NEXTARG,0,setifprefixlen},
>>> -{ "vlan",NEXTARG,0,setvlantag },
>>> -{ "vlandev",NEXTARG,0,setvlandev },
>>> -{ "-vlandev",1,0,unsetvlandev },
>>> +{ "vlan",NEXTARG,0,setvnetid },
>>> +{ "vlandev",NEXTARG,0,setifparent },
>>> +{ "-vlandev",1,0,delifparent },
>>>  { "group",NEXTARG,0,setifgroup },
>>>  { "-group",NEXTARG,0,unsetifgroup },
>>>  { "autoconf",1,0,setautoconf },
>>> @@ -3768,83 +3763,6 @@ getencap(void)
>>>  }
>>> 
>>>  printf("\n");
>>> -}
>>> -
>>> -static int __tag = 0;
>>> -static int __have_tag = 0;
>>> -
>>> -/* ARGSUSED */
>>> -void
>>> -setvlantag(const char *val, int d)
>>> -{
>>> -u_int16_t tag;
>>> -struct vlanreq vreq;
>>> -const char *errmsg = NULL;
>>> -
>>> -__tag = tag = strtonum(val, 0, 4095, );
>>> -if (errmsg)
>>> -errx(1, "vlan tag %s: %s", val, errmsg);
>>> -__have_tag = 1;
>>> -
>>> -bzero((char *), sizeof(struct vlanreq));
>>> -ifr.ifr_data = 

Re: remove vlan specific ifconfig settings

[Reyk Floeter]

> Am 05.06.2017 um 09:35 schrieb Reyk Floeter :
> 
> 
>> Am 05.06.2017 um 09:26 schrieb David Gwynne :
>> 
>> 
>>> On 5 Jun 2017, at 17:05, Reyk Floeter  wrote:
>>> 
>>> Well, not just muscle memory but the fact that some people including me had 
>>> hostname.vlanX files without an explicit "vlan X" in it.
>> 
>> hrm. yes.
>> 
>> that means if you change the vnetid on such an interface at runtime, and 
>> then re-run netstart later on to try and reset the stack, it wont configure 
>> it back to the state it would be on boot. im not sure we support running 
>> netstart after boot though, so maybe it doesnt matter.
>> 
> 
> We do and I run it all the time, for example
> 
> # sh /etc/netstart vlan5
> 
> But I'd usually destroy a cloner before creating it again.
> 

Let me rephrase:

I'm fine with the change as long as we never error out on vnetid/parent 
transitions. The kernel should just cope with it, eg. running the following 
commands in sequence should be fine and not throw EBUSY or similar:

# ifconfig vlan5 create
# ifconfig vlan5 up
# ifconfig vlan5 vlandev em0
# ifconfig vlan5 vlan 5
# ifconfig vlan5 vlan 6
# ifconfig vlan5 vlandev em1

It think that is the case now. I once updated vlan and a few other cloners to 
stop giving errors in certain "unconfigured" states because it made it hard in 
scripts to gather enough input before it works. I also have to be able to 
change the underlying parent or vnetid on the fly.

Reyk

>>> 
>>> And I did like the implicit tags, despite your vlan6000 problem that nobody 
>>> ever had ;-)
>> 
>> i had the opposite problem where new vlan interfaces would collide with 
>> existing ones.
>> 
> 
> Having the same tag on multiple interfaces? Yes, that was a bit clunky with 
> the implicit tags.
> 
>>> 
>>> But it is time to move on, we have to cope with it.
>>> 
>>> So no objections anymore, OK reyk
>> 
>> thank you.
>> 
> 
> Reyk
> 
>>> 
 Am 05.06.2017 um 07:28 schrieb David Gwynne :
 
 vlan(4) handles the vnetid and ifparent ioctls, so we dont need the
 vlan specific handling. i previously removed the code that requests
 info with a vlan specific ioctl, but this removes the vlan settings
 code.
 
 there's a couple of semantic changes to note though.
 
 firstly, this aliases the "vlan", "vlandev" and "-vlandev" config
 options to setvnetid, setifparent, and delifparent respectively.
 this means hostname.vlan and muscle memory will largely keep working.
 
 secondly, if ifconfig didnt think you'd already configured a vlan
 tag on an interface, it would set one for you based on the unit
 number of the interface. eg, the following:
 
 # ifconfig vlan7 create
 # ifconfig vlan7 vlandev em0
 
 ... would cause vlan7 to be configured with a vnetid of 7 too.
 
 i have proposed this change before, but some people pushed back,
 mostly claiming they didnt want to learn new muscle memory.
 
 id still like to do this though. the main reasons are: muscle memory
 hasn't been an excuse not to change things. eg, im still learning
 to cope with the removal of the sparc architecture, and i still
 type sudo a few times before remembering it's doas now.
 
 secondly, the implicit vlan tag generation is inconsistent, both
 with other drivers, and with vlan itself. if you create vlan6000,
 this doesnt kick in. if you create vxlan0, vxlan1, vxlan6000, there's
 no implicit tag there either.
 
 id rather the config was explicit, rather than conditially implicit.
 
 ok?
 
 Index: ifconfig.c
 ===
 RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
 retrieving revision 1.342
 diff -u -p -r1.342 ifconfig.c
 --- ifconfig.c5 Jun 2017 05:10:23 -1.342
 +++ ifconfig.c5 Jun 2017 05:16:36 -
 @@ -91,8 +91,6 @@
 
 #include 
 
 -#include 
 -
 #include 
 
 #include 
 @@ -216,9 +214,6 @@ voidsetmpwencap(const char *, int);
 voidsetmpwlabel(const char *, const char *);
 voidsetmpwneighbor(const char *, int);
 voidsetmpwcontrolword(const char *, int);
 -voidsetvlantag(const char *, int);
 -voidsetvlandev(const char *, int);
 -voidunsetvlandev(const char *, int);
 voidmpe_status(void);
 voidmpw_status(void);
 voidsetrdomain(const char *, int);
 @@ -367,9 +362,9 @@ const structcmd {
 { "scan",NEXTARG0,0,setifscan },
 { "broadcast",NEXTARG,0,setifbroadaddr },
 { "prefixlen",  NEXTARG,0,setifprefixlen},
 -{ "vlan",NEXTARG,0,setvlantag },
 -{ "vlandev",NEXTARG,0,setvlandev },
 -{ "-vlandev",1,0,unsetvlandev },
 +{ "vlan",

ifconfig.8 doco for vnetid and parent options

[David Gwynne]

this adds doco for the parent options in ifconfig, and moves vnetid
up into generic options list.

the vlan(4) specific doco has enough lies and omissions that id
rather get rid of it.

ok?

Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.282
diff -u -p -r1.282 ifconfig.8
--- ifconfig.8  12 May 2017 15:11:02 -  1.282
+++ ifconfig.8  6 Jun 2017 01:54:10 -
@@ -415,6 +415,14 @@ and 0's for the host part.
 The mask should contain at least the standard network portion,
 and the subnet field should be contiguous with the network
 portion.
+.It Cm parent Ar parent-interface
+Associate with interface
+.Ar parent-interface .
+Packets transmitted through the interface will be encapsulated and
+diverted to the specified parent interface
+.It Cm -parent
+Disassociate from the parent interface.
+This breaks the link between the interface and its parent.
 .It Cm prefixlen Ar n
 (inet and inet6 only)
 Effect is similar to
@@ -461,6 +469,20 @@ This may be used to enable an interface 
 It happens automatically when setting the first address on an interface.
 If the interface was reset when previously marked down,
 the hardware will be re-initialized.
+.It Cm vnetid Ar network-id Ns | Ns Cm any
+Set the virtual network identifier.
+This is a number which is used by encapsulation interfaces such as
+.Xr vlan 4
+or
+.Xr vxlan 4
+to identify packets with a virtual network.
+If supported by the interface,
+the value can also be set to
+.Ar any
+to accept packets with arbitrary network identifiers (for example for
+multipoint-to-multipoint modes).
+.It Cm -vnetid
+Clear the virtual network identifier.
 .It Cm wol
 Enable Wake on LAN (WoL).
 When enabled, reception of a WoL frame will cause the network card to
@@ -1531,7 +1553,6 @@ for a complete list of the available pro
 .Op Oo Fl Oc Ns Cm keepalive Ar period count
 .Op Cm tunnel Ar src_address dest_address
 .Op Cm tunneldomain Ar tableid
-.Op Oo Fl Oc Ns Cm vnetid Ar network-id
 .Ek
 .nr nS 0
 .Pp
@@ -1583,21 +1604,6 @@ can be set to any valid routing table ID
 the corresponding routing domain is derived from this table.
 .It Cm tunnelttl Ar ttl
 Set the IP or multicast TTL of the tunnel packets.
-.It Cm vnetid Ar network-id
-Set the virtual network identifier.
-This is a number which is used by tunnel protocols such as
-.Xr vxlan 4
-to identify packets with a virtual network.
-The accepted size of the number depends on the individual tunnel protocol;
-it is a 24-bit number for
-.Xr vxlan 4 .
-If supported by the tunnel protocol,
-the value can also be set to
-.Ar any
-to accept packets with arbitrary network identifiers (for example for
-multipoint-to-multipoint modes).
-.It Cm -vnetid
-Clear the virtual network identifier.
 .El
 .Sh UMB
 .nr nS 1
@@ -1660,52 +1666,6 @@ Disable data roaming.
 As soon as the interface is marked as "up", the
 .Xr umb 4
 device will try to establish a data connection with the service provider.
-.El
-.Sh VLAN
-.nr nS 1
-.Bk -words
-.Nm ifconfig
-.Ar vlan-interface
-.Op Cm vlan Ar vlan-tag
-.Op Oo Fl Oc Ns Cm vlandev Ar parent-interface
-.Ek
-.nr nS 0
-.Pp
-The following options are available for a
-.Xr vlan 4
-interface:
-.Bl -tag -width Ds
-.It Cm vlan Ar vlan-tag
-Set the vlan tag value
-to
-.Ar vlan-tag .
-This value is a 12-bit number which is used to create an 802.1Q
-vlan header for packets sent from the vlan interface.
-This value cannot be changed once it is set for an interface.
-.It Cm vlandev Ar parent-interface
-Associate with interface
-.Ar parent-interface .
-Packets transmitted through the vlan interface will be
-diverted to the specified interface
-.Ar parent-interface
-with 802.1Q vlan tagging.
-Packets with 802.1Q tagging received
-by the parent interface with the correct vlan tag will be diverted to
-the associated vlan pseudo-device.
-The vlan interface is assigned a
-copy of the parent interface's flags and the parent's Ethernet address.
-If
-.Cm vlandev
-and
-.Cm vlan
-are not set at the same time, the vlan tag will be inferred from
-the interface name, for instance
-.Cm vlan5
-will be assigned 802.1Q tag 5.
-.It Cm -vlandev
-Disassociate from the parent interface.
-This breaks the link between the vlan interface and its parent,
-clears its vlan tag, flags, and link address, and shuts the interface down.
 .El
 .Sh EXAMPLES
 Assign the

Re: ksh(1): don't output invalid UTF-8 characters

[Walter Alejandro Iglesias]

On Mon, Jun 05, 2017 at 06:06:34PM +0200, Ingo Schwarze wrote:
> Hi Walter,
> 
> Walter Alejandro Iglesias wrote on Mon, Jun 05, 2017 at 04:50:21PM +0200:
> 
> > report (I'm on chapter 2 of K :-)).  I wish with time I'll learn how
> > to do it.
> 
> IIRC, you said you saw some undesirable behaviour with ksh input.
> 
> I assume you have a sequence of key presses on your keyboard that
> demonstrate the undesirable behaviour.  To capture the sequence,
>

I will *study* all the indications you gave me.  But this time
I don't think you need a capture of the sequence.  Just use *any*
latin-1 character whose hex value is smaller than \xc0.

To facilitate you the test, in xterm after setting "setxkbmap de":

  AltGr + Shift + 1

prints me the opening exclamation mark (\xa1) we also use in Spanish.
In console or a C xterm, type that merged among random ascii characters,
then move the cursor from the first to the last column passing over that
character.  Assuming you're running current, see what happens.


Anyway, to be honest, these bugs don't hurt, you can live with them.
What I'm trying to say with these reports is I'm not truly convinced
utf8 support in console is a good idea.

Another test you can do, this time in a utf-8 xterm: if you activate the
bell and go with the cursor to the end of the line it'll beep.  Now type
some utf-8 character at the end and do the same, it won't beep, because
the cursor is in the first byte of the utf-8 character, *it can't reach
the real end of the line*.  Nobody will die because this issue or the
other above.  My point is utf8 will always be a mess.  KEN, DO YOU HEAR
ME?, IT WAS YOUR OWN CHILD, KEN! :-)

I wonder how plan9 handle utf8.


[...]

>
> 
> For testing, go to the regress directory:
> 
>$ cd /usr/src/regress/bin/ksh
>$ cvs up -dP
>$ cd edit
>$ make obj
>$ make cleandir
>$ make regress
>$ ./obj/edit < input.txt | hexdump -C
>     24 20 78 79 08 c3 a9 79  08 0a   |$ xy...y..|
>   000a


I've been wondering how to work with this.  Thanks!


[...]

> > By the way, something the last paragraph of the new utf8(7) man page
> > isn't clear enough (I mentioned this to tedu@).
> 
> Which paragraph exactly, and what is unclear?  Maybe we can fix it
> quickly.

As I told you, the _last_ one:

   Encodings using more bytes than required are invalid.  In particular,
   1100 and 1101 are not valid start bytes, the byte after
   1110 must be at least 1010, and the byte after  must
   be at least 1001.

I don't understand the 'at least' assumptions.  Some examples in which
the byte after 1110 is *smaller* than 1010:

Euro sign:
11100010 1010 10101100

Em dash:
11100010 1000 10010100

Double quotes:
11100010 1000 10011100
11100010 1000 10011101

You can find examples where the byte after 1110 is *grater* than
1010 here:

http://www.utf8-chartable.de/



Thank you for your advices I'll study your whole message carefuly.


> 
> Yours,
>   Ingo

Re: ksh(1): don't output invalid UTF-8 characters

[Kurt H Maier]

On Mon, Jun 05, 2017 at 09:21:31PM +0200, Walter Alejandro Iglesias wrote:
> 
> I wonder how plan9 handle utf8.
> 

In general, by getting rid of TTYs and character-addressed interfaces
almost entirely.  Probably not the best fit for OpenBSD.

khm

Re: ksh(1): don't output invalid UTF-8 characters

[Walter Alejandro Iglesias]

In article <20170605192131.ga60...@server.roquesor.com> you wrote:
> 
>Encodings using more bytes than required are invalid.  In particular,
>1100 and 1101 are not valid start bytes, the byte after
>1110 must be at least 1010, and the byte after  must
>be at least 1001.
> 

Someone off list explained me this is true just for the exact 1110
byte.  I'd assumed the man page was referring to any 1110 sequence.
Now I understand.

Re: ksh(1): don't output invalid UTF-8 characters

[Ingo Schwarze]

Hi Walter,

Walter Alejandro Iglesias wrote on Mon, Jun 05, 2017 at 09:21:31PM +0200:
> On Mon, Jun 05, 2017 at 06:06:34PM +0200, Ingo Schwarze wrote:
>> Walter Alejandro Iglesias wrote on Mon, Jun 05, 2017 at 04:50:21PM +0200:

> But this time I don't think you need a capture of the sequence.

Well *you* obviously know what problem you have, but *I* don't
understand it, so please don't tell me that i understand when i
don't, and don't tell me that i don't need the details i need.  In
general, when reporting bugs, do not guess which information might
or might not be needed, but provide what people trying to fix it
ask for.

> Just use *any*
> latin-1 character whose hex value is smaller than \xc0.
> 
> To facilitate you the test, in xterm after setting "setxkbmap de":
> 
>   AltGr + Shift + 1
> 
> prints me the opening exclamation mark (\xa1) we also use in Spanish.

Yes, i can confirm that.

> In console

The console is not UTF-8 capable but expects latin-1 encoding,
which ksh doesn't support at all.  So at the console, you cannot
do command line editing if you type in non-ASCII characters,
or all bets are off.

> or a C xterm, type that merged among random ascii characters, then
> move the cursor from the first to the last column passing over that
> character.  Assuming you're running current, see what happens.

Everything is just fine, i don't see any problem.  Wenn reporting a
bug, it is not sufficient to say "see what happens"; you need to
describe what you see that happens and what you would expect too
happen instead.

Oh, after playing around a bit, i started an xterm with

   $ export LC_CTYPE=C
   $ xterm +u8   # DON'T DO THAT on OpenBSD, ever

Now if i type

   echo abcdef

in that situation and move around, the notions ksh and xterm have
of the cursor position get out of sync, and the display gets
garbled.  Is that what you mean?  That is expected.

In that situation, the mark is represented as 0xa1, which ksh
treats as an invalid zero-width byte, while xterm treats it as
a printable character in that mode.

The problem here is not that ksh does something wrong.  The problem
is that i forced xterm into a mode using a character encoding that
is not supported and cannot be used on OpenBSD at all.

> Anyway, to be honest, these bugs don't hurt, you can live with them.
> What I'm trying to say with these reports is I'm not truly convinced
> utf8 support in console is a good idea.

Oh, having the console grok UTF-8 (just like xterm) would be great,
but its not trivial because it has tentacles into the kernel.

> Another test you can do, this time in a utf-8 xterm: if you activate the
> bell and go with the cursor to the end of the line it'll beep.

More precisely, in emacs mode, if the curser is after the last
character and i type Ctrl-F, it beeps, yes.  In vi command mode,
if the cursor is on the last character and i type "l", it beeps.

> Now type some utf-8 character at the end and do the same,
> it won't beep, because the cursor is in the first byte of the
> utf-8 character, *it can't reach the real end of the line*.

I don't see such a problem in emacs mode, but i do see that in
vi mode.  That's an admittedly small, but real bug...
I should probably have a look how to fix that.

> Nobody will die because this issue or the other above.  My point
> is utf8 will always be a mess.

Yes, it is not likely to ever be perfect, but we can make it
better (and more usable for simple practical tasks).

> KEN, DO YOU HEAR ME?, IT WAS YOUR OWN CHILD, KEN! :-)

To be honest, i doubt that it is possible to design a character
encoding that is substantially better than UTF-8.  Of course, it
is easy to design other encodings of the same quality of UTF-8
(colour of the bikeshed), but none of those are used in practice.
UTF-8 is optimal in a large number of ways and very elegant, and
while it is not trivial to handle, it is as simple as possible.

By contrast, Unicode has large numbers of problems and is not
only not trivial, but so large that is very hard to get an adequate
understanding of it.  Some of those problems (for example, surrogates
and the arbitrary U+10 limit) extend into UTF-8's domain, but
those are Unicode problems, not UTF-8 problems.

>>$ ./obj/edit < input.txt | hexdump -C

> I've been wondering how to work with this.  Thanks!

Maybe somebody should write /usr/src/regress/bin/ksh/edit/edit.1,
like we have /usr/src/regress/usr.bin/mandoc/db/dbm_dump/dbm_dump.1.

>Encodings using more bytes than required are invalid.  In particular,
>1100 and 1101 are not valid start bytes, the byte after
>1110 must be at least 1010, and the byte after  must
>be at least 1001.
> 
> I don't understand the 'at least' assumptions.  Some examples in which
> the byte after 1110 is *smaller* than 1010:
> 
> Euro sign:
> 11100010 1010 10101100
> 
> Em dash:
> 11100010 1000 10010100
> 
> Double quotes:
> 11100010 1000 10011100
> 

Re: ksh(1): don't output invalid UTF-8 characters

[Walter Alejandro Iglesias]

On Mon, Jun 05, 2017 at 10:46:49PM +0200, Ingo Schwarze wrote:
> Hi Walter,
> 
> Walter Alejandro Iglesias wrote on Mon, Jun 05, 2017 at 09:21:31PM +0200:
> > On Mon, Jun 05, 2017 at 06:06:34PM +0200, Ingo Schwarze wrote:
> >> Walter Alejandro Iglesias wrote on Mon, Jun 05, 2017 at 04:50:21PM +0200:
> 
> > But this time I don't think you need a capture of the sequence.
> 
> Well *you* obviously know what problem you have, but *I* don't
> understand it, so please don't tell me that i understand when i
> don't, and don't tell me that i don't need the details i need.  In
> general, when reporting bugs, do not guess which information might
> or might not be needed, but provide what people trying to fix it
> ask for.

First, you're who is assuming wrong.  I don't contact free software
developers to ask them to solve *my* problems.

Second, I didn't provided you more details now than the first time.  The
information I provided was enough to understand and to reproduce the
bug.

Finally, I'm doing this for free too.  If you're not happy with the way I
explain myself feel free to ignore me.


Greetings.

Re: let's add PF_LOCK()

[Alexandr Nedvedicky]

Hello,


> I don't think it matters.  We still need more diffs to be able to test
> that.
> 
> Now I'd really like if you could commit this so we continue improving it
> in-tree.

I've commit the patch _without_ the diff to sys/conf/GENERIC, we can add it
later, once more diffs will be in.

regards
sasha

Re: release(8): add command example to step 3 for sysmerge(8)/MAKEDEV(8)

[Theo Buehler]

On Sun, Jun 04, 2017 at 05:57:01PM -0500, Scott Cheloha wrote:
> Hi,
> 
> There is no command-line sample at the end of step 3 in release(8):
> 
>   After the build is completed, update /etc, /var, and /dev,
>   using sysmerge(8) and MAKEDEV(8).
> 
> While needs vary by site, providing a sample for the typical case
> is useful here because MAKEDEV operates in the current working
> directory while sysmerge(8) "just works" from any directory.
> 
> That MAKEDEV(8) operates on the current working directory is not
> stated in its manpage.  My guess is that this is not obvious
> to a person who is new to building releases.  You could add this
> detail to the description in the MAKEDEV(8) page itself, but even
> then the command example would still be useful in release(8) as an
> applied example.
> 
> Unless it's a very important rite of passage to fill /usr/src with
> device nodes by accident the first time one updates a machine from
> source (laugh it up :D), why not emphasize through example that
> MAKEDEV(8) makes device nodes in the current working directory?
> 
> Am I missing something here?

No not at all. Committed with 'cd /dev && ./MAKEDEV all' instead of the
three separate commands. A clarification for MAKEDEV(8) is currently
being discussed.

Thanks!

> 
> --
> Scott Cheloha
> 
> Index: share/man/man8/release.8
> ===
> RCS file: /cvs/src/share/man/man8/release.8,v
> retrieving revision 1.88
> diff -u -p -r1.88 release.8
> --- share/man/man8/release.818 Apr 2017 16:21:06 -  1.88
> +++ share/man/man8/release.84 Jun 2017 22:15:02 -
> @@ -153,7 +153,12 @@ and
>  using
>  .Xr sysmerge 8
>  and
> -.Xr MAKEDEV 8 .
> +.Xr MAKEDEV 8 :
> +.Bd -literal -offset indent
> +# sysmerge
> +# cd /dev
> +# ./MAKEDEV all
> +.Ed
>  .Pp
>  At this point, the base system is up to date and running the code
>  that will be made into a release.
>