Re: Does grep need a version?

[Bob Beck]

Since I can't think of a sane reason to check this and care in any
script, yes, kill it.
ok beck@

On Mon, Dec 7, 2015 at 10:06 PM, Michael McConville  wrote:
> It's been 0.9 since the original import in 2003...
>
>
> Index: grep.1
> ===
> RCS file: /cvs/src/usr.bin/grep/grep.1,v
> retrieving revision 1.43
> diff -u -p -r1.43 grep.1
> --- grep.1  13 Jan 2015 04:45:34 -  1.43
> +++ grep.1  8 Dec 2015 04:59:35 -
> @@ -244,9 +244,6 @@ Nonexistent and unreadable files are ign
>  (i.e. their error messages are suppressed).
>  .It Fl U
>  Search binary files, but do not attempt to print them.
> -.It Fl V
> -Display version information.
> -All other options are ignored.
>  .It Fl v
>  Selected lines are those
>  .Em not
> Index: grep.c
> ===
> RCS file: /cvs/src/usr.bin/grep/grep.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 grep.c
> --- grep.c  28 Nov 2015 01:17:12 -  1.55
> +++ grep.c  8 Dec 2015 04:59:35 -
> @@ -137,7 +137,6 @@ static const struct option long_options[
> {"basic-regexp",no_argument,NULL, 'G'},
> {"with-filename",   no_argument,NULL, 'H'},
> {"binary",  no_argument,NULL, 'U'},
> -   {"version", no_argument,NULL, 'V'},
> {"text",no_argument,NULL, 'a'},
> {"byte-offset", no_argument,NULL, 'b'},
> {"count",   no_argument,NULL, 'c'},
> @@ -328,10 +327,6 @@ main(int argc, char *argv[])
> break;
> case 'U':
> binbehave = BIN_FILE_BIN;
> -   break;
> -   case 'V':
> -   fprintf(stderr, "grep version %u.%u\n", VER_MAJ, 
> VER_MIN);
> -   exit(0);
> break;
>  #ifndef NOZ
> case 'Z':
> Index: grep.h
> ===
> RCS file: /cvs/src/usr.bin/grep/grep.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 grep.h
> --- grep.h  7 Dec 2015 18:50:06 -   1.23
> +++ grep.h  8 Dec 2015 04:59:35 -
> @@ -34,9 +34,6 @@
>  #include 
>  #include 
>
> -#define VER_MAJ 0
> -#define VER_MIN 9
> -
>  #define BIN_FILE_BIN   0
>  #define BIN_FILE_SKIP  1
>  #define BIN_FILE_TEXT  2
>

Re: Does grep need a version?

[Michael McConville]

Bob Beck wrote:
> Since I can't think of a sane reason to check this and care in any
> script, yes, kill it.
> ok beck@

Theo pointed out that there may be ports that check the version. I was
considering that too. However, we haven't bumped from 0.9 since 2003.
ggrep is now on 2.22, so I have hope that this isn't the case anymore.

Re: Does grep need a version?

[Theo de Raadt]

> Bob Beck wrote:
> > Since I can't think of a sane reason to check this and care in any
> > script, yes, kill it.
> > ok beck@
> 
> Theo pointed out that there may be ports that check the version. I was
> considering that too. However, we haven't bumped from 0.9 since 2003.
> ggrep is now on 2.22, so I have hope that this isn't the case anymore.

I was only pointing out something in ports may want -V to do something,
rather than error out poorly.  That's all.

As to what version it spits out, nothing will care.  0.9, 3.14, 999,
nothing will care.

Re: ksh(1): utf8 in emacs editing mode

[Sebastien Marie]

On Tue, Dec 08, 2015 at 01:19:35AM +0100, Ingo Schwarze wrote:
> Hi,
> 
> i'd like to propose a simplified version of this patch Frederic Nowak
> posted a few weeks ago for commit.  Our experience is probably
> not yet sufficient to develop a full-blown solution for all UTF-8
> problems in ksh(1), but this is very non-intrusive and makes the
> following commands better without breaking anything:
> 
>  * Ctrl-b, Ctrl-xd, Esc-[D, Esc-OD (backward-char)
>  * Ctrl-f, Ctrl-xc, Esc-[C, Esc-OC (forward-char)
>  * Ctrl-h, Ctrl-? (delete-char-backward)
>  * Esc-[3~ (delete-char-forward)
>  * Ctrl-k (kill-to-eol)
> 

Hi Ingo,

While testing ksh, I found the following problem, that I will try to
describe. But I dunno if it is just a case no-managed by your patch. It
looks like a problem in "inserting a UTF-8 char inside the line (opposed
to 'at end of line')".

Else your patch is pretty readable, and it would be a good first step.

# type echo aàa
$ echo aàa
  ^ cursor here

# move two char backward (using arrow or using Ctrl-b)
$ echo aàa
^ cursor here

# add 'é' char (UTF-8) (there are no problem with non UTF-8 char like 'b')
# => visual problem starts
$ echo àaa
   ^ cursor here

# hit enter
$ echo àaa
aéàa

###
another way (maybe more simple):

# type echo abc
$ echo abc
  ^ cursor here
# go start-of-line with Ctrl+A
$ echo abc
  ^ cursor here
# insert é UTF-char
$echo abcc
 ^ cursor here

in the same manner, it is just a "visual effect", as the line is really
"éecho abc" (tested with cut/paste: Ctrl+U Ctrl+Y)

Thanks
-- 
Sebastien Marie

Re: taskctx and revisiting if_start serialisation

[David Gwynne]

> On 7 Dec 2015, at 11:34 PM, Martin Pieuchot  wrote:
> 
> On 07/12/15(Mon) 19:37, David Gwynne wrote:
>> On Sun, Dec 06, 2015 at 03:51:26PM +0100, Martin Pieuchot wrote:
>>> On 06/12/15(Sun) 14:00, David Gwynne wrote:
 [...]
 the idea is you have a taskctx, which represents a serialising
 context for tasks. tasks are submitted to the taskctx, and the code
 will try to run the tasks immediately rather than defer them to a
 thread. if there is contention on the context, the contending cpu
 yields after queueing the task because the other cpu is responsible
 for running all pending tasks to completion.
 
 it also simplifies the barrier operations a lot.
 
 the diff below implements a generic taskctx framework, and cuts the
 mpsafe if_start() implementation over to it.
>>> 
>>> I'm not sure this should be implemented as a generic framework.  I'd
>>> suggest to keep it specific to if_start().  As you say above the least
>>> worst mechanism we have is currently taskqs, but maybe this could/will
>>> change?
>> 
>> im in two minds about where the ctx code should sit. i obviously
>> lean toward keeping it with taskqs, but i also cant come up with
>> another use case for it. i also havent tried very hard to think of
>> one.
> 
> Even if you have another use case for it I find this generic framework
> confusing.  It is build on top of taskq, but it's not part of the taskq
> API.  So putting the documentation in the same manual just confuse me.
> Your task API is simple and people use it easily.  I think it should
> stay like that.
> 
> Now we need a way to serialize start and txeof and you came up with a
> solution...
> 
>> are you asking if taskqs will get better, or if something better
>> than taskqs will come along?
> 
> ...I'm just saying that the "implementation" of your serialisation
> mechanism should not matter.  I'm fine with having a context for tasks
> but I'd suggest that this should be abstracted in a simple API instead
> of offering a framework to build upon.

ill tweak it in the morning.

> Could you make it such that you don't need a task in myx_softc?

sure.

the end extreme would be having an if_txeof() function in if.c that wraps a lot 
of this up.

however, i dont want to dictate that start and txeof have to be serialised. if 
you're careful it is possible to operate both the start and txeof side of a 
ring concurrently. the only coordination necessary is around the oactive 
handling.

if some drivers want to completely serialise both start and txeof they should 
be free to do, but if they want to make it as concurrent as possible then they 
should be able to what myx is doing and only coordinate oactive.

ill tinker with it tomorrow though.

> 
>>> I cannot understand what's happening by reading the myx(4) chunk itself.
>>> So I believe the interface needs to be polished.  Would it be possible
>>> to implement this serialization without introducing if_restart()?
>> 
>> that is by far my least favourite bit of this diff. in my defense,
>> i wrote it while my mother was trying to have a conversation with
>> me, so it didn't get my full attention. ive torn if_restart out and
>> implemented it completely in myx below.
>> 
 myx is also changed to only clr oactive from within the taskctx
 serialiser, thereby avoiding the race, but keeps the bulk of txeof
 outside the serialiser so it can run concurrently with start.
 
 other nics are free to serialise start and txeof within the
 ifq_serializer if they want, or not, it is up to them.
 
 thoughts? tests? opinions on messy .h files?
>>> 
>>> It appears to me that you have unrelated changes: if_enter/if_leave.
>> 
>> oops, yes. i suck at juggling diffs.  maybe we should call if.c
>> full and split it up ;)
> 
> I think we should start with that because the I'm afraid we're already
> cooking spaghetti.
> 
> My question is the barrier and mpsafe code is related to a queue or to
> an interface?  In other words does it make sense to serialize on a
> context in the sending queue?  What about multiple queues?

it's per queue.

ifnet and ifqueue are mixed together atm cos we've long had a 1:1 mapping 
between them in our stack. right now im trying to make the code mpsafe. keeping 
the api change simple means passing an ifp when an ifq is technically more 
correctly. making it work for multiple queues is something to do properly in 
the future.

ill try to be more careful about this now to avoid confusion though by at least 
keeping the data structures grouped properly.

dlg

Re: ugenctl for attaching USB devices to ugen instead of their specific driver

[Martin Pieuchot]

On 07/12/15(Mon) 22:18, Xiaofan Chen wrote:
> On Mon, Dec 7, 2015 at 7:42 PM, Martin Pieuchot  wrote:
> > I'd really prefer a solution that doesn't need any button.  In other
> > words to always be able to use your device from userland if the kernel
> > is not using it.
> >
> > One way would be to always attach a ugen(4) driver to every USB device,
> > I think that's what FreeBSD does.  The other way would be to use the
> > usb(4) bus interface to talk to your device.  In both cases some work
> > is needed to guarantee that an endpoint is only driven by one driver at
> > a time.  Right now this is done by attaching only one driver.
> 
> It will be good if this limitation is removed (I know the FreeBSD approach
> but FreeBSD libusb is not under libusb project due to license concerns)
> and then libusb will be more useful. Or a method like Linux where the
> kernel driver can be dynamically detached (so that usbfs generic driver
> is attached and libusb uses usbfs under Linux).

I agree, but somebody has to do the work 8)

Starting with a HID device is the way to go.  The first step should be to
deprecate the libusbhid(3) and use the libusb instead.  Userland should
stop using it, then we disable it in the kernel making every HID device
attach as ugen(4) and switch the ports currently using libusbhid to
libusb.

> > To which driver your device currently attaches?  What kind of transfers
> > to you want to submit from userland?  The libusb already allow your to
> > submit(some) control transfers.  It would be nice if you could improve
> > this rather than adding a hack like this one.
> >
> 
> As mentioned in another email reply, PICKit 2 is a generic USB device
> and it uses control transfer and interrupt transfer. It is a bit unique that
> PICkit 2 has two USB configurations, one for HID and one for custom
> USB ( a legacy from the PICKit 1 days).

Well having two configurations should not be a problem.

Re: ugenctl for attaching USB devices to ugen instead of their specific driver

[Stuart Henderson]

On 2015/12/07 22:11, Xiaofan Chen wrote:
> Not so sure if there is a port of hidapi under OpenBSD.
> Ref: http://www.signal11.us/oss/hidapi/

It isn't ported to OpenBSD, and it's one of those projects where they
don't provide any infrastructure to build a library and suggest that
people copy it to their source tree and add to their own Makefiles..

rtdeletemsg & KASSERT

[Martin Pieuchot]

The rtrequest_delete() refactoring exposed an existing bug and
introduced a regression, both triggered by the same KASSERT().

The regression has been reported there:
  https://marc.info/?l=openbsd-bugs=144943901304713=2

The problem is that rt_if_remove() will triggers a rtflushclone1() if a
RTF_CLONING route is attached to an interface.  In this case
rtdeletemsg() tries to get the interface pointer from rt_ifidx and the
KASSERT() fires.

The bug exposed by the same KASSERT() can be triggered by a call to
rt_ifa_del() when dhclient(8) tries to add an address already configured
on the system.  In this case the kernel calls rtrequest_delete() with
a wrong ifp.

Diff below fixes these two bugs.  It's big because I don't want to put
two customs checks in a generic function.  This diff is built upon my
previous rtdeletemsg() one.

It introduces rt_delete() which deletes a route *without* doing a
lookup.  There's various good reasons for that.
  - First of all I don't want to take the risk of matching a similar
multipath route.  The rt_ifa_del() bug is an example. 
  - Secondly I'd like to avoid a recursive rtable_* call when routes
are deleted from rtable_walk().
  - Then I'd rather keep userland-specific checks inside rtrequest()
which will ends up in rtsock.c one day.
  - Finally it cleans one use of "struct rt_addrinfo" which should be
limited to userland in order to get rid of RTA_NETMASK in kernel.

Note that it introduces a behavior change.  If rtdeletemsg() fails to
delete a route no message is reported to userland.  I believe this was
only used for debugging.

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.289
diff -u -p -r1.289 route.c
--- net/route.c 5 Dec 2015 10:07:55 -   1.289
+++ net/route.c 7 Dec 2015 15:10:15 -
@@ -159,8 +159,7 @@ struct sockaddr *rt_plentosa(sa_family_t
 
 struct ifaddr *ifa_ifwithroute(int, struct sockaddr *, struct sockaddr *,
u_int);
-intrtrequest_delete(struct rt_addrinfo *, u_int8_t, struct ifnet *,
-   struct rtentry **, u_int);
+intrt_delete(struct rtentry *, struct ifnet *, unsigned int);
 
 #ifdef DDB
 void   db_print_sa(struct sockaddr *);
@@ -613,38 +612,29 @@ out:
 }
 
 /*
- * Delete a route and generate a message
+ * Delete a route and generate a message.  The caller must hold a reference
+ * on ``rt''.
  */
 int
-rtdeletemsg(struct rtentry *rt, struct ifnet *ifp, u_int tableid)
+rtdeletemsg(struct rtentry *rt, struct ifnet *ifp, unsigned int rtableid)
 {
int error;
-   struct rt_addrinfo  info;
-   unsigned intifidx;
-   struct sockaddr_in6 sa_mask;
 
-   /*
-* Request the new route so that the entry is not actually
-* deleted.  That will allow the information being reported to
-* be accurate (and consistent with route_output()).
-*/
-   bzero((caddr_t), sizeof(info));
-   info.rti_info[RTAX_DST] = rt_key(rt);
-   if (!ISSET(rt->rt_flags, RTF_HOST))
-   info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt, _mask);
-   info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
-   info.rti_flags = rt->rt_flags;
-   ifidx = rt->rt_ifidx;
-   error = rtrequest_delete(, rt->rt_priority, ifp, , tableid);
-   rt_missmsg(RTM_DELETE, , info.rti_flags, ifidx, error, tableid);
+   KASSERT(rt->rt_ifidx == ifp->if_index);
+
+   error = rt_delete(rt, ifp, rtableid);
if (error == 0)
-   rtfree(rt);
+   rt_sendmsg(rt, RTM_DELETE, rtableid);
+
return (error);
 }
 
 static inline int
 rtequal(struct rtentry *a, struct rtentry *b)
 {
+   if (a == b)
+   return 1;
+
if (memcmp(rt_key(a), rt_key(b), rt_key(a)->sa_len) == 0 &&
rt_plen(a) == rt_plen(b))
return 1;
@@ -656,10 +646,25 @@ int
 rtflushclone1(struct rtentry *rt, void *arg, u_int id)
 {
struct rtentry *parent = arg;
+   struct ifnet *ifp;
+
+   ifp = if_get(rt->rt_ifidx);
+
+   /*
+* This happens when an interface with a RTF_CLONING route is
+* being detached.  In this case it's safe to bail because all
+* the routes are being purged by rt_if_remove().
+*/
+   if (ifp == NULL)
+   return 0;
 
-   if ((rt->rt_flags & RTF_CLONED) != 0 && (rt->rt_parent == parent ||
-   rtequal(rt->rt_parent, parent)))
-   rtdeletemsg(rt, NULL, id);
+   if (ISSET(rt->rt_flags, RTF_CLONED) && rtequal(rt->rt_parent, parent)) {
+   rtref(rt);
+   rtdeletemsg(rt, ifp, id);
+   rtfree(rt);
+   }
+
+   if_put(ifp);
return 0;
 }
 
@@ -801,53 +806,20 @@ rt_getifa(struct rt_addrinfo *info, u_in
 }
 
 int
-rtrequest_delete(struct rt_addrinfo *info, u_int8_t prio, struct ifnet *ifp,
-struct rtentry **ret_nrt, u_int tableid)

Re: preparing multitouch support - request for tests

[Matthieu Herrb]

On Thu, Dec 03, 2015 at 12:20:15AM +0100, Ulf Brosziewski wrote:
> The diffs below contain a complete and extensive rewrite of the
> input-processing parts of wsmouse and the interface it provides to
> the hardware drivers. It prepares the support for various kinds of
> multitouch input, as well as an extended support for touchpads by
> wsmouse.
> 

Hi,

it looks like the diff got some white space mangled somewhere on the
path to my mailbox. Would you have a version available somewhere with
a more reliable transport than email ? 

Thank you. 

-- 
Matthieu Herrb


pgpD4R376sQfx.pgp
Description: PGP signature

Re: taskctx and revisiting if_start serialisation

[Martin Pieuchot]

On 07/12/15(Mon) 19:37, David Gwynne wrote:
> On Sun, Dec 06, 2015 at 03:51:26PM +0100, Martin Pieuchot wrote:
> > On 06/12/15(Sun) 14:00, David Gwynne wrote:
> > > [...]
> > > the idea is you have a taskctx, which represents a serialising
> > > context for tasks. tasks are submitted to the taskctx, and the code
> > > will try to run the tasks immediately rather than defer them to a
> > > thread. if there is contention on the context, the contending cpu
> > > yields after queueing the task because the other cpu is responsible
> > > for running all pending tasks to completion.
> > > 
> > > it also simplifies the barrier operations a lot.
> > > 
> > > the diff below implements a generic taskctx framework, and cuts the
> > > mpsafe if_start() implementation over to it.
> > 
> > I'm not sure this should be implemented as a generic framework.  I'd
> > suggest to keep it specific to if_start().  As you say above the least
> > worst mechanism we have is currently taskqs, but maybe this could/will
> > change?
> 
> im in two minds about where the ctx code should sit. i obviously
> lean toward keeping it with taskqs, but i also cant come up with
> another use case for it. i also havent tried very hard to think of
> one.

Even if you have another use case for it I find this generic framework
confusing.  It is build on top of taskq, but it's not part of the taskq
API.  So putting the documentation in the same manual just confuse me.
Your task API is simple and people use it easily.  I think it should
stay like that.

Now we need a way to serialize start and txeof and you came up with a
solution...

> are you asking if taskqs will get better, or if something better
> than taskqs will come along?

...I'm just saying that the "implementation" of your serialisation
mechanism should not matter.  I'm fine with having a context for tasks
but I'd suggest that this should be abstracted in a simple API instead
of offering a framework to build upon.

Could you make it such that you don't need a task in myx_softc?

> > I cannot understand what's happening by reading the myx(4) chunk itself.
> > So I believe the interface needs to be polished.  Would it be possible
> > to implement this serialization without introducing if_restart()?
> 
> that is by far my least favourite bit of this diff. in my defense,
> i wrote it while my mother was trying to have a conversation with
> me, so it didn't get my full attention. ive torn if_restart out and
> implemented it completely in myx below.
> 
> > > myx is also changed to only clr oactive from within the taskctx
> > > serialiser, thereby avoiding the race, but keeps the bulk of txeof
> > > outside the serialiser so it can run concurrently with start.
> > > 
> > > other nics are free to serialise start and txeof within the
> > > ifq_serializer if they want, or not, it is up to them.
> > >
> > > thoughts? tests? opinions on messy .h files?
> > 
> > It appears to me that you have unrelated changes: if_enter/if_leave.
> 
> oops, yes. i suck at juggling diffs.  maybe we should call if.c
> full and split it up ;)

