Re: quiz(6): update european countries

[Otto Moerbeek]

On Mon, Jul 04, 2022 at 05:44:33PM -0400, Daniel Dickman wrote:

> 
> 
> On Sun, 3 Jul 2022, Daniel Dickman wrote:
> 
> > On Sat, Jul 2, 2022 at 9:26 PM Ben Fuller  wrote:
> > >
> > > I noticed Montenegro doesn't have an entry. Presumably this file hasn't
> > > been updated since before 2006!
> > 
> > These files could use a big overhaul and are very dated.
> > 
> > In the below, make sure to keep the file sorted.
> > 
> > In the Europe file there are other issues. For example I think
> > Macedonia is now North Macedonia, etc.
> > 
> > There are many issues in the other files. For example africa doesn't
> > have Eswatini, etc, etc.
> > 
> > Do you want to take a stab at doing a bigger update of the 4 quiz
> > files? africa, america, europe, asia?
> > 
> 
> I had some time on my hand and took a stab on this. A few notes:
> 
> Added multiple capitals for countries with more than one capital city that 
> were missing these:
> - Eswatini
> - South Africa
> - Malaysia
> - Sri Lanka
> 
> Removed duplicate entries:
> - Turkey: remove from europe, keep in asia
> - Georgia: remove from europe, keep in asia

Both are debatable. Certainly Instanbul is in Europe.
Many view Georgia as European.

> 
> For the above I followed this wikipedia classification:
> https://en.wikipedia.org/wiki/List_of_sovereign_states_and_dependent_territories_by_continent

Which list both Georgia and Turkey as European (and Turkey also as
Asian).

> africa:
> - removed Dahomey as the the pre-1975 name for Benin; it's been close to 
>   50 years since the rename.
> - updated Swaziland -> Eswatini. I left Swaziland since Eswatini rename 
>   happened in 2018. So going by the Benin example, seems like when we get 
>   closer to 50 years from the rename we can drop the old name
> - added "The" as an optional prefix for Gambia
> - removed "Republic of" from South Africa for consistency with other 
>   countries which don't generally include the formal prefix as part of the 
>   name.
> 
> americas:
> - added many missing countries
> - added "The" as an optional prefix for Bahamas
> - added missing suffix of "City" from many capitals like Guatemala, 
>   Mexico, and Panama
> 
> asia:
> - added many missing countries in Oceania (like New Zealand, Fiji, etc)
> - added UAE in addition to United Arab Emirates
> 
> europe:
> - added Montenegro
> - added Vatican City
> - renamed Macedonia to North Macedonia
> - Yugoslavia -> Serbia
> 
> I also looked at NetBSD a bit and noticed they added a lot of territories 
> and their capitals, but I decided not to add those in this update.
> 
> I also noticed NetBSD removed The Hague as a capital of The Netherlands, 
> but I disagree with that. Maybe some of the Dutch folks on this mailing 
> list can weigh in which is right.

The Dutch constitiution defines Amsterdam as the capital. The Hague
only is the seat of gorvernment.

-Otto


> 
> anyone want to ok this update?
> 
> Index: africa
> ===
> RCS file: /cvs/src/games/quiz/datfiles/africa,v
> retrieving revision 1.5
> diff -u -p -u -r1.5 africa
> --- africa2 Dec 2014 12:43:09 -   1.5
> +++ africa4 Jul 2022 21:34:34 -
> @@ -1,6 +1,6 @@
>  Algeria:Alg[iers|er]
>  Angola:Luanda
> -Benin|Dahomey:Porto[-| ]Novo
> +Benin:Porto[-| ]Novo
>  Botswana:Gaborone
>  Burkina Faso:Ouagadougou
>  Burundi:Bujumbura
> @@ -16,9 +16,10 @@ Djibouti:Djibouti City
>  Egypt:Cairo
>  Equatorial Guinea:Malabo
>  Eritrea:Asmara
> +Eswatini|Swaziland:Mbabane|Lobamba
>  Ethiopia:Addis Ababa
>  Gabon:Libreville
> -Gambia:Banjul
> +{The }Gambia:Banjul
>  Ghana:Accra
>  Guinea-Bissau:Bissau
>  Guinea:Conakry
> @@ -42,10 +43,9 @@ Senegal:Dakar
>  Seychelles:Victoria
>  Sierra Leone:Freetown
>  Somalia:Mogadishu
> -{Rep{ublic} of }South Africa:Pretoria
> +South Africa:Pretoria|Cape Town|Bloemfontein
>  South Sudan:Juba
>  Sudan:Khartoum
> -Swaziland:Mbabane
>  Tanzania:Dodoma
>  Togo:Lome
>  Tunisia:Tunis
> Index: america
> ===
> RCS file: /cvs/src/games/quiz/datfiles/america,v
> retrieving revision 1.2
> diff -u -p -u -r1.2 america
> --- america   2 Dec 2014 12:43:09 -   1.2
> +++ america   4 Jul 2022 21:34:34 -
> @@ -1,6 +1,8 @@
> +Antigua and Barbuda:St. John's
>  Argentina:Buenos Aires
> -Bahamas:Nassau
> +{The }Bahamas:Nassau
>  Barbados:Bridgetown
> +Belize:Belmopan
>  Bolivia:La Paz|Sucre
>  Bra[z|s]il:Brasilia
>  Canada:Ottawa
> @@ -8,19 +10,25 @@ Chile:Santiago
>  Colombia:Bogota
>  Costa Rica:San Jose
>  Cuba:Ha[v|b]ana
> +Dominica:Roseau
>  Dominican Republic:Santo Domingo
>  Ecuador:Quito
>  El Salvador:San Salvador
> -Guatemala:Guatemala
> +Greenland:Nuuk
> +Grenada:St. George's
> +Guatemala:Guatemala City
>  Guyana:Georgetown
> -Haiti:Port au Prince
> +Haiti:Port[-| ]au[-| ]Prince
>  Honduras:Tegucigalpa
>  Jamaica:Kingston
> -Mexico:Mexico
> +Mexico:Mexico City
>  Nicaragua:Managua
> -Panama:Panama
> +Panama:Panama 

Re: quiz(6): update european countries

[Daniel Dickman]

On Mon, Jul 4, 2022 at 8:09 PM Daniel Dickman  wrote:
>
>
>
> > On Jul 4, 2022, at 8:07 PM, Ben Fuller  wrote:
> >
> > On Mon, Jul 04, 2022 at 19:30:19 -0400, Daniel Dickman wrote:
>  Maldive Islands:Male
> >>> A more common name is just "{The }Maldives".
> >>
> >> I don't think so. See this article:
> >> https://www.bbc.com/news/magazine-18233844
> >>
> >> which states:
> >>
> >> "...according to several authoritative sources, such as the CIA World
> >> Factbook, the Times Comprehensive Atlas of the World and the US Department
> >> of State, only two countries, The Bahamas and The Gambia, should
> >> officially be referred to with the article."
> >
> > That may be so, but for the purposes of a quiz game, if I am asked "of
> > what country is Male the capital", I would answer The Maldives, and
> > would be annoyed if that was marked incorrect because only "Maldive
> > Islands" is accepted. Also, the official name is "Republic of Maldives"
> > or just "Maldives". The point is that this is a game, not a precise
> > database.
>
>
> No you would answer “the Maldives” because it is a group of island. Not “The 
> Maldives” (note the capital T) which is not the name of the country.
>
> I’m afraid I still don’t agree with you.

