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
Re: sndiod: -F does not switch back to preferred device
On Thu, Dec 09, 2021 at 09:02:07PM -0700, Theo de Raadt wrote:
>
> I think drivers, or maybe this can be done in higher-level subsystems, need
> to be modified such that events get sent when a device is *actually ready*,
> and the call from config_attach() should be deleted.
>
For audio and MIDI, I think we're not very far from this.
Currently, as soon as audio_attach() returns, both hardware and
audio(4) internal structures are properly initialized. So all the
audio_{open,read,write,ioctl,close}() interfaces are safe to call.
What happens in higher layers is unclear to me? for instance, what
needs to be done for open(2) to reach audioopen() with no races?
Re: sndiod: -F does not switch back to preferred device
Theo de Raadt wrote: > I think drivers, or maybe this can be done in higher-level subsystems, need > to be modified such that events get sent when a device is *actually ready*, > and the call from config_attach() should be deleted. Worth mentioning the numbers below are devclass DV_* fields, which is pretty ugly. DV_* was only supposed to be used for boot device selection, and it got abused and exported to userland. I think a new numeric scheme should be used. But for example, 0 generic, no special info Maybe this continues using the config_attach() scheme, except I am pretty sure this includes audio, so I recommend breaking audio out as a new scheme. 1 CPU (carries resource utilization) This makes no sense. 2 disk drive see disk_attach() and disk_detach() 3 network interface see if_attach(), and if_detach() 4 tape device No subsystem API. 5 serial line interface Surprisingly tty devices only register themselves when they are opened, or closed. The ttymalloc() placement in most drivers is closest to putting a tty registration call, but some device drivers remain unfixed and still call ttymalloc() at open time instead of attach time. tty drivers also handle detach events in a very raw way, with vgonel and other junk. [new] 6 audio See audio_attach(), probably a detach?
Re: sndiod: -F does not switch back to preferred device
As I said back then, I don't see how this solves the problem.
If you try to open a /dev/sd2i partition 1msec after the hotplug
indicates the arrival of sd2, that open will fail. (There is a small
chance it will succeed, but why are we exposing our users to races?)
The hotplug event is caused by this in being directly in config_attach()
(*ca->ca_attach)(parent, dev, aux);
config_process_deferred_children(dev);
#if NHOTPLUG > 0
if (!cold)
hotplug_device_attach(cd->cd_class, dev->dv_xname);
#endif
Unfortunately, an attach routine being called does not mean a driver
is ready. It might have 20 years ago in a simpler kernel, but today
that is simply not true.
The moment an sd(4) device is attached it does not mean the device nodes
respond, you cannot call open(), and you cannot call ioctl() to get the
disklabel.
I do not understand what you are trying to solve, is the proposal to put
hooks into the slow drivers? I think this problem with hotplug applies
to all devices, and will only become more visible in time due to the
increasing use of callbacks in device drivers to make them become ready,
especially callbacks in other layers(networking, scsi, audio, usb, or
whatever, and quite often layered, like scsi on usb).
I don't think this can be papered over in hotplug.c, the problem starts
because of the hotplug_device_attach() call in config_attach().
The hotplug(4) manual page carefully describes:
The hotplug pseudo-device passes device attachment and detachment events
to userland.
That is true and accurate, but things go awry in the first sentence in
hotplugd(8):
The hotplugd daemon monitors the hotplug(4) pseudo-device, acting on
signaled events by executing the scripts in the /etc/hotplug directory.
hotplug only creates an early alert about a new device coming, but hotplugd
believe this is a ready alert, and I've seen scripts where people try to cope
with this dissonance, by adding delays or looping on device use.
The way this is architected, hotplug(4) cannot provide a 'ready alert', and
I do not understand how your change makes it a "ready alert".
I think drivers, or maybe this can be done in higher-level subsystems, need
to be modified such that events get sent when a device is *actually ready*,
and the call from config_attach() should be deleted.
joshua stein wrote:
> On Thu, 09 Dec 2021 at 16:46:24 -0700, Theo de Raadt wrote:
> > Please do not recommend people use hotplugd, for any purpose.
> >
> > It is on my queue for deletion, because noone has stepped up and
> > fixed it.
> >
> > hotplug is COMPLETELY FLAWED. It reports when a device is attached,
> > not when it is ready. As a result everyone uses it wrong.
> >
> > As a result, your script is broken. There is no assurance that the
> > audio device's ioctl routines are ready 1ms after subr_autoconf returns
> > from the xxattach() function.
>
> I sent diffs for this in 2018 and nobody wanted them. I wanted
> multiple reader support on /dev/hotplug for Xorg and you gave this
> same complaint about hotplug being flawed, so I proposed a simple
> way for particular drivers to defer their hotplug announcement until
> they were ready. From 2018:
>
> I chatted with Theo about that and I proposed that we just have a
> way for those specific drivers to tell hotplug to defer their
> attachment announcement and then they can manually request
> announcement later when they're ready.
>
> However, I just searched the bugs@ archives and all of the mentions
> of hotplugd were like 8 years ago, and they were all related to USB
> disks. Then some of the bugs were closed after an apparent change
> to hotplug.c was made:
>
> https://marc.info/?l=openbsd-bugs=127990199813627=2
>
> After that, I can't find any reports of panics related to using
> hotplugd. Are we sure this is even a problem anymore?
>
> Nobody stepped up with any drivers that actually needed it.
>
> Here's the diff from 2018, and another to implement multiple readers
> on /dev/hotplug so things like sndiod could use it. If anyone
> actually cares about them this time, I can update them to a current
> tree.
>
>
> diff --git sys/dev/hotplug.c sys/dev/hotplug.c
> index fccd5a7c9e3..f75e830 100644
> --- sys/dev/hotplug.c
> +++ sys/dev/hotplug.c
> @@ -89,6 +89,12 @@ hotplug_device_detach(enum devclass class, char *name)
> hotplug_put_event();
> }
>
> +void
> +hotplug_defer_attach(struct device *dev)
> +{
> + dev->dv_flags |= DVF_HOTPLUG_DEFER;
> +}
> +
> void
> hotplug_put_event(struct hotplug_event *he)
> {
> diff --git sys/kern/subr_autoconf.c sys/kern/subr_autoconf.c
> index 3fff153e8ad..a31ca509e35 100644
> --- sys/kern/subr_autoconf.c
> +++ sys/kern/subr_autoconf.c
> @@ -401,7 +401,7 @@ config_attach(struct device *parent, void *match, void
> *aux, cfprint_t print)
> (*ca->ca_attach)(parent, dev, aux);
>
Re: sndiod: -F does not switch back to preferred device
On Thu, 09 Dec 2021 at 16:46:24 -0700, Theo de Raadt wrote:
> Please do not recommend people use hotplugd, for any purpose.
>
> It is on my queue for deletion, because noone has stepped up and
> fixed it.
>
> hotplug is COMPLETELY FLAWED. It reports when a device is attached,
> not when it is ready. As a result everyone uses it wrong.
>
> As a result, your script is broken. There is no assurance that the
> audio device's ioctl routines are ready 1ms after subr_autoconf returns
> from the xxattach() function.
I sent diffs for this in 2018 and nobody wanted them. I wanted
multiple reader support on /dev/hotplug for Xorg and you gave this
same complaint about hotplug being flawed, so I proposed a simple
way for particular drivers to defer their hotplug announcement until
they were ready. From 2018:
I chatted with Theo about that and I proposed that we just have a
way for those specific drivers to tell hotplug to defer their
attachment announcement and then they can manually request
announcement later when they're ready.
However, I just searched the bugs@ archives and all of the mentions
of hotplugd were like 8 years ago, and they were all related to USB
disks. Then some of the bugs were closed after an apparent change
to hotplug.c was made:
https://marc.info/?l=openbsd-bugs=127990199813627=2
After that, I can't find any reports of panics related to using
hotplugd. Are we sure this is even a problem anymore?
Nobody stepped up with any drivers that actually needed it.
Here's the diff from 2018, and another to implement multiple readers
on /dev/hotplug so things like sndiod could use it. If anyone
actually cares about them this time, I can update them to a current
tree.
diff --git sys/dev/hotplug.c sys/dev/hotplug.c
index fccd5a7c9e3..f75e830 100644
--- sys/dev/hotplug.c
+++ sys/dev/hotplug.c
@@ -89,6 +89,12 @@ hotplug_device_detach(enum devclass class, char *name)
hotplug_put_event();
}
+void
+hotplug_defer_attach(struct device *dev)
+{
+ dev->dv_flags |= DVF_HOTPLUG_DEFER;
+}
+
void
hotplug_put_event(struct hotplug_event *he)
{
diff --git sys/kern/subr_autoconf.c sys/kern/subr_autoconf.c
index 3fff153e8ad..a31ca509e35 100644
--- sys/kern/subr_autoconf.c
+++ sys/kern/subr_autoconf.c
@@ -401,7 +401,7 @@ config_attach(struct device *parent, void *match, void
*aux, cfprint_t print)
(*ca->ca_attach)(parent, dev, aux);
config_process_deferred_children(dev);
#if NHOTPLUG > 0
- if (!cold)
+ if (!cold && !(dev->dv_flags & DVF_HOTPLUG_DEFER))
hotplug_device_attach(cd->cd_class, dev->dv_xname);
#endif
diff --git sys/sys/device.h sys/sys/device.h
index 1faa0192a99..596ef1a26d5 100644
--- sys/sys/device.h
+++ sys/sys/device.h
@@ -81,7 +81,8 @@ struct device {
};
/* dv_flags */
-#defineDVF_ACTIVE 0x0001 /* device is activated */
+#defineDVF_ACTIVE 0x0001 /* device is activated
*/
+#defineDVF_HOTPLUG_DEFER 0x0002 /* defer hotplug
announce */
TAILQ_HEAD(devicelist, device);
diff --git sys/sys/hotplug.h sys/sys/hotplug.h
index f2da3f6cfd8..7e68482a428 100644
--- sys/sys/hotplug.h
+++ sys/sys/hotplug.h
@@ -35,6 +35,7 @@ struct hotplug_event {
#ifdef _KERNEL
void hotplug_device_attach(enum devclass, char *);
void hotplug_device_detach(enum devclass, char *);
+void hotplug_defer_attach(struct device *);
#endif
#endif /* _SYS_HOTPLUG_H_ */
Index: sys/dev/hotplug.c
===
RCS file: /cvs/src/sys/dev/hotplug.c,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 hotplug.c
--- sys/dev/hotplug.c 7 Jun 2016 01:31:54 - 1.16
+++ sys/dev/hotplug.c 2 Sep 2018 17:42:50 -
@@ -1,5 +1,6 @@
/* $OpenBSD: hotplug.c,v 1.16 2016/06/07 01:31:54 tedu Exp $ */
/*
+ * Copyright (c) 2018 joshua stein
* Copyright (c) 2004 Alexander Yurchenko
*
* Permission to use, copy, modify, and distribute this software for any
@@ -25,15 +26,24 @@
#include
#include
#include
+#include
#include
#include
#define HOTPLUG_MAXEVENTS 64
-static int opened;
+struct hotplug_dev {
+ int hd_unit;
+ int hd_evqueue_head;
+ struct selinfo hd_sel;
+ LIST_ENTRY(hotplug_dev) hd_list;
+};
+
+LIST_HEAD(, hotplug_dev) hotplug_dev_list;
+struct hotplug_dev *hotplug_lookup(int);
+
static struct hotplug_event evqueue[HOTPLUG_MAXEVENTS];
-static int evqueue_head, evqueue_tail, evqueue_count;
-static struct selinfo hotplug_sel;
+static int evqueue_head, evqueue_count;
void filt_hotplugrdetach(struct knote *);
int filt_hotplugread(struct knote *, long);
@@ -43,19 +53,18 @@ struct filterops hotplugread_filtops =
#define EVQUEUE_NEXT(p) (p == HOTPLUG_MAXEVENTS - 1 ? 0 : p + 1)
-
-int hotplug_put_event(struct hotplug_event *);
-int hotplug_get_event(struct hotplug_event *);
+void
Re: gprof: Profiling a multi-threaded application
Any comments about this topic?
On 7/12/21 18:09, Yuichiro NAITO wrote:
Hi, Martin
n 2021/07/10 16:55, Martin Pieuchot wrote:
Hello Yuichiro, thanks for your work !
Thanks for the response.
On 2021/06/16 16:34, Yuichiro NAITO wrote:
When I compile a multi-threaded application with '-pg' option, I always get no
results in gmon.out. With bad luck, my application gets SIGSEGV while running.
Because the buffer that holds number of caller/callee count is the only one
in the process and will be broken by multi-threaded access.
I get the idea to solve this problem from NetBSD. NetBSD has individual buffers
for each thread and merges them at the end of profiling.
Note that the kernel use a similar approach but doesn't merge the buffer,
instead it generates a file for each CPU.
Yes, so the number of output files are limited by the number of CPUs in case of
the kernel profiling. I think number of application threads can be increased
more
casually. I don't want to see dozens of 'gmon.out' files.
NetBSD stores the reference to the individual buffer by pthread_setspecific(3).
I think it causes infinite recursive call if whole libc library (except
mcount.c) is compiled with -pg.
The compiler generates '_mcount' function call at the beginning of every
functions. If '_mcount' calls pthread_getspecific(3) to get the individual
buffer, pthread_getspecific() calls '_mcount' again and causes infinite
recursion.
NetBSD prevents from infinite recursive call by checking a global variable. But
this approach also prevents simultaneously call of '_mcount' on a multi-threaded
application. It makes a little bit wrong results of profiling.
So I added a pointer to the buffer in `struct pthread` that can be accessible
via macro call as same as pthread_self(3). This approach can prevent of
infinite recursive call of '_mcount'.
Not calling a libc function for this makes sense. However I'm not
convinced that accessing `tib_thread' before _rthread_init() has been
called is safe.
Before '_rthread_init’ is called, '__isthreaded' global variable is kept to be
0.
My patch doesn't access tib_thread in this case.
After calling `_rthread_init`, `pthread_create()` changes `__isthreaded` to 1.
Tib_thread will be accessed by all threads that are newly created and the
initial one.
I believe tib of the initial thread has been initialized in `_libc_preinit'
function
in 'lib/libc/dlfcn/init.c'.
I'm not sure where is the cleanest way to place the per-thread buffer,
I'd suggest you ask guenther@ about this.
I added guenther@ in CC of this mail.
I hope I can get an advise about per-thread buffer.
Maybe the initialization can be done outside of _mcount()?
Yes, I think tib is initialized in `pthread_create()` and `_libc_preinit()`.
I obtained merging function from NetBSD that is called in '_mcleanup' function.
Merging function needs to walk through all the individual buffers,
I added SLIST_ENTRY member in 'struct gmonparam' to make a list of the buffers.
And also added '#ifndef _KERNEL' for the SLIST_ENTRY member not to be used for
the kernel.
But I still use pthread_getspecific(3) for that can call destructor when
a thread is terminated. The individual buffer is moved to free list to reuse
for a new thread.
Here is a patch for this approach.
I have various comments:
We choose not to use C++ style comment (// comment) in the tree, could you
fix yours?
There's two copies of mcount.c, the other one lies in sys/lib/libkern could
you keep them in sync?
gmon.c is only compiled in userland and don't need #ifndef _KERNEL
In libc there's also the use of _MUTEX_LOCK/UNLOCK() macro instead of
calling pthread_mutex* directly. This might help reduce the differences
between ST and MT paths.
Thanks for letting me know them.
I updated my patch as following according to the advice.
diff --git a/lib/libc/gmon/gmon.c b/lib/libc/gmon/gmon.c
index 1ce0a1c289e..5169fa70449 100644
--- a/lib/libc/gmon/gmon.c
+++ b/lib/libc/gmon/gmon.c
@@ -42,6 +42,15 @@
struct gmonparam _gmonparam = { GMON_PROF_OFF };
+#include
+#include
+
+SLIST_HEAD(, gmonparam) _gmonfree = SLIST_HEAD_INITIALIZER(_gmonfree);
+SLIST_HEAD(, gmonparam) _gmoninuse = SLIST_HEAD_INITIALIZER(_gmoninuse);
+void* _gmonlock = NULL;
+pthread_key_t _gmonkey;
+struct gmonparam _gmondummy;
+
static ints_scale;
/* see profil(2) where this is describe (incorrectly) */
#define SCALE_1_TO_10x1L
@@ -52,6 +61,11 @@ PROTO_NORMAL(moncontrol);
PROTO_DEPRECATED(monstartup);
static int hertz(void);
+static void _gmon_destructor(void *);
+struct gmonparam *_gmon_alloc(void);
+static void _gmon_merge(void);
+static void _gmon_merge_two(struct gmonparam *, struct gmonparam *);
+
void
monstartup(u_long lowpc, u_long highpc)
{
@@ -114,6 +128,9 @@ monstartup(u_long lowpc, u_long highpc)
} else
s_scale = SCALE_1_TO_1;
+ _gmondummy.state = GMON_PROF_BUSY;
+ pthread_key_create(&_gmonkey,
Re: sndiod: -F does not switch back to preferred device
The real thing going on here is that if /dev/hotplug actually worked, then sndiod would be able to use by itself to be smart. But hotplug doesn't work, it is unfit for purpose. So we have discouraged Alexandre from making sndiod look at hotplugd. Now you propose to extranalize this, using the additional daemon which exposes exactly the same problems, and people should copy-paste a racey recipe out of a manual page, and chmod it and put it on their machines. All of this because noone goes and fixes hotplug to report - new device has arrived and is ready* rather than what it currently does which is - new device has arrived and YOU CAN BET IT IS NOT READY
Re: sndiod: -F does not switch back to preferred device
In addition, such detailed text with examples does not belong in that manual page. What I see there is an example of the "HOWTO" mentality.
Re: sndiod: -F does not switch back to preferred device
Please do not recommend people use hotplugd, for any purpose.
It is on my queue for deletion, because noone has stepped up and
fixed it.
hotplug is COMPLETELY FLAWED. It reports when a device is attached,
not when it is ready. As a result everyone uses it wrong.
As a result, your script is broken. There is no assurance that the
audio device's ioctl routines are ready 1ms after subr_autoconf returns
from the xxattach() function.
I have considered putting a sleep(10) into hotplugd to ensure the devices
are ready, and then noone gets hurt. But also to mock the situation.
Oh, people won't like that. But maybe it is time to prove the point.
Then maybe someone will sit down and figure out a generic "when is the
device ready" mechanism, and adapt hotplug to signal that INSTEAD of
what it currently does.
Klemens Nanni wrote:
> On Thu, Dec 09, 2021 at 10:21:56AM +0100, Alexandre Ratchov wrote:
> > On Wed, Dec 08, 2021 at 10:30:08PM +, Klemens Nanni wrote:
> > > Following https://www.openbsd.org/faq/faq13.html#usbaudio and reading
> > > sndiod(8)'s
> > >
> > > -F device
> > > Specify an alternate device to use. If it doesn't work, the one
> > > given with the last -f or -F options will be used. For
> > > instance,
> > > specifying a USB device following a PCI device allows sndiod to
> > > use the USB one preferably when it's connected and to fall back
> > > to the PCI one when it's disconnected. Alternate devices may be
> > > switched with the server.device control of the sndioctl(1)
> > > utility.
> > >
> > > I configured things as follows in order to play audio via USB and
> > > fall back to internal sound if USB is not available:
> > >
> > > $ rcctl get sndiod flags
> > > -f rsnd/0 -F rsnd/1
> > >
> > > Plugging in an USB headset and restarting sndiod or forcing the device
> > > with `sndioctl server.device=1' then plays sound via USB.
> > >
> > > Unplugging the device makes playback fall back to internal sound.
> > >
> > > But plugging USB back in does not prefer USB to internal as I'd expect
> > > now. I am currently resorting to the following hotplugd(8) script to
> > > always select the USB sound device whenever I plug it in:
> > >
> > > #!/bin/ksh
> > > set -Cefu -o pipefail
> > >
> > > readonly DEVCLASS=$1 DEVNAME=$2
> > > typeset -i devid
> > >
> > > case "${DEVCLASS}-${DEVNAME}" in
> > > 0-audio*) # switch sndio(4) to USB headset when plugging it in
> > > devid=${DEVNAME#audio}
> > > sndioctl server.device=${devid}
> > > ;;
> > > esac
> > >
> >
> > IFAIU, audio devices always match the audio[0-9] pattern, so testing
> > the device class is not necessary, is it? FWIW, I use a similar
> > script.
>
> I do it that way because /etc/hotplug/attach handles *all* events, so
> extending it with other cases where DEVCLASS matters would be obvious
> and audio(4) does not have to be treated specially.
>
> > > I'd expect sndiod to *always* use USB whenever possible.
> > >
> > > Is this uni-directional behaviour of sndiod intentional/by-design?
> >
> > sndiod doesn't use directly hotplug(4), so it can't obtain by itself a
> > notification when a new device is plugged in order to switch to
> > it. Using hotplugd(8) is the right way to make audio hotplug work,
> > both daemons complete each other.
>
> Alright, hotplugd it is, then.
>
> > When sndiod needs the audio device (i.e. when the first client
> > connects), it will try all alternate devices (reverse -F options
> > order, the USB headset is first in your case). This gives a false
> > impression of hotplug support (indeed, you plug the USB headset, start
> > an audio player and it just works!). But that's just a side effect of
> > device priority/fail-over: if the internal device is already open when
> > you plug the USB headset, sndiod won't switch to it until it needs to
> > reopen the device.
> >
> > Certain programs (including browsers), tend to keep the device open
> > even when they remain silent. So when actual playback starts sndiod
> > doesn't need to open a device and doesn't "see" the new USB headset. I
> > guess that's what happens on your system, but hotplugd(8) handles
> > this.
> >
> > When devices are unplugged, we don't need hotplug because the device
> > stops working (input/output error) and sndiod switch to the next one
> > of the fail-over list.
> >
> > > If so, can we clarify the manual?
> >
> > Sure. While -F option description seems exact, maybe we need an extra
> > paragraph or FAQ entry to explain how to use it with other tools like
> > hotplugd and sndioctl. What about this wording?
>
> Exact, but it could be clearer about the uni-directional fallack
> semantic, which is only relevant upon startup. Then I wouldn't think
> it switches back and forth at runtime.
>
> > HOT-PLUG SUPPORT
> > If a device is unplugged while in use, sndiod will attempt to switch to
> >
ipsec tdb flags mutex
Hi,
I would like to protect the write access to the TDB flags field
with a mutex. Clearing the timeout flags just before pool put in
tdb_free() does not make sense. Move this to tdb_delete().
While there make the parentheses in the flag check consistent.
ok?
bluhm
Index: net/pfkeyv2_convert.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2_convert.c,v
retrieving revision 1.76
diff -u -p -r1.76 pfkeyv2_convert.c
--- net/pfkeyv2_convert.c 25 Nov 2021 13:46:02 - 1.76
+++ net/pfkeyv2_convert.c 9 Dec 2021 15:15:34 -
@@ -122,6 +122,7 @@ import_sa(struct tdb *tdb, struct sadb_s
if (!sadb_sa)
return;
+ mtx_enter(>tdb_mtx);
if (ii) {
ii->ii_encalg = sadb_sa->sadb_sa_encrypt;
ii->ii_authalg = sadb_sa->sadb_sa_auth;
@@ -145,6 +146,7 @@ import_sa(struct tdb *tdb, struct sadb_s
if (sadb_sa->sadb_sa_state != SADB_SASTATE_MATURE)
tdb->tdb_flags |= TDBF_INVALID;
+ mtx_leave(>tdb_mtx);
}
/*
@@ -282,6 +284,7 @@ import_lifetime(struct tdb *tdb, struct
if (!sadb_lifetime)
return;
+ mtx_enter(>tdb_mtx);
switch (type) {
case PFKEYV2_LIFETIME_HARD:
if ((tdb->tdb_exp_allocations =
@@ -348,6 +351,7 @@ import_lifetime(struct tdb *tdb, struct
tdb->tdb_established = sadb_lifetime->sadb_lifetime_addtime;
tdb->tdb_first_use = sadb_lifetime->sadb_lifetime_usetime;
}
+ mtx_leave(>tdb_mtx);
}
/*
Index: netinet/ip_ah.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ah.c,v
retrieving revision 1.168
diff -u -p -r1.168 ip_ah.c
--- netinet/ip_ah.c 2 Dec 2021 12:39:15 - 1.168
+++ netinet/ip_ah.c 9 Dec 2021 17:46:28 -
@@ -612,8 +612,8 @@ ah_input(struct mbuf **mp, struct tdb *t
ahstat_add(ahs_ibytes, ibytes);
/* Hard expiration. */
- if (tdb->tdb_flags & TDBF_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
+ if ((tdb->tdb_flags & TDBF_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
ipsecstat_inc(ipsec_exctdb);
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
tdb_delete(tdb);
@@ -621,11 +621,15 @@ ah_input(struct mbuf **mp, struct tdb *t
}
/* Notify on expiration. */
- if (tdb->tdb_flags & TDBF_SOFT_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes) {
+ mtx_enter(>tdb_mtx);
+ if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+ tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
+ mtx_leave(>tdb_mtx);
+ /* may sleep in solock() for the pfkey socket */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking. */
- }
+ } else
+ mtx_leave(>tdb_mtx);
/* Get crypto descriptors. */
crp = crypto_getreq(1);
@@ -952,8 +956,8 @@ ah_output(struct mbuf *m, struct tdb *td
ahstat_add(ahs_obytes, m->m_pkthdr.len - skip);
/* Hard expiration. */
- if (tdb->tdb_flags & TDBF_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
+ if ((tdb->tdb_flags & TDBF_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
ipsecstat_inc(ipsec_exctdb);
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
tdb_delete(tdb);
@@ -962,11 +966,15 @@ ah_output(struct mbuf *m, struct tdb *td
}
/* Notify on expiration. */
- if (tdb->tdb_flags & TDBF_SOFT_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes) {
+ mtx_enter(>tdb_mtx);
+ if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+ tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
+ mtx_leave(>tdb_mtx);
+ /* may sleep in solock() for the pfkey socket */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
- }
+ } else
+ mtx_leave(>tdb_mtx);
/*
* Loop through mbuf chain; if we find a readonly mbuf,
Index: netinet/ip_esp.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_esp.c,v
retrieving revision 1.188
diff -u -p -r1.188 ip_esp.c
--- netinet/ip_esp.c21 Nov 2021 16:17:48 - 1.188
+++ netinet/ip_esp.c9 Dec 2021 17:47:28 -
@@ -433,11 +433,15 @@ esp_input(struct mbuf **mp, struct tdb *
}
/* Notify on soft expiration */
+ mtx_enter(>tdb_mtx);
if
Re: sndiod: -F does not switch back to preferred device
On Thu, Dec 09, 2021 at 10:21:56AM +0100, Alexandre Ratchov wrote:
> On Wed, Dec 08, 2021 at 10:30:08PM +, Klemens Nanni wrote:
> > Following https://www.openbsd.org/faq/faq13.html#usbaudio and reading
> > sndiod(8)'s
> >
> > -F device
> > Specify an alternate device to use. If it doesn't work, the one
> > given with the last -f or -F options will be used. For
> > instance,
> > specifying a USB device following a PCI device allows sndiod to
> > use the USB one preferably when it's connected and to fall back
> > to the PCI one when it's disconnected. Alternate devices may be
> > switched with the server.device control of the sndioctl(1)
> > utility.
> >
> > I configured things as follows in order to play audio via USB and
> > fall back to internal sound if USB is not available:
> >
> > $ rcctl get sndiod flags
> > -f rsnd/0 -F rsnd/1
> >
> > Plugging in an USB headset and restarting sndiod or forcing the device
> > with `sndioctl server.device=1' then plays sound via USB.
> >
> > Unplugging the device makes playback fall back to internal sound.
> >
> > But plugging USB back in does not prefer USB to internal as I'd expect
> > now. I am currently resorting to the following hotplugd(8) script to
> > always select the USB sound device whenever I plug it in:
> >
> > #!/bin/ksh
> > set -Cefu -o pipefail
> >
> > readonly DEVCLASS=$1 DEVNAME=$2
> > typeset -i devid
> >
> > case "${DEVCLASS}-${DEVNAME}" in
> > 0-audio*) # switch sndio(4) to USB headset when plugging it in
> > devid=${DEVNAME#audio}
> > sndioctl server.device=${devid}
> > ;;
> > esac
> >
>
> IFAIU, audio devices always match the audio[0-9] pattern, so testing
> the device class is not necessary, is it? FWIW, I use a similar
> script.
I do it that way because /etc/hotplug/attach handles *all* events, so
extending it with other cases where DEVCLASS matters would be obvious
and audio(4) does not have to be treated specially.
> > I'd expect sndiod to *always* use USB whenever possible.
> >
> > Is this uni-directional behaviour of sndiod intentional/by-design?
>
> sndiod doesn't use directly hotplug(4), so it can't obtain by itself a
> notification when a new device is plugged in order to switch to
> it. Using hotplugd(8) is the right way to make audio hotplug work,
> both daemons complete each other.
Alright, hotplugd it is, then.
> When sndiod needs the audio device (i.e. when the first client
> connects), it will try all alternate devices (reverse -F options
> order, the USB headset is first in your case). This gives a false
> impression of hotplug support (indeed, you plug the USB headset, start
> an audio player and it just works!). But that's just a side effect of
> device priority/fail-over: if the internal device is already open when
> you plug the USB headset, sndiod won't switch to it until it needs to
> reopen the device.
>
> Certain programs (including browsers), tend to keep the device open
> even when they remain silent. So when actual playback starts sndiod
> doesn't need to open a device and doesn't "see" the new USB headset. I
> guess that's what happens on your system, but hotplugd(8) handles
> this.
>
> When devices are unplugged, we don't need hotplug because the device
> stops working (input/output error) and sndiod switch to the next one
> of the fail-over list.
>
> > If so, can we clarify the manual?
>
> Sure. While -F option description seems exact, maybe we need an extra
> paragraph or FAQ entry to explain how to use it with other tools like
> hotplugd and sndioctl. What about this wording?
Exact, but it could be clearer about the uni-directional fallack
semantic, which is only relevant upon startup. Then I wouldn't think
it switches back and forth at runtime.
> HOT-PLUG SUPPORT
> If a device is unplugged while in use, sndiod will attempt to switch to
> one of the alternate devices (-F), if any. This is seamless to programs
> connected to sndiod.
>
> Later, when the device is connected again, the server.device control of
> the sndioctl(1) utility could be used to switch back to it, without the
> need to restart all audio programs. This last step could be automated
> using hotplugd(8). For instance, if sndiod is started with:
>
>$ sndiod -f rsnd/0 -F rsnd/1 -F rsnd/2 -F rsnd/3
>
> then, the following hotplugd(8) attach script could be used to
> automatically switch to the last connected device:
>
>#!/bin/sh
>
>DEVNAME=$2
>
>case $DEVNAME in
>audio[0-3])
>sndioctl server.device=${DEVNAME#audio}
>;;
>esac
This is clear and gets users going in no time.
With this I would not even have looked at the FAQ (which follows release
and not -current, which can be a problem).
> Any
Re: sppp(4)/pppoe(4) - avoid endless loop in remote ip negotiation
On 2021-12-05 23:43, Stuart Henderson wrote: > On 2021/12/04 15:32, Krzysztof Kanas wrote: > > I tried the settings: > > > > > > inet 0.0.0.0 255.255.255.255 10.64.64.33 \ > > pppoedev em0 authproto chap \ > > authname 'testcaller' authkey 'secret' up > > !/sbin/route add default -ifp pppoe0 10.64.64.33 > > > > Seems to be working. > > > > I will test it more, but does this sound correct ? > > And if so then is it worth while to add this to man page to bugs section > > ? > I upgraded to 7.0, tried above configuration and it didn't work. I believe this could be due the test I ran didn't rebooted router (and ISP pppoe modem). Upgrade to 7.0 rebooted the router it and pppoe0 didn't come up (with configration below). I added debug option and below is part of sppp output that hint's the problem. pppoe0: chap success pppoe0: phase network pppoe0: ipcp open(initial) pppoe0: ipcp initial->starting pppoe0: ipv6cp_open(): no IPv6 interface pppoe0: ipcp up(starting) pppoe0: ipcp starting->req-sent pppoe0: ipcp output pppoe0 (8864) state=3, session=0xd862 output -> c0:14:fe:5b:6a:40, len=30 pppoe0: ipcp input(req-sent): pppoe0: ipcp parse opts: address pppoe0: ipcp parse opt values: address [addr requested] send conf-nak pppoe0: ipcp output pppoe0 (8864) state=3, session=0xd862 output -> c0:14:fe:5b:6a:40, len=18 pppoe0: ipcp input(req-sent): pppoe0: ipcp nak opts: primdns secdns pppoe0: ipcp output pppoe0 (8864) state=3, session=0xd862 output -> c0:14:fe:5b:6a:40, len=30 pppoe0: ipcp input(req-sent): pppoe0: ipcp parse opts: pppoe0: ipcp parse opt values: still need hisaddr send conf-nak pppoe0: ipcp output pppoe0 (8864) state=3, session=0xd862 output -> c0:14:fe:5b:6a:40, len=18 pppoe0: ipcp input(req-sent): pppoe0: ipcp req-sent->ack-rcvd pppoe0: ipcp input(ack-rcvd): pppoe0: ipcp parse opts: pppoe0: ipcp parse opt values: still need hisaddr send conf-nak pppoe0: ipcp output pppoe0 (8864) state=3, session=0xd862 output -> c0:14:fe:5b:6a:40, len=18 pppoe0: ipcp input(ack-rcvd): . and many more 'still need hisaddr' So I am back to using custom kernel due, to remote side never replies for hisaddr IPCP request. If hard coding arbitrary RFC 1918 IP is not an option, then adding additional option to ifconfig & sppp would be correct solution ? To make remote address more distinctive I used 10.20.30.22 in log above So inet 0.0.0.0 255.255.255.255 10.20.30.22 \ pppoedev em0 authproto chap \ authname 'testcaller' authkey 'secret' up !/sbin/route add default -ifp pppoe0 10.20.30.22 -- -- Best Regards Krzysztof Kanas
Make __EV_POLL flag specific to kqueue-based poll(2)
The system now has two flags for poll and select system calls,
__EV_POLL which currently covers both calls, and __EV_SELECT which is
used with select only.
To make the code easier to follow, this diff makes __EV_POLL specific
to poll(2). Consequently, __EV_SELECT has to be added when the condition
is relevant to select(2). __EV_HUP will be specific to kqueue-based
poll(2), so the code that sets __EV_HUP can keep on checking just
__EV_POLL.
OK?
Index: isofs/cd9660/cd9660_vnops.c
===
RCS file: src/sys/isofs/cd9660/cd9660_vnops.c,v
retrieving revision 1.90
diff -u -p -r1.90 cd9660_vnops.c
--- isofs/cd9660/cd9660_vnops.c 2 Oct 2021 08:51:41 - 1.90
+++ isofs/cd9660/cd9660_vnops.c 9 Dec 2021 14:59:41 -
@@ -1017,7 +1017,7 @@ filt_cd9660read(struct knote *kn, long h
return (1);
}
- if (kn->kn_flags & __EV_POLL)
+ if (kn->kn_flags & (__EV_POLL | __EV_SELECT))
return (1);
return (kn->kn_data != 0);
Index: kern/kern_event.c
===
RCS file: src/sys/kern/kern_event.c,v
retrieving revision 1.174
diff -u -p -r1.174 kern_event.c
--- kern/kern_event.c 29 Nov 2021 15:54:04 - 1.174
+++ kern/kern_event.c 9 Dec 2021 14:59:41 -
@@ -1779,7 +1779,7 @@ knote_remove(struct proc *p, struct kque
* This reuses the original knote for delivering the
* notification so as to avoid allocating memory.
*/
- if (!purge && (kn->kn_flags & __EV_POLL) &&
+ if (!purge && (kn->kn_flags & (__EV_POLL | __EV_SELECT)) &&
!(p->p_kq == kq &&
p->p_kq_serial > (unsigned long)kn->kn_udata) &&
kn->kn_fop != _filtops) {
Index: kern/spec_vnops.c
===
RCS file: src/sys/kern/spec_vnops.c,v
retrieving revision 1.106
diff -u -p -r1.106 spec_vnops.c
--- kern/spec_vnops.c 15 Oct 2021 06:30:06 - 1.106
+++ kern/spec_vnops.c 9 Dec 2021 14:59:41 -
@@ -404,7 +404,7 @@ spec_kqfilter(void *v)
switch (ap->a_vp->v_type) {
default:
- if (ap->a_kn->kn_flags & __EV_POLL)
+ if (ap->a_kn->kn_flags & (__EV_POLL | __EV_SELECT))
return seltrue_kqfilter(dev, ap->a_kn);
break;
case VCHR:
Index: kern/sys_generic.c
===
RCS file: src/sys/kern/sys_generic.c,v
retrieving revision 1.145
diff -u -p -r1.145 sys_generic.c
--- kern/sys_generic.c 8 Dec 2021 13:03:52 - 1.145
+++ kern/sys_generic.c 9 Dec 2021 14:59:41 -
@@ -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(, fd, evf[msk],
- EV_ADD|EV_ENABLE|__EV_POLL|__EV_SELECT,
+ EV_ADD|EV_ENABLE|__EV_SELECT,
evff[msk], 0, (void *)(p->p_kq_serial));
#ifdef KTRACE
if (KTRPOINT(p, KTR_STRUCT))
Index: kern/tty_tty.c
===
RCS file: src/sys/kern/tty_tty.c,v
retrieving revision 1.28
diff -u -p -r1.28 tty_tty.c
--- kern/tty_tty.c 10 Mar 2021 10:21:47 - 1.28
+++ kern/tty_tty.c 9 Dec 2021 14:59:41 -
@@ -159,7 +159,7 @@ cttykqfilter(dev_t dev, struct knote *kn
struct vnode *ttyvp = cttyvp(curproc);
if (ttyvp == NULL) {
- if (kn->kn_flags & __EV_POLL)
+ if (kn->kn_flags & (__EV_POLL | __EV_SELECT))
return (seltrue_kqfilter(dev, kn));
return (ENXIO);
}
Index: miscfs/fifofs/fifo_vnops.c
===
RCS file: src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.86
diff -u -p -r1.86 fifo_vnops.c
--- miscfs/fifofs/fifo_vnops.c 8 Dec 2021 13:03:52 - 1.86
+++ miscfs/fifofs/fifo_vnops.c 9 Dec 2021 14:59:41 -
@@ -531,7 +531,7 @@ fifo_kqfilter(void *v)
case EVFILT_WRITE:
if (!(ap->a_fflag & FWRITE)) {
/* Tell upper layer to ask for POLLUP only */
- if (ap->a_kn->kn_flags & __EV_POLL)
+ if (ap->a_kn->kn_flags & (__EV_POLL | __EV_SELECT))
return (EPERM);
return (EINVAL);
}
Index: miscfs/fuse/fuse_vnops.c
===
RCS file: src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.64
diff -u -p -r1.64 fuse_vnops.c
--- miscfs/fuse/fuse_vnops.c2 Oct 2021 17:29:28
Re: sndiod: -F does not switch back to preferred device
On Thu, Dec 09, 2021 at 02:04:06PM +0100, Solene Rapenne wrote: > > where does sndioctl server.device= come from? > > on my system I don't have the server.device property > There's one server.device knob for each "-s" to controls on which device are player (or recorded from). > > sndioctl server.device > server.device: no such control > > I use this line in rc.conf.local > sndiod_flags=-f rsnd/0 -m play,rec,mon -s rec -s mon -F rsnd/1 > > Do you have an AUDIODEVICE environment variable set? If so, could you verify that it's set to "snd/rec", "snd/mon" or "snd/default"? These are supposed to work: sndioctl -f snd/mon server.device sndioctl -f snd/rec server.device sndioctl -f snd/default server.device While these are not: sndioctl -f snd/0 server.device sndioctl -f snd/1 server.device because the 0 or 1 _is_ the device, so there's no device to choose. (this applies to -current, above were added after release)
Re: net write in pcb hash
On 08/12/21(Wed) 22:39, Alexander Bluhm wrote:
> On Wed, Dec 08, 2021 at 03:28:34PM -0300, Martin Pieuchot wrote:
> > On 04/12/21(Sat) 01:02, Alexander Bluhm wrote:
> > > Hi,
> > >
> > > As I want a read-only net lock for forwarding, all paths should be
> > > checked for the correct net lock and asserts. I found code in
> > > in_pcbhashlookup() where reading the PCB table results in a write
> > > to optimize the cache.
> > >
> > > Porperly protecting PCB hashes is out of scope for parallel forwarding.
> > > Can we get away with this hack? Only update the cache when we are
> > > in TCP oder UDP stack with the write lock. The access from pf is
> > > read-only.
> > >
> > > NET_WLOCKED() indicates whether we may change data structures.
> >
> > I recall that we currently do not want to introduce such idiom: change
> > the behavior of the code depending on which lock is held by the caller.
> >
> > Can we instead assert that a write-lock is held before modifying the
> > list?
>
> We could also pass down the kind of lock that is used. Goal is
> that pf uses shared net lock. TCP and UDP will keep the exclusice
> net lock for a while.
Changing the logic of a function based on the type of a lock is not
different from the previous approach.
> Diff gets longer but perhaps a bit clearer what is going on.
I believe we want to split in_pcblookup_listen() into two functions.
One which is read-only and one which modifies the head of the hash
chain.
The read-only asserts for any lock and the one that modifies the hash
calls the former and assert for a write lock.
Alternatively, we could protect the PCB hash with a mutex. This has the
advantage of not making the scope of the NET_LOCK() more complex. In
the end we all know something like that will be done. I don't know how
other BSD did this but I'm sure this will help getting the remaining
socket layer out of the NET_LOCK().
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1122
> diff -u -p -r1.1122 pf.c
> --- net/pf.c 7 Jul 2021 18:38:25 - 1.1122
> +++ net/pf.c 8 Dec 2021 21:16:16 -
> @@ -3317,14 +3317,12 @@ pf_socket_lookup(struct pf_pdesc *pd)
> sport = pd->hdr.tcp.th_sport;
> dport = pd->hdr.tcp.th_dport;
> PF_ASSERT_LOCKED();
> - NET_ASSERT_LOCKED();
> tb =
> break;
> case IPPROTO_UDP:
> sport = pd->hdr.udp.uh_sport;
> dport = pd->hdr.udp.uh_dport;
> PF_ASSERT_LOCKED();
> - NET_ASSERT_LOCKED();
> tb =
> break;
> default:
> @@ -3348,22 +3346,24 @@ pf_socket_lookup(struct pf_pdesc *pd)
>* Fails when rtable is changed while evaluating the ruleset
>* The socket looked up will not match the one hit in the end.
>*/
> - inp = in_pcbhashlookup(tb, saddr->v4, sport, daddr->v4, dport,
> - pd->rdomain);
> + NET_ASSERT_LOCKED();
> + inp = in_pcbhashlookup_wlocked(tb, saddr->v4, sport, daddr->v4,
> + dport, pd->rdomain, 0);
> if (inp == NULL) {
> - inp = in_pcblookup_listen(tb, daddr->v4, dport,
> - NULL, pd->rdomain);
> + inp = in_pcblookup_listen_wlocked(tb, daddr->v4, dport,
> + NULL, pd->rdomain, 0);
> if (inp == NULL)
> return (-1);
> }
> break;
> #ifdef INET6
> case AF_INET6:
> - inp = in6_pcbhashlookup(tb, >v6, sport, >v6,
> - dport, pd->rdomain);
> + NET_ASSERT_LOCKED();
> + inp = in6_pcbhashlookup_wlocked(tb, >v6, sport,
> + >v6, dport, pd->rdomain, 0);
> if (inp == NULL) {
> - inp = in6_pcblookup_listen(tb, >v6, dport,
> - NULL, pd->rdomain);
> + inp = in6_pcblookup_listen_wlocked(tb, >v6,
> + dport, NULL, pd->rdomain, 0);
> if (inp == NULL)
> return (-1);
> }
> Index: netinet/in_pcb.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.256
> diff -u -p -r1.256 in_pcb.c
> --- netinet/in_pcb.c 25 Oct 2021 22:20:47 - 1.256
> +++ netinet/in_pcb.c 8 Dec 2021 21:16:16 -
> @@ -1051,6 +1051,15 @@ in_pcbresize(struct inpcbtable *table, i
> int in_pcbnotifymiss = 0;
> #endif
>
> +struct inpcb *
> +in_pcbhashlookup(struct inpcbtable *table, struct in_addr faddr,
> +u_int fport_arg, struct in_addr laddr, u_int lport_arg, u_int rtable)
> +{
> + NET_ASSERT_WLOCKED();
> + return
Re: sndiod: -F does not switch back to preferred device
On Thu, Dec 09, 2021 at 10:21:56AM +0100, Alexandre Ratchov wrote:
> On Wed, Dec 08, 2021 at 10:30:08PM +, Klemens Nanni wrote:
> > Following https://www.openbsd.org/faq/faq13.html#usbaudio and reading
> > sndiod(8)'s
> >
> > -F device
> > Specify an alternate device to use. If it doesn't work, the one
> > given with the last -f or -F options will be used. For
> > instance,
> > specifying a USB device following a PCI device allows sndiod to
> > use the USB one preferably when it's connected and to fall back
> > to the PCI one when it's disconnected. Alternate devices may be
> > switched with the server.device control of the sndioctl(1)
> > utility.
> >
> > I configured things as follows in order to play audio via USB and
> > fall back to internal sound if USB is not available:
> >
> > $ rcctl get sndiod flags
> > -f rsnd/0 -F rsnd/1
> >
> > Plugging in an USB headset and restarting sndiod or forcing the device
> > with `sndioctl server.device=1' then plays sound via USB.
> >
> > Unplugging the device makes playback fall back to internal sound.
> >
> > But plugging USB back in does not prefer USB to internal as I'd expect
> > now. I am currently resorting to the following hotplugd(8) script to
> > always select the USB sound device whenever I plug it in:
> >
> > #!/bin/ksh
> > set -Cefu -o pipefail
> >
> > readonly DEVCLASS=$1 DEVNAME=$2
> > typeset -i devid
> >
> > case "${DEVCLASS}-${DEVNAME}" in
> > 0-audio*) # switch sndio(4) to USB headset when plugging it in
> > devid=${DEVNAME#audio}
> > sndioctl server.device=${devid}
> > ;;
> > esac
> >
>
> IFAIU, audio devices always match the audio[0-9] pattern, so testing
> the device class is not necessary, is it? FWIW, I use a similar
> script.
>
> > I'd expect sndiod to *always* use USB whenever possible.
> >
> > Is this uni-directional behaviour of sndiod intentional/by-design?
>
> sndiod doesn't use directly hotplug(4), so it can't obtain by itself a
> notification when a new device is plugged in order to switch to
> it. Using hotplugd(8) is the right way to make audio hotplug work,
> both daemons complete each other.
>
> When sndiod needs the audio device (i.e. when the first client
> connects), it will try all alternate devices (reverse -F options
> order, the USB headset is first in your case). This gives a false
> impression of hotplug support (indeed, you plug the USB headset, start
> an audio player and it just works!). But that's just a side effect of
> device priority/fail-over: if the internal device is already open when
> you plug the USB headset, sndiod won't switch to it until it needs to
> reopen the device.
>
> Certain programs (including browsers), tend to keep the device open
> even when they remain silent. So when actual playback starts sndiod
> doesn't need to open a device and doesn't "see" the new USB headset. I
> guess that's what happens on your system, but hotplugd(8) handles
> this.
>
> When devices are unplugged, we don't need hotplug because the device
> stops working (input/output error) and sndiod switch to the next one
> of the fail-over list.
>
> > If so, can we clarify the manual?
>
> Sure. While -F option description seems exact, maybe we need an extra
> paragraph or FAQ entry to explain how to use it with other tools like
> hotplugd and sndioctl. What about this wording?
>
> HOT-PLUG SUPPORT
> If a device is unplugged while in use, sndiod will attempt to switch to
> one of the alternate devices (-F), if any. This is seamless to programs
> connected to sndiod.
>
> Later, when the device is connected again, the server.device control of
> the sndioctl(1) utility could be used to switch back to it, without the
> need to restart all audio programs. This last step could be automated
> using hotplugd(8). For instance, if sndiod is started with:
>
>$ sndiod -f rsnd/0 -F rsnd/1 -F rsnd/2 -F rsnd/3
>
> then, the following hotplugd(8) attach script could be used to
> automatically switch to the last connected device:
>
>#!/bin/sh
>
>DEVNAME=$2
>
>case $DEVNAME in
>audio[0-3])
>sndioctl server.device=${DEVNAME#audio}
>;;
>esac
>
> Any opinions where to put such information?
>
hi.
i guess the info is more relevant to sndiod. you could just add your
text to the existing EXAMPLES section i think.
jmc
Re: sndiod: -F does not switch back to preferred device
On Wed, Dec 08, 2021 at 10:30:08PM +, Klemens Nanni wrote:
> Following https://www.openbsd.org/faq/faq13.html#usbaudio and reading
> sndiod(8)'s
>
> -F device
> Specify an alternate device to use. If it doesn't work, the one
> given with the last -f or -F options will be used. For
> instance,
> specifying a USB device following a PCI device allows sndiod to
> use the USB one preferably when it's connected and to fall back
> to the PCI one when it's disconnected. Alternate devices may be
> switched with the server.device control of the sndioctl(1)
> utility.
>
> I configured things as follows in order to play audio via USB and
> fall back to internal sound if USB is not available:
>
> $ rcctl get sndiod flags
> -f rsnd/0 -F rsnd/1
>
> Plugging in an USB headset and restarting sndiod or forcing the device
> with `sndioctl server.device=1' then plays sound via USB.
>
> Unplugging the device makes playback fall back to internal sound.
>
> But plugging USB back in does not prefer USB to internal as I'd expect
> now. I am currently resorting to the following hotplugd(8) script to
> always select the USB sound device whenever I plug it in:
>
> #!/bin/ksh
> set -Cefu -o pipefail
>
> readonly DEVCLASS=$1 DEVNAME=$2
> typeset -i devid
>
> case "${DEVCLASS}-${DEVNAME}" in
> 0-audio*) # switch sndio(4) to USB headset when plugging it in
> devid=${DEVNAME#audio}
> sndioctl server.device=${devid}
> ;;
> esac
>
IFAIU, audio devices always match the audio[0-9] pattern, so testing
the device class is not necessary, is it? FWIW, I use a similar
script.
> I'd expect sndiod to *always* use USB whenever possible.
>
> Is this uni-directional behaviour of sndiod intentional/by-design?
sndiod doesn't use directly hotplug(4), so it can't obtain by itself a
notification when a new device is plugged in order to switch to
it. Using hotplugd(8) is the right way to make audio hotplug work,
both daemons complete each other.
When sndiod needs the audio device (i.e. when the first client
connects), it will try all alternate devices (reverse -F options
order, the USB headset is first in your case). This gives a false
impression of hotplug support (indeed, you plug the USB headset, start
an audio player and it just works!). But that's just a side effect of
device priority/fail-over: if the internal device is already open when
you plug the USB headset, sndiod won't switch to it until it needs to
reopen the device.
Certain programs (including browsers), tend to keep the device open
even when they remain silent. So when actual playback starts sndiod
doesn't need to open a device and doesn't "see" the new USB headset. I
guess that's what happens on your system, but hotplugd(8) handles
this.
When devices are unplugged, we don't need hotplug because the device
stops working (input/output error) and sndiod switch to the next one
of the fail-over list.
> If so, can we clarify the manual?
Sure. While -F option description seems exact, maybe we need an extra
paragraph or FAQ entry to explain how to use it with other tools like
hotplugd and sndioctl. What about this wording?
HOT-PLUG SUPPORT
If a device is unplugged while in use, sndiod will attempt to switch to
one of the alternate devices (-F), if any. This is seamless to programs
connected to sndiod.
Later, when the device is connected again, the server.device control of
the sndioctl(1) utility could be used to switch back to it, without the
need to restart all audio programs. This last step could be automated
using hotplugd(8). For instance, if sndiod is started with:
$ sndiod -f rsnd/0 -F rsnd/1 -F rsnd/2 -F rsnd/3
then, the following hotplugd(8) attach script could be used to
automatically switch to the last connected device:
#!/bin/sh
DEVNAME=$2
case $DEVNAME in
audio[0-3])
sndioctl server.device=${DEVNAME#audio}
;;
esac
Any opinions where to put such information?
Re: [patch] urndis: uncomment watchdog
On Tue, 30 Nov 2021 21:40:35 +0300
Mikhail wrote:
> Currently watchdog functionality for urndis driver is disabled
> (commented), I've tested it and it works properly - reset messages are
> correctly sent and cmplt packets are received according to RNDIS spec
> (I patched the driver to forcedly send reset message and act on it
> with device reset every 5 seconds). I suggest to uncomment it.
>
Well, I am sorry to perhaps hijack the thread, but it seems to me, it
is just your hardware. I am using urndis(4) interface occasionally,
with my cellphone, and it "just works".
so, if your hardware needs special treatment - may be it is better to
make a special case under if(){} branch?
--
With best regards,
Gregory Edigarov
