> On Jul 2, 2015, at 5:46 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> On Thu, Jul 02, 2015 at 06:19:00AM -0700, Jarno Rajahalme wrote:
>>> On Jun 24, 2015, at 10:57 AM, Ben Pfaff <b...@nicira.com> wrote:
>>> static bool choose_rule_to_evict(struct oftable *table, struct rule **rulep)
>>> OVS_REQUIRES(ofproto_mutex);
>>> -static uint32_t rule_eviction_priority(struct ofproto *ofproto, struct
>>> rule *)
>>> +static uint64_t rule_eviction_priority(struct ofproto *ofproto, struct
>>> rule *)
>>> OVS_REQUIRES(ofproto_mutex);;
>>
>> Could you remove the extra semicolon here?
>
> Thanks, fixed.
>
>>> +AT_CHECK([head -n 1 stderr | ofctl_strip], [0],
>>> + [OFPT_ERROR: OFPFMFC_TABLE_FULL
>>> +])
>>> +# Now set the eviction on timeout basis.
>>> +AT_CHECK(
>>> + [ovs-vsctl \
>>> + -- --id=@t0 create Flow_Table flow-limit=4 overflow-policy=evict \
>>> + -- set bridge br0 flow_tables:0=@t0 \
>>> + | ${PERL} $srcdir/uuidfilt.pl],
>>> + [0], [<0>
>>> +])
>>> +#Now add a new flow
>>> +AT_CHECK([ovs-ofctl -O Openflow14 add-flow br0
>>> importance=37,hard_timeout=507,priority=11,in_port=6,actions=drop])
>>
>> Importance 37 is higher than the existing ones, so this would still evict
>> based on importance rather than timeout?
>
> I don't understand this comment, can you explain further?
>
I see that this test was removed by the next patch, so maybe this is moot.
Anyway, my reading of “Now set the eviction on timeout basis” was that the test
would cause eviction based on the timeout, rather than importance. My picking
on the importance of the new flow was wrong, but out of the existing flows, the
evicted flow still had the lowest priority, so the eviction happened on the
basis of the priority rather than timeout. To test the timeout piece, there
should have been multiple flows at the lowest priority, I presume.
Maybe the fact that I snipped the flow dumps did not help either. Here they are:
Before:
+AT_CHECK([ovs-ofctl -O Openflow14 dump-flows br0 | ofctl_strip | sort], [0],
[dnl
+ hard_timeout=502, importance=32, priority=7 actions=drop
+ hard_timeout=503, importance=33, priority=8 actions=drop
+ hard_timeout=504, importance=34, priority=9 actions=drop
+ hard_timeout=505, importance=35, priority=10,in_port=2 actions=NORMAL
+OFPST_FLOW reply (OF1.4):
+])
After adding the new flow:
+AT_CHECK([ovs-ofctl -O Openflow14 dump-flows br0 | ofctl_strip | sort], [0],
[dnl
+ hard_timeout=503, importance=33, priority=8 actions=drop
+ hard_timeout=504, importance=34, priority=9 actions=drop
+ hard_timeout=505, importance=35, priority=10,in_port=2 actions=NORMAL
+ hard_timeout=507, importance=37, priority=11,in_port=6 actions=drop
+OFPST_FLOW reply (OF1.4):
+])
Jarno
>> Assuming this is fixed:
>>
>> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
> Thanks for the review!
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
