On 07.08.2017 23:24, Andy Zhou wrote: > On Mon, Aug 7, 2017 at 8:50 AM, Ilya Maximets <i.maxim...@samsung.com> wrote: >> Almost all batch usecases covered by the new API introduced >> in commit 72c84bc2db23 ("dp-packet: Enhance packet batch APIs.") >> except unsafe batch addition. It used in dpif-netdev for fast >> per-flow batches filling. Introduction of this new function will >> allow to avoid most direct accesses to the batch structure. >> >> Function defined as 'inline' in .h file. Should not impact on >> performance. >> >> Additionally unsafe version now used in dp_packet_batch_clone() >> to speed up it a little, and also fixed few cases missed while >> API enchancing. >> >> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> > > Those are worthwhile optimizations. I have a minor suggestion for you > to consider: > > Instead of adding a new API dp_packet_batch_add_unsafe(), how about > just redefine > current APIs. > > dp_packet_batch_add() currently does not do much, it simply call > dp_packet_batch_add__(), > We can just turn this API into the safe version, same as > dp_packet_batch_add__(). > > dp_packet_batch_add__() can be changed into the unsafe version. Use it > within the reset > of the patch where boundary checks can be avoided.
dp_packet_batch_add__() should be renamed in this case to *add_unsafe() or something else anyway because "__" suffix means "internal use only". We should not expose function with such name for global using. Also, dp_packet_batch_add__() used inside dp_packet_batch_refill(). We will need to modify refill logic somehow in this case. --> more API changes. What do you think? > > Thanks. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev