Re: Should the patterns(7) manual use C-style indexing of strings?
On Thu, Jun 25, 2015 at 02:31:24PM +0200, Theo Buehler wrote: > > Thus, I suggest to modify the patterns(7) manual as follows: > > Index: usr.sbin/httpd/patterns.7 > === > RCS file: /cvs/src/usr.sbin/httpd/patterns.7,v > retrieving revision 1.2 > diff -u -p -r1.2 patterns.7 > --- usr.sbin/httpd/patterns.7 23 Jun 2015 17:29:19 - 1.2 > +++ usr.sbin/httpd/patterns.7 25 Jun 2015 12:11:19 - > @@ -268,7 +268,7 @@ For instance, if we apply the pattern > .Qq ()aa() > on the string > .Qq flaaap , > -there will be two captures: 3 and 5. > +there will be two captures: 2 and 4. > .Sh SEE ALSO > .Xr fnmatch 3 , > .Xr re_format 7 , > Committed, thanks. -- Sebastien Marie
Brainy: Memory Leak in vax
Hi, I put here a bug among others: sys/arch/vax/if/if_qe.c --- 146 ring = malloc(PROBESIZE, M_TEMP, M_WAITOK | M_ZERO); bzero(sc, sizeof(struct qe_softc)); sc->sc_iot = ua->ua_iot; sc->sc_ioh = ua->ua_ioh; sc->sc_dmat = ua->ua_dmat; ubasc->uh_lastiv -= 4; QE_WCSR(QE_CSR_CSR, QE_RESET); QE_WCSR(QE_CSR_VECTOR, ubasc->uh_lastiv); /* * Map the ring area. Actually this is done only to be able to * send and receive a internal packet; some junk is loopbacked * so that the DEQNA has a reason to interrupt. */ ui.ui_size = PROBESIZE; ui.ui_vaddr = (caddr_t)&ring[0]; if ((error = uballoc((void *)parent, &ui, UBA_CANTWAIT))) return 0; 'ring' is leaked. Found by The Brainy Code Scanner. Maxime
Brainy: Memory Leak in ti
Hi,
I put here a bug among others:
sys/dev/ic/ti.c ---
648 MGETHDR(m_new, M_DONTWAIT, MT_DATA);
if (m_new == NULL)
return (ENOBUFS);
m_new->m_len = m_new->m_pkthdr.len = MHLEN;
m_adj(m_new, ETHER_ALIGN);
if (bus_dmamap_load_mbuf(sc->sc_dmatag, dmamap, m_new,
BUS_DMA_NOWAIT)) {
m_freem(m);
return (ENOBUFS);
}
'm_new' is leaked.
Found by The Brainy Code Scanner.
Maxime
one more fxp card
Hi. I've ordered two card from ebay and they are working fine on amd64. OK to commit? Index: fxp.4 === RCS file: /cvs/src/share/man/man4/fxp.4,v retrieving revision 1.45 diff -u -r1.45 fxp.4 --- fxp.4 15 Jan 2015 20:37:36 - 1.45 +++ fxp.4 26 Jun 2015 22:30:46 - @@ -63,6 +63,8 @@ Intel PRO/100 M .It Intel PRO/100 S +.It +Matrox NS-QNIC Quad-Port PCI .El .Pp The following media types and options (as given to
Re: Machine hangs on boot with snapshots 5.8-beta
On Fri, Jun 26, 2015 at 6:41 AM, Pedro Caetano wrote: > After applying this patch i was able to get the machines booting! Thanks for the confirmation; committed! Philip Guenther
Re: SMP steroids for PF
And the rest of us is watching and waiting for diffs to apply :) It is like exiting movie - I have popcorn in front. Culmination must be soon, I guess :) //mxb On 2015-06-26 19:09, Martin Pieuchot wrote: On 26/06/15(Fri) 17:19, Alexandr Nedvedicky wrote: On Fri, Jun 26, 2015 at 04:34:06PM +0200, Martin Pieuchot wrote: On 26/06/15(Fri) 16:00, Alexandr Nedvedicky wrote: 2) I saw that you found some ALTQ leftovers, you have some Solaris (2) I think ALTQs leftovers are still in CVS repo, will double check anyway. Stack alignment is not Solaris compatibility hack it's sparc compatibility. May be your C compiler takes care of this and grants 16/32/64 bit stack alignment. I have not examined build process that closely yet. By "Solaris compatibility" I'm referring to the size of ``sa_family_t'' and the corresponding changes in "struct pfr_table". I see. sa_family_t is kind of surprise it's defined as uint16_t on Solaris. PF at various places mixes sa_family_t with u_int8_t, so all af variables on Solaris had to be turned to sa_family_t. Some of those changes leaked backed during merge to current. Even if on OpenBSD sa_family_t is defined as uint8_t, I'd argue that for portability reasons we should use the correct type where it is appropriate. And here your "leak" is a good example of portability. If you have some changes that could improve the portability of the software and if you feel like sharing them, I'd be interested. Regards, Martin
Re: SMP steroids for PF
On 26/06/15(Fri) 17:19, Alexandr Nedvedicky wrote: > On Fri, Jun 26, 2015 at 04:34:06PM +0200, Martin Pieuchot wrote: > > On 26/06/15(Fri) 16:00, Alexandr Nedvedicky wrote: > > > > 2) I saw that you found some ALTQ leftovers, you have some Solaris > > > (2) I think ALTQs leftovers are still in CVS repo, will double check > > > anyway. Stack alignment is not Solaris compatibility hack it's sparc > > > compatibility. May be your C compiler takes care of this and grants > > > 16/32/64 bit stack alignment. I have not examined build process > > > that closely yet. > > > > By "Solaris compatibility" I'm referring to the size of ``sa_family_t'' > > and the corresponding changes in "struct pfr_table". > > > I see. sa_family_t is kind of surprise it's defined as uint16_t on Solaris. > PF at various places mixes sa_family_t with u_int8_t, so all af variables > on Solaris had to be turned to sa_family_t. Some of those changes leaked > backed during merge to current. Even if on OpenBSD sa_family_t is defined as uint8_t, I'd argue that for portability reasons we should use the correct type where it is appropriate. And here your "leak" is a good example of portability. If you have some changes that could improve the portability of the software and if you feel like sharing them, I'd be interested. Regards, Martin
Missing usb descriptor definition
This diff add the missing definition for the interface association descriptor.
The format isn't the same as with other descriptor, the struct
definition is separated from the typedef. Another diff should follow
doing that for every descriptor with long plan being to remove the
typedefs.
Index: sys/dev/usb/usb.h
===
RCS file: /cvs/src/sys/dev/usb/usb.h,v
retrieving revision 1.51
diff -u -p -r1.51 usb.h
--- sys/dev/usb/usb.h 22 Jun 2015 10:29:18 - 1.51
+++ sys/dev/usb/usb.h 26 Jun 2015 16:33:47 -
@@ -385,6 +385,18 @@ typedef struct {
#define UOTG_A_HNP_SUPPORT 4
#define UOTG_A_ALT_HNP_SUPPORT 5
+struct usb_iface_assoc_desc {
+uByte bLength;
+uByte bDescriptorType;
+uByte bFirstInterface;
+uByte bInterfaceCount;
+uByte bFunctionClass;
+uByte bFunctionSubClass;
+uByte bFunctionProtocol;
+uByte iFunction;
+} __packed;
+typedef struct usb_iface_assoc_desc usb_iface_assoc_descriptor_t;
+
typedef struct {
uWord wStatus;
/* Device status flags */
ntpd(8) constraint with a uniprocessor kernel
With ntpd(8) on a uniprocessor kernel (i386/amd64) I've found that poll(2)
often reacts to the SIGCHLD from a constraint child process before the
response message is received. Then constraint_check_child() closes the fd
and the response is lost.
# /usr/sbin/ntpd -dvs
ntp engine ready
constraint request to 150.101.170.174
constraint request to 150.101.170.153
constraint request to 150.101.170.146
constraint request to 150.101.170.166
constraint request to 150.101.170.159
constraint request to 150.101.170.152
constraint request to 150.101.170.160
constraint request to 150.101.170.187
no reply received in time, skipping initial time setting
^Cntp engine exiting
Terminating
The diff below improves the situation for me.
Nathanael
Index: constraint.c
===
RCS file: /cvs/src/usr.sbin/ntpd/constraint.c,v
retrieving revision 1.12
diff -u -p -r1.12 constraint.c
--- constraint.c28 May 2015 21:34:36 - 1.12
+++ constraint.c25 Jun 2015 02:11:46 -
@@ -277,7 +277,8 @@ constraint_check_child(void)
" received in time, next query %ds",
log_sockaddr((struct sockaddr *)
&cstr->addr->ss), CONSTRAINT_SCAN_INTERVAL);
- }
+ } else if (cstr->state < STATE_REPLY_RECEIVED)
+ constraint_dispatch_msg(cstr->fd);
if (fail || cstr->state < STATE_REPLY_RECEIVED) {
cstr->senderrors++;
@@ -372,7 +373,7 @@ constraint_remove(struct constraint *cst
}
int
-constraint_dispatch_msg(struct pollfd *pfd)
+constraint_dispatch_msg(int fd)
{
struct imsg imsg;
struct constraint *cstr;
@@ -380,20 +381,17 @@ constraint_dispatch_msg(struct pollfd *p
struct timeval tv[2];
double offset;
- if ((cstr = constraint_byfd(pfd->fd)) == NULL)
- return (0);
-
- if (!(pfd->revents & POLLIN))
+ if ((cstr = constraint_byfd(fd)) == NULL)
return (0);
if ((n = imsg_read(&cstr->ibuf)) == -1 || n == 0) {
- constraint_close(pfd->fd);
+ constraint_close(fd);
return (1);
}
for (;;) {
if ((n = imsg_get(&cstr->ibuf, &imsg)) == -1) {
- constraint_close(pfd->fd);
+ constraint_close(fd);
return (1);
}
if (n == 0)
Index: ntp.c
===
RCS file: /cvs/src/usr.sbin/ntpd/ntp.c,v
retrieving revision 1.132
diff -u -p -r1.132 ntp.c
--- ntp.c 25 May 2015 14:58:34 - 1.132
+++ ntp.c 25 Jun 2015 02:11:46 -
@@ -425,7 +421,11 @@ ntp_main(int pipe_prnt[2], int fd_ctl, s
}
for (; nfds > 0 && j < i; j++) {
- nfds -= constraint_dispatch_msg(&pfd[j]);
+ if (pfd[j].revents) {
+ nfds--;
+ if (pfd[j].revents & POLLIN)
+ constraint_dispatch_msg(pfd[j].fd);
+ }
}
for (s = TAILQ_FIRST(&conf->ntp_sensors); s != NULL;
Index: ntpd.h
===
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.h,v
retrieving revision 1.121
diff -u -p -r1.121 ntpd.h
--- ntpd.h 20 May 2015 13:32:39 - 1.121
+++ ntpd.h 25 Jun 2015 02:11:46 -
@@ -345,7 +345,7 @@ void constraint_add(struct constraint *
voidconstraint_remove(struct constraint *);
int constraint_init(struct constraint *);
int constraint_query(struct constraint *);
-int constraint_dispatch_msg(struct pollfd *);
+int constraint_dispatch_msg(int fd);
voidconstraint_check_child(void);
int constraint_check(double);
voidconstraint_dns(u_int32_t, u_int8_t *, size_t);
Re: SMP steroids for PF
On Fri, Jun 26, 2015 at 04:34:06PM +0200, Martin Pieuchot wrote: > On 26/06/15(Fri) 16:00, Alexandr Nedvedicky wrote: > > Hello Martin, > > > > I accept or your comments. I just have few quick notes/questions now. > > > > > 2) I saw that you found some ALTQ leftovers, you have some Solaris > > (2) I think ALTQs leftovers are still in CVS repo, will double check > > anyway. Stack alignment is not Solaris compatibility hack it's sparc > > compatibility. May be your C compiler takes care of this and grants > > 16/32/64 bit stack alignment. I have not examined build process > > that closely yet. > > By "Solaris compatibility" I'm referring to the size of ``sa_family_t'' > and the corresponding changes in "struct pfr_table". > I see. sa_family_t is kind of surprise it's defined as uint16_t on Solaris. PF at various places mixes sa_family_t with u_int8_t, so all af variables on Solaris had to be turned to sa_family_t. Some of those changes leaked backed during merge to current. > > (3) > > > use atomic operations rather than per-CPU counters or any other > > > solution? I'm also raising this question because some counters are > > can you point me to manual page or source code sample so I can have a look > > how > > to use per-CPU counter? > > There's no such manual. I was more asking about the reason for using > atomic operations. Is it because you're trying to use existing APIs? I'm taking what's available. > > > 5) I'm not sure to understand the goal of the new "pf_refcnt_t" type > > (5) Solaris defines pf_refcnt_t as 64-bit unsigned integer, pf_refcnt_t > > hopes > > to make porting easier. It can be defined as 32-bit on 32-bit machines. > > Using a long on OpenBSD will grantee that the value fits in a register, > so it should be fine. O.K. thanks for clarification. > > > > 7) The PF_SMP_INSERT_WQ() macro to replace SLIST_INSERT() seems over- > > PF_SMP_INSERT_WQ() purpose of those is to allow every CPU/thread to > > operate on its own work-queue of ktables/kentries. The current pf > > uses 'intrusive' link members pfrkt_workq/pfrke_workq in > > pfr_ktable/pfr_kentry. > > The only idea is to stay as much close to current version as possible. > > I understand that you want to stay close to the current version. I'm > just saying that we can also modify the current version to reduce the > size of your diff. I understand. I'll try to restructure the patches to make them easier for review. > > Regards, > Martin
ftp://ftp.fr
Hi. As of tomorrow morning (CET), ftp.fr will stop serving files over FTP. It is time people move to HTTP. Everything else will remain the same (cvs, rsync, ...); it's *only* the FTP service that is going away. Thank you. -- Antoine
Re: Better de(4) fix
> Date: Fri, 26 Jun 2015 15:17:41 +0200 (CEST) > From: Mark Kettenis > > > Date: Fri, 26 Jun 2015 14:58:36 +0200 > > From: Reyk Floeter > > > > On Thu, Jun 25, 2015 at 05:43:20PM -0700, Mike Larkin wrote: > > > On Fri, Jun 26, 2015 at 12:09:40AM +0200, Reyk Floeter wrote: > > > > On Thu, Jun 25, 2015 at 11:04:00PM +0200, Imre Vadasz wrote: > > > > > On 22:27 Thu 25 Jun , Reyk Floeter wrote: > > > > > > On Thu, Jun 25, 2015 at 09:21:11PM +0200, Mark Kettenis wrote: > > > > > > > There really is no excuse for using dma_alloc(9) if you have the > > > > > > > bus_dmatag_t available. > > > > > > > > > > > > > > This re-uses tulip_busdma_allocmem(), which simplifies the code > > > > > > > for > > > > > > > allocating the dmamap and such. > > > > > > > > > > > > > > Unfortunately I can't test this myself right now. > > > > > > > > > > > > > > > > > > > Fixes the panic on Hyper-V, see dmesg below. > > > > > > > > > > > > Unrelated to that, no interrupts on the nic with the ioapic enabled > > > > > > (no traffic and autoneg timeouts). > > > > > > > > > > > > Reyk > > > > > > > > > > On DragonFly BSD and NetBSD I was able to get the NIC interrupts > > > > > working > > > > > with enabled ioapic, by forcing the NIC's IRQ to be configured as > > > > > edge/high triggered instead of level/low in the ioapic. > > > > > > > > I had to try this: with a horrible hack(tm) in pci_intr_establish() I > > > > turned de0 into edge triggered and everything works. > > > > > > > > Reyk > > > > > > I suspect something is amiss in our interpretation of the interrupt config > > > on amd64 since de(4) works unmodified on i386 with Hyper-V. > > > > > > Maybe some AML parse screwup? > > > > > > > I can confirm this, I installed an i386 VM on the same Hyper-V machine > > and de works just fine without the hack. amd64's de does not. > > That is curious. There are quite a few differences between the amd64 > interrupt subsystem and the i386 interrupt subsystem, so there are > various possibilities on how things can go wrong. > > Are the Hyper-V ACPI tables available somewhere? Reyk provided an acpidump to me. The amd64 and i386 tables are pretty much identical, and I see no override for the de(4) interrupt, so it definitely should be active low, level triggered. So the non-working amd64 code is almost certainly a consequence of how OpenBSD handles things.
Re: SMP steroids for PF
On 26/06/15(Fri) 16:00, Alexandr Nedvedicky wrote: > Hello Martin, > > I accept or your comments. I just have few quick notes/questions now. > > > 2) I saw that you found some ALTQ leftovers, you have some Solaris > (2) I think ALTQs leftovers are still in CVS repo, will double check > anyway. Stack alignment is not Solaris compatibility hack it's sparc > compatibility. May be your C compiler takes care of this and grants > 16/32/64 bit stack alignment. I have not examined build process > that closely yet. By "Solaris compatibility" I'm referring to the size of ``sa_family_t'' and the corresponding changes in "struct pfr_table". > (3) > > use atomic operations rather than per-CPU counters or any other > > solution? I'm also raising this question because some counters are > can you point me to manual page or source code sample so I can have a look how > to use per-CPU counter? There's no such manual. I was more asking about the reason for using atomic operations. Is it because you're trying to use existing APIs? Are the macros implemented differently in Solaris? > > 5) I'm not sure to understand the goal of the new "pf_refcnt_t" type > (5) Solaris defines pf_refcnt_t as 64-bit unsigned integer, pf_refcnt_t hopes > to make porting easier. It can be defined as 32-bit on 32-bit machines. Using a long on OpenBSD will grantee that the value fits in a register, so it should be fine. > > 7) The PF_SMP_INSERT_WQ() macro to replace SLIST_INSERT() seems over- > PF_SMP_INSERT_WQ() purpose of those is to allow every CPU/thread to > operate on its own work-queue of ktables/kentries. The current pf > uses 'intrusive' link members pfrkt_workq/pfrke_workq in > pfr_ktable/pfr_kentry. > The only idea is to stay as much close to current version as possible. I understand that you want to stay close to the current version. I'm just saying that we can also modify the current version to reduce the size of your diff. Regards, Martin
Re: SMP steroids for PF
Hello Martin,
I accept or your comments. I just have few quick notes/questions now.
> 2) I saw that you found some ALTQ leftovers, you have some Solaris
(2) I think ALTQs leftovers are still in CVS repo, will double check
anyway. Stack alignment is not Solaris compatibility hack it's sparc
compatibility. May be your C compiler takes care of this and grants
16/32/64 bit stack alignment. I have not examined build process
that closely yet.
(3)
> use atomic operations rather than per-CPU counters or any other
> solution? I'm also raising this question because some counters are
can you point me to manual page or source code sample so I can have a look how
to use per-CPU counter?
> 5) I'm not sure to understand the goal of the new "pf_refcnt_t" type
(5) Solaris defines pf_refcnt_t as 64-bit unsigned integer, pf_refcnt_t hopes
to make porting easier. It can be defined as 32-bit on 32-bit machines.
> 7) The PF_SMP_INSERT_WQ() macro to replace SLIST_INSERT() seems over-
PF_SMP_INSERT_WQ() purpose of those is to allow every CPU/thread to
operate on its own work-queue of ktables/kentries. The current pf
uses 'intrusive' link members pfrkt_workq/pfrke_workq in pfr_ktable/pfr_kentry.
The only idea is to stay as much close to current version as possible.
I'll try to break the patch into smaller chunks of changes. And post them.
regards
sasha
On Fri, Jun 26, 2015 at 02:36:38PM +0200, Martin Pieuchot wrote:
> Hello Sasha,
>
> Alexandr Nedvedicky [alexandr.nedvedi...@oracle.com] wrote:
> > Hello,
> >
> > attached is SMP patch for PF. consider it as toxic proof of concept as it
> > has
> > paniced my amd64 system (see attached phone-shot). I have to figure out how
> > to
> > debug it yet. The problem is the USB keyboard has died, so I had no chance
> > to
> > type anything. fortunately the issue is 100% reproducible.
> >
> > The patch compiles in .MP and non-MP version. As you'll see more work is
> > needed to stabilize it and get full SMP support of PF. Those PF
> > features are not covered by SMP changes:
> > - packet queues
> > - packet logging
> > - pf-sync
>
> This is an impressive diff, wow! I started to look at it and my first
> impression is that it is too big. You should really try to split it
> in smaller pieces to get proper reviews.
>
> Anyway here are some comments about splitting/cleaning this diff, I'll
> need more time to be really able to comment on your work. I'd just
> want to say "wow" again. This is amazing!
>
> 1) Your diff includes a lot of "cleanups" which are not directly
> related to your SMP work. By cleanups I'm talking about the
> FALLTHROUGH, "#ifdef", comments, (void), etc that your added in
> various places. I'd suggest submitting a first diff including
> all these cleanups. It can be easily reviewed and committed and
> this will reduce the noise of your SMP work (and size of the diff).
>
> 2) I saw that you found some ALTQ leftovers, you have some Solaris
> compatibility goo, stack alignment tricks or when sometimes you
> need to return a variable to (de)reference it (ie pf_get_sport).
> These could also be single-shot easy-to-review diffs.
>
> 3) A lot of chunks in your diff are related to counter modifications.
> This could be a diff in itself. I'm a bit afraid by the number of
> different macro to deal with counters. Then why did you choose to
> use atomic operations rather than per-CPU counters or any other
> solution? I'm also raising this question because some counters are
> 64bit long and there's no atomic operation to modify such value on
> 32bit architectures.
>
> 4) Regarding reference counting around pool-allocated object, I'd
> subject to wrap the pool_{get,put} into their own function this
> would greatly reduce the "#ifdef _PF_SMP_/#else" dances like:
>
> +#ifdef _PF_SMP_
> + pf_rule_smp_rele(rule);
> +#else /* !_PF_SMP_ */
> pool_put(&pf_rule_pl, rule);
> +#endif /* !_PF_SMP_ */
>
>
> 5) I'm not sure to understand the goal of the new "pf_refcnt_t" type
> but using a long (why not unsigned?) makes sense with regards to
> atomic operations. Note however that your comment describing it
> is incorrect. I'd rather delete the comment. It's a good
> explanation for an email but the intend is quite obvious.
>
> 6) In pf_osfp.c rather than changing the signature of some functions
> in the _PF_SMP_ case only, I'd suggest to adapt the existing code.
> Having fewer "#ifdef _PF_SMP_/#else" makes it easier to understand,
> review and work with the code 8) This comment also applies to
> pfr_pool_get().
>
> 7) The PF_SMP_INSERT_WQ() macro to replace SLIST_INSERT() seems over-
> generic to me. Do you plan to use it with a different allocator?
> Can't we use it for the SP version of pf_table too or at least
> create a macro/function that behave differentl
Re: Machine hangs on boot with snapshots 5.8-beta
Hi, After applying this patch i was able to get the machines booting! Thank you for your help. Regards, Pedro Caetano On Wed, Jun 24, 2015 at 11:44 PM, Philip Guenther wrote: > On Wed, 24 Jun 2015, Pedro Caetano wrote: > > If case gmail wraps the lines output from dmesg and sendbug is attached > > Thanks. I believe the issue is that the ACPI "APIC" table has extra > LAPCI/processor entries which are marked as disabled...and which we let > overwrite the enabled entry for cpu0. > > Can you apply this patch, rebuild, and see if things are happier? > > > Philip Guenther > > > Index: acpimadt.c > === > RCS file: /data/src/openbsd/src/sys/dev/acpi/acpimadt.c,v > retrieving revision 1.31 > diff -u -p -r1.31 acpimadt.c > --- acpimadt.c 9 Feb 2015 08:15:19 - 1.31 > +++ acpimadt.c 24 Jun 2015 22:33:04 - > @@ -239,13 +239,13 @@ acpimadt_attach(struct device *parent, s > entry->madt_lapic.apic_id, > entry->madt_lapic.flags); > > + if ((entry->madt_lapic.flags & ACPI_PROC_ENABLE) > == 0) > + break; > + > lapic_map[entry->madt_lapic.acpi_proc_id] = > entry->madt_lapic.apic_id; > acpi_lapic_flags[entry->madt_lapic.acpi_proc_id] = > entry->madt_lapic.flags; > - > - if ((entry->madt_lapic.flags & ACPI_PROC_ENABLE) > == 0) > - break; > > memset(&caa, 0, sizeof(struct cpu_attach_args)); > if (lapic_cpu_number() == > entry->madt_lapic.apic_id) >
Re: Better de(4) fix
> Date: Fri, 26 Jun 2015 14:58:36 +0200 > From: Reyk Floeter > > On Thu, Jun 25, 2015 at 05:43:20PM -0700, Mike Larkin wrote: > > On Fri, Jun 26, 2015 at 12:09:40AM +0200, Reyk Floeter wrote: > > > On Thu, Jun 25, 2015 at 11:04:00PM +0200, Imre Vadasz wrote: > > > > On 22:27 Thu 25 Jun , Reyk Floeter wrote: > > > > > On Thu, Jun 25, 2015 at 09:21:11PM +0200, Mark Kettenis wrote: > > > > > > There really is no excuse for using dma_alloc(9) if you have the > > > > > > bus_dmatag_t available. > > > > > > > > > > > > This re-uses tulip_busdma_allocmem(), which simplifies the code for > > > > > > allocating the dmamap and such. > > > > > > > > > > > > Unfortunately I can't test this myself right now. > > > > > > > > > > > > > > > > Fixes the panic on Hyper-V, see dmesg below. > > > > > > > > > > Unrelated to that, no interrupts on the nic with the ioapic enabled > > > > > (no traffic and autoneg timeouts). > > > > > > > > > > Reyk > > > > > > > > On DragonFly BSD and NetBSD I was able to get the NIC interrupts working > > > > with enabled ioapic, by forcing the NIC's IRQ to be configured as > > > > edge/high triggered instead of level/low in the ioapic. > > > > > > I had to try this: with a horrible hack(tm) in pci_intr_establish() I > > > turned de0 into edge triggered and everything works. > > > > > > Reyk > > > > I suspect something is amiss in our interpretation of the interrupt config > > on amd64 since de(4) works unmodified on i386 with Hyper-V. > > > > Maybe some AML parse screwup? > > > > I can confirm this, I installed an i386 VM on the same Hyper-V machine > and de works just fine without the hack. amd64's de does not. That is curious. There are quite a few differences between the amd64 interrupt subsystem and the i386 interrupt subsystem, so there are various possibilities on how things can go wrong. Are the Hyper-V ACPI tables available somewhere?
Re: Better de(4) fix
On Thu, Jun 25, 2015 at 05:43:20PM -0700, Mike Larkin wrote: > On Fri, Jun 26, 2015 at 12:09:40AM +0200, Reyk Floeter wrote: > > On Thu, Jun 25, 2015 at 11:04:00PM +0200, Imre Vadasz wrote: > > > On 22:27 Thu 25 Jun , Reyk Floeter wrote: > > > > On Thu, Jun 25, 2015 at 09:21:11PM +0200, Mark Kettenis wrote: > > > > > There really is no excuse for using dma_alloc(9) if you have the > > > > > bus_dmatag_t available. > > > > > > > > > > This re-uses tulip_busdma_allocmem(), which simplifies the code for > > > > > allocating the dmamap and such. > > > > > > > > > > Unfortunately I can't test this myself right now. > > > > > > > > > > > > > Fixes the panic on Hyper-V, see dmesg below. > > > > > > > > Unrelated to that, no interrupts on the nic with the ioapic enabled > > > > (no traffic and autoneg timeouts). > > > > > > > > Reyk > > > > > > On DragonFly BSD and NetBSD I was able to get the NIC interrupts working > > > with enabled ioapic, by forcing the NIC's IRQ to be configured as > > > edge/high triggered instead of level/low in the ioapic. > > > > I had to try this: with a horrible hack(tm) in pci_intr_establish() I > > turned de0 into edge triggered and everything works. > > > > Reyk > > I suspect something is amiss in our interpretation of the interrupt config > on amd64 since de(4) works unmodified on i386 with Hyper-V. > > Maybe some AML parse screwup? > I can confirm this, I installed an i386 VM on the same Hyper-V machine and de works just fine without the hack. amd64's de does not. Reyk > -ml > > > > > OpenBSD 5.8-beta (GENERIC.MP) #17: Thu Jun 25 23:54:49 CEST 2015 > > root@openbsd.hyperv:/usr/src/sys/arch/amd64/compile/GENERIC.MP > > real mem = 8573091840 (8175MB) > > avail mem = 8309399552 (7924MB) > > mpath0 at root > > scsibus0 at mpath0: 256 targets > > mainbus0 at root > > bios0 at mainbus0: SMBIOS rev. 2.3 @ 0xf8ec0 (216 entries) > > bios0: vendor American Megatrends Inc. version "090006" date 05/23/2012 > > bios0: Microsoft Corporation Virtual Machine > > acpi0 at bios0: rev 0 > > acpi0: sleep states S0 S5 > > acpi0: tables DSDT FACP WAET SLIC OEM0 SRAT APIC OEMB > > acpi0: wakeup devices > > acpitimer0 at acpi0: 3579545 Hz, 32 bits > > acpimadt0 at acpi0 addr 0xfee0: PC-AT compat > > cpu0 at mainbus0: apid 0 (boot processor) > > cpu0: Intel(R) Core(TM) i5-4590 CPU @ 3.30GHz, 1647.71 MHz > > cpu0: > > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,LONG,LAHF,ABM,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,RTM > > cpu0: 256KB 64b/line 8-way L2 cache > > cpu0: smt 0, core 0, package 0 > > mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges > > cpu0: apic clock running at 120MHz > > cpu1 at mainbus0: apid 1 (application processor) > > cpu1: Intel(R) Core(TM) i5-4590 CPU @ 3.30GHz, 1975.61 MHz > > cpu1: > > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,LONG,LAHF,ABM,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,RTM > > cpu1: 256KB 64b/line 8-way L2 cache > > cpu1: smt 0, core 0, package 1 > > ioapic0 at mainbus0: apid 0 pa 0xfec0, version 11, 24 pins > > ioapic0: misconfigured as apic 2, remapped to apid 0 > > acpiprt0 at acpi0: bus 0 (PCI0) > > acpicpu0 at acpi0 > > acpicpu1 at acpi0 > > allocated pic ioapic0 type level pin 9 level 9 to cpu0 slot 9 idt entry 144 > > pci0 at mainbus0 bus 0 > > pchb0 at pci0 dev 0 function 0 "Intel 82443BX" rev 0x03 > > pcib0 at pci0 dev 7 function 0 "Intel 82371AB PIIX4 ISA" rev 0x01 > > pciide0 at pci0 dev 7 function 1 "Intel 82371AB IDE" rev 0x01: DMA, channel > > 0 wired to compatibility, channel 1 wired to compatibility > > allocated pic ioapic0 type edge pin 14 level 6 to cpu0 slot 14 idt entry 96 > > wd0 at pciide0 channel 0 drive 0: > > wd0: 128-sector PIO, LBA48, 10240MB, 20971520 sectors > > wd0(pciide0:0:0): using PIO mode 4, DMA mode 2 > > allocated pic ioapic0 type edge pin 15 level 6 to cpu0 slot 15 idt entry 97 > > atapiscsi0 at pciide0 channel 1 drive 0 > > scsibus1 at atapiscsi0: 2 targets > > cd0 at scsibus1 targ 0 lun 0: ATAPI 5/cdrom > > removable > > cd0(pciide0:1:0): using PIO mode 4, DMA mode 2 > > piixpm0 at pci0 dev 7 function 3 "Intel 82371AB Power" rev 0x02: SMBus > > disabled > > vga1 at pci0 dev 8 function 0 "Microsoft VGA" rev 0x00 > > wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) > > wsdisplay0: screen 1-5 added (80x25, vt100 emulation) > > de0 at pci0 dev 10 function 0 "DEC 21140" rev 0x20allocated pic ioapic0 > > type edge pin 11 level 7 to cpu0 slot 11 idt entry 112 > > , 21140A pass 2.0: apic 0 int 11, normal, address 00:15:5d:01:9b:03 > > isa0 at pcib0 > > isadma0 at isa0 > > fdc0 at isa0 port 0x3f0/6 irq 6 drq 2 > > allocated pic ioapic0 type edge pin 6 level 6 to cpu0 slot 6 idt entry 98 > > fd0 at f
Re: SMP steroids for PF
Hello Sasha,
Alexandr Nedvedicky [alexandr.nedvedi...@oracle.com] wrote:
> Hello,
>
> attached is SMP patch for PF. consider it as toxic proof of concept as it has
> paniced my amd64 system (see attached phone-shot). I have to figure out how to
> debug it yet. The problem is the USB keyboard has died, so I had no chance to
> type anything. fortunately the issue is 100% reproducible.
>
> The patch compiles in .MP and non-MP version. As you'll see more work is
> needed to stabilize it and get full SMP support of PF. Those PF
> features are not covered by SMP changes:
> - packet queues
> - packet logging
> - pf-sync
This is an impressive diff, wow! I started to look at it and my first
impression is that it is too big. You should really try to split it
in smaller pieces to get proper reviews.
Anyway here are some comments about splitting/cleaning this diff, I'll
need more time to be really able to comment on your work. I'd just
want to say "wow" again. This is amazing!
1) Your diff includes a lot of "cleanups" which are not directly
related to your SMP work. By cleanups I'm talking about the
FALLTHROUGH, "#ifdef", comments, (void), etc that your added in
various places. I'd suggest submitting a first diff including
all these cleanups. It can be easily reviewed and committed and
this will reduce the noise of your SMP work (and size of the diff).
2) I saw that you found some ALTQ leftovers, you have some Solaris
compatibility goo, stack alignment tricks or when sometimes you
need to return a variable to (de)reference it (ie pf_get_sport).
These could also be single-shot easy-to-review diffs.
3) A lot of chunks in your diff are related to counter modifications.
This could be a diff in itself. I'm a bit afraid by the number of
different macro to deal with counters. Then why did you choose to
use atomic operations rather than per-CPU counters or any other
solution? I'm also raising this question because some counters are
64bit long and there's no atomic operation to modify such value on
32bit architectures.
4) Regarding reference counting around pool-allocated object, I'd
subject to wrap the pool_{get,put} into their own function this
would greatly reduce the "#ifdef _PF_SMP_/#else" dances like:
+#ifdef _PF_SMP_
+ pf_rule_smp_rele(rule);
+#else /* !_PF_SMP_ */
pool_put(&pf_rule_pl, rule);
+#endif /* !_PF_SMP_ */
5) I'm not sure to understand the goal of the new "pf_refcnt_t" type
but using a long (why not unsigned?) makes sense with regards to
atomic operations. Note however that your comment describing it
is incorrect. I'd rather delete the comment. It's a good
explanation for an email but the intend is quite obvious.
6) In pf_osfp.c rather than changing the signature of some functions
in the _PF_SMP_ case only, I'd suggest to adapt the existing code.
Having fewer "#ifdef _PF_SMP_/#else" makes it easier to understand,
review and work with the code 8) This comment also applies to
pfr_pool_get().
7) The PF_SMP_INSERT_WQ() macro to replace SLIST_INSERT() seems over-
generic to me. Do you plan to use it with a different allocator?
Can't we use it for the SP version of pf_table too or at least
create a macro/function that behave differently for the SMP version
to reduce the "#ifdef" dances...
8) Your protection of the pfi_ifhead RB-tree principally correspond to
the existing splsoftnet(), don't you think it would make sense to
use a single macro for the IPL and on SMP rwlock?
Cheers,
Martin
Re: Pure L2TP client upload performance
2015-06-26 9:20 GMT+03:00 Claudio Jeker : > On Fri, Jun 26, 2015 at 04:59:32AM +0300, Sergey Ryazanov wrote: >> Hello, >> >> during building l2tp tunnel with xl2tpd-1.3.1 I was faced with its too >> low upload performance. When download, the speed is 20 mbit/s at >> nearly 100% CPU utilization. CPU is Pentium D 930 3 GHz. When upload, >> the speed is below 2 mbit/s at nearly zero CPU utilization. >> >> First, I examined xl2tpd code and did not find any potential issues. >> Then I compiled it with -pg option and do a quick test with help of >> iperf(1): 4 TCP flows, direction is toward the L2TP server, 2 min >> test. Then I run gprof and got pretty strange output: >> >> Full output: http://pastebin.com/dm4wz6VK >> granularity: each sample hit covers 4 byte(s) for 11.11% of 0.09 seconds >> % cumulative self self total >> time seconds secondscalls ms/call ms/call name >> 77.8 0.07 0.0724311 0.00 0.00 write [4] >> 22.2 0.09 0.0235897 0.00 0.00 _thread_sys_sendmsg [7] >> 0.0 0.09 0.00 109136 0.00 0.00 memset [46] >> 0.0 0.09 0.00 107997 0.00 0.00 ___errno [926] >> 0.0 0.09 0.0071740 0.00 0.00 read_packet [47] >> 0.0 0.09 0.0071726 0.00 0.00 _thread_sys_read [927] >> 0.0 0.09 0.0060154 0.00 0.00 >> _thread_sys_gettimeofday [928] >> 0.0 0.09 0.0060114 0.00 0.00 process_signal [32] >> 0.0 0.09 0.0060113 0.00 0.00 build_fdset [29] >> 0.0 0.09 0.0060113 0.00 0.00 process_schedule [26] >> 0.0 0.09 0.0060113 0.00 0.00 select [48] >> 0.0 0.09 0.0035897 0.00 0.00 udp_xmit [6] >> >> Another one run also gives something strange: >> >> Full output: http://pastebin.com/vXvHPuyj >> granularity: each sample hit covers 4 byte(s) for 14.29% of 0.07 seconds >> % cumulative self self total >> time seconds secondscalls ms/call ms/call name >> 71.4 0.05 0.0524348 0.00 0.00 write [4] >> 14.3 0.06 0.0160265 0.00 0.00 select [6] >> 14.3 0.07 0.01 10 1.00 1.00 calloc [7] >> 0.0 0.07 0.00 109324 0.00 0.00 memset [55] >> 0.0 0.07 0.00 108285 0.00 0.00 ___errno [925] >> 0.0 0.07 0.0071997 0.00 0.00 read_packet [56] >> 0.0 0.07 0.0071980 0.00 0.00 _thread_sys_read [926] >> 0.0 0.07 0.0060282 0.00 0.00 >> _thread_sys_gettimeofday [927] >> 0.0 0.07 0.0060266 0.00 0.00 process_signal [50] >> 0.0 0.07 0.0060265 0.00 0.00 build_fdset [49] >> 0.0 0.07 0.0060265 0.00 0.00 process_schedule [57] >> 0.0 0.07 0.0036020 0.00 0.00 _thread_sys_sendmsg >> [928] >> 0.0 0.07 0.0036020 0.00 0.00 udp_xmit [58] >> >> For comparison I changed direction and did a download test: >> >> Full output: http://pastebin.com/FFF6xGcy >> granularity: each sample hit covers 4 byte(s) for 0.01% of 114.30 seconds >> % cumulative self self total >> time seconds secondscalls ms/call ms/call name >> 93.7 107.06 107.06 370804 0.29 0.29 write [5] >> 1.8 109.08 2.02 370770 0.01 0.01 recvmsg [6] >> 1.4 110.67 1.59 370799 0.00 0.00 >> _thread_sys_gettimeofday [8] >> 1.1 111.90 1.23 370770 0.00 0.30 handle_packet [4] >> 1.0 113.01 1.11 370761 0.00 0.00 add_fcs [9] >> 0.7 113.81 0.81 370781 0.00 0.00 select [10] >> 0.2 114.03 0.211 210.00 114172.55 network_thread [3] >> 0.1 114.14 0.12 mcount [11] >> 0.0 114.19 0.05 370781 0.00 0.00 build_fdset [12] >> 0.0 114.23 0.04 370771 0.00 0.00 get_call [13] >> 0.0 114.27 0.03 1112701 0.00 0.00 memset [14] >> 0.0 114.28 0.02 370781 0.00 0.00 process_schedule [7] >> 0.0 114.30 0.01 29 0.34 0.34 _thread_sys_sendmsg >> [20] >> 0.0 114.30 0.018 0.62 0.74 inet_ntoa [24] >> 0.0 114.30 0.00 370785 0.00 0.00 swaps [110] >> 0.0 114.30 0.00 370782 0.00 0.00 process_signal [41] >> 0.0 114.30 0.00 370770 0.00 0.00 recycle_buf [111] >> >> During upload tests, everything looks like if xl2tpd doesn't perform >> any work and stucks somewhere in I/O operation. >> >> May be there are some options, what could be tuned to speedup ppp(4) >> I/O performance or did I missed something during my tests? I am in >> doubts. Any clues? >> > > Can you get a ktrac
Re: patterns: unused header and doubled % sign
On Fri, Jun 26, 2015 at 11:25:17AM +0200, Theo Buehler wrote: > Three minor nits following up on differences between the versions in > reyk@'s mails and the initial commit: > > - isn't used anymore, so it can be removed. > > - Sort the remaining headers alphabetically according to style(9). > > - There's one remaining '%%' in an error string that shouldn't > be doubled, I think. > Committed. We also add sys/types.h in include list for off_t, as it should be implicitely included from somewhere else. And I updated the regress testsuite for the changed error message. Thanks ! -- Sebastien Marie
patterns: unused header and doubled % sign
Three minor nits following up on differences between the versions in reyk@'s mails and the initial commit: - isn't used anymore, so it can be removed. - Sort the remaining headers alphabetically according to style(9). - There's one remaining '%%' in an error string that shouldn't be doubled, I think. Index: usr.sbin/httpd/patterns.c === RCS file: /cvs/src/usr.sbin/httpd/patterns.c,v retrieving revision 1.2 diff -u -p -r1.2 patterns.c --- usr.sbin/httpd/patterns.c 23 Jun 2015 15:35:20 - 1.2 +++ usr.sbin/httpd/patterns.c 26 Jun 2015 09:18:16 - @@ -30,12 +30,11 @@ * Standard library for string operations and pattern-matching */ +#include +#include #include #include #include -#include -#include -#include #include "patterns.h" @@ -98,7 +97,7 @@ classend(struct match_state *ms, const c case L_ESC: if (p == ms->p_end) match_error(ms, - "malformed pattern (ends with '%%')"); + "malformed pattern (ends with '%')"); return p + 1; case '[': if (*p == '^')
Re: upd(4) man page update
Hi, David Higgs wrote on Thu, Jun 25, 2015 at 11:08:52PM -0400: > If there's no further work on upd(4) prior to 5.8, at least make > the man page reflect present reality. > > - Update list of supported sensors, re-sorted by source file occurrence > - Explain why manual sensorsd.conf(5) intervention can be necessary > - Link to HID power specs > - Prefer ???a UPS??? over ???an UPS???, matches HID docs above > - Other misc linguistic bikeshedding > > Apologies, this is my first experience with mdoc(7). I have no idea about the content, but if anybody wants to commit this, it's OK mdoc(7)wise with the two exceptions noted inline. Yours, Ingo > Index: upd.4 > === > RCS file: /cvs/src/share/man/man4/upd.4,v > retrieving revision 1.3 > diff -u -p -r1.3 upd.4 > --- upd.4 19 Mar 2014 09:22:25 - 1.3 > +++ upd.4 26 Jun 2015 02:46:15 - > @@ -25,17 +25,23 @@ > .Sh DESCRIPTION > The > .Nm > -driver exposes data from USB Power Devices (such as an UPS), > -as hardware sensors via > -.Xr sysctl 3 . > +driver provides support for monitoring various sensors provided by > +USB Power Devices (such as a UPS). > +Supported sensor values are made available via the > +.Xr sysctl 8 interface. The .Xr macro cannot take additional arguments, so the previous line must be replaced by +.Xr sysctl 8 +interface. > .Pp > -The following sensors are provided by the > -.Nm > -driver, which can be monitored using > -.Xr sensorsd 8 : > +The following sensors are supported by the driver: > .Pp > .Bl -item -offset indent -compact > .It > +BatteryPresent > +.It > +ShutdownImminent > +.It > +ACPresent > +.It > +Overload > +.It > RelativeStateOfCharge > .It > AbsoluteStateOfCharge > @@ -48,19 +54,23 @@ Charging > .It > Discharging > .It > -BatteryPresent > +AtRateTimeToFull > .It > -ShutdownImminent > +AtRateTimeToEmpty > .It > -ACPresent > +RunTimeToEmpty > .It > -AtRateTimeToFull > +NeedReplacement > .El > .Sh SEE ALSO > .Xr intro 4 , > .Xr uhidev 4 , > .Xr sensorsd 8 , > +.Xr sensorsd.conf 5 , > .Xr sysctl 8 Wrong order, this should be: .Xr uhidev 4 , +.Xr sensorsd.conf 5 , .Xr sensorsd 8 , .Xr sysctl 8 > +.Pp > +The USB Power Devices specification can be found at > +.Lk http://www.usb.org/developers/hidpage/ > .Sh HISTORY > The > .Nm > @@ -71,3 +81,18 @@ The > .Nm > driver was written by > .An Andre de Oliveira . > +.Sh BUGS > +The > +.Nm > +driver does not indicate device health via aggregate or > +individual sensor status. > +Users who wish to monitor > +.Nm > +status using > +.Xr sensorsd 8 > +must manually establish > +.Dq high > +and > +.Dq low > +limits for sensor values of interest via > +.Xr sensorsd.conf 5 .
Re: SMP steroids for PF
Sorry, fingers were faster than head (again...) regards sasha On Thu, Jun 25, 2015 at 01:39:29PM -0700, Chris Cappuccio wrote: > You should re-post as a diff -u !! > > Alexandr Nedvedicky [alexandr.nedvedi...@oracle.com] wrote: > > Hello, > > > > attached is SMP patch for PF. consider it as toxic proof of concept as it > > has > > paniced my amd64 system (see attached phone-shot). I have to figure out how > > to > > debug it yet. The problem is the USB keyboard has died, so I had no chance > > to > > type anything. fortunately the issue is 100% reproducible. > > > > The patch compiles in .MP and non-MP version. As you'll see more work is > > needed to stabilize it and get full SMP support of PF. Those PF > > features are not covered by SMP changes: > > - packet queues > > - packet logging > > - pf-sync > > > > patch is attached. > > > > regards > > sasha > > > > -- > The bums will always lose pf_smp.amd64.2015-06-26.diff.gz Description: application/gunzip
