Re: audioctl: drop useless fields
On Wed, Sep 10, 2014 at 07:31:17PM +0200, Alexandre Ratchov wrote:
audioctl output is full of useless, misleading and/or unreliable
fields. Let's keep the usable ones only. The plan is to remove them
from the kernel as well.
OK?
I've been asked in private to explain the reason I think these
fields are useless/misleading, see comments inlined below.
But the interesting (and difficult) question is why we have these
fields at all. Probably for SunOS compatibility, for shell-scripts
written on SunOS back in 1997 to work on OpenBSD?
Index: audioctl.c
===
RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 audioctl.c
--- audioctl.c13 Nov 2013 18:50:05 - 1.23
+++ audioctl.c10 Sep 2014 12:15:06 -
@@ -80,17 +80,10 @@ struct field {
u_int oldval;
} fields[] = {
{ name, adev.name, STRING, READONLY },
- { version,adev.version, STRING, READONLY },
version is not reliable as not set by all devices, and nobody knows
the version of what this is (driver version? chipset version? API
version?).
- { config, adev.config, STRING, READONLY },
same here, certain devices return the driver/device name (ex.
azalia0), others parts of the chip name (already shown in dmesg).
Neither useful, nor reliable.
{ encodings, encbuf, STRING, READONLY },
{ properties, properties,PROPS, READONLY },
- { full_duplex,fullduplex,UINT, 0 },
- { fullduplex, fullduplex,UINT, 0 },
since ~2009 we don't support non-full-duplex mode; i.e. devices
opened in read-write mode are necessarily used in full-duplex, so
the information is available in the mode field.
- { blocksize, info.blocksize,UINT, 0 },
the block size of what? the play or record one? (hint: they are not
the same). And note that we already have a play.block_size and
record.block_size fields which are useful.
{ hiwat, info.hiwat,UINT, 0 },
{ lowat, info.lowat,UINT, 0 },
- { output_muted, info.output_muted, UCHAR, 0 },
this is an alias for a mixerctl control (outputs.master.mute in
most cases). It may not exist on certain devices, so it's not
reliable.
- { monitor_gain, info.monitor_gain, UINT, 0 },
same here, except that it works on very few devices.
{ mode, info.mode, P_R,READONLY },
{ play.rate, info.play.sample_rate, UINT, 0 },
{ play.sample_rate, info.play.sample_rate, UINT, ALIAS },
@@ -99,19 +92,9 @@ struct field {
{ play.bps, info.play.bps, UINT, 0 },
{ play.msb, info.play.msb, UINT, 0 },
{ play.encoding, info.play.encoding,ENC,0 },
- { play.gain, info.play.gain,UINT, 0 },
- { play.balance, info.play.balance, UCHAR, 0 },
- { play.port, info.play.port,XINT, 0 },
- { play.avail_ports, info.play.avail_ports, XINT, 0 },
ditto, mixerctl controls (when available)
- { play.seek, info.play.seek,UINT, READONLY },
this is not used.
{ play.samples, info.play.samples, UINT, READONLY },
- { play.eof, info.play.eof, UINT, READONLY },
not used
{ play.pause, info.play.pause, UCHAR, 0 },
- { play.error, info.play.error, UCHAR, READONLY },
alias for (play.errors 0) ? 1 : 0
- { play.waiting, info.play.waiting, UCHAR, READONLY },
not used, always 0
- { play.open, info.play.open,UCHAR, READONLY },
this is 1 if the mode field contains play and 0 otherwise; so
this is a redundant, let's drop it.
{ play.active,info.play.active, UCHAR, READONLY },
- { play.buffer_size, info.play.buffer_size, UINT, 0 },
misleading. Always contains 65536 which is the size of DMA-able
memory block. This is not the audio ring buffer size as the name
would suggest (which is smaller and must be multiple of the frame
size).
- { record.gain,info.record.gain, UINT, 0 },
- { record.balance, info.record.balance, UCHAR, 0 },
- { record.port,info.record.port, XINT, 0 },
- { record.avail_ports, info.record.avail_ports,XINT, 0 },
- { record.seek,info.record.seek, UINT, READONLY },
- { record.eof, info.record.eof, UINT, READONLY },
- { record.error, info.record.error, UCHAR, READONLY },
- { record.waiting, info.record.waiting, UCHAR, READONLY },
- { record.open,info.record.open, UCHAR, READONLY },
- {
Re: splnet() and SIOCSIFADDR
On 03/09/14(Wed) 20:59, Alexander Bluhm wrote:
On Wed, Sep 03, 2014 at 03:53:34PM +0200, Martin Pieuchot wrote:
@@ -1078,7 +1079,7 @@ in6_purgeaddr(struct ifaddr *ifa)
void
in6_unlink_ifa(struct in6_ifaddr *ia6, struct ifnet *ifp)
{
- int s = splnet();
+ splsoftassert(IPL_SOFTNET);
ifa_del(ifp, ia6-ia_ifa);
I think there are code paths that can trigger this assertion
netinet6/in6.c: in6_unlink_ifa()
netinet6/in6.c: in6_purgeaddr()
netinet6/nd6_rtr.c: purge_detached()
netinet6/nd6_rtr.c: nd6_prelist_add()
netinet6/in6.c: in6_control()
netinet/tcp_usrreq.c: tcp_usrreq()
kern/sys_socket.c:soo_ioctl()
netinet6/in6.c: in6_unlink_ifa()
netinet6/in6.c: in6_purgeaddr()
netinet6/nd6_rtr.c: purge_detached()
netinet6/nd6_rtr.c: nd6_prelist_add()
netinet6/in6_ifattach.c: in6_ifattach_linklocal()
netinet/ip_carp.c carp_set_enaddr()
netinet/ip_carp.c carp_ioctl()
...
nd6_prelist_add() does some splsoftnet() already. I think you
should put one around purge_detached() there.
Nice catch, updated diff below.
Index: net/if_loop.c
===
RCS file: /home/ncvs/src/sys/net/if_loop.c,v
retrieving revision 1.57
diff -u -p -r1.57 if_loop.c
--- net/if_loop.c 22 Jul 2014 11:06:09 - 1.57
+++ net/if_loop.c 11 Sep 2014 08:45:29 -
@@ -288,15 +288,13 @@ loioctl(struct ifnet *ifp, u_long cmd, c
{
struct ifaddr *ifa;
struct ifreq *ifr;
- int s, error = 0;
+ int error = 0;
switch (cmd) {
case SIOCSIFADDR:
- s = splnet();
ifp-if_flags |= IFF_RUNNING;
if_up(ifp); /* send up RTM_IFINFO */
- splx(s);
ifa = (struct ifaddr *)data;
if (ifa != 0)
Index: netinet/in.c
===
RCS file: /home/ncvs/src/sys/netinet/in.c,v
retrieving revision 1.103
diff -u -p -r1.103 in.c
--- netinet/in.c3 Sep 2014 08:59:06 - 1.103
+++ netinet/in.c11 Sep 2014 08:45:29 -
@@ -612,7 +612,7 @@ in_ifinit(struct ifnet *ifp, struct in_i
{
u_int32_t i = sin-sin_addr.s_addr;
struct sockaddr_in oldaddr;
- int s, error = 0;
+ int error = 0;
splsoftassert(IPL_SOFTNET);
@@ -627,7 +627,6 @@ in_ifinit(struct ifnet *ifp, struct in_i
rt_ifa_delloop(ia-ia_ifa);
ifa_del(ifp, ia-ia_ifa);
}
- s = splnet();
oldaddr = ia-ia_addr;
ia-ia_addr = *sin;
@@ -639,10 +638,8 @@ in_ifinit(struct ifnet *ifp, struct in_i
if (ifp-if_ioctl
(error = (*ifp-if_ioctl)(ifp, SIOCSIFADDR, (caddr_t)ia))) {
ia-ia_addr = oldaddr;
- splx(s);
goto out;
}
- splx(s);
if (ia-ia_netmask == 0) {
if (IN_CLASSA(i))
Index: netinet/ip_carp.c
===
RCS file: /home/ncvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.234
diff -u -p -r1.234 ip_carp.c
--- netinet/ip_carp.c 8 Sep 2014 06:24:13 - 1.234
+++ netinet/ip_carp.c 11 Sep 2014 08:45:29 -
@@ -2059,10 +2059,11 @@ carp_ioctl(struct ifnet *ifp, u_long cmd
struct ifaddr *ifa = (struct ifaddr *)addr;
struct ifreq *ifr = (struct ifreq *)addr;
struct ifnet *cdev = NULL;
- int i, error = 0;
+ int s, i, error = 0;
switch (cmd) {
case SIOCSIFADDR:
+ s = splnet();
switch (ifa-ifa_addr-sa_family) {
#ifdef INET
case AF_INET:
@@ -2087,6 +2088,7 @@ carp_ioctl(struct ifnet *ifp, u_long cmd
break;
}
break;
+ splx(s);
case SIOCSIFFLAGS:
vhe = LIST_FIRST(sc-carp_vhosts);
Index: netinet6/in6.c
===
RCS file: /home/ncvs/src/sys/netinet6/in6.c,v
retrieving revision 1.140
diff -u -p -r1.140 in6.c
--- netinet6/in6.c 26 Aug 2014 21:44:29 - 1.140
+++ netinet6/in6.c 11 Sep 2014 08:45:29 -
@@ -172,7 +172,7 @@ in6_control(struct socket *so, u_long cm
struct in6_ifaddr *ia6 = NULL;
struct in6_aliasreq *ifra = (struct in6_aliasreq *)data;
struct sockaddr_in6 *sa6;
- int privileged;
+ int s, privileged;
privileged = 0;
if ((so-so_state SS_PRIV) != 0)
@@ -463,7 +463,6 @@ in6_control(struct socket *so, u_long cm
{
int i, error = 0;
struct nd_prefix pr0, *pr;
- int s;
/* reject read-only flags */
if ((ifra-ifra_flags
Re: splnet() and SIOCSIFADDR
On 03/09/14(Wed) 23:59, Claudio Jeker wrote:
On Wed, Sep 03, 2014 at 03:25:34PM +0200, Martin Pieuchot wrote:
Drivers that need a splnet() protection inside their SIOCSIFADDR
generally raise the spl level themselves, so we should not need
to do that in in{6,}_ifinit(). One exception to this rule is,
as always, carp(4)...
So the diff below moves the spl dance inside carp's SIOCSIFADDR
handler, it's a baby step, so we can take care of carp_set_addr{6,}
later.
Hmm. My gut feeling is that this is scary. Calling ifp functions at
non-splnet is always a cause for troubles (e.g. the watchdog callbacks
miss often the needed splnet calls).
I think the ioctl code pathes in the various places may suffer from
similar problems with missing splnet() calls. Guess a bit of an audit is
needed in that area.
And the audit revealed that only lo(4) has an unneeded splnet() call.
Something else I wonder is if splsoftnet() is good enough when dealing
with ifp in general (aren't we detaching interfaces at splnet())?
Removable interfaces opened for sure a bucket sized can of worms.
I believe it is, or at least it should be. splnet() should only be
necessary inside drivers to protect the code that might interfere with
configuring the hardware or messing with the rings.
The rational is quite simple: packets are processed in softnet, so the
pieces of code modifying the global lists and tables should make sure
code run at softnet cannot be executed in parallel.
The fact that some of the operations are still done under splnet(), like
detatching interfaces, seems related to history. I'm trying to carefully
remove the unneeded splnet()s. One of the biggest problem was the routing
table update triggered by a link state change that *was* executed during a
IPL_NET interrupt. But that's no longer the case.
Re: audioctl: drop useless fields
On 09/11/14 09:58, Alexandre Ratchov wrote:
On Wed, Sep 10, 2014 at 07:31:17PM +0200, Alexandre Ratchov wrote:
audioctl output is full of useless, misleading and/or unreliable
fields. Let's keep the usable ones only. The plan is to remove them
from the kernel as well.
OK?
I've been asked in private to explain the reason I think these
fields are useless/misleading, see comments inlined below.
I love it. Now, when is mixerctl becoming grokable? ;-)
/Alexander
But the interesting (and difficult) question is why we have these
fields at all. Probably for SunOS compatibility, for shell-scripts
written on SunOS back in 1997 to work on OpenBSD?
Index: audioctl.c
===
RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 audioctl.c
--- audioctl.c 13 Nov 2013 18:50:05 - 1.23
+++ audioctl.c 10 Sep 2014 12:15:06 -
@@ -80,17 +80,10 @@ struct field {
u_int oldval;
} fields[] = {
{ name, adev.name, STRING, READONLY },
- { version, adev.version, STRING, READONLY },
version is not reliable as not set by all devices, and nobody knows
the version of what this is (driver version? chipset version? API
version?).
- { config, adev.config, STRING, READONLY },
same here, certain devices return the driver/device name (ex.
azalia0), others parts of the chip name (already shown in dmesg).
Neither useful, nor reliable.
{ encodings,encbuf, STRING, READONLY
},
{ properties, properties,PROPS,
READONLY },
- { full_duplex, fullduplex,UINT, 0 },
- { fullduplex, fullduplex,UINT, 0 },
since ~2009 we don't support non-full-duplex mode; i.e. devices
opened in read-write mode are necessarily used in full-duplex, so
the information is available in the mode field.
- { blocksize,info.blocksize,UINT, 0 },
the block size of what? the play or record one? (hint: they are not
the same). And note that we already have a play.block_size and
record.block_size fields which are useful.
{ hiwat,info.hiwat,UINT, 0 },
{ lowat,info.lowat,UINT, 0 },
- { output_muted, info.output_muted, UCHAR, 0 },
this is an alias for a mixerctl control (outputs.master.mute in
most cases). It may not exist on certain devices, so it's not
reliable.
- { monitor_gain, info.monitor_gain, UINT, 0 },
same here, except that it works on very few devices.
{ mode, info.mode, P_R,READONLY },
{ play.rate,info.play.sample_rate, UINT, 0 },
{ play.sample_rate, info.play.sample_rate, UINT, ALIAS },
@@ -99,19 +92,9 @@ struct field {
{ play.bps, info.play.bps, UINT, 0 },
{ play.msb, info.play.msb, UINT, 0 },
{ play.encoding,info.play.encoding,ENC,0 },
- { play.gain,info.play.gain,UINT, 0 },
- { play.balance, info.play.balance, UCHAR, 0 },
- { play.port,info.play.port,XINT, 0 },
- { play.avail_ports, info.play.avail_ports, XINT, 0 },
ditto, mixerctl controls (when available)
- { play.seek,info.play.seek,UINT, READONLY },
this is not used.
{ play.samples, info.play.samples, UINT, READONLY },
- { play.eof, info.play.eof, UINT, READONLY },
not used
{ play.pause, info.play.pause, UCHAR, 0 },
- { play.error, info.play.error, UCHAR, READONLY },
alias for (play.errors 0) ? 1 : 0
- { play.waiting, info.play.waiting, UCHAR, READONLY },
not used, always 0
- { play.open,info.play.open,UCHAR, READONLY },
this is 1 if the mode field contains play and 0 otherwise; so
this is a redundant, let's drop it.
{ play.active, info.play.active, UCHAR, READONLY },
- { play.buffer_size, info.play.buffer_size, UINT, 0 },
misleading. Always contains 65536 which is the size of DMA-able
memory block. This is not the audio ring buffer size as the name
would suggest (which is smaller and must be multiple of the frame
size).
- { record.gain, info.record.gain, UINT, 0 },
- { record.balance, info.record.balance, UCHAR, 0 },
- { record.port, info.record.port, XINT, 0 },
- { record.avail_ports, info.record.avail_ports,XINT, 0 },
- { record.seek, info.record.seek, UINT, READONLY },
- { record.eof, info.record.eof, UINT, READONLY },
- { record.error,
Re: audioctl: drop useless fields
On Thu, Sep 11, 2014 at 11:54:08AM +0200, Alexander Hall wrote: On 09/11/14 09:58, Alexandre Ratchov wrote: On Wed, Sep 10, 2014 at 07:31:17PM +0200, Alexandre Ratchov wrote: audioctl output is full of useless, misleading and/or unreliable fields. Let's keep the usable ones only. The plan is to remove them from the kernel as well. OK? I've been asked in private to explain the reason I think these fields are useless/misleading, see comments inlined below. I love it. Now, when is mixerctl becoming grokable? ;-) Just use audio/cmixer :p Landry
Re: audioctl: drop useless fields
ok armani@
2014-09-10 19:31 GMT+02:00 Alexandre Ratchov a...@caoua.org:
audioctl output is full of useless, misleading and/or unreliable
fields. Let's keep the usable ones only. The plan is to remove them
from the kernel as well.
OK?
Index: audioctl.c
===
RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 audioctl.c
--- audioctl.c 13 Nov 2013 18:50:05 - 1.23
+++ audioctl.c 10 Sep 2014 12:15:06 -
@@ -80,17 +80,10 @@ struct field {
u_int oldval;
} fields[] = {
{ name, adev.name, STRING, READONLY
},
- { version,adev.version, STRING, READONLY },
- { config, adev.config, STRING, READONLY },
{ encodings, encbuf, STRING, READONLY },
{ properties, properties,PROPS, READONLY },
- { full_duplex,fullduplex,UINT, 0 },
- { fullduplex, fullduplex,UINT, 0 },
- { blocksize, info.blocksize,UINT, 0 },
{ hiwat, info.hiwat,UINT, 0 },
{ lowat, info.lowat,UINT, 0 },
- { output_muted, info.output_muted, UCHAR, 0 },
- { monitor_gain, info.monitor_gain, UINT, 0 },
{ mode, info.mode, P_R,READONLY },
{ play.rate, info.play.sample_rate, UINT, 0 },
{ play.sample_rate, info.play.sample_rate, UINT, ALIAS },
@@ -99,19 +92,9 @@ struct field {
{ play.bps, info.play.bps, UINT, 0 },
{ play.msb, info.play.msb, UINT, 0 },
{ play.encoding, info.play.encoding,ENC,0 },
- { play.gain, info.play.gain,UINT, 0 },
- { play.balance, info.play.balance, UCHAR, 0 },
- { play.port, info.play.port,XINT, 0 },
- { play.avail_ports, info.play.avail_ports, XINT, 0 },
- { play.seek, info.play.seek,UINT, READONLY },
{ play.samples, info.play.samples, UINT, READONLY },
- { play.eof, info.play.eof, UINT, READONLY },
{ play.pause, info.play.pause, UCHAR, 0 },
- { play.error, info.play.error, UCHAR, READONLY },
- { play.waiting, info.play.waiting, UCHAR, READONLY },
- { play.open, info.play.open,UCHAR, READONLY },
{ play.active,info.play.active, UCHAR, READONLY },
- { play.buffer_size, info.play.buffer_size, UINT, 0 },
{ play.block_size,info.play.block_size, UINT, 0 },
{ play.errors,perrors, INT,READONLY },
{ record.rate,info.record.sample_rate,UINT, 0 },
@@ -121,19 +104,9 @@ struct field {
{ record.bps, info.record.bps, UINT, 0 },
{ record.msb, info.record.msb, UINT, 0 },
{ record.encoding,info.record.encoding, ENC,0 },
- { record.gain,info.record.gain, UINT, 0 },
- { record.balance, info.record.balance, UCHAR, 0 },
- { record.port,info.record.port, XINT, 0 },
- { record.avail_ports, info.record.avail_ports,XINT, 0 },
- { record.seek,info.record.seek, UINT, READONLY },
{ record.samples, info.record.samples, UINT, READONLY },
- { record.eof, info.record.eof, UINT, READONLY },
{ record.pause, info.record.pause, UCHAR, 0 },
- { record.error, info.record.error, UCHAR, READONLY },
- { record.waiting, info.record.waiting, UCHAR, READONLY },
- { record.open,info.record.open, UCHAR, READONLY },
{ record.active, info.record.active,UCHAR, READONLY },
- { record.buffer_size, info.record.buffer_size,UINT, 0 },
{ record.block_size, info.record.block_size,UINT, 0 },
{ record.errors, rerrors, INT,READONLY },
{ 0 }
Re: audioctl: drop useless fields
I like it. ok shadchin@
On Wed, Sep 10, 2014 at 10:31 PM, Alexandre Ratchov a...@caoua.org wrote:
audioctl output is full of useless, misleading and/or unreliable
fields. Let's keep the usable ones only. The plan is to remove them
from the kernel as well.
OK?
Index: audioctl.c
===
RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 audioctl.c
--- audioctl.c 13 Nov 2013 18:50:05 - 1.23
+++ audioctl.c 10 Sep 2014 12:15:06 -
@@ -80,17 +80,10 @@ struct field {
u_int oldval;
} fields[] = {
{ name, adev.name, STRING, READONLY
},
- { version,adev.version, STRING, READONLY },
- { config, adev.config, STRING, READONLY },
{ encodings, encbuf, STRING, READONLY },
{ properties, properties,PROPS, READONLY },
- { full_duplex,fullduplex,UINT, 0 },
- { fullduplex, fullduplex,UINT, 0 },
- { blocksize, info.blocksize,UINT, 0 },
{ hiwat, info.hiwat,UINT, 0 },
{ lowat, info.lowat,UINT, 0 },
- { output_muted, info.output_muted, UCHAR, 0 },
- { monitor_gain, info.monitor_gain, UINT, 0 },
{ mode, info.mode, P_R,READONLY },
{ play.rate, info.play.sample_rate, UINT, 0 },
{ play.sample_rate, info.play.sample_rate, UINT, ALIAS },
@@ -99,19 +92,9 @@ struct field {
{ play.bps, info.play.bps, UINT, 0 },
{ play.msb, info.play.msb, UINT, 0 },
{ play.encoding, info.play.encoding,ENC,0 },
- { play.gain, info.play.gain,UINT, 0 },
- { play.balance, info.play.balance, UCHAR, 0 },
- { play.port, info.play.port,XINT, 0 },
- { play.avail_ports, info.play.avail_ports, XINT, 0 },
- { play.seek, info.play.seek,UINT, READONLY },
{ play.samples, info.play.samples, UINT, READONLY },
- { play.eof, info.play.eof, UINT, READONLY },
{ play.pause, info.play.pause, UCHAR, 0 },
- { play.error, info.play.error, UCHAR, READONLY },
- { play.waiting, info.play.waiting, UCHAR, READONLY },
- { play.open, info.play.open,UCHAR, READONLY },
{ play.active,info.play.active, UCHAR, READONLY },
- { play.buffer_size, info.play.buffer_size, UINT, 0 },
{ play.block_size,info.play.block_size, UINT, 0 },
{ play.errors,perrors, INT,READONLY },
{ record.rate,info.record.sample_rate,UINT, 0 },
@@ -121,19 +104,9 @@ struct field {
{ record.bps, info.record.bps, UINT, 0 },
{ record.msb, info.record.msb, UINT, 0 },
{ record.encoding,info.record.encoding, ENC,0 },
- { record.gain,info.record.gain, UINT, 0 },
- { record.balance, info.record.balance, UCHAR, 0 },
- { record.port,info.record.port, XINT, 0 },
- { record.avail_ports, info.record.avail_ports,XINT, 0 },
- { record.seek,info.record.seek, UINT, READONLY },
{ record.samples, info.record.samples, UINT, READONLY },
- { record.eof, info.record.eof, UINT, READONLY },
{ record.pause, info.record.pause, UCHAR, 0 },
- { record.error, info.record.error, UCHAR, READONLY },
- { record.waiting, info.record.waiting, UCHAR, READONLY },
- { record.open,info.record.open, UCHAR, READONLY },
{ record.active, info.record.active,UCHAR, READONLY },
- { record.buffer_size, info.record.buffer_size,UINT, 0 },
{ record.block_size, info.record.block_size,UINT, 0 },
{ record.errors, rerrors, INT,READONLY },
{ 0 }
--
Alexandr Shadchin
Re: PATCH: fix iwn(4) scan hangs
On Wed, Sep 10, 2014 at 08:52:42PM +0200, Marcin Piotr Pawlowski wrote: Hi, On 09/10/14 20:19, Fabian Raetz wrote: On Wed, Sep 10, 2014 at 02:42:43PM +0200, Marcin Piotr Pawlowski wrote: Yes, I think that it could be is possible to double clean the node cache. Updated diff with suggestion from Stefan. Hi, this patch works for me too. But is the problem not specific to the iwn(4) driver or at least only specific to drivers which override the default state machine and not calling into the original state machine when IEEE80211_S_SCAN is entered? ieee80211_begin_scan() calls ieee80211_free_allnodes() which prevents the problem for drivers which use the default scanning infrastructure. So would it make sense to move the call to ieee80211_clean_cached() into iwn(4) like in the modified patch below? Marcin pointed me to some other driver that could benefit from this and that iwn(4) is not that special as i thought, so i am ok with his original patch too (fwiw). Regards, Fabian I experimented a bit with calling ieee80211_free_allnodes() here as it is called in default scanning infrastructure but this does not work for me (yet) :) The ieee80211_begin_scan is used by ieee80211_newstate() but ifconfig scan is done by ioctl(SIOCS80211SCAN) which is processes by the ieee80211_ioctl() that then invokes ieee80211_new_state(). So both paths are independent and this issue is not specific to the iwn driver. I hope that I have not overlooked something. Best regards, mpp
Re: apmd hangs
On 09/08/14 23:35, Mark Kettenis wrote: The more code documentation I read, the more I'm convinced that coordinating state changes between logical processors isn't necessary and actually is responsible for the hangs people have been seeing. So here is a diff that does away with it all. I've tested it on a few laptops here, but it could use testing on a somewhat wider range of machines. I'm especially interested in seeing this tested on a dual socket machine with apmd -A. finally no more hangs on this machine running with apmd -A Cheers Giovanni OpenBSD 5.6-current (GENERIC.MP) #9: Wed Sep 10 12:33:43 CEST 2014 giova...@bigio.paclan.it:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 3835400192 (3657MB) avail mem = 3724570624 (3552MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xdae3a000 (60 entries) bios0: vendor LENOVO version H5ET69WW (1.12 ) date 11/15/2012 bios0: LENOVO 62742QG acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SLIC SSDT ASF! HPET APIC MCFG FPDT SSDT SSDT UEFI UEFI MSDM UEFI DBG2 acpi0: wakeup devices P0P1(S4) GLAN(S4) EHC1(S3) EHC2(S3) XHC_(S3) HDEF(S4) PXSX(S4) RP01(S4) PXSX(S4) RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i3-2348M CPU @ 2.30GHz, 2295.13 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE cpu1 at mainbus0: apid 1 (application processor) cpu1: Intel(R) Core(TM) i3-2348M CPU @ 2.30GHz, 2294.79 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 1, core 0, package 0 cpu2 at mainbus0: apid 2 (application processor) cpu2: Intel(R) Core(TM) i3-2348M CPU @ 2.30GHz, 2294.79 MHz cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC cpu2: 256KB 64b/line 8-way L2 cache cpu2: smt 0, core 1, package 0 cpu3 at mainbus0: apid 3 (application processor) cpu3: Intel(R) Core(TM) i3-2348M CPU @ 2.30GHz, 2294.79 MHz cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC cpu3: 256KB 64b/line 8-way L2 cache cpu3: smt 1, core 1, package 0 ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins acpimcfg0 at acpi0 addr 0xf800, bus 0-63 acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (P0P1) acpiprt2 at acpi0: bus 1 (RP01) acpiprt3 at acpi0: bus 2 (RP02) acpiprt4 at acpi0: bus -1 (RP03) acpiprt5 at acpi0: bus 3 (RP04) acpiprt6 at acpi0: bus -1 (RP05) acpiprt7 at acpi0: bus -1 (RP06) acpiprt8 at acpi0: bus -1 (RP07) acpiprt9 at acpi0: bus -1 (RP08) acpiprt10 at acpi0: bus -1 (PEG0) acpiprt11 at acpi0: bus -1 (PEG1) acpiprt12 at acpi0: bus -1 (PEG2) acpiprt13 at acpi0: bus -1 (PEG3) acpiec0 at acpi0 acpicpu0 at acpi0: C3, C2, C1, PSS acpicpu1 at acpi0: C3, C2, C1, PSS acpicpu2 at acpi0: C3, C2, C1, PSS acpicpu3 at acpi0: C3, C2, C1, PSS acpipwrres0 at acpi0: PUBS acpitz0 at acpi0: critical temperature is 83 degC acpitz1 at acpi0: critical temperature is 126 degC acpibat0 at acpi0: BAT0 model 45N1045 serial 2329 type LION oem 313100504d53 acpithinkpad0 at acpi0 acpiac0 at acpi0: AC unit offline acpibtn0 at acpi0: LID0 acpibtn1 at acpi0: SLPB cpu0: Enhanced SpeedStep 2295 MHz: speeds: 2300, 2200, 2100, 2000, 1900, 1800, 1700, 1600, 1500, 1400, 1300, 1200, 1100, 1000, 900, 800 MHz pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 Intel Core 2G Host rev 0x09 vga1 at pci0 dev 2 function 0 Intel HD Graphics 3000 rev 0x09 intagp at vga1 not configured inteldrm0 at vga1 drm0 at inteldrm0 drm: Memory usable by graphics device = 2048M inteldrm0: 1366x768 wsdisplay0 at vga1 mux 1: console (std, vt100 emulation) wsdisplay0: screen 1-5 added (std, vt100 emulation) Intel 7 Series MEI rev 0x04 at pci0 dev 22 function 0 not configured ehci0 at pci0 dev 26
Re: improve ressl config setting
On Wed, Sep 10, 2014 at 16:38, Ted Unangst wrote:
On Fri, Aug 15, 2014 at 13:06, Ted Unangst wrote:
Instead, ressl should copy all parameters as necessary and
free them. This does introduce an error case into formerly void
functions, but I think that's ok. The alternative would be to use
fixed arrays inside ressl_config, but that seems much worse.
Here's a complete diff.
(I think we should also zero the key memory if not null, but that's not
included in this change.)
Kent Spillner noticed I hadn't cleaned up the references to default
config in ressl.c. Here's a fixed diff, that also zeroes key memory.
Index: ressl.c
===
RCS file: /cvs/src/lib/libressl/ressl.c,v
retrieving revision 1.12
diff -u -p -r1.12 ressl.c
--- ressl.c 15 Aug 2014 16:55:32 - 1.12
+++ ressl.c 11 Sep 2014 19:18:49 -
@@ -29,7 +29,7 @@
#include ressl.h
#include ressl_internal.h
-extern struct ressl_config ressl_config_default;
+static struct ressl_config *ressl_config_default;
int
ressl_init(void)
@@ -42,6 +42,9 @@ ressl_init(void)
SSL_load_error_strings();
SSL_library_init();
+ if ((ressl_config_default = ressl_config_new()) == NULL)
+ return (-1);
+
ressl_initialised = 1;
return (0);
@@ -78,7 +81,7 @@ ressl_new(void)
if ((ctx = calloc(1, sizeof(*ctx))) == NULL)
return (NULL);
- ctx-config = ressl_config_default;
+ ctx-config = ressl_config_default;
ressl_reset(ctx);
@@ -89,7 +92,7 @@ int
ressl_configure(struct ressl *ctx, struct ressl_config *config)
{
if (config == NULL)
- config = ressl_config_default;
+ config = ressl_config_default;
ctx-config = config;
Index: ressl.h
===
RCS file: /cvs/src/lib/libressl/ressl.h,v
retrieving revision 1.13
diff -u -p -r1.13 ressl.h
--- ressl.h 27 Aug 2014 10:46:53 - 1.13
+++ ressl.h 10 Sep 2014 20:23:46 -
@@ -31,15 +31,15 @@ const char *ressl_error(struct ressl *ct
struct ressl_config *ressl_config_new(void);
void ressl_config_free(struct ressl_config *config);
-void ressl_config_set_ca_file(struct ressl_config *config, char *ca_file);
-void ressl_config_set_ca_path(struct ressl_config *config, char *ca_path);
-void ressl_config_set_cert_file(struct ressl_config *config, char *cert_file);
-void ressl_config_set_cert_mem(struct ressl_config *config, char *cert,
+int ressl_config_set_ca_file(struct ressl_config *config, char *ca_file);
+int ressl_config_set_ca_path(struct ressl_config *config, char *ca_path);
+int ressl_config_set_cert_file(struct ressl_config *config, char *cert_file);
+int ressl_config_set_cert_mem(struct ressl_config *config, char *cert,
size_t len);
-void ressl_config_set_ciphers(struct ressl_config *config, char *ciphers);
+int ressl_config_set_ciphers(struct ressl_config *config, char *ciphers);
int ressl_config_set_ecdhcurve(struct ressl_config *config, const char *);
-void ressl_config_set_key_file(struct ressl_config *config, char *key_file);
-void ressl_config_set_key_mem(struct ressl_config *config, char *key,
+int ressl_config_set_key_file(struct ressl_config *config, char *key_file);
+int ressl_config_set_key_mem(struct ressl_config *config, char *key,
size_t len);
void ressl_config_set_verify_depth(struct ressl_config *config,
int verify_depth);
Index: ressl_config.c
===
RCS file: /cvs/src/lib/libressl/ressl_config.c,v
retrieving revision 1.8
diff -u -p -r1.8 ressl_config.c
--- ressl_config.c 27 Aug 2014 10:46:53 - 1.8
+++ ressl_config.c 11 Sep 2014 19:14:27 -
@@ -21,27 +21,55 @@
#include ressl.h
#include ressl_internal.h
-/*
- * Default configuration.
- */
-struct ressl_config ressl_config_default = {
- .ca_file = _PATH_SSL_CA_FILE,
- .ca_path = NULL,
- .ciphers = NULL,
- .ecdhcurve = NID_X9_62_prime256v1,
- .verify = 1,
- .verify_depth = 6,
-};
+#define SET_STRING(config, name, val) do { \
+ free(config-name); \
+ config-name = NULL;\
+ if (val != NULL) { \
+ if ((config-name = strdup(val)) == NULL) \
+ return -1; \
+ } \
+} while (0)
+
+#define SET_MEM(config, name, namelen, val, vallen) do { \
+ free(config-name); \
+ config-name = NULL;\
+ if (val != NULL) { \
+ if ((config-name = memdup(val, vallen)) == NULL) \
+ return -1; \
+ config-namelen = vallen; \
+ } \
+} while (0)
+
+static void *
[PATCH] fix overflow handling in dd(1)
Hey, I'm resubmitting this patch since the source tree was locked last time I submitted. Any thoughts? Thanks, William Orr Index: bin/dd/args.c === RCS file: /cvs/src/bin/dd/args.c,v retrieving revision 1.25 diff -u -b -w -p -r1.25 args.c --- bin/dd/args.c 21 May 2014 06:23:02 - 1.25 +++ bin/dd/args.c 12 Sep 2014 04:51:07 - @@ -323,8 +323,12 @@ get_bsz(char *val) size_t num, t; char *expr; + if (strchr(val, '-')) + errx(1, %s: illegal numeric value, oper); + + errno = 0; num = strtoul(val, expr, 0); - if (num == SIZE_T_MAX) /* Overflow. */ + if (num == ULONG_MAX errno == ERANGE)/* Overflow. */ err(1, %s, oper); if (expr == val)/* No digits. */ errx(1, %s: illegal numeric value, oper);
[PATCH] src/usr.bin/ftp/fetch.c: free ressl_config
While reviewing tedu@'s libressl config cleanup diffs I noticed we're
not explicitly freeing ressl_config in ftp(1).
Ok?
Index: fetch.c
===
RCS file: /work/cvsroot/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.129
diff -p -u -r1.129 fetch.c
--- fetch.c 25 Aug 2014 15:36:00 - 1.129
+++ fetch.c 12 Sep 2014 03:56:34 -
@@ -978,6 +978,7 @@ cleanup_url_get:
#ifndef SMALL
if (ssl != NULL) {
ressl_close(ssl);
+ ressl_config_free(ressl_config);
ressl_free(ssl);
}
free(full_host);
