net: l2: ethernet: arp: explicit error codes

Update `net_arp_prepare` to return a return code instead of a pointer,
so that the various results of the function can be differentiated.

Signed-off-by: Jordan Yates <jordan@embeint.com>

Author: JordanYates

subsys/net/ip/ipv4_acd.c

@@ -74,7 +74,8 @@ static struct net_pkt *ipv4_acd_prepare_arp(struct net_if *iface,
 					    struct in_addr *sender_ip,
 					    struct in_addr *target_ip)
 {
-	struct net_pkt *pkt;
+	struct net_pkt *pkt, *arp;
+	int ret;
 
 	/* We provide AF_UNSPEC to the allocator: this packet does not
 	 * need space for any IPv4 header.
@@ -88,7 +89,13 @@ static struct net_pkt *ipv4_acd_prepare_arp(struct net_if *iface,
 	net_pkt_set_family(pkt, AF_INET);
 	net_pkt_set_ipv4_acd(pkt, true);
 
-	return net_arp_prepare(pkt, target_ip, sender_ip);
+	ret = net_arp_prepare(pkt, target_ip, sender_ip, &arp);
+	if (ret == NET_ARP_PKT_REPLACED) {
+		pkt = arp;
+	} else if (ret < 0)  {
+		pkt = NULL;
+	}
+	return pkt;
 }
 
 static void ipv4_acd_send_probe(struct net_if_addr *ifaddr)

subsys/net/l2/ethernet/arp.c

@@ -352,21 +352,23 @@ static inline struct net_pkt *arp_prepare(struct net_if *iface,
 	return pkt;
 }
 
-struct net_pkt *net_arp_prepare(struct net_pkt *pkt,
-				struct in_addr *request_ip,
-				struct in_addr *current_ip)
+int net_arp_prepare(struct net_pkt *pkt,
+		    struct in_addr *request_ip,
+		    struct in_addr *current_ip,
+		    struct net_pkt **arp_pkt)
 {
 	bool is_ipv4_ll_used = false;
 	struct arp_entry *entry;
 	struct in_addr *addr;
 
 	if (!pkt || !pkt->buffer) {
-		return NULL;
+		return -EINVAL;
 	}
 
 	if (net_pkt_ipv4_acd(pkt)) {
-		return arp_prepare(net_pkt_iface(pkt), request_ip, NULL,
-				   pkt, current_ip);
+		*arp_pkt = arp_prepare(net_pkt_iface(pkt), request_ip, NULL,
+				       pkt, current_ip);
+		return *arp_pkt ? NET_ARP_PKT_REPLACED : -ENOMEM;
 	}
 
 	if (IS_ENABLED(CONFIG_NET_IPV4_AUTO)) {
@@ -391,7 +393,7 @@ struct net_pkt *net_arp_prepare(struct net_pkt *pkt,
 					net_if_get_by_iface(net_pkt_iface(pkt)),
 					net_sprint_ipv4_addr(request_ip));
 
-				return NULL;
+				return -EINVAL;
 			}
 		} else {
 			addr = request_ip;
@@ -427,7 +429,7 @@ struct net_pkt *net_arp_prepare(struct net_pkt *pkt,
 				NET_DBG("Pending ARP request for %s, queuing pkt %p",
 					net_sprint_ipv4_addr(addr), pkt);
 				k_mutex_unlock(&arp_mutex);
-				return NULL;
+				return NET_ARP_PKT_QUEUED;
 			}
 
 			entry = NULL;
@@ -452,7 +454,8 @@ struct net_pkt *net_arp_prepare(struct net_pkt *pkt,
 		}
 
 		k_mutex_unlock(&arp_mutex);
-		return req;
+		*arp_pkt = req;
+		return req ? NET_ARP_PKT_REPLACED : -ENOMEM;
 	}
 
 	k_mutex_unlock(&arp_mutex);
@@ -469,7 +472,7 @@ struct net_pkt *net_arp_prepare(struct net_pkt *pkt,
 				   sizeof(struct net_eth_addr)),
 		net_sprint_ipv4_addr(NET_IPV4_HDR(pkt)->dst));
 
-	return pkt;
+	return NET_ARP_COMPLETE;
 }
 
 static void arp_gratuitous(struct net_if *iface,

subsys/net/l2/ethernet/arp.h

@@ -7,8 +7,6 @@
 #ifndef __ARP_H
 #define __ARP_H
 
-#if defined(CONFIG_NET_ARP) && defined(CONFIG_NET_NATIVE)
-
 #include <zephyr/sys/slist.h>
 #include <zephyr/net/ethernet.h>
 
@@ -23,6 +21,15 @@ extern "C" {
  * @{
  */
 
+/** @brief Address resolution complete, destination link address injected */
+#define NET_ARP_COMPLETE     0
+/** @brief Destination link address unknown, ARP request to be sent instead */
+#define NET_ARP_PKT_REPLACED 1
+/** @brief Destination link address unknown, ARP request pending, packet queued */
+#define NET_ARP_PKT_QUEUED   2
+
+#if defined(CONFIG_NET_ARP) && defined(CONFIG_NET_NATIVE)
+
 #define NET_ARP_HDR(pkt) ((struct net_arp_hdr *)net_pkt_data(pkt))
 
 struct net_arp_hdr {
@@ -43,9 +50,23 @@ struct net_arp_hdr {
 #define NET_ARP_REQUEST 1
 #define NET_ARP_REPLY   2
 
-struct net_pkt *net_arp_prepare(struct net_pkt *pkt,
-				struct in_addr *request_ip,
-				struct in_addr *current_ip);
+/**
+ * @brief Prepare an ARP request, if required
+ *
+ * @param pkt Packet that wants to be sent
+ * @param request_ip Destination address of the packet
+ * @param current_ip Sending IP (Can be NULL)
+ * @param arp_pkt ARP packet that should be sent in place of @a pkt, if not NULL
+ *
+ * @retval NET_ARP_COMPLETE ARP information populated in @a pkt
+ * @retval NET_ARP_PKT_REPLACED ARP request to be sent existing in @a arp_pkt
+ * @retval NET_ARP_PKT_QUEUED @a pkt was queued for transmission on ARP resolution
+ * @retval <0 on failure
+ */
+int net_arp_prepare(struct net_pkt *pkt,
+			struct in_addr *request_ip,
+			struct in_addr *current_ip,
+			struct net_pkt **arp_pkt);
 enum net_verdict net_arp_input(struct net_pkt *pkt,
 			       struct net_eth_addr *src,
 			       struct net_eth_addr *dst);
@@ -72,16 +93,9 @@ void net_arp_update(struct net_if *iface, struct in_addr *src,
 		    struct net_eth_addr *hwaddr, bool gratuitous,
 		    bool force);
 
-/**
- * @}
- */
-
-#ifdef __cplusplus
-}
-#endif
 
 #else /* CONFIG_NET_ARP */
-#define net_arp_prepare(_kt, _u1, _u2) _kt
+#define net_arp_prepare(_kt, _u1, _u2, _arp) NET_ARP_COMPLETE
 #define net_arp_input(...) NET_OK
 #define net_arp_clear_cache(...)
 #define net_arp_foreach(...) 0
@@ -91,4 +105,12 @@ void net_arp_update(struct net_if *iface, struct in_addr *src,
 
 #endif /* CONFIG_NET_ARP */
 
+/**
+ * @}
+ */
+
+#ifdef __cplusplus
+}
+#endif
+
 #endif /* __ARP_H */

subsys/net/l2/ethernet/ethernet.c

@@ -508,21 +508,23 @@ static struct net_pkt *ethernet_ll_prepare_on_ipv4(struct net_if *iface,
 
 	if (IS_ENABLED(CONFIG_NET_ARP)) {
 		struct net_pkt *arp_pkt;
+		int ret;
 
-		arp_pkt = net_arp_prepare(pkt, (struct in_addr *)NET_IPV4_HDR(pkt)->dst, NULL);
-		if (!arp_pkt) {
-			return NULL;
-		}
-
-		if (pkt != arp_pkt) {
+		ret = net_arp_prepare(pkt, (struct in_addr *)NET_IPV4_HDR(pkt)->dst, NULL, &arp_pkt);
+		if (ret == NET_ARP_COMPLETE) {
+			NET_DBG("Found ARP entry, sending pkt %p to iface %d (%p)",
+				pkt, net_if_get_by_iface(iface), iface);
+		} else if (ret == NET_ARP_PKT_REPLACED) {
 			NET_DBG("Sending arp pkt %p (orig %p) to iface %d (%p)",
 				arp_pkt, pkt, net_if_get_by_iface(iface), iface);
-			net_pkt_unref(pkt);
 			return arp_pkt;
+		} else if (ret == NET_ARP_PKT_QUEUED) {
+			NET_DBG("Pending ARP request, pkt %p queued", pkt);
+			return NULL;
+		} else {
+			NET_WARN("ARP failure (%d)", ret);
+			return NULL;
 		}
-
-		NET_DBG("Found ARP entry, sending pkt %p to iface %d (%p)",
-			pkt, net_if_get_by_iface(iface), iface);
 	}
 
 	return pkt;

tests/net/arp/src/main.c

@@ -320,7 +320,7 @@ ZTEST(arp_fn_tests, test_arp)
 	struct net_if_addr *ifaddr;
 	struct net_arp_hdr *arp_hdr;
 	struct net_ipv4_hdr *ipv4;
-	int len;
+	int len, ret;
 
 	struct in_addr dst = { { { 192, 0, 2, 2 } } };
 	struct in_addr dst_far = { { { 10, 11, 12, 13 } } };
@@ -367,7 +367,9 @@ ZTEST(arp_fn_tests, test_arp)
 
 	memcpy(net_buf_add(pkt->buffer, len), app_data, len);
 
-	pkt2 = net_arp_prepare(pkt, &dst, NULL);
+	ret = net_arp_prepare(pkt, &dst, NULL, &pkt2);
+
+	zassert_equal(NET_ARP_PKT_REPLACED, ret);
 
 	/* pkt2 is the ARP packet and pkt is the IPv4 packet and it was
 	 * stored in ARP table.
@@ -453,7 +455,9 @@ ZTEST(arp_fn_tests, test_arp)
 	/* Then a case where target is not in the same subnet */
 	net_ipv4_addr_copy_raw(ipv4->dst, (uint8_t *)&dst_far);
 
-	pkt2 = net_arp_prepare(pkt, &dst_far, NULL);
+	ret = net_arp_prepare(pkt, &dst_far, NULL, &pkt2);
+
+	zassert_equal(NET_ARP_PKT_REPLACED, ret);
 
 	zassert_not_equal((void *)(pkt2), (void *)(pkt),
 		"ARP cache should not find anything");
@@ -484,7 +488,9 @@ ZTEST(arp_fn_tests, test_arp)
 	 */
 	net_pkt_ref(pkt);
 
-	pkt2 = net_arp_prepare(pkt, &dst_far, NULL);
+	ret = net_arp_prepare(pkt, &dst_far, NULL, &pkt2);
+
+	zassert_equal(NET_ARP_PKT_REPLACED, ret);
 
 	zassert_not_null(pkt2,
 		"ARP cache is not sending the request again");
@@ -501,7 +507,9 @@ ZTEST(arp_fn_tests, test_arp)
 	 */
 	net_pkt_ref(pkt);
 
-	pkt2 = net_arp_prepare(pkt, &dst_far2, NULL);
+	ret = net_arp_prepare(pkt, &dst_far2, NULL, &pkt2);
+
+	zassert_equal(NET_ARP_PKT_REPLACED, ret);
 
 	zassert_not_null(pkt2,
 		"ARP cache did not send a req");