Re: acpiec on acer aspire S7 with CURRENT

[Philip Guenther]

On Fri, 14 Oct 2016, Ilya Kaliman wrote:
> dmesg:
> OpenBSD 6.0 (GENERIC.MP) #1: Thu Sep  8 16:32:34 PDT 2016
> i...@puffy.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
..
> acpiec0 at acpi0: Failed to read resource settings
...
> acpidump:

I've taken a brief look at this.  The definition of the resources for the 
embedded controller is odd:

Scope (_SB.PCI0.LPCB)
{
Device (EC0)
{
Name (_HID, EisaId ("PNP0C09") /* Embedded Controller Device */)  
// _HID: Hardware ID
...
Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
{
IO (Decode16,
0x0062, // Range Minimum
0x0062, // Range Maximum
0x00,   // Alignment
0x01,   // Length
)
IO (Decode16,
0x0066, // Range Minimum
0x0066, // Range Maximum
0x00,   // Alignment
0x01,   // Length
)
IO (Decode16,
0x0068, // Range Minimum
0x0068, // Range Maximum
0x01,   // Alignment
0x08,   // Length
)
Memory32Fixed (ReadWrite,
0xFE800E00, // Address Base
0x0001, // Address Length
)
})


Note that there are *four* resources...but even revision 6.1 of the ACPI 
standard only gives meaning for the first *three*, and the third resource 
is the GPIO for the SCI interrupt...but is only for hardware-reduced ACPI 
platforms, which this machine is not:
...
 Use APIC Physical Destination Mode (V4) : 0
   Hardware Reduced (V5) : 0
  Low Power S0 Idle (V5) : 0
...

So I guess there's two defects in acpicec.c right now:
 - we don't use a third resource for the SCI interrupt when hw-reduced
 - we barf on extra resources despite vendors deciding to throwing them 
   in for the hell of it


Perhaps acpiec should use aml_parse_resource() instead of the hand-crafted 
parsing it does right now?  That would solve the second problem, at 
least.

Until we have a better fix, simply deleting the end-of-buffer check as 
you've done should have the same end effect, but expect a diff to try out 
at some point here...


Philip Guenther

landisk: simply asid lookup

[Philip Guenther]

It used to be that access to a process's vmspace (and thus its pmap where 
the ASID can be found) was only via a thread's p_vmspace member.  Now that 
the process itself has a copy of that, the loops in the sh code that 
iterated across threads to find a valid pointer can be eliminated.

ok?


Philip Guenther

