Re: Larger kernel fonts in RAMDISK_CD?
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)
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?
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)
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
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
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
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
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
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
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
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
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
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; }