[ovs-dev] [loops 1/6] datapath: Don't track IP TOS value two different ways.

Ben Pfaff blp at nicira.com
Fri Jul 23 23:44:53 UTC 2010


Originally, the datapath didn't care about IP TOS at all.  Then, to support
NetFlow, we made it keep track of the last-seen IP TOS value on a per-flow
basis.  Then, to support OpenFlow 1.0, we added a nw_tos field to
odp_flow_key.  We don't need both methods, so this commit drops the
NetFlow-specific tracking.

This introduces a small kernel ABI break: upgrading the kernel module
without upgrading the OVS userspace will mean that NetFlow records will
all show an IP TOS value of 0.  I don't consider that to be a serious
problem.
---
 datapath/datapath.c                     |    3 +--
 datapath/flow.c                         |   11 ++++-------
 datapath/flow.h                         |    2 --
 include/openvswitch/datapath-protocol.h |    2 +-
 lib/dpif-netdev.c                       |   15 ++++-----------
 lib/flow.c                              |    2 --
 ofproto/netflow.c                       |    6 ++----
 ofproto/netflow.h                       |    3 +--
 ofproto/ofproto.c                       |    5 ++---
 9 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index eb260e3..e05608a 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -914,7 +914,7 @@ static void get_stats(struct sw_flow *flow, struct odp_flow_stats *stats)
 	}
 	stats->n_packets = flow->packet_count;
 	stats->n_bytes = flow->byte_count;
-	stats->ip_tos = flow->ip_tos;
+	stats->reserved = 0;
 	stats->tcp_flags = flow->tcp_flags;
 	stats->error = 0;
 }
@@ -923,7 +923,6 @@ static void clear_stats(struct sw_flow *flow)
 {
 	flow->used.tv_sec = flow->used.tv_nsec = 0;
 	flow->tcp_flags = 0;
-	flow->ip_tos = 0;
 	flow->packet_count = 0;
 	flow->byte_count = 0;
 }
