lib: net_buf: usage helper functions

[JordanYates]

Add helper functions for querying the internal fields ofstruct net_buf_pool when CONFIG_NET_BUF_POOL_USAGE is enabled.

[pdgendt]

Nit

[pdgendt]

Suggested change

	__ASSERT_NO_MSG(pool);
	__ASSERT_NO_MSG(pool != NULL);

[JordanYates]

I copied the pattern from the next function, I can update both or neither

[jhedberg]

I'm a bit hesitant about adding this, for a couple of reasons:

1. It's inherently racy (as even the documentation for it explains). Even if you understand the implications and deal with it for your use case, we'd be giving a foot gun to most developers.
2. Even though it's only the payload part of net_buf that you can do custom allocators for, I think it would be beneficial to evolve the design toward also being able to allocate the actual metadata (i.e. struct net_buf) from a heap. When we'd do that, this new API would loose meaning for such pools.

Unrelated to my reservations, but just something I initially reacted to: not having "pool" in the name (i.e. net_buf_pool_num_free()) makes the naming less intuitive. However, I later realized that this is in line with the names of other existing APIs that operate primarily on a pool rather than a specific buffer.

[JordanYates]

It's inherently racy (as even the documentation for it explains). Even if you understand the implications and deal with it for your use case, we'd be giving a foot gun to most developers.

It's not racy, its an instantaneous snapshot. Nothing will be corrupted with multiple users. Its the same with querying any other shared object in Zephyr. We still have the atomic API even though atomic_get could immediately return a different value if queried a second time.

This is solving an issue that can't currently be solved (without wrapping the entire net_buf API). For a given pool, is it close to running out of buffers to hand out? The goal is to be proactive about rate limiting, instead of only reacting once allocation fails, which in some cases is way too late due to data in flight. In that scenario it doesn't really matter whether the value is 2, 3, or 4 because some other thread freed a buffer, it just matters that the number is low, not high.

[jhedberg]

It's not racy, its an instantaneous snapshot. Nothing will be corrupted with multiple users. Its the same with querying any other shared object in Zephyr. We still have the atomic API even though atomic_get could immediately return a different value if queried a second time.

I didn't intend to suggest that anything would be corrupted. My point was that the moment the caller gets the value it may not reflect reality anymore, so any code that makes such assumptions would be prone to unexpected behavior.

This is solving an issue that can't currently be solved (without wrapping the entire net_buf API). For a given pool, is it close to running out of buffers to hand out? The goal is to be proactive about rate limiting, instead of only reacting once allocation fails, which in some cases is way too late due to data in flight. In that scenario it doesn't really matter whether the value is 2, 3, or 4 because some other thread freed a buffer, it just matters that the number is low, not high.

This is very useful information. Knowing the usecase opens up the possibility for considering various other kinds of solutions, even if the proposal here would be the best one.

Would it e.g. make sense to have some low water mark callback for the pool, where you define some minimum and when the available buffer count goes below that the app gets notified? Since the net_buf API allows defining custom pool implementations, that could also potentially have been another way to approach solving this.

[JordanYates]

Would it e.g. make sense to have some low water mark callback for the pool, where you define some minimum and when the available buffer count goes below that the app gets notified? Since the net_buf API allows defining custom pool implementations, that could also potentially have been another way to approach solving this.

Possibly, but it would be quite a different solution:

1. The callback would be global, whereas my problem only cares about exhaustion while performing a specific task (receiving high frequency data over GATT)
2. A pool can have multiple users, but a callback in the simplest case (without a list approach) can only handle one user
3. The callback would need to be more complicated than what you propose, since you also need to know when the buffer count goes above the threshold
4. Extra state tracking in the user to set a flag while the buffer count is low

Possible? sure. As simple as a uint16_t you can query at will? no.

[JordanYates]

IMO this is roughly the same behavior as every kernel function that can take K_NO_WAIT as an argument, it tells you something about the state of the system at the time of the call, nothing more. Its the users responsibility to use it correctly.

[jukkar]

Note that there is already avail_count variable that gives this information, one just needs to enable CONFIG_NET_BUF_POOL_USAGE to access it.

[JordanYates]

Thanks to @jukkar's comment, PR has been updated to simply provide wrapper functions around the existing functionality. I should have paid attention to the code 4 lines below where I was typing.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[JordanYates]

CI will fail until #98755 is merged (weird co-incidence, the PRs had nothing to do with each other)

[jhedberg]

I was going to get back to this after 4.3 (as this is a feature and not a bug fix), but since there are starting to be approvals from others I thought I'd save your time and get this API detail objection posted already now. Other than that I don't have any objections to this anymore, however I'm not sure what we'll do with these when we have a pool that allocates the net_buf structs from a heap, i.e. where you really don't know these limits until you hit an allocation failure.

[jhedberg]

The return value types of these should likely be size_t. That has been the general approach with net_buf APIs from the beginning, i.e. the internal types may be uint16_t, but as that's a pure memory footprint optimization it shouldn't be visible in the public API. Also see #98144.