I discussed this change with Alex, and I think there's a flaw with my
reasoning behind whether to re-create the xcache or not.
Consider an openflow pipeline with one rule that has actions A and B.
- We install a datapath flow to do A and B.
- Replace the rule in the pipeline with two rules, one that does A, and one
that does B. (I think it's possible to do this such that the datapath flow
would be the same).
- This causes revalidation. The flow is valid, but the xcache is not.
- In this version of the patch, we would keep a reference to the initial
rule and continue to attribute stats to it rather than fixing it when we
revalidate.
So, I think that the xcache should be used before revalidation, then
cleared/re-built.
On 5 June 2014 06:32, Ethan Jackson <et...@nicira.com> wrote:
> Acked
>
>
> On Tuesday, June 3, 2014, Joe Stringer <joestrin...@nicira.com> wrote:
>
>> One of the reasons that xlate_cache was introduced was to ensure that
>> statistics were attributed to the correct rules and interfaces according
>> to the flow that was installed into the datapath, rather than according
>> to the current state of the flow table.
>>
>> This patch makes the revalidators use the xlate_cache to attribute stats
>> when full revalidation is required. Furthermore, rather than deleting
>> the xcache when revalidating, keep it around.
>>
>> Signed-off-by: Joe Stringer <joestrin...@nicira.com>
>> ---
>> v2: Don't delete the old xcache when revalidating.
>> Removed extraneous "ukey->xcache = NULL".
>> ---
>> ofproto/ofproto-dpif-upcall.c | 23 ++++++++++++-----------
>> 1 file changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>> index 90e18e3..b4aa48b 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -1148,10 +1148,16 @@ revalidate_ukey(struct udpif *udpif, struct
>> udpif_key *ukey,
>> }
>>
>> may_learn = push.n_packets > 0;
>> - if (ukey->xcache && !udpif->need_revalidate) {
>> + if (ukey->xcache) {
>> xlate_push_stats(ukey->xcache, may_learn, &push);
>> - ok = true;
>> - goto exit;
>> + if (udpif->need_revalidate) {
>> + push.n_packets = 0;
>> + push.n_bytes = 0;
>> + may_learn = false;
>> + } else {
>> + ok = true;
>> + goto exit;
>> + }
>> }
>>
>> error = xlate_receive(udpif->backer, NULL, ukey->key, ukey->key_len,
>> &flow,
>> @@ -1160,16 +1166,11 @@ revalidate_ukey(struct udpif *udpif, struct
>> udpif_key *ukey,
>> goto exit;
>> }
>>
>> - if (udpif->need_revalidate) {
>> - xlate_cache_clear(ukey->xcache);
>> - }
>> - if (!ukey->xcache) {
>> - ukey->xcache = xlate_cache_new();
>> - }
>> -
>> xlate_in_init(&xin, ofproto, &flow, NULL, push.tcp_flags, NULL);
>> xin.resubmit_stats = push.n_packets ? &push : NULL;
>> - xin.xcache = ukey->xcache;
>> + if (!ukey->xcache) {
>> + ukey->xcache = xin.xcache = xlate_cache_new();
>> + }
>> xin.may_learn = may_learn;
>> xin.skip_wildcards = !udpif->need_revalidate;
>> xlate_actions(&xin, &xout);
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