I think we should start with that because the I'm afraid we're already
cooking spaghetti.

My question is the barrier and mpsafe code is related to a queue or to
an interface?  In other words does it make sense to serialize on a
context in the sending queue?  What about multiple queues?

> Index: sys/net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.422
> diff -u -p -r1.422 if.c
> --- sys/net/if.c  5 Dec 2015 10:07:55 -   1.422
> +++ sys/net/if.c  7 Dec 2015 06:30:42 -
> @@ -153,8 +153,9 @@ void  if_input_process(void *);
>  void ifa_print_all(void);
>  #endif
>  
> -void if_start_mpsafe(struct ifnet *ifp);
> -void if_start_locked(struct ifnet *ifp);
> +void if_start_mpsafe(struct ifnet *);
> +void if_start_locked(struct ifnet *);
> +void if_start_task(void *);
>  
>  /*
>   * interface index map
> @@ -513,6 +514,7 @@ if_attach_common(struct ifnet *ifp)
>   TAILQ_INIT(>if_maddrlist);
>  
>   ifq_init(>if_snd);
> + task_set(>if_start_ctx, if_start_task, ifp);
>  
>   ifp->if_addrhooks = malloc(sizeof(*ifp->if_addrhooks),
>   M_TEMP, M_WAITOK);
> @@ -557,66 +559,29 @@ if_start_locked(struct ifnet *ifp)
>   KERNEL_UNLOCK();
>  }
>  
> -static inline unsigned int
> -ifq_enter(struct ifqueue *ifq)
> -{
> - return (atomic_inc_int_nv(>ifq_serializer) == 1);
> -}
> -
> -static inline unsigned int
> -ifq_leave(struct ifqueue *ifq)
> +void
> +if_start_mpsafe(struct ifnet *ifp)
>  {
> - if (atomic_cas_uint(>ifq_serializer, 1, 0) == 1)
> - return (1);
> -
> - ifq->ifq_serializer = 1;
> -
> - return (0);
> + task_dispatch(>if_snd.ifq_serializer, >if_start_ctx);
>  }
>  
>  void
> -if_start_mpsafe(struct ifnet *ifp)
> +if_start_task(void *p)
>  {
> - struct ifqueue *ifq = >if_snd;
> + struct ifnet *ifp = p;
>  
> - if 

Do not pass NULL to rtdeletemsg()

[Martin Pieuchot]

If the interface is gone that means you're dealing with a cached route
so there's no need to try to remove it from the table.

Better be explicit and do that before calling rtdeletemsg() rather than
inside.

ok?

Index: netinet/ip_icmp.c
===
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.150
diff -u -p -r1.150 ip_icmp.c
--- netinet/ip_icmp.c   3 Dec 2015 21:11:53 -   1.150
+++ netinet/ip_icmp.c   7 Dec 2015 12:40:06 -
@@ -1042,19 +1042,21 @@ icmp_mtudisc(struct icmp *icp, u_int rta
 void
 icmp_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
 {
-   if (rt == NULL)
-   panic("icmp_mtudisc_timeout:  bad route to timeout");
+   struct ifnet *ifp;
+   int s;
 
-   if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
-   (RTF_DYNAMIC | RTF_HOST)) {
+   ifp = if_get(rt->rt_ifidx);
+   if (ifp == NULL)
+   return;
+
+   if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
void *(*ctlfunc)(int, struct sockaddr *, u_int, void *);
struct sockaddr_in sin;
-   int s;
 
sin = *satosin(rt_key(rt));
 
s = splsoftnet();
-   rtdeletemsg(rt, NULL, r->rtt_tableid);
+   rtdeletemsg(rt, ifp, r->rtt_tableid);
 
/* Notify TCP layer of increased Path MTU estimate */
ctlfunc = inetsw[ip_protox[IPPROTO_TCP]].pr_ctlinput;
@@ -1062,9 +1064,12 @@ icmp_mtudisc_timeout(struct rtentry *rt,
(*ctlfunc)(PRC_MTUINC, sintosa(),
r->rtt_tableid, NULL);
splx(s);
-   } else
+   } else {
if ((rt->rt_rmx.rmx_locks & RTV_MTU) == 0)
rt->rt_rmx.rmx_mtu = 0;
+   }
+
+   if_put(ifp);
 }
 
 /*
@@ -1088,17 +1093,20 @@ icmp_ratelimit(const struct in_addr *dst
 void
 icmp_redirect_timeout(struct rtentry *rt, struct rttimer *r)
 {
-   if (rt == NULL)
-   panic("icmp_redirect_timeout:  bad route to timeout");
+   struct ifnet *ifp;
+   int s;
 
-   if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
-   (RTF_DYNAMIC | RTF_HOST)) {
-   int s;
+   ifp = if_get(rt->rt_ifidx);
+   if (ifp == NULL)
+   return;
 
+   if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
-   rtdeletemsg(rt, NULL, r->rtt_tableid);
+   rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
}
+
+   if_put(ifp);
 }
 
 int
Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.182
diff -u -p -r1.182 icmp6.c
--- netinet6/icmp6.c3 Dec 2015 21:11:53 -   1.182
+++ netinet6/icmp6.c7 Dec 2015 12:39:28 -
@@ -1952,34 +1952,42 @@ icmp6_mtudisc_clone(struct sockaddr *dst
 void
 icmp6_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
 {
-   if (rt == NULL)
-   panic("icmp6_mtudisc_timeout: bad route to timeout");
-   if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
-   (RTF_DYNAMIC | RTF_HOST)) {
-   int s;
+   struct ifnet *ifp;
+   int s;
 
+   ifp = if_get(rt->rt_ifidx);
+   if (ifp == NULL)
+   return;
+
+   if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
-   rtdeletemsg(rt, NULL, r->rtt_tableid);
+   rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
} else {
if (!(rt->rt_rmx.rmx_locks & RTV_MTU))
rt->rt_rmx.rmx_mtu = 0;
}
+
+   if_put(ifp);
 }
 
 void
 icmp6_redirect_timeout(struct rtentry *rt, struct rttimer *r)
 {
-   if (rt == NULL)
-   panic("icmp6_redirect_timeout: bad route to timeout");
-   if ((rt->rt_flags & (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) ==
-   (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) {
-   int s;
+   struct ifnet *ifp;
+   int s;
 
+   ifp = if_get(rt->rt_ifidx);
+   if (ifp == NULL)
+   return;
+
+   if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
-   rtdeletemsg(rt, NULL, r->rtt_tableid);
+   rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
}
+
+   if_put(ifp);
 }
 
 int *icmpv6ctl_vars[ICMPV6CTL_MAXID] = ICMPV6CTL_VARS;

Re: ugenctl for attaching USB devices to ugen instead of their specific driver

[Xiaofan Chen]

On Mon, Dec 7, 2015 at 7:42 PM, Martin Pieuchot  wrote:
> I'd really prefer a solution that doesn't need any button.  In other
> words to always be able to use your device from userland if the kernel
> is not using it.
>
> One way would be to always attach a ugen(4) driver to every USB device,
> I think that's what FreeBSD does.  The other way would be to use the
> usb(4) bus interface to talk to your device.  In both cases some work
> is needed to guarantee that an endpoint is only driven by one driver at
> a time.  Right now this is done by attaching only one driver.

It will be good if this limitation is removed (I know the FreeBSD approach
but FreeBSD libusb is not under libusb project due to license concerns)
and then libusb will be more useful. Or a method like Linux where the
kernel driver can be dynamically detached (so that usbfs generic driver
is attached and libusb uses usbfs under Linux).

> To which driver your device currently attaches?  What kind of transfers
> to you want to submit from userland?  The libusb already allow your to
> submit(some) control transfers.  It would be nice if you could improve
> this rather than adding a hack like this one.
>

As mentioned in another email reply, PICKit 2 is a generic USB device
and it uses control transfer and interrupt transfer. It is a bit unique that
PICkit 2 has two USB configurations, one for HID and one for custom
USB ( a legacy from the PICKit 1 days).


-- 
Xiaofan

Re: Overflowable int -> size_t in grep

[Theo de Raadt]

> > Otto Moerbeek wrote:
> > > On Mon, Dec 07, 2015 at 01:36:22AM -0500, Michael McConville wrote:
> > > > This isn't a grave issue, but I came across it while exploring integer
> > > > overflow and think it's worth sharing.
> > > > 
> > > > grep represents line numbers with an int, which predictably overflows
> > > > for inputs with >= 2^31 newlines. This is easy to demonstrate using the
> > > > -n option and a debugging printf.
> > > > 
> > > > The below diff fixes this, and tunes up a for loop while I'm in there.
> > > 
> > > how does this fix work on 32-bit platforms? Better use offset_t.
> > 
> > Good catch, thanks.
> > 
> > Does this look better? Or is PRId64 preferred for off_t?
> 
> Neither is correct, off_t should be cast to intmax_t first and then %jd
> be used.

That is incorrect advice for the OpenBSD world.

Re: Make em(4) more mpsafe again

[Hrvoje Popovski]

On 5.12.2015. 15:41, Claudio Jeker wrote:
> So Mark and I spent some time to figure out what the issue was with ix(4)
> based on that info I resurected the em(4) mpsafe diff that got backed out
> and I applied the same fix. It is somewhat unclear if this fixes the
> watchdog timeouts since in theory the wdog timer should be stopped when
> hitting the race condition we hit in ix(4).
> 
> I'm currently hammering my test system with this and until now I have not
> seen a watchdog fired.
> 

Hi,

i have tested this patch on quad port i350 sending 1.1Mpps over routed
setup and i couldn't see OACTIVE flag. doing ifconfig down/up triggers
noting. this patch is combined with latest dlg@ "if_start serialisation​
" patch ...

# netstat -i 1
  em0 inem0 out  total in  total out
 packets  errs  packets  errs colls   packets  errs  packets  errs colls
2483 0 1722 0 0  849685845 8472337 846024217 0 0
   6 02 0 0   1163346 356432  1135822 0 0
   3 02 0 0   1045620 474468  1044110 0 0
   4 01 0 0878131 305165   878111 0 0
   1 01 0 0665390 495223   632937 0 0
   1 01 0 0863487 353536   835253 0 0
   1 01 0 0   1032162 479482  1030798 0 0
   1 01 0 0   1239190 657043  1236889 0 0
   3 01 0 0803644 607252   773553 0 0
   6 01 0 0   1033900 355989  1032140 0 0
   3 01 0 0884158 315178   884148 0 0
   3 01 0 0671153 422765   641784 0 0
   2 01 0 0   1142317 302342  1141093 0 0
   1 01 0 0   1227210 320112  1205720 0 0
   2 02 0 0319133 423012   319073 0 0
   1 01 0 0681295 480570   654932 0 0
   1 01 0 0670643 490736   639768 0 0
   2 01 0 0   1082213 352263  1080645 0 0
   2 01 0 0678592 327973   659225 0 0
   3 01 0 0898827 488901   898763 0 0
   2 02 0 0711612 245223   685159 0 0
   1 02 0 0685832 457282   652404 0 0
   7 01 0 0   1019866 514702   994254 0 0
   5 01 0 0   1167919 351723  1139686 0 0
   3 01 0 0661945 477068   636373 0 0
   2 01 0 0   1168973 478353  1142186 0 0
   1 01 0 0320694 404637   320653 0 0
   2 01 0 0667402 485570   638250 0 0
   1 01 0 0665822 488057   648361 0 0
   1 01 0 0673984 486514   645794 0 0
   1 01 0 0676755 475929   649052 0 0
   2 02 0 0673516 483569   645279 0 0
   3 01 0 0676958 478505   649647 0 0
   3 01 0 0671307 485305   643851 0 0
   2 01 0 0678013 476246   649943 0 0
   2 01 0 0673911 483511   644886 0 0
   1 01 0 0674985 480910   648788 0 0
   1 01 0 0673149 484188   645064 0 0
   1 01 0 0676980 478390   649883 0 0
   1 01 0 0672429 482019   644436 0 0
   1 01 0 0677097 478082   649769 0 0
   1 02 0 0676161 480272   648490 0 0
   1 01 0 0671737 480768   643312 0 0
   1 02 0 0838715 358990   809036 0 0
   2 01 0 0   1165676 880005  1137991 0 0
   2 01 0 0836085 354568   835948 0 0
   1 02 0 0618887 508218   590496 0 0
   1 01 0 0939546 489689   900721 0 0
   1 01 0 0936974 605481   908905 0 0
   1 01 0 0   1040186 355489  1038637 0 0
   1 01 0 0966385 322910   939540 0 0
   1 01 0 0829643 482648   829587 0 0
   1 01 0 0646072 409609   617855 0 0
   1 01 0 0915205 358060   915191 0 0
   2 01 0 0641707 402106   618920 0 0
   1 01 0 0906929 

umass: size for free

[Mathieu -]

Hello,

I worked a bit on umass(4) recently and had a diff to pass the
umassbus_softc's real size to free so here it is.  At some point I
pondered about deleting the whole abstraction, as it would simplify the
free'ing, for we only have one implementation (umass_scsi_softc, as atapi
uses it too). But I figured it would be against the whole design of the
umass driver, thoughts?

Index: usb/umass.c
===
RCS file: /cvs/src/sys/dev/usb/umass.c,v
retrieving revision 1.70
diff -u -p -r1.70 umass.c
--- usb/umass.c 14 Mar 2015 03:38:50 -  1.70
+++ usb/umass.c 7 Dec 2015 15:40:15 -
@@ -651,7 +651,7 @@ umass_detach(struct device *self, int fl
if (scbus != NULL) {
if (scbus->sc_child != NULL)
rv = config_detach(scbus->sc_child, flags);
-   free(scbus, M_DEVBUF, 0);
+   free(scbus, M_DEVBUF, scbus->sc_size);
sc->bus = NULL;
}
 
Index: usb/umass_scsi.c
===
RCS file: /cvs/src/sys/dev/usb/umass_scsi.c,v
retrieving revision 1.42
diff -u -p -r1.42 umass_scsi.c
--- usb/umass_scsi.c14 Mar 2015 03:38:50 -  1.42
+++ usb/umass_scsi.c7 Dec 2015 15:40:16 -
@@ -145,6 +145,7 @@ umass_scsi_setup(struct umass_softc *sc)
struct umass_scsi_softc *scbus;
 
scbus = malloc(sizeof(*scbus), M_DEVBUF, M_WAITOK | M_ZERO);
+   scbus->base.sc_size = sizeof(*scbus);
 
sc->bus = (struct umassbus_softc *)scbus;
 
Index: usb/umassvar.h
===
RCS file: /cvs/src/sys/dev/usb/umassvar.h,v
retrieving revision 1.14
diff -u -p -r1.14 umassvar.h
--- usb/umassvar.h  6 Nov 2013 14:37:31 -   1.14
+++ usb/umassvar.h  7 Dec 2015 15:40:16 -
@@ -146,6 +146,7 @@ struct umass_wire_methods {
 
 struct umassbus_softc {
struct device   *sc_child;  /* child device, for detach */
+   size_t   sc_size;
 };
 
 /* the per device structure */

