usb_init_task(9): correct type

2021-01-15 Thread Anton Lindqvist
Hi,
The usb_init_task(9) macro accepts a `struct usb_task'.

Comments? OK?

Index: share/man/man9/usb_add_task.9
===
RCS file: /cvs/src/share/man/man9/usb_add_task.9,v
retrieving revision 1.2
diff -u -p -r1.2 usb_add_task.9
--- share/man/man9/usb_add_task.9   15 Sep 2016 18:26:22 -  1.2
+++ share/man/man9/usb_add_task.9   16 Jan 2021 06:59:49 -
@@ -35,7 +35,7 @@
 .Ft void
 .Fn usb_rem_wait_task "struct usbd_device *dev" "struct usb_task *task"
 .Ft void
-.Fn usb_init_task "struct usbd_device *dev" "void (*fn)(void *)" "void *arg" 
"char type"
+.Fn usb_init_task "struct usb_task *task" "void (*fn)(void *)" "void *arg" 
"char type"
 .Sh DESCRIPTION
 The USB stack provides an API to manage task execution in a thread context at
 the soonest opportunity.



Re: bpf_mtap_ether doesnt need to encode packet priority

2021-01-15 Thread Klemens Nanni
On Fri, Jan 15, 2021 at 11:14:17AM +1000, David Gwynne wrote:
> bpf should be showing what will be or has been on the wire, which is
> what the ether_vtag in the mbuf has. the prio is either about to be
> decoded from the tag on the wya into the stack, or has been encoded by
> vlan(4) on the way out of the stack.
Yes, OK kn.



Re: New ujoy(4) device for USB gamecontrollers

2021-01-15 Thread Marcus Glocker
On Fri, 15 Jan 2021 11:37:47 -0500
Bryan Steele  wrote:

> On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote:
> > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:  
> > > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> > >   
> > > > > I have heard from others who tried the diff that the PS4
> > > > > controller is causing problems with the way it attaches. I
> > > > > ordered one to trial-and- error this myself at home. Could
> > > > > you share output of lsusb -vv? Thanks for giving it a try!  
> > > > 
> > > > Sure, here we go.
> > > > If I can find anything myself in the meantime I let you know.  
> > > 
> > > So the problem doesn't seem to be in your new ujoy(4) code, but
> > > how the dev/hid/hid.c:hid_is_collection() function tries to cope
> > > with the PS4 controller.  
> > 
> > So with the hid_is_collection() problem not easy to mitigate [1],
> > should we table the ujoy(4) proposal for now pending a fix for the
> > problems with the PS4 controller? Or is this interesting enough for
> > some to work on moving forward despite this issue and finding a
> > solution for this specific (and in some ways unusual) device later?
> >  
> 
> Considering the hid_is_collection() fix from NetBSD proposed by Marcus
> fixes the issue with ujoy(4) but breaks some other drivers (imt(4)
> and ims(4)), could we not inline it into ujoy(4) for now? It's fairly
> small helper function. Like hid_is_joy_collection()?

I think that could be an XXX workaround until somebody finds a proper,
generic fix for hid_is_collection() ...



Re: New ujoy(4) device for USB gamecontrollers

2021-01-15 Thread Marcus Glocker
On Fri, 15 Jan 2021 06:23:01 -0700
Thomas Frohwein  wrote:

> On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> >   
> > > > I have heard from others who tried the diff that the PS4
> > > > controller is causing problems with the way it attaches. I
> > > > ordered one to trial-and- error this myself at home. Could you
> > > > share output of lsusb -vv? Thanks for giving it a try!  
> > > 
> > > Sure, here we go.
> > > If I can find anything myself in the meantime I let you know.  
> > 
> > So the problem doesn't seem to be in your new ujoy(4) code, but how
> > the dev/hid/hid.c:hid_is_collection() function tries to cope with
> > the PS4 controller.  
> 
> So with the hid_is_collection() problem not easy to mitigate [1],
> should we table the ujoy(4) proposal for now pending a fix for the
> problems with the PS4 controller? Or is this interesting enough for
> some to work on moving forward despite this issue and finding a
> solution for this specific (and in some ways unusual) device later?
> 
> 3-4 have tested and reported to me so far. It seems so far that the
> only new breakage is with the PS4 controller, and there is probably
> another solution that can be found later that doesn't break other
> drivers like [1]?
> 
> [1] https://marc.info/?l=openbsd-tech=161043081617336=mbox

What I don't like when we import ujoy(4) as is;  It will break
currently supported devices like the PS4 controller.  The kernel change
won't even be the problem since the PS4 controller still would attach to
uhid(4), same as before.  But after the import we will need to start
changing the ports for those controllers which attach correctly, and
require the ports to query the new ujoy(4) device.  At this point the
PS4 controller won't work anymore ...

And unfortunately it doesn't look like there is a proper HID fix in
sight currently which would work for all the devices.



rw_assert_wrlock((9) man page fix

2021-01-15 Thread Vitaliy Makkoveev
rw_assert_wrlock(9) has prototype "void rw_assert_wrlock(struct rwlock *)".

Index: share/man/man9/rwlock.9
===
RCS file: /cvs/src/share/man/man9/rwlock.9,v
retrieving revision 1.25
diff -u -p -r1.25 rwlock.9
--- share/man/man9/rwlock.9 4 Nov 2019 18:16:27 -   1.25
+++ share/man/man9/rwlock.9 15 Jan 2021 19:49:31 -
@@ -58,7 +58,7 @@
 .Fn rw_exit_read "struct rwlock *rwl"
 .Ft void
 .Fn rw_exit_write "struct rwlock *rwl"
-.Ft int
+.Ft void
 .Fn rw_assert_wrlock "struct rwlock *rwl"
 .Ft void
 .Fn rw_assert_rdlock "struct rwlock *rwl"



Re: Add if_mreqn support to IP_MULTICAST_IF

2021-01-15 Thread Alexander Bluhm
On Fri, Jan 15, 2021 at 03:02:37PM +0100, Claudio Jeker wrote:
> On Fri, Jan 15, 2021 at 02:53:17PM +0100, Claudio Jeker wrote:
> > I forgot to add ip_mreqn support to IP_MULTICAST_IF and so the
> > IP_ADD_MEMBERSHIP change is not fixing all the issues I have.
> > 
> > Linux supports calling IP_MULTICAST_IF with a struct in_addr, a struct
> > ip_mreq, or a struct ip_mreqn. FreeBSD only does the first and last.
> > I followed the Linux way because doing that was not that hard. In the end
> > only the imr_ifindex field and the imr_address field need to be checked
> > and if the imr_ifindex is 0 then just use the old code. If the imr_ifindex
> > is set then use this for interface index and break early.
> > 
> > Any opinions about this?
> 
> This is the corresponding diff for ospfd.

With both diffs, kernel and ospfd, regress/usr.sbin/ospfd passes.

OK bluhm@ for both

> Additionally this initalizes the imr_address field. It is not used but we
> should not send stack garbage to the kernel.
> 
> -- 
> :wq Claudio
> 
> Index: interface.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v
> retrieving revision 1.85
> diff -u -p -r1.85 interface.c
> --- interface.c   12 Jan 2021 09:11:09 -  1.85
> +++ interface.c   15 Jan 2021 14:00:39 -
> @@ -734,6 +734,7 @@ if_join_group(struct iface *iface, struc
>   return (0);
>  
>   mreq.imr_multiaddr.s_addr = addr->s_addr;
> + mreq.imr_address.s_addr = 0;
>   mreq.imr_ifindex = iface->ifindex;
>  
>   if (setsockopt(iface->fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
> @@ -782,6 +783,7 @@ if_leave_group(struct iface *iface, stru
>   }
>  
>   mreq.imr_multiaddr.s_addr = addr->s_addr;
> + mreq.imr_address.s_addr = 0;
>   mreq.imr_ifindex = iface->ifindex;
>  
>   if (setsockopt(iface->fd, IPPROTO_IP, IP_DROP_MEMBERSHIP,
> @@ -808,11 +810,15 @@ if_leave_group(struct iface *iface, stru
>  int
>  if_set_mcast(struct iface *iface)
>  {
> + struct ip_mreqn  mreq;
> +
>   switch (iface->type) {
>   case IF_TYPE_POINTOPOINT:
>   case IF_TYPE_BROADCAST:
> + memset(, 0, sizeof(mreq));
> + mreq.imr_ifindex = iface->ifindex;
>   if (setsockopt(iface->fd, IPPROTO_IP, IP_MULTICAST_IF,
> - >addr.s_addr, sizeof(iface->addr.s_addr)) == -1) {
> + , sizeof(mreq)) == -1) {
>   log_warn("if_set_mcast: error setting "
>   "IP_MULTICAST_IF, interface %s", iface->name);
>   return (-1);



Re: tell pfctl(8) route-to and reply-to accept next-hop only

2021-01-15 Thread Alexandr Nedvedicky
Hello,

On Fri, Jan 15, 2021 at 06:26:48PM +0100, Alexander Bluhm wrote:
> On Tue, Jan 12, 2021 at 08:45:22PM +0100, Alexandr Nedvedicky wrote:
> > I think bluhm@ and dlg@ have committed part of that change already.
> 
> I have only commited a refactoring change.  Next step in kernel
> would be to remove the check in pf_find_state() and see what happens.
> 
> I was waiting for dlg@ to do it, but maybe he waited for me.
> 
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1098
> diff -u -p -r1.1098 pf.c
> --- net/pf.c  14 Jan 2021 09:44:33 -  1.1098
> +++ net/pf.c  15 Jan 2021 16:46:42 -
> @@ -1122,12 +1122,6 @@ pf_find_state(struct pf_pdesc *pd, struc
>   }
> 
>   *state = s;
> - if (pd->dir == PF_OUT && s->rt_kif != NULL && s->rt_kif != pd->kif &&
> - ((s->rule.ptr->rt == PF_ROUTETO &&
> - s->rule.ptr->direction == PF_OUT) ||
> - (s->rule.ptr->rt == PF_REPLYTO &&
> - s->rule.ptr->direction == PF_IN)))
> - return (PF_PASS);
> 
>   return (PF_MATCH);
>  }
> 

please go ahead and commit the diff to pf_find_state() above.

> > the proposed diff updates pfctl(8) so parser will do 'a right thing',
> 
> Does it work without the kernel changes from dlg@ ?

no it does not. my branch is ahead of tree. I've lost a track.
sorry for being impatient, creating more confusion here.

> 
> > the diff also breaks existing regression tests. We can update
> > them once, we will agree on proposed diff.
> 
> I have adapted my regress pf.conf and regress/sys/net/pf_forward
> fails in the route-to test.  It worked with dlg@'s diff.  So your
> standalone pfctl change does not seem to be sufficient.
> 

my diff is ahead of time. I'll resend, once tree will be ready.

regards
sashan



Re: tell pfctl(8) route-to and reply-to accept next-hop only

2021-01-15 Thread Alexander Bluhm
On Tue, Jan 12, 2021 at 08:45:22PM +0100, Alexandr Nedvedicky wrote:
> I think bluhm@ and dlg@ have committed part of that change already.

I have only commited a refactoring change.  Next step in kernel
would be to remove the check in pf_find_state() and see what happens.

I was waiting for dlg@ to do it, but maybe he waited for me.

Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1098
diff -u -p -r1.1098 pf.c
--- net/pf.c14 Jan 2021 09:44:33 -  1.1098
+++ net/pf.c15 Jan 2021 16:46:42 -
@@ -1122,12 +1122,6 @@ pf_find_state(struct pf_pdesc *pd, struc
}
 
*state = s;
-   if (pd->dir == PF_OUT && s->rt_kif != NULL && s->rt_kif != pd->kif &&
-   ((s->rule.ptr->rt == PF_ROUTETO &&
-   s->rule.ptr->direction == PF_OUT) ||
-   (s->rule.ptr->rt == PF_REPLYTO &&
-   s->rule.ptr->direction == PF_IN)))
-   return (PF_PASS);
 
return (PF_MATCH);
 }

> the proposed diff updates pfctl(8) so parser will do 'a right thing',

Does it work without the kernel changes from dlg@ ?

> the diff also breaks existing regression tests. We can update
> them once, we will agree on proposed diff.

I have adapted my regress pf.conf and regress/sys/net/pf_forward
fails in the route-to test.  It worked with dlg@'s diff.  So your
standalone pfctl change does not seem to be sufficient.

bluhm

> 8<---8<---8<--8<
> diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
> index 2b3e62b1a7e..536aec3286b 100644
> --- a/sbin/pfctl/parse.y
> +++ b/sbin/pfctl/parse.y
> @@ -3745,23 +3745,13 @@ pool_opt  : BITMASK   {
>   ;
>  
>  route_host   : STRING{
> - /* try to find @if0 address specs */
> - if (strrchr($1, '@') != NULL) {
> - if (($$ = host($1, pf->opts)) == NULL)  {
> - yyerror("invalid host for route spec");
> - YYERROR;
> - }
> + if (($$ = next_hop($1, pf->opts)) == NULL)  {
> + /* error. "any" is handled elsewhere */
>   free($1);
> - } else {
> - $$ = calloc(1, sizeof(struct node_host));
> - if ($$ == NULL)
> - err(1, "route_host: calloc");
> - $$->ifname = $1;
> - $$->addr.type = PF_ADDR_NONE;
> - set_ipmask($$, 128);
> - $$->next = NULL;
> - $$->tail = $$;
> + yyerror("could not parse host specification");
> + YYERROR;
>   }
> + free($1);
>   }
>   | STRING '/' STRING {
>   char*buf;
> @@ -3769,7 +3759,7 @@ route_host  : STRING{
>   if (asprintf(, "%s/%s", $1, $3) == -1)
>   err(1, "host: asprintf");
>   free($1);
> - if (($$ = host(buf, pf->opts)) == NULL) {
> + if (($$ = next_hop(buf, pf->opts)) == NULL) {
>   /* error. "any" is handled elsewhere */
>   free(buf);
>   yyerror("could not parse host specification");
> @@ -3795,33 +3785,6 @@ route_host : STRING{
>   $$->next = NULL;
>   $$->tail = $$;
>   }
> - | dynaddr '/' NUMBER{
> - struct node_host*n;
> -
> - if ($3 < 0 || $3 > 128) {
> - yyerror("bit number too big");
> - YYERROR;
> - }
> - $$ = $1;
> - for (n = $1; n != NULL; n = n->next)
> - set_ipmask(n, $3);
> - }
> - | '(' STRING host ')'   {
> - struct node_host*n;
> -
> - $$ = $3;
> - /* XXX check masks, only full mask should be allowed */
> - for (n = $3; n != NULL; n = n->next) {
> - if ($$->ifname) {
> - yyerror("cannot specify interface twice 
> "
> - "in route spec");
> - YYERROR;
> - }
> - if (($$->ifname = strdup($2)) == NULL)
> -  

Re: Add if_mreqn support to IP_MULTICAST_IF

2021-01-15 Thread Robert Nagy
On 15/01/21 14:53 +0100, Claudio Jeker wrote:
> I forgot to add ip_mreqn support to IP_MULTICAST_IF and so the
> IP_ADD_MEMBERSHIP change is not fixing all the issues I have.
> 
> Linux supports calling IP_MULTICAST_IF with a struct in_addr, a struct
> ip_mreq, or a struct ip_mreqn. FreeBSD only does the first and last.
> I followed the Linux way because doing that was not that hard. In the end
> only the imr_ifindex field and the imr_address field need to be checked
> and if the imr_ifindex is 0 then just use the old code. If the imr_ifindex
> is set then use this for interface index and break early.
> 
> Any opinions about this?

Following the linux way is the way to go because most of the 3rd party software
will expect that. As I said already, this fixed an issue in chromium already, so
I am okay with it.



Re: New ujoy(4) device for USB gamecontrollers

2021-01-15 Thread Bryan Steele
On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote:
> On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> > 
> > > > I have heard from others who tried the diff that the PS4 controller is
> > > > causing problems with the way it attaches. I ordered one to trial-and-
> > > > error this myself at home. Could you share output of lsusb -vv? Thanks
> > > > for giving it a try!
> > > 
> > > Sure, here we go.
> > > If I can find anything myself in the meantime I let you know.
> > 
> > So the problem doesn't seem to be in your new ujoy(4) code, but how the
> > dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4
> > controller.
> 
> So with the hid_is_collection() problem not easy to mitigate [1],
> should we table the ujoy(4) proposal for now pending a fix for the
> problems with the PS4 controller? Or is this interesting enough for
> some to work on moving forward despite this issue and finding a
> solution for this specific (and in some ways unusual) device later?

Considering the hid_is_collection() fix from NetBSD proposed by Marcus
fixes the issue with ujoy(4) but breaks some other drivers (imt(4)
and ims(4)), could we not inline it into ujoy(4) for now? It's fairly
small helper function. Like hid_is_joy_collection()?

-Bryan.

> 3-4 have tested and reported to me so far. It seems so far that the
> only new breakage is with the PS4 controller, and there is probably
> another solution that can be found later that doesn't break other
> drivers like [1]?
> 
> [1] https://marc.info/?l=openbsd-tech=161043081617336=mbox
> 
> > 
> > I'm not much in to HID, but when I sync up the hid_is_collection()
> > function with NetBSD, the PS4 controller attaches to ujoy(4) as
> > expected, and also can be accessed by usbhidctl(1), while my other
> > HID devices continue to work as before:
> > 
> > uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony Interactive
> > Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> > uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 4 ctls
> > audio1 at uaudio0
> > uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony Interactive
> > Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> > uhidev6: iclass 3/0, 180 report ids
> > ujoy0 at uhidev6 reportid 1: input=63, output=0, feature=0 <--
> > uhid6 at uhidev6 reportid 2: input=0, output=0, feature=36
> > uhid7 at uhidev6 reportid 4: input=0, output=0, feature=36
> > uhid8 at uhidev6 reportid 5: input=0, output=31, feature=0
> > uhid9 at uhidev6 reportid 8: input=0, output=0, feature=3
> > uhid10 at uhidev6 reportid 16: input=0, output=0, feature=4
> > uhid11 at uhidev6 reportid 17: input=0, output=0, feature=2
> > uhid12 at uhidev6 reportid 18: input=0, output=0, feature=15
> > uhid13 at uhidev6 reportid 19: input=0, output=0, feature=22
> > uhid14 at uhidev6 reportid 20: input=0, output=0, feature=16
> > uhid15 at uhidev6 reportid 21: input=0, output=0, feature=44
> > uhid16 at uhidev6 reportid 128: input=0, output=0, feature=6
> > uhid17 at uhidev6 reportid 129: input=0, output=0, feature=6
> > uhid18 at uhidev6 reportid 130: input=0, output=0, feature=5
> > uhid19 at uhidev6 reportid 131: input=0, output=0, feature=1
> > uhid20 at uhidev6 reportid 132: input=0, output=0, feature=4
> > uhid21 at uhidev6 reportid 133: input=0, output=0, feature=6
> > uhid22 at uhidev6 reportid 134: input=0, output=0, feature=6
> > uhid23 at uhidev6 reportid 135: input=0, output=0, feature=35
> > uhid24 at uhidev6 reportid 136: input=0, output=0, feature=34
> > uhid25 at uhidev6 reportid 137: input=0, output=0, feature=2
> > uhid26 at uhidev6 reportid 144: input=0, output=0, feature=5
> > uhid27 at uhidev6 reportid 145: input=0, output=0, feature=3
> > uhid28 at uhidev6 reportid 146: input=0, output=0, feature=3
> > uhid29 at uhidev6 reportid 147: input=0, output=0, feature=12
> > uhid30 at uhidev6 reportid 160: input=0, output=0, feature=6
> > uhid31 at uhidev6 reportid 161: input=0, output=0, feature=1
> > uhid32 at uhidev6 reportid 162: input=0, output=0, feature=1
> > uhid33 at uhidev6 reportid 163: input=0, output=0, feature=48
> > uhid34 at uhidev6 reportid 164: input=0, output=0, feature=13
> > uhid35 at uhidev6 reportid 165: input=0, output=0, feature=21
> > uhid36 at uhidev6 reportid 166: input=0, output=0, feature=21
> > uhid37 at uhidev6 reportid 167: input=0, output=0, feature=1
> > uhid38 at uhidev6 reportid 168: input=0, output=0, feature=1
> > uhid39 at uhidev6 reportid 169: input=0, output=0, feature=8
> > uhid40 at uhidev6 reportid 170: input=0, output=0, feature=1
> > uhid41 at uhidev6 reportid 171: input=0, output=0, feature=57
> > uhid42 at uhidev6 reportid 172: input=0, output=0, feature=57
> > uhid43 at uhidev6 reportid 173: input=0, output=0, feature=11
> > uhid44 at uhidev6 reportid 174: input=0, output=0, feature=1
> > uhid45 at uhidev6 reportid 175: input=0, output=0, feature=2
> > uhid46 

Re: pf af-to sysctl forwarding

2021-01-15 Thread Klemens Nanni
On Fri, Jan 15, 2021 at 04:03:09PM +0100, Alexander Bluhm wrote:
> On Fri, Jan 15, 2021 at 03:24:43PM +0100, Klemens Nanni wrote:
> > Existing routers doing NAT64 for IPv6-only networks will require
> > `net.inet.ip.forwarding=1' for NAT64 to work.
> 
> Actually you will need both of them.
> 
> When sending "IPv6 -> pf-router -> IPv4" you need ip forwarding as
> pf translates the packet and then it is forwarded.
Sure.