diff --git a/datapath/flow.c b/datapath/flow.c
index bf50e94..9a25837 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -101,13 +101,10 @@ void flow_used(struct sw_flow *flow, struct sk_buff *skb)
 {
 	u8 tcp_flags = 0;
 
-	if (flow->key.dl_type == htons(ETH_P_IP) && iphdr_ok(skb)) {
-		struct iphdr *nh = ip_hdr(skb);
-		flow->ip_tos = nh->tos;
-		if (flow->key.nw_proto == IPPROTO_TCP && tcphdr_ok(skb)) {
-			u8 *tcp = (u8 *)tcp_hdr(skb);
-			tcp_flags = *(tcp + TCP_FLAGS_OFFSET) & TCP_FLAG_MASK;
-		}
+	if (flow->key.dl_type == htons(ETH_P_IP) && iphdr_ok(skb) &&
+	    flow->key.nw_proto == IPPROTO_TCP && tcphdr_ok(skb)) {
+		u8 *tcp = (u8 *)tcp_hdr(skb);
+		tcp_flags = *(tcp + TCP_FLAGS_OFFSET) & TCP_FLAG_MASK;
 	}
 
 	spin_lock_bh(&flow->lock);
diff --git a/datapath/flow.h b/datapath/flow.h
index 9704489..dcca0c4 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -36,8 +36,6 @@ struct sw_flow {
 
 	struct timespec used;	/* Last used time. */
 
-	u8 ip_tos;		/* IP TOS value. */
-
 	spinlock_t lock;	/* Lock for values below. */
 	u64 packet_count;	/* Number of packets matched. */
 	u64 byte_count;		/* Number of bytes matched. */
diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index 13aa922..c3ec4dc 100644
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -210,7 +210,7 @@ struct odp_flow_stats {
     uint64_t used_sec;          /* Time last used, in system monotonic time. */
     uint32_t used_nsec;
     uint8_t  tcp_flags;
-    uint8_t  ip_tos;
+    uint8_t  reserved;
     uint16_t error;             /* Used by ODP_FLOW_GET. */
 };
 
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 15ff0d5..3c22e6d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -102,7 +102,6 @@ struct dp_netdev_flow {
 	struct timespec used;       /* Last used time. */
 	long long int packet_count; /* Number of packets matched. */
 	long long int byte_count;   /* Number of bytes matched. */
-	uint8_t ip_tos;             /* IP TOS value. */
 	uint16_t tcp_ctl;           /* Bitwise-OR of seen tcp_ctl values. */
 
     /* Actions. */
@@ -682,7 +681,7 @@ answer_flow_query(struct dp_netdev_flow *flow, uint32_t query_flags,
         odp_flow->stats.used_sec = flow->used.tv_sec;
         odp_flow->stats.used_nsec = flow->used.tv_nsec;
         odp_flow->stats.tcp_flags = TCP_FLAGS(flow->tcp_ctl);
-        odp_flow->stats.ip_tos = flow->ip_tos;
+        odp_flow->stats.reserved = 0;
         odp_flow->stats.error = 0;
         if (odp_flow->n_actions > 0) {
             unsigned int n = MIN(odp_flow->n_actions, flow->n_actions);
@@ -829,7 +828,6 @@ clear_stats(struct dp_netdev_flow *flow)
     flow->used.tv_nsec = 0;
     flow->packet_count = 0;
     flow->byte_count = 0;
-    flow->ip_tos = 0;
     flow->tcp_ctl = 0;
 }
 
@@ -1006,14 +1004,9 @@ dp_netdev_flow_used(struct dp_netdev_flow *flow, const flow_t *key,
     time_timespec(&flow->used);
     flow->packet_count++;
     flow->byte_count += packet->size;
-    if (key->dl_type == htons(ETH_TYPE_IP)) {
-        struct ip_header *nh = packet->l3;
-        flow->ip_tos = nh->ip_tos;
-
-        if (key->nw_proto == IPPROTO_TCP) {
-            struct tcp_header *th = packet->l4;
-            flow->tcp_ctl |= th->tcp_ctl;
-        }
+    if (key->dl_type == htons(ETH_TYPE_IP) && key->nw_proto == IPPROTO_TCP) {
+        struct tcp_header *th = packet->l4;
+        flow->tcp_ctl |= th->tcp_ctl;
     }
 }
 
diff --git a/lib/flow.c b/lib/flow.c
index 490c46b..72ee14f 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -231,8 +231,6 @@ flow_extract_stats(const flow_t *flow, struct ofpbuf *packet,
     memset(stats, '\0', sizeof(*stats));
 
     if ((flow->dl_type == htons(ETH_TYPE_IP)) && packet->l4) {
-        struct ip_header *ip = packet->l3;
-        stats->ip_tos = ip->ip_tos;
         if ((flow->nw_proto == IP_TYPE_TCP) && packet->l7) {
             struct tcp_header *tcp = packet->l4;
             stats->tcp_flags = TCP_FLAGS(tcp->tcp_ctl);
diff --git a/ofproto/netflow.c b/ofproto/netflow.c
index dd14a8b..3d913af 100644
--- a/ofproto/netflow.c
+++ b/ofproto/netflow.c
@@ -170,7 +170,7 @@ netflow_expire(struct netflow *nf, struct netflow_flow *nf_flow,
     }
     nf_rec->tcp_flags = nf_flow->tcp_flags;
     nf_rec->ip_proto = expired->flow.nw_proto;
-    nf_rec->ip_tos = nf_flow->ip_tos;
+    nf_rec->ip_tos = expired->flow.nw_tos;
 
     /* Update flow tracking data. */
     nf_flow->created = 0;
@@ -271,10 +271,8 @@ netflow_flow_update_time(struct netflow *nf, struct netflow_flow *nf_flow,
 }
 
 void
-netflow_flow_update_flags(struct netflow_flow *nf_flow, uint8_t ip_tos,
-                          uint8_t tcp_flags)
+netflow_flow_update_flags(struct netflow_flow *nf_flow, uint8_t tcp_flags)
 {
-    nf_flow->ip_tos = ip_tos;
     nf_flow->tcp_flags |= tcp_flags;
 }
 
diff --git a/ofproto/netflow.h b/ofproto/netflow.h
index f3099a1..7b5c1cb 100644
--- a/ofproto/netflow.h
+++ b/ofproto/netflow.h
@@ -66,8 +66,7 @@ void netflow_run(struct netflow *);
 void netflow_flow_clear(struct netflow_flow *);
 void netflow_flow_update_time(struct netflow *, struct netflow_flow *,
                               long long int used);
-void netflow_flow_update_flags(struct netflow_flow *, uint8_t ip_tos,
-                               uint8_t tcp_flags);
+void netflow_flow_update_flags(struct netflow_flow *, uint8_t tcp_flags);
 bool netflow_active_timeout_expired(struct netflow *, struct netflow_flow *);
 
 #endif /* netflow.h */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 66b957e..efdcbed 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3371,8 +3371,7 @@ update_stats(struct ofproto *ofproto, struct rule *rule,
         update_time(ofproto, rule, stats);
         rule->packet_count += stats->n_packets;
         rule->byte_count += stats->n_bytes;
-        netflow_flow_update_flags(&rule->nf_flow, stats->ip_tos,
-                                  stats->tcp_flags);
+        netflow_flow_update_flags(&rule->nf_flow, stats->tcp_flags);
     }
 }
 
@@ -4194,7 +4193,7 @@ active_timeout(struct ofproto *ofproto, struct rule *rule)
 
             if (odp_flow.stats.n_packets) {
                 update_time(ofproto, rule, &odp_flow.stats);
-                netflow_flow_update_flags(&rule->nf_flow, odp_flow.stats.ip_tos,
+                netflow_flow_update_flags(&rule->nf_flow,
                                           odp_flow.stats.tcp_flags);
             }
         }
-- 
1.7.1





More information about the dev mailing list