net: nbuf: Fix net_nbuf_compact() API

Current API description of net_nbuf_compact() is not very clear.
The first parameter needs to be the first net_buf in the chain.

The changes to this API are needed in order to clarify following
use cases:
1) User provides fragment that is not first of the chain and compact is
successfully done. In this case there is no free space in fragment list
after the input fragment. But there might be empty space in previous
fragments. So fragment chain is not completely compacted.

2) What if input fragment has been deleted and api returns the same
buf?

So this commit simplifies the API behavior. Now net_nbuf_compact()
expects the first parameter to be either TX or RX net_buf and then it
compacts it. It fails only if the input fragment is a data fragment.

Change-Id: I9e02dfcb6f3f2e2998826522a25ec207850a8056
Signed-off-by: Ravi kumar Veeramally <ravikumar.veeramally@linux.intel.com>

include/net/nbuf.h

@@ -566,12 +566,13 @@ int net_nbuf_linear_copy(struct net_buf *dst, struct net_buf *src,
  * @brief Compact the fragment list.
  *
  * @details After this there is no more any free space in individual fragments.
- * @param buf Network buffer fragment. This should be the first fragment (data)
- * in the fragment list.
+ * @param buf Network buffer fragment. This should be the Tx/Rx buffer.
+ *
+ * @return True if compact success, False otherwise. (Note that it fails only
+ *         when input is data fragment)
  *
- * @return Pointer to the start of the fragment list if ok, NULL otherwise.
  */
-struct net_buf *net_nbuf_compact(struct net_buf *buf);
+bool net_nbuf_compact(struct net_buf *buf);
 
 /**
  * @brief Check if the buffer chain is compact or not.

subsys/net/ip/6lo.c

@@ -784,7 +784,7 @@ end:
 	net_buf_frag_insert(buf, frag);
 
 	/* Compact the fragments, so that gaps will be filled */
-	net_nbuf_compact(buf->frags);
+	net_nbuf_compact(buf);
 
 	if (fragment) {
 		return fragment(buf, compressed - offset);
@@ -1372,7 +1372,7 @@ end:
 
 	/* Insert the fragment (this one holds uncompressed headers) */
 	net_buf_frag_insert(buf, frag);
-	net_nbuf_compact(buf->frags);
+	net_nbuf_compact(buf);
 
 	/* Set IPv6 header and UDP (if next header is) length */
 	len = net_buf_frags_len(buf) - NET_IPV6H_LEN;
@@ -1408,7 +1408,7 @@ static inline bool compress_ipv6_header(struct net_buf *buf,
 	net_buf_frag_insert(buf, frag);
 
 	/* Compact the fragments, so that gaps will be filled */
-	buf->frags = net_nbuf_compact(buf->frags);
+	net_nbuf_compact(buf);
 
 	if (fragment) {
 		return fragment(buf, -1);

subsys/net/ip/l2/arp.c

@@ -225,7 +225,7 @@ struct net_buf *net_arp_prepare(struct net_buf *buf)
 
 		net_buf_frag_insert(buf, header);
 
-		buf = net_nbuf_compact(buf);
+		net_nbuf_compact(buf);
 	}
 
 	hdr = (struct net_eth_hdr *)net_nbuf_ll(buf);

subsys/net/ip/nbuf.c

@@ -751,20 +751,19 @@ bool net_nbuf_is_compact(struct net_buf *buf)
 	return false;
 }
 
-struct net_buf *net_nbuf_compact(struct net_buf *buf)
+bool net_nbuf_compact(struct net_buf *buf)
 {
-	struct net_buf *first, *prev;
+	struct net_buf *prev;
 
-	first = buf;
-
-	if (!is_from_data_pool(buf)) {
-		NET_DBG("Buffer %p is not a data fragment", buf);
-		buf = buf->frags;
+	if (is_from_data_pool(buf)) {
+		NET_DBG("Buffer %p is a data fragment", buf);
+		return false;
 	}
 
-	prev = NULL;
+	NET_DBG("Compacting data to buf %p", buf);
 
-	NET_DBG("Compacting data to buf %p", first);
+	buf = buf->frags;
+	prev = NULL;
 
 	while (buf) {
 		if (buf->frags) {
@@ -804,9 +803,6 @@ struct net_buf *net_nbuf_compact(struct net_buf *buf)
 				/* Remove the last fragment because there is no
 				 * data in it.
 				 */
-				NET_ASSERT_INFO(prev,
-					"First element cannot be deleted!");
-
 				net_buf_frag_del(prev, buf);
 
 				break;
@@ -817,7 +813,7 @@ struct net_buf *net_nbuf_compact(struct net_buf *buf)
 		buf = buf->frags;
 	}
 
-	return first;
+	return true;
 }
 
 struct net_buf *net_nbuf_pull(struct net_buf *buf, size_t amount)

subsys/net/ip/tcp.c

@@ -307,7 +307,7 @@ static struct net_buf *prepare_segment(struct net_tcp *tcp,
 		return NULL;
 	}
 
-	buf = net_nbuf_compact(buf);
+	net_nbuf_compact(buf);
 
 	net_tcp_trace("", buf);
 

tests/net/nbuf/src/main.c

@@ -1083,7 +1083,7 @@ static int test_fragment_compact(void)
 		return -1;
 	}
 
-	buf = net_nbuf_compact(buf);
+	net_nbuf_compact(buf);
 
 	if (!net_nbuf_is_compact(buf)) {
 		printk("The buf should be in compact form. Test fails\n");
@@ -1091,7 +1091,7 @@ static int test_fragment_compact(void)
 	}
 
 	/* Try compacting again, nothing should happen */
-	buf = net_nbuf_compact(buf);
+	net_nbuf_compact(buf);
 
 	if (!net_nbuf_is_compact(buf)) {
 		printk("The buf should be compacted now. Test fails\n");
@@ -1109,7 +1109,7 @@ static int test_fragment_compact(void)
 
 	count = calc_fragments(buf);
 
-	buf = net_nbuf_compact(buf);
+	net_nbuf_compact(buf);
 
 	i = calc_fragments(buf);
 
@@ -1138,7 +1138,7 @@ static int test_fragment_compact(void)
 
 	count = calc_fragments(buf);
 
-	buf = net_nbuf_compact(buf);
+	net_nbuf_compact(buf);
 
 	i = calc_fragments(buf);
 
@@ -1167,7 +1167,7 @@ static int test_fragment_compact(void)
 
 	count = calc_fragments(buf);
 
-	buf = net_nbuf_compact(buf);
+	net_nbuf_compact(buf);
 
 	i = calc_fragments(buf);