On 28 September 2016 at 14:22, Daniele Di Proietto <diproiet...@ovn.org> wrote:
>
>
> 2016-09-20 18:47 GMT-07:00 Joe Stringer <j...@ovn.org>:
>>
>> To make more of the core revalidate() functions do just one thing and
>> not modify state on the way, refactor them to prepare the xcache then
>> defer the ukey modification and stats/side effects execution to the end
>> of successful revalidation.
>>
>> If revalidation causes deletion, then the xcache will be prepared and
>> attached to the ukey, but the actual execution will be skipped since it
>> will be executed on flow_delete very soon anyway with final stats.
>>
>> Signed-off-by: Joe Stringer <j...@ovn.org>
>> ---
>> ofproto/ofproto-dpif-upcall.c | 56
>> ++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 42 insertions(+), 14 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>> index 1caff84cfd38..eaf7c3b4dadc 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -1877,16 +1877,41 @@ xlate_key(struct udpif *udpif, const struct nlattr
>> *key, unsigned int len,
>>
>> static int
>> xlate_ukey(struct udpif *udpif, const struct udpif_key *ukey,
>> - const struct dpif_flow_stats *push, struct reval_context *ctx)
>> + uint16_t tcp_flags, struct reval_context *ctx)
>> {
>> - return xlate_key(udpif, ukey->key, ukey->key_len, push, ctx);
>> + struct dpif_flow_stats push = {
>> + .tcp_flags = tcp_flags,
>> + };
>>
>> + return xlate_key(udpif, ukey->key, ukey->key_len, &push, ctx);
>> +}
>> +
>> +static int
>> +populate_xcache(struct udpif *udpif, struct udpif_key *ukey,
>> + uint16_t tcp_flags)
>> + OVS_REQUIRES(ukey->mutex)
>> +{
>> + struct reval_context ctx = {
>> + .odp_actions = NULL,
>> + .netflow = NULL,
>> + .wc = NULL,
>> + };
>> + int error;
>> +
>> + ovs_assert(!ukey->xcache);
>> + ukey->xcache = ctx.xcache = xlate_cache_new();
>> + error = xlate_ukey(udpif, ukey, tcp_flags, &ctx);
>> + if (error) {
>> + return error;
>> + }
>> + xlate_out_uninit(&ctx.xout);
>> +
>> + return 0;
>> }
>>
>> static enum reval_result
>> revalidate_ukey__(struct udpif *udpif, struct udpif_key *ukey,
>> - const struct dpif_flow_stats *push,
>> - struct ofpbuf *odp_actions, uint64_t reval_seq,
>> - struct recirc_refs *recircs)
>> + uint16_t tcp_flags, struct ofpbuf *odp_actions,
>> + uint64_t reval_seq, struct recirc_refs *recircs)
>> OVS_REQUIRES(ukey->mutex)
>> {
>> bool need_revalidate = (ukey->reval_seq != reval_seq);
>> @@ -1911,7 +1936,7 @@ revalidate_ukey__(struct udpif *udpif, struct
>> udpif_key *ukey,
>> ukey->xcache = xlate_cache_new();
>> }
>> ctx.xcache = ukey->xcache;
>> - if (xlate_ukey(udpif, ukey, push, &ctx)) {
>> + if (xlate_ukey(udpif, ukey, tcp_flags, &ctx)) {
>> goto exit;
>> }
>> xoutp = &ctx.xout;
>> @@ -2008,23 +2033,26 @@ revalidate_ukey(struct udpif *udpif, struct
>> udpif_key *ukey,
>> }
>>
>> /* We will push the stats, so update the ukey stats cache. */
>
>
> I guess we should remove the comment too?
>
>>
>> - ukey->stats = *stats;
>>
>> if (!push.n_packets && !need_revalidate) {
>> result = UKEY_KEEP;
>> goto exit;
>> }
>
>
> I think the 4 lines above can be removed, because the condition is covered
> by the lines immediately below.
Good catches, yes I missed these. Thanks!
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
