On Sep 23, 2013, at 8:39 PM, Ben Pfaff <b...@nicira.com> wrote:

> [resend with Luca's email address fixed]
> 
> On Mon, Sep 23, 2013 at 08:35:05PM -0700, Ben Pfaff wrote:
>> Commit 97f63e57a8 (ofproto: Remove soon-to-be-invalid optimizations.)
>> changed ofproto_delete_flow() to use the general-purpose flow_mod
>> implementation.  However, the general-purpose flow_mod never matches hidden
>> flows, which are exactly the flows that ofproto_delete_flow()'s callers
>> want to delete.
>> 
>> This commit fixes the problem by allowing flow_mods that specify a priority
>> that can only be for a hidden flow to delete a hidden flow.  (This doesn't
>> allow OpenFlow clients to meddle with hidden flows because OpenFlow uses
>> only a 16-bit field to specify priority.)
>> 
>> I verified that, without this commit, if I change from one controller to
>> another with "ovs-vsctl set-controller", then the in-band rules for the
>> old controller remain.  With this commit, the old rules are removed.
>> 
>> Reported-by: YAMAMOTO Takashi <yamam...@valinux.co.jp>
>> Bug #19821.
>> Reported-by: Luca Giraudo <lgira...@vmware.com>
>> Bug #19888.
>> Reported-by: Ying Chen <yingc...@vmware.com>
>> Signed-off-by: Ben Pfaff <b...@nicira.com>
>> ---
>> ofproto/ofproto.c |    2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> index cfe8d2a..fcb3bca 100644
>> --- a/ofproto/ofproto.c
>> +++ b/ofproto/ofproto.c
>> @@ -3239,7 +3239,7 @@ collect_rule(struct rule *rule, const struct 
>> rule_criteria *c,
>>              struct rule_collection *rules)
>>     OVS_REQUIRES(ofproto_mutex)
>> {
>> -    if (ofproto_rule_is_hidden(rule)) {
>> +    if (ofproto_rule_is_hidden(rule) && c->cr.priority <= UINT16_MAX) {

It might be nice to add a comment that basically rephrases the second paragraph 
in your commit message about why you want to do this and why regular OpenFlow 
clients can't match hidden flows.  Without it, I think the code looks a little 
surprising.

Acked-by: Justin Pettit <jpet...@nicira.com>

Thanks,

--Justin


_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to