[ovs-dev] [PATCH v2 11/15] ofproto: Use ofproto_flow_mod for learn execution from xlate cache.

Jarno Rajahalme jarno at ovn.org
Sat Sep 10 00:10:11 UTC 2016


I’ll rebase this for v3 anyway, but see the answers to the questions below:

> On Aug 29, 2016, at 4:57 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Mon, Aug 22, 2016 at 04:31:37PM -0700, Jarno Rajahalme wrote:
>> Use ofproto_flow_mod with a reference to an existing or new rule
>> instead of ofputil_flow_mod for learn action execution from xlate
>> cache
>> 
>> Typically we would find that when a learn xlate cache entry is
>> created, a preceding upcall has already created the learned flow.  In
>> this case the xlate cache entry takes a reference to that flow and
>> keeps refreshing it without needing to perform any flow table lookups.
>> This is both faster and shrinks the memory cost of each learn cache
>> entry from ~3.5kb to about 0.3kb.
>> 
>> If the learned rule does not yet exist, it is created and attached to
>> the ofproto_flow_mod, from which it is then added.  If the referred
>> rule happens to expire and is removed from the classifier tables, we
>> create a new rule using the old rule as a template, so that we can
>> avoid storing the ofputil_flow_mod in all cases.
>> 
>> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
> 
> xlate_learn_action() is the only user of learn_execute().  Would there
> be any advantage in having learn_execute() provide its output in some
> form other than struct ofputil_flow_mod?
> 

No, since the ofproto_flow_mod_init() takes struct ofputil_flow_mod as an input anyway.

> Should the log message from the failure case in xlate_learn_action()
> report the actual error?
> 

Added.

> In the state == RULE_INITIALIZED case, ofproto_flow_mod_learn() does not
> report any error it encounters to the caller (it has a hard-coded
> "return 0;”)

> In the state == RULE_INITIALIZED case, there's an inner "enum ofperr

> error;" shadowing an outer one.
> 

Both fixed for v3.

Thanks for the review!

> Acked-by: Ben Pfaff <blp at ovn.org <mailto:blp at ovn.org>>




More information about the dev mailing list