diff options
| -rw-r--r-- | lib/dpif-netdev.c | 1 | ||||
| -rw-r--r-- | lib/dpif-netlink.c | 1 | ||||
| -rw-r--r-- | lib/dpif-provider.h | 8 | ||||
| -rw-r--r-- | lib/dpif.c | 6 | ||||
| -rw-r--r-- | lib/dpif.h | 1 | ||||
| -rw-r--r-- | ofproto/ofproto-dpif-upcall.c | 41 | ||||
| -rw-r--r-- | tests/system-offloads-traffic.at | 60 |
7 files changed, 116 insertions, 2 deletions
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index c9f7179c3..aed2c8fbb 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -9616,6 +9616,7 @@ dpif_netdev_bond_stats_get(struct dpif *dpif, uint32_t bond_id, const struct dpif_class dpif_netdev_class = { "netdev", true, /* cleanup_required */ + true, /* synced_dp_layers */ dpif_netdev_init, dpif_netdev_enumerate, dpif_netdev_port_open_type, diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 586fb8893..7875e573e 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -4515,6 +4515,7 @@ dpif_netlink_cache_set_size(struct dpif *dpif_, uint32_t level, uint32_t size) const struct dpif_class dpif_netlink_class = { "system", false, /* cleanup_required */ + false, /* synced_dp_layers */ NULL, /* init */ dpif_netlink_enumerate, NULL, diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 12477a24f..b8ead8a02 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -127,6 +127,14 @@ struct dpif_class { * datapaths that can not exist without it (e.g. netdev datapath). */ bool cleanup_required; + /* If 'true' the specific dpif implementation synchronizes the various + * datapath implementation layers, i.e., the dpif's layer in combination + * with the underlying netdev offload layers. For example, dpif-netlink + * does not sync its kernel flows with the tc ones, i.e., only one gets + * installed. On the other hand, dpif-netdev installs both flows, + * internally keeps track of both, and represents them as one. */ + bool synced_dp_layers; + /* Called when the dpif provider is registered, typically at program * startup. Returning an error from this function will prevent any * datapath with this class from being created. diff --git a/lib/dpif.c b/lib/dpif.c index fe4db83fb..3305401fe 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -2109,3 +2109,9 @@ dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t size) ? dpif->dpif_class->cache_set_size(dpif, level, size) : EOPNOTSUPP; } + +bool +dpif_synced_dp_layers(struct dpif *dpif) +{ + return dpif->dpif_class->synced_dp_layers; +} diff --git a/lib/dpif.h b/lib/dpif.h index 6cb4dae6d..129cbf6a1 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -939,6 +939,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif, odp_port_t port_no, char *dpif_get_dp_version(const struct dpif *); bool dpif_supports_tnl_push_pop(const struct dpif *); bool dpif_supports_explicit_drop_action(const struct dpif *); +bool dpif_synced_dp_layers(struct dpif *); /* Log functions. */ struct vlog_module; diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index fc94078cb..4031e766f 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -48,17 +48,20 @@ #define UPCALL_MAX_BATCH 64 #define REVALIDATE_MAX_BATCH 50 +#define UINT64_THREE_QUARTERS (UINT64_MAX / 4 * 3) VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall); COVERAGE_DEFINE(dumped_duplicate_flow); COVERAGE_DEFINE(dumped_new_flow); COVERAGE_DEFINE(handler_duplicate_upcall); -COVERAGE_DEFINE(upcall_ukey_contention); -COVERAGE_DEFINE(upcall_ukey_replace); COVERAGE_DEFINE(revalidate_missed_dp_flow); +COVERAGE_DEFINE(ukey_dp_change); +COVERAGE_DEFINE(ukey_invalid_stat_reset); COVERAGE_DEFINE(upcall_flow_limit_hit); COVERAGE_DEFINE(upcall_flow_limit_kill); +COVERAGE_DEFINE(upcall_ukey_contention); +COVERAGE_DEFINE(upcall_ukey_replace); /* A thread that reads upcalls from dpif, forwards each upcall's packet, * and possibly sets up a kernel flow as a cache. */ @@ -288,6 +291,7 @@ struct udpif_key { struct ovs_mutex mutex; /* Guards the following. */ struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/ + const char *dp_layer OVS_GUARDED; /* Last known dp_layer. */ long long int created OVS_GUARDED; /* Estimate of creation time. */ uint64_t dump_seq OVS_GUARDED; /* Tracks udpif->dump_seq. */ uint64_t reval_seq OVS_GUARDED; /* Tracks udpif->reval_seq. */ @@ -1771,6 +1775,7 @@ ukey_create__(const struct nlattr *key, size_t key_len, ukey->created = ukey->flow_time = time_msec(); memset(&ukey->stats, 0, sizeof ukey->stats); ukey->stats.used = used; + ukey->dp_layer = NULL; ukey->xcache = NULL; ukey->offloaded = false; @@ -2357,6 +2362,13 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey, ? stats->n_bytes - ukey->stats.n_bytes : 0); + if (stats->n_packets < ukey->stats.n_packets && + ukey->stats.n_packets < UINT64_THREE_QUARTERS) { + /* Report cases where the packet counter is lower than the previous + * instance, but exclude the potential wrapping of an uint64_t. */ + COVERAGE_INC(ukey_invalid_stat_reset); + } + if (need_revalidate) { if (should_revalidate(udpif, push.n_packets, ukey->stats.used)) { if (!ukey->xcache) { @@ -2470,6 +2482,15 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, size_t n_ops) push->tcp_flags = stats->tcp_flags | op->ukey->stats.tcp_flags; push->n_packets = stats->n_packets - op->ukey->stats.n_packets; push->n_bytes = stats->n_bytes - op->ukey->stats.n_bytes; + + if (stats->n_packets < op->ukey->stats.n_packets && + op->ukey->stats.n_packets < UINT64_THREE_QUARTERS) { + /* Report cases where the packet counter is lower than the + * previous instance, but exclude the potential wrapping of an + * uint64_t. */ + COVERAGE_INC(ukey_invalid_stat_reset); + } + ovs_mutex_unlock(&op->ukey->mutex); } else { push = stats; @@ -2774,6 +2795,22 @@ revalidate(struct revalidator *revalidator) continue; } + ukey->offloaded = f->attrs.offloaded; + if (!ukey->dp_layer + || (!dpif_synced_dp_layers(udpif->dpif) + && strcmp(ukey->dp_layer, f->attrs.dp_layer))) { + + if (ukey->dp_layer) { + /* The dp_layer has changed this is probably due to an + * earlier revalidate cycle moving it to/from hw offload. + * In this case we should reset the ukey stored statistics, + * as they are from the deleted DP flow. */ + COVERAGE_INC(ukey_dp_change); + memset(&ukey->stats, 0, sizeof ukey->stats); + } + ukey->dp_layer = f->attrs.dp_layer; + } + already_dumped = ukey->dump_seq == dump_seq; if (already_dumped) { /* The flow has already been handled during this flow dump diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index 7558812eb..eb331d6ce 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -690,3 +690,63 @@ OVS_CHECK_ACTIONS([check_pkt_len(size=200,gt(5),le(check_pkt_len(size=100,gt(5), OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + + +AT_SETUP([offloads - offload flow to none-offload]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) + +ADD_NAMESPACES(at_ns0, at_ns1) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +AT_DATA([flows.txt], [dnl +add in_port=ovs-p0,actions=ovs-p1 +add in_port=ovs-p1,actions=ovs-p0 +]) +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl +10 packets transmitted, 10 received, 0% packet loss, time 0ms +]) + +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc | grep "eth_type(0x0800)" | sort | strip_recirc | strip_used], [0], [dnl +recirc_id(<recirc>),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:756, used:0.0s, actions:3 +recirc_id(<recirc>),in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:756, used:0.0s, actions:2 +]) + +dnl Here we use an output action with truncate, which will force a kernel flow. +AT_DATA([flows2.txt], [dnl +modify in_port=ovs-p0,actions=output(port=ovs-p1, max_len=128) +modify in_port=ovs-p1,actions=output(port=ovs-p0, max_len=128) +]) +AT_CHECK([ovs-ofctl add-flows br0 flows2.txt]) +AT_CHECK([ovs-appctl revalidator/wait], [0]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl +10 packets transmitted, 10 received, 0% packet loss, time 0ms +]) + +AT_CHECK([ovs-appctl dpctl/dump-flows type=ovs | grep "eth_type(0x0800)" | sort | strip_recirc | strip_used], [0], [dnl +recirc_id(<recirc>),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:980, used:0.0s, actions:trunc(128),3 +recirc_id(<recirc>),in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:980, used:0.0s, actions:trunc(128),2 +]) + +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl revalidator/wait], [0]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl +10 packets transmitted, 10 received, 0% packet loss, time 0ms +]) + +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc | grep "eth_type(0x0800)" | sort | strip_recirc | strip_used], [0], [dnl +recirc_id(<recirc>),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:840, used:0.0s, actions:3 +recirc_id(<recirc>),in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:10, bytes:840, used:0.0s, actions:2 +]) + +AT_CHECK([ovs-appctl coverage/read-counter ukey_invalid_stat_reset], [0], [dnl +0 +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP |