There is more weird thing in that code, but not knowing the context it is hard to comment.
I.e why do you need to open host_if_frame for each packet, why do you use vlib_get_frame_to_node instead of simply registering next indices like 99% of VPP nodes are doing…. — Damjan > On 08.04.2021., at 14:48, David Gohberg <[email protected]> wrote: > > I overlooked this leak :) it wasn't the root cause, I thought it was > something stupid I'm missing but I guess it's not a > trivial issue. I'll continue investigating why this happens, thanks Damjan. > > On Thu, Apr 8, 2021 at 3:27 PM Damjan Marion <[email protected]> wrote: > > Executing `to_frame = vlib_get_frame_to_node (vm, node_index);` n_left_from > times and `vlib_put_frame_to_node (vm, node_index, to_frame);` only once > doesn’t sound sane. You are leaking frames… > > — > Damjan > > > > On 08.04.2021., at 14:12, David Gohberg <[email protected]> wrote: > > > > I edited the example for simplicity, frame->n_vectors is incremented on > > each iteration of the main loop. maybe that is the issue? > > Here is the code with more context: > > > > while (n_left_from > 0) > > { > > u32 bi0; > > bi0 = from[0]; > > from++; > > n_left_from--; > > to_frame = vlib_get_frame_to_node (vm, node_index); > > to_next = vlib_frame_vector_args (to_frame); > > // getting host interface node index from sw if index > > vnet_hw_interface_t * host_intf = vnet_get_sup_hw_interface (vnm, > > sw_if_index); > > u32 host_interface_node_index = host_intf->tx_node_index; > > // frame for the cloned packet > > vlib_frame_t *host_if_frame = vlib_get_frame_to_node(vm, > > host_interface_node_index); > > u32 *intf_host_to_next = vlib_frame_vector_args (host_if_frame); > > host_if_frame->n_vectors = 1; > > // cloning the buffer > > u32 cbi0[2]; > > u16 n_cloned = vlib_buffer_clone (vm, bi0, &cbi0, 2, > > VLIB_BUFFER_CLONE_HEAD_SIZE); > > // enqueuing both packets to two different nodes > > intf_host_to_next[0] = cbi0[1]; > > vlib_put_frame_to_node (vm, host_interface_node_index, > > host_if_frame); > > to_next[0] = bi0; > > to_next++; > > to_frame->n_vectors++; > > } > > vlib_put_frame_to_node (vm, node_index, to_frame); > > return from_frame->n_vectors; > > > > > > On Thu, Apr 8, 2021 at 2:35 PM Damjan Marion <[email protected]> wrote: > > > > Setting frame->n_vectors may help :) > > > > — > > Damjan > > > >> On 08.04.2021., at 13:11, David Gohberg <[email protected]> wrote: > >> > >> Thanks Damjan, > >> > >> This clarifies things regarding the cloning mechanism. However using > >> n_clones=2 and putting the 2nd clone in the frame still results > >> in the buffer being sent only to the host interface, the flow I'm using is > >> this: (minimal example) > >> > >> // getting host interface node index from sw if index > >> vnet_hw_interface_t * host_intf = vnet_get_sup_hw_interface (vnm, > >> sw_if_index); > >> u32 host_interface_node_index = host_intf->tx_node_index; > >> > >> // original frame > >> vlib_frame_t *frame = vlib_get_frame_to_node(vm, node_index); > >> u32 *to_next = vlib_frame_vector_args (frame); > >> > >> // frame for the cloned packet > >> vlib_frame_t *host_if_frame = vlib_get_frame_to_node(vm, > >> host_interface_node_index); > >> u32 *intf_host_to_next = vlib_frame_vector_args (host_if_frame); > >> host_if_frame->n_vectors = 1; > >> > >> // cloning the buffer > >> u32 cbi0[2]; > >> u16 n_cloned = vlib_buffer_clone (vm, bi0, &cbi0, 2, > >> VLIB_BUFFER_CLONE_HEAD_SIZE); > >> > >> // enqueuing both packets to two different nodes > >> to_next[0] = bi0; > >> intf_host_to_next[0] = cbi0[1]; > >> vlib_put_frame_to_node (vm, node_index, frame); > >> vlib_put_frame_to_node (vm, host_interface_node_index, host_if_frame); > >> > >> This results in the packet being sent only to host interface. I've checked > >> vpp source code for similar use cases that use buffer cloning > >> but didn't find a place when a node mirrors a packet and enqueues it to > >> two different nodes. > >> > >> On Thu, Apr 8, 2021 at 1:04 PM Damjan Marion <[email protected]> wrote: > >> > >> You need to say n_clones = 2 to get 2. > >> ref_cnt is updated only on tail buffer if tail buffer(s) exists. > >> Head buffers always have ref_cnt = 1. > >> > >> Reason for that is that you actually never want to clone head buffer > >> simply as if packet goes to 2 > >> different interfaces it needs to have different L2 headers. > >> > >> So it works as follows: > >> > >> if n_clones = 1, simply return same buffer index > >> > >> if n_clones > 1 and packet size is < VLIB_BUFFER_CLONE_HEAD_SIZE: > >> - allocate (n_clones-1) new buffers > >> - memcpy whole payload from original buffer to each allocated buffer > >> > >> if n_clones > 1 and packet size is >= VLIB_BUFFER_CLONE_HEAD_SIZE > >> - allocate (n_clones) new buffers > >> - for each allocated buffer: > >> - memcpy first VLIB_BUFFER_CLONE_HEAD_SIZE bytes of payload from > >> original buffer > >> - set b->next_buffer_index to point to the original buffer > >> - advance b->current_data for VLIB_BUFFER_CLONE_HEAD_SIZE > >> - set b->ref_cnt of the original buffer (and all his tails) to be n_clones > >> > >> > >> Hope this explains… > >> > >> — > >> Damjan > >> > >> > >> > >> > >> > >> > On 08.04.2021., at 11:37, David Gohberg <[email protected]> wrote: > >> > > >> > Hi, > >> > > >> > In my node processing function, I want some buffers to be cloned and > >> > sent to a specific host interface while also be sent to the "normal" > >> > interface resolved in the node. > >> > At the end, the node should be able to send the buffers to two different > >> > interfaces simultaneously. > >> > packet -> node -> interface 1 > >> > | > >> > V > >> > cloned packet > >> > to host interface > >> > > >> > In my single loop where I process the buffers I added a call to this > >> > function: > >> > > >> > static inline void replicate_and_send_buffer(vnet_main_t *vnm, > >> > vlib_main_t * vm, > >> > vlib_node_runtime_t * node, > >> > u32 bi0, > >> > u16 sw_if_index) > >> > { > >> > u32 cbi0; > >> > u16 n_cloned = vlib_buffer_clone (vm, bi0, &cbi0, 1, > >> > VLIB_BUFFER_CLONE_HEAD_SIZE); > >> > vnet_hw_interface_t * host_intf = vnet_get_sup_hw_interface > >> > (vnm, sw_if_index); > >> > u32 node_index = host_intf->tx_node_index; > >> > vlib_frame_t *host_if_frame = vlib_get_frame_to_node(vm, > >> > node_index); > >> > host_if_frame->n_vectors = 1; > >> > u32 *to_next = vlib_frame_vector_args (host_if_frame); > >> > to_next[0] = cbi0; > >> > vlib_put_frame_to_node (vm, node_index, host_if_frame); > >> > } > >> > > >> > What is happening here is that I clone the original buffer and put it in > >> > a frame corresponding to the host interface node (in addition to the > >> > frame of the main loop pointing to a different node) > >> > Running this code leads me to double-free crash. > >> > > >> > I noticed that vlib_buffer_clone, when asked to create 1 clone, > >> > presented me with a new buffer index that has the same address as the > >> > original buffer, but the ref_count stays 1. Looking at the > >> > vlib_buffer_clone confirms that it only copies the packet for n_clones > > >> > 1, but still not updating the ref_count. So it looks like I'm missing > >> > something wrt how the cloning works. > >> > > >> > I changed the code to create 2 clones and tried to use the 2nd clone in > >> > the host interface frame. This resulted in the packet being sent only to > >> > the host interface, but not to both of the interfaces. > >> > > >> > I'm probably doing something wrong, how can I achieve the desired > >> > behavior? > >> > > >> > Thanks, > >> > David > >> > > >> > > >> > > >> > > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#19149): https://lists.fd.io/g/vpp-dev/message/19149 Mute This Topic: https://lists.fd.io/mt/81938105/21656 Group Owner: [email protected] Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
