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 (#19148): https://lists.fd.io/g/vpp-dev/message/19148
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]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to