Re: pf.conf(5) about queueing may be wrong

2021-08-23 Thread Stuart Henderson
On 2021/08/23 22:21, Klemens Nanni wrote:
> On Mon, Aug 23, 2021 at 07:03:45PM +0200, Solene Rapenne wrote:
> > pf.conf says this in QUEUEING
> > https://man.openbsd.org/pf.conf#QUEUEING
> > 
> > > If the referenced queue does not exist on the outgoing interface,
> > > the default queue for that interface is used.
> 
> This text is outdated, pfctl gained the below error in 2014 "shortly"
> after henning reworked queueing in 2013 to where that manual text dates
> back.
> 
> > however, with this simple config
> > 
> > queue std on re0 bandwidth 100M
> > queue lan parent std bandwidth 100M
> > queue internet parent std bandwidth 900K flows 512 default
> > match proto udp from em0:network to any port 53 queue dns
> > 
> > when reloading the file with pfctl, I get the following error:
> > /etc/pf.conf:27: queue dns is not defined
> > 
> > From the man page, I understand that if the queue used in match
> > doesn't exist, the default queue is used, as if "queue dns" wasn't
> > written in the rule.
> 
> Specified queues must exist, rules without `[set] queue ...' do use the
> default queue, obviously.
> 
> > Either the man page is wrong or not easy to understand, or the
> > parser is wrong.
> 
> OK kn to remove that sentence from the manual.
> 

Not ok with me.



Re: pf.conf(5) about queueing may be wrong

2021-08-23 Thread Stuart Henderson
On 2021/08/23 19:03, Solene Rapenne wrote:
> pf.conf says this in QUEUEING
> https://man.openbsd.org/pf.conf#QUEUEING
> 
> > If the referenced queue does not exist on the outgoing interface,
> > the default queue for that interface is used.
> 
> however, with this simple config
> 
> queue std on re0 bandwidth 100M
> queue lan parent std bandwidth 100M
> queue internet parent std bandwidth 900K flows 512 default
> match proto udp from em0:network to any port 53 queue dns
> 
> when reloading the file with pfctl, I get the following error:
> /etc/pf.conf:27: queue dns is not defined

In your config, the queue "dns" does not exist _at all_ in the config.

> From the man page, I understand that if the queue used in match
> doesn't exist, the default queue is used, as if "queue dns" wasn't
> written in the rule.

The manual talks about something a bit different, a queue that does
not exist _on a particular interface_.

> Either the man page is wrong or not easy to understand, or the
> parser is wrong.

I don't think it is wrong or even really hard to understand.



resolv.conf(5): remove "either file" wording

2021-08-23 Thread Scott Bennett
In rev 1.61, references to resolv.conf.tail were removed, so it appears that
this page is now meant to solely document resolv.conf, a single file. So that
makes this sentence make not-so-much sense:

The configuration options (which may be placed in either file) are:

Diff below removes the "either file" wording.

Cheers,
Scott


diff c90212a02b599071a4e7b5e4709b67a0988d81b7 /usr/src
blob - 49ee20b2a5c863735baedf6c938512c029e76903
file + share/man/man5/resolv.conf.5
--- share/man/man5/resolv.conf.5
+++ share/man/man5/resolv.conf.5
@@ -69,11 +69,11 @@ or semicolon
 .Pq \&;
 in the file indicates the beginning of a comment;
 subsequent characters up to the end of the line are not interpreted by
 the routines that read the file.
 .Pp
-The configuration options (which may be placed in either file) are:
+The configuration options are:
 .Bl -tag -width nameserver
 .It Ic nameserver
 IPv4 address (in dot notation)
 or IPv6 address (in hex-and-colon notation)
 of a name server that the resolver should query.



Re: pf.conf(5) about queueing may be wrong

2021-08-23 Thread Klemens Nanni
On Mon, Aug 23, 2021 at 07:03:45PM +0200, Solene Rapenne wrote:
> pf.conf says this in QUEUEING
> https://man.openbsd.org/pf.conf#QUEUEING
> 
> > If the referenced queue does not exist on the outgoing interface,
> > the default queue for that interface is used.

This text is outdated, pfctl gained the below error in 2014 "shortly"
after henning reworked queueing in 2013 to where that manual text dates
back.

> however, with this simple config
> 
> queue std on re0 bandwidth 100M
> queue lan parent std bandwidth 100M
> queue internet parent std bandwidth 900K flows 512 default
> match proto udp from em0:network to any port 53 queue dns
> 
> when reloading the file with pfctl, I get the following error:
> /etc/pf.conf:27: queue dns is not defined
> 
> From the man page, I understand that if the queue used in match
> doesn't exist, the default queue is used, as if "queue dns" wasn't
> written in the rule.

Specified queues must exist, rules without `[set] queue ...' do use the
default queue, obviously.

> Either the man page is wrong or not easy to understand, or the
> parser is wrong.

OK kn to remove that sentence from the manual.



pf.conf(5) about queueing may be wrong

2021-08-23 Thread Solene Rapenne
pf.conf says this in QUEUEING
https://man.openbsd.org/pf.conf#QUEUEING

> If the referenced queue does not exist on the outgoing interface,
> the default queue for that interface is used.

however, with this simple config

queue std on re0 bandwidth 100M
queue lan parent std bandwidth 100M
queue internet parent std bandwidth 900K flows 512 default
match proto udp from em0:network to any port 53 queue dns

when reloading the file with pfctl, I get the following error:
/etc/pf.conf:27: queue dns is not defined

From the man page, I understand that if the queue used in match
doesn't exist, the default queue is used, as if "queue dns" wasn't
written in the rule.

Either the man page is wrong or not easy to understand, or the
parser is wrong.



Re: fortune(6): Veni, vidi, vici

2021-08-23 Thread Jason McIntyre
On Mon, Aug 23, 2021 at 08:18:13PM +0200, Alessandro De Laurenzis wrote:
> Greetings,
> 
> I was reluctant to submit this patch, since I'm not a native English 
> speaker and this could be a wordplay joke, but if not, and it is really 
> citing the Latin phrase popularly attributed to Julius Caesar (see e.g. 
> [1], but there are plenty on the net, of course), the wrong order warps 
> the meaning.
> 
> Please consider the attached diff.
> 
> All the best
> 
> [1] https://en.wikipedia.org/wiki/Veni,_vidi,_vici
> 
> -- 
> Alessandro De Laurenzis
> [mailto:jus...@atlantide.mooo.com]
> Web: http://www.atlantide.mooo.com
> LinkedIn: http://it.linkedin.com/in/delaurenzis

> --- /usr/src/games/fortune/datfiles/fortunes2-o.orig  Thu Jul 13 04:45:56 2017
> +++ /usr/src/games/fortune/datfiles/fortunes2-o   Mon Aug 23 20:07:23 2021
> @@ -14251,8 +14251,8 @@
>  to a rival.  Husbands, good or bad, always have rivals.  Lovers, never.
>   -- Helen Lawrenson, "Esquire"
>  %
> -Vidi, vici, veni.
> -(I saw, I conquered, I came.)
> +Veni, vidi, vici.
> +(I came, I saw, I conquered.)
>  %
>  Viennese Oyster: Lady who can cross her feet behind her head, lying on her
>  back, of course.  When she has done so, you hold her tightly round each 
> instep


hi.

it is a joke, yes. there's at least one other joke of this type in the
database:

Veni, Vidi, VISA:
I came, I saw, I did a little shopping.

however i also just spotted the actual quote is there too, and appears
to have a typo!

jmc

Index: fortunes2
===
RCS file: /cvs/src/games/fortune/datfiles/fortunes2,v
retrieving revision 1.52
diff -u -p -r1.52 fortunes2
--- fortunes2   27 Sep 2019 20:44:22 -  1.52
+++ fortunes2   23 Aug 2021 20:36:05 -
@@ -41833,7 +41833,7 @@ Hagar's note: The first definition is mu
 only by malcontents, the envious, and disgruntled owners of waterfront
 property.
 %
-Vini, vidi, vici.
+Veni, vidi, vici.
 [I came, I saw, I conquered].
-- Gaius Julius Caesar
 %



handle RTM_IFANNOUNCE in dhcpleased & slaacd

2021-08-23 Thread Florian Obser
So I was playing with a usb network adapter and noticed that dhcpleased
and slaacd would hold on to them when I unplugged them.
They would be listed as "unknown" because we can't find the if_name for
the if_index anymore.

Turns out we are not getting a RTM_IFINFO when an interface disappears
but instead we have to handle the RTM_IFANNOUNCE message.
While here fix a bug in slaacd where it would remove the interface only
in the engine process but not in the frontend when we lose an interface
while handing RTM_IFINFO.

OK?


diff --git sbin/dhcpleased/dhcpleased.c sbin/dhcpleased/dhcpleased.c
index 5ff5dcc9480..55c51d53b18 100644
--- sbin/dhcpleased/dhcpleased.c
+++ sbin/dhcpleased/dhcpleased.c
@@ -294,7 +294,8 @@ main(int argc, char *argv[])
AF_INET)) == -1)
fatal("route socket");
 
-   rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL);
+   rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL) |
+   ROUTE_FILTER(RTM_IFANNOUNCE);
if (setsockopt(frontend_routesock, AF_ROUTE, ROUTE_MSGFILTER,
, sizeof(rtfilter)) == -1)
fatal("setsockopt(ROUTE_MSGFILTER)");
diff --git sbin/dhcpleased/frontend.c sbin/dhcpleased/frontend.c
index 84ae1cda9a4..44a217cb51b 100644
--- sbin/dhcpleased/frontend.c
+++ sbin/dhcpleased/frontend.c
@@ -768,7 +768,10 @@ route_receive(int fd, short events, void *arg)
 void
 handle_route_message(struct rt_msghdr *rtm, struct sockaddr **rti_info)
 {
-   struct sockaddr_dl  *sdl = NULL;
+   struct sockaddr_dl  *sdl = NULL;
+   struct if_announcemsghdr*ifan;
+   uint32_t if_index;
+
switch (rtm->rtm_type) {
case RTM_IFINFO:
if (rtm->rtm_addrs & RTA_IFP && rti_info[RTAX_IFP]->sa_family
@@ -776,6 +779,15 @@ handle_route_message(struct rt_msghdr *rtm, struct 
sockaddr **rti_info)
sdl = (struct sockaddr_dl *)rti_info[RTAX_IFP];
update_iface((struct if_msghdr *)rtm, sdl);
break;
+   case RTM_IFANNOUNCE:
+   ifan = (struct if_announcemsghdr *)rtm;
+   if_index = ifan->ifan_index;
+if (ifan->ifan_what == IFAN_DEPARTURE) {
+   frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0,
+   _index, sizeof(if_index));
+   remove_iface(if_index);
+   }
+   break;
case RTM_PROPOSAL:
if (rtm->rtm_priority == RTP_PROPOSAL_SOLICIT) {
log_debug("RTP_PROPOSAL_SOLICIT");
@@ -784,7 +796,6 @@ handle_route_message(struct rt_msghdr *rtm, struct sockaddr 
**rti_info)
}
 #ifndef SMALL
else if (rtm->rtm_flags & RTF_PROTO3) {
-   uint32_t if_index;
char ifnamebuf[IF_NAMESIZE], *if_name;
 
if_index = rtm->rtm_index;
diff --git sbin/slaacd/frontend.c sbin/slaacd/frontend.c
index d3cb23a2925..27fbd212a66 100644
--- sbin/slaacd/frontend.c
+++ sbin/slaacd/frontend.c
@@ -778,6 +778,7 @@ void
 handle_route_message(struct rt_msghdr *rtm, struct sockaddr **rti_info)
 {
struct if_msghdr*ifm;
+   struct if_announcemsghdr*ifan;
struct imsg_del_addr del_addr;
struct imsg_del_routedel_route;
struct imsg_dup_addr dup_addr;
@@ -798,6 +799,7 @@ handle_route_message(struct rt_msghdr *rtm, struct sockaddr 
**rti_info)
if_index = ifm->ifm_index;
frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0,
_index, sizeof(if_index));
+   remove_iface(if_index);
} else {
xflags = get_xflags(if_name);
if (xflags == -1 || !(xflags & (IFXF_AUTOCONF6 |
@@ -817,6 +819,15 @@ handle_route_message(struct rt_msghdr *rtm, struct 
sockaddr **rti_info)
}
}
break;
+   case RTM_IFANNOUNCE:
+   ifan = (struct if_announcemsghdr *)rtm;
+   if_index = ifan->ifan_index;
+if (ifan->ifan_what == IFAN_DEPARTURE) {
+   frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0,
+   _index, sizeof(if_index));
+   remove_iface(if_index);
+   }
+   break;
case RTM_NEWADDR:
ifm = (struct if_msghdr *)rtm;
if_name = if_indextoname(ifm->ifm_index, ifnamebuf);
diff --git sbin/slaacd/slaacd.c sbin/slaacd/slaacd.c
index 0b31934c211..ca5e942af2f 100644
--- sbin/slaacd/slaacd.c
+++ sbin/slaacd/slaacd.c
@@ -255,7 +255,8 @@ main(int argc, char *argv[])
 
rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_NEWADDR) |
ROUTE_FILTER(RTM_DELADDR) | 

Re: fortune(6): Veni, vidi, vici

2021-08-23 Thread Claus Assmann
On Mon, Aug 23, 2021, Alessandro De Laurenzis wrote:

> and this could be a wordplay joke,

AFAICT it is.

-- 
Address is valid for this mailing list only, please do not reply
to it direcly, but to the list.



fortune(6): Veni, vidi, vici

2021-08-23 Thread Alessandro De Laurenzis

Greetings,

I was reluctant to submit this patch, since I'm not a native English 
speaker and this could be a wordplay joke, but if not, and it is really 
citing the Latin phrase popularly attributed to Julius Caesar (see e.g. 
[1], but there are plenty on the net, of course), the wrong order warps 
the meaning.


Please consider the attached diff.

All the best

[1] https://en.wikipedia.org/wiki/Veni,_vidi,_vici

--
Alessandro De Laurenzis
[mailto:jus...@atlantide.mooo.com]
Web: http://www.atlantide.mooo.com
LinkedIn: http://it.linkedin.com/in/delaurenzis
--- /usr/src/games/fortune/datfiles/fortunes2-o.orig	Thu Jul 13 04:45:56 2017
+++ /usr/src/games/fortune/datfiles/fortunes2-o	Mon Aug 23 20:07:23 2021
@@ -14251,8 +14251,8 @@
 to a rival.  Husbands, good or bad, always have rivals.  Lovers, never.
 		-- Helen Lawrenson, "Esquire"
 %
-Vidi, vici, veni.
-(I saw, I conquered, I came.)
+Veni, vidi, vici.
+(I came, I saw, I conquered.)
 %
 Viennese Oyster: Lady who can cross her feet behind her head, lying on her
 back, of course.  When she has done so, you hold her tightly round each instep


Re: Reference dhcpleased.conf(5)

2021-08-23 Thread Florian Obser
On 2021-08-22 18:36 -04, Scott Bennett  wrote:
> Like the rad(8) and unwind(8) manuals do, add references to
> dhcpleased.conf(5) in the appropriate places.

Committed, thanks!

>
> Cheers,
> Scott
>
> diff 4ccbc464479218d5b5f4125325c4d9358f653323 /usr/src
> blob - 7ee3d8f92a1d31880ce1729f21940fd38ec24930
> file + sbin/dhcpleased/dhcpleased.8
> --- sbin/dhcpleased/dhcpleased.8
> +++ sbin/dhcpleased/dhcpleased.8
> @@ -83,6 +83,7 @@ Default
>  configuration file.
>  .El
>  .Sh SEE ALSO
> +.Xr dhcpleased.conf 5 ,
>  .Xr hostname.if 5 ,
>  .Xr dhcpd 8 ,
>  .Xr dhcpleasectl 8 ,
> blob - b68d592a25dead357e2a057c2026081821e15dc7
> file + usr.sbin/dhcpleasectl/dhcpleasectl.8
> --- usr.sbin/dhcpleasectl/dhcpleasectl.8
> +++ usr.sbin/dhcpleasectl/dhcpleasectl.8
> @@ -72,6 +72,7 @@ socket used for communication with
>  .Xr dhcpleased 8 .
>  .El
>  .Sh SEE ALSO
> +.Xr dhcpleased.conf 5 ,
>  .Xr dhcpleased 8
>  .Sh HISTORY
>  The
>

-- 
I'm not entirely sure you are real.



Re: virtio(4): don't require legacy mode to have an I/O BAR

2021-08-23 Thread Mark Kettenis
> Date: Mon, 23 Aug 2021 17:53:06 +0200
> From: Patrick Wildt 
> 
> Hi,
> 
> so on the new Parallels version, when using the 'Other' OS setting,
> virtio(4) won't attach.  Apparently it's not Virtio 1.0, because even
> Fedora 34 falls back to the 'legacy' driver.
> 
> While our code expects (and requires) an I/O BAR, it seems to be that
> the PCI device only provides two memory BARs.
> 
> Linux still works, probably because they don't care about the type.
> So I figured, let's just do that as well.  With the following diff,
> virtio(4) attach es again, and I can install over vio(4).
> 
> I don't know if this violates any official virtio(4) spec, but on
> the other hand... it fixes as bug, makes it work, and just loosens
> up the requirement a little.
> 
> ok?

ok kettenis@

> diff --git a/sys/dev/pci/virtio_pci.c b/sys/dev/pci/virtio_pci.c
> index c99f50136cd..0a29293e16c 100644
> --- a/sys/dev/pci/virtio_pci.c
> +++ b/sys/dev/pci/virtio_pci.c
> @@ -508,7 +508,10 @@ int
>  virtio_pci_attach_09(struct virtio_pci_softc *sc, struct pci_attach_args *pa)
>  {
>   struct virtio_softc *vsc = >sc_sc;
> - if (pci_mapreg_map(pa, PCI_MAPREG_START, PCI_MAPREG_TYPE_IO, 0,
> + pcireg_t type;
> +
> + type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, PCI_MAPREG_START);
> + if (pci_mapreg_map(pa, PCI_MAPREG_START, type, 0,
>   >sc_iot, >sc_ioh, NULL, >sc_iosize, 0)) {
>   printf("%s: can't map i/o space\n", vsc->sc_dev.dv_xname);
>   return EIO;
> 
> 



virtio(4): don't require legacy mode to have an I/O BAR

2021-08-23 Thread Patrick Wildt
Hi,

so on the new Parallels version, when using the 'Other' OS setting,
virtio(4) won't attach.  Apparently it's not Virtio 1.0, because even
Fedora 34 falls back to the 'legacy' driver.

While our code expects (and requires) an I/O BAR, it seems to be that
the PCI device only provides two memory BARs.

Linux still works, probably because they don't care about the type.
So I figured, let's just do that as well.  With the following diff,
virtio(4) attach es again, and I can install over vio(4).

I don't know if this violates any official virtio(4) spec, but on
the other hand... it fixes as bug, makes it work, and just loosens
up the requirement a little.

ok?

Patrick

diff --git a/sys/dev/pci/virtio_pci.c b/sys/dev/pci/virtio_pci.c
index c99f50136cd..0a29293e16c 100644
--- a/sys/dev/pci/virtio_pci.c
+++ b/sys/dev/pci/virtio_pci.c
@@ -508,7 +508,10 @@ int
 virtio_pci_attach_09(struct virtio_pci_softc *sc, struct pci_attach_args *pa)
 {
struct virtio_softc *vsc = >sc_sc;
-   if (pci_mapreg_map(pa, PCI_MAPREG_START, PCI_MAPREG_TYPE_IO, 0,
+   pcireg_t type;
+
+   type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, PCI_MAPREG_START);
+   if (pci_mapreg_map(pa, PCI_MAPREG_START, type, 0,
>sc_iot, >sc_ioh, NULL, >sc_iosize, 0)) {
printf("%s: can't map i/o space\n", vsc->sc_dev.dv_xname);
return EIO;



Re: ihidev: fix data request size

2021-08-23 Thread Thomas Frohwein
On Mon, Aug 23, 2021 at 08:57:59AM -0500, joshua stein wrote:
> On Sun, 22 Aug 2021 at 22:37:51 -0600, Thomas Frohwein wrote:
[...]
> > I thought I had the same problem on my new Asus Expertbook B9400CEA.
> > During boot, while reordering libraries and later it would print:
> > 
> > dwiic2: timed out reading remaining 16
> > 
> > This is accompanied by generalized slowness of the system - boot is
> > noticeably slow, xenodm takes about 20 seconds to get to the login
> > window, and glxgears(1) slows down visibly with any mouse movement.
> 
> That is because dwiic has to use polling while being invoked by 
> ihidev's interrupt handler or else it panics in tsleep.  It has to 
> wait longer than normal using delay(), so it blocks other kernel 
> processing.
> 
> As for why the timeouts are happening in the first place, it could 
> be a similar issue with the max report size being bogus.  Does this 
> diff change anything?

Thanks for taking a look. The only thing that seems to change is that
the message is now:

dwiic2: timed out reading remaining 4

... not 16 anymore.

dmesg from a kernel built with this patch attached.

> 
> 
> diff --git sys/dev/i2c/ihidev.c sys/dev/i2c/ihidev.c
> index 92778c679ad..0baba616329 100644
> --- sys/dev/i2c/ihidev.c
> +++ sys/dev/i2c/ihidev.c
> @@ -152,7 +152,7 @@ ihidev_attach(struct device *parent, struct device *self, 
> void *aux)
>   }
>  
>   /* find largest report size and allocate memory for input buffer */
> - sc->sc_isize = letoh16(sc->hid_desc.wMaxInputLength);
> + sc->sc_isize = 4;
>   for (repid = 0; repid < sc->sc_nrepid; repid++) {
>   repsz = hid_report_size(sc->sc_report, sc->sc_reportlen,
>   hid_input, repid);
> @@ -643,7 +643,7 @@ ihidev_intr(void *arg)
>  
>   iic_acquire_bus(sc->sc_tag, I2C_F_POLL);
>   res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, NULL, 0,
> - sc->sc_ibuf, letoh16(sc->hid_desc.wMaxInputLength), I2C_F_POLL);
> + sc->sc_ibuf, sc->sc_isize, I2C_F_POLL);
>   iic_release_bus(sc->sc_tag, I2C_F_POLL);
>  
>   /*
> 
OpenBSD 7.0-beta (GENERIC.MP) #1: Mon Aug 23 08:20:12 MDT 2021
t...@thfr.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 16825262080 (16045MB)
avail mem = 16299372544 (15544MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.3 @ 0x5a4b2000 (95 entries)
bios0: vendor ASUSTeK COMPUTER INC. version "B9400CEA.304" date 06/02/2021
bios0: ASUSTeK COMPUTER INC. ASUS EXPERTBOOK B9400CEA_B9450CEA
acpi0 at bios0: ACPI 6.2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP MCFG SSDT FIDT ECDT MSDM SSDT SSDT SSDT SSDT HPET APIC 
SSDT NHLT LPIT SSDT DBGP DBG2 SSDT DMAR SSDT SSDT TPM2 BGRT PTDT WSMT FPDT
acpi0: wakeup devices PEGP(S4) PEGP(S4) PEGP(S4) PEG0(S4) PEGP(S4) GLAN(S4) 
XHCI(S3) XDCI(S4) HDAS(S4) CNVW(S4) TXHC(S3) TDM0(S4) TRP0(S3) PXSX(S4) 
TRP1(S3) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xc000, bus 0-255
acpiec0 at acpi0
acpihpet0 at acpi0: 1920 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 68555.29 MHz, 06-8c-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 256KB 64b/line disabled L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 38MHz
cpu0: mwait min=64, max=64, C-substates=0.2.0.1.2.1.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 4689.20 MHz, 06-8c-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu1: 256KB 64b/line disabled L2 cache
cpu1: disabling user TSC (skew=-1036510775)
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: 11th Gen Intel(R) Core(TM) 

Re: ihidev: fix data request size

2021-08-23 Thread joshua stein
On Sun, 22 Aug 2021 at 22:37:51 -0600, Thomas Frohwein wrote:
> On Sun, Aug 22, 2021 at 09:50:15PM -0500, joshua stein wrote:
> > On a particular laptop with a touchpad behind ihidev, dwiic would 
> > report a timeout every time it had to fetch touch data:
> > 
> > dwiic0: timed out reading remaining 2
> > 
> > On re-reading the i2c HID spec, the size supplied by wMaxInputLength 
> > is already supposed to account for the size and report id bytes so 
> > we shouldn't be adding them after the fact.  Otherwise ihidev would 
> > ask for more data than can be available and, on this laptop anyway, 
> > dwiic would have to wait for this transaction to timeout and fail.
> > 
> > This fix matches how the Linux i2c-hid driver operates.  I've tested 
> > this on 3 laptops with touchpads and touchscreens and it doesn't 
> > cause any regressions here while fixing the touchpad on one of them.  
> > I'd appreciate tests on other laptops to make sure it doesn't break 
> > anything and perhaps fixes your issue if you've also seen constant 
> > dwiic timeouts.
> > 
> 
> I thought I had the same problem on my new Asus Expertbook B9400CEA.
> During boot, while reordering libraries and later it would print:
> 
> dwiic2: timed out reading remaining 16
> 
> This is accompanied by generalized slowness of the system - boot is
> noticeably slow, xenodm takes about 20 seconds to get to the login
> window, and glxgears(1) slows down visibly with any mouse movement.

That is because dwiic has to use polling while being invoked by 
ihidev's interrupt handler or else it panics in tsleep.  It has to 
wait longer than normal using delay(), so it blocks other kernel 
processing.

As for why the timeouts are happening in the first place, it could 
be a similar issue with the max report size being bogus.  Does this 
diff change anything?


diff --git sys/dev/i2c/ihidev.c sys/dev/i2c/ihidev.c
index 92778c679ad..0baba616329 100644
--- sys/dev/i2c/ihidev.c
+++ sys/dev/i2c/ihidev.c
@@ -152,7 +152,7 @@ ihidev_attach(struct device *parent, struct device *self, 
void *aux)
}
 
/* find largest report size and allocate memory for input buffer */
-   sc->sc_isize = letoh16(sc->hid_desc.wMaxInputLength);
+   sc->sc_isize = 4;
for (repid = 0; repid < sc->sc_nrepid; repid++) {
repsz = hid_report_size(sc->sc_report, sc->sc_reportlen,
hid_input, repid);
@@ -643,7 +643,7 @@ ihidev_intr(void *arg)
 
iic_acquire_bus(sc->sc_tag, I2C_F_POLL);
res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, NULL, 0,
-   sc->sc_ibuf, letoh16(sc->hid_desc.wMaxInputLength), I2C_F_POLL);
+   sc->sc_ibuf, sc->sc_isize, I2C_F_POLL);
iic_release_bus(sc->sc_tag, I2C_F_POLL);
 
/*