Re: Larger kernel fonts in RAMDISK_CD?

2021-06-02 Thread Theo de Raadt
I am satisfied.

That is one architecture.  I suggest checking which others can use
the same treatment.


Frederic Cambus  wrote:

> On Mon, May 31, 2021 at 12:57:47PM +0200, Mark Kettenis wrote:
> 
> > > +option   FONT_SPLEEN8x16
> > > +option   FONT_SPLEEN12x24
> > > +option   FONT_SPLEEN16x32
> > > +option   FONT_SPLEEN32x64
> > > +
> > >  option   RAMDISK_HOOKS
> > >  option   MINIROOTSIZE=7360
> > >  
> > > Does this look reasonable?
> > 
> > I would skip some sizes.  8x16 is readable on any screen size where
> > 12x24 would be picked.  And maybe 16x32 is good enough for 4K screens
> > as well?
> 
> Right, the 16x32 variant is small but readable on my 27" 4k monitor, so
> it sounds reasonable to skip the 32x64 version for now. For the 12x24
> version I agree that it's not stricly necessary, just more comfortable.
> > 
> > > If it does and if we want to go this way, I can try to build a release
> > > and check if MINIROOTSIZE must be bumped on RAMDISK_CD. Then we could do
> > > the same for i386, armv7 and arm64.
> > 
> > I'm all for it, but last time this came up Theo didn't like it and
> > suggested adding code to scale up the fonts instead.  I really don't
> > think you want to upscale the 8x16 font to 32x64.  But if we add the
> > 16x32 font, upscaling that to 32x64 for the really big screens might
> > be an option and a reasonable compromise?
> 
> Indeed, if we want to go this way, I believe upscaling the 16x32 version
> to 32x64 would give good enough results on 4k screens.
> 
> > But figuring out how much things grow by adding the 16x32 font would
> > be a good start.
> 
> So I built three miniroot images using the same source tree, and
> results are below. There was no need to bump MINIROOTSIZE.
> 
> Result of running df -h on mounted vnode disks:
> 
> /dev/vnd0a 4.3M4.1M146K97%/mnt0
> /dev/vnd1a 4.3M4.1M146K97%/mnt1
> /dev/vnd2a 4.3M4.1M146K97%/mnt2
> 
> Sizes (in bytes) of compressed kernels extracted from miniroot images:
> 
> -r-xr-xr-x  1 root  wheel  4215406 Jun  1 14:47 bsd (baseline)
> -r-xr-xr-x  1 root  wheel  4217267 Jun  1 15:33 bsd (+Spleen 16x32)
> -r-xr-xr-x  1 root  wheel  4218986 Jun  1 20:26 bsd (+Spleen 12x24 and 16x32)
> 
> Size difference from baseline:
> 
> With Spleen 16x32:   +1861 bytes
> With Spleen 12x24 and 16x32: +3580 bytes
> 
> I would be fine with adding only the 16x32 version, but still provided
> numbers for the 12x24 in case we decide the size difference is small
> enough for its addition to be worthwhile.
> 



Re: 10gbase-r support for mvpp(4)

2021-06-02 Thread Patrick Wildt
Am Wed, Jun 02, 2021 at 10:37:36PM +0200 schrieb Mark Kettenis:
> Linux folks changed the device tree to use 10gbase-r instead of
> 10gbase-kr since "it is more correct".  Then the UEFI folks synched
> their device trees to Linux and the 10G ports broke.  So accept both
> in the code.
> 
> ok?
> 

Sigh. sure, ok patrick@.

Though you could probably put it into a single if (!strncmp ||
!strncmp), but I don't mind either way.