Index: arch/sh/sh/pmap.c
===
RCS file: /data/src/openbsd/src/sys/arch/sh/sh/pmap.c,v
retrieving revision 1.26
diff -u -p -r1.26 pmap.c
--- arch/sh/sh/pmap.c   15 Sep 2016 02:00:17 -  1.26
+++ arch/sh/sh/pmap.c   13 Oct 2016 04:44:41 -
@@ -1067,7 +1067,6 @@ int
 __pmap_asid_alloc(void)
 {
struct process *pr;
-   struct proc *p;
int i, j, k, n, map, asid;
 
/* Search free ASID */
@@ -1092,14 +1091,9 @@ __pmap_asid_alloc(void)
 * too many processes.
 */
LIST_FOREACH(pr, , ps_list) {
-   /* find a thread that still has the process vmspace attached */
-   TAILQ_FOREACH(p, >ps_threads, p_thr_link)
-   if (p->p_vmspace != NULL)
-   break;
-   if (p == NULL)
-   continue;
-   if ((asid = p->p_vmspace->vm_map.pmap->pm_asid) > 0) {
-   pmap_t pmap = p->p_vmspace->vm_map.pmap;
+   pmap_t pmap = pr->ps_vmspace->vm_map.pmap;
+
+   if ((asid = pmap->pm_asid) > 0) {
pmap->pm_asid = -1;
__pmap_asid.hint = asid;
/* Invalidate all old ASID entry */
Index: arch/sh/sh/db_interface.c
===
RCS file: /data/src/openbsd/src/sys/arch/sh/sh/db_interface.c,v
retrieving revision 1.8
diff -u -p -r1.8 db_interface.c
--- arch/sh/sh/db_interface.c   18 May 2016 20:21:13 -  1.8
+++ arch/sh/sh/db_interface.c   13 Oct 2016 00:36:31 -
@@ -368,15 +368,10 @@ __db_procname_by_asid(int asid)
 {
static char notfound[] = "---";
struct process *pr;
-   struct proc *p;
 
LIST_FOREACH(pr, , ps_list) {
-   /* find a thread that still has the process vmspace attached */
-   TAILQ_FOREACH(p, >ps_threads, p_thr_link)
-   if (p->p_vmspace != NULL)
-   break;
-   if (p != NULL && p->p_vmspace->vm_map.pmap->pm_asid == asid)
+   if (pr->ps_vmspace->vm_map.pmap->pm_asid == asid)
return (p->p_comm);
}
 
return (notfound);

Re: opencvs - fix update -r and -A

[Frederic Cambus]

On Fri, Oct 14, 2016 at 09:24:03AM -0600, Todd C. Miller wrote:
> On Fri, 14 Oct 2016 16:17:45 +0200, Joris Vink wrote:
> 
> > In certain cases update -r and update -A would not properly set or reset
> > the sticky tag for file(s). This patch fixes that problem.
> 
> After reading through the applicable parts of update.c I'm fairly
> certain this is correct.  OK millert@

Commited, thanks!

Re: acpiec on acer aspire S7 with CURRENT

[Ilya Kaliman]

dmesg:
OpenBSD 6.0 (GENERIC.MP) #1: Thu Sep  8 16:32:34 PDT 2016
i...@puffy.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
RTC BIOS diagnostic error 80
real mem = 8468033536 (8075MB)
avail mem = 8206921728 (7826MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xe6a80 (27 entries)
bios0: vendor Insyde Corp. version "V2.12" date 05/20/2014
bios0: Acer Aspire S7-392
acpi0 at bios0: rev 2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP TCPA UEFI FPDT MSDM ASF! HPET APIC MCFG SSDT
BOOT ASPT DBGP SSDT SSDT SSDT SSDT SSDT DMAR
acpi0: wakeup devices P0P1(S4) GLAN(S4) EHC1(S3) EHC2(S3) XHC_(S3)
HDEF(S4) TPD4(S4) PXSX(S4) RP01(S4) PXSX(S4) RP02(S4) PXSX(S4)
RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i5-4210U CPU @ 1.70GHz, 1596.68 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM) i5-4210U CPU @ 1.70GHz, 1596.31 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: Intel(R) Core(TM) i5-4210U CPU @ 1.70GHz, 1596.31 MHz
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 1, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i5-4210U CPU @ 1.70GHz, 1596.31 MHz
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 1, core 1, package 0
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 40 pins
acpimcfg0 at acpi0 addr 0xe000, bus 0-255
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (P0P1)
acpiprt2 at acpi0: bus -1 (RP01)
acpiprt3 at acpi0: bus -1 (RP02)
acpiprt4 at acpi0: bus 1 (RP03)
acpiprt5 at acpi0: bus -1 (RP04)
acpiprt6 at acpi0: bus -1 (RP05)
acpiprt7 at acpi0: bus -1 (RP06)
acpiprt8 at acpi0: bus -1 (RP07)
acpiprt9 at acpi0: bus -1 (RP08)
acpiprt10 at acpi0: bus -1 (PEG0)
acpiprt11 at acpi0: bus -1 (PEG1)
acpiprt12 at acpi0: bus -1 (PEG2)
acpiec0 at acpi0: Failed to read resource settings
acpicpu0 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33),
C1(1000@1 mwait.1), PSS
acpicpu1 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33),
C1(1000@1 mwait.1), PSS
acpicpu2 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33),
C1(1000@1 mwait.1), PSS
acpicpu3 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33),
C1(1000@1 mwait.1), PSS
acpitz0 at acpi0: critical temperature is 99 degC
acpitz1 at acpi0: critical temperature is 98 degC
"ACPI0008" at acpi0 not configured
acpiac0 at acpi0: AC unit online
acpibat0 at acpi0: BAT0 model "AP13F3N" serial  2358 type LION oem "4f594e4153"
acpibtn0 at acpi0: PWRB
"10250759" at acpi0 not configured
"SYN1B78" at acpi0 not configured
"PNP0C14" at acpi0 not configured
dwiic0 at acpi0: I2C1 addr 0xfe105000/0x1000 irq 7
iic0 at dwiic0
"BCM2E4E" at acpi0 not configured
acpibtn1 at acpi0: LID0
acpi0: WARNING EC not initialized
acpi0: WARNING EC not initialized
acpibtn2 at acpi0: SLPB
"PNP0C14" at acpi0 not configured
"INT340E" at acpi0 not configured
"INT33A0" at acpi0 not configured
"PNP0C31" at acpi0 not configured
acpivideo0 at acpi0: GFX0
acpivout0 at acpivideo0: DD1F
cpu0: Enhanced SpeedStep 1596 MHz: speeds: 2401, 2400, 2300, 2200,
2000, 1900, 1700, 

Re: use x2apic if it is enabled by BIOS

[Mike Larkin]

On Fri, Oct 14, 2016 at 04:49:31PM +0900, YASUOKA Masahiko wrote:
> Hi,
> 
> I'm working on NEC Express5800/R110h-1 (dmesg is attached).  On this
> machine, our kernel panics with following message.
> 
>   cpu0 at mainbus0panic: cpu at apic id 0 already attached?
> 
> This seems to happen since x2APIC on the machine is enabled by BIOS
> and the kernel doesn't assume that.  The diff makes the kernel use
> x2APIC if it is enabled by BIOS.
> 
> ok?
> 

This should go in snaps, or wait for reports from tech@ with test results
before it should be committed, IMO. We don't have full support for x2apic,
and blindly enabling it like this hoping that the bios set everything up
right is bound to be a bad assumption on at least one machine out there.

-ml

> Index: sys/arch/amd64/amd64/lapic.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 lapic.c
> --- sys/arch/amd64/amd64/lapic.c  22 Jun 2016 01:12:38 -  1.44
> +++ sys/arch/amd64/amd64/lapic.c  14 Oct 2016 07:45:50 -
> @@ -170,60 +170,55 @@ lapic_map(paddr_t lapic_base)
>   int s;
>   pt_entry_t *pte;
>   vaddr_t va;
> + u_int64_t msr;
>  
> - /*
> -  * On real hardware, x2apic must only be enabled if interrupt remapping
> -  * is also enabled. See 10.12.7 of the SDM vol 3.
> -  * On hypervisors, this is not necessary. Hypervisors can implement
> -  * x2apic support even if the host CPU does not support it.
> -  * Until we support interrupt remapping, use x2apic only if the
> -  * hypervisor flag is also set.
> -  */
> - if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_HV)) {
> - u_int64_t msr;
> -
> - disable_intr();
> - s = lapic_tpr;
> -
> - msr = rdmsr(MSR_APICBASE);
> - msr |= APICBASE_ENABLE_X2APIC;
> - wrmsr(MSR_APICBASE, msr);
> + disable_intr();
> + s = lapic_tpr;
> +
> + msr = rdmsr(MSR_APICBASE);
>  
> + if (ISSET(msr, APICBASE_ENABLE_X2APIC) ||
> + (ISSET(cpu_ecxfeature, CPUIDECX_HV) &&
> + ISSET(cpu_ecxfeature, CPUIDECX_X2APIC))) {
> + /*
> +  * If x2APIC is enabled already, use it since it is enabled
> +  * by BIOS.  On hypervisor, use it if it exists.
> +  */
> + if (!ISSET(msr, APICBASE_ENABLE_X2APIC)) {
> + msr |= APICBASE_ENABLE_X2APIC;
> + wrmsr(MSR_APICBASE, msr);
> + }
>   lapic_readreg = x2apic_readreg;
>   lapic_writereg = x2apic_writereg;
>  #ifdef MULTIPROCESSOR
>   x86_ipi = x2apic_ipi;
>  #endif
>   x2apic_enabled = 1;
> -
>   codepatch_call(CPTAG_EOI, _eoi);
>  
>   lapic_writereg(LAPIC_TPRI, s);
> - enable_intr();
> + } else {
> + /*
> +  * Map local apic.  If we have a local apic, it's safe to
> +  * assume we're on a 486 or better and can use invlpg and
> +  * non-cacheable PTE's
> +  *
> +  * Whap the PTE "by hand" rather than calling pmap_kenter_pa
> +  * because the latter will attempt to invoke TLB shootdown
> +  * code just as we might have changed the value of
> +  * cpu_number()..
> +  */
> + va = (vaddr_t)_apic;
> + pte = kvtopte(va);
> + *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
> + invlpg(va);
>  
> - return;
> + lapic_tpr = s;
>   }
>  
> - va = (vaddr_t)_apic;
> -
> - disable_intr();
> - s = lapic_tpr;
> -
> - /*
> -  * Map local apic.  If we have a local apic, it's safe to assume
> -  * we're on a 486 or better and can use invlpg and non-cacheable PTE's
> -  *
> -  * Whap the PTE "by hand" rather than calling pmap_kenter_pa because
> -  * the latter will attempt to invoke TLB shootdown code just as we
> -  * might have changed the value of cpu_number()..
> -  */
> -
> - pte = kvtopte(va);
> - *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
> - invlpg(va);
> -
> - lapic_tpr = s;
>   enable_intr();
> +
> + return;
>  }
>  
>  /*
> 
> OpenBSD 6.0-current (GENERIC.MP) #44: Fri Oct 14 16:32:03 JST 2016
> 
> yasu...@yasuoka-ob1.tokyo.iiji.jp:/source/yasuoka/openbsd/head/git/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 8207699968 (7827MB)
> avail mem = 7954391040 (7585MB)
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f92b000 (60 entries)
> bios0: vendor American Megatrends Inc. version "5.0.4012" date 06/15/2016
> bios0: NEC Express5800/R110h-1 [N8100-2316Y]
> acpi0 at bios0: rev 2
> acpi0: sleep states S0 S4 S5
> acpi0: tables DSDT FACP APIC FIDT MCFG HPET SSDT SSDT SSDT SSDT SPCR DMAR 

Re: ksh vi mode: fix region deletion and yanking

[Theo Buehler]

> Most of the word movement functions required complete rewrites,
> but the new versions aren't harder to read than the old ones.
> 
> I have done some testing, but a bit more might be in order.
> 
> OK?

The patch reads fine to me and behaves well in some testing.
It is ok tb@ as is.

I wonder if for the sake of consistency it would be worth merging the
'b' and 'B' cases as well as the 'w' and 'W' cases the same way as you
did merge the 'e' and 'E' cases below. The simplification is minimal,
but it would read slightly better, I think.

> @@ -1180,19 +1181,13 @@ domove(int argcnt, const char *cmd, int 
>   break;
>  
>   case 'e':
> - if (!sub && es->cursor + 1 >= es->linelen)
> - return -1;
> - ncursor = endword(argcnt);
> - if (sub && ncursor < es->linelen)
> - ncursor++;
> - break;
> -
>   case 'E':
>   if (!sub && es->cursor + 1 >= es->linelen)
>   return -1;
> - ncursor = Endword(argcnt);
> - if (sub && ncursor < es->linelen)
> - ncursor++;
> + ncursor = (*cmd == 'e' ? endword : Endword)(argcnt);
> + if (!sub)
> + while (isu8cont((unsigned char)es->cbuf[--ncursor]))
> + continue;

Re: ksh vi mode: fix region deletion and yanking

[Ingo Schwarze]

Hi Dmitrij,

Dmitrij D. Czarkoff wrote on Fri, Oct 14, 2016 at 01:11:16PM +0200:

> I've noticed that in ksh's vi mode ranged operations are performed
> without respect to cursor's position within utf8 byte sequence.  Eg.:
> 
>  1. type "echo тест | hexdump -C"
>  2. leave inseart mode
>  3. "0", "2E", "dh", Enter
>  4. you end up with "те" and 0x82
> (last byte of letter under cursor).
> 
> This happens because Endword() moves cursor to the whitespace after word
> and decrements cursor position by 1, so that it points to last byte of
> last letter.

That is exactly what i meant when saying "Some polishing may be
useful in tree, in particular adding utf8cont() support to a few
missing commands."  Thank you for paying attention to remaining
issues of this kind, for reporting them, and for writing patches.

> Then del_range() removes bytes between cursor position and
> preceding utf8 start byte, which include start byte of letter under
> cursor.
> 
> My diff makes del_range() (and yank_range() which operates in the same
> manner) always skip to the beginning of utf8 sequence it is in.
> Although this is a more of a bandaid -

I'm not thrilled about the prospect to always have to sanitize the
cursor position before being able to use it.  I expect there will
be very many places in the code needing such sanition if we allow
the cursor to go just anywhere.

> proper fix would be to make sure that cursor never rests
> on continuation byte -

I very strongly agree with that.  That's what the commands h, i, l,
r, and x already do, and what emacs mode does as well.  I think
we should do the same for the remaining commands.  Keeping the
cursor at sane places, we won't need to mess with code only using
the cursor.

> it is less invasive and
> does not hurt code readability too much.

In the long run, i don't think changing all places *using* the
cursor would be less invasive than changing all places *moving*
the cursor.

The case you found here is moving by words: b, B, e, E, w, W,
and we should probably handle | at the same time.

So i prefer the patch below, which has the following chunks:

 * 708, vi_cmd:
   When a movement command moves to the end of the line,
   move back to the last character, not to the last byte.
 * 1180, domove:
   When the movement command after a change, delete, or yank
   command moves forward to the end of a word,
   move on to the next character, not to the next byte.
 * 1266, domove:
   When a move to column command moves to the middle of a character,
   back up to the beginning of that character.
   Note that this is still moving to byte positions, not to
   display columns, but the latter isn't feasible without
   mbtowc(3) and wcwidth(3).
 * 1526, forwword:
   Move forward by characters, not bytes.
 * 1526, backword:
   Move backward by characters, not bytes.
 * 1526, endword:
   Move forward by characters, not bytes.
   Also return the position *after* the word, not the last byte,
   because that is easier to handle in the caller.
 * 1635, Endword:
   Return the position *after* the word, not the last byte,
   because that is easier to handle in the caller.

Most of the word movement functions required complete rewrites,
but the new versions aren't harder to read than the old ones.

I have done some testing, but a bit more might be in order.

OK?
  Ingo


Index: vi.c
===
RCS file: /cvs/src/bin/ksh/vi.c,v
retrieving revision 1.40
diff -u -p -r1.40 vi.c
--- vi.c11 Oct 2016 19:52:54 -  1.40
+++ vi.c14 Oct 2016 17:49:41 -
@@ -708,7 +708,8 @@ vi_cmd(int argcnt, const char *cmd)
if (is_move(*cmd)) {
if ((cur = domove(argcnt, cmd, 0)) >= 0) {
if (cur == es->linelen && cur != 0)
-   cur--;
+   while (isu8cont(es->cbuf[--cur]))
+   continue;
es->cursor = cur;
} else
return -1;
@@ -1180,19 +1181,13 @@ domove(int argcnt, const char *cmd, int 
break;
 
case 'e':
-   if (!sub && es->cursor + 1 >= es->linelen)
-   return -1;
-   ncursor = endword(argcnt);
-   if (sub && ncursor < es->linelen)
-   ncursor++;
-   break;
-
case 'E':
if (!sub && es->cursor + 1 >= es->linelen)
return -1;
-   ncursor = Endword(argcnt);
-   if (sub && ncursor < es->linelen)
-   ncursor++;
+   ncursor = (*cmd == 'e' ? endword : Endword)(argcnt);
+   if (!sub)
+   while (isu8cont((unsigned char)es->cbuf[--ncursor]))
+   continue;
break;
 
case 'f':
@@ -1266,6 +1261,8 @@ domove(int argcnt, const char *cmd, int 
   

Re: snmpd(8): teach how to fork+exec

[Rafael Zalamena]

On Mon, Sep 26, 2016 at 03:45:59PM +0200, Rafael Zalamena wrote:
> Lets teach snmpd(8) how to fork+exec using the proc.c file from the latest
> switchd(8) diff.
> 
> Note 1: I just tested the basic operations: startup and teardown.
> Note 2: the kill with close will be implemented in another diff with the
> ps_pid removal.
> 

I've update the diff with the latest proc.c changes.

Note: I still have to implement kill with close().

ok?

Index: proc.c
===
RCS file: /home/obsdcvs/src/usr.sbin/snmpd/proc.c,v
retrieving revision 1.20
diff -u -p -r1.20 proc.c
--- proc.c  7 Dec 2015 16:05:56 -   1.20
+++ proc.c  14 Oct 2016 15:42:19 -
@@ -1,7 +1,7 @@
 /* $OpenBSD: proc.c,v 1.20 2015/12/07 16:05:56 reyk Exp $  */
 
 /*
- * Copyright (c) 2010 - 2014 Reyk Floeter 
+ * Copyright (c) 2010 - 2016 Reyk Floeter 
  * Copyright (c) 2008 Pierre-Yves Ritschard 
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -22,6 +22,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -34,8 +35,12 @@
 
 #include "snmpd.h"
 
-voidproc_open(struct privsep *, struct privsep_proc *,
-   struct privsep_proc *, size_t);
+voidproc_exec(struct privsep *, struct privsep_proc *, unsigned int,
+   int, char **);
+voidproc_setup(struct privsep *, struct privsep_proc *, unsigned int);
+voidproc_open(struct privsep *, int, int);
+voidproc_accept(struct privsep *, int, enum privsep_procid,
+   unsigned int);
 voidproc_close(struct privsep *);
 int proc_ispeer(struct privsep_proc *, unsigned int, enum privsep_procid);
 voidproc_shutdown(struct privsep_proc *);
@@ -55,204 +60,383 @@ proc_ispeer(struct privsep_proc *procs, 
return (0);
 }
 
-void
-proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc)
+enum privsep_procid
+proc_getid(struct privsep_proc *procs, unsigned int nproc,
+const char *proc_name)
 {
-   unsigned int i, j, src, dst;
-   struct privsep_pipes*pp;
+   struct privsep_proc *p;
+   unsigned int proc;
 
-   /*
-* Allocate pipes for all process instances (incl. parent)
-*
-* - ps->ps_pipes: N:M mapping
-* N source processes connected to M destination processes:
-* [src][instances][dst][instances], for example
-* [PROC_RELAY][3][PROC_CA][3]
-*
-* - ps->ps_pp: per-process 1:M part of ps->ps_pipes
-* Each process instance has a destination array of socketpair fds:
-* [dst][instances], for example
-* [PROC_PARENT][0]
-*/
-   for (src = 0; src < PROC_MAX; src++) {
-   /* Allocate destination array for each process */
-   if ((ps->ps_pipes[src] = calloc(ps->ps_ninstances,
-   sizeof(struct privsep_pipes))) == NULL)
-   fatal("proc_init: calloc");
+   for (proc = 0; proc < nproc; proc++) {
+   p = [proc];
+   if (strcmp(p->p_title, proc_name))
+   continue;
 
-   for (i = 0; i < ps->ps_ninstances; i++) {
-   pp = >ps_pipes[src][i];
+   return (p->p_id);
+   }
 
-   for (dst = 0; dst < PROC_MAX; dst++) {
-   /* Allocate maximum fd integers */
-   if ((pp->pp_pipes[dst] =
-   calloc(ps->ps_ninstances,
-   sizeof(int))) == NULL)
-   fatal("proc_init: calloc");
+   return (PROC_MAX);
+}
 
-   /* Mark fd as unused */
-   for (j = 0; j < ps->ps_ninstances; j++)
-   pp->pp_pipes[dst][j] = -1;
-   }
-   }
-   }
+void
+proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
+int argc, char **argv)
+{
+   unsigned int proc, nargc, i, proc_i;
+   char**nargv;
+   struct privsep_proc *p;
+   char num[32];
+   int  fd;
+
+   /* Prepare the new process argv. */
+   nargv = calloc(argc + 5, sizeof(char *));
+   if (nargv == NULL)
+   fatal("%s: calloc", __func__);
+
+   /* Copy call argument first. */
+   nargc = 0;
+   nargv[nargc++] = argv[0];
+
+   /* Set process name argument and save the position. */
+   nargv[nargc++] = "-P";
+   proc_i = nargc;
+   nargc++;
+
+   /* Point process instance arg to stack and copy the original args. */
+   nargv[nargc++] = "-I";
+   nargv[nargc++] = num;
+   for (i = 1; i < (unsigned int) argc; i++)
+   nargv[nargc++] = argv[i];
 
- 

Re: malloc junk bytes

[Ted Unangst]

Otto Moerbeek wrote:
> Hi,
> 
> 0xdb is better dan 0xd0, since it is unaligned in more cases (think
> about the bytes being used as a pointer.

ok

Re: bgpd draft-ietf-idr-large-community

[Sebastian Benoit]

forgot to look at bgpctl in my first reply, sry

Peter Hessler(phess...@openbsd.org) on 2016.10.13 17:34:28 +0200:
> On 2016 Oct 11 (Tue) at 00:00:53 +0200 (+0200), Peter Hessler wrote:
> :Here is an initial implementation of draft-ietf-idr-large-community for
> :OpenBGPD.  I can connect and exchange routes with these attributes
> :against exabgp.
> :
> :Normal communities are two 16bit numbers.  With the addition of
> :32bit ASNs, those will not work if you wish to control one of
> :them.
> :
> :Large Communities are 32bit:32bit:32bit.  It seems the convention will be
> :::, with  and  being locally
> :defined.
> :
> :RFC status: currently accepted by the IDR-WG, is at version -02, the
> :wire format is set, the attribute codepoint is assigned by IANA, and it
> :seems that only trivial details need to be addressed.  Very likely to be
> :accepted.
> :
> :This was based on a partial implementation from Job Snijders, many
> :thanks!
> :
> :Comments?  OK?
> :
> 
> Updated diff:
>  - assert copyright for the non-trivial changes
>  - since the magic ASN matching canaries use valid bits, seperate the
> filter storage and a wire storage
>  - clean up warnings
>  - fix a few printing issues
> 
> OK?
> 
> 
> Index: usr.sbin/bgpctl/bgpctl.8
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpctl/bgpctl.8,v
> retrieving revision 1.69
> diff -u -p -u -p -r1.69 bgpctl.8
> --- usr.sbin/bgpctl/bgpctl.8  25 May 2016 14:15:59 -  1.69
> +++ usr.sbin/bgpctl/bgpctl.8  5 Sep 2016 13:41:29 -
> @@ -300,6 +300,9 @@ anywhere in the AS path.
>  .It Cm community Ar community
>  Show all entries with community
>  .Ar community .
> +.It Cm large-community Ar large-community
> +Show all entries with large-community
> +.Ar large-community .
>  .It Cm empty-as
>  Show all entries that are internal routes with no AS's in the AS path.
>  .It Cm memory
> Index: usr.sbin/bgpctl/bgpctl.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.188
> diff -u -p -u -p -r1.188 bgpctl.c
> --- usr.sbin/bgpctl/bgpctl.c  3 Jun 2016 17:36:37 -   1.188
> +++ usr.sbin/bgpctl/bgpctl.c  13 Oct 2016 15:32:30 -
> @@ -2,6 +2,8 @@
>  
>  /*
>   * Copyright (c) 2003 Henning Brauer 
> + * Copyright (c) 2016 Job Snijders 
> + * Copyright (c) 2016 Peter Hessler 
>   *
>   * Permission to use, copy, modify, and distribute this software for any
>   * purpose with or without fee is hereby granted, provided that the above
> @@ -82,6 +84,7 @@ void show_rib_brief(struct ctl_show_ri
>  void  show_rib_detail(struct ctl_show_rib *, u_char *, int);
>  void  show_attr(void *, u_int16_t);
>  void  show_community(u_char *, u_int16_t);
> +void  show_large_community(u_char *, u_int16_t);
>  void  show_ext_community(u_char *, u_int16_t);
>  char *fmt_mem(int64_t);
>  int   show_rib_memory_msg(struct imsg *);
> @@ -254,6 +257,13 @@ main(int argc, char *argv[])
>   sizeof(res->community));
>   type = IMSG_CTL_SHOW_RIB_COMMUNITY;
>   }
> + if (res->large_community.as != COMMUNITY_UNSET &&
> + res->large_community.ld1 != COMMUNITY_UNSET &&
> + res->large_community.ld2 != COMMUNITY_UNSET) {
> + memcpy(_community, >large_community,
> + sizeof(res->large_community));
> + type = IMSG_CTL_SHOW_RIB_LARGECOMMUNITY;
> + }
>   memcpy(, , sizeof(ribreq.neighbor));
>   strlcpy(ribreq.rib, res->rib, sizeof(ribreq.rib));
>   ribreq.aid = res->aid;
> @@ -275,6 +285,11 @@ main(int argc, char *argv[])
>   res->community.type != COMMUNITY_UNSET)
>   memcpy(, >community,
>   sizeof(res->community));
> + if (res->large_community.as != COMMUNITY_UNSET &&
> + res->large_community.ld1 != COMMUNITY_UNSET &&
> + res->large_community.ld2 != COMMUNITY_UNSET)
> + memcpy(_community, >large_community,
> + sizeof(res->large_community));
>   memcpy(, , sizeof(ribreq.neighbor));
>   ribreq.aid = res->aid;
>   ribreq.flags = res->flags;
> @@ -377,6 +392,11 @@ main(int argc, char *argv[])
>   res->community.type != COMMUNITY_UNSET)
>   memcpy(, >community,
>   sizeof(res->community));
> + if (res->large_community.as != COMMUNITY_UNSET &&
> + res->large_community.ld1 != COMMUNITY_UNSET &&
> + res->large_community.ld2 != COMMUNITY_UNSET)
> + memcpy(_community, >large_community,
> +

Re: relayd(8): proc.c sync and remove fd limit change

[Rafael Zalamena]

On Tue, Oct 11, 2016 at 02:02:46AM +0200, Rafael Zalamena wrote:
> This diff brings the relayd(8) proc.c up-to-date and removes the file limit
> alteration in relayd.c. The file limit alteration is not needed anymore
> since now the number of descriptors pre-allocated is very small (only one
> descriptor per child + 2 to distribute fds between child).
> 
> It would be nice to have some feedback in this diff since this daemon is
> the one that most uses the proc.c multiple instances of child process.

Here is an updated diff with the proc_flush_imsg() fix from reyk@.

Summary:
 * Fixes the msgbuf_write() usage idiom;
 * Add context to fatal() messages;
 * Use proc_flush_imsg() instead of manually using imsg_flush();
 * Use less fds on startup;

ok?

Index: proc.c
===
RCS file: /home/obsdcvs/src/usr.sbin/relayd/proc.c,v
retrieving revision 1.36
diff -u -p -r1.36 proc.c
--- proc.c  5 Oct 2016 17:31:28 -   1.36
+++ proc.c  14 Oct 2016 15:14:21 -
@@ -37,8 +37,6 @@
 
 voidproc_exec(struct privsep *, struct privsep_proc *, unsigned int,
int, char **);
-voidproc_connectpeer(struct privsep *, enum privsep_procid, int,
-   struct privsep_pipes *);
 voidproc_setup(struct privsep *, struct privsep_proc *, unsigned int);
 voidproc_open(struct privsep *, int, int);
 voidproc_accept(struct privsep *, int, enum privsep_procid,
@@ -157,72 +155,38 @@ proc_exec(struct privsep *ps, struct pri
 }
 
 void
-proc_connectpeer(struct privsep *ps, enum privsep_procid id, int inst,
-struct privsep_pipes *pp)
-{
-   unsigned int i, j;
-   struct privsep_fdpf;
-
-   for (i = 0; i < PROC_MAX; i++) {
-   /* Parent is already connected with everyone. */
-   if (i == PROC_PARENT)
-   continue;
-
-   for (j = 0; j < ps->ps_instances[i]; j++) {
-   /* Don't send socket to child itself. */
-   if (i == (unsigned int)id &&
-   j == (unsigned int)inst)
-   continue;
-   if (pp->pp_pipes[i][j] == -1)
-   continue;
-
-   pf.pf_procid = i;
-   pf.pf_instance = j;
-   proc_compose_imsg(ps, id, inst, IMSG_CTL_PROCFD,
-   -1, pp->pp_pipes[i][j], , sizeof(pf));
-   pp->pp_pipes[i][j] = -1;
-   }
-   }
-}
-
-/* Inter-connect all process except with ourself. */
-void
 proc_connect(struct privsep *ps)
 {
-   unsigned int src, i, j;
-   struct privsep_pipes*pp;
struct imsgev   *iev;
+   unsigned int src, dst, inst;
 
-   /* Listen on appropriate pipes. */
-   src = privsep_process;
-   pp = >ps_pipes[src][ps->ps_instance];
-
-   for (i = 0; i < PROC_MAX; i++) {
-   /* Don't listen to ourself. */
-   if (i == src)
-   continue;
+   /* Don't distribute any sockets if we are not really going to run. */
+   if (ps->ps_noaction)
+   return;
 
-   for (j = 0; j < ps->ps_instances[i]; j++) {
-   if (pp->pp_pipes[i][j] == -1)
-   continue;
+   for (dst = 0; dst < PROC_MAX; dst++) {
+   /* We don't communicate with ourselves. */
+   if (dst == PROC_PARENT)
+   continue;
 
-   iev = >ps_ievs[i][j];
-   imsg_init(>ibuf, pp->pp_pipes[i][j]);
+   for (inst = 0; inst < ps->ps_instances[dst]; inst++) {
+   iev = >ps_ievs[dst][inst];
+   imsg_init(>ibuf, ps->ps_pp->pp_pipes[dst][inst]);
event_set(>ev, iev->ibuf.fd, iev->events,
iev->handler, iev->data);
event_add(>ev, NULL);
}
}
 
-   /* Exchange pipes between process. */
-   for (i = 0; i < PROC_MAX; i++) {
-   /* Parent is already connected with everyone. */
-   if (i == PROC_PARENT)
-   continue;
+   /* Distribute the socketpair()s for everyone. */
+   for (src = 0; src < PROC_MAX; src++)
+   for (dst = src; dst < PROC_MAX; dst++) {
+   /* Parent already distributed its fds. */
+   if (src == PROC_PARENT || dst == PROC_PARENT)
+   continue;
 
-   for (j = 0; j < ps->ps_instances[i]; j++)
-   proc_connectpeer(ps, i, j, >ps_pipes[i][j]);
-   }
+   proc_open(ps, src, dst);
+   }
 }
 
 void
@@ -230,17 +194,41 @@ proc_init(struct privsep *ps, struct pri
 int argc, char **argv, enum privsep_procid proc_id)
 {

Re: opencvs - fix update -r and -A

[Todd C. Miller]

On Fri, 14 Oct 2016 16:17:45 +0200, Joris Vink wrote:

> In certain cases update -r and update -A would not properly set or reset
> the sticky tag for file(s). This patch fixes that problem.

After reading through the applicable parts of update.c I'm fairly
certain this is correct.  OK millert@

 - todd

Re: bgpd draft-ietf-idr-large-community

[Sebastian Benoit]

Peter Hessler(phess...@openbsd.org) on 2016.10.13 17:34:28 +0200:
> On 2016 Oct 11 (Tue) at 00:00:53 +0200 (+0200), Peter Hessler wrote:
> :Here is an initial implementation of draft-ietf-idr-large-community for
> :OpenBGPD.  I can connect and exchange routes with these attributes
> :against exabgp.
> :
> :Normal communities are two 16bit numbers.  With the addition of
> :32bit ASNs, those will not work if you wish to control one of
> :them.
> :
> :Large Communities are 32bit:32bit:32bit.  It seems the convention will be
> :::, with  and  being locally
> :defined.
> :
> :RFC status: currently accepted by the IDR-WG, is at version -02, the
> :wire format is set, the attribute codepoint is assigned by IANA, and it
> :seems that only trivial details need to be addressed.  Very likely to be
> :accepted.
> :
> :This was based on a partial implementation from Job Snijders, many
> :thanks!
> :
> :Comments?  OK?
> :
> 
> Updated diff:
>  - assert copyright for the non-trivial changes
>  - since the magic ASN matching canaries use valid bits, seperate the
> filter storage and a wire storage
>  - clean up warnings
>  - fix a few printing issues
> 
> OK?

nice, just a few questions below 

> Index: usr.sbin/bgpctl/bgpctl.8
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpctl/bgpctl.8,v
> retrieving revision 1.69
> diff -u -p -u -p -r1.69 bgpctl.8
> --- usr.sbin/bgpctl/bgpctl.8  25 May 2016 14:15:59 -  1.69
> +++ usr.sbin/bgpctl/bgpctl.8  5 Sep 2016 13:41:29 -
> @@ -300,6 +300,9 @@ anywhere in the AS path.
>  .It Cm community Ar community
>  Show all entries with community
>  .Ar community .
> +.It Cm large-community Ar large-community
> +Show all entries with large-community
> +.Ar large-community .
>  .It Cm empty-as
>  Show all entries that are internal routes with no AS's in the AS path.
>  .It Cm memory
> Index: usr.sbin/bgpctl/bgpctl.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.188
> diff -u -p -u -p -r1.188 bgpctl.c
> --- usr.sbin/bgpctl/bgpctl.c  3 Jun 2016 17:36:37 -   1.188
> +++ usr.sbin/bgpctl/bgpctl.c  13 Oct 2016 15:32:30 -
> @@ -2,6 +2,8 @@
>  
>  /*
>   * Copyright (c) 2003 Henning Brauer 
> + * Copyright (c) 2016 Job Snijders 
> + * Copyright (c) 2016 Peter Hessler 
>   *
>   * Permission to use, copy, modify, and distribute this software for any
>   * purpose with or without fee is hereby granted, provided that the above
> @@ -82,6 +84,7 @@ void show_rib_brief(struct ctl_show_ri
>  void  show_rib_detail(struct ctl_show_rib *, u_char *, int);
>  void  show_attr(void *, u_int16_t);
>  void  show_community(u_char *, u_int16_t);
> +void  show_large_community(u_char *, u_int16_t);
>  void  show_ext_community(u_char *, u_int16_t);
>  char *fmt_mem(int64_t);
>  int   show_rib_memory_msg(struct imsg *);
> @@ -254,6 +257,13 @@ main(int argc, char *argv[])
>   sizeof(res->community));
>   type = IMSG_CTL_SHOW_RIB_COMMUNITY;
>   }
> + if (res->large_community.as != COMMUNITY_UNSET &&
> + res->large_community.ld1 != COMMUNITY_UNSET &&
> + res->large_community.ld2 != COMMUNITY_UNSET) {
> + memcpy(_community, >large_community,
> + sizeof(res->large_community));
> + type = IMSG_CTL_SHOW_RIB_LARGECOMMUNITY;
> + }
>   memcpy(, , sizeof(ribreq.neighbor));
>   strlcpy(ribreq.rib, res->rib, sizeof(ribreq.rib));
>   ribreq.aid = res->aid;
> @@ -275,6 +285,11 @@ main(int argc, char *argv[])
>   res->community.type != COMMUNITY_UNSET)
>   memcpy(, >community,
>   sizeof(res->community));
> + if (res->large_community.as != COMMUNITY_UNSET &&
> + res->large_community.ld1 != COMMUNITY_UNSET &&
> + res->large_community.ld2 != COMMUNITY_UNSET)
> + memcpy(_community, >large_community,
> + sizeof(res->large_community));
>   memcpy(, , sizeof(ribreq.neighbor));
>   ribreq.aid = res->aid;
>   ribreq.flags = res->flags;
> @@ -377,6 +392,11 @@ main(int argc, char *argv[])
>   res->community.type != COMMUNITY_UNSET)
>   memcpy(, >community,
>   sizeof(res->community));
> + if (res->large_community.as != COMMUNITY_UNSET &&
> + res->large_community.ld1 != COMMUNITY_UNSET &&
> + res->large_community.ld2 != COMMUNITY_UNSET)
> + memcpy(_community, >large_community,
> + 

switch(4): kill unused function

[Rafael Zalamena]

The switch(4) device has a function called switch_forward_flooder()
which doesn't seem to be used anywhere.

In switchofp.c we have the swofp_action_output() which would be the place
where it would be likely called, however it already has the code that
does it.

Since it doesn't seem to fit anywhere I think we should just removed it,
and that's what this diff does.

ok?

Index: net/if_switch.c
===
RCS file: /home/obsdcvs/src/sys/net/if_switch.c,v
retrieving revision 1.9
diff -u -p -r1.9 if_switch.c
--- net/if_switch.c 8 Oct 2016 23:36:10 -   1.9
+++ net/if_switch.c 14 Oct 2016 13:28:18 -
@@ -85,8 +85,6 @@ intswitch_stop(struct ifnet *, int);
 struct mbuf
*switch_port_ingress(struct switch_softc *, struct ifnet *,
struct mbuf *);
-voidswitch_forward_flooder(struct switch_softc *,
-   struct switch_flow_classify *, struct mbuf *);
 voidswitch_port_egress(struct switch_softc *, struct switch_fwdp_queue *,
struct mbuf *);
 int switch_ifenqueue(struct switch_softc *, struct ifnet *,
@@ -725,25 +720,6 @@ switch_port_ingress(struct switch_softc 
 #endif /* NPF */
 
return (m);
-}
-
-void
-switch_forward_flooder(struct switch_softc *sc,
-struct switch_flow_classify *swfcl, struct mbuf *m)
-{
-   struct switch_port   *swpo;
-   struct switch_fwdp_queue fwdp_q;
-   uint32_t src_port_no;
-
-   src_port_no = swfcl->swfcl_in_port;
-   TAILQ_INIT(_q);
-   TAILQ_FOREACH(swpo, >sc_swpo_list, swpo_list_next) {
-   if (swpo->swpo_port_no == src_port_no)
-   continue;
-   TAILQ_INSERT_HEAD(_q, swpo, swpo_fwdp_next);
-   }
-
-   switch_port_egress(sc, _q, m);
 }
 
 void

Re: use x2apic if it is enabled by BIOS

[sf]

On Fri, 14 Oct 2016, YASUOKA Masahiko wrote:
> I'm working on NEC Express5800/R110h-1 (dmesg is attached).  On this
> machine, our kernel panics with following message.
> 
>   cpu0 at mainbus0panic: cpu at apic id 0 already attached?
> 
> This seems to happen since x2APIC on the machine is enabled by BIOS
> and the kernel doesn't assume that.  The diff makes the kernel use
> x2APIC if it is enabled by BIOS.
> 
> ok?

the code looks ok, but ...

> 
> Index: sys/arch/amd64/amd64/lapic.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 lapic.c
> --- sys/arch/amd64/amd64/lapic.c  22 Jun 2016 01:12:38 -  1.44
> +++ sys/arch/amd64/amd64/lapic.c  14 Oct 2016 07:45:50 -
> @@ -170,60 +170,55 @@ lapic_map(paddr_t lapic_base)
>   int s;
>   pt_entry_t *pte;
>   vaddr_t va;
> + u_int64_t msr;
>  
> - /*
> -  * On real hardware, x2apic must only be enabled if interrupt remapping
> -  * is also enabled. See 10.12.7 of the SDM vol 3.
> -  * On hypervisors, this is not necessary. Hypervisors can implement
> -  * x2apic support even if the host CPU does not support it.
> -  * Until we support interrupt remapping, use x2apic only if the
> -  * hypervisor flag is also set.
> -  */

... I would leave the comment here why we don't enable it on all hardware 
that supports it.

> - if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_HV)) {
> - u_int64_t msr;
> -
> - disable_intr();
> - s = lapic_tpr;
> -
> - msr = rdmsr(MSR_APICBASE);
> - msr |= APICBASE_ENABLE_X2APIC;
> - wrmsr(MSR_APICBASE, msr);
> + disable_intr();
> + s = lapic_tpr;
> +
> + msr = rdmsr(MSR_APICBASE);
>  
> + if (ISSET(msr, APICBASE_ENABLE_X2APIC) ||
> + (ISSET(cpu_ecxfeature, CPUIDECX_HV) &&
> + ISSET(cpu_ecxfeature, CPUIDECX_X2APIC))) {
> + /*
> +  * If x2APIC is enabled already, use it since it is enabled
> +  * by BIOS.  On hypervisor, use it if it exists.
> +  */
> + if (!ISSET(msr, APICBASE_ENABLE_X2APIC)) {
> + msr |= APICBASE_ENABLE_X2APIC;
> + wrmsr(MSR_APICBASE, msr);
> + }
>   lapic_readreg = x2apic_readreg;
>   lapic_writereg = x2apic_writereg;
>  #ifdef MULTIPROCESSOR
>   x86_ipi = x2apic_ipi;
>  #endif
>   x2apic_enabled = 1;
> -
>   codepatch_call(CPTAG_EOI, _eoi);
>  
>   lapic_writereg(LAPIC_TPRI, s);
> - enable_intr();
> + } else {
> + /*
> +  * Map local apic.  If we have a local apic, it's safe to
> +  * assume we're on a 486 or better and can use invlpg and
> +  * non-cacheable PTE's
> +  *
> +  * Whap the PTE "by hand" rather than calling pmap_kenter_pa
> +  * because the latter will attempt to invoke TLB shootdown
> +  * code just as we might have changed the value of
> +  * cpu_number()..
> +  */
> + va = (vaddr_t)_apic;
> + pte = kvtopte(va);
> + *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
> + invlpg(va);
>  
> - return;
> + lapic_tpr = s;
>   }
>  
> - va = (vaddr_t)_apic;
> -
> - disable_intr();
> - s = lapic_tpr;
> -
> - /*
> -  * Map local apic.  If we have a local apic, it's safe to assume
> -  * we're on a 486 or better and can use invlpg and non-cacheable PTE's
> -  *
> -  * Whap the PTE "by hand" rather than calling pmap_kenter_pa because
> -  * the latter will attempt to invoke TLB shootdown code just as we
> -  * might have changed the value of cpu_number()..
> -  */
> -
> - pte = kvtopte(va);
> - *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
> - invlpg(va);
> -
> - lapic_tpr = s;
>   enable_intr();
> +
> + return;
>  }

opencvs - fix update -r and -A

[Joris Vink]

Hi,

In certain cases update -r and update -A would not properly set or reset
the sticky tag for file(s). This patch fixes that problem.

.joris

Index: update.c
===
RCS file: /cvs/src/usr.bin/cvs/update.c,v
retrieving revision 1.172
diff -u -p -r1.172 update.c
--- update.c13 Oct 2016 20:51:25 -  1.172
+++ update.c14 Oct 2016 14:13:11 -
@@ -480,10 +480,8 @@ cvs_update_local(struct cvs_file *cf)
if (cvs_cmdop != CVS_OP_UPDATE)
break;
 
-   if (reset_tag != 1 && reset_option != 1)
-   break;
-
-   if (cf->file_ent != NULL && cf->file_ent->ce_tag == NULL)
+   if (reset_tag != 1 && reset_option != 1 &&
+   cvs_specified_tag == NULL && cvs_specified_date == -1)
break;
 
if (cf->file_rcs->rf_dead != 1 &&

Re: use x2apic if it is enabled by BIOS

[Stefan Fritsch]

On Friday, 14 October 2016 15:48:47 CEST Mark Kettenis wrote:
> > Date: Fri, 14 Oct 2016 16:49:31 +0900 (JST)
> > From: YASUOKA Masahiko 
> > 
> > Hi,
> > 
> > I'm working on NEC Express5800/R110h-1 (dmesg is attached).  On this
> > machine, our kernel panics with following message.
> > 
> >   cpu0 at mainbus0panic: cpu at apic id 0 already attached?
> > 
> > This seems to happen since x2APIC on the machine is enabled by BIOS
> > and the kernel doesn't assume that.  The diff makes the kernel use
> > x2APIC if it is enabled by BIOS.
> > 
> > ok?
> 
> is the APICBASE msr available on all amd64 machines?  If not, it will
> need to be protected with a CPUID check.

Yes. I have looked that up before, for its use in the suspend/resume code.

Stefan

Re: use x2apic if it is enabled by BIOS

[Mark Kettenis]

> Date: Fri, 14 Oct 2016 16:49:31 +0900 (JST)
> From: YASUOKA Masahiko 
> 
> Hi,
> 
> I'm working on NEC Express5800/R110h-1 (dmesg is attached).  On this
> machine, our kernel panics with following message.
> 
>   cpu0 at mainbus0panic: cpu at apic id 0 already attached?
> 
> This seems to happen since x2APIC on the machine is enabled by BIOS
> and the kernel doesn't assume that.  The diff makes the kernel use
> x2APIC if it is enabled by BIOS.
> 
> ok?

is the APICBASE msr available on all amd64 machines?  If not, it will
need to be protected with a CPUID check.

> Index: sys/arch/amd64/amd64/lapic.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 lapic.c
> --- sys/arch/amd64/amd64/lapic.c  22 Jun 2016 01:12:38 -  1.44
> +++ sys/arch/amd64/amd64/lapic.c  14 Oct 2016 07:45:50 -
> @@ -170,60 +170,55 @@ lapic_map(paddr_t lapic_base)
>   int s;
>   pt_entry_t *pte;
>   vaddr_t va;
> + u_int64_t msr;
>  
> - /*
> -  * On real hardware, x2apic must only be enabled if interrupt remapping
> -  * is also enabled. See 10.12.7 of the SDM vol 3.
> -  * On hypervisors, this is not necessary. Hypervisors can implement
> -  * x2apic support even if the host CPU does not support it.
> -  * Until we support interrupt remapping, use x2apic only if the
> -  * hypervisor flag is also set.
> -  */
> - if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_HV)) {
> - u_int64_t msr;
> -
> - disable_intr();
> - s = lapic_tpr;
> -
> - msr = rdmsr(MSR_APICBASE);
> - msr |= APICBASE_ENABLE_X2APIC;
> - wrmsr(MSR_APICBASE, msr);
> + disable_intr();
> + s = lapic_tpr;
> +
> + msr = rdmsr(MSR_APICBASE);
>  
> + if (ISSET(msr, APICBASE_ENABLE_X2APIC) ||
> + (ISSET(cpu_ecxfeature, CPUIDECX_HV) &&
> + ISSET(cpu_ecxfeature, CPUIDECX_X2APIC))) {
> + /*
> +  * If x2APIC is enabled already, use it since it is enabled
> +  * by BIOS.  On hypervisor, use it if it exists.
> +  */
> + if (!ISSET(msr, APICBASE_ENABLE_X2APIC)) {
> + msr |= APICBASE_ENABLE_X2APIC;
> + wrmsr(MSR_APICBASE, msr);
> + }
>   lapic_readreg = x2apic_readreg;
>   lapic_writereg = x2apic_writereg;
>  #ifdef MULTIPROCESSOR
>   x86_ipi = x2apic_ipi;
>  #endif
>   x2apic_enabled = 1;
> -
>   codepatch_call(CPTAG_EOI, _eoi);
>  
>   lapic_writereg(LAPIC_TPRI, s);
> - enable_intr();
> + } else {
> + /*
> +  * Map local apic.  If we have a local apic, it's safe to
> +  * assume we're on a 486 or better and can use invlpg and
> +  * non-cacheable PTE's
> +  *
> +  * Whap the PTE "by hand" rather than calling pmap_kenter_pa
> +  * because the latter will attempt to invoke TLB shootdown
> +  * code just as we might have changed the value of
> +  * cpu_number()..
> +  */
> + va = (vaddr_t)_apic;
> + pte = kvtopte(va);
> + *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
> + invlpg(va);
>  
> - return;
> + lapic_tpr = s;
>   }
>  
> - va = (vaddr_t)_apic;
> -
> - disable_intr();
> - s = lapic_tpr;
> -
> - /*
> -  * Map local apic.  If we have a local apic, it's safe to assume
> -  * we're on a 486 or better and can use invlpg and non-cacheable PTE's
> -  *
> -  * Whap the PTE "by hand" rather than calling pmap_kenter_pa because
> -  * the latter will attempt to invoke TLB shootdown code just as we
> -  * might have changed the value of cpu_number()..
> -  */
> -
> - pte = kvtopte(va);
> - *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
> - invlpg(va);
> -
> - lapic_tpr = s;
>   enable_intr();
> +
> + return;
>  }
>  
>  /*
> 
> OpenBSD 6.0-current (GENERIC.MP) #44: Fri Oct 14 16:32:03 JST 2016
> 
> yasu...@yasuoka-ob1.tokyo.iiji.jp:/source/yasuoka/openbsd/head/git/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 8207699968 (7827MB)
> avail mem = 7954391040 (7585MB)
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f92b000 (60 entries)
> bios0: vendor American Megatrends Inc. version "5.0.4012" date 06/15/2016
> bios0: NEC Express5800/R110h-1 [N8100-2316Y]
> acpi0 at bios0: rev 2
> acpi0: sleep states S0 S4 S5
> acpi0: tables DSDT FACP APIC FIDT MCFG HPET SSDT SSDT SSDT SSDT SPCR DMAR BERT
> acpi0: wakeup devices PEGP(S4) PEG0(S4) PEG1(S4) PEG2(S4) PXSX(S4) RP17(S4) 
> PXSX(S4) RP18(S4) PXSX(S4) RP19(S4) PXSX(S4) RP20(S4) PXSX(S4) RP01(S4) 
> 

ksh(1) does not need to look at LC_CTYPE

[Ingo Schwarze]

Hi,

when committing ksh(1) vi input mode UTF-8 support recently,
i added a setlocale(3) call to the shell.  Now, when considering
how to document LC_CTYPE in the ksh(1) manual, i realized that
inspecting that variable is not really useful, so we can simplify
things, see the diff below.

First, note that emacs mode doesn't use LC_CTYPE in the first
place, nor does anything else in the shell except vi mode.

Assuming you use vi mode (VISUAL=vi), three settings influence
how things work for you:

 1. Whether escaping non-ASCII bytes is disabled (set +o vi-show8,
the default) or enabled (set -o vi-show8).

 2. LC_CTYPE=C (the default) or UTF-8.

 3. Wether your xterm is UTF-8 enabled (-u8, the default)
or not (+u8).

So there are 2^3 = 8 case combinations.  Let's look at them
in turn.

 A. Escaping non-ASCII bytes enabled (set -o vi-show8):

In this case, the diff changes nothing because isu8cont()
always returns 0 already now, so LC_CTYPE is already
effectively ignored.

I considered whether this mode is useful at all or whether
it might be better to just delete the vi-show8 switch
outright and simplify the code.

But there is a potential use case, however rare it may be:
Sometimes, you may want to edit individual raw 8-bit bytes
on shell command lines, either for testing purposes or
to call programs that require binary command line arguments
or input.  That wish may occur for any LC_CTYPE setting
and on any terminal.

So 4 of the 8 cases are taken care of so far.

In the following, we know that non-ASCII bytes will not be
escaped (set +o vi-show8).

 B. The user wants to use UTF-8:

In that case, having LC_CTYPE=en_US.UTF-8 is required
and the patch changes nothing.

Of course, the xterm must also be UTF-8 enabled.
The combination with +u8 is just useless and dangerous,
with or without the patch.

So far, 6 of the 8 cases are taken care of.

What remains is set +o vi-show8 with LC_CTYPE=C (the default, actually).

 C. On a UTF-8 terminal (This is the default case!):

In this case, if the user presses non-ASCII keys on the
keyboard, they will result in UTF-8-encoded multibyte
strings in the shell's internal buffers and they will be
shown as Unicode glyphs.

In that situation, respecting LC_CTYPE=C allows the user
to move the cursor to individual bytes, but without being
able to see where the cursor really is, and to delete and
insert single bytes in the middle of characters, causing
the display to show stuff that disagrees with the actual
content of the buffers.  This is not useful at all and
potentially dangerous.

The diff below actually makes things better.  It improves
the chances that the display remains consistent with actual
buffer content, by effectively editing in UTF-8 mode.

Admittedly, that may not be what the user wants.
But if the user really wants to mess with arbitrary bytes,
they ought to set -o vi-show8 as explained above.

 D. On a terminal in non-UTF-8 legacy latin-1 mode:

The only reason i can imagine for using such a mode combination
is to manipulate arbitrary bytes individually, but actually,
this mode combination is unusable for that purpose because
several bytes will corrupt or lock up the terminal, both with
and without this patch.  So it doesn't really matter that the
patch changes behaviour here, the mode is useless and dangerous
in the first place.  Besides, the mode is hardly usable at all
because most characters that can be entered are interpreted and
shown as ISO-LATIN-1 characters, which is not a useful way to
represent arbitrary binary bytes.

So, the patch

 - makes the code simpler,
 - changes nothing for many use cases,
 - improves the default use case, and
 - besides, only affects a mode combination that is useless anyway.

OK to put it in?
  Ingo


Index: main.c
===
RCS file: /cvs/src/bin/ksh/main.c,v
retrieving revision 1.81
diff -u -p -r1.81 main.c
--- main.c  11 Oct 2016 19:52:54 -  1.81
+++ main.c  14 Oct 2016 12:27:31 -
@@ -8,7 +8,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -152,8 +151,6 @@ main(int argc, char *argv[])
pid_t ppid;
 
kshname = argv[0];
-
-   setlocale(LC_CTYPE, "");
 
if (pledge("stdio rpath wpath cpath fattr flock getpw proc exec tty",
NULL) == -1) {
Index: vi.c
===
RCS file: /cvs/src/bin/ksh/vi.c,v
retrieving revision 1.40
diff -u -p -r1.40 vi.c
--- vi.c11 Oct 2016 19:52:54 -  1.40
+++ vi.c14 Oct 2016 12:27:31 -
@@ -2224,6 +2224,6 @@ vi_macro_reset(void)
 static int
 isu8cont(unsigned char c)
 {
-   return MB_CUR_MAX > 1 && !Flag(FVISHOW8) && (c & (0x80 | 0x40)) == 0x80;
+   return !Flag(FVISHOW8) && (c & 

malloc junk bytes

[Otto Moerbeek]

Hi,

0xdb is better dan 0xd0, since it is unaligned in more cases (think
about the bytes being used as a pointer.

ok?

-Otto

Index: lib/libc/stdlib/malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.200
diff -u -p -r1.200 malloc.c
--- lib/libc/stdlib/malloc.c12 Oct 2016 07:36:38 -  1.200
+++ lib/libc/stdlib/malloc.c14 Oct 2016 11:37:44 -
@@ -81,8 +81,8 @@
  * when the 'J' option is enabled. Use SOME_JUNK right after alloc,
  * and SOME_FREEJUNK right before free.
  */
-#define SOME_JUNK  0xd0/* as in "Duh" :-) */
-#define SOME_FREEJUNK  0xdf
+#define SOME_JUNK  0xdb/* deadbeef */
+#define SOME_FREEJUNK  0xdf/* dead, free */
 
 #define MMAP(sz)   mmap(NULL, (sz), PROT_READ | PROT_WRITE, \
 MAP_ANON | MAP_PRIVATE, -1, 0)
Index: share/man/man5/malloc.conf.5
===
RCS file: /cvs/src/share/man/man5/malloc.conf.5,v
retrieving revision 1.7
diff -u -p -r1.7 malloc.conf.5
--- share/man/man5/malloc.conf.56 Jul 2016 20:32:02 -   1.7
+++ share/man/man5/malloc.conf.514 Oct 2016 11:37:45 -
@@ -84,10 +84,8 @@ Increase the junk level by one if it is 
 .Dq Less junking .
 Decrease the junk level by one if it is larger than 0.
 Junking writes some junk bytes into the area allocated.
-Currently junk is bytes of 0xd0 when allocating; this is pronounced
-.Dq Duh .
-\&:-)
-Freed chunks are filled with 0xdf.
+Currently junk is bytes of 0xdb when allocating;
+freed chunks are filled with 0xdf.
 By default the junk level is 1: small chunks are always junked
 and the first part of pages is junked after free.
 After a delay (if not switched off by the F option),

ksh vi mode: fix region deletion and yanking

[Dmitrij D. Czarkoff]

Hi!

I've noticed that in ksh's vi mode ranged operations are performed
without respect to cursor's position within utf8 byte sequence.  Eg.:

 1. type "echo тест | hexdump -C"
 2. leave inseart mode
 3. "0", "2E", "dh", Enter
 4. you end up with "те" and 0x82 (last byte of letter under cursor).

This happens because Endword() moves cursor to the whitespace after word
and decrements cursor position by 1, so that it points to last byte of
last letter.  Then del_range() removes bytes between cursor position and
preceding utf8 start byte, which include start byte of letter under
cursor.

My diff makes del_range() (and yank_range() which operates in the same
manner) always skip to the beginning of utf8 sequence it is in.
Although this is a more of a bandaid - proper fix would be to make sure
that cursor never rests on continuation byte - it is less invasive and
does not hurt code readability too much.

Comments?  OKs?

--
Dmitrij D. Czarkoff

Index: vi.c
===
RCS file: /var/cvs/src/bin/ksh/vi.c,v
retrieving revision 1.40
diff -u -p -r1.40 vi.c
--- vi.c11 Oct 2016 19:52:54 -  1.40
+++ vi.c14 Oct 2016 10:47:25 -
@@ -1323,6 +1323,10 @@ redo_insert(int count)
 static void
 yank_range(int a, int b)
 {
+   while (isu8cont((unsigned char)es->cbuf[a]))
+   a--;
+   while (isu8cont((unsigned char)es->cbuf[b]))
+   b--;
yanklen = b - a;
if (yanklen != 0)
memmove(ybuf, >cbuf[a], yanklen);
@@ -1493,6 +1497,10 @@ putbuf(const char *buf, int len, int rep
 static void
 del_range(int a, int b)
 {
+   while (isu8cont((unsigned char)es->cbuf[a]))
+   a--;
+   while (isu8cont((unsigned char)es->cbuf[b]))
+   b--;
if (es->linelen != b)
memmove(>cbuf[a], >cbuf[b], es->linelen - b);
es->linelen -= b - a;

Re: mcl2k2 mbuf clusters

[Mike Belopuhov]

On 14 October 2016 at 12:40, Mike Belopuhov  wrote:
> On Fri, Oct 14, 2016 at 07:58 +0200, Claudio Jeker wrote:
>> It is time to put the nasty comment from rl(4) into em(4) and ix(4).
>> Everybody knew how bad realtek was but thinks Intel nics are good. The
>> truth is that modern Intel nic are as bad as the cheepest and crapiest
>> 10/100 Mbps Ethernet chips from the last millenium.
>>
>> --
>> :wq Claudio
>>
>
> Then don't use them.  Vanity won't fix anything (if there's anything
> to fix at all).

I don't mean to offend you personally.  It's directed at all of us,
me included.  I apologize if that sounded too harsh.

Re: mcl2k2 mbuf clusters

[Mike Belopuhov]

On Fri, Oct 14, 2016 at 15:48 +1000, David Gwynne wrote:
> this adds a pool backend for MCLGETI thats 2k+2 bytes in size, which
> can be used on some very common nics that have annoying constraints
> on their rx descriptors.
> 
> this in turn simplifies the code in those drivers and lets them
> always operate on ETHER_ALIGN boundaries.
>

I would agree for using it under the _STRICT_ALIGNED ifdef instead
of wasting a page, but not in the generic case.  No technical reason
to do it.  Code simplifications brought by this change are miniscule.
In fact it adds more code to the uipc_mbuf than saves in the driver.

> the pool is cheap, pages will only be allocated in it if something
> asks for them, and it keeps this complexity out of the drivers.
> 
> ok?
> 

Not OK.  Not w/o *at least* the performance assessment of this change.
Right now it's just a distraction from more important issues.

Re: mcl2k2 mbuf clusters

[Mike Belopuhov]

On Fri, Oct 14, 2016 at 07:58 +0200, Claudio Jeker wrote:
> It is time to put the nasty comment from rl(4) into em(4) and ix(4).
> Everybody knew how bad realtek was but thinks Intel nics are good. The
> truth is that modern Intel nic are as bad as the cheepest and crapiest
> 10/100 Mbps Ethernet chips from the last millenium.
> 
> -- 
> :wq Claudio
> 

Then don't use them.  Vanity won't fix anything (if there's anything
to fix at all).

switch(4): fix packet_out message handling

[Rafael Zalamena]

The switch(4) packet_out handler wasn't handling some cases, so here is
the missing code.

1) pout_buffer_id is a 4 bytes field and it was using the wrong define
   to check for absence of buffers;
2) When a buffer_id was sent the code didn't handle this, now when this
   happens we send an error message back;
3) Call the classifier function again for packet_out otherwise the
   *_set_field* action functions will panic();

ok?

Index: net/if_switch.c
===
RCS file: /home/obsdcvs/src/sys/net/if_switch.c,v
retrieving revision 1.9
diff -u -p -r1.9 if_switch.c
--- net/if_switch.c 8 Oct 2016 23:36:10 -   1.9
+++ net/if_switch.c 13 Oct 2016 16:08:28 -
@@ -124,9 +124,6 @@ struct mbuf
 struct mbuf
*switch_flow_classifier_tunnel(struct mbuf *, int *,
struct switch_flow_classify *);
-struct mbuf
-   *switch_flow_classifier(struct mbuf *, uint32_t,
-   struct switch_flow_classify *);
 voidswitch_flow_classifier_dump(struct switch_softc *,
struct switch_flow_classify *);
 voidswitchattach(int);
Index: net/if_switch.h
===
RCS file: /home/obsdcvs/src/sys/net/if_switch.h,v
retrieving revision 1.4
diff -u -p -r1.4 if_switch.h
--- net/if_switch.h 7 Oct 2016 08:18:22 -   1.4
+++ net/if_switch.h 13 Oct 2016 16:08:49 -
@@ -216,6 +216,9 @@ void switch_port_egress(struct switch_s
 int switch_swfcl_dup(struct switch_flow_classify *,
struct switch_flow_classify *);
 voidswitch_swfcl_free(struct switch_flow_classify *);
+struct mbuf
+   *switch_flow_classifier(struct mbuf *, uint32_t,
+   struct switch_flow_classify *);
 
 /* switchctl.c */
 voidswitch_dev_destroy(struct switch_softc *);
Index: net/switchofp.c
===
RCS file: /home/obsdcvs/src/sys/net/switchofp.c,v
retrieving revision 1.13
diff -u -p -r1.13 switchofp.c
--- net/switchofp.c 12 Oct 2016 09:50:55 -  1.13
+++ net/switchofp.c 14 Oct 2016 10:22:53 -
@@ -5080,7 +5080,7 @@ swofp_recv_packet_out(struct switch_soft
pout = mtod(m, struct ofp_packet_out *);
 
al_start = offsetof(struct ofp_packet_out, pout_actions);
-   if (pout->pout_buffer_id != OFP_CONTROLLER_MAXLEN_NO_BUFFER) {
+   if (pout->pout_buffer_id == OFP_PKTOUT_NO_BUFFER) {
/*
 * It's not necessary to deep copy at here because it's done
 * in m_dup_pkt().
@@ -5098,6 +5098,17 @@ swofp_recv_packet_out(struct switch_soft
}
 
mc = mcn;
+   } else {
+   /* TODO We don't do buffering yet. */
+   swofp_send_error(sc, m, OFP_ERRTYPE_BAD_REQUEST,
+   OFP_ERRREQ_BUFFER_UNKNOWN);
+   return (0);
+   }
+
+   mc = switch_flow_classifier(mc, pout->pout_in_port, );
+   if (mc == NULL) {
+   m_freem(m);
+   return (0);
}
 
TAILQ_INIT(_fwdp_q);

Re: /usr/src beforeinstall: make prereq as BUILDUSER

[Martin Natano]

On Thu, Oct 13, 2016 at 02:48:07AM +0200, Theo Buehler wrote:
> On Wed, Oct 12, 2016 at 10:30:26PM +0200, Theo Buehler wrote:
> > Several people noticed that a few files from libstdc++-v3 in /usr/obj
> > end up being owned by root, namely:
> > 
> > c++config.h gthr.h gthr-single.h gthr-posix.h gthr-tpf.h gthr-default.h
> > 
> > The problem is that they are regenerated by root when beforeinstall does
> > 'make includes' directly from /usr/src/includes.  A cheap fix is to call
> > make includes from the main Makefile, which was already fixed to
> > de-escalate the prereq step.
> > 
> > Fixing that directly in gnu/lib/libstdc++-v3/Makefile would also be
> > possible, but it seems to be quite a bit messier since one would have to
> > touch quite a few targets to do the proper de-escalation.
> > 
> > With that, /usr/obj contains no root-owned files after 'make build'.
> > 
> 
> Here's an improved version of the patch that doesn't interrupt
> 'make release' by asking for BUILDUSER's password because of trying
> to do 'sh -c ${BUILDUSER}' as BUILDUSER.
> 
> Index: Makefile
> ===
> RCS file: /var/cvs/src/Makefile,v
> retrieving revision 1.129
> diff -u -p -r1.129 Makefile
> --- Makefile  6 Oct 2016 18:56:17 -   1.129
> +++ Makefile  13 Oct 2016 00:19:13 -
> @@ -57,7 +57,11 @@ includes:
>  beforeinstall:
>   cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} distrib-dirs
>   cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} install-mtree
> - cd ${.CURDIR}/include && exec ${MAKE} includes
> + @if [[ `id -u` -ne 0 ]]; then \
> + cd ${.CURDIR}/include && exec ${MAKE} includes; \
> + else \
> + exec ${MAKE} includes; \
> + fi

I think that should be

exec ${MAKE} includes

with the conditional in the 'includes' target. Like this (untested):

includes:
cd ${.CURDIR}/include && \
if [[ `id -u` -eq 0 ]]; then \
su ${BUILDUSER} -c 'exec ${MAKE} prereq'; \
else \
exec ${MAKE} prereq; \
fi && \
exec ${MAKE} includes

That way 'prereq' is always run, also when started as unprivileged user,
and the 'includes' target can be started as BUILDUSER when DESTDIR is
pointing inside a noperm FS.

>  
>  afterinstall:
>  .ifndef NOMAN
> 

Re: vmm: experimentation with networking on wifi interfaces

[Edd Barrett]

Hi,

On Fri, Oct 14, 2016 at 12:56:09AM +0200, Reyk Floeter wrote:
> Using vether in a bridge with the tap allows you to preconfigure a
> fixed interface on the host that doesn't depend on the vm state.

Ah. Yes, and this actually fixes the quirks I mentioned. Using tap
directly is actually *more* complicated.

> I will also add an option for setting the switch in vmctl start.

Thanks

> Now go ahead and configure vether0 as you configure tap0 below... I'm
> usually running dhcpd on vether0, but you can also use fixed IPs of
> course.

Yep, this just worked. Thanks for your help.

You just have to remember to change the DNS server addresses if you move
the host machine between different networks (in /etc/resolv.conf for a
static IP guest, or in the host DHCP server if the guest uses DHCP).

(At some point I'm going to start making an FAQ entry covering the
common vmm cases.)

-- 
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Re: show bgp unknown attributes in bgpctl and tcpdump

[Claudio Jeker]

On Fri, Oct 14, 2016 at 10:44:33AM +0200, Peter Hessler wrote:
> While working on Large Communities, I realized that I would really like
> to easily see and know when I am receiving "unknown" attributes.
> 
> Patch for tcpdump is easy, if it doesn't have a decoder, just print the
> type and length.  You can use -X to see the raw hex.
> 
> Path for bgpctl is a bit more involved.  This shows us the type, flags,
> len and if len is not zero, the local-endian hex dump.
> 
> OK?
> 
> 
> Example tcpdump:
>  BGP (UPDATE: (Path attributes: (ORIGIN[T] IGP)
> (AS_PATH[T] 65000)
> (NEXT_HOP[T] 192.168.50.62)
> (#30[OTP] unknown type 30 len:24))
> (NLRI: 1.2.3.4/32)) (DF) (ttl 64, id 14847, len 127)
> 
> Example bgpctl:
> 
> BGP routing table entry for 72.10.114.0/24
> 29140 2603 11164 22742
> Nexthop 172.16.255.1 (via 172.16.255.1) from 172.16.255.1 (217.31.95.174)
> Origin IGP, metric 0, localpref 100, weight 0, external, valid, best
> Last update: 2d15h36m ago
> Communities: 2603:302 2603:501 2603:11164 2603:64110 2603:64113 
> 11164:1160 11164:7500 11164:51240 11164:52100 11164:52200
> Ext. communities: rt 2603:43432, rt 22742:777
> Unknown Attribute #20 flags [OTP] len 14: 00 01 00 00 58 d6 00 00 02 8e 
> cf d2 8d b5
> 
> 
> 
> Index: usr.sbin/tcpdump/print-bgp.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/tcpdump/print-bgp.c,v
> retrieving revision 1.19
> diff -u -p -u -p -r1.19 print-bgp.c
> --- usr.sbin/tcpdump/print-bgp.c  13 Oct 2016 08:48:15 -  1.19
> +++ usr.sbin/tcpdump/print-bgp.c  14 Oct 2016 08:39:04 -
> @@ -713,6 +713,7 @@ bgp_attr_print(const struct bgp_attr *at
>   }
>   break;
>   default:
> + printf(" unknown type %u len %u", attr->bgpa_type, len);
>   break;
>   }
>   return 1;
> Index: usr.sbin/bgpctl/bgpctl.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.188
> diff -u -p -u -p -r1.188 bgpctl.c
> --- usr.sbin/bgpctl/bgpctl.c  3 Jun 2016 17:36:37 -   1.188
> +++ usr.sbin/bgpctl/bgpctl.c  14 Oct 2016 08:30:44 -
> @@ -1393,6 +1393,7 @@ show_attr(void *b, u_int16_t len)
>   u_int32_tas;
>   u_int16_talen, ioff;
>   u_int8_t flags, type;
> + int  i;
>  
>   if (len < 3)
>   errx(1, "show_attr: too short bgp attr");
> @@ -1448,8 +1449,29 @@ show_attr(void *b, u_int16_t len)
>   show_ext_community(data, alen);
>   printf("\n");
>   break;
> + case ATTR_ATOMIC_AGGREGATE:
> + /* ignore */
> + break;
>   default:
>   /* ignore unknown attributes */
> + printf("Unknown Attribute #%u", type);
> + if (flags) {
> + printf(" flags [");
> + if (flags & ATTR_OPTIONAL)
> + printf("O");
> + if (flags & ATTR_TRANSITIVE)
> + printf("T");
> + if (flags & ATTR_PARTIAL)
> + printf("P");
> + printf("]");
> + }
> + printf(" len %u", alen);
> + if (alen) {
> + printf(":");
> + for (i=0; i < alen; i++)
> + printf(" %02x", *(data+i) & 0xFF);
> + }
> + printf("\n");
>   break;
>   }
>  }
> 
> 

bgpctl diff OK claudio@. For tcpdump I think it is a good start but it
would be nice to dump more info as well (like the flags and maybe even the
data).

-- 
:wq Claudio

Re: rebound case randomization

[Stuart Henderson]

On 2016/10/13 22:55, Ted Unangst wrote:
> 16 bit IDs don't offer much security. This is well known. A trick to encode
> more bits into the query is to vary the case of the query name. It's case
> insensitive, but all known servers echo it back exactly, case preserving.

Unfortunately not. Many do but there are some cases, especially with
things like global-loadbalancer DNS servers, and firewalls doing DNS
content inspection where there are problems (either for all records, or
some records especially in-addr.arpa).

