net: buf: resolve issues with fixed size network buffer pool

doc/releases/migration-guide-3.6.rst

@@ -503,6 +503,11 @@ Networking
 * The zperf ratio between mbps and kbps, kbps and bps is changed to 1000, instead of 1024,
   to align with iperf ratios.
 
+* For network buffer pools maximum allocation size was added to a common structure
+  ``struct net_buf_data_alloc`` as a new field ``max_alloc_size``. Similar member ``data_size`` of
+  ``struct net_buf_pool_fixed`` that was specific only for buffer pools with a fixed size was
+  removed.
+
 zcbor
 =====
 

include/zephyr/net/buf.h

@@ -965,6 +965,7 @@ struct net_buf_data_cb {
 struct net_buf_data_alloc {
 	const struct net_buf_data_cb *cb;
 	void *alloc_data;
+	size_t max_alloc_size;
 };
 
 /**
@@ -1078,7 +1079,6 @@ extern const struct net_buf_data_alloc net_buf_heap_alloc;
 					 _destroy)
 
 struct net_buf_pool_fixed {
-	size_t data_size;
 	uint8_t *data_pool;
 };
 
@@ -1118,12 +1118,12 @@ extern const struct net_buf_data_cb net_buf_fixed_cb;
 	_NET_BUF_ARRAY_DEFINE(_name, _count, _ud_size);                        \
 	static uint8_t __noinit net_buf_data_##_name[_count][_data_size] __net_buf_align; \
 	static const struct net_buf_pool_fixed net_buf_fixed_##_name = {       \
-		.data_size = _data_size,                                       \
 		.data_pool = (uint8_t *)net_buf_data_##_name,                  \
 	};                                                                     \
 	static const struct net_buf_data_alloc net_buf_fixed_alloc_##_name = { \
 		.cb = &net_buf_fixed_cb,                                       \
 		.alloc_data = (void *)&net_buf_fixed_##_name,                  \
+		.max_alloc_size = _data_size,                                  \
 	};                                                                     \
 	static STRUCT_SECTION_ITERABLE(net_buf_pool, _name) =                  \
 		NET_BUF_POOL_INITIALIZER(_name, &net_buf_fixed_alloc_##_name,  \
@@ -1164,6 +1164,7 @@ extern const struct net_buf_data_cb net_buf_var_cb;
 	static const struct net_buf_data_alloc net_buf_data_alloc_##_name = {  \
 		.cb = &net_buf_var_cb,                                         \
 		.alloc_data = &net_buf_mem_pool_##_name,                       \
+		.max_alloc_size = 0,                                           \
 	};                                                                     \
 	static STRUCT_SECTION_ITERABLE(net_buf_pool, _name) =                  \
 		NET_BUF_POOL_INITIALIZER(_name, &net_buf_data_alloc_##_name,   \

subsys/net/buf.c

@@ -151,9 +151,9 @@ static uint8_t *fixed_data_alloc(struct net_buf *buf, size_t *size,
 	struct net_buf_pool *pool = net_buf_pool_get(buf->pool_id);
 	const struct net_buf_pool_fixed *fixed = pool->alloc->alloc_data;
 
-	*size = MIN(fixed->data_size, *size);
+	*size = pool->alloc->max_alloc_size;
 
-	return fixed->data_pool + fixed->data_size * net_buf_id(buf);
+	return fixed->data_pool + *size * net_buf_id(buf);
 }
 
 static void fixed_data_unref(struct net_buf *buf, uint8_t *data)
@@ -203,6 +203,7 @@ static const struct net_buf_data_cb net_buf_heap_cb = {
 
 const struct net_buf_data_alloc net_buf_heap_alloc = {
 	.cb = &net_buf_heap_cb,
+	.max_alloc_size = 0,
 };
 
 #endif /* K_HEAP_MEM_POOL_SIZE > 0 */
@@ -345,18 +346,14 @@ struct net_buf *net_buf_alloc_fixed_debug(struct net_buf_pool *pool,
 					  k_timeout_t timeout, const char *func,
 					  int line)
 {
-	const struct net_buf_pool_fixed *fixed = pool->alloc->alloc_data;
-
-	return net_buf_alloc_len_debug(pool, fixed->data_size, timeout, func,
+	return net_buf_alloc_len_debug(pool, pool->alloc->max_alloc_size, timeout, func,
 				       line);
 }
 #else
 struct net_buf *net_buf_alloc_fixed(struct net_buf_pool *pool,
 				    k_timeout_t timeout)
 {
-	const struct net_buf_pool_fixed *fixed = pool->alloc->alloc_data;
-
-	return net_buf_alloc_len(pool, fixed->data_size, timeout);
+	return net_buf_alloc_len(pool, pool->alloc->max_alloc_size, timeout);
 }
 #endif
 
@@ -659,6 +656,7 @@ size_t net_buf_append_bytes(struct net_buf *buf, size_t len,
 	struct net_buf *frag = net_buf_frag_last(buf);
 	size_t added_len = 0;
 	const uint8_t *value8 = value;
+	size_t max_size;
 
 	do {
 		uint16_t count = MIN(len, net_buf_tailroom(frag));
@@ -681,7 +679,10 @@ size_t net_buf_append_bytes(struct net_buf *buf, size_t len,
 			 * been provided.
 			 */
 			pool = net_buf_pool_get(buf->pool_id);
-			frag = net_buf_alloc_len(pool, len, timeout);
+			max_size = pool->alloc->max_alloc_size;
+			frag = net_buf_alloc_len(pool,
+						 max_size ? MIN(len, max_size) : len,
+						 timeout);
 		}
 
 		if (!frag) {

tests/net/buf/src/main.c

@@ -436,6 +436,12 @@ ZTEST(net_buf_tests, test_net_buf_fixed_pool)
 	buf = net_buf_alloc_len(&fixed_pool, 20, K_NO_WAIT);
 	zassert_not_null(buf, "Failed to get buffer");
 
+	/* Verify buffer's size and len - even though we requested less bytes we
+	 * should get a buffer with the fixed size.
+	 */
+	zassert_equal(buf->size, FIXED_BUFFER_SIZE, "Invalid fixed buffer size");
+	zassert_equal(buf->len, 0, "Invalid fixed buffer length");
+
 	net_buf_unref(buf);
 
 	zassert_equal(destroy_called, 1, "Incorrect destroy callback count");
@@ -797,4 +803,50 @@ ZTEST(net_buf_tests, test_net_buf_comparison)
 	net_buf_unref(buf);
 }
 
+ZTEST(net_buf_tests, test_net_buf_fixed_append)
+{
+	struct net_buf *buf;
+	uint8_t data[FIXED_BUFFER_SIZE * 2];
+
+	/* Fill data buffer */
+	for (int i = 0; i < sizeof(data); ++i) {
+		data[i] = (uint8_t)i;
+	}
+
+	/* Fixed Pool */
+	buf = net_buf_alloc(&fixed_pool, K_NO_WAIT);
+	zassert_not_null(buf, "Failed to get fixed buffer");
+	zassert_equal(buf->size, FIXED_BUFFER_SIZE, "Invalid fixed buffer size");
+
+	/* For fixed pool appending less bytes than buffer's free space must
+	 * not add a new fragment
+	 */
+	net_buf_append_bytes(buf, buf->size - 8, data, K_NO_WAIT, NULL, NULL);
+	zassert_equal(buf->len, buf->size - 8, "Invalid buffer len");
+	zassert_is_null(buf->frags, "Unexpected buffer fragment");
+
+	/* Filling rest of the space should not add an additional buffer */
+	net_buf_append_bytes(buf, 8, data, K_NO_WAIT, NULL, NULL);
+	zassert_equal(buf->len, buf->size, "Invalid buffer len");
+	zassert_is_null(buf->frags, "Unexpected buffer fragment");
+
+	/* Appending any number of bytes allocates an additional fragment */
+	net_buf_append_bytes(buf, 1, data, K_NO_WAIT, NULL, NULL);
+	zassert_not_null(buf->frags, "Lack of expected buffer fragment");
+	zassert_equal(buf->frags->len, 1, "Expected single byte in the new fragment");
+	zassert_equal(buf->frags->size, buf->size, "Different size of the fragment");
+
+	/* Remove 1-byte buffer */
+	net_buf_frag_del(buf, buf->frags);
+
+	/* Appending size bigger than single buffer's size will allocate multiple fragments */
+	net_buf_append_bytes(buf, sizeof(data), data, K_NO_WAIT, NULL, NULL);
+	zassert_not_null(buf->frags, "Missing first buffer fragment");
+	zassert_not_null(buf->frags->frags, "Missing second buffer fragment");
+	zassert_is_null(buf->frags->frags->frags, "Unexpected buffer fragment");
+
+	net_buf_unref(buf);
+}
+
+
 ZTEST_SUITE(net_buf_tests, NULL, NULL, NULL, NULL, NULL);

tests/net/net_pkt/src/main.c

@@ -956,7 +956,7 @@ ZTEST(net_pkt_test_suite, test_net_pkt_headroom)
 	net_pkt_unref(pkt);
 }
 
-NET_BUF_POOL_FIXED_DEFINE(test_net_pkt_headroom_copy_pool, 2, 4, 4, NULL);
+NET_BUF_POOL_VAR_DEFINE(test_net_pkt_headroom_copy_pool, 2, 128, 4, NULL);
 
 ZTEST(net_pkt_test_suite, test_net_pkt_headroom_copy)
 {