Ilya Maximets <i.maxim...@ovn.org> writes: > 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.
+1 from me - it's a bigger scoped change, but over-all, I think it's a better one, and makes the most sense. > 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? That makes sense to me, and I hope we will actually work on the ref-count stuff. I had started taking a look a few weeks back, but the idea of 'steal' is pretty ingrained throughout the code, so getting a ref-count semantic correct is a big effort. As an example, the odp-execute area expects that each sub-action will have its own copy to modify (and this is documented with the 'should_steal' semantics). Taking that out will be a big rework in that area. I think it makes the most sense, and we probably could implement something like COW to cover those cases where we really need to modify a packet without propagating those changes back up. > Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