Unbound had to add fallbacks for this (see the 'caps_fallback' bits in
iterator/iterator.c). Some strategies for this are discussed in
draft-vixie-dnsext-dns0x20-00.

show bgp unknown attributes in bgpctl and tcpdump

[Peter Hessler]

While working on Large Communities, I realized that I would really like
to easily see and know when I am receiving "unknown" attributes.

Patch for tcpdump is easy, if it doesn't have a decoder, just print the
type and length.  You can use -X to see the raw hex.

Path for bgpctl is a bit more involved.  This shows us the type, flags,
len and if len is not zero, the local-endian hex dump.

OK?


Example tcpdump:
 BGP (UPDATE: (Path attributes: (ORIGIN[T] IGP)
(AS_PATH[T] 65000)
(NEXT_HOP[T] 192.168.50.62)
(#30[OTP] unknown type 30 len:24))
(NLRI: 1.2.3.4/32)) (DF) (ttl 64, id 14847, len 127)

Example bgpctl:

BGP routing table entry for 72.10.114.0/24
29140 2603 11164 22742
Nexthop 172.16.255.1 (via 172.16.255.1) from 172.16.255.1 (217.31.95.174)
Origin IGP, metric 0, localpref 100, weight 0, external, valid, best
Last update: 2d15h36m ago
Communities: 2603:302 2603:501 2603:11164 2603:64110 2603:64113 11164:1160 
11164:7500 11164:51240 11164:52100 11164:52200
Ext. communities: rt 2603:43432, rt 22742:777
Unknown Attribute #20 flags [OTP] len 14: 00 01 00 00 58 d6 00 00 02 8e cf 
d2 8d b5



Index: usr.sbin/tcpdump/print-bgp.c
===
RCS file: /cvs/openbsd/src/usr.sbin/tcpdump/print-bgp.c,v
retrieving revision 1.19
diff -u -p -u -p -r1.19 print-bgp.c
--- usr.sbin/tcpdump/print-bgp.c13 Oct 2016 08:48:15 -  1.19
+++ usr.sbin/tcpdump/print-bgp.c14 Oct 2016 08:39:04 -
@@ -713,6 +713,7 @@ bgp_attr_print(const struct bgp_attr *at
}
break;
default:
+   printf(" unknown type %u len %u", attr->bgpa_type, len);
break;
}
return 1;
Index: usr.sbin/bgpctl/bgpctl.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.188
diff -u -p -u -p -r1.188 bgpctl.c
--- usr.sbin/bgpctl/bgpctl.c3 Jun 2016 17:36:37 -   1.188
+++ usr.sbin/bgpctl/bgpctl.c14 Oct 2016 08:30:44 -
@@ -1393,6 +1393,7 @@ show_attr(void *b, u_int16_t len)
u_int32_tas;
u_int16_talen, ioff;
u_int8_t flags, type;
+   int  i;
 
if (len < 3)
errx(1, "show_attr: too short bgp attr");
@@ -1448,8 +1449,29 @@ show_attr(void *b, u_int16_t len)
show_ext_community(data, alen);
printf("\n");
break;
+   case ATTR_ATOMIC_AGGREGATE:
+   /* ignore */
+   break;
default:
/* ignore unknown attributes */
+   printf("Unknown Attribute #%u", type);
+   if (flags) {
+   printf(" flags [");
+   if (flags & ATTR_OPTIONAL)
+   printf("O");
+   if (flags & ATTR_TRANSITIVE)
+   printf("T");
+   if (flags & ATTR_PARTIAL)
+   printf("P");
+   printf("]");
+   }
+   printf(" len %u", alen);
+   if (alen) {
+   printf(":");
+   for (i=0; i < alen; i++)
+   printf(" %02x", *(data+i) & 0xFF);
+   }
+   printf("\n");
break;
}
 }


-- 
Hardware, n.:
The parts of a computer system that can be kicked.

Re: Help me testing the netlock

[Simon Mages]

Hi,

i did some performace tests in current with and without your diff.

There is no difference in performance.

I will try to do performance tests with current on a regular base now.

2016-10-05 10:15 GMT+02:00, Martin Pieuchot :
> On 10/04/16 16:44, Martin Pieuchot wrote:
>> On 10/03/16 16:43, Martin Pieuchot wrote:
>>> Diff below introduces a single write lock that will be used to serialize
>>> access to ip_output().
>>>
>>> This lock will be then split in multiple readers and writers to allow
>>> multiple forwarding paths to run in parallel of each others but still
>>> serialized with the socket layer.
>>>
>>> I'm currently looking for people wanting to run this diff and try to
>>> break it.  In other words, your machine might panic with it and if it
>>> does report the panic to me so the diff can be improved.
>>>
>>> I tested NFS v2 and v3 so I'm quite confident, but I might have missed
>>> some obvious stuff.
>>
>> Updated diff attaced including a fix for syn_cache_timer(), problem
>> reported by Chris Jackman.
>
> Thanks to all testers!
>
> Here's a newer version that includes a fix for rt_timer_timer() also
> found by Chris Jackman.
>
>

Re: remove unused flags from the cp.c inside mv(1)

[Jan Stary]

ping