Re: Overflowable int -> size_t in grep

[Todd C. Miller]

On Mon, 07 Dec 2015 02:32:50 -0500, Michael McConville wrote:

> Does this look better? Or is PRId64 preferred for off_t?

As Otto said, we generally use %lld for printing off_t and use a
(long long) cast.  With that change OK millert@

 - todd

iwm association fix

[Stefan Sperling]

While debugging my 802.11n code I got tired of iwm randomly failing
to associate or even getting into a state where I had to reboot to
get it to work again.

In case it got hung, the newstate task was sleeping in "iwmauth" and
never woke up, which made me revisit how this driver transitions to
AUTH state. I couldn't figure out exactly how it got hung, but I
believe I found a good reason to remove this tsleep entirely.

What's going on here is that the firmware can hop channels in the
background whenever it feels like doing so. To prevent this from
happening during the association procedure, the driver schedules a
"time event" which makes the firmware stay on the current channel
for a specified amount of time. This approach is borrowed from the
Linux driver.

If all goes well, the following happens:

1) The driver's newstate task asks for the time event and goes into
   tsleep() to wait for the firmware to signal the beginning of the
   time event.
2) The firmware signals beginning of the time event.
3) The interrupt handler sets a flag in the softc which indicates
   "time event start" and wakes up the sleeping newstate task.
4) The newstate task is scheduled, checks the flag which says that
   the time event has started, and finishes moving to AUTH state.
5) Association continues and completes
6) The firmware signals the end of the time event.
7) The interrupt handler sets a flag in the softc which indicates
   "time event ended" and wakes up any task sleeping on this
   condition (there is none)

Note that steps 5 and 6/7 can happen "concurrently" if the "time-event
ended" interrupt triggers during step 5, which is no problem.

Sometimes, especially with IWM_DEBUG enabled (lots of printf), this
doesn't work out as planned and the order of events goes like this:

1) The driver's newstate task asks for the time event and goes into
   tsleep() to wait for the firmware to signal the beginning of the
   time event.
2) The firmware signals beginning of the time event.
3) The interrupt handler sets a flag in the softc which indicates
   "time event start" and wakes up the sleeping newstate task.
4) The firmware signals the end of the time event.
5) The interrupt handler sets a flag in the softc which indicates
   "time event ended" and wakes up the sleeping newstate task.
6) The newstate task is scheduled, checks the flag which says that
   the time event has started, but it is not set so association is
   aborted.

I have also observed other variants of these chains of events where
iwm does not manage to associate.

Leaving precise synchronization like this up to the scheduler in our
kernel is not a good idea. There is no guarantee that things will work
out as intended. Moreover, the numbers controlling timing were copied
from Linux which has different scheduling latency.

This diff does away with the unreliable extra checking. It schedules a
time event, busy-waits a short while for good measure, and life goes on.
The firmware might fail to schedule the event, and if so might wander off
the channel for a moment... in which case association is still more
likely to succeed than with these unreliable checks in place.
But generally the time event will be scheduled and things just work out.

While here, also copy some related docs in comments from the Linux driver
and remove an unused argument from iwm_mvm_protect_session().

OK?

