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

Tony van der Peet Tony.vanderPeet at alliedtelesis.co.nz
Mon May 31 02:39:55 UTC 2021


Just been thinking about this a bit more and the lowest level at which we know it's a packet_out we are processing is packet_execute, which calls dpif_execute passing a struct execute. This might be a better place to put a "meter_do_not_delete" flag (or maybe a "packet_out" flag)?

Tony
________________________________________
From: Tony van der Peet
Sent: Monday, 31 May 2021 10:40 a.m.
To: Ben Pfaff
Cc: bugs at openvswitch.org
Subject: Re: [ovs-discuss] Where does complex packet_out/meters behaviour test belong?

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)])


More information about the discuss mailing list