> But you also want IPv4 packets from the internet return to your
> local IPv6 network.  For that ip6 forwarding is needed.
Yes, I did not mention `net.inet6.ip6.forwarding=1' because that is
already needed to get native IPv6 traffic flowing per se, so I naturally
assumed it to be set on an IPv6 router regardless of NAT64 usage.

Telling IPv6 users that forwarding must be enabled on their router is
pointing out the obvious, `af-to' requiring IPv4 forwarding to be
enabled it less intuitive, I'd say.

> My argument is, that with ip forwarding = 0 no forwarded IPv4
> packet should leave your box.  ip6 forwarding should prevent
> IPv6 packets.
>
> Currently pf af-to forwards packets regardless of the sysctl settings.
> This feels wrong.
I agree.



Re: pf af-to sysctl forwarding

2021-01-15 Thread Alexander Bluhm
On Fri, Jan 15, 2021 at 03:24:43PM +0100, Klemens Nanni wrote:
> Existing routers doing NAT64 for IPv6-only networks will require
> `net.inet.ip.forwarding=1' for NAT64 to work.

Actually you will need both of them.

When sending "IPv6 -> pf-router -> IPv4" you need ip forwarding as
pf translates the packet and then it is forwarded.

But you also want IPv4 packets from the internet return to your
local IPv6 network.  For that ip6 forwarding is needed.

> I'd say we should make that clear with a current.html entry.

I will do that.

> Either way, I think that diff makes sense.

My argument is, that with ip forwarding = 0 no forwarded IPv4
packet should leave your box.  ip6 forwarding should prevent
IPv6 packets.

Currently pf af-to forwards packets regardless of the sysctl settings.
This feels wrong.

bluhm



Re: pf af-to sysctl forwarding

2021-01-15 Thread Klemens Nanni
On Fri, Jan 15, 2021 at 01:30:01PM +0100, Alexander Bluhm wrote:
> sysctl net.inet.ip.forwarding is checked before ip_input() passes
> the packet to ip_forward().  But with an af-to rule, pf(4) calls
> ip_forward() directly.  I think we should check the sysctl also in
> pf to get consistent behaviour.
Existing routers doing NAT64 for IPv6-only networks will require
`net.inet.ip.forwarding=1' for NAT64 to work.

