On 08.08.2017 21:36, Andy Zhou wrote: > On Mon, Aug 7, 2017 at 11:01 PM, Ilya Maximets <i.maxim...@samsung.com> wrote: >> 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. > > Because the add__() version omits boundary checks, it should not be used > by the end user. Rather it should only be used for implementing > dp_packet_batch_xxx APIs. > So, in this case, it is internal use only. I don't think add_unsafe() > adds any additional value.
Maybe I don't fully understand what you're trying to say, but I want to use unsafe function in dpif-netdev for per-flow packet batching (see the patch) and it should not be internal for that case. (It's safe to use unsafe function there because per-flow batches are guaranteed to be less than original rx batch.) >> >> 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. > > We can make add__() API takes one additional argument, "idx", Would this > help? _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev