Re: Better/more upd(4) timedelta sensors
Hello David, On 18/12/14(Thu) 00:45, David Higgs wrote: While my device does not seem to provide AtRateTimeToFull or AtRateTimeToEmpty, it does have RunTimeToEmpty. Then I found that SENSOR_TIMEDELTA values are in nanoseconds and that scaling for them was never implemented correctly. I am confused by the spec [1], though; see 4.2.5 - Battery Measures. The reported values are supposedly in minutes but hid_info.unit is 0x1001 (seconds) and observation of RunTimeToEmpty appears to agree. I don’t see any other drivers in the tree that pay attention to unit or unit_exponent fields, and don’t want to go down a rabbit hole if there’s no interest. As usual, feedback is welcome. Table 1 of section 3.2.3 indicates that the physical unit for time reports are in seconds. So I guess it is a mistake in the document. I like your diff but I'd prefer to see the scaling done based on the unit rather than the name. Why not adding new defines for the various Physical Unit defined in the table and do the switch on hitem.unit? And what about the exponent? [1] http://www.usb.org/developers/hidpage/pdcv10.pdf --david Index: upd.c === RCS file: /cvs/src/sys/dev/usb/upd.c,v retrieving revision 1.12 diff -u -p -r1.12 upd.c --- upd.c 11 Dec 2014 18:50:32 - 1.12 +++ upd.c 18 Dec 2014 05:02:30 - @@ -66,7 +66,11 @@ static struct upd_usage_entry upd_usage_ { HUP_BATTERY, HUB_AC_PRESENT, SENSOR_INDICATOR,ACPresent }, { HUP_BATTERY, HUB_ATRATE_TIMETOFULL, - SENSOR_TIMEDELTA,AtRateTimeToFull } + SENSOR_TIMEDELTA,AtRateTimeToFull }, + { HUP_BATTERY, HUB_ATRATE_TIMETOEMPTY, + SENSOR_TIMEDELTA,AtRateTimeToEmpty }, + { HUP_BATTERY, HUB_RUNTIMETO_EMPTY, + SENSOR_TIMEDELTA,RunTimeToEmpty }, }; struct upd_report { @@ -322,9 +326,9 @@ upd_update_sensors(struct upd_softc *sc, int repid) { struct upd_sensor *sensor; - ulong hdata, batpres; - ulong adjust; - int i; + int64_t adjust; + ulong batpres; + int hdata, i; sensor = upd_lookup_sensor(sc, HUP_BATTERY, HUB_BATTERY_PRESENT); batpres = sensor ? sensor-ksensor.value : -1; @@ -353,6 +357,11 @@ upd_update_sensors(struct upd_softc *sc, case HUB_FULLCHARGE_CAPACITY: adjust = 1000; /* scale adjust */ break; + case HUB_ATRATE_TIMETOFULL: + case HUB_ATRATE_TIMETOEMPTY: + case HUB_RUNTIMETO_EMPTY: + adjust = 10LL; /* XXX not minutes? */ + break; default: adjust = 1; /* no scale adjust */ break; @@ -363,7 +372,7 @@ upd_update_sensors(struct upd_softc *sc, sensor-ksensor.value = hdata * adjust; sensor-ksensor.status = SENSOR_S_OK; sensor-ksensor.flags = ~SENSOR_FINVALID; - DPRINTF((%s: hidget data: %lu\n, + DPRINTF((%s: hidget data: %d\n, sc-sc_sensordev.xname, hdata)); } }
Re: Better/more upd(4) timedelta sensors
On Fri, Dec 19, 2014 at 7:17 AM, Martin Pieuchot mpieuc...@nolizard.org wrote: Hello David, On 18/12/14(Thu) 00:45, David Higgs wrote: While my device does not seem to provide AtRateTimeToFull or AtRateTimeToEmpty, it does have RunTimeToEmpty. Then I found that SENSOR_TIMEDELTA values are in nanoseconds and that scaling for them was never implemented correctly. I am confused by the spec [1], though; see 4.2.5 - Battery Measures. The reported values are supposedly in minutes but hid_info.unit is 0x1001 (seconds) and observation of RunTimeToEmpty appears to agree. I don't see any other drivers in the tree that pay attention to unit or unit_exponent fields, and don't want to go down a rabbit hole if there's no interest. As usual, feedback is welcome. Table 1 of section 3.2.3 indicates that the physical unit for time reports are in seconds. So I guess it is a mistake in the document. I like your diff but I'd prefer to see the scaling done based on the unit rather than the name. Why not adding new defines for the various Physical Unit defined in the table and do the switch on hitem.unit? And what about the exponent? Thanks for looking at this. I've rethought my approach to upd(4) a bit, and will be addressing bugs prior to adding new sensors. There will be a diff to address units in a more coherent fashion soon. --david
pstat segfault
$ pstat -T 221/7030 open files Segmentation fault
Fix upd(4) sensor refresh
Split upd_update_sensors() behavior to gather all values prior to updating sensor contents. Because sensors were updated in report_ID order, it could take multiple passes of upd_refresh() to update some sensors. Specifically, battery-related sensors “prior” to a change in BatteryPresent would be stale/incorrect until the 2nd call to upd_refresh(). Also, change a confusing DPRINTF of report_ID from un-prefixed hex to decimal, to be consistent. This fix is needed for future improvements, where output or behavior of certain sensors depends on the current value of more than one report (e.g. RemainingCapacity should actually depend on CapacityMode). As always, feedback is welcome. --david Index: upd.c === RCS file: /cvs/src/sys/dev/usb/upd.c,v retrieving revision 1.12 diff -u -p -r1.12 upd.c --- upd.c 11 Dec 2014 18:50:32 - 1.12 +++ upd.c 19 Dec 2014 05:33:14 - @@ -77,6 +77,8 @@ struct upd_report { struct upd_sensor { struct ksensor ksensor; struct hid_item hitem; + int raw_value; + int value_read; int attached; }; @@ -98,7 +100,8 @@ void upd_attach(struct device *, struct int upd_detach(struct device *, int); void upd_refresh(void *); -void upd_update_sensors(struct upd_softc *, uint8_t *, unsigned int, int); +void upd_update_values(struct upd_softc *, uint8_t *, unsigned int, int); +void upd_update_sensors(struct upd_softc *); void upd_intr(struct uhidev *, void *, uint); struct upd_usage_entry *upd_lookup_usage_entry(const struct hid_item *); struct upd_sensor *upd_lookup_sensor(struct upd_softc *, int, int); @@ -275,7 +278,7 @@ upd_refresh(void *arg) UHID_FEATURE_REPORT, repid, buf, report-size); if (actlen == -1) { - DPRINTF((upd: failed to get report id=%02x\n, repid)); + DPRINTF((upd: failed to get report id=%d\n, repid)); continue; } @@ -283,8 +286,10 @@ upd_refresh(void *arg) if (actlen report-size) report-size = actlen; - upd_update_sensors(sc, buf, report-size, repid); + upd_update_values(sc, buf, report-size, repid); } + + upd_update_sensors(sc); } struct upd_usage_entry * @@ -318,33 +323,48 @@ upd_lookup_sensor(struct upd_softc *sc, } void -upd_update_sensors(struct upd_softc *sc, uint8_t *buf, unsigned int len, +upd_update_values(struct upd_softc *sc, uint8_t *buf, unsigned int len, int repid) { struct upd_sensor *sensor; - ulong hdata, batpres; - ulong adjust; int i; + for (i = 0; i sc-sc_num_sensors; i++) { + sensor = sc-sc_sensors[i]; + if (sensor-hitem.report_ID == repid sensor-attached) { + sensor-value_read = 1; + sensor-raw_value = + hid_get_data(buf, len, sensor-hitem.loc); + DPRINTF((%s: repid=%d, %s value=%d\n, + sc-sc_sensordev.xname, repid, + sensor-ksensor.desc, sensor-raw_value)); + } + } +} + +void +upd_update_sensors(struct upd_softc *sc) +{ + struct upd_sensor *sensor; + ulong adjust; + int i, batpres = 0; + sensor = upd_lookup_sensor(sc, HUP_BATTERY, HUB_BATTERY_PRESENT); - batpres = sensor ? sensor-ksensor.value : -1; + if (sensor sensor-value_read) + batpres = sensor-raw_value; for (i = 0; i sc-sc_num_sensors; i++) { sensor = sc-sc_sensors[i]; - if (!(sensor-hitem.report_ID == repid sensor-attached)) + sensor-ksensor.flags |= SENSOR_FINVALID; + sensor-ksensor.status = SENSOR_S_UNKNOWN; + if (!sensor-value_read) continue; - /* invalidate battery dependent sensors */ + /* ignore battery sensors when no battery present */ if (HID_GET_USAGE_PAGE(sensor-hitem.usage) == HUP_BATTERY - batpres = 0) { - /* exception to the battery sensor itself */ - if (HID_GET_USAGE(sensor-hitem.usage) != - HUB_BATTERY_PRESENT) { - sensor-ksensor.status = SENSOR_S_UNKNOWN; - sensor-ksensor.flags |= SENSOR_FINVALID; - continue; - } - } + HID_GET_USAGE(sensor-hitem.usage) != HUB_BATTERY_PRESENT + !batpres) + continue; switch
Re: Dell R630 high interrupts on acpi0
On Thu, Dec 18, 2014 at 11:14:54PM +0100, Frederic Nowak wrote: Hi! The diff for extracting memory ranges from ACPI was obviously not tested with ACPI disabled...so we definitely have to check if the values in pcimem_range make some sense. The diff below now uses the old values in case ACPI is disabled or does not return valid values. I hope this is somewhat interesting for someone else as well :) Please let me know if this is just noise or if there is anything else I am missing. This looks like the right direction, except I think we only really want to use the ACPI range information to add new ranges to the existing one that covers the 36-bit address space, rather than relying on it for everything. No point relying on the ACPI stuff being correct if we don't need to. I hacked this around a bit today and ended up with the diff below. This only deals with ranges outside the 36-bit space, so it makes no difference on most machines, and on the r630 that needs it, it adds two ranges, 0x0300 to 0x033F and 0x0340 to 0x037F. Index: arch/amd64/pci/pci_machdep.c === RCS file: /cvs/src/sys/arch/amd64/pci/pci_machdep.c,v retrieving revision 1.60 diff -u -p -r1.60 pci_machdep.c --- arch/amd64/pci/pci_machdep.c16 Dec 2014 23:13:20 - 1.60 +++ arch/amd64/pci/pci_machdep.c19 Dec 2014 12:54:44 - @@ -90,12 +90,17 @@ #include dev/pci/ppbreg.h #include ioapic.h +#include acpi.h #if NIOAPIC 0 #include machine/i82093var.h #include machine/mpbiosvar.h #endif +#if NACPI 0 +#include dev/acpi/acpivar.h +#endif + /* * Memory Mapped Configuration space access. * @@ -622,17 +627,13 @@ pci_init_extents(void) * here. As long as vendors continue to support * 32-bit operating systems, we should never see BARs * outside that region. -* -* Dell 13G servers have important devices outside the -* 36-bit address space. Until we can extract the address -* ranges from ACPI, expand the allowed range to suit. */ pcimem_ex = extent_create(pcimem, 0, 0xUL, M_DEVBUF, NULL, 0, EX_NOWAIT); if (pcimem_ex == NULL) return; - extent_alloc_region(pcimem_ex, 0x400UL, - 0xfc00UL, EX_NOWAIT); + extent_alloc_region(pcimem_ex, 0x10UL, + 0xfff0UL, EX_NOWAIT); for (bmp = bios_memmap; bmp-type != BIOS_MAP_END; bmp++) { /* @@ -657,6 +658,14 @@ pci_init_extents(void) /* Take out the video buffer area and BIOS areas. */ extent_alloc_region(pcimem_ex, IOM_BEGIN, IOM_SIZE, EX_CONFLICTOK | EX_NOWAIT); + +#if NACPI 0 + /* +* Free up any regions outside the 36-bit address space +* specified via ACPI. +*/ + acpi_pciroots_extents(pcimem_ex, 0x10UL); +#endif } if (pcibus_ex == NULL) { @@ -665,7 +674,6 @@ pci_init_extents(void) } } -#include acpi.h #if NACPI 0 void acpi_pci_match(struct device *, struct pci_attach_args *); pcireg_t acpi_pci_min_powerstate(pci_chipset_tag_t, pcitag_t); Index: dev/acpi/acpivar.h === RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v retrieving revision 1.79 diff -u -p -r1.79 acpivar.h --- dev/acpi/acpivar.h 8 Dec 2014 07:12:37 - 1.79 +++ dev/acpi/acpivar.h 19 Dec 2014 12:54:44 - @@ -47,6 +47,7 @@ extern u_int8_t acpi_lapic_flags[LAPIC_M struct klist; struct acpiec_softc; struct acpipwrres_softc; +struct extent; struct acpivideo_softc { struct device sc_dev; @@ -357,6 +358,7 @@ int acpi_acquire_glk(uint32_t *); intacpi_release_glk(uint32_t *); void acpi_pciroots_attach(struct device *, void *, cfprint_t); +void acpi_pciroots_extents(struct extent *, u_int64_t); #endif Index: dev/acpi/amltypes.h === RCS file: /cvs/src/sys/dev/acpi/amltypes.h,v retrieving revision 1.40 diff -u -p -r1.40 amltypes.h --- dev/acpi/amltypes.h 7 Sep 2012 19:19:59 - 1.40 +++ dev/acpi/amltypes.h 19 Dec 2014 12:54:44 - @@ -346,6 +346,12 @@ struct aml_value { #define aml_pkglen(v) ((v)-length) #define aml_pkgval(v,i)((v)-v_package[(i)]) +struct acpi_pci_range { + SLIST_ENTRY(acpi_pci_range) next; + uint64_tbase; + uint64_tlen; +}; + struct acpi_pci { TAILQ_ENTRY(acpi_pci) next; @@ -362,6 +368,8 @@ struct acpi_pci { int _s3w; int
relayd: source-hash and random for redirections
Hi, the following diff adds support for source-hash and random modes to relayd's redirections. It depends on the latest pf change. Example: ---snip--- redirect foo { listen on 0.0.0.0 port 8080 forward to foo check tcp port 80 mode source-hash # forward to foo check tcp port 80 mode source-hash my-fixed-key } ---snap--- Reyk Index: usr.sbin/relayd/parse.y === RCS file: /cvs/src/usr.sbin/relayd/parse.y,v retrieving revision 1.197 diff -u -p -u -p -r1.197 parse.y --- usr.sbin/relayd/parse.y 18 Dec 2014 20:55:01 - 1.197 +++ usr.sbin/relayd/parse.y 19 Dec 2014 13:45:45 - @@ -493,6 +493,9 @@ rdropts_l : rdropts_l rdroptsl nl rdroptsl : forwardmode TO tablespec interface{ if (hashkey != NULL) { + memcpy(rdr-conf.key, + hashkey, sizeof(rdr-conf.key)); + rdr-conf.flags |= F_HASHKEY; free(hashkey); hashkey = NULL; } @@ -776,15 +779,15 @@ tableopts : CHECK tablecheck switch ($2) { case RELAY_DSTMODE_LOADBALANCE: case RELAY_DSTMODE_HASH: - case RELAY_DSTMODE_RANDOM: - case RELAY_DSTMODE_SRCHASH: if (rdr != NULL) { yyerror(mode not supported for redirections); YYERROR; } /* FALLTHROUGH */ + case RELAY_DSTMODE_RANDOM: case RELAY_DSTMODE_ROUNDROBIN: + case RELAY_DSTMODE_SRCHASH: dstmode = $2; break; case RELAY_DSTMODE_LEASTSTATES: Index: usr.sbin/relayd/pfe_filter.c === RCS file: /cvs/src/usr.sbin/relayd/pfe_filter.c,v retrieving revision 1.53 diff -u -p -u -p -r1.53 pfe_filter.c --- usr.sbin/relayd/pfe_filter.c27 Apr 2013 16:39:30 - 1.53 +++ usr.sbin/relayd/pfe_filter.c19 Dec 2014 13:45:45 - @@ -485,9 +485,15 @@ sync_ruleset(struct relayd *env, struct } switch (rdr-conf.mode) { + case RELAY_DSTMODE_RANDOM: + rio.rule.rdr.opts = PF_POOL_RANDOM; + break; case RELAY_DSTMODE_ROUNDROBIN: rio.rule.rdr.opts = PF_POOL_ROUNDROBIN; break; + case RELAY_DSTMODE_SRCHASH: + rio.rule.rdr.opts = PF_POOL_SRCHASH; + break; case RELAY_DSTMODE_LEASTSTATES: rio.rule.rdr.opts = PF_POOL_LEASTSTATES; break; @@ -497,6 +503,9 @@ sync_ruleset(struct relayd *env, struct } if (rdr-conf.flags F_STICKY) rio.rule.rdr.opts |= PF_POOL_STICKYADDR; + if (rdr-conf.flags F_HASHKEY) + memcpy(rio.rule.rdr.key.key32, rdr-conf.key.data, + sizeof(rio.rule.rdr.key.key32)); if (rio.rule.rt == PF_ROUTETO) { memcpy(rio.rule.route, rio.rule.rdr, Index: usr.sbin/relayd/relayd.conf.5 === RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v retrieving revision 1.155 diff -u -p -u -p -r1.155 relayd.conf.5 --- usr.sbin/relayd/relayd.conf.5 18 Dec 2014 21:26:09 - 1.155 +++ usr.sbin/relayd/relayd.conf.5 19 Dec 2014 13:45:45 - @@ -413,7 +413,7 @@ the source IP address of the client, and This mode is only supported by relays. .It Ic mode random Distributes the outgoing connections randomly through all active hosts. -This mode is only supported by relays. +This mode is supported by redirections and relays. .It Ic mode roundrobin Distributes the outgoing connections using a round-robin scheduler through all active hosts. @@ -423,7 +423,7 @@ This mode is supported by redirections a Balances the outgoing connections across the active hosts based on the .Ar key and the source IP address of the client. -This mode is only supported by relays. +This mode is supported by redirections and relays. .El .Pp The optional
Re: Fix upd(4) sensor refresh
On 19/12/14(Fri) 08:04, David Higgs wrote: Split upd_update_sensors() behavior to gather all values prior to updating sensor contents. Because sensors were updated in report_ID order, it could take multiple passes of upd_refresh() to update some sensors. Specifically, battery-related sensors “prior” to a change in BatteryPresent would be stale/incorrect until the 2nd call to upd_refresh(). Also, change a confusing DPRINTF of report_ID from un-prefixed hex to decimal, to be consistent. What you're saying is that querying HUB_BATTERY_PRESENT after 6 sensors that depend on its value does not make sense, right? Well in this case I don't think that sensors should be queried in reportID order. I even don't think that sensors should be queried at all if we're going to discard the result anyway. It might not matter that much right now, but if the number of queried sensors grow s significantly we might end up waiting a lot of time in upd_refresh() prior to updating anything. This fix is needed for future improvements, where output or behavior of certain sensors depends on the current value of more than one report (e.g. RemainingCapacity should actually depend on CapacityMode). But do you need to read the value of CapacityMode more than once, at the begging or when you modify it? Anyway, when we discussed that with Andre, I suggested to have a table containing all sensors depending on an other one. This way you can start by querying HUB_BATTERY_PRESENT and if it is present, query the others. Another advantage is that it will simplify the code because all the sensors in a table will be in the same page. Since you're doing some great job with upd(4), here's another idea. Did you consider updating the values of the sensors via an asynchronous function and a callback? This way the thread does not need to way for the previous queries. That would involve adding a new function, let's say uhidev_get_report_async() that takes a custom callback since there's no such API for the moment. Regards, Martin
Re: Dell R630 high interrupts on acpi0
Date: Fri, 19 Dec 2014 23:41:56 +1000 From: Jonathan Matthew jonat...@d14n.org On Thu, Dec 18, 2014 at 11:14:54PM +0100, Frederic Nowak wrote: Hi! The diff for extracting memory ranges from ACPI was obviously not tested with ACPI disabled...so we definitely have to check if the values in pcimem_range make some sense. The diff below now uses the old values in case ACPI is disabled or does not return valid values. I hope this is somewhat interesting for someone else as well :) Please let me know if this is just noise or if there is anything else I am missing. This looks like the right direction, except I think we only really want to use the ACPI range information to add new ranges to the existing one that covers the 36-bit address space, rather than relying on it for everything. No point relying on the ACPI stuff being correct if we don't need to. I hacked this around a bit today and ended up with the diff below. This only deals with ranges outside the 36-bit space, so it makes no difference on most machines, and on the r630 that needs it, it adds two ranges, 0x0300 to 0x033F and 0x0340 to 0x037F. This is going in the right direction. The way I designed things though was that the acpi code would build up the complete extent purely from information provided by _CRS, and set pcimem_ex before pci_init_extents() gets called. Index: arch/amd64/pci/pci_machdep.c === RCS file: /cvs/src/sys/arch/amd64/pci/pci_machdep.c,v retrieving revision 1.60 diff -u -p -r1.60 pci_machdep.c --- arch/amd64/pci/pci_machdep.c 16 Dec 2014 23:13:20 - 1.60 +++ arch/amd64/pci/pci_machdep.c 19 Dec 2014 12:54:44 - @@ -90,12 +90,17 @@ #include dev/pci/ppbreg.h #include ioapic.h +#include acpi.h #if NIOAPIC 0 #include machine/i82093var.h #include machine/mpbiosvar.h #endif +#if NACPI 0 +#include dev/acpi/acpivar.h +#endif + /* * Memory Mapped Configuration space access. * @@ -622,17 +627,13 @@ pci_init_extents(void) * here. As long as vendors continue to support * 32-bit operating systems, we should never see BARs * outside that region. - * - * Dell 13G servers have important devices outside the - * 36-bit address space. Until we can extract the address - * ranges from ACPI, expand the allowed range to suit. */ pcimem_ex = extent_create(pcimem, 0, 0xUL, M_DEVBUF, NULL, 0, EX_NOWAIT); if (pcimem_ex == NULL) return; - extent_alloc_region(pcimem_ex, 0x400UL, - 0xfc00UL, EX_NOWAIT); + extent_alloc_region(pcimem_ex, 0x10UL, + 0xfff0UL, EX_NOWAIT); for (bmp = bios_memmap; bmp-type != BIOS_MAP_END; bmp++) { /* @@ -657,6 +658,14 @@ pci_init_extents(void) /* Take out the video buffer area and BIOS areas. */ extent_alloc_region(pcimem_ex, IOM_BEGIN, IOM_SIZE, EX_CONFLICTOK | EX_NOWAIT); + +#if NACPI 0 + /* + * Free up any regions outside the 36-bit address space + * specified via ACPI. + */ + acpi_pciroots_extents(pcimem_ex, 0x10UL); +#endif } if (pcibus_ex == NULL) { @@ -665,7 +674,6 @@ pci_init_extents(void) } } -#include acpi.h #if NACPI 0 void acpi_pci_match(struct device *, struct pci_attach_args *); pcireg_t acpi_pci_min_powerstate(pci_chipset_tag_t, pcitag_t); Index: dev/acpi/acpivar.h === RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v retrieving revision 1.79 diff -u -p -r1.79 acpivar.h --- dev/acpi/acpivar.h8 Dec 2014 07:12:37 - 1.79 +++ dev/acpi/acpivar.h19 Dec 2014 12:54:44 - @@ -47,6 +47,7 @@ extern u_int8_t acpi_lapic_flags[LAPIC_M struct klist; struct acpiec_softc; struct acpipwrres_softc; +struct extent; struct acpivideo_softc { struct device sc_dev; @@ -357,6 +358,7 @@ int acpi_acquire_glk(uint32_t *); int acpi_release_glk(uint32_t *); void acpi_pciroots_attach(struct device *, void *, cfprint_t); +void acpi_pciroots_extents(struct extent *, u_int64_t); #endif Index: dev/acpi/amltypes.h === RCS file: /cvs/src/sys/dev/acpi/amltypes.h,v retrieving revision 1.40 diff -u -p -r1.40 amltypes.h --- dev/acpi/amltypes.h 7 Sep 2012 19:19:59 - 1.40 +++ dev/acpi/amltypes.h 19 Dec 2014 12:54:44 - @@ -346,6 +346,12 @@ struct aml_value { #define aml_pkglen(v)
Re: pstat segfault
On 12/19/14 07:18, Stuart Cassoff wrote: $ pstat -T 221/7030 open files Segmentation fault I suppose more info from me is desired. $ gdb pstat GNU gdb 6.3 Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type show copying to see the conditions. There is absolutely no warranty for GDB. Type show warranty for details. This GDB was configured as i386-unknown-openbsd5.6... (no debugging symbols found) (gdb) run -T Starting program: /usr/sbin/pstat -T (no debugging symbols found) 205/7030 open files Program received signal SIGSEGV, Segmentation fault. kvm_read (kd=0x0, kva=0, buf=0xcfbc72c4, len=8) at /usr/src/lib/libkvm/kvm.c:845 845 if (ISALIVE(kd)) { (gdb) bt #0 kvm_read (kd=0x0, kva=0, buf=0xcfbc72c4, len=8) at /usr/src/lib/libkvm/kvm.c:845 #1 0x181e9ce0 in ?? () from /usr/sbin/pstat #2 0x in ?? () OpenBSD 5.6-current (GENERIC) #605: Thu Dec 18 01:56:16 MST 2014 dera...@i386.openbsd.org:/usr/src/sys/arch/i386/compile/GENERIC cpu0: Intel(R) Pentium(R) 4 CPU 3.00GHz (GenuineIntel 686-class) 3 GHz cpu0: FPU,V86,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,CNXT-ID,xTPR,PERF real mem = 2683277312 (2558MB) avail mem = 2627121152 (2505MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: AT/286+ BIOS, date 02/23/04, BIOS32 rev. 0 @ 0xffe90, SMBIOS rev. 2.3 @ 0xf0450 (65 entries) bios0: vendor Dell Computer Corporation version A05 date 02/23/2004 bios0: Dell Computer Corporation Dimension 8300 acpi0 at bios0: rev 0 acpi0: sleep states S0 S1 S3 S4 S5 acpi0: tables DSDT FACP SSDT APIC BOOT acpi0: wakeup devices VBTN(S4) PCI0(S3) USB0(S3) USB1(S3) USB2(S3) USB3(S3) PCI1(S5) KBD_(S3) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 199MHz ioapic0 at mainbus0: apid 1 pa 0xfec0, version 20, 24 pins ioapic0: misconfigured as apic 0, remapped to apid 1 acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus 2 (PCI1) acpicpu0 at acpi0 acpibtn0 at acpi0: VBTN bios0: ROM list: 0xc/0xf800 0xcf800/0x1800! 0xd1000/0x3000 pci0 at mainbus0 bus 0: configuration mode 1 (bios) pchb0 at pci0 dev 0 function 0 Intel 82875P Host rev 0x02 intelagp0 at pchb0 agp0 at intelagp0: aperture at 0xd000, size 0x1000 ppb0 at pci0 dev 1 function 0 Intel 82875P AGP rev 0x02 pci1 at ppb0 bus 1 vga1 at pci1 dev 0 function 0 NVIDIA GeForce FX 5200 rev 0xa1 wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) wsdisplay0: screen 1-5 added (80x25, vt100 emulation) uhci0 at pci0 dev 29 function 0 Intel 82801EB/ER USB rev 0x02: apic 1 int 16 uhci1 at pci0 dev 29 function 1 Intel 82801EB/ER USB rev 0x02: apic 1 int 19 uhci2 at pci0 dev 29 function 2 Intel 82801EB/ER USB rev 0x02: apic 1 int 18 uhci3 at pci0 dev 29 function 3 Intel 82801EB/ER USB rev 0x02: apic 1 int 16 ehci0 at pci0 dev 29 function 7 Intel 82801EB/ER USB2 rev 0x02: apic 1 int 23 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 Intel EHCI root hub rev 2.00/1.00 addr 1 ppb1 at pci0 dev 30 function 0 Intel 82801BA Hub-to-PCI rev 0xc2 pci2 at ppb1 bus 2 fxp0 at pci2 dev 8 function 0 Intel PRO/100 VE rev 0x02, i82562: apic 1 int 20, address 00:0c:f1:cd:b7:8d inphy0 at fxp0 phy 1: i82562ET 10/100 PHY, rev. 0 ichpcib0 at pci0 dev 31 function 0 Intel 82801EB/ER LPC rev 0x02 pciide0 at pci0 dev 31 function 1 Intel 82801EB/ER IDE rev 0x02: DMA, channel 0 configured to compatibility, channel 1 configured to compatibility wd0 at pciide0 channel 0 drive 0: ST380011A wd0: 16-sector PIO, LBA48, 76293MB, 15625 sectors wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 5 atapiscsi0 at pciide0 channel 1 drive 0 scsibus1 at atapiscsi0: 2 targets cd0 at scsibus1 targ 0 lun 0: SAMSUNG, DVD-ROM SD-616E, F501 ATAPI 5/cdrom removable cd0(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 2 pciide1 at pci0 dev 31 function 2 Intel 82801EB SATA rev 0x02: DMA, channel 0 configured to native-PCI, channel 1 configured to native-PCI pciide1: using apic 1 int 18 for native-PCI interrupt ichiic0 at pci0 dev 31 function 3 Intel 82801EB/ER SMBus rev 0x02: apic 1 int 17 iic0 at ichiic0 spdmem0 at iic0 addr 0x50: 256MB DDR SDRAM non-parity PC3200CL3.0 spdmem1 at iic0 addr 0x51: 1GB DDR SDRAM non-parity PC3200CL3.0 spdmem2 at iic0 addr 0x52: 256MB DDR SDRAM non-parity PC3200CL3.0 spdmem3 at iic0 addr 0x53: 1GB DDR SDRAM non-parity PC3200CL3.0 auich0 at pci0 dev 31 function 5 Intel 82801EB/ER AC97 rev 0x02: apic 1 int 17, ICH5 AC97 ac97: codec id 0x41445370 (Analog Devices AD1980) ac97: codec features headphone, 20 bit DAC, No 3D Stereo audio0 at auich0 usb1 at uhci0: USB revision 1.0 uhub1 at usb1 Intel UHCI root hub rev 1.00/1.00 addr 1
Re: pstat segfault
On Fri, Dec 19, 2014 at 13:04, Stuart Cassoff wrote: On 12/19/14 07:18, Stuart Cassoff wrote: $ pstat -T 221/7030 open files Segmentation fault I suppose more info from me is desired. I think that was sufficient. It should be fixed now.
Re: pstat segfault
On 12/19/14 18:12, Ted Unangst wrote: On Fri, Dec 19, 2014 at 13:04, Stuart Cassoff wrote: On 12/19/14 07:18, Stuart Cassoff wrote: $ pstat -T 221/7030 open files Segmentation fault I suppose more info from me is desired. I think that was sufficient. It should be fixed now. Hi Ted, It's fixed, but if I run pstat as an ordinary user I get: port:fred /usr/src/usr.sbin/pstat ./pstat -T pstat: kvm_openfiles: /dev/mem: Permission denied rather than: port:fred /usr/src/usr.sbin/pstat sudo ./pstat -T Password: 1477/7030 open files 34681 vnodes 0M/8311M swap space But pstat -f works as expected as an ordinary user. Cheers Fred PS This with pstat.c patched to 1.96
Re: Dell R630 high interrupts on acpi0
On 17.12.2014. 6:34, Philip Guenther wrote: Uh, ACPI *requires* that C1 exist. The halt instruction is defined as entering C1, so not having C1 would mean your CPU lacks a basic manadatory ia32 instruction. Hopefully the BIOS docs explain that you're just disabling deep C-states or something like that. If not, yell at the company that made it. With the exception of C1E, I wouldn't tell a BIOS to disable C-states unless it was causing the OS to have a problem or you're actively trying to use the computer to heat your house. C1E is a cross between C1 and C3; the issue is that bugs in multiple early hardware implementations mean it'll behave poorly depending on exactly how the OS handles it. This is something to test...and then test again with each release you install... Thank you, this is very informative. I have disabled C states because I have seen ifq.drops and netlivelocks when I run udp tcpbench over ix interface and C-states are enabled. This is between two Dell R620. With Dell R630 I have bigger problems so i can't test them right now. tcpbench client sends around 350kpps for 20s tcpbench server gets around 212kpps tcpbench udp server drops: Without C states and C1E: net.inet.ip.ifq.maxlen=2048 net.inet.ip.ifq.drops=0 kern.netlivelocks=3 With C states and C1E (DAPC): net.inet.ip.ifq.maxlen=2048 net.inet.ip.ifq.drops=19143 kern.netlivelocks=86 so I explained to myself that C states are bad for packet bursts and these boxes will be uber high speed firewalls :) and of course after reading you mail I only disable C1E and everything is fine. With C states and without C1E net.inet.ip.ifq.maxlen=2048 net.inet.ip.ifq.drops=0 kern.netlivelocks=3 Monitor/Mwait - Disabled I would suggest leaving that on. We ain't using it *right now*, but, well, the source tree on my laptop is, and more than ever. :-) mwait is enabled :)
NTP
The ntp daemon included in OpenBSD is our own openntpd, written from scratch. openntpd is not vulnerable. Around 10 years ago it was written by Henning, at my request because the ntpd source code scared the hell out of us. At the time communications with the ntp team showed they had little interest in removing unused functionality from the ntp.org code, or any help with our form of source code audit. Because it was a rewrite, the major benefit in openntpd is that it priviledge seperated. If problems like these were found, they would not be realistically exploitable. Furthermore openntpd is a modern piece of code 5000 lines long written using best known practices of the time, whereas ntp.org's codebase is reportedly 100,000 lines of unknown or largely unused code, poorly smithed in the past when these kinds of programming mistakes were not a significant consideration. This might be a good time to circle the conversation back to the common practice of: srand(time(NULL)); Sorry, getting really jaded. When will the software vendors WAKE THE HELL UP? This is not 2000 anymore. It has become abundantly clear that it is very difficult to push lessons regarding better software practices into the greater open source community and the vendors who live off the teat.
Re: NTP
On Fri, Dec 19, 2014 at 8:22 PM, Theo de Raadt dera...@cvs.openbsd.org wrote: whereas ntp.org's codebase is reportedly 100,000 lines of unknown or largely unused code That made me curious. Is it that bloated? $ for i in $(find . -name *.[ch]); do cat $i allcode; done $ egrep -v '[:blank:]*/?\*' allcode | grep -v ^ *$ | wc -l 192870 This is ntp-4.2.8 A rough estimate but close enough if we are comparing to a know solution that is 5000. Keep up the good work. Tim.
Re: NTP
Using the same test on OpenNTPD from OpenBSD-STABLE: # cd /usr/src/usr.sbin/ntpd/ # for i in $(find . -name *.[ch]); do cat $i /root/allcode; done # egrep -v '[:blank:]*/?\*' /root/allcode | grep -v ^ *$ | wc -l 2898 Quite a difference indeed. On Fri, Dec 19, 2014 at 7:26 PM, trondd tro...@gmail.com wrote: On Fri, Dec 19, 2014 at 8:22 PM, Theo de Raadt dera...@cvs.openbsd.org wrote: whereas ntp.org's codebase is reportedly 100,000 lines of unknown or largely unused code That made me curious. Is it that bloated? $ for i in $(find . -name *.[ch]); do cat $i allcode; done $ egrep -v '[:blank:]*/?\*' allcode | grep -v ^ *$ | wc -l 192870 This is ntp-4.2.8 A rough estimate but close enough if we are comparing to a know solution that is 5000. Keep up the good work. Tim. -- BSD is what happens when Unix programmers port Unix to the x86. Linux is what happens when x86 programmers write a Unix-like. Windows is what happens when x86 programmers run all of their programming textbooks through a blender, eat the ground up remains of the text, and then code up what they can read in the toilet 3 days later.
Re: NTP
# cd /usr/src/usr.sbin/ntpd/ # for i in $(find . -name *.[ch]); do cat $i /root/allcode; done # egrep -v '[:blank:]*/?\*' /root/allcode | grep -v ^ *$ | wc -l 2898 $ for i in $(find . -name *.[ch]); do cat $i allcode; done $ egrep -v '[:blank:]*/?\*' allcode | grep -v ^ *$ | wc -l 192870 This is ntp-4.2.8 A rough estimate but close enough if we are comparing to a know solution that is 5000. That is a factor of 66x. Shocking, it is larger than I remembered. So, what does that extra source code bring to the table? My perspective is that big code brings holes because fewer people want to read, audit, and maintain the code. But surely there must be some benefit. It has been claimed NTPD is more accurate. For that extra size, is it 66x more accurate? That's silly. Even if it is a little bit more accurate, what does 'more accurate' mean when edge devices that care for more than ~100ms accuracy are exceedingly rare? The devices which care for accuracy are doing time distribution, not time reception at the network edge. And there lies the problem, I think. Much of that code is likely for special purposes in an ecosystem which includes products, relationships with companies building products, special modes, special features, labratory code, etc. The result was that this team built a NTP daemon for all purposes great and small: One NTPD to rule them all, One NTPD to find them, One NTPD to bring them all and on the internet hole them I am going to guess the serious time distribution people are in control of this software stack, but 99% of their base is in the edge devices where the extra complexity is unwarranted? (Is it time for a coup?) I think the NTP people don't even know their own codebase because it is too large and full of legacy code. They don't know what parts of it do, but they don't want to upset their base by removing any of it. In another project we recently reviewed, there was code all over the place for some rotten platforms -- barely limping along -- but the existance and structure of that code would would be cumbersome and the effects would be detrimental to all platforms. Developers trying to change the code make mistakes -- or worse they get demoralized and stop trying. Such codebases purport to be about agility, but as it tries to reach that goal which is increasingly less relevant, it hurts the most important goals. You cannot properly serve 1995 and 2015. Back to NTPD, I suspect most of the people who struggled through it in the last decade have been looking for holes to exploit. It does not seem healthy. Healthy projects struggle with the effort of refactoring their code. Unhealthy project sit on their past results, and the future catches up. Certainly the codebase was also being run by strong-minded people, I know the type. The NTP codebase is larger than the SSH codebase. Explain that, and then explain why noone visibly called them on that. I recall the conversations I had with the NTP team bit around 10 years ago. They did not want help auditing their code. It was sad, and I found a more receptive audience in Henning. Once he wrote openntpd, we had to face public attacks from NTP team members. Apparently the openntpd math was bad and we would break internet time.. internet still seems fine, except a ntp clients spun up to 100 peers is seeing a fair number of ntp time distributions points are down... I am glad to hear NTPD is being rewritten. Because over time this will bring safety back back into focus. PHK, I hope you don't just focus on the math. Privsep it, please, but ensure DNS refresh is possible in the architecture you choose. There is a time-tested model which does not require the OS to have this or that non-standard security feature; we all know we can't mandate use of those features which barely help anyways. You rely on privsep everytime you login to another machine, so consider it. Very hard to push pressure upstream.