Re: CVS: cvs.openbsd.org: xenocara - pledge update for xterm
On Mon, Oct 24, 2016 at 01:31:35PM -0600, Matthieu Herrb wrote:
> CVSROOT: /cvs
> Module name: xenocara
> Changes by: matth...@cvs.openbsd.org2016/10/24 13:31:35
>
> Modified files:
> app/xterm : Makefile xterm.man xtermcfg.h
>
> Log message:
> Disable Tektronics 4014 emulation. ok natano@, naddy@, schwarze@
>
Hi,
With the disabling of Tektronics emulation, the pledge(2) promises could
be reduced a bit: no more "cpath" should be required.
The commit message for 1.35 which introduced pledge(2) in xterm(1)
stated that "cpath" was for Tek emulation window. I also reviewed
several functions for ensuring no others use of "cpath" after pledging.
Ideally, additionnal review would be welcome: xterm(1) is a big program,
and #ifdef maze is a bit complex to follow :)
Thanks.
--
Sebastien Marie
Index: main.c
===
RCS file: /cvs/xenocara/app/xterm/main.c,v
retrieving revision 1.39
diff -u -p -r1.39 main.c
--- main.c 7 Aug 2016 21:27:36 - 1.39
+++ main.c 25 Oct 2016 06:41:00 -
@@ -2634,12 +2634,12 @@ main(int argc, char *argv[]ENVP_ARG)
if (data &&
(strstr(data, "exec-formatted") || strstr(data,
"exec-selectable"))) {
-if (pledge("stdio rpath wpath cpath id proc exec tty", NULL) ==
-1) {
+if (pledge("stdio rpath wpath id proc exec tty", NULL) == -1) {
xtermWarning("pledge\n");
exit(1);
}
} else {
-if (pledge("stdio rpath wpath cpath id proc tty", NULL) == -1) {
+if (pledge("stdio rpath wpath id proc tty", NULL) == -1) {
xtermWarning("pledge\n");
exit(1);
}
Re: network performance fun
On 25 October 2016 at 08:25, Mike Belopuhov wrote: > On 25 October 2016 at 02:34, David Gwynne wrote: >>> I see. I will double check this tomorrow but your approach >>> looks solid. >> >> it's obviously an interaction between intel nics that do not align >> their ethernet headers correctly, and the M_PREPEND which i just >> changed (and you oked) in src/sys/net/if_ethersubr.c r1.240). >> >> basically the stack strips the 6 byte ethernet header before pushing >> the packet into the ip stack, and forwarding causes it to be output >> as an ethernet packet. the M_PREPEND of 8 bytes in ether_output >> causes an mbuf to be prefixed cos the frame has 6 bytes free, not >> 8. >> > > Ah right, it's the same cluster we're transmitting as we have Rx'ed... > You need to get your mcl2k2 change in then. >> the good news is that at least the prepended mbuf gets its ethernet >> header correctly aligned. >> >> dlg
Re: network performance fun
On 25 October 2016 at 02:34, David Gwynne wrote: >> I see. I will double check this tomorrow but your approach >> looks solid. > > it's obviously an interaction between intel nics that do not align > their ethernet headers correctly, and the M_PREPEND which i just > changed (and you oked) in src/sys/net/if_ethersubr.c r1.240). > > basically the stack strips the 6 byte ethernet header before pushing > the packet into the ip stack, and forwarding causes it to be output > as an ethernet packet. the M_PREPEND of 8 bytes in ether_output > causes an mbuf to be prefixed cos the frame has 6 bytes free, not > 8. > Ah right, it's the same cluster we're transmitting as we have Rx'ed... > the good news is that at least the prepended mbuf gets its ethernet > header correctly aligned. > > dlg
Re: acpiec on acer aspire S7 with CURRENT
On Mon, Oct 24, 2016 at 3:01 PM, Mark Kettenis wrote: >> Date: Mon, 24 Oct 2016 23:01:22 +0300 >> From: Paul Irofti >> >> On Mon, Oct 24, 2016 at 10:50:00PM +0300, Paul Irofti wrote: >> > On Mon, Oct 24, 2016 at 07:46:33PM +0200, Mark Kettenis wrote: >> > > > From: Paul Irofti >> > > > Date: Mon, 24 Oct 2016 17:12:01 +0300 >> > > > >> > > > Any thoughts on this? >> > > >> > > Sorry, yes. Adding the crs "index" as the last argument of the >> > > callback function seems a bit non-intuitive to me. I'd say the void * >> > > argument should remain the last argument, and the crs "number" should >> > > be the first, although I could live with it being the second. >> > > >> > > I feel a bit bad though for not suggesting that earlier. >> > >> > Sure, makes sense. I thought about doing that too, but I did not know >> > how much breakage I could do to the original function. >> > >> > What about crsno, do you prefer it to be called crsidx? That might be >> > a better name... > > I agree! > >> Like this? > > Yes, ok kettenis@ I agree as well. ok guenther@ The other diff is looking good to me as well, cleaning things up and eliminating the duplicate parsing better than I expected. Thanks for doing this work! Philip
Re: per-cpu caches for pools
On Mon, Oct 24, 2016 at 04:24:13PM +1000, David Gwynne wrote:
> ive posted this before as part of a much bigger diff, but smaller
> is better.
>
> it basically lets things ask for per-cpu item caches to be enabled
> on pools. the most obvious use case for this is the mbuf pools.
>
> the caches are modelled on whats described in the "Magazines and
> Vmem: Extending the Slab Allocator to Many CPUs and Arbitrary
> Resources" paper by Jeff Bonwick and Jonathan Adams. pools are
> modelled on slabs, which bonwick described in a previous paper.
>
> the main inspiration the paper provided was around how many objects
> to cache on each cpu, and how often to move sets of objects between
> the cpu caches and a global list of objects. unlike the paper we
> do not care about maintaining constructed objects on the free lists,
> so we reuse the objects themselves to build the free list.
>
> id like to get this in so we can tinker with it in tree. the things
> i think we need to tinker with are what poisioning we can get away
> with on the per cpu caches, and what limits can we enforce at the
> pool level.
>
> i think poisioning will be relatively simple to add. the limits one
> is more challenging because we dont want the pools to have to
> coordinate between cpus for every get or put operation. my thought
> there was to limit the number of pages that a pool can allocate
> from its backend rather than limit the items the pool can provide.
> limiting the pages could also be done at a lower level. eg, the
> mbuf clusters could share a common backend that limits the pages
> the pools can get, rather than have the cluster pools account for
> pages separately.
>
> anyway, either way i would like to get this in so we can work on
> this stuff.
>
> ok?
this adds per-cpu caches to the mbuf pools so people can actually
try and see if the code works or not.
Index: kern/subr_pool.c
===
RCS file: /cvs/src/sys/kern/subr_pool.c,v
retrieving revision 1.198
diff -u -p -r1.198 subr_pool.c
--- kern/subr_pool.c15 Sep 2016 02:00:16 - 1.198
+++ kern/subr_pool.c25 Oct 2016 00:28:14 -
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include
@@ -96,6 +97,33 @@ struct pool_item {
};
#define POOL_IMAGIC(ph, pi) ((u_long)(pi) ^ (ph)->ph_magic)
+#ifdef MULTIPROCESSOR
+struct pool_list {
+ struct pool_list*pl_next; /* next in list */
+ unsigned longpl_cookie;
+ struct pool_list*pl_nextl; /* next list */
+ unsigned longpl_nitems; /* items in list */
+};
+
+struct pool_cache {
+ struct pool_list*pc_actv;
+ unsigned longpc_nactv; /* cache pc_actv nitems */
+ struct pool_list*pc_prev;
+
+ uint64_t pc_gen;/* generation number */
+ uint64_t pc_gets;
+ uint64_t pc_puts;
+ uint64_t pc_fails;
+
+ int pc_nout;
+};
+
+void *pool_cache_get(struct pool *);
+voidpool_cache_put(struct pool *, void *);
+voidpool_cache_destroy(struct pool *);
+#endif
+voidpool_cache_info(struct pool *, struct kinfo_pool *);
+
#ifdef POOL_DEBUG
intpool_debug = 1;
#else
@@ -355,6 +383,11 @@ pool_destroy(struct pool *pp)
struct pool_item_header *ph;
struct pool *prev, *iter;
+#ifdef MULTIPROCESSOR
+ if (pp->pr_cache != NULL)
+ pool_cache_destroy(pp);
+#endif
+
#ifdef DIAGNOSTIC
if (pp->pr_nout != 0)
panic("%s: pool busy: still out: %u", __func__, pp->pr_nout);
@@ -421,6 +454,14 @@ pool_get(struct pool *pp, int flags)
void *v = NULL;
int slowdown = 0;
+#ifdef MULTIPROCESSOR
+ if (pp->pr_cache != NULL) {
+ v = pool_cache_get(pp);
+ if (v != NULL)
+ goto good;
+ }
+#endif
+
KASSERT(flags & (PR_WAITOK | PR_NOWAIT));
mtx_enter(&pp->pr_mtx);
@@ -453,6 +494,9 @@ pool_get(struct pool *pp, int flags)
v = mem.v;
}
+#ifdef MULTIPROCESSOR
+good:
+#endif
if (ISSET(flags, PR_ZERO))
memset(v, 0, pp->pr_size);
@@ -631,6 +675,13 @@ pool_put(struct pool *pp, void *v)
panic("%s: NULL item", __func__);
#endif
+#ifdef MULTIPROCESSOR
+ if (pp->pr_cache != NULL && TAILQ_EMPTY(&pp->pr_requests)) {
+ pool_cache_put(pp, v);
+ return;
+ }
+#endif
+
mtx_enter(&pp->pr_mtx);
splassert(pp->pr_ipl);
@@ -1333,6 +1384,8 @@ sysctl_dopool(int *name, u_int namelen,
pi.pr_nidle = pp->pr_nidle;
mtx_leave(&pp->pr_mtx);
+ pool_cache_info(pp, &pi);
+
rv = sysctl_rdstruct(oldp, oldlenp, NULL, &pi, sizeof(pi));
break;
}
@@ -1499,3 +1552,259 @@ pool_multi_free_ni(struct pool
Re: network performance fun
On Tue, Oct 25, 2016 at 12:50:50AM +0200, Mike Belopuhov wrote: > On Tue, Oct 25, 2016 at 00:22 +0200, Hrvoje Popovski wrote: > > On 24.10.2016. 23:36, Mike Belopuhov wrote: > > > On Mon, Oct 24, 2016 at 19:04 +0200, Hrvoje Popovski wrote: > > >> Hi all, > > >> > > >> OpenBSD box acts as transit router for /8 networks without pf and with > > >> this sysctls > > >> > > >> ddb.console=1 > > >> kern.pool_debug=0 > > >> net.inet.ip.forwarding=1 > > >> net.inet.ip.ifq.maxlen=8192 > > >> > > >> netstat > > >> 11/8 192.168.11.2 UGS0 114466419 - 8 > > >> ix0 > > >> 12/8 192.168.12.2 UGS00 - 8 > > >> ix1 > > >> 13/8 192.168.13.2 UGS00 - 8 > > >> myx0 > > >> 14/8 192.168.14.2 UGS00 - 8 > > >> myx1 > > >> 15/8 192.168.15.2 UGS00 - 8 > > >> em3 > > >> 16/8 192.168.16.2 UGS0 89907239 - 8 > > >> em2 > > >> 17/8 192.168.17.2 UGS0 65791508 - 8 > > >> bge0 > > >> 18/8 192.168.18.2 UGS00 - 8 > > >> bge1 > > >> > > >> while testing dlg@ "mcl2k2 mbuf clusters" patch with todays -current i > > >> saw that performance with plain -current drops for about 300Kpps vs > > >> -current from 06.10.2016. by bisecting cvs tree it seems that this > > >> commit is guilty for this > > >> > > >> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_ethersubr.c?rev=1.240&content-type=text/x-cvsweb-markup > > >> > > > > > > I don't see how this change can affect performance in such a way > > > unless you're sending jumbo packets, but then the packet rates > > > are too high. Are you 100% sure it's this particular change? > > > > > > > No, no, i'm not 100% sure. I was doing this to try to find bottleneck: > > > > cvs -q checkout -D "2016-10-XX" -P src > > > > 2016-10-06 - 900kpps > > 2016-10-07 - 900kpps > > 2016-10-10 - 900kpps > > 2016-10-11 - 650kpps > > 2016-10-11 with if_ethersubr.c 1.239 - 900kpps > > ... > > 2016-10-14 - 650kpps > > 2016-10-14 with dlg@ patch - 900kpps > > 2016-10-14 with dlg@ patch and with if_ethersubr.c 1.239 - 880kpps > > > > 2016-10-24 - results are in mail ... > > > > and then i looked at networking diffs from 2016-10-10 and 2016-10-11 and > > it seems that if_ethersubr.c is guilty > > > > tests was done over ix only ... > > > > although as you can see with today's plain -current i'm getting 690kpps > > and with today's -current with if_ethersubr.c 1.239 i'm getting 910kpps > > > > so i thought that there must be something with if_ethersubr.c > > > > I see. I will double check this tomorrow but your approach > looks solid. it's obviously an interaction between intel nics that do not align their ethernet headers correctly, and the M_PREPEND which i just changed (and you oked) in src/sys/net/if_ethersubr.c r1.240). basically the stack strips the 6 byte ethernet header before pushing the packet into the ip stack, and forwarding causes it to be output as an ethernet packet. the M_PREPEND of 8 bytes in ether_output causes an mbuf to be prefixed cos the frame has 6 bytes free, not 8. the good news is that at least the prepended mbuf gets its ethernet header correctly aligned. dlg
Re: (re)name cpumem_realloc to cpumem_malloc_ncpus
thank you for looking at this.
ive committed it with tweaks based on your suggestions.
> On 24 Oct 2016, at 22:15, Alexander Bluhm wrote:
>
> On Mon, Oct 24, 2016 at 02:48:03PM +1000, David Gwynne wrote:
>> cos its not resizing the allocation, its allocating them for new cpus.
>
> realloc is not a good name, so yes to renaming.
>
>> the same goes for counters_realloc being named counters_alloc_ncpus.
>
> Does the n mean new? This is never mentioned, I initially thought
> it means number. What about ..._newcpus or ..._allcpus?
>
>> +.Fn COUNTERS_BOOT_INITIALIZER
>> +is used to initialise a cpumem pointer with the memory that was previously
>> +allocated using
>> +.Fn COUNTERS_BOOT_MEMORY
>> +and identified by
>> +.Fa NAME .
>
> The boot counters are zero initially.
>
>> +The
>> +.Fa type
>> +argument specifies the type of memory that the counters will be
>> +allocated as via
>> +.Xr malloc 9 .
>> +The memory will be zeroed on allocation by passing
>> +.Fn M_ZERO
>
> Is the M_ZERO an implementation detail for counters that should not
> be mentionend in the man page?
>
>> +to
>> +.Xr malloc 9 .
>
> The counters on the boot cpu are preserved.
>
>> +.Fn CPUMEM_BOOT_INITIALIZER
>> +is used to initialise a cpumem pointer with the memory that was previously
>> +allocated using
>> +.Fn CPUMEM_BOOT_MEMORY
>> +and identified by
>> +.Fa NAME .
>
> The boot cpu uses static memory that is initially zero.
>
>> +.Fn cpumem_malloc_ncpus
>> +allocates
>> +.Fa sz
>> +bytes of
>> +.Fa type
>> +memory for each additional CPU using
>> +.Xr malloc 9 .
>> +The memory will be zeroed on allocation by passing
>> +.Fn M_ZERO
>> +to
>> +.Xr malloc 9 .
>
> The the memory of the boot cpu is preserved.
>
>> ===
>> RCS file: /cvs/src/sys/kern/subr_percpu.c,v
>> retrieving revision 1.3
>> diff -u -p -r1.3 subr_percpu.c
>> --- sys/kern/subr_percpu.c 24 Oct 2016 03:15:38 - 1.3
>> +++ sys/kern/subr_percpu.c 24 Oct 2016 04:43:34 -
>> @@ -76,7 +76,7 @@ cpumem_malloc(size_t sz, int type)
>> }
>>
>> struct cpumem *
>> -cpumem_realloc(struct cpumem *bootcm, size_t sz, int type)
>> +cpumem_alloc_ncpus(struct cpumem *bootcm, size_t sz, int type)
>
> This should be cpumem_malloc_ncpus.
>
>> {
>> struct cpumem *cm;
>> unsigned int cpu;
>> @@ -146,10 +146,10 @@ counters_alloc(unsigned int n, int type)
>> }
>>
>> struct cpumem *
>> -counters_realloc(struct cpumem *cm, unsigned int n, int type)
>> +counters_alloc_ncpus(struct cpumem *cm, unsigned int n, int type)
>> {
>> n++; /* the generation number */
>> -return (cpumem_realloc(cm, n * sizeof(uint64_t), type));
>> +return (cpumem_alloc_ncpus(cm, n * sizeof(uint64_t), type));
>
> This should be cpumem_malloc_ncpus.
>
>> }
>>
>> void
>> @@ -259,7 +259,7 @@ cpumem_malloc(size_t sz, int type)
>> }
>>
>> struct cpumem *
>> -cpumem_realloc(struct cpumem *cm, size_t sz, int type)
>> +cpumem_allod_ncpus(struct cpumem *cm, size_t sz, int type)
>
> This should be cpumem_malloc_ncpus.
>
>> {
>> return (cm);
>> }
>> @@ -291,10 +291,10 @@ counters_alloc(unsigned int n, int type)
>> }
>>
>> struct cpumem *
>> -counters_realloc(struct cpumem *cm, unsigned int n, int type)
>> +counters_alloc_ncpus(struct cpumem *cm, unsigned int n, int type)
>> {
>> /* this is unecessary, but symmetrical */
>> -return (cpumem_realloc(cm, n * sizeof(uint64_t), type));
>> +return (cpumem_alloc_ncpus(cm, n * sizeof(uint64_t), type));
>
> This should be cpumem_malloc_ncpus.
>
>> }
>>
>> void
>> Index: sys/sys/percpu.h
>> ===
>> RCS file: /cvs/src/sys/sys/percpu.h,v
>> retrieving revision 1.2
>> diff -u -p -r1.2 percpu.h
>> --- sys/sys/percpu.h 24 Oct 2016 03:15:35 - 1.2
>> +++ sys/sys/percpu.h 24 Oct 2016 04:43:34 -
>> @@ -54,7 +54,7 @@ struct cpumem *cpumem_get(struct pool *)
>> void cpumem_put(struct pool *, struct cpumem *);
>>
>> struct cpumem*cpumem_malloc(size_t, int);
>> -struct cpumem *cpumem_realloc(struct cpumem *, size_t, int);
>> +struct cpumem *cpumem_malloc_ncpus(struct cpumem *, size_t, int);
>
> I wonder how this compiled as you use cpumem_malloc_ncpus correctly
> here.
>
>> void cpumem_free(struct cpumem *, int, size_t);
>>
>> void *cpumem_first(struct cpumem_iter *, struct cpumem *);
>> @@ -111,7 +111,7 @@ static struct {
>> \
>> */
>>
>> struct cpumem*counters_alloc(unsigned int, int);
>> -struct cpumem *counters_realloc(struct cpumem *, unsigned int, int);
>> +struct cpumem *counters_alloc_ncpus(struct cpumem *, unsigned int,
>> int);
>> void counters_free(struct cpumem *, int, unsigned int);
>> void counters_read(struct cpumem *, uint64_t *, unsigned int);
>> void counters_zero(struct cpumem *, unsigned int);
[PATCH] faq/current.html - small correction
Hi all, Recent commit[1] set the font on the rest of the faq/current.html[1] page (i.e. the footer) to boldface. Remove redundant 'rm' while there. [0] http://cvsweb.openbsd.org/cgi-bin/cvsweb/www/faq/current.html.diff?r1=1.741&r2=1.742 [1] http://www.openbsd.org/faq/current.html Cheers, Raf Index: faq/current.html === RCS file: /cvs/www/faq/current.html,v retrieving revision 1.742 diff -u -p -r1.742 current.html --- faq/current.html24 Oct 2016 19:56:53 - 1.742 +++ faq/current.html24 Oct 2016 23:32:06 - @@ -357,7 +357,7 @@ upgrading to -current: # cd /usr/X11R6 # rm bin/koi8rxterm bin/uxterm # rm share/X11/app-defaults/KOI8RXTerm share/X11/app-defaults/UXTerm -# rm man/man1/koi8rxterm.1 rm man/man1/uxterm.1 +# rm man/man1/koi8rxterm.1 man/man1/uxterm.1 $OpenBSD: current.html,v 1.742 2016/10/24 19:56:53 matthieu Exp $
Re: network performance fun
On Tue, Oct 25, 2016 at 00:22 +0200, Hrvoje Popovski wrote:
> On 24.10.2016. 23:36, Mike Belopuhov wrote:
> > On Mon, Oct 24, 2016 at 19:04 +0200, Hrvoje Popovski wrote:
> >> Hi all,
> >>
> >> OpenBSD box acts as transit router for /8 networks without pf and with
> >> this sysctls
> >>
> >> ddb.console=1
> >> kern.pool_debug=0
> >> net.inet.ip.forwarding=1
> >> net.inet.ip.ifq.maxlen=8192
> >>
> >> netstat
> >> 11/8 192.168.11.2 UGS0 114466419 - 8
> >> ix0
> >> 12/8 192.168.12.2 UGS00 - 8 ix1
> >> 13/8 192.168.13.2 UGS00 - 8
> >> myx0
> >> 14/8 192.168.14.2 UGS00 - 8
> >> myx1
> >> 15/8 192.168.15.2 UGS00 - 8 em3
> >> 16/8 192.168.16.2 UGS0 89907239 - 8 em2
> >> 17/8 192.168.17.2 UGS0 65791508 - 8
> >> bge0
> >> 18/8 192.168.18.2 UGS00 - 8
> >> bge1
> >>
> >> while testing dlg@ "mcl2k2 mbuf clusters" patch with todays -current i
> >> saw that performance with plain -current drops for about 300Kpps vs
> >> -current from 06.10.2016. by bisecting cvs tree it seems that this
> >> commit is guilty for this
> >>
> >> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_ethersubr.c?rev=1.240&content-type=text/x-cvsweb-markup
> >>
> >
> > I don't see how this change can affect performance in such a way
> > unless you're sending jumbo packets, but then the packet rates
> > are too high. Are you 100% sure it's this particular change?
> >
>
> No, no, i'm not 100% sure. I was doing this to try to find bottleneck:
>
> cvs -q checkout -D "2016-10-XX" -P src
>
> 2016-10-06 - 900kpps
> 2016-10-07 - 900kpps
> 2016-10-10 - 900kpps
> 2016-10-11 - 650kpps
> 2016-10-11 with if_ethersubr.c 1.239 - 900kpps
> ...
> 2016-10-14 - 650kpps
> 2016-10-14 with dlg@ patch - 900kpps
> 2016-10-14 with dlg@ patch and with if_ethersubr.c 1.239 - 880kpps
>
> 2016-10-24 - results are in mail ...
>
> and then i looked at networking diffs from 2016-10-10 and 2016-10-11 and
> it seems that if_ethersubr.c is guilty
>
> tests was done over ix only ...
>
> although as you can see with today's plain -current i'm getting 690kpps
> and with today's -current with if_ethersubr.c 1.239 i'm getting 910kpps
>
> so i thought that there must be something with if_ethersubr.c
>
I see. I will double check this tomorrow but your approach
looks solid.
> > What kind of traffic are you testing this with?
> > I assume small IP or UDP packets, correct?
> >
>
> yes, 64 byte UDP without flowcontrol..
>
> > Actually I'd like to know what causes this.
> >
> > So far I've noticed that the code generating ICMP error doesn't
> > reserve space for the link header but it's unlikely a culprit.
> > (The diff was only compile tested so far...)
> >
>
>
> with -current from few minutes ago and with this diff i'm getting panic
>
MH_ALIGN gets in the way... This should solve it, but needs to be
tested with large packets.
diff --git sys/netinet/ip_icmp.c sys/netinet/ip_icmp.c
index cdd60aa..5542f64 100644
--- sys/netinet/ip_icmp.c
+++ sys/netinet/ip_icmp.c
@@ -210,7 +210,8 @@ icmp_do_error(struct mbuf *n, int type, int code, u_int32_t
dest, int destmtu)
icmplen = MCLBYTES - ICMP_MINLEN - sizeof (struct ip);
m = m_gethdr(M_DONTWAIT, MT_HEADER);
- if (m && (sizeof (struct ip) + icmplen + ICMP_MINLEN > MHLEN)) {
+ if (m && (max_linkhdr + sizeof(struct ip) + icmplen +
+ ICMP_MINLEN > MHLEN)) {
MCLGET(m, M_DONTWAIT);
if ((m->m_flags & M_EXT) == 0) {
m_freem(m);
@@ -224,6 +225,8 @@ icmp_do_error(struct mbuf *n, int type, int code, u_int32_t
dest, int destmtu)
m->m_len = icmplen + ICMP_MINLEN;
if ((m->m_flags & M_EXT) == 0)
MH_ALIGN(m, m->m_len);
+ else
+ m->m_data += max_linkhdr;
icp = mtod(m, struct icmp *);
if ((u_int)type > ICMP_MAXTYPE)
panic("icmp_error");
Re: network performance fun
On 25.10.2016. 0:22, Hrvoje Popovski wrote: > On 24.10.2016. 23:36, Mike Belopuhov wrote: >> On Mon, Oct 24, 2016 at 19:04 +0200, Hrvoje Popovski wrote: >>> Hi all, >>> >>> OpenBSD box acts as transit router for /8 networks without pf and with >>> this sysctls >>> >>> ddb.console=1 >>> kern.pool_debug=0 >>> net.inet.ip.forwarding=1 >>> net.inet.ip.ifq.maxlen=8192 >>> >>> netstat >>> 11/8 192.168.11.2 UGS0 114466419 - 8 ix0 >>> 12/8 192.168.12.2 UGS00 - 8 ix1 >>> 13/8 192.168.13.2 UGS00 - 8 myx0 >>> 14/8 192.168.14.2 UGS00 - 8 myx1 >>> 15/8 192.168.15.2 UGS00 - 8 em3 >>> 16/8 192.168.16.2 UGS0 89907239 - 8 em2 >>> 17/8 192.168.17.2 UGS0 65791508 - 8 bge0 >>> 18/8 192.168.18.2 UGS00 - 8 bge1 >>> >>> while testing dlg@ "mcl2k2 mbuf clusters" patch with todays -current i >>> saw that performance with plain -current drops for about 300Kpps vs >>> -current from 06.10.2016. by bisecting cvs tree it seems that this >>> commit is guilty for this >>> >>> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_ethersubr.c?rev=1.240&content-type=text/x-cvsweb-markup >>> >> >> I don't see how this change can affect performance in such a way >> unless you're sending jumbo packets, but then the packet rates >> are too high. Are you 100% sure it's this particular change? >> > > No, no, i'm not 100% sure. I was doing this to try to find bottleneck: > > cvs -q checkout -D "2016-10-XX" -P src > > 2016-10-06 - 900kpps > 2016-10-07 - 900kpps > 2016-10-10 - 900kpps > 2016-10-11 - 650kpps > 2016-10-11 with if_ethersubr.c 1.239 - 900kpps > ... > 2016-10-14 - 650kpps > 2016-10-14 with dlg@ patch - 900kpps > 2016-10-14 with dlg@ patch and with if_ethersubr.c 1.239 - 880kpps > > 2016-10-24 - results are in mail ... > > and then i looked at networking diffs from 2016-10-10 and 2016-10-11 and > it seems that if_ethersubr.c is guilty > > tests was done over ix only ... > > although as you can see with today's plain -current i'm getting 690kpps > and with today's -current with if_ethersubr.c 1.239 i'm getting 910kpps > just please see that bge performance are the same with if_ethersubr.c 1.239 or 1.241. i haven't test myx, will do that ...
Re: network performance fun
On 24.10.2016. 23:36, Mike Belopuhov wrote:
> On Mon, Oct 24, 2016 at 19:04 +0200, Hrvoje Popovski wrote:
>> Hi all,
>>
>> OpenBSD box acts as transit router for /8 networks without pf and with
>> this sysctls
>>
>> ddb.console=1
>> kern.pool_debug=0
>> net.inet.ip.forwarding=1
>> net.inet.ip.ifq.maxlen=8192
>>
>> netstat
>> 11/8 192.168.11.2 UGS0 114466419 - 8 ix0
>> 12/8 192.168.12.2 UGS00 - 8 ix1
>> 13/8 192.168.13.2 UGS00 - 8 myx0
>> 14/8 192.168.14.2 UGS00 - 8 myx1
>> 15/8 192.168.15.2 UGS00 - 8 em3
>> 16/8 192.168.16.2 UGS0 89907239 - 8 em2
>> 17/8 192.168.17.2 UGS0 65791508 - 8 bge0
>> 18/8 192.168.18.2 UGS00 - 8 bge1
>>
>> while testing dlg@ "mcl2k2 mbuf clusters" patch with todays -current i
>> saw that performance with plain -current drops for about 300Kpps vs
>> -current from 06.10.2016. by bisecting cvs tree it seems that this
>> commit is guilty for this
>>
>> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_ethersubr.c?rev=1.240&content-type=text/x-cvsweb-markup
>>
>
> I don't see how this change can affect performance in such a way
> unless you're sending jumbo packets, but then the packet rates
> are too high. Are you 100% sure it's this particular change?
>
No, no, i'm not 100% sure. I was doing this to try to find bottleneck:
cvs -q checkout -D "2016-10-XX" -P src
2016-10-06 - 900kpps
2016-10-07 - 900kpps
2016-10-10 - 900kpps
2016-10-11 - 650kpps
2016-10-11 with if_ethersubr.c 1.239 - 900kpps
...
2016-10-14 - 650kpps
2016-10-14 with dlg@ patch - 900kpps
2016-10-14 with dlg@ patch and with if_ethersubr.c 1.239 - 880kpps
2016-10-24 - results are in mail ...
and then i looked at networking diffs from 2016-10-10 and 2016-10-11 and
it seems that if_ethersubr.c is guilty
tests was done over ix only ...
although as you can see with today's plain -current i'm getting 690kpps
and with today's -current with if_ethersubr.c 1.239 i'm getting 910kpps
so i thought that there must be something with if_ethersubr.c
> What kind of traffic are you testing this with?
> I assume small IP or UDP packets, correct?
>
yes, 64 byte UDP without flowcontrol..
> Actually I'd like to know what causes this.
>
> So far I've noticed that the code generating ICMP error doesn't
> reserve space for the link header but it's unlikely a culprit.
> (The diff was only compile tested so far...)
>
> diff --git sys/netinet/ip_icmp.c sys/netinet/ip_icmp.c
> index cdd60aa..b3ddea4 100644
> --- sys/netinet/ip_icmp.c
> +++ sys/netinet/ip_icmp.c
> @@ -208,19 +208,21 @@ icmp_do_error(struct mbuf *n, int type, int code,
> u_int32_t dest, int destmtu)
>
> if (icmplen + ICMP_MINLEN > MCLBYTES)
> icmplen = MCLBYTES - ICMP_MINLEN - sizeof (struct ip);
>
> m = m_gethdr(M_DONTWAIT, MT_HEADER);
> - if (m && (sizeof (struct ip) + icmplen + ICMP_MINLEN > MHLEN)) {
> + if (m && (max_linkhdr + sizeof(struct ip) + icmplen +
> + ICMP_MINLEN > MHLEN)) {
> MCLGET(m, M_DONTWAIT);
> if ((m->m_flags & M_EXT) == 0) {
> m_freem(m);
> m = NULL;
> }
> }
> if (m == NULL)
> goto freeit;
> + m->m_data += max_linkhdr;
> /* keep in same rtable */
> m->m_pkthdr.ph_rtableid = n->m_pkthdr.ph_rtableid;
> m->m_len = icmplen + ICMP_MINLEN;
> if ((m->m_flags & M_EXT) == 0)
> MH_ALIGN(m, m->m_len);
>
with -current from few minutes ago and with this diff i'm getting panic
login: panic: pool_do_get: mbufpl free list modified: page
0xff00697e8000; item addr 0xff00697e8800; offset 0x0=
0x384500081c56 != 0xf2a4b1392c5839b2
Stopped at Debugger+0x9: leave
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
*11010 11010 830x100012 02 ntpd
Debugger() at Debugger+0x9
panic() at panic+0xfe
pool_runqueue() at pool_runqueue
pool_get() at pool_get+0xb5
m_get() at m_get+0x28
m_getuio() at m_getuio+0x5c
sosend() at sosend+0x268
sendit() at sendit+0x258
sys_sendmsg() at sys_sendmsg+0xc0
syscall() at syscall+0x27b
--- syscall (number 28) ---
end of kernel
end trace frame: 0x7f7f11f0, count: 5
0xd9f5f7f362a:
https://www.openbsd.org/ddb.html describes the minimum info required in bug
reports. Insufficient info makes it difficult to find and fix bugs.
ddb{2}> show panic
pool_do_get: mbufpl free list modified: page 0xff00697e8000; item
addr 0xff
00697e8800; offset 0x0=0x384500081c56 != 0xf2a4b1392c5839b2
ddb{2}> trace
Debugger() at Debugger+0x9
panic() at panic+0xfe
pool_runqueue() at pool_runqueue
pool_get() at pool_get+0xb5
m_get() at m_get+0x28
m_getuio() at m_getuio+0x5c
s
Re: acpiec on acer aspire S7 with CURRENT
> Date: Mon, 24 Oct 2016 23:01:22 +0300
> From: Paul Irofti
>
> On Mon, Oct 24, 2016 at 10:50:00PM +0300, Paul Irofti wrote:
> > On Mon, Oct 24, 2016 at 07:46:33PM +0200, Mark Kettenis wrote:
> > > > From: Paul Irofti
> > > > Date: Mon, 24 Oct 2016 17:12:01 +0300
> > > >
> > > > Any thoughts on this?
> > >
> > > Sorry, yes. Adding the crs "index" as the last argument of the
> > > callback function seems a bit non-intuitive to me. I'd say the void *
> > > argument should remain the last argument, and the crs "number" should
> > > be the first, although I could live with it being the second.
> > >
> > > I feel a bit bad though for not suggesting that earlier.
> >
> > Sure, makes sense. I thought about doing that too, but I did not know
> > how much breakage I could do to the original function.
> >
> > What about crsno, do you prefer it to be called crsidx? That might be
> > a better name...
I agree!
> Like this?
Yes, ok kettenis@
> Index: acpi.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> retrieving revision 1.316
> diff -u -p -u -p -r1.316 acpi.c
> --- acpi.c18 Sep 2016 23:56:45 - 1.316
> +++ acpi.c24 Oct 2016 20:00:30 -
> @@ -513,10 +513,10 @@ TAILQ_HEAD(, acpi_pci) acpi_pcirootdevs
> TAILQ_HEAD_INITIALIZER(acpi_pcirootdevs);
>
> int acpi_getpci(struct aml_node *node, void *arg);
> -int acpi_getminbus(union acpi_resource *crs, void *arg);
> +int acpi_getminbus(int crsidx, union acpi_resource *crs, void *arg);
>
> int
> -acpi_getminbus(union acpi_resource *crs, void *arg)
> +acpi_getminbus(int crsidx, union acpi_resource *crs, void *arg)
> {
> int *bbn = arg;
> int typ = AML_CRSTYPE(crs);
> Index: acpiprt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpiprt.c,v
> retrieving revision 1.47
> diff -u -p -u -p -r1.47 acpiprt.c
> --- acpiprt.c 14 Mar 2015 03:38:46 - 1.47
> +++ acpiprt.c 24 Oct 2016 20:00:31 -
> @@ -60,8 +60,8 @@ SIMPLEQ_HEAD(, acpiprt_map) acpiprt_map_
>
> int acpiprt_match(struct device *, void *, void *);
> void acpiprt_attach(struct device *, struct device *, void *);
> -int acpiprt_getirq(union acpi_resource *crs, void *arg);
> -int acpiprt_chooseirq(union acpi_resource *, void *);
> +int acpiprt_getirq(int, union acpi_resource *, void *);
> +int acpiprt_chooseirq(int, union acpi_resource *, void *);
>
> struct acpiprt_softc {
> struct device sc_dev;
> @@ -137,7 +137,7 @@ acpiprt_attach(struct device *parent, st
> }
>
> int
> -acpiprt_getirq(union acpi_resource *crs, void *arg)
> +acpiprt_getirq(int crsidx, union acpi_resource *crs, void *arg)
> {
> struct acpiprt_irq *irq = arg;
> int typ, len;
> @@ -190,7 +190,7 @@ acpiprt_pri[16] = {
> };
>
> int
> -acpiprt_chooseirq(union acpi_resource *crs, void *arg)
> +acpiprt_chooseirq(int crsidx, union acpi_resource *crs, void *arg)
> {
> struct acpiprt_irq *irq = arg;
> int typ, len, i, pri = -1;
> Index: bytgpio.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/bytgpio.c,v
> retrieving revision 1.11
> diff -u -p -u -p -r1.11 bytgpio.c
> --- bytgpio.c 8 May 2016 11:08:01 - 1.11
> +++ bytgpio.c 24 Oct 2016 20:00:31 -
> @@ -104,7 +104,7 @@ const int byt_sus_pins[] = {
> 56, 54, 49, 55, 48, 57, 50, 58, 52, 53, 59, 40
> };
>
> -int bytgpio_parse_resources(union acpi_resource *, void *);
> +int bytgpio_parse_resources(int, union acpi_resource *, void *);
> int bytgpio_read_pin(void *, int);
> void bytgpio_write_pin(void *, int, int);
> void bytgpio_intr_establish(void *, int, int, int (*)(), void *);
> @@ -238,7 +238,7 @@ free:
> }
>
> int
> -bytgpio_parse_resources(union acpi_resource *crs, void *arg)
> +bytgpio_parse_resources(int crsidx, union acpi_resource *crs, void *arg)
> {
> struct bytgpio_softc *sc = arg;
> int type = AML_CRSTYPE(crs);
> Index: chvgpio.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/chvgpio.c,v
> retrieving revision 1.5
> diff -u -p -u -p -r1.5 chvgpio.c
> --- chvgpio.c 8 May 2016 18:18:42 - 1.5
> +++ chvgpio.c 24 Oct 2016 20:00:31 -
> @@ -143,7 +143,7 @@ const int chv_southeast_pins[] = {
> 8, 12, 6, 8, 10, 11, -1
> };
>
> -int chvgpio_parse_resources(union acpi_resource *, void *);
> +int chvgpio_parse_resources(int, union acpi_resource *, void *);
> int chvgpio_check_pin(struct chvgpio_softc *, int);
> int chvgpio_read_pin(void *, int);
> void chvgpio_write_pin(void *, int, int);
> @@ -264,7 +264,7 @@ unmap:
> }
>
> int
> -chvgpio_parse_resources(union acpi_resource *crs, void *arg)
> +chvgpio_parse_resources(int crsidx, union acpi_resource *crs, void *arg)
> {
> struct chvgpio_softc *sc = arg;
> int type = AML_CRSTYPE(crs);
> Index: dsdt.c
>
Re: network performance fun
On Mon, Oct 24, 2016 at 19:04 +0200, Hrvoje Popovski wrote:
> Hi all,
>
> OpenBSD box acts as transit router for /8 networks without pf and with
> this sysctls
>
> ddb.console=1
> kern.pool_debug=0
> net.inet.ip.forwarding=1
> net.inet.ip.ifq.maxlen=8192
>
> netstat
> 11/8 192.168.11.2 UGS0 114466419 - 8 ix0
> 12/8 192.168.12.2 UGS00 - 8 ix1
> 13/8 192.168.13.2 UGS00 - 8 myx0
> 14/8 192.168.14.2 UGS00 - 8 myx1
> 15/8 192.168.15.2 UGS00 - 8 em3
> 16/8 192.168.16.2 UGS0 89907239 - 8 em2
> 17/8 192.168.17.2 UGS0 65791508 - 8 bge0
> 18/8 192.168.18.2 UGS00 - 8 bge1
>
> while testing dlg@ "mcl2k2 mbuf clusters" patch with todays -current i
> saw that performance with plain -current drops for about 300Kpps vs
> -current from 06.10.2016. by bisecting cvs tree it seems that this
> commit is guilty for this
>
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_ethersubr.c?rev=1.240&content-type=text/x-cvsweb-markup
>
I don't see how this change can affect performance in such a way
unless you're sending jumbo packets, but then the packet rates
are too high. Are you 100% sure it's this particular change?
What kind of traffic are you testing this with?
I assume small IP or UDP packets, correct?
>
> -current from 24.10.2016
>
> ix
> send receive
> 690Kpps 690Kpps
> 700Kpps 680Kpps
> 800Kpps 350Kpps
> 1.4Mpps 305Kpps
> 14Mpps305Kpps
>
> em
> send receive
> 690Kpps 690Kpps
> 700Kpps 680Kpps
> 800Kpps 700Kpps
> 1.4Mpps 700Kpps
>
> bge
> send receive
> 620Kpps 620Kpps
> 630Kpps 515Kpps
> 700Kpps 475Kpps
> 800Kpps 430Kpps
> 1.4Mpps 350Kpps
>
>
> -current with if_ethersubr.c version 1.239
>
> ix
> send receive
> 910Kpps 910Kpps
> 920Kpps 820Kpps
> 1Mpps 825Kpps
> 1.4Mpps 700Kpps
> 14Mpps700Kpps
>
> em
> send receive
> 940Kpps 940Kpps
> 950Kpps 845Kpps
> 1Mpps 855Kpps
> 1.4Mpps 845Kpps
>
> bge
> send receive
> 620Kpps 620Kpps
> 630Kpps 525Kpps
> 700Kpps 485Kpps
> 800Kpps 435Kpps
> 1.4Mpps 350Kpps
>
>
> applying dlg@ "mcl2k2 mbuf clusters" patch to todays -current brings
> performance back to ix and em ... bge is emotional as always :)
>
> ix
> send receive
> 900Kpps 900Kpps
> 910Kpps 895Kpps
> 1Mpps 895Kpps
> 1.4Mpps 810Kpps
> 14Mpps815Kpps
>
> em
> send receive
> 940Kpps 940Kpps
> 950Kpps 930Kpps
> 1Mpps 920Kpps
> 1.4Mpps 930Kpps
>
> bge
> send receive
> 620Kpps 620Kpps
> 630Kpps 520Kpps
> 700Kpps 475Kpps
> 800Kpps 430Kpps
> 1.4Mpps 366Kpps
>
>
>
> i honestly don't know what all that means, i'm just writing my
> observation ...
>
Actually I'd like to know what causes this.
So far I've noticed that the code generating ICMP error doesn't
reserve space for the link header but it's unlikely a culprit.
(The diff was only compile tested so far...)
diff --git sys/netinet/ip_icmp.c sys/netinet/ip_icmp.c
index cdd60aa..b3ddea4 100644
--- sys/netinet/ip_icmp.c
+++ sys/netinet/ip_icmp.c
@@ -208,19 +208,21 @@ icmp_do_error(struct mbuf *n, int type, int code,
u_int32_t dest, int destmtu)
if (icmplen + ICMP_MINLEN > MCLBYTES)
icmplen = MCLBYTES - ICMP_MINLEN - sizeof (struct ip);
m = m_gethdr(M_DONTWAIT, MT_HEADER);
- if (m && (sizeof (struct ip) + icmplen + ICMP_MINLEN > MHLEN)) {
+ if (m && (max_linkhdr + sizeof(struct ip) + icmplen +
+ ICMP_MINLEN > MHLEN)) {
MCLGET(m, M_DONTWAIT);
if ((m->m_flags & M_EXT) == 0) {
m_freem(m);
m = NULL;
}
}
if (m == NULL)
goto freeit;
+ m->m_data += max_linkhdr;
/* keep in same rtable */
m->m_pkthdr.ph_rtableid = n->m_pkthdr.ph_rtableid;
m->m_len = icmplen + ICMP_MINLEN;
if ((m->m_flags & M_EXT) == 0)
MH_ALIGN(m, m->m_len);
Re: acpiec on acer aspire S7 with CURRENT
On Mon, Oct 24, 2016 at 10:50:00PM +0300, Paul Irofti wrote:
> On Mon, Oct 24, 2016 at 07:46:33PM +0200, Mark Kettenis wrote:
> > > From: Paul Irofti
> > > Date: Mon, 24 Oct 2016 17:12:01 +0300
> > >
> > > Any thoughts on this?
> >
> > Sorry, yes. Adding the crs "index" as the last argument of the
> > callback function seems a bit non-intuitive to me. I'd say the void *
> > argument should remain the last argument, and the crs "number" should
> > be the first, although I could live with it being the second.
> >
> > I feel a bit bad though for not suggesting that earlier.
>
> Sure, makes sense. I thought about doing that too, but I did not know
> how much breakage I could do to the original function.
>
> What about crsno, do you prefer it to be called crsidx? That might be
> a better name...
Like this?
Index: acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.316
diff -u -p -u -p -r1.316 acpi.c
--- acpi.c 18 Sep 2016 23:56:45 - 1.316
+++ acpi.c 24 Oct 2016 20:00:30 -
@@ -513,10 +513,10 @@ TAILQ_HEAD(, acpi_pci) acpi_pcirootdevs
TAILQ_HEAD_INITIALIZER(acpi_pcirootdevs);
int acpi_getpci(struct aml_node *node, void *arg);
-int acpi_getminbus(union acpi_resource *crs, void *arg);
+int acpi_getminbus(int crsidx, union acpi_resource *crs, void *arg);
int
-acpi_getminbus(union acpi_resource *crs, void *arg)
+acpi_getminbus(int crsidx, union acpi_resource *crs, void *arg)
{
int *bbn = arg;
int typ = AML_CRSTYPE(crs);
Index: acpiprt.c
===
RCS file: /cvs/src/sys/dev/acpi/acpiprt.c,v
retrieving revision 1.47
diff -u -p -u -p -r1.47 acpiprt.c
--- acpiprt.c 14 Mar 2015 03:38:46 - 1.47
+++ acpiprt.c 24 Oct 2016 20:00:31 -
@@ -60,8 +60,8 @@ SIMPLEQ_HEAD(, acpiprt_map) acpiprt_map_
intacpiprt_match(struct device *, void *, void *);
void acpiprt_attach(struct device *, struct device *, void *);
-intacpiprt_getirq(union acpi_resource *crs, void *arg);
-intacpiprt_chooseirq(union acpi_resource *, void *);
+intacpiprt_getirq(int, union acpi_resource *, void *);
+intacpiprt_chooseirq(int, union acpi_resource *, void *);
struct acpiprt_softc {
struct device sc_dev;
@@ -137,7 +137,7 @@ acpiprt_attach(struct device *parent, st
}
int
-acpiprt_getirq(union acpi_resource *crs, void *arg)
+acpiprt_getirq(int crsidx, union acpi_resource *crs, void *arg)
{
struct acpiprt_irq *irq = arg;
int typ, len;
@@ -190,7 +190,7 @@ acpiprt_pri[16] = {
};
int
-acpiprt_chooseirq(union acpi_resource *crs, void *arg)
+acpiprt_chooseirq(int crsidx, union acpi_resource *crs, void *arg)
{
struct acpiprt_irq *irq = arg;
int typ, len, i, pri = -1;
Index: bytgpio.c
===
RCS file: /cvs/src/sys/dev/acpi/bytgpio.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 bytgpio.c
--- bytgpio.c 8 May 2016 11:08:01 - 1.11
+++ bytgpio.c 24 Oct 2016 20:00:31 -
@@ -104,7 +104,7 @@ const int byt_sus_pins[] = {
56, 54, 49, 55, 48, 57, 50, 58, 52, 53, 59, 40
};
-intbytgpio_parse_resources(union acpi_resource *, void *);
+intbytgpio_parse_resources(int, union acpi_resource *, void *);
intbytgpio_read_pin(void *, int);
void bytgpio_write_pin(void *, int, int);
void bytgpio_intr_establish(void *, int, int, int (*)(), void *);
@@ -238,7 +238,7 @@ free:
}
int
-bytgpio_parse_resources(union acpi_resource *crs, void *arg)
+bytgpio_parse_resources(int crsidx, union acpi_resource *crs, void *arg)
{
struct bytgpio_softc *sc = arg;
int type = AML_CRSTYPE(crs);
Index: chvgpio.c
===
RCS file: /cvs/src/sys/dev/acpi/chvgpio.c,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 chvgpio.c
--- chvgpio.c 8 May 2016 18:18:42 - 1.5
+++ chvgpio.c 24 Oct 2016 20:00:31 -
@@ -143,7 +143,7 @@ const int chv_southeast_pins[] = {
8, 12, 6, 8, 10, 11, -1
};
-intchvgpio_parse_resources(union acpi_resource *, void *);
+intchvgpio_parse_resources(int, union acpi_resource *, void *);
intchvgpio_check_pin(struct chvgpio_softc *, int);
intchvgpio_read_pin(void *, int);
void chvgpio_write_pin(void *, int, int);
@@ -264,7 +264,7 @@ unmap:
}
int
-chvgpio_parse_resources(union acpi_resource *crs, void *arg)
+chvgpio_parse_resources(int crsidx, union acpi_resource *crs, void *arg)
{
struct chvgpio_softc *sc = arg;
int type = AML_CRSTYPE(crs);
Index: dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.225
diff -u -p -u -p -r1.225 dsdt.c
--- dsdt.c 27 Sep 2016 10:04:19 - 1.225
+++ dsdt.c 24 Oct 2016 20:00:31 -
@@ -1621,14 +1621,14 @@ aml
Re: acpiec on acer aspire S7 with CURRENT
On Mon, Oct 24, 2016 at 07:46:33PM +0200, Mark Kettenis wrote: > > From: Paul Irofti > > Date: Mon, 24 Oct 2016 17:12:01 +0300 > > > > Any thoughts on this? > > Sorry, yes. Adding the crs "index" as the last argument of the > callback function seems a bit non-intuitive to me. I'd say the void * > argument should remain the last argument, and the crs "number" should > be the first, although I could live with it being the second. > > I feel a bit bad though for not suggesting that earlier. Sure, makes sense. I thought about doing that too, but I did not know how much breakage I could do to the original function. What about crsno, do you prefer it to be called crsidx? That might be a better name...
Re: acpiec on acer aspire S7 with CURRENT
> From: Paul Irofti
> Date: Mon, 24 Oct 2016 17:12:01 +0300
>
> Any thoughts on this?
Sorry, yes. Adding the crs "index" as the last argument of the
callback function seems a bit non-intuitive to me. I'd say the void *
argument should remain the last argument, and the crs "number" should
be the first, although I could live with it being the second.
I feel a bit bad though for not suggesting that earlier.
> -Mesaj original-
> De la: "Paul Irofti"
> Trimis: â22.â10.â2016 15:34
> CÄtre: "Mark Kettenis"
> Cc: "guent...@gmail.com" ; "tech@openbsd.org"
>
> Subiect: Re: acpiec on acer aspire S7 with CURRENT
>
> > I'm not really happy about the use of the static variable in the
> > resource parsing function. It feels like the aml_parse_resource API
> > should be changed to pass the resource number to the callback
> > function.
>
> I hated doing that as well, but I was afraid it wouldn't be justified to
> change the prototype and all the callers for this particular use case.
> Glad you suggested it. Here is a diff to add support for that in the
> current code.
>
>
> Index: acpi.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> retrieving revision 1.316
> diff -u -p -u -p -r1.316 acpi.c
> --- acpi.c18 Sep 2016 23:56:45 - 1.316
> +++ acpi.c22 Oct 2016 12:29:11 -
> @@ -513,10 +513,10 @@ TAILQ_HEAD(, acpi_pci) acpi_pcirootdevs
> TAILQ_HEAD_INITIALIZER(acpi_pcirootdevs);
>
> int acpi_getpci(struct aml_node *node, void *arg);
> -int acpi_getminbus(union acpi_resource *crs, void *arg);
> +int acpi_getminbus(union acpi_resource *crs, void *arg, int crsno);
>
> int
> -acpi_getminbus(union acpi_resource *crs, void *arg)
> +acpi_getminbus(union acpi_resource *crs, void *arg, int crsno)
> {
> int *bbn = arg;
> int typ = AML_CRSTYPE(crs);
> Index: acpiprt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpiprt.c,v
> retrieving revision 1.47
> diff -u -p -u -p -r1.47 acpiprt.c
> --- acpiprt.c 14 Mar 2015 03:38:46 - 1.47
> +++ acpiprt.c 22 Oct 2016 12:29:11 -
> @@ -60,8 +60,8 @@ SIMPLEQ_HEAD(, acpiprt_map) acpiprt_map_
>
> int acpiprt_match(struct device *, void *, void *);
> void acpiprt_attach(struct device *, struct device *, void *);
> -int acpiprt_getirq(union acpi_resource *crs, void *arg);
> -int acpiprt_chooseirq(union acpi_resource *, void *);
> +int acpiprt_getirq(union acpi_resource *crs, void *arg, int crsno);
> +int acpiprt_chooseirq(union acpi_resource *, void *, int);
>
> struct acpiprt_softc {
> struct device sc_dev;
> @@ -137,7 +137,7 @@ acpiprt_attach(struct device *parent, st
> }
>
> int
> -acpiprt_getirq(union acpi_resource *crs, void *arg)
> +acpiprt_getirq(union acpi_resource *crs, void *arg, int crsno)
> {
> struct acpiprt_irq *irq = arg;
> int typ, len;
> @@ -190,7 +190,7 @@ acpiprt_pri[16] = {
> };
>
> int
> -acpiprt_chooseirq(union acpi_resource *crs, void *arg)
> +acpiprt_chooseirq(union acpi_resource *crs, void *arg, int crsno)
> {
> struct acpiprt_irq *irq = arg;
> int typ, len, i, pri = -1;
> Index: bytgpio.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/bytgpio.c,v
> retrieving revision 1.11
> diff -u -p -u -p -r1.11 bytgpio.c
> --- bytgpio.c 8 May 2016 11:08:01 - 1.11
> +++ bytgpio.c 22 Oct 2016 12:29:11 -
> @@ -104,7 +104,7 @@ const int byt_sus_pins[] = {
> 56, 54, 49, 55, 48, 57, 50, 58, 52, 53, 59, 40
> };
>
> -int bytgpio_parse_resources(union acpi_resource *, void *);
> +int bytgpio_parse_resources(union acpi_resource *, void *, int crsno);
> int bytgpio_read_pin(void *, int);
> void bytgpio_write_pin(void *, int, int);
> void bytgpio_intr_establish(void *, int, int, int (*)(), void *);
> @@ -238,7 +238,7 @@ free:
> }
>
> int
> -bytgpio_parse_resources(union acpi_resource *crs, void *arg)
> +bytgpio_parse_resources(union acpi_resource *crs, void *arg, int crsno)
> {
> struct bytgpio_softc *sc = arg;
> int type = AML_CRSTYPE(crs);
> Index: chvgpio.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/chvgpio.c,v
> retrieving revision 1.5
> diff -u -p -u -p -r1.5 chvgpio.c
> --- chvgpio.c 8 May 2016 18:18:42 - 1.5
> +++ chvgpio.c 22 Oct 2016 12:29:11 -
> @@ -143,7 +143,7 @@ const int chv_southeast_pins[] = {
> 8, 12, 6, 8, 10, 11, -1
> };
>
> -int chvgpio_parse_resources(union acpi_resource *, void *);
> +int chvgpio_parse_resources(union acpi_resource *, void *, int);
> int chvgpio_check_pin(struct chvgpio_softc *, int);
> int chvgpio_read_pin(void *, int);
> void chvgpio_write_pin(void *, int, int);
> @@ -264,7 +264,7 @@ unmap:
> }
>
> int
> -chvgpio_parse_resources(union acpi_resource *crs, void *arg)
> +chvgpio_parse_resources(union
Re: switchd(8): add flow_mod validation
On Wed, Oct 12, 2016 at 05:39:17PM +0200, Rafael Zalamena wrote:
> This diff teaches switchd(8) how to validate flow_mod messages, more
> specifically the flow instructions and actions. The oxm validations
> were already implemented so we get them for free here.
I've updated the flow_mod diff to also include the following changes:
- Better loop detection like I did for tcpdump(8);
- A small fix in packet-in OXM parsing (see note below);
- Reuse actions validation for packet-out and implement missing
payload truncation check;
- Moved the new code away from packet-out to make diff looks less
confusing;
Note:
In packet-in we shouldn't use omlen with header size included, it only
works because the padding is zeroed out. To avoid one more loop and
errornous zero header reading we should remove the header size.
ok?
Index: sys/net/ofp.h
===
RCS file: /home/obsdcvs/src/sys/net/ofp.h,v
retrieving revision 1.2
diff -u -p -r1.2 ofp.h
--- sys/net/ofp.h 30 Sep 2016 12:40:00 - 1.2
+++ sys/net/ofp.h 12 Oct 2016 15:22:59 -
@@ -315,14 +315,14 @@ struct ofp_action_push {
uint16_tap_type;
uint16_tap_len;
uint16_tap_ethertype;
- uint8_t pad[2];
+ uint8_t ap_pad[2];
} __packed;
struct ofp_action_pop_mpls {
uint16_tapm_type;
uint16_tapm_len;
uint16_tapm_ethertype;
- uint8_t pad[2];
+ uint8_t apm_pad[2];
} __packed;
struct ofp_action_group {
@@ -342,6 +342,12 @@ struct ofp_action_set_field {
uint16_tasf_type;
uint16_tasf_len;
uint8_t asf_field[4];
+} __packed;
+
+struct ofp_action_set_queue {
+ uint16_tasq_type;
+ uint16_tasq_len;
+ uint32_tasq_queue_id;
} __packed;
/* Packet-Out Message */
Index: usr.sbin/switchd/ofp13.c
===
RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.21
diff -u -p -r1.21 ofp13.c
--- usr.sbin/switchd/ofp13.c13 Oct 2016 08:29:14 - 1.21
+++ usr.sbin/switchd/ofp13.c24 Oct 2016 16:49:46 -
@@ -59,6 +59,12 @@ int ofp13_features_reply(struct switchd
int ofp13_validate_error(struct switchd *,
struct sockaddr_storage *, struct sockaddr_storage *,
struct ofp_header *, struct ibuf *);
+int ofp13_validate_action(struct switchd *, struct ofp_header *,
+ struct ibuf *, off_t *, struct ofp_action_header *);
+int ofp13_validate_instruction(struct switchd *, struct ofp_header *,
+ struct ibuf *, off_t *, struct ofp_instruction *);
+int ofp13_validate_flow_mod(struct switchd *, struct sockaddr_storage *,
+ struct sockaddr_storage *, struct ofp_header *, struct ibuf *);
int ofp13_validate_oxm_basic(struct ibuf *, off_t, int, uint8_t);
int ofp13_validate_oxm(struct switchd *, struct ofp_ox_match *,
struct ofp_header *, struct ibuf *, off_t);
@@ -129,7 +135,7 @@ struct ofp_callback ofp13_callbacks[] =
{ OFP_T_FLOW_REMOVED, ofp13_flow_removed, NULL },
{ OFP_T_PORT_STATUS,NULL, NULL },
{ OFP_T_PACKET_OUT, NULL, ofp13_validate_packet_out },
- { OFP_T_FLOW_MOD, NULL, NULL },
+ { OFP_T_FLOW_MOD, NULL, ofp13_validate_flow_mod },
{ OFP_T_GROUP_MOD, NULL, NULL },
{ OFP_T_PORT_MOD, NULL, NULL },
{ OFP_T_TABLE_MOD, NULL, NULL },
@@ -421,6 +427,7 @@ ofp13_validate_packet_in(struct switchd
log_debug("\tmatch type %s length %zu (padded to %zu)",
print_map(ntohs(om->om_type), ofp_match_map),
mlen, OFP_ALIGN(mlen) + ETHER_ALIGN);
+ mlen -= sizeof(*om);
/* current match offset, aligned offset after all matches */
moff = off + sizeof(*om);
@@ -468,10 +475,9 @@ ofp13_validate_packet_out(struct switchd
struct ofp_header *oh, struct ibuf *ibuf)
{
struct ofp_packet_out *pout;
- size_t len;
- off_toff;
+ size_t len, plen, diff;
+ off_toff, noff;
struct ofp_action_header*ah;
- struct ofp_action_output*ao;
off = 0;
if ((pout = ibuf_seek(ibuf, off, sizeof(*pout))) == NULL) {
@@ -480,36 +486,43 @@ ofp13_validate_packet_out(struct switchd
return (-1);
}
- log_debug("\tbuffer %d port %s "
- "actions length %u",
+ off += sizeof(*pout);
+ len = ntohs(pout->pout_actions_len);
+ log_debug("\tbuffer %d in_port %s actions_len %lu",
ntohl(pout->pout_buffer_id),
- print_map(ntohl(pout->pout_in_port), ofp_port_ma
network performance fun
Hi all, OpenBSD box acts as transit router for /8 networks without pf and with this sysctls ddb.console=1 kern.pool_debug=0 net.inet.ip.forwarding=1 net.inet.ip.ifq.maxlen=8192 netstat 11/8 192.168.11.2 UGS0 114466419 - 8 ix0 12/8 192.168.12.2 UGS00 - 8 ix1 13/8 192.168.13.2 UGS00 - 8 myx0 14/8 192.168.14.2 UGS00 - 8 myx1 15/8 192.168.15.2 UGS00 - 8 em3 16/8 192.168.16.2 UGS0 89907239 - 8 em2 17/8 192.168.17.2 UGS0 65791508 - 8 bge0 18/8 192.168.18.2 UGS00 - 8 bge1 while testing dlg@ "mcl2k2 mbuf clusters" patch with todays -current i saw that performance with plain -current drops for about 300Kpps vs -current from 06.10.2016. by bisecting cvs tree it seems that this commit is guilty for this http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_ethersubr.c?rev=1.240&content-type=text/x-cvsweb-markup -current from 24.10.2016 ix sendreceive 690Kpps 690Kpps 700Kpps 680Kpps 800Kpps 350Kpps 1.4Mpps 305Kpps 14Mpps 305Kpps em sendreceive 690Kpps 690Kpps 700Kpps 680Kpps 800Kpps 700Kpps 1.4Mpps 700Kpps bge sendreceive 620Kpps 620Kpps 630Kpps 515Kpps 700Kpps 475Kpps 800Kpps 430Kpps 1.4Mpps 350Kpps -current with if_ethersubr.c version 1.239 ix sendreceive 910Kpps 910Kpps 920Kpps 820Kpps 1Mpps 825Kpps 1.4Mpps 700Kpps 14Mpps 700Kpps em sendreceive 940Kpps 940Kpps 950Kpps 845Kpps 1Mpps 855Kpps 1.4Mpps 845Kpps bge sendreceive 620Kpps 620Kpps 630Kpps 525Kpps 700Kpps 485Kpps 800Kpps 435Kpps 1.4Mpps 350Kpps applying dlg@ "mcl2k2 mbuf clusters" patch to todays -current brings performance back to ix and em ... bge is emotional as always :) ix sendreceive 900Kpps 900Kpps 910Kpps 895Kpps 1Mpps 895Kpps 1.4Mpps 810Kpps 14Mpps 815Kpps em sendreceive 940Kpps 940Kpps 950Kpps 930Kpps 1Mpps 920Kpps 1.4Mpps 930Kpps bge sendreceive 620Kpps 620Kpps 630Kpps 520Kpps 700Kpps 475Kpps 800Kpps 430Kpps 1.4Mpps 366Kpps i honestly don't know what all that means, i'm just writing my observation ... OpenBSD 6.0-current (GENERIC.MP) #5: Mon Oct 24 18:12:28 CEST 2016 r...@x3550m4.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP RTC BIOS diagnostic error 80 real mem = 34315051008 (32725MB) avail mem = 33270534144 (31729MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0x7e67c000 (84 entries) bios0: vendor IBM version "-[D7E154BUS-2.21]-" date 08/08/2016 bios0: IBM IBM System x3550 M4 Server -[7914T91]- acpi0 at bios0: rev 2 acpi0: sleep states S0 S5 acpi0: tables DSDT FACP TCPA ERST HEST HPET APIC MCFG OEM0 OEM1 SLIT SRAT SLIC SSDT SSDT SSDT SSDT DMAR acpi0: wakeup devices MRP1(S4) DCC0(S4) MRP3(S4) MRP5(S4) EHC2(S5) PEX0(S5) PEX7(S5) EHC1(S5) IP2P(S3) MRPB(S4) MRPC(S4) MRPD(S4) MRPF(S4) MRPG(S4) MRPH(S4) MRPI(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) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.36 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,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT 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, IBE cpu1 at mainbus0: apid 2 (application processor) cpu1: Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.00 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,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 4 (application processor) cpu2: Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2399.99 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,
Re: fix carp "carppeer" + "real mac-address"
On Mon, Oct 24, 2016 at 12:04:37PM +0900, YASUOKA Masahiko wrote:
> FYI. I just commited regress tests on regress/netinet/carp/ .
> carp_4.sh demostrates the problem which the diff fixes.
root@ot1:.../carp# make
ksh /usr/src/regress/sys/netinet/carp/carp_1.sh -R "11 12" -I "11 12"
ksh /usr/src/regress/sys/netinet/carp/carp_2.sh -R "11 12" -I "11 12"
ksh /usr/src/regress/sys/netinet/carp/carp_3.sh -R "11 12" -I "11 12"
ksh /usr/src/regress/sys/netinet/carp/carp_4.sh -R "11 12" -I "11 12"
*** Error 1 in . (Makefile:13 'carp_4')
FAILED
*** Error 1 in target 'regress' (ignored)
A test that does not print a reason when it fails is hard to debug.
Even with -v it becomes not clear why it fails. You may want to
print the failed condition when you do FAILS=$((FAILS + 1)).
> On Tue, 18 Oct 2016 19:33:01 +0900 (JST)
> YASUOKA Masahiko wrote:
> > Currenlty when carppeer + "real mac-address" are used at once,
> > changing MASTER by carpdemote causes "MASTER-MASTER" problem, BACKUP
> > is to become MASTER but the present MASTER also will keep its state.
> >
> > The diff following will fix this problem.
> >
> > ok?
I cannot see a downside of accepting such a packet.
OK bluhm@
> >
> > Accept CARP advertisement packets whose destination is not for multicast.
> > When both "carppeer" and "real mac-address" are used at once and the
> > BACKUP is to take over the new MASTER, the present MASTER receives
> > such packets.
> >
> > found by and diff from nagasaka at iij
> >
> > diff --git a/sys/netinet/ip_carp.c b/sys/netinet/ip_carp.c
> > index 9eb5552..bfbf430 100644
> > --- a/sys/netinet/ip_carp.c
> > +++ b/sys/netinet/ip_carp.c
> > @@ -453,7 +453,7 @@ carp_proto_input_if(struct ifnet *ifp, struct mbuf *m,
> > int hlen)
> > ismulti = IN_MULTICAST(ip->ip_dst.s_addr);
> >
> > /* check if received on a valid carp interface */
> > - if (!((ifp->if_type == IFT_CARP && ismulti) ||
> > + if (!(ifp->if_type == IFT_CARP ||
> > (ifp->if_type != IFT_CARP && !ismulti && ifp->if_carp != NULL))) {
> > carpstats.carps_badif++;
> > CARP_LOG(LOG_INFO, sc,
> >
Re: acpiec on acer aspire S7 with CURRENT
Any thoughts on this?
-Mesaj original-
De la: "Paul Irofti"
Trimis: 22.10.2016 15:34
Către: "Mark Kettenis"
Cc: "guent...@gmail.com" ; "tech@openbsd.org"
Subiect: Re: acpiec on acer aspire S7 with CURRENT
> I'm not really happy about the use of the static variable in the
> resource parsing function. It feels like the aml_parse_resource API
> should be changed to pass the resource number to the callback
> function.
I hated doing that as well, but I was afraid it wouldn't be justified to
change the prototype and all the callers for this particular use case.
Glad you suggested it. Here is a diff to add support for that in the
current code.
Index: acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.316
diff -u -p -u -p -r1.316 acpi.c
--- acpi.c 18 Sep 2016 23:56:45 - 1.316
+++ acpi.c 22 Oct 2016 12:29:11 -
@@ -513,10 +513,10 @@ TAILQ_HEAD(, acpi_pci) acpi_pcirootdevs
TAILQ_HEAD_INITIALIZER(acpi_pcirootdevs);
int acpi_getpci(struct aml_node *node, void *arg);
-int acpi_getminbus(union acpi_resource *crs, void *arg);
+int acpi_getminbus(union acpi_resource *crs, void *arg, int crsno);
int
-acpi_getminbus(union acpi_resource *crs, void *arg)
+acpi_getminbus(union acpi_resource *crs, void *arg, int crsno)
{
int *bbn = arg;
int typ = AML_CRSTYPE(crs);
Index: acpiprt.c
===
RCS file: /cvs/src/sys/dev/acpi/acpiprt.c,v
retrieving revision 1.47
diff -u -p -u -p -r1.47 acpiprt.c
--- acpiprt.c 14 Mar 2015 03:38:46 - 1.47
+++ acpiprt.c 22 Oct 2016 12:29:11 -
@@ -60,8 +60,8 @@ SIMPLEQ_HEAD(, acpiprt_map) acpiprt_map_
intacpiprt_match(struct device *, void *, void *);
void acpiprt_attach(struct device *, struct device *, void *);
-intacpiprt_getirq(union acpi_resource *crs, void *arg);
-intacpiprt_chooseirq(union acpi_resource *, void *);
+intacpiprt_getirq(union acpi_resource *crs, void *arg, int crsno);
+intacpiprt_chooseirq(union acpi_resource *, void *, int);
struct acpiprt_softc {
struct device sc_dev;
@@ -137,7 +137,7 @@ acpiprt_attach(struct device *parent, st
}
int
-acpiprt_getirq(union acpi_resource *crs, void *arg)
+acpiprt_getirq(union acpi_resource *crs, void *arg, int crsno)
{
struct acpiprt_irq *irq = arg;
int typ, len;
@@ -190,7 +190,7 @@ acpiprt_pri[16] = {
};
int
-acpiprt_chooseirq(union acpi_resource *crs, void *arg)
+acpiprt_chooseirq(union acpi_resource *crs, void *arg, int crsno)
{
struct acpiprt_irq *irq = arg;
int typ, len, i, pri = -1;
Index: bytgpio.c
===
RCS file: /cvs/src/sys/dev/acpi/bytgpio.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 bytgpio.c
--- bytgpio.c 8 May 2016 11:08:01 - 1.11
+++ bytgpio.c 22 Oct 2016 12:29:11 -
@@ -104,7 +104,7 @@ const int byt_sus_pins[] = {
56, 54, 49, 55, 48, 57, 50, 58, 52, 53, 59, 40
};
-intbytgpio_parse_resources(union acpi_resource *, void *);
+intbytgpio_parse_resources(union acpi_resource *, void *, int crsno);
intbytgpio_read_pin(void *, int);
void bytgpio_write_pin(void *, int, int);
void bytgpio_intr_establish(void *, int, int, int (*)(), void *);
@@ -238,7 +238,7 @@ free:
}
int
-bytgpio_parse_resources(union acpi_resource *crs, void *arg)
+bytgpio_parse_resources(union acpi_resource *crs, void *arg, int crsno)
{
struct bytgpio_softc *sc = arg;
int type = AML_CRSTYPE(crs);
Index: chvgpio.c
===
RCS file: /cvs/src/sys/dev/acpi/chvgpio.c,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 chvgpio.c
--- chvgpio.c 8 May 2016 18:18:42 - 1.5
+++ chvgpio.c 22 Oct 2016 12:29:11 -
@@ -143,7 +143,7 @@ const int chv_southeast_pins[] = {
8, 12, 6, 8, 10, 11, -1
};
-intchvgpio_parse_resources(union acpi_resource *, void *);
+intchvgpio_parse_resources(union acpi_resource *, void *, int);
intchvgpio_check_pin(struct chvgpio_softc *, int);
intchvgpio_read_pin(void *, int);
void chvgpio_write_pin(void *, int, int);
@@ -264,7 +264,7 @@ unmap:
}
int
-chvgpio_parse_resources(union acpi_resource *crs, void *arg)
+chvgpio_parse_resources(union acpi_resource *crs, void *arg, int crsno)
{
struct chvgpio_softc *sc = arg;
int type = AML_CRSTYPE(crs);
Index: dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.225
diff -u -p -u -p -r1.225 dsdt.c
--- dsdt.c 27 Sep 2016 10:04:19 - 1.225
+++ dsdt.c 22 Oct 2016 12:29:12 -
@@ -1621,14 +1621,14 @@ aml_mapresource(union acpi_resource *crs
int
aml_parse_resource(struct aml_value *res,
-int (*crs_enum)(union acpi_resource *, void *), void
Re: (re)name cpumem_realloc to cpumem_malloc_ncpus
On Mon, Oct 24, 2016 at 02:48:03PM +1000, David Gwynne wrote:
> cos its not resizing the allocation, its allocating them for new cpus.
realloc is not a good name, so yes to renaming.
> the same goes for counters_realloc being named counters_alloc_ncpus.
Does the n mean new? This is never mentioned, I initially thought
it means number. What about ..._newcpus or ..._allcpus?
> +.Fn COUNTERS_BOOT_INITIALIZER
> +is used to initialise a cpumem pointer with the memory that was previously
> +allocated using
> +.Fn COUNTERS_BOOT_MEMORY
> +and identified by
> +.Fa NAME .
The boot counters are zero initially.
> +The
> +.Fa type
> +argument specifies the type of memory that the counters will be
> +allocated as via
> +.Xr malloc 9 .
> +The memory will be zeroed on allocation by passing
> +.Fn M_ZERO
Is the M_ZERO an implementation detail for counters that should not
be mentionend in the man page?
> +to
> +.Xr malloc 9 .
The counters on the boot cpu are preserved.
> +.Fn CPUMEM_BOOT_INITIALIZER
> +is used to initialise a cpumem pointer with the memory that was previously
> +allocated using
> +.Fn CPUMEM_BOOT_MEMORY
> +and identified by
> +.Fa NAME .
The boot cpu uses static memory that is initially zero.
> +.Fn cpumem_malloc_ncpus
> +allocates
> +.Fa sz
> +bytes of
> +.Fa type
> +memory for each additional CPU using
> +.Xr malloc 9 .
> +The memory will be zeroed on allocation by passing
> +.Fn M_ZERO
> +to
> +.Xr malloc 9 .
The the memory of the boot cpu is preserved.
> ===
> RCS file: /cvs/src/sys/kern/subr_percpu.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 subr_percpu.c
> --- sys/kern/subr_percpu.c24 Oct 2016 03:15:38 - 1.3
> +++ sys/kern/subr_percpu.c24 Oct 2016 04:43:34 -
> @@ -76,7 +76,7 @@ cpumem_malloc(size_t sz, int type)
> }
>
> struct cpumem *
> -cpumem_realloc(struct cpumem *bootcm, size_t sz, int type)
> +cpumem_alloc_ncpus(struct cpumem *bootcm, size_t sz, int type)
This should be cpumem_malloc_ncpus.
> {
> struct cpumem *cm;
> unsigned int cpu;
> @@ -146,10 +146,10 @@ counters_alloc(unsigned int n, int type)
> }
>
> struct cpumem *
> -counters_realloc(struct cpumem *cm, unsigned int n, int type)
> +counters_alloc_ncpus(struct cpumem *cm, unsigned int n, int type)
> {
> n++; /* the generation number */
> - return (cpumem_realloc(cm, n * sizeof(uint64_t), type));
> + return (cpumem_alloc_ncpus(cm, n * sizeof(uint64_t), type));
This should be cpumem_malloc_ncpus.
> }
>
> void
> @@ -259,7 +259,7 @@ cpumem_malloc(size_t sz, int type)
> }
>
> struct cpumem *
> -cpumem_realloc(struct cpumem *cm, size_t sz, int type)
> +cpumem_allod_ncpus(struct cpumem *cm, size_t sz, int type)
This should be cpumem_malloc_ncpus.
> {
> return (cm);
> }
> @@ -291,10 +291,10 @@ counters_alloc(unsigned int n, int type)
> }
>
> struct cpumem *
> -counters_realloc(struct cpumem *cm, unsigned int n, int type)
> +counters_alloc_ncpus(struct cpumem *cm, unsigned int n, int type)
> {
> /* this is unecessary, but symmetrical */
> - return (cpumem_realloc(cm, n * sizeof(uint64_t), type));
> + return (cpumem_alloc_ncpus(cm, n * sizeof(uint64_t), type));
This should be cpumem_malloc_ncpus.
> }
>
> void
> Index: sys/sys/percpu.h
> ===
> RCS file: /cvs/src/sys/sys/percpu.h,v
> retrieving revision 1.2
> diff -u -p -r1.2 percpu.h
> --- sys/sys/percpu.h 24 Oct 2016 03:15:35 - 1.2
> +++ sys/sys/percpu.h 24 Oct 2016 04:43:34 -
> @@ -54,7 +54,7 @@ struct cpumem *cpumem_get(struct pool *)
> void cpumem_put(struct pool *, struct cpumem *);
>
> struct cpumem*cpumem_malloc(size_t, int);
> -struct cpumem*cpumem_realloc(struct cpumem *, size_t, int);
> +struct cpumem*cpumem_malloc_ncpus(struct cpumem *, size_t, int);
I wonder how this compiled as you use cpumem_malloc_ncpus correctly
here.
> void cpumem_free(struct cpumem *, int, size_t);
>
> void *cpumem_first(struct cpumem_iter *, struct cpumem *);
> @@ -111,7 +111,7 @@ static struct {
> \
> */
>
> struct cpumem*counters_alloc(unsigned int, int);
> -struct cpumem*counters_realloc(struct cpumem *, unsigned int, int);
> +struct cpumem*counters_alloc_ncpus(struct cpumem *, unsigned int,
> int);
> void counters_free(struct cpumem *, int, unsigned int);
> void counters_read(struct cpumem *, uint64_t *, unsigned int);
> void counters_zero(struct cpumem *, unsigned int);
Re: Quirks for pkcbc?
On Sun, Oct 23, 2016 at 03:42:59PM -0600, Theo de Raadt wrote: > [...] > I ask you to figure out logical conditions under which this should > happen, or if some interaction is wrong. > [...] Understood. I'll try tracking down the exact circumstances of the issue and come back with a better diff. -- Gregor signature.asc Description: PGP signature
iwn: stop forcing RTS for all frames in 11n mode
This is similar to a recent change in iwm, but it only affects 11n mode
in this driver.
The condition being removed below was added by me before the RTS threshold
was enabled in net80211. So now we should not need it anymore and removing
it might actually improve things by removing RTS overhead for small frames.
Please test.
Index: if_iwn.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.174
diff -u -p -r1.174 if_iwn.c
--- if_iwn.c8 Oct 2016 14:44:36 - 1.174
+++ if_iwn.c24 Oct 2016 09:13:36 -
@@ -2959,8 +2959,6 @@ iwn_tx(struct iwn_softc *sc, struct mbuf
else if (ic->ic_protmode == IEEE80211_PROT_RTSCTS)
flags |= IWN_TX_NEED_RTS;
}
- else if (ni->ni_flags & IEEE80211_NODE_HT)
- flags |= IWN_TX_NEED_RTS;
if (flags & (IWN_TX_NEED_RTS | IWN_TX_NEED_CTS)) {
if (sc->hw_type != IWN_HW_REV_TYPE_4965) {
Re: tun(4)/tap(4): fix mbuf header space check
On Mon, Oct 24, 2016 at 09:56:13AM +0200, Rafael Zalamena wrote:
> tun(4)/tap(4) function tun_dev_write() is checking for the wrong size for
> the mbuf packet header. We must check against MHLEN (the mbuf header data
> storage size) and not MINCLSIZE (smallest amount of data of a cluster).
>
> For the curious:
> MGETHDR() calls m_gethdr() which uses mbpool to get the mbuf data
> storage. mbpool is initialized at mbinit() which sets the members
> allocation size to MSIZE.
>
> - MSIZE is "256";
> - MLEN is "(MSIZE - sizeof(struct m_hdr))"
> - MHLEN is "(MLEN - sizeof(pkthdr))";
> - MINCLSIZE is "MHLEN + MLEN + 1";
>
> ok?
No, the current code is correct. At least there is no problem with it.
The idea is that for up to MINCLSIZE bytes no cluster is allocated and
instead up to 2 mbufs are connected. This was an optimisation because
allocating clusters was hard many years ago. It could well be that this no
longer makes sense but in that case it would make more sense to adjust
MINCLSIZE and get the benefit on all loops building mbuf chains at the
same time.
>
> Index: sys/net/if_tun.c
> ===
> RCS file: /home/obsdcvs/src/sys/net/if_tun.c,v
> retrieving revision 1.169
> diff -u -p -r1.169 if_tun.c
> --- sys/net/if_tun.c 4 Sep 2016 15:46:39 - 1.169
> +++ sys/net/if_tun.c 24 Oct 2016 07:43:03 -
> @@ -895,7 +895,7 @@ tun_dev_write(struct tun_softc *tp, stru
> if (m == NULL)
> return (ENOBUFS);
> mlen = MHLEN;
> - if (uio->uio_resid >= MINCLSIZE) {
> + if (uio->uio_resid >= MHLEN) {
> MCLGET(m, M_DONTWAIT);
> if (!(m->m_flags & M_EXT)) {
> m_free(m);
> @@ -926,7 +926,7 @@ tun_dev_write(struct tun_softc *tp, stru
> break;
> }
> mlen = MLEN;
> - if (uio->uio_resid >= MINCLSIZE) {
> + if (uio->uio_resid >= MHLEN) {
> MCLGET(m, M_DONTWAIT);
> if (!(m->m_flags & M_EXT)) {
> error = ENOBUFS;
>
--
:wq Claudio
Re: (re)name cpumem_realloc to cpumem_malloc_ncpus
David Gwynne wrote: > cos its not resizing the allocation, its allocating them for new cpus. > > the same goes for counters_realloc being named counters_alloc_ncpus. > > this adds doco for them too. Hi, FWIW it makes sense to me, for when I was looking at the per-cpu mbstat diff, I blocked a bit on 'init' doing a realloc, and had to look at the impl to understand its purpose. My 2 cents..
tun(4)/tap(4): fix mbuf header space check
tun(4)/tap(4) function tun_dev_write() is checking for the wrong size for
the mbuf packet header. We must check against MHLEN (the mbuf header data
storage size) and not MINCLSIZE (smallest amount of data of a cluster).
For the curious:
MGETHDR() calls m_gethdr() which uses mbpool to get the mbuf data
storage. mbpool is initialized at mbinit() which sets the members
allocation size to MSIZE.
- MSIZE is "256";
- MLEN is "(MSIZE - sizeof(struct m_hdr))"
- MHLEN is "(MLEN - sizeof(pkthdr))";
- MINCLSIZE is "MHLEN + MLEN + 1";
ok?
Index: sys/net/if_tun.c
===
RCS file: /home/obsdcvs/src/sys/net/if_tun.c,v
retrieving revision 1.169
diff -u -p -r1.169 if_tun.c
--- sys/net/if_tun.c4 Sep 2016 15:46:39 - 1.169
+++ sys/net/if_tun.c24 Oct 2016 07:43:03 -
@@ -895,7 +895,7 @@ tun_dev_write(struct tun_softc *tp, stru
if (m == NULL)
return (ENOBUFS);
mlen = MHLEN;
- if (uio->uio_resid >= MINCLSIZE) {
+ if (uio->uio_resid >= MHLEN) {
MCLGET(m, M_DONTWAIT);
if (!(m->m_flags & M_EXT)) {
m_free(m);
@@ -926,7 +926,7 @@ tun_dev_write(struct tun_softc *tp, stru
break;
}
mlen = MLEN;
- if (uio->uio_resid >= MINCLSIZE) {
+ if (uio->uio_resid >= MHLEN) {
MCLGET(m, M_DONTWAIT);
if (!(m->m_flags & M_EXT)) {
error = ENOBUFS;