There has not been a need for it on such routers, i.e. my home box only
has `net.inet6.ip6.forwarding=1' in /etc/sysctl.conf so far.

I'd say we should make that clear with a current.html entry.

Either way, I think that diff makes sense.
OK kn



Re: Add if_mreqn support to IP_MULTICAST_IF

2021-01-15 Thread Claudio Jeker
On Fri, Jan 15, 2021 at 02:53:17PM +0100, Claudio Jeker wrote:
> I forgot to add ip_mreqn support to IP_MULTICAST_IF and so the
> IP_ADD_MEMBERSHIP change is not fixing all the issues I have.
> 
> Linux supports calling IP_MULTICAST_IF with a struct in_addr, a struct
> ip_mreq, or a struct ip_mreqn. FreeBSD only does the first and last.
> I followed the Linux way because doing that was not that hard. In the end
> only the imr_ifindex field and the imr_address field need to be checked
> and if the imr_ifindex is 0 then just use the old code. If the imr_ifindex
> is set then use this for interface index and break early.
> 
> Any opinions about this?

This is the corresponding diff for ospfd.

Additionally this initalizes the imr_address field. It is not used but we
should not send stack garbage to the kernel.

-- 
:wq Claudio

Index: interface.c
===
RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v
retrieving revision 1.85
diff -u -p -r1.85 interface.c
--- interface.c 12 Jan 2021 09:11:09 -  1.85
+++ interface.c 15 Jan 2021 14:00:39 -
@@ -734,6 +734,7 @@ if_join_group(struct iface *iface, struc
return (0);
 
mreq.imr_multiaddr.s_addr = addr->s_addr;
+   mreq.imr_address.s_addr = 0;
mreq.imr_ifindex = iface->ifindex;
 
if (setsockopt(iface->fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
@@ -782,6 +783,7 @@ if_leave_group(struct iface *iface, stru
}
 
mreq.imr_multiaddr.s_addr = addr->s_addr;
+   mreq.imr_address.s_addr = 0;
mreq.imr_ifindex = iface->ifindex;
 
if (setsockopt(iface->fd, IPPROTO_IP, IP_DROP_MEMBERSHIP,
@@ -808,11 +810,15 @@ if_leave_group(struct iface *iface, stru
 int
 if_set_mcast(struct iface *iface)
 {
+   struct ip_mreqn  mreq;
+
switch (iface->type) {
case IF_TYPE_POINTOPOINT:
case IF_TYPE_BROADCAST:
+   memset(, 0, sizeof(mreq));
+   mreq.imr_ifindex = iface->ifindex;
if (setsockopt(iface->fd, IPPROTO_IP, IP_MULTICAST_IF,
-   >addr.s_addr, sizeof(iface->addr.s_addr)) == -1) {
+   , sizeof(mreq)) == -1) {
log_warn("if_set_mcast: error setting "
"IP_MULTICAST_IF, interface %s", iface->name);
return (-1);



Add if_mreqn support to IP_MULTICAST_IF

2021-01-15 Thread Claudio Jeker
I forgot to add ip_mreqn support to IP_MULTICAST_IF and so the
IP_ADD_MEMBERSHIP change is not fixing all the issues I have.

Linux supports calling IP_MULTICAST_IF with a struct in_addr, a struct
ip_mreq, or a struct ip_mreqn. FreeBSD only does the first and last.
I followed the Linux way because doing that was not that hard. In the end
only the imr_ifindex field and the imr_address field need to be checked
and if the imr_ifindex is 0 then just use the old code. If the imr_ifindex
is set then use this for interface index and break early.

Any opinions about this?
-- 
:wq Claudio

Index: netinet/ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.360
diff -u -p -r1.360 ip_output.c
--- netinet/ip_output.c 11 Jan 2021 13:28:53 -  1.360
+++ netinet/ip_output.c 15 Jan 2021 12:20:26 -
@@ -1423,11 +1423,40 @@ ip_setmoptions(int optname, struct ip_mo
/*
 * Select the interface for outgoing multicast packets.
 */
-   if (m == NULL || m->m_len != sizeof(struct in_addr)) {
+   if (m == NULL) {
+   error = EINVAL;
+   break;
+   }
+   if (m->m_len == sizeof(struct in_addr)) {
+   addr = *(mtod(m, struct in_addr *));
+   } else if (m->m_len == sizeof(struct ip_mreq) ||
+   m->m_len == sizeof(struct ip_mreqn)) {
+   memset(, 0, sizeof(mreqn));
+   memcpy(, mtod(m, void *), m->m_len);
+
+   /*
+* If an interface index is given use this
+* index to set the imo_ifidx but check first
+* that the interface actually exists.
+* In the other case just set the addr to
+* the imr_address and fall through to the
+* regular code.
+*/
+   if (mreqn.imr_ifindex != 0) {
+   ifp = if_get(mreqn.imr_ifindex);
+   if (ifp == NULL) {
+   error = EADDRNOTAVAIL;
+   break;
+   }
+   imo->imo_ifidx = ifp->if_index;
+   if_put(ifp);
+   break;
+   } else
+   addr = mreqn.imr_address;
+   } else {
error = EINVAL;
break;
}
-   addr = *(mtod(m, struct in_addr *));
/*
 * INADDR_ANY is used to remove a previous selection.
 * When no interface is selected, a default one is



Re: New ujoy(4) device for USB gamecontrollers

2021-01-15 Thread Raf Czlonka
On Fri, Jan 15, 2021 at 01:23:01PM GMT, Thomas Frohwein wrote:
> On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> > 
> > > > I have heard from others who tried the diff that the PS4 controller is
> > > > causing problems with the way it attaches. I ordered one to trial-and-
> > > > error this myself at home. Could you share output of lsusb -vv? Thanks
> > > > for giving it a try!
> > > 
> > > Sure, here we go.
> > > If I can find anything myself in the meantime I let you know.
> > 
> > So the problem doesn't seem to be in your new ujoy(4) code, but how the
> > dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4
> > controller.
> 
> So with the hid_is_collection() problem not easy to mitigate [1],
> should we table the ujoy(4) proposal for now pending a fix for the
> problems with the PS4 controller? Or is this interesting enough for
> some to work on moving forward despite this issue and finding a
> solution for this specific (and in some ways unusual) device later?
> 
> 3-4 have tested and reported to me so far. It seems so far that the
> only new breakage is with the PS4 controller, and there is probably
> another solution that can be found later that doesn't break other
> drivers like [1]?
> 
> [1] https://marc.info/?l=openbsd-tech=161043081617336=mbox

Hi Thomas,

Hadn't had a chance to test your diff yet but, FWIW, I rely on PS4
controller working so would appreciate if it remained in a working
state :^)

Cheers,

Raf



Re: sysctl ip.forwarding 2

2021-01-15 Thread Vitaliy Makkoveev
On Fri, Jan 15, 2021 at 02:07:56PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> As documented in sysctl(2) net.inet.ip.forwarding can be 2.
> 
> netinet/ip_output.c:448
>   if (ipsec_in_use && (flags & IP_FORWARDING) && (ipforwarding == 2) &&
> 
> Current input validation prevents this.  
> # sysctl net.inet.ip.forwarding=2
> sysctl: net.inet.ip.forwarding: Invalid argument
> 
> Also change bool check to integer comparison consistently.
> ip6_forwarding misses the feature, but that is a different story.
> 
> ok?
> 
> bluhm
> 

ok mvs@

> Index: netinet/ip_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.353
> diff -u -p -r1.353 ip_input.c
> --- netinet/ip_input.c11 Jan 2021 13:28:53 -  1.353
> +++ netinet/ip_input.c15 Jan 2021 12:45:41 -
> @@ -115,7 +115,7 @@ const struct sysctl_bounded_args ipctl_v
>  #ifdef MROUTING
>   { IPCTL_MRTPROTO, _mrtproto, 1, 0 },
>  #endif
> - { IPCTL_FORWARDING, , 0, 1 },
> + { IPCTL_FORWARDING, , 0, 2 },
>   { IPCTL_SENDREDIRECTS, , 0, 1 },
>   { IPCTL_DEFTTL, _defttl, 0, 255 },
>   { IPCTL_DIRECTEDBCAST, _directedbcast, 0, 1 },
> @@ -1251,7 +1251,7 @@ ip_dooptions(struct mbuf *m, struct ifne
>   }
>   }
>   KERNEL_UNLOCK();
> - if (forward && ipforwarding) {
> + if (forward && ipforwarding > 0) {
>   ip_forward(m, ifp, NULL, 1);
>   return (1);
>   }
> 



Re: XBox One gamecontroller support

2021-01-15 Thread Thomas Frohwein
On Sat, Jan 09, 2021 at 10:50:35AM -0700, Thomas Frohwein wrote:
> Hi,
> 
> This diff adds support for the XBox One gamecontroller in a similar way
> to what we have for the (older) XBox 360 controller [1][2]. This diff
> is based on the pertinent code in NetBSD's uhidev.c.
> 
> Similarities include that the device doesn't provide a report
> descriptor, so this diff adds one to uhid_rdesc.h.
> 
> The biggest difference (that held this up for a while) is that the
> controller needs an init byte sequence to "wake up."
> 
> The original report descriptor from NetBSD just maps the buttons and
> axes in the order that they appear in the report. I modified the report
> descriptor to assign the buttons/axes in the most logical order for
> compatibility, which results in the same layout that we already have
> for the XBox 360 controller.
> 
> The addition of the member sc_flags to struct uhidev_softc follows the
> NetBSD example.
> 
> I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
> package), and a couple of SDL2 games (Owlboy, Cryptark).
> 
> If you have an XBox One controller, the easiest way to test is with:
> 
> $ usbhidctl -f /dev/uhidX -l
> 
> Make sure to pick the right uhid device and that you have read
> permissions for it.
> 
> Of course, this works only with the controller connected via USB, and
> doesn't magically add wireless support.
> 
> If you test actual SDL2 applications, the button layout will likely
> default to an odd configuration. I am simultaneously preparing a diff
> for sdl2-2.0.14p0 that improves recognition and automatic mapping and
> solves any issues with XBox One default button layout.

I have an update to this diff. 2 testers used a more recent XBox One
controller model (model number 1708 in both cases) and ran into
problems with the original diff. Below is a new diff that uses a longer
5-byte init sequence taken from Linux' xpad.c [1], compared to the
2-byte sequence from NetBSD's uhidev.c that I offered in the initial
diff. This fixed the issue for the 2 testers.

My own XBox One controller is model number 1537 and still works with
this diff.

[1] https://github.com/paroj/xpad/blob/master/xpad.c#L479

Index: uhid_rdesc.h
===
RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
retrieving revision 1.1
diff -u -p -r1.1 uhid_rdesc.h
--- uhid_rdesc.h25 Oct 2013 03:09:59 -  1.1
+++ uhid_rdesc.h9 Jan 2021 15:11:30 -
@@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
 0x95, 0x01,/*  REPORT COUNT (1)*/
 0x81, 0x01,/*  INPUT (Constant)*/
 0xc0,  /* END COLLECTION   */
+};
+
+static const uByte uhid_xbonegp_report_descr[] = {
+0x05, 0x01,/* USAGE PAGE (Generic Desktop) 
*/
+0x09, 0x05,/* USAGE (Gamepad)  
*/
+0xa1, 0x01,/* COLLECTION (Application) 
*/
+/* Button packet */
+0xa1, 0x00,/*  COLLECTION (Physical)   
*/
+0x85, 0x20,/*   REPORT ID (0x20)   
*/
+/* Skip unknown field and counter */
+0x09, 0x00,/*   USAGE (Undefined)  
*/
+0x75, 0x08,/*   REPORT SIZE (8)
*/
+0x95, 0x02,/*   REPORT COUNT (2)   
*/
+0x81, 0x03,/*   INPUT (Constant, Variable, 
Absolute)   */
+/* Payload size */
+0x09, 0x3b,/*   USAGE (Byte Count) 
*/
+0x95, 0x01,/*   REPORT COUNT (1)   
*/
+0x81, 0x02,/*   INPUT (Data, Variable, Absolute)   
*/
+/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
+ *9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
+ */
+/* Skip the first 2 as they are not used */
+0x75, 0x01,/*   REPORT SIZE (1)
*/
+0x95, 0x02,/*   REPORT COUNT (2)   
*/
+0x81, 0x01,/*   INPUT (Constant)   
*/
+/* Assign buttons Start(7), Back(8), ABXY(1-4) */
+0x15, 0x00,/*   LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/*   LOGICAL MAXIMUM (1)
*/
+0x95, 0x06,/*   REPORT COUNT (6)   
*/
+0x05, 0x09,/*   USAGE PAGE (Button)
*/
+0x09, 0x08,/*   USAGE (Button 8)   
   

Re: sysctl ip.forwarding 2

2021-01-15 Thread Klemens Nanni
On Fri, Jan 15, 2021 at 02:07:56PM +0100, Alexander Bluhm wrote:
> As documented in sysctl(2) net.inet.ip.forwarding can be 2.
> 
> netinet/ip_output.c:448
>   if (ipsec_in_use && (flags & IP_FORWARDING) && (ipforwarding == 2) &&
> 
> Current input validation prevents this.  
> # sysctl net.inet.ip.forwarding=2
> sysctl: net.inet.ip.forwarding: Invalid argument
> 
> Also change bool check to integer comparison consistently.
That reads OK (still building to test the other af-to diff on my router).



Re: New ujoy(4) device for USB gamecontrollers

2021-01-15 Thread Thomas Frohwein
On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> 
> > > I have heard from others who tried the diff that the PS4 controller is
> > > causing problems with the way it attaches. I ordered one to trial-and-
> > > error this myself at home. Could you share output of lsusb -vv? Thanks
> > > for giving it a try!
> > 
> > Sure, here we go.
> > If I can find anything myself in the meantime I let you know.
> 
> So the problem doesn't seem to be in your new ujoy(4) code, but how the
> dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4
> controller.

So with the hid_is_collection() problem not easy to mitigate [1],
should we table the ujoy(4) proposal for now pending a fix for the
problems with the PS4 controller? Or is this interesting enough for
some to work on moving forward despite this issue and finding a
solution for this specific (and in some ways unusual) device later?

3-4 have tested and reported to me so far. It seems so far that the
only new breakage is with the PS4 controller, and there is probably
another solution that can be found later that doesn't break other
drivers like [1]?

[1] https://marc.info/?l=openbsd-tech=161043081617336=mbox

> 
> I'm not much in to HID, but when I sync up the hid_is_collection()
> function with NetBSD, the PS4 controller attaches to ujoy(4) as
> expected, and also can be accessed by usbhidctl(1), while my other
> HID devices continue to work as before:
> 
> uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony Interactive
> Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 4 ctls
> audio1 at uaudio0
> uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony Interactive
> Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> uhidev6: iclass 3/0, 180 report ids
> ujoy0 at uhidev6 reportid 1: input=63, output=0, feature=0 <--
> uhid6 at uhidev6 reportid 2: input=0, output=0, feature=36
> uhid7 at uhidev6 reportid 4: input=0, output=0, feature=36
> uhid8 at uhidev6 reportid 5: input=0, output=31, feature=0
> uhid9 at uhidev6 reportid 8: input=0, output=0, feature=3
> uhid10 at uhidev6 reportid 16: input=0, output=0, feature=4
> uhid11 at uhidev6 reportid 17: input=0, output=0, feature=2
> uhid12 at uhidev6 reportid 18: input=0, output=0, feature=15
> uhid13 at uhidev6 reportid 19: input=0, output=0, feature=22
> uhid14 at uhidev6 reportid 20: input=0, output=0, feature=16
> uhid15 at uhidev6 reportid 21: input=0, output=0, feature=44
> uhid16 at uhidev6 reportid 128: input=0, output=0, feature=6
> uhid17 at uhidev6 reportid 129: input=0, output=0, feature=6
> uhid18 at uhidev6 reportid 130: input=0, output=0, feature=5
> uhid19 at uhidev6 reportid 131: input=0, output=0, feature=1
> uhid20 at uhidev6 reportid 132: input=0, output=0, feature=4
> uhid21 at uhidev6 reportid 133: input=0, output=0, feature=6
> uhid22 at uhidev6 reportid 134: input=0, output=0, feature=6
> uhid23 at uhidev6 reportid 135: input=0, output=0, feature=35
> uhid24 at uhidev6 reportid 136: input=0, output=0, feature=34
> uhid25 at uhidev6 reportid 137: input=0, output=0, feature=2
> uhid26 at uhidev6 reportid 144: input=0, output=0, feature=5
> uhid27 at uhidev6 reportid 145: input=0, output=0, feature=3
> uhid28 at uhidev6 reportid 146: input=0, output=0, feature=3
> uhid29 at uhidev6 reportid 147: input=0, output=0, feature=12
> uhid30 at uhidev6 reportid 160: input=0, output=0, feature=6
> uhid31 at uhidev6 reportid 161: input=0, output=0, feature=1
> uhid32 at uhidev6 reportid 162: input=0, output=0, feature=1
> uhid33 at uhidev6 reportid 163: input=0, output=0, feature=48
> uhid34 at uhidev6 reportid 164: input=0, output=0, feature=13
> uhid35 at uhidev6 reportid 165: input=0, output=0, feature=21
> uhid36 at uhidev6 reportid 166: input=0, output=0, feature=21
> uhid37 at uhidev6 reportid 167: input=0, output=0, feature=1
> uhid38 at uhidev6 reportid 168: input=0, output=0, feature=1
> uhid39 at uhidev6 reportid 169: input=0, output=0, feature=8
> uhid40 at uhidev6 reportid 170: input=0, output=0, feature=1
> uhid41 at uhidev6 reportid 171: input=0, output=0, feature=57
> uhid42 at uhidev6 reportid 172: input=0, output=0, feature=57
> uhid43 at uhidev6 reportid 173: input=0, output=0, feature=11
> uhid44 at uhidev6 reportid 174: input=0, output=0, feature=1
> uhid45 at uhidev6 reportid 175: input=0, output=0, feature=2
> uhid46 at uhidev6 reportid 176: input=0, output=0, feature=63
> uhid47 at uhidev6 reportid 177: input=0, output=0, feature=2
> uhid48 at uhidev6 reportid 178: input=0, output=0, feature=2
> uhid49 at uhidev6 reportid 179: input=0, output=0, feature=63
> uhid50 at uhidev6 reportid 180: input=0, output=0, feature=63
> 
>   $ usbhidctl -f /dev/ujoy/0 -l
>   Game_Pad.X=126
>   Game_Pad.Y=126
>   Game_Pad.Z=125
>   Game_Pad.Rz=129
>   Game_Pad.Hat_Switch=8
>   Game_Pad.Button_1=0

sysctl ip.forwarding 2

2021-01-15 Thread Alexander Bluhm
Hi,

As documented in sysctl(2) net.inet.ip.forwarding can be 2.

netinet/ip_output.c:448
if (ipsec_in_use && (flags & IP_FORWARDING) && (ipforwarding == 2) &&

Current input validation prevents this.  
# sysctl net.inet.ip.forwarding=2
sysctl: net.inet.ip.forwarding: Invalid argument

Also change bool check to integer comparison consistently.
ip6_forwarding misses the feature, but that is a different story.

ok?

bluhm

Index: netinet/ip_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.353
diff -u -p -r1.353 ip_input.c
--- netinet/ip_input.c  11 Jan 2021 13:28:53 -  1.353
+++ netinet/ip_input.c  15 Jan 2021 12:45:41 -
@@ -115,7 +115,7 @@ const struct sysctl_bounded_args ipctl_v
 #ifdef MROUTING
{ IPCTL_MRTPROTO, _mrtproto, 1, 0 },
 #endif
-   { IPCTL_FORWARDING, , 0, 1 },
+   { IPCTL_FORWARDING, , 0, 2 },
{ IPCTL_SENDREDIRECTS, , 0, 1 },
{ IPCTL_DEFTTL, _defttl, 0, 255 },
{ IPCTL_DIRECTEDBCAST, _directedbcast, 0, 1 },
@@ -1251,7 +1251,7 @@ ip_dooptions(struct mbuf *m, struct ifne
}
}
KERNEL_UNLOCK();
-   if (forward && ipforwarding) {
+   if (forward && ipforwarding > 0) {
ip_forward(m, ifp, NULL, 1);
return (1);
}



ugen(4) and uhidev(4) data toggle problem

2021-01-15 Thread Marcus Glocker
There are a few threads going on related to problems with ugen(4) and
uhidev(4) devices on xhci(4).  This is related to the issue patrick@
already explained; while ehci(4) can save the last data toggle state,
xhci(4) resets it on every open/close cycle, getting out of sync with
the device.

This diff addresses a possible solution for ugen(4) and uhidev(4).
For ugen(4) we already did some positive testing related to scanner
devices, see e.g.:

https://marc.info/?l=openbsd-bugs=161056827312265=2

I would appreciate some regression testing and feedback.  On xhci(4)
it might resolve issues related to that, and on ehci(4) it shouldn't
break your existing setup.


Index: dev/usb/ugen.c
===
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.109
diff -u -p -u -p -r1.109 ugen.c
--- dev/usb/ugen.c  25 Dec 2020 12:59:52 -  1.109
+++ dev/usb/ugen.c  13 Jan 2021 21:38:30 -
@@ -46,9 +46,12 @@
 #include 
 #include 
 
+#include 
+
 #include 
 #include 
 #include 
+#include 
 
 #ifdef UGEN_DEBUG
 #define DPRINTF(x) do { if (ugendebug) printf x; } while (0)
@@ -114,6 +117,7 @@ int ugen_do_close(struct ugen_softc *, i
 int ugen_set_config(struct ugen_softc *sc, int configno);
 int ugen_set_interface(struct ugen_softc *, int, int);
 int ugen_get_alt_index(struct ugen_softc *sc, int ifaceidx);
+void ugen_clear_iface_eps(struct ugen_softc *, struct usbd_interface *);
 
 #define UGENUNIT(n) ((minor(n) >> 4) & 0xf)
 #define UGENENDPOINT(n) (minor(n) & 0xf)
@@ -302,6 +306,8 @@ ugenopen(dev_t dev, int flag, int mode, 
DPRINTFN(5, ("ugenopen: sc=%p, endpt=%d, dir=%d, sce=%p\n",
 sc, endpt, dir, sce));
edesc = sce->edesc;
+   /* Clear device endpoint toggle. */
+   ugen_clear_iface_eps(sc, sce->iface);
switch (UE_GET_XFERTYPE(edesc->bmAttributes)) {
case UE_INTERRUPT:
if (dir == OUT) {
@@ -329,6 +335,8 @@ ugenopen(dev_t dev, int flag, int mode, 
clfree(>q);
return (EIO);
}
+   /* Clear HC endpoint toggle. */
+   usbd_clear_endpoint_toggle(sce->pipeh);
DPRINTFN(5, ("ugenopen: interrupt open done\n"));
break;
case UE_BULK:
@@ -336,6 +344,8 @@ ugenopen(dev_t dev, int flag, int mode, 
  edesc->bEndpointAddress, 0, >pipeh);
if (err)
return (EIO);
+   /* Clear HC endpoint toggle. */
+   usbd_clear_endpoint_toggle(sce->pipeh);
break;
case UE_ISOCHRONOUS:
if (dir == OUT)
@@ -1417,4 +1427,42 @@ ugenkqfilter(dev_t dev, struct knote *kn
splx(s);
 
return (0);
+}
+
+void
+ugen_clear_iface_eps(struct ugen_softc *sc, struct usbd_interface *iface)
+{
+   usb_interface_descriptor_t *id;
+   usb_endpoint_descriptor_t *ed;
+   uint8_t xfertype;
+   int i;
+
+   /* Only clear interface endpoints when none are in use. */
+   for (i = 0; i < USB_MAX_ENDPOINTS; i++) {
+   if (i == USB_CONTROL_ENDPOINT)
+   continue;
+   if (sc->sc_is_open[i] != 0)
+   return;
+   }
+   DPRINTFN(1,("%s: clear interface eps\n", __func__));
+
+   id = usbd_get_interface_descriptor(iface);
+   if (id == NULL)
+   goto bad;
+
+   for (i = 0; i < id->bNumEndpoints; i++) {
+   ed = usbd_interface2endpoint_descriptor(iface, i);
+   if (ed == NULL)
+   goto bad;
+
+   xfertype = UE_GET_XFERTYPE(ed->bmAttributes);
+   if (xfertype == UE_BULK || xfertype == UE_INTERRUPT) {
+   if (usbd_clear_endpoint_feature(sc->sc_udev,
+   ed->bEndpointAddress, UF_ENDPOINT_HALT))
+   goto bad;
+   }
+   }
+   return;
+bad:
+   printf("%s: clear endpoints failed!\n", __func__);
 }
Index: dev/usb/uhidev.c
===
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.83
diff -u -p -u -p -r1.83 uhidev.c
--- dev/usb/uhidev.c31 Aug 2020 12:26:49 -  1.83
+++ dev/usb/uhidev.c13 Jan 2021 21:38:31 -
@@ -98,6 +98,7 @@ int uhidev_activate(struct device *, int
 
 void uhidev_get_report_async_cb(struct usbd_xfer *, void *, usbd_status);
 void uhidev_set_report_async_cb(struct usbd_xfer *, void *, usbd_status);
+void uhidev_clear_iface_eps(struct uhidev_softc *, struct usbd_interface *);
 
 struct cfdriver uhidev_cd = {
NULL, "uhidev", DV_DULL
@@ -508,6 +509,9 @@ uhidev_open(struct uhidev *scd)

pf af-to sysctl forwarding

2021-01-15 Thread Alexander Bluhm
Hi,

sysctl net.inet.ip.forwarding is checked before ip_input() passes
the packet to ip_forward().  But with an af-to rule, pf(4) calls
ip_forward() directly.  I think we should check the sysctl also in
pf to get consistent behaviour.

ok?

bluhm

Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1098
diff -u -p -r1.1098 pf.c
--- net/pf.c14 Jan 2021 09:44:33 -  1.1098
+++ net/pf.c15 Jan 2021 11:08:31 -
@@ -7259,20 +7259,32 @@ done:
pd.m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
switch (pd.naf) {
case AF_INET:
-   if (pd.dir == PF_IN)
+   if (pd.dir == PF_IN) {
+   if (ipforwarding == 0) {
+   ipstat_inc(ips_cantforward);
+   action = PF_DROP;
+   break;
+   }
ip_forward(pd.m, ifp, NULL, 1);
-   else
+   } else
ip_output(pd.m, NULL, NULL, 0, NULL, NULL, 0);
break;
case AF_INET6:
-   if (pd.dir == PF_IN)
+   if (pd.dir == PF_IN) {
+   if (ip6_forwarding == 0) {
+   ip6stat_inc(ip6s_cantforward);
+   action = PF_DROP;
+   break;
+   }
ip6_forward(pd.m, NULL, 1);
-   else
+   } else
ip6_output(pd.m, NULL, NULL, 0, NULL, NULL);
break;
}
-   pd.m = NULL;
-   action = PF_PASS;
+   if (action != PF_DROP) {
+   pd.m = NULL;
+   action = PF_PASS;
+   }
break;
 #endif /* INET6 */
case PF_DROP: