Index | Thread | Search

From:
Denis Fondras <openbsd@ledeuns.net>
Subject:
Re: snmpd/agentx compatibility issue
To:
Martijn van Duren <openbsd+tech@list.imperialat.at>
Cc:
tech@openbsd.org
Date:
Sun, 17 Mar 2024 09:31:53 +0100

Download raw body.

Thread
Le Thu, Mar 14, 2024 at 10:39:19AM +0100, Martijn van Duren a écrit :
> I've played around a bit with the libagentx bits and there's plenty to
> improve (it's almost as if one gets new insights with experience). But I
> reckon this is the most straightforward diff to fix the packetid issue.
> 
> OK?
> 
> martijn@
> 
> diff db9d9b4b3c06c791a4f448a5521c340160f4705e 3f7b601986d696996ff30f574183096979f23ec7
> commit - db9d9b4b3c06c791a4f448a5521c340160f4705e
> commit + 3f7b601986d696996ff30f574183096979f23ec7
> blob - 191892d6687fb364b54ff66dd5d572ba8346c77a
> blob + ad51662c9ca9cb82afa9cf5c532c4d9c141ebc4b
> --- agentx.c
> +++ agentx.c
> @@ -187,7 +187,7 @@ static void agentx_varbind_nosuchinstance(struct agent
>  static void agentx_varbind_endofmibview(struct agentx_varbind *);
>  static void agentx_varbind_error_type(struct agentx_varbind *,
>      enum ax_pdu_error, int);
> -static int agentx_request(struct agentx *, uint32_t,
> +static struct agentx_request *agentx_request(struct agentx *,
>      int (*)(struct ax_pdu *, void *), void *);
>  static int agentx_request_cmp(struct agentx_request *,
>      struct agentx_request *);
> @@ -447,7 +447,7 @@ static int
>  agentx_session_start(struct agentx_session *axs)
>  {
>  	struct agentx *ax = axs->axs_ax;
> -	uint32_t packetid;
> +	struct agentx_request *axr;
>  
>  #ifdef AX_DEBUG
>  	if (ax->ax_cstate != AX_CSTATE_OPEN ||
> @@ -456,18 +456,20 @@ agentx_session_start(struct agentx_session *axs)
>  		agentx_log_ax_fatalx(ax, "%s: unexpected session open",
>  		    __func__);
>  #endif
> -	packetid = ax_open(ax->ax_ax, axs->axs_timeout, &(axs->axs_oid),
> -	    &(axs->axs_descr));
> -	if (packetid == 0) {
> +	if ((axr = agentx_request(ax, agentx_session_finalize, axs)) == NULL)
> +		return -1;
> +	if (ax_open(ax->ax_ax, axs->axs_timeout, axr->axr_packetid,
> +	    &(axs->axs_oid), &(axs->axs_descr)) == -1) {
>  		agentx_log_ax_warn(ax, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_OPEN));
>  		agentx_reset(ax);

Should not you free() axr here ?

>  		return -1;
>  	}
> -	axs->axs_packetid = packetid;
> +	axs->axs_packetid = axr->axr_packetid;
>  	agentx_log_ax_info(ax, "opening session");
>  	axs->axs_cstate = AX_CSTATE_WAITOPEN;
> -	return agentx_request(ax, packetid, agentx_session_finalize, axs);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -510,14 +512,17 @@ agentx_session_close(struct agentx_session *axs,
>      enum ax_close_reason reason)
>  {
>  	struct agentx *ax = axs->axs_ax;
> -	uint32_t packetid;
> +	struct agentx_request *axr;
>  
>  #ifdef AX_DEBUG
>  	if (axs->axs_cstate != AX_CSTATE_OPEN)
>  		agentx_log_ax_fatalx(ax, "%s: unexpected session close",
>  		    __func__);
>  #endif
> -	if ((packetid = ax_close(ax->ax_ax, axs->axs_id, reason)) == 0) {
> +	axr = agentx_request(ax, agentx_session_close_finalize, axs);
> +	if (axr == NULL)
> +		return -1;
> +	if (ax_close(ax->ax_ax, axs->axs_id, axr->axr_packetid, reason) == -1) {
>  		agentx_log_axs_warn(axs, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_CLOSE));
>  		agentx_reset(ax);
> @@ -528,8 +533,8 @@ agentx_session_close(struct agentx_session *axs,
>  	    ax_closereason2string(reason));
>  
>  	axs->axs_cstate = AX_CSTATE_WAITCLOSE;
> -	return agentx_request(ax, packetid, agentx_session_close_finalize,
> -	    axs);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -920,7 +925,7 @@ agentx_agentcaps_start(struct agentx_agentcaps *axa)
>  	struct agentx_context *axc = axa->axa_axc;
>  	struct agentx_session *axs = axc->axc_axs;
>  	struct agentx *ax = axs->axs_ax;
> -	uint32_t packetid;
> +	struct agentx_request *axr;
>  
>  #ifdef AX_DEBUG
>  	if (axc->axc_cstate != AX_CSTATE_OPEN ||
> @@ -930,9 +935,10 @@ agentx_agentcaps_start(struct agentx_agentcaps *axa)
>  		    "%s: unexpected region registration", __func__);
>  #endif
>  
> -	packetid = ax_addagentcaps(ax->ax_ax, axs->axs_id,
> -	    AGENTX_CONTEXT_CTX(axc), &(axa->axa_oid), &(axa->axa_descr));
> -	if (packetid == 0) {
> +	if ((axr = agentx_request(ax, agentx_agentcaps_finalize, axa)) == NULL)
> +		return -1;
> +	if (ax_addagentcaps(ax->ax_ax, axs->axs_id, axr->axr_packetid,
> +	    AGENTX_CONTEXT_CTX(axc), &(axa->axa_oid), &(axa->axa_descr)) == -1) {
>  		agentx_log_axc_warn(axc, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_ADDAGENTCAPS));
>  		agentx_reset(ax);
> @@ -941,8 +947,8 @@ agentx_agentcaps_start(struct agentx_agentcaps *axa)
>  	agentx_log_axc_info(axc, "agentcaps %s: opening",
>  	    ax_oid2string(&(axa->axa_oid)));
>  	axa->axa_cstate = AX_CSTATE_WAITOPEN;
> -	return agentx_request(ax, packetid, agentx_agentcaps_finalize,
> -	    axa);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -983,7 +989,7 @@ agentx_agentcaps_close(struct agentx_agentcaps *axa)
>  	struct agentx_context *axc = axa->axa_axc;
>  	struct agentx_session *axs = axc->axc_axs;
>  	struct agentx *ax = axs->axs_ax;
> -	uint32_t packetid;
> +	struct agentx_request *axr;
>  
>  #ifdef AX_DEBUG
>  	if (axa->axa_cstate != AX_CSTATE_OPEN)
> @@ -995,9 +1001,11 @@ agentx_agentcaps_close(struct agentx_agentcaps *axa)
>  	if (axs->axs_cstate == AX_CSTATE_WAITCLOSE)
>  		return 0;
>  
> -	packetid = ax_removeagentcaps(ax->ax_ax, axs->axs_id,
> -	    AGENTX_CONTEXT_CTX(axc), &(axa->axa_oid));
> -	if (packetid == 0) {
> +	axr = agentx_request(ax, agentx_agentcaps_close_finalize, axa);
> +	if (axr == NULL)
> +		return -1;
> +	if (ax_removeagentcaps(ax->ax_ax, axs->axs_id, axr->axr_packetid,
> +	    AGENTX_CONTEXT_CTX(axc), &(axa->axa_oid)) == -1) {
>  		agentx_log_axc_warn(axc, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_REMOVEAGENTCAPS));
>  		agentx_reset(ax);
> @@ -1005,8 +1013,8 @@ agentx_agentcaps_close(struct agentx_agentcaps *axa)
>  	}
>  	agentx_log_axc_info(axc, "agentcaps %s: closing",
>  	    ax_oid2string(&(axa->axa_oid)));
> -	return agentx_request(ax, packetid,
> -	    agentx_agentcaps_close_finalize, axa);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -1181,7 +1189,7 @@ agentx_region_start(struct agentx_region *axr)
>  	struct agentx_context *axc = axr->axr_axc;
>  	struct agentx_session *axs = axc->axc_axs;
>  	struct agentx *ax = axs->axs_ax;
> -	uint32_t packetid;
> +	struct agentx_request *axreq;
>  
>  #ifdef AX_DEBUG
>  	if (axc->axc_cstate != AX_CSTATE_OPEN ||
> @@ -1191,10 +1199,11 @@ agentx_region_start(struct agentx_region *axr)
>  		    "%s: unexpected region registration", __func__);
>  #endif
>  
> -	packetid = ax_register(ax->ax_ax, 0, axs->axs_id,
> +	if ((axreq = agentx_request(ax, agentx_region_finalize, axr)) == NULL)
> +		return -1;
> +	if (ax_register(ax->ax_ax, 0, axs->axs_id, axreq->axr_packetid,
>  	    AGENTX_CONTEXT_CTX(axc), axr->axr_timeout, axr->axr_priority,
> -	    0, &(axr->axr_oid), 0);
> -	if (packetid == 0) {
> +	    0, &(axr->axr_oid), 0) == -1) {
>  		agentx_log_axc_warn(axc, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_REGISTER));
>  		agentx_reset(ax);
> @@ -1203,7 +1212,8 @@ agentx_region_start(struct agentx_region *axr)
>  	agentx_log_axc_info(axc, "region %s: opening",
>  	    ax_oid2string(&(axr->axr_oid)));
>  	axr->axr_cstate = AX_CSTATE_WAITOPEN;
> -	return agentx_request(ax, packetid, agentx_region_finalize, axr);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -1267,7 +1277,7 @@ agentx_region_close(struct agentx_region *axr)
>  	struct agentx_context *axc = axr->axr_axc;
>  	struct agentx_session *axs = axc->axc_axs;
>  	struct agentx *ax = axs->axs_ax;
> -	uint32_t packetid;
> +	struct agentx_request *axreq;
>  
>  #ifdef AX_DEBUG
>  	if (axr->axr_cstate != AX_CSTATE_OPEN)
> @@ -1279,10 +1289,12 @@ agentx_region_close(struct agentx_region *axr)
>  	if (axs->axs_cstate == AX_CSTATE_WAITCLOSE)
>  		return 0;
>  
> -	packetid = ax_unregister(ax->ax_ax, axs->axs_id,
> +	axreq = agentx_request(ax, agentx_region_close_finalize, axr);
> +	if (axreq == NULL)
> +		return -1;
> +	if (ax_unregister(ax->ax_ax, axs->axs_id, axreq->axr_packetid,
>  	    AGENTX_CONTEXT_CTX(axc), axr->axr_priority, 0, &(axr->axr_oid),
> -	    0);
> -	if (packetid == 0) {
> +	    0) == -1) {
>  		agentx_log_axc_warn(axc, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_UNREGISTER));
>  		agentx_reset(ax);
> @@ -1290,8 +1302,8 @@ agentx_region_close(struct agentx_region *axr)
>  	}
>  	agentx_log_axc_info(axc, "region %s: closing",
>  	    ax_oid2string(&(axr->axr_oid)));
> -	return agentx_request(ax, packetid, agentx_region_close_finalize,
> -	    axr);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -1683,7 +1695,7 @@ agentx_index_start(struct agentx_index *axi)
>  	struct agentx_context *axc = axr->axr_axc;
>  	struct agentx_session *axs = axc->axc_axs;
>  	struct agentx *ax = axs->axs_ax;
> -	uint32_t packetid;
> +	struct agentx_request *axreq;
>  	int flags = 0;
>  
>  #ifdef AX_DEBUG
> @@ -1705,10 +1717,11 @@ agentx_index_start(struct agentx_index *axi)
>  		return 0;
>  	}
>  
> +	if ((axreq = agentx_request(ax, agentx_index_finalize, axi)) == NULL)
> +		return -1;
>  	/* We might be able to bundle, but if we fail we'd have to reorganise */
> -	packetid = ax_indexallocate(ax->ax_ax, flags, axs->axs_id,
> -	    AGENTX_CONTEXT_CTX(axc), &(axi->axi_vb), 1);
> -	if (packetid == 0) {
> +	if (ax_indexallocate(ax->ax_ax, flags, axs->axs_id, axreq->axr_packetid,
> +	    AGENTX_CONTEXT_CTX(axc), &(axi->axi_vb), 1) == -1) {
>  		agentx_log_axc_warn(axc, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_INDEXDEALLOCATE));
>  		agentx_reset(ax);
> @@ -1724,8 +1737,8 @@ agentx_index_start(struct agentx_index *axi)
>  	else if (axi->axi_type == AXI_TYPE_NEW)
>  		agentx_log_axc_info(axc, "index %s: allocating new index",
>  		    ax_oid2string(&(axi->axi_vb.avb_oid)));
> -
> -	return agentx_request(ax, packetid, agentx_index_finalize, axi);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -1883,7 +1896,7 @@ agentx_index_close(struct agentx_index *axi)
>  	struct agentx_context *axc = axr->axr_axc;
>  	struct agentx_session *axs = axc->axc_axs;
>  	struct agentx *ax = axs->axs_ax;
> -	uint32_t packetid;
> +	struct agentx_request *axreq;
>  
>  #ifdef AX_DEBUG
>  	if (axi->axi_cstate != AX_CSTATE_OPEN)
> @@ -1895,10 +1908,12 @@ agentx_index_close(struct agentx_index *axi)
>  	if (axs->axs_cstate == AX_CSTATE_WAITCLOSE)
>  		return 0;
>  
> +	axreq = agentx_request(ax, agentx_index_close_finalize, axi);
> +	if (axreq == NULL)
> +		return -1;
>  	/* We might be able to bundle, but if we fail we'd have to reorganise */
> -	packetid = ax_indexdeallocate(ax->ax_ax, axs->axs_id,
> -	    AGENTX_CONTEXT_CTX(axc), &(axi->axi_vb), 1);
> -	if (packetid == 0) {
> +	if (ax_indexdeallocate(ax->ax_ax, axs->axs_id, axreq->axr_packetid,
> +	    AGENTX_CONTEXT_CTX(axc), &(axi->axi_vb), 1) == -1) {
>  		agentx_log_axc_warn(axc, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_INDEXDEALLOCATE));
>  		agentx_reset(ax);
> @@ -1906,8 +1921,8 @@ agentx_index_close(struct agentx_index *axi)
>  	}
>  	agentx_log_axc_info(axc, "index %s: deallocating",
>  	    ax_oid2string(&(axi->axi_vb.avb_oid)));
> -	return agentx_request(ax, packetid, agentx_index_close_finalize,
> -	    axi);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -2156,7 +2171,7 @@ agentx_object_start(struct agentx_object *axo)
>  	char oids[1024];
>  	size_t i;
>  	int needregister = 0;
> -	uint32_t packetid;
> +	struct agentx_request *axreq;
>  	uint8_t flags = AX_PDU_FLAG_INSTANCE_REGISTRATION;
>  
>  #ifdef AX_DEBUG
> @@ -2196,10 +2211,11 @@ agentx_object_start(struct agentx_object *axo)
>  		oid.aoi_id[oid.aoi_idlen++] =
>  		    axo->axo_index[i]->axi_vb.avb_data.avb_int32;
>  	}
> -	packetid = ax_register(ax->ax_ax, flags, axs->axs_id,
> +	if ((axreq = agentx_request(ax, agentx_object_finalize, axo)) == NULL)
> +		return -1;
> +	if (ax_register(ax->ax_ax, flags, axs->axs_id, axreq->axr_packetid,
>  	    AGENTX_CONTEXT_CTX(axc), axo->axo_timeout,
> -	    AX_PRIORITY_DEFAULT, 0, &oid, 0);
> -	if (packetid == 0) {
> +	    AX_PRIORITY_DEFAULT, 0, &oid, 0) == -1) {
>  		agentx_log_axc_warn(axc, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_REGISTER));
>  		agentx_reset(ax);
> @@ -2209,7 +2225,8 @@ agentx_object_start(struct agentx_object *axo)
>  	agentx_log_axc_info(axc, "object %s (%s %s): opening",
>  	    oids, flags ? "instance" : "region", ax_oid2string(&(oid)));
>  	axo->axo_cstate = AX_CSTATE_WAITOPEN;
> -	return agentx_request(ax, packetid, agentx_object_finalize, axo);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -2312,7 +2329,7 @@ agentx_object_close(struct agentx_object *axo)
>  	char oids[1024];
>  	size_t i;
>  	int needclose = 0;
> -	uint32_t packetid;
> +	struct agentx_request *axr;
>  	uint8_t flags = 1;
>  
>  #ifdef AX_DEBUG
> @@ -2353,9 +2370,11 @@ agentx_object_close(struct agentx_object *axo)
>  		oid.aoi_id[oid.aoi_idlen++] =
>  		    axo->axo_index[i]->axi_vb.avb_data.avb_int32;
>  	}
> -	packetid = ax_unregister(ax->ax_ax, axs->axs_id,
> -	    AGENTX_CONTEXT_CTX(axc), AX_PRIORITY_DEFAULT, 0, &oid, 0);
> -	if (packetid == 0) {
> +	axr = agentx_request(ax, agentx_object_close_finalize, axo);
> +	if (axr == NULL)
> +		return -1;
> +	if (ax_unregister(ax->ax_ax, axs->axs_id, axr->axr_packetid,
> +	    AGENTX_CONTEXT_CTX(axc), AX_PRIORITY_DEFAULT, 0, &oid, 0) == -1) {
>  		agentx_log_axc_warn(axc, "couldn't generate %s",
>  		    ax_pdutype2string(AX_PDU_TYPE_UNREGISTER));
>  		agentx_reset(ax);
> @@ -2364,8 +2383,8 @@ agentx_object_close(struct agentx_object *axo)
>  	strlcpy(oids, ax_oid2string(&(axo->axo_oid)), sizeof(oids));
>  	agentx_log_axc_info(axc, "object %s (%s %s): closing",
>  	    oids, flags ? "instance" : "region", ax_oid2string(&(oid)));
> -	return agentx_request(ax, packetid, agentx_object_close_finalize,
> -	    axo);
> +	agentx_wantwrite(ax, ax->ax_fd);
> +	return 0;
>  }
>  
>  static int
> @@ -3906,40 +3925,25 @@ agentx_varbind_set_index_ipaddress(struct agentx_varbi
>  #endif
>  }
>  
> -static int
> -agentx_request(struct agentx *ax, uint32_t packetid,
> -    int (*cb)(struct ax_pdu *, void *), void *cookie)
> +static struct agentx_request *
> +agentx_request(struct agentx *ax, int (*cb)(struct ax_pdu *, void *),
> +    void *cookie)
>  {
>  	struct agentx_request *axr;
>  
> -#ifdef AX_DEBUG
> -	if (ax->ax_ax->ax_wblen == 0)
> -		agentx_log_ax_fatalx(ax, "%s: no data to be written",
> -		    __func__);
> -#endif
> -
>  	if ((axr = calloc(1, sizeof(*axr))) == NULL) {
>  		agentx_log_ax_warn(ax, "couldn't create request context");
>  		agentx_reset(ax);
> -		return -1;
> +		return NULL;
>  	}
>  
> -	axr->axr_packetid = packetid;
>  	axr->axr_cb = cb;
>  	axr->axr_cookie = cookie;
> -	if (RB_INSERT(ax_requests, &(ax->ax_requests), axr) != NULL) {
> -#ifdef AX_DEBUG
> -		agentx_log_ax_fatalx(ax, "%s: duplicate packetid", __func__);
> -#else
> -		agentx_log_ax_warnx(ax, "%s: duplicate packetid", __func__);
> -		free(axr);
> -		agentx_reset(ax);
> -		return -1;
> -#endif
> -	}
> +	do {
> +		axr->axr_packetid = arc4random();
> +	} while (RB_INSERT(ax_requests, &(ax->ax_requests), axr) != NULL);
>  
> -	agentx_wantwrite(ax, ax->ax_fd);
> -	return 0;
> +	return axr;
>  }
>  
>  static int
> @@ -4023,7 +4027,7 @@ agentx_read(struct agentx *ax)
>  			break;
>  	}
>  	if (axs == NULL) {
> -		agentx_log_ax_warnx(ax, "received unexpected session: %d",
> +		agentx_log_ax_warnx(ax, "received unexpected session: %u",
>  		    pdu->ap_header.aph_sessionid);
>  		ax_pdu_free(pdu);
>  		agentx_reset(ax);
> blob - 4377a82d6803d1d36c11f438f9ffbbadb944b91f
> blob + 1fff36032df3331f50f005473f1cd450ff9c20a1
> --- ax.c
> +++ ax.c
> @@ -37,8 +37,7 @@ static int ax_pdu_need(struct ax *, size_t);
>  static int ax_pdu_header(struct ax *,
>      enum ax_pdu_type, uint8_t, uint32_t, uint32_t, uint32_t,
>      struct ax_ostring *);
> -static uint32_t ax_packetid(struct ax *);
> -static uint32_t ax_pdu_queue(struct ax *);
> +static int ax_pdu_queue(struct ax *);
>  static int ax_pdu_add_uint16(struct ax *, uint16_t);
>  static int ax_pdu_add_uint32(struct ax *, uint32_t);
>  static int ax_pdu_add_uint64(struct ax *, uint64_t);
> @@ -89,7 +88,6 @@ ax_free(struct ax *ax)
>  	close(ax->ax_fd);
>  	free(ax->ax_rbuf);
>  	free(ax->ax_wbuf);
> -	free(ax->ax_packetids);
>  	free(ax);
>  }
>  
> @@ -103,11 +101,10 @@ ax_recv(struct ax *ax)
>  	struct ax_pdu_searchrangelist *srl = NULL;
>  	struct ax_pdu_varbindlist *vbl;
>  	struct ax_searchrange *sr;
> -	size_t rbsize, packetidx = 0, i, rawlen;
> +	size_t rbsize, rawlen;
>  	ssize_t nread;
>  	uint8_t *u8;
>  	uint8_t *rbuf;
> -	int found;
>  
>  	/* Only read a single packet at a time to make sure libevent triggers */
>  	if (ax->ax_rblen < AX_PDU_HEADER) {
> @@ -267,24 +264,6 @@ ax_recv(struct ax *ax)
>  		}
>  		break;
>  	case AX_PDU_TYPE_RESPONSE:
> -		if (ax->ax_packetids != NULL) {
> -			found = 0;
> -			for (i = 0; ax->ax_packetids[i] != 0; i++) {
> -				if (ax->ax_packetids[i] ==
> -				    pdu->ap_header.aph_packetid) {
> -					packetidx = i;
> -					found = 1;
> -				}
> -			}
> -			if (found) {
> -				ax->ax_packetids[packetidx] =
> -				    ax->ax_packetids[i - 1];
> -				ax->ax_packetids[i - 1] = 0;
> -			} else {
> -				errno = EPROTO;
> -				goto fail;
> -			}
> -		}
>  		if (rawlen < 8) {
>  			errno = EPROTO;
>  			goto fail;
> @@ -397,35 +376,35 @@ ax_send(struct ax *ax)
>  	return ax->ax_wblen;
>  }
>  
> -uint32_t
> -ax_open(struct ax *ax, uint8_t timeout, struct ax_oid *oid,
> +int
> +ax_open(struct ax *ax, uint8_t timeout, uint32_t packetid, struct ax_oid *oid,
>      struct ax_ostring *descr)
>  {
> -	if (ax_pdu_header(ax, AX_PDU_TYPE_OPEN, 0, 0, 0, 0,
> +	if (ax_pdu_header(ax, AX_PDU_TYPE_OPEN, 0, 0, 0, packetid,
>  	    NULL) == -1)
> -		return 0;
> +		return -1;
>  	ax_pdu_need(ax, 4);
>  	ax->ax_wbuf[ax->ax_wbtlen++] = timeout;
>  	memset(&(ax->ax_wbuf[ax->ax_wbtlen]), 0, 3);
>  	ax->ax_wbtlen += 3;
>  	if (ax_pdu_add_oid(ax, oid, 0) == -1)
> -		return 0;
> +		return -1;
>  	if (ax_pdu_add_str(ax, descr) == -1)
> -		return 0;
> +		return -1;
>  
>  	return ax_pdu_queue(ax);
>  }
>  
> -uint32_t
> -ax_close(struct ax *ax, uint32_t sessionid,
> +int
> +ax_close(struct ax *ax, uint32_t sessionid, uint32_t packetid,
>      enum ax_close_reason reason)
>  {
> -	if (ax_pdu_header(ax, AX_PDU_TYPE_CLOSE, 0, sessionid, 0, 0,
> +	if (ax_pdu_header(ax, AX_PDU_TYPE_CLOSE, 0, sessionid, 0, packetid,
>  	    NULL) == -1)
> -		return 0;
> +		return -1;
>  
>  	if (ax_pdu_need(ax, 4) == -1)
> -		return 0;
> +		return -1;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = (uint8_t)reason;
>  	memset(&(ax->ax_wbuf[ax->ax_wbtlen]), 0, 3);
>  	ax->ax_wbtlen += 3;
> @@ -433,119 +412,120 @@ ax_close(struct ax *ax, uint32_t sessionid,
>  	return ax_pdu_queue(ax);
>  }
>  
> -uint32_t
> +int
>  ax_indexallocate(struct ax *ax, uint8_t flags, uint32_t sessionid,
> -    struct ax_ostring *context, struct ax_varbind *vblist, size_t nvb)
> +    uint32_t packetid, struct ax_ostring *context, struct ax_varbind *vblist,
> +    size_t nvb)
>  {
>  	if (flags & ~(AX_PDU_FLAG_NEW_INDEX | AX_PDU_FLAG_ANY_INDEX)) {
>  		errno = EINVAL;
> -		return 0;
> +		return -1;
>  	}
>  
>  	if (ax_pdu_header(ax, AX_PDU_TYPE_INDEXALLOCATE, flags,
> -	    sessionid, 0, 0, context) == -1)
> -		return 0;
> +	    sessionid, 0, packetid, context) == -1)
> +		return -1;
>  
>  	if (ax_pdu_add_varbindlist(ax, vblist, nvb) == -1)
> -		return 0;
> +		return -1;
>  
>  	return ax_pdu_queue(ax);
>  }
>  
> -uint32_t
> -ax_indexdeallocate(struct ax *ax, uint32_t sessionid,
> +int
> +ax_indexdeallocate(struct ax *ax, uint32_t sessionid, uint32_t packetid,
>      struct ax_ostring *context, struct ax_varbind *vblist, size_t nvb)
>  {
>  	if (ax_pdu_header(ax, AX_PDU_TYPE_INDEXDEALLOCATE, 0,
> -	    sessionid, 0, 0, context) == -1)
> -		return 0;
> +	    sessionid, 0, packetid, context) == -1)
> +		return -1;
>  
>  	if (ax_pdu_add_varbindlist(ax, vblist, nvb) == -1)
> -		return 0;
> +		return -1;
>  
>  	return ax_pdu_queue(ax);
>  }
>  
> -uint32_t
> -ax_addagentcaps(struct ax *ax, uint32_t sessionid,
> +int
> +ax_addagentcaps(struct ax *ax, uint32_t sessionid, uint32_t packetid,
>      struct ax_ostring *context, struct ax_oid *id,
>      struct ax_ostring *descr)
>  {
>  	if (ax_pdu_header(ax, AX_PDU_TYPE_ADDAGENTCAPS, 0,
> -	    sessionid, 0, 0, context) == -1)
> -		return 0;
> +	    sessionid, 0, packetid, context) == -1)
> +		return -1;
>  	if (ax_pdu_add_oid(ax, id, 0) == -1)
> -		return 0;
> +		return -1;
>  	if (ax_pdu_add_str(ax, descr) == -1)
> -		return 0;
> +		return -1;
>  
>  	return ax_pdu_queue(ax);
>  }
>  
> -uint32_t
> -ax_removeagentcaps(struct ax *ax, uint32_t sessionid,
> +int
> +ax_removeagentcaps(struct ax *ax, uint32_t sessionid, uint32_t packetid,
>      struct ax_ostring *context, struct ax_oid *id)
>  {
>  	if (ax_pdu_header(ax, AX_PDU_TYPE_REMOVEAGENTCAPS, 0,
> -	    sessionid, 0, 0, context) == -1)
> -		return 0;
> +	    sessionid, 0, packetid, context) == -1)
> +		return -1;
>  	if (ax_pdu_add_oid(ax, id, 0) == -1)
> -		return 0;
> +		return -1;
>  
>  	return ax_pdu_queue(ax);
>  
>  }
>  
> -uint32_t
> -ax_register(struct ax *ax, uint8_t flags, uint32_t sessionid,
> +int
> +ax_register(struct ax *ax, uint8_t flags, uint32_t sessionid, uint32_t packetid,
>      struct ax_ostring *context, uint8_t timeout, uint8_t priority,
>      uint8_t range_subid, struct ax_oid *subtree, uint32_t upperbound)
>  {
>  	if (flags & ~(AX_PDU_FLAG_INSTANCE_REGISTRATION)) {
>  		errno = EINVAL;
> -		return 0;
> +		return -1;
>  	}
>  
>  	if (ax_pdu_header(ax, AX_PDU_TYPE_REGISTER, flags,
> -	    sessionid, 0, 0, context) == -1)
> -		return 0;
> +	    sessionid, 0, packetid, context) == -1)
> +		return -1;
>  
>  	if (ax_pdu_need(ax, 4) == -1)
> -		return 0;
> +		return -1;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = timeout;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = priority;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = range_subid;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = 0;
>  	if (ax_pdu_add_oid(ax, subtree, 0) == -1)
> -		return 0;
> +		return -1;
>  	if (range_subid != 0) {
>  		if (ax_pdu_add_uint32(ax, upperbound) == -1)
> -			return 0;
> +			return -1;
>  	}
>  
>  	return ax_pdu_queue(ax);
>  }
>  
> -uint32_t
> -ax_unregister(struct ax *ax, uint32_t sessionid,
> +int
> +ax_unregister(struct ax *ax, uint32_t sessionid, uint32_t packetid,
>      struct ax_ostring *context, uint8_t priority, uint8_t range_subid,
>      struct ax_oid *subtree, uint32_t upperbound)
>  {
>  	if (ax_pdu_header(ax, AX_PDU_TYPE_UNREGISTER, 0,
> -	    sessionid, 0, 0, context) == -1)
> -		return 0;
> +	    sessionid, 0, packetid, context) == -1)
> +		return -1;
>  
>  	if (ax_pdu_need(ax, 4) == -1)
> -		return 0;
> +		return -1;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = 0;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = priority;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = range_subid;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = 0;
>  	if (ax_pdu_add_oid(ax, subtree, 0) == -1)
> -		return 0;
> +		return -1;
>  	if (range_subid != 0) {
>  		if (ax_pdu_add_uint32(ax, upperbound) == -1)
> -			return 0;
> +			return -1;
>  	}
>  
>  	return ax_pdu_queue(ax);
> @@ -567,9 +547,7 @@ ax_response(struct ax *ax, uint32_t sessionid, uint32_
>  
>  	if (ax_pdu_add_varbindlist(ax, vblist, nvb) == -1)
>  		return -1;
> -	if (ax_pdu_queue(ax) == 0)
> -		return -1;
> -	return 0;
> +	return ax_pdu_queue(ax);
>  }
>  
>  void
> @@ -954,23 +932,22 @@ ax_oid_add(struct ax_oid *oid, uint32_t value)
>  	return 0;
>  }
>  
> -static uint32_t
> +static int
>  ax_pdu_queue(struct ax *ax)
>  {
>  	struct ax_pdu_header header;
> -	uint32_t packetid, plength;
> +	uint32_t plength;
>  	size_t wbtlen = ax->ax_wbtlen;
>  
>  	header.aph_flags = ax->ax_byteorder == AX_BYTE_ORDER_BE ?
>  	    AX_PDU_FLAG_NETWORK_BYTE_ORDER : 0;
> -	packetid = ax_pdutoh32(&header, &(ax->ax_wbuf[ax->ax_wblen + 12]));
>  	plength = (ax->ax_wbtlen - ax->ax_wblen) - AX_PDU_HEADER;
>  	ax->ax_wbtlen = ax->ax_wblen + 16;
>  	(void)ax_pdu_add_uint32(ax, plength);
>  
>  	ax->ax_wblen = ax->ax_wbtlen = wbtlen;
>  
> -	return packetid;
> +	return 0;
>  }
>  
>  static int
> @@ -993,8 +970,6 @@ ax_pdu_header(struct ax *ax, enum ax_pdu_type type, ui
>  		flags |= AX_PDU_FLAG_NETWORK_BYTE_ORDER;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = flags;
>  	ax->ax_wbuf[ax->ax_wbtlen++] = 0;
> -	if (packetid == 0)
> -		packetid = ax_packetid(ax);
>  	if (ax_pdu_add_uint32(ax, sessionid) == -1 ||
>  	    ax_pdu_add_uint32(ax, transactionid) == -1 ||
>  	    ax_pdu_add_uint32(ax, packetid) == -1 ||
> @@ -1009,40 +984,6 @@ ax_pdu_header(struct ax *ax, enum ax_pdu_type type, ui
>  	return 0;
>  }
>  
> -static uint32_t
> -ax_packetid(struct ax *ax)
> -{
> -	uint32_t packetid, *packetids;
> -	size_t npackets = 0, i;
> -	int found;
> -
> -	if (ax->ax_packetids != NULL) {
> -		for (npackets = 0; ax->ax_packetids[npackets] != 0; npackets++)
> -			continue;
> -	}
> -	if (ax->ax_packetidsize == 0 || npackets == ax->ax_packetidsize - 1) {
> -		packetids = recallocarray(ax->ax_packetids, ax->ax_packetidsize,
> -		    ax->ax_packetidsize + 25, sizeof(*packetids));
> -		if (packetids == NULL)
> -			return 0;
> -		ax->ax_packetidsize += 25;
> -		ax->ax_packetids = packetids;
> -	}
> -	do {
> -		found = 0;
> -		packetid = arc4random();
> -		for (i = 0; ax->ax_packetids[i] != 0; i++) {
> -			if (ax->ax_packetids[i] == packetid) {
> -				found = 1;
> -				break;
> -			}
> -		}
> -	} while (packetid == 0 || found);
> -	ax->ax_packetids[npackets] = packetid;
> -
> -	return packetid;
> -}
> -
>  static int
>  ax_pdu_add_uint16(struct ax *ax, uint16_t value)
>  {
> blob - f4de378f238abece8162483a3f3c9e731666e23e
> blob + 0f9aeda495ab58dacca43e89b5c098fef806fb30
> --- ax.h
> +++ ax.h
> @@ -122,8 +122,6 @@ struct ax {
>  	size_t ax_wblen;
>  	size_t ax_wbtlen;
>  	size_t ax_wbsize;
> -	uint32_t *ax_packetids;
> -	size_t ax_packetidsize;
>  };
>  
>  #ifndef AX_PRIMITIVE
> @@ -203,21 +201,21 @@ struct ax *ax_new(int);
>  void ax_free(struct ax *);
>  struct ax_pdu *ax_recv(struct ax *);
>  ssize_t ax_send(struct ax *);
> -uint32_t ax_open(struct ax *, uint8_t, struct ax_oid *,
> +int ax_open(struct ax *, uint8_t, uint32_t, struct ax_oid *,
>      struct ax_ostring *);
> -uint32_t ax_close(struct ax *, uint32_t, enum ax_close_reason);
> -uint32_t ax_indexallocate(struct ax *, uint8_t, uint32_t,
> +int ax_close(struct ax *, uint32_t, uint32_t, enum ax_close_reason);
> +int ax_indexallocate(struct ax *, uint8_t, uint32_t, uint32_t,
>      struct ax_ostring *, struct ax_varbind *, size_t);
> -uint32_t ax_indexdeallocate(struct ax *, uint32_t,
> +int ax_indexdeallocate(struct ax *, uint32_t, uint32_t,
>      struct ax_ostring *, struct ax_varbind *, size_t);
> -uint32_t ax_addagentcaps(struct ax *, uint32_t, struct ax_ostring *,
> +int ax_addagentcaps(struct ax *, uint32_t, uint32_t, struct ax_ostring *,
>      struct ax_oid *, struct ax_ostring *);
> -uint32_t ax_removeagentcaps(struct ax *, uint32_t,
> +int ax_removeagentcaps(struct ax *, uint32_t, uint32_t,
>      struct ax_ostring *, struct ax_oid *);
> -uint32_t ax_register(struct ax *, uint8_t, uint32_t,
> +int ax_register(struct ax *, uint8_t, uint32_t, uint32_t,
>      struct ax_ostring *, uint8_t, uint8_t, uint8_t, struct ax_oid *,
>      uint32_t);
> -uint32_t ax_unregister(struct ax *, uint32_t, struct ax_ostring *,
> +int ax_unregister(struct ax *, uint32_t, uint32_t, struct ax_ostring *,
>      uint8_t, uint8_t, struct ax_oid *, uint32_t);
>  int ax_response(struct ax *, uint32_t, uint32_t, uint32_t,
>      uint32_t, uint16_t, uint16_t, struct ax_varbind *, size_t);
>