Skip to content

net_buf: support large size #97440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

net_buf: support large size #97440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions include/zephyr/net_buf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ struct net_buf_simple {
*
* To determine the max length, use net_buf_simple_max_len(), not #size!
*/
uint16_t len;
uint32_t len;

/** Amount of data that net_buf_simple#__buf can store. */
uint16_t size;
uint32_t size;
Comment on lines -98 to +101
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you misunderstood me a little bit. I wasn't proposing to change this unconditionally to uint32_t, rather you'd need some explicit enabling of the larger size, through Kconfig, like you had before. Otherwise you're causing memory overflows to platforms what are very constrained (we have at least some build configurations for 16k platforms where even just a few bytes increase in RAM consumption will cause the build to fail).


/** Start of the data storage. Not to be accessed directly
* (the data pointer should be used instead).
Expand Down Expand Up @@ -938,7 +938,7 @@ size_t net_buf_simple_tailroom(const struct net_buf_simple *buf);
*
* @return Number of bytes usable behind the net_buf_simple::data pointer.
*/
uint16_t net_buf_simple_max_len(const struct net_buf_simple *buf);
size_t net_buf_simple_max_len(const struct net_buf_simple *buf);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in a separate commit, with the commit message explaining that it's just aligning this with the rest of the net_buf APIs.


/**
* @brief Parsing state of a buffer.
Expand All @@ -949,9 +949,9 @@ uint16_t net_buf_simple_max_len(const struct net_buf_simple *buf);
*/
struct net_buf_simple_state {
/** Offset of the data pointer from the beginning of the storage */
uint16_t offset;
uint32_t offset;
/** Length of data */
uint16_t len;
uint32_t len;
Comment on lines -952 to +954
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. This should be build-time conditional.

Copy link
Contributor Author

@MarkWangChinese MarkWangChinese Oct 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I will add back the net_buf_size_t too. Do you have concern about using typedef? IMO, both are OK.

#if defined(CONFIG_NET_BUF_LARGE_SIZE)
/** @brief net buf size type */
typedef uint32_t net_buf_size_t;
#else
/** @brief net buf size type */
typedef uint16_t net_buf_size_t;
#endif

...
	net_buf_size_t len;
...

or

...
#if defined(CONFIG_NET_BUF_LARGE_SIZE)
	uint16_t len;
#else
	uint32_t len;
#endif
...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that you need to have the correct type in multiple structs and multiple member variables, I think the typedef would be the simplest option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, I'd appreciate some more thoughts from @jfischer-no, since he seemed to be against this. @jfischer-no changing unconditionally to a larger size will not work without also finding a solution for our memory constrained platforms that can't currently take any additional memory hit wrt to net_buf sizes.

};

/**
Expand All @@ -965,7 +965,7 @@ struct net_buf_simple_state {
static inline void net_buf_simple_save(const struct net_buf_simple *buf,
struct net_buf_simple_state *state)
{
state->offset = (uint16_t)net_buf_simple_headroom(buf);
state->offset = (uint32_t)net_buf_simple_headroom(buf);
state->len = buf->len;
}

Expand Down Expand Up @@ -1032,10 +1032,10 @@ struct net_buf {
uint8_t *data;

/** Length of the data behind the data pointer. */
uint16_t len;
uint32_t len;

/** Amount of data that this buffer can store. */
uint16_t size;
uint32_t size;
Comment on lines -1035 to +1038
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here.

Initially I think we should make the larger size opt in. We can then discuss if eventually it'd be opt-out, and then we make the memory-constrained configurations explicitly select the smaller size.


/** Start of the data storage. Not to be accessed
* directly (the data pointer should be used
Expand Down Expand Up @@ -1097,7 +1097,7 @@ struct net_buf_pool {
atomic_t avail_count;

/** Total size of the pool. */
const uint16_t pool_size;
const uint32_t pool_size;

/** Maximum count of used buffers. */
uint16_t max_used;
Expand Down Expand Up @@ -2523,7 +2523,7 @@ static inline size_t net_buf_headroom(const struct net_buf *buf)
*
* @return Number of bytes usable behind the net_buf::data pointer.
*/
static inline uint16_t net_buf_max_len(const struct net_buf *buf)
static inline size_t net_buf_max_len(const struct net_buf *buf)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate commit.

{
return net_buf_simple_max_len(&buf->b);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/net_buf/buf_simple.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ size_t net_buf_simple_tailroom(const struct net_buf_simple *buf)
return buf->size - net_buf_simple_headroom(buf) - buf->len;
}

uint16_t net_buf_simple_max_len(const struct net_buf_simple *buf)
size_t net_buf_simple_max_len(const struct net_buf_simple *buf)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate commit

{
return buf->size - net_buf_simple_headroom(buf);
}
Loading