Re: Improve ure(4) performance

[Mikolaj Kucharski]

On Sun, Jul 26, 2020 at 06:47:32PM -0700, Jonathon Fletcher wrote:
> Mikolaj,
> 
> Thank you for testing my patch.
> 
> I am very interested to know what xhci (or ehci) hardware you have.
> 
> I have the following:
> 
> xhci0 at pci0 dev 20 function 0 "Intel 9 Series xHCI" rev 0x03: msi, xHCI 1.0
> usb0 at xhci0: USB revision 3.0
> uhub0 at usb0 configuration 1 interface 0 "Intel xHCI root hub" rev 3.00/1.00 
> addr 1
> ure0 at uhub0 port 14 configuration 1 interface 0 "Realtek USB 10/100/1G/2.5G 
> LAN" rev 3.20/30.00 addr 3
> ure0: RTL8156 (0x7030), address 00:e0:4c:ab:64:5a
> 
> My ure0 is https://www.amazon.com/gp/product/B07Z8S6PN4 
> 
> 
> I do not get any watchdog errors with this.
> 
> Kevin has been testing my patch and giving me good feedback. He has seen some 
> watchdog errors with an RTL8153B also.
> 
> I am starting to suspect that I have the tx xfer size wrong (too big) for the 
> 8153 / 8153B devices. I am also trying to eliminate the XHCI hardware as a 
> cause.
> 
> Please could you post your dmesg? At a minimum it would help me a lot to see 
> your usb hardware.
> 

Sure, no problem. I'm testing your patch with:

https://www.amazon.co.uk/gp/product/B081YGDBG7/


OpenBSD 6.7-current (GENERIC.MP) #20: Sun Jul 26 19:49:23 UTC 2020

r...@pc1.home.local:/home/mkucharski/openbsd/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8302006272 (7917MB)
avail mem = 8035315712 (7663MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f114000 (64 entries)
bios0: vendor HUAWEI version "2.00" date 11/07/2017
bios0: HUAWEI HUAWEI MateBook X
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP UEFI UEFI ECDT SSDT MSDM SSDT SSDT SSDT ASPT BOOT HPET 
APIC MCFG SSDT WSMT SSDT DBGP DBG2 SSDT SSDT DMAR NHLT FPDT BGRT
acpi0: wakeup devices GLAN(S4) XHC_(S3) XDCI(S4) HDAS(S4) RP01(S4) PXSX(S4) 
RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) 
RP06(S4) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpiec0 at acpi0
acpihpet0 at acpi0: 2399 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2595.04 MHz, 06-8e-09
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,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu0: 256KB 64b/line 8-way 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 24MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.97 MHz, 06-8e-09
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,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 1 (application processor)
cpu2: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.96 MHz, 06-8e-09
cpu2: 
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,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 1, core 0, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.97 MHz, 06-8e-09
cpu3: 
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,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLE

Re: Improve ure(4) performance

[Jonathon Fletcher]

> On Jul 26, 2020, at 1:24 PM, Mikolaj Kucharski  wrote:
> 
> Hi Kevin, Jonathon,
> 
> On Tue, Jul 21, 2020 at 02:38:55PM +0800, Kevin Lo wrote:
>> Hi,
>> 
>> Jonathon Fletcher has been helping get the better performance out of ure(4).
>> I ran the tcpbench with ure (RTL8156) for 5 minutes:
>> 
>> 71538432760 bytes sent over 300.999 seconds
>> bandwidth min/avg/max/std-dev = 12.071/1901.448/1947.873/135.283 Mbps
>> 
>> A big thanks to Jonathon for his hard work.
> 
> I've tested this on amd64 with following ure(4) card:
> 
> ure0 at uhub2 port 2 configuration 1 interface 0 "Realtek USB 10/100/1000 
> LAN" rev 2.10/30.00 addr 4
> ure0: RTL8153 (0x5c30), address 00:e0:4c:04:09:7d
> 
> I could see speedtest-cli improvement from around 150 Mbit/s before the
> diff, to 245 Mbit/s after the diff.
> 
> Big thank you for this patch. Will run it for next couple of days.
> Before the diff I faced following errors with ure(4):
> 
> usbd_start_next: error=5
> ure0: watchdog timeout
> ure0: usb errors on rx: IOERROR
> 
> and unplug/re-plug dance was needed to bring back the network. That
> usually happend during Google Meet video call, so I need few days to see
> how this diff goes against web video calls.

Mikolaj,

Thank you for testing my patch.

I am very interested to know what xhci (or ehci) hardware you have.

I have the following:

xhci0 at pci0 dev 20 function 0 "Intel 9 Series xHCI" rev 0x03: msi, xHCI 1.0
usb0 at xhci0: USB revision 3.0
uhub0 at usb0 configuration 1 interface 0 "Intel xHCI root hub" rev 3.00/1.00 
addr 1
ure0 at uhub0 port 14 configuration 1 interface 0 "Realtek USB 10/100/1G/2.5G 
LAN" rev 3.20/30.00 addr 3
ure0: RTL8156 (0x7030), address 00:e0:4c:ab:64:5a

My ure0 is https://www.amazon.com/gp/product/B07Z8S6PN4 


I do not get any watchdog errors with this.

Kevin has been testing my patch and giving me good feedback. He has seen some 
watchdog errors with an RTL8153B also.

I am starting to suspect that I have the tx xfer size wrong (too big) for the 
8153 / 8153B devices. I am also trying to eliminate the XHCI hardware as a 
cause.

Please could you post your dmesg? At a minimum it would help me a lot to see 
your usb hardware.

Thanks,
Jonathon

Re: ssh(1), getrrsetbyname(3), SSHFP and DNSSEC

[Jeremie Courreges-Anglas]

On Sun, Jul 26 2020, Jesper Wallin  wrote:
> On Sat, Jul 25, 2020 at 02:01:06PM +0200, Jeremie Courreges-Anglas wrote:
>> 
>> For those two reasons I think the feature should be opt-in.
>
> Yeah, I agree with you.  My first approach was to have it check what
> kind of DNS record that was requested, and add the AD-flag only if type
> was SSHFP, but that felt even uglier.  I also wasn't so sure my approach
> was the right one after reading the RFCs Peter J. Philipp mentioned.

The quote from RFC6840 seems clear to me, care to share why you had some
doubts if they still exist?

> Perhaps another approach would be to make use of the currently unused
> flags argument in getrrsetbyname(3)?  This way, only getrrsetbyname(3)
> and certain requests are affected by it.

I thought about that too, but getrrsetbyname(3) isn't the only function
of interest.  There's also res_query(3) and friends, which are in more
widely use in the larger ecosystem.  I guess we could restrict AD flag
tweaking to APIs where the caller can actually access the AD flag in the
response, but the "default vs opt-in" question is still present.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

timeout(9): remove unused timeout_add_ts(9), timeout_add_bt(9)

[Scott Cheloha]

timeout_add_ts(9) and timeout_add_bt(9) are not used anywhere.

ok to remove them?

Index: sys/kern/kern_timeout.c
===
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.76
diff -u -p -r1.76 kern_timeout.c
--- sys/kern/kern_timeout.c 25 Jul 2020 00:48:04 -  1.76
+++ sys/kern/kern_timeout.c 27 Jul 2020 01:22:20 -
@@ -297,35 +297,6 @@ timeout_add_tv(struct timeout *to, const
 }
 
 int
-timeout_add_ts(struct timeout *to, const struct timespec *ts)
-{
-   uint64_t to_ticks;
-
-   to_ticks = (uint64_t)hz * ts->tv_sec + ts->tv_nsec / (tick * 1000);
-   if (to_ticks > INT_MAX)
-   to_ticks = INT_MAX;
-   if (to_ticks == 0 && ts->tv_nsec > 0)
-   to_ticks = 1;
-
-   return timeout_add(to, (int)to_ticks);
-}
-
-int
-timeout_add_bt(struct timeout *to, const struct bintime *bt)
-{
-   uint64_t to_ticks;
-
-   to_ticks = (uint64_t)hz * bt->sec + (long)(((uint64_t)100 *
-   (uint32_t)(bt->frac >> 32)) >> 32) / tick;
-   if (to_ticks > INT_MAX)
-   to_ticks = INT_MAX;
-   if (to_ticks == 0 && bt->frac > 0)
-   to_ticks = 1;
-
-   return timeout_add(to, (int)to_ticks);
-}
-
-int
 timeout_add_sec(struct timeout *to, int secs)
 {
uint64_t to_ticks;
Index: sys/sys/timeout.h
===
RCS file: /cvs/src/sys/sys/timeout.h,v
retrieving revision 1.37
diff -u -p -r1.37 timeout.h
--- sys/sys/timeout.h   25 Jul 2020 00:48:03 -  1.37
+++ sys/sys/timeout.h   27 Jul 2020 01:22:20 -
@@ -110,15 +110,11 @@ int timeout_sysctl(void *, size_t *, voi
 
 #define TIMEOUT_INITIALIZER(_f, _a) TIMEOUT_INITIALIZER_FLAGS((_f), (_a), 0)
 
-struct bintime;
-
 void timeout_set(struct timeout *, void (*)(void *), void *);
 void timeout_set_flags(struct timeout *, void (*)(void *), void *, int);
 void timeout_set_proc(struct timeout *, void (*)(void *), void *);
 int timeout_add(struct timeout *, int);
 int timeout_add_tv(struct timeout *, const struct timeval *);
-int timeout_add_ts(struct timeout *, const struct timespec *);
-int timeout_add_bt(struct timeout *, const struct bintime *);
 int timeout_add_sec(struct timeout *, int);
 int timeout_add_msec(struct timeout *, int);
 int timeout_add_usec(struct timeout *, int);
Index: share/man/man9/timeout.9
===
RCS file: /cvs/src/share/man/man9/timeout.9,v
retrieving revision 1.50
diff -u -p -r1.50 timeout.9
--- share/man/man9/timeout.93 Jan 2020 02:16:38 -   1.50
+++ share/man/man9/timeout.927 Jul 2020 01:22:20 -
@@ -35,8 +35,6 @@
 .Nm timeout_add_nsec ,
 .Nm timeout_add_usec ,
 .Nm timeout_add_tv ,
-.Nm timeout_add_ts ,
-.Nm timeout_add_bt ,
 .Nm timeout_del ,
 .Nm timeout_del_barrier ,
 .Nm timeout_barrier ,
@@ -75,10 +73,6 @@
 .Ft int
 .Fn timeout_add_tv "struct timeout *to" "struct timeval *"
 .Ft int
-.Fn timeout_add_ts "struct timeout *to" "struct timespec *"
-.Ft int
-.Fn timeout_add_bt "struct timeout *to" "struct bintime *"
-.Ft int
 .Fn timeout_add_sec "struct timeout *to" "int sec"
 .Ft int
 .Fn timeout_add_msec "struct timeout *to" "int msec"
@@ -212,8 +206,6 @@ functions clear the triggered state for 
 .Pp
 When possible, use the
 .Fn timeout_add_tv ,
-.Fn timeout_add_ts ,
-.Fn timeout_add_bt ,
 .Fn timeout_add_sec ,
 .Fn timeout_add_msec ,
 .Fn timeout_add_usec ,
@@ -257,8 +249,6 @@ context.
 .Fn timeout_add_nsec ,
 .Fn timeout_add_usec ,
 .Fn timeout_add_tv ,
-.Fn timeout_add_ts ,
-.Fn timeout_add_bt ,
 .Fn timeout_del ,
 .Fn timeout_pending ,
 .Fn timeout_initialized ,
@@ -289,10 +279,8 @@ flag was given at initialization.
 .Fn timeout_add_msec ,
 .Fn timeout_add_nsec ,
 .Fn timeout_add_usec ,
-.Fn timeout_add_tv ,
-.Fn timeout_add_ts ,
 and
-.Fn timeout_add_bt
+.Fn timeout_add_tv
 will return 1 if the timeout
 .Fa to
 was added to the timeout schedule or 0 if it was already queued.

Re: ldapd: adding bsd.schema

[Aisha Tammy]

On 7/26/20 5:21 PM, Aisha Tammy wrote:
> Hi,
>   Am reviving an old thread from 
> https://marc.info/?l=openbsd-tech&m=152663835315469&w=4
> (i did cc reyk@ sorry if it is noise)
> 
> For some reason seems like the patch didn't go through...
> 
> I am reattaching it here, maybe someone can take a look and 
> see if it can be merged ?
> Getting sshPublicKey would be really nice!
> 
> Aisha
>  


reattaching it because thunderbird
Index: etc/examples/ldapd.conf
===
RCS file: /cvs/src/etc/examples/ldapd.conf,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 ldapd.conf
--- etc/examples/ldapd.conf	11 Jul 2014 21:20:10 -	1.1
+++ etc/examples/ldapd.conf	18 May 2018 10:09:45 -
@@ -3,6 +3,7 @@
 schema "/etc/ldap/core.schema"
 schema "/etc/ldap/inetorgperson.schema"
 schema "/etc/ldap/nis.schema"
+schema "/etc/ldap/bsd.schema"
 
 listen on lo0
 listen on "/var/run/ldapi"
Index: usr.sbin/ldapd/Makefile
===
RCS file: /cvs/src/usr.sbin/ldapd/Makefile,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 Makefile
--- usr.sbin/ldapd/Makefile	20 Jan 2017 11:55:08 -	1.15
+++ usr.sbin/ldapd/Makefile	18 May 2018 10:09:45 -
@@ -17,7 +17,8 @@ CFLAGS+=	-Wshadow -Wpointer-arith -Wcast
 CFLAGS+=	-Wsign-compare
 CLEANFILES+=	y.tab.h parse.c
 
-SCHEMA_FILES=	core.schema \
+SCHEMA_FILES=	bsd.schema \
+		core.schema \
 		inetorgperson.schema \
 		nis.schema
 
Index: usr.sbin/ldapd/schema/bsd.schema
===
RCS file: usr.sbin/ldapd/schema/bsd.schema
diff -N usr.sbin/ldapd/schema/bsd.schema
--- /dev/null	1 Jan 1970 00:00:00 -
+++ usr.sbin/ldapd/schema/bsd.schema	18 May 2018 10:09:45 -
@@ -0,0 +1,17 @@
+attributetype ( 1.3.6.1.4.1.30155.115.2 NAME 'shadowPassword'
+	DESC 'POSIX hashed password'
+	EQUALITY caseExactIA5Match
+	SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+
+attributetype ( 1.3.6.1.4.1.30155.115.3 NAME 'sshPublicKey'
+	DESC 'SSH public key'
+	EQUALITY caseExactIA5Match
+	SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+
+objectclass ( 1.3.6.1.4.1.30155.115.1 NAME 'bsdAccount'
+ 	SUP top
+ 	AUXILIARY
+ 	DESC 'Abstraction of an account with OpenBSD attributes'
+	MUST ( uid )
+	MAY ( shadowPassword $ shadowExpire $ modifyTimestamp $ userClass $
+		sshPublicKey ))

ldapd: adding bsd.schema

[Aisha Tammy]

Hi,
  Am reviving an old thread from 
https://marc.info/?l=openbsd-tech&m=152663835315469&w=4
(i did cc reyk@ sorry if it is noise)

For some reason seems like the patch didn't go through...

I am reattaching it here, maybe someone can take a look and 
see if it can be merged ?
Getting sshPublicKey would be really nice!

Aisha
 

Index: etc/examples/ldapd.conf

===

RCS file: /cvs/src/etc/examples/ldapd.conf,v

retrieving revision 1.1

diff -u -p -u -p -r1.1 ldapd.conf

--- etc/examples/ldapd.conf 11 Jul 2014 21:20:10 -  1.1

+++ etc/examples/ldapd.conf 18 May 2018 10:09:45 -

@@ -3,6 +3,7 @@

 schema "/etc/ldap/core.schema"

 schema "/etc/ldap/inetorgperson.schema"

 schema "/etc/ldap/nis.schema"

+schema "/etc/ldap/bsd.schema"

 

 listen on lo0

 listen on "/var/run/ldapi"

Index: usr.sbin/ldapd/Makefile

===

RCS file: /cvs/src/usr.sbin/ldapd/Makefile,v

retrieving revision 1.15

diff -u -p -u -p -r1.15 Makefile

--- usr.sbin/ldapd/Makefile 20 Jan 2017 11:55:08 -  1.15

+++ usr.sbin/ldapd/Makefile 18 May 2018 10:09:45 -

@@ -17,7 +17,8 @@ CFLAGS+=  -Wshadow -Wpointer-arith -Wcast

 CFLAGS+=   -Wsign-compare

 CLEANFILES+=   y.tab.h parse.c

 

-SCHEMA_FILES=  core.schema \

+SCHEMA_FILES=  bsd.schema \

+   core.schema \

inetorgperson.schema \

nis.schema

 

Index: usr.sbin/ldapd/schema/bsd.schema

===

RCS file: usr.sbin/ldapd/schema/bsd.schema

diff -N usr.sbin/ldapd/schema/bsd.schema

--- /dev/null   1 Jan 1970 00:00:00 -

+++ usr.sbin/ldapd/schema/bsd.schema18 May 2018 10:09:45 -

@@ -0,0 +1,17 @@

+attributetype ( 1.3.6.1.4.1.30155.115.2 NAME 'shadowPassword'

+   DESC 'POSIX hashed password'

+   EQUALITY caseExactIA5Match

+   SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

+

+attributetype ( 1.3.6.1.4.1.30155.115.3 NAME 'sshPublicKey'

+   DESC 'SSH public key'

+   EQUALITY caseExactIA5Match

+   SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

+

+objectclass ( 1.3.6.1.4.1.30155.115.1 NAME 'bsdAccount'

+   SUP top

+   AUXILIARY

+   DESC 'Abstraction of an account with OpenBSD attributes'

+   MUST ( uid )

+   MAY ( shadowPassword $ shadowExpire $ modifyTimestamp $ userClass $

+   sshPublicKey ))

route: add size to free(9) calls

[Klemens Nanni]

Those are for the gateway sockaddrs which get allocated in rt_setgate()
with the same ROUNDUP(sa_len) approach.

mpi already added a sizes for a few rt_gateway sockaddrs in two commits,
these are the last one in route.c leaving only ifafree() behind.

Also tested on a few machines during last weeks.

Feedback? OK?


Index: route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.394
diff -u -p -r1.394 route.c
--- route.c 24 Jun 2020 22:03:43 -  1.394
+++ route.c 27 Jul 2020 00:10:17 -
@@ -932,7 +932,8 @@ rtrequest(int req, struct rt_addrinfo *i
ifafree(ifa);
rtfree(rt->rt_parent);
rt_putgwroute(rt);
-   free(rt->rt_gateway, M_RTABLE, 0);
+   free(rt->rt_gateway, M_RTABLE,
+   ROUNDUP(rt->rt_gateway->sa_len));
free(ndst, M_RTABLE, ndst->sa_len);
pool_put(&rtentry_pool, rt);
return (error);
@@ -964,7 +965,8 @@ rtrequest(int req, struct rt_addrinfo *i
ifafree(ifa);
rtfree(rt->rt_parent);
rt_putgwroute(rt);
-   free(rt->rt_gateway, M_RTABLE, 0);
+   free(rt->rt_gateway, M_RTABLE,
+   ROUNDUP(rt->rt_gateway->sa_len));
free(ndst, M_RTABLE, ndst->sa_len);
pool_put(&rtentry_pool, rt);
return (EEXIST);

Re: change ktime to nanoseconds in drm

[Scott Cheloha]

On Fri, Jul 24, 2020 at 04:05:18PM +1000, Jonathan Gray wrote:
> On Tue, Jul 21, 2020 at 05:10:02PM -0500, Scott Cheloha wrote:
> > On Tue, Jul 21, 2020 at 07:33:21PM +1000, Jonathan Gray wrote:
> > > Change from using timevals for ktime to 64 bit count of nanoseconds
> > > to closer match linux.  From a discussion with cheloha@
> > > 
> > > 
> > > 
> > > @@ -67,70 +65,66 @@ ktime_get_raw_ns(void)
> > >  }
> > >  
> > >  static inline struct timespec64
> > > -ktime_to_timespec64(struct timeval tv)
> > > +ktime_to_timespec64(ktime_t k)
> > >  {
> > >   struct timespec64 ts;
> > > - ts.tv_sec = tv.tv_sec;
> > > - ts.tv_nsec = tv.tv_usec * NSEC_PER_USEC;
> > > + ts.tv_sec = k / NSEC_PER_SEC;
> > > + ts.tv_nsec = k % NSEC_PER_SEC;
> > 
> > This isn't quite right.  You need to handle negative values of k, too.
> > 
> > ts.tv_sec = k / NSEC_PER_SEC;
> > ts.tv_nsec = k % NSEC_PER_SEC;
> > if (ts.tv_nsec < 0) {
> > ts.tv_sec--;
> > ts.tv_nsec += NSEC_PER_SEC;
> > }
> > 
> > 
> 
> ah right, tv_nsec should not be negative in that case

Been running with this a few days without any issues.

ok cheloha@

fwiw, my dmesg:

OpenBSD 6.7-current (GENERIC.MP) #309: Sun Jul 26 11:35:57 CDT 2020
ssc@jetsam.local:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 16895528960 (16112MB)
avail mem = 16368386048 (15610MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x9f03b000 (63 entries)
bios0: vendor LENOVO version "N23ET59W (1.34 )" date 11/08/2018
bios0: LENOVO 20KHCTO1WW
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP SSDT SSDT TPM2 UEFI SSDT SSDT HPET APIC MCFG ECDT SSDT 
SSDT BOOT BATB SSDT SSDT SSDT LPIT WSMT SSDT SSDT SSDT DBGP DBG2 MSDM DMAR NHLT 
ASF! FPDT UEFI
acpi0: wakeup devices GLAN(S4) XHC_(S3) XDCI(S4) HDAS(S4) RP01(S4) PXSX(S4) 
RP02(S4) PXSX(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) RP06(S4) 
PXSX(S4) RP07(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 2399 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1790.03 MHz, 06-8e-0a
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,SMX,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,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: TSC skew=0 observed drift=0
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 24MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
TSC skew=4
cpu1: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1795.82 MHz, 06-8e-0a
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,SMX,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,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: TSC skew=4 observed drift=0
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
TSC skew=-5
cpu2: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1794.99 MHz, 06-8e-0a
cpu2: 
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,SMX,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,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: TSC skew=-5 observed drift=0
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
TSC skew=-1
cpu3: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1795.82 MHz, 06-8e-0a
cpu3: 
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,SMX,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,ITS

Re: ssh(1), getrrsetbyname(3), SSHFP and DNSSEC

[Jeremie Courreges-Anglas]

On Sat, Jul 25 2020, Sebastian Benoit  wrote:
> Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2020.07.25 14:01:06 +0200:
>> On Fri, Jul 17 2020, Jesper Wallin  wrote:
>> > Hi all,
>> >
>> > I recently decided to add SSHFP records for my servers, since I never
>> > memorize or write down my key fingerprints.  I learned that if I
>> > want ssh(1) to trust these records, DNSSEC needs to be enabled for my
>> > zone.  To validate these records, ssh(1) is using getrrsetbyname(3),
>> > which checks if the AD (Authentic Data) flag is set in the response.
>> >
>> > To get a response with the AD flag set, the request itself also needs
>> > to have the AD flag set.  It turns out that getrrsetbyname(3) doesn't
>> > set this and will therefore never get a validated response, unless the
>> > resolver is configured to always send it, no matter what the client
>> > requested.
>> >
>> > It seems like unwind(8) behaves this way but it also responds with the
>> > RRSIG records, which is extra overhead and ignored by getrrsetbyname(3).
>> >
>> > This was mentioned a few years ago [0] and the solution suggested was
>> > to add the RES_USE_DNSSEC to _res.options, which also makes the resolver
>> > respond with the extra RRSIG records.
>> >
>> > Instead, by only setting the AD flag, both the request and the response
>> > has the same size as without the flag set.  The patch below will add
>> > RES_USE_AD as an option to _res.options and set it by default.
>> > This is also the default behaviour in dig(1), which I understand is a
>> > bit different, but that sure added some confusion while debugging this.
>> >
>> > This let you run unbound(8) or any other validating resolver on your
>> > local network and getrrsetbyname(3) will trust it.  Do read the CAVEATS
>> > in the manual of getrrsetbyname(3) though.
>> >
>> > As a side note, I noticed that the default value of _res.options was the
>> > same value as RES_DEFAULT, so I changed it to RES_DEFAULT instead, for
>> > the sake of consistency.
>> >
>> > Thoughts?
>> 
>> Thanks for addressing this longstanding issue.
>> 
>> I think using the AD bit in queries is a good idea. IIRC Peter J.
>> Philipp (cc'd) suggested using it but I was not thrilled because:
>> 
>> 1. the meaning of the AD bit in queries is relatively recent (2013
>>   I think[0])
>> 2. getrrsetbyname also collects signature records, and for this you need
>>   EDNS + the DO bit set.  Implementing this in was not 100% trivial,
>>   I think we had something working but Eric or I were not 100% happy
>>   with it.
>> 
>> 1. is probably not a concern, after all you're supposed to use
>> a trustworthy resolver, which should be modern and understand the
>> purpose of the AD bit in queries.
>> 
>> 2. is probably not a concern either.  I guess that all getrrsetbyname(3)
>> callers only care about the target records and the AD bit, not about the
>> signature records.  (Why would they use it for anyway?).  In the base
>> system, only ssh(1) and traceroute(8) use getrrsetbyname(3).
>> AFAIK no other system provides getrrsetbyname(3), and ISC has removed
>> getrrsetbyname and the whole lwres API in 2017[1].  So I'd say we're
>> free to improve our version of getrrsetbyname(3) as we see fit.
>
> This is a concern for the stub resolver, because edns and AD does not work
> everywhere.
> Indeed when we switched unbound to validate by default we
> learned that this is not a good idea for everyone - which lead to the
> development of unwind(8).

Do you by chance have any data regarding fallout because of the AD bit
set in queries?  I would expect it to be ignored when not supported.
EDNS and the DNSSEC DO bit is a different story indeed.

>> Now let's discuss the implementation.  There are two main things
>> I dislike with your diff:
>> - it affects all DNS queries done through libc, even if the user doesn't
>>   care at all about DNSSEC.  I suspect there's potential for fallout
>>   here, but I have no data to back this.
>> - trusting the AD bit by default looks a bit too easy.  The
>>   documentation says that people should only trust the AD bit /
>>   RRSET_VALIDATED if the communication between libc and the resolver is
>>   secure, but who actually reads the documentation?
>> 
>> For those two reasons I think the feature should be opt-in.  That's what
>> Florian Weimer did in glibc, with the implementation of RES_TRUSTAD[2][3].
>> The diff below implements the same semantics:
>> - by default, don't send queries the AD flag, and don't trust its value
>>   in replies
>> - if "options trust-ad" is set in resolv.conf(5), set the AD flag in
>>   queries and allow applications to look at the AD flag in replies.
>> 
>> There's one more thing I'd like to check in the res_* API, but I'm
>> sitting on this idea since too long already, and I'm happy with my test
>> results so far.
>> 
>> Thoughts?  Comments?
>
> Your observation that this should not be on by default is probably correct.
>
> If you enable trust-ad on a system that moves around, e

Re: Improve ure(4) performance

[Mikolaj Kucharski]

Hi Kevin, Jonathon,

On Tue, Jul 21, 2020 at 02:38:55PM +0800, Kevin Lo wrote:
> Hi,
> 
> Jonathon Fletcher has been helping get the better performance out of ure(4).
> I ran the tcpbench with ure (RTL8156) for 5 minutes:
> 
> 71538432760 bytes sent over 300.999 seconds
> bandwidth min/avg/max/std-dev = 12.071/1901.448/1947.873/135.283 Mbps
> 
> A big thanks to Jonathon for his hard work.

I've tested this on amd64 with following ure(4) card:

ure0 at uhub2 port 2 configuration 1 interface 0 "Realtek USB 10/100/1000 LAN" 
rev 2.10/30.00 addr 4
ure0: RTL8153 (0x5c30), address 00:e0:4c:04:09:7d

I could see speedtest-cli improvement from around 150 Mbit/s before the
diff, to 245 Mbit/s after the diff.

Big thank you for this patch. Will run it for next couple of days.
Before the diff I faced following errors with ure(4):

usbd_start_next: error=5
ure0: watchdog timeout
ure0: usb errors on rx: IOERROR

and unplug/re-plug dance was needed to bring back the network. That
usually happend during Google Meet video call, so I need few days to see
how this diff goes against web video calls.

> ok?
> 
> Index: sys/dev/usb/if_ure.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_ure.c,v
> retrieving revision 1.16
> diff -u -p -u -p -r1.16 if_ure.c
> --- sys/dev/usb/if_ure.c  10 Jul 2020 13:26:41 -  1.16
> +++ sys/dev/usb/if_ure.c  21 Jul 2020 05:37:45 -
> @@ -117,11 +117,13 @@ voidure_miibus_writereg(struct device 
>  void ure_lock_mii(struct ure_softc *);
>  void ure_unlock_mii(struct ure_softc *);
>  
> -int  ure_encap(struct ure_softc *, struct mbuf *);
> +int  ure_encap_txpkt(struct mbuf *, char *, uint32_t);
> +int  ure_encap_xfer(struct ifnet *, struct ure_softc *, struct 
> ure_chain *);
>  void ure_rxeof(struct usbd_xfer *, void *, usbd_status);
>  void ure_txeof(struct usbd_xfer *, void *, usbd_status);
> -int  ure_rx_list_init(struct ure_softc *);
> -int  ure_tx_list_init(struct ure_softc *);
> +int  ure_xfer_list_init(struct ure_softc *, struct ure_chain *,
> + uint32_t, int);
> +void ure_xfer_list_free(struct ure_softc *, struct ure_chain *, int);
>  
>  void ure_tick_task(void *);
>  void ure_tick(void *);
> @@ -625,12 +627,36 @@ void
>  ure_watchdog(struct ifnet *ifp)
>  {
>   struct ure_softc*sc = ifp->if_softc;
> + struct ure_chain*c;
> + usbd_status err;
> + int i, s;
> +
> + ifp->if_timer = 0;
>  
>   if (usbd_is_dying(sc->ure_udev))
>   return;
>  
> + if ((ifp->if_flags & (IFF_RUNNING|IFF_UP)) != (IFF_RUNNING|IFF_UP))
> + return;
> +
> + sc = ifp->if_softc;
> + s = splnet();
> +
>   ifp->if_oerrors++;
> - printf("%s: watchdog timeout\n", sc->ure_dev.dv_xname);
> + DPRINTF(("watchdog timeout\n"));
> +
> + for (i = 0; i < URE_TX_LIST_CNT; i++) {
> + c = &sc->ure_cdata.ure_tx_chain[i];
> + if (ml_len(&c->uc_mbuf_list) > 0) {
> + usbd_get_xfer_status(c->uc_xfer, NULL, NULL, NULL,
> + &err);
> + ure_txeof(c->uc_xfer, c, err);
> + }
> + }
> +
> + if (ifq_is_oactive(&ifp->if_snd))
> + ifq_restart(&ifp->if_snd);
> + splx(s);
>  }
>  
>  void
> @@ -653,18 +679,26 @@ ure_init(void *xsc)
>   else
>   ure_rtl8153_nic_reset(sc);
>  
> - if (ure_rx_list_init(sc) == ENOBUFS) {
> + if (ure_xfer_list_init(sc, sc->ure_cdata.ure_rx_chain,
> + sc->ure_rxbufsz, URE_RX_LIST_CNT) == ENOBUFS) {
>   printf("%s: rx list init failed\n", sc->ure_dev.dv_xname);
>   splx(s);
>   return;
>   }
>  
> - if (ure_tx_list_init(sc) == ENOBUFS) {
> + if (ure_xfer_list_init(sc, sc->ure_cdata.ure_tx_chain,
> + sc->ure_txbufsz, URE_TX_LIST_CNT) == ENOBUFS) {
>   printf("%s: tx list init failed\n", sc->ure_dev.dv_xname);
>   splx(s);
>   return;
>   }
>  
> + /* Initialize the SLIST we are using for the multiple tx buffers */
> + SLIST_INIT(&sc->ure_cdata.ure_tx_free);
> + for (i = 0; i < URE_TX_LIST_CNT; i++)
> + SLIST_INSERT_HEAD(&sc->ure_cdata.ure_tx_free,
> + &sc->ure_cdata.ure_tx_chain[i], ure_list);
> +
>   /* Set MAC address. */
>   ure_write_1(sc, URE_PLA_CRWECR, URE_MCU_TYPE_PLA, URE_CRWECR_CONFIG);
>   ure_write_mem(sc, URE_PLA_IDR, URE_MCU_TYPE_PLA | URE_BYTE_EN_SIX_BYTES,
> @@ -739,9 +773,9 @@ ure_init(void *xsc)
>  
>   /* Start up the receive pipe. */
>   for (i = 0; i < URE_RX_LIST_CNT; i++) {
> - c = &sc->ure_cdata.rx_chain[i];
> + c = &sc->ure_cdata.ure_rx_chain[i];
>   usbd_setup_xfer(c->uc_xfer, sc->ure_ep[URE_ENDPT_RX],
> -

mtx locking against myself panic message with __func__

[Mikolaj Kucharski]

Hi,

I've faced recently couple of panics like:

panic: mtx 0x8211ed38: locking against myself

and modified messages as with the below diff. I guess you may consider
that with the nature of this type of panic, function name don't help
much to norrow down the problem, nonetheless would following diff be
okay to commit?

See https://marc.info/?l=openbsd-bugs&m=159553956423219&w=2 for the
reference of the panic.

I've compile and runtime tested this only on amd64.


Index: arch/hppa/hppa/mutex.c
===
RCS file: /cvs/src/sys/arch/hppa/hppa/mutex.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 mutex.c
--- arch/hppa/hppa/mutex.c  23 Apr 2019 13:35:12 -  1.17
+++ arch/hppa/hppa/mutex.c  17 Jul 2020 07:08:54 -
@@ -74,7 +74,7 @@ mtx_enter_try(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
 
asm volatile (
@@ -108,7 +108,7 @@ mtx_enter(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
 
if (mtx->mtx_wantipl != IPL_NONE)
Index: arch/m88k/m88k/mutex.c
===
RCS file: /cvs/src/sys/arch/m88k/m88k/mutex.c,v
retrieving revision 1.1
diff -u -p -u -r1.1 mutex.c
--- arch/m88k/m88k/mutex.c  26 May 2020 11:55:10 -  1.1
+++ arch/m88k/m88k/mutex.c  17 Jul 2020 07:08:55 -
@@ -104,7 +104,7 @@ mtx_enter_try(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
if (mtx->mtx_wantipl != IPL_NONE)
splx(s);
@@ -126,7 +126,7 @@ mtx_enter(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
 
if (mtx->mtx_wantipl != IPL_NONE)
Index: kern/kern_lock.c
===
RCS file: /cvs/src/sys/kern/kern_lock.c,v
retrieving revision 1.71
diff -u -p -u -r1.71 kern_lock.c
--- kern/kern_lock.c5 Mar 2020 09:28:31 -   1.71
+++ kern/kern_lock.c17 Jul 2020 07:08:55 -
@@ -293,7 +293,7 @@ mtx_enter_try(struct mutex *mtx)
owner = atomic_cas_ptr(&mtx->mtx_owner, NULL, ci);
 #ifdef DIAGNOSTIC
if (__predict_false(owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
if (owner == NULL) {
membar_enter_after_atomic();
@@ -326,7 +326,7 @@ mtx_enter(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
 
if (mtx->mtx_wantipl != IPL_NONE)
Index: kern/kern_rwlock.c
===
RCS file: /cvs/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.45
diff -u -p -u -r1.45 kern_rwlock.c
--- kern/kern_rwlock.c  2 Mar 2020 17:07:49 -   1.45
+++ kern/kern_rwlock.c  17 Jul 2020 07:08:55 -
@@ -170,8 +170,8 @@ rw_enter_diag(struct rwlock *rwl, int fl
case RW_WRITE:
case RW_READ:
if (RW_PROC(curproc) == RW_PROC(rwl->rwl_owner))
-   panic("rw_enter: %s locking against myself",
-   rwl->rwl_name);
+   panic("%s: %s locking against myself",
+   __func__, rwl->rwl_name);
break;
case RW_DOWNGRADE:
/*


-- 
Regards,
 Mikolaj

Re: ifconfig.8: document aggr(4) under TRUNK

[Klemens Nanni]

On Sun, Jul 26, 2020 at 06:47:14PM +0100, Jason McIntyre wrote:
> certainly ok by me. i think this commit could add aggr to the list of
> devices in "create".
Right, thanks.
 
> bridge is a bit different i think. i wouldn;t like to have those
> synopses dropped because i think they're useful. but bridge was
> basically an entire, large, manpage that got shoehorned into ifconfig.8.
Yes, it came from brconfig(8) and is somewhat on its own.

I'll do at least the TRUNK synopsis then for correctness, it reads off
otherwise.

New diff with both additions.



Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.352
diff -u -p -r1.352 ifconfig.8
--- ifconfig.8  27 Jun 2020 17:46:29 -  1.352
+++ ifconfig.8  26 Jul 2020 18:02:45 -
@@ -181,6 +181,7 @@ The default broadcast address is the add
 Create the specified network pseudo-device.
 At least the following devices can be created on demand:
 .Pp
+.Xr aggr 4 ,
 .Xr bridge 4 ,
 .Xr carp 4 ,
 .Xr egre 4 ,
@@ -1759,15 +1760,19 @@ from all protected domains.
 .It Cm up
 Start the switch processing packets.
 .El
-.Sh TRUNK
+.Sh TRUNK (LINK AGGREGATION)
 .Nm ifconfig
 .Ar trunk-interface
+.Op Cm lacpmode Cm active Ns | Ns Cm passive
+.Op Cm lacptimeout Cm fast Ns | Ns Cm slow
 .Op Oo Fl Oc Ns Cm trunkport Ar child-iface
 .Op Cm trunkproto Ar proto
 .Pp
-The following options are available for a
+The following options are available for
+.Xr aggr 4
+and
 .Xr trunk 4
-interface:
+interfaces:
 .Bl -tag -width Ds
 .It Cm lacpmode Cm active Ns | Ns Cm passive
 Set the LACP trunk mode to either
@@ -1787,7 +1792,9 @@ as a trunk port.
 Remove the trunk port
 .Ar child-iface .
 .It Cm trunkproto Ar proto
-Set the trunk protocol.
+Set the link aggregation protocol on
+.Xr trunk 4
+interfaces.
 Refer to
 .Xr trunk 4
 for a complete list of the available protocols.

Re: ifconfig.8: document aggr(4) under TRUNK

[Jason McIntyre]

On Sun, Jul 26, 2020 at 07:33:55PM +0200, Klemens Nanni wrote:
> Except for `trunkproto' wich his trunk(4) specific, aggr(4) has the same
> options so I'd like to merge it into the same section just like TUNNEL
> documents mostly identical interfaces with differences mentioned in at
> specific options.
> 
> The wording "trunk" seems clear under OpenBSD, but other vendors give it
> a different meaning (e.g. Cisco), so I slightly enhanced the section
> name as well.
> 
> Feedback? Objections? OK?
> 

certainly ok by me. i think this commit could add aggr to the list of
devices in "create".

> Another thing: Note that `lacpmode' and `lacptimeout' are missing from
> the existing TRUNK synopsis - I hesitate to add them because it feels
> like too much duplication.  Would it perhaps be worthwile to drop each
> interface type section's synopsis all together?  Most options are under
> DESCRIPTION and the second biggest section is BRIDGE which does not have
> a synopsis, either.  Less seems more here.
> 
> If at all, this seems best handled in a different commit.
> 

bridge is a bit different i think. i wouldn;t like to have those
synopses dropped because i think they're useful. but bridge was
basically an entire, large, manpage that got shoehorned into ifconfig.8.

jmc

> NB: At least tpmr(4) is missing as well, which I'll handle soon as well.
> 
> 
> Index: ifconfig.8
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.352
> diff -u -p -r1.352 ifconfig.8
> --- ifconfig.827 Jun 2020 17:46:29 -  1.352
> +++ ifconfig.826 Jul 2020 17:22:24 -
> @@ -1759,15 +1759,17 @@ from all protected domains.
>  .It Cm up
>  Start the switch processing packets.
>  .El
> -.Sh TRUNK
> +.Sh TRUNK (LINK AGGREGATION)
>  .Nm ifconfig
>  .Ar trunk-interface
>  .Op Oo Fl Oc Ns Cm trunkport Ar child-iface
>  .Op Cm trunkproto Ar proto
>  .Pp
> -The following options are available for a
> +The following options are available for
> +.Xr aggr 4
> +and
>  .Xr trunk 4
> -interface:
> +interfaces:
>  .Bl -tag -width Ds
>  .It Cm lacpmode Cm active Ns | Ns Cm passive
>  Set the LACP trunk mode to either
> @@ -1787,7 +1789,9 @@ as a trunk port.
>  Remove the trunk port
>  .Ar child-iface .
>  .It Cm trunkproto Ar proto
> -Set the trunk protocol.
> +Set the link aggregation protocol on
> +.Xr trunk 4
> +interfaces.
>  Refer to
>  .Xr trunk 4
>  for a complete list of the available protocols.
> 

ifconfig.8: document aggr(4) under TRUNK

[Klemens Nanni]

Except for `trunkproto' wich his trunk(4) specific, aggr(4) has the same
options so I'd like to merge it into the same section just like TUNNEL
documents mostly identical interfaces with differences mentioned in at
specific options.

The wording "trunk" seems clear under OpenBSD, but other vendors give it
a different meaning (e.g. Cisco), so I slightly enhanced the section
name as well.

Feedback? Objections? OK?

Another thing: Note that `lacpmode' and `lacptimeout' are missing from
the existing TRUNK synopsis - I hesitate to add them because it feels
like too much duplication.  Would it perhaps be worthwile to drop each
interface type section's synopsis all together?  Most options are under
DESCRIPTION and the second biggest section is BRIDGE which does not have
a synopsis, either.  Less seems more here.

If at all, this seems best handled in a different commit.

NB: At least tpmr(4) is missing as well, which I'll handle soon as well.


Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.352
diff -u -p -r1.352 ifconfig.8
--- ifconfig.8  27 Jun 2020 17:46:29 -  1.352
+++ ifconfig.8  26 Jul 2020 17:22:24 -
@@ -1759,15 +1759,17 @@ from all protected domains.
 .It Cm up
 Start the switch processing packets.
 .El
-.Sh TRUNK
+.Sh TRUNK (LINK AGGREGATION)
 .Nm ifconfig
 .Ar trunk-interface
 .Op Oo Fl Oc Ns Cm trunkport Ar child-iface
 .Op Cm trunkproto Ar proto
 .Pp
-The following options are available for a
+The following options are available for
+.Xr aggr 4
+and
 .Xr trunk 4
-interface:
+interfaces:
 .Bl -tag -width Ds
 .It Cm lacpmode Cm active Ns | Ns Cm passive
 Set the LACP trunk mode to either
@@ -1787,7 +1789,9 @@ as a trunk port.
 Remove the trunk port
 .Ar child-iface .
 .It Cm trunkproto Ar proto
-Set the trunk protocol.
+Set the link aggregation protocol on
+.Xr trunk 4
+interfaces.
 Refer to
 .Xr trunk 4
 for a complete list of the available protocols.

Re: Python 3.8 os.listdir EINVAL on large directories

[Aaron Miller]

On Sun, 2020-07-26 at 17:05 +0100, Stuart Henderson wrote:
> Moving to tech.
> 
> In gmane.os.openbsd.misc, you wrote:
> > Hi all,
> > 
> > I am getting a stacktrace from the borg command in the borgbackup
> > package while checking a backup (see bottom of email for full
> > output, since it's verbose). The relevant part is this:
> > 
> > filenames = os.listdir(os.path.join(data_path, dir))
> >   OSError: [Errno 22] Invalid argument:
> > '/mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12'
> > 
> > This is same error is reproducible with a test Python 3.8 program:
> > 
> >  #!/usr/bin/env python
> > 
> >  import os
> >  os.listdir('/mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12/')
> > 
> > Running ktrace & kdump reveals the error is from calling
> > getdents(2):
> > 
> >  76903 python3.8
> > CALL  open(0x1ec7f06de3b0,0x3)
> >  76903 python3.8
> > NAMI  "/mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12/"
> >  76903 python3.8 RET   open 3
> >  [...]
> >  76903 python3.8 CALL  getdents(3,0x1ec7c9257000,0x4000)
> >  76903 python3.8 RET   getdents 16384/0x4000
> >  [...]
> >  76903 python3.8 CALL  getdents(3,0x1ec7c9257000,0x4000)
> >  76903 python3.8 RET   getdents 16384/0x4000
> >  [...]
> >  76903 python3.8 CALL  getdents(3,0x1ec7c9257000,0x4000)
> >  76903 python3.8 RET   getdents 16384/0x4000
> >  [...]
> >  76903 python3.8 CALL  getdents(3,0x1ec7c9257000,0x4000)
> >  76903 python3.8 RET   getdents -1 errno 22 Invalid argument
> > 
> > Looking at the man page for getdents(2), I found it interesting
> > that it says this call "is not a portable interface and should not
> > be used directly by applications" and it recommends using
> > readdir(3) instead.
> 
> ktrace only shows system calls not library functions. I don't
> see python calling getdents directly, there is a fair chance that
> python is calling readdir, and readdir is calling getdents.
> 
> > To give you a rough idea of the number of files and filename sizes
> > in this directory:
> > 
> >   $ ls /mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12/ | wc
> >   15341534   10738
> 
> I haven't been able to recreate this including over nfs yet.
> What filesystem type and mount options?

It's a freshly created 1.3 TB ext3 filesystem created with
mkfs.ext3 on Linux. No mount options, just:

# mount /dev/sd2k  /mnt

Even though I can read it just fine on Linux and OpenBSD (other
than with os.listdir in Python), fsck_ext2fs(8) shows errors:

# fsck_ext2fs -d /dev/sd2k
** /dev/rsd2k
superblock mismatches
offset 94, original 387159720, alternate 0
BAD SUPER BLOCK: VALUES IN SUPER BLOCK DISAGREE WITH THOSE IN
FIRST ALTERNATE
/dev/rsd2k: BLOCK SIZE DETERMINED TO BE ZERO

If you think it would help, I can make a .tar.gz of a directory
with zero-sized files having the same exact file names.

> 
> getdents(2) :-
> 
>format.  Up to nbytes of data will be transferred.  nbytes must be greater
>than or equal to the block size associated with the file (see stat(2)).
>Some filesystems may not support getdents() with buffers smaller than this
>size.
> ...
>getdents() will fail if:
> ...
>[EINVAL]   The file referenced by fd is not a directory, or nbytes
>   is too small for returning a directory entry or block of
>   entries, or the current position pointer is invalid.
> 
> > Where does the problem lie -- the upstream Python code, the
> > OpenBSD-specific patches in its port definition, or somewhere
> > else? And in case it matters, this is a -current amd64 system,
> > with "sysupgrade -s" executed on 7/15.
> > 
> > Thank you,
> > Aaron Miller
> > 
> > --
> > Exception ignored in:  > 0x1e17e13fd310>
> > Traceback (most recent call last):
> >   File "/usr/local/lib/python3.8/site-
> > packages/borg/repository.py", line 180, in __del__
> > assert False, "cleanup happened in Repository.__del__"
> > AssertionError: cleanup happened in Repository.__del__
> > Local Exception
> > Traceback (most recent call last):
> >   File "/usr/local/lib/python3.8/site-packages/borg/archiver.py",
> > line 4565, in main
> > exit_code = archiver.run(args)
> >   File "/usr/local/lib/python3.8/site-packages/borg/archiver.py",
> > line 4497, in run
> > return set_ec(func(args))
> >   File "/usr/local/lib/python3.8/site-packages/borg/archiver.py",
> > line 161, in wrapper
> > with repository:
> >   File "/usr/local/lib/python3.8/site-
> > packages/borg/repository.py", line 190, in __enter__
> > self.open(self.path, bool(self.exclusive),
> > lock_wait=self.lock_wait, lock=self.do_lock)
> >   File "/usr/local/lib/python3.8/site-
> > packages/borg/repository.py", line 450, in open
> > segment = self.io.get_latest_segment()
> >   File "/usr/local/lib/python3.8/site-
> > packages/borg/repository.py", line 1253, in get_latest_segment
> > for segment, filename in self.segment_iterator(reverse=True):
> >   File "/usr/local/lib/python3.8/site-
> > packages/borg/repository.py", line 12

Re: bge(4) fix

[Klemens Nanni]

On Sun, Jul 26, 2020 at 06:07:07PM +0200, Mark Kettenis wrote:
> Booted up the old v210 to test something and noticed that it prints a
> couple of:
> 
>   bge0: nvram lock timed out
> 
> warnings when booting up.  These are the on-board network interfaces
> and we already established in the past that these come without
> EEPROM/NVRAM and instead rely on the firmware to provide the MAC
> address.
OK kn

bge(4) fix

[Mark Kettenis]

Booted up the old v210 to test something and noticed that it prints a
couple of:

  bge0: nvram lock timed out

warnings when booting up.  These are the on-board network interfaces
and we already established in the past that these come without
EEPROM/NVRAM and instead rely on the firmware to provide the MAC
address.

The diff below kills these messages.

ok?


Index: dev/pci/if_bge.c
===
RCS file: /cvs/src/sys/dev/pci/if_bge.c,v
retrieving revision 1.391
diff -u -p -r1.391 if_bge.c
--- dev/pci/if_bge.c10 Jul 2020 13:26:37 -  1.391
+++ dev/pci/if_bge.c26 Jul 2020 16:04:43 -
@@ -3235,7 +3235,8 @@ bge_reset(struct bge_softc *sc)
write_op = bge_writereg_ind;
 
if (BGE_ASICREV(sc->bge_chipid) != BGE_ASICREV_BCM5700 &&
-   BGE_ASICREV(sc->bge_chipid) != BGE_ASICREV_BCM5701) {
+   BGE_ASICREV(sc->bge_chipid) != BGE_ASICREV_BCM5701 &&
+   !(sc->bge_flags & BGE_NO_EEPROM)) {
CSR_WRITE_4(sc, BGE_NVRAM_SWARB, BGE_NVRAMSWARB_SET1);
for (i = 0; i < 8000; i++) {
if (CSR_READ_4(sc, BGE_NVRAM_SWARB) &

Re: Python 3.8 os.listdir EINVAL on large directories

[Stuart Henderson]

Moving to tech.

In gmane.os.openbsd.misc, you wrote:
> Hi all,
>
> I am getting a stacktrace from the borg command in the borgbackup
> package while checking a backup (see bottom of email for full
> output, since it's verbose). The relevant part is this:
>
> filenames = os.listdir(os.path.join(data_path, dir))
>   OSError: [Errno 22] Invalid argument:
> '/mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12'
>
> This is same error is reproducible with a test Python 3.8 program:
>
>  #!/usr/bin/env python
>
>  import os
>  os.listdir('/mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12/')
>
> Running ktrace & kdump reveals the error is from calling
> getdents(2):
>
>  76903 python3.8
> CALL  open(0x1ec7f06de3b0,0x3)
>  76903 python3.8
> NAMI  "/mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12/"
>  76903 python3.8 RET   open 3
>  [...]
>  76903 python3.8 CALL  getdents(3,0x1ec7c9257000,0x4000)
>  76903 python3.8 RET   getdents 16384/0x4000
>  [...]
>  76903 python3.8 CALL  getdents(3,0x1ec7c9257000,0x4000)
>  76903 python3.8 RET   getdents 16384/0x4000
>  [...]
>  76903 python3.8 CALL  getdents(3,0x1ec7c9257000,0x4000)
>  76903 python3.8 RET   getdents 16384/0x4000
>  [...]
>  76903 python3.8 CALL  getdents(3,0x1ec7c9257000,0x4000)
>  76903 python3.8 RET   getdents -1 errno 22 Invalid argument
>
> Looking at the man page for getdents(2), I found it interesting
> that it says this call "is not a portable interface and should not
> be used directly by applications" and it recommends using
> readdir(3) instead.

ktrace only shows system calls not library functions. I don't
see python calling getdents directly, there is a fair chance that
python is calling readdir, and readdir is calling getdents.

> To give you a rough idea of the number of files and filename sizes
> in this directory:
>
>   $ ls /mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12/ | wc
>   15341534   10738

I haven't been able to recreate this including over nfs yet.
What filesystem type and mount options?

getdents(2) :-

   format.  Up to nbytes of data will be transferred.  nbytes must be greater
   than or equal to the block size associated with the file (see stat(2)).
   Some filesystems may not support getdents() with buffers smaller than this
   size.
...
   getdents() will fail if:
...
   [EINVAL]   The file referenced by fd is not a directory, or nbytes
  is too small for returning a directory entry or block of
  entries, or the current position pointer is invalid.

> Where does the problem lie -- the upstream Python code, the
> OpenBSD-specific patches in its port definition, or somewhere
> else? And in case it matters, this is a -current amd64 system,
> with "sysupgrade -s" executed on 7/15.
>
> Thank you,
> Aaron Miller
>
> --
> Exception ignored in:  0x1e17e13fd310>
> Traceback (most recent call last):
>   File "/usr/local/lib/python3.8/site-
> packages/borg/repository.py", line 180, in __del__
> assert False, "cleanup happened in Repository.__del__"
> AssertionError: cleanup happened in Repository.__del__
> Local Exception
> Traceback (most recent call last):
>   File "/usr/local/lib/python3.8/site-packages/borg/archiver.py",
> line 4565, in main
> exit_code = archiver.run(args)
>   File "/usr/local/lib/python3.8/site-packages/borg/archiver.py",
> line 4497, in run
> return set_ec(func(args))
>   File "/usr/local/lib/python3.8/site-packages/borg/archiver.py",
> line 161, in wrapper
> with repository:
>   File "/usr/local/lib/python3.8/site-
> packages/borg/repository.py", line 190, in __enter__
> self.open(self.path, bool(self.exclusive),
> lock_wait=self.lock_wait, lock=self.do_lock)
>   File "/usr/local/lib/python3.8/site-
> packages/borg/repository.py", line 450, in open
> segment = self.io.get_latest_segment()
>   File "/usr/local/lib/python3.8/site-
> packages/borg/repository.py", line 1253, in get_latest_segment
> for segment, filename in self.segment_iterator(reverse=True):
>   File "/usr/local/lib/python3.8/site-
> packages/borg/repository.py", line 1241, in segment_iterator
> filenames = os.listdir(os.path.join(data_path, dir))
> OSError: [Errno 22] Invalid argument:
> '/mnt/thinkpad_void_obsd_borg/thinkpad.borg/data/12'
>
> Platform: OpenBSD millipede.iforgotmy.name 6.7 GENERIC.MP#348
> amd64
> Borg: 1.1.13  Python: CPython 3.8.3 msgpack: 0.5.6
> PID: 31745  CWD: /mnt/thinkpad_void_obsd_borg
> sys.argv: ['/usr/local/bin/borg', 'check', 'thinkpad.borg']
> SSH_ORIGINAL_COMMAND: None
>
>

Re: xhci(4) isoc: fix bogus handling of chained TRBs

[Marcus Glocker]

On Sun, 26 Jul 2020 13:27:34 +
sc.dy...@gmail.com wrote:

> On 2020/07/26 10:54, Marcus Glocker wrote:
> > On Sat, 25 Jul 2020 20:31:44 +
> > sc.dy...@gmail.com wrote:
> >   
> >> On 2020/07/25 18:10, Marcus Glocker wrote:  
> >>> On Sun, Jul 19, 2020 at 02:12:21PM +, sc.dy...@gmail.com
> >>> wrote: 
>  On 2020/07/19 11:25, Marcus Glocker wrote:
> > On Sun, 19 Jul 2020 02:25:30 +
> > sc.dy...@gmail.com wrote:
> >
> >> hi,
> >>
> >> It works on AMD Bolton xHCI (78141022), Intel PCH (1e318086),
> >> and ASM1042 (10421b21).
> >> I simply play with ffplay -f v4l2 /dev/video0 to test.
> >
> > If your cam supports MJPEG it's good to add '-input_format
> > mjpeg' with higher resolutions like 1280x720, because that will
> > generated varying image sizes, which hit the 64k memory boundary
> > more often, and thus generate potentially more chained TDs.
> 
>  Thank you for useful information.
>  My webcam supprots at most 640x480, but 1024 bytes/frame x (2+1)
>  maxburst x 40 frames = 122880 bytes/xfer is enough to observe TD
>  fragmentation.
> 
> 
> >> At this moment it does not work on VL805, but I have no idea.
> >> I'll investigate furthermore...
> >>>
> >>> Did you already had a chance to figure out something regarding the
> >>> issue you faced on your VL805 controller?
> >>>
> >>> I'm running the diff here since then on the Intel xHCI controller
> >>> and couldn't re-produce any errors using different uvideo(4) and
> >>> uaudio(4) devices.
> >>> 
> >>
> >> No, yet -- all I know about this problem is VL805 genegates
> >> many MISSED_SRV Transfer Event for Isoch-IN pipe.
> >>
> >> xhci0: slot 3 missed srv with 123 TRB
> >>  :
> >>
> >> Even if I increase UVIDEO_IXFERS in uvideo.h to 6, HC still detects
> >> MISSED_SRV. When I disable splitting TD, it works well.
> >> I added printf paddr in the event TRB but each paddr of MISSED_SRV
> >> is 0, that does not meet 4.10.3.2.
> >> Parameters in this endpoint context are
> >>
> >> xhci0: slot 3 dci 3 ival 0 mps 1024 maxb 2 mep 3072 atl 3072 mult 0
> >>
> >> looks sane.  
> > 
> > Hmm, I see.
> > 
> > I currently have also no idea what exactly is causing the missed
> > service events.  I was reading a little bit yesterday about the
> > VL805 and could find some statements where people say it's not
> > fully compliant with the xHCI specs, and in Linux it took some
> > cooperation with the vendor to make it work.
> > 
> > One thing I still wanted to ask you to understand whether the
> > problem on your VL805 is only related with my last diff;  Are the
> > multi-trb transfers working fine with your last diff on the VL805?  
> 
> On VL805 ffplay plays the movie sometimes smoothly, sometimes laggy.
> The multi-TRB transfer itself works on VL805 with your patch.
> Not all splitted TD fail to transfer. Successful splitted transfer
> works as intended.
> I think MISSED_SRV is caused by other reason, maybe isochronous
> scheduling problem.
> Thus, IMO your patch can be committed.
> 
> VL805 also has same problem that AMD Bolton has.
> It may generate the 1. TRB event w/ cc = SHORT_PKT and
> remain = requested length (that is, transferred length = 0),
> but the 2. TRB w/ cc = SUCCESS and remain = 0.
> remain of 2. TRB should be given length, and CC should be SHORT_PKT.
> Your patch fixes this problem.

OK, that's what I wanted to understand.
I also have the impression that the MISSED_SRV issue on the VL805 is
related to another problem which we should trace separately from the
multi-trb problem.  Thanks for that useful feedback.

Attached the latest version of my patch including all the inputs
received (mostly related to malloc/free).

Patrick, Martin, would you be fine to OK that?


Index: xhci.c
===
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.116
diff -u -p -u -p -r1.116 xhci.c
--- xhci.c  30 Jun 2020 10:21:59 -  1.116
+++ xhci.c  19 Jul 2020 06:51:58 -
@@ -82,7 +82,7 @@ void  xhci_event_xfer(struct xhci_softc *
 intxhci_event_xfer_generic(struct xhci_softc *, struct usbd_xfer *,
struct xhci_pipe *, uint32_t, int, uint8_t, uint8_t, uint8_t);
 intxhci_event_xfer_isoc(struct usbd_xfer *, struct xhci_pipe *,
-   uint32_t, int);
+   uint32_t, int, uint8_t);
 void   xhci_event_command(struct xhci_softc *, uint64_t);
 void   xhci_event_port_change(struct xhci_softc *, uint64_t, uint32_t);
 intxhci_pipe_init(struct xhci_softc *, struct usbd_pipe *);
@@ -800,7 +800,7 @@ xhci_event_xfer(struct xhci_softc *sc, u
return;
break;
case UE_ISOCHRONOUS:
-   if (xhci_event_xfer_isoc(xfer, xp, remain, trb_idx))
+   if (xhci_event_xfer_isoc(xfer, xp, remain, trb_idx, code))
return;
break;
default:
@@ -92

Re: xhci(4) isoc: fix bogus handling of chained TRBs

[sc . dying]

On 2020/07/26 10:54, Marcus Glocker wrote:
> On Sat, 25 Jul 2020 20:31:44 +
> sc.dy...@gmail.com wrote:
> 
>> On 2020/07/25 18:10, Marcus Glocker wrote:
>>> On Sun, Jul 19, 2020 at 02:12:21PM +, sc.dy...@gmail.com wrote:
>>>   
 On 2020/07/19 11:25, Marcus Glocker wrote:  
> On Sun, 19 Jul 2020 02:25:30 +
> sc.dy...@gmail.com wrote:
>  
>> hi,
>>
>> It works on AMD Bolton xHCI (78141022), Intel PCH (1e318086),
>> and ASM1042 (10421b21).
>> I simply play with ffplay -f v4l2 /dev/video0 to test.  
>
> If your cam supports MJPEG it's good to add '-input_format mjpeg'
> with higher resolutions like 1280x720, because that will
> generated varying image sizes, which hit the 64k memory boundary
> more often, and thus generate potentially more chained TDs.  

 Thank you for useful information.
 My webcam supprots at most 640x480, but 1024 bytes/frame x (2+1)
 maxburst x 40 frames = 122880 bytes/xfer is enough to observe TD
 fragmentation.

  
>> At this moment it does not work on VL805, but I have no idea.
>> I'll investigate furthermore...  
>>>
>>> Did you already had a chance to figure out something regarding the
>>> issue you faced on your VL805 controller?
>>>
>>> I'm running the diff here since then on the Intel xHCI controller
>>> and couldn't re-produce any errors using different uvideo(4) and
>>> uaudio(4) devices.
>>>   
>>
>> No, yet -- all I know about this problem is VL805 genegates
>> many MISSED_SRV Transfer Event for Isoch-IN pipe.
>>
>> xhci0: slot 3 missed srv with 123 TRB
>>  :
>>
>> Even if I increase UVIDEO_IXFERS in uvideo.h to 6, HC still detects
>> MISSED_SRV. When I disable splitting TD, it works well.
>> I added printf paddr in the event TRB but each paddr of MISSED_SRV is
>> 0, that does not meet 4.10.3.2.
>> Parameters in this endpoint context are
>>
>> xhci0: slot 3 dci 3 ival 0 mps 1024 maxb 2 mep 3072 atl 3072 mult 0
>>
>> looks sane.
> 
> Hmm, I see.
> 
> I currently have also no idea what exactly is causing the missed service
> events.  I was reading a little bit yesterday about the VL805 and could
> find some statements where people say it's not fully compliant with the
> xHCI specs, and in Linux it took some cooperation with the vendor to
> make it work.
> 
> One thing I still wanted to ask you to understand whether the problem
> on your VL805 is only related with my last diff;  Are the multi-trb
> transfers working fine with your last diff on the VL805?

On VL805 ffplay plays the movie sometimes smoothly, sometimes laggy.
The multi-TRB transfer itself works on VL805 with your patch.
Not all splitted TD fail to transfer. Successful splitted transfer
works as intended.
I think MISSED_SRV is caused by other reason, maybe isochronous
scheduling problem.
Thus, IMO your patch can be committed.

VL805 also has same problem that AMD Bolton has.
It may generate the 1. TRB event w/ cc = SHORT_PKT and
remain = requested length (that is, transferred length = 0),
but the 2. TRB w/ cc = SUCCESS and remain = 0.
remain of 2. TRB should be given length, and CC should be SHORT_PKT.
Your patch fixes this problem.

[Patch] httpd - delete unused function canonicalize_host()

[Ross L Richardson]

Function is unused and can go.

Ross
--

Index: httpd.c
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
retrieving revision 1.68
diff -u -p -r1.68 httpd.c
--- httpd.c 9 Sep 2018 21:06:51 -   1.68
+++ httpd.c 26 Jul 2020 13:07:26 -
@@ -552,59 +552,6 @@ expand_string(char *label, size_t len, c
 }
 
 const char *
-canonicalize_host(const char *host, char *name, size_t len)
-{
-   struct sockaddr_in   sin4;
-   struct sockaddr_in6  sin6;
-   size_t   i, j;
-   size_t   plen;
-   char c;
-
-   if (len < 2)
-   goto fail;
-
-   /*
-* Canonicalize an IPv4/6 address
-*/
-   if (inet_pton(AF_INET, host, &sin4) == 1)
-   return (inet_ntop(AF_INET, &sin4, name, len));
-   if (inet_pton(AF_INET6, host, &sin6) == 1)
-   return (inet_ntop(AF_INET6, &sin6, name, len));
-
-   /*
-* Canonicalize a hostname
-*/
-
-   /* 1. remove repeated dots and convert upper case to lower case */
-   plen = strlen(host);
-   memset(name, 0, len);
-   for (i = j = 0; i < plen; i++) {
-   if (j >= (len - 1))
-   goto fail;
-   c = tolower((unsigned char)host[i]);
-   if ((c == '.') && (j == 0 || name[j - 1] == '.'))
-   continue;
-   name[j++] = c;
-   }
-
-   /* 2. remove trailing dots */
-   for (i = j; i > 0; i--) {
-   if (name[i - 1] != '.')
-   break;
-   name[i - 1] = '\0';
-   j--;
-   }
-   if (j <= 0)
-   goto fail;
-
-   return (name);
-
- fail:
-   errno = EINVAL;
-   return (NULL);
-}
-
-const char *
 url_decode(char *url)
 {
char*p, *q;
Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.147
diff -u -p -r1.147 httpd.h
--- httpd.h 25 Jul 2020 21:12:49 -  1.147
+++ httpd.h 26 Jul 2020 13:07:26 -
@@ -710,7 +710,6 @@ void event_again(struct event *, int, 
 int expand_string(char *, size_t, const char *, const char *);
 const char *url_decode(char *);
 char   *url_encode(const char *);
-const char *canonicalize_host(const char *, char *, size_t);
 const char *canonicalize_path(const char *, char *, size_t);
 size_t  path_info(char *);
 char   *escape_html(const char *);

Re: xhci(4) isoc: fix bogus handling of chained TRBs

[Marcus Glocker]

On Sat, 25 Jul 2020 20:31:44 +
sc.dy...@gmail.com wrote:

> On 2020/07/25 18:10, Marcus Glocker wrote:
> > On Sun, Jul 19, 2020 at 02:12:21PM +, sc.dy...@gmail.com wrote:
> >   
> >> On 2020/07/19 11:25, Marcus Glocker wrote:  
> >>> On Sun, 19 Jul 2020 02:25:30 +
> >>> sc.dy...@gmail.com wrote:
> >>>  
>  hi,
> 
>  It works on AMD Bolton xHCI (78141022), Intel PCH (1e318086),
>  and ASM1042 (10421b21).
>  I simply play with ffplay -f v4l2 /dev/video0 to test.  
> >>>
> >>> If your cam supports MJPEG it's good to add '-input_format mjpeg'
> >>> with higher resolutions like 1280x720, because that will
> >>> generated varying image sizes, which hit the 64k memory boundary
> >>> more often, and thus generate potentially more chained TDs.  
> >>
> >> Thank you for useful information.
> >> My webcam supprots at most 640x480, but 1024 bytes/frame x (2+1)
> >> maxburst x 40 frames = 122880 bytes/xfer is enough to observe TD
> >> fragmentation.
> >>
> >>  
>  At this moment it does not work on VL805, but I have no idea.
>  I'll investigate furthermore...  
> > 
> > Did you already had a chance to figure out something regarding the
> > issue you faced on your VL805 controller?
> > 
> > I'm running the diff here since then on the Intel xHCI controller
> > and couldn't re-produce any errors using different uvideo(4) and
> > uaudio(4) devices.
> >   
> 
> No, yet -- all I know about this problem is VL805 genegates
> many MISSED_SRV Transfer Event for Isoch-IN pipe.
> 
> xhci0: slot 3 missed srv with 123 TRB
>  :
> 
> Even if I increase UVIDEO_IXFERS in uvideo.h to 6, HC still detects
> MISSED_SRV. When I disable splitting TD, it works well.
> I added printf paddr in the event TRB but each paddr of MISSED_SRV is
> 0, that does not meet 4.10.3.2.
> Parameters in this endpoint context are
> 
> xhci0: slot 3 dci 3 ival 0 mps 1024 maxb 2 mep 3072 atl 3072 mult 0
> 
> looks sane.

Hmm, I see.

I currently have also no idea what exactly is causing the missed service
events.  I was reading a little bit yesterday about the VL805 and could
find some statements where people say it's not fully compliant with the
xHCI specs, and in Linux it took some cooperation with the vendor to
make it work.

One thing I still wanted to ask you to understand whether the problem
on your VL805 is only related with my last diff;  Are the multi-trb
transfers working fine with your last diff on the VL805?

relayd: set group and divert-reply

[YASUOKA Masahiko]

Hi,

I'd like to run relayd as _relayd group always so that we can use
"group _relayd" in a pf rule.  This makes it possible to write a pf
rule easily which is to match only connections from relayd(8).

Also as for relayd.conf(5), I'd like to mention that "divert-reply" is
required for "transparent forward" and add an example pf rule which
uses "group _relayd".

ok?

Run relayd(8) as _relayd group user.

Index: usr.sbin/relayd/relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.182
diff -u -p -r1.182 relayd.c
--- usr.sbin/relayd/relayd.c15 Sep 2019 19:23:29 -  1.182
+++ usr.sbin/relayd/relayd.c26 Jul 2020 08:39:27 -
@@ -201,6 +201,11 @@ main(int argc, char *argv[])
if ((ps->ps_pw =  getpwnam(RELAYD_USER)) == NULL)
errx(1, "unknown user %s", RELAYD_USER);
 
+   if (setgroups(1, &ps->ps_pw->pw_gid) == -1 ||
+   setresgid(ps->ps_pw->pw_gid, ps->ps_pw->pw_gid, ps->ps_pw->pw_gid)
+   == -1)
+   err(1, "unable to set group ids");
+
log_init(debug, LOG_DAEMON);
log_setverbose(verbose);
 

Add a mention that "divert-reply" rule is required for "transparent
forward" and add an example which uses "group _relayd" to match the
outgoing connections.

Index: usr.sbin/relayd/relayd.conf.5
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
retrieving revision 1.198
diff -u -p -r1.198 relayd.conf.5
--- usr.sbin/relayd/relayd.conf.5   1 Jul 2020 06:47:18 -   1.198
+++ usr.sbin/relayd/relayd.conf.5   26 Jul 2020 08:39:27 -
@@ -622,6 +622,10 @@ Use the
 .Ic transparent
 keyword to enable fully-transparent mode; the source address of the
 client will be retained in this case.
+For this case,
+additional
+.Xr pf 4
+rule with divert-reply option is required for the outgoing connection.
 .Pp
 The
 .Ic with tls
@@ -1627,6 +1631,31 @@ relay tlsinspect {
protocol httpfilter
forward with tls to destination
 }
+.Ed
+.Pp
+If you want to use fully-transparent mode,
+you can add the
+.Ic transparent
+keyword to
+.Ic forward
+option:
+.Bd -literal -offset indent
+relay tlsinspect {
+   listen on 127.0.0.1 port 8443 tls
+   protocol httpfilter
+   transparent forward with tls to destination
+}
+.Ed
+.Pp
+And add a matching divert-reply rule in
+.Xr pf.conf 5 .
+You can use
+.Dq group _relayd
+to match only connections from
+.Xr relayd 8
+precisely:
+.Bd -literal -offset indent
+pass out proto tcp to port 443 group _relayd divert-reply
 .Ed
 .Pp
 The next simple router configuration example can be used to run

Re: pf_remove_divert_state

[YASUOKA Masahiko]

Thanks,

On Sat, 25 Jul 2020 15:00:07 +0200
Alexander Bluhm  wrote:
> On Sat, Jul 25, 2020 at 09:37:37PM +0900, YASUOKA Masahiko wrote:
>> Is this part a reason why we have "divert-reply"?
> 
> Yes.
> 
> Divert rules pass packets to the local network stack.  With divert-to
> you specify the socket address.  This works for incomming connections.
> The divert-to address can be 127.0.0.1 or anything else with
> SO_BINDANY.
> 
> When you use SO_BINDANY for outgoing connections and you don't know
> the addresses when writing pf.conf, use divert-reply.
> 
> As dangling states interfere with new connections, I added the
> divert state cleanup.  This is especially necessary for DGRAM or
> RAW sockets.

Yes.  My first message shows it is neccessary for TCP. 

Also my diff was totally wrong it deletes the states regardless of
it's for divert or not.

>> > Is that not possible for you?
>> 
>> It's possible.
> 
> Fine, then use divert-reply instead of changing the semantics.

I have thought it's hard to create a divert-reply rule for relayd's
"transparent forward to destination" case.  But I noticed tftp-proxy
is using "group _tftp_proxy" to match connections only from the
program precisely.

I'll send diffs to do the same thing for relayd in a separated mail.