[ovs-dev] OpenFlow rule deletion during port destroy

[Zoltan Kiss]

On 18/04/14 02:42, Jarno Rajahalme wrote:
>
> On Apr 17, 2014, at 11:26 AM, Zoltan Kiss <zoltan.kiss at citrix.com> wrote:
>
>> On 16/04/14 18:00, Justin Pettit wrote:
>>> On April 16, 2014 at 9:00:15 AM, Zoltan Kiss (zoltan.kiss at citrix.com) wrote:
>>>
>>>> My actual problem is that an important rule gets deleted:
>>>>
>>>> cookie=0x0, duration=1581.083s, table=0, n_packets=52804,
>>>> n_bytes=88968151, idle_age=0, priority=0,in_port=ANY actions=NORMAL
>>>>
>>>> ...
>>>>
>>>> There is no sign the controller did anything about deleting those rules,
>>>> but somehow it still happened. Does anyone knows
>>>> Unfortunately it is hard to reproduce the problem, it is only
>>>> intermittent in one of our testcases.
>>>
>>> I'm not sure that it's related, but it sounds similar to bug NIC-512 that I filed with Citrix over a year ago.  Here's the relevant part:
>> Many thanks Justin it was this problem indeed! Now I poked the right people to fix this quite forgotten problem!
>>
>>> I did commit a change that should lessen the impact:
>>>
>>>    http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=05dddba
>> Yes, this would solve the problem, but this patch removed it on the assumption that ofputil_port_from_string() solved it anyway:
>>
>> http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=33ab38d9
>>
>> But it doesn't, in fact it does some pretty mad conversions:
>> - ofputil_port_from_string pass a uint32_t* to str_to_uint
>> - it is casted to an int*, which is fortunately 32 bit in most places, but it is a dangerous assumption
>> - the string is converted to a 'long long' and then copied into that int*
>> - so if the string was "-1", the value of port32 will be 0xFFFFFFFF
>>
>> I think the best place to catch this problem would be to check in str_to_uint() if the returned '(int *) u' is a negative number, and return false in that case. What do you think?
>>
>
> It seems that the check for the minus sign needs to be done in ofputil_port_from_string(). The check for negative return values from str_to_uint() would not work as the special port numbers for OpenFlow 1.1+ are in that “negative” range.
>
> I just posted this patch, could you verify it solves your problem:
>
> http://openvswitch.org/pipermail/dev/2014-April/039070.html
Yes, it seems it solves the problem, but I haven't tested it yet. And 
that conversion madness is still a potential problem source. I'll send a 
patch shortly to propose a fix for it.

Zoli

>
>    Jarno
>
>