[ovs-discuss] Where does complex packet_out/meters behaviour test belong?

[Ben Pfaff]

It looks like you followed up on this with a patch and that you and Ilya
worked it out.  If I should look deeper, let me know!

On Sun, May 30, 2021 at 10:40:27PM +0000, Tony van der Peet wrote:
> Thanks Ben. I have written a test and can confirm that latest OVS has a vswitchd crash in this case. After a long chain of calls starting with handle_packet_out calling  ofproto_packet_out_finish, and ending with dp_netdev_run_meter, dp_packet_delete is called to dispose of the message. Then handle_packet_out calls ofproto_packet_out_uninit which calls dp_packet_delete to delete the same message.
> 
> I am a bit stumped as to how to fix this. Stopping the meter code from deleting the packet would not work for other cases which don't seem to crash, stopping the packet_out code from deleting the packet would not work for when the meter is not invoked. Creating a copy of the packet wouldn't work because it would then have to be disposed of in the normal, non-meter case. Adding special flags would be a major change, as would reference counting on the packet buffer. About the best I can think of is a flag to cover this specific case, maybe added to the packet metadata, which would need to be initialised false, and then be set true by the packet_out code. Maybe call it "meter_do_not_delete"? I can try this to see if it works but wonder if packet_metadata can be altered just like that. Or if there are better ways of doing this.
> 
> Tony
> ________________________________________
> From: Ben Pfaff <blp at ovn.org>
> Sent: Saturday, 29 May 2021 5:27 a.m.
> To: Tony van der Peet
> Cc: bugs at openvswitch.org
> Subject: Re: [ovs-discuss] Where does complex packet_out/meters behaviour test belong?
> 
> On Fri, May 28, 2021 at 05:00:46AM +0000, Tony van der Peet wrote:
> > I want to add a unit test that checks what happens when a packet_out message with output port OFPP_TABLE is dropped due to the action of a meter (our version of vswitchd crashes when I run this test with our oftest framework).
> >
> >
> > Where do you suggest I put this test?
> 
> There are a few packet-out tests in http://scanmail.trustwave.com/?c=20988&d=iKix4E8S7LYunNE_BL4IXm1Ak3fMb7yMJ8K3pmQBJw&u=http%3a%2f%2fofproto-dpif%2eat perhaps just after
> this cluster:
> 
> ofproto-dpif.at:AT_SETUP([ofproto-dpif packet-out controller])
> ofproto-dpif.at:AT_SETUP([ofproto-dpif packet-out controller (patch port)])
> ofproto-dpif.at:AT_SETUP([ofproto-dpif packet-out pipeline match field (OpenFlow 1.5)])
> ofproto-dpif.at:AT_SETUP([ofproto-dpif packet-out goto_table])
> ofproto-dpif.at:AT_SETUP([ofproto-dpif packet-out table-miss (continue)])