datapath: Don't track IP TOS value two different ways.

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.

diff --git a/datapath/datapath.c b/datapath/datapath.c
index d0db55078237ff8d7ececbbb5ab1703e9f9f415d..c3402849840d6436992b7a1cdd8d1503924aa24b 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -926,7 +926,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;
 }
@@ -935,7 +935,6 @@ static void clear_stats(struct sw_flow *flow)
 {
        flow->used = 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 a9c742208d6401942ad49cc8c14bef0e958115ef..7b3be47c94af7c254b7c67523fb8bba3cd16eabd 100644 (file)
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -96,13 +96,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) &&
+           flow->key.nw_proto == IPPROTO_TCP) {
+               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 29a1ac47bcc76612b686b815079adbf1bfd5bafe..f0e7697577ef349787e84303fa6a1744ae41d9fd 100644 (file)
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -35,8 +35,6 @@ struct sw_flow {
        struct odp_flow_key key;
        struct sw_flow_actions *sf_acts;
 
-       u8 ip_tos;              /* IP TOS value. */
-
        spinlock_t lock;        /* Lock for values below. */
        unsigned long used;     /* Last used time (in jiffies). */
        u64 packet_count;       /* Number of packets matched. */

diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index 13aa92251d07e8fccbc6a884060e7d24d9882076..c3ec4dcf2f7a88417aa592b65532d94966b41d74 100644 (file)
--- 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 15ff0d551289380b36888528354413bfdb088b8b..3c22e6dc1b72dc1af770a5b007ce523c78ae9d50 100644 (file)
--- 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 490c46bc1770a6960771b877a928557df3591629..72ee14f2a087f33dc66b718116a8af4e621f843c 100644 (file)
--- 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 dd14a8b0d121446673915f51914f185c05e738fd..3d913af2c609fc23d032e8305ddf7f12a4c0405f 100644 (file)
--- 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 f3099a184afd7bfc39ccf509f8c151346efd4572..f6fad62b092f92eb0d8e6ce7c8e6bd5449b9b887 100644 (file)
--- a/ofproto/netflow.h
+++ b/ofproto/netflow.h
@@ -52,7 +52,6 @@ struct netflow_flow {
     uint64_t byte_count_off;      /* Byte count at last time out. */
 
     uint16_t output_iface;        /* Output interface index. */
-    uint8_t ip_tos;               /* Last-seen IP type-of-service. */
     uint8_t tcp_flags;            /* Bitwise-OR of all TCP flags seen. */
 };
 
@@ -66,8 +65,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 52e4fe3b622ad9f9ec435fceb1c1419f512f7444..461053ab430e80d5d381797fd1235f1b9719f76b 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3388,8 +3388,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);
     }
 }
 
@@ -4211,7 +4210,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);
             }
         }