On Oct 10 21:39:16, h...@stare.cz wrote:
> The embedded cpmain() will never have _any_ flags set,
> as mv.c calls it as
> 
>   argv[0] = from;
>   argv[1] = to;
>   argv[2] = NULL;
>   cpmain(2, argv);
> 
> There is probably more code that could be romoved
> form the embedded cp.c, along the lines of tedu's recent
> cleanup of the embedded rm.c
> 
>   Jan
> 
> 
> Index: cp.c
> ===
> RCS file: /cvs/src/bin/mv/cp.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 cp.c
> --- cp.c  27 Dec 2015 01:25:57 -  1.7
> +++ cp.c  10 Oct 2016 19:27:02 -
> @@ -84,7 +84,6 @@ static int setfile(struct stat *, in
>  extern char *__progname;
>  
>  static uid_t myuid;
> -static int fflag, iflag;
>  static mode_t myumask;
>  
>  enum op { FILE_TO_FILE, FILE_TO_DIR, DIR_TO_DNE };
> @@ -463,13 +462,6 @@ copy_file(FTSENT *entp, int dne)
>   fs = entp->fts_statp;
>  
>   /*
> -  * In -f (force) mode, we always unlink the destination first
> -  * if it exists.  Note that -i and -f are mutually exclusive.
> -  */
> - if (!dne && fflag)
> - (void)unlink(to.p_path);
> -
> - /*
>* If the file exists and we're interactive, verify with the user.
>* If the file DNE, set the mode to be the from file, minus setuid
>* bits, modified by the umask; arguably wrong, but it makes copying
> @@ -477,17 +469,7 @@ copy_file(FTSENT *entp, int dne)
>* other choice is 666 or'ed with the execute bits on the from file
>* modified by the umask.)
>*/
> - if (!dne && !fflag) {
> - if (iflag) {
> - (void)fprintf(stderr, "overwrite %s? ", to.p_path);
> - checkch = ch = getchar();
> - while (ch != '\n' && ch != EOF)
> - ch = getchar();
> - if (checkch != 'y' && checkch != 'Y') {
> - (void)close(from_fd);
> - return (0);
> - }
> - }
> + if (!dne) {
>   to_fd = open(to.p_path, O_WRONLY | O_TRUNC, 0);
>   } else
>   to_fd = open(to.p_path, O_WRONLY | O_TRUNC | O_CREAT,
> 

use x2apic if it is enabled by BIOS

[YASUOKA Masahiko]

Hi,

I'm working on NEC Express5800/R110h-1 (dmesg is attached).  On this
machine, our kernel panics with following message.

  cpu0 at mainbus0panic: cpu at apic id 0 already attached?

This seems to happen since x2APIC on the machine is enabled by BIOS
and the kernel doesn't assume that.  The diff makes the kernel use
x2APIC if it is enabled by BIOS.

ok?

Index: sys/arch/amd64/amd64/lapic.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.44
diff -u -p -r1.44 lapic.c
--- sys/arch/amd64/amd64/lapic.c22 Jun 2016 01:12:38 -  1.44
+++ sys/arch/amd64/amd64/lapic.c14 Oct 2016 07:45:50 -
@@ -170,60 +170,55 @@ lapic_map(paddr_t lapic_base)
int s;
pt_entry_t *pte;
vaddr_t va;
+   u_int64_t msr;
 
-   /*
-* On real hardware, x2apic must only be enabled if interrupt remapping
-* is also enabled. See 10.12.7 of the SDM vol 3.
-* On hypervisors, this is not necessary. Hypervisors can implement
-* x2apic support even if the host CPU does not support it.
-* Until we support interrupt remapping, use x2apic only if the
-* hypervisor flag is also set.
-*/
-   if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_HV)) {
-   u_int64_t msr;
-
-   disable_intr();
-   s = lapic_tpr;
-
-   msr = rdmsr(MSR_APICBASE);
-   msr |= APICBASE_ENABLE_X2APIC;
-   wrmsr(MSR_APICBASE, msr);
+   disable_intr();
+   s = lapic_tpr;
+
+   msr = rdmsr(MSR_APICBASE);
 
+   if (ISSET(msr, APICBASE_ENABLE_X2APIC) ||
+   (ISSET(cpu_ecxfeature, CPUIDECX_HV) &&
+   ISSET(cpu_ecxfeature, CPUIDECX_X2APIC))) {
+   /*
+* If x2APIC is enabled already, use it since it is enabled
+* by BIOS.  On hypervisor, use it if it exists.
+*/
+   if (!ISSET(msr, APICBASE_ENABLE_X2APIC)) {
+   msr |= APICBASE_ENABLE_X2APIC;
+   wrmsr(MSR_APICBASE, msr);
+   }
lapic_readreg = x2apic_readreg;
lapic_writereg = x2apic_writereg;
 #ifdef MULTIPROCESSOR
x86_ipi = x2apic_ipi;
 #endif
x2apic_enabled = 1;
-
codepatch_call(CPTAG_EOI, _eoi);
 
lapic_writereg(LAPIC_TPRI, s);
-   enable_intr();
+   } else {
+   /*
+* Map local apic.  If we have a local apic, it's safe to
+* assume we're on a 486 or better and can use invlpg and
+* non-cacheable PTE's
+*
+* Whap the PTE "by hand" rather than calling pmap_kenter_pa
+* because the latter will attempt to invoke TLB shootdown
+* code just as we might have changed the value of
+* cpu_number()..
+*/
+   va = (vaddr_t)_apic;
+   pte = kvtopte(va);
+   *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
+   invlpg(va);
 
-   return;
+   lapic_tpr = s;
}
 
-   va = (vaddr_t)_apic;
-
-   disable_intr();
-   s = lapic_tpr;
-
-   /*
-* Map local apic.  If we have a local apic, it's safe to assume
-* we're on a 486 or better and can use invlpg and non-cacheable PTE's
-*
-* Whap the PTE "by hand" rather than calling pmap_kenter_pa because
-* the latter will attempt to invoke TLB shootdown code just as we
-* might have changed the value of cpu_number()..
-*/
-
-   pte = kvtopte(va);
-   *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
-   invlpg(va);
-
-   lapic_tpr = s;
enable_intr();
+
+   return;
 }
 
 /*

OpenBSD 6.0-current (GENERIC.MP) #44: Fri Oct 14 16:32:03 JST 2016

yasu...@yasuoka-ob1.tokyo.iiji.jp:/source/yasuoka/openbsd/head/git/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8207699968 (7827MB)
avail mem = 7954391040 (7585MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f92b000 (60 entries)
bios0: vendor American Megatrends Inc. version "5.0.4012" date 06/15/2016
bios0: NEC Express5800/R110h-1 [N8100-2316Y]
acpi0 at bios0: rev 2
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC FIDT MCFG HPET SSDT SSDT SSDT SSDT SPCR DMAR BERT
acpi0: wakeup devices PEGP(S4) PEG0(S4) PEG1(S4) PEG2(S4) PXSX(S4) RP17(S4) 
PXSX(S4) RP18(S4) PXSX(S4) RP19(S4) PXSX(S4) RP20(S4) PXSX(S4) RP01(S4) 
PXSX(S4) RP02(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Xeon(R) CPU E3-1220 v5 @ 3.00GHz, 3301.20 MHz
cpu0: 

remove KOI8 after 5.9 is out

[Jan Stary]

Index: io.c
===
RCS file: /cvs/src/usr.bin/calendar/io.c,v
retrieving revision 1.44
diff -u -p -r1.44 io.c
--- io.c31 Aug 2016 09:38:47 -  1.44
+++ io.c14 Oct 2016 07:27:52 -
@@ -89,13 +89,9 @@ cal(void)
if (strncmp(buf, "LANG=", 5) == 0) {
(void) setlocale(LC_ALL, buf + 5);
setnnames();
-   /* XXX remove KOI8 lines after 5.9 is out */
if (!strcmp(buf + 5, "ru_RU.UTF-8") ||
!strcmp(buf + 5, "uk_UA.UTF-8") ||
-   !strcmp(buf + 5, "by_BY.UTF-8") ||
-   !strcmp(buf + 5, "ru_RU.KOI8-R") ||
-   !strcmp(buf + 5, "uk_UA.KOI8-U") ||
-   !strcmp(buf + 5, "by_BY.KOI8-B")) {
+   !strcmp(buf + 5, "by_BY.UTF-8")) {
bodun_maybe++;
bodun = 0;
free(prefix);

Re: mcl2k2 mbuf clusters

[David Gwynne]

> On 14 Oct 2016, at 16:17, Ted Unangst  wrote:
> 
> David Gwynne wrote:
>> this adds a pool backend for MCLGETI thats 2k+2 bytes in size, which
>> can be used on some very common nics that have annoying constraints
>> on their rx descriptors.
>> 
>> this in turn simplifies the code in those drivers and lets them
>> always operate on ETHER_ALIGN boundaries.
>> 
>> the pool is cheap, pages will only be allocated in it if something
>> asks for them, and it keeps this complexity out of the drivers.
> 
> pool is efficient, but it can't work miracles. on archs like amd64, this is
> going to only pack 15 clusters in a 32k "page" instead of 16.

it is only about 70 bytes per cluster though. i could raise the size by that to 
avoid slack space if you want, or we can let the page colouring have some fun 
here.

> 
> if i understand correctly, currently em is using 4k clusters on strict
> alignment? i think it makes a lot of sense to add a 2k2 pool because that
> means using less memory. but not strict alignment shouldn't be forced to use
> more memory. that's a regression.

you could also argue that 2k clusters waste ~500 bytes in the most common case 
since the vast majority of large packets are still only 1514 bytes. we could 
pack 10 1600 byte clusters into 16k instead of the 8 we do now.

more seriously though, im happy to burn a bit more memory to keep the code 
simple, and overall our memory consumption by network cards is much lower than 
other platforms because of the rx ring moderation we do.

dlg

> 
> 
>> 
>> ok?
>> 
>> Index: net/if.h
>> ===
>> RCS file: /cvs/src/sys/net/if.h,v
>> retrieving revision 1.179
>> diff -u -p -r1.179 if.h
>> --- net/if.h 4 Sep 2016 15:10:59 -   1.179
>> +++ net/if.h 14 Oct 2016 03:46:22 -
>> @@ -68,7 +68,7 @@ struct if_clonereq {
>>  char*ifcr_buffer;   /* buffer for cloner names */
>> };
>> 
>> -#define MCLPOOLS7   /* number of cluster pools */
>> +#define MCLPOOLS8   /* number of cluster pools */
>> 
>> struct if_rxring {
>>  int rxr_adjusted;
>> Index: kern/uipc_mbuf.c
>> ===
>> RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
>> retrieving revision 1.233
>> diff -u -p -r1.233 uipc_mbuf.c
>> --- kern/uipc_mbuf.c 10 Oct 2016 00:41:17 -  1.233
>> +++ kern/uipc_mbuf.c 14 Oct 2016 03:46:22 -
>> @@ -107,6 +107,7 @@ struct   pool mtagpool;
>> /* mbuf cluster pools */
>> u_intmclsizes[MCLPOOLS] = {
>>  MCLBYTES,   /* must be at slot 0 */
>> +MCLBYTES + 2,   /* ETHER_ALIGNED 2k mbufs */
>>  4 * 1024,
>>  8 * 1024,
>>  9 * 1024,
>> @@ -142,6 +143,7 @@ void
>> mbinit(void)
>> {
>>  int i;
>> +unsigned int lowbits;
>> 
>> #if DIAGNOSTIC
>>  if (mclsizes[0] != MCLBYTES)
>> @@ -158,9 +160,15 @@ mbinit(void)
>>  IPL_NET, 0, "mtagpl", NULL);
>> 
>>  for (i = 0; i < nitems(mclsizes); i++) {
>> -snprintf(mclnames[i], sizeof(mclnames[0]), "mcl%dk",
>> -mclsizes[i] >> 10);
>> -pool_init([i], mclsizes[i], 0, IPL_NET, 0,
>> +lowbits = mclsizes[i] & ((1 << 10) - 1);
>> +if (lowbits) {
>> +snprintf(mclnames[i], sizeof(mclnames[0]),
>> +"mcl%dk%u", mclsizes[i] >> 10, lowbits);
>> +} else {
>> +snprintf(mclnames[i], sizeof(mclnames[0]), "mcl%dk",
>> +mclsizes[i] >> 10);
>> +}
>> +pool_init([i], mclsizes[i], 64, IPL_NET, 0,
>>  mclnames[i], NULL);
>>  pool_set_constraints([i], _dma_contig);
>>  pool_setlowat([i], mcllowat);
>> Index: dev/pci/if_em.c
>> ===
>> RCS file: /cvs/src/sys/dev/pci/if_em.c,v
>> retrieving revision 1.331
>> diff -u -p -r1.331 if_em.c
>> --- dev/pci/if_em.c  13 Apr 2016 10:34:32 -  1.331
>> +++ dev/pci/if_em.c  14 Oct 2016 03:47:21 -
>> @@ -2450,9 +2450,7 @@ em_get_buf(struct em_softc *sc, int i)
>>  return (ENOBUFS);
>>  }
>>  m->m_len = m->m_pkthdr.len = EM_MCLBYTES;
>> -#ifdef __STRICT_ALIGNMENT
>>  m_adj(m, ETHER_ALIGN);
>> -#endif
>> 
>>  error = bus_dmamap_load_mbuf(sc->sc_dmat, pkt->pkt_map,
>>  m, BUS_DMA_NOWAIT);
>> Index: dev/pci/if_ix.c
>> ===
>> RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
>> retrieving revision 1.132
>> diff -u -p -r1.132 if_ix.c
>> --- dev/pci/if_ix.c  13 Apr 2016 10:34:32 -  1.132
>> +++ dev/pci/if_ix.c  14 Oct 2016 03:47:21 -
>> @@ -616,11 +616,7 @@ ixgbe_init(void *arg)
>>  ixgbe_initialize_transmit_units(sc);
>> 
>>  /* Use 2k clusters, even for jumbo frames */
>> -#ifdef __STRICT_ALIGNMENT
>>  sc->rx_mbuf_sz = MCLBYTES + ETHER_ALIGN;
>> -#else
>> -   

Re: acpiec on acer aspire S7 with CURRENT

[Paul Irofti]

> Which acpi table should I attach?

Use sendbug(1). See also

http://www.openbsd.org/report.html#bugtypes

Re: mcl2k2 mbuf clusters

[Ted Unangst]

David Gwynne wrote:
> this adds a pool backend for MCLGETI thats 2k+2 bytes in size, which
> can be used on some very common nics that have annoying constraints
> on their rx descriptors.
> 
> this in turn simplifies the code in those drivers and lets them
> always operate on ETHER_ALIGN boundaries.
> 
> the pool is cheap, pages will only be allocated in it if something
> asks for them, and it keeps this complexity out of the drivers.

pool is efficient, but it can't work miracles. on archs like amd64, this is
going to only pack 15 clusters in a 32k "page" instead of 16.

if i understand correctly, currently em is using 4k clusters on strict
alignment? i think it makes a lot of sense to add a 2k2 pool because that
means using less memory. but not strict alignment shouldn't be forced to use
more memory. that's a regression.


> 
> ok?
> 
> Index: net/if.h
> ===
> RCS file: /cvs/src/sys/net/if.h,v
> retrieving revision 1.179
> diff -u -p -r1.179 if.h
> --- net/if.h  4 Sep 2016 15:10:59 -   1.179
> +++ net/if.h  14 Oct 2016 03:46:22 -
> @@ -68,7 +68,7 @@ struct if_clonereq {
>   char*ifcr_buffer;   /* buffer for cloner names */
>  };
>  
> -#define MCLPOOLS 7   /* number of cluster pools */
> +#define MCLPOOLS 8   /* number of cluster pools */
>  
>  struct if_rxring {
>   int rxr_adjusted;
> Index: kern/uipc_mbuf.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
> retrieving revision 1.233
> diff -u -p -r1.233 uipc_mbuf.c
> --- kern/uipc_mbuf.c  10 Oct 2016 00:41:17 -  1.233
> +++ kern/uipc_mbuf.c  14 Oct 2016 03:46:22 -
> @@ -107,6 +107,7 @@ structpool mtagpool;
>  /* mbuf cluster pools */
>  u_intmclsizes[MCLPOOLS] = {
>   MCLBYTES,   /* must be at slot 0 */
> + MCLBYTES + 2,   /* ETHER_ALIGNED 2k mbufs */
>   4 * 1024,
>   8 * 1024,
>   9 * 1024,
> @@ -142,6 +143,7 @@ void
>  mbinit(void)
>  {
>   int i;
> + unsigned int lowbits;
>  
>  #if DIAGNOSTIC
>   if (mclsizes[0] != MCLBYTES)
> @@ -158,9 +160,15 @@ mbinit(void)
>   IPL_NET, 0, "mtagpl", NULL);
>  
>   for (i = 0; i < nitems(mclsizes); i++) {
> - snprintf(mclnames[i], sizeof(mclnames[0]), "mcl%dk",
> - mclsizes[i] >> 10);
> - pool_init([i], mclsizes[i], 0, IPL_NET, 0,
> + lowbits = mclsizes[i] & ((1 << 10) - 1);
> + if (lowbits) {
> + snprintf(mclnames[i], sizeof(mclnames[0]),
> + "mcl%dk%u", mclsizes[i] >> 10, lowbits);
> + } else {
> + snprintf(mclnames[i], sizeof(mclnames[0]), "mcl%dk",
> + mclsizes[i] >> 10);
> + }
> + pool_init([i], mclsizes[i], 64, IPL_NET, 0,
>   mclnames[i], NULL);
>   pool_set_constraints([i], _dma_contig);
>   pool_setlowat([i], mcllowat);
> Index: dev/pci/if_em.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_em.c,v
> retrieving revision 1.331
> diff -u -p -r1.331 if_em.c
> --- dev/pci/if_em.c   13 Apr 2016 10:34:32 -  1.331
> +++ dev/pci/if_em.c   14 Oct 2016 03:47:21 -
> @@ -2450,9 +2450,7 @@ em_get_buf(struct em_softc *sc, int i)
>   return (ENOBUFS);
>   }
>   m->m_len = m->m_pkthdr.len = EM_MCLBYTES;
> -#ifdef __STRICT_ALIGNMENT
>   m_adj(m, ETHER_ALIGN);
> -#endif
>  
>   error = bus_dmamap_load_mbuf(sc->sc_dmat, pkt->pkt_map,
>   m, BUS_DMA_NOWAIT);
> Index: dev/pci/if_ix.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
> retrieving revision 1.132
> diff -u -p -r1.132 if_ix.c
> --- dev/pci/if_ix.c   13 Apr 2016 10:34:32 -  1.132
> +++ dev/pci/if_ix.c   14 Oct 2016 03:47:21 -
> @@ -616,11 +616,7 @@ ixgbe_init(void *arg)
>   ixgbe_initialize_transmit_units(sc);
>  
>   /* Use 2k clusters, even for jumbo frames */
> -#ifdef __STRICT_ALIGNMENT
>   sc->rx_mbuf_sz = MCLBYTES + ETHER_ALIGN;
> -#else
> - sc->rx_mbuf_sz = MCLBYTES;
> -#endif
>  
>   /* Prepare receive descriptors and buffers */
>   if (ixgbe_setup_receive_structures(sc)) {
> @@ -2458,9 +2454,7 @@ ixgbe_get_buf(struct rx_ring *rxr, int i
>   return (ENOBUFS);
>  
>   mp->m_len = mp->m_pkthdr.len = sc->rx_mbuf_sz;
> -#ifdef __STRICT_ALIGNMENT
>   m_adj(mp, ETHER_ALIGN);
> -#endif
>  
>   error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->map,
>   mp, BUS_DMA_NOWAIT);
> @@ -2667,11 +2661,7 @@ ixgbe_initialize_receive_units(struct ix
>   hlreg |= IXGBE_HLREG0_JUMBOEN;
>   IXGBE_WRITE_REG(>hw, IXGBE_HLREG0, hlreg);
>  
> -#ifdef __STRICT_ALIGNMENT
>   bufsz = (sc->rx_mbuf_sz - ETHER_ALIGN) >>