Re: TP-Link TL-WN822N-EU v5 USB ID
On Mon, May 25, 2020 at 11:27:27PM +0300, Tero Koskinen wrote: > Hi, > > I have TP-Link TL-WN822N-EU v5 USB adapter[1] with > USB VID/PID 2357:0108: > > $ usbdevs > Controller /dev/usb0: > addr 01: 1022: AMD, xHCI root hub > addr 02: 2357:0108 Realtek, 802.11n NIC > Controller /dev/usb1: > addr 01: 1022: AMD, EHCI root hub > addr 02: 0438:7900 Advanced Micro Devices, Hub > $ > > Patch at the end adds it to the usbdevs. I hope I got it right > as I added it between existing 0x0107 and 0x0109 IDs. > > Relevant dmesg bits with the patch: > urtwn0 at uhub0 port 3 configuration 1 interface 0 "Realtek 802.11n NIC" rev > 2.10/2.00 addr 2 > urtwn0: MAC/BB RTL8192EU, RF 6052 2T2R, address 50:3e:aa:24:32:9e > uhub2 at uhub1 port 1 configuration 1 interface 0 "Advanced Micro Devices > Hub" rev 2.00/0.18 addr 2 > > Full dmesg at https://tkoskine.me/dmesg.tplink-TL-WN822N.txt > > Yours, > Tero thanks, committed
Re: WireGuard patchset for OpenBSD
On Mon, May 25, 2020 at 2:16 PM Theo de Raadt wrote: > > I'll make a comment that I am quite unhappy about this ioctl > methodology. I don't like void *'s and varying sizes. > > I would be much happier if this was a fixed structure, filled with > known objects. > > It looks fragile. Indeed the fragile linked list magic went away pretty shortly after Matt posted the initial patchset. We'll have v2 out hopefully tomorrow if all goes well, where we've been using a much more boring approach of stacked structs, which in practice is very easy to program for and deal with. Stay tuned. And sorry about the weird linked list thing. Jason
Re: locale thread support
Hi, my impression is there are two totally independent topics in this thread. 1. Whether and how to make things safer, ideally terminating the program in a controlled manner, if it uses functions that are not thread-safe in an invalid manner. I'm not addressing that topic in this mail. 2. Whether we want additional non-standard xlocale.h functions in our libc. Regarding topic 2, let me say up front that i do not feel strongly either way. It seems to me this is mostly a question for porters. If some functions are widely used - even if non-standard - and help avoid porting hassle, then sure, let's have them unless they cause excessive fuss. But the latter does not seem likely here. There are a number of functions that seem likely useful to me off the top of my head, for example: querylocale, nl_langinfo_l, MB_CUR_MAX_L, wcwidth_l In general, i must say i like the whole xlocale.h business not all that much: in a nutshell, it is doubling the number of half the functions under the sun, which usually indicates poor design in the first place, and then the vast majority of these additional functions are almost useless on any operating system and totally useless on OpenBSD. Like, i mean, btowc_l(3)? Or even the isdigit_l(3) which we already have? You must be choking, Mr. Chisnall! I don't think stuff should be added because it is out there, but only if there is at least some porting benefit. Regarding the FreeBSD headers, i like them even less. There are both terrible design choices and terrible implementation choices. Regarding design, it appears that you *must* #include after other headers - if you include it before, it won't work. Regarding implementation, there is quite repulsive macro abuse in xlocale/_ctype.h; but probably that can be unrolled in LibreSSL style. Either way, none of that hinders providing them if doing so yields benefit. See below for a list of functions that i drafted extremely quickly, so beware of errors and omissions. The point isn't to present a definite plan. The point is merely to demonstrate the sheer fatness of the animal and to give a first impression of the degree to which it stinks and grunts from most of its ends. If any of this is needed, do porters already know which functions are in particularly high demand? Do you have any idea how to find out which ones are actually useful for porting purposes? Yours, Ingo Functions we already have marked are with a '*'. useful: querylocale useful: nl_langinfo_l* useful: MB_CUR_MAX_L marginally useful: mbtowc_l mbstowcs_l wctomb_l wcstombs_l useless: atof_l atoi_l atol_l atoll_l strtod_l strtof_l strtol_l strtold_l strtoll_l strtoul_l strtoull_l mblen_l useful: wcwidth_l wcswidth_l marginally useful: fgetwc_l fgetws_l fputwc_l fputws_l getwc_l getwchar_l putwc_l putwchar_l ungetwc_l fwprintf_l fwscanf_l swprintf_l swscanf_l vfwprintf_l vswprintf_l vwprintf_l wprintf_l wscanf_l vfwscanf_l vswscanf_l vwscanf_l mbrlen_l mbrtowc_l mbsinit_l mbsrtowcs_l wcrtomb_l wcsrtombs_l wctob_l mbsnrtowcs_l wcsnrtombs_l useless: btowc_l wcsftime_l wcstod_l wcstol_l wcstoul_l wcstof_l wcstold_l wcstoll_l wcstoull_l useless: strcasestr_l strcoll_l* strxfrm_l* useless: strcasecmp_l* strncasecmp_l* useless: strtoimax_l strtoumax_l wcstoimax_l wcstoumax_l useless: strfmon_l useless: strftime_l* strptime_l marginally useful: towlower_l* towupper_l* iswctype_l* towctrans_l* not sure: nextwctype_l wctype_l wctrans_l* digittoint_l useless: isalnum_l* etc. tolower_l* toupper_l* include useless: asprintf_l dprintf_l fprintf_l fscanf_l printf_l scanf_l snprintf_l sprintf_l sscanf_l vfprintf_l vprintf_l vsprintf_l vfscanf_l vscanf_l vsnprintf_l vsscanf_l vdprintf_l vasprintf_l not sure: c16rtomb_l c32rtomb_l mbrtoc16_l mbrtoc32_l
Re: sysupgrade change to allow installing from url
Solene Rapenne(sol...@perso.pw) on 2020.05.25 15:25:40 +0200:
> Hi,
>
> I don't know if this will be accepted but I propose to add a -u [url]
> parameter to use older snapshots from an archive server for example.
>
> I wanted to add an optional parameter to -s at first but in case of
> sysupgrade -s [installurl] the install url would have been mistaken by
> this.
>
> If you specify a -u url and installurl at the same time, -u url
> superseed installurl.
>
> This would allow easier bisecting using older snapshots.
I have fixed the way the directories are laid out, so now you can do this
# echo "http://ftp.hostserver.de/archive/2020-05-14-0105"; > /etc/installurl
# sysupgrade -n
# strings /home/_sysupgrade/bsd | grep 6.7-current
@(#)OpenBSD 6.7-current (GENERIC) #181: Wed May 13 12:43:30 MDT 2020
> Index: sysupgrade.8
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
> retrieving revision 1.10
> diff -u -p -r1.10 sysupgrade.8
> --- sysupgrade.8 3 Oct 2019 12:43:58 - 1.10
> +++ sysupgrade.8 25 May 2020 13:23:06 -
> @@ -24,6 +24,7 @@
> .Nm
> .Op Fl fkn
> .Op Fl r | s
> +.Op Fl u Pa url
> .Op Ar installurl
> .Sh DESCRIPTION
> .Nm
> @@ -66,6 +67,13 @@ This is the default if the system is cur
> .It Fl s
> Upgrade to a snapshot.
> This is the default if the system is currently running a snapshot.
> +.It Fl u Pa url
> +Fetch and verify the files downloaded from
> +.Pa url .
> +This is superseeding
> +.Op Pa installurl ,
> +its usage is intended for installing older snapshots available under url not
> following
> +the usual installurl format.
> .El
> .Sh FILES
> .Bl -tag -width "/auto_upgrade.conf" -compact
> Index: sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.37
> diff -u -p -r1.37 sysupgrade.sh
> --- sysupgrade.sh 26 Jan 2020 22:08:36 - 1.37
> +++ sysupgrade.sh 25 May 2020 13:23:06 -
> @@ -34,7 +34,7 @@ ug_err()
>
> usage()
> {
> - ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
> + ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-u url] [installurl]"
> }
>
> unpriv()
> @@ -79,13 +79,14 @@ FORCE=false
> KEEP=false
> REBOOT=true
>
> -while getopts fknrs arg; do
> +while getopts fknrsu: arg; do
> case ${arg} in
> f) FORCE=true;;
> k) KEEP=true;;
> n) REBOOT=false;;
> r) RELEASE=true;;
> s) SNAP=true;;
> + u) SNAPURL=${OPTARG};;
> *) usage;;
> esac
> done
> @@ -121,7 +122,11 @@ else
> fi
>
> if $SNAP; then
> - URL=${MIRROR}/snapshots/${ARCH}/
> + if [[ -n "$SNAPURL" ]]; then
> + URL=$SNAPURL
> + else
> + URL=${MIRROR}/snapshots/${ARCH}/
> + fi
> else
> URL=${MIRROR}/${NEXT_VERSION}/${ARCH}/
> fi
>
Re: Provide ldomctl(8) better error messages when parsing ldom.conf
On Mon, May 25, 2020 at 05:55:39PM +0200, Mark Kettenis wrote: > Including numbers is good! I feel, quite strongly, that lowercase > vcpu isn't right. So please use VCPU (or plural VCPUs) instead. I > also think that your proposal is a bit inconsistent regarding > and > "larger than". So: > > ldomctl: requested VCPUs (256) larger than available resources (64) > ldomctl: requested memory (213674622976) larger than availableresources > (137428467712) I concur. > Now that last message is a bit too long and I think those numbers look > a bit silly, so better would be: > > ldomctl: requested memory (203776M) larger than available resources (131062M) > > I think MB is the right unit. you could avaid a silly x > x by > rounding the requested memory up to the nearest MB. MB is the smallest common unit; also, as far as I can remember the specs, the hypervisor requires memory for domains to be somewhat aligned, e.g., a multiple of 128M or so; that is to say any "odd" amount of memory is unlikely to work anyway and printing errors with MB seems the most sanest approach. I also mentioned to kmos that we could make the error messages more user friendly: "resource_id larger than max_guests" might be grasped, but "resource_id larger than max_ldcs", etc. only seem confusing unless you know what logical domain channels are. So these should probably say something along the lines of "Too many domains defined, maximum is " or so.
TP-Link TL-WN822N-EU v5 USB ID
Hi,
I have TP-Link TL-WN822N-EU v5 USB adapter[1] with
USB VID/PID 2357:0108:
$ usbdevs
Controller /dev/usb0:
addr 01: 1022: AMD, xHCI root hub
addr 02: 2357:0108 Realtek, 802.11n NIC
Controller /dev/usb1:
addr 01: 1022: AMD, EHCI root hub
addr 02: 0438:7900 Advanced Micro Devices, Hub
$
Patch at the end adds it to the usbdevs. I hope I got it right
as I added it between existing 0x0107 and 0x0109 IDs.
Relevant dmesg bits with the patch:
urtwn0 at uhub0 port 3 configuration 1 interface 0 "Realtek 802.11n NIC" rev
2.10/2.00 addr 2
urtwn0: MAC/BB RTL8192EU, RF 6052 2T2R, address 50:3e:aa:24:32:9e
uhub2 at uhub1 port 1 configuration 1 interface 0 "Advanced Micro Devices Hub"
rev 2.00/0.18 addr 2
Full dmesg at https://tkoskine.me/dmesg.tplink-TL-WN822N.txt
Yours,
Tero
[1] https://www.tp-link.com/fi/home-networking/adapter/tl-wn822n/
>From 4345fd8a74bb732f0653257a8dd5aac21078852a Mon Sep 17 00:00:00 2001
From: Tero Koskinen
Date: Mon, 25 May 2020 21:35:50 +0300
Subject: [PATCH] Add USB VID/PID for TP-Link TL-WN822N(EU) ver 5.0.
---
sys/dev/usb/if_urtwn.c | 3 ++-
sys/dev/usb/usbdevs| 3 ++-
sys/dev/usb/usbdevs.h | 3 ++-
sys/dev/usb/usbdevs_data.h | 4
4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/sys/dev/usb/if_urtwn.c b/sys/dev/usb/if_urtwn.c
index a51dc1003b3..1553d4573db 100644
--- a/sys/dev/usb/if_urtwn.c
+++ b/sys/dev/usb/if_urtwn.c
@@ -332,7 +332,8 @@ static const struct urtwn_type {
URTWN_DEV_8192EU(DLINK, DWA131E1),
URTWN_DEV_8192EU(REALTEK, RTL8192EU),
URTWN_DEV_8192EU(TPLINK,RTL8192EU),
- URTWN_DEV_8192EU(TPLINK,RTL8192EU_2)
+ URTWN_DEV_8192EU(TPLINK,RTL8192EU_2),
+ URTWN_DEV_8192EU(TPLINK,RTL8192EU_3)
};
#define urtwn_lookup(v, p) \
diff --git a/sys/dev/usb/usbdevs b/sys/dev/usb/usbdevs
index d5cd124eb04..b98a6f73a2d 100644
--- a/sys/dev/usb/usbdevs
+++ b/sys/dev/usb/usbdevs
@@ -4294,7 +4294,8 @@ product TOSHIBA HSDPA 0x1302 HSDPA
product TPLINK RTL8192CU 0x0100 RTL8192CU
product TPLINK RTL8812AU 0x0101 RTL8812AU
product TPLINK RTL8192EU 0x0107 RTL8192EU
-product TPLINK RTL8192EU_2 0x0109 RTL8192EU
+product TPLINK RTL8192EU_2 0x0108 RTL8192EU
+product TPLINK RTL8192EU_3 0x0109 RTL8192EU
product TPLINK RTL8188EUS 0x010c RTL8188EUS
/* Trek Technology products */
diff --git a/sys/dev/usb/usbdevs.h b/sys/dev/usb/usbdevs.h
index 88dac9726cb..a5335540bb3 100644
--- a/sys/dev/usb/usbdevs.h
+++ b/sys/dev/usb/usbdevs.h
@@ -4301,7 +4301,8 @@
#defineUSB_PRODUCT_TPLINK_RTL8192CU0x0100 /* RTL8192CU */
#defineUSB_PRODUCT_TPLINK_RTL8812AU0x0101 /* RTL8812AU */
#defineUSB_PRODUCT_TPLINK_RTL8192EU0x0107 /* RTL8192EU */
-#defineUSB_PRODUCT_TPLINK_RTL8192EU_2 0x0109 /* RTL8192EU */
+#defineUSB_PRODUCT_TPLINK_RTL8192EU_2 0x0108 /* RTL8192EU */
+#defineUSB_PRODUCT_TPLINK_RTL8192EU_3 0x0109 /* RTL8192EU */
#defineUSB_PRODUCT_TPLINK_RTL8188EUS 0x010c /* RTL8188EUS */
/* Trek Technology products */
diff --git a/sys/dev/usb/usbdevs_data.h b/sys/dev/usb/usbdevs_data.h
index 13cda812e87..9ac144a27bc 100644
--- a/sys/dev/usb/usbdevs_data.h
+++ b/sys/dev/usb/usbdevs_data.h
@@ -11005,6 +11005,10 @@ const struct usb_known_product usb_known_products[] = {
USB_VENDOR_TPLINK, USB_PRODUCT_TPLINK_RTL8192EU_2,
"RTL8192EU",
},
+ {
+ USB_VENDOR_TPLINK, USB_PRODUCT_TPLINK_RTL8192EU_3,
+ "RTL8192EU",
+ },
{
USB_VENDOR_TPLINK, USB_PRODUCT_TPLINK_RTL8188EUS,
"RTL8188EUS",
--
2.26.2
Re: WireGuard patchset for OpenBSD
I'll make a comment that I am quite unhappy about this ioctl
methodology. I don't like void *'s and varying sizes.
I would be much happier if this was a fixed structure, filled with
known objects.
It looks fragile.
Tobias Heider wrote:
> Hi Matt,
>
> i finally found some time to look at your diff and it looks pretty good
> to me so far. I have a few question about the SIOCGWG ioctl.
>
> > +void
> > +wg_status(void)
> > +{
> > + size_t i, j, size = 0;
> > + struct timespec now;
> > + char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
> > + char key[WG_BASE64_KEY_LEN + 1];
> > + struct wg_data_iowgdata;
> > + struct wg_interface_io *iface;
> > +
> > + strlcpy(wgdata.wgd_name, ifname, sizeof(wgdata.wgd_name));
> > + wgdata.wgd_size = 0;
> > + wgdata.wgd_mem = NULL;
> > +
> > + if (ioctl(sock, SIOCGWG, (caddr_t)&wgdata) == -1 &&
> > + (errno == ENOTTY || errno == EPERM))
> > + return;
> > +
> > + while (size < wgdata.wgd_size) {
> > + size = wgdata.wgd_size;
> > + wgdata.wgd_mem = realloc(wgdata.wgd_mem, size);
> > + if (ioctl(sock, SIOCGWG, (caddr_t)&wgdata) == -1)
> > + err(1, "SIOCGWG");
> > + }
>
> As I understand it you call the IOCTL at least twice. The first time
> you pass in 'wgd_size == 0'. The IOCTL handler calculates the required size
> and returns it in wgd_size. Then you pass in a realloced wgdata until the
> ioctl returns 0 and the size has not changed.
>
> > +
> > + iface = wgdata.wgd_mem;
> > +int
> > +wg_ioctl_get(struct wg_softc *sc, struct wg_data_io *data)
> > +{
> > + struct wg_interface_io *iface_p, iface_o;
> > + struct wg_peer_io *peer_p, peer_o;
> > + struct wg_aip_io*aip_p, aip_o;
> > +
> > + struct wg_peer *peer;
> > + struct wg_aip *aip;
> > +
> > + size_t size, i;
> > + void*mem;
> > + int ret = 0;
> > +
> > + rw_enter_read(&sc->sc_lock);
> > +
> > + size = sizeof(struct wg_interface_io);
> > + if (((SIZE_MAX - size) / sizeof(struct wg_peer_io)) < sc->sc_peer_num)
> > + goto error;
> > + size += sizeof(struct wg_peer_io) * sc->sc_peer_num;
> > + if (((SIZE_MAX - size) / sizeof(struct wg_aip_io)) < sc->sc_aip_num)
> > + goto error;
> > + size += sizeof(struct wg_aip_io) * sc->sc_aip_num;
> > +
> > + if (data->wgd_size < size)
> > + goto error;
>
> The (data->wgd_size < size) is for the loop in ifconfig. If the input size
> was too small you 'goto error', which is an unfortunate name because it
> is not actually an error but sets 'wgd_size' to the required 'size' and
> returns 0. Maybe 'cleanup' would be a better name.
>
> What I don't understand is why the two previous checks that make sure the
> result fits into SIZE_MAX 'goto error' without setting ret != 0 (thus breaking
> the loop in ifconfig without data actually being written).
> It looks a bit like the error got lost in refactoring.
> ERANGE would probably be a good choice:
>
> 34 ERANGE Result too large. A result of the function was too large to fit
> in the available space (perhaps exceeded precision).
>
> > +
> > +error:
> > + rw_exit_read(&sc->sc_lock);
> > + explicit_bzero(&iface_o, sizeof(iface_o));
> > + explicit_bzero(&peer_o, sizeof(peer_o));
> > + data->wgd_size = size;
> > + return ret;
> > +}
> > +
>
Re: WireGuard patchset for OpenBSD
Hi Matt,
i finally found some time to look at your diff and it looks pretty good
to me so far. I have a few question about the SIOCGWG ioctl.
> +void
> +wg_status(void)
> +{
> + size_t i, j, size = 0;
> + struct timespec now;
> + char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
> + char key[WG_BASE64_KEY_LEN + 1];
> + struct wg_data_iowgdata;
> + struct wg_interface_io *iface;
> +
> + strlcpy(wgdata.wgd_name, ifname, sizeof(wgdata.wgd_name));
> + wgdata.wgd_size = 0;
> + wgdata.wgd_mem = NULL;
> +
> + if (ioctl(sock, SIOCGWG, (caddr_t)&wgdata) == -1 &&
> + (errno == ENOTTY || errno == EPERM))
> + return;
> +
> + while (size < wgdata.wgd_size) {
> + size = wgdata.wgd_size;
> + wgdata.wgd_mem = realloc(wgdata.wgd_mem, size);
> + if (ioctl(sock, SIOCGWG, (caddr_t)&wgdata) == -1)
> + err(1, "SIOCGWG");
> + }
As I understand it you call the IOCTL at least twice. The first time
you pass in 'wgd_size == 0'. The IOCTL handler calculates the required size
and returns it in wgd_size. Then you pass in a realloced wgdata until the
ioctl returns 0 and the size has not changed.
> +
> + iface = wgdata.wgd_mem;
> +int
> +wg_ioctl_get(struct wg_softc *sc, struct wg_data_io *data)
> +{
> + struct wg_interface_io *iface_p, iface_o;
> + struct wg_peer_io *peer_p, peer_o;
> + struct wg_aip_io*aip_p, aip_o;
> +
> + struct wg_peer *peer;
> + struct wg_aip *aip;
> +
> + size_t size, i;
> + void*mem;
> + int ret = 0;
> +
> + rw_enter_read(&sc->sc_lock);
> +
> + size = sizeof(struct wg_interface_io);
> + if (((SIZE_MAX - size) / sizeof(struct wg_peer_io)) < sc->sc_peer_num)
> + goto error;
> + size += sizeof(struct wg_peer_io) * sc->sc_peer_num;
> + if (((SIZE_MAX - size) / sizeof(struct wg_aip_io)) < sc->sc_aip_num)
> + goto error;
> + size += sizeof(struct wg_aip_io) * sc->sc_aip_num;
> +
> + if (data->wgd_size < size)
> + goto error;
The (data->wgd_size < size) is for the loop in ifconfig. If the input size
was too small you 'goto error', which is an unfortunate name because it
is not actually an error but sets 'wgd_size' to the required 'size' and
returns 0. Maybe 'cleanup' would be a better name.
What I don't understand is why the two previous checks that make sure the
result fits into SIZE_MAX 'goto error' without setting ret != 0 (thus breaking
the loop in ifconfig without data actually being written).
It looks a bit like the error got lost in refactoring.
ERANGE would probably be a good choice:
34 ERANGE Result too large. A result of the function was too large to fit
in the available space (perhaps exceeded precision).
> +
> +error:
> + rw_exit_read(&sc->sc_lock);
> + explicit_bzero(&iface_o, sizeof(iface_o));
> + explicit_bzero(&peer_o, sizeof(peer_o));
> + data->wgd_size = size;
> + return ret;
> +}
> +
Re: [PATCH] pipex(4): rework PPP input
Hello Vitaliy, On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev wrote: > > On 23 May 2020, at 13:11, Sergey Ryazanov wrote: > > On Wed, May 20, 2020 at 10:13 PM Vitaliy Makkoveev > > wrote: > >> On Wed, May 20, 2020 at 04:08:01AM +0300, Sergey Ryazanov wrote: > >>> On Tue, May 19, 2020 at 12:11 PM Vitaliy Makkoveev > >>> wrote: > On Mon, May 04, 2020 at 10:03:40PM +0300, Sergey Ryazanov wrote: > > Split checks from frame accepting with header removing in the common > > PPP input function. This should fix packet capture on a PPP interfaces. > > Can you describe the problem you fix? As mpi@ pointed to me, reviewers > are stupid and have no telepathy skills :) > >>> > >>> When I tried to capture packets on a ppp (4) interface (with pipex > >>> activated), I noticed that all the PPP CCP frames were ok, but all the > >>> ingress PPP IP frames were mangled, and they did not contain the PPP > >>> header at all. > >> > >> This time only pppx(4) and pppac(4) have pipex(4) support. > > > > Yes, and as I wrote in the first mail, now I am working on ppp(4) & > > pipex(4) integration to speed up client side of L2TP. > > May be you can share you work? Not for commit, but for feedback. > I send a couple of diffs in separate mails. First change is for ppp(4) to support pipex(4) acceleration in DL path. Second diff adds a new option for pppd to control pipex activation. We will need also to update xl2tpd package to teach it how to configure pppd for pipex usage. This is quite simple change, but it still require some cleanup so I do not publish it yet. > For example, each pipex session should have unique pair of `protocol’ and > `session_id’. These values are passed from userland. While the only > instance of npppd(8) uses pipex(4) this is not the problem. But you > introduce the case while pipex(4) will be used by multiple independent > userland programs. At least, I have interest how you handle this. This should not be a problem here. npppd(8) support server mode only. While my work is to implement acceleration for client side of L2TP connection. Anyway if a session collision become a case, we could additionally demux ingress traffic by the destination UDP port. -- Sergey
[RFC] ppp: add pipex(4) support
Add minimal support for integration with pipex(4). Initialize interface
structure, pass IOCTLs to pipex(4) specific handler, etc.
Acceleration is working only in the DL path only at the moment.
Modification is not ready for applying. Work still in process.
---
sys/net/if_ppp.c| 16
sys/net/if_pppvar.h | 4
sys/net/ppp_tty.c | 17 +
3 files changed, 37 insertions(+)
diff --git sys/net/if_ppp.c sys/net/if_ppp.c
index 3f03d18b6ab..482eef37732 100644
--- sys/net/if_ppp.c
+++ sys/net/if_ppp.c
@@ -129,6 +129,10 @@
#include
#include
+#ifdef PIPEX
+#include
+#endif
+
#include "bpfilter.h"
#ifdef VJC
@@ -198,6 +202,9 @@ pppattach(void)
{
LIST_INIT(&ppp_softc_list);
if_clone_attach(&ppp_cloner);
+#ifdef PIPEX
+ pipex_init();
+#endif
}
int
@@ -228,6 +235,11 @@ ppp_clone_create(struct if_clone *ifc, int unit)
#if NBPFILTER > 0
bpfattach(&ifp->if_bpf, ifp, DLT_PPP, PPP_HDRLEN);
#endif
+
+#ifdef PIPEX
+ pipex_iface_init(&sc->sc_pipex_iface, &sc->sc_if);
+#endif
+
NET_LOCK();
LIST_INSERT_HEAD(&ppp_softc_list, sc, sc_list);
NET_UNLOCK();
@@ -247,6 +259,10 @@ ppp_clone_destroy(struct ifnet *ifp)
LIST_REMOVE(sc, sc_list);
NET_UNLOCK();
+#ifdef PIPEX
+ pipex_iface_fini(&sc->sc_pipex_iface);
+#endif
+
if_detach(ifp);
free(sc, M_DEVBUF, 0);
diff --git sys/net/if_pppvar.h sys/net/if_pppvar.h
index 1c4dd9977b5..57f3b76c4f6 100644
--- sys/net/if_pppvar.h
+++ sys/net/if_pppvar.h
@@ -139,6 +139,10 @@ struct ppp_softc {
u_char sc_rawin[16]; /* chars as received */
int sc_rawin_count; /* # in sc_rawin */
LIST_ENTRY(ppp_softc) sc_list; /* all ppp interfaces */
+
+#ifdef PIPEX
+ struct pipex_iface_context sc_pipex_iface; /* pipex context */
+#endif
};
#ifdef _KERNEL
diff --git sys/net/ppp_tty.c sys/net/ppp_tty.c
index 124b9df59ac..93bcd62ae4b 100644
--- sys/net/ppp_tty.c
+++ sys/net/ppp_tty.c
@@ -122,6 +122,10 @@
#include
#endif
+#ifdef PIPEX
+#include
+#endif
+
#include
#include
#include
@@ -425,6 +429,19 @@ ppptioctl(struct tty *tp, u_long cmd, caddr_t data, int
flag, struct proc *p)
bcopy(sc->sc_asyncmap, data, sizeof(sc->sc_asyncmap));
break;
+#ifdef PIPEX
+case PIPEXSMODE:
+case PIPEXGMODE:
+case PIPEXASESSION:
+case PIPEXDSESSION:
+case PIPEXCSESSION:
+case PIPEXGSTAT:
+ if ((error = suser(p)) != 0)
+ break;
+ error = pipex_ioctl(&sc->sc_pipex_iface, cmd, data);
+ break;
+#endif
+
default:
NET_LOCK();
error = pppioctl(sc, cmd, data, flag, p);
--
2.26.0
[RFC] pppd: add pipex(4) L2TP control support
Add dedicated option to activate kernel L2TP acceleration via
the pipex(4). The options should be passed by a L2TP tunnel
management daemon (e.g. xl2tpd).
This diff is complete, but kernel support in ppp(4) is not ready.
---
usr.sbin/pppd/ipcp.c| 5 +++
usr.sbin/pppd/options.c | 82 +
usr.sbin/pppd/pppd.8| 18 +
usr.sbin/pppd/pppd.h| 7
usr.sbin/pppd/sys-bsd.c | 56
5 files changed, 168 insertions(+)
diff --git usr.sbin/pppd/ipcp.c usr.sbin/pppd/ipcp.c
index 1296d897b14..e7a6dbd6ee7 100644
--- usr.sbin/pppd/ipcp.c
+++ usr.sbin/pppd/ipcp.c
@@ -1251,6 +1251,10 @@ ipcp_up(f)
return;
}
+ /* enable pipex(4), keep working if failed */
+ if (pipex_conf.pr_protocol && !apipex(go->ouraddr, ho->hisaddr, mask))
+ IPCPDEBUG((LOG_WARNING, "apipex failed"));
+
#if (defined(SVR4) && (defined(SNI) || defined(__USLC__)))
if (!sifaddr(f->unit, go->ouraddr, ho->hisaddr, mask)) {
IPCPDEBUG((LOG_WARNING, "sifaddr failed"));
@@ -1304,6 +1308,7 @@ ipcp_down(f)
if (demand) {
sifnpmode(f->unit, PPP_IP, NPMODE_QUEUE);
} else {
+ dpipex();
sifdown(f->unit);
ipcp_clear_addrs(f->unit);
}
diff --git usr.sbin/pppd/options.c usr.sbin/pppd/options.c
index 828f7cdce65..fe0e2e6b54b 100644
--- usr.sbin/pppd/options.c
+++ usr.sbin/pppd/options.c
@@ -144,6 +144,9 @@ struct bpf_program pass_filter;/* Filter program for
packets to pass */
struct bpf_program active_filter; /* Filter program for link-active pkts */
pcap_t pc;/* Fake struct pcap so we can compile expr */
#endif
+struct pipex_session_req pipex_conf = {/* pipex(4) session
configuration */
+ .pr_protocol = 0,
+};
/*
* Prototypes
@@ -253,6 +256,8 @@ static int setactivefilter(char **);
static int setmslanman(char **);
#endif
+static int setpipexl2tp(char **);
+
static int number_option(char *, u_int32_t *, int);
static int int_option(char *, int *);
static int readable(int fd);
@@ -391,6 +396,8 @@ static struct cmd {
{"ms-lanman", 0, setmslanman}, /* Use LanMan psswd when using MS-CHAP
*/
#endif
+{"pipex-l2tp", 6, setpipexl2tp}, /* set pipex(4) L2TP parameters */
+
{NULL, 0, NULL}
};
@@ -2283,3 +2290,78 @@ setmslanman(argv)
return (1);
}
#endif
+
+static int
+inet_atoss(cp, ss)
+char *cp;
+struct sockaddr_storage *ss;
+{
+struct sockaddr_in *sin = (struct sockaddr_in *)ss;
+char *c, *p;
+unsigned port;
+
+if ((c = strchr(cp, ':')) == NULL)
+ return 0;
+*c = '\0';
+if (!inet_aton(cp, &sin->sin_addr))
+ return 0;
+if (!(port = strtoul(c + 1, &p, 10)) || *p != '\0')
+ return 0;
+sin->sin_port = htons(port);
+sin->sin_family = AF_INET;
+ss->ss_len = sizeof(*sin);
+
+return 1;
+}
+
+static int
+strtou16(str, valp)
+char *str;
+uint16_t *valp;
+{
+char *ptr;
+
+*valp = strtoul(str, &ptr, 0);
+
+return *ptr == '\0' ? 1 : 0;
+}
+
+static int
+setpipexl2tp(argv)
+char **argv;
+{
+BZERO((char *)&pipex_conf, sizeof(pipex_conf));
+
+if (!inet_atoss(argv[0], &pipex_conf.pr_local_address)) {
+ option_error("pipex-l2tp: invalid local address of tunnel: %s",
argv[0]);
+ return 0;
+}
+if (!inet_atoss(argv[1], &pipex_conf.pr_peer_address)) {
+ option_error("pipex-l2tp: invalid peer address of tunnel: %s", argv[1]);
+ return 0;
+}
+if (!strtou16(argv[2], &pipex_conf.pr_proto.l2tp.tunnel_id)) {
+ option_error("pipex-l2tp: invalid local tunnel id: %s", argv[2]);
+ return 0;
+}
+if (!strtou16(argv[3], &pipex_conf.pr_proto.l2tp.peer_tunnel_id)) {
+ option_error("pipex-l2tp: invalid peer tunnel id: %s", argv[3]);
+ return 0;
+}
+if (!strtou16(argv[4], &pipex_conf.pr_session_id)) {
+ option_error("pipex-l2tp: invalid local call id: %s", argv[4]);
+ return 0;
+}
+if (!strtou16(argv[5], &pipex_conf.pr_peer_session_id)) {
+ option_error("pipex-l2tp: invalid peer call id: %s", argv[5]);
+ return 0;
+}
+
+/* Indicate address field presense */
+pipex_conf.pr_ppp_flags = PIPEX_PPP_HAS_ACF;
+
+/* Finally set the protocol type to implicitly indicate config validity */
+pipex_conf.pr_protocol = PIPEX_PROTO_L2TP;
+
+return 1;
+}
diff --git usr.sbin/pppd/pppd.8 usr.sbin/pppd/pppd.8
index 5fba6f1714d..6a7f6e01c09 100644
--- usr.sbin/pppd/pppd.8
+++ usr.sbin/pppd/pppd.8
@@ -829,6 +829,24 @@ option is used.
.It Cm xonxoff
Use software flow control (i.e., XON/XOFF) to control the flow of data on
the serial port.
+.It Cm pipex-l2tp Ar ltunaddr ptunaddr ltunid ptunid lcallid pcallid
+OpenBSD specific. Activate and configure kernel L2TP acceleration. Set
+pipex(4) local tunnel address to
+.Ar ltunaddr ,
+peer tunnel address to
+.Ar ptunaddr ,
+local L2TP tunnel id to
+.Ar ltunid ,
+peer L2TP tunnel id
Ncurses: issue with "rep" capability and --enable-bsdpad (BSD_TPUTS)
Hi,
I've noticed an issue when testing the "rep" capability to st (terminal
emulator) on OpenBSD.
After some digging I found the issue is when BSD_TPUTS is defined
(ncurses_cfg.h). This enables code for BSD prefix padding and a workaround for
nethack and ancient BSD terminals as the C comment says in the file
tinfo/lib_tputs.c.
I noticed while using the lynx browser, it will incorrectly print repeated
digit characters. For example:
echo "Z000" | lynx -stdin
will print: ""
To reproduce it:
* Use a terminal with the "rep" capability supported and exposed:
rep=%p1%c\E[%p2%{1}%-%db
infocmp | grep rep=
The sequence is documented in terminfo(5).
* Compile ncurses with BSD_TPUTS defined (or ./configure --enable-bsdpad on
Linux).
BSD_TPUTS is set by default on OpenBSD in lib/libcurses/ncurses_cfg.h
I could reproduce the issue on Void Linux also on the latest ncurses snapshot,
but a fix has been sent upstream and added in 20200523:
https://github.com/ThomasDickey/ncurses-snapshots/blob/master/NEWS
"+ add a check in EmitRange to guard against repeat_char emitting digits which
could be interpreted as BSD-style padding when --enable-bsdpad is configured
(report/patch by Hiltjo Posthuma)."
A small program to reproduce it:
#include
#include
#include
int
main(void)
{
WINDOW *win;
win = initscr();
printw("Z000");
refresh();
sleep(5);
return 0;
}
This program will incorrectly print "" also.
The backported below patch from ncurses snapshot 20200523 to -current fixes it:
diff --git a/lib/libcurses/tty/tty_update.c b/lib/libcurses/tty/tty_update.c
index bd2f088adf6..3d10b76cce2 100644
--- a/lib/libcurses/tty/tty_update.c
+++ b/lib/libcurses/tty/tty_update.c
@@ -540,7 +540,11 @@ EmitRange(const NCURSES_CH_T * ntext, int num)
} else {
return 1; /* cursor stays in the middle */
}
- } else if (repeat_char && runcount > SP->_rep_cost) {
+ } else if (repeat_char &&
+#if BSD_TPUTS
+ !isdigit(UChar(CharOf(ntext0))) &&
+#endif
+ runcount > SP->_rep_cost) {
bool wrap_possible = (SP->_curscol + runcount >=
screen_columns);
int rep_count = runcount;
Is there a specific reason why OpenBSD still uses an old ncurses-based version?
--
Kind regards,
Hiltjo
Re: [patch] fuse_main.3 - fix example to compile without warnings and apply style changes
On Sat, May 23, 2020 at 01:12:41PM -0500, Edgar Pettijohn wrote:
>
fixed, thanks.
jmc
> Index: fuse_main.3
> ===
> RCS file: /cvs/src/lib/libfuse/fuse_main.3,v
> retrieving revision 1.6
> diff -u -p -u -r1.6 fuse_main.3
> --- fuse_main.3 28 Nov 2018 21:19:11 - 1.6
> +++ fuse_main.3 23 May 2020 18:11:16 -
> @@ -56,39 +56,46 @@ Here is a simple example of a FUSE imple
>
> static int
> fs_readdir(const char *path, void *data, fuse_fill_dir_t filler,
> -off_t off, struct fuse_file_info *ffi)
> + off_t off, struct fuse_file_info *ffi)
> {
> if (strcmp(path, "/") != 0)
> - return (-ENOENT);
> + return -ENOENT;
>
> filler(data, ".", NULL, 0);
> filler(data, "..", NULL, 0);
> filler(data, "file", NULL, 0);
> - return (0);
> + return 0;
> }
>
> static int
> fs_read(const char *path, char *buf, size_t size, off_t off,
> -struct fuse_file_info *ffi)
> +struct fuse_file_info *ffi)
> {
> - if (off >= 5)
> - return (0);
> + size_t len;
> + const char *file_contents = "fuse filesystem example\\n";
>
> - size = 5 - off;
> - memcpy(buf, "data." + off, size);
> - return (size);
> + len = strlen(file_contents);
> +
> + if (off < len) {
> + if (off + size > len)
> + size = len - off;
> + memcpy(buf, file_contents + off, size);
> + } else
> + size = 0;
> +
> + return size;
> }
>
> static int
> fs_open(const char *path, struct fuse_file_info *ffi)
> {
> if (strncmp(path, "/file", 10) != 0)
> - return (-ENOENT);
> + return -ENOENT;
>
> if ((ffi->flags & 3) != O_RDONLY)
> - return (-EACCES);
> + return -EACCES;
>
> - return (0);
> + return 0;
> }
>
> static int
> @@ -104,10 +111,10 @@ fs_getattr(const char *path, struct stat
> st->st_nlink = 1;
> st->st_size = 5;
> } else {
> - return (-ENOENT);
> + return -ENOENT;
> }
>
> - return (0);
> + return 0;
> }
>
> struct fuse_operations fsops = {
> @@ -118,9 +125,9 @@ struct fuse_operations fsops = {
> };
>
> int
> -main(int ac, char **av)
> +main(int argc, char **argv)
> {
> - return (fuse_main(ac, av, &fsops, NULL));
> + return (fuse_main(argc, argv, &fsops, NULL));
> }
> .Ed
> .Sh SEE ALSO
Re: random(4): use arc4random_ctx_buf() for large device reads
On Mon, May 25, 2020 at 05:27:37PM +0200, Christian Weisgerber wrote:
> Sebastien Marie:
>
> > > For large reads from /dev/random, use the arc4random_ctx_*() functions
> > > instead of hand-rolling the same code to set up a temporary ChaCha
> > > instance.
> >
> > Eventually, I would get ride of myctx, initialize lctx to NULL, and use
> > (lctx == NULL) to replace (myctx == 0).
>
> Right. Here we go:
Thanks. ok semarie@
> Index: rnd.c
> ===
> RCS file: /cvs/src/sys/dev/rnd.c,v
> retrieving revision 1.213
> diff -u -p -r1.213 rnd.c
> --- rnd.c 18 May 2020 15:00:16 - 1.213
> +++ rnd.c 25 May 2020 14:58:43 -
> @@ -589,6 +589,9 @@ arc4random_ctx_free(struct arc4random_ct
> void
> arc4random_ctx_buf(struct arc4random_ctx *ctx, void *buf, size_t n)
> {
> +#ifndef KEYSTREAM_ONLY
> + memset(buf, 0, n);
> +#endif
> chacha_encrypt_bytes((chacha_ctx *)ctx, buf, buf, n);
> }
>
> @@ -701,40 +704,31 @@ randomclose(dev_t dev, int flag, int mod
> int
> randomread(dev_t dev, struct uio *uio, int ioflag)
> {
> - u_char lbuf[KEYSZ+IVSZ];
> - chacha_ctx lctx;
> + struct arc4random_ctx *lctx = NULL;
> size_t total = uio->uio_resid;
> u_char *buf;
> - int myctx = 0, ret = 0;
> + int ret = 0;
>
> if (uio->uio_resid == 0)
> return 0;
>
> buf = malloc(POOLBYTES, M_TEMP, M_WAITOK);
> - if (total > RND_MAIN_MAX_BYTES) {
> - arc4random_buf(lbuf, sizeof(lbuf));
> - chacha_keysetup(&lctx, lbuf, KEYSZ * 8);
> - chacha_ivsetup(&lctx, lbuf + KEYSZ, NULL);
> - explicit_bzero(lbuf, sizeof(lbuf));
> - myctx = 1;
> - }
> + if (total > RND_MAIN_MAX_BYTES)
> + lctx = arc4random_ctx_new();
>
> while (ret == 0 && uio->uio_resid > 0) {
> size_t n = ulmin(POOLBYTES, uio->uio_resid);
>
> - if (myctx) {
> -#ifndef KEYSTREAM_ONLY
> - memset(buf, 0, n);
> -#endif
> - chacha_encrypt_bytes(&lctx, buf, buf, n);
> - } else
> + if (lctx != NULL)
> + arc4random_ctx_buf(lctx, buf, n);
> + else
> arc4random_buf(buf, n);
> ret = uiomove(buf, n, uio);
> if (ret == 0 && uio->uio_resid > 0)
> yield();
> }
> - if (myctx)
> - explicit_bzero(&lctx, sizeof(lctx));
> + if (lctx != NULL)
> + arc4random_ctx_free(lctx);
> explicit_bzero(buf, POOLBYTES);
> free(buf, M_TEMP, POOLBYTES);
> return ret;
> --
> Christian "naddy" Weisgerber na...@mips.inka.de
--
Sebastien Marie
Re: sysupgrade change to allow installing from url
On Mon, May 25, 2020 at 03:25:40PM +0200, Solene Rapenne wrote:
> Hi,
>
> I don't know if this will be accepted but I propose to add a -u [url]
> parameter to use older snapshots from an archive server for example.
>
> I wanted to add an optional parameter to -s at first but in case of
> sysupgrade -s [installurl] the install url would have been mistaken by
> this.
>
> If you specify a -u url and installurl at the same time, -u url
> superseed installurl.
>
> This would allow easier bisecting using older snapshots.
>
> Index: sysupgrade.8
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
> retrieving revision 1.10
> diff -u -p -r1.10 sysupgrade.8
> --- sysupgrade.8 3 Oct 2019 12:43:58 - 1.10
> +++ sysupgrade.8 25 May 2020 13:23:06 -
> @@ -24,6 +24,7 @@
> .Nm
> .Op Fl fkn
> .Op Fl r | s
> +.Op Fl u Pa url
> .Op Ar installurl
> .Sh DESCRIPTION
> .Nm
> @@ -66,6 +67,13 @@ This is the default if the system is cur
> .It Fl s
> Upgrade to a snapshot.
> This is the default if the system is currently running a snapshot.
> +.It Fl u Pa url
> +Fetch and verify the files downloaded from
> +.Pa url .
> +This is superseeding
> +.Op Pa installurl ,
> +its usage is intended for installing older snapshots available under url not
> following
> +the usual installurl format.
hi. i would rewrite this a little:
It is intended for installing older snapshots available which do not
follow
the usual installurl format;
it takes precedence over
.Pa installurl .
sth like that
i don;t know whether you want to try and make it like -u url | installurl
jmc
> .El
> .Sh FILES
> .Bl -tag -width "/auto_upgrade.conf" -compact
> Index: sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.37
> diff -u -p -r1.37 sysupgrade.sh
> --- sysupgrade.sh 26 Jan 2020 22:08:36 - 1.37
> +++ sysupgrade.sh 25 May 2020 13:23:06 -
> @@ -34,7 +34,7 @@ ug_err()
>
> usage()
> {
> - ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
> + ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-u url] [installurl]"
> }
>
> unpriv()
> @@ -79,13 +79,14 @@ FORCE=false
> KEEP=false
> REBOOT=true
>
> -while getopts fknrs arg; do
> +while getopts fknrsu: arg; do
> case ${arg} in
> f) FORCE=true;;
> k) KEEP=true;;
> n) REBOOT=false;;
> r) RELEASE=true;;
> s) SNAP=true;;
> + u) SNAPURL=${OPTARG};;
> *) usage;;
> esac
> done
> @@ -121,7 +122,11 @@ else
> fi
>
> if $SNAP; then
> - URL=${MIRROR}/snapshots/${ARCH}/
> + if [[ -n "$SNAPURL" ]]; then
> + URL=$SNAPURL
> + else
> + URL=${MIRROR}/snapshots/${ARCH}/
> + fi
> else
> URL=${MIRROR}/${NEXT_VERSION}/${ARCH}/
> fi
>
Re: Provide ldomctl(8) better error messages when parsing ldom.conf
> Date: Sun, 24 May 2020 18:59:09 -0400
> From: Kurt Mosiejczuk
>
> Currently ldomctl init-system will error out in many situations. A number of
> them involve requesting more resources than there are available. Right now
> onw just gets error messages like:
>
> resource_id larger than max_mblocks
> resource_id larger than max_guests
> not enough VCPU resources available
> not enough memory available
>
> With this diff, we put out numbers for most of these.
> ldomctl: requested vcpus (256) > vcpu resources (64)
> ldomctl: requested memory (213674622976) > memory resources (137428467712)
>
> The other change in here is moving max_cpus to uint64_t. max_cpus was an
> int but is only compared to uint_64t. So I moved max_cpus to uint64_t.
> I suppose I could have gone the other way. I don't know if anyone has
> strong feelings on it.
>
> Thoughts? Comments? ok?
Including numbers is good! I feel, quite strongly, that lowercase
vcpu isn't right. So please use VCPU (or plural VCPUs) instead. I
also think that your proposal is a bit inconsistent regarding > and
"larger than". So:
ldomctl: requested VCPUs (256) larger than available resources (64)
ldomctl: requested memory (213674622976) larger than availableresources
(137428467712)
Now that last message is a bit too long and I think those numbers look
a bit silly, so better would be:
ldomctl: requested memory (203776M) larger than available resources (131062M)
I think MB is the right unit. you could avaid a silly x > x by
rounding the requested memory up to the nearest MB.
Cheers,
Mark
> Index: config.c
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/config.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 config.c
> --- config.c 24 May 2020 22:08:54 - 1.40
> +++ config.c 24 May 2020 22:10:56 -
> @@ -122,7 +122,7 @@ void guest_fixup_phys_io(struct guest *)
>
> TAILQ_HEAD(, frag) free_frags = TAILQ_HEAD_INITIALIZER(free_frags);
> TAILQ_HEAD(, cpu) free_cpus = TAILQ_HEAD_INITIALIZER(free_cpus);
> -int total_cpus;
> +uint64_t total_cpus;
> TAILQ_HEAD(, mblock) free_memory = TAILQ_HEAD_INITIALIZER(free_memory);
> uint64_t total_memory;
>
> @@ -601,7 +601,8 @@ hvmd_init_mblock(struct md *md, struct m
> errx(1, "missing resource_id property in mblock node");
>
> if (resource_id >= max_mblocks)
> - errx(1, "resource_id larger than max_mblocks");
> + errx(1, "resource_id %lld larger than max_mblocks %lld", \
> + resource_id, max_mblocks);
>
> mblock = xzalloc(sizeof(*mblock));
> md_get_prop_val(md, node, "membase", &mblock->membase);
> @@ -632,7 +633,8 @@ hvmd_init_console(struct md *md, struct
> errx(1, "missing resource_id property in console node");
>
> if (resource_id >= max_guests)
> - errx(1, "resource_id larger than max_guests");
> + errx(1, "resource_id %lld larger than max_guests %lld", \
> + resource_id, max_guests);
>
> console = xzalloc(sizeof(*console));
> md_get_prop_val(md, node, "ino", &console->ino);
> @@ -655,7 +657,8 @@ hvmd_init_cpu(struct md *md, struct md_n
> errx(1, "missing resource_id property in cpu node");
>
> if (resource_id >= max_cpus)
> - errx(1, "resource_id larger than max-cpus");
> + errx(1, "resource_id %lld larger than max-cpus %lld", \
> + resource_id, max_cpus);
>
> if (!md_get_prop_val(md, node, "pid", &pid))
> errx(1, "missing pid property in cpu node");
> @@ -698,7 +701,8 @@ hvmd_init_device(struct md *md, struct m
> errx(1, "missing resource_id property in ldc_endpoint node");
>
> if (resource_id >= max_devices)
> - errx(1, "resource_id larger than max_devices");
> + errx(1, "resource_id %lld larger than max_devices %lld", \
> + resource_id, max_devices);
>
> device = xzalloc(sizeof(*device));
> md_get_prop_val(md, node, "gid", &device->gid);
> @@ -754,7 +758,8 @@ hvmd_init_endpoint(struct md *md, struct
> errx(1, "missing resource_id property in ldc_endpoint node");
>
> if (resource_id >= max_guest_ldcs)
> - errx(1, "resource_id larger than max_guest_ldcs");
> + errx(1, "resource_id %lld larger than max_guest_ldcs %lld", \
> + resource_id, max_guest_ldcs);
>
> if (ldc_endpoints[resource_id]) {
> /*
> @@ -800,7 +805,8 @@ hvmd_init_guest(struct md *md, struct md
> errx(1, "missing resource_id property in guest node");
>
> if (resource_id >= max_guests)
> - errx(1, "resource_id larger than max_guests");
> + errx(1, "resource_id %lld larger than max_guests %lld", \
> + resource_id, max_guests);
>
> guest = xzalloc(sizeof(*guest));
> TAILQ_INIT(&guest->cpu_list);
> @@
Re: random(4): use arc4random_ctx_buf() for large device reads
Sebastien Marie:
> > For large reads from /dev/random, use the arc4random_ctx_*() functions
> > instead of hand-rolling the same code to set up a temporary ChaCha
> > instance.
>
> Eventually, I would get ride of myctx, initialize lctx to NULL, and use
> (lctx == NULL) to replace (myctx == 0).
Right. Here we go:
Index: rnd.c
===
RCS file: /cvs/src/sys/dev/rnd.c,v
retrieving revision 1.213
diff -u -p -r1.213 rnd.c
--- rnd.c 18 May 2020 15:00:16 - 1.213
+++ rnd.c 25 May 2020 14:58:43 -
@@ -589,6 +589,9 @@ arc4random_ctx_free(struct arc4random_ct
void
arc4random_ctx_buf(struct arc4random_ctx *ctx, void *buf, size_t n)
{
+#ifndef KEYSTREAM_ONLY
+ memset(buf, 0, n);
+#endif
chacha_encrypt_bytes((chacha_ctx *)ctx, buf, buf, n);
}
@@ -701,40 +704,31 @@ randomclose(dev_t dev, int flag, int mod
int
randomread(dev_t dev, struct uio *uio, int ioflag)
{
- u_char lbuf[KEYSZ+IVSZ];
- chacha_ctx lctx;
+ struct arc4random_ctx *lctx = NULL;
size_t total = uio->uio_resid;
u_char *buf;
- int myctx = 0, ret = 0;
+ int ret = 0;
if (uio->uio_resid == 0)
return 0;
buf = malloc(POOLBYTES, M_TEMP, M_WAITOK);
- if (total > RND_MAIN_MAX_BYTES) {
- arc4random_buf(lbuf, sizeof(lbuf));
- chacha_keysetup(&lctx, lbuf, KEYSZ * 8);
- chacha_ivsetup(&lctx, lbuf + KEYSZ, NULL);
- explicit_bzero(lbuf, sizeof(lbuf));
- myctx = 1;
- }
+ if (total > RND_MAIN_MAX_BYTES)
+ lctx = arc4random_ctx_new();
while (ret == 0 && uio->uio_resid > 0) {
size_t n = ulmin(POOLBYTES, uio->uio_resid);
- if (myctx) {
-#ifndef KEYSTREAM_ONLY
- memset(buf, 0, n);
-#endif
- chacha_encrypt_bytes(&lctx, buf, buf, n);
- } else
+ if (lctx != NULL)
+ arc4random_ctx_buf(lctx, buf, n);
+ else
arc4random_buf(buf, n);
ret = uiomove(buf, n, uio);
if (ret == 0 && uio->uio_resid > 0)
yield();
}
- if (myctx)
- explicit_bzero(&lctx, sizeof(lctx));
+ if (lctx != NULL)
+ arc4random_ctx_free(lctx);
explicit_bzero(buf, POOLBYTES);
free(buf, M_TEMP, POOLBYTES);
return ret;
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: [PATCH} efifb support for wsmoused + smaller fonts
> Date: Tue, 26 May 2020 00:27:31 +1000
> From: Jonathan Gray
>
> On Mon, May 25, 2020 at 08:49:23AM -0500, Lucas Raab wrote:
> > On Mon, May 25, 2020 at 09:13:43PM +1000, Jonathan Gray wrote:
> > > On Sun, May 24, 2020 at 05:01:16PM -0700, jo...@armadilloaerospace.com
> > > wrote:
> > > > The efifb driver behaves almost identically to the inteldrm driver
> > > > for wscons, but did not implement the getchar() accessops, so
> > > > wsmoused would fail at startup.
> > >
> > > This seems reasonable, though your mail client has line wrapped the
> > > patch so it won't apply. In this case it is simple enough to apply by
> > > hand.
> > >
> > > > Separately, I increased the maximum screen dimensions to 160x50 to
> > > > allow the 12x24 font to be used on an 1920 monitor, which looks great!
> > >
> > > Something similiar was done and reverted as it somehow broke inteldrm
> > > taking the fb over from efifb on a 4k display. Perhaps someone with a
> > > 4k display can confirm if this is still a problem.
> > >
> > > https://marc.info/?l=openbsd-bugs&m=155337866226121&w=2
> > >
> > > That was back before we deferred most of inteldrm to when the root fs is
> > > mounted and interrupts are available. And just before the last big drm
> > > update by the look of it.
> >
> > I tried out the patch twice, once with John's value for EFIFB_HEIGHT of
> > 50 and once with the original original value of 160. The behavior of
> > the bug report I submitted doesn't reappear so it looks everything is
> > working as intended. This is with the same monitor as in the bug report
>
> Thanks for testing. I've committed the wsmoused part on it's own, so
> how about we try rev 1.21 again?
ok kettenis@
This adds roughly 175KB of uninitialized memory to the kernel (.bss),
but that is probably acceptable since it doesn't occupy any disk space.
> Index: efifb.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/efifb.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 efifb.c
> --- efifb.c 25 May 2020 14:12:04 - 1.30
> +++ efifb.c 25 May 2020 14:20:25 -
> @@ -115,8 +115,8 @@ const struct cfattach efifb_ca = {
> sizeof(struct efifb_softc), efifb_match, efifb_attach, NULL
> };
>
> -#define EFIFB_WIDTH 100
> -#define EFIFB_HEIGHT31
> +#define EFIFB_WIDTH 160
> +#define EFIFB_HEIGHT160
>
> struct wsscreen_descr efifb_std_descr = { "std" };
>
>
>
Re: [PATCH} efifb support for wsmoused + smaller fonts
On Mon, May 25, 2020 at 08:49:23AM -0500, Lucas Raab wrote:
> On Mon, May 25, 2020 at 09:13:43PM +1000, Jonathan Gray wrote:
> > On Sun, May 24, 2020 at 05:01:16PM -0700, jo...@armadilloaerospace.com
> > wrote:
> > > The efifb driver behaves almost identically to the inteldrm driver
> > > for wscons, but did not implement the getchar() accessops, so
> > > wsmoused would fail at startup.
> >
> > This seems reasonable, though your mail client has line wrapped the
> > patch so it won't apply. In this case it is simple enough to apply by
> > hand.
> >
> > > Separately, I increased the maximum screen dimensions to 160x50 to
> > > allow the 12x24 font to be used on an 1920 monitor, which looks great!
> >
> > Something similiar was done and reverted as it somehow broke inteldrm
> > taking the fb over from efifb on a 4k display. Perhaps someone with a
> > 4k display can confirm if this is still a problem.
> >
> > https://marc.info/?l=openbsd-bugs&m=155337866226121&w=2
> >
> > That was back before we deferred most of inteldrm to when the root fs is
> > mounted and interrupts are available. And just before the last big drm
> > update by the look of it.
>
> I tried out the patch twice, once with John's value for EFIFB_HEIGHT of
> 50 and once with the original original value of 160. The behavior of
> the bug report I submitted doesn't reappear so it looks everything is
> working as intended. This is with the same monitor as in the bug report
Thanks for testing. I've committed the wsmoused part on it's own, so
how about we try rev 1.21 again?
Index: efifb.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/efifb.c,v
retrieving revision 1.30
diff -u -p -r1.30 efifb.c
--- efifb.c 25 May 2020 14:12:04 - 1.30
+++ efifb.c 25 May 2020 14:20:25 -
@@ -115,8 +115,8 @@ const struct cfattach efifb_ca = {
sizeof(struct efifb_softc), efifb_match, efifb_attach, NULL
};
-#defineEFIFB_WIDTH 100
-#defineEFIFB_HEIGHT31
+#defineEFIFB_WIDTH 160
+#defineEFIFB_HEIGHT160
struct wsscreen_descr efifb_std_descr = { "std" };
pipex(4): document struct members locking
This time it's not clean which lock is really required to protect
pipex(4) data structures. In fact KERNL_LOCK() and NET_LOCK() are
used simultaneously. It's time to document it.
Only [k] used do mark mutable members due to NET_LOCK() looks be
unnecesary and can be killed in future.
Index: sys/net/pipex.h
===
RCS file: /cvs/src/sys/net/pipex.h,v
retrieving revision 1.21
diff -u -p -r1.21 pipex.h
--- sys/net/pipex.h 24 Jan 2017 10:08:30 - 1.21
+++ sys/net/pipex.h 25 May 2020 10:12:33 -
@@ -183,11 +183,16 @@ extern intpipex_enable;
struct pipex_session;
-/* pipex context for a interface. */
+/* pipex context for a interface
+ *
+ * Locks used to protect struct members:
+ * I immutable after creation
+ * k kernel lock
+ */
struct pipex_iface_context {
- struct ifnet *ifnet_this; /* outer interface */
- u_int pipexmode; /* pipex mode */
- /* virtual pipex_session entry for multicast routing */
+ struct ifnet *ifnet_this; /* [I] outer interface */
+ u_int pipexmode; /* [k] pipex mode */
+ /* [I] virtual pipex_session entry for multicast routing */
struct pipex_session *multicast_session;
};
Index: sys/net/pipex_local.h
===
RCS file: /cvs/src/sys/net/pipex_local.h,v
retrieving revision 1.33
diff -u -p -r1.33 pipex_local.h
--- sys/net/pipex_local.h 6 Apr 2020 12:31:30 - 1.33
+++ sys/net/pipex_local.h 25 May 2020 10:12:33 -
@@ -53,43 +53,50 @@
#define PIPEX_MPPE_NOLDKEY 64 /* should be power of two */
#define PIPEX_MPPE_OLDKEYMASK (PIPEX_MPPE_NOLDKEY - 1)
+/*
+ * Locks used to protect struct members:
+ * I immutable after creation
+ * k kernel lock
+ */
+
#ifdef PIPEX_MPPE
/* mppe rc4 key */
struct pipex_mppe {
- int16_t stateless:1,/* key change mode */
- resetreq:1,
+ int16_t stateless:1,/* [I] key change mode */
+ resetreq:1, /* [k] */
reserved:14;
- int16_t keylenbits; /* key length */
- int16_t keylen;
- uint16_t coher_cnt; /* cohency counter */
- struct rc4_ctx rc4ctx;
- u_char master_key[PIPEX_MPPE_KEYLEN]; /* master key of MPPE */
- u_char session_key[PIPEX_MPPE_KEYLEN]; /* session key of MPPE */
- u_char (*old_session_keys)[PIPEX_MPPE_KEYLEN]; /* old session keys */
+ int16_t keylenbits; /* [I] key length */
+ int16_t keylen; /* [I] */
+ uint16_t coher_cnt; /* [k] cohency counter */
+ struct rc4_ctx rc4ctx; /* [k] */
+ u_char master_key[PIPEX_MPPE_KEYLEN]; /* [k] master key of MPPE */
+ u_char session_key[PIPEX_MPPE_KEYLEN]; /* [k] session key of MPPE */
+ u_char (*old_session_keys)[PIPEX_MPPE_KEYLEN];
+ /* [k] old session keys */
};
#endif /* PIPEX_MPPE */
#ifdef PIPEX_PPPOE
struct pipex_pppoe_session {
- u_intover_ifidx; /* ether interface */
+ u_intover_ifidx;/* [I] ether interface */
};
#endif /* PIPEX_PPPOE */
#ifdef PIPEX_PPTP
struct pipex_pptp_session {
/* sequence number gap between pipex and userland */
- int32_t snd_gap;/* gap of our sequence */
- int32_t rcv_gap;/* gap of peer's sequence */
- int32_t ul_snd_una; /* userland send acked seq */
-
- uint32_t snd_nxt; /* send next */
- uint32_t rcv_nxt; /* receive next */
- uint32_t snd_una; /* send acked sequence */
- uint32_t rcv_acked; /* recv acked sequence */
-
- int winsz; /* windows size */
- int maxwinsz; /* max windows size */
- int peer_maxwinsz; /* peer's max windows size */
+ int32_t snd_gap;/* [k] gap of our sequence */
+ int32_t rcv_gap;/* [k] gap of peer's sequence */
+ int32_t ul_snd_una; /* [k] userland send acked seq */
+
+ uint32_t snd_nxt; /* [k] send next */
+ uint32_t rcv_nxt; /* [k] receive next */
+ uint32_t snd_una; /* [k] send acked sequence */
+ uint32_t rcv_acked; /* [k] recv acked sequence */
+
+ int winsz; /* [I] windows size *
Re: [PATCH} efifb support for wsmoused + smaller fonts
> Date: Mon, 25 May 2020 21:13:43 +1000 > From: Jonathan Gray > > On Sun, May 24, 2020 at 05:01:16PM -0700, jo...@armadilloaerospace.com wrote: > > The efifb driver behaves almost identically to the inteldrm driver > > for wscons, but did not implement the getchar() accessops, so > > wsmoused would fail at startup. > > This seems reasonable, though your mail client has line wrapped the > patch so it won't apply. In this case it is simple enough to apply by > hand. > > > Separately, I increased the maximum screen dimensions to 160x50 to > > allow the 12x24 font to be used on an 1920 monitor, which looks great! > > Something similiar was done and reverted as it somehow broke inteldrm > taking the fb over from efifb on a 4k display. Perhaps someone with a > 4k display can confirm if this is still a problem. > > https://marc.info/?l=openbsd-bugs&m=155337866226121&w=2 > > That was back before we deferred most of inteldrm to when the root fs is > mounted and interrupts are available. And just before the last big drm > update by the look of it. I think we also handle "stolen" memory a bit better now. Enough reason to believe that this may be fixed now that we can change it again and see what happens? > > revision 1.22 > date: 2019/03/25 14:17:58; author: fcambus; state: Exp; lines: +3 -3; > commitid: hL9P1vyWGBp5FHhE; > Revert back to using previous values for EFIFB_WIDTH and EFIFB_HEIGHT, > as raising them expose an issue which breaks inteldrm on large screen > resolutions. > > Reported by chris@, and by Lucas Raab on bugs@. Thanks! > > revision 1.21 > date: 2019/03/16 13:16:49; author: fcambus; state: Exp; lines: +3 -3; > commitid: wsqjyS7rXDKbqGS3; > Now that efifb(4) supports remapping the framebuffer in write combining > mode, it's fast enough to raise the maximum size of the EFI framebuffer > console. > > Set it to 160 columns and 160 rows, to match inteldrm and radeondrm. > > OK jcs@, kettenis@ > > > > > > Unlike the intel driver, efifb has a static buffer of that size, > > which is a non-trivial amount of memory. Not setting the backing > > store to save the memory results in an ultra-slow console until the > > driver gets fully initialized, because it is doing scrolls by reading > > from write combined memory instead of the redrawing all the > > characters. Yes, that's why this "shadow buffer" was implemented. > > I plan on changing that to a dynamic allocation once I am more > > comfortable with the reinitialization that goes on when autoconf > > gets around to the console display device, so feel free to punt on > > that if you want to wait. > > Drivers which can act as a console have a cnattach function for very > early init and then later probe again and run attach. In the case of > efifb there is also efifb_cnremap() which is called when mainbus > attaches. The drm drivers call efifb_detach() to stop autoconf from > doing the normal probe/attach when taking over the console. Be aware that the first initailization (efifb_cnattach) happens before allocating memory is possible; even pmap_steal_memory() won't work at this point. And we want to be able to print messages as early as possible in the kernel. The second initialization happens bear the start of autoconf straight after printing "mainbus at root". At this point memory allocation is possible the normal way. We don't print a lot of information between these steps and shouldn't need to do any scrolling. So maybe a solution where we initialize without the RI_WRONLY flag and add it later is feasable. We need to keep track what has been printed though such that the shadow buffer can be pre-initialized with watever is already on the screen at that point. That could be done in a simple char buffer though, which takes up much less space. In fact, it may be possible to simply replay the contents of msgbuf to fill the shadow buffer.
Re: [PATCH} efifb support for wsmoused + smaller fonts
On Mon, May 25, 2020 at 09:13:43PM +1000, Jonathan Gray wrote: > On Sun, May 24, 2020 at 05:01:16PM -0700, jo...@armadilloaerospace.com wrote: > > The efifb driver behaves almost identically to the inteldrm driver > > for wscons, but did not implement the getchar() accessops, so > > wsmoused would fail at startup. > > This seems reasonable, though your mail client has line wrapped the > patch so it won't apply. In this case it is simple enough to apply by > hand. > > > Separately, I increased the maximum screen dimensions to 160x50 to > > allow the 12x24 font to be used on an 1920 monitor, which looks great! > > Something similiar was done and reverted as it somehow broke inteldrm > taking the fb over from efifb on a 4k display. Perhaps someone with a > 4k display can confirm if this is still a problem. > > https://marc.info/?l=openbsd-bugs&m=155337866226121&w=2 > > That was back before we deferred most of inteldrm to when the root fs is > mounted and interrupts are available. And just before the last big drm > update by the look of it. I tried out the patch twice, once with John's value for EFIFB_HEIGHT of 50 and once with the original original value of 160. The behavior of the bug report I submitted doesn't reappear so it looks everything is working as intended. This is with the same monitor as in the bug report > > > revision 1.22 > date: 2019/03/25 14:17:58; author: fcambus; state: Exp; lines: +3 -3; > commitid: hL9P1vyWGBp5FHhE; > Revert back to using previous values for EFIFB_WIDTH and EFIFB_HEIGHT, > as raising them expose an issue which breaks inteldrm on large screen > resolutions. > > Reported by chris@, and by Lucas Raab on bugs@. Thanks! > > revision 1.21 > date: 2019/03/16 13:16:49; author: fcambus; state: Exp; lines: +3 -3; > commitid: wsqjyS7rXDKbqGS3; > Now that efifb(4) supports remapping the framebuffer in write combining > mode, it's fast enough to raise the maximum size of the EFI framebuffer > console. > > Set it to 160 columns and 160 rows, to match inteldrm and radeondrm. > > OK jcs@, kettenis@ > > > > > > Unlike the intel driver, efifb has a static buffer of that size, > > which is a non-trivial amount of memory. Not setting the backing > > store to save the memory results in an ultra-slow console until the > > driver gets fully initialized, because it is doing scrolls by reading > > from write combined memory instead of the redrawing all the > > characters. > > > > I plan on changing that to a dynamic allocation once I am more > > comfortable with the reinitialization that goes on when autoconf > > gets around to the console display device, so feel free to punt on > > that if you want to wait. > > Drivers which can act as a console have a cnattach function for very > early init and then later probe again and run attach. In the case of > efifb there is also efifb_cnremap() which is called when mainbus > attaches. The drm drivers call efifb_detach() to stop autoconf from > doing the normal probe/attach when taking over the console. >
sysupgrade change to allow installing from url
Hi,
I don't know if this will be accepted but I propose to add a -u [url]
parameter to use older snapshots from an archive server for example.
I wanted to add an optional parameter to -s at first but in case of
sysupgrade -s [installurl] the install url would have been mistaken by
this.
If you specify a -u url and installurl at the same time, -u url
superseed installurl.
This would allow easier bisecting using older snapshots.
Index: sysupgrade.8
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
retrieving revision 1.10
diff -u -p -r1.10 sysupgrade.8
--- sysupgrade.83 Oct 2019 12:43:58 - 1.10
+++ sysupgrade.825 May 2020 13:23:06 -
@@ -24,6 +24,7 @@
.Nm
.Op Fl fkn
.Op Fl r | s
+.Op Fl u Pa url
.Op Ar installurl
.Sh DESCRIPTION
.Nm
@@ -66,6 +67,13 @@ This is the default if the system is cur
.It Fl s
Upgrade to a snapshot.
This is the default if the system is currently running a snapshot.
+.It Fl u Pa url
+Fetch and verify the files downloaded from
+.Pa url .
+This is superseeding
+.Op Pa installurl ,
+its usage is intended for installing older snapshots available under url not
following
+the usual installurl format.
.El
.Sh FILES
.Bl -tag -width "/auto_upgrade.conf" -compact
Index: sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.37
diff -u -p -r1.37 sysupgrade.sh
--- sysupgrade.sh 26 Jan 2020 22:08:36 - 1.37
+++ sysupgrade.sh 25 May 2020 13:23:06 -
@@ -34,7 +34,7 @@ ug_err()
usage()
{
- ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
+ ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-u url] [installurl]"
}
unpriv()
@@ -79,13 +79,14 @@ FORCE=false
KEEP=false
REBOOT=true
-while getopts fknrs arg; do
+while getopts fknrsu: arg; do
case ${arg} in
f) FORCE=true;;
k) KEEP=true;;
n) REBOOT=false;;
r) RELEASE=true;;
s) SNAP=true;;
+ u) SNAPURL=${OPTARG};;
*) usage;;
esac
done
@@ -121,7 +122,11 @@ else
fi
if $SNAP; then
- URL=${MIRROR}/snapshots/${ARCH}/
+ if [[ -n "$SNAPURL" ]]; then
+ URL=$SNAPURL
+ else
+ URL=${MIRROR}/snapshots/${ARCH}/
+ fi
else
URL=${MIRROR}/${NEXT_VERSION}/${ARCH}/
fi
pppx(4): kill useless rwlock `pppx_ifs_lk'
`pppx_ifs_lk' used to protect `pppx_ifs' tree, But this tree is accessed
under KERNL_LOCK() and there is no concurency. Also we don't sleep while
holding this lock. So it's useless, kill it for simplification.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.84
diff -u -p -r1.84 if_pppx.c
--- sys/net/if_pppx.c 18 Apr 2020 04:03:56 - 1.84
+++ sys/net/if_pppx.c 25 May 2020 08:31:20 -
@@ -165,7 +165,6 @@ pppx_if_cmp(const struct pppx_if *a, con
return memcmp(&a->pxi_key, &b->pxi_key, sizeof(a->pxi_key));
}
-struct rwlock pppx_ifs_lk = RWLOCK_INITIALIZER("pppxifs");
RBT_HEAD(pppx_ifs, pppx_if)pppx_ifs = RBT_INITIALIZER(&pppx_ifs);
RBT_PROTOTYPE(pppx_ifs, pppx_if, pxi_entry, pppx_if_cmp);
@@ -620,8 +619,6 @@ pppx_if_next_unit(void)
struct pppx_if *pxi;
int unit = 0;
- rw_assert_wrlock(&pppx_ifs_lk);
-
/* this is safe without splnet since we're not modifying it */
do {
int found = 0;
@@ -650,11 +647,9 @@ pppx_if_find(struct pppx_dev *pxd, int s
key.pxik_session_id = session_id;
key.pxik_protocol = protocol;
- rw_enter_read(&pppx_ifs_lk);
pxi = RBT_FIND(pppx_ifs, &pppx_ifs, (struct pppx_if *)&key);
if (pxi && pxi->pxi_ready == 0)
pxi = NULL;
- rw_exit_read(&pppx_ifs_lk);
return pxi;
}
@@ -828,12 +823,10 @@ pppx_add_session(struct pppx_dev *pxd, s
#endif
/* try to set the interface up */
- rw_enter_write(&pppx_ifs_lk);
unit = pppx_if_next_unit();
if (unit < 0) {
pool_put(pppx_if_pl, pxi);
error = ENOMEM;
- rw_exit_write(&pppx_ifs_lk);
goto out;
}
@@ -846,14 +839,12 @@ pppx_add_session(struct pppx_dev *pxd, s
if (RBT_FIND(pppx_ifs, &pppx_ifs, pxi) != NULL) {
pool_put(pppx_if_pl, pxi);
error = EADDRINUSE;
- rw_exit_write(&pppx_ifs_lk);
goto out;
}
if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL)
panic("%s: pppx_ifs modified while lock was held", __func__);
LIST_INSERT_HEAD(&pxd->pxd_pxis, pxi, pxi_list);
- rw_exit_write(&pppx_ifs_lk);
snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit);
ifp->if_mtu = req->pr_peer_mru; /* XXX */
@@ -935,9 +926,7 @@ pppx_add_session(struct pppx_dev *pxd, s
} else {
if_addrhooks_run(ifp);
}
- rw_enter_write(&pppx_ifs_lk);
pxi->pxi_ready = 1;
- rw_exit_write(&pppx_ifs_lk);
out:
return (error);
@@ -1038,11 +1027,9 @@ pppx_if_destroy(struct pppx_dev *pxd, st
if_detach(ifp);
NET_LOCK();
- rw_enter_write(&pppx_ifs_lk);
if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
panic("%s: pppx_ifs modified while lock was held", __func__);
LIST_REMOVE(pxi, pxi_list);
- rw_exit_write(&pppx_ifs_lk);
pool_put(pppx_if_pl, pxi);
}
Re: kqueue_scan() refactoring
On 10/04/20(Fri) 14:43, Martin Pieuchot wrote:
> Diff below reduces kqueue_scan() to the collect of events on a given
> kqueue and let its caller, sys_kevent(), responsible for the copyout(9).
>
> Apart from the code simplification, this refactoring clearly separates
> kqueue_scan() from the syscall logic. That should allow us to re-use
> the function in a different context and to address its need for locking
> independently.
>
> Since the number of events that are ready to be collected can be bigger
> than the size of the array, the pair kqueue_scan()/copyout() may be
> called multiple times. In that case, successive calls should no longer
> block, this is performed by using a zero, but not NULL, timeout which
> correspond to a non-blocking scan.
>
> This is the next piece of the ongoing work to move select/poll/kqueue.
> I'd like to be sure it doesn't introduce any regression.
Updated diff based on recent changes and incorporating feedbacks from
visa@.
Comments? Oks?
Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.132
diff -u -p -r1.132 kern_event.c
--- kern/kern_event.c 17 May 2020 10:53:14 - 1.132
+++ kern/kern_event.c 25 May 2020 11:54:08 -
@@ -62,9 +62,6 @@ void KQREF(struct kqueue *);
void KQRELE(struct kqueue *);
intkqueue_sleep(struct kqueue *, struct timespec *);
-intkqueue_scan(struct kqueue_scan_state *scan, int maxevents,
- struct kevent *ulistp, struct timespec *timeout,
- struct proc *p, int *retval);
intkqueue_read(struct file *, struct uio *, int);
intkqueue_write(struct file *, struct uio *, int);
@@ -545,6 +542,7 @@ sys_kevent(struct proc *p, void *v, regi
struct timespec ts;
struct timespec *tsp = NULL;
int i, n, nerrors, error;
+ int ready, total;
struct kevent kev[KQ_NEVENTS];
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
@@ -573,9 +571,9 @@ sys_kevent(struct proc *p, void *v, regi
kq = fp->f_data;
nerrors = 0;
- while (SCARG(uap, nchanges) > 0) {
- n = SCARG(uap, nchanges) > KQ_NEVENTS ?
- KQ_NEVENTS : SCARG(uap, nchanges);
+ while ((n = SCARG(uap, nchanges)) > 0) {
+ if (n > nitems(kev))
+ n = nitems(kev);
error = copyin(SCARG(uap, changelist), kev,
n * sizeof(struct kevent));
if (error)
@@ -611,14 +609,41 @@ sys_kevent(struct proc *p, void *v, regi
goto done;
}
+
KQREF(kq);
FRELE(fp, p);
+ /*
+* Collect as many events as we can. The timeout on successive
+* loops is disabled (kqueue_scan() becomes non-blocking).
+*/
+ total = 0;
+ error = 0;
kqueue_scan_setup(&scan, kq);
- error = kqueue_scan(&scan, SCARG(uap, nevents), SCARG(uap, eventlist),
- tsp, p, &n);
+ while ((n = SCARG(uap, nevents) - total) > 0) {
+ if (n > nitems(kev))
+ n = nitems(kev);
+ ready = kqueue_scan(&scan, n, kev, tsp, p, &error);
+ if (ready == 0)
+ break;
+ error = copyout(kev, SCARG(uap, eventlist) + total,
+ sizeof(struct kevent) * ready);
+#ifdef KTRACE
+ if (KTRPOINT(p, KTR_STRUCT))
+ ktrevent(p, kev, ready);
+#endif
+ total += ready;
+ /*
+* Stop if there was an error or if we had enough
+* place to collect all events that were ready.
+*/
+ if (error || ready < n)
+ break;
+ tsp = &ts; /* successive loops non-blocking */
+ timespecclear(tsp);
+ }
kqueue_scan_finish(&scan);
KQRELE(kq);
- *retval = n;
+ *retval = total;
return (error);
done:
@@ -872,15 +897,18 @@ kqueue_sleep(struct kqueue *kq, struct t
return (error);
}
+/*
+ * Scan the kqueue, blocking if necessary until the target time is reached.
+ * If tsp is NULL we block indefinitely. If tsp->ts_secs/nsecs are both
+ * 0 we do not block at all.
+ */
int
kqueue_scan(struct kqueue_scan_state *scan, int maxevents,
-struct kevent *ulistp, struct timespec *tsp, struct proc *p, int *retval)
+struct kevent *kevp, struct timespec *tsp, struct proc *p, int *errorp)
{
- struct kevent *kevp;
struct knote *kn;
struct kqueue *kq = scan->kqs_kq;
int s, count, nkev = 0, error = 0;
- struct kevent kev[KQ_NEVENTS];
count = maxevents;
if (count == 0)
@@ -892,7 +920,6 @@ retry:
goto done;
}
- kevp = &kev[0];
s = splhigh();
if (kq->kq_count == 0) {
if ((tsp != NULL && !timespec
Introduce kqueue_terminate() & kqueue_free()
Small refactoring needed to manage per-thread kqueues. Such kqueue are
not associated to a file descriptor, that's why the functions below take
a "struct kqueue *" as argument.
ok?
Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.132
diff -u -p -r1.132 kern_event.c
--- kern/kern_event.c 17 May 2020 10:53:14 - 1.132
+++ kern/kern_event.c 25 May 2020 11:24:03 -
@@ -57,6 +57,8 @@
#include
#include
+void kqueue_terminate(struct proc *p, struct kqueue *);
+void kqueue_free(struct kqueue *);
void kqueue_init(void);
void KQREF(struct kqueue *);
void KQRELE(struct kqueue *);
@@ -181,8 +183,13 @@ KQRELE(struct kqueue *kq)
fdpunlock(fdp);
}
- free(kq->kq_knlist, M_KEVENT, kq->kq_knlistsize *
- sizeof(struct knlist));
+ kqueue_free(kq);
+}
+
+void
+kqueue_free(struct kqueue *kq)
+{
+ free(kq->kq_knlist, M_KEVENT, kq->kq_knlistsize * sizeof(struct klist));
hashfree(kq->kq_knhash, KN_HASHSIZE, M_KEVENT);
pool_put(&kqueue_pool, kq);
}
@@ -492,13 +499,10 @@ static const struct filterops dead_filto
.f_event= filt_dead,
};
-int
-sys_kqueue(struct proc *p, void *v, register_t *retval)
+struct kqueue *
+kqueue_alloc(struct filedesc *fdp)
{
- struct filedesc *fdp = p->p_fd;
struct kqueue *kq;
- struct file *fp;
- int fd, error;
kq = pool_get(&kqueue_pool, PR_WAITOK | PR_ZERO);
kq->kq_refs = 1;
@@ -506,6 +510,19 @@ sys_kqueue(struct proc *p, void *v, regi
TAILQ_INIT(&kq->kq_head);
task_set(&kq->kq_task, kqueue_task, kq);
+ return (kq);
+}
+
+int
+sys_kqueue(struct proc *p, void *v, register_t *retval)
+{
+ struct filedesc *fdp = p->p_fd;
+ struct kqueue *kq;
+ struct file *fp;
+ int fd, error;
+
+ kq = kqueue_alloc(fdp);
+
fdplock(fdp);
error = falloc(p, &fp, &fd);
if (error)
@@ -1115,13 +1132,12 @@ kqueue_stat(struct file *fp, struct stat
return (0);
}
-int
-kqueue_close(struct file *fp, struct proc *p)
+void
+kqueue_terminate(struct proc *p, struct kqueue *kq)
{
- struct kqueue *kq = fp->f_data;
int i;
- KERNEL_LOCK();
+ KERNEL_ASSERT_LOCKED();
for (i = 0; i < kq->kq_knlistsize; i++)
knote_remove(p, &kq->kq_knlist[i]);
@@ -1129,13 +1145,22 @@ kqueue_close(struct file *fp, struct pro
for (i = 0; i < kq->kq_knhashmask + 1; i++)
knote_remove(p, &kq->kq_knhash[i]);
}
- fp->f_data = NULL;
-
kq->kq_state |= KQ_DYING;
kqueue_wakeup(kq);
KASSERT(klist_empty(&kq->kq_sel.si_note));
task_del(systq, &kq->kq_task);
+
+}
+
+int
+kqueue_close(struct file *fp, struct proc *p)
+{
+ struct kqueue *kq = fp->f_data;
+
+ KERNEL_LOCK();
+ kqueue_terminate(p, kq);
+ fp->f_data = NULL;
KQRELE(kq);
Re: [PATCH} efifb support for wsmoused + smaller fonts
On Sun, May 24, 2020 at 05:01:16PM -0700, jo...@armadilloaerospace.com wrote: > The efifb driver behaves almost identically to the inteldrm driver > for wscons, but did not implement the getchar() accessops, so > wsmoused would fail at startup. This seems reasonable, though your mail client has line wrapped the patch so it won't apply. In this case it is simple enough to apply by hand. > Separately, I increased the maximum screen dimensions to 160x50 to > allow the 12x24 font to be used on an 1920 monitor, which looks great! Something similiar was done and reverted as it somehow broke inteldrm taking the fb over from efifb on a 4k display. Perhaps someone with a 4k display can confirm if this is still a problem. https://marc.info/?l=openbsd-bugs&m=155337866226121&w=2 That was back before we deferred most of inteldrm to when the root fs is mounted and interrupts are available. And just before the last big drm update by the look of it. revision 1.22 date: 2019/03/25 14:17:58; author: fcambus; state: Exp; lines: +3 -3; commitid: hL9P1vyWGBp5FHhE; Revert back to using previous values for EFIFB_WIDTH and EFIFB_HEIGHT, as raising them expose an issue which breaks inteldrm on large screen resolutions. Reported by chris@, and by Lucas Raab on bugs@. Thanks! revision 1.21 date: 2019/03/16 13:16:49; author: fcambus; state: Exp; lines: +3 -3; commitid: wsqjyS7rXDKbqGS3; Now that efifb(4) supports remapping the framebuffer in write combining mode, it's fast enough to raise the maximum size of the EFI framebuffer console. Set it to 160 columns and 160 rows, to match inteldrm and radeondrm. OK jcs@, kettenis@ > > Unlike the intel driver, efifb has a static buffer of that size, > which is a non-trivial amount of memory. Not setting the backing > store to save the memory results in an ultra-slow console until the > driver gets fully initialized, because it is doing scrolls by reading > from write combined memory instead of the redrawing all the > characters. > > I plan on changing that to a dynamic allocation once I am more > comfortable with the reinitialization that goes on when autoconf > gets around to the console display device, so feel free to punt on > that if you want to wait. Drivers which can act as a console have a cnattach function for very early init and then later probe again and run attach. In the case of efifb there is also efifb_cnremap() which is called when mainbus attaches. The drm drivers call efifb_detach() to stop autoconf from doing the normal probe/attach when taking over the console.
vmx(4) msi-x
This prepares vmx(4) for multi-queue operation, first by making use of msi-x
where available, and second by rearranging the queue structures to fit the
direction we're heading in. As with other drivers, here I'm reserving msi-x
vector 0 for events, then mapping tx/rx queues to the subsequent vectors.
Aside from the interrupt setup itself, the only change in behaviour here is
that queue setup is done after interrupt setup, as we'll need to know what type
of interrupt we're using to decide how many queues to use. This is how other
vmx drivers work so this must be safe.
I've tested this with esxi 6.7 and qemu. Can somone try this out on vmware
workstation or player please? I wouldn't expect those to be any different to
esxi in this respect, but it's always possible.
ok?
Index: if_vmx.c
===
RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
retrieving revision 1.55
diff -u -p -r1.55 if_vmx.c
--- if_vmx.c27 Oct 2019 22:24:40 - 1.55
+++ if_vmx.c25 May 2020 09:35:33 -
@@ -42,8 +42,7 @@
#include
#include
-#define NRXQUEUE 1
-#define NTXQUEUE 1
+#define VMX_MAX_QUEUES 1
#define NTXDESC 512 /* tx ring size */
#define NTXSEGS 8 /* tx descriptors per packet */
@@ -95,6 +94,7 @@ struct vmxnet3_txqueue {
struct vmxnet3_txring cmd_ring;
struct vmxnet3_comp_ring comp_ring;
struct vmxnet3_txq_shared *ts;
+ struct ifqueue *ifq;
};
struct vmxnet3_rxqueue {
@@ -103,6 +103,14 @@ struct vmxnet3_rxqueue {
struct vmxnet3_rxq_shared *rs;
};
+struct vmxnet3_queue {
+ struct vmxnet3_txqueue tx;
+ struct vmxnet3_rxqueue rx;
+ struct vmxnet3_softc *sc;
+ char intrname[8];
+ int intr;
+};
+
struct vmxnet3_softc {
struct device sc_dev;
struct arpcom sc_arpcom;
@@ -114,9 +122,11 @@ struct vmxnet3_softc {
bus_space_handle_t sc_ioh1;
bus_dma_tag_t sc_dmat;
void *sc_ih;
+ void *sc_qih[VMX_MAX_QUEUES];
+ int sc_nintr;
+ int sc_nqueues;
- struct vmxnet3_txqueue sc_txq[NTXQUEUE];
- struct vmxnet3_rxqueue sc_rxq[NRXQUEUE];
+ struct vmxnet3_queue sc_q[VMX_MAX_QUEUES];
struct vmxnet3_driver_shared *sc_ds;
u_int8_t *sc_mcast;
};
@@ -153,8 +163,8 @@ struct {
int vmxnet3_match(struct device *, void *, void *);
void vmxnet3_attach(struct device *, struct device *, void *);
int vmxnet3_dma_init(struct vmxnet3_softc *);
-int vmxnet3_alloc_txring(struct vmxnet3_softc *, int);
-int vmxnet3_alloc_rxring(struct vmxnet3_softc *, int);
+int vmxnet3_alloc_txring(struct vmxnet3_softc *, int, int);
+int vmxnet3_alloc_rxring(struct vmxnet3_softc *, int, int);
void vmxnet3_txinit(struct vmxnet3_softc *, struct vmxnet3_txqueue *);
void vmxnet3_rxinit(struct vmxnet3_softc *, struct vmxnet3_rxqueue *);
void vmxnet3_txstop(struct vmxnet3_softc *, struct vmxnet3_txqueue *);
@@ -164,6 +174,8 @@ void vmxnet3_enable_all_intrs(struct vmx
void vmxnet3_disable_all_intrs(struct vmxnet3_softc *);
int vmxnet3_intr(void *);
int vmxnet3_intr_intx(void *);
+int vmxnet3_intr_event(void *);
+int vmxnet3_intr_queue(void *);
void vmxnet3_evintr(struct vmxnet3_softc *);
void vmxnet3_txintr(struct vmxnet3_softc *, struct vmxnet3_txqueue *);
void vmxnet3_rxintr(struct vmxnet3_softc *, struct vmxnet3_rxqueue *);
@@ -212,6 +224,7 @@ vmxnet3_attach(struct device *parent, st
u_int memtype, ver, macl, mach, intrcfg;
u_char enaddr[ETHER_ADDR_LEN];
int (*isr)(void *);
+ int i;
memtype = pci_mapreg_type(pa->pa_pc, pa->pa_tag, 0x10);
if (pci_mapreg_map(pa, 0x10, memtype, 0, &sc->sc_iot0, &sc->sc_ioh0,
@@ -241,18 +254,22 @@ vmxnet3_attach(struct device *parent, st
WRITE_BAR1(sc, VMXNET3_BAR1_UVRS, 1);
sc->sc_dmat = pa->pa_dmat;
- if (vmxnet3_dma_init(sc)) {
- printf(": failed to setup DMA\n");
- return;
- }
WRITE_CMD(sc, VMXNET3_CMD_GET_INTRCFG);
intrcfg = READ_BAR1(sc, VMXNET3_BAR1_CMD);
isr = vmxnet3_intr;
+ sc->sc_nintr = 0;
+ sc->sc_nqueues = 1;
switch (intrcfg & VMXNET3_INTRCFG_TYPE_MASK) {
case VMXNET3_INTRCFG_TYPE_AUTO:
case VMXNET3_INTRCFG_TYPE_MSIX:
+ if (pci_intr_map_msix(pa, 0, &ih) == 0) {
+ isr = vmxnet3_intr_event;
+ sc->sc_nintr = sc->sc_nqueues + 1;
+ break;
+ }
+
/* FALLTHROUGH */
case VMXNET3_INTRCFG_TYPE_MSI:
if (pci_intr_map_msi(pa, &ih) == 0)
@@ -273,6 +290,35 @@ vmxnet3_attach(struct device *parent, st
if (intrstr)
printf(": %s", intrstr);
+ if (sc->sc_nintr > 1) {
+ for (i = 0; i < sc->sc_nqueues; i++) {
+ struct vmxnet3_queue *q;
+ int vec;
+
+ q = &sc->sc_q[i];
+ vec = i
Re: vmd(8) and thread safety: a quick proof of concept using libevent 2.1 from ports
On Mon, May 25, 2020 at 02:14:18AM -0700, Pratik Vyas wrote: > * Claudio Jeker [2020-05-25 10:14:11 +0200]: > > > The problem is that the vm exit is handled by a different thread then the > > event loop. So some event_add/evtimer_add calls are done from a different > > thread then the main event loop (mainly interrupt processing). > > > > One approach to fix this is to ensure that the event functions are > > exclusivly used in the main event thread but as usual it is easy to > > introduce bugs again. > > Hi Claudio, > > I think it is possible to do this without too much complexity till we > have one only one cpu. Bascially, we have two threads right now that do > event_add/evtimer_add, > 1) vmm_pipe / imsg comm channel between vmm and vm process > 2) vcpu0 loop > > I think it's possible to make the vmm_pipe be not its own thread and > just be an event based implementation. > > Dave actually discovered that if you disable vmm_pipe, this libevent > state corruption does not occour. > > I will take a stab at this. Sure, this is a simple fix. In the long run I guess we need to fix this better (also the way devices are implemented could benefit from this, since they are currently too synchronous to get a lot of performance out of them). -- :wq Claudio
Re: vmd(8) and thread safety: a quick proof of concept using libevent 2.1 from ports
* Claudio Jeker [2020-05-25 10:14:11 +0200]: > The problem is that the vm exit is handled by a different thread then the > event loop. So some event_add/evtimer_add calls are done from a different > thread then the main event loop (mainly interrupt processing). > > One approach to fix this is to ensure that the event functions are > exclusivly used in the main event thread but as usual it is easy to > introduce bugs again. Hi Claudio, I think it is possible to do this without too much complexity till we have one only one cpu. Bascially, we have two threads right now that do event_add/evtimer_add, 1) vmm_pipe / imsg comm channel between vmm and vm process 2) vcpu0 loop I think it's possible to make the vmm_pipe be not its own thread and just be an event based implementation. Dave actually discovered that if you disable vmm_pipe, this libevent state corruption does not occour. I will take a stab at this. -- Pratik
Re: random(4): use arc4random_ctx_buf() for large device reads
On Sun, May 24, 2020 at 09:33:27PM +0200, Christian Weisgerber wrote:
> (This is in a different part of the file from Theo's current efforts.)
>
> For large reads from /dev/random, use the arc4random_ctx_*() functions
> instead of hand-rolling the same code to set up a temporary ChaCha
> instance.
>
> ok?
ok semarie@
Eventually, I would get ride of myctx, initialize lctx to NULL, and use
(lctx == NULL) to replace (myctx == 0). But feel free to discard my remark.
>
> Index: rnd.c
> ===
> RCS file: /cvs/src/sys/dev/rnd.c,v
> retrieving revision 1.213
> diff -u -p -r1.213 rnd.c
> --- rnd.c 18 May 2020 15:00:16 - 1.213
> +++ rnd.c 24 May 2020 15:54:00 -
> @@ -589,6 +589,9 @@ arc4random_ctx_free(struct arc4random_ct
> void
> arc4random_ctx_buf(struct arc4random_ctx *ctx, void *buf, size_t n)
> {
> +#ifndef KEYSTREAM_ONLY
> + memset(buf, 0, n);
> +#endif
> chacha_encrypt_bytes((chacha_ctx *)ctx, buf, buf, n);
> }
>
> @@ -701,8 +704,7 @@ randomclose(dev_t dev, int flag, int mod
> int
> randomread(dev_t dev, struct uio *uio, int ioflag)
> {
> - u_char lbuf[KEYSZ+IVSZ];
> - chacha_ctx lctx;
> + struct arc4random_ctx *lctx;
> size_t total = uio->uio_resid;
> u_char *buf;
> int myctx = 0, ret = 0;
> @@ -712,29 +714,23 @@ randomread(dev_t dev, struct uio *uio, i
>
> buf = malloc(POOLBYTES, M_TEMP, M_WAITOK);
> if (total > RND_MAIN_MAX_BYTES) {
> - arc4random_buf(lbuf, sizeof(lbuf));
> - chacha_keysetup(&lctx, lbuf, KEYSZ * 8);
> - chacha_ivsetup(&lctx, lbuf + KEYSZ, NULL);
> - explicit_bzero(lbuf, sizeof(lbuf));
> + lctx = arc4random_ctx_new();
> myctx = 1;
> }
>
> while (ret == 0 && uio->uio_resid > 0) {
> size_t n = ulmin(POOLBYTES, uio->uio_resid);
>
> - if (myctx) {
> -#ifndef KEYSTREAM_ONLY
> - memset(buf, 0, n);
> -#endif
> - chacha_encrypt_bytes(&lctx, buf, buf, n);
> - } else
> + if (myctx)
> + arc4random_ctx_buf(lctx, buf, n);
> + else
> arc4random_buf(buf, n);
> ret = uiomove(buf, n, uio);
> if (ret == 0 && uio->uio_resid > 0)
> yield();
> }
> if (myctx)
> - explicit_bzero(&lctx, sizeof(lctx));
> + arc4random_ctx_free(lctx);
> explicit_bzero(buf, POOLBYTES);
> free(buf, M_TEMP, POOLBYTES);
> return ret;
> --
> Christian "naddy" Weisgerber na...@mips.inka.de
>
--
Sebastien Marie
Re: vmd(8) and thread safety: a quick proof of concept using libevent 2.1 from ports
On 25/05/20(Mon) 10:14, Claudio Jeker wrote: > [...] > One approach to fix this is to ensure that the event functions are > exclusivly used in the main event thread but as usual it is easy to > introduce bugs again. This is a common issue with multi-threaded applications using non thread-safe libraries. Isn't it possible to have a mechanism to add a warning or trigger an abort(3) if a thread other than the main event thread calls a libevent function?
Re: vmd(8) and thread safety: a quick proof of concept using libevent 2.1 from ports
On Mon, May 25, 2020 at 10:02:27AM +0200, Martin Pieuchot wrote: > On 24/05/20(Sun) 07:56, Dave Voutila wrote: > > On Sat, May 23, 2020 at 9:38 PM Dave Voutila wrote: > > > > > > Hello tech@, > > > > > > Attached is a diff that patches vmd(8) to utilize libevent 2.1 (from > > > ports) in an attempt to test the hypothesis that thread safety will > > > help stabilize Linux guest support. There's some longer detail below > > > about this hypothesis, but let me cut to the chase: > > > > I failed to pose the hypothesis more concretely. > > > > If I understand vmd(8) correctly, the process created by the 'vmm' > > process (after privsep) forks to create a 'vm' process representing > > the guest. After this fork, there's a call to vmm.c::vmm_pipe(), which > > adds an event to the event_base to deal with imsg's. Once in > > vm.c::run_vm(), a pthread is created (the 'event_thread') that > > initiates the event pump/loop. My speculation is this design is > > leading to spurious issues with the libevent state in the event_base. > >[...] > > > > > > - Rework the vm.c event handling to not be multi-threaded? > > I'd say that or fix base's libevent. Both imply understanding precisely > what the race/corruption is. > > Do all the threads need to modify libevent's states? What for? The problem is that the vm exit is handled by a different thread then the event loop. So some event_add/evtimer_add calls are done from a different thread then the main event loop (mainly interrupt processing). One approach to fix this is to ensure that the event functions are exclusivly used in the main event thread but as usual it is easy to introduce bugs again. I don't think there is a way to fix/change our libevent code without major drawbacks for all other libevent users. -- :wq Claudio
Re: vmd(8) and thread safety: a quick proof of concept using libevent 2.1 from ports
On 24/05/20(Sun) 07:56, Dave Voutila wrote: > On Sat, May 23, 2020 at 9:38 PM Dave Voutila wrote: > > > > Hello tech@, > > > > Attached is a diff that patches vmd(8) to utilize libevent 2.1 (from > > ports) in an attempt to test the hypothesis that thread safety will > > help stabilize Linux guest support. There's some longer detail below > > about this hypothesis, but let me cut to the chase: > > I failed to pose the hypothesis more concretely. > > If I understand vmd(8) correctly, the process created by the 'vmm' > process (after privsep) forks to create a 'vm' process representing > the guest. After this fork, there's a call to vmm.c::vmm_pipe(), which > adds an event to the event_base to deal with imsg's. Once in > vm.c::run_vm(), a pthread is created (the 'event_thread') that > initiates the event pump/loop. My speculation is this design is > leading to spurious issues with the libevent state in the event_base. >[...] > > > > - Rework the vm.c event handling to not be multi-threaded? I'd say that or fix base's libevent. Both imply understanding precisely what the race/corruption is. Do all the threads need to modify libevent's states? What for?
Re: pppx(4): avoid direct usage of pxi owned session.
On 24/05/20(Sun) 22:58, Vitaliy Makkoveev wrote: > > Please yes, remove the duplication before trying to address concurrency > > issues. It will make review easier :) > > We stopped at `pppx_ifs_lk’ removal, so let’s continue but after [1] > finished. > > 1. https://marc.info/?t=15899861152&r=1&w=2 Please, re-send the diff that need to be committed.
Re: fix pppx(4) with net/ifq.c rev 1.38
On 23/05/20(Sat) 15:38, Vitaliy Makkoveev wrote:
> > On 23 May 2020, at 12:54, Martin Pieuchot wrote:
> > On 22/05/20(Fri) 13:25, Vitaliy Makkoveev wrote:
> >> On Fri, May 22, 2020 at 07:57:13AM +1000, David Gwynne wrote:
> >>> [...]
> >>> can you try the following diff?
> >>>
> >>
> >> I tested this diff and it works for me. But the problem I pointed is
> >> about pipex(4) locking.
> >>
> >> pipex(4) requires NET_LOCK() be grabbed not only for underlaying
> >> ip{,6}_output() but for itself too. But since pppac_start() has
> >> unpredictable behavior I suggested to make it predictable [1].
> >
> > What needs the NET_LOCK() in their? We're talking about
> > pipex_ppp_output(), right? Does it really need the NET_LOCK() or
> > the KERNEL_LOCK() is what protects those data structures?
>
> Yes, about pipex_ppp_output() and pipex_output(). Except
> ip{,6}_output() nothing requires NET_LOCK(). As David Gwynne pointed,
> they can be replaced by ip{,6}_send().
Locks protect data structures, you're talking about functions, which
data structures are serialized by this lock? I'm questioning whether
there is one.
> [...]
> > In case of pipex(4) is isn't clear that the NET_LOCK() is necessary.
>
> I guess, pipex(4) was wrapped by NET_LOCK() to protect it while it’s
> accessed through `pr_input’. Is NET_LOCK() required for this case?
pipex(4) like all the network stack has been wrapped in the NET_LOCK()
because it was easy to do. That means it isn't a concious decision or
design. The fact that pipex(4) code runs under the NET_LOCK() is a side
effect of how the rest of the stack evolved. I'm questioning whether
this lock is required there. In theory it shouldn't. What is the
reality?
