Re: [patch] italic text on the console!
The earlier version of this patch leaked memory if a font was locked multiple times. This version fixes the leak: Index: dev/rasops/rasops.c === RCS file: /cvs/src/sys/dev/rasops/rasops.c,v retrieving revision 1.69 diff -u -p -r1.69 rasops.c --- dev/rasops/rasops.c 18 Jan 2023 11:08:49 - 1.69 +++ dev/rasops/rasops.c 18 Jan 2023 23:55:25 - @@ -561,7 +561,7 @@ rasops_pack_cattr(void *cookie, int fg, if ((flg & WSATTR_HILIT) != 0 && fg < 8) fg += 8; - *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE); + *attr = (bg << 16) | (fg << 24) | flg; return (0); } @@ -585,7 +585,7 @@ rasops_pack_mattr(void *cookie, int fg, bg = swap; } - *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE); + *attr = (bg << 16) | (fg << 24) | flg; return (0); } Index: dev/rasops/rasops32.c === RCS file: /cvs/src/sys/dev/rasops/rasops32.c,v retrieving revision 1.13 diff -u -p -r1.13 rasops32.c --- dev/rasops/rasops32.c 18 Jan 2023 11:08:49 - 1.13 +++ dev/rasops/rasops32.c 18 Jan 2023 23:55:25 - @@ -107,7 +107,26 @@ rasops32_putchar(void *cookie, int row, } } else { uc -= ri->ri_font->firstchar; - fr = (u_char *)ri->ri_font->data + uc * ri->ri_fontscale; + + /* Choose font data based on bold and italic attributes */ + + u_char * font_dataset; + + switch (attr & (WSATTR_HILIT | WSATTR_ITALIC)) { + case WSATTR_HILIT: + font_dataset=(u_char *)ri->ri_font->data_bold; + break; + case WSATTR_ITALIC: + font_dataset=(u_char *)ri->ri_font->data_italic; + break; + case (WSATTR_HILIT | WSATTR_ITALIC): + font_dataset=(u_char *)ri->ri_font->data_bolditalic; + break; + default: + font_dataset=(u_char *)ri->ri_font->data; + } + + fr = (font_dataset + uc * ri->ri_fontscale); fs = ri->ri_font->stride; /* double-pixel special cases for the common widths */ Index: dev/wscons/wsconsio.h === RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v retrieving revision 1.98 diff -u -p -r1.98 wsconsio.h --- dev/wscons/wsconsio.h 15 Jul 2022 17:57:27 - 1.98 +++ dev/wscons/wsconsio.h 18 Jan 2023 23:55:27 - @@ -536,6 +536,9 @@ struct wsdisplay_font { #defineWSDISPLAY_FONTORDER_R2L 2 void *cookie; void *data; + void *data_bold; + void *data_italic; + void *data_bolditalic; }; #define WSDISPLAYIO_LDFONT _IOW ('W', 77, struct wsdisplay_font) #defineWSDISPLAYIO_LSFONT _IOWR('W', 78, struct wsdisplay_font) Index: dev/wscons/wsdisplayvar.h === RCS file: /cvs/src/sys/dev/wscons/wsdisplayvar.h,v retrieving revision 1.38 diff -u -p -r1.38 wsdisplayvar.h --- dev/wscons/wsdisplayvar.h 13 Sep 2020 10:05:46 - 1.38 +++ dev/wscons/wsdisplayvar.h 18 Jan 2023 23:55:27 - @@ -99,6 +99,7 @@ struct wsdisplay_emulops { #define WSATTR_BLINK 4 #define WSATTR_UNDERLINE 8 #define WSATTR_WSCOLORS 16 +#define WSATTR_ITALIC 32 }; #defineWSSCREEN_NAME_SIZE 16 Index: dev/wscons/wsemul_vt100_subr.c === RCS file: /cvs/src/sys/dev/wscons/wsemul_vt100_subr.c,v retrieving revision 1.29 diff -u -p -r1.29 wsemul_vt100_subr.c --- dev/wscons/wsemul_vt100_subr.c 12 Jan 2023 20:39:37 - 1.29 +++ dev/wscons/wsemul_vt100_subr.c 18 Jan 2023 23:55:27 - @@ -588,6 +588,9 @@ wsemul_vt100_handle_csi(struct wsemul_vt case 1: /* bold */ flags |= WSATTR_HILIT; break; + case 3: /* italic */ + flags |= WSATTR_ITALIC; + break; case 4: /* underline */ flags |= WSATTR_UNDERLINE; break; @@ -599,6 +602,9 @@ wsemul_vt100_handle_csi(struct wsemul_vt break; case 22: /* ~bold VT300 only */ flags &= ~WSATTR_HILIT; + break; + case 23: /* ~italic */ + flags &= ~WSATTR_ITALIC; break; case 24: /* ~underline VT300 only */ flags &= ~WSATTR_UNDERLINE; Ind
Re: refactor mbuf parsing on driver level
> On 19 Jan 2023, at 01:39, Jan Klemkow wrote: > > On Wed, Jan 18, 2023 at 10:50:25AM +0300, Vitaliy Makkoveev wrote: >> On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote: >>> we have several drivers which have to parse the content of mbufs. This >>> diff suggest a central parsing function for this. Thus, we can reduce >>> redundant code. >>> >>> I just start with ix(4) and ixl(4) because it was easy to test for me. >>> But, this could also improve em(4), igc(4), ale(4) and oce(4). >>> >>> I'm not sure about the name, the api nor the place of this code. So, if >>> someone has a better idea: i'm open to anything. >> >> I like code this deduplication. >> >> This newly introduced function doesn't touch ifnet but only extracts >> protocol headers from mbuf(9). I guess mbuf_extract_headers() or >> something like is much better for name with the ern/uipc_mbuf2.c as >> place. > > Good Point. Updates diff below. > > + > +/* Parse different TCP/IP protocol headers for a quick view inside an mbuf. > */ > +void > +m_exract_headers(struct mbuf *mp, struct ether_header **eh, struct ip **ip4, > +struct ip6_hdr **ip6, struct tcphdr **tcp, struct udphdr **udp) > + Should be m_extract_headers(). The rest of the diff looks good to me.
Re: refactor mbuf parsing on driver level
Jan Klemkow wrote: > On Wed, Jan 18, 2023 at 10:50:25AM +0300, Vitaliy Makkoveev wrote: > > On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote: > > > we have several drivers which have to parse the content of mbufs. This > > > diff suggest a central parsing function for this. Thus, we can reduce > > > redundant code. > > > > > > I just start with ix(4) and ixl(4) because it was easy to test for me. > > > But, this could also improve em(4), igc(4), ale(4) and oce(4). > > > > > > I'm not sure about the name, the api nor the place of this code. So, if > > > someone has a better idea: i'm open to anything. > > > > I like code this deduplication. > > > > This newly introduced function doesn't touch ifnet but only extracts > > protocol headers from mbuf(9). I guess mbuf_extract_headers() or > > something like is much better for name with the ern/uipc_mbuf2.c as > > place. > > Good Point. Updates diff below. I agree, "extract" is a better name. dlg, do you have a comment?
Re: refactor mbuf parsing on driver level
On Wed, Jan 18, 2023 at 10:50:25AM +0300, Vitaliy Makkoveev wrote: > On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote: > > we have several drivers which have to parse the content of mbufs. This > > diff suggest a central parsing function for this. Thus, we can reduce > > redundant code. > > > > I just start with ix(4) and ixl(4) because it was easy to test for me. > > But, this could also improve em(4), igc(4), ale(4) and oce(4). > > > > I'm not sure about the name, the api nor the place of this code. So, if > > someone has a better idea: i'm open to anything. > > I like code this deduplication. > > This newly introduced function doesn't touch ifnet but only extracts > protocol headers from mbuf(9). I guess mbuf_extract_headers() or > something like is much better for name with the ern/uipc_mbuf2.c as > place. Good Point. Updates diff below. Thanks, Jan Index: dev/pci/if_ix.c === RCS file: /cvs/src/sys/dev/pci/if_ix.c,v retrieving revision 1.189 diff -u -p -r1.189 if_ix.c --- dev/pci/if_ix.c 2 Sep 2022 14:08:09 - 1.189 +++ dev/pci/if_ix.c 18 Jan 2023 21:06:58 - @@ -2477,23 +2477,18 @@ static inline int ixgbe_csum_offload(struct mbuf *mp, uint32_t *vlan_macip_lens, uint32_t *type_tucmd_mlhl, uint32_t *olinfo_status) { - struct ether_header *eh = mtod(mp, struct ether_header *); - struct mbuf *m; - int hoff; + struct ether_header *eh = NULL; + struct ip *ip = NULL; + struct ip6_hdr *ip6 = NULL; int offload = 0; uint32_t iphlen; uint8_t ipproto; - *vlan_macip_lens |= (sizeof(*eh) << IXGBE_ADVTXD_MACLEN_SHIFT); + m_exract_headers(mp, &eh, &ip, &ip6, NULL, NULL); - switch (ntohs(eh->ether_type)) { - case ETHERTYPE_IP: { - struct ip *ip; - - m = m_getptr(mp, sizeof(*eh), &hoff); - KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip)); - ip = (struct ip *)(mtod(m, caddr_t) + hoff); + *vlan_macip_lens |= (sizeof(*eh) << IXGBE_ADVTXD_MACLEN_SHIFT); + if (ip) { iphlen = ip->ip_hl << 2; ipproto = ip->ip_p; @@ -2503,26 +2498,14 @@ ixgbe_csum_offload(struct mbuf *mp, uint } *type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV4; - break; - } - #ifdef INET6 - case ETHERTYPE_IPV6: { - struct ip6_hdr *ip6; - - m = m_getptr(mp, sizeof(*eh), &hoff); - KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip6)); - ip6 = (struct ip6_hdr *)(mtod(m, caddr_t) + hoff); - + } else if (ip6) { iphlen = sizeof(*ip6); ipproto = ip6->ip6_nxt; *type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV6; - break; - } #endif - - default: + } else { return offload; } Index: dev/pci/if_ixl.c === RCS file: /cvs/src/sys/dev/pci/if_ixl.c,v retrieving revision 1.84 diff -u -p -r1.84 if_ixl.c --- dev/pci/if_ixl.c5 Aug 2022 13:57:16 - 1.84 +++ dev/pci/if_ixl.c18 Jan 2023 20:47:01 - @@ -2784,12 +2784,15 @@ ixl_load_mbuf(bus_dma_tag_t dmat, bus_dm static uint64_t ixl_tx_setup_offload(struct mbuf *m0) { - struct mbuf *m; - int hoff; + struct ether_header *eh = NULL; + struct ip *ip = NULL; + struct ip6_hdr *ip6 = NULL; + struct tcphdr *th = NULL; uint64_t hlen; uint8_t ipproto; uint64_t offload = 0; + if (ISSET(m0->m_flags, M_VLANTAG)) { uint64_t vtag = m0->m_pkthdr.ether_vtag; offload |= IXL_TX_DESC_CMD_IL2TAG1; @@ -2800,39 +2803,23 @@ ixl_tx_setup_offload(struct mbuf *m0) M_IPV4_CSUM_OUT|M_TCP_CSUM_OUT|M_UDP_CSUM_OUT)) return (offload); - switch (ntohs(mtod(m0, struct ether_header *)->ether_type)) { - case ETHERTYPE_IP: { - struct ip *ip; - - m = m_getptr(m0, ETHER_HDR_LEN, &hoff); - KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip)); - ip = (struct ip *)(mtod(m, caddr_t) + hoff); + m_exract_headers(m0, &eh, &ip, &ip6, &th, NULL); + if (ip) { offload |= ISSET(m0->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT) ? IXL_TX_DESC_CMD_IIPT_IPV4_CSUM : IXL_TX_DESC_CMD_IIPT_IPV4; hlen = ip->ip_hl << 2; ipproto = ip->ip_p; - break; - } - #ifdef INET6 - case ETHERTYPE_IPV6: { - struct ip6_hdr *ip6; - - m = m_getptr(m0, ETHER_HDR_LEN, &hoff); - KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip6)); - ip6 = (struct ip6_hdr *)(mtod(m, caddr_t) + hoff); - + } else if (ip6) { offload |
Re: ifconfig.c redundancy the second
On Fri, Jan 13, 2023 at 09:18:30PM +0100, Mathias Koehler wrote: > Ehm well it should look like this, sorry: This code duplication has now been removed. Thanks! > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v > retrieving revision 1.460 > diff -u -p -u -p -r1.460 ifconfig.c > --- ifconfig.c 18 Dec 2022 18:56:38 - 1.460 > +++ ifconfig.c 13 Jan 2023 18:52:48 - > @@ -1907,12 +1907,14 @@ delifjoinlist(const char *val, int d) > memset(&join, 0, sizeof(join)); > join.i_flags |= (IEEE80211_JOIN_DEL | IEEE80211_JOIN_DEL_ALL); > > + /* > if (d == -1) { > ifr.ifr_data = (caddr_t)&join; > if (ioctl(sock, SIOCS80211JOIN, (caddr_t)&ifr) == -1) > err(1, "SIOCS80211JOIN"); > return; > } > + */ > > ifr.ifr_data = (caddr_t)&join; > >
Re: libevent manuals
Hi Ted, Ted Bullock wrote on Mon, Jan 16, 2023 at 12:56:06PM -0700: > The impetus is that the event(3) manual page isn't all that good for > documenting how to use the library and it is missing functions that are > included in the API and available in the shared library. That seems true, and those are good reasons to improve the manual pages. > Upstream of course has moved on to version 2.x, as far as I can tell > there is no more maintenance being done on the 1.4 version According to https://github.com/libevent/libevent/tree/patches-1.4 that seems accurate to me: it appears the 1.4 branch was last touched in the git repo eight years ago, and it is marked "stale", along with the 2.0 and 2.1 branches. > that OpenBSD ships except the work that is done internally in the OS. Given that our event.3 says .\" Copyright (c) 2000 Artur Grabowski and that the libevent git repo contains a Jurassic version of our event.3 in the 1.4 branch and does no longer appear to contain *any* kind of documentation in the master branch - in particular, there is no event.3 in the master branch any longer - i think maintaining the libevent manual pages ourselves is perfectly fine. I see no danger that any documentation might need to be merged from the libevent git: it appears the libevent project regards documentation as a useless distraction at best... :-o > Anyway, here is what I came up with for a few functions after reading > the source tree. I also went back through historical releases to see > how the API evolved over time. I think this is likely a worthwhile project, but still needs a lot of work before it can be considered for commit. > I'm planning to finish off documenting the rest of the library, if > folks have feedback, I'm interested. Given how much feedback i have on your first file, i'm not yet reading your second and third file right now. We do development in steps, and the tree needs to build and be better after each commit than before that commit. So to be able to be committed, your patch for the first file should also delete the now redundant information from event.3. Regard it as splitting one well-defined part out of the excessively large event.3 and adding lots of missing information while splitting it out. > event_init.3 > At this point, one line is missing: .\" $OpenBSD$ The command `mandoc -T lint` tells you: mandoc: event_init.3: STYLE: RCS id missing: (OpenBSD) > .\" Copyright (c) 2023 Ted Bullock > .\" > .\" Permission to use, copy, modify, and distribute this software for any > .\" purpose with or without fee is hereby granted, provided that the above > .\" copyright notice and this permission notice appear in all copies. > .\" > .\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > .\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > .\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > .\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > .\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > .\" > .Dd $Mdocdate: January 9 2023 $ > .Dt EVENT_INIT 3 > .Os > .Sh NAME > .Nm event_base_new , > .Nm event_init This is not nice. Unless there is a good reason to name the page after a function that is not the first function in the NAME section, put the function corresponding to the file name first in NAME and in all other sections. > .Nd initializes an > .Vt event_base > instance While nesting markup inside .Nd is technically possible, it is fragile, looks ugly, and we certainly don't do it in OpenBSD. > .Sh SYNOPSIS > .In event.h > .Ft "struct event_base *" > .Fn event_base_new void > .Ft "struct event_base *" > .Fn event_init void > .Sh DESCRIPTION > The functions > .Fn event_base_new > and > .Fn event_init > initialize the > .Lb libevent We don't normally use the .Lb macro in OpenBSD. Maybe just talk about "the event library" with no markup? The existing event.3 talks about The .Nm event API ... which is not wrong, but i'm not convinced the .Nm markup provides much value here. > and need to be called prior to scheduling an event or > starting the event-loop. The two functions are similar, although global > variables are set when calling > .Fn event_init > to help simplify the API for programs requiring only one event-loop. This is not good. Don't start comparing functions without precisely saying first what each function actually does, in particular not in the first sentence of a manual page. Instead, the first sentence should only state the general purpose of the functions documented in the page. In this page, it might not be needed at all, you could probably start with event_base_new(3) right away. After the introductory sentence(s), if any, start with one function and document it thoroughly. A
Re: rpki-client: require version 4 UUIDs in RRDP session IDs
On Wed, Jan 18, 2023 at 07:09:56PM +0100, Theo Buehler wrote: > On Wed, Jan 18, 2023 at 06:01:46PM +, Job Snijders wrote: > > All RRDP servers in the field now issue session IDs using the correct > > UUID version & type. > > Thanks for taking care of this. Indeed. > > OK? > > ok OK > > > > Kind regards, > > > > Job > > > > Index: validate.c > > === > > RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v > > retrieving revision 1.53 > > diff -u -p -r1.53 validate.c > > --- validate.c 18 Jan 2023 00:27:10 - 1.53 > > +++ validate.c 18 Jan 2023 17:58:03 - > > @@ -566,7 +566,6 @@ valid_uuid(const char *s) > > if (s[n] != '-') > > return 0; > > break; > > -#ifdef NOTYET /* World is not yet ready to enfoce UUID version and > > variant */ > > /* Check UUID is version 4 */ > > case 14: > > if (s[n] != '4') > > @@ -578,7 +577,6 @@ valid_uuid(const char *s) > > s[n] != 'A' && s[n] != 'b' && s[n] != 'B') > > return 0; > > break; > > -#endif > > case 36: > > return s[n] == '\0'; > > default: > > > -- :wq Claudio
Re: mem.4: be more accurate about securelevel
But you should not start a sentence with also. Also you should not start a sentence with but. Not the best english. jmc can weight in perhaps. Jan Klemkow wrote: > On Tue, Jan 17, 2023 at 11:02:07PM +0100, Theo Buehler wrote: > > > at least this tool works for me: > > > > Surely you have kern.allowkmem=1 set. > > This diff should phrase it correctly. > > ok? > > Thanks, > Jan > > Index: man4.alpha/mem.4 > === > RCS file: /cvs/src/share/man/man4/man4.alpha/mem.4,v > retrieving revision 1.6 > diff -u -p -r1.6 mem.4 > --- man4.alpha/mem.4 12 Jan 2018 04:36:44 - 1.6 > +++ man4.alpha/mem.4 18 Jan 2023 19:25:27 - > @@ -63,11 +63,12 @@ kernel virtual memory begins at > .Pp > Even with sufficient file system permissions, > these devices can only be opened when the > -.Xr securelevel 7 > -is insecure or when the > .Va kern.allowkmem > .Xr sysctl 2 > variable is set. > +Also the > +.Xr securelevel 7 > +insecure is needed, to open the device writable. > .Sh FILES > .Bl -tag -width /dev/kmem -compact > .It /dev/mem > Index: man4.amd64/mem.4 > === > RCS file: /cvs/src/share/man/man4/man4.amd64/mem.4,v > retrieving revision 1.6 > diff -u -p -r1.6 mem.4 > --- man4.amd64/mem.4 12 Jan 2018 04:36:44 - 1.6 > +++ man4.amd64/mem.4 18 Jan 2023 19:26:59 - > @@ -64,11 +64,12 @@ The kernel virtual memory begins at addr > .Pp > Even with sufficient file system permissions, > these devices can only be opened when the > -.Xr securelevel 7 > -is insecure or when the > .Va kern.allowkmem > .Xr sysctl 2 > variable is set. > +Also the > +.Xr securelevel 7 > +insecure is needed, to open the device writable. > .Sh FILES > .Bl -tag -width Pa -compact > .It Pa /dev/mem > Index: man4.hppa/mem.4 > === > RCS file: /cvs/src/share/man/man4/man4.hppa/mem.4,v > retrieving revision 1.4 > diff -u -p -r1.4 mem.4 > --- man4.hppa/mem.4 12 Jan 2018 04:36:44 - 1.4 > +++ man4.hppa/mem.4 18 Jan 2023 19:29:07 - > @@ -52,11 +52,12 @@ address 0; kernel virtual memory begins > .Pp > Even with sufficient file system permissions, > these devices can only be opened when the > -.Xr securelevel 7 > -is insecure or when the > .Va kern.allowkmem > .Xr sysctl 2 > variable is set. > +Also the > +.Xr securelevel 7 > +insecure is needed, to open the device writable. > .Sh FILES > .Bl -tag -width /dev/kmem -compact > .It Pa /dev/mem > Index: man4.i386/mem.4 > === > RCS file: /cvs/src/share/man/man4/man4.i386/mem.4,v > retrieving revision 1.12 > diff -u -p -r1.12 mem.4 > --- man4.i386/mem.4 12 Jan 2018 04:36:44 - 1.12 > +++ man4.i386/mem.4 18 Jan 2023 19:30:18 - > @@ -64,11 +64,12 @@ long, and ends at virtual address > .Pp > Even with sufficient file system permissions, > these devices can only be opened when the > -.Xr securelevel 7 > -is insecure or when the > .Va kern.allowkmem > .Xr sysctl 2 > variable is set. > +Also the > +.Xr securelevel 7 > +insecure is needed, to open the device writable. > .Sh FILES > .Bl -tag -width Pa -compact > .It Pa /dev/mem > Index: man4.landisk/mem.4 > === > RCS file: /cvs/src/share/man/man4/man4.landisk/mem.4,v > retrieving revision 1.4 > diff -u -p -r1.4 mem.4 > --- man4.landisk/mem.412 Jan 2018 04:36:44 - 1.4 > +++ man4.landisk/mem.418 Jan 2023 19:31:28 - > @@ -59,11 +59,12 @@ The kernel virtual memory begins at addr > .Pp > Even with sufficient file system permissions, > these devices can only be opened when the > -.Xr securelevel 7 > -is insecure or when the > .Va kern.allowkmem > .Xr sysctl 2 > variable is set. > +Also the > +.Xr securelevel 7 > +insecure is needed, to open the device writable. > .Sh FILES > .Bl -tag -width Pa -compact > .It Pa /dev/mem > Index: man4.loongson/mem.4 > === > RCS file: /cvs/src/share/man/man4/man4.loongson/mem.4,v > retrieving revision 1.4 > diff -u -p -r1.4 mem.4 > --- man4.loongson/mem.4 12 Jan 2018 04:36:44 - 1.4 > +++ man4.loongson/mem.4 18 Jan 2023 19:32:44 - > @@ -89,11 +89,12 @@ The kernel virtual memory begins at addr > .Pp > Even with sufficient file system permissions, > these devices can only be opened when the > -.Xr securelevel 7 > -is insecure or when the > .Va kern.allowkmem > .Xr sysctl 2 > variable is set. > +Also the > +.Xr securelevel 7 > +insecure is needed, to open the device writable. > .Sh FILES > .Bl -tag -width Pa -compact > .It Pa /dev/mem > Index: man4.luna88k/mem.4 > === > RCS file: /cvs/src/share/man/man4/man4.luna88k/mem.4,v > retrieving revision 1.4 > dif
Re: mem.4: be more accurate about securelevel
On Tue, Jan 17, 2023 at 11:02:07PM +0100, Theo Buehler wrote: > > at least this tool works for me: > > Surely you have kern.allowkmem=1 set. This diff should phrase it correctly. ok? Thanks, Jan Index: man4.alpha/mem.4 === RCS file: /cvs/src/share/man/man4/man4.alpha/mem.4,v retrieving revision 1.6 diff -u -p -r1.6 mem.4 --- man4.alpha/mem.412 Jan 2018 04:36:44 - 1.6 +++ man4.alpha/mem.418 Jan 2023 19:25:27 - @@ -63,11 +63,12 @@ kernel virtual memory begins at .Pp Even with sufficient file system permissions, these devices can only be opened when the -.Xr securelevel 7 -is insecure or when the .Va kern.allowkmem .Xr sysctl 2 variable is set. +Also the +.Xr securelevel 7 +insecure is needed, to open the device writable. .Sh FILES .Bl -tag -width /dev/kmem -compact .It /dev/mem Index: man4.amd64/mem.4 === RCS file: /cvs/src/share/man/man4/man4.amd64/mem.4,v retrieving revision 1.6 diff -u -p -r1.6 mem.4 --- man4.amd64/mem.412 Jan 2018 04:36:44 - 1.6 +++ man4.amd64/mem.418 Jan 2023 19:26:59 - @@ -64,11 +64,12 @@ The kernel virtual memory begins at addr .Pp Even with sufficient file system permissions, these devices can only be opened when the -.Xr securelevel 7 -is insecure or when the .Va kern.allowkmem .Xr sysctl 2 variable is set. +Also the +.Xr securelevel 7 +insecure is needed, to open the device writable. .Sh FILES .Bl -tag -width Pa -compact .It Pa /dev/mem Index: man4.hppa/mem.4 === RCS file: /cvs/src/share/man/man4/man4.hppa/mem.4,v retrieving revision 1.4 diff -u -p -r1.4 mem.4 --- man4.hppa/mem.4 12 Jan 2018 04:36:44 - 1.4 +++ man4.hppa/mem.4 18 Jan 2023 19:29:07 - @@ -52,11 +52,12 @@ address 0; kernel virtual memory begins .Pp Even with sufficient file system permissions, these devices can only be opened when the -.Xr securelevel 7 -is insecure or when the .Va kern.allowkmem .Xr sysctl 2 variable is set. +Also the +.Xr securelevel 7 +insecure is needed, to open the device writable. .Sh FILES .Bl -tag -width /dev/kmem -compact .It Pa /dev/mem Index: man4.i386/mem.4 === RCS file: /cvs/src/share/man/man4/man4.i386/mem.4,v retrieving revision 1.12 diff -u -p -r1.12 mem.4 --- man4.i386/mem.4 12 Jan 2018 04:36:44 - 1.12 +++ man4.i386/mem.4 18 Jan 2023 19:30:18 - @@ -64,11 +64,12 @@ long, and ends at virtual address .Pp Even with sufficient file system permissions, these devices can only be opened when the -.Xr securelevel 7 -is insecure or when the .Va kern.allowkmem .Xr sysctl 2 variable is set. +Also the +.Xr securelevel 7 +insecure is needed, to open the device writable. .Sh FILES .Bl -tag -width Pa -compact .It Pa /dev/mem Index: man4.landisk/mem.4 === RCS file: /cvs/src/share/man/man4/man4.landisk/mem.4,v retrieving revision 1.4 diff -u -p -r1.4 mem.4 --- man4.landisk/mem.4 12 Jan 2018 04:36:44 - 1.4 +++ man4.landisk/mem.4 18 Jan 2023 19:31:28 - @@ -59,11 +59,12 @@ The kernel virtual memory begins at addr .Pp Even with sufficient file system permissions, these devices can only be opened when the -.Xr securelevel 7 -is insecure or when the .Va kern.allowkmem .Xr sysctl 2 variable is set. +Also the +.Xr securelevel 7 +insecure is needed, to open the device writable. .Sh FILES .Bl -tag -width Pa -compact .It Pa /dev/mem Index: man4.loongson/mem.4 === RCS file: /cvs/src/share/man/man4/man4.loongson/mem.4,v retrieving revision 1.4 diff -u -p -r1.4 mem.4 --- man4.loongson/mem.4 12 Jan 2018 04:36:44 - 1.4 +++ man4.loongson/mem.4 18 Jan 2023 19:32:44 - @@ -89,11 +89,12 @@ The kernel virtual memory begins at addr .Pp Even with sufficient file system permissions, these devices can only be opened when the -.Xr securelevel 7 -is insecure or when the .Va kern.allowkmem .Xr sysctl 2 variable is set. +Also the +.Xr securelevel 7 +insecure is needed, to open the device writable. .Sh FILES .Bl -tag -width Pa -compact .It Pa /dev/mem Index: man4.luna88k/mem.4 === RCS file: /cvs/src/share/man/man4/man4.luna88k/mem.4,v retrieving revision 1.4 diff -u -p -r1.4 mem.4 --- man4.luna88k/mem.4 12 Jan 2018 04:36:44 - 1.4 +++ man4.luna88k/mem.4 18 Jan 2023 19:33:50 - @@ -63,11 +63,12 @@ kernel virtual memory begins at .Pp Even with sufficient file system permissions, these devices can only be opened when the -.Xr securelevel 7 -is insecure or when the .Va kern.allowkmem .Xr sysctl 2 variable is set. +Also the +.Xr securelevel 7 +insecure is needed, to open the device writable. .Sh FILES .Bl -tag
Re: rpki-client: require version 4 UUIDs in RRDP session IDs
On Wed, Jan 18, 2023 at 06:01:46PM +, Job Snijders wrote: > All RRDP servers in the field now issue session IDs using the correct > UUID version & type. Thanks for taking care of this. > OK? ok > > Kind regards, > > Job > > Index: validate.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v > retrieving revision 1.53 > diff -u -p -r1.53 validate.c > --- validate.c18 Jan 2023 00:27:10 - 1.53 > +++ validate.c18 Jan 2023 17:58:03 - > @@ -566,7 +566,6 @@ valid_uuid(const char *s) > if (s[n] != '-') > return 0; > break; > -#ifdef NOTYET/* World is not yet ready to enfoce UUID version and > variant */ > /* Check UUID is version 4 */ > case 14: > if (s[n] != '4') > @@ -578,7 +577,6 @@ valid_uuid(const char *s) > s[n] != 'A' && s[n] != 'b' && s[n] != 'B') > return 0; > break; > -#endif > case 36: > return s[n] == '\0'; > default: >
rpki-client: require version 4 UUIDs in RRDP session IDs
All RRDP servers in the field now issue session IDs using the correct UUID version & type. OK? Kind regards, Job Index: validate.c === RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v retrieving revision 1.53 diff -u -p -r1.53 validate.c --- validate.c 18 Jan 2023 00:27:10 - 1.53 +++ validate.c 18 Jan 2023 17:58:03 - @@ -566,7 +566,6 @@ valid_uuid(const char *s) if (s[n] != '-') return 0; break; -#ifdef NOTYET /* World is not yet ready to enfoce UUID version and variant */ /* Check UUID is version 4 */ case 14: if (s[n] != '4') @@ -578,7 +577,6 @@ valid_uuid(const char *s) s[n] != 'A' && s[n] != 'b' && s[n] != 'B') return 0; break; -#endif case 36: return s[n] == '\0'; default:
Re: bgpd, use vstate from filterstate for update functions
> But those calls have either a break or continue, so either the loop is > exited or restarted (depending on PEERFLAG_EVALUATE_ALL). That's what I was missing. Not sure how. > So there should be no way to go from a rde_filterstate_clean(&state) to > prefix_adjout_update(new). > > There is a missing rde_filterstate_clean(&state) in up_generate_update(). > if (up_enforce_open_policy(peer, &state)) { > rde_filterstate_clean(&state); > if (peer->flags & PEERFLAG_EVALUATE_ALL) { > new = TAILQ_NEXT(new, entry.list.rib); > if (new != NULL && prefix_eligible(new)) > continue; > } > break; > } > > /* check if this was actually a withdraw */ > if (need_withdraw) > here the filterstate is leaked, which is bad. > break; > > /* from here on we know this is an update */ > > up_prep_adjout(peer, &state, addr.aid); > prefix_adjout_update(p, peer, &state, &addr, > new->pt->prefixlen, new->path_id_tx); > rde_filterstate_clean(&state); > > Below a diff that fixes the extra leak. Agreed. I'm now happy with the diff. Sorry about the confusion. ok tb
Re: bgpd, use vstate from filterstate for update functions
On Wed, Jan 18, 2023 at 05:53:10PM +0100, Theo Buehler wrote: > On Wed, Jan 18, 2023 at 05:37:37PM +0100, Claudio Jeker wrote: > > On Wed, Jan 18, 2023 at 05:18:58PM +0100, Theo Buehler wrote: > > > On Wed, Jan 18, 2023 at 02:46:19PM +0100, Claudio Jeker wrote: > > > > This is the next step in vstate cleanup. > > > > Since the vstate is now part of struct filterstate use that information > > > > instead of passing an explicit vstate to the various update functions. > > > > > > It took me a moment to understand that rde_filterstate_clean() does not > > > zero the vstate, so the changes in rde_update.c preserve behavior. > > > That's not super intuitive to me, but if that's bound to stay this way, > > > > Uhm, now I'm a bit confused. Where is the vstate used after calling > > rde_filterstate_clean()? I did not spot that issue in rde_update.c > > I think we could zero everyting in the filterstate during clean. Just to > > be sure. > > It is super confusing. For example in up_generate_addpath() it starts > with: > > rde_filterstate_prep(&state, new); > > After this, state.vstate == prefix_roa_vstate(new). > > if (rde_filter(rules, peer, prefix_peer(new), &addr, > prefixlen, &state) == ACTION_DENY) { > rde_filterstate_clean(&state); > continue; > } > > if (up_enforce_open_policy(peer, &state)) { > rde_filterstate_clean(&state); > continue; > } > > /* from here on we know this is an update */ > p = prefix_adjout_get(peer, new->path_id_tx, &addr, prefixlen); > > up_prep_adjout(peer, &state, addr.aid); > > None of the above code touches state.vstate. > > prefix_adjout_update(p, peer, &state, &addr, > prefixlen, new->path_id_tx); > > and here you dropped the prefix_roa_vstate(new) argument that is now > taken from state, which is correct. Agreed. > > In up_generate_update() I think there's something wrong. > > Again, the vstate is copied from new at the start > > rde_filterstate_prep(&state, new); > > then new may or may not change over the next few lines, and the two > rde_filterstate_clean(&state) before prefix_adjout_update() don't > change state's vstate, and state.vstate may or may not match > prefix_adjout_update(new). But those calls have either a break or continue, so either the loop is exited or restarted (depending on PEERFLAG_EVALUATE_ALL). So there should be no way to go from a rde_filterstate_clean(&state) to prefix_adjout_update(new). There is a missing rde_filterstate_clean(&state) in up_generate_update(). if (up_enforce_open_policy(peer, &state)) { rde_filterstate_clean(&state); if (peer->flags & PEERFLAG_EVALUATE_ALL) { new = TAILQ_NEXT(new, entry.list.rib); if (new != NULL && prefix_eligible(new)) continue; } break; } /* check if this was actually a withdraw */ if (need_withdraw) here the filterstate is leaked, which is bad. break; /* from here on we know this is an update */ up_prep_adjout(peer, &state, addr.aid); prefix_adjout_update(p, peer, &state, &addr, new->pt->prefixlen, new->path_id_tx); rde_filterstate_clean(&state); Below a diff that fixes the extra leak. -- :wq Claudio Index: rde.c === RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v retrieving revision 1.588 diff -u -p -r1.588 rde.c --- rde.c 18 Jan 2023 13:20:00 - 1.588 +++ rde.c 18 Jan 2023 17:20:10 - @@ -1744,7 +1744,7 @@ rde_update_update(struct rde_peer *peer, /* add original path to the Adj-RIB-In */ if (prefix_update(rib_byid(RIB_ADJ_IN), peer, path_id, path_id_tx, - in, prefix, prefixlen, in->vstate) == 1) + in, prefix, prefixlen) == 1) peer->prefix_cnt++; /* max prefix checker */ @@ -1772,7 +1772,7 @@ rde_update_update(struct rde_peer *peer, &state.nexthop->exit_nexthop, prefix, prefixlen); prefix_update(rib, peer, path_id, path_id_tx, &state, - prefix, prefixlen, in->vstate); + prefix, prefixlen); } else if (prefix_withdraw(rib, peer, path_id, prefix, prefixlen)) { rde_update_log(wmsg, i, peer, @@ -3847,8 +3847,7 @@ rde_softreconfig_in(struct rib_entry *re /* update Local-RIB */
italic text test program
Here is a trivial program to test the italic text patch I just sent to -tech: #include int main() { printf ("\n\x1b[mThis is normal text.\n"); printf ("\x1b[1mThis is bold text.\n"); printf ("\x1b[22;3mThis is italic text.\n"); printf ("\x1b[1mThis is bold and italic text.\n"); printf ("\x1b[mBack to normal.\n\n"); }
[patch] italic text on the console!
Wouldn't it be nice if we could do italic text on the console? Well, now we can! I've separated out this diff from my main 'console enhancement' patchset for easier testing. This is against -current as of a few minutes ago, and adds just the following features: * Italic text. * Bold text using a real bold font instead of just brighter colour. *** * Important note: * *** Since neither the vt220 nor the pccon terminfo entries include the sitm and ritm sequences, curses-based programs will not recognise the availability of the italic attribute. The xterm terminfo entry _does_ include the sitm and ritm sequences, and with the changes that have gone into -current in the last few days, you should now be able to set TERM=xterm at least for display testing purposes, (the main issue being that F1-F4 won't work yet, because the necessary changes have not been committed yet). But in any case, even without TERM=xterm, you can test these new graphical effects directly with the corresponding escape sequences. I'll send a simple test program in a separate email. Index: dev/rasops/rasops.c === RCS file: /cvs/src/sys/dev/rasops/rasops.c,v retrieving revision 1.69 diff -u -p -r1.69 rasops.c --- dev/rasops/rasops.c 18 Jan 2023 11:08:49 - 1.69 +++ dev/rasops/rasops.c 18 Jan 2023 17:14:42 - @@ -561,7 +561,7 @@ rasops_pack_cattr(void *cookie, int fg, if ((flg & WSATTR_HILIT) != 0 && fg < 8) fg += 8; - *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE); + *attr = (bg << 16) | (fg << 24) | flg; return (0); } @@ -585,7 +585,7 @@ rasops_pack_mattr(void *cookie, int fg, bg = swap; } - *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE); + *attr = (bg << 16) | (fg << 24) | flg; return (0); } Index: dev/rasops/rasops32.c === RCS file: /cvs/src/sys/dev/rasops/rasops32.c,v retrieving revision 1.13 diff -u -p -r1.13 rasops32.c --- dev/rasops/rasops32.c 18 Jan 2023 11:08:49 - 1.13 +++ dev/rasops/rasops32.c 18 Jan 2023 17:14:42 - @@ -107,7 +107,26 @@ rasops32_putchar(void *cookie, int row, } } else { uc -= ri->ri_font->firstchar; - fr = (u_char *)ri->ri_font->data + uc * ri->ri_fontscale; + + /* Choose font data based on bold and italic attributes */ + + u_char * font_dataset; + + switch (attr & (WSATTR_HILIT | WSATTR_ITALIC)) { + case WSATTR_HILIT: + font_dataset=(u_char *)ri->ri_font->data_bold; + break; + case WSATTR_ITALIC: + font_dataset=(u_char *)ri->ri_font->data_italic; + break; + case (WSATTR_HILIT | WSATTR_ITALIC): + font_dataset=(u_char *)ri->ri_font->data_bolditalic; + break; + default: + font_dataset=(u_char *)ri->ri_font->data; + } + + fr = (font_dataset + uc * ri->ri_fontscale); fs = ri->ri_font->stride; /* double-pixel special cases for the common widths */ Index: dev/wscons/wsconsio.h === RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v retrieving revision 1.98 diff -u -p -r1.98 wsconsio.h --- dev/wscons/wsconsio.h 15 Jul 2022 17:57:27 - 1.98 +++ dev/wscons/wsconsio.h 18 Jan 2023 17:14:43 - @@ -536,6 +536,9 @@ struct wsdisplay_font { #defineWSDISPLAY_FONTORDER_R2L 2 void *cookie; void *data; + void *data_bold; + void *data_italic; + void *data_bolditalic; }; #define WSDISPLAYIO_LDFONT _IOW ('W', 77, struct wsdisplay_font) #defineWSDISPLAYIO_LSFONT _IOWR('W', 78, struct wsdisplay_font) Index: dev/wscons/wsdisplayvar.h === RCS file: /cvs/src/sys/dev/wscons/wsdisplayvar.h,v retrieving revision 1.38 diff -u -p -r1.38 wsdisplayvar.h --- dev/wscons/wsdisplayvar.h 13 Sep 2020 10:05:46 - 1.38 +++ dev/wscons/wsdisplayvar.h 18 Jan 2023 17:14:43 - @@ -99,6 +99,7 @@ struct wsdisplay_emulops { #define WSATTR_BLINK 4 #define WSATTR_UNDERLINE 8 #define WSATTR_WSCOLORS 16 +#define WSATTR_ITALIC 32 }; #defineWSSCREEN_NAME_SIZE 16 Index: dev/wscons/wsemul_vt100_subr.c === RCS file: /cvs/src/sys/dev/wscons/wsemul_vt100_subr.c,v retrieving revision 1.29 diff -u -p -r1.29 wsemul_vt100_subr.c --- dev/wscons/wsemul_vt100_subr.c 12 Jan 2023 20:39:37 - 1.29 +++ dev/wscons/wsemul_vt100_subr.c 18 Jan 2023 17:14:43 - @@ -588,6 +5
Re: bgpd, use vstate from filterstate for update functions
On Wed, Jan 18, 2023 at 05:37:37PM +0100, Claudio Jeker wrote: > On Wed, Jan 18, 2023 at 05:18:58PM +0100, Theo Buehler wrote: > > On Wed, Jan 18, 2023 at 02:46:19PM +0100, Claudio Jeker wrote: > > > This is the next step in vstate cleanup. > > > Since the vstate is now part of struct filterstate use that information > > > instead of passing an explicit vstate to the various update functions. > > > > It took me a moment to understand that rde_filterstate_clean() does not > > zero the vstate, so the changes in rde_update.c preserve behavior. > > That's not super intuitive to me, but if that's bound to stay this way, > > Uhm, now I'm a bit confused. Where is the vstate used after calling > rde_filterstate_clean()? I did not spot that issue in rde_update.c > I think we could zero everyting in the filterstate during clean. Just to > be sure. It is super confusing. For example in up_generate_addpath() it starts with: rde_filterstate_prep(&state, new); After this, state.vstate == prefix_roa_vstate(new). if (rde_filter(rules, peer, prefix_peer(new), &addr, prefixlen, &state) == ACTION_DENY) { rde_filterstate_clean(&state); continue; } if (up_enforce_open_policy(peer, &state)) { rde_filterstate_clean(&state); continue; } /* from here on we know this is an update */ p = prefix_adjout_get(peer, new->path_id_tx, &addr, prefixlen); up_prep_adjout(peer, &state, addr.aid); None of the above code touches state.vstate. prefix_adjout_update(p, peer, &state, &addr, prefixlen, new->path_id_tx); and here you dropped the prefix_roa_vstate(new) argument that is now taken from state, which is correct. In up_generate_update() I think there's something wrong. Again, the vstate is copied from new at the start rde_filterstate_prep(&state, new); then new may or may not change over the next few lines, and the two rde_filterstate_clean(&state) before prefix_adjout_update() don't change state's vstate, and state.vstate may or may not match prefix_adjout_update(new).
Re: bgpd, use vstate from filterstate for update functions
On Wed, Jan 18, 2023 at 05:18:58PM +0100, Theo Buehler wrote: > On Wed, Jan 18, 2023 at 02:46:19PM +0100, Claudio Jeker wrote: > > This is the next step in vstate cleanup. > > Since the vstate is now part of struct filterstate use that information > > instead of passing an explicit vstate to the various update functions. > > It took me a moment to understand that rde_filterstate_clean() does not > zero the vstate, so the changes in rde_update.c preserve behavior. > That's not super intuitive to me, but if that's bound to stay this way, Uhm, now I'm a bit confused. Where is the vstate used after calling rde_filterstate_clean()? I did not spot that issue in rde_update.c I think we could zero everyting in the filterstate during clean. Just to be sure. -- :wq Claudio
Re: xonly on amd64: testing wanted
Matthias Schmidt wrote: > I have the kernel patch running on my amd64 machine since you posted it > and noticed no regression so far. All my programs continue to work > (terminal stuff + FF, Chrome, Tor Browser). you mean your old binaries continue working. new ones, there are some problems. It's not quite so simple, but the ports team is hard at work finding and fixing application bugs related to this and soon we can start activating more of this. thank you for the test confirmation.
Re: xonly on amd64: testing wanted
Hi, * Theo de Raadt wrote: > Some of you have probably noticed activity about "xonly" happening > to a bunch of architectures. First arm64, then riscv64, then hppa, > and ongoing efforts with octeon, sparc64 (sun4u only), and more of this > is going to come in the future. > > Like past work decades ago (and I suppose continually also) on W^X, and > increasing use of .rodata, the idea here is to have code (text segments) > not be readable. Or in a more generic sense, if you mprotect a region > with only PROT_EXEC, it is not readable. > > This has a number of nice characteristics. It makes BROP techniques not > work as well (when accompanied by the effects of many other migitations), > it makes complex gadgets containing side effects harder to use (if the > registers involved in the side effect contain pointers to code), etc etc. > > But most of us have amd64 machines. Thrilling news: > > It turns out we can do this fairly modern modern 64-bit x86 machines > from Intel and AMD, if operating in LONG mode, aka. amd64. The cpu > needs to have a feature called PKU. The way this works is not 100% > perfect, but it a serious enough hinderance. A PKU memory key is > instantiated for all memory which is PROT_EXEC-only, and that key is > told to block memory reads. Instruction reads are still permitted. Now > some of you may know how PKU works, so you will say that userland can > change the behaviour and make the memory readable. That is true. Until > a system call happens. Then we force it back to blocking read. Or a > memory fault, we force it back. Or an interrupt, even a clock > interrupt. We force it back. Generally if an attacker is trying to > read code it is because they don't have a lot of (turing complete or a > subset) of flexibility and want more information. Imagine they are able > to generate a the "wrpkru" sequence to disable it, and then do something > else? My guess is if they can do two things in a row, then they already > have power, and won't be doing this. So this is a protection method > against a lower-level attack construction. The concept is this: If you > can bypass this to gain a tiny foothold, you would not have bothered, > because you have more power and would be reaching higher. > > As I mention, some other architectures have crossed already, but not > without little bits of pain. Especially in the ports tree, where > unprepared code is more common. Mostly this consists of assembly language > files that have put read-only data into the .text region, instead of into > the .rodata (some people still haven't read the memos from around 1997). > > So I'd like to recruit some help from those of you capable of building > your own kernels. Can you apply the following kernel diff, and try the > applications you are used to. A list of applications that fail on some > way would be handy. Be sure to ktrace -di then, and check there is a > SIGSEGV near the end, and include a snippet of that information. > > If you don't know how to do this, please don't ask for help. Just let > others do it. > > The kernel diff isn't perfect yet, because it is less than 24 hours > since this started working. But it appears good enough for testing, > while we work out the wrinkles. I have the kernel patch running on my amd64 machine since you posted it and noticed no regression so far. All my programs continue to work (terminal stuff + FF, Chrome, Tor Browser). Cheers Matthias userland kernel ld.so 0xa518a30d640 readable readable mmap xz0xa5196f17000 unreadable unreadable mmap x 0xa51c7907000 unreadable unreadable mmap nrx 0xa5237b3b000 unreadable unreadable mmap nwx 0xa51aaaea000 unreadable unreadable mmap xnwx 0xa52074ba000 unreadable unreadable main 0xa4f5adab490 readable readable libc 0xa51da456410 readable readable OpenBSD 7.2-current (GENERIC.MP) #10: Sun Jan 15 11:17:26 CET 2023 x...@kronos.xosc.net:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 34064420864 (32486MB) avail mem = 33012658176 (31483MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 3.3 @ 0x43ca1000 (119 entries) bios0: vendor American Megatrends International, LLC. version "N.1.15A09" date 03/24/2022 bios0: TUXEDO TUXEDO InfinityBook Pro 14 Gen6 efi0 at bios0: UEFI 2.7 efi0: American Megatrends rev 0x50013 acpi0 at bios0: ACPI 6.2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP MCFG SSDT FIDT SSDT SSDT SSDT HPET APIC SSDT SSDT NHLT UEFI LPIT SSDT SSDT DBGP DBG2 SSDT DMAR SSDT SSDT BGRT PTDT WSMT FPDT acpi0: wakeup devices PEGP(S4) PEGP(S4) PEGP(S4) PEG0(S4) PEGP(S4) RP01(S4) PXSX(S4) RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) RP06(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits
Re: bgpd, use vstate from filterstate for update functions
On Wed, Jan 18, 2023 at 02:46:19PM +0100, Claudio Jeker wrote: > This is the next step in vstate cleanup. > Since the vstate is now part of struct filterstate use that information > instead of passing an explicit vstate to the various update functions. It took me a moment to understand that rde_filterstate_clean() does not zero the vstate, so the changes in rde_update.c preserve behavior. That's not super intuitive to me, but if that's bound to stay this way, ok tb
Re: Preferred TERM for pkg_add
Is there any benefit to pkg_add's TERM handling now that it no longers uses the full terminal width? To my eye the visual output looks the same with TERM=dumb (though presumably it will avoid the intermittent problem where somewhere between pkg_add, termcap and urxvt, pkg_add -u output jumps to the top of the terminal window and overwrites its previous output).
bgpd, use vstate from filterstate for update functions
This is the next step in vstate cleanup. Since the vstate is now part of struct filterstate use that information instead of passing an explicit vstate to the various update functions. -- :wq Claudio Index: rde.c === RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v retrieving revision 1.587 diff -u -p -r1.587 rde.c --- rde.c 17 Jan 2023 16:09:01 - 1.587 +++ rde.c 18 Jan 2023 13:31:23 - @@ -1744,7 +1744,7 @@ rde_update_update(struct rde_peer *peer, /* add original path to the Adj-RIB-In */ if (prefix_update(rib_byid(RIB_ADJ_IN), peer, path_id, path_id_tx, - in, prefix, prefixlen, in->vstate) == 1) + in, prefix, prefixlen) == 1) peer->prefix_cnt++; /* max prefix checker */ @@ -1772,7 +1772,7 @@ rde_update_update(struct rde_peer *peer, &state.nexthop->exit_nexthop, prefix, prefixlen); prefix_update(rib, peer, path_id, path_id_tx, &state, - prefix, prefixlen, in->vstate); + prefix, prefixlen); } else if (prefix_withdraw(rib, peer, path_id, prefix, prefixlen)) { rde_update_log(wmsg, i, peer, @@ -3847,8 +3847,7 @@ rde_softreconfig_in(struct rib_entry *re /* update Local-RIB */ prefix_update(rib, peer, p->path_id, p->path_id_tx, &state, - &prefix, pt->prefixlen, - prefix_roa_vstate(p)); + &prefix, pt->prefixlen); } else if (action == ACTION_DENY) { /* remove from Local-RIB */ prefix_withdraw(rib, peer, p->path_id, &prefix, @@ -3986,8 +3985,7 @@ rde_roa_softreload(struct rib_entry *re, /* update Local-RIB */ prefix_update(rib, peer, p->path_id, p->path_id_tx, &state, - &prefix, pt->prefixlen, - prefix_roa_vstate(p)); + &prefix, pt->prefixlen); } else if (action == ACTION_DENY) { /* remove from Local-RIB */ prefix_withdraw(rib, peer, p->path_id, &prefix, @@ -4187,7 +4185,6 @@ network_add(struct network_config *nc, s struct filter_set_head *vpnset = NULL; struct in_addr prefix4; struct in6_addr prefix6; - uint8_t vstate; uint16_t i; uint32_t path_id_tx; @@ -4249,14 +4246,16 @@ network_add(struct network_config *nc, s rde_apply_set(vpnset, peerself, peerself, state, nc->prefix.aid); + path_id_tx = pathid_assign(peerself, 0, &nc->prefix, nc->prefixlen); + #if NOTYET - state.aspath.aspa_state = ASPA_NEVER_KNOWN; + state->aspath.aspa_state = ASPA_NEVER_KNOWN; #endif - vstate = rde_roa_validity(&rde_roa, &nc->prefix, + state->vstate = rde_roa_validity(&rde_roa, &nc->prefix, nc->prefixlen, aspath_origin(state->aspath.aspath)); - path_id_tx = pathid_assign(peerself, 0, &nc->prefix, nc->prefixlen); + if (prefix_update(rib_byid(RIB_ADJ_IN), peerself, 0, path_id_tx, - state, &nc->prefix, nc->prefixlen, vstate) == 1) + state, &nc->prefix, nc->prefixlen) == 1) peerself->prefix_cnt++; for (i = RIB_LOC_START; i < rib_size; i++) { struct rib *rib = rib_byid(i); @@ -4266,7 +4265,7 @@ network_add(struct network_config *nc, s state->nexthop ? &state->nexthop->exit_nexthop : NULL, &nc->prefix, nc->prefixlen); prefix_update(rib, peerself, 0, path_id_tx, state, &nc->prefix, - nc->prefixlen, vstate); + nc->prefixlen); } filterset_free(&nc->attrset); } Index: rde.h === RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v retrieving revision 1.279 diff -u -p -r1.279 rde.h --- rde.h 17 Jan 2023 16:09:01 - 1.279 +++ rde.h 17 Jan 2023 16:09:50 - @@ -624,14 +624,12 @@ struct prefix *prefix_adjout_lookup(stru int); struct prefix *prefix_adjout_next(struct rde_peer *, struct prefix *); int prefix_update(struct rib *, struct rde_peer *, uint32_t, - uint32_t, struct filterstate *, struct bgpd_addr *, - int, uint8_t); + uint32_t, struct filterstate *, struct bgpd_addr *, int); int prefix_withd
Re: bgpd, small optimisation
On Wed, Jan 18, 2023 at 12:06:08PM +0100, Claudio Jeker wrote: > In the RDE the poll loop needs to know if any additional work is pending. > This is done calling various functions and if anyone has pending work the > timeout is reduced to 0. > > Now some of the functions will more often trigger than others. So it is > best to order them accordingly. Check for incoming and outgoing updates > first (these are most frequently true). Then nexthop_pending() since it is > cheap and finally rib_dump_pending(). > > Also try to make these functions as cheap as possible. In the case for > peer_imsg_pending() this can be done by a simple imsg_pending counter. > This way there is no need to loop over all peers. Makes sense and readas fine. ok tb > Similar changes may be possible for other checks but they are a bit more > complicated (apart from nexthop_pending() which is already minimal). > > -- > :wq Claudio > > Index: rde.c > === > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v > retrieving revision 1.586 > diff -u -p -r1.586 rde.c > --- rde.c 16 Jan 2023 10:37:08 - 1.586 > +++ rde.c 18 Jan 2023 10:50:54 - > @@ -248,8 +248,8 @@ rde_main(int debug, int verbose) > } > } > > - if (rib_dump_pending() || rde_update_queue_pending() || > - nexthop_pending() || peer_imsg_pending()) > + if (peer_imsg_pending() || rde_update_queue_pending() || > + nexthop_pending() || rib_dump_pending()) > timeout = 0; > > if (poll(pfd, i, timeout) == -1) { > Index: rde_peer.c > === > RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v > retrieving revision 1.25 > diff -u -p -r1.25 rde_peer.c > --- rde_peer.c23 Sep 2022 15:49:20 - 1.25 > +++ rde_peer.c18 Jan 2023 10:51:57 - > @@ -28,6 +28,7 @@ > > struct peer_tree peertable; > struct rde_peer *peerself; > +static long imsg_pending; > > CTASSERT(sizeof(peerself->recv_eor) * 8 > AID_MAX); > CTASSERT(sizeof(peerself->sent_eor) * 8 > AID_MAX); > @@ -610,6 +611,7 @@ peer_imsg_push(struct rde_peer *peer, st > fatal(NULL); > imsg_move(&iq->imsg, imsg); > SIMPLEQ_INSERT_TAIL(&peer->imsg_queue, iq, entry); > + imsg_pending++; > } > > /* > @@ -629,29 +631,18 @@ peer_imsg_pop(struct rde_peer *peer, str > > SIMPLEQ_REMOVE_HEAD(&peer->imsg_queue, entry); > free(iq); > + imsg_pending--; > > return 1; > } > > -static void > -peer_imsg_queued(struct rde_peer *peer, void *arg) > -{ > - int *p = arg; > - > - *p = *p || !SIMPLEQ_EMPTY(&peer->imsg_queue); > -} > - > /* > * Check if any imsg are pending, return 0 if none are pending > */ > int > peer_imsg_pending(void) > { > - int pending = 0; > - > - peer_foreach(peer_imsg_queued, &pending); > - > - return pending; > + return imsg_pending != 0; > } > > /* > @@ -665,5 +656,6 @@ peer_imsg_flush(struct rde_peer *peer) > while ((iq = SIMPLEQ_FIRST(&peer->imsg_queue)) != NULL) { > SIMPLEQ_REMOVE_HEAD(&peer->imsg_queue, entry); > free(iq); > + imsg_pending--; > } > } >
bgpd, small optimisation
In the RDE the poll loop needs to know if any additional work is pending. This is done calling various functions and if anyone has pending work the timeout is reduced to 0. Now some of the functions will more often trigger than others. So it is best to order them accordingly. Check for incoming and outgoing updates first (these are most frequently true). Then nexthop_pending() since it is cheap and finally rib_dump_pending(). Also try to make these functions as cheap as possible. In the case for peer_imsg_pending() this can be done by a simple imsg_pending counter. This way there is no need to loop over all peers. Similar changes may be possible for other checks but they are a bit more complicated (apart from nexthop_pending() which is already minimal). -- :wq Claudio Index: rde.c === RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v retrieving revision 1.586 diff -u -p -r1.586 rde.c --- rde.c 16 Jan 2023 10:37:08 - 1.586 +++ rde.c 18 Jan 2023 10:50:54 - @@ -248,8 +248,8 @@ rde_main(int debug, int verbose) } } - if (rib_dump_pending() || rde_update_queue_pending() || - nexthop_pending() || peer_imsg_pending()) + if (peer_imsg_pending() || rde_update_queue_pending() || + nexthop_pending() || rib_dump_pending()) timeout = 0; if (poll(pfd, i, timeout) == -1) { Index: rde_peer.c === RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v retrieving revision 1.25 diff -u -p -r1.25 rde_peer.c --- rde_peer.c 23 Sep 2022 15:49:20 - 1.25 +++ rde_peer.c 18 Jan 2023 10:51:57 - @@ -28,6 +28,7 @@ struct peer_treepeertable; struct rde_peer*peerself; +static long imsg_pending; CTASSERT(sizeof(peerself->recv_eor) * 8 > AID_MAX); CTASSERT(sizeof(peerself->sent_eor) * 8 > AID_MAX); @@ -610,6 +611,7 @@ peer_imsg_push(struct rde_peer *peer, st fatal(NULL); imsg_move(&iq->imsg, imsg); SIMPLEQ_INSERT_TAIL(&peer->imsg_queue, iq, entry); + imsg_pending++; } /* @@ -629,29 +631,18 @@ peer_imsg_pop(struct rde_peer *peer, str SIMPLEQ_REMOVE_HEAD(&peer->imsg_queue, entry); free(iq); + imsg_pending--; return 1; } -static void -peer_imsg_queued(struct rde_peer *peer, void *arg) -{ - int *p = arg; - - *p = *p || !SIMPLEQ_EMPTY(&peer->imsg_queue); -} - /* * Check if any imsg are pending, return 0 if none are pending */ int peer_imsg_pending(void) { - int pending = 0; - - peer_foreach(peer_imsg_queued, &pending); - - return pending; + return imsg_pending != 0; } /* @@ -665,5 +656,6 @@ peer_imsg_flush(struct rde_peer *peer) while ((iq = SIMPLEQ_FIRST(&peer->imsg_queue)) != NULL) { SIMPLEQ_REMOVE_HEAD(&peer->imsg_queue, entry); free(iq); + imsg_pending--; } }