iwm(4) rx reset fix

[Stefan Sperling]

Properly reset the RX ring by clearing RX buffer status exposed to hardware.
Found by Matthew Dillon while porting FreeBSD's iwm(4) to Dragonfly.

ok?

Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.46
diff -u -p -r1.46 if_iwm.c
--- if_iwm.c19 Jul 2015 21:40:00 -  1.46
+++ if_iwm.c22 Sep 2015 16:52:10 -
@@ -1046,6 +1046,7 @@ iwm_reset_rx_ring(struct iwm_softc *sc, 
iwm_nic_unlock(sc);
}
ring->cur = 0;
+   memset(ring->stat, 0, sizeof(*ring->stat));
 }
 
 void

iwm(4) bssid addr fix

[Stefan Sperling]

Adrian Chadd noted that Linux iwlwifi sends the broadcast address to
the firmware when not associated, rather than zeros.

ok?

Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.46
diff -u -p -r1.46 if_iwm.c
--- if_iwm.c19 Jul 2015 21:40:00 -  1.46
+++ if_iwm.c22 Sep 2015 17:14:32 -
@@ -4718,7 +4718,7 @@ iwm_mvm_mac_ctxt_cmd_common(struct iwm_s
if (in->in_assoc) {
IEEE80211_ADDR_COPY(cmd->bssid_addr, ni->ni_bssid);
} else {
-   memset(cmd->bssid_addr, 0, sizeof(cmd->bssid_addr));
+   IEEE80211_ADDR_COPY(cmd->bssid_addr, etherbroadcastaddr);
}
iwm_mvm_ack_rates(sc, in, _ack_rates, _ack_rates);
cmd->cck_rates = htole32(cck_ack_rates);

Re: Merge rt_use counters

[Sebastian Benoit]

Claudio Jeker(cje...@diehard.n-r-g.com) on 2015.09.22 16:01:34 +0200:
> On Tue, Sep 22, 2015 at 03:14:18PM +0200, Martin Pieuchot wrote:
> > Instead of incrementing the rt_use counter when a rtalloc(9) call
> > succeeds, let's do it inside ralloc(9).
> > 
> > The route(8) regress tests will need to be updated because all the
> > paths calling rtalloc(9) do not increment rt_use.
> > 
> > This change gives us a better understanding of which routes are queried
> > and might need a cache.
> > 
> > It will also help me with upcoming counter handling for MP.
> > 
> > ok?

yes ok
 
> It kind of changes the meaning of the use counter but I think that is
> fair. In the end I think this is only used for debugging. As mentioned
> earlier I have no problems to remove this counter since I don't see a huge
> benefit having it.

regarding removing it: i havent needed it often and can do without.


> -- 
> :wq Claudio
>  
> > Index: net/pf.c
> > ===
> > RCS file: /cvs/src/sys/net/pf.c,v
> > retrieving revision 1.944
> > diff -u -p -r1.944 pf.c
> > --- net/pf.c13 Sep 2015 17:53:44 -  1.944
> > +++ net/pf.c22 Sep 2015 13:02:42 -
> > @@ -5520,7 +5520,6 @@ pf_route(struct mbuf **m, struct pf_rule
> > }
> >  
> > ifp = rt->rt_ifp;
> > -   rt->rt_use++;
> >  
> > if (rt->rt_flags & RTF_GATEWAY)
> > dst = satosin(rt->rt_gateway);
> > Index: net/route.c
> > ===
> > RCS file: /cvs/src/sys/net/route.c,v
> > retrieving revision 1.241
> > diff -u -p -r1.241 route.c
> > --- net/route.c 22 Sep 2015 10:05:00 -  1.241
> > +++ net/route.c 22 Sep 2015 13:04:07 -
> > @@ -350,12 +350,15 @@ rtalloc(struct sockaddr *dst, int flags,
> > rt0 = rt;
> > error = rtrequest1(RTM_RESOLVE, , RTP_DEFAULT,
> > , tableid);
> > -   if (error)
> > +   if (error) {
> > +   rt0->rt_use++;
> > goto miss;
> > +   }
> > /* Inform listeners of the new route */
> > rt_sendmsg(rt, RTM_ADD, tableid);
> > rtfree(rt0);
> > }
> > +   rt->rt_use++;
> > } else {
> > rtstat.rts_unreach++;
> >  miss:
> > Index: netinet/ip_icmp.c
> > ===
> > RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
> > retrieving revision 1.140
> > diff -u -p -r1.140 ip_icmp.c
> > --- netinet/ip_icmp.c   11 Sep 2015 15:12:29 -  1.140
> > +++ netinet/ip_icmp.c   22 Sep 2015 13:02:36 -
> > @@ -758,7 +758,6 @@ icmp_reflect(struct mbuf *m, struct mbuf
> > }
> >  
> > ia = ifatoia(rt->rt_ifa);
> > -   rt->rt_use++;
> > rtfree(rt);
> > }
> >  
> > Index: netinet/ip_output.c
> > ===
> > RCS file: /cvs/src/sys/netinet/ip_output.c,v
> > retrieving revision 1.298
> > diff -u -p -r1.298 ip_output.c
> > --- netinet/ip_output.c 13 Sep 2015 17:53:44 -  1.298
> > +++ netinet/ip_output.c 22 Sep 2015 13:02:33 -
> > @@ -207,7 +207,6 @@ reroute:
> > ifp = if_ref(ro->ro_rt->rt_ifp);
> > if ((mtu = ro->ro_rt->rt_rmx.rmx_mtu) == 0)
> > mtu = ifp->if_mtu;
> > -   ro->ro_rt->rt_use++;
> >  
> > if (ro->ro_rt->rt_flags & RTF_GATEWAY)
> > dst = satosin(ro->ro_rt->rt_gateway);
> > Index: netinet6/ip6_output.c
> > ===
> > RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
> > retrieving revision 1.188
> > diff -u -p -r1.188 ip6_output.c
> > --- netinet6/ip6_output.c   13 Sep 2015 13:57:07 -  1.188
> > +++ netinet6/ip6_output.c   22 Sep 2015 13:02:29 -
> > @@ -558,12 +558,6 @@ reroute:
> > *dst = dstsock;
> > }
> >  
> > -   /*
> > -* then rt (for unicast) and ifp must be non-NULL valid values.
> > -*/
> > -   if (rt)
> > -   rt->rt_use++;
> > -
> > if (rt && !IN6_IS_ADDR_MULTICAST(>ip6_dst)) {
> > if (opt && opt->ip6po_nextroute.ro_rt) {
> > /*
> > Index: netmpls/mpls_input.c
> > ===
> > RCS file: /cvs/src/sys/netmpls/mpls_input.c,v
> > retrieving revision 1.49
> > diff -u -p -r1.49 mpls_input.c
> > --- netmpls/mpls_input.c13 Sep 2015 17:53:44 -  1.49
> > +++ netmpls/mpls_input.c22 Sep 2015 13:02:23 -
> > @@ -182,7 +182,6 @@ do_v6:
> > goto done;
> > }
> >  
> > -   rt->rt_use++;
> > rt_mpls = (struct rt_mpls *)rt->rt_llinfo;
> >  
> > if (rt_mpls == NULL 

Re: bsd.port.mk.5: Remove unneeded args to Nm

[Ingo Schwarze]

Hi Michael,

Michael Reed wrote on Tue, Sep 22, 2015 at 04:24:37PM -0400:

> In bsd.port.mk.5, the use of arguments to Nm is inconsistent,
> and when they are used it's seemingly redundant; this patch
> fixes that.
> 
> I verified there's no change in the formatted manual with this:
> $ diff <{mandoc bsd.port.mk.5.orig} <{mandoc bsd.port.mk.5}

Committed, thanks.
  Ingo


> Index: bsd.port.mk.5
> ===
> RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
> retrieving revision 1.422
> diff -u -p -r1.422 bsd.port.mk.5
> --- bsd.port.mk.5 21 Sep 2015 09:06:33 -  1.422
> +++ bsd.port.mk.5 22 Sep 2015 20:15:22 -
> @@ -48,14 +48,14 @@ and they do have manpages under
>  Other
>  .Bx
>  variants, as well as older versions of
> -.Nm bsd.port.mk ,
> +.Nm ,
>  include other targets and variables.
>  Conversion methods are outlined here.
>  .Pp
>  Most variables and targets are documented, with very few exceptions.
>  .Pp
>  This documentation covers the current targets, variables and paths used by
> -.Nm bsd.port.mk .
> +.Nm .
>  There is a separate section covering the fake framework, a section
>  explaining flavors and multi-packages, and a section covering
>  the generation of package information.
> @@ -2089,7 +2089,7 @@ Set to
>  .Sq Yes
>  if platform does not support shared libraries.
>  To be tested after including
> -.Nm bsd.port.mk ,
> +.Nm ,
>  or
>  .Xr bsd.port.arch.mk 5 ,
>  if neither PFRAG.shared nor CONFIGURE_SHARED are enough.
> @@ -4155,7 +4155,7 @@ Old requirement script.
>  Was mostly unused anyway.
>  .It Pa /usr/share/mk/bsd.port.mk
>  Original location of
> -.Nm bsd.port.mk .
> +.Nm .
>  The current file lives under
>  .Pa ${PORTSDIR}/infrastructure/mk/bsd.port.mk ,
>  whereas

Re: bsd.port.mk.5: Two small tweaks

[Ingo Schwarze]

Hi Michael,

Michael Reed wrote on Tue, Sep 22, 2015 at 04:11:26PM -0400:

> I'm pretty sure the use of Nm was previously wrong,

Indeed.  The .Nm macro is only intended for command names that
occur in the NAME section of the respective manual.

> but let me know if I'm off the mark.
> 
> Index: src/share/man/man5/bsd.port.mk.5
> ===
> RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
> retrieving revision 1.422
> diff -u -p -r1.422 bsd.port.mk.5
> --- src/share/man/man5/bsd.port.mk.5  21 Sep 2015 09:06:33 -  1.422
> +++ src/share/man/man5/bsd.port.mk.5  22 Sep 2015 20:01:33 -
> @@ -1216,7 +1216,7 @@ If set to
>  every package build will verify that shared libraries are correctly
>  registered.
>  This is essentially the same as running
> -.Nm make Ar lib-depends-check
> +.Cm make lib-depends-check
>  after each package build.
>  Defaults to
>  .Sq \ ,

Fixed differently, with a .Ql in-line literal display.

> @@ -2567,7 +2567,7 @@ If set to true,
>  .Ar checksum
>  will analyze ${CHECKSUM_FILE}, and try retrieving files with the correct
>  checksum off
> -.Pa ftp.openbsd.org ,
> +.Lk ftp.openbsd.org ,
>  in the directory
>  .Pa /pub/OpenBSD/distfiles/$cipher/$value/$file .
>  .It Ev REORDER_DEPENDENCIES

Committed.

Thanks,
  Ingo

Re: Purge route entries when an address is removed

[Sebastian Benoit]

Martin Pieuchot(m...@openbsd.org) on 2015.09.13 16:08:50 +0200:
> On 13/09/15(Sun) 15:51, Alexander Bluhm wrote:
> > On Sun, Sep 13, 2015 at 11:15:50AM +0200, Martin Pieuchot wrote:
> > > This makes the kernel simpler as it no longer try to find a new ifa
> > > when a route with a stale address is being used.
> > 
> > This makes the code simpler, which is good.
> > 
> > I am still not convinced that we want to loose the feature that the
> > routes jump to another interface address.  When we have multiple
> > suiteable addresses and one gets deleted, the system can use another
> > one.
> 
> This is the price to pay for making the code simpler.  I strongly
> believe this "feature" is a side effect of history that should not
> have been added in the first place.
> 
> However I'd like to fix potential issues with this diff before committing
> it, so tests are welcome :)

Hi,

i found the time to play with your diff.

On a router (where you probably wouldn't to this operationaly) you get tons
of "route xxx vanished before delete" from bgpd and ospfd, but they continue
to work, apparently as intended.

I havent found a box with pppoe to test. it would be nice if someone could
test that.

as far as it goes, ok from me.

/Benno
 
> > The patch itself looks correct.  Just one question:
> > 
> > > @@ -850,22 +850,10 @@ rtrequest1(int req, struct rt_addrinfo *
> > >   return (EINVAL);
> > >   if ((rt->rt_flags & RTF_CLONING) == 0)
> > >   return (EINVAL);
> > > - if (rt->rt_ifa->ifa_ifp) {
> > > - info->rti_ifa = rt->rt_ifa;
> > > - } else {
> > > - /*
> > > -  * The address of the cloning route is not longer
> > > -  * configured on an interface, but its descriptor
> > > -  * is still there because of reference counting.
> > > -  *
> > > -  * Try to find a similar active address and use
> > > -  * it for the cloned route.  The cloning route
> > > -  * will get the new address and interface later.
> > > -  */
> > > - info->rti_ifa = NULL;
> > > - info->rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
> > > - }
> > > + if (rt->rt_ifa->ifa_ifp == NULL)
> > > + return (EAGAIN);
> > 
> > Why return EAGAIN here?  Should it be EINVAL like in the other
> > cases?  Can this happen at all?
> 
> This should never happen but I'd like to take a safe approach and be
> able to differentiate the error code if this still happens.  I'd
> happily turn this into a KASSERT() but not right now.

+1

bsd.port.mk.5: Two small tweaks

[Michael Reed]

I'm pretty sure the use of Nm was previously wrong, but let me
know if I'm off the mark.

Index: src/share/man/man5/bsd.port.mk.5
===
RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
retrieving revision 1.422
diff -u -p -r1.422 bsd.port.mk.5
--- src/share/man/man5/bsd.port.mk.521 Sep 2015 09:06:33 -  1.422
+++ src/share/man/man5/bsd.port.mk.522 Sep 2015 20:01:33 -
@@ -1216,7 +1216,7 @@ If set to
 every package build will verify that shared libraries are correctly
 registered.
 This is essentially the same as running
-.Nm make Ar lib-depends-check
+.Cm make lib-depends-check
 after each package build.
 Defaults to
 .Sq \ ,
@@ -2567,7 +2567,7 @@ If set to true,
 .Ar checksum
 will analyze ${CHECKSUM_FILE}, and try retrieving files with the correct
 checksum off
-.Pa ftp.openbsd.org ,
+.Lk ftp.openbsd.org ,
 in the directory
 .Pa /pub/OpenBSD/distfiles/$cipher/$value/$file .
 .It Ev REORDER_DEPENDENCIES

bsd.port.mk.5: Remove unneeded args to Nm

[Michael Reed]

In bsd.port.mk.5, the use of arguments to Nm is inconsistent,
and when they are used it's seemingly redundant; this patch
fixes that.

I verified there's no change in the formatted manual with this:
$ diff <{mandoc bsd.port.mk.5.orig} <{mandoc bsd.port.mk.5}

Finally, if anyone is wondering, line 1219 is dealt with in
the other diff I just sent out.


Index: bsd.port.mk.5
===
RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
retrieving revision 1.422
diff -u -p -r1.422 bsd.port.mk.5
--- bsd.port.mk.5   21 Sep 2015 09:06:33 -  1.422
+++ bsd.port.mk.5   22 Sep 2015 20:15:22 -
@@ -48,14 +48,14 @@ and they do have manpages under
 Other
 .Bx
 variants, as well as older versions of
-.Nm bsd.port.mk ,
+.Nm ,
 include other targets and variables.
 Conversion methods are outlined here.
 .Pp
 Most variables and targets are documented, with very few exceptions.
 .Pp
 This documentation covers the current targets, variables and paths used by
-.Nm bsd.port.mk .
+.Nm .
 There is a separate section covering the fake framework, a section
 explaining flavors and multi-packages, and a section covering
 the generation of package information.
@@ -2089,7 +2089,7 @@ Set to
 .Sq Yes
 if platform does not support shared libraries.
 To be tested after including
-.Nm bsd.port.mk ,
+.Nm ,
 or
 .Xr bsd.port.arch.mk 5 ,
 if neither PFRAG.shared nor CONFIGURE_SHARED are enough.
@@ -4155,7 +4155,7 @@ Old requirement script.
 Was mostly unused anyway.
 .It Pa /usr/share/mk/bsd.port.mk
 Original location of
-.Nm bsd.port.mk .
+.Nm .
 The current file lives under
 .Pa ${PORTSDIR}/infrastructure/mk/bsd.port.mk ,
 whereas

Re: Purge route entries when an address is removed

[Stuart Henderson]

On 2015/09/22 23:07, Sebastian Benoit wrote:
> Martin Pieuchot(m...@openbsd.org) on 2015.09.13 16:08:50 +0200:
> > On 13/09/15(Sun) 15:51, Alexander Bluhm wrote:
> > > On Sun, Sep 13, 2015 at 11:15:50AM +0200, Martin Pieuchot wrote:
> > > > This makes the kernel simpler as it no longer try to find a new ifa
> > > > when a route with a stale address is being used.
> > > 
> > > This makes the code simpler, which is good.
> > > 
> > > I am still not convinced that we want to loose the feature that the
> > > routes jump to another interface address.  When we have multiple
> > > suiteable addresses and one gets deleted, the system can use another
> > > one.
> > 
> > This is the price to pay for making the code simpler.  I strongly
> > believe this "feature" is a side effect of history that should not
> > have been added in the first place.
> > 
> > However I'd like to fix potential issues with this diff before committing
> > it, so tests are welcome :)
> 
> Hi,
> 
> i found the time to play with your diff.
> 
> On a router (where you probably wouldn't to this operationaly) you get tons
> of "route xxx vanished before delete" from bgpd and ospfd, but they continue
> to work, apparently as intended.
> 
> I havent found a box with pppoe to test. it would be nice if someone could
> test that.

Preferably someone with a relatively unstable pppoe. Maybe one of those
providers which kills sessions every day and forces an address change..

Re: panic with latest ugen.c and pcsc-lite

[Martin Pieuchot]

On 17/09/15(Thu) 02:34, Grant Czajkowski wrote:
> On Tue, Sep 15, 2015 at 12:04:35PM +0200, Martin Pieuchot wrote:
> > On 15/09/15(Tue) 04:50, Grant Czajkowski wrote:
> > > On Fri, Sep 11, 2015 at 02:41:04AM -0600, David Coppa wrote:
> > > > 
> > > > Hi!
> > > > 
> > > > Repeatedly hit the panic below with latest ugen.c code (v 1.88) and
> > > > pcsc-lite.
> > > 
> > > Thanks for the report.  Could you please try this patch?
> > 
> > ok mpi@
> > 
> > Grant what do you you think about doing an audit of the tree to see if
> > we're missing this check in other drivers?  I might be interesting to
> > search bugs@ archives for similar reports.
> 
> These are similar instances I found under /sys/dev/usb/*.

This is scary.  Don't you think it's possible to handle this error code
in the stack?  I think the "only" complex driver in this regard is
umass(4).  By looking at its code I found this funky comment:

/*
 [...]
 * The control pipe has already been unstalled by the
 * USB stack.

Having a single place where pipes are unstalled would make thing easier
for driver authors and simplify our like to test/fix this code.

By looking at the code around your diff I also found a lot of places where
drivers check for USBD_NOT_STARTED.  This seems also wrong,
usb_transfer_complete() is only called when a transfer is aborted in which
case it has an USBD_TIMEOUT or USBD_CANCELLED status or when it finishes.

> 
> Index: ubsa.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ubsa.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 ubsa.c
> --- ubsa.c14 Mar 2015 03:38:49 -  1.64
> +++ ubsa.c17 Sep 2015 02:22:47 -
> @@ -651,7 +651,8 @@ ubsa_intr(struct usbd_xfer *xfer, void *
>  
>   DPRINTF(("%s: ubsa_intr: abnormal status: %s\n",
>   sc->sc_dev.dv_xname, usbd_errstr(status)));
> - usbd_clear_endpoint_stall_async(sc->sc_intr_pipe);
> + if (status == USBD_STALLED)
> + usbd_clear_endpoint_stall_async(sc->sc_intr_pipe);
>   return;
>   }
>  
> Index: uchcom.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uchcom.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 uchcom.c
> --- uchcom.c  14 Apr 2015 07:57:33 -  1.24
> +++ uchcom.c  17 Sep 2015 02:22:47 -
> @@ -931,7 +931,8 @@ uchcom_intr(struct usbd_xfer *xfer, void
>  
>   DPRINTF(("%s: abnormal status: %s\n",
>sc->sc_dev.dv_xname, usbd_errstr(status)));
> - usbd_clear_endpoint_stall_async(sc->sc_intr_pipe);
> + if (status == USBD_STALLED)
> + usbd_clear_endpoint_stall_async(sc->sc_intr_pipe);
>   return;
>   }
>   DPRINTF(("%s: intr: 0x%02X 0x%02X 0x%02X 0x%02X "
> Index: ucom.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ucom.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 ucom.c
> --- ucom.c14 Mar 2015 03:38:49 -  1.65
> +++ ucom.c17 Sep 2015 02:22:47 -
> @@ -1060,7 +1060,9 @@ ucomwritecb(struct usbd_xfer *xfer, void
>  
>   if (sc->sc_bulkin_pipe != NULL) {
>   if (status) {
> - usbd_clear_endpoint_stall_async(sc->sc_bulkin_pipe);
> + if (status == USBD_STALLED)
> + usbd_clear_endpoint_stall_async(
> + sc->sc_bulkin_pipe);
>   /* XXX we should restart after some delay. */
>   goto error;
>   }
> @@ -1145,7 +1147,9 @@ ucomreadcb(struct usbd_xfer *xfer, void 
>  
>   if (status) {
>   if (sc->sc_bulkin_pipe != NULL) {
> - usbd_clear_endpoint_stall_async(sc->sc_bulkin_pipe);
> + if (status == USBD_STALLED)
> + usbd_clear_endpoint_stall_async(
> + sc->sc_bulkin_pipe);
>   /* XXX we should restart after some delay. */
>   return;
>   }
> Index: ugen.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ugen.c,v
> retrieving revision 1.89
> diff -u -p -r1.89 ugen.c
> --- ugen.c15 Sep 2015 13:37:44 -  1.89
> +++ ugen.c17 Sep 2015 02:22:47 -
> @@ -721,7 +721,9 @@ ugen_do_write(struct ugen_softc *sc, int
>   flags, sce->timeout, NULL);
>   err = usbd_transfer(xfer);
>   if (err) {
> - usbd_clear_endpoint_stall(sce->pipeh);
> + if (err == USBD_STALLED)
> + usbd_clear_endpoint_stall(sce->pipeh);
> +
>   if (err == USBD_INTERRUPTED)
>  

Re: Merge rt_use counters

[Claudio Jeker]

On Tue, Sep 22, 2015 at 03:14:18PM +0200, Martin Pieuchot wrote:
> Instead of incrementing the rt_use counter when a rtalloc(9) call
> succeeds, let's do it inside ralloc(9).
> 
> The route(8) regress tests will need to be updated because all the
> paths calling rtalloc(9) do not increment rt_use.
> 
> This change gives us a better understanding of which routes are queried
> and might need a cache.
> 
> It will also help me with upcoming counter handling for MP.
> 
> ok?

It kind of changes the meaning of the use counter but I think that is
fair. In the end I think this is only used for debugging. As mentioned
earlier I have no problems to remove this counter since I don't see a huge
benefit having it.

-- 
:wq Claudio
 
> Index: net/pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.944
> diff -u -p -r1.944 pf.c
> --- net/pf.c  13 Sep 2015 17:53:44 -  1.944
> +++ net/pf.c  22 Sep 2015 13:02:42 -
> @@ -5520,7 +5520,6 @@ pf_route(struct mbuf **m, struct pf_rule
>   }
>  
>   ifp = rt->rt_ifp;
> - rt->rt_use++;
>  
>   if (rt->rt_flags & RTF_GATEWAY)
>   dst = satosin(rt->rt_gateway);
> Index: net/route.c
> ===
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.241
> diff -u -p -r1.241 route.c
> --- net/route.c   22 Sep 2015 10:05:00 -  1.241
> +++ net/route.c   22 Sep 2015 13:04:07 -
> @@ -350,12 +350,15 @@ rtalloc(struct sockaddr *dst, int flags,
>   rt0 = rt;
>   error = rtrequest1(RTM_RESOLVE, , RTP_DEFAULT,
>   , tableid);
> - if (error)
> + if (error) {
> + rt0->rt_use++;
>   goto miss;
> + }
>   /* Inform listeners of the new route */
>   rt_sendmsg(rt, RTM_ADD, tableid);
>   rtfree(rt0);
>   }
> + rt->rt_use++;
>   } else {
>   rtstat.rts_unreach++;
>  miss:
> Index: netinet/ip_icmp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
> retrieving revision 1.140
> diff -u -p -r1.140 ip_icmp.c
> --- netinet/ip_icmp.c 11 Sep 2015 15:12:29 -  1.140
> +++ netinet/ip_icmp.c 22 Sep 2015 13:02:36 -
> @@ -758,7 +758,6 @@ icmp_reflect(struct mbuf *m, struct mbuf
>   }
>  
>   ia = ifatoia(rt->rt_ifa);
> - rt->rt_use++;
>   rtfree(rt);
>   }
>  
> Index: netinet/ip_output.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.298
> diff -u -p -r1.298 ip_output.c
> --- netinet/ip_output.c   13 Sep 2015 17:53:44 -  1.298
> +++ netinet/ip_output.c   22 Sep 2015 13:02:33 -
> @@ -207,7 +207,6 @@ reroute:
>   ifp = if_ref(ro->ro_rt->rt_ifp);
>   if ((mtu = ro->ro_rt->rt_rmx.rmx_mtu) == 0)
>   mtu = ifp->if_mtu;
> - ro->ro_rt->rt_use++;
>  
>   if (ro->ro_rt->rt_flags & RTF_GATEWAY)
>   dst = satosin(ro->ro_rt->rt_gateway);
> Index: netinet6/ip6_output.c
> ===
> RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
> retrieving revision 1.188
> diff -u -p -r1.188 ip6_output.c
> --- netinet6/ip6_output.c 13 Sep 2015 13:57:07 -  1.188
> +++ netinet6/ip6_output.c 22 Sep 2015 13:02:29 -
> @@ -558,12 +558,6 @@ reroute:
>   *dst = dstsock;
>   }
>  
> - /*
> -  * then rt (for unicast) and ifp must be non-NULL valid values.
> -  */
> - if (rt)
> - rt->rt_use++;
> -
>   if (rt && !IN6_IS_ADDR_MULTICAST(>ip6_dst)) {
>   if (opt && opt->ip6po_nextroute.ro_rt) {
>   /*
> Index: netmpls/mpls_input.c
> ===
> RCS file: /cvs/src/sys/netmpls/mpls_input.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 mpls_input.c
> --- netmpls/mpls_input.c  13 Sep 2015 17:53:44 -  1.49
> +++ netmpls/mpls_input.c  22 Sep 2015 13:02:23 -
> @@ -182,7 +182,6 @@ do_v6:
>   goto done;
>   }
>  
> - rt->rt_use++;
>   rt_mpls = (struct rt_mpls *)rt->rt_llinfo;
>  
>   if (rt_mpls == NULL || (rt->rt_flags & RTF_MPLS) == 0) {
> @@ -449,7 +448,6 @@ mpls_do_error(struct mbuf *m, int type, 
>   m_freem(m);
>   return (NULL);
>   }
> - rt->rt_use++;
>   KERNEL_LOCK();
>   rtfree(rt);
>   if 

Merge rt_use counters

[Martin Pieuchot]

Instead of incrementing the rt_use counter when a rtalloc(9) call
succeeds, let's do it inside ralloc(9).

The route(8) regress tests will need to be updated because all the
paths calling rtalloc(9) do not increment rt_use.

This change gives us a better understanding of which routes are queried
and might need a cache.

It will also help me with upcoming counter handling for MP.

ok?

Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.944
diff -u -p -r1.944 pf.c
--- net/pf.c13 Sep 2015 17:53:44 -  1.944
+++ net/pf.c22 Sep 2015 13:02:42 -
@@ -5520,7 +5520,6 @@ pf_route(struct mbuf **m, struct pf_rule
}
 
ifp = rt->rt_ifp;
-   rt->rt_use++;
 
if (rt->rt_flags & RTF_GATEWAY)
dst = satosin(rt->rt_gateway);
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.241
diff -u -p -r1.241 route.c
--- net/route.c 22 Sep 2015 10:05:00 -  1.241
+++ net/route.c 22 Sep 2015 13:04:07 -
@@ -350,12 +350,15 @@ rtalloc(struct sockaddr *dst, int flags,
rt0 = rt;
error = rtrequest1(RTM_RESOLVE, , RTP_DEFAULT,
, tableid);
-   if (error)
+   if (error) {
+   rt0->rt_use++;
goto miss;
+   }
/* Inform listeners of the new route */
rt_sendmsg(rt, RTM_ADD, tableid);
rtfree(rt0);
}
+   rt->rt_use++;
} else {
rtstat.rts_unreach++;
 miss:
Index: netinet/ip_icmp.c
===
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.140
diff -u -p -r1.140 ip_icmp.c
--- netinet/ip_icmp.c   11 Sep 2015 15:12:29 -  1.140
+++ netinet/ip_icmp.c   22 Sep 2015 13:02:36 -
@@ -758,7 +758,6 @@ icmp_reflect(struct mbuf *m, struct mbuf
}
 
ia = ifatoia(rt->rt_ifa);
-   rt->rt_use++;
rtfree(rt);
}
 
Index: netinet/ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.298
diff -u -p -r1.298 ip_output.c
--- netinet/ip_output.c 13 Sep 2015 17:53:44 -  1.298
+++ netinet/ip_output.c 22 Sep 2015 13:02:33 -
@@ -207,7 +207,6 @@ reroute:
ifp = if_ref(ro->ro_rt->rt_ifp);
if ((mtu = ro->ro_rt->rt_rmx.rmx_mtu) == 0)
mtu = ifp->if_mtu;
-   ro->ro_rt->rt_use++;
 
if (ro->ro_rt->rt_flags & RTF_GATEWAY)
dst = satosin(ro->ro_rt->rt_gateway);
Index: netinet6/ip6_output.c
===
RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.188
diff -u -p -r1.188 ip6_output.c
--- netinet6/ip6_output.c   13 Sep 2015 13:57:07 -  1.188
+++ netinet6/ip6_output.c   22 Sep 2015 13:02:29 -
@@ -558,12 +558,6 @@ reroute:
*dst = dstsock;
}
 
-   /*
-* then rt (for unicast) and ifp must be non-NULL valid values.
-*/
-   if (rt)
-   rt->rt_use++;
-
if (rt && !IN6_IS_ADDR_MULTICAST(>ip6_dst)) {
if (opt && opt->ip6po_nextroute.ro_rt) {
/*
Index: netmpls/mpls_input.c
===
RCS file: /cvs/src/sys/netmpls/mpls_input.c,v
retrieving revision 1.49
diff -u -p -r1.49 mpls_input.c
--- netmpls/mpls_input.c13 Sep 2015 17:53:44 -  1.49
+++ netmpls/mpls_input.c22 Sep 2015 13:02:23 -
@@ -182,7 +182,6 @@ do_v6:
goto done;
}
 
-   rt->rt_use++;
rt_mpls = (struct rt_mpls *)rt->rt_llinfo;
 
if (rt_mpls == NULL || (rt->rt_flags & RTF_MPLS) == 0) {
@@ -449,7 +448,6 @@ mpls_do_error(struct mbuf *m, int type, 
m_freem(m);
return (NULL);
}
-   rt->rt_use++;
KERNEL_LOCK();
rtfree(rt);
if (icmp_reflect(m, NULL, ia)) {
Index: netmpls/mpls_output.c
===
RCS file: /cvs/src/sys/netmpls/mpls_output.c,v
retrieving revision 1.24
diff -u -p -r1.24 mpls_output.c
--- netmpls/mpls_output.c   13 Sep 2015 17:53:44 -  1.24
+++ netmpls/mpls_output.c   22 Sep 2015 13:02:13 -
@@ -133,7 +133,6 @@ mpls_output(struct ifnet *ifp0, struct m
error = EHOSTUNREACH;
 

Re: lighter sleep

[Benny Lofgren]

On 2015-09-21 16:45, Mark Kettenis wrote:
>> From: Christian Weisgerber 
>> Date: Mon, 21 Sep 2015 14:29:03 + (UTC)
>>
>> On 2015-09-21, Stefan Sperling  wrote:
>>
> You could argue that the thousands separator should be supported though:
> 
>   $ sleep 1.000.000
> 
> if your locale is something vaguely european, and
> 
>   # sleep 1,000,000
> 
> for the north-americans.
> 
> But let's not go there...

In my locale (Sweden), the thousands separator is space... try to
meaningfully parse sleep 1 000 000 without all of a sudden introducing
mandatory quoting. :-) So no, let's not go there...


Like many others I've seen plenty of trainwrecks caused by mindless
internationalization in other operating systems, and how it wreaks havoc
in scripts and interpretation of piped data. It is NOT pretty.

The stance that the base system should only speak English and use
time-tried and known formats is IMO the correct one.


With that said, we do live in an international world and I was alarmed
about something Stefan wrote earlier in this thread:

> When we still had latin1 etc. it was possible in theory that values between
> 128 and 255 represented a digit. But now, isdigit() does the same regardless
> of locale setting (C or UTF-8) since it cannot be given a multibyte sequence,
> i.e. it will not deal with character values above 127.

Maybe I'm the only one that this is news to (and I admit I obviosly
haven't followed tech@ and misc@ closely enough lately), but am I
interpreting this correctly in understanding that 8-bit locales except
for the "C" locale (which is effectively a 7-bit locale) are gone?


That would be extremely unfortunate, since UTF-8 (even if it was fully
implemented in OpenBSD which of course it isn't) is *not* the sole
answer to the world's localization problems. At least not in the
forseeable future.

For example, I still have plenty of systems running that absolutely
relies on a working ISO 8859-1 environment, and not just in the sense
that it is 8-bit transparent but in understanding collation orders,
proper case folding and functioning of the is...() macros/functions etc.


I tried to google for more on this issue, but came up pretty thin. Has
this been discussed publicly somewhere? If this is due to shortage of
manpower or interest or something like that, maybe we can do something
about that.

I'm sure it isn't still a matter of lack of knowledge of the problems
facing us non-native English speakers - since I know OpenBSD developers
come from all corners of the world - so please someone tell me I've
misunderstood, or that it's just a bad dream and I'll wake up soon. :-)


* * *

Are there more people like me who really could use more complete i18n
support in OpenBSD (multibyte as well as legacy 8-bit) and are willing
to make efforts into implementing it?

Is there a coordinated effort somewhere? If not, let's discuss it!
Offlist is fine of course if it isn't of general interest (but I think
it is). And by "implementing", of course I mean doing it right, the
OpenBSD way.

If there is interest but no coordination, I can start it off and try to
make an inventory of the current state of implementation and what
efforts may already be ongoing (or dormant or abandoned, due to lack of
interest/time/manpower/resources/pizza/beer/hikes).

What say ye?

* * *


Regards,
/Benny

Re: lighter sleep

[Brian Conway]

I recall this discussion on UTF-8 and locales:

http://undeadly.org/cgi?action=article=20150722182236

I imagine there was more elsewhere that I didn't see.

Brian

On Sep 22, 2015 7:07 AM, "Benny Lofgren"  wrote:

> On 2015-09-21 16:45, Mark Kettenis wrote:
> >> From: Christian Weisgerber 
> >> Date: Mon, 21 Sep 2015 14:29:03 + (UTC)
> >>
> >> On 2015-09-21, Stefan Sperling  wrote:
> >>
> > You could argue that the thousands separator should be supported though:
> >
> >   $ sleep 1.000.000
> >
> > if your locale is something vaguely european, and
> >
> >   # sleep 1,000,000
> >
> > for the north-americans.
> >
> > But let's not go there...
>
> In my locale (Sweden), the thousands separator is space... try to
> meaningfully parse sleep 1 000 000 without all of a sudden introducing
> mandatory quoting. :-) So no, let's not go there...
>
>
> Like many others I've seen plenty of trainwrecks caused by mindless
> internationalization in other operating systems, and how it wreaks havoc
> in scripts and interpretation of piped data. It is NOT pretty.
>
> The stance that the base system should only speak English and use
> time-tried and known formats is IMO the correct one.
>
>
> With that said, we do live in an international world and I was alarmed
> about something Stefan wrote earlier in this thread:
>
> > When we still had latin1 etc. it was possible in theory that values
> between
> > 128 and 255 represented a digit. But now, isdigit() does the same
> regardless
> > of locale setting (C or UTF-8) since it cannot be given a multibyte
> sequence,
> > i.e. it will not deal with character values above 127.
>
> Maybe I'm the only one that this is news to (and I admit I obviosly
> haven't followed tech@ and misc@ closely enough lately), but am I
> interpreting this correctly in understanding that 8-bit locales except
> for the "C" locale (which is effectively a 7-bit locale) are gone?
>
>
> That would be extremely unfortunate, since UTF-8 (even if it was fully
> implemented in OpenBSD which of course it isn't) is *not* the sole
> answer to the world's localization problems. At least not in the
> forseeable future.
>
> For example, I still have plenty of systems running that absolutely
> relies on a working ISO 8859-1 environment, and not just in the sense
> that it is 8-bit transparent but in understanding collation orders,
> proper case folding and functioning of the is...() macros/functions etc.
>
>
> I tried to google for more on this issue, but came up pretty thin. Has
> this been discussed publicly somewhere? If this is due to shortage of
> manpower or interest or something like that, maybe we can do something
> about that.
>
> I'm sure it isn't still a matter of lack of knowledge of the problems
> facing us non-native English speakers - since I know OpenBSD developers
> come from all corners of the world - so please someone tell me I've
> misunderstood, or that it's just a bad dream and I'll wake up soon. :-)
>
>
> * * *
>
> Are there more people like me who really could use more complete i18n
> support in OpenBSD (multibyte as well as legacy 8-bit) and are willing
> to make efforts into implementing it?
>
> Is there a coordinated effort somewhere? If not, let's discuss it!
> Offlist is fine of course if it isn't of general interest (but I think
> it is). And by "implementing", of course I mean doing it right, the
> OpenBSD way.
>
> If there is interest but no coordination, I can start it off and try to
> make an inventory of the current state of implementation and what
> efforts may already be ongoing (or dormant or abandoned, due to lack of
> interest/time/manpower/resources/pizza/beer/hikes).
>
> What say ye?
>
> * * *
>
>
> Regards,
> /Benny
>
>

Re: poll magic for pflogd

[Todd C. Miller]

On Mon, 21 Sep 2015 20:13:05 -0400, "Ted Unangst" wrote:

> We can put a "long" poll() in front of pcap to wait until there are packets
> (maybe never if you aren't using pf logging), and then let the timeout work
> it's magic.

This:

> + if (poll(, 1, INFTIM) == 1) {

will also match POLLHUP and POLLERR which doesn't appear to be what
you want.  You need to explicitly check for pfd.revents & POLLIN.

 - todd

Re: poll magic for pflogd

[Ted Unangst]

Todd C. Miller wrote:
> On Mon, 21 Sep 2015 20:13:05 -0400, "Ted Unangst" wrote:
> 
> > We can put a "long" poll() in front of pcap to wait until there are packets
> > (maybe never if you aren't using pf logging), and then let the timeout work
> > it's magic.
> 
> This:
> 
> > +   if (poll(, 1, INFTIM) == 1) {
> 
> will also match POLLHUP and POLLERR which doesn't appear to be what
> you want.  You need to explicitly check for pfd.revents & POLLIN.

poll, you sneaky bastard. I wonder if this program could also benefit from
handling signals via kevent, but that's a bigger change.