Below the diff is a trace which shows how events play out now.
The lines saying "iwm0: TE notif status = 0x01 action = 0x01" indicate
time event start, and the same with "action = 0x02" indicates the end
of the time event -- well after association is complete.

Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.68
diff -u -p -r1.68 if_iwm.c
--- if_iwm.c25 Nov 2015 03:09:59 -  1.68
+++ if_iwm.c7 Dec 2015 15:13:22 -
@@ -272,7 +272,7 @@ int iwm_mvm_send_time_event_cmd(struct i
 intiwm_mvm_time_event_send_add(struct iwm_softc *, struct iwm_node *,
void *, struct iwm_time_event_cmd_v2 *);
 void   iwm_mvm_protect_session(struct iwm_softc *, struct iwm_node *,
-   uint32_t, uint32_t, uint32_t);
+   uint32_t, uint32_t);
 intiwm_nvm_read_chunk(struct iwm_softc *, uint16_t, uint16_t, uint16_t,
uint8_t *, uint16_t *);
 intiwm_nvm_read_section(struct iwm_softc *, uint16_t, uint8_t *,
@@ -2216,7 +2216,7 @@ iwm_mvm_time_event_send_add(struct iwm_s
 
 void
 iwm_mvm_protect_session(struct iwm_softc *sc, struct iwm_node *in,
-   uint32_t duration, uint32_t min_duration, uint32_t max_delay)
+   uint32_t duration, uint32_t max_delay)
 {
struct iwm_time_event_cmd_v2 time_cmd;
 
@@ -4988,7 +4988,6 @@ iwm_auth(struct iwm_softc *sc)
struct ieee80211com *ic = >sc_ic;
struct iwm_node *in = (void *)ic->ic_bss;
uint32_t 

dhclient hang

[Martin Pieuchot]

If for some reason SIOCAIFADDR fails, exit instead of hanging.

Without this diff I need to kill dhclient(8) with ctrl+C during boot
if I happen to have the address offered by the server configured on
a different interface.

WARNING: do not try to do that on -current without my rt_delete diffs
or your kernel with panic!

Ok?

Index: kroute.c
===
RCS file: /cvs/src/sbin/dhclient/kroute.c,v
retrieving revision 1.75
diff -u -p -r1.75 kroute.c
--- kroute.c10 Feb 2015 04:20:26 -  1.75
+++ kroute.c7 Dec 2015 15:51:17 -
@@ -473,7 +473,7 @@ priv_add_address(struct imsg_add_address
/* No need to set broadcast address. Kernel can figure it out. */
 
if (ioctl(s, SIOCAIFADDR, ) == -1)
-   warning("SIOCAIFADDR failed (%s): %s", inet_ntoa(imsg->addr),
+   error("SIOCAIFADDR failed (%s): %s", inet_ntoa(imsg->addr),
strerror(errno));
 
close(s);

Re: taskctx and revisiting if_start serialisation

[David Gwynne]

On Sun, Dec 06, 2015 at 03:51:26PM +0100, Martin Pieuchot wrote:
> On 06/12/15(Sun) 14:00, David Gwynne wrote:
> > the current code for serialising if_start calls for mpsafe nics does
> > what it says.
> > 
> > however, kettenis realised it doesnt help us much when we're trying
> > to coordinate between the start and txeof side of a driver when
> > setting or clearing oactive. in particular, a start routine can
> > figure out there's no more space, and then set oactive. txeof could
> > be running on another cpu emptying the ring and clearing it. if
> > that clear runs in between the other cpus space check and
> > ifq_set_oactive, then the nic will be marked full and the stack
> > wont ever call start on it again.
> > 
> > so it can be argued that start and txeof should be serialised.
> > indeed, other platforms do exactly that.
> > 
> > the least worst mechanism we have for doing that is taskqs. however,
> > all my experiments deferring start to a taskq end up significantly
> > hurting performance.
> > [...] 
> > while toying with ideas on how to solve kettenis' oactive problem,
> > i came up with the following.
> > 
> > it combines tasks with direct dispatch, and borrows the current
> > ifq_serialiser/pool/scsi serialisation algorithm.
> 
> I like the idea.

\o/

> > the idea is you have a taskctx, which represents a serialising
> > context for tasks. tasks are submitted to the taskctx, and the code
> > will try to run the tasks immediately rather than defer them to a
> > thread. if there is contention on the context, the contending cpu
> > yields after queueing the task because the other cpu is responsible
> > for running all pending tasks to completion.
> > 
> > it also simplifies the barrier operations a lot.
> > 
> > the diff below implements a generic taskctx framework, and cuts the
> > mpsafe if_start() implementation over to it.
> 
> I'm not sure this should be implemented as a generic framework.  I'd
> suggest to keep it specific to if_start().  As you say above the least
> worst mechanism we have is currently taskqs, but maybe this could/will
> change?

im in two minds about where the ctx code should sit. i obviously
lean toward keeping it with taskqs, but i also cant come up with
another use case for it. i also havent tried very hard to think of
one.

are you asking if taskqs will get better, or if something better
than taskqs will come along?

> I cannot understand what's happening by reading the myx(4) chunk itself.
> So I believe the interface needs to be polished.  Would it be possible
> to implement this serialization without introducing if_restart()?

that is by far my least favourite bit of this diff. in my defense,
i wrote it while my mother was trying to have a conversation with
me, so it didn't get my full attention. ive torn if_restart out and
implemented it completely in myx below.

> > myx is also changed to only clr oactive from within the taskctx
> > serialiser, thereby avoiding the race, but keeps the bulk of txeof
> > outside the serialiser so it can run concurrently with start.
> > 
> > other nics are free to serialise start and txeof within the
> > ifq_serializer if they want, or not, it is up to them.
> >
> > thoughts? tests? opinions on messy .h files?
> 
> It appears to me that you have unrelated changes: if_enter/if_leave.

oops, yes. i suck at juggling diffs.  maybe we should call if.c
full and split it up ;)

Index: share/man/man9/Makefile
===
RCS file: /cvs/src/share/man/man9/Makefile,v
retrieving revision 1.262
diff -u -p -r1.262 Makefile
--- share/man/man9/Makefile 25 Nov 2015 03:09:57 -  1.262
+++ share/man/man9/Makefile 7 Dec 2015 06:30:42 -
@@ -397,9 +397,14 @@ MLINKS+=systrace.9 systrace_redirect.9 \
systrace.9 systrace_fork.9 systrace.9 systrace_exit.9
 MLINKS+=task_add.9 taskq_create.9 \
task_add.9 taskq_destroy.9 \
+   task_add.9 taskq_barrier.9 \
task_add.9 task_set.9 \
task_add.9 task_del.9 \
-   task_add.9 TASK_INITIALIZER.9
+   task_add.9 TASK_INITIALIZER.9 \
+   task_add.9 taskctx_init.9 \
+   task_add.9 TASKCTX_INITIALIZER.9 \
+   task_add.9 taskctx_barrier.9 \
+   task_add.9 task_dispatch.9
 MLINKS+=time.9 boottime.9 time.9 mono_time.9 time.9 runtime.9
 MLINKS+=timeout.9 timeout_add.9 timeout.9 timeout_set.9 \
timeout.9 timeout_pending.9 timeout.9 timeout_del.9 \
Index: share/man/man9/task_add.9
===
RCS file: /cvs/src/share/man/man9/task_add.9,v
retrieving revision 1.16
diff -u -p -r1.16 task_add.9
--- share/man/man9/task_add.9   14 Sep 2015 15:14:55 -  1.16
+++ share/man/man9/task_add.9   7 Dec 2015 06:30:42 -
@@ -18,15 +18,23 @@
 .Dt TASK_ADD 9
 .Os
 .Sh NAME
+.Nm task_set ,
+.Nm TASK_INITIALIZER ,
 .Nm taskq_create ,
 .Nm taskq_destroy ,
-.Nm task_set ,
+.Nm taskq_barrier ,
 .Nm task_add ,
 .Nm task_del ,
-.Nm TASK_INITIALIZER

Re: [patch] Convert modulus to arc4random_uniform

[Theo Buehler]

On Mon, Dec 07, 2015 at 12:49:17AM -0600, Matthew Martin wrote:
> 
> Theo's diff inspired me to look for other cases of modulo bias. The
> following diff converts most modulus operations on a random number to
> use arc4random_uniform or & as appropriate. I excluded
> 
> lib/libsqlite3/src/resolve.c
> regress/lib/libevent/test-time.c
> usr.sbin/nsd/rrl.c
> usr.sbin/nsd/util.c
> usr.sbin/nsd/xfrd.c
> 
> as they seem to have upstreams. The only other case is games/wump/wump.c
> which has
> 
> if (arc4random() % 2 == 1)
> 
> This is safe and seems obvious enough to me.
> 
> - Matthew Martin

Thank you.  I was going to do the same :)

I think some of these are ok, but I'm unsure about some of the others.
Here are some of my concerns:

- since arc4random_uniform can potentially loop indefinitely, it
  might interfere with predictable timing of some routines.  I can't
  tell if this is harmless in all cases below.  This applies in
  particular to the proposed changes in the kernel.

- changing random() to arc4random() in games might have undesired
  side-effects like interfering with the reproducibility of tests or
  games.  I think this might apply to awk for the same reason as in the
  shells.  

> Index: games/atc/update.c
> ===

ok

> Index: games/hack/hack.mklev.c
> ===
> Index: games/hack/rnd.c
> ===

unsure about these two

> Index: lib/libc/stdlib/rand.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/rand.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 rand.c
> --- lib/libc/stdlib/rand.c13 Sep 2015 08:31:47 -  1.15
> +++ lib/libc/stdlib/rand.c7 Dec 2015 06:42:17 -
> @@ -50,7 +50,7 @@ int
>  rand(void)
>  {
>   if (rand_deterministic == 0)
> - return (arc4random() % ((u_int)RAND_MAX + 1));
> + return (arc4random_uniform((u_int)RAND_MAX + 1));
>   return (rand_r());
>  }
>  

this is modulo 2^n, so I think this one is fine as it is.

> Index: sbin/dhclient/dhclient.c
> ===

I have already done this independently.

> Index: sys/dev/ic/sili.c
> ===
> Index: sys/netinet6/nd6_rtr.c
> ===
> Index: sys/ufs/ffs/ffs_alloc.c
> ===

These must definitely be looked at by somebody else.

> Index: usr.bin/awk/run.c
> ===

Unsure about this one.  I think deterministic sequences might be desired
in some circumstances (this one is deterministic when a seed was given).

> Index: usr.sbin/npppd/common/slist.c
> ===
> Index: usr.sbin/npppd/l2tp/l2tpd.c
> ===
> Index: usr.sbin/npppd/pppoe/pppoed.c
> ===
> Index: usr.sbin/npppd/pptp/pptpd.c
> ===

These four are ok with me.

Re: Overflowable int -> size_t in grep

[Otto Moerbeek]

On Mon, Dec 07, 2015 at 02:32:50AM -0500, Michael McConville wrote:

> Otto Moerbeek wrote:
> > On Mon, Dec 07, 2015 at 01:36:22AM -0500, Michael McConville wrote:
> > > This isn't a grave issue, but I came across it while exploring integer
> > > overflow and think it's worth sharing.
> > > 
> > > grep represents line numbers with an int, which predictably overflows
> > > for inputs with >= 2^31 newlines. This is easy to demonstrate using the
> > > -n option and a debugging printf.
> > > 
> > > The below diff fixes this, and tunes up a for loop while I'm in there.
> > 
> > how does this fix work on 32-bit platforms? Better use offset_t.
> 
> Good catch, thanks.
> 
> Does this look better? Or is PRId64 preferred for off_t?

I don't think off_t is related to ptrdiff_t. The only thing defined for
off_t is that it's a signed integer type, meant to express offsets in a file.

Just cast to long long and use %lld.

And please remove the unrelated for loop change.

-Otto


> 
> 
> Index: grep.h
> ===
> RCS file: /cvs/src/usr.bin/grep/grep.h,v
> retrieving revision 1.22
> diff -u -p -r1.22 grep.h
> --- grep.h16 Mar 2015 13:26:52 -  1.22
> +++ grep.h7 Dec 2015 07:30:52 -
> @@ -43,7 +43,7 @@
>  
>  typedef struct {
>   size_t   len;
> - int  line_no;
> + off_tline_no;
>   off_toff;
>   char*file;
>   char*dat;
> Index: util.c
> ===
> RCS file: /cvs/src/usr.bin/grep/util.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 util.c
> --- util.c25 Jun 2015 02:04:08 -  1.50
> +++ util.c7 Dec 2015 07:30:52 -
> @@ -124,7 +124,7 @@ procfile(char *fn)
>  
>   if (Bflag > 0)
>   initqueue();
> - for (c = 0;  c == 0 || !(lflag || qflag); ) {
> + for (c = 0;  c == 0 || !(lflag || qflag); c += t) {
>   ln.off += ln.len + 1;
>   if ((ln.dat = grep_fgetln(f, )) == NULL)
>   break;
> @@ -138,7 +138,6 @@ procfile(char *fn)
>   enqueue();
>   linesqueued++;
>   }
> - c += t;
>   }
>   if (Bflag > 0)
>   clearqueue();
> @@ -623,7 +622,7 @@ printline(str_t *line, int sep, regmatch
>   if (nflag) {
>   if (n)
>   putchar(sep);
> - printf("%d", line->line_no);
> + printf("%jd", line->line_no);
>   ++n;
>   }
>   if (bflag) {

Re: Overflowable int -> size_t in grep

[Joerg Sonnenberger]

On Mon, Dec 07, 2015 at 02:32:50AM -0500, Michael McConville wrote:
> Otto Moerbeek wrote:
> > On Mon, Dec 07, 2015 at 01:36:22AM -0500, Michael McConville wrote:
> > > This isn't a grave issue, but I came across it while exploring integer
> > > overflow and think it's worth sharing.
> > > 
> > > grep represents line numbers with an int, which predictably overflows
> > > for inputs with >= 2^31 newlines. This is easy to demonstrate using the
> > > -n option and a debugging printf.
> > > 
> > > The below diff fixes this, and tunes up a for loop while I'm in there.
> > 
> > how does this fix work on 32-bit platforms? Better use offset_t.
> 
> Good catch, thanks.
> 
> Does this look better? Or is PRId64 preferred for off_t?

Neither is correct, off_t should be cast to intmax_t first and then %jd
be used.

Joerg

Re: ugenctl for attaching USB devices to ugen instead of their specific driver

[Mickael Torres]

On 2015-12-07 12:42, Martin Pieuchot wrote:

On 07/12/15(Mon) 00:36, Mickael Torres wrote:

On 2015-12-06 20:10, Ian Darwin wrote:
>On 2015-12-06 12:23 PM, Stuart Henderson wrote:
>>On 2015/12/06 06:02, Mickael Torres wrote:
>>>Hello,
>>>
>>>This is a kernel patch plus a utility called ugenctl I use to allow
>>>selected USB devices to attach as ugen(4) instead of their more
>>>specific
>>>driver. My use case is a Microchip "PICkit 2 Microcontroller
>>>Programmer"
>>>that attaches as uhid(4), but the command line utility pk2cmd wants a
>>>udev(4) device. There maybe are some other use cases.
>>If a device is generally pointless with uhid, we can just knock it
>>out completely in the kernel, see the UQ_BAD_HID mechanism. I think this
>>applies to your device.
>>
>>The bigger problem is "dual use" devices; e.g. some want UPS to attach
>>to upd(4), others want ugen(4) for use with NUT/apcupsd. Your code is
>>partially useful for these, but because it just changes things at
>>attach,
>>won't survive a reboot - if it could instead force a device to detach
>>and reattach to ugen it would help a lot more cases.
>>
>One fairly common (I believe) use is with USB printers that attach as ulpt
>but CUPS wants as ugen - for these, this is useful as it stands - just run
>it
>from rc. Do people want their UPS to sometimes be upd and other times be
>a ugen? I can see there might be "I want to change this now" cases, but I
>suspect the majority could be done once at each boot and be quite useful.

Hello,


Hello Mickael,

an updated version that detach/reattach if a device is already 
connected,

when
adding and when removing from the ugen list.
Seems to work well, but I don't have a lot of hardware to test it.
What do you think ?


I'd really prefer a solution that doesn't need any button.  In other
words to always be able to use your device from userland if the kernel
is not using it.

One way would be to always attach a ugen(4) driver to every USB device,
I think that's what FreeBSD does.  The other way would be to use the
usb(4) bus interface to talk to your device.  In both cases some work
is needed to guarantee that an endpoint is only driven by one driver at
a time.  Right now this is done by attaching only one driver.

To which driver your device currently attaches?  What kind of transfers
to you want to submit from userland?  The libusb already allow your to
submit(some) control transfers.  It would be nice if you could improve
this rather than adding a hack like this one.

Cheers,
Martin


Hello Martin,

Actually the device attaches as uhid. I don't know which kind of 
transfer

is used, I'll have to look into this, but the soft (pk2cmd) uses libusb
and only works when the device is attached as ugen.
And another soft (pic32prog) which also uses libusb seems to only detect
the device when it is attached as uhid.
I'll try to take a look into libusb.

Cheers,
Mike.

Re: taskctx and revisiting if_start serialisation

[Hrvoje Popovski]

On 6.12.2015. 15:56, Hrvoje Popovski wrote:
> On 6.12.2015. 5:00, David Gwynne wrote:
>> the current code for serialising if_start calls for mpsafe nics does what it 
>> says.
>>
>> however, kettenis realised it doesnt help us much when we're trying
>> to coordinate between the start and txeof side of a driver when
>> setting or clearing oactive. in particular, a start routine can
>> figure out there's no more space, and then set oactive. txeof could
>> be running on another cpu emptying the ring and clearing it. if
>> that clear runs in between the other cpus space check and
>> ifq_set_oactive, then the nic will be marked full and the stack
>> wont ever call start on it again.
>>
>> so it can be argued that start and txeof should be serialised.
>> indeed, other platforms do exactly that.
>>
>> the least worst mechanism we have for doing that is taskqs. however,
>> all my experiments deferring start to a taskq end up significantly
>> hurting performance.
>>
>> dragonfly appears to have some of the semantics we want. according
>> to sephe, start and txeof are serialised, but they can be directly
>> called from anywhere. however, if one cpu is trying to run start
>> while the other is in txeof, it figures it out and makes the other
>> cpu run txeof on the first cpus behalf. the first cpu then simply
>> returns cos it knows the other cpu will end up doing the work.
>>
>> the implementation is tied very much to that specific situation,
>> and its hard for me to grok cos im not familiar with their locking
>> infrastructure.
>>
>> the dfly code has the (slight) caveat that you cant run txeof and
>> start concurrently, it forces them to be serialised.
>>
>> while toying with ideas on how to solve kettenis' oactive problem,
>> i came up with the following.
>>
>> it combines tasks with direct dispatch, and borrows the current
>> ifq_serialiser/pool/scsi serialisation algorithm.
>>
>> the idea is you have a taskctx, which represents a serialising
>> context for tasks. tasks are submitted to the taskctx, and the code
>> will try to run the tasks immediately rather than defer them to a
>> thread. if there is contention on the context, the contending cpu
>> yields after queueing the task because the other cpu is responsible
>> for running all pending tasks to completion.
>>
>> it also simplifies the barrier operations a lot.
>>
>> the diff below implements a generic taskctx framework, and cuts the
>> mpsafe if_start() implementation over to it.
>>
>> myx is also changed to only clr oactive from within the taskctx
>> serialiser, thereby avoiding the race, but keeps the bulk of txeof
>> outside the serialiser so it can run concurrently with start.
>>
>> other nics are free to serialise start and txeof within the
>> ifq_serializer if they want, or not, it is up to them.
>>
>> thoughts? tests? opinions on messy .h files?
> 
> 
> Hi,
> 
> after applying this patches over cvs source from few hours (no
> additional patches for ix and em) it seems that something isn't right...
> 
> freshly rebooted box, sending 2Mpps over ix (82599) and i'm getting
> around 50kpps on receiver... over x540 around 100kpps


With latest patch 50kpps and 100kpps problem is gone.

Re: [patch] dvmrpd: strings header cleanup

[Michael McConville]

Serguey Parkhomovsky wrote:
> Fixes implicit memcpy declarations by using string.h instead of
> strings.h, and removes strings.h from files that don't need it. Also,
> change bzero(3) to memset(3).

Thanks for this.

I just took care of the existing implicit declarations. Below is a diff
for the rest. I think you might have missed a couple bzero's.

I'll look into the include removals soon. In a rush at the moment.

ok?


Index: control.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/control.c,v
retrieving revision 1.21
diff -u -p -r1.21 control.c
--- control.c   5 Dec 2015 13:11:00 -   1.21
+++ control.c   7 Dec 2015 19:03:15 -
@@ -52,7 +52,7 @@ control_init(void)
return (-1);
}
 
-   bzero(, sizeof(sun));
+   memset(, 0, sizeof(sun));
sun.sun_family = AF_UNIX;
strlcpy(sun.sun_path, DVMRPD_SOCKET, sizeof(sun.sun_path));
 
Index: igmp.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/igmp.c,v
retrieving revision 1.3
diff -u -p -r1.3 igmp.c
--- igmp.c  18 Nov 2014 20:54:28 -  1.3
+++ igmp.c  7 Dec 2015 19:03:15 -
@@ -51,7 +51,7 @@ send_igmp_query(struct iface *iface, str
fatal("send_igmp_query");
 
/* IGMP header */
-   bzero(_hdr, sizeof(igmp_hdr));
+   memset(_hdr, 0, sizeof(igmp_hdr));
igmp_hdr.type = PKT_TYPE_MEMBER_QUERY;
 
if (group == NULL) {
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/kroute.c,v
retrieving revision 1.12
diff -u -p -r1.12 kroute.c
--- kroute.c27 Sep 2015 17:29:46 -  1.12
+++ kroute.c7 Dec 2015 19:03:15 -
@@ -136,7 +136,7 @@ kif_find(int ifindex)
 {
struct kif_node s;
 
-   bzero(, sizeof(s));
+   memset(, 0, sizeof(s));
s.k.ifindex = ifindex;
 
return (RB_FIND(kif_tree, , ));
Index: packet.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/packet.c,v
retrieving revision 1.4
diff -u -p -r1.4 packet.c
--- packet.c7 Dec 2015 18:59:31 -   1.4
+++ packet.c7 Dec 2015 19:03:15 -
@@ -48,7 +48,7 @@ gen_dvmrp_hdr(struct ibuf *buf, struct i
 {
struct dvmrp_hdrdvmrp_hdr;
 
-   bzero(_hdr, sizeof(dvmrp_hdr));
+   memset(_hdr, 0, sizeof(dvmrp_hdr));
dvmrp_hdr.type = PKT_TYPE_DVMRP;
dvmrp_hdr.code = code;
dvmrp_hdr.chksum = 0;   /* updated later */
Index: prune.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/prune.c,v
retrieving revision 1.5
diff -u -p -r1.5 prune.c
--- prune.c 5 May 2015 01:26:37 -   1.5
+++ prune.c 7 Dec 2015 19:03:15 -
@@ -23,7 +23,7 @@
 #include 
 
 #include 
-#include 
+#include 
 
 #include "igmp.h"
 #include "dvmrpd.h"
@@ -47,7 +47,7 @@ send_prune(struct nbr *nbr, struct prune
if (nbr->iface->passive)
return (0);
 
-   bzero(, sizeof(prune));
+   memset(, 0, sizeof(prune));
 
dst.sin_family = AF_INET;
dst.sin_len = sizeof(struct sockaddr_in);
@@ -97,7 +97,7 @@ recv_prune(struct nbr *nbr, char *buf, u
return;
}
 
-   bzero(, sizeof(p));
+   memset(, 0, sizeof(p));
 
prune = (struct prune_hdr *)buf;
 
Index: rde_mfc.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/rde_mfc.c,v
retrieving revision 1.9
diff -u -p -r1.9 rde_mfc.c
--- rde_mfc.c   6 Apr 2011 11:36:26 -   1.9
+++ rde_mfc.c   7 Dec 2015 19:03:15 -
@@ -251,7 +251,7 @@ mfc_send_prune(struct rt_node *rn, struc
 {
struct prunep;
 
-   bzero(, sizeof(p));
+   memset(, 0, sizeof(p));
 
p.origin.s_addr = (mn->origin.s_addr &
htonl(prefixlen2mask(rn->prefixlen)));

Re: ugenctl for attaching USB devices to ugen instead of their specific driver

[Martin Pieuchot]

On 07/12/15(Mon) 13:19, Mickael Torres wrote:
> [...] 
> Actually the device attaches as uhid. I don't know which kind of transfer
> is used, I'll have to look into this, but the soft (pk2cmd) uses libusb
> and only works when the device is attached as ugen.

That's easy because the uhid(4) driver does nothing but offers an
userland interface.

> And another soft (pic32prog) which also uses libusb seems to only detect
> the device when it is attached as uhid.

This generally means it uses the libusbhid.  Could you try to build the
port using libusb instead?

Re: dhclient hang

[Kenneth Westerback]

I don't understand what you mean by hang? Does it hang the box? I would
expect dhclient to reject the offer and try again. Does dhclient never try
again?

I'll take a closer look tomorrow, but if it isn't now it should be easy to
fix dhclient to try again for a different lease.

 Ken
On 7 Dec 2015 10:57, "Martin Pieuchot"  wrote:

> If for some reason SIOCAIFADDR fails, exit instead of hanging.
>
> Without this diff I need to kill dhclient(8) with ctrl+C during boot
> if I happen to have the address offered by the server configured on
> a different interface.
>
> WARNING: do not try to do that on -current without my rt_delete diffs
> or your kernel with panic!
>
> Ok?
>
> Index: kroute.c
> ===
> RCS file: /cvs/src/sbin/dhclient/kroute.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 kroute.c
> --- kroute.c10 Feb 2015 04:20:26 -  1.75
> +++ kroute.c7 Dec 2015 15:51:17 -
> @@ -473,7 +473,7 @@ priv_add_address(struct imsg_add_address
> /* No need to set broadcast address. Kernel can figure it out. */
>
> if (ioctl(s, SIOCAIFADDR, ) == -1)
> -   warning("SIOCAIFADDR failed (%s): %s",
> inet_ntoa(imsg->addr),
> +   error("SIOCAIFADDR failed (%s): %s", inet_ntoa(imsg->addr),
> strerror(errno));
>
> close(s);
>

Re: dhclient hang

[Bob Beck]

I think trying again for a different lease is the right thing..  I'm
really tired of dhclient exiting for stupid reasons ;)

On Mon, Dec 7, 2015 at 3:11 PM, Kenneth Westerback
 wrote:
> I don't understand what you mean by hang? Does it hang the box? I would
> expect dhclient to reject the offer and try again. Does dhclient never try
> again?
>
> I'll take a closer look tomorrow, but if it isn't now it should be easy to
> fix dhclient to try again for a different lease.
>
>  Ken
> On 7 Dec 2015 10:57, "Martin Pieuchot"  wrote:
>
>> If for some reason SIOCAIFADDR fails, exit instead of hanging.
>>
>> Without this diff I need to kill dhclient(8) with ctrl+C during boot
>> if I happen to have the address offered by the server configured on
>> a different interface.
>>
>> WARNING: do not try to do that on -current without my rt_delete diffs
>> or your kernel with panic!
>>
>> Ok?
>>
>> Index: kroute.c
>> ===
>> RCS file: /cvs/src/sbin/dhclient/kroute.c,v
>> retrieving revision 1.75
>> diff -u -p -r1.75 kroute.c
>> --- kroute.c10 Feb 2015 04:20:26 -  1.75
>> +++ kroute.c7 Dec 2015 15:51:17 -
>> @@ -473,7 +473,7 @@ priv_add_address(struct imsg_add_address
>> /* No need to set broadcast address. Kernel can figure it out. */
>>
>> if (ioctl(s, SIOCAIFADDR, ) == -1)
>> -   warning("SIOCAIFADDR failed (%s): %s",
>> inet_ntoa(imsg->addr),
>> +   error("SIOCAIFADDR failed (%s): %s", inet_ntoa(imsg->addr),
>> strerror(errno));
>>
>> close(s);
>>

Re: [patch] dvmrpd: strings header cleanup

[Michael McConville]

Serguey Parkhomovsky wrote:
> Fixes implicit memcpy declarations by using string.h instead of
> strings.h, and removes strings.h from files that don't need it. Also,
> change bzero(3) to memset(3).

This is all committed now. Thanks!

> Index: ask_nbrs2.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/ask_nbrs2.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 ask_nbrs2.c
> --- ask_nbrs2.c 5 May 2015 01:26:37 -   1.4
> +++ ask_nbrs2.c 7 Dec 2015 18:20:06 -
> @@ -23,7 +23,6 @@
>  #include 
>  
>  #include 
> -#include 
>  
>  #include "igmp.h"
>  #include "dvmrpd.h"
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/control.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 control.c
> --- control.c   5 Dec 2015 13:11:00 -   1.21
> +++ control.c   7 Dec 2015 18:20:06 -
> @@ -52,7 +52,7 @@ control_init(void)
> return (-1);
> }
>  
> -   bzero(, sizeof(sun));
> +   memset(, 0, sizeof(sun));
> sun.sun_family = AF_UNIX;
> strlcpy(sun.sun_path, DVMRPD_SOCKET, sizeof(sun.sun_path));
>  
> Index: graft.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/graft.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 graft.c
> --- graft.c 5 May 2015 01:26:37 -   1.4
> +++ graft.c 7 Dec 2015 18:20:06 -
> @@ -23,7 +23,6 @@
>  #include 
>  
>  #include 
> -#include 
>  
>  #include "igmp.h"
>  #include "dvmrpd.h"
> Index: graft_ack.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/graft_ack.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 graft_ack.c
> --- graft_ack.c 5 May 2015 01:26:37 -   1.4
> +++ graft_ack.c 7 Dec 2015 18:20:06 -
> @@ -23,7 +23,6 @@
>  #include 
>  
>  #include 
> -#include 
>  
>  #include "igmp.h"
>  #include "dvmrpd.h"
> Index: igmp.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/igmp.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 igmp.c
> --- igmp.c  18 Nov 2014 20:54:28 -  1.3
> +++ igmp.c  7 Dec 2015 18:20:06 -
> @@ -51,7 +51,7 @@ send_igmp_query(struct iface *iface, str
> fatal("send_igmp_query");
>  
> /* IGMP header */
> -   bzero(_hdr, sizeof(igmp_hdr));
> +   memset(_hdr, 0, sizeof(igmp_hdr));
> igmp_hdr.type = PKT_TYPE_MEMBER_QUERY;
>  
> if (group == NULL) {
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/kroute.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 kroute.c
> --- kroute.c27 Sep 2015 17:29:46 -  1.12
> +++ kroute.c7 Dec 2015 18:20:06 -
> @@ -136,7 +136,7 @@ kif_find(int ifindex)
>  {
> struct kif_node s;
>  
> -   bzero(, sizeof(s));
> +   memset(, 0, sizeof(s));
> s.k.ifindex = ifindex;
>  
> return (RB_FIND(kif_tree, , ));
> Index: nbrs2.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/nbrs2.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 nbrs2.c
> --- nbrs2.c 5 May 2015 01:26:37 -   1.4
> +++ nbrs2.c 7 Dec 2015 18:20:06 -
> @@ -23,7 +23,6 @@
>  #include 
>  
>  #include 
> -#include 
>  
>  #include "igmp.h"
>  #include "dvmrpd.h"
> Index: packet.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/packet.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 packet.c
> --- packet.c25 Oct 2014 03:23:49 -  1.3
> +++ packet.c7 Dec 2015 18:20:07 -
> @@ -28,7 +28,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  
>  #include "igmp.h"
>  #include "dvmrpd.h"
> @@ -48,7 +48,7 @@ gen_dvmrp_hdr(struct ibuf *buf, struct i
>  {
> struct dvmrp_hdrdvmrp_hdr;
>  
> -   bzero(_hdr, sizeof(dvmrp_hdr));
> +   memset(_hdr, 0, sizeof(dvmrp_hdr));
> dvmrp_hdr.type = PKT_TYPE_DVMRP;
> dvmrp_hdr.code = code;
> dvmrp_hdr.chksum = 0;   /* updated later */
> Index: prune.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/prune.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 prune.c
> --- prune.c 5 May 2015 01:26:37 -   1.5
> +++ prune.c 7 Dec 2015 18:20:07 -
> @@ -23,7 +23,7 @@
>  #include 
>  
>  #include 
> -#include 
> +#include 
>  
>  #include "igmp.h"
>  #include "dvmrpd.h"
> @@ -47,7 +47,7 @@ send_prune(struct nbr *nbr, struct prune
> if (nbr->iface->passive)
> return (0);
>  
> -   bzero(, sizeof(prune));
> +   memset(, 0, sizeof(prune));
>  
> dst.sin_family = AF_INET;
> dst.sin_len = sizeof(struct sockaddr_in);
> @@ -97,7 +97,7 @@ recv_prune(struct nbr *nbr, char *buf, u
> return;
> }
>  
> -   bzero(, sizeof(p));
> +   memset(, 0, sizeof(p));
>  
> prune = (struct prune_hdr *)buf;
>  
> Index: rde_mfc.c
> 

Re: ugenctl for attaching USB devices to ugen instead of their specific driver

[Kirill Bychkov]

On Sun, December 6, 2015 22:10, Ian Darwin wrote:
>
>
> On 2015-12-06 12:23 PM, Stuart Henderson wrote:
>> On 2015/12/06 06:02, Mickael Torres wrote:
>>> Hello,
>>>
>>> This is a kernel patch plus a utility called ugenctl I use to allow
>>> selected USB devices to attach as ugen(4) instead of their more specific
>>> driver. My use case is a Microchip "PICkit 2 Microcontroller Programmer"
>>> that attaches as uhid(4), but the command line utility pk2cmd wants a
>>> udev(4) device. There maybe are some other use cases.
>> If a device is generally pointless with uhid, we can just knock it
>> out completely in the kernel, see the UQ_BAD_HID mechanism. I think this
>> applies to your device.
>>
>> The bigger problem is "dual use" devices; e.g. some want UPS to attach
>> to upd(4), others want ugen(4) for use with NUT/apcupsd. Your code is
>> partially useful for these, but because it just changes things at attach,
>> won't survive a reboot - if it could instead force a device to detach
>> and reattach to ugen it would help a lot more cases.
>>
> One fairly common (I believe) use is with USB printers that attach as ulpt
> but CUPS wants as ugen - for these, this is useful as it stands - just
> run it
> from rc. Do people want their UPS to sometimes be upd and other times be
> a ugen? I can see there might be "I want to change this now" cases, but I
> suspect the majority could be done once at each boot and be quite useful.
>
>
I guess no. I have to configure kernel after every update to make my ups
attach as ugen and not upd. So this tool would be useful for me with a config
file so I can configure it once and don't bother on every reboot/update.

Re: [patch] dvmrpd: strings header cleanup

[Claudio Jeker]

On Mon, Dec 07, 2015 at 02:04:18PM -0500, Michael McConville wrote:
> Serguey Parkhomovsky wrote:
> > Fixes implicit memcpy declarations by using string.h instead of
> > strings.h, and removes strings.h from files that don't need it. Also,
> > change bzero(3) to memset(3).
> 
> Thanks for this.
> 
> I just took care of the existing implicit declarations. Below is a diff
> for the rest. I think you might have missed a couple bzero's.
> 
> I'll look into the include removals soon. In a rush at the moment.
> 
> ok?
> 

OK claudio@

> 
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/control.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 control.c
> --- control.c 5 Dec 2015 13:11:00 -   1.21
> +++ control.c 7 Dec 2015 19:03:15 -
> @@ -52,7 +52,7 @@ control_init(void)
>   return (-1);
>   }
>  
> - bzero(, sizeof(sun));
> + memset(, 0, sizeof(sun));
>   sun.sun_family = AF_UNIX;
>   strlcpy(sun.sun_path, DVMRPD_SOCKET, sizeof(sun.sun_path));
>  
> Index: igmp.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/igmp.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 igmp.c
> --- igmp.c18 Nov 2014 20:54:28 -  1.3
> +++ igmp.c7 Dec 2015 19:03:15 -
> @@ -51,7 +51,7 @@ send_igmp_query(struct iface *iface, str
>   fatal("send_igmp_query");
>  
>   /* IGMP header */
> - bzero(_hdr, sizeof(igmp_hdr));
> + memset(_hdr, 0, sizeof(igmp_hdr));
>   igmp_hdr.type = PKT_TYPE_MEMBER_QUERY;
>  
>   if (group == NULL) {
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/kroute.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 kroute.c
> --- kroute.c  27 Sep 2015 17:29:46 -  1.12
> +++ kroute.c  7 Dec 2015 19:03:15 -
> @@ -136,7 +136,7 @@ kif_find(int ifindex)
>  {
>   struct kif_node s;
>  
> - bzero(, sizeof(s));
> + memset(, 0, sizeof(s));
>   s.k.ifindex = ifindex;
>  
>   return (RB_FIND(kif_tree, , ));
> Index: packet.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/packet.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 packet.c
> --- packet.c  7 Dec 2015 18:59:31 -   1.4
> +++ packet.c  7 Dec 2015 19:03:15 -
> @@ -48,7 +48,7 @@ gen_dvmrp_hdr(struct ibuf *buf, struct i
>  {
>   struct dvmrp_hdrdvmrp_hdr;
>  
> - bzero(_hdr, sizeof(dvmrp_hdr));
> + memset(_hdr, 0, sizeof(dvmrp_hdr));
>   dvmrp_hdr.type = PKT_TYPE_DVMRP;
>   dvmrp_hdr.code = code;
>   dvmrp_hdr.chksum = 0;   /* updated later */
> Index: prune.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/prune.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 prune.c
> --- prune.c   5 May 2015 01:26:37 -   1.5
> +++ prune.c   7 Dec 2015 19:03:15 -
> @@ -23,7 +23,7 @@
>  #include 
>  
>  #include 
> -#include 
> +#include 
>  
>  #include "igmp.h"
>  #include "dvmrpd.h"
> @@ -47,7 +47,7 @@ send_prune(struct nbr *nbr, struct prune
>   if (nbr->iface->passive)
>   return (0);
>  
> - bzero(, sizeof(prune));
> + memset(, 0, sizeof(prune));
>  
>   dst.sin_family = AF_INET;
>   dst.sin_len = sizeof(struct sockaddr_in);
> @@ -97,7 +97,7 @@ recv_prune(struct nbr *nbr, char *buf, u
>   return;
>   }
>  
> - bzero(, sizeof(p));
> + memset(, 0, sizeof(p));
>  
>   prune = (struct prune_hdr *)buf;
>  
> Index: rde_mfc.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/rde_mfc.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 rde_mfc.c
> --- rde_mfc.c 6 Apr 2011 11:36:26 -   1.9
> +++ rde_mfc.c 7 Dec 2015 19:03:15 -
> @@ -251,7 +251,7 @@ mfc_send_prune(struct rt_node *rn, struc
>  {
>   struct prunep;
>  
> - bzero(, sizeof(p));
> + memset(, 0, sizeof(p));
>  
>   p.origin.s_addr = (mn->origin.s_addr &
>   htonl(prefixlen2mask(rn->prefixlen)));
> 

-- 
:wq Claudio

Re: [patch] Convert modulus to arc4random_uniform

[Matthew Martin]

On Mon, Dec 07, 2015 at 09:33:47AM +0100, Theo Buehler wrote:
> I think some of these are ok, but I'm unsure about some of the others.
> Here are some of my concerns:
> 
> - since arc4random_uniform can potentially loop indefinitely, it
>   might interfere with predictable timing of some routines.  I can't
>   tell if this is harmless in all cases below.  This applies in
>   particular to the proposed changes in the kernel.

I hadn't considered timing problems. I'll look at it again tonight, but
someone more familiar with the code should certainly look at it before
committing.

> - changing random() to arc4random() in games might have undesired
>   side-effects like interfering with the reproducibility of tests or
>   games.  I think this might apply to awk for the same reason as in the
>   shells.  

The patch for awk was wrong. Updated patch below. I'll look into hack
tonight when I have more time.


> > Index: lib/libc/stdlib/rand.c
> > ===
> > RCS file: /cvs/src/lib/libc/stdlib/rand.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 rand.c
> > --- lib/libc/stdlib/rand.c  13 Sep 2015 08:31:47 -  1.15
> > +++ lib/libc/stdlib/rand.c  7 Dec 2015 06:42:17 -
> > @@ -50,7 +50,7 @@ int
> >  rand(void)
> >  {
> > if (rand_deterministic == 0)
> > -   return (arc4random() % ((u_int)RAND_MAX + 1));
> > +   return (arc4random_uniform((u_int)RAND_MAX + 1));
> > return (rand_r());
> >  }
> >  
> 
> this is modulo 2^n, so I think this one is fine as it is.

It's safe but takes a bit of thinking. I first had it as
return (arc4random() & RAND_MAX);
which to me is more obviously correct, but since it's safe as is. I have
no strong opinion on this.


> > Index: usr.bin/awk/run.c
> > ===
> 
> Unsure about this one.  I think deterministic sequences might be desired
> in some circumstances (this one is deterministic when a seed was given).

theo@ also pointed out that awk can be deterministic. Since RAND_MAX is
1 below a power of 2, & is safe. How about

Index: run.c
===
RCS file: /cvs/src/usr.bin/awk/run.c,v
retrieving revision 1.39
diff -u -p -r1.39 run.c
--- run.c   5 Sep 2015 22:07:10 -   1.39
+++ run.c   7 Dec 2015 19:28:31 -
@@ -1581,7 +1581,7 @@ Cell *bltin(Node **a, int n)  /* builtin 
u = (Awkfloat) system(getsval(x)) / 256;   /* 256 is unix-dep */
break;
case FRAND:
-   u = (Awkfloat) (random() % RAND_MAX) / RAND_MAX;
+   u = (Awkfloat) (random() & RAND_MAX) / ((u_int)RAND_MAX + 1);
break;
case FSRAND:
if (isrec(x)) { /* no argument provided */

Re: [patch] Convert modulus to arc4random_uniform

[Theo Buehler]

> I'll look into hack tonight when I have more time.

Honestly, I would prefer to leave hack as it is right now since it will
take some work to repair it anyway.  I would not want to add another
layer of (potential) complications.

> > > Index: lib/libc/stdlib/rand.c
> > > ===

> It's safe but takes a bit of thinking. I first had it as
>   return (arc4random() & RAND_MAX);
> which to me is more obviously correct, but since it's safe as is. I have
> no strong opinion on this.

I have a mild preference for this version, and I think this version
would be preferable from the point of view of uniformity of usage,
especially after your patches have gone in.

> > > Index: usr.bin/awk/run.c
> > > ===
> > 
> > Unsure about this one.  I think deterministic sequences might be desired
> > in some circumstances (this one is deterministic when a seed was given).
> 
> theo@ also pointed out that awk can be deterministic. Since RAND_MAX is
> 1 below a power of 2, & is safe. How about
> 
> Index: run.c
> ===
> RCS file: /cvs/src/usr.bin/awk/run.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 run.c
> --- run.c 5 Sep 2015 22:07:10 -   1.39
> +++ run.c 7 Dec 2015 19:28:31 -
> @@ -1581,7 +1581,7 @@ Cell *bltin(Node **a, int n)/* builtin 
>   u = (Awkfloat) system(getsval(x)) / 256;   /* 256 is unix-dep */
>   break;
>   case FRAND:
> - u = (Awkfloat) (random() % RAND_MAX) / RAND_MAX;
> + u = (Awkfloat) (random() & RAND_MAX) / ((u_int)RAND_MAX + 1);
>   break;
>   case FSRAND:
>   if (isrec(x)) { /* no argument provided */
> 

ok from me on this one.

Re: [vi] Remove needless (m|c)alloc aliases

[Theo Buehler]

On Mon, Dec 07, 2015 at 03:44:12PM -0500, Michael McConville wrote:
> No binary change. ok?

ok tb@

Re: Do not pass NULL to rtdeletemsg()

[Vincent Gross]

On 12/07/15 14:57, Martin Pieuchot wrote:
> If the interface is gone that means you're dealing with a cached route
> so there's no need to try to remove it from the table.
> 
> Better be explicit and do that before calling rtdeletemsg() rather than
> inside.
> 
> ok?

ok vgross@

> 
> Index: netinet/ip_icmp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
> retrieving revision 1.150
> diff -u -p -r1.150 ip_icmp.c
> --- netinet/ip_icmp.c 3 Dec 2015 21:11:53 -   1.150
> +++ netinet/ip_icmp.c 7 Dec 2015 12:40:06 -
> @@ -1042,19 +1042,21 @@ icmp_mtudisc(struct icmp *icp, u_int rta
>  void
>  icmp_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
>  {
> - if (rt == NULL)
> - panic("icmp_mtudisc_timeout:  bad route to timeout");
> + struct ifnet *ifp;
> + int s;
>  
> - if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
> - (RTF_DYNAMIC | RTF_HOST)) {
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + return;
> +
> + if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
>   void *(*ctlfunc)(int, struct sockaddr *, u_int, void *);
>   struct sockaddr_in sin;
> - int s;
>  
>   sin = *satosin(rt_key(rt));
>  
>   s = splsoftnet();
> - rtdeletemsg(rt, NULL, r->rtt_tableid);
> + rtdeletemsg(rt, ifp, r->rtt_tableid);
>  
>   /* Notify TCP layer of increased Path MTU estimate */
>   ctlfunc = inetsw[ip_protox[IPPROTO_TCP]].pr_ctlinput;
> @@ -1062,9 +1064,12 @@ icmp_mtudisc_timeout(struct rtentry *rt,
>   (*ctlfunc)(PRC_MTUINC, sintosa(),
>   r->rtt_tableid, NULL);
>   splx(s);
> - } else
> + } else {
>   if ((rt->rt_rmx.rmx_locks & RTV_MTU) == 0)
>   rt->rt_rmx.rmx_mtu = 0;
> + }
> +
> + if_put(ifp);
>  }
>  
>  /*
> @@ -1088,17 +1093,20 @@ icmp_ratelimit(const struct in_addr *dst
>  void
>  icmp_redirect_timeout(struct rtentry *rt, struct rttimer *r)
>  {
> - if (rt == NULL)
> - panic("icmp_redirect_timeout:  bad route to timeout");
> + struct ifnet *ifp;
> + int s;
>  
> - if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
> - (RTF_DYNAMIC | RTF_HOST)) {
> - int s;
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + return;
>  
> + if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
>   s = splsoftnet();
> - rtdeletemsg(rt, NULL, r->rtt_tableid);
> + rtdeletemsg(rt, ifp, r->rtt_tableid);
>   splx(s);
>   }
> +
> + if_put(ifp);
>  }
>  
>  int
> Index: netinet6/icmp6.c
> ===
> RCS file: /cvs/src/sys/netinet6/icmp6.c,v
> retrieving revision 1.182
> diff -u -p -r1.182 icmp6.c
> --- netinet6/icmp6.c  3 Dec 2015 21:11:53 -   1.182
> +++ netinet6/icmp6.c  7 Dec 2015 12:39:28 -
> @@ -1952,34 +1952,42 @@ icmp6_mtudisc_clone(struct sockaddr *dst
>  void
>  icmp6_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
>  {
> - if (rt == NULL)
> - panic("icmp6_mtudisc_timeout: bad route to timeout");
> - if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
> - (RTF_DYNAMIC | RTF_HOST)) {
> - int s;
> + struct ifnet *ifp;
> + int s;
>  
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + return;
> +
> + if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
>   s = splsoftnet();
> - rtdeletemsg(rt, NULL, r->rtt_tableid);
> + rtdeletemsg(rt, ifp, r->rtt_tableid);
>   splx(s);
>   } else {
>   if (!(rt->rt_rmx.rmx_locks & RTV_MTU))
>   rt->rt_rmx.rmx_mtu = 0;
>   }
> +
> + if_put(ifp);
>  }
>  
>  void
>  icmp6_redirect_timeout(struct rtentry *rt, struct rttimer *r)
>  {
> - if (rt == NULL)
> - panic("icmp6_redirect_timeout: bad route to timeout");
> - if ((rt->rt_flags & (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) ==
> - (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) {
> - int s;
> + struct ifnet *ifp;
> + int s;
>  
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + return;
> +
> + if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
>   s = splsoftnet();
> - rtdeletemsg(rt, NULL, r->rtt_tableid);
> + rtdeletemsg(rt, ifp, r->rtt_tableid);
>   splx(s);
>   }
> +
> + if_put(ifp);
>  }
>  
>  int *icmpv6ctl_vars[ICMPV6CTL_MAXID] = ICMPV6CTL_VARS;
> 

[patch] mailwrapper: remove broken fallback code

[Serguey Parkhomovsky]

If /etc/mailer.conf doesn't exist, mailwrapper tries to run sendmail,
giving a confusing error message:

mailwrapper: cannot exec /usr/libexec/sendmail/sendmail: No such
file or directory

This patch removes this fallback code. I believe this is cleaner than
updating the fallback since we would have to put two paths in: one for
sendmail/send-mail/mailq and one for makemap/newaliases.

Index: mailwrapper.c
===
RCS file: /cvs/src/usr.sbin/mailwrapper/mailwrapper.c,v
retrieving revision 1.20
diff -u -p -r1.20 mailwrapper.c
--- mailwrapper.c   12 Oct 2015 22:01:08 -  1.20
+++ mailwrapper.c   7 Dec 2015 21:33:57 -
@@ -36,12 +36,10 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
 #define _PATH_MAILERCONF   "/etc/mailer.conf"
-#define _PATH_DEFAULTMTA   "/usr/libexec/sendmail/sendmail"
 
 struct arglist {
size_t argc, maxc;
@@ -100,21 +98,11 @@ main(int argc, char *argv[], char *envp[
for (len = 0; len < argc; len++)
addarg(, argv[len], 0);
 
-   config = fopen(_PATH_MAILERCONF, "r");
+   if ((config = fopen(_PATH_MAILERCONF, "r")) == NULL)
+   err(1, "cannot open %s", _PATH_MAILERCONF);
 
if (pledge("stdio exec", NULL) == -1)
err(1, "pledge");
-
-   if (config == NULL) {
-   addarg(, NULL, 0);
-   openlog(__progname, LOG_PID, LOG_MAIL);
-   syslog(LOG_INFO, "cannot open %s, using %s as default MTA",
-   _PATH_MAILERCONF, _PATH_DEFAULTMTA);
-   closelog();
-   execve(_PATH_DEFAULTMTA, al.argv, envp);
-   err(1, "cannot exec %s", _PATH_DEFAULTMTA);
-   /*NOTREACHED*/
-   }
 
for (;;) {
if ((line = fparseln(config, , , NULL, 0)) == NULL) {

Re: [patch] fix urtw on big-endian CPU (PPC)

[Stefan Sperling]

On Mon, Dec 07, 2015 at 09:31:11PM +0100, Cédric TESSIER wrote:
> Hi,
> 
> I'm bringing my old Mac Mini G4 back to life, and I had an issue with a
> wireless dongle (RTL8187 rev 0x04, RFv2).
> 
> urtw interface was available, but wasn't working at all.
> 
> Investigations highlighted an endianness related issue.
> 
> I've written a quick and dirty patch which make the interface fully working
> again.
> 
> I've tested it on PPC and i386 (5.8 and snapshot).
> 
> 
> Cédric

Thanks. I do have the necessary hardware to test this.
I'll try to get that done this week and get your fix committed.

> 
> 
> 
> --- if_urtw.c.orig2015-11-25 04:10:00.0 +0100
> +++ if_urtw.c 2015-12-07 11:40:05.0 +0100
> @@ -1078,6 +1078,7 @@
>   USETW(req.wIndex, index);
>   USETW(req.wLength, sizeof(uint16_t));
>  
> + data = htole16(data);   
>   return (usbd_do_request(sc->sc_udev, , ));
>  }
>  
> @@ -1587,6 +1588,7 @@
>   USETW(req.wLength, sizeof(uint16_t));
>  
>   error = usbd_do_request(sc->sc_udev, , data);
> + *data = letoh16(*data);
>   return (error);
>  }
>  
> @@ -1603,6 +1605,7 @@
>   USETW(req.wLength, sizeof(uint32_t));
>  
>   error = usbd_do_request(sc->sc_udev, , data);
> + *data = letoh32(*data);
>   return (error);
>  }
>  
> @@ -1645,6 +1648,7 @@
>   USETW(req.wIndex, idx & 0x03);
>   USETW(req.wLength, sizeof(uint16_t));
>  
> + data = htole16(data);   
>   return (usbd_do_request(sc->sc_udev, , ));
>  }
>  
> @@ -1659,6 +1663,7 @@
>   USETW(req.wIndex, idx & 0x03);
>   USETW(req.wLength, sizeof(uint32_t));
>  
> + data = htole32(data);   
>   return (usbd_do_request(sc->sc_udev, , ));
>  }
>  
> 

patch 2/4 add generic keyboard backlight support

[Joerg Jung]

Hi,

here comes the second part of the series for generic keyboard backlight
support.

Please find below the userland part which adds a backlight variable to
wsconsctl(8).

I have chosen to use the percentage format, so it looks like this:

   $ doas wsconsctl keyboard.backlight=80  
   keyboard.backlight -> 80.00%

Comments, OK?

Thanks,
Regards,
Joerg


Index: keyboard.c
===
RCS file: /cvs/src/sbin/wsconsctl/keyboard.c,v
retrieving revision 1.12
diff -u -p -r1.12 keyboard.c
--- keyboard.c  21 Mar 2013 06:04:05 -  1.12
+++ keyboard.c  7 Dec 2015 21:03:22 -
@@ -50,6 +50,7 @@ static struct wskbd_keyrepeat_data repea
 static struct wskbd_keyrepeat_data dfrepeat;
 static int ledstate;
 static kbd_t kbdencoding;
+static struct field_pc backlight;
 
 struct field keyboard_field_tab[] = {
 { "type",  ,FMT_KBDTYPE,FLG_RDONLY },
@@ -66,12 +67,15 @@ struct field keyboard_field_tab[] = {
 { "repeat.deln.default",   , FMT_UINT,   FLG_MODIFY },
 { "ledstate",  ,  FMT_UINT,   0 },
 { "encoding",  ,   FMT_KBDENC, FLG_MODIFY },
+{ "backlight", , FMT_PC, 
FLG_MODIFY|FLG_INIT },
 { NULL }
 };
 
 void
 keyboard_get_values(int fd)
 {
+   struct wskbd_backlight kbl;
+
if (field_by_value(keyboard_field_tab, )->flags & FLG_GET)
if (ioctl(fd, WSKBDIO_GTYPE, ) < 0)
warn("WSKBDIO_GTYPE");
@@ -131,11 +135,24 @@ keyboard_get_values(int fd)
if (field_by_value(keyboard_field_tab, )->flags & FLG_GET)
if (ioctl(fd, WSKBDIO_GETENCODING, ) < 0)
warn("WSKBDIO_GETENCODING");
+
+   if (field_by_value(keyboard_field_tab, )->flags & FLG_GET) {
+   if (ioctl(fd, WSKBDIO_GETBACKLIGHT, ) < 0)
+   warn("WSKBDIO_GETBACKLIGHT");
+   else {
+backlight.min = kbl.min;
+backlight.cur = kbl.curval;
+backlight.max = kbl.max;
+   }
+   }
+   
 }
 
 int
 keyboard_put_values(int fd)
 {
+   struct wskbd_backlight kbl;
+
bell.which = 0;
if (field_by_value(keyboard_field_tab, )->flags & FLG_SET)
bell.which |= WSKBD_BELL_DOPITCH;
@@ -200,6 +217,16 @@ keyboard_put_values(int fd)
if (field_by_value(keyboard_field_tab, )->flags & FLG_SET) {
if (ioctl(fd, WSKBDIO_SETENCODING, ) < 0) {
warn("WSKBDIO_SETENCODING");
+   return 1;
+   }
+   }
+
+   if (field_by_value(keyboard_field_tab, )->flags & FLG_SET) {
+   kbl.min = backlight.min;
+   kbl.curval = backlight.cur;
+   kbl.max = backlight.max;
+   if (ioctl(fd, WSKBDIO_SETBACKLIGHT, ) < 0) {
+   warn("WSKBDIO_SETBACKLIGHT");
return 1;
}
}

patch 1/4 add generic keyboard backlight support

[Joerg Jung]

Hi,

here comes a series of small diffs which add generic support for
keyboard backlights.

Please find below the first diff, which adds new ioctls to wskbd(4) to
control keyboard backlights.

In contrast to an earlier diff from jcs, I have chosen to use a struct
in favor of a simple (unsigned) int as depending on the vendor
(Thinkpad, Apple, ...) different min/max values for the brightness of
the keyboard backlight are possible.

Comments, OK?

Thanks,
Regards,
Joerg



Index: wsconsio.h
===
RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.72
diff -u -p -r1.72 wsconsio.h
--- wsconsio.h  30 Aug 2015 10:05:09 -  1.72
+++ wsconsio.h  7 Dec 2015 21:03:45 -
@@ -180,6 +180,13 @@ struct wskbd_map_data {
 #define WSKBDIO_GETENCODING_IOR('W', 15, kbd_t)
 #define WSKBDIO_SETENCODING_IOW('W', 16, kbd_t)
 
+/* Get/set keyboard backlight.  Not applicable to all keyboard types. */
+struct wskbd_backlight {
+   unsigned int min, max, curval;
+};
+#defineWSKBDIO_GETBACKLIGHT_IOR('W', 17, struct wskbd_backlight)
+#defineWSKBDIO_SETBACKLIGHT_IOW('W', 18, struct wskbd_backlight)
+
 /* internal use only */
 #define WSKBDIO_SETMODE_IOW('W', 19, int)
 #define WSKBDIO_GETMODE_IOR('W', 20, int)
Index: wskbd.c
===
RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
retrieving revision 1.82
diff -u -p -r1.82 wskbd.c
--- wskbd.c 10 Sep 2015 18:14:52 -  1.82
+++ wskbd.c 7 Dec 2015 21:03:45 -
@@ -230,6 +230,9 @@ int wskbd_mux_close(struct wsevsrc *);
 intwskbd_do_open(struct wskbd_softc *, struct wseventvar *);
 intwskbd_do_ioctl(struct device *, u_long, caddr_t, int, struct proc *);
 
+int(*wskbd_get_backlight)(struct wskbd_backlight *);
+int(*wskbd_set_backlight)(struct wskbd_backlight *);
+
 struct cfdriver wskbd_cd = {
NULL, "wskbd", DV_TTY
 };
@@ -1010,6 +1013,7 @@ wskbd_displayioctl(struct device *dev, u
case WSKBDIO_SETDEFAULTKEYREPEAT:
case WSKBDIO_SETMAP:
case WSKBDIO_SETENCODING:
+   case WSKBDIO_SETBACKLIGHT:
if ((flag & FWRITE) == 0)
return (EACCES);
}
@@ -1155,6 +1159,18 @@ getkeyrepeat:
wsmux_set_layout(sc->sc_base.me_parent, enc);
 #endif
return (0);
+
+   case WSKBDIO_GETBACKLIGHT:
+   if (wskbd_get_backlight != NULL)
+   return (*wskbd_get_backlight)((struct wskbd_backlight 
*)data);
+   error = ENOTTY;
+   break;
+
+   case WSKBDIO_SETBACKLIGHT:
+   if (wskbd_set_backlight != NULL)
+   return (*wskbd_set_backlight)((struct wskbd_backlight 
*)data);
+   error = ENOTTY;
+   break;
}
 
/*

patch 3/4 add generic keyboard backlight support

[Joerg Jung]

Hi,

here comes the third part of the series for generic keyboard backlight
support.

Please find below a diff which adds they key(code)s for keyboard
backlight control, as found on all recent Intel based Apple Laptop
Keyboards.  While here, also add keys for display brightness control
found on Apple Keyboards as well.

This is based on a similar diff from Sven-Volker Nowarra and is a NOOP
for now, as these keys are not used yet. Using them is target for a
later diff.

I'm not familar with keycodes, most of the diff below is 'educated
guess', so please, hints are welcome!

Comments, OK?

Thanks,
Regards,
Joerg


Index: pckbc/wskbdmap_mfii.c
===
RCS file: /cvs/src/sys/dev/pckbc/wskbdmap_mfii.c,v
retrieving revision 1.43
diff -u -p -r1.43 wskbdmap_mfii.c
--- pckbc/wskbdmap_mfii.c   14 Apr 2013 19:32:52 -  1.43
+++ pckbc/wskbdmap_mfii.c   7 Dec 2015 22:22:27 -
@@ -149,6 +149,10 @@ static const keysym_t pckbd_keydesc_us[]
 KC(170),   KS_Print_Screen,
 KC(174),   KS_AudioLower,
 KC(176),   KS_AudioRaise,
+KC(177),   KS_BrightnessDown,
+KC(178),   KS_BrightnessUp,
+KC(179),   KS_KbdBacklightDown,
+KC(180),   KS_KbdBacklightUp,
 KC(181),   KS_KP_Divide,
 KC(183),   KS_Print_Screen,
 KC(184), KS_Cmd2,  KS_Alt_R,   KS_Multi_key,
Index: wscons/wsksymdef.h
===
RCS file: /cvs/src/sys/dev/wscons/wsksymdef.h,v
retrieving revision 1.36
diff -u -p -r1.36 wsksymdef.h
--- wscons/wsksymdef.h  26 Jan 2014 17:48:08 -  1.36
+++ wscons/wsksymdef.h  7 Dec 2015 22:22:27 -
@@ -633,6 +633,10 @@
 #define KS_AudioMute   0xf3d1
 #define KS_AudioLower  0xf3d2
 #define KS_AudioRaise  0xf3d3
+#define KS_BrightnessDown  0xf3d4
+#define KS_BrightnessUp0xf3d5
+#define KS_KbdBacklightDown0xf3d6
+#define KS_KbdBacklightUp  0xf3d7
 
 /*
  * Group 4 (command)
Index: usb/makemap.awk
===
RCS file: /cvs/src/sys/dev/usb/makemap.awk,v
retrieving revision 1.14
diff -u -p -r1.14 makemap.awk
--- usb/makemap.awk 20 Nov 2013 17:27:32 -  1.14
+++ usb/makemap.awk 7 Dec 2015 22:22:27 -
@@ -153,6 +153,10 @@ BEGIN {
conv[170] = 70
conv[174] = 129
conv[176] = 128
+   conv[177] = 131
+   conv[178] = 132
+   conv[179] = 133
+   conv[180] = 134
conv[181] = 84
conv[184] = 230
# 198 is #if 0 in the PS/2 map...
Index: usb/ukbd.c
===
RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.71
diff -u -p -r1.71 ukbd.c
--- usb/ukbd.c  14 Mar 2015 03:38:50 -  1.71
+++ usb/ukbd.c  7 Dec 2015 22:22:27 -
@@ -469,13 +469,15 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
static const struct ukbd_translation apple_fn_trans[] = {
{ 40, 73 }, /* return -> insert */
{ 42, 76 }, /* backspace -> delete */
+   { 58, 131 },/* F1 -> screen brightness down */
+   { 59, 132 },/* F2 -> screen brightness up */
 #ifdef notyet
-   { 58, 0 },  /* F1 -> screen brightness down */
-   { 59, 0 },  /* F2 -> screen brightness up */
{ 60, 0 },  /* F3 */
{ 61, 0 },  /* F4 */
-   { 62, 0 },  /* F5 -> keyboard backlight down */
-   { 63, 0 },  /* F6 -> keyboard backlight up */
+#endif
+   { 62, 133 },/* F5 -> keyboard backlight down */
+   { 63, 134 },/* F6 -> keyboard backlight up */
+#ifdef notyet
{ 64, 0 },  /* F7 -> audio back */
{ 65, 0 },  /* F8 -> audio pause/play */
{ 66, 0 },  /* F9 -> audio next */
Index: usb/ukbdmap.c
===
RCS file: /cvs/src/sys/dev/usb/ukbdmap.c,v
retrieving revision 1.41
diff -u -p -r1.41 ukbdmap.c
--- usb/ukbdmap.c   20 Nov 2013 17:28:00 -  1.41
+++ usb/ukbdmap.c   7 Dec 2015 22:22:27 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: ukbdmap.c,v 1.41 2013/11/20 17:28:00 miod Exp $   */
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMAGICALLY GENERATED.  DO NOT EDIT.
@@ -176,6 +176,10 @@ static const keysym_t ukbd_keydesc_us[] 
 KC(127),   KS_AudioMute,
 KC(128),   KS_AudioRaise,
 KC(129),   KS_AudioLower,
+KC(131),   KS_BrightnessDown,
+KC(132),   KS_BrightnessUp,
+KC(133),   KS_KbdBacklightDown,
+KC(134),   KS_KbdBacklightUp,
 KC(224),   KS_Cmd1,KS_Control_L,
 KC(225),   KS_Shift_L,
 KC(226),   KS_Cmd2,KS_Alt_L,

patch 4/4 add keyboard backlight support to asmc(4)

[Joerg Jung]

Hi,

here comes another diff of the series for the keyboard backlight
support.

Please find below a diff which enables keyboard backlight control on
Intel Apple Laptops via asmc(4).

This diff uses introduced wskbd(4) hooks from previously sent diffs and
also introduces locking and minor refactoring around asmc_try().

Comments, tests, OKs?

Thanks,
Regards,
Joerg


Index: asmc.c
===
RCS file: /cvs/src/sys/dev/isa/asmc.c,v
retrieving revision 1.13
diff -u -p -r1.13 asmc.c
--- asmc.c  29 Oct 2015 13:29:04 -  1.13
+++ asmc.c  7 Dec 2015 22:50:02 -
@@ -23,12 +23,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include 
 
 #include 
+#include 
 
 #define ASMC_BASE  0x300   /* SMC base address */
 #define ASMC_IOSIZE32  /* I/O region size 0x300-0x31f */
@@ -66,10 +68,13 @@ struct asmc_softc {
uint8_t  sc_init;   /* initialization done? */
uint8_t  sc_nfans;  /* number of fans */
uint8_t  sc_lightlen;   /* light data len */
+   uint8_t  sc_kbdled; /* backlight led value */
 
+   struct rwlocksc_lock;
struct taskq*sc_taskq;
struct task  sc_task_init;
struct task  sc_task_refresh;
+   struct task  sc_task_backlight;
 
struct ksensor   sc_sensor_temp[ASMC_MAXTEMP];
struct ksensor   sc_sensor_fan[ASMC_MAXFAN];
@@ -81,7 +86,6 @@ struct asmc_softc {
 
 uint8_tasmc_status(struct asmc_softc *);
 intasmc_try(struct asmc_softc *, int, const char *, uint8_t *, uint8_t);
-void   asmc_kbdled(struct asmc_softc *, uint8_t);
 
 void   asmc_init(void *);
 void   asmc_refresh(void *);
@@ -91,6 +95,13 @@ int  asmc_match(struct device *, void *, 
 void   asmc_attach(struct device *, struct device *, void *);
 intasmc_detach(struct device *, int);
 
+/* wskbd hook functions */
+void   asmc_kbdled(void *);
+intasmc_get_backlight(struct wskbd_backlight *);
+intasmc_set_backlight(struct wskbd_backlight *);
+extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
+extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
+
 const struct cfattach asmc_ca = {
sizeof(struct asmc_softc), asmc_match, asmc_attach
 };
@@ -256,7 +267,7 @@ asmc_attach(struct device *parent, struc
struct asmc_softc *sc = (struct asmc_softc *)self;
struct isa_attach_args *ia = aux;
uint8_t buf[6];
-   int i;
+   int i, r;
 
if (bus_space_map(ia->ia_iot, ia->ia_iobase, ia->ia_iosize, 0,
>sc_ioh)) {
@@ -265,23 +276,34 @@ asmc_attach(struct device *parent, struc
}
sc->sc_iot = ia->ia_iot;
 
-   if (asmc_try(sc, ASMC_READ, "REV ", buf, 6)) {
-   printf(": revision failed (0x%x)\n", asmc_status(sc));
+   rw_init(>sc_lock, sc->sc_dev.dv_xname);
+
+   if ((r = asmc_try(sc, ASMC_READ, "REV ", buf, 6))) {
+   printf(": revision failed (0x%x)\n", r);
bus_space_unmap(ia->ia_iot, ia->ia_iobase, ASMC_IOSIZE);
return;
}
printf(": rev %x.%x%x%x", buf[0], buf[1], buf[2],
ntohs(*(uint16_t *)buf + 4));
 
-   if (asmc_try(sc, ASMC_READ, "#KEY", buf, 4)) {
-   printf(", no of keys failed (0x%x)\n", asmc_status(sc));
+   if ((r = asmc_try(sc, ASMC_READ, "#KEY", buf, 4))) {
+   printf(", no of keys failed (0x%x)\n", r);
bus_space_unmap(ia->ia_iot, ia->ia_iobase, ASMC_IOSIZE);
return;
}
printf(", %u key%s\n", ntohl(*(uint32_t *)buf),
(ntohl(*(uint32_t *)buf) == 1) ? "" : "s");
 
-   asmc_kbdled(sc, 127);
+   /* keyboard backlight led is optional */
+   sc->sc_kbdled = buf[0] = 127, buf[1] = 0;
+   if ((r = asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2))) {
+   if (r != ASMC_NOTFOUND)
+   printf("%s: keyboard backlight failed (0x%x)\n",
+   sc->sc_dev.dv_xname, r);
+   } else {
+   wskbd_get_backlight = asmc_get_backlight;
+   wskbd_set_backlight = asmc_set_backlight;
+   }
 
if (!(sc->sc_taskq = taskq_create("asmc", 1, IPL_NONE, 0))) {
printf("%s: can't create task queue\n", sc->sc_dev.dv_xname);
@@ -290,6 +312,7 @@ asmc_attach(struct device *parent, struc
}
task_set(>sc_task_init, asmc_init, sc);
task_set(>sc_task_refresh, asmc_refresh, sc);
+   task_set(>sc_task_backlight, asmc_kbdled, sc);
 
strlcpy(sc->sc_sensor_dev.xname, sc->sc_dev.dv_xname,
sizeof(sc->sc_sensor_dev.xname));
@@ -327,6 +350,7 @@ int
 asmc_detach(struct device *self, int flags)
 {
struct asmc_softc *sc = (struct asmc_softc *)self;
+   uint8_t buf[2] = { (sc->sc_kbdled = 0), 0 };
int i;
 
   

Re: [PATCH] doas authentication type

[Stuart Henderson]

On 2015/11/25 00:14, Stuart Henderson wrote:
> On 2015/11/24 11:24, Richard Johnson wrote:
> > We use 2-factor authn for sudo & doas, as well as for most logins.
> > Presently, we transport Yubikey and other HOTP strings across RADIUS to an
> > otpd authserver
> 
> Interesting...is that a fork of the TRI-D otpd? I found the googlecode
> one and a github export but nothing that seems currently active and
> nothing that supports Yubikey. (I'm on the lookout for things which
> handles central Yubikey auth, none of the programs that I've found so
> far are very appealing).
> 
> > This is on systems with 1200+ user accounts, about 30 active daily.  Users
> > expect that if they can log in as username:radius or username:skey, they
> > should be able to sudo -a radius or sudo -a skey.
> > 
> > Moving away from Kerberos means possible increasing use of sudo or doas by
> > regular users to run transfer commands to data archives.  For this, it would
> > be useful if doas supported "-a skey".  Then I could just use doas; the
> > command is otherwise plain enough.
> > 
> > But that's not a lot of users across the entire OpenBSD installed base.
> > 
> > Installing sudo from ports is still an option. I need to debug the -a
> > failure there now. ;)
> > 
> > 
> > Richard
> > 
> 
> Personally my take on this is that as long as it's just done as -a
> then it's small and simple to implement (pass a string from args to
> auth_userokay), and there's no other way to provide access to this which
> is an important, though lesser-known, part of bsd_auth. We already trust
> auth_userokay with network-supplied strings for this (e.g. as part of
> the username from ssh) so this doesn't seem to add any real exposure
> risk.
> 

Here's an updated version of Renaud's diff against -current after the change
to auth_userchallenge.

Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.46
diff -u -p -r1.46 doas.c
--- doas.c  3 Dec 2015 08:12:15 -   1.46
+++ doas.c  8 Dec 2015 01:26:19 -
@@ -37,7 +37,8 @@
 static void __dead
 usage(void)
 {
-   fprintf(stderr, "usage: doas [-ns] [-C config] [-u user] command 
[args]\n");
+   fprintf(stderr, "usage: doas [-ns] [-a style] [-C config] [-u user]"
+   " command [args]\n");
exit(1);
 }
 
@@ -323,6 +324,7 @@ main(int argc, char **argv, char **envp)
int nflag = 0;
char cwdpath[PATH_MAX];
const char *cwd;
+   char *login_style = NULL;
 
if (pledge("stdio rpath getpw tty proc exec id", NULL) == -1)
err(1, "pledge");
@@ -331,8 +333,11 @@ main(int argc, char **argv, char **envp)
 
uid = getuid();
 
-   while ((ch = getopt(argc, argv, "C:nsu:")) != -1) {
+   while ((ch = getopt(argc, argv, "a:C:nsu:")) != -1) {
switch (ch) {
+   case 'a':
+   login_style = optarg;
+   break;
case 'C':
confpath = optarg;
break;
@@ -412,7 +417,7 @@ main(int argc, char **argv, char **envp)
if (nflag)
errx(1, "Authorization required");
 
-   if (!(as = auth_userchallenge(myname, NULL, "auth-doas",
+   if (!(as = auth_userchallenge(myname, login_style, "auth-doas",
)))
err(1, "auth challenge failed");
if (!challenge) {
Index: doas.1
===
RCS file: /cvs/src/usr.bin/doas/doas.1,v
retrieving revision 1.14
diff -u -p -r1.14 doas.1
--- doas.1  27 Jul 2015 17:57:06 -  1.14
+++ doas.1  8 Dec 2015 01:26:19 -
@@ -22,6 +22,7 @@
 .Sh SYNOPSIS
 .Nm doas
 .Op Fl ns
+.Op Fl a Ar style
 .Op Fl C Ar config
 .Op Fl u Ar user
 .Ar command
@@ -40,6 +41,19 @@ is specified.
 .Pp
 The options are as follows:
 .Bl -tag -width tenletters
++.It Fl a Ar style
++The
++.Fl a
++(authentication style) option causes
++.Nm
++to use the specified authentication style when validating the user,
++as allowed by
++.Pa /etc/login.conf .
++The system administrator may specify a list of doas-specific
++authentication methods by adding an
++.Sq auth-doas
++entry in
++.Pa /etc/login.conf .
 .It Fl C Ar config
 Parse and check the configuration file
 .Ar config ,

vlan doesn thave to handle SIOCGIFADDR

[David Gwynne]

cos the stack can do it for it.

ok?

Index: if_vlan.c
===
RCS file: /cvs/src/sys/net/if_vlan.c,v
retrieving revision 1.149
diff -u -p -r1.149 if_vlan.c
--- if_vlan.c   5 Dec 2015 10:07:55 -   1.149
+++ if_vlan.c   8 Dec 2015 01:00:01 -
@@ -599,16 +599,6 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
error = EINVAL;
break;
 
-   case SIOCGIFADDR:
-   {
-   struct sockaddr *sa;
-
-   sa = (struct sockaddr *)>ifr_data;
-   bcopy(((struct arpcom *)ifp->if_softc)->ac_enaddr,
-   (caddr_t) sa->sa_data, ETHER_ADDR_LEN);
-   }
-   break;
-
case SIOCSIFMTU:
if (ifv->ifv_p != NULL) {
if (ifr->ifr_mtu < ETHERMIN ||

Re: preparing multitouch support - request for tests

[Ulf Brosziewski]

Hi,

here are the diffs again, as tgz archive (it's the version I posted
last week, which doesn't include the latest bug fix: hilms is
incomplete in that version and won't work with tablets, only with
mice).

On 12/07/15 15:59, Matthieu Herrb wrote:

On Thu, Dec 03, 2015 at 12:20:15AM +0100, Ulf Brosziewski wrote:

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



Hi,

it looks like the diff got some white space mangled somewhere on the
path to my mailbox. Would you have a version available somewhere with
a more reliable transport than email ?

Thank you.





wsmouseinput.tgz
Description: Binary data

Does grep need a version?

[Michael McConville]

It's been 0.9 since the original import in 2003...


Index: grep.1
===
RCS file: /cvs/src/usr.bin/grep/grep.1,v
retrieving revision 1.43
diff -u -p -r1.43 grep.1
--- grep.1  13 Jan 2015 04:45:34 -  1.43
+++ grep.1  8 Dec 2015 04:59:35 -
@@ -244,9 +244,6 @@ Nonexistent and unreadable files are ign
 (i.e. their error messages are suppressed).
 .It Fl U
 Search binary files, but do not attempt to print them.
-.It Fl V
-Display version information.
-All other options are ignored.
 .It Fl v
 Selected lines are those
 .Em not
Index: grep.c
===
RCS file: /cvs/src/usr.bin/grep/grep.c,v
retrieving revision 1.55
diff -u -p -r1.55 grep.c
--- grep.c  28 Nov 2015 01:17:12 -  1.55
+++ grep.c  8 Dec 2015 04:59:35 -
@@ -137,7 +137,6 @@ static const struct option long_options[
{"basic-regexp",no_argument,NULL, 'G'},
{"with-filename",   no_argument,NULL, 'H'},
{"binary",  no_argument,NULL, 'U'},
-   {"version", no_argument,NULL, 'V'},
{"text",no_argument,NULL, 'a'},
{"byte-offset", no_argument,NULL, 'b'},
{"count",   no_argument,NULL, 'c'},
@@ -328,10 +327,6 @@ main(int argc, char *argv[])
break;
case 'U':
binbehave = BIN_FILE_BIN;
-   break;
-   case 'V':
-   fprintf(stderr, "grep version %u.%u\n", VER_MAJ, 
VER_MIN);
-   exit(0);
break;
 #ifndef NOZ
case 'Z':
Index: grep.h
===
RCS file: /cvs/src/usr.bin/grep/grep.h,v
retrieving revision 1.23
diff -u -p -r1.23 grep.h
--- grep.h  7 Dec 2015 18:50:06 -   1.23
+++ grep.h  8 Dec 2015 04:59:35 -
@@ -34,9 +34,6 @@
 #include 
 #include 
 
-#define VER_MAJ 0
-#define VER_MIN 9
-
 #define BIN_FILE_BIN   0
 #define BIN_FILE_SKIP  1
 #define BIN_FILE_TEXT  2

Re: 3rd party Xbox 360 USB controller support

[Jeremy Evans]

On 12/07 12:52, Martin Pieuchot wrote:
> On 05/12/15(Sat) 15:22, Christian Heckendorf wrote:
> > The previous thread[1] discussing these controllers includes two
> > patches but they seem to have been merged for the commit in a way
> > that limits support to only Microsoft controllers. 3rd party Xbox 360
> > controllers have their own vendor and product IDs but use the same
> > subclass and protocol as the Microsoft controllers.
> > 
> > Here's a diff based on the first patch that will match controllers
> > and assign the report descriptor more generally using subclass/protocol
> > rather than vendor/product. Is it more correct to create an array
> > of known vendors/products and match against a call to usb_lookup()?
> 
> It's fine since the same check is used in uhidev_use_rdesc().
> 
> Could you try this on a Microsoft controller?  Does it still work?
> Jeremy could you check this out?

It still attaches when using a Microsoft controller:

uhidev2 at uhub2 port 1 configuration 1 interface 0 "\M-)Microsoft Corporation 
Controller" rev 2.00/1.14 addr 3
uhidev2: iclass 255/93
uhid2 at uhidev2: input=20, output=0, feature=0
ugen0 at uhub2 port 1 configuration 1 "\M-)Microsoft Corporation Controller" 
rev 2.00/1.14 addr 3

The SDL joystick program I use for testing works fine with the patch.

Thanks,
Jeremy


> 
> > 
> > [1] http://marc.info/?l=openbsd-tech=138229619410284=2
> > 
> > Thanks,
> > Christian
> > 
> > 
> > Index: uhidev.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
> > retrieving revision 1.70
> > diff -u -p -r1.70 uhidev.c
> > --- uhidev.c28 Feb 2015 08:42:41 -  1.70
> > +++ uhidev.c5 Dec 2015 20:14:49 -
> > @@ -62,7 +62,10 @@
> >  #ifndef SMALL_KERNEL
> >  /* Replacement report descriptors for devices shipped with broken ones */
> >  #include 
> > -int uhidev_use_rdesc(struct uhidev_softc *, int, int, void **, int *);
> > +int uhidev_use_rdesc(struct uhidev_softc *, usb_interface_descriptor_t *,
> > +   int, int, void **, int *);
> > +#define UISUBCLASS_XBOX360_CONTROLLER 0x5d
> > +#define UIPROTO_XBOX360_GAMEPAD 0x01
> >  #endif /* !SMALL_KERNEL */
> >  
> >  #define DEVNAME(sc)((sc)->sc_dev.dv_xname)
> > @@ -118,10 +121,10 @@ uhidev_match(struct device *parent, void
> > if (id == NULL)
> > return (UMATCH_NONE);
> >  #ifndef SMALL_KERNEL
> > -   if (uaa->vendor == USB_VENDOR_MICROSOFT &&
> > -   uaa->product == USB_PRODUCT_MICROSOFT_XBOX360_CONTROLLER &&
> > -   id->bInterfaceNumber == 0)
> > -   return (UMATCH_VENDOR_PRODUCT);
> > +   if (id->bInterfaceClass == UICLASS_VENDOR &&
> > +   id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> > +   id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD)
> > +   return (UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO);
> >  #endif /* !SMALL_KERNEL */
> > if (id->bInterfaceClass != UICLASS_HID)
> > return (UMATCH_NONE);
> > @@ -191,7 +194,7 @@ uhidev_attach(struct device *parent, str
> > }
> >  
> >  #ifndef SMALL_KERNEL
> > -   if (uhidev_use_rdesc(sc, uaa->vendor, uaa->product, , ))
> > +   if (uhidev_use_rdesc(sc, id, uaa->vendor, uaa->product, , ))
> > return;
> >  #endif /* !SMALL_KERNEL */
> >  
> > @@ -275,8 +278,8 @@ uhidev_attach(struct device *parent, str
> >  
> >  #ifndef SMALL_KERNEL
> >  int
> > -uhidev_use_rdesc(struct uhidev_softc *sc, int vendor, int product,
> > -void **descp, int *sizep)
> > +uhidev_use_rdesc(struct uhidev_softc *sc, usb_interface_descriptor_t *id,
> > +   int vendor, int product, void **descp, int *sizep)
> >  {
> > static uByte reportbuf[] = {2, 2};
> > const void *descptr = NULL;
> > @@ -300,8 +303,9 @@ uhidev_use_rdesc(struct uhidev_softc *sc
> > default:
> > break;
> > }
> > -   } else if (vendor == USB_VENDOR_MICROSOFT &&
> > -   product == USB_PRODUCT_MICROSOFT_XBOX360_CONTROLLER) {
> > +   } else if ((id->bInterfaceClass == UICLASS_VENDOR &&
> > +  id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> > +  id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD)) {
> > /* The Xbox 360 gamepad has no report descriptor. */
> > size = sizeof(uhid_xb360gp_report_descr);
> > descptr = uhid_xb360gp_report_descr;
> >