patch: atfork unlock
Hi. As first mentioned on misc[1], I've identified a deadlock in libc
when a process forks, the children are multi-threaded, and they set
one or more atfork callbacks. The diff below causes ATFORK_UNLOCK() to
release the lock even when the process isn't multi-threaded. This
avoids the deadlock. With this patch applied, the test case I have for
this issue succeeds and there are no new failures during a full 'make
regress'.
Threading is outside my area of expertise so I've no idea if the fix
proposed here is appropriate. I'm happy to take or test feedback.
The diff is below and a clean copy is here:
https://www.packetmischief.ca/files/patches/atfork_on_fork.diff.
.joel
[1] https://marc.info/?l=openbsd-misc=166926508819790=2
Index: lib/libc/include/thread_private.h
===
RCS file: /data/cvs-mirror/OpenBSD/src/lib/libc/include/thread_private.h,v
retrieving revision 1.36
diff -p -u -r1.36 thread_private.h
--- lib/libc/include/thread_private.h 6 Jan 2021 19:54:17 - 1.36
+++ lib/libc/include/thread_private.h 8 Dec 2022 04:28:45 -
@@ -228,7 +228,7 @@ __END_HIDDEN_DECLS
} while (0)
#define _ATFORK_UNLOCK() \
do { \
- if (__isthreaded) \
+ if (_thread_cb.tc_atfork_unlock != NULL) \
_thread_cb.tc_atfork_unlock(); \
} while (0)
Index: regress/lib/libpthread/pthread_atfork_on_fork/Makefile
===
RCS file: regress/lib/libpthread/pthread_atfork_on_fork/Makefile
diff -N regress/lib/libpthread/pthread_atfork_on_fork/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libpthread/pthread_atfork_on_fork/Makefile 7 Dec 2022
04:38:39 -
@@ -0,0 +1,9 @@
+# $OpenBSD$
+
+PROG= pthread_atfork_on_fork
+
+REGRESS_TARGETS= timeout
+timeout:
+ timeout 10s ./${PROG}
+
+.include
Index: regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
===
RCS file: regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
diff -N regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
7 Dec 2022 04:59:10 -
@@ -0,0 +1,94 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2022 Joel Knight
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*
+ * This test exercises atfork lock/unlock through multiple generations of
+ * forked child processes where each child also becomes multi-threaded.
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+#define NUMBER_OF_GENERATIONS 4
+#define NUMBER_OF_TEST_THREADS 2
+
+#define SAY(...) do { \
+fprintf(stderr, "pid %5i ", getpid()); \
+fprintf(stderr, __VA_ARGS__); \
+} while (0)
+
+void
+prepare(void)
+{
+/* Do nothing */
+}
+
+void *
+thread(void *arg)
+{
+return (NULL);
+}
+
+int
+test(int fork_level)
+{
+pid_t proc_pid;
+size_t thread_index;
+pthread_t threads[NUMBER_OF_TEST_THREADS];
+
+proc_pid = fork();
+fork_level = fork_level - 1;
+
+if (proc_pid == 0) {
+SAY("generation %i\n", fork_level);
+pthread_atfork(prepare, NULL, NULL);
+
+for (thread_index = 0; thread_index < NUMBER_OF_TEST_THREADS;
thread_index++) {
+pthread_create([thread_index], NULL, thread, NULL);
+}
+
+for (thread_index = 0; thread_index < NUMBER_OF_TEST_THREADS;
thread_index++) {
+pthread_join(threads[thread_index], NULL);
+}
+
+if (fork_level > 0) {
+test(fork_level);
+}
+
+SAY("exiting\n");
+exit(0);
+}
+else {
+SAY("parent waiting on child %i\n", proc_pid);
+waitpid(proc_pid, 0, 0);
+}
+
+return (0);
+}
+
+int
+main(int argc, char *argv[])
+{
+test(NUMBER_OF_GENERATIONS);
+
+return (0);
+}
Re: dhcpleased - set ciaddr per RFC
We have a winner here. I tested from INIT through to REBINDING and the
behavior matches what's in the RFC now. ok joel@
One cosmetic thing I noticed this time around: log_dhcp_hdr() in
engine.c should be printing dhcp_hdr->xid in host byte order.
.joel
On Fri, Dec 3, 2021 at 5:21 AM Florian Obser wrote:
>
> Last one, Joel spotted one more bug in the previous one, I was missing
> an assignment to dhcp_server in the ACK case.
>
> This is a rewrite of the package construction logic to send the correct
> information in the right states:
>
> RFC 2131 4.3.6, Table 4
> -
> | |REBOOTING|REQUESTING |RENEWING |REBINDING |
> -
> |broad/unicast |broadcast|broadcast|unicast |broadcast |
> |server-ip |MUST NOT |MUST |MUST NOT |MUST NOT |
> |requested-ip |MUST |MUST |MUST NOT |MUST NOT |
> |ciaddr|zero |zero |IP address |IP address|
> -
>
> Previous this logic was all over the place and difficult to check.
> This diff moves it all into the engine.c functions
> request_dhcp_discover() and request_dhcp_request().
>
> The frontend then leaves out dhcp options that are set to INADDR_ARPA,
> but it does not need to know in which state the engine is.
>
> OK?
>
>
> diff --git dhcpleased.h dhcpleased.h
> index b3b4938ad3c..6df38be6f5b 100644
> --- dhcpleased.h
> +++ dhcpleased.h
> @@ -287,15 +287,10 @@ struct imsg_dhcp {
> uint8_t packet[1500];
> };
>
> -
> -struct imsg_req_discover {
> - uint32_tif_index;
> - uint32_txid;
> -};
> -
> -struct imsg_req_request {
> +struct imsg_req_dhcp {
> uint32_tif_index;
> uint32_txid;
> + struct in_addr ciaddr;
> struct in_addr requested_ip;
> struct in_addr server_identifier;
> struct in_addr dhcp_server;
> diff --git engine.c engine.c
> index 17e65fbe789..3169bd80d40 100644
> --- engine.c
> +++ engine.c
> @@ -1170,6 +1170,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
> imsg_dhcp *dhcp)
> return;
> }
> iface->server_identifier.s_addr = server_identifier.s_addr;
> + iface->dhcp_server.s_addr = server_identifier.s_addr;
> iface->requested_ip.s_addr = dhcp_hdr->yiaddr.s_addr;
> state_transition(iface, IF_REQUESTING);
> break;
> @@ -1228,6 +1229,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
> imsg_dhcp *dhcp)
>
> clock_gettime(CLOCK_MONOTONIC, >request_time);
> iface->server_identifier.s_addr = server_identifier.s_addr;
> + iface->dhcp_server.s_addr = server_identifier.s_addr;
> iface->requested_ip.s_addr = dhcp_hdr->yiaddr.s_addr;
> iface->mask.s_addr = subnet_mask.s_addr;
> #ifndef SMALL
> @@ -1354,11 +1356,8 @@ state_transition(struct dhcpleased_iface *iface, enum
> if_state new_state)
> case IF_REBOOTING:
> if (old_state == IF_REBOOTING)
> iface->timo.tv_sec *= 2;
> - else {
> - /* make sure we send broadcast */
> - iface->dhcp_server.s_addr = INADDR_ANY;
> + else
> iface->timo.tv_sec = START_EXP_BACKOFF;
> - }
> request_dhcp_request(iface);
> break;
> case IF_REQUESTING:
> @@ -1377,10 +1376,6 @@ state_transition(struct dhcpleased_iface *iface, enum
> if_state new_state)
> break;
> case IF_RENEWING:
> if (old_state == IF_BOUND) {
> - iface->dhcp_server.s_addr =
> - iface->server_identifier.s_addr;
> - iface->server_identifier.s_addr = INADDR_ANY;
> -
> iface->timo.tv_sec = (iface->rebinding_time -
> iface->renewal_time) / 2; /* RFC 2131 4.4.5 */
> } else
> @@ -1392,7 +1387,6 @@ state_transition(struct dhcpleased_iface *iface, enum
> if_state new_state)
> break;
> case IF_REBINDING:
> if (old_state == IF_RENEWING) {
> - iface->dhcp_server.s_addr = INADDR_ANY;
> iface->timo.tv_sec = (iface->lease_time -
> iface->rebinding_time) / 2; /* RFC 2131 4.4.5 */
> } else
> @@ -1470,28 +1464,90 @@ iface_timeout(int fd, short events, void *arg)
> void
> request_dhcp_discover(struct dhcpleased_iface *iface)
> {
> - struct imsg_req_discover imsg_req_discover;
> + struct
Re: dhcpleased - set ciaddr per RFC
On Wed, Nov 24, 2021 at 4:46 AM Florian Obser wrote:
> Thanks, I had indeed missed this. I went through the RFC and found that
> we MUST NOT send the server identifier in rebooting state. While here I
> also made it explicit that we are not sending server identifier in
> rebinding state. This was already the case since we only transition from
> renewing to rebinding, but this is clearer.
The behavior for this option looks consistent with prior behavior.
However, the RFC confuses me wrt this option and when it should/should
not be set so I'm only relying on the fact that broadcast and unicast
requests are working fine in testing.
> @@ -1486,9 +1488,17 @@ request_dhcp_request(struct dhcpleased_iface *iface)
> iface->xid = arc4random();
> imsg_req_request.if_index = iface->if_index;
> imsg_req_request.xid = iface->xid;
> + if (iface->state == IF_RENEWING || iface->state == IF_REBINDING)
> + imsg_req_request.ciaddr.s_addr = iface->requested_ip.s_addr;
> + else
> + imsg_req_request.ciaddr.s_addr = 0;
> imsg_req_request.server_identifier.s_addr =
> iface->server_identifier.s_addr;
> - imsg_req_request.requested_ip.s_addr = iface->requested_ip.s_addr;
> + if (iface->state == IF_RENEWING || iface->state == IF_REBINDING)
> + imsg_req_request.requested_ip.s_addr = 0;
> + else
> + imsg_req_request.requested_ip.s_addr =
> + iface->requested_ip.s_addr;
These two if/else statements could be merged.
> ssize_t
> build_packet(uint8_t message_type, char *if_name, uint32_t xid,
> -struct ether_addr *hw_address, struct in_addr *requested_ip,
> -struct in_addr *server_identifier)
> +struct ether_addr *hw_address, struct in_addr *ciaddr, struct in_addr
> +*requested_ip, struct in_addr *server_identifier)
> {
> static uint8_t dhcp_cookie[] = DHCP_COOKIE;
> static uint8_t dhcp_message_type[] = {DHO_DHCP_MESSAGE_TYPE, 1,
> @@ -926,6 +929,7 @@ build_packet(uint8_t message_type, char *if_name,
> uint32_t xid,
> hdr->hops = 0;
> hdr->xid = xid;
> hdr->secs = 0;
> + hdr->ciaddr.s_addr = ciaddr->s_addr;
> memcpy(hdr->chaddr, hw_address, sizeof(*hw_address));
> p += sizeof(struct dhcp_hdr);
> memcpy(p, dhcp_cookie, sizeof(dhcp_cookie));
The problem with build_packet() now is that it will unconditionally
insert opt 50 (requested IP address) even if ciaddr is being set. The
request packet ends up with ciaddr as the correct client IP and opt 50
as "0.0.0.0".
.joel
Re: dhcpleased - set ciaddr per RFC
On Fri, Nov 19, 2021 at 1:01 PM Joel Knight wrote:
>
> One thing that got missed in the refactor was that the requested-ip
> option should not be set in a RENEWING or BINDING state (or in other
> words, when ciaddr is set). This chunk on top of your diff also works
> as expected (successful unicast renewal at T1).
Hi Florian,
Does the chunk below make sense?
> Index: frontend.c
> ===
> RCS file: /data/cvs-mirror/OpenBSD/src/sbin/dhcpleased/frontend.c,v
> retrieving revision 1.23
> diff -p -u -r1.23 frontend.c
> --- frontend.c 20 Oct 2021 07:04:49 - 1.23
> +++ frontend.c 19 Nov 2021 16:25:18 -
> @@ -963,11 +966,13 @@ build_packet(uint8_t message_type, char
> p += sizeof(dhcp_req_list);
>
> if (message_type == DHCPREQUEST) {
> - memcpy(dhcp_requested_address + 2, requested_ip,
> - sizeof(*requested_ip));
> - memcpy(p, dhcp_requested_address,
> - sizeof(dhcp_requested_address));
> - p += sizeof(dhcp_requested_address);
> + if (ciaddr->s_addr == 0) {
> + memcpy(dhcp_requested_address + 2, requested_ip,
> + sizeof(*requested_ip));
> + memcpy(p, dhcp_requested_address,
> + sizeof(dhcp_requested_address));
> + p += sizeof(dhcp_requested_address);
> + }
>
> if (server_identifier->s_addr != INADDR_ANY) {
> memcpy(dhcp_server_identifier + 2, server_identifier,
Re: dhcpleased - set ciaddr per RFC
On Tue, Nov 16, 2021 at 9:56 AM Joel Knight wrote:
>
> Inspecting the REQUEST packets showed that dhclient was setting the
> ciaddr to the existing leased IP address while dhcpleased was not
> setting this field. RFC 2131 4.3.2 (with a nice summary at 4.3.6) is
> pretty strict about when ciaddr and the 'requested ip' option should
> be used. The diff below modifies dhcpleased to set ciaddr and
> 'requested ip' at the appropriate times.
Hey Florian,
I wasn't subscribed to tech@ at the time, so don't have your reply in
my inbox. Thanks for explaining the approach. Your patch works
(thankfully T1 is only 2 hours with my ISP :)).
One thing that got missed in the refactor was that the requested-ip
option should not be set in a RENEWING or BINDING state (or in other
words, when ciaddr is set). This chunk on top of your diff also works
as expected (successful unicast renewal at T1).
.joel
Index: frontend.c
===
RCS file: /data/cvs-mirror/OpenBSD/src/sbin/dhcpleased/frontend.c,v
retrieving revision 1.23
diff -p -u -r1.23 frontend.c
--- frontend.c 20 Oct 2021 07:04:49 - 1.23
+++ frontend.c 19 Nov 2021 16:25:18 -
@@ -963,11 +966,13 @@ build_packet(uint8_t message_type, char
p += sizeof(dhcp_req_list);
if (message_type == DHCPREQUEST) {
- memcpy(dhcp_requested_address + 2, requested_ip,
- sizeof(*requested_ip));
- memcpy(p, dhcp_requested_address,
- sizeof(dhcp_requested_address));
- p += sizeof(dhcp_requested_address);
+ if (ciaddr->s_addr == 0) {
+ memcpy(dhcp_requested_address + 2, requested_ip,
+ sizeof(*requested_ip));
+ memcpy(p, dhcp_requested_address,
+ sizeof(dhcp_requested_address));
+ p += sizeof(dhcp_requested_address);
+ }
if (server_identifier->s_addr != INADDR_ANY) {
memcpy(dhcp_server_identifier + 2, server_identifier,
dhcpleased - set ciaddr per RFC
Hi. On a firewall recently upgraded to 7.0, I noticed that dhcpleased
was not getting a reply from my ISP's DHCP server during renewal at
T1. At T2, dhcpleased would broadcast the REQUEST which would be
answered. Testing with dhclient showed successful renewals at T1.
Inspecting the REQUEST packets showed that dhclient was setting the
ciaddr to the existing leased IP address while dhcpleased was not
setting this field. RFC 2131 4.3.2 (with a nice summary at 4.3.6) is
pretty strict about when ciaddr and the 'requested ip' option should
be used. The diff below modifies dhcpleased to set ciaddr and
'requested ip' at the appropriate times.
While here, I also noticed that if you configure an interface in
dhcpleased.conf but do not use "set client id", dhcpleased will not
send the default client id as is stated in dhcpleased.conf(5).
Since gmail will undoubtedly muck up this diff, a clean copy is here:
www.packetmischief.ca/files/patches/dhcpleased.ciaddr.diff
.joel
Index: src/sbin/dhcpleased/dhcpleased.h
===
RCS file: /data/cvs-mirror/OpenBSD/src/sbin/dhcpleased/dhcpleased.h,v
retrieving revision 1.11
diff -p -u -r1.11 dhcpleased.h
--- src/sbin/dhcpleased/dhcpleased.h 12 Aug 2021 12:41:08 - 1.11
+++ src/sbin/dhcpleased/dhcpleased.h 16 Nov 2021 04:06:06 -
@@ -191,6 +191,18 @@ struct dhcp_route {
struct in_addr gw;
};
+enum if_state {
+ IF_DOWN,
+ IF_INIT,
+ /* IF_SELECTING, */
+ IF_REQUESTING,
+ IF_BOUND,
+ IF_RENEWING,
+ IF_REBINDING,
+ /* IF_INIT_REBOOT, */
+ IF_REBOOTING,
+};
+
enum imsg_type {
IMSG_NONE,
#ifndef SMALL
@@ -294,6 +306,7 @@ struct imsg_req_discover {
};
struct imsg_req_request {
+ enum if_state if_state;
uint32_t if_index;
uint32_t xid;
struct in_addr requested_ip;
Index: src/sbin/dhcpleased/engine.c
===
RCS file: /data/cvs-mirror/OpenBSD/src/sbin/dhcpleased/engine.c,v
retrieving revision 1.29
diff -p -u -r1.29 engine.c
--- src/sbin/dhcpleased/engine.c 14 Nov 2021 18:13:19 - 1.29
+++ src/sbin/dhcpleased/engine.c 16 Nov 2021 04:06:34 -
@@ -60,18 +60,6 @@
#define MAX_EXP_BACKOFF_FAST 2
#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
-enum if_state {
- IF_DOWN,
- IF_INIT,
- /* IF_SELECTING, */
- IF_REQUESTING,
- IF_BOUND,
- IF_RENEWING,
- IF_REBINDING,
- /* IF_INIT_REBOOT, */
- IF_REBOOTING,
-};
-
const char* if_state_name[] = {
"Down",
"Init",
@@ -1485,6 +1473,7 @@ request_dhcp_request(struct dhcpleased_i
iface->xid = arc4random();
imsg_req_request.if_index = iface->if_index;
+ imsg_req_request.if_state = iface->state;
imsg_req_request.xid = iface->xid;
imsg_req_request.server_identifier.s_addr =
iface->server_identifier.s_addr;
Index: src/sbin/dhcpleased/frontend.c
===
RCS file: /data/cvs-mirror/OpenBSD/src/sbin/dhcpleased/frontend.c,v
retrieving revision 1.23
diff -p -u -r1.23 frontend.c
--- src/sbin/dhcpleased/frontend.c 20 Oct 2021 07:04:49 - 1.23
+++ src/sbin/dhcpleased/frontend.c 16 Nov 2021 04:14:23 -
@@ -74,6 +74,7 @@ struct iface {
struct in_addr server_identifier;
struct in_addr dhcp_server;
int udpsock;
+ enum if_state state;
};
__dead void frontend_shutdown(void);
@@ -91,7 +92,7 @@ struct iface *get_iface_by_id(uint32_t);
void remove_iface(uint32_t);
void set_bpfsock(int, uint32_t);
ssize_t build_packet(uint8_t, char *, uint32_t, struct ether_addr *,
- struct in_addr *, struct in_addr *);
+ struct in_addr *, struct in_addr *, enum if_state *);
void send_discover(struct iface *);
void send_request(struct iface *);
void bpf_send_packet(struct iface *, uint8_t *, ssize_t);
@@ -511,6 +512,7 @@ frontend_dispatch_engine(int fd, short e
imsg_req_request.server_identifier.s_addr;
iface->dhcp_server.s_addr =
imsg_req_request.dhcp_server.s_addr;
+ iface->state = imsg_req_request.if_state;
send_request(iface);
}
break;
@@ -888,7 +890,7 @@ bpf_receive(int fd, short events, void *
ssize_t
build_packet(uint8_t message_type, char *if_name, uint32_t xid,
struct ether_addr *hw_address, struct in_addr *requested_ip,
-struct in_addr *server_identifier)
+struct in_addr *server_identifier, enum if_state *if_state)
{
static uint8_t dhcp_cookie[] = DHCP_COOKIE;
static uint8_t dhcp_message_type[] = {DHO_DHCP_MESSAGE_TYPE, 1,
@@ -926,6 +928,9 @@ build_packet(uint8_t message_type, char
hdr->hops = 0;
hdr->xid = xid;
hdr->secs = 0;
+ if (message_type == DHCPREQUEST && (*if_state == IF_RENEWING ||
+ *if_state == IF_REBINDING))
+ memcpy(>ciaddr, requested_ip, sizeof(*requested_ip));
memcpy(hdr->chaddr, hw_address, sizeof(*hw_address));
p += sizeof(struct dhcp_hdr);
memcpy(p, dhcp_cookie, sizeof(dhcp_cookie));
@@ -946,6 +951,11 @@ build_packet(uint8_t message_type, char
/* XXX check space */
memcpy(p, iface_conf->c_id, iface_conf->c_id_len);
p += iface_conf->c_id_len;
+
[patch] OPENBSD-PF-MIB pfCntProtoCksum description
Small fix to the description of the pfCntProtoCksum oid:
Index: share/snmp/OPENBSD-PF-MIB.txt
===
RCS file: /data/cvs-mirror/OpenBSD/src/share/snmp/OPENBSD-PF-MIB.txt,v
retrieving revision 1.5
diff -p -u -r1.5 OPENBSD-PF-MIB.txt
--- share/snmp/OPENBSD-PF-MIB.txt 10 Jun 2015 10:03:59 - 1.5
+++ share/snmp/OPENBSD-PF-MIB.txt 16 Jun 2018 18:16:23 -
@@ -197,7 +197,7 @@ pfCntProtoCksum OBJECT-TYPE
MAX-ACCESS read-only
STATUS current
DESCRIPTION
- "The number of packets that were dropped due to memory limitations."
+ "The number of packets that were dropped due to TCP checksum failures."
::= { pfCounters 10 }
pfCntStateMismatch OBJECT-TYPE
(Incase gmail mangles the copy/paste:
https://www.packetmischief.ca/files/patches/openbsd-pf-mib.diff)
>From Joel Carnat joel at carnat net
.joel
Re: [patch] snmpd hrStorageSize negative values
On Thu, Mar 9, 2017 at 10:02 PM, Joel Knight <knight.j...@gmail.com> wrote:
> Hi.
>
> snmpd(8) uses unsigned ints internally to represent the size and used
> space of a file system. The HOST-RESOURCES-MIB defines the valid
> values for those OIDs as 0..2147483647. With sufficiently large file
> systems, this can cause negative numbers to be returned for the size
> and used space OIDs.
>
> .1.3.6.1.2.1.25.2.3.1.5.36=-1573167768
Hi. Just wanted to bump this again and see if anyone that cares about
snmp could take a look? Looking for oks and someone who wouldn't mind
committing it.
> At sthen's suggestion, do what net-snmp does and fiddle with the
> values to prevent wrapping. Yes this mucks with the actual values of
> size, used space, and block size, but it allows snmpd to convey the
> proper size and used space of the file system which is what most
> everybody is really interested in.
>
> In case gmail hoses this diff, it's also here:
> https://www.packetmischief.ca/files/patches/snmpd.hrstorage2.diff
Index: usr.sbin/snmpd/mib.c
===
RCS file: /data/cvs-mirror/OpenBSD/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.80
diff -p -u -r1.80 mib.c
--- usr.sbin/snmpd/mib.c17 Nov 2015 12:30:23 - 1.80
+++ usr.sbin/snmpd/mib.c19 Feb 2017 20:01:46 -
@@ -643,6 +643,14 @@ mib_hrstorage(struct oid *oid, struct be
units = mnt->f_bsize;
size = mnt->f_blocks;
used = mnt->f_blocks - mnt->f_bfree;
+
+ /* for large filesystems, do not overflow hrStorageSize */
+ while (size > INT32_MAX) {
+ size = size >> 1;
+ units = units << 1;
+ used = used >> 1;
+ }
+
sop = [3];
break;
}
[patch] snmpd hrStorageSize negative values
Hi.
snmpd(8) uses unsigned ints internally to represent the size and used
space of a file system. The HOST-RESOURCES-MIB defines the valid
values for those OIDs as 0..2147483647. With sufficiently large file
systems, this can cause negative numbers to be returned for the size
and used space OIDs.
.1.3.6.1.2.1.25.2.3.1.5.36=-1573167768
At sthen's suggestion, do what net-snmp does and fiddle with the
values to prevent wrapping. Yes this mucks with the actual values of
size, used space, and block size, but it allows snmpd to convey the
proper size and used space of the file system which is what most
everybody is really interested in.
In case gmail hoses this diff, it's also here:
https://www.packetmischief.ca/files/patches/snmpd.hrstorage2.diff
Index: usr.sbin/snmpd/mib.c
===
RCS file: /data/cvs-mirror/OpenBSD/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.80
diff -p -u -r1.80 mib.c
--- usr.sbin/snmpd/mib.c 17 Nov 2015 12:30:23 - 1.80
+++ usr.sbin/snmpd/mib.c 19 Feb 2017 20:01:46 -
@@ -643,6 +643,14 @@ mib_hrstorage(struct oid *oid, struct be
units = mnt->f_bsize;
size = mnt->f_blocks;
used = mnt->f_blocks - mnt->f_bfree;
+
+ /* for large filesystems, do not overflow hrStorageSize */
+ while (size > INT32_MAX) {
+ size = size >> 1;
+ units = units << 1;
+ used = used >> 1;
+ }
+
sop = [3];
break;
}
.joel
Re: isakmpd nat-t patch
It does not. 0.010267 openbsd.4500 asa.4500: [bad udp cksum 6d4c!] udpencap: isakmp v1.0 exchange QUICK_MODE cookie: 1dc820688b0e577c-9abdf94cdd39ebb0 msgid: 0b77fb8d len: 292 payload: HASH len: 24 payload: SA len: 56 DOI: 1(IPSEC) situation: IDENTITY_ONLY payload: PROPOSAL len: 44 proposal: 1 proto: IPSEC_ESP spisz: 4 xforms: 1 SPI: 0x30a85144 payload: TRANSFORM len: 32 transform: 1 ID: AES attribute LIFE_TYPE = SECONDS attribute LIFE_DURATION = 28800 attribute ENCAPSULATION_MODE = TUNNEL attribute AUTHENTICATION_ALGORITHM = HMAC_SHA attribute GROUP_DESCRIPTION = 2 attribute KEY_LENGTH = 256 On Sat, Feb 1, 2014 at 5:39 PM, Stuart Henderson s...@spacehopper.orgwrote: isakmpd already sends the values from the RFC doesn't it? On 2 February 2014 00:23:19 GMT+00:00, Joel Knight knight.j...@gmail.com wrote: Hi. I found an old post of sthen's to tech@ about NAT-T interop between isakmpd(8) and Cisco ASA. In summary, when isakmpd negotiates NAT-T with ASA, it doesn't send the proper encapsulation mode (as per RFC 3947). Original post is here: http://openbsd.7691.n7.nabble.com/isakmpd-NAT-T-interoperability-td173004.html The original patch had isakmpd send the encap mode values as specified in the NAT-T draft (modes 61443/61444) which I found ASA 9.x code rejected (rightly or wrongly). Having isakmpd send the RFC values of 3/4 allows the tunnel to come up. I don't know enough to say what the correct behavior should be here, but as sthen points out, the current behavior is definitely wrong. http://packetmischief.ca/files/openbsd/patches/isakmpd-nat-t-encap-mode.diff .joel
isakmpd nat-t patch
Hi. I found an old post of sthen's to tech@ about NAT-T interop between isakmpd(8) and Cisco ASA. In summary, when isakmpd negotiates NAT-T with ASA, it doesn't send the proper encapsulation mode (as per RFC 3947). Original post is here: http://openbsd.7691.n7.nabble.com/isakmpd-NAT-T-interoperability-td173004.html The original patch had isakmpd send the encap mode values as specified in the NAT-T draft (modes 61443/61444) which I found ASA 9.x code rejected (rightly or wrongly). Having isakmpd send the RFC values of 3/4 allows the tunnel to come up. I don't know enough to say what the correct behavior should be here, but as sthen points out, the current behavior is definitely wrong. http://packetmischief.ca/files/openbsd/patches/isakmpd-nat-t-encap-mode.diff .joel
snmpd OPENBSD-PF-MIB table 'match' counters
Hi,
This diff adds the table packet/byte counters for match rules to PF-MIB.
In case gmail mucks up the formatting, the diff is here too:
http://www.packetmischief.ca/files/openbsd/patches/snmpd.match.diff
ok?
.joel
Index: OPENBSD-PF-MIB.txt
===
RCS file: /cvs/src/share/snmp/OPENBSD-PF-MIB.txt,v
retrieving revision 1.2
diff -p -u -r1.2 OPENBSD-PF-MIB.txt
--- OPENBSD-PF-MIB.txt 11 Mar 2013 19:49:37 - 1.2
+++ OPENBSD-PF-MIB.txt 2 Sep 2013 22:28:49 -
@@ -1,6 +1,6 @@
-- $OpenBSD: OPENBSD-PF-MIB.txt,v 1.2 2013/03/11 19:49:37 sthen Exp $
--
--- Copyright (c) 2004-2012 Joel Knight knight.j...@gmail.com
+-- Copyright (c) 2004-2013 Joel Knight knight.j...@gmail.com
--
-- Permission to use, copy, modify, and distribute this document for any
-- purpose with or without fee is hereby granted, provided that the above
@@ -43,6 +43,8 @@ pfMIBObjects MODULE-IDENTITY
DESCRIPTION The MIB module for gathering information from
OpenBSD's packet filter.
+REVISION 201308310446Z
+DESCRIPTION Add pf(4) table byte/packet counters for 'match' rules
REVISION 201302242033Z
DESCRIPTION Add separate counter for failed translations
REVISION 20120126Z
@@ -919,7 +921,11 @@ TblEntry ::=
pfTblOutBlockBytes Counter64,
pfTblOutXPassPkts Counter64,
pfTblOutXPassBytes Counter64,
- pfTblStatsCleared TimeTicks
+ pfTblStatsCleared TimeTicks,
+ pfTblInMatchPkts Counter64,
+ pfTblInMatchBytes Counter64,
+ pfTblOutMatchPkts Counter64,
+ pfTblOutMatchBytes Counter64
}
pfTblIndex OBJECT-TYPE
@@ -1092,6 +1098,44 @@ pfTblStatsCleared OBJECT-TYPE
for this pf table were zeroed.
::= { pfTblEntry 20 }
+pfTblInMatchPkts OBJECT-TYPE
+ SYNTAX Counter64
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The number of inbound packets that hit a 'match' rule where this
+ particular table was referenced by the rule.
+ ::= { pfTblEntry 21 }
+
+pfTblInMatchBytes OBJECT-TYPE
+ SYNTAX Counter64
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The total size in bytes of all inbound packets that hit a
+ 'match' rule where this particular table was referenced by
+ the rule.
+ ::= { pfTblEntry 22 }
+
+pfTblOutMatchPkts OBJECT-TYPE
+ SYNTAX Counter64
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The number of outbound packets that hit a 'match' rule where this
+ particular table was referenced by the rule.
+ ::= { pfTblEntry 23 }
+
+pfTblOutMatchBytes OBJECT-TYPE
+ SYNTAX Counter64
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The total size in bytes of all outbound packets that hit a
+ 'match' rule where this particular table was referenced by
+ the rule.
+ ::= { pfTblEntry 24 }
+
pfTblAddrTable OBJECT-TYPE
SYNTAX SEQUENCE OF TblAddrEntry
MAX-ACCESS not-accessible
@@ -1124,7 +1168,11 @@ TblAddrEntry ::=
pfTblAddrOutBlockPkts Counter64,
pfTblAddrOutBlockBytes Counter64,
pfTblAddrOutPassPkts Counter64,
- pfTblAddrOutPassBytes Counter64
+ pfTblAddrOutPassBytes Counter64,
+ pfTblAddrInMatchPkts Counter64,
+ pfTblAddrInMatchBytes Counter64,
+ pfTblAddrOutMatchPkts Counter64,
+ pfTblAddrOutMatchBytes Counter64
}
pfTblAddrTblIndex OBJECT-TYPE
@@ -1235,6 +1283,42 @@ pfTblAddrOutPassBytes OBJECT-TYPE
The number of outbound bytes passed as a result of matchg
this table entry.
::= { pfTblAddrEntry 12 }
+
+pfTblAddrInMatchPkts OBJECT-TYPE
+ SYNTAX Counter64
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The number of inbound packets that hit a 'match' rule where
+ this table entry was referenced.
+ ::= { pfTblAddrEntry 13 }
+
+pfTblAddrInMatchBytes OBJECT-TYPE
+ SYNTAX Counter64
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The total size in bytes of all inbound packets that hit
+ a 'match' rule where this table entry was referenced.
+ ::= { pfTblAddrEntry 14 }
+
+pfTblAddrOutMatchPkts OBJECT-TYPE
+ SYNTAX Counter64
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The number of outbound packets that hit a 'match' rule where
+ this table entry was referenced.
+ ::= { pfTblAddrEntry 15 }
+
+pfTblAddrOutMatchBytes OBJECT-TYPE
+ SYNTAX Counter64
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The total size in bytes of all outbound packets that hit
+ a 'match' rule where this table entry was referenced.
+ ::= { pfTblAddrEntry 16 }
-- pfLabels
Index: mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.64
diff -p -u -r1.64 mib.c
--- mib.c 11 Mar 2013 19:49:37 - 1.64
+++ mib.c 2 Sep 2013 22:29:54 -
@@ -1548,6 +1548,10 @@ static struct oid openbsd_mib[] = {
{ MIB(pfTblOutXPassPkts), OID_TRD, mib_pftables },
{ MIB(pfTblOutXPassBytes), OID_TRD, mib_pftables },
{ MIB(pfTblStatsCleared), OID_TRD, mib_pftables },
+ { MIB(pfTblInMatchPkts), OID_TRD, mib_pftables },
+ { MIB(pfTblInMatchBytes), OID_TRD, mib_pftables },
+ { MIB(pfTblOutMatchPkts), OID_TRD
Re: Memory leak in snmpd(8)
On Thu, May 24, 2012 at 8:16 AM, Kenneth R Westerback
kwesterb...@rogers.com wrote:
Calling mib_carpget() seems a tad over complex. Wouldn't the diff
below make it cleaner? Untested except by gcc.
And doesn't the socket 's' leak too, or does SIOCGVH returning -1
mean 's' was closed?
Ken,
Your diff looks good to me. I also found a few spots that leak in
pf.c. All combined I've come up with this diff. Ok?
Index: src/usr.sbin/snmpd/mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.52
diff -p -u -r1.52 mib.c
--- src/usr.sbin/snmpd/mib.c20 Mar 2012 03:01:26 - 1.52
+++ src/usr.sbin/snmpd/mib.c26 May 2012 05:14:36 -
@@ -1360,7 +1360,7 @@ intmib_carpstats(struct oid *, struct
int mib_carpiftable(struct oid *, struct ber_oid *, struct ber_element **);
int mib_carpifnum(struct oid *, struct ber_oid *, struct ber_element **);
struct carpif
- *mib_carpifget(struct carpif *, u_int);
+ *mib_carpifget(u_int);
int mib_memiftable(struct oid *, struct ber_oid *, struct ber_element **);
static struct oid openbsd_mib[] = {
@@ -2647,9 +2647,10 @@ mib_carpifnum(struct oid *oid, struct be
}
struct carpif *
-mib_carpifget(struct carpif *cif, u_int idx)
+mib_carpifget(u_int idx)
{
struct kif *kif;
+ struct carpif *cif;
int s;
struct ifreq ifr;
struct carpreq carpr;
@@ -2689,12 +2690,17 @@ mib_carpifget(struct carpif *cif, u_int
memset((char *)carpr, 0, sizeof(carpr));
ifr.ifr_data = (caddr_t)carpr;
- if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
+ if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1) {
+ close(s);
return (NULL);
+ }
- memset(cif, 0, sizeof(struct carpif));
- memcpy(cif-carpr, carpr, sizeof(struct carpreq));
- memcpy(cif-kif, kif, sizeof(struct kif));
+ cif = malloc(sizeof(struct carpif));
+ if (cif != NULL) {
+ memset(cif, 0, sizeof(struct carpif));
+ memcpy(cif-carpr, carpr, sizeof(struct carpreq));
+ memcpy(cif-kif, kif, sizeof(struct kif));
+ }
close(s);
@@ -2707,16 +2713,11 @@ mib_carpiftable(struct oid *oid, struct
u_int32_tidx;
struct carpif *cif;
- if ((cif = malloc(sizeof(struct carpif))) == NULL)
- return (1);
-
/* Get and verify the current row index */
idx = o-bo_id[OIDIDX_carpIfEntry];
- if ((cif = mib_carpifget(cif, idx)) == NULL) {
- free(cif);
+ if ((cif = mib_carpifget(idx)) == NULL)
return (1);
- }
/* Tables need to prepend the OID on their own */
o-bo_id[OIDIDX_carpIfEntry] = cif-kif.if_index;
Index: src/usr.sbin/snmpd/pf.c
===
RCS file: /cvs/src/usr.sbin/snmpd/pf.c,v
retrieving revision 1.2
diff -p -u -r1.2 pf.c
--- src/usr.sbin/snmpd/pf.c 14 May 2012 00:02:33 - 1.2
+++ src/usr.sbin/snmpd/pf.c 26 May 2012 05:22:04 -
@@ -228,8 +228,10 @@ pfi_count(void)
struct pfi_kif *p;
int c = 0;
- if (pfi_get(b, NULL))
+ if (pfi_get(b, NULL)) {
+ free(b.pfrb_caddr);
return (-1);
+ }
PFRB_FOREACH(p, b)
c++;
@@ -245,8 +247,10 @@ pfi_get_if(struct pfi_kif *rp, int idx)
struct pfi_kif *p;
int i = 1;
- if (pfi_get(b, NULL))
+ if (pfi_get(b, NULL)) {
+ free(b.pfrb_caddr);
return (-1);
+ }
PFRB_FOREACH(p, b) {
if (i == idx)
@@ -290,9 +294,11 @@ pft_get_table(struct pfr_tstats *rts, in
struct pfr_tstats *ts;
int i = 1;
- if (pft_get(b, NULL))
+ if (pft_get(b, NULL)) {
+ free(b.pfrb_caddr);
return (-1);
-
+ }
+
PFRB_FOREACH(ts, b) {
if (!(ts-pfrts_flags PFR_TFLAG_ACTIVE))
continue;
@@ -319,8 +325,10 @@ pft_count(void)
struct pfr_tstats *ts;
int c = 0;
- if (pft_get(b, NULL))
+ if (pft_get(b, NULL)) {
+ free(b.pfrb_caddr);
return (-1);
+ }
PFRB_FOREACH(ts, b) {
if (!(ts-pfrts_flags PFR_TFLAG_ACTIVE))
PF-MIB for snmpd
Hi, Here's the patch to bring the PF-MIB to snmpd. It provides the same OIDs that my patches for net-snmp provide. Some differences: - This implementation uses the OpenBSD enterprise number (30155) instead of a private/reserved number - Many of the OID names have been changed to make them less ambiguous and to avoid current or future naming conflicts (eg, running changed to pfRunning) I've been running this on a couple of boxes with no issues. The diff is pretty long so I'm only going to post a link. Please test. http://www.packetmischief.ca/files/openbsd/patches/snmpd.pf.diff .joel
Re: PF-MIB for snmpd
Looks like there's an issue pulling the diff with lynx or curl. If you use Firefox or Chrome it seems to work fine. Not sure what's going on. The diff is also on dropbox now. http://dl.dropbox.com/u/34631638/snmpd.pf.diff On Sat, Feb 18, 2012 at 2:34 PM, Joel Knight knight.j...@gmail.com wrote: Hi, Here's the patch to bring the PF-MIB to snmpd. It provides the same OIDs that my patches for net-snmp provide. Some differences: - This implementation uses the OpenBSD enterprise number (30155) instead of a private/reserved number - Many of the OID names have been changed to make them less ambiguous and to avoid current or future naming conflicts (eg, running changed to pfRunning) I've been running this on a couple of boxes with no issues. The diff is pretty long so I'm only going to post a link. Please test. http://www.packetmischief.ca/files/openbsd/patches/snmpd.pf.diff .joel
Re: PF-MIB for snmpd
On Sat, Feb 18, 2012 at 5:02 PM, Peter N. M. Hansteen pe...@bsdly.net wrote: Joel Knight knight.j...@gmail.com writes: Looks like there's an issue pulling the diff with lynx or curl. If you use Firefox or Chrome it seems to work fine. Not sure what's going on. looks clean when fetched with ftp(1) as well, didn't actually try applying though. it's against a recent -current, right? Yes, you'll need a recent checkout of -current for this to apply cleanly. .joel
Re: Add OPENBSD-CARP-MIB to snmpd(8)
On Fri, Jan 13, 2012 at 2:53 PM, Joel Knight knight.j...@gmail.com wrote:
Hi,
This diff integrates my existing snmp MIB for carp(4) into the base
snmpd. I brought the MIB straight across with no changes. The only
limitation I'm aware of is that it doesn't support the multiple
carpnodes style loadbalancing; it only reports status on the main
carpnode.
http://www.packetmischief.ca/files/openbsd/patches/snmpd.carp.diff
Updated diff based on feedback from camield@
- relayd(8) already uses .30155.3 as its based OID for sending traps
so move carp to .30155.6
- fix file descriptor leak
.joel
Index: src/share/snmp/Makefile
===
RCS file: /cvs/src/share/snmp/Makefile,v
retrieving revision 1.1
diff -p -u -r1.1 Makefile
--- src/share/snmp/Makefile 23 Dec 2008 18:32:10 - 1.1
+++ src/share/snmp/Makefile 29 Jan 2012 13:29:42 -
@@ -2,6 +2,7 @@
FILES= OPENBSD-SNMPD-CONF.txt OPENBSD-BASE-MIB.txt
FILES+=OPENBSD-MEM-MIB.txt OPENBSD-SENSORS-MIB.txt
+FILES+= OPENBSD-CARP-MIB.txt
all clean cleandir depend lint obj tags: _SUBDIRUSE
Index: src/share/snmp/OPENBSD-CARP-MIB.txt
===
RCS file: src/share/snmp/OPENBSD-CARP-MIB.txt
diff -N src/share/snmp/OPENBSD-CARP-MIB.txt
--- /dev/null 1 Jan 1970 00:00:00 -
+++ src/share/snmp/OPENBSD-CARP-MIB.txt 29 Jan 2012 13:29:42 -
@@ -0,0 +1,312 @@
+--
+-- $OpenBSD$
+--
+--
+-- Copyright (c) 2006-2011 Joel Knight knight.j...@gmail.com
+--
+-- Permission to use, copy, modify, and distribute this document for any
+-- purpose with or without fee is hereby granted, provided that the above
+-- copyright notice and this permission notice appear in all copies.
+--
+-- THE DOCUMENT IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+-- WITH REGARD TO THIS DOCUMENT INCLUDING ALL IMPLIED WARRANTIES OF
+-- MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+-- ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+-- WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+-- ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+-- OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS DOCUMENT.
+
+
+OPENBSD-CARP-MIB DEFINITIONS ::= BEGIN
+
+IMPORTS
+ MODULE-IDENTITY, NOTIFICATION-TYPE, OBJECT-TYPE,
+ Counter64, Integer32, enterprises
+ FROM SNMPv2-SMI
+
+ TruthValue
+ FROM SNMPv2-TC
+
+ openBSD
+ FROM OPENBSD-BASE-MIB
+
+ MODULE-COMPLIANCE, OBJECT-GROUP
+ FROM SNMPv2-CONF;
+
+carpMIBObjects MODULE-IDENTITY
+LAST-UPDATED 20110513Z
+ORGANIZATION OpenBSD
+CONTACT-INFO
+ Author: Joel Knight
+ email: knight.j...@gmail.com
+ www:www.packetmischief.ca/openbsd-snmp-mibs/
+
+DESCRIPTION The MIB module for gathering information about
+ Common Address Redundancy Protocol (CARP) interfaces.
+
+::= { openBSD 6 }
+
+
+-- define the sections of the MIB
+
+carpSysctl OBJECT IDENTIFIER ::= { carpMIBObjects 1 }
+carpIf OBJECT IDENTIFIER ::= { carpMIBObjects 2 }
+carpStats OBJECT IDENTIFIER ::= { carpMIBObjects 3 }
+
+
+-- carpSysctl
+carpAllow OBJECT-TYPE
+ SYNTAX TruthValue
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ Indicates whether the node will respond to CARP packets.
+ ::= { carpSysctl 1 }
+
+carpPreempt OBJECT-TYPE
+ SYNTAX TruthValue
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ Indicates whether preemption is enabled.
+ ::= { carpSysctl 2 }
+
+carpLog OBJECT-TYPE
+ SYNTAX TruthValue
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ Indicates whether logging of invalud CARP packets is enabled.
+ ::= { carpSysctl 3 }
+
+
+-- carpIf
+
+carpIfNumber OBJECT-TYPE
+ SYNTAX Integer32
+ MAX-ACCESS read-only
+ STATUS current
+ DESCRIPTION
+ The number of CARP interfaces present on this system.
+ ::= { carpIf 1 }
+
+carpIfTable OBJECT-TYPE
+ SYNTAX SEQUENCE OF CarpIfEntry
+ MAX-ACCESS not-accessible
+ STATUS current
+ DESCRIPTION
+ A list of individual CARP interfaces. The number of entries is
+ given by the value of carpIfNumber.
+ ::= { carpIf 2 }
+
+carpIfEntry OBJECT-TYPE
+ SYNTAX CarpIfEntry
+ MAX-ACCESS not-accessible
+ STATUS current
+ DESCRIPTION
+ An entry containing management information applicable to a
+ particular CARP interface.
+ INDEX { carpIfIndex }
+ ::= { carpIfTable 1 }
+
+CarpIfEntry ::=
+ SEQUENCE
snmpd sensor descriptions
Hi,
If the kernel sensor doesn't have a description, snmpd should fill
something into that column to avoid an empty value. An empty value
makes it really hard to tell what the sensor is.
Before:
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.1 =
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.2 =
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.3 =
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.4 =
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.5 =
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.6 =
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.7 = STRING: VCore A
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.8 = STRING: VCore B
After:
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.1 = STRING: temp0
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.2 = STRING: temp1
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.3 = STRING: temp2
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.4 = STRING: fan0
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.5 = STRING: fan1
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.6 = STRING: fan2
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.7 = STRING: VCore A
SNMPv2-SMI::enterprises.30155.2.1.2.1.2.8 = STRING: VCore B
ok?
.joel
Index: src/usr.sbin/snmpd/mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.47
diff -p -u -r1.47 mib.c
--- src/usr.sbin/snmpd/mib.c16 Sep 2011 20:52:48 - 1.47
+++ src/usr.sbin/snmpd/mib.c30 Jan 2012 05:22:45 -
@@ -1271,6 +1271,7 @@ mib_sensors(struct oid *oid, struct ber_
size_t len = sizeof(sensordev);
struct sensorsensor;
size_t slen = sizeof(sensor);
+ char desc[32];
int mib[] = { CTL_HW, HW_SENSORS, 0, 0, 0 };
int i, j, k;
u_int32_tidx = 0, n;
@@ -1315,7 +1316,13 @@ mib_sensors(struct oid *oid, struct ber_
ber = ber_add_integer(ber, (int32_t)n);
break;
case 2:
- ber = ber_add_string(ber, sensor.desc);
+ if (sensor.desc[0] == '\0') {
+ snprintf(desc, sizeof(desc), %s%d,
+ sensor_type_s[sensor.type],
+ sensor.numt);
+ ber = ber_add_string(ber, desc);
+ } else
+ ber = ber_add_string(ber, sensor.desc);
break;
case 3:
ber = ber_add_integer(ber, sensor.type);
Re: Add OPENBSD-CARP-MIB to snmpd(8)
On Fri, Jan 13, 2012 at 2:53 PM, Joel Knight knight.j...@gmail.com wrote: Hi, This diff integrates my existing snmp MIB for carp(4) into the base snmpd. I brought the MIB straight across with no changes. The only limitation I'm aware of is that it doesn't support the multiple carpnodes style loadbalancing; it only reports status on the main carpnode. http://www.packetmischief.ca/files/openbsd/patches/snmpd.carp.diff The MIB definition is here: http://www.packetmischief.ca/files/openbsd/snmp/OPENBSD-CARP-MIB.txt Has anyone tried running this or read through the diff? .joel
Add OPENBSD-CARP-MIB to snmpd(8)
Hi,
This diff integrates my existing snmp MIB for carp(4) into the base
snmpd. I brought the MIB straight across with no changes. The only
limitation I'm aware of is that it doesn't support the multiple
carpnodes style loadbalancing; it only reports status on the main
carpnode.
http://www.packetmischief.ca/files/openbsd/patches/snmpd.carp.diff
The MIB definition is here:
http://www.packetmischief.ca/files/openbsd/snmp/OPENBSD-CARP-MIB.txt
Comments?
The pf MIB will follow this one.
.joel
(Chrome/Gmail is messing with the formatting of the diff. Use the URL
above for a properly stylized version.)
Index: src/usr.sbin/snmpd/mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.45
diff -p -u -r1.45 mib.c
--- src/usr.sbin/snmpd/mib.c4 Jul 2011 04:34:14 - 1.45
+++ src/usr.sbin/snmpd/mib.c13 Jan 2012 21:21:23 -
@@ -27,13 +27,16 @@
#include sys/sysctl.h
#include sys/sensors.h
#include sys/sched.h
+#include sys/socket.h
#include sys/mount.h
+#include sys/ioctl.h
#include net/if.h
#include net/if_types.h
#include netinet/in.h
#include netinet/in_systm.h
#include netinet/ip.h
+#include netinet/ip_carp.h
#include netinet/ip_var.h
#include arpa/inet.h
@@ -1210,14 +1213,27 @@ mib_ifrcvtable(struct oid *oid, struct b
}
/*
- * Defined in OPENBSD-SENSORS-MIB.txt
- * (http://packetmischief.ca/openbsd/snmp/)
- */
+ * Defined in
+ * - OPENBSD-SENSORS-MIB.txt
+ * - OPENBSD-CARP-MIB.txt
+ * (http://www.packetmischief.ca/openbsd-snmp-mibs/)
+ */
+
+struct carpif {
+ struct carpreq carpr;
+ struct kif kif;
+};
int mib_sensornum(struct oid *, struct ber_oid *, struct ber_element **);
int mib_sensors(struct oid *, struct ber_oid *, struct ber_element **);
const char *mib_sensorunit(struct sensor *);
char *mib_sensorvalue(struct sensor *);
+int mib_carpsysctl(struct oid *, struct ber_oid *, struct ber_element **);
+int mib_carpstats(struct oid *, struct ber_oid *, struct ber_element **);
+int mib_carpiftable(struct oid *, struct ber_oid *, struct ber_element **);
+int mib_carpifnum(struct oid *, struct ber_oid *, struct ber_element **);
+struct carpif
+ *mib_carpifget(struct carpif *, u_int);
int mib_memiftable(struct oid *, struct ber_oid *, struct ber_element **);
static struct oid openbsd_mib[] = {
@@ -1230,6 +1246,33 @@ static struct oid openbsd_mib[] = {
{ MIB(sensorValue), OID_TRD, mib_sensors },
{ MIB(sensorUnits), OID_TRD, mib_sensors },
{ MIB(sensorStatus),OID_TRD, mib_sensors },
+ { MIB(carpMIBObjects), OID_MIB },
+ { MIB(carpAllow), OID_RD, mib_carpsysctl },
+ { MIB(carpPreempt), OID_RD, mib_carpsysctl },
+ { MIB(carpLog), OID_RD, mib_carpsysctl },
+ { MIB(carpIpPktsRecv), OID_RD, mib_carpstats },
+ { MIB(carpIp6PktsRecv), OID_RD, mib_carpstats },
+ { MIB(carpPktDiscardsBadIface), OID_RD, mib_carpstats },
+ { MIB(carpPktDiscardsBadTtl), OID_RD, mib_carpstats },
+ { MIB(carpPktShorterThanHdr), OID_RD, mib_carpstats },
+ { MIB(carpDiscardsBadCksum),OID_RD, mib_carpstats },
+ { MIB(carpDiscardsBadVersion), OID_RD, mib_carpstats },
+ { MIB(carpDiscardsTooShort),OID_RD, mib_carpstats },
+ { MIB(carpDiscardsBadAuth), OID_RD, mib_carpstats },
+ { MIB(carpDiscardsBadVhid), OID_RD, mib_carpstats },
+ { MIB(carpDiscardsBadAddrList), OID_RD, mib_carpstats },
+ { MIB(carpIpPktsSent), OID_RD, mib_carpstats },
+ { MIB(carpIp6PktsSent), OID_RD, mib_carpstats },
+ { MIB(carpNoMemory),OID_RD, mib_carpstats },
+ { MIB(carpTransitionsToMaster), OID_RD, mib_carpstats },
+ { MIB(carpIfNumber),OID_RD, mib_carpifnum },
+ { MIB(carpIfIndex), OID_TRD, mib_carpiftable },
+ { MIB(carpIfDescr), OID_TRD, mib_carpiftable },
+ { MIB(carpIfVhid), OID_TRD, mib_carpiftable },
+ { MIB(carpIfDev ), OID_TRD, mib_carpiftable },
+ { MIB(carpIfAdvbase), OID_TRD, mib_carpiftable },
+ { MIB(carpIfAdvskew), OID_TRD, mib_carpiftable },
+ { MIB(carpIfState), OID_TRD, mib_carpiftable },
{ MIB(memMIBObjects), OID_MIB },
{ MIB(memMIBVersion), OID_RD, mps_getint, NULL, NULL,
OIDVER_OPENBSD_MEM },
@@ -1407,6 +1450,190 @@ mib_sensorvalue(struct sensor *s)
if (ret == -1)
return (NULL);
return (v);
+}
+
+int
+mib_carpsysctl(struct oid *oid, struct ber_oid *o, struct ber_element **elm)
+{
+ size_t len;
+ int mib[] = { CTL_NET, PF_INET, IPPROTO_CARP, 0 };
+ int v;
+
+ mib[3] = oid-o_oid[OIDIDX_carpsysctl];
+ len = sizeof(v);
+
+
