On 6/17/21 11:59 PM, Ben Pfaff wrote: > All these flags for stealing, allowing stealing, blah blah, are just > ways to do some kind of dumb reference counting without actually have a > reference count. When it gets super complex like this, maybe > introducing a reference count is the way to go. It would be a bigger > change, but perhaps more maintainable over time. >
Yes, I absolutely agree. We just removed one of these hacky flags from the struct dp_packet_batch and we need to get rid of the rest of them. One thing that bothers me is that some parts of the code treats 'should_steal=false' as "do not modify". For example, I don't really understand why these functions are carrying cutlen around and clones the packet before truncating if 'should_steal=false'. Some action executors also have optimizations that allows to not clone the packet before the fatal action if this action is the last one. So, in general, yes, we need to get rid of these flags and accurately re-work all the packet paths. And yes, this would be not a small change. For now, I think, we need to find a less ugly as possible solution for existing crash (maybe the one that I suggested), so we will be able to backport it and continue working on correct reference counting. What do you think? Tony? Aaron? Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
