Hi Yoann,
Shouldn’t the clone code only read s->total_length_not_including_first_buffer
only if VLIB_BUFFER_NEXT_PRESENT flag is set for s? I’m thinking about this
line in particular:
d->total_length_not_including_first_buffer =
s->total_length_not_including_first_buffer + s->current_length -
head_end_offset;
If you only add s->total_length_not_including_first_buffer if
VLIB_BUFFER_NEXT_PRESENT is set, do you still see the issue?
Florin
> On May 23, 2018, at 8:13 AM, Yoann Desmouceaux <ydesm...@cisco.com> wrote:
>
> Hello,
>
> I noticed that dpdk devices set the VLIB_BUFFER_TOTAL_LENGTH_VALID flag when
> receiving packets, regardless of the value of
> total_length_not_including_first_buffer. Since dpdk only copies one cacheline
> from its vlib_buffer template into the buffer upon receipt of a packet,
> b->total_length_not_including_first_buffer is left unitialized (because it's
> in the second cacheline).
>
> In most cases, this doesn't cause issues — for non-chained buffers the
> vlib_buffer_length_in_chain() function will still return the correct value
> because it only looks at b->current_length.
>
> However, if the buffer is later vlib_buffer_clone()d, the stale
> total_length_not_including_first_buffer value will propagate to the new head
> of the chain, for which vlib_buffer_length_in_chain() will now return a wrong
> value. This can be observed for instance when using BIER, with which
> replicated packets will sometimes have oversized lengths.
>
> An obvious fix for that is to remove the VLIB_BUFFER_TOTAL_LENGTH_VALID flag
> from the buffer_flags_template in dpdk_init(). I'm concerned about the
> performance of this approach though, because vlib_buffer_length_in_chain()
> will then have to go through the slow path for all chained packets, causing
> branches. The other solution would be to, in addition, add something along
> the lines of the following in dpdk_process_rx_burst():
> b[0]->flags |= (!b[0]->total_length_not_including_first_buffer) <<
> VLIB_BUFFER_LOG2_TOTAL_LENGTH_VALID;
> so as to set the flag in correct packets as early as possible.
> This would be branchless code, but would cost a few cycles for every packet
> received by dpdk. Any thoughts?
>
> Also, does anyone know if this can happen with other devices than dpdk?
>
> Best,
> Yoann
>
