Re: git: f4e35c044c89 - main - bus: Set the current VNET in device_attach()
On Sat, Oct 19, 2024 at 11:36:32AM -0400, John Baldwin wrote: > On 10/19/24 09:04, Mark Johnston wrote: > > The branch main has been updated by markj: > > > > URL: > > https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021 > > > > commit f4e35c044c8988b7452cefbdcc417f5fd723e021 > > Author: Mark Johnston > > AuthorDate: 2024-10-19 13:03:56 + > > Commit: Mark Johnston > > CommitDate: 2024-10-19 13:03:56 + > > > > bus: Set the current VNET in device_attach() > > Some drivers, in particular anything which creates an ifnet during > > attach, need to have the current VNET set, as if_attach_internal() and > > its callees access VNET-global variables. > > device_probe_and_attach() handles this, but this is not the only way to > > arrive in DEVICE_ATTACH. In particular, bus drivers may invoke > > device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler. > > So, set the current VNET in device_attach() instead. > > I believe it is always safe to use vnet0, as devctl2 ioctls are not > > permitted within a jail. > > PR: 282168 > > Reviewed by:zlei, kevans, bz, imp, glebius > > MFC after: 1 week > > Differential Revision: https://reviews.freebsd.org/D47174 > > Hmm, there was some other review I thought that had a completely different > change. > That change removed all the vnet stuff from new-bus and instead handled it in > if.c. Specifically, that if_attach would set a default vnet to vnet0 if there > wasn't an active vnet at the time. See all the discussion in > https://reviews.freebsd.org/D42678 which has a patch that I think is correct > in the comments. > In fact, I think that bus level is better. At least, I know that detach also might need something by vnet (e.g. mce(4) needs to clear the IPSEC offload database on detach, although it still does not do). It sounds as if bus_topo_lock() is the right place. May be some other name for it is better, like bus_topo_changes_enter().
Re: git: f4e35c044c89 - main - bus: Set the current VNET in device_attach()
On 10/19/24 09:04, Mark Johnston wrote: The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021 commit f4e35c044c8988b7452cefbdcc417f5fd723e021 Author: Mark Johnston AuthorDate: 2024-10-19 13:03:56 + Commit: Mark Johnston CommitDate: 2024-10-19 13:03:56 + bus: Set the current VNET in device_attach() Some drivers, in particular anything which creates an ifnet during attach, need to have the current VNET set, as if_attach_internal() and its callees access VNET-global variables. device_probe_and_attach() handles this, but this is not the only way to arrive in DEVICE_ATTACH. In particular, bus drivers may invoke device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler. So, set the current VNET in device_attach() instead. I believe it is always safe to use vnet0, as devctl2 ioctls are not permitted within a jail. PR: 282168 Reviewed by:zlei, kevans, bz, imp, glebius MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D47174 Hmm, there was some other review I thought that had a completely different change. That change removed all the vnet stuff from new-bus and instead handled it in if.c. Specifically, that if_attach would set a default vnet to vnet0 if there wasn't an active vnet at the time. See all the discussion in https://reviews.freebsd.org/D42678 which has a patch that I think is correct in the comments. -- John Baldwin
git: ef48b2954e24 - main - stand: Fix defaults file
The branch main has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=ef48b2954e240312e489a06435ac66ed12694a7c commit ef48b2954e240312e489a06435ac66ed12694a7c Author: Warner Losh AuthorDate: 2024-10-19 18:53:41 + Commit: Warner Losh CommitDate: 2024-10-19 18:53:41 + stand: Fix defaults file Currently, quotes in a comment lead to mal-formed lines warnings. Remove the quotes since it doesn't matter. The underlying bug likely should be fixed, but since stability week is neigh, workaround this. Fixes: 7df3e400ea687 Sponsored by: Netflix Reviewed by:bcran Differential Revision: https://reviews.freebsd.org/D47201 --- stand/defaults/loader.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stand/defaults/loader.conf b/stand/defaults/loader.conf index 8678c5c50d4f..d266c240955a 100644 --- a/stand/defaults/loader.conf +++ b/stand/defaults/loader.conf @@ -27,7 +27,7 @@ vesa_load="NO"# Set this to YES to load the vesa module bitmap_load="NO" # Set this to YES if you want splash screen! bitmap_name="splash.bmp" # Set this to the name of the file bitmap_type="splash_image_data" # and place it on the module_path -splash="/boot/images/freebsd-logo-rev.png" # Set boot_mute="YES" to load it +splash="/boot/images/freebsd-logo-rev.png" # Set boot_mute=YES to load it ### Screen saver modules ### # This is best done in rc.conf
git: 92cd5abb64dd - main - membarrier: man page improvements
The branch main has been updated by emaste:
URL:
https://cgit.FreeBSD.org/src/commit/?id=92cd5abb64dd70c305535c9504c6a2b73552147f
commit 92cd5abb64dd70c305535c9504c6a2b73552147f
Author: Ed Maste
AuthorDate: 2024-10-09 18:50:07 +
Commit: Ed Maste
CommitDate: 2024-10-19 20:18:18 +
membarrier: man page improvements
Reported by:fernape (in D46967)
Fixes: 1fc766e3b41d ("membarrier: Add manual page")
Sponsored by: The FreeBSD Foundation
---
lib/libsys/membarrier.2 | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lib/libsys/membarrier.2 b/lib/libsys/membarrier.2
index fe2e6ff763d5..87db620975ef 100644
--- a/lib/libsys/membarrier.2
+++ b/lib/libsys/membarrier.2
@@ -60,7 +60,7 @@ This is an alias for
.It Dv MEMBARRIER_CMD_GLOBAL_EXPEDITED
Execute a memory barrier on all running threads of all processes registered
with
-.Dv MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED
+.Dv MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED .
.It Dv MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED
Register the process to receive
.Dv MEMBARRIER_CMD_GLOBAL_EXPEDITED
@@ -102,8 +102,9 @@ The
argument is ignored.
.Sh RETURN VALUES
If the
-.Dv cmd
-is MEMBARRIER_CMD_QUERY
+.Fa cmd
+is
+.Dv MEMBARRIER_CMD_QUERY
a bitmask of supported commands is returned.
Otherwise, on success,
.Nm
Re: git: f4e35c044c89 - main - bus: Set the current VNET in device_attach()
On Sat, 19 Oct 2024, Mark Johnston wrote:
On Sat, Oct 19, 2024 at 11:50:40PM +, Bjoern A. Zeeb wrote:
On Sat, 19 Oct 2024, Mark Johnston wrote:
On Sat, Oct 19, 2024 at 07:10:53PM +0300, Konstantin Belousov wrote:
On Sat, Oct 19, 2024 at 11:36:32AM -0400, John Baldwin wrote:
On 10/19/24 09:04, Mark Johnston wrote:
The branch main has been updated by markj:
URL:
https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021
commit f4e35c044c8988b7452cefbdcc417f5fd723e021
Author: Mark Johnston
AuthorDate: 2024-10-19 13:03:56 +
Commit: Mark Johnston
CommitDate: 2024-10-19 13:03:56 +
bus: Set the current VNET in device_attach()
Some drivers, in particular anything which creates an ifnet during
attach, need to have the current VNET set, as if_attach_internal() and
its callees access VNET-global variables.
device_probe_and_attach() handles this, but this is not the only way to
arrive in DEVICE_ATTACH. In particular, bus drivers may invoke
device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler.
So, set the current VNET in device_attach() instead.
I believe it is always safe to use vnet0, as devctl2 ioctls are not
permitted within a jail.
PR: 282168
Reviewed by:zlei, kevans, bz, imp, glebius
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D47174
Hmm, there was some other review I thought that had a completely different
change.
That change removed all the vnet stuff from new-bus and instead handled it in
if.c. Specifically, that if_attach would set a default vnet to vnet0 if there
wasn't an active vnet at the time. See all the discussion in
https://reviews.freebsd.org/D42678 which has a patch that I think is correct
in the comments.
There it was; thanks I didn't misremeber but couldn't find it.
Gleb's proposal, described a bit in D47147, is to require device-based
ifnet drivers to fully detach themselves from the parent bus in order to
change VNETs. The intent is to eliminate the need for if_vmove() and
all the complexity it entails. This would also eliminate the need for a
"home" VNET, referenced in the patch that you reference here.
Will it?
I asked for a discussion elsewhere but it seems we are having it here now...
I'm responding to John's question and Kostik's follow-up, nothing else.
The inline patch in D42678 seems fine, I don't have strong feelings
about it, but I believe it is not sufficient to fix the PR in question
(it still assumes that the current vnet is already set).
I am still inclined to ask:
- how do you want a vnet to attach an unknown to itself device? From
the outside?
- How to you pass it to a child-vnet without escalating priviledges to
outside of the host system (vnet0)?
- Is, e.g., a vcc device [CXGBE(4)] a physical interface?
- How do you pass a controlled set of other non-clonable devices in (or
did we get rid of them all)? The inital idea was still that the
"host" has somehow control over what child can create...
{ I recently tried tuntap in a vnet and it blew up badly by not going
away }
- exmaple: I really would love, e.g., a vlan interface to be passed to a
vnet but but not the pyhsical interface. Can we?
- How will we do with wlan interfaces (which are cloned) but may not own
the hardware (kind-of similar to the vcc example)? I know there are
special PRIV checks for those currently.
- how does a detach in a vnet work and where will the physical interface
re-appear for (automatic) attachment? just detached in that jail?
vnet0? the parent jail?
- what happens on vnet destroy? (same as last question)?
- Are we just going to build a vmove on a layer which doesn't have
anything to do with networking per-se as a special case for some
interfaces but not others?
These are excellent questions which should be posed to Gleb when his
proposal is fleshed out. In the meantime, I only aimed to fix an
obvious shortcoming of an existing hack which has been around for over
10 years.
ACK & ACK
--
Bjoern A. Zeeb r15:7
git: 6d42d5dbdd67 - main - vm_glue: use vm_page_alloc_domain_after
The branch main has been updated by dougm:
URL:
https://cgit.FreeBSD.org/src/commit/?id=6d42d5dbdd677c3422bdb3867770639f48c6df7a
commit 6d42d5dbdd677c3422bdb3867770639f48c6df7a
Author: Doug Moore
AuthorDate: 2024-10-19 20:22:20 +
Commit: Doug Moore
CommitDate: 2024-10-19 20:22:20 +
vm_glue: use vm_page_alloc_domain_after
Drop the function vm_page_alloc_domain, used only in
vm_thread_stack_back, and replace it with vm_page_alloc_domain_after
there, with the extra mpred argument either computed on the first
iteration or retrieved from previous iterations. Define a function
vm_page_mpred() for computing that first mpred argument.
Reviewed by:bnovkov
Differential Revision: https://reviews.freebsd.org/D47054
---
sys/vm/vm_glue.c | 5 +++--
sys/vm/vm_page.c | 27 +++
sys/vm/vm_page.h | 2 +-
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/sys/vm/vm_glue.c b/sys/vm/vm_glue.c
index 5128c46a1d9f..0090904785ab 100644
--- a/sys/vm/vm_glue.c
+++ b/sys/vm/vm_glue.c
@@ -623,8 +623,9 @@ vm_thread_stack_back(vm_offset_t ks, vm_page_t ma[], int
npages, int req_class,
m = vm_page_grab(obj, pindex + n,
VM_ALLOC_NOCREAT | VM_ALLOC_WIRED);
if (m == NULL) {
- m = vm_page_alloc_domain(obj, pindex + n, domain,
- req_class | VM_ALLOC_WIRED);
+ m = n > 0 ? ma[n - 1] : vm_page_mpred(obj, pindex);
+ m = vm_page_alloc_domain_after(obj, pindex + n, domain,
+ req_class | VM_ALLOC_WIRED, m);
}
if (m == NULL)
break;
diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c
index f2b3baf419a0..054832e3f19a 100644
--- a/sys/vm/vm_page.c
+++ b/sys/vm/vm_page.c
@@ -2013,6 +2013,18 @@ vm_page_rename(vm_page_t m, vm_object_t new_object,
vm_pindex_t new_pindex)
return (0);
}
+/*
+ * vm_page_mpred:
+ *
+ * Return the greatest page of the object with index <= pindex,
+ * or NULL, if there is none. Assumes object lock is held.
+ */
+vm_page_t
+vm_page_mpred(vm_object_t object, vm_pindex_t pindex)
+{
+ return (vm_radix_lookup_le(&object->rtree, pindex));
+}
+
/*
* vm_page_alloc:
*
@@ -2040,16 +2052,7 @@ vm_page_alloc(vm_object_t object, vm_pindex_t pindex,
int req)
{
return (vm_page_alloc_after(object, pindex, req,
- vm_radix_lookup_le(&object->rtree, pindex)));
-}
-
-vm_page_t
-vm_page_alloc_domain(vm_object_t object, vm_pindex_t pindex, int domain,
-int req)
-{
-
- return (vm_page_alloc_domain_after(object, pindex, domain, req,
- vm_radix_lookup_le(&object->rtree, pindex)));
+ vm_page_mpred(object, pindex)));
}
/*
@@ -2390,7 +2393,7 @@ vm_page_alloc_contig_domain(vm_object_t object,
vm_pindex_t pindex, int domain,
object));
KASSERT(npages > 0, ("vm_page_alloc_contig: npages is zero"));
- mpred = vm_radix_lookup_le(&object->rtree, pindex);
+ mpred = vm_page_mpred(object, pindex);
KASSERT(mpred == NULL || mpred->pindex != pindex,
("vm_page_alloc_contig: pindex already allocated"));
for (;;) {
@@ -5073,7 +5076,7 @@ vm_page_grab_pages(vm_object_t object, vm_pindex_t
pindex, int allocflags,
pflags = vm_page_grab_pflags(allocflags);
i = 0;
retrylookup:
- m = vm_radix_lookup_le(&object->rtree, pindex + i);
+ m = vm_page_mpred(object, pindex + i);
if (m == NULL || m->pindex != pindex + i) {
mpred = m;
m = NULL;
diff --git a/sys/vm/vm_page.h b/sys/vm/vm_page.h
index c7c1ec3872a8..b85342b784de 100644
--- a/sys/vm/vm_page.h
+++ b/sys/vm/vm_page.h
@@ -606,8 +606,8 @@ void vm_page_free_zero(vm_page_t m);
void vm_page_activate (vm_page_t);
void vm_page_advise(vm_page_t m, int advice);
+vm_page_t vm_page_mpred(vm_object_t, vm_pindex_t);
vm_page_t vm_page_alloc(vm_object_t, vm_pindex_t, int);
-vm_page_t vm_page_alloc_domain(vm_object_t, vm_pindex_t, int, int);
vm_page_t vm_page_alloc_after(vm_object_t, vm_pindex_t, int, vm_page_t);
vm_page_t vm_page_alloc_domain_after(vm_object_t, vm_pindex_t, int, int,
vm_page_t);
git: f4e35c044c89 - main - bus: Set the current VNET in device_attach()
The branch main has been updated by markj:
URL:
https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021
commit f4e35c044c8988b7452cefbdcc417f5fd723e021
Author: Mark Johnston
AuthorDate: 2024-10-19 13:03:56 +
Commit: Mark Johnston
CommitDate: 2024-10-19 13:03:56 +
bus: Set the current VNET in device_attach()
Some drivers, in particular anything which creates an ifnet during
attach, need to have the current VNET set, as if_attach_internal() and
its callees access VNET-global variables.
device_probe_and_attach() handles this, but this is not the only way to
arrive in DEVICE_ATTACH. In particular, bus drivers may invoke
device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler.
So, set the current VNET in device_attach() instead.
I believe it is always safe to use vnet0, as devctl2 ioctls are not
permitted within a jail.
PR: 282168
Reviewed by:zlei, kevans, bz, imp, glebius
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D47174
---
sys/kern/subr_bus.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 70e5e22854f3..eeba75ac574a 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -2538,10 +2539,7 @@ device_probe_and_attach(device_t dev)
else if (error != 0)
return (error);
- CURVNET_SET_QUIET(vnet0);
- error = device_attach(dev);
- CURVNET_RESTORE();
- return error;
+ return (device_attach(dev));
}
/**
@@ -2577,6 +2575,10 @@ device_attach(device_t dev)
return (ENXIO);
}
+ KASSERT(IS_DEFAULT_VNET(TD_TO_VNET(curthread)),
+ ("device_attach: curthread is not in default vnet"));
+ CURVNET_SET_QUIET(TD_TO_VNET(curthread));
+
device_sysctl_init(dev);
if (!device_is_quiet(dev))
device_print_child(dev->parent, dev);
@@ -2609,8 +2611,10 @@ device_attach(device_t dev)
KASSERT(dev->busy == 0, ("attach failed but busy"));
dev->state = DS_NOTPRESENT;
}
+ CURVNET_RESTORE();
return (error);
}
+ CURVNET_RESTORE();
dev->flags |= DF_ATTACHED_ONCE;
/*
* We only need the low bits of this time, but ranges from tens to
thousands
Re: git: 4f2ca36c7bec - main - arm64: Disable coverage sanitization of `pmap_update_strided`
On Fri, Oct 18, 2024 at 10:01:18PM +0100, Jessica Clarke wrote: > On 11 Oct 2024, at 17:53, Mark Johnston wrote: > > > > The branch main has been updated by markj: > > > > URL: > > https://cgit.FreeBSD.org/src/commit/?id=4f2ca36c7becd20b21ce5ef7256bbd42f732dafe > > > > commit 4f2ca36c7becd20b21ce5ef7256bbd42f732dafe > > Author: Zhuo Ying Jiang Li > > AuthorDate: 2024-10-11 16:41:49 + > > Commit: Mark Johnston > > CommitDate: 2024-10-11 16:52:53 + > > > >arm64: Disable coverage sanitization of `pmap_update_strided` > > > >The break-before-make update invalidates PTEs, including the PTE > >pointing to curthread, causing a fault in `trace_pc`. This > >addresses a similar issue in > >01bb9a2a3557bc9389f628d301cd691e08266f1d. > > This broke* GitHub Actions due to the use of Clang 12 (newest Clang in > Ubuntu 20.04 LTS which isn’t EOL until April 2025) which doesn’t > support this sanitiser. Probably the easiest thing to do is to leave > the relevant __nosanitizefoo defined to nothing unless that sanitiser > is actually enabled (via __has_feature), as it’s not necessary outside > of that, and if it’s enabled then you know it’s supported? This seems to work, in that I reproduced the build failure locally with CROSS_TOOLCHAIN=llvm12 and then verified this patch: https://reviews.freebsd.org/D47193 > Jess > > * Masked by the OpenZFS import breaking it earlier in both commit > history and build order >
git: 6e24259590a3 - main - RELNOTES: Add mididump(1)
The branch main has been updated by christos: URL: https://cgit.FreeBSD.org/src/commit/?id=6e24259590a3c3b7784c5c656f6be1bee72d0bfe commit 6e24259590a3c3b7784c5c656f6be1bee72d0bfe Author: Christos Margiolis AuthorDate: 2024-10-19 12:05:03 + Commit: Christos Margiolis CommitDate: 2024-10-19 14:29:33 + RELNOTES: Add mididump(1) Sponsored by: The FreeBSD Foundation --- RELNOTES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELNOTES b/RELNOTES index a6f27df32d87..71dea5561ed6 100644 --- a/RELNOTES +++ b/RELNOTES @@ -10,6 +10,9 @@ newline. Entries should be separated by a newline. Changes to this file should not be MFCed. +f57efe95cc25: + New mididump(1) utility which dumps MIDI 1.0 events in real time. + ddfc6f84f242: Update unicode to 16.0.0 and CLDR to 45.0.0.
git: 53314e34d5e8 - main - mididump(1): Use nitems()
The branch main has been updated by christos:
URL:
https://cgit.FreeBSD.org/src/commit/?id=53314e34d5e8e7f781ab990805b22f7a56bc0580
commit 53314e34d5e8e7f781ab990805b22f7a56bc0580
Author: Christos Margiolis
AuthorDate: 2024-10-19 14:34:27 +
Commit: Christos Margiolis
CommitDate: 2024-10-19 14:34:27 +
mididump(1): Use nitems()
Reported by:kevans
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by:markj, emaste
Differential Revision: https://reviews.freebsd.org/D47191
---
usr.bin/mididump/mididump.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/usr.bin/mididump/mididump.c b/usr.bin/mididump/mididump.c
index 16bd775f10d4..8ebcce547ac4 100644
--- a/usr.bin/mididump/mididump.c
+++ b/usr.bin/mididump/mididump.c
@@ -28,6 +28,7 @@
* SUCH DAMAGE.
*/
+#include
#include
#include
@@ -39,7 +40,6 @@
#include
#include
-#define ARRLEN(x) (sizeof(x) / sizeof(x[0]))
#define NOTE2OCTAVE(n) (n / 12 - 1)
#define NOTE2FREQ(n) (440 * pow(2.0f, ((float)n - 69) / 12))
#define CHAN_MASK 0x0f
@@ -189,7 +189,7 @@ main(int argc, char *argv[])
case 0x90 ... 0x9f:
b1 = read_byte(fd);
b2 = read_byte(fd);
- pn = ¬es[b1 % ARRLEN(notes)];
+ pn = ¬es[b1 % nitems(notes)];
snprintf(buf, sizeof(buf), "%s%d", pn->name,
NOTE2OCTAVE(b1));
if (pn->alt != NULL) {
@@ -211,7 +211,7 @@ main(int argc, char *argv[])
case 0xb0 ... 0xbf:
b1 = read_byte(fd);
b2 = read_byte(fd);
- if (b1 > ARRLEN(ctls) - 1)
+ if (b1 > nitems(ctls) - 1)
break;
printf("Control/Mode change channel=%d, "
"control=%d (%s), value=%d",
git: feb9ba2993cf - main - mididump(1): Use LIBADD instead of LDFLAGS
The branch main has been updated by christos:
URL:
https://cgit.FreeBSD.org/src/commit/?id=feb9ba2993cf6aefa49b7b17ca49c52210c26035
commit feb9ba2993cf6aefa49b7b17ca49c52210c26035
Author: Christos Margiolis
AuthorDate: 2024-10-19 14:34:20 +
Commit: Christos Margiolis
CommitDate: 2024-10-19 14:34:20 +
mididump(1): Use LIBADD instead of LDFLAGS
Reported by:kevans
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by:markj, emaste
Differential Revision: https://reviews.freebsd.org/D47190
---
usr.bin/mididump/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/usr.bin/mididump/Makefile b/usr.bin/mididump/Makefile
index f3b273172b1e..758bbb3a1189 100644
--- a/usr.bin/mididump/Makefile
+++ b/usr.bin/mididump/Makefile
@@ -3,6 +3,6 @@
PROG= mididump
SRCS= ${PROG}.c
MAN= ${PROG}.1
-LDFLAGS+= -lm
+LIBADD+= m
.include
Re: git: f4e35c044c89 - main - bus: Set the current VNET in device_attach()
On Sat, 19 Oct 2024, Mark Johnston wrote:
On Sat, Oct 19, 2024 at 07:10:53PM +0300, Konstantin Belousov wrote:
On Sat, Oct 19, 2024 at 11:36:32AM -0400, John Baldwin wrote:
On 10/19/24 09:04, Mark Johnston wrote:
The branch main has been updated by markj:
URL:
https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021
commit f4e35c044c8988b7452cefbdcc417f5fd723e021
Author: Mark Johnston
AuthorDate: 2024-10-19 13:03:56 +
Commit: Mark Johnston
CommitDate: 2024-10-19 13:03:56 +
bus: Set the current VNET in device_attach()
Some drivers, in particular anything which creates an ifnet during
attach, need to have the current VNET set, as if_attach_internal() and
its callees access VNET-global variables.
device_probe_and_attach() handles this, but this is not the only way to
arrive in DEVICE_ATTACH. In particular, bus drivers may invoke
device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler.
So, set the current VNET in device_attach() instead.
I believe it is always safe to use vnet0, as devctl2 ioctls are not
permitted within a jail.
PR: 282168
Reviewed by:zlei, kevans, bz, imp, glebius
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D47174
Hmm, there was some other review I thought that had a completely different
change.
That change removed all the vnet stuff from new-bus and instead handled it in
if.c. Specifically, that if_attach would set a default vnet to vnet0 if there
wasn't an active vnet at the time. See all the discussion in
https://reviews.freebsd.org/D42678 which has a patch that I think is correct
in the comments.
There it was; thanks I didn't misremeber but couldn't find it.
Gleb's proposal, described a bit in D47147, is to require device-based
ifnet drivers to fully detach themselves from the parent bus in order to
change VNETs. The intent is to eliminate the need for if_vmove() and
all the complexity it entails. This would also eliminate the need for a
"home" VNET, referenced in the patch that you reference here.
Will it?
I asked for a discussion elsewhere but it seems we are having it here now...
I am still inclined to ask:
- how do you want a vnet to attach an unknown to itself device? From
the outside?
- How to you pass it to a child-vnet without escalating priviledges to
outside of the host system (vnet0)?
- Is, e.g., a vcc device [CXGBE(4)] a physical interface?
- How do you pass a controlled set of other non-clonable devices in (or
did we get rid of them all)? The inital idea was still that the
"host" has somehow control over what child can create...
{ I recently tried tuntap in a vnet and it blew up badly by not going
away }
- exmaple: I really would love, e.g., a vlan interface to be passed to a
vnet but but not the pyhsical interface. Can we?
- How will we do with wlan interfaces (which are cloned) but may not own
the hardware (kind-of similar to the vcc example)? I know there are
special PRIV checks for those currently.
- how does a detach in a vnet work and where will the physical interface
re-appear for (automatic) attachment? just detached in that jail?
vnet0? the parent jail?
- what happens on vnet destroy? (same as last question)?
- Are we just going to build a vmove on a layer which doesn't have
anything to do with networking per-se as a special case for some
interfaces but not others?
In fact, I think that bus level is better. At least, I know that detach
also might need something by vnet (e.g. mce(4) needs to clear the IPSEC
offload database on detach, although it still does not do).
Shouldn't something like this be handled by
if_detach()/ether_ifdetach()? Posed another way, why does device detach
itself need to care about the VNET?
IPSec should probably be handled in the per-AF specific data of the
interface and be cleared up automatically. If that needs downcalls into
HW that's likely where they belong. It has nothing to do with ether(4);
wrong layer.
I tend to agree that having VNET knowledge in subr_bus.c is a hack. My
aim was just to fix the panic without making the hack worse.
It sounds as if bus_topo_lock() is the right place. May be some other
name for it is better, like bus_topo_changes_enter().
--
Bjoern A. Zeeb r15:7
Re: git: f4e35c044c89 - main - bus: Set the current VNET in device_attach()
On Sat, Oct 19, 2024 at 07:10:53PM +0300, Konstantin Belousov wrote: > On Sat, Oct 19, 2024 at 11:36:32AM -0400, John Baldwin wrote: > > On 10/19/24 09:04, Mark Johnston wrote: > > > The branch main has been updated by markj: > > > > > > URL: > > > https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021 > > > > > > commit f4e35c044c8988b7452cefbdcc417f5fd723e021 > > > Author: Mark Johnston > > > AuthorDate: 2024-10-19 13:03:56 + > > > Commit: Mark Johnston > > > CommitDate: 2024-10-19 13:03:56 + > > > > > > bus: Set the current VNET in device_attach() > > > Some drivers, in particular anything which creates an ifnet during > > > attach, need to have the current VNET set, as if_attach_internal() > > > and > > > its callees access VNET-global variables. > > > device_probe_and_attach() handles this, but this is not the only way > > > to > > > arrive in DEVICE_ATTACH. In particular, bus drivers may invoke > > > device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler. > > > So, set the current VNET in device_attach() instead. > > > I believe it is always safe to use vnet0, as devctl2 ioctls are not > > > permitted within a jail. > > > PR: 282168 > > > Reviewed by:zlei, kevans, bz, imp, glebius > > > MFC after: 1 week > > > Differential Revision: https://reviews.freebsd.org/D47174 > > > > Hmm, there was some other review I thought that had a completely different > > change. > > That change removed all the vnet stuff from new-bus and instead handled it > > in > > if.c. Specifically, that if_attach would set a default vnet to vnet0 if > > there > > wasn't an active vnet at the time. See all the discussion in > > https://reviews.freebsd.org/D42678 which has a patch that I think is correct > > in the comments. Gleb's proposal, described a bit in D47147, is to require device-based ifnet drivers to fully detach themselves from the parent bus in order to change VNETs. The intent is to eliminate the need for if_vmove() and all the complexity it entails. This would also eliminate the need for a "home" VNET, referenced in the patch that you reference here. > In fact, I think that bus level is better. At least, I know that detach > also might need something by vnet (e.g. mce(4) needs to clear the IPSEC > offload database on detach, although it still does not do). Shouldn't something like this be handled by if_detach()/ether_ifdetach()? Posed another way, why does device detach itself need to care about the VNET? I tend to agree that having VNET knowledge in subr_bus.c is a hack. My aim was just to fix the panic without making the hack worse. > It sounds as if bus_topo_lock() is the right place. May be some other > name for it is better, like bus_topo_changes_enter().
Re: git: f4e35c044c89 - main - bus: Set the current VNET in device_attach()
On Sat, Oct 19, 2024 at 11:50:40PM +, Bjoern A. Zeeb wrote:
> On Sat, 19 Oct 2024, Mark Johnston wrote:
>
> > On Sat, Oct 19, 2024 at 07:10:53PM +0300, Konstantin Belousov wrote:
> > > On Sat, Oct 19, 2024 at 11:36:32AM -0400, John Baldwin wrote:
> > > > On 10/19/24 09:04, Mark Johnston wrote:
> > > > > The branch main has been updated by markj:
> > > > >
> > > > > URL:
> > > > > https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021
> > > > >
> > > > > commit f4e35c044c8988b7452cefbdcc417f5fd723e021
> > > > > Author: Mark Johnston
> > > > > AuthorDate: 2024-10-19 13:03:56 +
> > > > > Commit: Mark Johnston
> > > > > CommitDate: 2024-10-19 13:03:56 +
> > > > >
> > > > > bus: Set the current VNET in device_attach()
> > > > > Some drivers, in particular anything which creates an ifnet
> > > > > during
> > > > > attach, need to have the current VNET set, as
> > > > > if_attach_internal() and
> > > > > its callees access VNET-global variables.
> > > > > device_probe_and_attach() handles this, but this is not the only
> > > > > way to
> > > > > arrive in DEVICE_ATTACH. In particular, bus drivers may invoke
> > > > > device_attach() directly, as does devctl2's DEV_ENABLE ioctl
> > > > > handler.
> > > > > So, set the current VNET in device_attach() instead.
> > > > > I believe it is always safe to use vnet0, as devctl2 ioctls are
> > > > > not
> > > > > permitted within a jail.
> > > > > PR: 282168
> > > > > Reviewed by:zlei, kevans, bz, imp, glebius
> > > > > MFC after: 1 week
> > > > > Differential Revision: https://reviews.freebsd.org/D47174
> > > >
> > > > Hmm, there was some other review I thought that had a completely
> > > > different change.
> > > > That change removed all the vnet stuff from new-bus and instead handled
> > > > it in
> > > > if.c. Specifically, that if_attach would set a default vnet to vnet0
> > > > if there
> > > > wasn't an active vnet at the time. See all the discussion in
> > > > https://reviews.freebsd.org/D42678 which has a patch that I think is
> > > > correct
> > > > in the comments.
>
> There it was; thanks I didn't misremeber but couldn't find it.
>
>
> > Gleb's proposal, described a bit in D47147, is to require device-based
> > ifnet drivers to fully detach themselves from the parent bus in order to
> > change VNETs. The intent is to eliminate the need for if_vmove() and
> > all the complexity it entails. This would also eliminate the need for a
> > "home" VNET, referenced in the patch that you reference here.
>
> Will it?
>
> I asked for a discussion elsewhere but it seems we are having it here now...
I'm responding to John's question and Kostik's follow-up, nothing else.
The inline patch in D42678 seems fine, I don't have strong feelings
about it, but I believe it is not sufficient to fix the PR in question
(it still assumes that the current vnet is already set).
> I am still inclined to ask:
> - how do you want a vnet to attach an unknown to itself device? From
> the outside?
> - How to you pass it to a child-vnet without escalating priviledges to
> outside of the host system (vnet0)?
> - Is, e.g., a vcc device [CXGBE(4)] a physical interface?
> - How do you pass a controlled set of other non-clonable devices in (or
> did we get rid of them all)? The inital idea was still that the
> "host" has somehow control over what child can create...
> { I recently tried tuntap in a vnet and it blew up badly by not going
> away }
> - exmaple: I really would love, e.g., a vlan interface to be passed to a
> vnet but but not the pyhsical interface. Can we?
> - How will we do with wlan interfaces (which are cloned) but may not own
> the hardware (kind-of similar to the vcc example)? I know there are
> special PRIV checks for those currently.
> - how does a detach in a vnet work and where will the physical interface
> re-appear for (automatic) attachment? just detached in that jail?
> vnet0? the parent jail?
> - what happens on vnet destroy? (same as last question)?
> - Are we just going to build a vmove on a layer which doesn't have
> anything to do with networking per-se as a special case for some
> interfaces but not others?
These are excellent questions which should be posed to Gleb when his
proposal is fleshed out. In the meantime, I only aimed to fix an
obvious shortcoming of an existing hack which has been around for over
10 years.
git: 0077477f215c - main - tests/sys/fs/fusefs: include iomanip header
The branch main has been updated by ngie:
URL:
https://cgit.FreeBSD.org/src/commit/?id=0077477f215c851fe15c9ea12cfb005125c4238a
commit 0077477f215c851fe15c9ea12cfb005125c4238a
Author: Enji Cooper
AuthorDate: 2024-10-19 15:34:07 +
Commit: Enji Cooper
CommitDate: 2024-10-20 01:50:48 +
tests/sys/fs/fusefs: include iomanip header
io.cc relies on `std::setw(..)`, which is exported by the iomanip C++
header. Newer versions of GoogleTest don't export this header, so add
the explicit include.
This unbreaks the build with GoogleTest 1.15.2.
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D47194
---
tests/sys/fs/fusefs/io.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/sys/fs/fusefs/io.cc b/tests/sys/fs/fusefs/io.cc
index 99b5eae34e09..f8684ee02100 100644
--- a/tests/sys/fs/fusefs/io.cc
+++ b/tests/sys/fs/fusefs/io.cc
@@ -38,6 +38,8 @@ extern "C" {
#include
}
+#include
+
#include "mockfs.hh"
#include "utils.hh"
git: 5ca8c28cd8c7 - main - contrib/googletest: update from 1.14.0 to 1.15.2
The branch main has been updated by ngie: URL: https://cgit.FreeBSD.org/src/commit/?id=5ca8c28cd8c725b81781201cfdb5f9969396f934 commit 5ca8c28cd8c725b81781201cfdb5f9969396f934 Merge: 0077477f215c 14f7077fed7d Author: Enji Cooper AuthorDate: 2024-10-20 01:51:18 + Commit: Enji Cooper CommitDate: 2024-10-20 01:54:01 + contrib/googletest: update from 1.14.0 to 1.15.2 The changes between the two versions can be found in this diff of the two release tags: https://github.com/google/googletest/compare/v1.14.0...v1.15.2 One notable change is that GoogleTest 1.15.x now officially requires C++-14 (1.14.x required C++-11). MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D47197 Merge commit '14f7077fed7d82046bdcbe347004132f08aba886' contrib/googletest/BUILD.bazel | 17 ++ contrib/googletest/CMakeLists.txt | 13 +- contrib/googletest/CONTRIBUTING.md | 8 +- contrib/googletest/CONTRIBUTORS| 1 + contrib/googletest/MODULE.bazel| 69 + contrib/googletest/README.md | 22 +- contrib/googletest/WORKSPACE | 29 +- contrib/googletest/WORKSPACE.bzlmod| 35 +++ contrib/googletest/ci/linux-presubmit.sh | 10 +- contrib/googletest/ci/macos-presubmit.sh | 3 +- contrib/googletest/ci/windows-presubmit.bat| 11 +- contrib/googletest/docs/advanced.md| 36 ++- contrib/googletest/docs/faq.md | 57 ++-- contrib/googletest/docs/gmock_cook_book.md | 41 ++- contrib/googletest/docs/gmock_for_dummies.md | 6 +- contrib/googletest/docs/primer.md | 39 ++- contrib/googletest/docs/reference/assertions.md| 2 +- contrib/googletest/docs/reference/mocking.md | 3 +- contrib/googletest/docs/reference/testing.md | 29 +- contrib/googletest/fake_fuchsia_sdk.bzl| 33 +++ contrib/googletest/googlemock/CMakeLists.txt | 27 +- contrib/googletest/googlemock/README.md| 6 +- .../googlemock/include/gmock/gmock-actions.h | 44 ++- .../include/gmock/gmock-function-mocker.h | 9 +- .../googlemock/include/gmock/gmock-matchers.h | 179 ++--- .../googlemock/include/gmock/gmock-more-actions.h | 7 +- .../googletest/googlemock/include/gmock/gmock.h| 15 +- .../include/gmock/internal/gmock-internal-utils.h | 14 +- .../googlemock/include/gmock/internal/gmock-port.h | 8 +- .../googlemock/src/gmock-internal-utils.cc | 5 +- .../googletest/googlemock/src/gmock-matchers.cc| 29 +- .../googlemock/src/gmock-spec-builders.cc | 3 +- .../test/gmock-matchers-comparisons_test.cc| 9 + .../test/gmock-matchers-containers_test.cc | 17 +- .../googlemock/test/gmock-more-actions_test.cc | 40 ++- .../googlemock/test/gmock-spec-builders_test.cc| 2 +- .../googletest/googlemock/test/gmock_link_test.h | 9 + contrib/googletest/googletest/CMakeLists.txt | 28 +- contrib/googletest/googletest/README.md| 4 +- .../googletest/googletest/cmake/Config.cmake.in| 4 + .../googletest/cmake/internal_utils.cmake | 42 +-- .../include/gtest/gtest-assertion-result.h | 2 +- .../googletest/include/gtest/gtest-death-test.h| 8 +- .../googletest/include/gtest/gtest-message.h | 19 +- .../googletest/include/gtest/gtest-param-test.h| 8 +- .../googletest/include/gtest/gtest-printers.h | 90 +-- .../googletest/include/gtest/gtest-typed-test.h| 126 - .../googletest/googletest/include/gtest/gtest.h| 55 ++-- .../gtest/internal/gtest-death-test-internal.h | 51 ++-- .../include/gtest/internal/gtest-filepath.h| 8 +- .../include/gtest/internal/gtest-internal.h| 99 +++ .../include/gtest/internal/gtest-param-util.h | 154 +-- .../include/gtest/internal/gtest-port-arch.h | 2 + .../googletest/include/gtest/internal/gtest-port.h | 141 +++--- .../include/gtest/internal/gtest-type-util.h | 6 +- .../googletest/googletest/src/gtest-death-test.cc | 36 +-- .../googletest/googletest/src/gtest-filepath.cc| 2 +- .../googletest/googletest/src/gtest-internal-inl.h | 46 ++-- contrib/googletest/googletest/src/gtest-port.cc| 98 +-- contrib/googletest/googletest/src/gtest.cc | 294 - .../googletest/test/googletest-color-test.py | 1 + .../googletest/test/googletest-death-test-test.cc | 78 +++--- .../test/googletest-json-output-unittest.py| 15 ++ .../googletest/test/googletest-options-test.cc | 5 +- .../test/googletest-output-test-golden-lin.txt | 5 - .../googletest/test/googletest-port-test.cc| 6 +- .../googletest/test/googletest-printers-test.cc
