[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