waitid(2)
Our devel/boost uses waitid(2) but OpenBSD does not implement it yet. In file included from /usr/local/include/boost/process.hpp:25: In file included from /usr/local/include/boost/process/group.hpp:32: /usr/local/include/boost/process/detail/posix/wait_group.hpp:38:17: error: no member named 'waitid' in the global namespace ret = ::waitid(P_PGID, p.grp, &status, WEXITED | WNOHANG); ~~^ /usr/local/include/boost/process/detail/posix/wait_group.hpp:38:24: error: use of undeclared identifier 'P_PGID' ret = ::waitid(P_PGID, p.grp, &status, WEXITED | WNOHANG); ^ /usr/local/include/boost/process/detail/posix/wait_group.hpp:38:48: error: use of undeclared identifier 'WEXITED' ret = ::waitid(P_PGID, p.grp, &status, WEXITED | WNOHANG); ^ /usr/local/include/boost/process/detail/posix/wait_group.hpp:74:17: error: no member named 'waitid' in the global namespace ret = ::waitid(P_PGID, p.grp, &siginfo, WEXITED | WSTOPPED | WNOHANG); ~~^ /usr/local/include/boost/process/detail/posix/wait_group.hpp:74:24: error: use of undeclared identifier 'P_PGID' ret = ::waitid(P_PGID, p.grp, &siginfo, WEXITED | WSTOPPED | WNOHANG); ^ /usr/local/include/boost/process/detail/posix/wait_group.hpp:74:49: error: use of undeclared identifier 'WEXITED' ret = ::waitid(P_PGID, p.grp, &siginfo, WEXITED | WSTOPPED | WNOHANG); ^ /usr/local/include/boost/process/detail/posix/wait_group.hpp:74:59: error: use of undeclared identifier 'WSTOPPED' ret = ::waitid(P_PGID, p.grp, &siginfo, WEXITED | WSTOPPED | WNOHANG); Brad (boost co-maintainer) has found out the following, which I would like to share here and ask for feedback: waitid(2) is part of newer POSIX. Looks like waitid(2) was introduced around Boost 1.69. https://github.com/boostorg/process/commits/develop/include/boost/process/detail/posix/wait_group.hpp https://github.com/boostorg/process/commit/eea73753b572ec5df69b2a624cbacf7569fde835 NetBSD imported it in 2016: https://github.com/NetBSD/src/commit/1abafffcad6331f872b7d8e74ea533ed83e864b4 Is there anything against importing this into OpenBSD? Does anyone have this in their diff stack already? I would love to test it with an port witch depends on boot/process.hpp Rafael
Re: IPsec TDB locking
On Wed, Dec 08, 2021 at 02:41:47AM +0300, Vitaliy Makkoveev wrote: > [IN] looks strange. If this field modified after creation it is > mutable. There are cases when such field could modified only once, > but it still is atomic. > > We have cases where we do assignment only once, like `unp_addr’ > when we bind(2)ing socket and we don’t modify if until socket’s > destruction. Since we could connect to successfully bind(2)ed > socket only we could say within unp_connect() that `unp_addr’ > could be accessed without lock. > > We also have the case where we could bind(2) already connected > socket. I such case we could say `unp_addr’ is atomic because > this is the pointer assignment which points to read-only data > and this data immutable until socket’s destruction. > > But `unp_addr’ is still mutable. Please stop saying that a thing is simply "atomic". If one sees "atomic" in a locking annotation, one has no idea what exactly it means. Nearly everything in the kernel can be seen as "atomic" by choosing a suitable story. Whenever a piece of code accesses shared data without locking, there has to be a well-defined protocol for the accesses. Otherwise the code cannot function reliably. Locking annotations should at least hint toward the protocol. Annotation "immutable after creation" or "immutable after setting" are clearly better than just "atomic".
relayd: small ssl.c cleanup
BIO_new_mem_buf has had const since 2018, so this workaround is no
longer needed.
Index: ssl.c
===
RCS file: /cvs/src/usr.sbin/relayd/ssl.c,v
retrieving revision 1.35
diff -u -p -r1.35 ssl.c
--- ssl.c 27 Jan 2021 20:33:05 - 1.35
+++ ssl.c 8 Dec 2021 03:47:48 -
@@ -123,16 +123,9 @@ ssl_update_certificate(const uint8_t *ol
BIO *in, *out = NULL;
BUF_MEM *bptr = NULL;
X509*cert = NULL;
- uint8_t *newcert = NULL, *foo = NULL;
+ uint8_t *newcert = NULL;
- /* XXX BIO_new_mem_buf is not using const so work around this */
- if ((foo = malloc(oldlen)) == NULL) {
- log_warn("%s: malloc", __func__);
- return (NULL);
- }
- memcpy(foo, oldcert, oldlen);
-
- if ((in = BIO_new_mem_buf(foo, oldlen)) == NULL) {
+ if ((in = BIO_new_mem_buf(oldcert, oldlen)) == NULL) {
log_warnx("%s: BIO_new_mem_buf failed", __func__);
goto done;
}
@@ -193,7 +186,6 @@ ssl_update_certificate(const uint8_t *ol
*newlen = bptr->length;
done:
- free(foo);
if (in)
BIO_free(in);
if (out)
Re: IPsec TDB locking
[IN] looks strange. If this field modified after creation it is
mutable. There are cases when such field could modified only once,
but it still is atomic.
We have cases where we do assignment only once, like `unp_addr’
when we bind(2)ing socket and we don’t modify if until socket’s
destruction. Since we could connect to successfully bind(2)ed
socket only we could say within unp_connect() that `unp_addr’
could be accessed without lock.
We also have the case where we could bind(2) already connected
socket. I such case we could say `unp_addr’ is atomic because
this is the pointer assignment which points to read-only data
and this data immutable until socket’s destruction.
But `unp_addr’ is still mutable.
> - union sockaddr_uniontdb_dst;/* Destination address */
> - union sockaddr_uniontdb_src;/* Source address */
> + union sockaddr_uniontdb_dst;/* [IN] Destination address */
> + union sockaddr_uniontdb_src;/* [IN] Source address */
> On 8 Dec 2021, at 02:06, Alexander Bluhm wrote:
>
> Hi,
>
> I have started documenting the locking strategy of struct tdb fields.
> Note that gettdb_dir() is MP safe now.
>
> In udpencap_ctlinput() we had an unprotected access to tdb_snext.
> Grab the tdb_sadb_mtx mutex there. Make the braces consistently
> for all these loops.
>
> Move NET_ASSERT_LOCKED() into the functions where the read access
> happens.
>
> ok?
>
> bluhm
>
> Index: net/pfkeyv2.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.226
> diff -u -p -r1.226 pfkeyv2.c
> --- net/pfkeyv2.c 3 Dec 2021 19:04:49 - 1.226
> +++ net/pfkeyv2.c 7 Dec 2021 22:16:20 -
> @@ -800,6 +800,8 @@ pfkeyv2_get(struct tdb *tdb, void **head
> int rval, i;
> void *p;
>
> + NET_ASSERT_LOCKED();
> +
> /* Find how much space we need */
> i = sizeof(struct sadb_sa) + sizeof(struct sadb_lifetime) +
> sizeof(struct sadb_x_counter);
> @@ -2347,6 +2349,8 @@ pfkeyv2_expire(struct tdb *tdb, u_int16_
> struct sadb_msg *smsg;
> int rval = 0;
> int i;
> +
> + NET_ASSERT_LOCKED();
>
> switch (tdb->tdb_sproto) {
> case IPPROTO_AH:
> Index: netinet/ip_ipsp.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
> retrieving revision 1.262
> diff -u -p -r1.262 ip_ipsp.c
> --- netinet/ip_ipsp.c 7 Dec 2021 17:28:46 - 1.262
> +++ netinet/ip_ipsp.c 7 Dec 2021 22:39:59 -
> @@ -243,8 +243,6 @@ reserve_spi(u_int rdomain, u_int32_t ssp
> u_int32_t spi;
> int nums;
>
> - NET_ASSERT_LOCKED();
> -
> /* Don't accept ranges only encompassing reserved SPIs. */
> if (sproto != IPPROTO_IPCOMP &&
> (tspi < sspi || tspi <= SPI_RESERVED_MAX)) {
> @@ -343,6 +341,8 @@ gettdb_dir(u_int rdomain, u_int32_t spi,
> u_int32_t hashval;
> struct tdb *tdbp;
>
> + NET_ASSERT_LOCKED();
> +
> mtx_enter(&tdb_sadb_mtx);
> hashval = tdb_hash(spi, dst, proto);
>
> @@ -374,7 +374,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
> mtx_enter(&tdb_sadb_mtx);
> hashval = tdb_hash(0, src, proto);
>
> - for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext)
> + for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext) {
> if (tdbp->tdb_sproto == proto &&
> (spi == 0 || tdbp->tdb_spi == spi) &&
> ((!reverse && tdbp->tdb_rdomain == rdomain) ||
> @@ -384,7 +384,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
> !memcmp(&tdbp->tdb_dst, dst, dst->sa.sa_len)) &&
> !memcmp(&tdbp->tdb_src, src, src->sa.sa_len))
> break;
> -
> + }
> if (tdbp != NULL) {
> tdb_ref(tdbp);
> mtx_leave(&tdb_sadb_mtx);
> @@ -395,7 +395,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
> su_null.sa.sa_len = sizeof(struct sockaddr);
> hashval = tdb_hash(0, &su_null, proto);
>
> - for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext)
> + for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext) {
> if (tdbp->tdb_sproto == proto &&
> (spi == 0 || tdbp->tdb_spi == spi) &&
> ((!reverse && tdbp->tdb_rdomain == rdomain) ||
> @@ -405,7 +405,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
> !memcmp(&tdbp->tdb_dst, dst, dst->sa.sa_len)) &&
> tdbp->tdb_src.sa.sa_family == AF_UNSPEC)
> break;
> -
> + }
> tdb_ref(tdbp);
> mtx_leave(&tdb_sadb_mtx);
> return tdbp;
> @@ -494,7 +494,7 @@ gettdbbysrc(u_int rdomain, union sockadd
> mtx_enter(&tdb_sadb_mtx);
> hashval = tdb_hash(0, src, sproto);
>
> - for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->
IPsec TDB locking
Hi,
I have started documenting the locking strategy of struct tdb fields.
Note that gettdb_dir() is MP safe now.
In udpencap_ctlinput() we had an unprotected access to tdb_snext.
Grab the tdb_sadb_mtx mutex there. Make the braces consistently
for all these loops.
Move NET_ASSERT_LOCKED() into the functions where the read access
happens.
ok?
bluhm
Index: net/pfkeyv2.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.226
diff -u -p -r1.226 pfkeyv2.c
--- net/pfkeyv2.c 3 Dec 2021 19:04:49 - 1.226
+++ net/pfkeyv2.c 7 Dec 2021 22:16:20 -
@@ -800,6 +800,8 @@ pfkeyv2_get(struct tdb *tdb, void **head
int rval, i;
void *p;
+ NET_ASSERT_LOCKED();
+
/* Find how much space we need */
i = sizeof(struct sadb_sa) + sizeof(struct sadb_lifetime) +
sizeof(struct sadb_x_counter);
@@ -2347,6 +2349,8 @@ pfkeyv2_expire(struct tdb *tdb, u_int16_
struct sadb_msg *smsg;
int rval = 0;
int i;
+
+ NET_ASSERT_LOCKED();
switch (tdb->tdb_sproto) {
case IPPROTO_AH:
Index: netinet/ip_ipsp.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.262
diff -u -p -r1.262 ip_ipsp.c
--- netinet/ip_ipsp.c 7 Dec 2021 17:28:46 - 1.262
+++ netinet/ip_ipsp.c 7 Dec 2021 22:39:59 -
@@ -243,8 +243,6 @@ reserve_spi(u_int rdomain, u_int32_t ssp
u_int32_t spi;
int nums;
- NET_ASSERT_LOCKED();
-
/* Don't accept ranges only encompassing reserved SPIs. */
if (sproto != IPPROTO_IPCOMP &&
(tspi < sspi || tspi <= SPI_RESERVED_MAX)) {
@@ -343,6 +341,8 @@ gettdb_dir(u_int rdomain, u_int32_t spi,
u_int32_t hashval;
struct tdb *tdbp;
+ NET_ASSERT_LOCKED();
+
mtx_enter(&tdb_sadb_mtx);
hashval = tdb_hash(spi, dst, proto);
@@ -374,7 +374,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
mtx_enter(&tdb_sadb_mtx);
hashval = tdb_hash(0, src, proto);
- for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext)
+ for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext) {
if (tdbp->tdb_sproto == proto &&
(spi == 0 || tdbp->tdb_spi == spi) &&
((!reverse && tdbp->tdb_rdomain == rdomain) ||
@@ -384,7 +384,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
!memcmp(&tdbp->tdb_dst, dst, dst->sa.sa_len)) &&
!memcmp(&tdbp->tdb_src, src, src->sa.sa_len))
break;
-
+ }
if (tdbp != NULL) {
tdb_ref(tdbp);
mtx_leave(&tdb_sadb_mtx);
@@ -395,7 +395,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
su_null.sa.sa_len = sizeof(struct sockaddr);
hashval = tdb_hash(0, &su_null, proto);
- for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext)
+ for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext) {
if (tdbp->tdb_sproto == proto &&
(spi == 0 || tdbp->tdb_spi == spi) &&
((!reverse && tdbp->tdb_rdomain == rdomain) ||
@@ -405,7 +405,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
!memcmp(&tdbp->tdb_dst, dst, dst->sa.sa_len)) &&
tdbp->tdb_src.sa.sa_family == AF_UNSPEC)
break;
-
+ }
tdb_ref(tdbp);
mtx_leave(&tdb_sadb_mtx);
return tdbp;
@@ -494,7 +494,7 @@ gettdbbysrc(u_int rdomain, union sockadd
mtx_enter(&tdb_sadb_mtx);
hashval = tdb_hash(0, src, sproto);
- for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext)
+ for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext) {
if ((tdbp->tdb_sproto == sproto) &&
(tdbp->tdb_rdomain == rdomain) &&
((tdbp->tdb_flags & TDBF_INVALID) == 0) &&
@@ -504,7 +504,7 @@ gettdbbysrc(u_int rdomain, union sockadd
continue;
break;
}
-
+ }
tdb_ref(tdbp);
mtx_leave(&tdb_sadb_mtx);
return tdbp;
@@ -900,8 +900,7 @@ tdb_unlink_locked(struct tdb *tdbp)
if (tdbsrc[hashval] == tdbp) {
tdbsrc[hashval] = tdbp->tdb_snext;
- }
- else {
+ } else {
for (tdbpp = tdbsrc[hashval]; tdbpp != NULL;
tdbpp = tdbpp->tdb_snext) {
if (tdbpp->tdb_snext == tdbp) {
@@ -1031,8 +1030,6 @@ struct tdb *
tdb_alloc(u_int rdomain)
{
struct tdb *tdbp;
-
- NET_ASSERT_LOCKED();
tdbp = pool_get(&tdb_pool, PR_WAITOK | PR_ZERO);
Index: netinet/ip_ipsp.h
===
RCS file: /data/mirror/open
Re: netstart: create virtual interfaces upfront when passing specific ones
On Tue, Nov 23, 2021 at 01:17:14AM +, Klemens Nanni wrote:
> On Tue, Nov 16, 2021 at 11:09:40PM +, Klemens Nanni wrote:
> > Run on boot without arguments, netstart(8) creates all virtual
> > interfaces *for which hostname.if files exist* before configuring them.
> >
> > This prevents ordering problems with bridges and its members, as dlg's
> > commit message from 2018 reminds us.
> >
> > But it also helps interface types like pair(4) which pair one another
> > in whatever way the user says:
> >
> > $ cat /etc/hostname.pair1
> > patch pair2
> > $ cat /etc/hostname.pair2
> > rdomain 1
> >
> > On boot this works, but `sh /etc/netstart pair1 pair2' won't work
> > because pair2 does not exist a creation time of pair1 because netstart
> > does not create virtual interfaces upfront.
> >
> > I just hit this exact use case when setting up gelatod(8) (see ports@).
> >
> > To fix this, pass the list of interfaces to vifscreate() and make it
> > create only those iff given.
> >
> > Regular boot, i.e. `sh /etc/netstart', stays uneffected by this and
> > selective runs as shown work as expected without requring users to know
> > the order in which netstart creates/configures interfaces.
> >
> > The installer's internal version of netstart doesn't need this at all;
> > neither does it have the selective semantic nor does vifscreate() exist.
>
> Anyone?
>
> It seems only logical to treat subsets of interfaces the same way as
> a full `sh /etc/netstart'.
>
> A pair of pair(4) is one example, I'm certain there are more scenarios
> where you craft interfaces with `ifconfig ...' in the shell, then set up
> the hostname.* files and test them with `sh /etc/netstart bridge0 ...'
> where pseudo interfaces are involved.
Anyone?
This is really practical and fixes things at least for me when I destroy
interfaces, reconfigure and recreate them together, for example like so:
# ifconfig pair2 destroy
# ifconfig pair1 destroy
... edit hostname.*
# sh /etc/netstart pair1 pair2
ifconfig: patch pair2: No such file or directory
add net default: gateway 192.0.0.1
(redoing it because who knows what failed due to the order problem and
what didn't...)
# ifconfig pair2 destroy
# ifconfig pair1 destroy
# sh /usr/src/etc/netstart pair1 pair2
add net default: gateway 192.0.0.1
Feedback? Objection? OK?
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.216
diff -u -p -r1.216 netstart
--- netstart2 Sep 2021 19:38:20 - 1.216
+++ netstart7 Dec 2021 20:08:16 -
@@ -94,9 +94,11 @@ ifcreate() {
}
# Create interfaces for network pseudo-devices referred to by hostname.if
files.
-# Usage: vifscreate
+# Optionally, limit creation to given interfaces only.
+# Usage: vifscreate [if ...]
vifscreate() {
- local _vif _hn _if
+ local _vif _hn _if _ifs
+ set -A _ifs -- "$@"
for _vif in $(ifconfig -C); do
for _hn in /etc/hostname.${_vif}+([[:digit:]]); do
@@ -106,6 +108,9 @@ vifscreate() {
# loopback for routing domain is created by kernel
[[ -n ${_if##lo[1-9]*} ]] || continue
+ ((${#_ifs[*]} > 0)) && [[ ${_ifs[*]} != *${_if}* ]] &&
+ continue
+
if ! ifcreate $_if; then
print -u2 "${0##*/}: create for '$_if' failed."
fi
@@ -303,6 +308,7 @@ $PRINT_ONLY || [[ ! -f /etc/soii.key ]]
# If we were invoked with a list of interface names, just reconfigure these
# interfaces (or bridges), add default routes and return.
if (($# > 0)); then
+ vifscreate "$@"
for _if; do ifstart $_if; done
defaultroute
return
Re: Please test: UVM fault unlocking (aka vmobjlock)
On Tue, Dec 07, 2021 at 10:04:04AM -0600, Scott Cheloha wrote: > On Mon, Nov 29, 2021 at 10:50:32PM +0100, Martin Pieuchot wrote: > > On 24/11/21(Wed) 11:16, Martin Pieuchot wrote: > > > Diff below unlock the bottom part of the UVM fault handler. I'm > > > interested in squashing the remaining bugs. Please test with your usual > > > setup & report back. > > > > Thanks to all the testers, here's a new version that includes a bug fix. > > > > Tests on !x86 architectures are much appreciated! > > [...] > > witness: lock order reversal: > 1st 0xfd83d37b23c8 uobjlk (&uobj->vmobjlock) > 2nd 0x815fdb00 objmm (&obj->mm.lock) > lock order "&obj->mm.lock"(rwlock) -> "&uobj->vmobjlock"(rwlock) first seen > at: > #0 rw_enter+0x68 > #1 uvm_obj_wire+0x4f > #2 shmem_get_pages+0xae > #3 __i915_gem_object_get_pages+0x85 > #4 i915_vma_pin_ww+0x451 > #5 i915_ggtt_pin+0x61 > #6 intel_execlists_submission_setup+0x396 > #7 intel_engines_init+0x2ff > #8 intel_gt_init+0x136 > #9 i915_gem_init+0x9d > #10 i915_driver_probe+0x760 > #11 inteldrm_attachhook+0x46 > #12 config_process_deferred_mountroot+0x5b > #13 main+0x743 > lock order "&uobj->vmobjlock"(rwlock) -> "&obj->mm.lock"(rwlock) first seen > at: > #0 rw_enter+0x68 > #1 __i915_gem_object_get_pages+0x29 > #2 i915_gem_fault+0x1cb > #3 drm_fault+0x163 > #4 uvm_fault+0x19b > #5 upageflttrap+0x5e > #6 usertrap+0x190 > #7 recall_trap+0x8 Seeing the same reversal with the same stack trace during all boots. Also, just saw a real novelty, a simultaneous trap panic and a warning printf from vref(). The console showed this: pWaAnRiNcI:NG: vSrPeLf OTs eLdO ER e Dr eO Nv gTeRtA Pr eEqXuIiTr eda 0 Which I deciphered into: WARNING: vref used where vget required and panic: SPL NOT LOWERED ON TRAP EXIT a 0 The warning printf is from vref() in vfs_subr.c. The panic could be from Xintr_user_exit() in locore.S or alltraps_kern() in vector.S. Not familiar enough with that code to say which. The console locked up and I was not able to look at anything in ddb. Same machine as before: > dmesg: > > OpenBSD 7.0-current (GENERIC.MP) #17: Tue Dec 7 09:39:06 CST 2021 > ssc@jetsam.local:/usr/src/sys/arch/amd64/compile/GENERIC.MP > real mem = 16895528960 (16112MB) > avail mem = 16237621248 (15485MB) > random: good seed from bootblocks > mpath0 at root > scsibus0 at mpath0: 256 targets > mainbus0 at root > bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x9f03b000 (63 entries) > bios0: vendor LENOVO version "N23ET59W (1.34 )" date 11/08/2018 > bios0: LENOVO 20KHCTO1WW > acpi0 at bios0: ACPI 5.0 > acpi0: sleep states S0 S3 S4 S5 > acpi0: tables DSDT FACP SSDT SSDT TPM2 UEFI SSDT SSDT HPET APIC MCFG ECDT > SSDT SSDT BOOT BATB SSDT SSDT SSDT LPIT WSMT SSDT SSDT SSDT DBGP DBG2 MSDM > DMAR NHLT ASF! FPDT UEFI > acpi0: wakeup devices GLAN(S4) XHC_(S3) XDCI(S4) HDAS(S4) RP01(S4) PXSX(S4) > RP02(S4) PXSX(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) RP06(S4) > PXSX(S4) RP07(S4) [...] > acpitimer0 at acpi0: 3579545 Hz, 24 bits > acpihpet0 at acpi0: 2399 Hz > acpimadt0 at acpi0 addr 0xfee0: PC-AT compat > cpu0 at mainbus0: apid 0 (boot processor) > cpu0: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1795.82 MHz, 06-8e-0a > cpu0: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN > cpu0: 256KB 64b/line 8-way L2 cache > cpu0: smt 0, core 0, package 0 > mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges > cpu0: apic clock running at 24MHz > cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE > cpu1 at mainbus0: apid 2 (application processor) > tsc: cpu0/cpu1 sync round 1: 1865 regressions > tsc: cpu0 lags cpu1 by 0 cycles > tsc: cpu1 lags cpu0 by 56 cycles > cpu1: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1795.82 MHz, 06-8e-0a > cpu1: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN > cpu1: 256KB 64b/line 8-way L2 cache > cpu1: smt 0, core 1, package 0 > cpu2 at mainbus0: apid 4 (application processor) > tsc: cpu0/cpu2 sync round 1: 1921 regressions > tsc: cpu0 lags cpu2 by 0 cycles >
Re: Add EVFILT_EXCEPT for pipes
On Tue, 07 Dec 2021 16:25:59 +, Visa Hankala wrote: > In simple cases that do not have out-of-band data, the system can skip > registering the EVFILT_EXCEPT filter. A new flag could indicate when > the registration is done for select(2). The patch below does the > tweaking for FIFOs and pipes, to demonstrate the idea. Ptys and sockets > need to be adjusted as well. > > The flags should probably be refactored so that __EV_POLL would mean > poll(2) only. However, that results in some mechanical code churn that > I would do separately if the use of __EV_SELECT appears viable in the > first place. That looks like it should result in the expected behavior. OK millert@ - todd
Re: IPsec delete TDB in ipo cache
> On 7 Dec 2021, at 18:31, Alexander Bluhm wrote:
>
> Hi,
>
> In ipo_tdb the flow contains a reference counted TDB cache. This
> may prevent that tdb_free() is called. It is not a real leak as
> ipsecctl -F or termination of iked flush this cache. The kernel
> does the cleanup itself if we move the code from tdb_free() to
> tdb_delete().
>
> ok?
>
With your early TDB refcounting diff we it was exposed we had TDB
linked to it's `tdb_policy_head’.
ok mvs@
> bluhm
>
> Index: netinet/ip_ipsp.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
> retrieving revision 1.261
> diff -u -p -r1.261 ip_ipsp.c
> --- netinet/ip_ipsp.c 3 Dec 2021 19:04:49 - 1.261
> +++ netinet/ip_ipsp.c 7 Dec 2021 15:27:16 -
> @@ -923,6 +923,19 @@ tdb_unlink_locked(struct tdb *tdbp)
> }
>
> void
> +tdb_cleanspd(struct tdb *tdbp)
> +{
> + struct ipsec_policy *ipo;
> +
> + while ((ipo = TAILQ_FIRST(&tdbp->tdb_policy_head)) != NULL) {
> + TAILQ_REMOVE(&tdbp->tdb_policy_head, ipo, ipo_tdb_next);
> + tdb_unref(ipo->ipo_tdb);
> + ipo->ipo_tdb = NULL;
> + ipo->ipo_last_searched = 0; /* Force a re-search. */
> + }
> +}
> +
> +void
> tdb_unbundle(struct tdb *tdbp)
> {
> if (tdbp->tdb_onext != NULL) {
> @@ -1001,6 +1014,8 @@ tdb_dodelete(struct tdb *tdbp, int locke
> else
> tdb_unlink(tdbp);
>
> + /* cleanup SPD references */
> + tdb_cleanspd(tdbp);
> /* release tdb_onext/tdb_inext references */
> tdb_unbundle(tdbp);
> /* delete timeouts and release references */
> @@ -1043,8 +1058,6 @@ tdb_alloc(u_int rdomain)
> void
> tdb_free(struct tdb *tdbp)
> {
> - struct ipsec_policy *ipo;
> -
> NET_ASSERT_LOCKED();
>
> if (tdbp->tdb_xform) {
> @@ -1057,13 +1070,7 @@ tdb_free(struct tdb *tdbp)
> pfsync_delete_tdb(tdbp);
> #endif
>
> - /* Cleanup SPD references. */
> - while ((ipo = TAILQ_FIRST(&tdbp->tdb_policy_head)) != NULL) {
> - TAILQ_REMOVE(&tdbp->tdb_policy_head, ipo, ipo_tdb_next);
> - tdb_unref(ipo->ipo_tdb);
> - ipo->ipo_tdb = NULL;
> - ipo->ipo_last_searched = 0; /* Force a re-search. */
> - }
> + KASSERT(TAILQ_EMPTY(&tdbp->tdb_policy_head));
>
> if (tdbp->tdb_ids) {
> ipsp_ids_free(tdbp->tdb_ids);
> Index: netinet/ip_ipsp.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.h,v
> retrieving revision 1.227
> diff -u -p -r1.227 ip_ipsp.h
> --- netinet/ip_ipsp.h 3 Dec 2021 19:04:49 - 1.227
> +++ netinet/ip_ipsp.h 7 Dec 2021 13:48:51 -
> @@ -577,6 +577,7 @@ void tdb_free(struct tdb *);
> int tdb_init(struct tdb *, u_int16_t, struct ipsecinit *);
> void tdb_unlink(struct tdb *);
> void tdb_unlink_locked(struct tdb *);
> +void tdb_cleanspd(struct tdb *);
> void tdb_unbundle(struct tdb *);
> void tdb_deltimeouts(struct tdb *);
> int tdb_walk(u_int, int (*)(struct tdb *, void *, int), void *);
>
Re: Add EVFILT_EXCEPT for pipes
On Tue, Dec 07, 2021 at 08:40:53AM -0700, Todd C. Miller wrote:
> On Mon, 06 Dec 2021 17:05:39 +, Visa Hankala wrote:
>
> > This adds EVFILT_EXCEPT handler for pipes. It is a subset of
> > EVFILT_READ; it triggers on EOF only. This captures the POLLHUP
> > condition of pipe_poll(), used with select(2)'s exceptfds and when
> > poll(2) has events without (POLLIN | POLLRDNORM | POLLOUT | POLLWRNORM).
>
> This looks wrong. We should only be setting exceptfds for out of
> band data on sockets and ptys.
>
> Translating POLLHUP to exceptfds was a bug introduced by the previous
> select -> poll conversion in the kernel. I'm probably responsible
> for that, sorry. This non-standard behavior has resulted in at
> least one bug in the tree with ssh-keyscan being subtly broken on
> other systems.
Very well. Let's adjust the code already now, then.
In simple cases that do not have out-of-band data, the system can skip
registering the EVFILT_EXCEPT filter. A new flag could indicate when
the registration is done for select(2). The patch below does the
tweaking for FIFOs and pipes, to demonstrate the idea. Ptys and sockets
need to be adjusted as well.
The flags should probably be refactored so that __EV_POLL would mean
poll(2) only. However, that results in some mechanical code churn that
I would do separately if the use of __EV_SELECT appears viable in the
first place.
Index: kern/sys_generic.c
===
RCS file: src/sys/kern/sys_generic.c,v
retrieving revision 1.144
diff -u -p -r1.144 sys_generic.c
--- kern/sys_generic.c 30 Nov 2021 02:58:33 - 1.144
+++ kern/sys_generic.c 7 Dec 2021 15:57:45 -
@@ -762,7 +762,7 @@ pselregister(struct proc *p, fd_set *pib
DPRINTFN(2, "select fd %d mask %d serial %lu\n",
fd, msk, p->p_kq_serial);
EV_SET(&kev, fd, evf[msk],
- EV_ADD|EV_ENABLE|__EV_POLL,
+ EV_ADD|EV_ENABLE|__EV_POLL|__EV_SELECT,
evff[msk], 0, (void *)(p->p_kq_serial));
#ifdef KTRACE
if (KTRPOINT(p, KTR_STRUCT))
@@ -775,7 +775,8 @@ pselregister(struct proc *p, fd_set *pib
/* FALLTHROUGH */
case EOPNOTSUPP:/* No underlying kqfilter */
case EINVAL:/* Unimplemented filter */
- case EPERM: /* Specific to FIFO */
+ case EPERM: /* Specific to FIFO and
+* __EV_SELECT */
error = 0;
break;
case EPIPE: /* Specific to pipes */
Index: kern/sys_pipe.c
===
RCS file: src/sys/kern/sys_pipe.c,v
retrieving revision 1.130
diff -u -p -r1.130 sys_pipe.c
--- kern/sys_pipe.c 7 Dec 2021 14:06:16 - 1.130
+++ kern/sys_pipe.c 7 Dec 2021 15:57:45 -
@@ -922,6 +922,11 @@ pipe_kqfilter(struct file *fp, struct kn
klist_insert_locked(&wpipe->pipe_sel.si_note, kn);
break;
case EVFILT_EXCEPT:
+ if (kn->kn_flags & __EV_SELECT) {
+ /* Prevent triggering exceptfds. */
+ error = EPERM;
+ break;
+ }
kn->kn_fop = &pipe_efiltops;
kn->kn_hook = rpipe;
klist_insert_locked(&rpipe->pipe_sel.si_note, kn);
Index: miscfs/fifofs/fifo_vnops.c
===
RCS file: src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.85
diff -u -p -r1.85 fifo_vnops.c
--- miscfs/fifofs/fifo_vnops.c 24 Oct 2021 11:23:22 - 1.85
+++ miscfs/fifofs/fifo_vnops.c 7 Dec 2021 15:57:45 -
@@ -540,6 +540,10 @@ fifo_kqfilter(void *v)
sb = &so->so_snd;
break;
case EVFILT_EXCEPT:
+ if (ap->a_kn->kn_flags & __EV_SELECT) {
+ /* Prevent triggering exceptfds. */
+ return (EPERM);
+ }
ap->a_kn->kn_fop = &fifoexcept_filtops;
so = fip->fi_readsock;
sb = &so->so_rcv;
Index: sys/event.h
===
RCS file: src/sys/sys/event.h,v
retrieving revision 1.59
diff -u -p -r1.59 event.h
--- sys/event.h 29 Nov 2021 15:54:04 - 1.59
+++ sys/event.h 7 Dec 2021 15:57:45 -
@@ -74,7 +74,7 @@ struct kevent {
#define EV_RECEIPT 0x0040 /* force EV_ERROR on success, data=0 */
#define EV_DISPATCH0x0080 /* disable event after reporting */
-#define EV_SYSFLAGS0xF000 /* reserved by system */
Re: Please test: UVM fault unlocking (aka vmobjlock)
On Mon, Nov 29, 2021 at 10:50:32PM +0100, Martin Pieuchot wrote: > On 24/11/21(Wed) 11:16, Martin Pieuchot wrote: > > Diff below unlock the bottom part of the UVM fault handler. I'm > > interested in squashing the remaining bugs. Please test with your usual > > setup & report back. > > Thanks to all the testers, here's a new version that includes a bug fix. > > Tests on !x86 architectures are much appreciated! At the risk of not being appreciated, with this patch I see this reversal during boot: witness: lock order reversal: 1st 0xfd83d37b23c8 uobjlk (&uobj->vmobjlock) 2nd 0x815fdb00 objmm (&obj->mm.lock) lock order "&obj->mm.lock"(rwlock) -> "&uobj->vmobjlock"(rwlock) first seen at: #0 rw_enter+0x68 #1 uvm_obj_wire+0x4f #2 shmem_get_pages+0xae #3 __i915_gem_object_get_pages+0x85 #4 i915_vma_pin_ww+0x451 #5 i915_ggtt_pin+0x61 #6 intel_execlists_submission_setup+0x396 #7 intel_engines_init+0x2ff #8 intel_gt_init+0x136 #9 i915_gem_init+0x9d #10 i915_driver_probe+0x760 #11 inteldrm_attachhook+0x46 #12 config_process_deferred_mountroot+0x5b #13 main+0x743 lock order "&uobj->vmobjlock"(rwlock) -> "&obj->mm.lock"(rwlock) first seen at: #0 rw_enter+0x68 #1 __i915_gem_object_get_pages+0x29 #2 i915_gem_fault+0x1cb #3 drm_fault+0x163 #4 uvm_fault+0x19b #5 upageflttrap+0x5e #6 usertrap+0x190 #7 recall_trap+0x8 dmesg: OpenBSD 7.0-current (GENERIC.MP) #17: Tue Dec 7 09:39:06 CST 2021 ssc@jetsam.local:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 16895528960 (16112MB) avail mem = 16237621248 (15485MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x9f03b000 (63 entries) bios0: vendor LENOVO version "N23ET59W (1.34 )" date 11/08/2018 bios0: LENOVO 20KHCTO1WW acpi0 at bios0: ACPI 5.0 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SSDT SSDT TPM2 UEFI SSDT SSDT HPET APIC MCFG ECDT SSDT SSDT BOOT BATB SSDT SSDT SSDT LPIT WSMT SSDT SSDT SSDT DBGP DBG2 MSDM DMAR NHLT ASF! FPDT UEFI acpi0: wakeup devices GLAN(S4) XHC_(S3) XDCI(S4) HDAS(S4) RP01(S4) PXSX(S4) RP02(S4) PXSX(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) RP06(S4) PXSX(S4) RP07(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 2399 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1795.82 MHz, 06-8e-0a cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 24MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE cpu1 at mainbus0: apid 2 (application processor) tsc: cpu0/cpu1 sync round 1: 1865 regressions tsc: cpu0 lags cpu1 by 0 cycles tsc: cpu1 lags cpu0 by 56 cycles cpu1: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1795.82 MHz, 06-8e-0a cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 4 (application processor) tsc: cpu0/cpu2 sync round 1: 1921 regressions tsc: cpu0 lags cpu2 by 0 cycles tsc: cpu2 lags cpu0 by 60 cycles cpu2: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, 1794.85 MHz, 06-8e-0a cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SRBDS_CTRL,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN cpu2: 256KB 64b/line 8-way L2 cache cpu2: smt 0, core 2, package 0 cpu3 at mainbus0: apid 6 (application processor) tsc: cpu0/cpu3 sync round 1: 1819 regressions tsc: cpu0 lags cpu3 by 0 cycles ts
Re: Add EVFILT_EXCEPT for pipes
On Mon, 06 Dec 2021 17:05:39 +, Visa Hankala wrote: > This adds EVFILT_EXCEPT handler for pipes. It is a subset of > EVFILT_READ; it triggers on EOF only. This captures the POLLHUP > condition of pipe_poll(), used with select(2)'s exceptfds and when > poll(2) has events without (POLLIN | POLLRDNORM | POLLOUT | POLLWRNORM). This looks wrong. We should only be setting exceptfds for out of band data on sockets and ptys. Translating POLLHUP to exceptfds was a bug introduced by the previous select -> poll conversion in the kernel. I'm probably responsible for that, sorry. This non-standard behavior has resulted in at least one bug in the tree with ssh-keyscan being subtly broken on other systems. - todd
IPsec delete TDB in ipo cache
Hi,
In ipo_tdb the flow contains a reference counted TDB cache. This
may prevent that tdb_free() is called. It is not a real leak as
ipsecctl -F or termination of iked flush this cache. The kernel
does the cleanup itself if we move the code from tdb_free() to
tdb_delete().
ok?
bluhm
Index: netinet/ip_ipsp.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.261
diff -u -p -r1.261 ip_ipsp.c
--- netinet/ip_ipsp.c 3 Dec 2021 19:04:49 - 1.261
+++ netinet/ip_ipsp.c 7 Dec 2021 15:27:16 -
@@ -923,6 +923,19 @@ tdb_unlink_locked(struct tdb *tdbp)
}
void
+tdb_cleanspd(struct tdb *tdbp)
+{
+ struct ipsec_policy *ipo;
+
+ while ((ipo = TAILQ_FIRST(&tdbp->tdb_policy_head)) != NULL) {
+ TAILQ_REMOVE(&tdbp->tdb_policy_head, ipo, ipo_tdb_next);
+ tdb_unref(ipo->ipo_tdb);
+ ipo->ipo_tdb = NULL;
+ ipo->ipo_last_searched = 0; /* Force a re-search. */
+ }
+}
+
+void
tdb_unbundle(struct tdb *tdbp)
{
if (tdbp->tdb_onext != NULL) {
@@ -1001,6 +1014,8 @@ tdb_dodelete(struct tdb *tdbp, int locke
else
tdb_unlink(tdbp);
+ /* cleanup SPD references */
+ tdb_cleanspd(tdbp);
/* release tdb_onext/tdb_inext references */
tdb_unbundle(tdbp);
/* delete timeouts and release references */
@@ -1043,8 +1058,6 @@ tdb_alloc(u_int rdomain)
void
tdb_free(struct tdb *tdbp)
{
- struct ipsec_policy *ipo;
-
NET_ASSERT_LOCKED();
if (tdbp->tdb_xform) {
@@ -1057,13 +1070,7 @@ tdb_free(struct tdb *tdbp)
pfsync_delete_tdb(tdbp);
#endif
- /* Cleanup SPD references. */
- while ((ipo = TAILQ_FIRST(&tdbp->tdb_policy_head)) != NULL) {
- TAILQ_REMOVE(&tdbp->tdb_policy_head, ipo, ipo_tdb_next);
- tdb_unref(ipo->ipo_tdb);
- ipo->ipo_tdb = NULL;
- ipo->ipo_last_searched = 0; /* Force a re-search. */
- }
+ KASSERT(TAILQ_EMPTY(&tdbp->tdb_policy_head));
if (tdbp->tdb_ids) {
ipsp_ids_free(tdbp->tdb_ids);
Index: netinet/ip_ipsp.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.h,v
retrieving revision 1.227
diff -u -p -r1.227 ip_ipsp.h
--- netinet/ip_ipsp.h 3 Dec 2021 19:04:49 - 1.227
+++ netinet/ip_ipsp.h 7 Dec 2021 13:48:51 -
@@ -577,6 +577,7 @@ voidtdb_free(struct tdb *);
inttdb_init(struct tdb *, u_int16_t, struct ipsecinit *);
void tdb_unlink(struct tdb *);
void tdb_unlink_locked(struct tdb *);
+void tdb_cleanspd(struct tdb *);
void tdb_unbundle(struct tdb *);
void tdb_deltimeouts(struct tdb *);
inttdb_walk(u_int, int (*)(struct tdb *, void *, int), void *);
Re: lsearch(3): implement with lfind(3)
On Mon, 06 Dec 2021 22:37:02 -0600, Scott Cheloha wrote: > lsearch(3) is just lfind(3) + append. If we write it that way we > shrink lsearch.c. The third function, linear_base(), is just added > complexity. The indirection buys us nothing. > > I don't think we need to keep the historical comment, either. > lsearch(3) is pure computation, it does not set errno: The End. > The spec defines no errors, either. This results in clearer code. OK millert@ - todd
Re: lsearch(3): implement with lfind(3)
On Mon, Dec 06, 2021 at 10:37:02PM -0600, Scott Cheloha wrote:
> lsearch(3) is just lfind(3) + append. If we write it that way we
> shrink lsearch.c. The third function, linear_base(), is just added
> complexity. The indirection buys us nothing.
>
> I don't think we need to keep the historical comment, either.
> lsearch(3) is pure computation, it does not set errno: The End.
> The spec defines no errors, either.
>
> libc housekeeping:
>
> - lfind becomes PROTO_NORMAL because we're now using it internally.
> - lfind then needs DEF_WEAK because it is not ISO C.
>
> Unsure if I'm missing other libc housekeeping.
>
> If not, ok?
guenther@ says off-list that I should show that lib/check_sym is
happy with the change. After building, but before installing
the patched libc, I get this:
$ cd /usr/src/lib/libc
$ doas make -j4 build > /dev/null
$ /usr/src/lib/check_sym
/usr/lib/libc.so.96.1 --> obj/libc.so.96.1
No dynamic export changes
Which he says is the right thing.
Index: stdlib/lsearch.c
===
RCS file: /cvs/src/lib/libc/stdlib/lsearch.c,v
retrieving revision 1.6
diff -u -p -r1.6 lsearch.c
--- stdlib/lsearch.c7 Dec 2021 04:01:45 - 1.6
+++ stdlib/lsearch.c7 Dec 2021 04:32:34 -
@@ -37,53 +37,34 @@
#include
typedef int (*cmp_fn_t)(const void *, const void *);
-static void *linear_base(const void *, const void *, size_t *, size_t,
-cmp_fn_t, int);
void *
lsearch(const void *key, void *base, size_t *nelp, size_t width,
cmp_fn_t compar)
{
+ void *element = lfind(key, base, nelp, width, compar);
- return(linear_base(key, base, nelp, width, compar, 1));
+ /*
+* Use memmove(3) to ensure the key is copied cleanly into the
+* array, even if the key overlaps with the end of the array.
+*/
+ if (element == NULL) {
+ element = memmove((char *)base + *nelp * width, key, width);
+ *nelp += 1;
+ }
+ return element;
}
void *
lfind(const void *key, const void *base, size_t *nelp, size_t width,
cmp_fn_t compar)
{
- return(linear_base(key, base, nelp, width, compar, 0));
-}
-
-static void *
-linear_base(const void *key, const void *base, size_t *nelp, size_t width,
- cmp_fn_t compar, int add_flag)
-{
const char *element, *end;
end = (const char *)base + *nelp * width;
for (element = base; element < end; element += width)
if (!compar(key, element)) /* key found */
return((void *)element);
-
- if (!add_flag) /* key not found */
- return(NULL);
-
- /*
-* The UNIX System User's Manual, 1986 edition claims that
-* a NULL pointer is returned by lsearch with errno set
-* appropriately, if there is not enough room in the table
-* to add a new item. This can't be done as none of these
-* routines have any method of determining the size of the
-* table. This comment isn't in the 1986-87 System V
-* manual.
-*/
- ++*nelp;
-
- /*
-* Use memmove(3) to ensure the key is copied cleanly into the
-* array, even if the key overlaps with the end of the array.
-*/
- memmove((void *)end, key, width);
- return((void *)end);
+ return NULL;
}
+DEF_WEAK(lfind);
Index: hidden/search.h
===
RCS file: /cvs/src/lib/libc/hidden/search.h,v
retrieving revision 1.1
diff -u -p -r1.1 search.h
--- hidden/search.h 4 Oct 2015 08:36:57 - 1.1
+++ hidden/search.h 7 Dec 2021 04:32:34 -
@@ -24,7 +24,7 @@ PROTO_DEPRECATED(hcreate);
PROTO_DEPRECATED(hdestroy);
PROTO_DEPRECATED(hsearch);
PROTO_DEPRECATED(insque);
-PROTO_DEPRECATED(lfind);
+PROTO_NORMAL(lfind);
PROTO_DEPRECATED(lsearch);
PROTO_DEPRECATED(remque);
PROTO_DEPRECATED(tdelete);
Re: com(4) at acpi(4) on amd64
On Tue, Dec 07, 2021 at 01:08:45PM +0100, Mark Kettenis wrote: > > Date: Tue, 7 Dec 2021 11:30:48 +0100 > > From: Anton Lindqvist > > > > On Mon, Dec 06, 2021 at 10:25:34PM +0100, Mark Kettenis wrote: > > > > Date: Mon, 6 Dec 2021 21:45:03 +0100 > > > > From: Anton Lindqvist > > > > > > > > On Mon, Dec 06, 2021 at 09:23:45PM +0100, Mark Kettenis wrote: > > > > > > Date: Mon, 6 Dec 2021 21:08:04 +0100 > > > > > > From: Patrick Wildt > > > > > > > > > > > > Hi, > > > > > > > > > > > > On one machine I had the pleasure of having to try and use the > > > > > > Serial-over-LAN feature which shows up as just another com(4) > > > > > > device. Instead of having to manually add a com(4) at isa(4) > > > > > > I figured it would be nicer to have them attach via ACPI. At > > > > > > least on that machine, the SOL definitely shows up in the DSDT. > > > > > > > > > > > > Since I don't want to break any legacy machines, I figured I'd > > > > > > keep ignoring the isa(4) addresses specified in amd64's GENERIC. > > > > > > > > > > > > Right now this diff is more about putting it out there, not about > > > > > > asking for OKs, as amd64 isn't really my strong suit. If people > > > > > > are interested, I can definitely put in all the feedback there is. > > > > > > > > > > > > Patrick > > > > > > > > > > anton@ has a better diff he's working on > > > > > > > > Here's the diff in its current state. There's one thing I haven't had > > > > the time to figure out yet: in order to get interrupts working on my > > > > apu2 I had to explicit pass LR_EXTIRQ_MODE (aaa_irq_flags is zero) to > > > > acpi_intr_establish() which causes IST_EDGE to be passed to > > > > intr_establish(). Worth noting is that this matches what com at isa > > > > already does. This was however not necessary on my amd64 build machine > > > > where aaa_irq_flags also is zero. > > > > > > Actually, it seems we're parsing the ACPI interrupt resource > > > descriptor wrong. The decompiled DSDTs I have here seem to use > > > IRQNoFlags() for the interrupts, which apparently implies the > > > interrupt is edge-triggered. > > > > > > Let me see if I can cook a diff. > > > > Updated diff now that kettenis@ fixed parsing of the irq flags. > > A few comments below. > > > diff --git share/man/man4/com.4 share/man/man4/com.4 > > index 73b421f2ca7..e54255fe0d6 100644 > > --- share/man/man4/com.4 > > +++ share/man/man4/com.4 > > @@ -61,11 +61,13 @@ > > .Cd "com0 at isa? port 0x3f8 irq 4" > > .Cd "com1 at isa? port 0x2f8 irq 3" > > .Pp > > +.Cd "# arm64 and amd64" > > +.Cd "com* at acpi?" > > +.Pp > > .Cd "# armv7" > > .Cd "com* at fdt?" > > .Pp > > .Cd "# arm64" > > -.Cd "com* at acpi?" > > .Cd "com* at fdt?" > > The armv7 and arm64 entries can be combined now > > > .Pp > > .Cd "# hppa" > > diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC > > index ecccd1323d9..87fcaced306 100644 > > --- sys/arch/amd64/conf/GENERIC > > +++ sys/arch/amd64/conf/GENERIC > > @@ -65,6 +65,11 @@ amdgpio* at acpi? > > aplgpio* at acpi? > > bytgpio* at acpi? > > chvgpio* at acpi? > > +com0 at acpi? > > +com1 at acpi? > > +com2 at acpi? > > +com3 at acpi? > > +com* at acpi? > > glkgpio* at acpi? > > pchgpio* at acpi? > > sdhc* at acpi? > > diff --git sys/arch/amd64/conf/RAMDISK sys/arch/amd64/conf/RAMDISK > > index 6041293b287..a5f10b357dd 100644 > > --- sys/arch/amd64/conf/RAMDISK > > +++ sys/arch/amd64/conf/RAMDISK > > @@ -34,6 +34,9 @@ acpipci* at acpi? > > acpiprt* at acpi? > > acpimadt0 at acpi? > > #acpitz* at acpi? > > +com0 at acpi? > > +com1 at acpi? > > +com* at acpi? > > > > mpbios0at bios0 > > As Theo pointed out, this may not be entirely fool-proof. We probe > acpi(4) before isa(4), which is good. Since we prevent ISA devices > from attaching to addresses that have been claimed by something else, > this means that we won't attach a com(4) to isa(4) if we have attached > an ACPI com(4) attached at the same address. But if there is no ACPI > com(4) at an address the an ISA com(4) may still attach. As long as > the ACPI serial ports are listed in canonical order and use the > standard IRQ, everything should be fine. If the ACPI com(4) ports are > not in canonical order any additional ISA com(4) may no longer attach. > For example, if there is a single ACPI com(4) at address 0x2f8, we > will now attach it as com0. But since com0 is now attached, the > isa(4) code will skip its com0 and won't probe com(4) at address > 0x3f8. I think this is fine. I expect ACPI to list all usable com(4) > ports so any ports sucessfully probed by isa(4) would be "ghost" ports > that aren't actually connected. > > The case of non-standard IRQs is also interesting. Suppose we have an > ACPI com(4) at address 0x3f8 but with the non-standard IRQ 3. We may > still end up probing an ISA com
Re: com(4) at acpi(4) on amd64
> Date: Tue, 7 Dec 2021 11:30:48 +0100 > From: Anton Lindqvist > > On Mon, Dec 06, 2021 at 10:25:34PM +0100, Mark Kettenis wrote: > > > Date: Mon, 6 Dec 2021 21:45:03 +0100 > > > From: Anton Lindqvist > > > > > > On Mon, Dec 06, 2021 at 09:23:45PM +0100, Mark Kettenis wrote: > > > > > Date: Mon, 6 Dec 2021 21:08:04 +0100 > > > > > From: Patrick Wildt > > > > > > > > > > Hi, > > > > > > > > > > On one machine I had the pleasure of having to try and use the > > > > > Serial-over-LAN feature which shows up as just another com(4) > > > > > device. Instead of having to manually add a com(4) at isa(4) > > > > > I figured it would be nicer to have them attach via ACPI. At > > > > > least on that machine, the SOL definitely shows up in the DSDT. > > > > > > > > > > Since I don't want to break any legacy machines, I figured I'd > > > > > keep ignoring the isa(4) addresses specified in amd64's GENERIC. > > > > > > > > > > Right now this diff is more about putting it out there, not about > > > > > asking for OKs, as amd64 isn't really my strong suit. If people > > > > > are interested, I can definitely put in all the feedback there is. > > > > > > > > > > Patrick > > > > > > > > anton@ has a better diff he's working on > > > > > > Here's the diff in its current state. There's one thing I haven't had > > > the time to figure out yet: in order to get interrupts working on my > > > apu2 I had to explicit pass LR_EXTIRQ_MODE (aaa_irq_flags is zero) to > > > acpi_intr_establish() which causes IST_EDGE to be passed to > > > intr_establish(). Worth noting is that this matches what com at isa > > > already does. This was however not necessary on my amd64 build machine > > > where aaa_irq_flags also is zero. > > > > Actually, it seems we're parsing the ACPI interrupt resource > > descriptor wrong. The decompiled DSDTs I have here seem to use > > IRQNoFlags() for the interrupts, which apparently implies the > > interrupt is edge-triggered. > > > > Let me see if I can cook a diff. > > Updated diff now that kettenis@ fixed parsing of the irq flags. A few comments below. > diff --git share/man/man4/com.4 share/man/man4/com.4 > index 73b421f2ca7..e54255fe0d6 100644 > --- share/man/man4/com.4 > +++ share/man/man4/com.4 > @@ -61,11 +61,13 @@ > .Cd "com0 at isa? port 0x3f8 irq 4" > .Cd "com1 at isa? port 0x2f8 irq 3" > .Pp > +.Cd "# arm64 and amd64" > +.Cd "com* at acpi?" > +.Pp > .Cd "# armv7" > .Cd "com* at fdt?" > .Pp > .Cd "# arm64" > -.Cd "com* at acpi?" > .Cd "com* at fdt?" The armv7 and arm64 entries can be combined now > .Pp > .Cd "# hppa" > diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC > index ecccd1323d9..87fcaced306 100644 > --- sys/arch/amd64/conf/GENERIC > +++ sys/arch/amd64/conf/GENERIC > @@ -65,6 +65,11 @@ amdgpio* at acpi? > aplgpio* at acpi? > bytgpio* at acpi? > chvgpio* at acpi? > +com0 at acpi? > +com1 at acpi? > +com2 at acpi? > +com3 at acpi? > +com* at acpi? > glkgpio* at acpi? > pchgpio* at acpi? > sdhc*at acpi? > diff --git sys/arch/amd64/conf/RAMDISK sys/arch/amd64/conf/RAMDISK > index 6041293b287..a5f10b357dd 100644 > --- sys/arch/amd64/conf/RAMDISK > +++ sys/arch/amd64/conf/RAMDISK > @@ -34,6 +34,9 @@ acpipci*at acpi? > acpiprt* at acpi? > acpimadt0at acpi? > #acpitz* at acpi? > +com0 at acpi? > +com1 at acpi? > +com* at acpi? > > mpbios0 at bios0 As Theo pointed out, this may not be entirely fool-proof. We probe acpi(4) before isa(4), which is good. Since we prevent ISA devices from attaching to addresses that have been claimed by something else, this means that we won't attach a com(4) to isa(4) if we have attached an ACPI com(4) attached at the same address. But if there is no ACPI com(4) at an address the an ISA com(4) may still attach. As long as the ACPI serial ports are listed in canonical order and use the standard IRQ, everything should be fine. If the ACPI com(4) ports are not in canonical order any additional ISA com(4) may no longer attach. For example, if there is a single ACPI com(4) at address 0x2f8, we will now attach it as com0. But since com0 is now attached, the isa(4) code will skip its com0 and won't probe com(4) at address 0x3f8. I think this is fine. I expect ACPI to list all usable com(4) ports so any ports sucessfully probed by isa(4) would be "ghost" ports that aren't actually connected. The case of non-standard IRQs is also interesting. Suppose we have an ACPI com(4) at address 0x3f8 but with the non-standard IRQ 3. We may still end up probing an ISA com(4) at address 0x2f8. This probe would succeed if the hardware responds to the probe. But establishing the interrupt at IRQ 3 would fail since sharing of edge-triggered interrupts isn't allowed. So the attach should fail in that case. And that should be fine. So I think this approach is actually fine. If it turns out the
Re: com(4) at acpi(4) on amd64
On Mon, Dec 06, 2021 at 10:25:34PM +0100, Mark Kettenis wrote:
> > Date: Mon, 6 Dec 2021 21:45:03 +0100
> > From: Anton Lindqvist
> >
> > On Mon, Dec 06, 2021 at 09:23:45PM +0100, Mark Kettenis wrote:
> > > > Date: Mon, 6 Dec 2021 21:08:04 +0100
> > > > From: Patrick Wildt
> > > >
> > > > Hi,
> > > >
> > > > On one machine I had the pleasure of having to try and use the
> > > > Serial-over-LAN feature which shows up as just another com(4)
> > > > device. Instead of having to manually add a com(4) at isa(4)
> > > > I figured it would be nicer to have them attach via ACPI. At
> > > > least on that machine, the SOL definitely shows up in the DSDT.
> > > >
> > > > Since I don't want to break any legacy machines, I figured I'd
> > > > keep ignoring the isa(4) addresses specified in amd64's GENERIC.
> > > >
> > > > Right now this diff is more about putting it out there, not about
> > > > asking for OKs, as amd64 isn't really my strong suit. If people
> > > > are interested, I can definitely put in all the feedback there is.
> > > >
> > > > Patrick
> > >
> > > anton@ has a better diff he's working on
> >
> > Here's the diff in its current state. There's one thing I haven't had
> > the time to figure out yet: in order to get interrupts working on my
> > apu2 I had to explicit pass LR_EXTIRQ_MODE (aaa_irq_flags is zero) to
> > acpi_intr_establish() which causes IST_EDGE to be passed to
> > intr_establish(). Worth noting is that this matches what com at isa
> > already does. This was however not necessary on my amd64 build machine
> > where aaa_irq_flags also is zero.
>
> Actually, it seems we're parsing the ACPI interrupt resource
> descriptor wrong. The decompiled DSDTs I have here seem to use
> IRQNoFlags() for the interrupts, which apparently implies the
> interrupt is edge-triggered.
>
> Let me see if I can cook a diff.
Updated diff now that kettenis@ fixed parsing of the irq flags.
diff --git share/man/man4/com.4 share/man/man4/com.4
index 73b421f2ca7..e54255fe0d6 100644
--- share/man/man4/com.4
+++ share/man/man4/com.4
@@ -61,11 +61,13 @@
.Cd "com0 at isa? port 0x3f8 irq 4"
.Cd "com1 at isa? port 0x2f8 irq 3"
.Pp
+.Cd "# arm64 and amd64"
+.Cd "com* at acpi?"
+.Pp
.Cd "# armv7"
.Cd "com* at fdt?"
.Pp
.Cd "# arm64"
-.Cd "com* at acpi?"
.Cd "com* at fdt?"
.Pp
.Cd "# hppa"
diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC
index ecccd1323d9..87fcaced306 100644
--- sys/arch/amd64/conf/GENERIC
+++ sys/arch/amd64/conf/GENERIC
@@ -65,6 +65,11 @@ amdgpio* at acpi?
aplgpio* at acpi?
bytgpio* at acpi?
chvgpio* at acpi?
+com0 at acpi?
+com1 at acpi?
+com2 at acpi?
+com3 at acpi?
+com* at acpi?
glkgpio* at acpi?
pchgpio* at acpi?
sdhc* at acpi?
diff --git sys/arch/amd64/conf/RAMDISK sys/arch/amd64/conf/RAMDISK
index 6041293b287..a5f10b357dd 100644
--- sys/arch/amd64/conf/RAMDISK
+++ sys/arch/amd64/conf/RAMDISK
@@ -34,6 +34,9 @@ acpipci* at acpi?
acpiprt* at acpi?
acpimadt0 at acpi?
#acpitz* at acpi?
+com0 at acpi?
+com1 at acpi?
+com* at acpi?
mpbios0at bios0
diff --git sys/arch/amd64/conf/RAMDISK_CD sys/arch/amd64/conf/RAMDISK_CD
index 8bd646b4ea3..d007c0102d0 100644
--- sys/arch/amd64/conf/RAMDISK_CD
+++ sys/arch/amd64/conf/RAMDISK_CD
@@ -51,6 +51,10 @@ bytgpio* at acpi?
sdhc* at acpi?
acpihve* at acpi?
chvgpio*at acpi?
+com0 at acpi?
+com1 at acpi?
+com2 at acpi?
+com* at acpi?
glkgpio* at acpi?
mpbios0at bios0
diff --git sys/dev/acpi/acpi.c sys/dev/acpi/acpi.c
index 7577424e8a2..e89869aedbd 100644
--- sys/dev/acpi/acpi.c
+++ sys/dev/acpi/acpi.c
@@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
"PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
"PNP0400", /* Standard LPT Parallel Port */
"PNP0401", /* ECP Parallel Port */
- "PNP0501", /* 16550A-compatible COM Serial Port */
"PNP0700", /* PC-class Floppy Disk Controller */
"PNP0F03", /* Microsoft PS/2-style Mouse */
"PNP0F13", /* PS/2 Mouse */
diff --git sys/dev/acpi/com_acpi.c sys/dev/acpi/com_acpi.c
index 852be6c71b3..e4001861032 100644
--- sys/dev/acpi/com_acpi.c
+++ sys/dev/acpi/com_acpi.c
@@ -49,10 +49,12 @@ struct cfattach com_acpi_ca = {
const char *com_hids[] = {
"HISI0031",
+ "PNP0501",
NULL
};
intcom_acpi_is_console(struct com_acpi_softc *);
+intcom_acpi_is_designware(const char *);
intcom_acpi_intr_designware(void *);
int
@@ -69,7 +71,7 @@ com_acpi_attach(struct device *parent, struct device *self,
void *aux)
{
struct com_acpi_softc *sc = (struct com_acpi_softc *)self;
struct acpi_attach_args *aaa = aux;
- uint32_t freq;
+ int (*intrfn)(void *) = comintr;
sc->sc
Re: dhcpleased - set ciaddr per RFC
anyone else?
On 2021-12-04 17:26 -07, Joel Knight wrote:
> We have a winner here. I tested from INIT through to REBINDING and the
> behavior matches what's in the RFC now. ok joel@
>
> One cosmetic thing I noticed this time around: log_dhcp_hdr() in
> engine.c should be printing dhcp_hdr->xid in host byte order.
>
>
>
> .joel
>
> On Fri, Dec 3, 2021 at 5:21 AM Florian Obser wrote:
>>
>> Last one, Joel spotted one more bug in the previous one, I was missing
>> an assignment to dhcp_server in the ACK case.
>>
>> This is a rewrite of the package construction logic to send the correct
>> information in the right states:
>>
>> RFC 2131 4.3.6, Table 4
>> -
>> | |REBOOTING|REQUESTING |RENEWING |REBINDING |
>> -
>> |broad/unicast |broadcast|broadcast|unicast |broadcast |
>> |server-ip |MUST NOT |MUST |MUST NOT |MUST NOT |
>> |requested-ip |MUST |MUST |MUST NOT |MUST NOT |
>> |ciaddr|zero |zero |IP address |IP address|
>> -
>>
>> Previous this logic was all over the place and difficult to check.
>> This diff moves it all into the engine.c functions
>> request_dhcp_discover() and request_dhcp_request().
>>
>> The frontend then leaves out dhcp options that are set to INADDR_ARPA,
>> but it does not need to know in which state the engine is.
>>
>> OK?
>>
>>
>> diff --git dhcpleased.h dhcpleased.h
>> index b3b4938ad3c..6df38be6f5b 100644
>> --- dhcpleased.h
>> +++ dhcpleased.h
>> @@ -287,15 +287,10 @@ struct imsg_dhcp {
>> uint8_t packet[1500];
>> };
>>
>> -
>> -struct imsg_req_discover {
>> - uint32_tif_index;
>> - uint32_txid;
>> -};
>> -
>> -struct imsg_req_request {
>> +struct imsg_req_dhcp {
>> uint32_tif_index;
>> uint32_txid;
>> + struct in_addr ciaddr;
>> struct in_addr requested_ip;
>> struct in_addr server_identifier;
>> struct in_addr dhcp_server;
>> diff --git engine.c engine.c
>> index 17e65fbe789..3169bd80d40 100644
>> --- engine.c
>> +++ engine.c
>> @@ -1170,6 +1170,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
>> imsg_dhcp *dhcp)
>> return;
>> }
>> iface->server_identifier.s_addr = server_identifier.s_addr;
>> + iface->dhcp_server.s_addr = server_identifier.s_addr;
>> iface->requested_ip.s_addr = dhcp_hdr->yiaddr.s_addr;
>> state_transition(iface, IF_REQUESTING);
>> break;
>> @@ -1228,6 +1229,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
>> imsg_dhcp *dhcp)
>>
>> clock_gettime(CLOCK_MONOTONIC, &iface->request_time);
>> iface->server_identifier.s_addr = server_identifier.s_addr;
>> + iface->dhcp_server.s_addr = server_identifier.s_addr;
>> iface->requested_ip.s_addr = dhcp_hdr->yiaddr.s_addr;
>> iface->mask.s_addr = subnet_mask.s_addr;
>> #ifndef SMALL
>> @@ -1354,11 +1356,8 @@ state_transition(struct dhcpleased_iface *iface, enum
>> if_state new_state)
>> case IF_REBOOTING:
>> if (old_state == IF_REBOOTING)
>> iface->timo.tv_sec *= 2;
>> - else {
>> - /* make sure we send broadcast */
>> - iface->dhcp_server.s_addr = INADDR_ANY;
>> + else
>> iface->timo.tv_sec = START_EXP_BACKOFF;
>> - }
>> request_dhcp_request(iface);
>> break;
>> case IF_REQUESTING:
>> @@ -1377,10 +1376,6 @@ state_transition(struct dhcpleased_iface *iface, enum
>> if_state new_state)
>> break;
>> case IF_RENEWING:
>> if (old_state == IF_BOUND) {
>> - iface->dhcp_server.s_addr =
>> - iface->server_identifier.s_addr;
>> - iface->server_identifier.s_addr = INADDR_ANY;
>> -
>> iface->timo.tv_sec = (iface->rebinding_time -
>> iface->renewal_time) / 2; /* RFC 2131 4.4.5 */
>> } else
>> @@ -1392,7 +1387,6 @@ state_transition(struct dhcpleased_iface *iface, enum
>> if_state new_state)
>> break;
>> case IF_REBINDING:
>> if (old_state == IF_RENEWING) {
>> - iface->dhcp_server.s_addr = INADDR_ANY;
>> iface->timo.tv_sec = (iface->lease_time -
>> iface->rebinding_time) / 2; /* RFC 2131 4.4.5 */
>> } else
>> @@ -1470,28 +1464,90 @@ iface_timeout(int
