From abfec865566e6cce961cc8660de1ddfdc85dae5f Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 27 Jul 2010 10:02:07 -0700 Subject: [PATCH] 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. --- 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 | 4 +--- ofproto/ofproto.c | 5 ++--- 9 files changed, 15 insertions(+), 35 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index d0db5507..c3402849 100644 --- 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 a9c74220..7b3be47c 100644 --- 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 29a1ac47..f0e76975 100644 --- 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 13aa9225..c3ec4dcf 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 15ff0d55..3c22e6dc 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 490c46bc..72ee14f2 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 dd14a8b0..3d913af2 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 f3099a18..f6fad62b 100644 --- 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 52e4fe3b..461053ab 100644 --- 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); } } -- 2.30.2