diff --git a/proto/snmp/bgp4_mib.c b/proto/snmp/bgp4_mib.c index 32ec536a..134b3d3b 100644 --- a/proto/snmp/bgp4_mib.c +++ b/proto/snmp/bgp4_mib.c @@ -26,9 +26,6 @@ /* hash table only store ip4 addresses */ #define SNMP_HASH_LESS(ip1, ip2) SNMP_HASH_LESS4(ip1,ip2) -// delete me -#define SNMP_MANAGE_TBUF(...) (void)0 - #define DECLARE_BGP4(addr, proto, conn, stats, config) \ ip4_addr addr; \ const struct bgp_proto UNUSED *proto; \ @@ -63,6 +60,7 @@ snmp_bgp_last_error(const struct bgp_proto *bgp, char err[2]) static void snmp_bgp_reg_ok(struct snmp_proto *p, const struct agentx_response *res, struct snmp_registration *reg) { + /* TODO(contexts): add meaningful action */ const struct oid * const oid = reg->oid; (void)oid; (void)p; @@ -72,6 +70,7 @@ snmp_bgp_reg_ok(struct snmp_proto *p, const struct agentx_response *res, struct static void snmp_bgp_reg_failed(struct snmp_proto *p, const struct agentx_response *res, struct snmp_registration *reg) { + /* TODO(contexts): add meaningful action */ const struct oid * const oid = reg->oid; (void) res; (void)oid; @@ -379,6 +378,13 @@ populate_bgp4(struct snmp_pdu *c, ip4_addr *addr, const struct bgp_proto **proto return SNMP_SEARCH_OK; } + +/* + * + * MIB tree fill hooks + * + */ + static enum snmp_search_res fill_bgp_version(struct mib_walk_state *walk UNUSED, struct snmp_pdu *c) { @@ -785,7 +791,11 @@ fill_local_id(struct mib_walk_state *walk UNUSED, struct snmp_pdu *c) } /* - * bgp4_next_peer + * bgp4_next_peer - find next BGP peer with IPv4 address + * @state: MIB tree walk state + * @c: SNMP PDU context data + * + * Update TX-buffer VarBind name to next peer address. */ static int bgp4_next_peer(struct mib_walk_state *state, struct snmp_pdu *c) @@ -821,8 +831,7 @@ bgp4_next_peer(struct mib_walk_state *state, struct snmp_pdu *c) ASSUME(oid->n_subid == 9); - /* full path BGP4-MIB::bgpPeerEntry.x: .1.3.6.1.2.1.15.3.1.x - * index offset = ARRAY_SIZE(snmp_internet) + 1 + 4 + 1 */ + /* Stack has one more node for empty prefix (tree root) */ ASSUME(state->stack_pos > 10); oid->ids[4] = state->stack[10]->empty.id; @@ -871,8 +880,8 @@ snmp_bgp4_start(struct snmp_proto *p) struct snmp_config *cf = SKIP_BACK(struct snmp_config, cf, p->p.cf); - /* Create binding to BGP protocols */ + /* Create binding to BGP protocols */ struct snmp_bond *b; WALK_LIST(b, cf->bgp_entries) { diff --git a/proto/snmp/snmp.c b/proto/snmp/snmp.c index f3bf5431..2168e490 100644 --- a/proto/snmp/snmp.c +++ b/proto/snmp/snmp.c @@ -151,10 +151,10 @@ void agentx_get_mib_init(pool *p) /* * agentx_get_mib - classify an OID based on MIB prefix - * */ enum agentx_mibs agentx_get_mib(const struct oid *o) { + /* TODO: move me into MIB tree as hooks/MIB module root */ enum agentx_mibs mib = AGENTX_MIB_UNKNOWN; for (uint i = 0; i < AGENTX_MIB_COUNT + 1; i++) { @@ -214,19 +214,6 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) TRACE(D_EVENTS, "SNMP changing state to %u", state); - if (state == SNMP_DOWN && (last == SNMP_REGISTER || last == SNMP_CONN)) - { - /* We have a connection established (at least send out agentx-Open-PDU) */ - state = SNMP_STOP; - } - /* else - We did not send any packet, we perform protocol cleanup only. */ - - if (last == SNMP_RESET) - { - rfree(p->sock); - p->sock = NULL; - } - p->state = state; switch (state) @@ -321,14 +308,17 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) DBG("snmp -> SNMP_STOP\n"); ASSUME(last == SNMP_REGISTER || last == SNMP_CONN); snmp_stop_subagent(p); + // FIXME: special treatment for SNMP_OPEN last state? p->sock->rx_hook = snmp_rx_skip; p->sock->tx_hook = snmp_tx_skip; + p->startup_timer->hook = snmp_stop_timeout; tm_start(p->startup_timer, p->timeout); return PS_STOP; case SNMP_DOWN: DBG("snmp -> SNMP_DOWN\n"); + ASSERT(last == SNMP_STOP || last == SNMP_RESET); snmp_cleanup(p); // FIXME: handle the state in which we call proto_notify_state and // immediately return PS_DOWN from snmp_shutdown() @@ -338,7 +328,8 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) DBG("snmp -> SNMP_RESET\n"); ASSUME(last == SNMP_REGISTER || last == SNMP_CONN); ASSUME(p->sock); - snmp_stop_subagent(p); + tm_stop(p->ping_timer); + // FIXME: special treatment for SNMP_OPEN last state? p->sock->rx_hook = snmp_rx_skip; p->sock->tx_hook = snmp_tx_skip; return PS_STOP; @@ -423,20 +414,35 @@ snmp_connected(sock *sk) } /* - * snmp_reset - end the communication on AgentX session - * @p - SNMP protocol instance + * snmp_reset - reset AgentX session + * @p: SNMP protocol instance * - * End the communication on AgentX session by downing the whole procotol. This - * causes socket closure that implies AgentX session disconnection. - * This function is internal and shouldn't be used outside the SNMP module. + * We wait until the last PDU written into the socket is send while ignoring all + * incomming PDUs. Then we hard reset the connection by socket closure. The + * protocol instance is automatically restarted by nest. */ void snmp_reset(struct snmp_proto *p) { - tm_stop(p->ping_timer); - proto_notify_state(&p->p, snmp_set_state(p, SNMP_DOWN)); + proto_notify_state(&p->p, snmp_set_state(p, SNMP_RESET)); } + +/* + * snmp_stop - close AgentX session + * @p: SNMP protocol instance + * + * We write agentx-Close-PDU into the socket, wait until all written PDUs are + * send and then close the socket. The protocol instance is automatically + * restarted by nest. + */ +void +snmp_stop(struct snmp_proto *p) +{ + proto_notify_state(&p->p, snmp_set_state(p, SNMP_STOP)); +} + + /* * snmp_sock_err - handle errors on socket by reopenning the socket * @sk - socket owned by SNMP protocol instance diff --git a/proto/snmp/snmp.h b/proto/snmp/snmp.h index 2e73c313..f1fc5e21 100644 --- a/proto/snmp/snmp.h +++ b/proto/snmp/snmp.h @@ -171,6 +171,7 @@ void snmp_reconnect(timer *tm); int snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state); void snmp_reset(struct snmp_proto *p); +void snmp_stop(struct snmp_proto *p); extern const char agentx_master_addr[sizeof(AGENTX_MASTER_ADDR)]; diff --git a/proto/snmp/subagent.c b/proto/snmp/subagent.c index 1a1adbef..6e23e90c 100644 --- a/proto/snmp/subagent.c +++ b/proto/snmp/subagent.c @@ -20,9 +20,6 @@ */ /** - * - * - * * * Handling of malformed packet: * @@ -91,7 +88,7 @@ snmp_header(struct agentx_header *h, enum agentx_pdu_types type, u8 flags) } /* - * snmp_blank_header - create header with no flags except default + * snmp_blank_header - create header with no flags except byte order * @h: pointer to created header in TX-buffer * @type: create PDU type * @@ -239,7 +236,7 @@ snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *data, uint size, in if (snmp_tbuf_reserve(&c, sz)) snmp_log("agentx-Notify-PDU small buffer"); - struct agentx_header *h = (void *) c.buffer; + struct agentx_header *h = (struct agentx_header *) c.buffer; ADVANCE(c.buffer, c.size, AGENTX_HEADER_SIZE); snmp_blank_header(h, AGENTX_NOTIFY_PDU); p->packet_id++; /* New packet id */ @@ -351,7 +348,7 @@ un_register_pdu(struct snmp_proto *p, struct oid *oid, u32 bound, uint index, en STORE_U8(ur->reserved, 0); ADVANCE(c.buffer, c.size, sizeof(struct agentx_un_register_hdr)); - snmp_put_oid(c.buffer, oid); + (void) snmp_put_oid(c.buffer, oid); ADVANCE(c.buffer, c.size, snmp_oid_size(oid)); /* place upper-bound if needed */ @@ -522,7 +519,7 @@ parse_test_set_pdu(struct snmp_proto *p, byte * const pkt_start) if (c.error != AGENTX_RES_NO_ERROR) { response_err_ind(p, res, c.error, c.index + 1); - snmp_reset(p); // error + snmp_reset(p); } else if (all_possible) { @@ -676,24 +673,24 @@ space_for_response(const sock *sk) static uint parse_pkt(struct snmp_proto *p, byte *pkt, uint size) { - snmp_log("parse_pkt %t", current_time()); - /* TX-buffer free space */ if (size < AGENTX_HEADER_SIZE) return 0; - struct agentx_header *h = (void *) pkt; + struct agentx_header *h = (struct agentx_header *) pkt; if (h->flags & AGENTX_NETWORK_BYTE_ORDER) { TRACE(D_PACKETS, "SNMP received PDU with unexpected byte order"); + snmp_simple_response(p, AGENTX_RES_GEN_ERROR, 0); snmp_reset(p); return 0; } - uint pkt_size = LOAD_U32(h->payload); + u32 pkt_size = LOAD_U32(h->payload); /* RX side checks - too big packet */ if (pkt_size > SNMP_PKT_SIZE_MAX) { + TRACE(D_PACKETS, "SNMP received PDU is too long"); snmp_simple_response(p, AGENTX_RES_GEN_ERROR, 0); snmp_reset(p); return 0; /* no bytes parsed */ @@ -703,7 +700,8 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size) if (size < pkt_size + AGENTX_HEADER_SIZE) return 0; /* no bytes parsed */ - /* We need to see the responses for PDU such as + /* + * We need to see the responses for PDU such as * agentx-Open-PDU, agentx-Register-PDU, ... * even when we are outside the SNMP_CONNECTED state */ @@ -726,7 +724,6 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size) p->session_id = copy.session_id; p->transaction_id = copy.transaction_id; p->packet_id = copy.packet_id; - snmp_log("restoring packet_id %u from temporal state", p->packet_id); /* * After unexpected state, we simply reset the session @@ -740,11 +737,12 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size) { TRACE(D_PACKETS, "SNMP received PDU with non-default context"); snmp_simple_response(p, AGENTX_RES_UNSUPPORTED_CONTEXT, 0); - return pkt_size + AGENTX_HEADER_SIZE; + snmp_reset(p); + return 0; } refresh_ids(p, h); - switch (h->type) + switch (LOAD_U8(h->type)) { case AGENTX_GET_PDU: case AGENTX_GET_NEXT_PDU: @@ -768,8 +766,8 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size) default: /* We reset the connection for malformed packet (Unknown packet type) */ - TRACE(D_PACKETS, "SNMP received unknown packet with type %u", h->type); - snmp_set_state(p, SNMP_RESET); + TRACE(D_PACKETS, "SNMP received unknown packet with type %u", LOAD_U8(h->type)); + snmp_reset(p); return 0; } } @@ -821,8 +819,8 @@ parse_response(struct snmp_proto *p, byte *res) case AGENTX_RES_PARSE_ERROR: case AGENTX_RES_PROCESSING_ERR: default: - DBG("SNMP agentx-Response-PDU with unexpected error %u", r->error); - snmp_set_state(p, SNMP_DOWN); + TRACE(D_PACKETS, "SNMP agentx-Response-PDU with unexepected error %u", r->error); + snmp_stop(p); break; } @@ -924,7 +922,7 @@ snmp_vb_to_tx(struct snmp_pdu *c, const struct oid *oid) snmp_log("SNMP vb_to_tx small buffer"); ASSERT(c->size >= vb_hdr_size); - struct agentx_varbind *vb = (void *) c->buffer; + struct agentx_varbind *vb = (struct agentx_varbind *) c->buffer; ADVANCE(c->buffer, c->size, sizeof(struct agentx_varbind) - sizeof(struct oid)); /* Move the c->buffer so that is points at &vb->name */ snmp_set_varbind_type(vb, AGENTX_NULL); @@ -973,12 +971,12 @@ update_packet_size(struct agentx_header *start, byte *end) static inline void response_err_ind(struct snmp_proto *p, struct agentx_response *res, enum agentx_response_errs err, u16 ind) { - STORE_U32(res->error, (u16) err); + STORE_U16(res->error, (u16) err); // TODO deal with auto-incrementing of snmp_pdu context c.ind if (err != AGENTX_RES_NO_ERROR && err != AGENTX_RES_GEN_ERROR) { TRACE(D_PACKETS, "Last PDU resulted in error %u", err); - STORE_U32(res->index, ind); + STORE_U16(res->index, ind); TRACE(D_PACKETS, "Storing packet size %u (was %u)", sizeof(struct agentx_response) - AGENTX_HEADER_SIZE, LOAD_U32(res->h.payload)); STORE_U32(res->h.payload, sizeof(struct agentx_response) - AGENTX_HEADER_SIZE); @@ -986,13 +984,13 @@ response_err_ind(struct snmp_proto *p, struct agentx_response *res, enum agentx_ else if (err == AGENTX_RES_GEN_ERROR) { TRACE(D_PACKETS, "Last PDU resulted in error %u genErr", err); - STORE_U32(res->index, 0); + STORE_U16(res->index, 0); TRACE(D_PACKETS, "Storing packet size %u (was %u)", sizeof(struct agentx_response) - AGENTX_HEADER_SIZE, LOAD_U32(res->h.payload)); STORE_U32(res->h.payload, sizeof(struct agentx_response) - AGENTX_HEADER_SIZE); } else - STORE_U32(res->index, 0); + STORE_U16(res->index, 0); } static inline uint @@ -1263,8 +1261,7 @@ snmp_stop_subagent(struct snmp_proto *p) int snmp_rx(sock *sk, uint size) { - snmp_log("snmp_rx with size %u", size); - struct snmp_proto *p = sk->data; + struct snmp_proto *p = (struct snmp_proto *) sk->data; byte *pkt_start = sk->rbuf; byte *end = pkt_start + size; diff --git a/proto/snmp/subagent.h b/proto/snmp/subagent.h index 234b137a..cf53a212 100644 --- a/proto/snmp/subagent.h +++ b/proto/snmp/subagent.h @@ -148,7 +148,6 @@ enum agentx_flags { #define COPY_STR(proto, buf, str, length) ({ \ length = LOAD_PTR(buf); \ - /*log(L_INFO "LOAD_STR(), %p %u", proto->pool, length + 1); */ \ str = mb_alloc(proto->pool, length + 1); \ memcpy(str, buf+4, length); \ str[length] = '\0'; /* set term. char */ \ @@ -235,6 +234,13 @@ struct agentx_response { STATIC_ASSERT(4 + 2 + 2 + AGENTX_HEADER_SIZE == sizeof(struct agentx_response)); +struct agentx_open_pdu { + struct agentx_header h; + u8 timeout; + u8 reserved1; /* reserved u24 */ + u16 reserved2; /* whole u24 is always zero filled */ +}; + struct agentx_close_pdu { struct agentx_header h; u8 reason;