forwarding in parallel
Hi,
Thank a lot to Hrvoje Popovski for testing my diff and to sashan@
and dlg@ for fixing all the fallout in pf and pseudo drivers.
Are there any bugs left? I think everything has been fixed.
To make progress I think it should be commited. Then we get MP
pressure on the network stack in real live. This allows us to fix
bugs, optimize, and unlock further.
Performance comarison is here:
http://bluhm.genua.de/perform/results/2021-07-06T08:17:07Z/perform.html
I think the spreading of the results comes from the kernel lock
around arp and neighbor discovery. These should be relaxed carefully
later.
Here you see that the CPUs are much more busy during forwarding:
http://bluhm.genua.de/perform/results/2021-07-06T08:17:07Z/2021-07-06T00%3A00%3A00Z/btrace/ssh_perform%40lt13_iperf3_-c10.3.46.36_-P10_-t10-btrace-kstack.0.svg
http://bluhm.genua.de/perform/results/2021-07-06T08:17:07Z/patch-sys-ip-multiqueue-arp-klock.0/btrace/ssh_perform%40lt13_iperf3_-c10.3.46.36_-P10_-t10-btrace-kstack.0.svg
ok to commit?
bluhm
Index: net/if.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
retrieving revision 1.642
diff -u -p -r1.642 if.c
--- net/if.c30 Jun 2021 13:23:33 - 1.642
+++ net/if.c6 Jul 2021 15:37:41 -
@@ -238,7 +238,7 @@ int ifq_congestion;
int netisr;
-#defineNET_TASKQ 1
+#defineNET_TASKQ 4
struct taskq *nettqmp[NET_TASKQ];
struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL);
@@ -834,10 +834,10 @@ if_input_process(struct ifnet *ifp, stru
* to PF globals, pipex globals, unicast and multicast addresses
* lists and the socket layer.
*/
- NET_LOCK();
+ NET_RLOCK_IN_SOFTNET();
while ((m = ml_dequeue(ml)) != NULL)
(*ifp->if_input)(ifp, m);
- NET_UNLOCK();
+ NET_RUNLOCK_IN_SOFTNET();
}
void
@@ -895,6 +895,12 @@ if_netisr(void *unused)
KERNEL_UNLOCK();
}
#endif
+ if (n & (1 << NETISR_IP))
+ ipintr();
+#ifdef INET6
+ if (n & (1 << NETISR_IPV6))
+ ip6intr();
+#endif
#if NPPP > 0
if (n & (1 << NETISR_PPP)) {
KERNEL_LOCK();
@@ -3316,12 +3322,15 @@ unhandled_af(int af)
* globals aren't ready to be accessed by multiple threads in
* parallel.
*/
-int nettaskqs = NET_TASKQ;
+int nettaskqs;
struct taskq *
net_tq(unsigned int ifindex)
{
struct taskq *t = NULL;
+
+ if (nettaskqs == 0)
+ nettaskqs = min(NET_TASKQ, ncpus);
t = nettqmp[ifindex % nettaskqs];
Index: net/if_ethersubr.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.274
diff -u -p -r1.274 if_ethersubr.c
--- net/if_ethersubr.c 7 Mar 2021 06:02:32 - 1.274
+++ net/if_ethersubr.c 6 Jul 2021 15:37:47 -
@@ -222,7 +222,10 @@ ether_resolve(struct ifnet *ifp, struct
switch (af) {
case AF_INET:
+ KERNEL_LOCK();
+ /* XXXSMP there is a MP race in arpresolve() */
error = arpresolve(ifp, rt, m, dst, eh->ether_dhost);
+ KERNEL_UNLOCK();
if (error)
return (error);
eh->ether_type = htons(ETHERTYPE_IP);
@@ -245,7 +248,10 @@ ether_resolve(struct ifnet *ifp, struct
break;
#ifdef INET6
case AF_INET6:
+ KERNEL_LOCK();
+ /* XXXSMP there is a MP race in nd6_resolve() */
error = nd6_resolve(ifp, rt, m, dst, eh->ether_dhost);
+ KERNEL_UNLOCK();
if (error)
return (error);
eh->ether_type = htons(ETHERTYPE_IPV6);
Index: net/ifq.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/ifq.c,v
retrieving revision 1.43
diff -u -p -r1.43 ifq.c
--- net/ifq.c 20 Feb 2021 04:37:26 - 1.43
+++ net/ifq.c 6 Jul 2021 15:37:13 -
@@ -243,7 +243,7 @@ void
ifq_init(struct ifqueue *ifq, struct ifnet *ifp, unsigned int idx)
{
ifq->ifq_if = ifp;
- ifq->ifq_softnet = net_tq(ifp->if_index); /* + idx */
+ ifq->ifq_softnet = net_tq(ifp->if_index + idx);
ifq->ifq_softc = NULL;
mtx_init(&ifq->ifq_mtx, IPL_NET);
@@ -617,7 +617,7 @@ void
ifiq_init(struct ifiqueue *ifiq, struct ifnet *ifp, unsigned int idx)
{
ifiq->ifiq_if = ifp;
- ifiq->ifiq_softnet = net_tq(ifp->if_index); /* + idx */
+ ifiq->ifiq_softnet = net_tq(ifp->if_index + idx);
ifiq->ifiq_softc = NULL;
mtx_init(&ifiq->ifiq_mtx, IPL_NET);
Index: net/netisr.h
===
RCS file: /data/mirror/openbsd/cvs/
Re: Correct minor lie in re_format(7)
On Tue, 06 Jul 2021 11:01:06 +0200, Martijn van Duren wrote: > There are equivalents for '+' and '?' in BRE. OK millert@ - todd
Re: vscsi/iscsid: wait for scsi_probe to complete after connections are established
Ping again. Diffs re-attached below.
Ashton Fagg writes:
> Friendly ping, really hoping someone can take a look. Diffs re-attached.
>
> Thanks,
>
> Ash
>
> Ashton Fagg writes:
>
>> Updated diffs attached.
>>
>> - I read style(9) a little more closely and worked out I had some
>> issues, so I corrected those.
>>
>> - Revisited the wording in my proposed documentation to make things a
>> clearer.
>>
>> - My mandoc changes were not lint clean - also fixed.
>>
>> No functional changes to the original diffs.
>>
>> Thanks,
>> ajf
>>
>> Ashton Fagg writes:
>>
>>> I have been working on fixing an issue (which was partially fixed by a
>>> diff I sent in earlier this year) with iscsid. With iscsi disks in
>>> /etc/fstab, sometimes the devices aren't fully up and ready before fsck
>>> tries to run - causing the machine to go into single user mode on boot.
>>>
>>> The diff that was merged earlier in the year added a poll routine which
>>> monitors for connection success before letting iscsictl return. This
>>> fixed the issue in some cases, however it's still only half the fix. The
>>> principled fix is to, additionally, wait until the scsi_probe calls are
>>> complete - at which point we can reasonably assume the disk device are
>>> ready for use. This requires some work to the vscsi driver to make this
>>> happen, as well as changes to both iscsid and iscsictl. The diffs
>>> attached here are a full implementation of this.
>>>
>>> I was still encountering this issue on one of my machines (much slower
>>> than my normal machine), where the connections would be up but the
>>> scsi_probe calls would not have completed - even with my earlier diff
>>> this would still cause the machine to go to single user mode. However,
>>> this indeed fixes that problem completely and I've been running it for a
>>> couple of weeks with no problems.
>>>
>>> The diffs are designed to be applied in the order they appear. In
>>> summary, the proposed changes are:
>>>
>>> (1) taskq.diff adds a taskq_empty function, which lets us check to see
>>> if a taskq is, well, empty. This is used in (2). Updates the man pages
>>> for taskq/task_add to reflect this.
>>>
>>> (2) vscsi.diff adds an ioctl to the vscsi driver which lets us monitor
>>> for device event queue completion. To aid in this it also separates
>>> calls to scsi_probe and scsi_detach to a dedicated taskq, rather than
>>> using systq. Updates the man pages for vscsi to reflect this.
>>>
>>> (3) iscsid.diff does the plumbing to actually call the new ioctl and
>>> provide that information back to iscsictl during status poll.
>>>
>>> (4) iscsictl.diff integrates the device queue information into the
>>> polling routine. Updates the man page for iscsictl to describe the new
>>> behavior.
>>>
>>> Based on commmit messages around vscsi it seems there was some plan to
>>> do this quite some years ago but it's hard for me to know what the
>>> proposed method was (though I assume what was envisaged might be similar
>>> to something like this).
>>>
>>> Feedback sought and greatly welcomed. I am basically certain there are
>>> ways this can be improved.
Index: sys/kern/kern_task.c
===
RCS file: /cvs/src/sys/kern/kern_task.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 kern_task.c
--- sys/kern/kern_task.c 1 Aug 2020 08:40:20 - 1.31
+++ sys/kern/kern_task.c 20 Jun 2021 22:05:13 -
@@ -394,6 +394,18 @@ task_del(struct taskq *tq, struct task *
}
int
+taskq_empty(struct taskq *tq)
+{
+ int rv;
+
+ mtx_enter(&tq->tq_mtx);
+ rv = TAILQ_EMPTY(&tq->tq_worklist);
+ mtx_leave(&tq->tq_mtx);
+
+ return (rv);
+}
+
+int
taskq_next_work(struct taskq *tq, struct task *work)
{
struct task *next;
Index: share/man/man9/task_add.9
===
RCS file: /cvs/src/share/man/man9/task_add.9,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 task_add.9
--- share/man/man9/task_add.9 8 Jun 2020 00:29:51 - 1.22
+++ share/man/man9/task_add.9 20 Jun 2021 22:05:30 -
@@ -20,6 +20,7 @@
.Sh NAME
.Nm taskq_create ,
.Nm taskq_destroy ,
+.Nm taskq_empty ,
.Nm taskq_barrier ,
.Nm taskq_del_barrier ,
.Nm task_set ,
@@ -43,6 +44,8 @@
.Fn taskq_barrier "struct taskq *tq"
.Ft void
.Fn taskq_del_barrier "struct taskq *tq" "struct task *t"
+.Ft int
+.Fn taskq_empty "struct taskq *tq"
.Ft void
.Fn task_set "struct task *t" "void (*fn)(void *)" "void *arg"
.Ft int
@@ -86,6 +89,9 @@ argument:
The threads servicing the taskq will be run without the kernel big lock.
.El
.Pp
+.Fn taskq_empty
+indicates whether there are any queued tasks.
+.Pp
.Fn taskq_destroy
causes the resources associated with a previously created taskq to be freed.
It will wait till all the tasks in the work queue are completed before
@@ -220,6 +226,9 @@ or 0 if the task was not already on the
.Fn task_pending
will return non-zero if the task is queued to run, or 0 if the task
is not queu
Re: iwx not getting to status: active
Stefan Sperling writes:
> Taking a fresh look at this in the morning, I think we should be checking
> for errors from ieee80211_set_key() before flagging the group key as valid.
> The only reason this could fail in your case is ENOMEM so it shouldn't
> make a difference regarding your test case.
I tested it for good measure.
> still ok?
OK
>
> There is still room for improvement but that's left for later. I suppose we
> should move the driver back into SCAN state if it fails to set link state UP,
> rather than having it hang. Otherwise an AP could trigger a hang accidentally
> or deliberately by not sending a group key.
>
> diff 7faf78381a333a9545f245f931e6a51077ba6762 /usr/src
> blob - bdf8ce3e1afa332f698e3dc56af77e6acb4f8689
> file + sys/dev/pci/if_iwx.c
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -6677,11 +6677,16 @@ iwx_set_key(struct ieee80211com *ic, struct ieee80211_
> struct ieee80211_key *k)
> {
> struct iwx_softc *sc = ic->ic_softc;
> + struct iwx_node *in = (void *)ni;
> struct iwx_setkey_task_arg *a;
> + int err;
>
> if (k->k_cipher != IEEE80211_CIPHER_CCMP) {
> /* Fallback to software crypto for other ciphers. */
> - return (ieee80211_set_key(ic, ni, k));
> + err = ieee80211_set_key(ic, ni, k);
> + if (!err && (k->k_flags & IEEE80211_KEY_GROUP))
> + in->in_flags |= IWX_NODE_FLAG_HAVE_GROUP_KEY;
> + return err;
> }
>
> if (sc->setkey_nkeys >= nitems(sc->setkey_arg))
Re: spamd(8) use tls_config_set_{cert,key}_file instead of relying on tls_load_file(3)
On Tue, Jul 06, 2021 at 02:37:34PM +0100, Ricardo Mestre wrote: > You got the order wrong on my diff :) > > Before, the certs were loaded by root in memory and then set by _spamd, with > my > diff they are still loaded by root but now also set, everything else > still has the same order so it should be: > > tls_config_set_*_file() > fork() > setres*id() > pledge() Glad you're right: spamd_tls_init() happens way earlier in main(). I misread the diff and thought spamd_tls_init() landed shortly before pledge(). OK kn
Re: spamd(8) use tls_config_set_{cert,key}_file instead of relying on tls_load_file(3)
Hey kn,
You got the order wrong on my diff :)
Before, the certs were loaded by root in memory and then set by _spamd, with my
diff they are still loaded by root but now also set, everything else
still has the same order so it should be:
tls_config_set_*_file()
fork()
setres*id()
pledge()
On 12:58 Tue 06 Jul , Klemens Nanni wrote:
> On Wed, Jun 30, 2021 at 01:11:38PM +0100, Ricardo Mestre wrote:
> > Hi,
> >
> > I may have seen it elsewhere, or probably not, but after checking on kn's
> > commit
> > to tls_load_file(3) it seems it's now possible to set the ca/cert/key
> > directly
> > without having to load them first from disk and set them afterwards from
> > memory.
>
> That's correct and the tls_config_set_*_{mem -> file}() changes are
> correct.
>
> > That being said the below applies this to spamd(8), no functional change
> > apart
> > from setting up TLS upfront, and now also before pledge(2). I tested it on
> > one
> > of my servers without issues, but also with only the cert or just with the
> > key
> > to ensure it would still error out.
>
> I am not a spamd user but this is what I see from code inspection:
> Currently, things happen in this order of
>
> tls_load_file()
> fork()
> setres*id()
> pledge()
> tls_config_set_*_mem()
>
> and your diff looks like changing that to
>
> fork()
> setres*id()
> tls_config_set_*_file()
> pledge()
>
> So it seems that currently file permissions on both TLS certificate and
> privat key files don't matter as they're read as root, while with your
> diff they'd be read as the unprivileged _spamd user?
>
> I didn't find details about required permissions for those files in our
> manual pages, but I'd expect them to be `root:wheel 0700' or so, i.e.
> not readable by anyone but root.
>
> Am I missing something here or are your TLS files readable by _spamd so
> that the diff works (for you)?
>
>
> > Index: spamd.c
> > ===
> > RCS file: /cvs/src/libexec/spamd/spamd.c,v
> > retrieving revision 1.156
> > diff -u -p -u -r1.156 spamd.c
> > --- spamd.c 6 Aug 2019 13:34:36 - 1.156
> > +++ spamd.c 30 Jun 2021 11:53:18 -
> > @@ -136,10 +136,6 @@ u_short cfg_port;
> > u_short sync_port;
> > struct tls_config *tlscfg;
> > struct tls *tlsctx;
> > -uint8_t*pubcert;
> > -size_t pubcertlen;
> > -uint8_t*privkey;
> > -size_t privkeylen;
> > char *tlskeyfile = NULL;
> > char *tlscertfile = NULL;
> >
> > @@ -464,9 +460,9 @@ spamd_tls_init()
> > if (tls_config_set_ciphers(tlscfg, "all") != 0)
> > errx(1, "failed to set tls ciphers");
> >
> > - if (tls_config_set_cert_mem(tlscfg, pubcert, pubcertlen) == -1)
> > + if (tls_config_set_cert_file(tlscfg, tlscertfile) == -1)
> > errx(1, "unable to set TLS certificate file %s", tlscertfile);
> > - if (tls_config_set_key_mem(tlscfg, privkey, privkeylen) == -1)
> > + if (tls_config_set_key_file(tlscfg, tlskeyfile) == -1)
> > errx(1, "unable to set TLS key file %s", tlskeyfile);
> > if (tls_configure(tlsctx, tlscfg) != 0)
> > errx(1, "failed to configure TLS - %s", tls_error(tlsctx));
> > @@ -1392,12 +1388,7 @@ main(int argc, char *argv[])
> > } else if (maxblack > maxcon)
> > usage();
> >
> > - if (tlscertfile &&
> > - (pubcert=tls_load_file(tlscertfile, &pubcertlen, NULL)) == NULL)
> > - errx(1, "unable to load TLS certificate file %s", tlscertfile);
> > - if (tlskeyfile &&
> > - (privkey=tls_load_file(tlskeyfile, &privkeylen, NULL)) == NULL)
> > - errx(1, "unable to load TLS key file %s", tlskeyfile);
> > + spamd_tls_init();
> >
> > rlp.rlim_cur = rlp.rlim_max = maxcon + 15;
> > if (setrlimit(RLIMIT_NOFILE, &rlp) == -1)
> > @@ -1546,8 +1537,6 @@ main(int argc, char *argv[])
> > jail:
> > if (pledge("stdio inet", NULL) == -1)
> > err(1, "pledge");
> > -
> > - spamd_tls_init();
> >
> > if (listen(smtplisten, 10) == -1)
> > err(1, "listen");
> >
>
Re: spamd(8) use tls_config_set_{cert,key}_file instead of relying on tls_load_file(3)
On Tue, Jul 06, 2021 at 12:58:37PM +, Klemens Nanni wrote:
> On Wed, Jun 30, 2021 at 01:11:38PM +0100, Ricardo Mestre wrote:
> > Hi,
> >
> > I may have seen it elsewhere, or probably not, but after checking on kn's
> > commit
> > to tls_load_file(3) it seems it's now possible to set the ca/cert/key
> > directly
> > without having to load them first from disk and set them afterwards from
> > memory.
>
> That's correct and the tls_config_set_*_{mem -> file}() changes are
> correct.
>
> > That being said the below applies this to spamd(8), no functional change
> > apart
> > from setting up TLS upfront, and now also before pledge(2). I tested it on
> > one
> > of my servers without issues, but also with only the cert or just with the
> > key
> > to ensure it would still error out.
>
> I am not a spamd user but this is what I see from code inspection:
> Currently, things happen in this order of
>
> tls_load_file()
> fork()
> setres*id()
> pledge()
> tls_config_set_*_mem()
>
> and your diff looks like changing that to
>
> fork()
> setres*id()
> tls_config_set_*_file()
> pledge()
>
> So it seems that currently file permissions on both TLS certificate and
> privat key files don't matter as they're read as root, while with your
> diff they'd be read as the unprivileged _spamd user?
>
> I didn't find details about required permissions for those files in our
> manual pages, but I'd expect them to be `root:wheel 0700' or so, i.e.
> not readable by anyone but root.
>
> Am I missing something here or are your TLS files readable by _spamd so
> that the diff works (for you)?
I can confirm that my 6.9-stable running spamd(8) for couple of years
now has following:
# rcctl get spamd
spamd_class=daemon
spamd_flags=-C /etc/mail/certs/example.com-full.crt -K
/etc/mail/certs/example.com.key -h example.com -v
spamd_logger=
spamd_rtable=0
spamd_timeout=30
spamd_user=root
# ls -l /etc/mail/certs/example.com-full.crt /etc/mail/certs/example.com.key
-r--r--r-- 1 root wheel 3778 May 2 20:04
/etc/mail/certs/example.com-full.crt
-r 1 root wheel 3272 Sep 16 2019 /etc/mail/certs/example.com.key
> > Index: spamd.c
> > ===
> > RCS file: /cvs/src/libexec/spamd/spamd.c,v
> > retrieving revision 1.156
> > diff -u -p -u -r1.156 spamd.c
> > --- spamd.c 6 Aug 2019 13:34:36 - 1.156
> > +++ spamd.c 30 Jun 2021 11:53:18 -
> > @@ -136,10 +136,6 @@ u_short cfg_port;
> > u_short sync_port;
> > struct tls_config *tlscfg;
> > struct tls *tlsctx;
> > -uint8_t*pubcert;
> > -size_t pubcertlen;
> > -uint8_t*privkey;
> > -size_t privkeylen;
> > char *tlskeyfile = NULL;
> > char *tlscertfile = NULL;
> >
> > @@ -464,9 +460,9 @@ spamd_tls_init()
> > if (tls_config_set_ciphers(tlscfg, "all") != 0)
> > errx(1, "failed to set tls ciphers");
> >
> > - if (tls_config_set_cert_mem(tlscfg, pubcert, pubcertlen) == -1)
> > + if (tls_config_set_cert_file(tlscfg, tlscertfile) == -1)
> > errx(1, "unable to set TLS certificate file %s", tlscertfile);
> > - if (tls_config_set_key_mem(tlscfg, privkey, privkeylen) == -1)
> > + if (tls_config_set_key_file(tlscfg, tlskeyfile) == -1)
> > errx(1, "unable to set TLS key file %s", tlskeyfile);
> > if (tls_configure(tlsctx, tlscfg) != 0)
> > errx(1, "failed to configure TLS - %s", tls_error(tlsctx));
> > @@ -1392,12 +1388,7 @@ main(int argc, char *argv[])
> > } else if (maxblack > maxcon)
> > usage();
> >
> > - if (tlscertfile &&
> > - (pubcert=tls_load_file(tlscertfile, &pubcertlen, NULL)) == NULL)
> > - errx(1, "unable to load TLS certificate file %s", tlscertfile);
> > - if (tlskeyfile &&
> > - (privkey=tls_load_file(tlskeyfile, &privkeylen, NULL)) == NULL)
> > - errx(1, "unable to load TLS key file %s", tlskeyfile);
> > + spamd_tls_init();
> >
> > rlp.rlim_cur = rlp.rlim_max = maxcon + 15;
> > if (setrlimit(RLIMIT_NOFILE, &rlp) == -1)
> > @@ -1546,8 +1537,6 @@ main(int argc, char *argv[])
> > jail:
> > if (pledge("stdio inet", NULL) == -1)
> > err(1, "pledge");
> > -
> > - spamd_tls_init();
> >
> > if (listen(smtplisten, 10) == -1)
> > err(1, "listen");
> >
>
--
Regards,
Mikolaj
Re: spamd(8) use tls_config_set_{cert,key}_file instead of relying on tls_load_file(3)
On Wed, Jun 30, 2021 at 01:11:38PM +0100, Ricardo Mestre wrote:
> Hi,
>
> I may have seen it elsewhere, or probably not, but after checking on kn's
> commit
> to tls_load_file(3) it seems it's now possible to set the ca/cert/key directly
> without having to load them first from disk and set them afterwards from
> memory.
That's correct and the tls_config_set_*_{mem -> file}() changes are
correct.
> That being said the below applies this to spamd(8), no functional change apart
> from setting up TLS upfront, and now also before pledge(2). I tested it on one
> of my servers without issues, but also with only the cert or just with the key
> to ensure it would still error out.
I am not a spamd user but this is what I see from code inspection:
Currently, things happen in this order of
tls_load_file()
fork()
setres*id()
pledge()
tls_config_set_*_mem()
and your diff looks like changing that to
fork()
setres*id()
tls_config_set_*_file()
pledge()
So it seems that currently file permissions on both TLS certificate and
privat key files don't matter as they're read as root, while with your
diff they'd be read as the unprivileged _spamd user?
I didn't find details about required permissions for those files in our
manual pages, but I'd expect them to be `root:wheel 0700' or so, i.e.
not readable by anyone but root.
Am I missing something here or are your TLS files readable by _spamd so
that the diff works (for you)?
> Index: spamd.c
> ===
> RCS file: /cvs/src/libexec/spamd/spamd.c,v
> retrieving revision 1.156
> diff -u -p -u -r1.156 spamd.c
> --- spamd.c 6 Aug 2019 13:34:36 - 1.156
> +++ spamd.c 30 Jun 2021 11:53:18 -
> @@ -136,10 +136,6 @@ u_short cfg_port;
> u_short sync_port;
> struct tls_config *tlscfg;
> struct tls *tlsctx;
> -uint8_t *pubcert;
> -size_tpubcertlen;
> -uint8_t *privkey;
> -size_tprivkeylen;
> char *tlskeyfile = NULL;
> char *tlscertfile = NULL;
>
> @@ -464,9 +460,9 @@ spamd_tls_init()
> if (tls_config_set_ciphers(tlscfg, "all") != 0)
> errx(1, "failed to set tls ciphers");
>
> - if (tls_config_set_cert_mem(tlscfg, pubcert, pubcertlen) == -1)
> + if (tls_config_set_cert_file(tlscfg, tlscertfile) == -1)
> errx(1, "unable to set TLS certificate file %s", tlscertfile);
> - if (tls_config_set_key_mem(tlscfg, privkey, privkeylen) == -1)
> + if (tls_config_set_key_file(tlscfg, tlskeyfile) == -1)
> errx(1, "unable to set TLS key file %s", tlskeyfile);
> if (tls_configure(tlsctx, tlscfg) != 0)
> errx(1, "failed to configure TLS - %s", tls_error(tlsctx));
> @@ -1392,12 +1388,7 @@ main(int argc, char *argv[])
> } else if (maxblack > maxcon)
> usage();
>
> - if (tlscertfile &&
> - (pubcert=tls_load_file(tlscertfile, &pubcertlen, NULL)) == NULL)
> - errx(1, "unable to load TLS certificate file %s", tlscertfile);
> - if (tlskeyfile &&
> - (privkey=tls_load_file(tlskeyfile, &privkeylen, NULL)) == NULL)
> - errx(1, "unable to load TLS key file %s", tlskeyfile);
> + spamd_tls_init();
>
> rlp.rlim_cur = rlp.rlim_max = maxcon + 15;
> if (setrlimit(RLIMIT_NOFILE, &rlp) == -1)
> @@ -1546,8 +1537,6 @@ main(int argc, char *argv[])
> jail:
> if (pledge("stdio inet", NULL) == -1)
> err(1, "pledge");
> -
> - spamd_tls_init();
>
> if (listen(smtplisten, 10) == -1)
> err(1, "listen");
>
Re: Correct minor lie in re_format(7)
On Tue, Jul 06, 2021 at 11:01:06AM +0200, Martijn van Duren wrote: > There are equivalents for '+' and '?' in BRE. OK kn
Re: vmd(8): simplify vcpu logic, removing uart & net reads
Looking for an OK for this one now. Anyone?
Dave Voutila writes:
> Dave Voutila writes:
>
>> Looking for some broader testing of the following diff. It cleans up
>> some complicated logic predominantly left over from the early days of
>> vmd prior to its having a dedicated device thread.
>
> Still looking for tester feedback. I've been running this diff while
> hosting multiple guests continously (OpenBSD-current, Alpine 3.14,
> Debian 10.10, Ubuntu 20.04) with no issues.
>
> I know a few folks have told me they've applied the diff and have not
> seen issues.
I've had positive reports from 4 people. Thanks everyone that tested and
provided feedback!
>
> I'll prod for OK next week, so if you've tested the diff please let me
> know!
OK to commit?
>
>>
>> In summary, this diff:
>>
>> - Removes vionet "rx pending" state handling and removes the code path
>> for the vcpu thread to possibly take control of the virtio net device
>> and attempt a read of the underlying tap(4). (virtio.{c,h}, vm.c)
>>
>> - Removes ns8250 "rcv pending" state handling and removes the code path
>> for the vcpu thread to read the pty via com_rcv(). (ns8250.{c,h})
>>
>> In both of the above cases, the event handling thread will be notified
>> of readable data and deal with it.
>>
>> Why remove them? The logic is overly complicated and hard to reason
>> about for zero gain. (This diff results in no intended functional
>> change.) Plus, some of the above logic I helped add to deal with the
>> race conditions and state corruption over a year ago. The logic was
>> needed once upon a time, but shouldn't be needed at present.
>>
>> I've had positive testing feedback from abieber@ so far with at least
>> the ns8250/uart diff, but want to cast a broader net here with both
>> before either part is committed. I debated splitting these up, but
>> they're thematically related.
>>
>> -dv
>>
>> Index: virtio.c
>> ===
>> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
>> retrieving revision 1.91
>> diff -u -p -r1.91 virtio.c
>> --- virtio.c 21 Jun 2021 02:38:18 - 1.91
>> +++ virtio.c 23 Jun 2021 11:28:03 -
>> @@ -1254,12 +1254,12 @@ static int
>> vionet_rx(struct vionet_dev *dev)
>> {
>> char buf[PAGE_SIZE];
>> -int hasdata, num_enq = 0, spc = 0;
>> +int num_enq = 0, spc = 0;
>> struct ether_header *eh;
>> ssize_t sz;
>>
>> do {
>> -sz = read(dev->fd, buf, sizeof buf);
>> +sz = read(dev->fd, buf, sizeof(buf));
>> if (sz == -1) {
>> /*
>> * If we get EAGAIN, No data is currently available.
>> @@ -1270,21 +1270,17 @@ vionet_rx(struct vionet_dev *dev)
>> "device");
>> } else if (sz > 0) {
>> eh = (struct ether_header *)buf;
>> -if (!dev->lockedmac || sz < ETHER_HDR_LEN ||
>> +if (!dev->lockedmac ||
>> ETHER_IS_MULTICAST(eh->ether_dhost) ||
>> memcmp(eh->ether_dhost, dev->mac,
>> sizeof(eh->ether_dhost)) == 0)
>> num_enq += vionet_enq_rx(dev, buf, sz, &spc);
>> } else if (sz == 0) {
>> log_debug("process_rx: no data");
>> -hasdata = 0;
>> break;
>> }
>> +} while (spc > 0 && sz > 0);
>>
>> -hasdata = fd_hasdata(dev->fd);
>> -} while (spc && hasdata);
>> -
>> -dev->rx_pending = hasdata;
>> return (num_enq);
>> }
>>
>> @@ -1301,16 +1297,6 @@ vionet_rx_event(int fd, short kind, void
>>
>> mutex_lock(&dev->mutex);
>>
>> -/*
>> - * We already have other data pending to be received. The data that
>> - * has become available now will be enqueued to the vionet_dev
>> - * later.
>> - */
>> -if (dev->rx_pending) {
>> -mutex_unlock(&dev->mutex);
>> -return;
>> -}
>> -
>> if (vionet_rx(dev) > 0) {
>> /* XXX: vcpu_id */
>> vcpu_assert_pic_irq(dev->vm_id, 0, dev->irq);
>> @@ -1320,40 +1306,6 @@ vionet_rx_event(int fd, short kind, void
>> }
>>
>> /*
>> - * vionet_process_rx
>> - *
>> - * Processes any remaining pending receivable data for a vionet device.
>> - * Called on VCPU exit. Although we poll on the tap file descriptor of
>> - * a vionet_dev in a separate thread, this function still needs to be
>> - * called on VCPU exit: it can happen that not all data fits into the
>> - * receive queue of the vionet_dev immediately. So any outstanding data
>> - * is handled here.
>> - *
>> - * Parameters:
>> - * vm_id: VM ID of the VM for which to process vionet events
>> - */
>> -void
>> -vionet_process_rx(uint32_t vm_id)
>> -{
>> -int i;
>> -
>> -for (i = 0 ; i < nr_vionet; i++) {
>> -mutex_lock(&vionet[i].mutex);
>> -if (!vionet[i].rx_adde
Re: iwx not getting to status: active
Confirmed it is also work. iwx0 at pci0 dev 20 function 3 "Intel Wi-Fi 6 AX201" rev 0x00, msix Regards, zxystd
Correct minor lie in re_format(7)
There are equivalents for '+' and '?' in BRE.
OK?
martijn@
Index: re_format.7
===
RCS file: /cvs/src/lib/libc/regex/re_format.7,v
retrieving revision 1.22
diff -u -p -r1.22 re_format.7
--- re_format.7 14 Sep 2015 20:06:58 - 1.22
+++ re_format.7 6 Jul 2021 08:59:28 -
@@ -524,13 +524,6 @@ or
Basic regular expressions differ in several respects:
.Bl -bullet -offset 3n
.It
-.Sq | ,
-.Sq + ,
-and
-.Sq ?\&
-are ordinary characters and there is no equivalent
-for their functionality.
-.It
The delimiters for bounds are
.Sq \e{
and
@@ -540,6 +533,20 @@ with
and
.Sq }
by themselves ordinary characters.
+.It
+.Sq | ,
+.Sq + ,
+and
+.Sq ?\&
+are ordinary characters.
+.Sq \e{1,\e}
+is equivalent to
+.Sq + .
+.Sq \e{0,1\e}
+is equivalent to
+.Sq ?\& .
+There is no equivalent for
+.Sq | .
.It
The parentheses for nested subexpressions are
.Sq \e(