> 
> Index: dev/fdt/if_mvpp.c
> ===
> RCS file: /cvs/src/sys/dev/fdt/if_mvpp.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 if_mvpp.c
> --- dev/fdt/if_mvpp.c 12 Dec 2020 11:48:52 -  1.44
> +++ dev/fdt/if_mvpp.c 2 Jun 2021 20:34:30 -
> @@ -1354,7 +1354,9 @@ mvpp2_port_attach(struct device *parent,
>  
>   phy_mode = malloc(len, M_TEMP, M_WAITOK);
>   OF_getprop(sc->sc_node, "phy-mode", phy_mode, len);
> - if (!strncmp(phy_mode, "10gbase-kr", strlen("10gbase-kr")))
> + if (!strncmp(phy_mode, "10gbase-r", strlen("10gbase-r")))
> + sc->sc_phy_mode = PHY_MODE_10GBASER;
> + else if (!strncmp(phy_mode, "10gbase-kr", strlen("10gbase-kr")))
>   sc->sc_phy_mode = PHY_MODE_10GBASER;
>   else if (!strncmp(phy_mode, "2500base-x", strlen("2500base-x")))
>   sc->sc_phy_mode = PHY_MODE_2500BASEX;
> 



Re: Larger kernel fonts in RAMDISK_CD?

2021-06-02 Thread Frederic Cambus
On Mon, May 31, 2021 at 12:57:47PM +0200, Mark Kettenis wrote:

> > +option FONT_SPLEEN8x16
> > +option FONT_SPLEEN12x24
> > +option FONT_SPLEEN16x32
> > +option FONT_SPLEEN32x64
> > +
> >  option RAMDISK_HOOKS
> >  option MINIROOTSIZE=7360
> >  
> > Does this look reasonable?
> 
> I would skip some sizes.  8x16 is readable on any screen size where
> 12x24 would be picked.  And maybe 16x32 is good enough for 4K screens
> as well?

Right, the 16x32 variant is small but readable on my 27" 4k monitor, so
it sounds reasonable to skip the 32x64 version for now. For the 12x24
version I agree that it's not stricly necessary, just more comfortable.
> 
> > If it does and if we want to go this way, I can try to build a release
> > and check if MINIROOTSIZE must be bumped on RAMDISK_CD. Then we could do
> > the same for i386, armv7 and arm64.
> 
> I'm all for it, but last time this came up Theo didn't like it and
> suggested adding code to scale up the fonts instead.  I really don't
> think you want to upscale the 8x16 font to 32x64.  But if we add the
> 16x32 font, upscaling that to 32x64 for the really big screens might
> be an option and a reasonable compromise?

Indeed, if we want to go this way, I believe upscaling the 16x32 version
to 32x64 would give good enough results on 4k screens.

> But figuring out how much things grow by adding the 16x32 font would
> be a good start.

So I built three miniroot images using the same source tree, and
results are below. There was no need to bump MINIROOTSIZE.

Result of running df -h on mounted vnode disks:

/dev/vnd0a 4.3M4.1M146K97%/mnt0
/dev/vnd1a 4.3M4.1M146K97%/mnt1
/dev/vnd2a 4.3M4.1M146K97%/mnt2

Sizes (in bytes) of compressed kernels extracted from miniroot images:

-r-xr-xr-x  1 root  wheel  4215406 Jun  1 14:47 bsd (baseline)
-r-xr-xr-x  1 root  wheel  4217267 Jun  1 15:33 bsd (+Spleen 16x32)
-r-xr-xr-x  1 root  wheel  4218986 Jun  1 20:26 bsd (+Spleen 12x24 and 16x32)

Size difference from baseline:

With Spleen 16x32:   +1861 bytes
With Spleen 12x24 and 16x32: +3580 bytes

I would be fine with adding only the 16x32 version, but still provided
numbers for the 12x24 in case we decide the size difference is small
enough for its addition to be worthwhile.



10gbase-r support for mvpp(4)

2021-06-02 Thread Mark Kettenis
Linux folks changed the device tree to use 10gbase-r instead of
10gbase-kr since "it is more correct".  Then the UEFI folks synched
their device trees to Linux and the 10G ports broke.  So accept both
in the code.

ok?


Index: dev/fdt/if_mvpp.c
===
RCS file: /cvs/src/sys/dev/fdt/if_mvpp.c,v
retrieving revision 1.44
diff -u -p -r1.44 if_mvpp.c
--- dev/fdt/if_mvpp.c   12 Dec 2020 11:48:52 -  1.44
+++ dev/fdt/if_mvpp.c   2 Jun 2021 20:34:30 -
@@ -1354,7 +1354,9 @@ mvpp2_port_attach(struct device *parent,
 
phy_mode = malloc(len, M_TEMP, M_WAITOK);
OF_getprop(sc->sc_node, "phy-mode", phy_mode, len);
-   if (!strncmp(phy_mode, "10gbase-kr", strlen("10gbase-kr")))
+   if (!strncmp(phy_mode, "10gbase-r", strlen("10gbase-r")))
+   sc->sc_phy_mode = PHY_MODE_10GBASER;
+   else if (!strncmp(phy_mode, "10gbase-kr", strlen("10gbase-kr")))
sc->sc_phy_mode = PHY_MODE_10GBASER;
else if (!strncmp(phy_mode, "2500base-x", strlen("2500base-x")))
sc->sc_phy_mode = PHY_MODE_2500BASEX;



Patch: ksh: fix input handling for 4 byte UTF-8 sequences

2021-06-02 Thread Ingo Schwarze
Hi,

feeling hesitant to commit into ksh without at least one proper OK,
i'm resending this patch here, sorry if i missed private feedback.

What the existing code does:
It tries to make sure that multi-byte UTF-8 characters get passed on by
the shell without fragmentation, not one byte at time.  I heard people
say that some software, for example tmux(1), may sometimes get confused
when receiving a UTF-8 character in a piecemeal manner.

Which problem needs fixing:
Of the four-byte UTF-8 sequences, only a subset is identified by the
existing code.  The other four-byte UTF-8 sequences still get chopped
up resulting in individual bytes being passed on.


I'm also adding a few comments as suggested by jca@.  Parsing of UTF-8
is less trivial than one might think, witnessed once again by the fact
that i got this code wrong in the first place.

I also changed "cc & 0x20" to "cc > 0x9f" and "cc & 0x30" to "cc > 0x8f"
for uniformity and readabilty - UTF-8-parsing is bad enough without
needless micro-optimization, right?


Note that even with the patch below, moving backward and forward
over a blowfish icon on the command line still does not work because
the character is width 2 and the ksh code intentionally does not
use wcwidth(3).  But maybe it improves something in tmux?  Not sure.

Either way, unless it causes regressions, this (or a further improved
version) should go in because what is there is clearly wrong.

OK?
  Ingo


Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.87
diff -u -p -r1.87 emacs.c
--- emacs.c 8 May 2020 14:30:42 -   1.87
+++ emacs.c 13 May 2021 18:16:13 -
@@ -1851,11 +1851,17 @@ x_e_getu8(char *buf, int off)
return -1;
buf[off++] = c;
 
-   if (c == 0xf4)
+   /*
+* In the following, comments refer to violations of
+* the inequality tests at the ends of the lines.
+* See the utf8(7) manual page for details.
+*/
+
+   if ((c & 0xf8) == 0xf0 && c < 0xf5)  /* beyond Unicode */
len = 4;
else if ((c & 0xf0) == 0xe0)
len = 3;
-   else if ((c & 0xe0) == 0xc0 && c > 0xc1)
+   else if ((c & 0xe0) == 0xc0 && c > 0xc1)  /* use single byte */
len = 2;
else
len = 1;
@@ -1865,9 +1871,10 @@ x_e_getu8(char *buf, int off)
if (cc == -1)
break;
if (isu8cont(cc) == 0 ||
-   (c == 0xe0 && len == 3 && cc < 0xa0) ||
-   (c == 0xed && len == 3 && cc & 0x20) ||
-   (c == 0xf4 && len == 4 && cc & 0x30)) {
+   (c == 0xe0 && len == 3 && cc < 0xa0) ||  /* use 2 bytes */
+   (c == 0xed && len == 3 && cc > 0x9f) ||  /* surrogates  */
+   (c == 0xf0 && len == 4 && cc < 0x90) ||  /* use 3 bytes */
+   (c == 0xf4 && len == 4 && cc > 0x8f)) {  /* beyond Uni. */
x_e_ungetc(cc);
break;
}



Re: Update vmctl(8) to use TERMINATE_VM_EVENTs

2021-06-02 Thread Dave Voutila


ping

Dave Voutila writes:

> Dave Voutila writes:
>
>> The conclusion of my previous fixes to vmd(8) [1] changes the event
>> handling in vmctl(8) to support receiving IMSG_VMDOP_TERMINATE_VM_EVENTs
>> from the control process. (This removes a XXX comment from vmd.)
>>
>> For clarity, the messaging logic was changed previously:
>>
>> - ...TERMINATE_VM_RESPONSE conveying success/failure of the request to
>>   terminate a guest regardless of waiting for termination
>> - ...TERMINATE_VM_EVENT conveying the actual termination of a guest
>>
>> This diff finishes bringing that logic from vmd(8) to vmctl(8).
>>
>> OK?
>
> Ping. Looking to close this gap.
>
> Note: this diff does preserve some errno abuse in vmd & vmctl that I'm
> working on separately.
>
>>
>> -dv
>>
>>
>> Index: usr.sbin/vmd/control.c
>> ===
>> RCS file: /cvs/src/usr.sbin/vmd/control.c,v
>> retrieving revision 1.35
>> diff -u -p -r1.35 control.c
>> --- usr.sbin/vmd/control.c   26 Apr 2021 22:58:27 -  1.35
>> +++ usr.sbin/vmd/control.c   30 Apr 2021 12:31:22 -
>> @@ -154,9 +154,8 @@ control_dispatch_vmd(int fd, struct priv
>>  if (notify->ctl_vmid != vmr.vmr_id)
>>  continue;
>>  if ((c = control_connbyfd(notify->ctl_fd)) != NULL) {
>> -/* XXX vmctl expects *_RESPONSE, not *_EVENT */
>> -imsg_compose_event(>iev,
>> -IMSG_VMDOP_TERMINATE_VM_RESPONSE,
>> +/* Forward to the vmctl(8) client */
>> +imsg_compose_event(>iev, imsg->hdr.type,
>>  0, 0, -1, imsg->data, IMSG_DATA_SIZE(imsg));
>>  TAILQ_REMOVE(_notify_q, notify, entry);
>>  free(notify);
>> Index: usr.sbin/vmctl/vmctl.c
>> ===
>> RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
>> retrieving revision 1.77
>> diff -u -p -r1.77 vmctl.c
>> --- usr.sbin/vmctl/vmctl.c   22 Mar 2021 18:50:11 -  1.77
>> +++ usr.sbin/vmctl/vmctl.c   30 Apr 2021 12:31:22 -
>> @@ -461,7 +461,7 @@ terminate_vm(uint32_t terminate_id, cons
>>   * terminate_vm_complete
>>   *
>>   * Callback function invoked when we are expecting an
>> - * IMSG_VMDOP_TERMINATE_VM_RESPONSE message indicating the completion of
>> + * IMSG_VMDOP_TERMINATE_VM_EVENT message indicating the completion of
>>   * a terminate vm operation.
>>   *
>>   * Parameters:
>> @@ -484,41 +484,50 @@ terminate_vm_complete(struct imsg *imsg,
>>  struct vmop_result *vmr;
>>  int res;
>>
>> -if (imsg->hdr.type == IMSG_VMDOP_TERMINATE_VM_RESPONSE) {
>> +switch (imsg->hdr.type) {
>> +case IMSG_VMDOP_TERMINATE_VM_RESPONSE:
>> +IMSG_SIZE_CHECK(imsg, );
>>  vmr = (struct vmop_result *)imsg->data;
>>  res = vmr->vmr_result;
>> -if (res) {
>> -switch (res) {
>> -case VMD_VM_STOP_INVALID:
>> -fprintf(stderr,
>> -"cannot stop vm that is not running\n");
>> -*ret = EINVAL;
>> -break;
>> -case ENOENT:
>> -fprintf(stderr, "vm not found\n");
>> -*ret = EIO;
>> -break;
>> -case EINTR:
>> -fprintf(stderr, "interrupted call\n");
>> -*ret = EIO;
>> -break;
>> -default:
>> -errno = res;
>> -fprintf(stderr, "failed: %s\n",
>> -strerror(res));
>> -*ret = EIO;
>> -}
>> -} else if (flags & VMOP_WAIT) {
>> +
>> +switch (res) {
>> +case 0:
>> +fprintf(stderr, "requested to shutdown vm %d\n",
>> +vmr->vmr_id);
>> +*ret = 0;
>> +break;
>> +case VMD_VM_STOP_INVALID:
>> +fprintf(stderr,
>> +"cannot stop vm that is not running\n");
>> +*ret = EINVAL;
>> +break;
>> +case ENOENT:
>> +fprintf(stderr, "vm not found\n");
>> +*ret = EIO;
>> +break;
>> +case EINTR:
>> +fprintf(stderr, "interrupted call\n");
>> +*ret = EIO;
>> +break;
>> +default:
>> +errno = res;
>> +fprintf(stderr, "failed: %s\n",
>> +strerror(res));
>> +*ret = 

Re: vmd: Fix grammar for random lladdr

2021-06-02 Thread Dave Voutila


Claudio Jeker writes:

> On Wed, Jun 02, 2021 at 08:24:53AM -0400, Dave Voutila wrote:
>>
>> Martin Vahlensieck writes:
>>
>> > Index: parse.y
>> > ===
>> > retrieving revision 1.56
>> > diff -u -p -r1.56 parse.y
>> > --- parse.y23 Sep 2020 19:18:18 -  1.56
>> > +++ parse.y2 Jun 2021 06:48:12 -
>> > @@ -694,6 +694,9 @@ lladdr : STRING{
>> >
>> >memcpy($$, ea, ETHER_ADDR_LEN);
>> >}
>> > +  | /* empty */ {
>> > +  memset($$, 0, ETHER_ADDR_LEN);
>> > +  }
>> >;
>> >
>> >  local : /* empty */   { $$ = 0; }
>>
>> ok dv@.
>>
>> I'll commit this later today.
>
> This is also OK claudio@

Committed. Thanks, Martin.



Re: let ipv4_check remember if the ip checksum was good

2021-06-02 Thread Vitaliy Makkoveev
On Wed, Jun 02, 2021 at 11:09:13AM +1000, David Gwynne wrote:
> if a bridge checked the ipv4 checksum and it was good, we can avoid
> checking it again in ip_input.
> 
> ok?
> 

ok mvs@

> Index: ip_input.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.361
> diff -u -p -r1.361 ip_input.c
> --- ip_input.c2 Jun 2021 00:09:57 -   1.361
> +++ ip_input.c2 Jun 2021 01:07:35 -
> @@ -287,8 +287,8 @@ ipv4_check(struct ifnet *ifp, struct mbu
>   }
>   }
>  
> - if ((m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_OK) == 0) {
> - if (m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_BAD) {
> + if (!ISSET(m->m_pkthdr.csum_flags, M_IPV4_CSUM_IN_OK)) {
> + if (ISSET(m->m_pkthdr.csum_flags, M_IPV4_CSUM_IN_BAD)) {
>   ipstat_inc(ips_badsum);
>   goto bad;
>   }
> @@ -298,6 +298,8 @@ ipv4_check(struct ifnet *ifp, struct mbu
>   ipstat_inc(ips_badsum);
>   goto bad;
>   }
> +
> + SET(m->m_pkthdr.csum_flags, M_IPV4_CSUM_IN_OK);
>   }
>  
>   /* Retrieve the packet length. */
> 



Re: vmd: Fix grammar for random lladdr

2021-06-02 Thread Claudio Jeker
On Wed, Jun 02, 2021 at 08:24:53AM -0400, Dave Voutila wrote:
> 
> Martin Vahlensieck writes:
> 
> > Index: parse.y
> > ===
> > retrieving revision 1.56
> > diff -u -p -r1.56 parse.y
> > --- parse.y 23 Sep 2020 19:18:18 -  1.56
> > +++ parse.y 2 Jun 2021 06:48:12 -
> > @@ -694,6 +694,9 @@ lladdr  : STRING{
> >
> > memcpy($$, ea, ETHER_ADDR_LEN);
> > }
> > +   | /* empty */ {
> > +   memset($$, 0, ETHER_ADDR_LEN);
> > +   }
> > ;
> >
> >  local  : /* empty */   { $$ = 0; }
> 
> ok dv@.
> 
> I'll commit this later today.

This is also OK claudio@ 

-- 
:wq Claudio



Re: vmd: Fix grammar for random lladdr

2021-06-02 Thread Dave Voutila


Martin Vahlensieck writes:

> Index: parse.y
> ===
> retrieving revision 1.56
> diff -u -p -r1.56 parse.y
> --- parse.y   23 Sep 2020 19:18:18 -  1.56
> +++ parse.y   2 Jun 2021 06:48:12 -
> @@ -694,6 +694,9 @@ lladdr: STRING{
>
>   memcpy($$, ea, ETHER_ADDR_LEN);
>   }
> + | /* empty */ {
> + memset($$, 0, ETHER_ADDR_LEN);
> + }
>   ;
>
>  local: /* empty */   { $$ = 0; }

ok dv@.

I'll commit this later today.



Re: sdmmc(4): check and retry bus width change

2021-06-02 Thread Raf Czlonka
On Tue, Jun 01, 2021 at 09:29:55AM BST, Patrick Wildt wrote:
> Am Mon, Feb 22, 2021 at 08:10:21PM +0100 schrieb Patrick Wildt:
> > Hi,
> > 
> > it seems like some eMMCs are not capable of doing 8-bit operation,
> > even if the controller supports it.  I was questioning our drivers
> > first, but it looks like it's the same on Linux.  In the case that
> > 8-bit doesn't work, they seem to fall back to lower values to make
> > that HW work.
> > 
> > This diff implements a mechanism that tries 8-bit, if available,
> > then 4-bit and in the end falls back to 1-bit.  This makes my HW
> > work, but I would like to have this tested by a broader audience.
> > 
> > Apparently there's a "bus test" command, but it isn't implemented
> > on all host controllers.  Hence I simply try to read the EXT_CSD
> > to make sure the transfer works.
> > 
> > For testing, a print like
> > 
> > printf("%s: using %u-bit width\n", DEVNAME(sc), width);
> > 
> > could be added at line 928.
> > 
> > What could possible regressions be?  The width could become smaller
> > then previously.  This would reduce the read/write transfer speed.
> > Also it's possible that eMMCs are not recognized/initialized anymore.
> > 
> > What could possible improvements be?  eMMCs that previously didn't
> > work now work, with at least 1-bit or 4-bit wide transfers.
> > 
> > Please note that this only works for eMMCs.  SD cards are *not* using
> > this code path.  SD cards have a different initialization code path.
> > 
> > Please report any changes or non-changes.  If nothing changes, that's
> > perfect.
> > 
> > Patrick
> 
> Anyone want to give this a try?  It's basically relevant for all
> ARM machines with eMMC ('soldered SD cards'), and they should work
> as well as before.

Hi Patrick,

I've managed to build the kernel with your patch and the machine
booted up just fine the first time - it was as slow as before ;^)

Then, I rebooted to the old kernel to compare dmesg before and after
- they're identical, BTW - and I shut the machine down.

I got a crash (ddb) on the next boot - don't have the time to
transcribe it right now, I'm afraid.

https://photos.app.goo.gl/Kr54uoppT4b6gR179

On subsequent boot I got:

ehci_device_bulk_start: not done, ex=0xff800800f378

scrolling(?) continuously on the screen.

Next boot, ddb again.

Old kernel booted up just fine.

Regards,

Raf

OpenBSD 6.9-current (GENERIC.MP) #0: Wed Jun  2 09:36:58 BST 2021
r...@foo.example.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP
real mem  = 2013835264 (1920MB)
avail mem = 1919496192 (1830MB)
random: good seed from bootblocks
mainbus0 at root: Pinebook
psci0 at mainbus0: PSCI 1.1, SMCCC 1.2
cpu0 at mainbus0 mpidr 0: ARM Cortex-A53 r0p4
cpu0: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache
cpu0: 512KB 64b/line 16-way L2 cache
cpu0: CRC32,SHA2,SHA1,AES+PMULL,ASID16
cpu1 at mainbus0 mpidr 1: ARM Cortex-A53 r0p4
cpu1: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache
cpu1: 512KB 64b/line 16-way L2 cache
cpu1: CRC32,SHA2,SHA1,AES+PMULL,ASID16
cpu2 at mainbus0 mpidr 2: ARM Cortex-A53 r0p4
cpu2: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache
cpu2: 512KB 64b/line 16-way L2 cache
cpu2: CRC32,SHA2,SHA1,AES+PMULL,ASID16
cpu3 at mainbus0 mpidr 3: ARM Cortex-A53 r0p4
cpu3: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache
cpu3: 512KB 64b/line 16-way L2 cache
cpu3: CRC32,SHA2,SHA1,AES+PMULL,ASID16
efi0 at mainbus0: UEFI 2.8
efi0: Das U-Boot rev 0x20200400
apm0 at mainbus0
"osc24M_clk" at mainbus0 not configured
"osc32k_clk" at mainbus0 not configured
"internal-osc-clk" at mainbus0 not configured
simpleaudio0 at mainbus0
"spdif-out" at mainbus0 not configured
agtimer0 at mainbus0: 24000 kHz
simplebus0 at mainbus0: "soc"
sxisyscon0 at simplebus0
sxisid0 at simplebus0
sxiccmu0 at simplebus0
sxipio0 at simplebus0: 103 pins
ampintc0 at simplebus0 nirq 224, ncpu 4 ipi: 0, 1: "interrupt-controller"
sxirtc0 at simplebus0
sxiccmu1 at simplebus0
sxipio1 at simplebus0: 13 pins
sxirsb0 at simplebus0
axppmic0 at sxirsb0 addr 0x3a3: AXP803
"de2" at simplebus0 not configured
"dma-controller" at simplebus0 not configured
"lcd-controller" at simplebus0 not configured
"lcd-controller" at simplebus0 not configured
sximmc0 at simplebus0
sdmmc0 at sximmc0: 4-bit, sd high-speed, mmc high-speed, dma
sximmc1 at simplebus0
sdmmc1 at sximmc1: 4-bit, sd high-speed, mmc high-speed, dma
sximmc2 at simplebus0
sdmmc2 at sximmc2: 8-bit, sd high-speed, mmc high-speed, dma
"phy" at simplebus0 not configured
ehci0 at simplebus0
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 configuration 1 interface 0 "Generic EHCI root hub" rev 2.00/1.00 
addr 1
ohci0 at simplebus0: version 1.0
ehci1 at simplebus0
usb1 at ehci1: USB revision 2.0
uhub1 at usb1 configuration 1 interface 0 "Generic EHCI root hub" rev 2.00/1.00 
addr 1
ohci1 at simplebus0: version 1.0
com0 at simplebus0: ns16550, no working fifo
sxipwm0 at simplebus0
"hdmi-phy" at 

Re: [External] : Re: parallel forwarding vs. bridges

2021-06-02 Thread David Gwynne
On Wed, May 19, 2021 at 01:48:26AM +0200, Alexandr Nedvedicky wrote:
> Hello,
> 
> just for the record...
> 
> 
> 
> > > in current tree the ether_input() is protected by NET_LOCK(), which is 
> > > grabbed
> > > by caller as a writer. bluhm's diff changes NET_LOCK() readlock, so
> > > ether_input() can run concurrently. Switching NET_LOCK() to r-lock has
> > > implications on smr read section above. The ting is the call to 
> > > eb->eb_input()
> > > can sleep now. This is something what needs to be avoided within smr 
> > > section.
> > 
> > Is the new sleeping point introduced by the fact the PF_LOCK() is a
> > rwlock?  Did you consider using a mutex, at least for the time being,
> > in order to not run in such issues?
> 
> below is a diff, which trades both pf(4) rw-locks for mutexes.
> 
> diff compiles, it still needs testing/experimenting.

hi. i'm trying to get my head back into this space, so i'm trying to
have a look at these diffs properly.

moving pf locks to mutexes makes sense to me, but like you say,
this will need testing and experimentation. one of the issues
identified in another part of this thread (and on icb?) is that the
ioctl path does some stuff which can sleep, but you can't sleep
while holding a mutex which would now be protecting the data
structures you're trying to look at.

a more specific example is if you're trying to dump the state table.
like you say, the state table is currently protected by the
pf_state_lock. iterating over the state table and giving up this lock to
copyout the entries is... not fun.

the diff below is a work in progress attempt to address this. it works
by locking the pf state list separately so it can support traversal
without (long) holds of locks needed in the forwarding path. the very
short explanation is that the TAILQ holding the states is locked
separately to the links between the states in the list. a much longer
explanataion is in the diff in the pfvar_priv.h chunk.

inserting states into the TAILQ while processing packets uses a mutex.
iterating over the list in the pf purge processing, ioctl paths, and
pfsync can largely rely on an rwlock. the rwlock would also allow
concurrent access to the list of states, ie, you can dump the list of
states while the gc thread is looking for states to collect, also while
pfsync is sending out a bulk update. it also converts the ioctl handling
for flushing states to using the list instead of one of the RB trees
holding states. i'm sure everyone wants an omgoptimised state flushing
mechanism.

this is a work in progress because i've screwed up the pf_state
reference counting somehow. states end up with an extra reference, and i
can't see where that's coming from.

anyway. apart from the refcounting thing it seems to be working well,
and would take us a step closer to using mutexes for pf locks.

even if we throw this away, id still like to move the pf purge
processing from out of nettq0 into systq, just to avoid having a place
where a nettq thread has to spin waiting for the kernel lock.

Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1118
diff -u -p -r1.1118 pf.c
--- pf.c1 Jun 2021 09:57:11 -   1.1118
+++ pf.c2 Jun 2021 07:52:21 -
@@ -308,7 +308,7 @@ static __inline void pf_set_protostate(s
 struct pf_src_tree tree_src_tracking;
 
 struct pf_state_tree_id tree_id;
-struct pf_state_queue state_list;
+struct pf_state_list pf_state_list = PF_STATE_LIST_INITIALIZER(pf_state_list);
 
 RB_GENERATE(pf_src_tree, pf_src_node, entry, pf_src_compare);
 RB_GENERATE(pf_state_tree, pf_state_key, entry, pf_state_compare_key);
@@ -440,6 +440,37 @@ pf_check_threshold(struct pf_threshold *
return (threshold->count > threshold->limit);
 }
 
+void
+pf_state_list_insert(struct pf_state_list *pfs, struct pf_state *st)
+{
+   /*
+* we can always put states on the end of the list.
+*
+* things reading the list should take a read lock, then
+* the mutex, get the head and tail pointers, release the
+* mutex, and then they can iterate between the head and tail.
+*/
+
+   pf_state_ref(st); /* get a ref for the list */
+
+   mtx_enter(>pfs_mtx);
+   TAILQ_INSERT_TAIL(>pfs_list, st, entry_list);
+   mtx_leave(>pfs_mtx);
+}
+
+void
+pf_state_list_remove(struct pf_state_list *pfs, struct pf_state *st)
+{
+   /* states can only be removed when the write lock is held */
+   rw_assert_wrlock(>pfs_rwl);
+
+   mtx_enter(>pfs_mtx);
+   TAILQ_REMOVE(>pfs_list, st, entry_list);
+   mtx_leave(>pfs_mtx);
+
+   pf_state_unref(st); /* list no longer references the state */
+}
+
 int
 pf_src_connlimit(struct pf_state **state)
 {
@@ -986,7 +1017,7 @@ pf_state_insert(struct pfi_kif *kif, str
PF_STATE_EXIT_WRITE();
return (-1);
}
-   TAILQ_INSERT_TAIL(_list, s, entry_list);
+   

Re: vmd: Fix grammar for random lladdr

2021-06-02 Thread Martin Vahlensieck
Hi Dave

On Tue, Jun 01, 2021 at 08:23:45PM -0400, Dave Voutila wrote:
> 
> Martin Vahlensieck writes:
> 
> > Hi
> >
> > The grammar for lladdr of interfaces is according to the manpage:
> >
> >   [locked] lladdr [etheraddr]
> >
> > This implies that `locked lladdr' is OK but looking at parse.y this
> > does not seem to be the case.  Making it optional would lead to a
> > `lladdr' all by itself being valid, which I find weird.  So I copied
> > the way ifconfig does it and now the syntax is:
> >
> >   [locked] lladdr etheraddr|random
> >
> > so to have a random locked lladdr one would have to write
> >
> >   locked lladdr random
> >
> > Is this a good approach?
> 
> Part of me thinks just specifying:
> 
>   locked lladdr
> 
> should give you a random address but enable the source mac
> filtering. Having to specify "random" seems odd to me. Thoughts?
Your variant matches what should be possible according to the man
page. It is also how I tried it and discovered it didn't work.  I
don't have a preference between the two, a diff to make the syntax
in the man page work is attached.  Syntax is OK for the following
config:
vm "test" {
memory 1G
interface {lladdr f0:e4:08:ef:5f:0a}
interface {lladdr}
interface {locked lladdr}
interface {locked lladdr f0:e4:08:ef:5f:0a}
}
> 
> I see what you're saying with how ifconfig(8) does it, but it's a bit
> different in this context as there's the "locked" modifier, so it's not
> exactly the same.
> 
> I'm not sure about the man page changes regardless. Will need another
> set of eyes on the syntax.
Sure.

Thanks for the feedback!

Best,

Martin
> 
> >
> > Best,
> >
> > Martin
> >
> > Index: parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
> > retrieving revision 1.56
> > diff -u -p -r1.56 parse.y
> > --- parse.y 23 Sep 2020 19:18:18 -  1.56
> > +++ parse.y 22 May 2021 07:55:18 -
> > @@ -685,14 +685,16 @@ string: STRING string 
> > {
> >  lladdr : STRING{
> > struct ether_addr *ea;
> >
> > -   if ((ea = ether_aton($1)) == NULL) {
> > +   if (strcmp($1, "random") == 0) {
> > +   memset($$, 0, ETHER_ADDR_LEN);
> > +   } else if ((ea = ether_aton($1)) != NULL) {
> > +   memcpy($$, ea, ETHER_ADDR_LEN);
> > +   } else {
> > yyerror("invalid address: %s\n", $1);
> > free($1);
> > YYERROR;
> > }
> > free($1);
> > -
> > -   memcpy($$, ea, ETHER_ADDR_LEN);
> > }
> > ;
> >
> > Index: vm.conf.5
> > ===
> > RCS file: /cvs/src/usr.sbin/vmd/vm.conf.5,v
> > retrieving revision 1.56
> > diff -u -p -r1.56 vm.conf.5
> > --- vm.conf.5   1 Mar 2021 14:27:44 -   1.56
> > +++ vm.conf.5   22 May 2021 07:55:18 -
> > @@ -237,10 +237,12 @@ The
> >  must not be longer than 15 characters or end with a digit,
> >  as described in
> >  .Xr ifconfig 8 .
> > -.It Oo Cm locked Oc Cm lladdr Op Ar etheraddr
> > +.It Oo Cm locked Oc Cm lladdr Ar etheraddr Ns | Ns Cm random
> >  Change the link layer address (MAC address) of the interface on the
> >  VM guest side.
> > -If not specified, a randomized address will be assigned by
> > +If
> > +.Cm random
> > +is specified, a randomized address will be assigned by
> >  .Xr vmd 8 .
> >  If the
> >  .Cm locked
> 

Index: parse.y
===
retrieving revision 1.56
diff -u -p -r1.56 parse.y
--- parse.y 23 Sep 2020 19:18:18 -  1.56
+++ parse.y 2 Jun 2021 06:48:12 -
@@ -694,6 +694,9 @@ lladdr  : STRING{
 
memcpy($$, ea, ETHER_ADDR_LEN);
}
+   | /* empty */ {
+   memset($$, 0, ETHER_ADDR_LEN);
+   }
;
 
 local  : /* empty */   { $$ = 0; }