Ipv4 cleanup

subsys/net/ip/icmpv4.c

@@ -19,6 +19,7 @@
 #include <net/net_pkt.h>
 #include <net/net_if.h>
 #include "net_private.h"
+#include "ipv4.h"
 #include "icmpv4.h"
 #include "net_stats.h"
 
@@ -91,7 +92,7 @@ struct net_buf *net_icmpv4_set_chksum(struct net_pkt *pkt,
 	return frag;
 }
 
-static inline enum net_verdict handle_echo_request(struct net_pkt *pkt)
+static inline enum net_verdict icmpv4_handle_echo_request(struct net_pkt *pkt)
 {
 	/* Note that we send the same data packets back and just swap
 	 * the addresses etc.
@@ -116,7 +117,7 @@ static inline enum net_verdict handle_echo_request(struct net_pkt *pkt)
 	icmp_hdr = net_icmpv4_get_hdr(pkt, &hdr);
 	icmp_hdr->type = NET_ICMPV4_ECHO_REPLY;
 	icmp_hdr->code = 0;
-	icmp_hdr->chksum = 0;
+
 	net_icmpv4_set_hdr(pkt, icmp_hdr);
 	net_icmpv4_set_chksum(pkt, pkt->frags);
 
@@ -137,35 +138,16 @@ static inline enum net_verdict handle_echo_request(struct net_pkt *pkt)
 	return NET_OK;
 }
 
-#define NET_ICMPV4_UNUSED_LEN 4
-
-static inline void setup_ipv4_header(struct net_pkt *pkt, u8_t extra_len,
-				     u8_t ttl, u8_t icmp_type,
-				     u8_t icmp_code)
+static void icmpv4_create(struct net_pkt *pkt, u8_t icmp_type, u8_t icmp_code)
 {
 	struct net_buf *frag = pkt->frags;
 	u16_t pos;
 
-	NET_IPV4_HDR(pkt)->vhl = 0x45;
-	NET_IPV4_HDR(pkt)->tos = 0x00;
-	NET_IPV4_HDR(pkt)->len[0] = 0;
-	NET_IPV4_HDR(pkt)->len[1] = sizeof(struct net_ipv4_hdr) +
-		NET_ICMPH_LEN + extra_len + NET_ICMPV4_UNUSED_LEN;
-
-	NET_IPV4_HDR(pkt)->proto = IPPROTO_ICMP;
-	NET_IPV4_HDR(pkt)->ttl = ttl;
-	NET_IPV4_HDR(pkt)->offset[0] = NET_IPV4_HDR(pkt)->offset[1] = 0;
-	NET_IPV4_HDR(pkt)->id[0] = NET_IPV4_HDR(pkt)->id[1] = 0;
-
-	net_pkt_set_ip_hdr_len(pkt, sizeof(struct net_ipv4_hdr));
-
-	NET_IPV4_HDR(pkt)->chksum = 0;
-	NET_IPV4_HDR(pkt)->chksum = ~net_calc_chksum_ipv4(pkt);
+	net_buf_add(frag, sizeof(struct net_icmp_hdr));
 
 	frag = net_pkt_write_u8(pkt, frag, net_pkt_ip_hdr_len(pkt), &pos,
 				icmp_type);
 	frag = net_pkt_write_u8(pkt, frag, pos, &pos, icmp_code);
-	net_pkt_write_be32(pkt, frag, pos, &pos, 0);
 }
 
 int net_icmpv4_send_echo_request(struct net_if *iface,
@@ -176,7 +158,6 @@ int net_icmpv4_send_echo_request(struct net_if *iface,
 	struct net_if_ipv4 *ipv4 = iface->config.ip.ipv4;
 	const struct in_addr *src;
 	struct net_pkt *pkt;
-	struct net_buf *frag;
 
 	if (!ipv4) {
 		return -EINVAL;
@@ -192,26 +173,19 @@ int net_icmpv4_send_echo_request(struct net_if *iface,
 	pkt = net_pkt_get_reserve_tx(net_if_get_ll_reserve(iface,
 					      (const struct in6_addr *)dst),
 				     K_FOREVER);
-
-	frag = net_pkt_get_frag(pkt, K_FOREVER);
-
-	net_pkt_frag_add(pkt, frag);
-	net_pkt_set_family(pkt, AF_INET);
 	net_pkt_set_iface(pkt, iface);
 
-	net_buf_add(pkt->frags, sizeof(struct net_ipv4_hdr) +
-		    sizeof(struct net_icmp_hdr) +
-		    sizeof(struct net_icmpv4_echo_req));
+	net_ipv4_create(pkt, src, dst, iface, IPPROTO_ICMP);
 
-	net_ipaddr_copy(&NET_IPV4_HDR(pkt)->src, src);
-	net_ipaddr_copy(&NET_IPV4_HDR(pkt)->dst, dst);
+	icmpv4_create(pkt, NET_ICMPV4_ECHO_REQUEST, 0);
 
-	setup_ipv4_header(pkt, 0, net_if_ipv4_get_ttl(iface),
-			  NET_ICMPV4_ECHO_REQUEST, 0);
+	net_buf_add(pkt->frags, sizeof(struct net_icmpv4_echo_req));
 
 	NET_ICMPV4_ECHO_REQ(pkt)->identifier = htons(identifier);
 	NET_ICMPV4_ECHO_REQ(pkt)->sequence = htons(sequence);
 
+	net_ipv4_finalize(pkt, IPPROTO_ICMP);
+
 #if defined(CONFIG_NET_DEBUG_ICMPV4)
 	do {
 		char out[NET_IPV4_ADDR_LEN];
@@ -225,8 +199,6 @@ int net_icmpv4_send_echo_request(struct net_if *iface,
 	} while (0);
 #endif /* CONFIG_NET_DEBUG_ICMPV4 */
 
-	net_icmpv4_set_chksum(pkt, pkt->frags);
-
 	if (net_send_data(pkt) >= 0) {
 		net_stats_update_icmp_sent(iface);
 		return 0;
@@ -241,12 +213,14 @@ int net_icmpv4_send_echo_request(struct net_if *iface,
 
 int net_icmpv4_send_error(struct net_pkt *orig, u8_t type, u8_t code)
 {
+	int err = -EIO;
 	struct net_pkt *pkt;
 	struct net_buf *frag;
-	struct net_if *iface = net_pkt_iface(orig);
-	size_t extra_len, reserve;
-	struct in_addr addr, *src, *dst;
-	int err = -EIO;
+	struct net_if *iface;
+	size_t copy_len;
+	const struct in_addr *src, *dst;
+
+	iface = net_pkt_iface(orig);
 
 	if (NET_IPV4_HDR(orig)->proto == IPPROTO_ICMP) {
 		struct net_icmp_hdr icmp_hdr[1];
@@ -259,68 +233,49 @@ int net_icmpv4_send_error(struct net_pkt *orig, u8_t type, u8_t code)
 		}
 	}
 
-	iface = net_pkt_iface(orig);
+	dst = &NET_IPV4_HDR(orig)->src;
+	src = &NET_IPV4_HDR(orig)->dst;
 
-	pkt = net_pkt_get_reserve_tx(0, PKT_WAIT_TIME);
+	pkt = net_pkt_get_reserve_tx(net_if_get_ll_reserve(iface,
+					      (const struct in6_addr *)dst),
+				     PKT_WAIT_TIME);
 	if (!pkt) {
 		err = -ENOMEM;
 		goto drop_no_pkt;
 	}
 
-	reserve = sizeof(struct net_ipv4_hdr) + sizeof(struct net_icmp_hdr) +
-		NET_ICMPV4_UNUSED_LEN;
+	net_pkt_set_iface(pkt, iface);
+
+	net_ipv4_create(pkt, src, dst, iface, IPPROTO_ICMP);
+
+	icmpv4_create(pkt, type, code);
+
+	/* Appending unused part, filled with 0s */
+	net_pkt_append_be32(pkt, 0);
 
 	if (NET_IPV4_HDR(orig)->proto == IPPROTO_UDP) {
-		extra_len = sizeof(struct net_ipv4_hdr) +
+		copy_len = sizeof(struct net_ipv4_hdr) +
 			sizeof(struct net_udp_hdr);
 	} else if (NET_IPV4_HDR(orig)->proto == IPPROTO_TCP) {
-		extra_len = sizeof(struct net_ipv4_hdr);
+		copy_len = sizeof(struct net_ipv4_hdr);
 		/* FIXME, add TCP header length too */
 	} else {
-		size_t space = CONFIG_NET_BUF_DATA_SIZE -
-			net_if_get_ll_reserve(iface, NULL);
-
-		if (reserve > space) {
-			extra_len = 0;
-		} else {
-			extra_len = space - reserve;
-		}
+		copy_len = 0;
 	}
 
-	/* We need to remember the original location of source and destination
-	 * addresses as the net_pkt_copy() will mangle the original packet.
-	 */
-	src = &NET_IPV4_HDR(orig)->src;
-	dst = &NET_IPV4_HDR(orig)->dst;
-
-	/* We only copy minimal IPv4 + next header from original message.
-	 * This is so that the memory pressure is minimized.
-	 */
-	frag = net_pkt_copy(orig, extra_len, reserve, PKT_WAIT_TIME);
+	frag = net_pkt_copy(orig, copy_len, 0, PKT_WAIT_TIME);
 	if (!frag) {
 		err = -ENOMEM;
 		goto drop;
 	}
 
 	net_pkt_frag_add(pkt, frag);
-	net_pkt_set_family(pkt, AF_INET);
-	net_pkt_set_iface(pkt, iface);
-	net_pkt_set_ll_reserve(pkt, net_buf_headroom(frag));
 
-	net_ipaddr_copy(&addr, src);
-	net_ipaddr_copy(&NET_IPV4_HDR(pkt)->src, dst);
-	net_ipaddr_copy(&NET_IPV4_HDR(pkt)->dst, &addr);
-
-	setup_ipv4_header(pkt, extra_len, net_if_ipv4_get_ttl(iface),
-			  type, code);
+	net_ipv4_finalize(pkt, IPPROTO_ICMP);
 
-	net_pkt_ll_src(pkt)->addr = net_pkt_ll_dst(orig)->addr;
-	net_pkt_ll_src(pkt)->len = net_pkt_ll_dst(orig)->len;
 	net_pkt_ll_dst(pkt)->addr = net_pkt_ll_src(orig)->addr;
 	net_pkt_ll_dst(pkt)->len = net_pkt_ll_src(orig)->len;
 
-	net_icmpv4_set_chksum(pkt, pkt->frags);
-
 #if defined(CONFIG_NET_DEBUG_ICMPV4)
 	do {
 		char out[sizeof("xxx.xxx.xxx.xxx")];
@@ -388,7 +343,7 @@ enum net_verdict net_icmpv4_input(struct net_pkt *pkt)
 static struct net_icmpv4_handler echo_request_handler = {
 	.type = NET_ICMPV4_ECHO_REQUEST,
 	.code = 0,
-	.handler = handle_echo_request,
+	.handler = icmpv4_handle_echo_request,
 };
 
 void net_icmpv4_init(void)

subsys/net/ip/ipv4.c

@@ -41,7 +41,7 @@ struct net_pkt *net_ipv4_create(struct net_pkt *pkt,
 
 	NET_IPV4_HDR(pkt)->vhl = 0x45;
 	NET_IPV4_HDR(pkt)->tos = 0x00;
-	NET_IPV4_HDR(pkt)->proto = 0;
+	NET_IPV4_HDR(pkt)->proto = next_header_proto;
 
 	/* User can tweak the default TTL if needed */
 	NET_IPV4_HDR(pkt)->ttl = net_pkt_ipv4_ttl(pkt);
@@ -55,8 +55,6 @@ struct net_pkt *net_ipv4_create(struct net_pkt *pkt,
 	net_ipaddr_copy(&NET_IPV4_HDR(pkt)->dst, dst);
 	net_ipaddr_copy(&NET_IPV4_HDR(pkt)->src, src);
 
-	NET_IPV4_HDR(pkt)->proto = next_header_proto;
-
 	net_pkt_set_ip_hdr_len(pkt, sizeof(struct net_ipv4_hdr));
 	net_pkt_set_family(pkt, AF_INET);
 
@@ -65,7 +63,7 @@ struct net_pkt *net_ipv4_create(struct net_pkt *pkt,
 	return pkt;
 }
 
-int net_ipv4_finalize(struct net_pkt *pkt, u8_t next_header_proto)
+void net_ipv4_finalize(struct net_pkt *pkt, u8_t next_header_proto)
 {
 	/* Set the length of the IPv4 header */
 	size_t total_len;
@@ -79,22 +77,20 @@ int net_ipv4_finalize(struct net_pkt *pkt, u8_t next_header_proto)
 
 	NET_IPV4_HDR(pkt)->chksum = 0;
 
-#if defined(CONFIG_NET_UDP)
-	if (next_header_proto == IPPROTO_UDP &&
-	    net_if_need_calc_tx_checksum(net_pkt_iface(pkt))) {
-		NET_IPV4_HDR(pkt)->chksum = ~net_calc_chksum_ipv4(pkt);
-		net_udp_set_chksum(pkt, pkt->frags);
-	}
-#endif
-#if defined(CONFIG_NET_TCP)
-	if (next_header_proto == IPPROTO_TCP &&
-	    net_if_need_calc_tx_checksum(net_pkt_iface(pkt))) {
+	if (net_if_need_calc_tx_checksum(net_pkt_iface(pkt)) ||
+	    next_header_proto == IPPROTO_ICMP) {
 		NET_IPV4_HDR(pkt)->chksum = ~net_calc_chksum_ipv4(pkt);
-		net_tcp_set_chksum(pkt, pkt->frags);
-	}
-#endif
 
-	return 0;
+		if (IS_ENABLED(CONFIG_NET_UDP) &&
+		    next_header_proto == IPPROTO_UDP) {
+			net_udp_set_chksum(pkt, pkt->frags);
+		} else if (IS_ENABLED(CONFIG_NET_TCP) &&
+			   next_header_proto == IPPROTO_TCP) {
+			net_tcp_set_chksum(pkt, pkt->frags);
+		} else if (next_header_proto == IPPROTO_ICMP) {
+			net_icmpv4_set_chksum(pkt, pkt->frags);
+		}
+	}
 }
 
 const struct in_addr *net_ipv4_unspecified_address(void)
@@ -139,8 +135,8 @@ enum net_verdict net_ipv4_process_pkt(struct net_pkt *pkt)
 
 	if (!net_is_my_ipv4_addr(&hdr->dst) &&
 	    !net_is_ipv4_addr_mcast(&hdr->dst)) {
-#if defined(CONFIG_NET_DHCPV4)
-		if (hdr->proto == IPPROTO_UDP &&
+		if (IS_ENABLED(CONFIG_NET_DHCPV4) &&
+		    hdr->proto == IPPROTO_UDP &&
 		    net_ipv4_addr_cmp(&hdr->dst,
 				      net_ipv4_broadcast_address())) {
 
@@ -149,7 +145,7 @@ enum net_verdict net_ipv4_process_pkt(struct net_pkt *pkt)
 				return verdict;
 			}
 		}
-#endif
+
 		NET_DBG("IPv4 packet in pkt %p not for me", pkt);
 		goto drop;
 	}

subsys/net/ip/ipv4.h

@@ -48,8 +48,7 @@ struct net_pkt *net_ipv4_create(struct net_pkt *pkt,
  * @param pkt Network packet
  * @param next_header_proto Protocol type of the next header after IPv4 header.
  *
- * @return Return 0 on Success, < 0 on Failure.
  */
-int net_ipv4_finalize(struct net_pkt *pkt, u8_t next_header_proto);
+void net_ipv4_finalize(struct net_pkt *pkt, u8_t next_header_proto);
 
 #endif /* __IPV4_H */

subsys/net/ip/net_context.c

@@ -914,7 +914,7 @@ static int create_udp_packet(struct net_context *context,
 
 		pkt = tmp;
 
-		r = net_ipv4_finalize(pkt, net_context_get_ip_proto(context));
+		net_ipv4_finalize(pkt, net_context_get_ip_proto(context));
 	} else
 #endif /* CONFIG_NET_IPV4 */
 	{

subsys/net/ip/tcp.c

@@ -361,8 +361,7 @@ static int finalize_segment(struct net_context *context, struct net_pkt *pkt)
 {
 #if defined(CONFIG_NET_IPV4)
 	if (net_pkt_family(pkt) == AF_INET) {
-		return net_ipv4_finalize(pkt,
-					 net_context_get_ip_proto(context));
+		net_ipv4_finalize(pkt, net_context_get_ip_proto(context));
 	} else
 #endif
 #if defined(CONFIG_NET_IPV6)