Re: include cpuid 0 string in dmesg for fw_update
On Wed, Jul 27, 2022 at 06:47:56AM -0700, Andrew Hewus Fresh wrote:
> On Wed, Jul 27, 2022 at 11:06:39AM +0200, Alexander Hall wrote:
> > I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking
> > for.
>
> As I was falling asleep last night I was trying to figure out how to get
> '*([!$_nl])' into the match and couldn't think of a good one. Your
> solution is much better.
Except for that issue with ksh patterns inside a variable.
I had some time to think about this while doing some construction.
Finally had time to try to implement it and it seems like although it's
a bit ugly, it is performant and seems to allow me to use "^cpu0:*Intel"
in the patterns file and (I believe) just match a line that starts with
cpu0: and contains Intel.
My alpha agrees with the speed assessment, the current implementation
that doesn't support the "*" in the patterns runs at about 5.5 seconds
(as before), while this new version takes 6.5 seconds. That's a lot
better than the 17.5 seconds it took to match against each line (and
still didn't actually expand the *).
Index: fw_update.sh
===
RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
retrieving revision 1.42
diff -u -p -r1.42 fw_update.sh
--- fw_update.sh20 Feb 2022 21:53:04 - 1.42
+++ fw_update.sh3 Aug 2022 01:11:53 -
@@ -168,21 +168,32 @@ verify() {
}
firmware_in_dmesg() {
- local _d _m _line _dmesgtail _last='' _nl=$( echo )
+ local IFS
+ local _d _m _dmesgtail _last='' _nl='
+'
# The dmesg can contain multiple boots, only look in the last one
_dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}'
/var/run/dmesg.boot )"
grep -v '^[[:space:]]*#' "$FWPATTERNS" |
while read -r _d _m; do
- [ "$_d" = "$_last" ] && continue
+ [ "$_d" = "$_last" ] && continue
[ "$_m" ] || _m="${_nl}${_d}[0-9] at "
[ "$_m" = "${_m#^}" ] || _m="${_nl}${_m#^}"
- if [[ $_dmesgtail = *$_m* ]]; then
- echo "$_d"
- _last="$_d"
- fi
+ IFS='*'
+ set -- $_m
+ unset IFS
+
+ case $# in
+ 1) [[ $_dmesgtail = *$** ]] ||
continue;;
+ 2) [[ $_dmesgtail = *$1*([!$_nl])$2* ]] ||
continue;;
+ 3) [[ $_dmesgtail = *$1*([!$_nl])$2*([!$_nl])$3* ]] ||
continue;;
+ *) echo "bad pattern $_m"; exit 1 ;;
+ esac
+
+ echo "$_d"
+ _last="$_d"
done
}
Re: [v5] amd64: simplify TSC sync testing
On 2.8.2022. 23:40, Stuart Henderson wrote: > On 2022/08/02 22:28, Hrvoje Popovski wrote: >> >> this is report from Dell R7515 with AMD EPYC 7702P 64-Core Processor >> >> >> r7515$ sysctl | grep tsc >> kern.timecounter.choice=i8254(0) mcx1(-100) mcx0(-100) tsc(-1000) >> acpihpet0(1000) acpitimer0(1000) >> machdep.tscfreq=1996246800 >> machdep.invarianttsc=1 > > Just to be sure, are you able to check kern.timecounter.hardware > on this one please? With the fix in v5 this should now use > acpihpet or acpitimer (which are reasonable choices given the > sync issues with tsc on this hw). > > (The kther machines you sent details for have enough to tell that > they're working as expected). > > Thanks for the tests on these various interesting CPUs. > yes, yes i've started with "|grep tsc" and with laptop i thought i am missing some information here then everything from start :)
Re: [v5] amd64: simplify TSC sync testing
On 2022/08/02 22:28, Hrvoje Popovski wrote: > > this is report from Dell R7515 with AMD EPYC 7702P 64-Core Processor > > > r7515$ sysctl | grep tsc > kern.timecounter.choice=i8254(0) mcx1(-100) mcx0(-100) tsc(-1000) > acpihpet0(1000) acpitimer0(1000) > machdep.tscfreq=1996246800 > machdep.invarianttsc=1 Just to be sure, are you able to check kern.timecounter.hardware on this one please? With the fix in v5 this should now use acpihpet or acpitimer (which are reasonable choices given the sync issues with tsc on this hw). (The kther machines you sent details for have enough to tell that they're working as expected). Thanks for the tests on these various interesting CPUs.
Re: [v5] amd64: simplify TSC sync testing
On 2.8.2022. 22:28, Hrvoje Popovski wrote: > Hi, > > this is report from Dell R7515 with AMD EPYC 7702P 64-Core Processor > > > r7515$ sysctl | grep tsc > kern.timecounter.choice=i8254(0) mcx1(-100) mcx0(-100) tsc(-1000) > acpihpet0(1000) acpitimer0(1000) > machdep.tscfreq=1996246800 > machdep.invarianttsc=1 snapshot OpenBSD 7.2-beta (GENERIC.MP) #662: Tue Aug 2 11:58:47 MDT 2022 r7515# sysctl kern.timecounter kern.timecounter.tick=1 kern.timecounter.timestepwarnings=0 kern.timecounter.hardware=i8254 kern.timecounter.choice=i8254(0) mcx1(-100) mcx0(-100) tsc(-1000) acpihpet0(1000) acpitimer0(1000) v5 diff r7515# sysctl kern.timecounter kern.timecounter.tick=1 kern.timecounter.timestepwarnings=0 kern.timecounter.hardware=acpihpet0 kern.timecounter.choice=i8254(0) mcx1(-100) mcx0(-100) tsc(-1000) acpihpet0(1000) acpitimer0(1000)
Re: [v5] amd64: simplify TSC sync testing
On 31.7.2022. 5:13, Scott Cheloha wrote: > Hi, > > At the urging of sthen@ and dv@, here is v5. > > Two major changes from v4: > > - Add the function tc_reset_quality() to kern_tc.c and use it > to lower the quality of the TSC timecounter if we fail the > sync test. > > tc_reset_quality() will choose a new active timecounter if, > after the quality change, the given timecounter is no longer > the best timecounter. > > The upshot is: if you fail the TSC sync test you should boot > with the HPET as your active timecounter. If you don't have > an HPET you'll be using something else. > > - Drop the SMT accomodation from the hot loop. It hasn't been > necessary since last year when I rewrote the test to run without > a mutex. In the rewritten test, the two CPUs in the hot loop > are not competing for any resources so they should not be able > to starve one another. > > dv: Could you double-check that this still chooses the right > timecounter on your machine? If so, I will ask deraadt@ to > put this into snaps to replace v4. > > Additional test reports are welcome. Include your dmesg. Hi, this is report from Lenovo ThinPad E14 gen2 with 6 x AMD Ryzen 5 4500U e14gen2# sysctl | grep kern.timecounter kern.timecounter.tick=1 kern.timecounter.timestepwarnings=0 kern.timecounter.hardware=acpihpet0 kern.timecounter.choice=i8254(0) tsc(-1000) acpihpet0(1000) acpitimer0(1000) OpenBSD 7.2-beta (GENERIC.MP) #20: Tue Aug 2 23:13:28 CEST 2022 hrv...@e14gen2.srce.hr:/sys/arch/amd64/compile/GENERIC.MP real mem = 7713394688 (7356MB) avail mem = 7462240256 (7116MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xbf912000 (63 entries) bios0: vendor LENOVO version "R1AET42W (1.18 )" date 03/03/2022 bios0: LENOVO 20T6000TSC acpi0 at bios0: ACPI 6.3 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SSDT SSDT IVRS SSDT SSDT TPM2 SSDT MSDM BATB HPET APIC MCFG SBST WSMT VFCT SSDT CRAT CDIT FPDT SSDT SSDT SSDT BGRT UEFI SSDT SSDT acpi0: wakeup devices GPP3(S3) GPP4(S4) GPP5(S3) XHC0(S3) XHC1(S3) GP19(S3) LID_(S4) SLPB(S3) acpitimer0 at acpi0: 3579545 Hz, 32 bits acpihpet0 at acpi0: 14318180 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: AMD Ryzen 5 4500U with Radeon Graphics, 2370.90 MHz, 17-60-01 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu0: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 8-way L2 cache, 4MB 64b/line 16-way L3 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, C-substates=1.1, IBE cpu1 at mainbus0: apid 1 (application processor) cpu1: AMD Ryzen 5 4500U with Radeon Graphics, 2370.55 MHz, 17-60-01 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu1: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 8-way L2 cache, 4MB 64b/line 16-way L3 cache cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 2 (application processor) cpu2: AMD Ryzen 5 4500U with Radeon Graphics, 2370.55 MHz, 17-60-01 cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu2: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 8-way L2 cache, 4MB 64b/line 16-way L3 cache cpu2: smt 0, core 2, package 0 cpu3 at mainbus0: apid 4 (application processor) tsc: cpu0/cpu3: sync test round 1/2 failed tsc: cpu0/cpu3: cpu3: 51485 lags 5130 cycles cpu3: AMD Ryzen 5 4500U with Radeon Graphics, 2370.55 MHz, 17-60-01 cpu3:
Re: patch(1): fix locate_hunk for empty files
On Mon, Aug 01, 2022 at 03:38:59PM +0200, Omar Polo wrote:
> there's an edge case in locate_hunk with empty files; if `first_guess'
> is zero then main() assumes that locate_hunk has failed and aborts the
> patch operation. Instead, make sure to return 1 (the line number) so
> that the patch operation can continue.
>
> this issue was found by Neels Hofmeyr (I presume) in the diff
> implementation for got. The regress suite for diff.git expects that
> `patch && patch -R' is an identity and so currently fails with base
> patch(1).
>
> (while here i've also added a comment about t19 that was missing.)
>
> ok?
ok stsp@
Unrelated to the problem you are fixing, but worth mentioning anyway:
Both 'got patch' and Larry's patch(1) fail to create t20.in during
the application of your diff. This can be trivially worked around
by running "touch t20.in".
I am unsure how a new empty file should be represented in unidiff.
I would expect to see at least a hunk header of the form @@ -0,0 +0,0 @@
but none of 'cvs diff', 'svn diff', and 'git diff' produce any hunk
headers for added empty files. They just produce their own custom
meta-data lines. ('hg diff' is even worse and doesn't print anything).
Which probably means patch(1) would need to be tought about lines such
as "Index" or similar, which are usually treated as noise? Horrible.
Testing the above hunk header idea, this unidiff successfully tricks
Larry patch(1) into creating an empty file:
--- /dev/null
+++ regress/usr.bin/patch/t20.in
@@ -0,0 +0,0 @@
So I suppose this issue should be fixed in diff tooling rather than
trying to work around it in patch(1).
Re: patch(1) fix dwim for reversed patch
On Mon, Aug 01, 2022 at 12:48:16PM +0200, Omar Polo wrote:
> i was looking for a different issue in patch when i found this.
> patch(1) fails to recognize the reversal application of a patch that
> creates a file (this is test t3).
>
> since an empty context always matches, the idea is to run the dwim code
> also when locate_hunk succeeds, but only if the patch would create a
> file (pch_ptrn_lines() == 0) and the match is on the first line.
>
> regress now passes completely.
>
> ok?
ok stsp@
> diff /usr/src
> commit - faf550173e173cd2ef8642601dc48202a09fd44f
> path + /usr/src
> blob - 73781e33724010ac3ce470e6c079eaa17c3d6365
> file + regress/usr.bin/patch/Makefile
> --- regress/usr.bin/patch/Makefile
> +++ regress/usr.bin/patch/Makefile
> @@ -7,10 +7,6 @@ CLEANFILES= *.copy *.orig *.rej t5 d19/*
> REGRESS_TARGETS= t1 t2 t3 t4 t5 t6 t7 t8 t9 \
> t10 t11 t12 t13 t14 t15 t16 t17 t18 t19
>
> -t3:
> - @echo ${*} currently fails
> - @echo DISABLED
> -
> # .in: input file
> # .diff: patch
> # .out: desired result after patching
> @@ -18,7 +14,7 @@ t3:
> # t1: diff contains invalid line number 0.
> # t2: diff contains invalid line numbers beyond end of input file.
> # t3: a case where patch should detect a previously applied patch.
> -# Diff transform an empty file into a single line one. Currently fails.
> +# Diff transform an empty file into a single line one.
> # t4: a case where patch has to detect a previously applied patch.
> # Diff transform a file with a single line with an eol into a single
> # line without eol.
> @@ -47,6 +43,7 @@ t3:
> @(! ${PATCH} ${PATCHFLAGS} ${*}.copy ${.CURDIR}/${*}.diff)
> @cmp -s ${*}.copy ${.CURDIR}/${*}.out || \
> (echo "XXX ${*} failed" && false)
> +
> t4:
> @echo ${*}
> @cp ${.CURDIR}/${*}.in ${*}.copy
> blob - 1d9070b01842d982b3b3ce6d5f810913a739813e
> file + usr.bin/patch/patch.c
> --- usr.bin/patch/patch.c
> +++ usr.bin/patch/patch.c
> @@ -260,7 +260,8 @@ main(int argc, char *argv[])
> if (!skip_rest_of_patch) {
> do {
> where = locate_hunk(fuzz);
> - if (hunk == 1 && where == 0 && !force) {
> + if ((hunk == 1 && where == 0 && !force)
> ||
> + (where == 1 && pch_ptrn_lines() ==
> 0 && !force)) {
> /* dwim for reversed patch? */
> if (!pch_swap()) {
> if (fuzz == 0)
> @@ -276,6 +277,10 @@ main(int argc, char *argv[])
> /* put it back
> to normal */
> fatal("lost
> hunk on alloc error!\n");
> reverse = !reverse;
> +
> + /* restore position if
> this patch creates a file */
> + if (pch_ptrn_lines() ==
> 0)
> + where = 1;
> } else if (noreverse) {
> if (!pch_swap())
> /* put it back
> to normal */
>
>
Re: [v5] amd64: simplify TSC sync testing
On 31.7.2022. 5:13, Scott Cheloha wrote: > Hi, > > At the urging of sthen@ and dv@, here is v5. > > Two major changes from v4: > > - Add the function tc_reset_quality() to kern_tc.c and use it > to lower the quality of the TSC timecounter if we fail the > sync test. > > tc_reset_quality() will choose a new active timecounter if, > after the quality change, the given timecounter is no longer > the best timecounter. > > The upshot is: if you fail the TSC sync test you should boot > with the HPET as your active timecounter. If you don't have > an HPET you'll be using something else. > > - Drop the SMT accomodation from the hot loop. It hasn't been > necessary since last year when I rewrote the test to run without > a mutex. In the rewritten test, the two CPUs in the hot loop > are not competing for any resources so they should not be able > to starve one another. > > dv: Could you double-check that this still chooses the right > timecounter on your machine? If so, I will ask deraadt@ to > put this into snaps to replace v4. > > Additional test reports are welcome. Include your dmesg. Hi, this is report from Supermicro AS-1114S-WTRT with AMD EPYC 7413 24-Core Processor smc24# sysctl | grep tsc kern.timecounter.hardware=tsc kern.timecounter.choice=i8254(0) mcx1(-100) mcx0(-100) tsc(2000) acpihpet0(1000) acpitimer0(1000) machdep.tscfreq=2650005829 machdep.invarianttsc=1 smc24# smc24# dmesg OpenBSD 7.2-beta (GENERIC.MP) #12: Tue Aug 2 22:41:12 CEST 2022 hrv...@smc24.srce.hr:/sys/arch/amd64/compile/GENERIC.MP real mem = 68497051648 (65323MB) avail mem = 66403737600 (63327MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 3.3 @ 0xa9d1c000 (71 entries) bios0: vendor American Megatrends Inc. version "2.4" date 04/13/2022 bios0: Supermicro AS -1114S-WTRT acpi0 at bios0: ACPI 6.0 acpi0: sleep states S0 S5 acpi0: tables DSDT FACP SSDT SPMI SSDT FIDT MCFG SSDT SSDT BERT HPET IVRS PCCT SSDT CRAT CDIT SSDT WSMT APIC ERST HEST acpi0: wakeup devices B000(S3) C000(S3) B010(S3) C010(S3) B030(S3) C030(S3) B020(S3) C020(S3) B100(S3) C100(S3) B110(S3) C110(S3) B130(S3) C130(S3) B120(S3) C120(S3) acpitimer0 at acpi0: 3579545 Hz, 32 bits acpimcfg0 at acpi0 acpimcfg0: addr 0xe000, bus 0-255 acpihpet0 at acpi0: 14318180 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: AMD EPYC 7413 24-Core Processor, 2650.38 MHz, 19-01-01 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,INVPCID,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,PKU,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu0: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 8-way L2 cache, 32MB 64b/line 16-way L3 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 100MHz cpu0: mwait min=64, max=64, C-substates=1.1, IBE cpu1 at mainbus0: apid 1 (application processor) cpu1: AMD EPYC 7413 24-Core Processor, 2650.01 MHz, 19-01-01 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,INVPCID,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,PKU,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu1: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 8-way L2 cache, 32MB 64b/line 16-way L3 cache cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 2 (application processor) cpu2: AMD EPYC 7413 24-Core Processor, 2650.00 MHz, 19-01-01 cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,INVPCID,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,PKU,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES cpu2: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 512KB 64b/line 8-way L2 cache, 32MB 64b/line 16-way L3 cache cpu2: smt 0, core 2, package 0 cpu3 at mainbus0: apid 3 (application processor) cpu3: AMD EPYC 7413 24-Core Processor, 2650.00 MHz, 19-01-01 cpu3:
Re: [v5] amd64: simplify TSC sync testing
On 31.7.2022. 5:13, Scott Cheloha wrote: > Hi, > > At the urging of sthen@ and dv@, here is v5. > > Two major changes from v4: > > - Add the function tc_reset_quality() to kern_tc.c and use it > to lower the quality of the TSC timecounter if we fail the > sync test. > > tc_reset_quality() will choose a new active timecounter if, > after the quality change, the given timecounter is no longer > the best timecounter. > > The upshot is: if you fail the TSC sync test you should boot > with the HPET as your active timecounter. If you don't have > an HPET you'll be using something else. > > - Drop the SMT accomodation from the hot loop. It hasn't been > necessary since last year when I rewrote the test to run without > a mutex. In the rewritten test, the two CPUs in the hot loop > are not competing for any resources so they should not be able > to starve one another. > > dv: Could you double-check that this still chooses the right > timecounter on your machine? If so, I will ask deraadt@ to > put this into snaps to replace v4. > > Additional test reports are welcome. Include your dmesg. Hi, this is report from Dell R7515 with AMD EPYC 7702P 64-Core Processor r7515$ sysctl | grep tsc kern.timecounter.choice=i8254(0) mcx1(-100) mcx0(-100) tsc(-1000) acpihpet0(1000) acpitimer0(1000) machdep.tscfreq=1996246800 machdep.invarianttsc=1 r7515$ dmesg | grep tsc tsc: cpu0/cpu4: sync test round 1/2 failed tsc: cpu0/cpu4: cpu4: 3 lags 340 cycles tsc: cpu0/cpu10: sync test round 1/2 failed tsc: cpu0/cpu10: cpu10: 3 lags 260 cycles tsc: cpu0/cpu12: sync test round 1/2 failed tsc: cpu0/cpu12: cpu12: 6155 lags 280 cycles tsc: cpu0/cpu13: sync test round 1/2 failed tsc: cpu0/cpu13: cpu13: 15659 lags 220 cycles tsc: cpu0/cpu14: sync test round 1/2 failed tsc: cpu0/cpu14: cpu14: 42 lags 160 cycles tsc: cpu0/cpu15: sync test round 1/2 failed tsc: cpu0/cpu15: cpu15: 337 lags 160 cycles tsc: cpu0/cpu16: sync test round 1/2 failed tsc: cpu0/cpu16: cpu16: 973 lags 160 cycles tsc: cpu0/cpu17: sync test round 1/2 failed tsc: cpu0/cpu17: cpu17: 43 lags 180 cycles tsc: cpu0/cpu18: sync test round 1/2 failed tsc: cpu0/cpu18: cpu18: 30 lags 180 cycles tsc: cpu0/cpu19: sync test round 1/2 failed tsc: cpu0/cpu19: cpu19: 42 lags 160 cycles tsc: cpu0/cpu20: sync test round 1/2 failed tsc: cpu0/cpu20: cpu20: 1739 lags 180 cycles tsc: cpu0/cpu21: sync test round 1/2 failed tsc: cpu0/cpu21: cpu21: 6180 lags 260 cycles tsc: cpu0/cpu22: sync test round 1/2 failed tsc: cpu0/cpu22: cpu22: 696 lags 160 cycles tsc: cpu0/cpu23: sync test round 1/2 failed tsc: cpu0/cpu23: cpu23: 36 lags 160 cycles tsc: cpu0/cpu24: sync test round 1/2 failed tsc: cpu0/cpu24: cpu24: 64 lags 140 cycles tsc: cpu0/cpu25: sync test round 1/2 failed tsc: cpu0/cpu25: cpu25: 6086 lags 320 cycles tsc: cpu0/cpu26: sync test round 1/2 failed tsc: cpu0/cpu26: cpu26: 40 lags 160 cycles tsc: cpu0/cpu27: sync test round 1/2 failed tsc: cpu0/cpu27: cpu27: 4 lags 300 cycles tsc: cpu0/cpu28: sync test round 1/2 failed tsc: cpu0/cpu28: cpu28: 2 lags 160 cycles tsc: cpu0/cpu29: sync test round 1/2 failed tsc: cpu0/cpu29: cpu29: 2 lags 180 cycles tsc: cpu0/cpu30: sync test round 1/2 failed tsc: cpu0/cpu30: cpu30: 2 lags 140 cycles tsc: cpu0/cpu31: sync test round 1/2 failed tsc: cpu0/cpu31: cpu31: 32 lags 300 cycles tsc: cpu0/cpu32: sync test round 1/2 failed tsc: cpu0/cpu32: cpu32: 48 lags 200 cycles tsc: cpu0/cpu33: sync test round 1/2 failed tsc: cpu0/cpu33: cpu33: 2 lags 100 cycles tsc: cpu0/cpu34: sync test round 1/2 failed tsc: cpu0/cpu34: cpu34: 2 lags 120 cycles tsc: cpu0/cpu35: sync test round 1/2 failed tsc: cpu0/cpu35: cpu35: 2 lags 140 cycles tsc: cpu0/cpu36: sync test round 1/2 failed tsc: cpu0/cpu36: cpu36: 2 lags 120 cycles tsc: cpu0/cpu37: sync test round 1/2 failed tsc: cpu0/cpu37: cpu37: 13144 lags 200 cycles tsc: cpu0/cpu38: sync test round 1/2 failed tsc: cpu0/cpu38: cpu38: 28 lags 180 cycles tsc: cpu0/cpu39: sync test round 1/2 failed tsc: cpu0/cpu39: cpu39: 2 lags 120 cycles tsc: cpu0/cpu40: sync test round 1/2 failed tsc: cpu0/cpu40: cpu40: 13167 lags 240 cycles tsc: cpu0/cpu41: sync test round 1/2 failed tsc: cpu0/cpu41: cpu41: 2 lags 120 cycles tsc: cpu0/cpu42: sync test round 1/2 failed tsc: cpu0/cpu42: cpu42: 2 lags 120 cycles tsc: cpu0/cpu43: sync test round 1/2 failed tsc: cpu0/cpu43: cpu43: 12558 lags 180 cycles tsc: cpu0/cpu44: sync test round 1/2 failed tsc: cpu0/cpu44: cpu44: 13 lags 200 cycles tsc: cpu0/cpu45: sync test round 1/2 failed tsc: cpu0/cpu45: cpu45: 6 lags 180 cycles tsc: cpu0/cpu46: sync test round 1/2 failed tsc: cpu0/cpu46: cpu46: 189 lags 200 cycles tsc: cpu0/cpu47: sync test round 1/2 failed tsc: cpu0/cpu47: cpu47: 3 lags 140 cycles tsc: cpu0/cpu48: sync test round 1/2 failed tsc: cpu0/cpu48: cpu48: 2 lags 160 cycles tsc: cpu0/cpu49: sync test round 1/2 failed tsc: cpu0/cpu49: cpu49: 8 lags 180 cycles tsc: cpu0/cpu50: sync test round 1/2 failed tsc: cpu0/cpu50: cpu50: 2 lags 160 cycles tsc:
Re: dmesg(8): fail if given positional arguments
On Tue, 02 Aug 2022 11:52:33 -0500, Scott Cheloha wrote: > dmesg(8) doesn't use any positional arguments. It's a usage error if > any are present. Sure. Ok millert@ - todd
Re: rpki-client: add connect() MAX_CONTIMEOUT for rsync/rrdp
I think you intend for that to be two seperate diffs, not merged into one. For connect < 15 seconds, I think that is a bit strict. For IO stalling 15 seconds, I suspect such IO stalls happen more than we know, and will do harm to RPKI processing results. I don't see any way this can be tested in less than 24 hours. Job Snijders wrote: > On Tue, Aug 02, 2022 at 03:59:40PM +0200, Claudio Jeker wrote: > > What makes you think that 15sec is enough to open connections in all > > scenarios? I feel this is one of those changes that just shows that > > maybe the current connect timeout from the system is too conservative. > > Yeah, maybe. How about this instead? > > Seems to work well for me. > > Kind regards, > > Job > > Index: extern.h > === > RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v > retrieving revision 1.143 > diff -u -p -r1.143 extern.h > --- extern.h 27 Jun 2022 10:18:27 - 1.143 > +++ extern.h 2 Aug 2022 18:10:00 - > @@ -727,6 +727,9 @@ int mkpathat(int, const char *); > #define MAX_HTTP_REQUESTS64 > #define MAX_RSYNC_REQUESTS 16 > > +/* How many seconds to wait for a connection to succeed. */ > +#define MAX_CONTIMEOUT 15 > + > /* Maximum allowd repositories per tal */ > #define MAX_REPO_PER_TAL 1000 > > Index: http.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v > retrieving revision 1.62 > diff -u -p -r1.62 http.c > --- http.c24 May 2022 09:22:45 - 1.62 > +++ http.c2 Aug 2022 18:10:01 - > @@ -70,7 +70,7 @@ > #define HTTP_USER_AGENT "OpenBSD rpki-client" > #define HTTP_BUF_SIZE(32 * 1024) > #define HTTP_IDLE_TIMEOUT10 > -#define HTTP_IO_TIMEOUT (3 * 60) > +#define HTTP_IO_TIMEOUT 15 > #define MAX_CONTENTLEN (2 * 1024 * 1024 * 1024LL) > #define NPFDS(MAX_HTTP_REQUESTS + 1) > > Index: rsync.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v > retrieving revision 1.38 > diff -u -p -r1.38 rsync.c > --- rsync.c 24 May 2022 09:20:49 - 1.38 > +++ rsync.c 2 Aug 2022 18:10:01 - > @@ -312,6 +312,7 @@ proc_rsync(char *prog, char *bind_addr, > args[i++] = "-rt"; > args[i++] = "--no-motd"; > args[i++] = "--max-size=" STRINGIFY(MAX_FILE_SIZE); > + args[i++] = "--contimeout=" STRINGIFY(MAX_CONTIMEOUT); > args[i++] = "--timeout=180"; > args[i++] = "--include=*/"; > args[i++] = "--include=*.cer"; >
Re: rpki-client: add connect() MAX_CONTIMEOUT for rsync/rrdp
On Tue, Aug 02, 2022 at 03:59:40PM +0200, Claudio Jeker wrote: > What makes you think that 15sec is enough to open connections in all > scenarios? I feel this is one of those changes that just shows that > maybe the current connect timeout from the system is too conservative. Yeah, maybe. How about this instead? Seems to work well for me. Kind regards, Job Index: extern.h === RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v retrieving revision 1.143 diff -u -p -r1.143 extern.h --- extern.h27 Jun 2022 10:18:27 - 1.143 +++ extern.h2 Aug 2022 18:10:00 - @@ -727,6 +727,9 @@ int mkpathat(int, const char *); #define MAX_HTTP_REQUESTS 64 #define MAX_RSYNC_REQUESTS 16 +/* How many seconds to wait for a connection to succeed. */ +#define MAX_CONTIMEOUT 15 + /* Maximum allowd repositories per tal */ #define MAX_REPO_PER_TAL 1000 Index: http.c === RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v retrieving revision 1.62 diff -u -p -r1.62 http.c --- http.c 24 May 2022 09:22:45 - 1.62 +++ http.c 2 Aug 2022 18:10:01 - @@ -70,7 +70,7 @@ #define HTTP_USER_AGENT"OpenBSD rpki-client" #define HTTP_BUF_SIZE (32 * 1024) #define HTTP_IDLE_TIMEOUT 10 -#define HTTP_IO_TIMEOUT(3 * 60) +#define HTTP_IO_TIMEOUT15 #define MAX_CONTENTLEN (2 * 1024 * 1024 * 1024LL) #define NPFDS (MAX_HTTP_REQUESTS + 1) Index: rsync.c === RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v retrieving revision 1.38 diff -u -p -r1.38 rsync.c --- rsync.c 24 May 2022 09:20:49 - 1.38 +++ rsync.c 2 Aug 2022 18:10:01 - @@ -312,6 +312,7 @@ proc_rsync(char *prog, char *bind_addr, args[i++] = "-rt"; args[i++] = "--no-motd"; args[i++] = "--max-size=" STRINGIFY(MAX_FILE_SIZE); + args[i++] = "--contimeout=" STRINGIFY(MAX_CONTIMEOUT); args[i++] = "--timeout=180"; args[i++] = "--include=*/"; args[i++] = "--include=*.cer";
Re: powerpc64: retrigger deferred DEC interrupts from splx(9)
> Date: Tue, 2 Aug 2022 11:56:59 -0500
> From: Scott Cheloha
>
> On Mon, Jul 25, 2022 at 06:44:31PM -0500, Scott Cheloha wrote:
> > On Mon, Jul 25, 2022 at 01:52:36PM +0200, Mark Kettenis wrote:
> > > > Date: Sun, 24 Jul 2022 19:33:57 -0500
> > > > From: Scott Cheloha
> > > >
> > > > On Sat, Jul 23, 2022 at 08:14:32PM -0500, Scott Cheloha wrote:
> > > > >
> > > > > [...]
> > > > >
> > > > > I don't have a powerpc64 machine, so this is untested. [...]
> > > >
> > > > gkoehler@ has pointed out two dumb typos in the prior patch. My bad.
> > > >
> > > > Here is a corrected patch that, according to gkoehler@, actually
> > > > compiles.
> > >
> > > Thanks. I already figured that bit out myself. Did some limited
> > > testing, but it seems to work correctly. No noticable effect on the
> > > timekeeping even when building clang on all the (4) cores.
> >
> > I wouldn't expect this patch to impact timekeeping. All we're doing
> > is calling hardclock(9) a bit sooner than we normally would a few
> > times every second.
> >
> > I would expect to see slightly more distinct interrupts (uvmexp.intrs)
> > per second because we aren't actively batching hardclock(9) and
> > statclock calls.
> >
> > ... by the way, uvmexp.intrs should probably be incremented
> > atomically, no?
> >
> > > Regarding the diff, I think it would be better to avoid changing
> > > trap.c. That function is complicated enough and splitting the logic
> > > for this over three files makes it a bit harder to understand. So you
> > > could have:
> > >
> > > void
> > > decr_intr(struct trapframe *frame)
> > > {
> > > struct cpu_info *ci = curcpu();
> > > ...
> > > int s;
> > >
> > > if (ci->ci_cpl >= IPL_CLOCK) {
> > > ci->ci_dec_deferred = 1;
> > > mtdec(UINT32_MAX >> 1); /* clear DEC exception */
> > > return;
> > > }
> > >
> > > ci->ci_dec_deferred = 0;
> > >
> > > ...
> > > }
> > >
> > > That has the downside of course that it will be slightly less
> > > efficient if we're at IPL_CLOCK or above, but that really shouldn't
> > > happen often enough for it to matter.
> >
> > Yep. It's an extra function call, the overhead is small.
> >
> > Updated patch below.
>
> At what point do we consider the patch safe? Have you seen any hangs?
>
> Wanna run with it another week?
Sorry. I'm not set up to run my powerpc64 machine for extended
periods of time. But I'm fine with this going in if George can
confirm that this diff builds.
> Index: include/cpu.h
> ===
> RCS file: /cvs/src/sys/arch/powerpc64/include/cpu.h,v
> retrieving revision 1.31
> diff -u -p -r1.31 cpu.h
> --- include/cpu.h 6 Jul 2021 09:34:07 - 1.31
> +++ include/cpu.h 25 Jul 2022 23:43:47 -
> @@ -74,9 +74,9 @@ struct cpu_info {
> uint64_tci_lasttb;
> uint64_tci_nexttimerevent;
> uint64_tci_nextstatevent;
> - int ci_statspending;
>
> volatile intci_cpl;
> + volatile intci_dec_deferred;
> uint32_tci_ipending;
> uint32_tci_idepth;
> #ifdef DIAGNOSTIC
> Index: powerpc64/clock.c
> ===
> RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/clock.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 clock.c
> --- powerpc64/clock.c 23 Feb 2021 04:44:31 - 1.3
> +++ powerpc64/clock.c 25 Jul 2022 23:43:47 -
> @@ -98,6 +98,17 @@ decr_intr(struct trapframe *frame)
> int s;
>
> /*
> + * If the clock interrupt is masked, postpone all work until
> + * it is unmasked in splx(9).
> + */
> + if (ci->ci_cpl >= IPL_CLOCK) {
> + ci->ci_dec_deferred = 1;
> + mtdec(UINT32_MAX >> 1); /* clear DEC exception */
> + return;
> + }
> + ci->ci_dec_deferred = 0;
> +
> + /*
>* Based on the actual time delay since the last decrementer reload,
>* we arrange for earlier interrupt next time.
>*/
> @@ -130,30 +141,23 @@ decr_intr(struct trapframe *frame)
> mtdec(nextevent - tb);
> mtdec(nextevent - mftb());
>
> - if (ci->ci_cpl >= IPL_CLOCK) {
> - ci->ci_statspending += nstats;
> - } else {
> - nstats += ci->ci_statspending;
> - ci->ci_statspending = 0;
> -
> - s = splclock();
> - intr_enable();
> -
> - /*
> - * Do standard timer interrupt stuff.
> - */
> - while (ci->ci_lasttb < prevtb) {
> - ci->ci_lasttb += tick_increment;
> - clock_count.ec_count++;
> - hardclock((struct clockframe *)frame);
> - }
> + s = splclock();
> + intr_enable();
>
> - while (nstats-- > 0)
> - statclock((struct clockframe *)frame);
> -
> - intr_disable();
>
Re: openrsync: add --contimeout
Job Snijders wrote:
> - /* Set up non-blocking mode. */
> -
> if ((flags = fcntl(*sd, F_GETFL, 0)) == -1) {
> - ERR("fcntl");
> - return -1;
> - } else if (fcntl(*sd, F_SETFL, flags|O_NONBLOCK) == -1) {
> ERR("fcntl");
> return -1;
> }
It looks like you can delete F_GETFL, and the flags variable. Unused.
Re: openrsync: add --contimeout
On Tue, Aug 02, 2022 at 04:15:39PM +0200, Claudio Jeker wrote:
> On Tue, Aug 02, 2022 at 12:42:26PM +, Job Snijders wrote:
> > This adds '--contimeout' to rsync(1)
> >
> > $ time openrsync --contimeout=5 -rt rsync://203.119.21.1/test /tmp/k
> > openrsync: warning: connect timeout: 203.119.21.1, 203.119.21.1
> > openrsync: error: cannot connect to host: 203.119.21.1
> > 0m05.01s real 0m00.00s user 0m00.01s system
> >
> > OK?
> >
...
> > @@ -99,8 +103,28 @@ inet_connect(int *sd, const struct sourc
>
> You make the socket non-blocking by default. Please remove the fcntl
> at the end of the function. There is no need to double up the
> non-blocking here.
>
> Also I would move the ETIMEDOUT handling up into the connect block. No
> need to do this complicated errno game.
>
> You can use a switch for the poll call: switch (poll()) { case 0, case
> 1, default). I don't like how c is used over and over again.
Thanks for the feedback. Like so?
Kind regards,
Job
Index: socket.c
===
RCS file: /cvs/src/usr.bin/rsync/socket.c,v
retrieving revision 1.31
diff -u -p -r1.31 socket.c
--- socket.c30 Jun 2021 13:10:04 - 1.31
+++ socket.c2 Aug 2022 16:54:45 -
@@ -75,14 +75,18 @@ static int
inet_connect(int *sd, const struct source *src, const char *host,
const struct source *bsrc, size_t bsrcsz)
{
- int c, flags;
+ struct pollfd pfd;
+ socklen_t optlen;
+ int c, flags;
+ int optval;
if (*sd != -1)
close(*sd);
LOG2("trying: %s, %s", src->ip, host);
- if ((*sd = socket(src->family, SOCK_STREAM, 0)) == -1) {
+ if ((*sd = socket(src->family, SOCK_STREAM | SOCK_NONBLOCK, 0))
+ == -1) {
ERR("socket");
return -1;
}
@@ -94,30 +98,42 @@ inet_connect(int *sd, const struct sourc
/*
* Initiate blocking connection.
-* We use the blocking connect() instead of passing NONBLOCK to
-* the socket() function because we don't need to do anything
-* while waiting for this to finish.
+* We use non-blocking connect() so we can poll() for contimeout.
*/
- c = connect(*sd, (const struct sockaddr *)>sa, src->salen);
+ if ((c = connect(*sd, (const struct sockaddr *)>sa, src->salen))
+ != 0 && errno == EINPROGRESS) {
+ pfd.fd = *sd;
+ pfd.events = POLLOUT;
+ switch (c = poll(, 1, poll_contimeout)) {
+ case 1:
+ optlen = sizeof(optval);
+ if ((c = getsockopt(*sd, SOL_SOCKET, SO_ERROR, ,
+ )) == 0) {
+ errno = optval;
+ c = optval == 0 ? 0 : -1;
+ }
+ break;
+ case 0:
+ errno = ETIMEDOUT;
+ WARNX("connect timeout: %s, %s", src->ip, host);
+ return 0;
+ default:
+ err(1, "poll failed");
+ }
+ }
if (c == -1) {
if (errno == EADDRNOTAVAIL)
return 0;
if (errno == ECONNREFUSED || errno == EHOSTUNREACH) {
- WARNX("connect refused: %s, %s",
- src->ip, host);
+ WARNX("connect refused: %s, %s", src->ip, host);
return 0;
}
ERR("connect");
return -1;
}
- /* Set up non-blocking mode. */
-
if ((flags = fcntl(*sd, F_GETFL, 0)) == -1) {
- ERR("fcntl");
- return -1;
- } else if (fcntl(*sd, F_SETFL, flags|O_NONBLOCK) == -1) {
ERR("fcntl");
return -1;
}
Re: powerpc64: retrigger deferred DEC interrupts from splx(9)
On Mon, Jul 25, 2022 at 06:44:31PM -0500, Scott Cheloha wrote:
> On Mon, Jul 25, 2022 at 01:52:36PM +0200, Mark Kettenis wrote:
> > > Date: Sun, 24 Jul 2022 19:33:57 -0500
> > > From: Scott Cheloha
> > >
> > > On Sat, Jul 23, 2022 at 08:14:32PM -0500, Scott Cheloha wrote:
> > > >
> > > > [...]
> > > >
> > > > I don't have a powerpc64 machine, so this is untested. [...]
> > >
> > > gkoehler@ has pointed out two dumb typos in the prior patch. My bad.
> > >
> > > Here is a corrected patch that, according to gkoehler@, actually
> > > compiles.
> >
> > Thanks. I already figured that bit out myself. Did some limited
> > testing, but it seems to work correctly. No noticable effect on the
> > timekeeping even when building clang on all the (4) cores.
>
> I wouldn't expect this patch to impact timekeeping. All we're doing
> is calling hardclock(9) a bit sooner than we normally would a few
> times every second.
>
> I would expect to see slightly more distinct interrupts (uvmexp.intrs)
> per second because we aren't actively batching hardclock(9) and
> statclock calls.
>
> ... by the way, uvmexp.intrs should probably be incremented
> atomically, no?
>
> > Regarding the diff, I think it would be better to avoid changing
> > trap.c. That function is complicated enough and splitting the logic
> > for this over three files makes it a bit harder to understand. So you
> > could have:
> >
> > void
> > decr_intr(struct trapframe *frame)
> > {
> > struct cpu_info *ci = curcpu();
> > ...
> > int s;
> >
> > if (ci->ci_cpl >= IPL_CLOCK) {
> > ci->ci_dec_deferred = 1;
> > mtdec(UINT32_MAX >> 1); /* clear DEC exception */
> > return;
> > }
> >
> > ci->ci_dec_deferred = 0;
> >
> > ...
> > }
> >
> > That has the downside of course that it will be slightly less
> > efficient if we're at IPL_CLOCK or above, but that really shouldn't
> > happen often enough for it to matter.
>
> Yep. It's an extra function call, the overhead is small.
>
> Updated patch below.
At what point do we consider the patch safe? Have you seen any hangs?
Wanna run with it another week?
Index: include/cpu.h
===
RCS file: /cvs/src/sys/arch/powerpc64/include/cpu.h,v
retrieving revision 1.31
diff -u -p -r1.31 cpu.h
--- include/cpu.h 6 Jul 2021 09:34:07 - 1.31
+++ include/cpu.h 25 Jul 2022 23:43:47 -
@@ -74,9 +74,9 @@ struct cpu_info {
uint64_tci_lasttb;
uint64_tci_nexttimerevent;
uint64_tci_nextstatevent;
- int ci_statspending;
volatile intci_cpl;
+ volatile intci_dec_deferred;
uint32_tci_ipending;
uint32_tci_idepth;
#ifdef DIAGNOSTIC
Index: powerpc64/clock.c
===
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/clock.c,v
retrieving revision 1.3
diff -u -p -r1.3 clock.c
--- powerpc64/clock.c 23 Feb 2021 04:44:31 - 1.3
+++ powerpc64/clock.c 25 Jul 2022 23:43:47 -
@@ -98,6 +98,17 @@ decr_intr(struct trapframe *frame)
int s;
/*
+* If the clock interrupt is masked, postpone all work until
+* it is unmasked in splx(9).
+*/
+ if (ci->ci_cpl >= IPL_CLOCK) {
+ ci->ci_dec_deferred = 1;
+ mtdec(UINT32_MAX >> 1); /* clear DEC exception */
+ return;
+ }
+ ci->ci_dec_deferred = 0;
+
+ /*
* Based on the actual time delay since the last decrementer reload,
* we arrange for earlier interrupt next time.
*/
@@ -130,30 +141,23 @@ decr_intr(struct trapframe *frame)
mtdec(nextevent - tb);
mtdec(nextevent - mftb());
- if (ci->ci_cpl >= IPL_CLOCK) {
- ci->ci_statspending += nstats;
- } else {
- nstats += ci->ci_statspending;
- ci->ci_statspending = 0;
-
- s = splclock();
- intr_enable();
-
- /*
-* Do standard timer interrupt stuff.
-*/
- while (ci->ci_lasttb < prevtb) {
- ci->ci_lasttb += tick_increment;
- clock_count.ec_count++;
- hardclock((struct clockframe *)frame);
- }
+ s = splclock();
+ intr_enable();
- while (nstats-- > 0)
- statclock((struct clockframe *)frame);
-
- intr_disable();
- splx(s);
+ /*
+* Do standard timer interrupt stuff.
+*/
+ while (ci->ci_lasttb < prevtb) {
+ ci->ci_lasttb += tick_increment;
+ clock_count.ec_count++;
+ hardclock((struct clockframe *)frame);
}
+
+ while (nstats-- > 0)
+ statclock((struct clockframe *)frame);
+
+
dmesg(8): fail if given positional arguments
dmesg(8) doesn't use any positional arguments. It's a usage error if
any are present.
ok?
Index: dmesg.c
===
RCS file: /cvs/src/sbin/dmesg/dmesg.c,v
retrieving revision 1.31
diff -u -p -r1.31 dmesg.c
--- dmesg.c 24 Dec 2019 13:20:44 - 1.31
+++ dmesg.c 2 Aug 2022 16:48:13 -
@@ -89,6 +89,9 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
+ if (argc != 0)
+ usage();
+
if (memf == NULL && nlistf == NULL) {
int mib[2], msgbufsize;
size_t len;
Re: bgpd force fib sync in fetchtable
On Tue, Aug 02, 2022 at 12:58:13PM +0100, Stuart Henderson wrote:
> On 2022/08/02 12:34, Claudio Jeker wrote:
> > On startup we load the routing table in bgpd and at that moment a cleanup
> > of old bgpd routes should happen. I noticed this is not the case because
> > fib_sync is not set and so send_rtmsg() just returns.
> > I think we need to force fib_sync in fetchtable() to make sure the cleanup
> > happens correctly.
>
> How much of a problem is it that this clears any existing bgpd routes
> if "fib update no" is set?
>
> I guess in the common case there won't be any anyway, but is there
> any use-case for running two copies of bgpd, one with fib update, one
> without, on the same machine?
One case I see is if you run two bgpd in on two different rtables (not
rdomains). In that case the table used for nexthop lookups will be used by
both daemons. Not sure if this is an issue or not.
The problem with fib_sync being 0 comes from parse.y::parse_config()
if ((rr = add_rib("Adj-RIB-In")) == NULL)
fatal("add_rib failed");
rr->flags = F_RIB_NOFIB | F_RIB_NOEVALUATE;
if ((rr = add_rib("Loc-RIB")) == NULL)
fatal("add_rib failed");
rib_add_fib(rr, conf->default_tableid);
rr->flags = F_RIB_LOCAL;
First the Adj-RIB-In is added then the Loc-RIB. The Adj-RIB-In has
F_RIB_NOFIB | F_RIB_NOEVALUATE and so fib_sync is off when fetchtable is
called in ktable_new().
Afterwards the ktable is updated by Loc-RIB and fib_sync is enabled but
the cleanup did not happen.
So a quick fix is to switch the order in parse.y but heck that probably
breaks later on if used with other tables.
I guess fetchtable() needs to become smarter so that it can be called more
than once. That would also allow to handle RTM_DESYNC in bgpd.
> > OK?
> > --
> > :wq Claudio
> >
> > Index: kroute.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> > retrieving revision 1.285
> > diff -u -p -r1.285 kroute.c
> > --- kroute.c28 Jul 2022 14:05:13 - 1.285
> > +++ kroute.c2 Aug 2022 10:13:59 -
> > @@ -2726,6 +2726,7 @@ fetchtable(struct ktable *kt)
> > char*buf = NULL, *next, *lim;
> > struct rt_msghdr*rtm;
> > struct kroute_full kf;
> > + int fib_sync;
> >
> > mib[0] = CTL_NET;
> > mib[1] = PF_ROUTE;
> > @@ -2754,6 +2755,10 @@ fetchtable(struct ktable *kt)
> > }
> > }
> >
> > + /* force fib_sync on during fetch */
> > + fib_sync = kt->fib_sync;
> > + kt->fib_sync = 1;
> > +
> > lim = buf + len;
> > for (next = buf; next < lim; next += rtm->rtm_msglen) {
> > rtm = (struct rt_msghdr *)next;
> > @@ -2768,6 +2773,8 @@ fetchtable(struct ktable *kt)
> > else
> > kroute_insert(kt, );
> > }
> > + kt->fib_sync = fib_sync;
> > +
> > free(buf);
> > return (0);
> > }
> >
>
--
:wq Claudio
Re: openrsync: add --contimeout
On Tue, Aug 02, 2022 at 12:42:26PM +, Job Snijders wrote:
> Hi all,
>
> This adds '--contimeout' to rsync(1)
>
> $ time openrsync --contimeout=5 -rt rsync://203.119.21.1/test /tmp/k
> openrsync: warning: connect timeout: 203.119.21.1, 203.119.21.1
> openrsync: error: cannot connect to host: 203.119.21.1
> 0m05.01s real 0m00.00s user 0m00.01s system
>
> OK?
>
> Kind regards,
>
> Job
>
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.bin/rsync/extern.h,v
> retrieving revision 1.43
> diff -u -p -r1.43 extern.h
> --- extern.h 29 Oct 2021 08:00:59 - 1.43
> +++ extern.h 2 Aug 2022 12:34:49 -
> @@ -70,6 +70,11 @@
> extern int poll_timeout;
>
> /*
> + * Use this for --contimeout.
> + */
> +extern int poll_contimeout;
> +
> +/*
> * Operating mode for a client or a server.
> * Sender means we synchronise local files with those from remote.
> * Receiver is the opposite.
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/main.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 main.c
> --- main.c3 Nov 2021 14:42:12 - 1.63
> +++ main.c2 Aug 2022 12:34:49 -
> @@ -31,6 +31,7 @@
> #include "extern.h"
>
> int verbose;
> +int poll_contimeout;
> int poll_timeout;
>
> /*
> @@ -286,6 +287,7 @@ static struct opts opts;
> #define OP_LINK_DEST 1011
> #define OP_MAX_SIZE 1012
> #define OP_MIN_SIZE 1013
> +#define OP_CONTIMEOUT1014
>
> const struct option lopts[] = {
> { "address", required_argument, NULL,OP_ADDRESS },
> @@ -296,6 +298,7 @@ const struct optionlopts[] = {
> { "link-dest", required_argument, NULL,OP_LINK_DEST },
> #endif
> { "compress",no_argument,NULL, 'z' },
> +{ "contimeout", required_argument, NULL,OP_CONTIMEOUT },
> { "del", no_argument,, 1 },
> { "delete", no_argument,, 1 },
> { "devices", no_argument,, 1 },
> @@ -411,6 +414,12 @@ main(int argc, char *argv[])
> case OP_ADDRESS:
> opts.address = optarg;
> break;
> + case OP_CONTIMEOUT:
> + poll_contimeout = strtonum(optarg, 0, 60*60, );
> + if (errstr != NULL)
> + errx(ERR_SYNTAX, "timeout is %s: %s",
> + errstr, optarg);
> + break;
> case OP_PORT:
> opts.port = optarg;
> break;
> @@ -503,9 +512,16 @@ basedir:
> if (opts.port == NULL)
> opts.port = "rsync";
>
> + /* by default and for --contimeout=0 disable poll_contimeout */
> + if (poll_contimeout == 0)
> + poll_contimeout = -1;
> + else
> + poll_contimeout *= 1000;
> +
> /* by default and for --timeout=0 disable poll_timeout */
> if (poll_timeout == 0)
> - poll_timeout = -1; else
> + poll_timeout = -1;
> + else
> poll_timeout *= 1000;
>
> /*
> @@ -614,10 +630,11 @@ basedir:
> usage:
> fprintf(stderr, "usage: %s"
> " [-aDglnoprtvx] [-e program] [--address=sourceaddr]\n"
> - "\t[--compare-dest=dir] [--del] [--exclude] [--exclude-from=file]\n"
> - "\t[--include] [--include-from=file] [--no-motd] [--numeric-ids]\n"
> - "\t[--port=portnumber] [--rsync-path=program] [--timeout=seconds]\n"
> - "\t[--version] source ... directory\n",
> + "\t[--contimeout=seconds [--compare-dest=dir] [--del] [--exclude]\n"
> + "\t[--exclude-from=file] [--include] [--include-from=file]\n"
> + "\t[--no-motd] [--numeric-ids] [--port=portnumber]\n"
> + "\t[--rsync-path=program] [--timeout=seconds] [--version]\n"
> + "\tsource ... directory\n",
> getprogname());
> exit(ERR_SYNTAX);
> }
> Index: rsync.1
> ===
> RCS file: /cvs/src/usr.bin/rsync/rsync.1,v
> retrieving revision 1.29
> diff -u -p -r1.29 rsync.1
> --- rsync.1 26 Nov 2021 03:41:39 - 1.29
> +++ rsync.1 2 Aug 2022 12:34:49 -
> @@ -26,6 +26,7 @@
> .Op Fl e Ar program
> .Op Fl -address Ns = Ns Ar sourceaddr
> .Op Fl -compare-dest Ns = Ns Ar directory
> +.Op Fl -contimeout Ns = Ns Ar seconds
> .Op Fl -del
> .Op Fl -exclude Ar pattern
> .Op Fl -exclude-from Ns = Ns Ar file
> @@ -77,6 +78,10 @@ directories may be provided.
> If
> .Ar directory
> is a relative path, it is relative to the destination directory.
> +.It Fl -contimeout Ns = Ns Ar seconds
> +Set the connection timeout in seconds.
> +Exit if no connection established within the specified time.
> +The default is 0, which means no timeout.
> .It Fl D
Re: rpki-client: add connect() MAX_CONTIMEOUT for rsync/rrdp
On Tue, Aug 02, 2022 at 01:42:43PM +, Job Snijders wrote:
> Hi,
>
> We were doing a lot of waiting in connect() for some (currently) broken
> repositories. Move on with life after MAX_CONTIMEOUT seconds.
>
> This changeset reduces the real time spent fetching the RIPE TAL (with
> hot cache) from 7m14.57s to 3m20.10s on an IPv4+IPv6 dual-stacked host.
>
> This changeset depends on
> https://marc.info/?l=openbsd-tech=165944397404967=2
>
> Kind regards,
>
> Job
>
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.143
> diff -u -p -r1.143 extern.h
> --- extern.h 27 Jun 2022 10:18:27 - 1.143
> +++ extern.h 2 Aug 2022 13:21:19 -
> @@ -727,6 +727,9 @@ int mkpathat(int, const char *);
> #define MAX_HTTP_REQUESTS64
> #define MAX_RSYNC_REQUESTS 16
>
> +/* How many seconds to wait for a connection to succeed. */
> +#define MAX_CONTIMEOUT 15
> +
What makes you think that 15sec is enough to open connections in all
scenarios? I feel this is one of those changes that just shows that maybe
the current connect timeout from the system is too conservative.
> /* Maximum allowd repositories per tal */
> #define MAX_REPO_PER_TAL 1000
>
> Index: http.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 http.c
> --- http.c24 May 2022 09:22:45 - 1.62
> +++ http.c2 Aug 2022 13:21:20 -
> @@ -801,6 +801,9 @@ static enum res
> http_connect(struct http_connection *conn)
> {
> const char *cause = NULL;
> + struct pollfd pfd;
> + socklen_t optlen;
> + int optval, r;
>
> assert(conn->fd == -1);
> conn->state = STATE_CONNECT;
> @@ -834,10 +837,17 @@ http_connect(struct http_connection *con
> }
> }
>
> - if (connect(conn->fd, res->ai_addr, res->ai_addrlen) == -1) {
> - if (errno == EINPROGRESS) {
> - /* wait for async connect to finish. */
> - return WANT_POLLOUT;
> + if ((r = connect(conn->fd, res->ai_addr, res->ai_addrlen))
> + != 0 && errno == EINPROGRESS) {
> + pfd.fd = conn->fd;
> + pfd.events = POLLOUT;
> + if ((r = poll(, 1, MAX_CONTIMEOUT * 1000)) == 1) {
> + optlen = sizeof(optval);
> + if ((r = getsockopt(conn->fd, SOL_SOCKET,
> + SO_ERROR, , )) == 0) {
> + errno = optval;
> + r = optval == 0 ? 0 : -1;
> + }
> } else {
> save_errno = errno;
> close(conn->fd);
This is not correct. You can't just poll for this connection and have all
other http connection wait until that is done. You need to handle this in
the main poll loop.
> Index: rsync.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 rsync.c
> --- rsync.c 24 May 2022 09:20:49 - 1.38
> +++ rsync.c 2 Aug 2022 13:21:20 -
> @@ -312,6 +312,7 @@ proc_rsync(char *prog, char *bind_addr,
> args[i++] = "-rt";
> args[i++] = "--no-motd";
> args[i++] = "--max-size=" STRINGIFY(MAX_FILE_SIZE);
> + args[i++] = "--contimeout=" STRINGIFY(MAX_CONTIMEOUT);
> args[i++] = "--timeout=180";
> args[i++] = "--include=*/";
> args[i++] = "--include=*.cer";
>
--
:wq Claudio
rpki-client: add connect() MAX_CONTIMEOUT for rsync/rrdp
Hi,
We were doing a lot of waiting in connect() for some (currently) broken
repositories. Move on with life after MAX_CONTIMEOUT seconds.
This changeset reduces the real time spent fetching the RIPE TAL (with
hot cache) from 7m14.57s to 3m20.10s on an IPv4+IPv6 dual-stacked host.
This changeset depends on
https://marc.info/?l=openbsd-tech=165944397404967=2
Kind regards,
Job
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.143
diff -u -p -r1.143 extern.h
--- extern.h27 Jun 2022 10:18:27 - 1.143
+++ extern.h2 Aug 2022 13:21:19 -
@@ -727,6 +727,9 @@ int mkpathat(int, const char *);
#define MAX_HTTP_REQUESTS 64
#define MAX_RSYNC_REQUESTS 16
+/* How many seconds to wait for a connection to succeed. */
+#define MAX_CONTIMEOUT 15
+
/* Maximum allowd repositories per tal */
#define MAX_REPO_PER_TAL 1000
Index: http.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
retrieving revision 1.62
diff -u -p -r1.62 http.c
--- http.c 24 May 2022 09:22:45 - 1.62
+++ http.c 2 Aug 2022 13:21:20 -
@@ -801,6 +801,9 @@ static enum res
http_connect(struct http_connection *conn)
{
const char *cause = NULL;
+ struct pollfd pfd;
+ socklen_t optlen;
+ int optval, r;
assert(conn->fd == -1);
conn->state = STATE_CONNECT;
@@ -834,10 +837,17 @@ http_connect(struct http_connection *con
}
}
- if (connect(conn->fd, res->ai_addr, res->ai_addrlen) == -1) {
- if (errno == EINPROGRESS) {
- /* wait for async connect to finish. */
- return WANT_POLLOUT;
+ if ((r = connect(conn->fd, res->ai_addr, res->ai_addrlen))
+ != 0 && errno == EINPROGRESS) {
+ pfd.fd = conn->fd;
+ pfd.events = POLLOUT;
+ if ((r = poll(, 1, MAX_CONTIMEOUT * 1000)) == 1) {
+ optlen = sizeof(optval);
+ if ((r = getsockopt(conn->fd, SOL_SOCKET,
+ SO_ERROR, , )) == 0) {
+ errno = optval;
+ r = optval == 0 ? 0 : -1;
+ }
} else {
save_errno = errno;
close(conn->fd);
Index: rsync.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v
retrieving revision 1.38
diff -u -p -r1.38 rsync.c
--- rsync.c 24 May 2022 09:20:49 - 1.38
+++ rsync.c 2 Aug 2022 13:21:20 -
@@ -312,6 +312,7 @@ proc_rsync(char *prog, char *bind_addr,
args[i++] = "-rt";
args[i++] = "--no-motd";
args[i++] = "--max-size=" STRINGIFY(MAX_FILE_SIZE);
+ args[i++] = "--contimeout=" STRINGIFY(MAX_CONTIMEOUT);
args[i++] = "--timeout=180";
args[i++] = "--include=*/";
args[i++] = "--include=*.cer";
openrsync: add --contimeout
Hi all,
This adds '--contimeout' to rsync(1)
$ time openrsync --contimeout=5 -rt rsync://203.119.21.1/test /tmp/k
openrsync: warning: connect timeout: 203.119.21.1, 203.119.21.1
openrsync: error: cannot connect to host: 203.119.21.1
0m05.01s real 0m00.00s user 0m00.01s system
OK?
Kind regards,
Job
Index: extern.h
===
RCS file: /cvs/src/usr.bin/rsync/extern.h,v
retrieving revision 1.43
diff -u -p -r1.43 extern.h
--- extern.h29 Oct 2021 08:00:59 - 1.43
+++ extern.h2 Aug 2022 12:34:49 -
@@ -70,6 +70,11 @@
extern int poll_timeout;
/*
+ * Use this for --contimeout.
+ */
+extern int poll_contimeout;
+
+/*
* Operating mode for a client or a server.
* Sender means we synchronise local files with those from remote.
* Receiver is the opposite.
Index: main.c
===
RCS file: /cvs/src/usr.bin/rsync/main.c,v
retrieving revision 1.63
diff -u -p -r1.63 main.c
--- main.c 3 Nov 2021 14:42:12 - 1.63
+++ main.c 2 Aug 2022 12:34:49 -
@@ -31,6 +31,7 @@
#include "extern.h"
int verbose;
+int poll_contimeout;
int poll_timeout;
/*
@@ -286,6 +287,7 @@ static struct opts opts;
#define OP_LINK_DEST 1011
#define OP_MAX_SIZE1012
#define OP_MIN_SIZE1013
+#define OP_CONTIMEOUT 1014
const struct option lopts[] = {
{ "address", required_argument, NULL,OP_ADDRESS },
@@ -296,6 +298,7 @@ const struct option lopts[] = {
{ "link-dest", required_argument, NULL,OP_LINK_DEST },
#endif
{ "compress", no_argument,NULL, 'z' },
+{ "contimeout",required_argument, NULL,OP_CONTIMEOUT },
{ "del", no_argument,, 1 },
{ "delete",no_argument,, 1 },
{ "devices", no_argument,, 1 },
@@ -411,6 +414,12 @@ main(int argc, char *argv[])
case OP_ADDRESS:
opts.address = optarg;
break;
+ case OP_CONTIMEOUT:
+ poll_contimeout = strtonum(optarg, 0, 60*60, );
+ if (errstr != NULL)
+ errx(ERR_SYNTAX, "timeout is %s: %s",
+ errstr, optarg);
+ break;
case OP_PORT:
opts.port = optarg;
break;
@@ -503,9 +512,16 @@ basedir:
if (opts.port == NULL)
opts.port = "rsync";
+ /* by default and for --contimeout=0 disable poll_contimeout */
+ if (poll_contimeout == 0)
+ poll_contimeout = -1;
+ else
+ poll_contimeout *= 1000;
+
/* by default and for --timeout=0 disable poll_timeout */
if (poll_timeout == 0)
- poll_timeout = -1; else
+ poll_timeout = -1;
+ else
poll_timeout *= 1000;
/*
@@ -614,10 +630,11 @@ basedir:
usage:
fprintf(stderr, "usage: %s"
" [-aDglnoprtvx] [-e program] [--address=sourceaddr]\n"
- "\t[--compare-dest=dir] [--del] [--exclude] [--exclude-from=file]\n"
- "\t[--include] [--include-from=file] [--no-motd] [--numeric-ids]\n"
- "\t[--port=portnumber] [--rsync-path=program] [--timeout=seconds]\n"
- "\t[--version] source ... directory\n",
+ "\t[--contimeout=seconds [--compare-dest=dir] [--del] [--exclude]\n"
+ "\t[--exclude-from=file] [--include] [--include-from=file]\n"
+ "\t[--no-motd] [--numeric-ids] [--port=portnumber]\n"
+ "\t[--rsync-path=program] [--timeout=seconds] [--version]\n"
+ "\tsource ... directory\n",
getprogname());
exit(ERR_SYNTAX);
}
Index: rsync.1
===
RCS file: /cvs/src/usr.bin/rsync/rsync.1,v
retrieving revision 1.29
diff -u -p -r1.29 rsync.1
--- rsync.1 26 Nov 2021 03:41:39 - 1.29
+++ rsync.1 2 Aug 2022 12:34:49 -
@@ -26,6 +26,7 @@
.Op Fl e Ar program
.Op Fl -address Ns = Ns Ar sourceaddr
.Op Fl -compare-dest Ns = Ns Ar directory
+.Op Fl -contimeout Ns = Ns Ar seconds
.Op Fl -del
.Op Fl -exclude Ar pattern
.Op Fl -exclude-from Ns = Ns Ar file
@@ -77,6 +78,10 @@ directories may be provided.
If
.Ar directory
is a relative path, it is relative to the destination directory.
+.It Fl -contimeout Ns = Ns Ar seconds
+Set the connection timeout in seconds.
+Exit if no connection established within the specified time.
+The default is 0, which means no timeout.
.It Fl D
Also transfer device and special files.
Shorthand for
Index: socket.c
===
RCS file: /cvs/src/usr.bin/rsync/socket.c,v
retrieving revision 1.31
diff -u -p -r1.31 socket.c
---
Re: bgpd force fib sync in fetchtable
On 2022/08/02 12:34, Claudio Jeker wrote:
> On startup we load the routing table in bgpd and at that moment a cleanup
> of old bgpd routes should happen. I noticed this is not the case because
> fib_sync is not set and so send_rtmsg() just returns.
> I think we need to force fib_sync in fetchtable() to make sure the cleanup
> happens correctly.
How much of a problem is it that this clears any existing bgpd routes
if "fib update no" is set?
I guess in the common case there won't be any anyway, but is there
any use-case for running two copies of bgpd, one with fib update, one
without, on the same machine?
> OK?
> --
> :wq Claudio
>
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.285
> diff -u -p -r1.285 kroute.c
> --- kroute.c 28 Jul 2022 14:05:13 - 1.285
> +++ kroute.c 2 Aug 2022 10:13:59 -
> @@ -2726,6 +2726,7 @@ fetchtable(struct ktable *kt)
> char*buf = NULL, *next, *lim;
> struct rt_msghdr*rtm;
> struct kroute_full kf;
> + int fib_sync;
>
> mib[0] = CTL_NET;
> mib[1] = PF_ROUTE;
> @@ -2754,6 +2755,10 @@ fetchtable(struct ktable *kt)
> }
> }
>
> + /* force fib_sync on during fetch */
> + fib_sync = kt->fib_sync;
> + kt->fib_sync = 1;
> +
> lim = buf + len;
> for (next = buf; next < lim; next += rtm->rtm_msglen) {
> rtm = (struct rt_msghdr *)next;
> @@ -2768,6 +2773,8 @@ fetchtable(struct ktable *kt)
> else
> kroute_insert(kt, );
> }
> + kt->fib_sync = fib_sync;
> +
> free(buf);
> return (0);
> }
>
Re: bgpd force fib sync in fetchtable
On Tue, Aug 02, 2022 at 01:44:42PM +0200, Theo Buehler wrote:
> On Tue, Aug 02, 2022 at 12:34:40PM +0200, Claudio Jeker wrote:
> > On startup we load the routing table in bgpd and at that moment a cleanup
> > of old bgpd routes should happen. I noticed this is not the case because
> > fib_sync is not set and so send_rtmsg() just returns.
> > I think we need to force fib_sync in fetchtable() to make sure the cleanup
> > happens correctly.
>
> If I understand correctly, this ignores the fs flag of ktable_new() for
> the fetchtable() call. In particular, 'fib-update no' is ignored in this
> situation. Is that intentional?
I think it is. At least I thought that even with 'fib-update no' bgpd
should cleanup the FIB. Now that may be a problem when people run multiple
bgpds with 'fib-update no'.
I see if I can find a solution that is a smaller hammer to fix this.
> >
> > OK?
> > --
> > :wq Claudio
> >
> > Index: kroute.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> > retrieving revision 1.285
> > diff -u -p -r1.285 kroute.c
> > --- kroute.c28 Jul 2022 14:05:13 - 1.285
> > +++ kroute.c2 Aug 2022 10:13:59 -
> > @@ -2726,6 +2726,7 @@ fetchtable(struct ktable *kt)
> > char*buf = NULL, *next, *lim;
> > struct rt_msghdr*rtm;
> > struct kroute_full kf;
> > + int fib_sync;
> >
> > mib[0] = CTL_NET;
> > mib[1] = PF_ROUTE;
> > @@ -2754,6 +2755,10 @@ fetchtable(struct ktable *kt)
> > }
> > }
> >
> > + /* force fib_sync on during fetch */
> > + fib_sync = kt->fib_sync;
> > + kt->fib_sync = 1;
> > +
> > lim = buf + len;
> > for (next = buf; next < lim; next += rtm->rtm_msglen) {
> > rtm = (struct rt_msghdr *)next;
> > @@ -2768,6 +2773,8 @@ fetchtable(struct ktable *kt)
> > else
> > kroute_insert(kt, );
> > }
> > + kt->fib_sync = fib_sync;
> > +
> > free(buf);
> > return (0);
> > }
> >
>
--
:wq Claudio
Re: bgpd force fib sync in fetchtable
On Tue, Aug 02, 2022 at 12:34:40PM +0200, Claudio Jeker wrote:
> On startup we load the routing table in bgpd and at that moment a cleanup
> of old bgpd routes should happen. I noticed this is not the case because
> fib_sync is not set and so send_rtmsg() just returns.
> I think we need to force fib_sync in fetchtable() to make sure the cleanup
> happens correctly.
If I understand correctly, this ignores the fs flag of ktable_new() for
the fetchtable() call. In particular, 'fib-update no' is ignored in this
situation. Is that intentional?
>
> OK?
> --
> :wq Claudio
>
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.285
> diff -u -p -r1.285 kroute.c
> --- kroute.c 28 Jul 2022 14:05:13 - 1.285
> +++ kroute.c 2 Aug 2022 10:13:59 -
> @@ -2726,6 +2726,7 @@ fetchtable(struct ktable *kt)
> char*buf = NULL, *next, *lim;
> struct rt_msghdr*rtm;
> struct kroute_full kf;
> + int fib_sync;
>
> mib[0] = CTL_NET;
> mib[1] = PF_ROUTE;
> @@ -2754,6 +2755,10 @@ fetchtable(struct ktable *kt)
> }
> }
>
> + /* force fib_sync on during fetch */
> + fib_sync = kt->fib_sync;
> + kt->fib_sync = 1;
> +
> lim = buf + len;
> for (next = buf; next < lim; next += rtm->rtm_msglen) {
> rtm = (struct rt_msghdr *)next;
> @@ -2768,6 +2773,8 @@ fetchtable(struct ktable *kt)
> else
> kroute_insert(kt, );
> }
> + kt->fib_sync = fib_sync;
> +
> free(buf);
> return (0);
> }
>
Re: gzip: fix pledge violation
Is there something wrong with my diff? I don't see it. Please review it.
bgpd force fib sync in fetchtable
On startup we load the routing table in bgpd and at that moment a cleanup
of old bgpd routes should happen. I noticed this is not the case because
fib_sync is not set and so send_rtmsg() just returns.
I think we need to force fib_sync in fetchtable() to make sure the cleanup
happens correctly.
OK?
--
:wq Claudio
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.285
diff -u -p -r1.285 kroute.c
--- kroute.c28 Jul 2022 14:05:13 - 1.285
+++ kroute.c2 Aug 2022 10:13:59 -
@@ -2726,6 +2726,7 @@ fetchtable(struct ktable *kt)
char*buf = NULL, *next, *lim;
struct rt_msghdr*rtm;
struct kroute_full kf;
+ int fib_sync;
mib[0] = CTL_NET;
mib[1] = PF_ROUTE;
@@ -2754,6 +2755,10 @@ fetchtable(struct ktable *kt)
}
}
+ /* force fib_sync on during fetch */
+ fib_sync = kt->fib_sync;
+ kt->fib_sync = 1;
+
lim = buf + len;
for (next = buf; next < lim; next += rtm->rtm_msglen) {
rtm = (struct rt_msghdr *)next;
@@ -2768,6 +2773,8 @@ fetchtable(struct ktable *kt)
else
kroute_insert(kt, );
}
+ kt->fib_sync = fib_sync;
+
free(buf);
return (0);
}
Re: fix NAT round-robin and random
Hello,
On Mon, Aug 01, 2022 at 06:37:58PM +0200, Hrvoje Popovski wrote:
> On 20.7.2022. 22:27, Alexandr Nedvedicky wrote:
> > Hello,
> >
> > below is a final version of patch for NAT issue discussed at bugs@ [1].
> > Patch below is updated according to feedback I got from Chris, claudio@
> > and hrvoje@.
> >
> > The summary of changes is as follows:
> >
> > - prevent infinite loop when packet hits NAT rule as follows:
> > pass out on em0 from 172.16.0.0/16 to any nat-to { 49/27 }
> > the issue has been introduced by my earlier commit [2]. The earlier
> > change makes pf(4) to interpret 49/27 as single IP address
> > (POOL_NONE)
> > this is wrong, because pool 49/27 actually contains 32 addresses.
> >
> > - while investigating the issue I've realized 'random' pool should
> > rather be using arc4_uniform() with upper limit derived from mask.
> > also the random number should be turned to netorder.
> >
> > - also while I was debugging my change I've noticed we should be using
> > pf_poolmask() to obtain address as a combination of pool address
> > and result of generator (round-robin all random).
> >
> > OK to commit?
> >
> > thanks and
> > regards
> > sashan
> >
> >
> > [1] https://marc.info/?t=16581336821=1=2
> > https://marc.info/?t=16573254651=1=2
> > https://marc.info/?l=openbsd-bugs=165817500514813=2
> >
> > [2] https://marc.info/?l=openbsd-cvs=164500117319660=2
>
>
> Hi all,
>
> I've tested this diff and from what I see NAT behaves as it should and
> it's changing ip addresses quite nicely
>
>
thank you Hrvoje for carrying independent test. I'll commit the diff
later today if there will be no objection.
thanks and
regards
sashan
ksh: PROMPT_COMMAND
Hi,
bash has a PROMPT_COMMAND that allows a command to be executed before
each PS1 prompt is displayed. I've found this useful on occasion, so
this is the same thing for ksh(1).
In particular, this allows PROMPT_COMMAND to be set to a user-defined
shell function that can modify PS1, though it could also produce its
own output directly.
Is anyone other than me interested in this? If so, review by someone
familiar with ksh's guts would be welcome.
-d
Index: ksh.1
===
RCS file: /cvs/src/bin/ksh/ksh.1,v
retrieving revision 1.216
diff -u -p -r1.216 ksh.1
--- ksh.1 31 Mar 2022 17:27:14 - 1.216
+++ ksh.1 2 Aug 2022 09:07:43 -
@@ -1520,6 +1520,15 @@ See
below.
.It Ev PPID
The process ID of the shell's parent (read-only).
+.It Ev PROMPT_COMMAND
+Used to specify a shell command that is executed before
+.Ev PS1
+is displayed.
+.Ev PROMPT_COMMAND
+may be a normal environment variable, in which case it is executed directly,
+or may be an array of commands that will be executed in order.
+Commands may be functions that modify or replace
+.Ev PS1 .
.It Ev PS1
The primary prompt for interactive shells.
Parameter, command, and arithmetic
Index: lex.c
===
RCS file: /cvs/src/bin/ksh/lex.c,v
retrieving revision 1.78
diff -u -p -r1.78 lex.c
--- lex.c 15 Jan 2018 14:58:05 - 1.78
+++ lex.c 2 Aug 2022 09:07:43 -
@@ -1173,6 +1173,37 @@ special_prompt_expand(char *str)
return str;
}
+static void
+prompt_command(char *str)
+{
+ struct op *t;
+ Source *s;
+
+ s = pushs(SSTRING, ATEMP);
+ s->start = s->str = str;
+ t = compile(s);
+ if (t == NULL || t->type == TEOF)
+ return;
+ exstat = execute(t, XERROK, NULL);
+}
+
+/* If $PROMPT_COMMAND is set, then try to execute it before expanding $PS1 */
+static void
+try_prompt_command(void)
+{
+ struct tbl *e;
+
+ e = global("PROMPT_COMMAND");
+ /* Handle regular variables and arrays */
+ for (; e; e = e->u.array) {
+ if (!(e->flag & ISSET))
+ continue;
+ prompt_command(str_val(e));
+ if (!(e->flag & ARRAY))
+ return;
+ }
+}
+
void
set_prompt(int to)
{
@@ -1183,6 +1214,7 @@ set_prompt(int to)
switch (to) {
case PS1: /* command */
+ try_prompt_command();
ps1 = str_save(str_val(global("PS1")), ATEMP);
saved_atemp = ATEMP;/* ps1 is freed by substitute() */
newenv(E_ERRH);
