On Fri, Jul 03, 2015 at 04:33:47AM -0700, Jarno Rajahalme wrote:
>
> > 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.
I think I'll just delete that in the current patch. It is derived from
the original patch that was submitted by the coauthor, which originally
allowed eviction based on importance to be enabled and disabled, but I
removed that feature (it didn't seem useful) and perhaps I screwed up
the test.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
