Hi Joe, I re sent the patch via my private email to avoid the legal warning. The diff was generated with 'svn diff' based on change from the OVS 2.3.0 tar ball. I tested that meters get cleared. My project does not deal with groups. I added a flush for them per Ben's request but I could not test this part.
I am not familiar with the locking logic in OVS it is possible that delete_group requires ofproto_mutex. Regards, Gur From: Joe Stringer [mailto:joestrin...@nicira.com] Sent: Wednesday, October 01, 2014 12:08 To: Gur Stavi Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH] ofproto: flush groups and meters in ofproto_flush__ Hi Gur, looks straightforward. I have a few broad feedback points though: If you haven't read it yet, please take a look at the CONTRIBUTING file in the top of the source tree. I'm not sure what format this patch is in, but git won't accept it in my setup. I suspect it's due to the footer message that your email client has added. The footer message doesn't look consistent with the licence of Open vSwitch, which involves distributing and copying the code. It's surprising to me that delete_group() is performed without holding the ofproto_mutex. Thanks, Joe On 1 October 2014 18:28, Gur Stavi <gst...@mrv.com<mailto:gst...@mrv.com>> wrote: In fail-open mode on disconnect from controller rules are flushed. It makes sense to flush groups and meters as well. Signed-off-by: Gur Stavi gst...@mrv.com<mailto:gst...@mrv.com><mailto:gst...@mrv.com<mailto:gst...@mrv.com>> --- Original issue was discussed here: https://www.mail-archive.com/discuss@openvswitch.org/msg10930.html Index: ofproto/ofproto.c =================================================================== --- ofproto/ofproto.c +++ ofproto/ofproto.c @@ -1276,6 +1276,8 @@ ovs_mutex_unlock(&ofproto_mutex); } +static void delete_group(struct ofproto *ofproto, uint32_t group_id); + static void ofproto_flush__(struct ofproto *ofproto) OVS_EXCLUDED(ofproto_mutex) @@ -1305,10 +1307,18 @@ } } ovs_mutex_unlock(&ofproto_mutex); + + /* Flush groups */ + delete_group(ofproto, OFPG_ALL); + + /* Flush meters */ + ovs_mutex_lock(&ofproto_mutex); + if (ofproto->meters) { + meter_delete(ofproto, 1, ofproto->meter_features.max_meters); + } + ovs_mutex_unlock(&ofproto_mutex); } -static void delete_group(struct ofproto *ofproto, uint32_t group_id); - static void ofproto_destroy__(struct ofproto *ofproto) OVS_EXCLUDED(ofproto_mutex) [E-Banner]<http://www.mrv.com/products/optidriver> The contents of this message, together with any attachments, are intended only for the use of the person(s) to whom they are addressed and may contain confidential and/or privileged information. If you are not the intended recipient, immediately advise the sender, delete this message and any attachments and note that any distribution, or copying of this message, or any attachment, is prohibited. _______________________________________________ dev mailing list dev@openvswitch.org<mailto:dev@openvswitch.org> http://openvswitch.org/mailman/listinfo/dev [E-Banner]<http://www.mrv.com/products/optidriver> The contents of this message, together with any attachments, are intended only for the use of the person(s) to whom they are addressed and may contain confidential and/or privileged information. If you are not the intended recipient, immediately advise the sender, delete this message and any attachments and note that any distribution, or copying of this message, or any attachment, is prohibited. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
