[ovs-dev] [PATCH] dpif-netdev: Fix crash when PACKET_OUT is metered

Ilya Maximets i.maximets at ovn.org
Fri Jun 18 13:54:17 UTC 2021


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.


More information about the dev mailing list