Rereading what you wrote, I agree the NetBSD approach is better than
our current approach of "Maldive Islands". So my local diff now has
the same as NetBSD:

Maldiv{e Island}s:Male

For the prefix "The" -- we also do not have "The United States of
America" or "The Sudan" or "The Philippines" and similar. So I don't
think the optional prefix "The" should be added except for the 2
official cases.

[v3] amd64: simplify TSC sync testing

[Scott Cheloha]

Hi,

Once again, I am trying to change our approach to TSC sync testing to
eliminate false positive results.  Instead of trying to repair the TSC
by measuring skew, we just spin in a lockless loop looking for skew
and mark the TSC as broken if we detect any.

This is motivated in part by some multisocket machines that do not use
the TSC as a timecounter because the current sync test confuses NUMA
latency for TSC skew.

The only difference between this version and the prior version (v2) is
that we check whether we have the IA32_TSC_ADJUST register by hand in
tsc_reset_adjust().  If someone wants to help me rearrange cpu_hatch()
so we do CPU identification before TSC sync testing we can remove the
workaround later.

If you have the IA32_TSC_ADJUST register and it is non-zero going into
the test, you will see something on the console like this:

tsc: cpu5: IA32_TSC_ADJUST: -150 -> 0

This does *not* mean you failed the test.  It just means you probably
have a bug in your BIOS (or some other firmware) and you should report
it to your vendor.

If you fail the test you will see something like this:

tsc: cpu0/cpu2: sync test round 1/2 failed
tsc: cpu0/cpu2: cpu2: 13043 lags 438 cycles

A printout like this would mean that the sync test for cpu2 failed.
In particular, cpu2's TSC trails cpu0's TSC by at least 438 cycles.
If this happens for *any* CPU we mark the TSC timecounter as
defective.

--

Please test!  Send your dmesg, pass or fail.

I am especially interested in:

1. A test from dv@.  Your dual-socket machine has the IA32_TSC_ADJUST
   register but it failed the test running patch v2.  Maybe it will pass
   with this version?

2. Other multisocket machines.

3. There were reports of TSC issues with OpenBSD VMs running on ESXi.
   What happens when you run with this patch?

4. OpenBSD VMs on other hypervisors.

5. Non-Lenovo machines, non-Intel machines.

-Scott

Index: amd64/tsc.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
retrieving revision 1.24
diff -u -p -r1.24 tsc.c
--- amd64/tsc.c 31 Aug 2021 15:11:54 -  1.24
+++ amd64/tsc.c 5 Jul 2022 01:52:10 -
@@ -36,13 +36,6 @@ int  tsc_recalibrate;
 uint64_t   tsc_frequency;
 inttsc_is_invariant;
 
-#defineTSC_DRIFT_MAX   250
-#define TSC_SKEW_MAX   100
-int64_ttsc_drift_observed;
-
-volatile int64_t   tsc_sync_val;
-volatile struct cpu_info   *tsc_sync_cpu;
-
 u_int  tsc_get_timecount(struct timecounter *tc);
 void   tsc_delay(int usecs);
 
