[ovs-dev] [PATCH] datapath: Fix tracking of flags seen in TCP flows.

[Jarno Rajahalme]

Thanks for fixing this,

  Jarno

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>

On Apr 7, 2014, at 3:35 PM, Ben Pfaff <blp at nicira.com> wrote:

> Flow statistics need to take into account the TCP flags from the packet
> currently being processed (in 'key'), not the TCP flags matched by the
> flow found in the kernel flow table (in 'flow').
> 
> This bug made the Open vSwitch userspace fin_timeout action have no effect
> in many cases.
> 
> Bug #1219516.
> Reported-by: Len Gao <leng at vmware.com>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> datapath/datapath.c |    4 ++--
> datapath/flow.c     |    4 ++--
> datapath/flow.h     |    5 +++--
> 3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index c6d42db..25edd7d 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2007-2013 Nicira, Inc.
> + * Copyright (c) 2007-2014 Nicira, Inc.
>  *
>  * This program is free software; you can redistribute it and/or
>  * modify it under the terms of version 2 of the GNU General Public
> @@ -278,7 +278,7 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
> 	OVS_CB(skb)->flow = flow;
> 	OVS_CB(skb)->pkt_key = &key;
> 
> -	ovs_flow_stats_update(OVS_CB(skb)->flow, skb);
> +	ovs_flow_stats_update(OVS_CB(skb)->flow, key.tp.flags, skb);
> 	ovs_execute_actions(dp, skb);
> 	stats_counter = &stats->n_hit;
> 
> diff --git a/datapath/flow.c b/datapath/flow.c
> index f270f3a..c52081b 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
> @@ -62,10 +62,10 @@ u64 ovs_flow_used_time(unsigned long flow_jiffies)
> 
> #define TCP_FLAGS_BE16(tp) (*(__be16 *)&tcp_flag_word(tp) & htons(0x0FFF))
> 
> -void ovs_flow_stats_update(struct sw_flow *flow, struct sk_buff *skb)
> +void ovs_flow_stats_update(struct sw_flow *flow, __be16 tcp_flags,
> +			   struct sk_buff *skb)
> {
> 	struct flow_stats *stats;
> -	__be16 tcp_flags = flow->key.tp.flags;
> 	int node = numa_node_id();
> 
> 	stats = rcu_dereference(flow->stats[node]);
> diff --git a/datapath/flow.h b/datapath/flow.h
> index 5043a6e..1bb6ce0 100644
> --- a/datapath/flow.h
> +++ b/datapath/flow.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2007-2013 Nicira, Inc.
> + * Copyright (c) 2007-2014 Nicira, Inc.
>  *
>  * This program is free software; you can redistribute it and/or
>  * modify it under the terms of version 2 of the GNU General Public
> @@ -182,7 +182,8 @@ struct arp_eth_header {
> 	unsigned char       ar_tip[4];		/* target IP address        */
> } __packed;
> 
> -void ovs_flow_stats_update(struct sw_flow *, struct sk_buff *);
> +void ovs_flow_stats_update(struct sw_flow *, __be16 tcp_flags,
> +			   struct sk_buff *);
> void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
> 			unsigned long *used, __be16 *tcp_flags);
> void ovs_flow_stats_clear(struct sw_flow *);
> -- 
> 1.7.10.4
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev