[Bug] Zenoh Pico multicast receive fragments memory

[PetervdPerk-NXP]

Describe the bug

The way how Zenoh pico multicast now is implemented it allocates and frees the address + port causing to memory to fragment a lot.

zenoh-pico/src/transport/multicast/link/task/read.c

Line 51 in 5b47341

_z_bytes_t addr = _z_bytes_wrap(NULL, 0);

Since the (address + port) is relatively small I would propose to allocate the address on multicast read task stack.
However this a bit more complex since addr can be either IPv4 or IPv6 thus the size might differ.

A proposal from would be to introduce a new Zenoh datatype i.e. _z_addr_t
That includes both ip address (IPv4 & IPv6) and the port.

struct _z_addr_t
{
  union
  {
     in_addr_t in4_addr;
     struct in6_addr in6_addr;
  } _z_address_u;
  in_port_t port
  bool isipv6;
};

Any suggestions or am I missing something here?

To reproduce

Start a multicast UDP Rx session

System info

NuttX

[cguimaraes]

In a first glance, the proposed solution would break the current abstraction for platforms and links. Also, we shall also consider that there are other type of addresses coming from non-IP transports that are currently supported in some platforms, thus cannot make it work just for IPv4 and IPv6.

Still, you point to an enhancement that can be made, where we alloc once and reuse after. We just need to make sure that the allocated _z_bytes_t is big enough for any kind of supported transport.
I will need to further look at it.

[cguimaraes]

@p-avital was this tackled with the lastest protocol refactor? I believe you had to touch this part of the code in the process.

[p-avital]

Hi there,

At a glance, there are a few things that could be done:

• _z_bytes_t could wrap a static variable in the link's implementation, allowing it never allocate. However, this means that this addr can never be stolen (which it currently doesn't seem to be). It's an additional safety invariant, but should be relatively low-effort to implement.
• I'm against the initial proposal for _z_addr_t, as links are not meant to always be IP, so I don't want any IP specific stuff in there; but we could have something like the following, which would allow small buffers (like most addresses would be) to be written on stack:

union {
  _z_bytes_t bytes;
  struct {
    char buf[127];
    char len; \*negative len = zbytes variant active*\
  } inline;
}

Either way, we currently have higher priority tasks on our stack, so we can't commit to any timeline for implementing either of these ideas. Feel free to make a PR with solution 2 if it sounds useful to you, as it has the advantage of not adding any safety invariants that might be hard to review.