[ovs-dev] [ovs-dev, v4] dpif-netdev: Avoid reordering of packets in a batch with same megaflow
i.maximets at samsung.com
Tue Jul 10 11:28:10 UTC 2018
On 10.07.2018 14:14, Vishal Deep Ajmera wrote:
>>> + /* preserve the order of packet for flow batching */
>>> + index_map[packets_->count - 1] = map_cnt;
>> Using the "packets_->count" without accessor + inside a refill loop is highly
>> inaccurate/potentially dangerous. Isn't it equal to "n_missed"?
> Hi Ilya,
> Thanks for the review. However I could not understand your point about
> inaccurate/potentially dangerous code. Can you elaborate on a scenario
> where this is likely to fail/crash ?
This is potentially dangerous from the future modifications and hard to
read for reviewer/person who tries to understand how it works.
Current implementation will fail if someone will change the logic of
'DP_PACKET_BATCH_REFILL_FOR_EACH', for example.
The key point is that 'DP_PACKET_BATCH_REFILL_FOR_EACH' and
'dp_packet_batch_refill' manipulates with the batch size in a hidden
from the end user manner. Using of the 'packets_->count' directly
requires knowledge of how refilling works internally, but these
functions was intended to hide internals of batch manipulations.
Also, as I understand, you're storing 'map_cnt' for each missed packet.
So, why not just use 'n_missed' for that purpose?
Best regards, Ilya Maximets.
More information about the dev