net: Change the net_linkaddr struct to not use pointers

Previously the net_linkaddr struct had pointers to the link address.
This is error prone and difficult to handle if cloning the packet as
those pointers can point to wrong place. Mitigate this issue by
allocating the space for link address in net_linkaddr struct. This will
increase the size of the net_pkt by 4 octets for IEEE 802.15.4 where the
link address length is 8, but there no increase in size if link address
is 6 bytes like in Ethernet/Wi-Fi.

Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>

Author: jukkar

drivers/ethernet/dsa_ksz8xxx.c

@@ -938,8 +938,8 @@ struct net_pkt *dsa_ksz8xxx_xmit_pkt(struct net_if *iface, struct net_pkt *pkt)
 	struct net_buf *buf;
 	size_t len, pad = 0;
 
-	lladst.len = sizeof(hdr->dst.addr);
-	lladst.addr = &hdr->dst.addr[0];
+	(void)net_linkaddr_create(&lladst, hdr->dst.addr, sizeof(hdr->dst.addr),
+				  NET_LINK_ETHERNET);
 
 	len = net_pkt_get_len(pkt);
 	/*

drivers/ethernet/eth_native_tap.c

@@ -202,8 +202,8 @@ static int eth_send(const struct device *dev, struct net_pkt *pkt)
 
 static struct net_linkaddr *eth_get_mac(struct eth_context *ctx)
 {
-	ctx->ll_addr.addr = ctx->mac_addr;
-	ctx->ll_addr.len = sizeof(ctx->mac_addr);
+	(void)net_linkaddr_set(&ctx->ll_addr, ctx->mac_addr,
+			       sizeof(ctx->mac_addr));
 
 	return &ctx->ll_addr;
 }

drivers/net/ppp.c

@@ -976,8 +976,7 @@ static int ppp_driver_init(const struct device *dev)
 
 static inline struct net_linkaddr *ppp_get_mac(struct ppp_driver_context *ppp)
 {
-	ppp->ll_addr.addr = ppp->mac_addr;
-	ppp->ll_addr.len = sizeof(ppp->mac_addr);
+	(void)net_linkaddr_set(&ppp->ll_addr, ppp->mac_addr, sizeof(ppp->mac_addr));
 
 	return &ppp->ll_addr;
 }

drivers/net/slip.c

@@ -366,8 +366,8 @@ int slip_init(const struct device *dev)
 
 static inline struct net_linkaddr *slip_get_mac(struct slip_context *slip)
 {
-	slip->ll_addr.addr = slip->mac_addr;
-	slip->ll_addr.len = sizeof(slip->mac_addr);
+	(void)net_linkaddr_set(&slip->ll_addr, slip->mac_addr,
+			       sizeof(slip->mac_addr));
 
 	return &slip->ll_addr;
 }

include/zephyr/net/ieee802154.h

@@ -296,7 +296,7 @@ struct ieee802154_context {
 	uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH];
 
 	/** Link layer address (in big endian) */
-	struct net_linkaddr_storage linkaddr;
+	struct net_linkaddr linkaddr;
 
 #ifdef CONFIG_NET_L2_IEEE802154_SECURITY
 	/** Security context */

include/zephyr/net/net_if.h

@@ -1246,13 +1246,20 @@ static inline int net_if_set_link_addr_unlocked(struct net_if *iface,
 						uint8_t *addr, uint8_t len,
 						enum net_link_type type)
 {
+	int ret;
+
 	if (net_if_flag_is_set(iface, NET_IF_RUNNING)) {
 		return -EPERM;
 	}
 
-	net_if_get_link_addr(iface)->addr = addr;
-	net_if_get_link_addr(iface)->len = len;
-	net_if_get_link_addr(iface)->type = type;
+	if (len > sizeof(net_if_get_link_addr(iface)->addr)) {
+		return -EINVAL;
+	}
+
+	ret = net_linkaddr_create(net_if_get_link_addr(iface), addr, len, type);
+	if (ret < 0) {
+		return ret;
+	}
 
 	net_hostname_set_postfix(addr, len);
 

include/zephyr/net/net_linkaddr.h

@@ -61,22 +61,6 @@ enum net_link_type {
 	NET_LINK_CANBUS_RAW,
 } __packed;
 
-/**
- *  @brief Hardware link address structure
- *
- *  Used to hold the link address information
- */
-struct net_linkaddr {
-	/** The array of byte representing the address */
-	uint8_t *addr; /* in big endian */
-
-	/** Length of that address array */
-	uint8_t len;
-
-	/** What kind of address is this for */
-	uint8_t type;
-};
-
 /**
  *  @brief Hardware link address structure
  *
@@ -87,7 +71,7 @@ struct net_linkaddr {
  *  handled differently than uint8_t addr[] and the fields are purposely
  *  in different order.
  */
-struct net_linkaddr_storage {
+struct net_linkaddr {
 	/** What kind of address is this for */
 	uint8_t type;
 
@@ -124,28 +108,100 @@ static inline bool net_linkaddr_cmp(struct net_linkaddr *lladdr1,
  *
  * @brief Set the member data of a link layer address storage structure.
  *
- * @param lladdr_store The link address storage structure to change.
+ * @param lladdr The link address storage structure to change.
  * @param new_addr Array of bytes containing the link address.
  * @param new_len Length of the link address array.
  * This value should always be <= NET_LINK_ADDR_MAX_LENGTH.
+ * @return 0 if ok, <0 if error
  */
-static inline int net_linkaddr_set(struct net_linkaddr_storage *lladdr_store,
-				   uint8_t *new_addr, uint8_t new_len)
+static inline int net_linkaddr_set(struct net_linkaddr *lladdr,
+				   const uint8_t *new_addr,
+				   uint8_t new_len)
 {
-	if (!lladdr_store || !new_addr) {
+	if (lladdr == NULL || new_addr == NULL) {
 		return -EINVAL;
 	}
 
 	if (new_len > NET_LINK_ADDR_MAX_LENGTH) {
 		return -EMSGSIZE;
 	}
 
-	lladdr_store->len = new_len;
-	memcpy(lladdr_store->addr, new_addr, new_len);
+	lladdr->len = new_len;
+	memcpy(lladdr->addr, new_addr, new_len);
 
 	return 0;
 }
 
+/**
+ * @brief Copy link address from one variable to another.
+ *
+ * @param dst The link address structure destination.
+ * @param src The link address structure to source.
+ * @return 0 if ok, <0 if error
+ */
+static inline int net_linkaddr_copy(struct net_linkaddr *dst,
+				    const struct net_linkaddr *src)
+{
+	if (dst == NULL || src == NULL) {
+		return -EINVAL;
+	}
+
+	if (src->len > NET_LINK_ADDR_MAX_LENGTH) {
+		return -EMSGSIZE;
+	}
+
+	dst->type = src->type;
+	dst->len = src->len;
+	memcpy(dst->addr, src->addr, src->len);
+
+	return 0;
+}
+
+/**
+ * @brief Create a link address structure.
+ *
+ * @param lladdr The link address structure to change.
+ * @param addr Array of bytes containing the link address. If set to NULL,
+ * the address will be cleared.
+ * @param len Length of the link address array.
+ * @param type Type of the link address.
+ * @return 0 if ok, <0 if error
+ */
+static inline int net_linkaddr_create(struct net_linkaddr *lladdr,
+				      const uint8_t *addr, uint8_t len,
+				      enum net_link_type type)
+{
+	if (lladdr == NULL) {
+		return -EINVAL;
+	}
+
+	if (len > NET_LINK_ADDR_MAX_LENGTH) {
+		return -EMSGSIZE;
+	}
+
+	if (addr == NULL) {
+		memset(lladdr->addr, 0, NET_LINK_ADDR_MAX_LENGTH);
+	} else {
+		memcpy(lladdr->addr, addr, len);
+	}
+
+	lladdr->type = type;
+	lladdr->len = len;
+
+	return 0;
+}
+
+/**
+ * @brief Clear link address.
+ *
+ * @param lladdr The link address structure.
+ * @return 0 if ok, <0 if error
+ */
+static inline int net_linkaddr_clear(struct net_linkaddr *lladdr)
+{
+	return net_linkaddr_create(lladdr, NULL, 0, NET_LINK_UNKNOWN);
+}
+
 /**
  * @}
  */

include/zephyr/net/net_pkt.h

@@ -1284,16 +1284,23 @@ static inline struct net_linkaddr *net_pkt_lladdr_dst(struct net_pkt *pkt)
 
 static inline void net_pkt_lladdr_swap(struct net_pkt *pkt)
 {
-	uint8_t *addr = net_pkt_lladdr_src(pkt)->addr;
+	struct net_linkaddr tmp;
 
-	net_pkt_lladdr_src(pkt)->addr = net_pkt_lladdr_dst(pkt)->addr;
-	net_pkt_lladdr_dst(pkt)->addr = addr;
+	memcpy(tmp.addr,
+	       net_pkt_lladdr_src(pkt)->addr,
+	       net_pkt_lladdr_src(pkt)->len);
+	memcpy(net_pkt_lladdr_src(pkt)->addr,
+	       net_pkt_lladdr_dst(pkt)->addr,
+	       net_pkt_lladdr_dst(pkt)->len);
+	memcpy(net_pkt_lladdr_dst(pkt)->addr,
+	       tmp.addr,
+	       net_pkt_lladdr_src(pkt)->len);
 }
 
 static inline void net_pkt_lladdr_clear(struct net_pkt *pkt)
 {
-	net_pkt_lladdr_src(pkt)->addr = NULL;
-	net_pkt_lladdr_src(pkt)->len = 0U;
+	(void)net_linkaddr_clear(net_pkt_lladdr_src(pkt));
+	(void)net_linkaddr_clear(net_pkt_lladdr_dst(pkt));
 }
 
 static inline uint16_t net_pkt_ll_proto_type(struct net_pkt *pkt)

include/zephyr/net/virtual.h

@@ -160,7 +160,7 @@ struct virtual_interface_context {
 	bool is_init;
 
 	/** Link address for this network interface */
-	struct net_linkaddr_storage lladdr;
+	struct net_linkaddr lladdr;
 
 	/** User friendly name of this L2 layer. */
 	char name[VIRTUAL_MAX_NAME_LEN];

samples/net/dsa/src/dsa_lldp.c

@@ -36,11 +36,12 @@ static const uint8_t eth_filter_l2_addr_base[][6] = {
 enum net_verdict dsa_ll_addr_switch_cb(struct net_if *iface, struct net_pkt *pkt)
 {
 	struct net_eth_hdr *hdr = NET_ETH_HDR(pkt);
-	struct net_linkaddr lladst;
+	struct net_linkaddr lladst = { 0 };
 
 	net_pkt_cursor_init(pkt);
-	lladst.len = sizeof(hdr->dst.addr);
-	lladst.addr = &hdr->dst.addr[0];
+
+	(void)net_linkaddr_set(&lladst, (const uint8_t *)hdr->dst.addr,
+			       sizeof(hdr->dst.addr));
 
 	/*
 	 * Pass packet to lan1..3 when matching one from