@@ -236,22 +229,12 @@ cpu_recalibrate_tsc(struct timecounter *
 u_int
 tsc_get_timecount(struct timecounter *tc)
 {
-   return rdtsc_lfence() + curcpu()->ci_tsc_skew;
+   return rdtsc_lfence();
 }
 
 void
 tsc_timecounter_init(struct cpu_info *ci, uint64_t cpufreq)
 {
-#ifdef TSC_DEBUG
-   printf("%s: TSC skew=%lld observed drift=%lld\n", ci->ci_dev->dv_xname,
-   (long long)ci->ci_tsc_skew, (long long)tsc_drift_observed);
-#endif
-   if (ci->ci_tsc_skew < -TSC_SKEW_MAX || ci->ci_tsc_skew > TSC_SKEW_MAX) {
-   printf("%s: disabling user TSC (skew=%lld)\n",
-   ci->ci_dev->dv_xname, (long long)ci->ci_tsc_skew);
-   tsc_timecounter.tc_user = 0;
-   }
-
if (!(ci->ci_flags & CPUF_PRIMARY) ||
!(ci->ci_flags & CPUF_CONST_TSC) ||
!(ci->ci_flags & CPUF_INVAR_TSC))
@@ -268,111 +251,267 @@ tsc_timecounter_init(struct cpu_info *ci
calibrate_tsc_freq();
}
 
-   if (tsc_drift_observed > TSC_DRIFT_MAX) {
-   printf("ERROR: %lld cycle TSC drift observed\n",
-   (long long)tsc_drift_observed);
-   tsc_timecounter.tc_quality = -1000;
-   tsc_timecounter.tc_user = 0;
-   tsc_is_invariant = 0;
-   }
-
tc_init(_timecounter);
 }
 
-/*
- * Record drift (in clock cycles).  Called during AP startup.
- */
 void
-tsc_sync_drift(int64_t drift)
+tsc_delay(int usecs)
 {
-   if (drift < 0)
-   drift = -drift;
-   if (drift > tsc_drift_observed)
-   tsc_drift_observed = drift;
+   uint64_t interval, start;
+
+   interval = (uint64_t)usecs * tsc_frequency / 100;
+   start = rdtsc_lfence();
+   while (rdtsc_lfence() - start < interval)
+   CPU_BUSY_CYCLE();
 }
 
+#ifdef MULTIPROCESSOR
+
+#define TSC_DEBUG 1
+
 /*
- * Called during startup of APs, by the boot processor.  Interrupts
- * are disabled on entry.
+ * Protections for global variables in this code:
+ *
+ * a   Modified atomically
+ * b   Protected by a barrier
+ * p   Only modified by the primary CPU
  */
+
+#define TSC_TEST_MS1   /* Test round duration */
+#define TSC_TEST_ROUNDS2   /* Number of test rounds */
+
+struct tsc_test_status {
+   volatile uint64_t val 

Re: [patch] remove nexgen detection from i386

[Jonathan Gray]

On Mon, Jul 04, 2022 at 08:04:14PM -0400, Daniel Dickman wrote:
> The patch below removes the (small) amount of code used to identify NexGen 
> CPUs. I'm doubtful that these CPUs would be supported anymore and we don't 
> do anything with the information once we identify a NexGen CPU.
> 
> The code does a division early in the i386 boot process (in locore0) and 
> checks the result of the ZF flag. Apparently ZF would have a different 
> value on a NexGen vs an Intel CPU.
> 
> According to my research some NexGen CPUs were:
>   Nx586-P66
>   Nx586-P75
>   Nx586-P80
>   Nx586-P90
>   Nx586-P100
>   Nx586-P110
>   Nx586-P120
>   Nx586-P133
> 
> And:
>   Nx586-PF100
>   Nx586-PF110
> 
> The models with a "P" did not include an FPU, while the models with "PF" 
> did include an integrated FPU.
> 
> According to this site, there were originally plans for a separate Nx587 
> FPU but it was never officially released:
> 
> https://www.x86-guide.net/en/fpu/NexGen-Nx587-2.html
> 
> On our i386 page we state that we support:
> 
> "All CPUs compatible with the Intel Pentium or later, with 
> Intel-compatible hardware floating point support should work."
> 
> Therefore none of the FPU-less NexGen CPUs are ones we state we support.
> 
> Looking at what we implement for our recognition code, we only implement 
> half of the algorithm to identify NexGen cpus that don't have the cpuid 
> instruction. We ignore NexGen cpus that support the cpuid instruction 
> (which would return the string "NexGenDriven"). So the ones lacking cpuid 
> support probably don't have an FPU and aren't supported. While the ones 
> with the FPU are likely ones that support the cpuid instruction, but our 
> detection code doesn't detect them.
> 
> So it looks like neither case would be supported in the current code.
> 
> One thing I was not able to find out is the precise details of which of 
> these CPUs do support the CPUID instruction and which ones don't. I've 
> looked for these CPUs on ebay for the past few years but they seem to be 
> fairly impossible to find with the last ones probably sitting in the hands 
> of collectors. So the above is just a guess on my part.
> 
> Finally I keep reading that the Nx586 is not actually Pentium clone, but 
> rather an i386 clone. I don't have a good way to test this, but I've read 
> that instructions like CMPXCHG8B (pentium), INVLPG (486), XADD (486) might 
> not be directly supported by NexGen cpus. There is discussion of this 
> here:
> 
> https://www.cpu-world.com/forum/viewtopic.php?p=189409
> 
> "...the truth is that the Nx586 is an extremely fast 386 processor. Later 
> they added some 486 instructions via hypercode, which is stored in the 
> BIOS EEPROM, but only the application-oriented ones. So while you can run 
> most of the applications that were compiled for an 486, you can't run an 
> operating system which requires a 486 or even a Pentium."
> 
> Below diff tested on my transmeta notebook running i386 which continues to 
> run fine and doesn't change the transmeta detection code.
> 
> In the diff I left the ".Lis386" label even though it isn't needed 
> anymore, but I think it's a useful comment to keep.
> 
> ok?

CPU_* and CPUVENDOR_* aren't renumbered.  I can't find use
where they are used as an index to an array so should be fine.

ok jsg@

> 
> 
> Index: i386/locore0.S
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/locore0.S,v
> retrieving revision 1.5
> diff -u -p -u -r1.5 locore0.S
> --- i386/locore0.S31 Dec 2021 10:44:05 -  1.5
> +++ i386/locore0.S3 Jul 2022 21:06:50 -
> @@ -132,25 +132,6 @@ start:   movw$0x1234,0x472   # warm 
> boot
>   testl   %eax,%eax
>   jnz .Ltry486
>  
> - /*
> -  * Try the test of a NexGen CPU -- ZF will not change on a DIV
> -  * instruction on a NexGen, it will on an i386.  Documented in
> -  * Nx586 Processor Recognition Application Note, NexGen, Inc.
> -  */
> - movl$0x,%eax
> - xorl%edx,%edx
> - movl$2,%ecx
> - divl%ecx
> - jnz .Lis386
> -
> -.Lisnx586:
> - /*
> -  * Don't try cpuid, as Nx586s reportedly don't support the
> -  * PSL_ID bit.
> -  */
> - movl$CPU_NX586,RELOC(_C_LABEL(cpu))
> - jmp 2f
> -
>  .Lis386:
>   movl$CPU_386,RELOC(_C_LABEL(cpu))
>   jmp 2f
> Index: i386/machdep.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
> retrieving revision 1.648
> diff -u -p -u -r1.648 machdep.c
> --- i386/machdep.c1 Feb 2022 20:29:55 -   1.648
> +++ i386/machdep.c3 Jul 2022 21:06:51 -
> @@ -510,8 +510,6 @@ const struct cpu_nocpuid_nameclass i386_
>   NULL},  /* CPU_486DLC */
>   { CPUVENDOR_CYRIX, "Cyrix", "6x86", CPUCLASS_486,
>   cyrix6x86_cpu_setup},   /* CPU_6x86 */
> -   

Re: quiz(6): update european countries

[Daniel Dickman]

> On Jul 4, 2022, at 8:07 PM, Ben Fuller  wrote:
> 
> On Mon, Jul 04, 2022 at 19:30:19 -0400, Daniel Dickman wrote:
 Maldive Islands:Male
>>> A more common name is just "{The }Maldives".
>> 
>> I don't think so. See this article: 
>> https://www.bbc.com/news/magazine-18233844
>> 
>> which states:
>> 
>> "...according to several authoritative sources, such as the CIA World 
>> Factbook, the Times Comprehensive Atlas of the World and the US Department 
>> of State, only two countries, The Bahamas and The Gambia, should 
>> officially be referred to with the article."
> 
> That may be so, but for the purposes of a quiz game, if I am asked "of
> what country is Male the capital", I would answer The Maldives, and
> would be annoyed if that was marked incorrect because only "Maldive
> Islands" is accepted. Also, the official name is "Republic of Maldives"
> or just "Maldives". The point is that this is a game, not a precise
> database.


No you would answer “the Maldives” because it is a group of island. Not “The 
Maldives” (note the capital T) which is not the name of the country.

I’m afraid I still don’t agree with you.

[patch] remove nexgen detection from i386

[Daniel Dickman]

The patch below removes the (small) amount of code used to identify NexGen 
CPUs. I'm doubtful that these CPUs would be supported anymore and we don't 
do anything with the information once we identify a NexGen CPU.

The code does a division early in the i386 boot process (in locore0) and 
checks the result of the ZF flag. Apparently ZF would have a different 
value on a NexGen vs an Intel CPU.

According to my research some NexGen CPUs were:
  Nx586-P66
  Nx586-P75
  Nx586-P80
  Nx586-P90
  Nx586-P100
  Nx586-P110
  Nx586-P120
  Nx586-P133

And:
  Nx586-PF100
  Nx586-PF110

The models with a "P" did not include an FPU, while the models with "PF" 
did include an integrated FPU.

According to this site, there were originally plans for a separate Nx587 
FPU but it was never officially released:

https://www.x86-guide.net/en/fpu/NexGen-Nx587-2.html

On our i386 page we state that we support:

"All CPUs compatible with the Intel Pentium or later, with 
Intel-compatible hardware floating point support should work."

Therefore none of the FPU-less NexGen CPUs are ones we state we support.

Looking at what we implement for our recognition code, we only implement 
half of the algorithm to identify NexGen cpus that don't have the cpuid 
instruction. We ignore NexGen cpus that support the cpuid instruction 
(which would return the string "NexGenDriven"). So the ones lacking cpuid 
support probably don't have an FPU and aren't supported. While the ones 
with the FPU are likely ones that support the cpuid instruction, but our 
detection code doesn't detect them.

So it looks like neither case would be supported in the current code.

One thing I was not able to find out is the precise details of which of 
these CPUs do support the CPUID instruction and which ones don't. I've 
looked for these CPUs on ebay for the past few years but they seem to be 
fairly impossible to find with the last ones probably sitting in the hands 
of collectors. So the above is just a guess on my part.

Finally I keep reading that the Nx586 is not actually Pentium clone, but 
rather an i386 clone. I don't have a good way to test this, but I've read 
that instructions like CMPXCHG8B (pentium), INVLPG (486), XADD (486) might 
not be directly supported by NexGen cpus. There is discussion of this 
here:

https://www.cpu-world.com/forum/viewtopic.php?p=189409

"...the truth is that the Nx586 is an extremely fast 386 processor. Later 
they added some 486 instructions via hypercode, which is stored in the 
BIOS EEPROM, but only the application-oriented ones. So while you can run 
most of the applications that were compiled for an 486, you can't run an 
operating system which requires a 486 or even a Pentium."

Below diff tested on my transmeta notebook running i386 which continues to 
run fine and doesn't change the transmeta detection code.

In the diff I left the ".Lis386" label even though it isn't needed 
anymore, but I think it's a useful comment to keep.

ok?


Index: i386/locore0.S
===
RCS file: /cvs/src/sys/arch/i386/i386/locore0.S,v
retrieving revision 1.5
diff -u -p -u -r1.5 locore0.S
--- i386/locore0.S  31 Dec 2021 10:44:05 -  1.5
+++ i386/locore0.S  3 Jul 2022 21:06:50 -
@@ -132,25 +132,6 @@ start: movw$0x1234,0x472   # warm 
boot
testl   %eax,%eax
jnz .Ltry486
 
-   /*
-* Try the test of a NexGen CPU -- ZF will not change on a DIV
-* instruction on a NexGen, it will on an i386.  Documented in
-* Nx586 Processor Recognition Application Note, NexGen, Inc.
-*/
-   movl$0x,%eax
-   xorl%edx,%edx
-   movl$2,%ecx
-   divl%ecx
-   jnz .Lis386
-
-.Lisnx586:
-   /*
-* Don't try cpuid, as Nx586s reportedly don't support the
-* PSL_ID bit.
-*/
-   movl$CPU_NX586,RELOC(_C_LABEL(cpu))
-   jmp 2f
-
 .Lis386:
movl$CPU_386,RELOC(_C_LABEL(cpu))
jmp 2f
Index: i386/machdep.c
===
RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.648
diff -u -p -u -r1.648 machdep.c
--- i386/machdep.c  1 Feb 2022 20:29:55 -   1.648
+++ i386/machdep.c  3 Jul 2022 21:06:51 -
@@ -510,8 +510,6 @@ const struct cpu_nocpuid_nameclass i386_
NULL},  /* CPU_486DLC */
{ CPUVENDOR_CYRIX, "Cyrix", "6x86", CPUCLASS_486,
cyrix6x86_cpu_setup},   /* CPU_6x86 */
-   { CPUVENDOR_NEXGEN,"NexGen","586",  CPUCLASS_386,
-   NULL},  /* CPU_NX586 */
 };
 
 const char *classnames[] = {
Index: include/cputypes.h
===
RCS file: /cvs/src/sys/arch/i386/include/cputypes.h,v
retrieving revision 1.10
diff -u -p -u -r1.10 cputypes.h
--- 

Re: quiz(6): update european countries

[Daniel Dickman]

On Mon, 4 Jul 2022, Ben Fuller wrote:

> On Mon, Jul 04, 2022 at 17:44:33 -0400, Daniel Dickman wrote:
> > I had some time on my hand and took a stab on this. A few notes:
> 
> Ach, you beat me to sending a patch. I made almost exactly the same changes as
> you have. I've noted some of my thoughts below.
> 
> > I also looked at NetBSD a bit and noticed they added a lot of territories 
> > and their capitals, but I decided not to add those in this update.
> 
> I wonder what their opinion on Kosovo was.

I've left out disputed areas in this update.

> 
> >  Maldive Islands:Male
> 
> A more common name is just "{The }Maldives".

I don't think so. See this article: 
https://www.bbc.com/news/magazine-18233844

which states:

"...according to several authoritative sources, such as the CIA World 
Factbook, the Times Comprehensive Atlas of the World and the US Department 
of State, only two countries, The Bahamas and The Gambia, should 
officially be referred to with the article."


> 
> >  Netherlands|Holland:The Hague|'sGravenhage|den Haag|Amsterdam
> 
> To the best of my knowledge, Amsterdam is the formal capital (perhaps it
> should come first in the alternatives) but for the purposes of a quiz
> game I think The Hague is also an acceptable answer.

Researching this more, I feel like I'm coming into agreement with the 
NetBSD change after all about removing The Hague.

> 
> >  R[u|o]mania:Bucharest|Bucuresti
> 
> Perhaps switch the order of [u|o] here so that Romania is printed (the
> more common spelling, I think)

I think it's right to make it Romania. Rumania is common in non-English 
languages and my internet research leads me to believe that Romania is the 
official anglicization preferred by the Romanian government (at least 
according to their consulate web pages that I visited).

> 
> >  Ukraine:Kiev|Kyiv
> There's been some commotion about preferring the Ukrainian spelling Kyiv to 
> the
> Russian Kiev.
> 

Maybe let's leave for a separate commit as I want to steer clear of 
contentious issues.

I do note this reference which I've read through:
https://en.wikipedia.org/wiki/KyivNotKiev

And again, I see the spelling as Kyiv on the English section of a .ua govt 
site I visited.

Re: quiz(6): update european countries

[Joerg Sonnenberger]

On Mon, Jul 04, 2022 at 05:44:33PM -0400, Daniel Dickman wrote:
> I also noticed NetBSD removed The Hague as a capital of The Netherlands, 
> but I disagree with that. Maybe some of the Dutch folks on this mailing 
> list can weigh in which is right.

Amsterdam is the capital, The Hague is the seat of government. Similar
to what Germany had for a while after the reunion with Bonn as seat of
government and Berlin as capital. There are a couple of other examples
like that.

Joerg

Re: ts(1): make timespec-handling code more obvious

[Scott Cheloha]

On Mon, Jul 04, 2022 at 11:15:24PM +0200, Claudio Jeker wrote:
> On Mon, Jul 04, 2022 at 01:28:12PM -0500, Scott Cheloha wrote:
> > Hi,
> > 
> > Couple things:
> > 
> > [...]
> 
> I don't like the introduction of all these local variables that are just
> hard to follow and need extra code pathes. Happy to rename roff to offset,
> start_offset or something similar. Also moving the localtime call into
> fmtfmt() is fine.

You need an "elapsed" variable to avoid overwriting "now" in the
-i flag case to avoid calling clock_gettime(2) twice.

We can get rid of "utc_start" and just reuse "now" for the initial
value of CLOCK_REALTIME.

How is this?

Index: ts.c
===
RCS file: /cvs/src/usr.bin/ts/ts.c,v
retrieving revision 1.6
diff -u -p -r1.6 ts.c
--- ts.c4 Jul 2022 17:29:03 -   1.6
+++ ts.c4 Jul 2022 22:06:56 -
@@ -32,7 +32,7 @@ static char   *buf;
 static char*outbuf;
 static size_t   bufsize;
 
-static void fmtfmt(struct tm *, long);
+static void fmtfmt(const struct timespec *);
 static void __dead  usage(void);
 
 int
@@ -40,8 +40,7 @@ main(int argc, char *argv[])
 {
int iflag, mflag, sflag;
int ch, prev;
-   struct timespec roff, start, now;
-   struct tm *tm;
+   struct timespec elapsed, now, start, utc_offset;
clockid_t clock = CLOCK_REALTIME;
 
if (pledge("stdio", NULL) == -1)
@@ -93,22 +92,25 @@ main(int argc, char *argv[])
if (setenv("TZ", "UTC", 1) == -1)
err(1, "setenv UTC");
 
-   clock_gettime(CLOCK_REALTIME, );
clock_gettime(clock, );
-   timespecsub(, , );
+   if (mflag) {
+   clock_gettime(CLOCK_REALTIME, );
+   timespecsub(, , _offset);
+   }
 
for (prev = '\n'; (ch = getchar()) != EOF; prev = ch) {
if (prev == '\n') {
clock_gettime(clock, );
-   if (iflag || sflag)
-   timespecsub(, , );
-   else if (mflag)
-   timespecadd(, , );
-   if (iflag)
-   clock_gettime(clock, );
-   if ((tm = localtime(_sec)) == NULL)
-   err(1, "localtime");
-   fmtfmt(tm, now.tv_nsec);
+   if (iflag || sflag) {
+   timespecsub(, , );
+   if (iflag)
+   start = now;
+   fmtfmt();
+   } else {
+   if (mflag)
+   timespecadd(, _offset, );
+   fmtfmt();
+   }
}
if (putchar(ch) == EOF)
break;
@@ -132,11 +134,15 @@ usage(void)
  * so you can format while you format
  */
 static void
-fmtfmt(struct tm *tm, long tv_nsec)
+fmtfmt(const struct timespec *ts)
 {
+   struct tm *tm;
char *f, ms[7];
 
-   snprintf(ms, sizeof(ms), "%06ld", tv_nsec / 1000);
+   if ((tm = localtime(>tv_sec)) == NULL)
+   err(1, "localtime");
+
+   snprintf(ms, sizeof(ms), "%06ld", ts->tv_nsec / 1000);
strlcpy(buf, format, bufsize);
f = buf;
 

Re: quiz(6): update european countries

[Daniel Dickman]

On Sun, 3 Jul 2022, Daniel Dickman wrote:

> On Sat, Jul 2, 2022 at 9:26 PM Ben Fuller  wrote:
> >
> > I noticed Montenegro doesn't have an entry. Presumably this file hasn't
> > been updated since before 2006!
> 
> These files could use a big overhaul and are very dated.
> 
> In the below, make sure to keep the file sorted.
> 
> In the Europe file there are other issues. For example I think
> Macedonia is now North Macedonia, etc.
> 
> There are many issues in the other files. For example africa doesn't
> have Eswatini, etc, etc.
> 
> Do you want to take a stab at doing a bigger update of the 4 quiz
> files? africa, america, europe, asia?
> 

I had some time on my hand and took a stab on this. A few notes:

Added multiple capitals for countries with more than one capital city that 
were missing these:
- Eswatini
- South Africa
- Malaysia
- Sri Lanka

Removed duplicate entries:
- Turkey: remove from europe, keep in asia
- Georgia: remove from europe, keep in asia

For the above I followed this wikipedia classification:
https://en.wikipedia.org/wiki/List_of_sovereign_states_and_dependent_territories_by_continent

africa:
- removed Dahomey as the the pre-1975 name for Benin; it's been close to 
  50 years since the rename.
- updated Swaziland -> Eswatini. I left Swaziland since Eswatini rename 
  happened in 2018. So going by the Benin example, seems like when we get 
  closer to 50 years from the rename we can drop the old name
- added "The" as an optional prefix for Gambia
- removed "Republic of" from South Africa for consistency with other 
  countries which don't generally include the formal prefix as part of the 
  name.

americas:
- added many missing countries
- added "The" as an optional prefix for Bahamas
- added missing suffix of "City" from many capitals like Guatemala, 
  Mexico, and Panama

asia:
- added many missing countries in Oceania (like New Zealand, Fiji, etc)
- added UAE in addition to United Arab Emirates

europe:
- added Montenegro
- added Vatican City
- renamed Macedonia to North Macedonia
- Yugoslavia -> Serbia

I also looked at NetBSD a bit and noticed they added a lot of territories 
and their capitals, but I decided not to add those in this update.

I also noticed NetBSD removed The Hague as a capital of The Netherlands, 
but I disagree with that. Maybe some of the Dutch folks on this mailing 
list can weigh in which is right.

anyone want to ok this update?

Index: africa
===
RCS file: /cvs/src/games/quiz/datfiles/africa,v
retrieving revision 1.5
diff -u -p -u -r1.5 africa
--- africa  2 Dec 2014 12:43:09 -   1.5
+++ africa  4 Jul 2022 21:34:34 -
@@ -1,6 +1,6 @@
 Algeria:Alg[iers|er]
 Angola:Luanda
-Benin|Dahomey:Porto[-| ]Novo
+Benin:Porto[-| ]Novo
 Botswana:Gaborone
 Burkina Faso:Ouagadougou
 Burundi:Bujumbura
@@ -16,9 +16,10 @@ Djibouti:Djibouti City
 Egypt:Cairo
 Equatorial Guinea:Malabo
 Eritrea:Asmara
+Eswatini|Swaziland:Mbabane|Lobamba
 Ethiopia:Addis Ababa
 Gabon:Libreville
-Gambia:Banjul
+{The }Gambia:Banjul
 Ghana:Accra
 Guinea-Bissau:Bissau
 Guinea:Conakry
@@ -42,10 +43,9 @@ Senegal:Dakar
 Seychelles:Victoria
 Sierra Leone:Freetown
 Somalia:Mogadishu
-{Rep{ublic} of }South Africa:Pretoria
+South Africa:Pretoria|Cape Town|Bloemfontein
 South Sudan:Juba
 Sudan:Khartoum
-Swaziland:Mbabane
 Tanzania:Dodoma
 Togo:Lome
 Tunisia:Tunis
Index: america
===
RCS file: /cvs/src/games/quiz/datfiles/america,v
retrieving revision 1.2
diff -u -p -u -r1.2 america
--- america 2 Dec 2014 12:43:09 -   1.2
+++ america 4 Jul 2022 21:34:34 -
@@ -1,6 +1,8 @@
+Antigua and Barbuda:St. John's
 Argentina:Buenos Aires
-Bahamas:Nassau
+{The }Bahamas:Nassau
 Barbados:Bridgetown
+Belize:Belmopan
 Bolivia:La Paz|Sucre
 Bra[z|s]il:Brasilia
 Canada:Ottawa
@@ -8,19 +10,25 @@ Chile:Santiago
 Colombia:Bogota
 Costa Rica:San Jose
 Cuba:Ha[v|b]ana
+Dominica:Roseau
 Dominican Republic:Santo Domingo
 Ecuador:Quito
 El Salvador:San Salvador
-Guatemala:Guatemala
+Greenland:Nuuk
+Grenada:St. George's
+Guatemala:Guatemala City
 Guyana:Georgetown
-Haiti:Port au Prince
+Haiti:Port[-| ]au[-| ]Prince
 Honduras:Tegucigalpa
 Jamaica:Kingston
-Mexico:Mexico
+Mexico:Mexico City
 Nicaragua:Managua
-Panama:Panama
+Panama:Panama City
 Paraguay:Asuncion
 Peru:Lima
+Saint Lucia:Castries
+Saint Vincent and the Grenadines:Kingstown
+Suriname:Paramaribo
 Trinidad{ and Tobago}:Port of Spain
 United States|US{A}:Washington
 Uruguay:Montevideo
Index: asia
===
RCS file: /cvs/src/games/quiz/datfiles/asia,v
retrieving revision 1.5
diff -u -p -u -r1.5 asia
--- asia24 Nov 2014 06:31:50 -  1.5
+++ asia4 Jul 2022 21:34:34 -
@@ -10,6 +10,7 @@ Cambodia:P{h}nom Penh
 China:Beijing|Peking
 Cyprus:Nicosia
 East Timor:Dili
+Fiji:Suva
 Georgia:Tbilisi
 India:New Delhi
 

Re: ts(1): make timespec-handling code more obvious

[Claudio Jeker]

On Mon, Jul 04, 2022 at 01:28:12PM -0500, Scott Cheloha wrote:
> Hi,
> 
> Couple things:
> 
> - Use additional timespec variables to make our intent more obvious.
> 
>   Add "elapsed", "utc_offset", and "utc_start".
> 
>   "roff" is a confusing name, "utc_offset" is better.
> 
>   Yes, I know the clock is called CLOCK_REALTIME, but that's a
>   historical accident.  Ideally they would have called it CLOCK_UTC
>   or CLOCK_UNIX.  Sigh.
> 
> - Before the loop, we only need to compute utc_offset in the -m flag
>   case.
> 
> - Before the loop, we only need to do two clock_gettime(2) calls in
>   the -m flag case.
> 
> - In the loop, we can use the new variables to help clarify what we're
>   doing:
> 
>   + In the -i and -s flag cases we're using an elapsed time for the
> timestamp, so compute "elapsed".
> 
>   + In the default and -m cases we're using an absolute time for the
> timestamp, so (if necessary) compute "now".
> 
> - We don't need to call clock_gettime(2) twice in the -i flag case.
>   Just compute "elapsed" and then assign "now" to "start".  Easy.
> 
> - I think the pimp my ride joke is cute, but calling the function
>   "print_timestamp()" is a bit more obvious.  It also tells the
>   reader that there's a side effect.
> 
> - Because we always call localtime(3), we can move that call into
>   print_timestamp() and just pass the timespec as argument.
> 
>   This makes it clearer where the nanosecond value is coming from,
>   which in turn makes it clearer that the string "ms" will be exactly
>   6 bytes in length.
> 
>   I think "ms" stands for "microseconds", in which case a better name
>   is "us" or "usecs", but that's outside the scope of this patch.
> 
> --
> 
> ok?
> 
> Index: ts.c
> ===
> RCS file: /cvs/src/usr.bin/ts/ts.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 ts.c
> --- ts.c  4 Jul 2022 17:29:03 -   1.6
> +++ ts.c  4 Jul 2022 18:17:20 -
> @@ -32,7 +32,7 @@ static char *buf;
>  static char  *outbuf;
>  static size_t bufsize;
>  
> -static void   fmtfmt(struct tm *, long);
> +static void   print_timestamp(const struct timespec *);
>  static void __deadusage(void);
>  
>  int
> @@ -40,8 +40,7 @@ main(int argc, char *argv[])
>  {
>   int iflag, mflag, sflag;
>   int ch, prev;
> - struct timespec roff, start, now;
> - struct tm *tm;
> + struct timespec elapsed, now, start, utc_offset, utc_start;
>   clockid_t clock = CLOCK_REALTIME;
>  
>   if (pledge("stdio", NULL) == -1)
> @@ -93,22 +92,28 @@ main(int argc, char *argv[])
>   if (setenv("TZ", "UTC", 1) == -1)
>   err(1, "setenv UTC");
>  
> - clock_gettime(CLOCK_REALTIME, );
> - clock_gettime(clock, );
> - timespecsub(, , );
> + if (clock != CLOCK_REALTIME) {
> + clock_gettime(clock, );
> + if (mflag) {
> + clock_gettime(CLOCK_REALTIME, _start);
> + timespecsub(_start, , _offset);
> + }
> + } else
> + clock_gettime(CLOCK_REALTIME, );
>  
>   for (prev = '\n'; (ch = getchar()) != EOF; prev = ch) {
>   if (prev == '\n') {
>   clock_gettime(clock, );
> - if (iflag || sflag)
> - timespecsub(, , );
> - else if (mflag)
> - timespecadd(, , );
> - if (iflag)
> - clock_gettime(clock, );
> - if ((tm = localtime(_sec)) == NULL)
> - err(1, "localtime");
> - fmtfmt(tm, now.tv_nsec);
> + if (iflag || sflag) {
> + timespecsub(, , );
> + if (iflag)
> + start = now;
> + print_timestamp();
> + } else {
> + if (mflag)
> + timespecadd(, _offset, );
> + print_timestamp();
> + }
>   }
>   if (putchar(ch) == EOF)
>   break;
> @@ -132,11 +137,15 @@ usage(void)
>   * so you can format while you format
>   */
>  static void
> -fmtfmt(struct tm *tm, long tv_nsec)
> +print_timestamp(const struct timespec *ts)
>  {
> + struct tm *tm;
>   char *f, ms[7];
>  
> - snprintf(ms, sizeof(ms), "%06ld", tv_nsec / 1000);
> + if ((tm = localtime(>tv_sec)) == NULL)
> + err(1, "localtime");
> +
> + snprintf(ms, sizeof(ms), "%06ld", ts->tv_nsec / 1000);
>   strlcpy(buf, format, bufsize);
>   f = buf;
>  
> 

I don't like the introduction of all these local variables that are just
hard to follow and need extra code pathes. Happy to rename roff to offset,
start_offset or 

arm64 pwmbl(4): simplify ramp case

[Miod Vallat]

When the fdt does not provide a list of brightness states, pwmbl(4)
builds a 256 state ramp (i.e. state[i] = i with 0 <= i < 256).

The following diff keeps that behaviour, but gets rid of the malloc
call for that ramp, since the values are trivially known.

Compiles but not tested due to the lack of such hardware.

Index: sys/dev/fdt/pwmbl.c
===
RCS file: /OpenBSD/src/sys/dev/fdt/pwmbl.c,v
retrieving revision 1.6
diff -u -p -r1.6 pwmbl.c
--- sys/dev/fdt/pwmbl.c 24 Oct 2021 17:52:26 -  1.6
+++ sys/dev/fdt/pwmbl.c 4 Jul 2022 18:45:16 -
@@ -35,7 +35,7 @@ struct pwmbl_softc {
struct device   sc_dev;
uint32_t*sc_pwm;
int sc_pwm_len;
-   uint32_t*sc_levels;
+   uint32_t*sc_levels; /* NULL if simple ramp */
int sc_nlevels;
uint32_tsc_max_level;
uint32_tsc_def_level;
@@ -73,7 +73,7 @@ pwmbl_attach(struct device *parent, stru
struct pwmbl_softc *sc = (struct pwmbl_softc *)self;
struct fdt_attach_args *faa = aux;
uint32_t *gpios;
-   int i, len;
+   int len;
 
len = OF_getproplen(faa->fa_node, "pwms");
if (len < 0) {
@@ -95,7 +95,7 @@ pwmbl_attach(struct device *parent, stru
}
 
len = OF_getproplen(faa->fa_node, "brightness-levels");
-   if (len > 0) {
+   if (len >= sizeof(uint32_t)) {
sc->sc_levels = malloc(len, M_DEVBUF, M_WAITOK);
OF_getpropintarray(faa->fa_node, "brightness-levels",
sc->sc_levels, len);
@@ -107,13 +107,9 @@ pwmbl_attach(struct device *parent, stru
sc->sc_def_level = sc->sc_nlevels - 1;
sc->sc_def_level = sc->sc_levels[sc->sc_def_level];
} else {
+   /* No levels, assume a simple 0..255 ramp. */
sc->sc_nlevels = 256;
-   sc->sc_levels = mallocarray(sc->sc_nlevels,
-   sizeof(uint32_t), M_DEVBUF, M_WAITOK);
-   for (i = 0; i < sc->sc_nlevels; i++)
-   sc->sc_levels[i] = i;
-   sc->sc_max_level = sc->sc_levels[sc->sc_nlevels - 1];
-   sc->sc_def_level = sc->sc_levels[sc->sc_nlevels - 1];
+   sc->sc_max_level = sc->sc_def_level = sc->sc_nlevels - 1;
}
 
printf("\n");
@@ -144,17 +140,22 @@ pwmbl_find_brightness(struct pwmbl_softc
uint32_t mid;
int i;
 
-   for (i = 0; i < sc->sc_nlevels - 1; i++) {
-   mid = (sc->sc_levels[i] + sc->sc_levels[i + 1]) / 2;
-   if (sc->sc_levels[i] <= level && level <= mid)
+   if (sc->sc_levels) {
+   for (i = 0; i < sc->sc_nlevels - 1; i++) {
+   mid = (sc->sc_levels[i] + sc->sc_levels[i + 1]) / 2;
+   if (sc->sc_levels[i] <= level && level <= mid)
+   return sc->sc_levels[i];
+   if (mid < level && level <= sc->sc_levels[i + 1])
+   return sc->sc_levels[i + 1];
+   }
+   if (level < sc->sc_levels[0])
+   return sc->sc_levels[0];
+   else
return sc->sc_levels[i];
-   if (mid < level && level <= sc->sc_levels[i + 1])
-   return sc->sc_levels[i + 1];
+
+   } else {
+   return level < sc->sc_nlevels ? level : sc->sc_nlevels - 1;
}
-   if (level < sc->sc_levels[0])
-   return sc->sc_levels[0];
-   else
-   return sc->sc_levels[i];
 }
 
 int

ts(1): make timespec-handling code more obvious

[Scott Cheloha]

Hi,

Couple things:

- Use additional timespec variables to make our intent more obvious.

  Add "elapsed", "utc_offset", and "utc_start".

  "roff" is a confusing name, "utc_offset" is better.

  Yes, I know the clock is called CLOCK_REALTIME, but that's a
  historical accident.  Ideally they would have called it CLOCK_UTC
  or CLOCK_UNIX.  Sigh.

- Before the loop, we only need to compute utc_offset in the -m flag
  case.

- Before the loop, we only need to do two clock_gettime(2) calls in
  the -m flag case.

- In the loop, we can use the new variables to help clarify what we're
  doing:

  + In the -i and -s flag cases we're using an elapsed time for the
timestamp, so compute "elapsed".

  + In the default and -m cases we're using an absolute time for the
timestamp, so (if necessary) compute "now".

- We don't need to call clock_gettime(2) twice in the -i flag case.
  Just compute "elapsed" and then assign "now" to "start".  Easy.

- I think the pimp my ride joke is cute, but calling the function
  "print_timestamp()" is a bit more obvious.  It also tells the
  reader that there's a side effect.

- Because we always call localtime(3), we can move that call into
  print_timestamp() and just pass the timespec as argument.

  This makes it clearer where the nanosecond value is coming from,
  which in turn makes it clearer that the string "ms" will be exactly
  6 bytes in length.

  I think "ms" stands for "microseconds", in which case a better name
  is "us" or "usecs", but that's outside the scope of this patch.

--

ok?

Index: ts.c
===
RCS file: /cvs/src/usr.bin/ts/ts.c,v
retrieving revision 1.6
diff -u -p -r1.6 ts.c
--- ts.c4 Jul 2022 17:29:03 -   1.6
+++ ts.c4 Jul 2022 18:17:20 -
@@ -32,7 +32,7 @@ static char   *buf;
 static char*outbuf;
 static size_t   bufsize;
 
-static void fmtfmt(struct tm *, long);
+static void print_timestamp(const struct timespec *);
 static void __dead  usage(void);
 
 int
@@ -40,8 +40,7 @@ main(int argc, char *argv[])
 {
int iflag, mflag, sflag;
int ch, prev;
-   struct timespec roff, start, now;
-   struct tm *tm;
+   struct timespec elapsed, now, start, utc_offset, utc_start;
clockid_t clock = CLOCK_REALTIME;
 
if (pledge("stdio", NULL) == -1)
@@ -93,22 +92,28 @@ main(int argc, char *argv[])
if (setenv("TZ", "UTC", 1) == -1)
err(1, "setenv UTC");
 
-   clock_gettime(CLOCK_REALTIME, );
-   clock_gettime(clock, );
-   timespecsub(, , );
+   if (clock != CLOCK_REALTIME) {
+   clock_gettime(clock, );
+   if (mflag) {
+   clock_gettime(CLOCK_REALTIME, _start);
+   timespecsub(_start, , _offset);
+   }
+   } else
+   clock_gettime(CLOCK_REALTIME, );
 
for (prev = '\n'; (ch = getchar()) != EOF; prev = ch) {
if (prev == '\n') {
clock_gettime(clock, );
-   if (iflag || sflag)
-   timespecsub(, , );
-   else if (mflag)
-   timespecadd(, , );
-   if (iflag)
-   clock_gettime(clock, );
-   if ((tm = localtime(_sec)) == NULL)
-   err(1, "localtime");
-   fmtfmt(tm, now.tv_nsec);
+   if (iflag || sflag) {
+   timespecsub(, , );
+   if (iflag)
+   start = now;
+   print_timestamp();
+   } else {
+   if (mflag)
+   timespecadd(, _offset, );
+   print_timestamp();
+   }
}
if (putchar(ch) == EOF)
break;
@@ -132,11 +137,15 @@ usage(void)
  * so you can format while you format
  */
 static void
-fmtfmt(struct tm *tm, long tv_nsec)
+print_timestamp(const struct timespec *ts)
 {
+   struct tm *tm;
char *f, ms[7];
 
-   snprintf(ms, sizeof(ms), "%06ld", tv_nsec / 1000);
+   if ((tm = localtime(>tv_sec)) == NULL)
+   err(1, "localtime");
+
+   snprintf(ms, sizeof(ms), "%06ld", ts->tv_nsec / 1000);
strlcpy(buf, format, bufsize);
f = buf;
 

Re: netstart.8: interfaces or bridges

[Jason McIntyre]

On Mon, Jul 04, 2022 at 05:22:51PM +, Klemens Nanni wrote:
> There's not just bridge(4) but veb(4) also, but netstart(8) does not treat
> either of them specially, so stick to just "bridges" without Xr.
> 
> Swap one mention for consistency with the rest.
> 
> Feedback? OK?
> 

i agree this makes sense. what i'd hoped was that anyone running "man -k
bridge" would pull up both those drivers. although it does, it kind of
gets lost in the entries for various hardware bridges.

anyway, i wonder if we could sharpen the Nd entries here:

bridge(4) - Ethernet bridge interface
veb, vport(4) - Virtual Ethernet Bridge network device

veb(4) should really not have capitals for "Virtual" or "Bridge", but i
wonder if we can choose one of "interface" or "network device". e.g.

bridge(4) - Ethernet bridge interface
veb, vport(4) - virtual Ethernet bridge interface

would that make sense?

anyway, apologies from distracting from your diff. it's ok by me.

jmc

> Index: netstart.8
> ===
> RCS file: /cvs/src/share/man/man8/netstart.8,v
> retrieving revision 1.26
> diff -u -p -r1.26 netstart.8
> --- netstart.821 Jun 2022 11:55:55 -  1.26
> +++ netstart.84 Jul 2022 17:20:36 -
> @@ -43,7 +43,7 @@ it performs network initialization.
>  .Pp
>  The
>  .Nm
> -script can also be used to start newly created bridges or interfaces.
> +script can also be used to start newly created interfaces or bridges.
>  The behaviour of this script is (or can be) controlled to some
>  extent by variables defined in
>  .Xr rc.conf 8 ,
> @@ -88,9 +88,7 @@ and
>  .El
>  .Pp
>  After the system is completely initialized, it is possible to start a
> -newly created interface or
> -.Xr bridge 4 ,
> -or apply the configuration from a
> +newly created interface or bridges or apply the configuration from a
>  .Xr hostname.if 5
>  file to an existing interface, by invoking the following, where
>  .Ar foo0
> 

netstart: tweak ifmstart() usage comment

[Klemens Nanni]

Make it look like a proper usage.

365:ifmstart "" "aggr trunk svlan vlan carp pppoe tun tap gif etherip gre egre 
nvgre eoip vxlan pflow wg"
370:ifmstart "aggr trunk svlan vlan carp pppoe"
397:ifmstart "tun tap gif etherip gre egre nvgre eoip vxlan pflow wg"

Feedback? OK?

Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.219
diff -u -p -r1.219 netstart
--- netstart3 Jul 2022 12:14:36 -   1.219
+++ netstart4 Jul 2022 17:38:30 -
@@ -176,7 +176,7 @@ ifstart() {
 }
 
 # Start multiple interfaces by driver name.
-# Usage: ifmstart "em iwm" "trunk vlan"
+# Usage: ifmstart "driver ..." ["driver ..."]
 #   Start "$1" interfaces in order or all interfaces if empty.
 #   Don't start "$2" interfaces. "$2" is optional.
 ifmstart() {

netstart.8: interfaces or bridges

[Klemens Nanni]

There's not just bridge(4) but veb(4) also, but netstart(8) does not treat
either of them specially, so stick to just "bridges" without Xr.

Swap one mention for consistency with the rest.

Feedback? OK?

Index: netstart.8
===
RCS file: /cvs/src/share/man/man8/netstart.8,v
retrieving revision 1.26
diff -u -p -r1.26 netstart.8
--- netstart.8  21 Jun 2022 11:55:55 -  1.26
+++ netstart.8  4 Jul 2022 17:20:36 -
@@ -43,7 +43,7 @@ it performs network initialization.
 .Pp
 The
 .Nm
-script can also be used to start newly created bridges or interfaces.
+script can also be used to start newly created interfaces or bridges.
 The behaviour of this script is (or can be) controlled to some
 extent by variables defined in
 .Xr rc.conf 8 ,
@@ -88,9 +88,7 @@ and
 .El
 .Pp
 After the system is completely initialized, it is possible to start a
-newly created interface or
-.Xr bridge 4 ,
-or apply the configuration from a
+newly created interface or bridges or apply the configuration from a
 .Xr hostname.if 5
 file to an existing interface, by invoking the following, where
 .Ar foo0

Re: Bug in iked

[Sibar Soumi]

Hi

Thanks for your reply.

Yes, sure. This fix would also be correct.

Best Regards

Sibar Soumi
Software Developer

achelos GmbH | Vattmannstraße 1 | 33100 Paderborn | GERMANY 
sibar.so...@achelos.de | www.achelos.de | www.iot.achelos.com | Follow us: 
LinkedIn | XING  | YouTube
 

-Ursprüngliche Nachricht-
Von: Tobias Heider  
Gesendet: Sonntag, 3. Juli 2022 14:30
An: Sibar Soumi 
Cc: tech@openbsd.org; Claudia Priesterjahn 
Betreff: Re: Bug in iked

On Wed, Jun 22, 2022 at 01:02:17PM +, Sibar Soumi wrote:
> Dear OpenBSD developers
> 
>  
> 
> I would like to report an error in iked.
> 
>  
> 
> The error occurs with the processing logic in case of simultaneous Child SA 
> rekeying. That is, by simultaneous rekeying, two Child SAs are created and 
> “the SA created with the lowest of the four nonces used in the two exchanges 
> SHOULD be closed by the endpoint that created it” (RFC7296 section 2.8.1).
> 
>  
> 
> This decision is made in the iked implementation in ikev2.c in the if block 
> from L4390 
> 
>   until L4407 
> 
>  .
> 
>  
> 
> But nr is not set to the minimum nonce for exchange initiated by peer but by 
> us, and ni which comes from sa->sa_simulat is already set to the minimum 
> nonce for exchange initiated by peer.
> 
>  
> 
> Therefore, the comment in line 4393 shall be corrected and the comparison in 
> line 4402 shall be “ikev2_nonce_cmp(nr, ni) < 0” instead of 
> “ikev2_nonce_cmp(ni, nr) < 0” because the SA that has just been created by us 
> shall be deleted, if nr 
>  
> 
> Best regards
> 
>  
> 
>  
> 
> Sibar Soumi
> 
> Software Developer
> 
>  
> 
> achelos GmbH | Vattmannstraße 1 | 33100 Paderborn | GERMANY
> 
> sibar.so...@achelos.de   | www.achelos.de 
>   | www.iot.achelos.com 
>   | Follow us: LinkedIn 
>   | XING 
>    | YouTube 
>  
> 
>  
> 
> Die achelos GmbH ist nach ISO 9001 und ISO 27001 zertifiziert. | achelos GmbH 
> is certified according to ISO 9001 and ISO 27001.
> 
> Geschäftsführung | Executive Board: Kathrin Asmuth, Thomas Freitag
> 
> Registergericht | register court: Paderborn, HRB 8817 | USt-IdNr. | 
> VAT ID number: DE260414872
> 
>  
> 
> Diese Mitteilung ist vertraulich. Wenn Sie nicht der beabsichtigte 
> Empfänger sind, ist jegliche Verwendung, Beeinträchtigung,
> 
> Offenlegung oder Vervielfältigung dieses Materials unautorisiert und 
> verboten. Bitte informieren Sie uns umgehend und
> 
> vernichten Sie die E-Mail. | This communication is confidential. If 
> you are not the intended recipient, any use, interference with,
> 
> disclosure or copying of this material is unauthorised and prohibited. Please 
> inform us immediately and destroy the email.
> 

Hi Sibar,

thanks for the report!
It looks like you are right, the current comparison is indeed wrong.
I think the best fix would be to switch ni and nr, which I think was the 
original intention here. ni should be the nonce for the exchange where we are 
initiator, nr is where we are responder.
RFC7296 says that when our nonce < peer nonce we delete our simultaneously 
proposed child SA, so this should fix the comparsion below at

4402 if (ikev2_nonce_cmp(ni, nr) < 0) {
4403 ret = ikev2_childsa_delete_proposed(env, sa,
4404 >sa_proposals);

Below is my proposed fix.

ok?

Index: ikev2.c
===
RCS file: /cvs/src/sbin/iked/ikev2.c,v
retrieving revision 1.347
diff -u -p -r1.347 ikev2.c
--- ikev2.c 28 May 2022 18:51:16 -  1.347
+++ ikev2.c 3 Jul 2022 12:20:33 -
@@ -4387,14 +4387,14 @@ ikev2_init_create_child_sa(struct iked *
sa->sa_rnonce = msg->msg_nonce;
msg->msg_nonce = NULL;
 
-   if (csa && (ni = sa->sa_simult) != NULL) {
+   if (csa && (nr = sa->sa_simult) != NULL) {
log_info("%s: resolving simultaneous CHILD SA rekeying",
SPI_SA(sa, __func__));
-   /* set nr to minimum nonce for exchange initiated by peer */
+   /* set ni to minimum nonce for exchange initiated by us */
if (ikev2_nonce_cmp(sa->sa_inonce, sa->sa_rnonce) < 0)
-   nr = sa->sa_inonce;
+   ni = sa->sa_inonce;
else
-   nr = sa->sa_rnonce;
+   ni = sa->sa_rnonce;
/*
 * If the exchange initated by us has smaller nonce,
 * then we have to delete our SAs.


openpgp-digital-signature.asc
Description: