Ilya Maximets <[email protected]> writes: > Aaron, ovsrobot doesn't check this patch for some reason. > I suspect, it's because "PATCH" and "v8" are glued together. > Could you, please, take a look.
It won't. But not for that reason. The patch info says that this is patch 1 of 3. But 2/3 and 3/3 haven't been posted. So doing a pull from patchwork states: "total":3,"received_total":1,"received_all":false Because the patchwork thinks it hasn't received all the patches, it defers trying to process the series. In this case, the submitter may have intended to post 2 more patches that never hit the list. Maybe we can revisit the way the robot works now that it creates a special branch for each series that can be modified. Perhaps this should have been posted without the '1/3' series counter? > Anju, patch breaks a lot of unit tests. ovs-vswitchd crashes with > traces like this: > ==21473== > ==21473== Thread 13 monitor11: > ==21473== Invalid free() / delete / delete[] / realloc() > ==21473== at 0x4C30D3B: free (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==21473== by 0xA47F2F: dp_packet_delete (dp-packet.h:182) > ==21473== by 0xA47F2F: dp_packet_delete_batch (dp-packet.h:852) > ==21473== by 0xA47F2F: odp_execute_actions (odp-execute.c:1011) > ==21473== by 0xA05857: dp_netdev_execute_actions (dpif-netdev.c:7311) > ==21473== by 0xA05857: dpif_netdev_execute (dpif-netdev.c:3731) > ==21473== by 0xA05857: dpif_netdev_operate (dpif-netdev.c:3762) > ==21473== by 0xA166F5: dpif_operate (dpif.c:1363) > ==21473== by 0xA16DC7: dpif_execute (dpif.c:1317) > ==21473== by 0x9B9718: ofproto_dpif_execute_actions__ (ofproto-dpif.c:3990) > ==21473== by 0x9B99E1: ofproto_dpif_execute_actions (ofproto-dpif.c:4007) > ==21473== by 0x9DE9A1: xlate_send_packet (ofproto-dpif-xlate.c:7658) > ==21473== by 0x9B9E44: ofproto_dpif_send_packet (ofproto-dpif.c:4980) > ==21473== by 0x9CCF01: monitor_mport_run (ofproto-dpif-monitor.c:287) > ==21473== by 0x9CCB3C: monitor_run (ofproto-dpif-monitor.c:227) > ==21473== by 0x9CCB3C: monitor_main (ofproto-dpif-monitor.c:189) > ==21473== by 0xA96201: ovsthread_wrapper (ovs-thread.c:353) > ==21473== by 0x5B366DA: start_thread (pthread_create.c:463) > ==21473== by 0x6A7E88E: clone (clone.S:95) > ==21473== Address 0xd21efc0 is on thread 13's stack > ==21473== in frame #10, created by monitor_main (ofproto-dpif-monitor.c:186) > > Before submitting patches to the list you may run 'make check' on your local > machine to be sure that all the unit tests are OK. > > Some code related comments and build failure inline. > > Best regards, Ilya Maximets. > > On 08.02.2019 7:37, Anju Thomas wrote: >> Currently OVS maintains explicit packet drop/error counters only on port >> level. Packets that are dropped as part of normal OpenFlow processing are >> counted in flow stats of “drop” flows or as table misses in table stats. >> These can only be interpreted by controllers that know the semantics of >> the configured OpenFlow pipeline. Without that knowledge, it is impossible >> for an OVS user to obtain e.g. the total number of packets dropped due to >> OpenFlow rules. >> >> Furthermore, there are numerous other reasons for which packets can be >> dropped by OVS slow path that are not related to the OpenFlow pipeline. >> The generated datapath flow entries include a drop action to avoid further >> expensive upcalls to the slow path, but subsequent packets dropped by the >> datapath are not accounted anywhere. >> >> Finally, the datapath itself drops packets in certain error situations. >> Also, these drops are today not accounted for. >> >> This makes it difficult for OVS users to monitor packet drop in an OVS >> instance and to alert a management system in case of a unexpected increase >> of such drops. Also OVS trouble-shooters face difficulties in analysing >> packet drops. >> >> With this patch we implement following changes to address the issues >> mentioned above. >> >> 1. Identify and account all the silent packet drop scenarios >> >> 2. Display these drops in ovs-appctl coverage/show >> >> A detailed presentation on this was presented at OvS conference 2017 and >> link for the corresponding presentation is available at: >> >> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329 >> >> Co-authored-by: Rohith Basavaraja <[email protected]> >> Co-authored-by: Keshav Gupta <[email protected]> >> Signed-off-by: Anju Thomas <[email protected]> >> Signed-off-by: Rohith Basavaraja <[email protected]> >> Signed-off-by: Keshav Gupta <[email protected]> >> --- >> datapath/linux/compat/include/linux/openvswitch.h | 1 + >> lib/dpif-netdev.c | 44 ++++- >> lib/dpif.c | 7 + >> lib/dpif.h | 3 + >> lib/odp-execute.c | 81 +++++++++- >> lib/odp-util.c | 9 ++ >> ofproto/ofproto-dpif-ipfix.c | 1 + >> ofproto/ofproto-dpif-sflow.c | 1 + >> ofproto/ofproto-dpif-xlate.c | 101 +++++++----- >> ofproto/ofproto-dpif-xlate.h | 3 + >> ofproto/ofproto-dpif.c | 8 + >> ofproto/ofproto-dpif.h | 7 +- >> tests/automake.mk | 3 +- >> tests/dpif-netdev.at | 8 + >> tests/drop-stats.at | 189 >> ++++++++++++++++++++++ >> tests/ofproto-dpif.at | 2 +- >> tests/testsuite.at | 1 + >> tests/tunnel-push-pop.at | 28 +++- >> tests/tunnel.at | 14 +- >> 19 files changed, 465 insertions(+), 46 deletions(-) >> create mode 100644 tests/drop-stats.at >> >> diff --git a/datapath/linux/compat/include/linux/openvswitch.h >> b/datapath/linux/compat/include/linux/openvswitch.h >> index 9b087f1..559b296 100644 >> --- a/datapath/linux/compat/include/linux/openvswitch.h >> +++ b/datapath/linux/compat/include/linux/openvswitch.h >> @@ -938,6 +938,7 @@ enum ovs_action_attr { >> OVS_ACTION_ATTR_POP_NSH, /* No argument. */ >> OVS_ACTION_ATTR_METER, /* u32 meter number. */ >> OVS_ACTION_ATTR_CLONE, /* Nested OVS_CLONE_ATTR_*. */ >> + OVS_ACTION_ATTR_DROP, /* Drop action. */ >> >> #ifndef __KERNEL__ >> OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/ >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> index 0f57e3f..9d03de4 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -100,6 +100,17 @@ enum { MAX_METERS = 65536 }; /* Maximum number of >> meters. */ >> enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */ >> enum { N_METER_LOCKS = 64 }; /* Maximum number of meters. */ >> >> +COVERAGE_DEFINE(datapath_drop_meter); >> +COVERAGE_DEFINE(datapath_drop_upcall_error); >> +COVERAGE_DEFINE(datapath_drop_lock_error); >> +COVERAGE_DEFINE(datapath_drop_userspace_action_error); >> +COVERAGE_DEFINE(datapath_drop_tunnel_push_error); >> +COVERAGE_DEFINE(datapath_drop_tunnel_pop_error); >> +COVERAGE_DEFINE(datapath_drop_recirc_error); >> +COVERAGE_DEFINE(datapath_drop_invalid_port); >> +COVERAGE_DEFINE(datapath_drop_invalid_tnl_port); >> +COVERAGE_DEFINE(datapath_drop_rx_invalid_packet); >> + >> /* Protects against changes to 'dp_netdevs'. */ >> static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER; >> >> @@ -5644,6 +5655,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct >> dp_packet_batch *packets_, >> band->packet_count += 1; >> band->byte_count += dp_packet_size(packet); >> >> + COVERAGE_INC(datapath_drop_meter); >> dp_packet_delete(packet); >> } else { >> /* Meter accepts packet. */ >> @@ -6399,6 +6411,7 @@ dfc_processing(struct dp_netdev_pmd_thread *pmd, >> >> if (OVS_UNLIKELY(dp_packet_size(packet) < ETH_HEADER_LEN)) { >> dp_packet_delete(packet); >> + COVERAGE_INC(datapath_drop_rx_invalid_packet); >> continue; >> } >> >> @@ -6525,6 +6538,7 @@ handle_packet_upcall(struct dp_netdev_pmd_thread *pmd, >> put_actions); >> if (OVS_UNLIKELY(error && error != ENOSPC)) { >> dp_packet_delete(packet); >> + COVERAGE_INC(datapath_drop_upcall_error); >> return error; >> } >> >> @@ -6656,6 +6670,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd, >> DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) { >> if (OVS_UNLIKELY(!rules[i])) { >> dp_packet_delete(packet); >> + COVERAGE_INC(datapath_drop_lock_error); >> upcall_fail_cnt++; >> } >> } >> @@ -6925,6 +6940,7 @@ dp_execute_userspace_action(struct >> dp_netdev_pmd_thread *pmd, >> actions->data, actions->size); >> } else if (should_steal) { >> dp_packet_delete(packet); >> + COVERAGE_INC(datapath_drop_userspace_action_error); >> } >> } >> >> @@ -6939,6 +6955,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch >> *packets_, >> struct dp_netdev *dp = pmd->dp; >> int type = nl_attr_type(a); >> struct tx_port *p; >> + uint32_t packet_count, packet_dropped; >> >> switch ((enum ovs_action_attr)type) { >> case OVS_ACTION_ATTR_OUTPUT: >> @@ -6980,6 +6997,9 @@ dp_execute_cb(void *aux_, struct dp_packet_batch >> *packets_, >> dp_packet_batch_add(&p->output_pkts, packet); >> } >> return; >> + } else { >> + COVERAGE_ADD(datapath_drop_invalid_port, >> + dp_packet_batch_size(packets_)); >> } >> break; >> >> @@ -6989,10 +7009,16 @@ dp_execute_cb(void *aux_, struct dp_packet_batch >> *packets_, >> * the ownership of these packets. Thus, we can avoid performing >> * the action, because the caller will not use the result >> anyway. >> * Just break to free the batch. */ >> + COVERAGE_ADD(datapath_drop_tunnel_push_error, >> + dp_packet_batch_size(packets_)); >> break; >> } >> dp_packet_batch_apply_cutlen(packets_); >> - push_tnl_action(pmd, a, packets_); >> + packet_count = dp_packet_batch_size(packets_); >> + if (push_tnl_action(pmd, a, packets_)) { >> + COVERAGE_ADD(datapath_drop_tunnel_push_error, >> + packet_count); >> + } >> return; >> >> case OVS_ACTION_ATTR_TUNNEL_POP: >> @@ -7012,7 +7038,13 @@ dp_execute_cb(void *aux_, struct dp_packet_batch >> *packets_, >> >> dp_packet_batch_apply_cutlen(packets_); >> >> + packet_count = packets_->count; >> netdev_pop_header(p->port->netdev, packets_); >> + packet_dropped = packet_count - packets_->count; > > dp_packet_batch_size() should be used in two places above. > >> + if (packet_dropped) { >> + COVERAGE_ADD(datapath_drop_tunnel_pop_error, >> + packet_dropped); >> + } >> if (dp_packet_batch_is_empty(packets_)) { >> return; >> } >> @@ -7027,6 +7059,11 @@ dp_execute_cb(void *aux_, struct dp_packet_batch >> *packets_, >> (*depth)--; >> return; >> } >> + COVERAGE_ADD(datapath_drop_invalid_tnl_port, >> + dp_packet_batch_size(packets_)); >> + } else { >> + COVERAGE_ADD(datapath_drop_recirc_error, >> + dp_packet_batch_size(packets_)); >> } >> break; >> >> @@ -7071,6 +7108,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch >> *packets_, >> >> return; >> } >> + COVERAGE_ADD(datapath_drop_lock_error, >> + dp_packet_batch_size(packets_)); >> break; >> >> case OVS_ACTION_ATTR_RECIRC: >> @@ -7094,6 +7133,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch >> *packets_, >> return; >> } >> >> + COVERAGE_ADD(datapath_drop_recirc_error, >> + dp_packet_batch_size(packets_)); >> VLOG_WARN("Packet dropped. Max recirculation depth exceeded."); >> break; >> >> @@ -7246,6 +7287,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch >> *packets_, >> case OVS_ACTION_ATTR_PUSH_NSH: >> case OVS_ACTION_ATTR_POP_NSH: >> case OVS_ACTION_ATTR_CT_CLEAR: >> + case OVS_ACTION_ATTR_DROP: >> case __OVS_ACTION_ATTR_MAX: >> OVS_NOT_REACHED(); >> } >> diff --git a/lib/dpif.c b/lib/dpif.c >> index e35f111..abdb679 100644 >> --- a/lib/dpif.c >> +++ b/lib/dpif.c >> @@ -1274,6 +1274,7 @@ dpif_execute_helper_cb(void *aux_, struct >> dp_packet_batch *packets_, >> case OVS_ACTION_ATTR_POP_NSH: >> case OVS_ACTION_ATTR_CT_CLEAR: >> case OVS_ACTION_ATTR_UNSPEC: >> + case OVS_ACTION_ATTR_DROP: >> case __OVS_ACTION_ATTR_MAX: >> OVS_NOT_REACHED(); >> } >> @@ -1879,6 +1880,12 @@ dpif_supports_tnl_push_pop(const struct dpif *dpif) >> return dpif_is_netdev(dpif); >> } >> >> +bool >> +dpif_supports_explicit_drop_action(const struct dpif *dpif) >> +{ >> + return dpif_is_netdev(dpif); >> +} >> + >> /* Meters */ >> void >> dpif_meter_get_features(const struct dpif *dpif, >> diff --git a/lib/dpif.h b/lib/dpif.h >> index 475d5a6..e799da8 100644 >> --- a/lib/dpif.h >> +++ b/lib/dpif.h >> @@ -888,6 +888,9 @@ 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 *); >> +int dpif_show_drop_stats_support(struct dpif *dpif, bool detail, >> + struct ds *reply); >> >> /* Log functions. */ >> struct vlog_module; >> diff --git a/lib/odp-execute.c b/lib/odp-execute.c >> index 3b6890e..a0811bb 100644 >> --- a/lib/odp-execute.c >> +++ b/lib/odp-execute.c >> @@ -25,6 +25,7 @@ >> #include <stdlib.h> >> #include <string.h> >> >> +#include "coverage.h" >> #include "dp-packet.h" >> #include "dpif.h" >> #include "netlink.h" >> @@ -36,6 +37,74 @@ >> #include "util.h" >> #include "csum.h" >> #include "conntrack.h" >> +#include "ofproto/ofproto-dpif-xlate.h" >> +#include "openvswitch/vlog.h" >> + >> +VLOG_DEFINE_THIS_MODULE(odp_execute) >> +COVERAGE_DEFINE(dp_sample_error_drop); >> +COVERAGE_DEFINE(dp_nsh_decap_error_drop); > > Above two considered as datapath drops ? > In this case should they have similar name like 'datapath_drop_*' ? > >> +COVERAGE_DEFINE(drop_action_of_pipeline); >> +COVERAGE_DEFINE(drop_action_bridge_not_found); >> +COVERAGE_DEFINE(drop_action_recursion_too_deep); >> +COVERAGE_DEFINE(drop_action_too_many_resubmit); >> +COVERAGE_DEFINE(drop_action_stack_too_deep); >> +COVERAGE_DEFINE(drop_action_no_recirculation_context); >> +COVERAGE_DEFINE(drop_action_recirculation_conflict); >> +COVERAGE_DEFINE(drop_action_too_many_mpls_labels); >> +COVERAGE_DEFINE(drop_action_invalid_tunnel_metadata); >> +COVERAGE_DEFINE(drop_action_unsupported_packet_type); >> +COVERAGE_DEFINE(drop_action_congestion); >> +COVERAGE_DEFINE(drop_action_forwarding_disabled); >> + >> +static void >> +dp_update_drop_action_counter(enum xlate_error drop_reason, >> + int delta) >> +{ >> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); >> + >> + switch (drop_reason) { >> + case XLATE_OK: >> + COVERAGE_ADD(drop_action_of_pipeline, delta); >> + break; >> + case XLATE_BRIDGE_NOT_FOUND: >> + COVERAGE_ADD(drop_action_bridge_not_found, delta); >> + break; >> + case XLATE_RECURSION_TOO_DEEP: >> + COVERAGE_ADD(drop_action_recursion_too_deep, delta); >> + break; >> + case XLATE_TOO_MANY_RESUBMITS: >> + COVERAGE_ADD(drop_action_too_many_resubmit, delta); >> + break; >> + case XLATE_STACK_TOO_DEEP: >> + COVERAGE_ADD(drop_action_stack_too_deep, delta); >> + break; >> + case XLATE_NO_RECIRCULATION_CONTEXT: >> + COVERAGE_ADD(drop_action_no_recirculation_context, delta); >> + break; >> + case XLATE_RECIRCULATION_CONFLICT: >> + COVERAGE_ADD(drop_action_recirculation_conflict, delta); >> + break; >> + case XLATE_TOO_MANY_MPLS_LABELS: >> + COVERAGE_ADD(drop_action_too_many_mpls_labels, delta); >> + break; >> + case XLATE_INVALID_TUNNEL_METADATA: >> + COVERAGE_ADD(drop_action_invalid_tunnel_metadata, delta); >> + break; >> + case XLATE_UNSUPPORTED_PACKET_TYPE: >> + COVERAGE_ADD(drop_action_unsupported_packet_type, delta); >> + break; >> + case XLATE_CONGESTION_DROP: >> + COVERAGE_ADD(drop_action_congestion, delta); >> + break; >> + case XLATE_FORWARDING_DISABLED: >> + COVERAGE_ADD(drop_action_forwarding_disabled, delta); >> + break; >> + case XLATE_MAX: >> + default: >> + VLOG_ERR_RL(&rl, "Invalid Drop reason type:%d", drop_reason); >> + } >> +} >> + >> >> /* Masked copy of an ethernet address. 'src' is already properly masked. */ >> static void >> @@ -589,6 +658,7 @@ odp_execute_sample(void *dp, struct dp_packet *packet, >> bool steal, >> case OVS_SAMPLE_ATTR_PROBABILITY: >> if (random_uint32() >= nl_attr_get_u32(a)) { >> if (steal) { >> + COVERAGE_ADD(dp_sample_error_drop, 1); >> dp_packet_delete(packet); >> } >> return; >> @@ -673,6 +743,7 @@ requires_datapath_assistance(const struct nlattr *a) >> case OVS_ACTION_ATTR_PUSH_NSH: >> case OVS_ACTION_ATTR_POP_NSH: >> case OVS_ACTION_ATTR_CT_CLEAR: >> + case OVS_ACTION_ATTR_DROP: >> return false; >> >> case OVS_ACTION_ATTR_UNSPEC: >> @@ -889,6 +960,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch >> *batch, bool steal, >> if (pop_nsh(packet)) { >> dp_packet_batch_refill(batch, packet, i); >> } else { >> + COVERAGE_INC(dp_nsh_decap_error_drop); >> dp_packet_delete(packet); >> } >> } >> @@ -899,7 +971,14 @@ odp_execute_actions(void *dp, struct dp_packet_batch >> *batch, bool steal, >> conntrack_clear(packet); >> } >> break; >> - >> + case OVS_ACTION_ATTR_DROP: { >> + const enum xlate_error *drop_reason = nl_attr_get(a); >> + if (*drop_reason < XLATE_MAX) { >> + dp_update_drop_action_counter(*drop_reason, batch->count); >> + } >> + dp_packet_delete_batch(batch, true); >> + return; >> + } >> case OVS_ACTION_ATTR_OUTPUT: >> case OVS_ACTION_ATTR_TUNNEL_PUSH: >> case OVS_ACTION_ATTR_TUNNEL_POP: >> diff --git a/lib/odp-util.c b/lib/odp-util.c >> index 778c00e..40625cc 100644 >> --- a/lib/odp-util.c >> +++ b/lib/odp-util.c >> @@ -43,6 +43,7 @@ >> #include "uuid.h" >> #include "openvswitch/vlog.h" >> #include "openvswitch/match.h" >> +#include "ofproto/ofproto-dpif-xlate.h" >> >> VLOG_DEFINE_THIS_MODULE(odp_util); >> >> @@ -131,6 +132,7 @@ odp_action_len(uint16_t type) >> case OVS_ACTION_ATTR_CLONE: return ATTR_LEN_VARIABLE; >> case OVS_ACTION_ATTR_PUSH_NSH: return ATTR_LEN_VARIABLE; >> case OVS_ACTION_ATTR_POP_NSH: return 0; >> + case OVS_ACTION_ATTR_DROP: return sizeof(enum xlate_error); >> >> case OVS_ACTION_ATTR_UNSPEC: >> case __OVS_ACTION_ATTR_MAX: >> @@ -1181,6 +1183,9 @@ format_odp_action(struct ds *ds, const struct nlattr >> *a, >> case OVS_ACTION_ATTR_POP_NSH: >> ds_put_cstr(ds, "pop_nsh()"); >> break; >> + case OVS_ACTION_ATTR_DROP: >> + ds_put_cstr(ds, "drop"); >> + break; >> case OVS_ACTION_ATTR_UNSPEC: >> case __OVS_ACTION_ATTR_MAX: >> default: >> @@ -2427,8 +2432,12 @@ odp_actions_from_string(const char *s, const struct >> simap *port_names, >> struct ofpbuf *actions) >> { >> size_t old_size; >> + enum xlate_error drop_action; >> >> if (!strcasecmp(s, "drop")) { >> + drop_action = XLATE_OK; >> + nl_msg_put_unspec(actions, OVS_ACTION_ATTR_DROP, >> + &drop_action, sizeof drop_action); >> return 0; >> } >> >> diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c >> index 4029806..1d23a5a 100644 >> --- a/ofproto/ofproto-dpif-ipfix.c >> +++ b/ofproto/ofproto-dpif-ipfix.c >> @@ -3015,6 +3015,7 @@ dpif_ipfix_read_actions(const struct flow *flow, >> case OVS_ACTION_ATTR_PUSH_NSH: >> case OVS_ACTION_ATTR_POP_NSH: >> case OVS_ACTION_ATTR_UNSPEC: >> + case OVS_ACTION_ATTR_DROP: >> case __OVS_ACTION_ATTR_MAX: >> default: >> break; >> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c >> index 7da3175..69ed7b8 100644 >> --- a/ofproto/ofproto-dpif-sflow.c >> +++ b/ofproto/ofproto-dpif-sflow.c >> @@ -1222,6 +1222,7 @@ dpif_sflow_read_actions(const struct flow *flow, >> case OVS_ACTION_ATTR_PUSH_NSH: >> case OVS_ACTION_ATTR_POP_NSH: >> case OVS_ACTION_ATTR_UNSPEC: >> + case OVS_ACTION_ATTR_DROP: >> case __OVS_ACTION_ATTR_MAX: >> default: >> break; >> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c >> index 6c6df66..fb1a3ae 100644 >> --- a/ofproto/ofproto-dpif-xlate.c >> +++ b/ofproto/ofproto-dpif-xlate.c >> @@ -444,6 +444,10 @@ const char *xlate_strerror(enum xlate_error error) >> return "Invalid tunnel metadata"; >> case XLATE_UNSUPPORTED_PACKET_TYPE: >> return "Unsupported packet type"; >> + case XLATE_CONGESTION_DROP: >> + return "Congestion Drop"; >> + case XLATE_FORWARDING_DISABLED: >> + return "Forwarding is disabled"; > > Clang build complains: > > ofproto/ofproto-dpif-xlate.c:426:13: > error: enumeration value 'XLATE_MAX' not handled in switch [-Werror,-Wswitch] > switch (error) { > ^ > >> } >> return "Unknown error"; >> } >> @@ -5928,6 +5932,14 @@ put_ct_label(const struct flow *flow, struct ofpbuf >> *odp_actions, >> } >> >> static void >> +put_drop_action(struct ofpbuf *odp_actions, enum xlate_error error) >> +{ >> + nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_DROP, >> + &error, sizeof error); >> + >> +} >> + >> +static void >> put_ct_helper(struct xlate_ctx *ctx, >> struct ofpbuf *odp_actions, struct ofpact_conntrack *ofc) >> { >> @@ -7390,48 +7402,51 @@ xlate_actions(struct xlate_in *xin, struct xlate_out >> *xout) >> } >> size_t sample_actions_len = ctx.odp_actions->size; >> >> - if (tnl_process_ecn(flow) >> - && (!in_port || may_receive(in_port, &ctx))) { >> - const struct ofpact *ofpacts; >> - size_t ofpacts_len; >> - >> - if (xin->ofpacts) { >> - ofpacts = xin->ofpacts; >> - ofpacts_len = xin->ofpacts_len; >> - } else if (ctx.rule) { >> - const struct rule_actions *actions >> - = rule_get_actions(&ctx.rule->up); >> - ofpacts = actions->ofpacts; >> - ofpacts_len = actions->ofpacts_len; >> - ctx.rule_cookie = ctx.rule->up.flow_cookie; >> - } else { >> - OVS_NOT_REACHED(); >> - } >> + if (!tnl_process_ecn(flow)) { >> + ctx.error = XLATE_CONGESTION_DROP; >> + } else { >> + if (!in_port || may_receive(in_port, &ctx)) { >> + const struct ofpact *ofpacts; >> + size_t ofpacts_len; >> + >> + if (xin->ofpacts) { >> + ofpacts = xin->ofpacts; >> + ofpacts_len = xin->ofpacts_len; >> + } else if (ctx.rule) { >> + const struct rule_actions *actions >> + = rule_get_actions(&ctx.rule->up); >> + ofpacts = actions->ofpacts; >> + ofpacts_len = actions->ofpacts_len; >> + ctx.rule_cookie = ctx.rule->up.flow_cookie; >> + } else { >> + OVS_NOT_REACHED(); >> + } >> >> - mirror_ingress_packet(&ctx); >> - do_xlate_actions(ofpacts, ofpacts_len, &ctx, true, false); >> - if (ctx.error) { >> - goto exit; >> - } >> + mirror_ingress_packet(&ctx); >> + do_xlate_actions(ofpacts, ofpacts_len, &ctx, true, false); >> + if (ctx.error) { >> + goto exit; >> + } >> >> - /* We've let OFPP_NORMAL and the learning action look at the >> - * packet, so cancel all actions and freezing if forwarding is >> - * disabled. */ >> - if (in_port && (!xport_stp_forward_state(in_port) || >> - !xport_rstp_forward_state(in_port))) { >> - ctx.odp_actions->size = sample_actions_len; >> - ctx_cancel_freeze(&ctx); >> - ofpbuf_clear(&ctx.action_set); >> - } >> + /* We've let OFPP_NORMAL and the learning action look at the >> + * packet, so cancel all actions and freezing if forwarding >> is >> + * disabled. */ >> + if (in_port && (!xport_stp_forward_state(in_port) || >> + !xport_rstp_forward_state(in_port))) { >> + ctx.odp_actions->size = sample_actions_len; >> + ctx_cancel_freeze(&ctx); >> + ofpbuf_clear(&ctx.action_set); >> + ctx.error = XLATE_FORWARDING_DISABLED; >> + } >> >> - if (!ctx.freezing) { >> - xlate_action_set(&ctx); >> - } >> - if (ctx.freezing) { >> - finish_freezing(&ctx); >> + if (!ctx.freezing) { >> + xlate_action_set(&ctx); >> + } >> + if (ctx.freezing) { >> + finish_freezing(&ctx); >> + } >> } >> } > > > Above change looks huge and unnecessary. How about following change instead: > > --- > @@ -7382,9 +7383,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out > *xout) > compose_ipfix_action(&ctx, ODPP_NONE); > } > size_t sample_actions_len = ctx.odp_actions->size; > + bool ecn_drop = !tnl_process_ecn(flow); > > - if (tnl_process_ecn(flow) > - && (!in_port || may_receive(in_port, &ctx))) { > + if (!ecn_drop && (!in_port || may_receive(in_port, &ctx))) { > const struct ofpact *ofpacts; > size_t ofpacts_len; > > @@ -7415,6 +7416,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out > *xout) > ctx.odp_actions->size = sample_actions_len; > ctx_cancel_freeze(&ctx); > ofpbuf_clear(&ctx.action_set); > + ctx.error = XLATE_FORWARDING_DISABLED; > } > > if (!ctx.freezing) { > @@ -7423,6 +7425,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out > *xout) > if (ctx.freezing) { > finish_freezing(&ctx); > } > + } else if (ecn_drop) { > + ctx.error = XLATE_CONGESTION_DROP; > } > > /* Output only fully processed packets. */ > --- > >> - >> /* Output only fully processed packets. */ >> if (!ctx.freezing >> && xbridge->has_in_band >> @@ -7529,6 +7544,18 @@ exit: >> ofpbuf_clear(xin->odp_actions); >> } >> } >> + >> + /* >> + * If we are going to install "drop" action, check whether >> + * datapath supports explicit "drop"action. If datapath >> + * supports explicit "drop"action then install the "drop" >> + * action containing the drop reason. >> + */ >> + if (xin->odp_actions && !xin->odp_actions->size && >> + ovs_explicit_drop_action_supported(ctx.xbridge->ofproto)) { >> + put_drop_action(xin->odp_actions, ctx.error); >> + } >> + >> return ctx.error; >> } >> >> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h >> index 0a5a528..496bdbf 100644 >> --- a/ofproto/ofproto-dpif-xlate.h >> +++ b/ofproto/ofproto-dpif-xlate.h >> @@ -216,6 +216,9 @@ enum xlate_error { >> XLATE_TOO_MANY_MPLS_LABELS, >> XLATE_INVALID_TUNNEL_METADATA, >> XLATE_UNSUPPORTED_PACKET_TYPE, >> + XLATE_CONGESTION_DROP, >> + XLATE_FORWARDING_DISABLED, >> + XLATE_MAX, >> }; >> >> const char *xlate_strerror(enum xlate_error error); >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >> index 14fe6fc..aa4e6dd 100644 >> --- a/ofproto/ofproto-dpif.c >> +++ b/ofproto/ofproto-dpif.c >> @@ -827,6 +827,12 @@ ovs_native_tunneling_is_on(struct ofproto_dpif *ofproto) >> && atomic_count_get(&ofproto->backer->tnl_count); >> } >> >> +bool >> +ovs_explicit_drop_action_supported(struct ofproto_dpif *ofproto) >> +{ >> + return ofproto->backer->rt_support.explicit_drop_action; >> +} >> + >> /* Tests whether 'backer''s datapath supports recirculation. Only newer >> * datapaths support OVS_KEY_ATTR_RECIRC_ID in keys. We need to disable >> some >> * features on older datapaths that don't support this feature. >> @@ -1397,6 +1403,8 @@ check_support(struct dpif_backer *backer) >> backer->rt_support.ct_eventmask = check_ct_eventmask(backer); >> backer->rt_support.ct_clear = check_ct_clear(backer); >> backer->rt_support.max_hash_alg = check_max_dp_hash_alg(backer); >> + backer->rt_support.explicit_drop_action = >> + dpif_supports_explicit_drop_action(backer->dpif); >> >> /* Flow fields. */ >> backer->rt_support.odp.ct_state = check_ct_state(backer); >> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h >> index 1a404c8..8d8edca 100644 >> --- a/ofproto/ofproto-dpif.h >> +++ b/ofproto/ofproto-dpif.h >> @@ -192,7 +192,10 @@ struct group_dpif *group_dpif_lookup(struct >> ofproto_dpif *, >> DPIF_SUPPORT_FIELD(bool, ct_clear, "Conntrack clear") >> \ >> >> \ >> /* Highest supported dp_hash algorithm. */ >> \ >> - DPIF_SUPPORT_FIELD(size_t, max_hash_alg, "Max dp_hash algorithm") >> + DPIF_SUPPORT_FIELD(size_t, max_hash_alg, "Max dp_hash algorithm") >> \ >> + >> \ >> + /* True if the datapath supports explicit drop action. */ >> \ >> + DPIF_SUPPORT_FIELD(bool, explicit_drop_action, "Explicit Drop action") >> >> /* Stores the various features which the corresponding backer supports. */ >> struct dpif_backer_support { >> @@ -361,4 +364,6 @@ int ofproto_dpif_delete_internal_flow(struct >> ofproto_dpif *, struct match *, >> >> bool ovs_native_tunneling_is_on(struct ofproto_dpif *); >> >> +bool ovs_explicit_drop_action_supported(struct ofproto_dpif *); >> + >> #endif /* ofproto-dpif.h */ >> diff --git a/tests/automake.mk b/tests/automake.mk >> index 92d56b2..a4da75e 100644 >> --- a/tests/automake.mk >> +++ b/tests/automake.mk >> @@ -108,7 +108,8 @@ TESTSUITE_AT = \ >> tests/ovn-controller-vtep.at \ >> tests/mcast-snooping.at \ >> tests/packet-type-aware.at \ >> - tests/nsh.at >> + tests/nsh.at \ >> + tests/drop-stats.at >> >> EXTRA_DIST += $(FUZZ_REGRESSION_TESTS) >> FUZZ_REGRESSION_TESTS = \ >> diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at >> index 6915d43..5221c10 100644 >> --- a/tests/dpif-netdev.at >> +++ b/tests/dpif-netdev.at >> @@ -337,6 +337,14 @@ meter:2 flow_count:1 packet_in_count:10 >> byte_in_count:600 duration:0.0s bands: >> 0: packet_count:5 byte_count:300 >> ]) >> >> +ovs-appctl time/warp 5000 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "datapath_drop_meter" | cut -d':' -f2|sed >> 's/ //' >> +], [0], [dnl >> +14 >> +]) >> + >> AT_CHECK([cat ovs-vswitchd.log | filter_flow_install | >> strip_xout_keep_actions], [0], [dnl >> >> recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), >> actions:meter(0),7 >> >> recirc_id(0),in_port(2),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), >> actions:8 >> diff --git a/tests/drop-stats.at b/tests/drop-stats.at >> new file mode 100644 >> index 0000000..6b90aae >> --- /dev/null >> +++ b/tests/drop-stats.at >> @@ -0,0 +1,189 @@ >> +AT_BANNER([drop-stats]) >> + >> +AT_SETUP([drop-stats - cli tests]) >> + >> +OVS_VSWITCHD_START([dnl >> + set bridge br0 datapath_type=dummy \ >> + protocols=OpenFlow10,OpenFlow13,OpenFlow14,OpenFlow15 -- \ >> + add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1]) >> + >> +AT_DATA([flows.txt], [dnl >> +table=0,in_port=1,actions=drop >> +]) >> + >> +AT_CHECK([ >> + ovs-ofctl del-flows br0 >> + ovs-ofctl -Oopenflow13 add-flows br0 flows.txt >> + ovs-ofctl -Oopenflow13 dump-flows br0 | ofctl_strip | sort | grep >> actions >> +], [0], [dnl >> + in_port=1 actions=drop >> +]) >> + >> +AT_CHECK([ >> + ovs-appctl netdev-dummy/receive p1 >> 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637 >> + ovs-appctl netdev-dummy/receive p1 >> 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637 >> + ovs-appctl netdev-dummy/receive p1 >> 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637 >> +], [0], [ignore]) >> + >> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed >> 's/used:[[0-9]].[[0-9]]*s/used:0.0/' | sort], [0], >> +[flow-dump from non-dpdk interfaces: >> +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), >> packets:2, bytes:196, used:0.0, actions:drop >> +]) >> + >> +sleep 1 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "drop_action_of_pipeline" | cut -d':' >> -f2|sed 's/ //' >> +], [0], [dnl >> +3 >> +]) >> + >> +OVS_VSWITCHD_STOP >> +AT_CLEANUP >> + >> +AT_SETUP([drop-stats - pipeline and recurssion drops]) >> + >> +OVS_VSWITCHD_START([dnl >> + set bridge br0 datapath_type=dummy \ >> + protocols=OpenFlow10,OpenFlow13,OpenFlow14,OpenFlow15 -- \ >> + add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1 -- \ >> + add-port br0 p2 -- set Interface p2 type=dummy ofport_request=2]) >> + >> +AT_DATA([flows.txt], [dnl >> +table=0,in_port=1,actions=drop >> +]) >> + >> +AT_CHECK([ >> + ovs-ofctl del-flows br0 >> + ovs-ofctl -Oopenflow13 add-flows br0 flows.txt >> + ovs-ofctl -Oopenflow13 dump-flows br0 | ofctl_strip | sort | grep >> actions >> +], [0], [dnl >> + in_port=1 actions=drop >> +]) >> + >> +AT_CHECK([ >> + ovs-appctl netdev-dummy/receive p1 >> 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637 >> +], [0], [ignore]) >> + >> +sleep 1 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "drop_action_of_pipeline" | cut -d':' >> -f2|sed 's/ //' >> +], [0], [dnl >> +1 >> +]) >> + >> +AT_DATA([flows.txt], [dnl >> +table=0, in_port=1, actions=goto_table:1 >> +table=1, in_port=1, actions=goto_table:2 >> +table=2, in_port=1, actions=resubmit(,1) >> +]) >> + >> +AT_CHECK([ >> + ovs-ofctl del-flows br0 >> + ovs-ofctl -Oopenflow13 add-flows br0 flows.txt >> + ovs-ofctl -Oopenflow13 dump-flows br0 | ofctl_strip | sort | grep >> actions >> +], [0], [ignore]) >> + >> +AT_CHECK([ >> + ovs-appctl netdev-dummy/receive p1 >> 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637 >> +], [0], [ignore]) >> + >> +sleep 1 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "drop_action_recursion_too_deep" | cut >> -d':' -f2|sed 's/ //' >> +], [0], [dnl >> +1 >> +]) >> + >> +OVS_VSWITCHD_STOP(["/|WARN|/d"]) >> +AT_CLEANUP >> + >> +AT_SETUP([drop-stats - too many resubmit]) >> + >> +OVS_VSWITCHD_START >> +add_of_ports br0 1 >> +(for i in `seq 1 64`; do >> + j=`expr $i + 1` >> + echo "in_port=$i, actions=resubmit:$j, resubmit:$j, local" >> + done >> + echo "in_port=65, actions=local") > flows.txt >> + >> +AT_CHECK([ >> + ovs-ofctl del-flows br0 >> + ovs-ofctl -Oopenflow13 add-flows br0 flows.txt >> +], [0], [ignore]) >> + >> +ovs-appctl netdev-dummy/receive p1 >> 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x1234)' >> + >> +sleep 1 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "drop_action_too_many_resubmit" | cut -d':' >> -f2|sed 's/ //' >> +], [0], [dnl >> +1 >> +]) >> + >> +OVS_VSWITCHD_STOP(["/|WARN|/d"]) >> +AT_CLEANUP >> + >> +AT_SETUP([drop-stats - stack too deep]) >> +OVS_VSWITCHD_START >> +add_of_ports br0 1 >> +(for i in `seq 1 12`; do >> + j=`expr $i + 1` >> + echo "in_port=$i, actions=resubmit:$j, resubmit:$j, local" >> + done >> + push="push:NXM_NX_REG0[[]]" >> + echo "in_port=13, >> actions=$push,$push,$push,$push,$push,$push,$push,$push") > flows >> + AT_CHECK([ovs-ofctl add-flows br0 flows]) >> + >> +ovs-appctl netdev-dummy/receive p1 >> 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x1234)' >> + >> +sleep 1 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "drop_action_stack_too_deep" | cut -d':' >> -f2|sed 's/ //' >> +], [0], [dnl >> +1 >> +]) >> + >> +OVS_VSWITCHD_STOP(["/resubmits yielded over 64 kB of stack/d"]) >> +AT_CLEANUP >> + >> +AT_SETUP([drop-stats - too many mpls labels]) >> + >> +OVS_VSWITCHD_START([dnl >> + set bridge br0 datapath_type=dummy \ >> + protocols=OpenFlow10,OpenFlow13,OpenFlow14,OpenFlow15 -- \ >> + add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1 -- \ >> + add-port br0 p2 -- set Interface p2 type=dummy ofport_request=2]) >> + >> +AT_DATA([flows.txt], [dnl >> +table=0, in_port=1, actions=push_mpls:0x8847, resubmit:3 >> +table=0, in_port=3, actions=push_mpls:0x8847, set_field:10->mpls_label, >> set_field:15->mpls_label, resubmit:4 >> +table=0, in_port=4, actions=push_mpls:0x8847, set_field:11->mpls_label, >> resubmit:5 >> +table=0, in_port=5, actions=push_mpls:0x8847, set_field:12->mpls_label, >> resubmit:6 >> +table=0, in_port=6, actions=push_mpls:0x8847, set_field:13->mpls_label, >> output:2 >> +]) >> + >> +AT_CHECK([ >> + ovs-ofctl del-flows br0 >> + ovs-ofctl -Oopenflow13 add-flows br0 flows.txt >> +]) >> + >> +AT_CHECK([ >> + ovs-appctl netdev-dummy/receive p1 >> 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637 >> +], [0], [ignore]) >> + >> +sleep 1 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "drop_action_too_many_mpls_labels" | cut >> -d':' -f2|sed 's/ //' >> +], [0], [dnl >> +1 >> +]) >> + >> +OVS_VSWITCHD_STOP(["/|WARN|/d"]) >> +AT_CLEANUP >> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at >> index ded2ef0..2c8cdce 100644 >> --- a/tests/ofproto-dpif.at >> +++ b/tests/ofproto-dpif.at >> @@ -9384,7 +9384,7 @@ >> recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x8100),vlan(vid=99,pcp= >> # are wildcarded. >> AT_CHECK([grep '\(modify\)\|\(flow_add\)' ovs-vswitchd.log | strip_ufid ], >> [0], [dnl >> dpif_netdev|DBG|flow_add: >> recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x1234), actions:100 >> -dpif|DBG|dummy@ovs-dummy: put[[modify]] >> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x1234) >> +dpif|DBG|dummy@ovs-dummy: put[[modify]] >> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x1234), >> actions:drop >> dpif|DBG|dummy@ovs-dummy: put[[modify]] >> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x1234), >> actions:100 >> dpif_netdev|DBG|flow_add: >> recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x8100),vlan(vid=99,pcp=7/0x0),encap(eth_type(0x1234)), >> actions:drop >> ]) >> diff --git a/tests/testsuite.at b/tests/testsuite.at >> index b840dbf..922ba48 100644 >> --- a/tests/testsuite.at >> +++ b/tests/testsuite.at >> @@ -82,3 +82,4 @@ m4_include([tests/ovn-controller-vtep.at]) >> m4_include([tests/mcast-snooping.at]) >> m4_include([tests/packet-type-aware.at]) >> m4_include([tests/nsh.at]) >> +m4_include([tests/drop-stats.at]) >> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at >> index f717243..045b1a7 100644 >> --- a/tests/tunnel-push-pop.at >> +++ b/tests/tunnel-push-pop.at >> @@ -447,6 +447,27 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port >> 7'], [0], [dnl >> port 7: rx pkts=3, bytes=252, drop=?, errs=?, frame=?, over=?, crc=? >> ]) >> >> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> 'aa55aa550000001b213cab6408004500007079464000402fba600101025c0101025820000800000001c845000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) >> + >> +ovs-appctl time/warp 5000 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "datapath_drop_tunnel_pop_error" | cut >> -d':' -f2|sed 's/ //' >> +], [0], [dnl >> +1 >> +]) >> + >> +sleep 1 >> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> 'aa55aa550000001b213cab6408004503007079464000402fba600101025c0101025820000800000001c845000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) >> + >> +ovs-appctl time/warp 5000 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "drop_action_congestion" | cut -d':' >> -f2|sed 's/ //' >> +], [0], [dnl >> +1 >> +]) >> + >> dnl Check GREL3 only accepts non-fragmented packets? >> AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> 'aa55aa550000001b213cab6408004500007e79464000402fba550101025c0101025820000800000001c8fe71d883724fbeb6f4e1494a080045000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) >> >> @@ -455,7 +476,7 @@ ovs-appctl time/warp 1000 >> >> AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port [[37]]' | sort], [0], >> [dnl >> port 3: rx pkts=3, bytes=294, drop=?, errs=?, frame=?, over=?, crc=? >> - port 7: rx pkts=4, bytes=350, drop=?, errs=?, frame=?, over=?, crc=? >> + port 7: rx pkts=5, bytes=434, drop=?, errs=?, frame=?, over=?, crc=? >> ]) >> >> dnl Check decapsulation of Geneve packet with options >> @@ -478,7 +499,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 5'], >> [0], [dnl >> port 5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=? >> ]) >> AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], >> [dnl >> -tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), >> packets:0, bytes:0, used:never, >> actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535)) >> +tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), >> packets:0, bytes:0, used:never, >> actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535)) >> ]) >> >> ovs-appctl time/warp 10000 >> @@ -510,7 +531,8 @@ AT_CHECK([ovs-appctl tnl/ports/show |sort], [0], [dnl >> Listening ports: >> ]) >> >> -OVS_VSWITCHD_STOP >> +OVS_VSWITCHD_STOP(["/dropping tunnel packet marked ECN CE but is not ECN >> capable/d >> +/ip packet has invalid checksum/d"]) >> AT_CLEANUP >> >> AT_SETUP([tunnel_push_pop - packet_out]) >> diff --git a/tests/tunnel.at b/tests/tunnel.at >> index 55fb1d3..7bd5b48 100644 >> --- a/tests/tunnel.at >> +++ b/tests/tunnel.at >> @@ -102,10 +102,12 @@ Datapath actions: set(ipv4(tos=0x3/0x3)),2 >> >> dnl Tunnel CE and encapsulated packet Non-ECT >> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy >> 'tunnel(src=1.1.1.1,dst=2.2.2.2,tos=0x3,ttl=64,flags()),in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=9)'], >> [0], [stdout]) >> -AT_CHECK([tail -2 stdout], [0], >> +AT_CHECK([tail -3 stdout], [0], >> [Megaflow: >> recirc_id=0,eth,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-csum-key,in_port=1,nw_ecn=0,nw_frag=no >> Datapath actions: drop >> +Translation failed (Congestion Drop), packet is dropped. >> ]) >> + >> OVS_VSWITCHD_STOP(["/dropping tunnel packet marked ECN CE but is not ECN >> capable/d"]) >> AT_CLEANUP >> >> @@ -193,6 +195,16 @@ AT_CHECK([ovs-appctl ofproto/trace ovs-dummy >> 'in_port(2),eth(src=50:54:00:00:00: >> AT_CHECK([tail -1 stdout], [0], >> [Datapath actions: >> set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x2)),1 >> ]) >> + >> +AT_CHECK([ovs-appctl netdev-dummy/receive p2 >> 'aa55aa550001f8bc124434b6080045000054ba20000040018486010103580101037001004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) >> + >> +sleep 2 >> + >> +AT_CHECK([ >> +ovs-appctl coverage/show | grep "datapath_drop_invalid_port" | cut -d':' >> -f2|sed 's/ //' >> +], [0], [dnl >> +1 >> +]) >> OVS_VSWITCHD_STOP >> AT_CLEANUP >> >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
