From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202412 header.b=Fi61oBdA; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 6F2015A0621 for ; Fri, 20 Dec 2024 09:58:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202412; t=1734685098; bh=ScUbkwtK9y24IA7u84A2DCef+1+IE4J6DmdH7CpQqdw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Fi61oBdAudpePg7RvkXntINPAXJor09dO9OFMNAKbSRTxq75crac8jbEKWtqBbHwO XrJ9tkjl5qIjgCOF/cEHU4Luzif9wVz+l+LcS+/pVpVVCBodF3aCgZ2KxL9SVYw2OT sTGVerxtHX7JgF52Q/lkVhEaEFhZ8rx7KPIItLOR5f6JzVjnc/woaWN45/BdMHLD3z m855Hq2jpJwJyBEUqtkPoNSnhTTsi69BKQwOhL5VoJ6IVINsyFWgPFpIrUUsPiQ14o odA5vrfrn7EPcxqtgdX8ocq2f7TjycgZJVbO4/UutpXwJj83hKsD6+411yxEBweP5h 9iizl7x5CjSyA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4YF1Xp1QJLz4xf7; Fri, 20 Dec 2024 19:58:18 +1100 (AEDT) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH v2 07/12] packet: Remove unhelpful packet_get_try() macro Date: Fri, 20 Dec 2024 19:35:30 +1100 Message-ID: <20241220083535.1372523-8-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20241220083535.1372523-1-david@gibson.dropbear.id.au> References: <20241220083535.1372523-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: XYAXMFS2BK3LLIBN7J7JT3DXH2T7QZCH X-Message-ID-Hash: XYAXMFS2BK3LLIBN7J7JT3DXH2T7QZCH X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Two places in the code use the packet_get_try() variant on packet_get(). The difference is that packet_get_try() passes a NULL 'func' to packet_get_do(), which suppresses log messages. The places we use this are ones where we expect to sometimes have a failure retreiving the packet range, even in normal cases. So, suppressing the log messages seems like it makes sense, except: 1) It suppresses log messages on all errors. We (somewhat) expect to hit the case where the requested range is not within the received packet. However, it also suppresses message in cases where the requested packet index doesn't exist, the requested range has a nonsensical length or doesn't lie in even the right vague part of memory. None of those latter cases are expected, and the message would be helpful if we ever actually hit them. 2) The suppressed messages aren't actually that disruptive. For the case in ip.c, we'd log only if we run out of IPv6 packet before reaching a (non-option) L4 header. That shouldn't be the case in normal operation so getting a message (at trave level) is not unreasonable. For the case in dhcpv6.c we do suppress a message every time we look for but don't find a specific DHCPv6 option. That can happen in perfectly ok cases, but, again these are trace() level and DHCPv6 transactions aren't that common. Suppressing the message for this case doesn't seem worth the drawbacks of (1). Signed-off-by: David Gibson --- dhcpv6.c | 2 +- ip.c | 2 +- packet.c | 18 +++++------------- packet.h | 3 --- 4 files changed, 7 insertions(+), 18 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index 0523bba8..c0d05131 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -271,7 +271,7 @@ static struct opt_hdr *dhcpv6_opt(const struct pool *p, size_t *offset, if (!*offset) *offset = sizeof(struct udphdr) + sizeof(struct msg_hdr); - while ((o = packet_get_try(p, 0, *offset, sizeof(*o), &left))) { + while ((o = packet_get(p, 0, *offset, sizeof(*o), &left))) { unsigned int opt_len = ntohs(o->l) + sizeof(*o); if (ntohs(o->l) > left) diff --git a/ip.c b/ip.c index 2cc7f654..e391f81b 100644 --- a/ip.c +++ b/ip.c @@ -51,7 +51,7 @@ char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto, if (!IPV6_NH_OPT(nh)) goto found; - while ((o = packet_get_try(p, idx, offset, sizeof(*o), dlen))) { + while ((o = packet_get(p, idx, offset, sizeof(*o), dlen))) { nh = o->nexthdr; hdrlen = (o->hdrlen + 1) * 8; diff --git a/packet.c b/packet.c index bcac0375..c921aa15 100644 --- a/packet.c +++ b/packet.c @@ -112,27 +112,19 @@ void *packet_get_do(const struct pool *p, size_t idx, size_t offset, char *ptr; if (idx >= p->size || idx >= p->count) { - if (func) { - trace("packet %zu from pool size: %zu, count: %zu, " - "%s:%i", idx, p->size, p->count, func, line); - } + trace("packet %zu from pool size: %zu, count: %zu, %s:%i", + idx, p->size, p->count, func, line); return NULL; } if (len > PACKET_MAX_LEN) { - if (func) { - trace("packet data length %zu, %s:%i", - len, func, line); - } + trace("packet data length %zu, %s:%i", len, func, line); return NULL; } if (len + offset > p->pkt[idx].iov_len) { - if (func) { - trace("data length %zu, offset %zu from length %zu, " - "%s:%i", len, offset, p->pkt[idx].iov_len, - func, line); - } + trace("data length %zu, offset %zu from length %zu, %s:%i", + len, offset, p->pkt[idx].iov_len, func, line); return NULL; } diff --git a/packet.h b/packet.h index 05d37695..f95cda08 100644 --- a/packet.h +++ b/packet.h @@ -45,9 +45,6 @@ void pool_flush(struct pool *p); #define packet_get(p, idx, offset, len, left) \ packet_get_do(p, idx, offset, len, left, __func__, __LINE__) -#define packet_get_try(p, idx, offset, len, left) \ - packet_get_do(p, idx, offset, len, left, NULL, 0) - #define PACKET_POOL_DECL(_name, _size, _buf) \ struct _name ## _t { \ char *buf; \ -- 2.47.1