Message ID | 1575622195-30195-1-git-send-email-anju.thomas@ericsson.com |
---|---|
State | Changes Requested |
Delegated to: | Ilya Maximets |
Headers | show |
Series | [ovs-dev,v17] Improved Packet Drop Statistics in OVS | expand |
On 06.12.2019 09:49, 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 > > Co-authored-by: Rohith Basavaraja <rohith.basavaraja@gmail.com> > Co-authored-by: Keshav Gupta <keshugupta1@gmail.com> > Signed-off-by: Anju Thomas <anju.thomas@ericsson.com> > Signed-off-by: Rohith Basavaraja <rohith.basavaraja@gmail.com> > Signed-off-by: Keshav Gupta <keshugupta1@gmail.com> > Acked-by: Eelco Chaudron <echaudro@redhat.com > --- Thanks for a new version. I really hope that v18 will be the last one. This patch needs some minor rebase on top of current master. 6 more comments inline (some of them are really minor, but I decided to point to them as you'll re-spin the patch anyway). Please, fix them and send v18. Best regards, Ilya Maximets. > v17 (Ilya's comments) > 1. comments for xlate_error > 2. define xlate_error in ifndef __KERNEL__ > 3. move declaration of xlate_error after OVS_TUNNEL_KEY_ATTR_MAX > 4. comment change for DROP action > 5. Add in datapath capability > > v16 (Ilya and Eelco comments) > 1. remove old declaration of xlate_error in v15 > 2. kernel file formatting in openvswitch.h for xlate_error > 3. remove drop_action from odp_actions_from_string > > v15(Ilya's comments) > 1. Description in openvswitch.h > 2. Move xlate_error to openvswitch.h to not include ofproto heeader in dp > 3. Return u32 for instead of size of enum > 4. Formatting > 5. Add drop-stats in testsuite.at > 6. Use coverage-read-counter > 7. Use flow instead of raw packet data wherever possible > 8. Change sleep to warp > > v14 (Eelco's comments) > 1. Remove definition of dpif_show_drop_stats_support > 2. Remove extra check for drop reason in odp_execute_actions > > v13(Eelco and Illya's comments) > 1. packet_dropped changed to packets_dropped > 2. Uses dp_packet_batch_size instead of packet->size > 3. Add version history > > v12(Ben's comments) > 1. new dp action in kernel block in openvswitch.h > 2. change xlate_error enum to be used as u32 > 3. resetting xlate error in case of congestion drop > and forwarding disabled > --- > datapath/linux/compat/include/linux/openvswitch.h | 24 +++ > lib/dpif-netdev.c | 47 +++++- > lib/dpif.c | 7 + > lib/dpif.h | 2 + > lib/odp-execute.c | 77 +++++++++ > lib/odp-util.c | 5 + > ofproto/ofproto-dpif-ipfix.c | 1 + > ofproto/ofproto-dpif-sflow.c | 1 + > ofproto/ofproto-dpif-xlate.c | 33 +++- > ofproto/ofproto-dpif-xlate.h | 13 -- > ofproto/ofproto-dpif.c | 10 ++ > ofproto/ofproto-dpif.h | 8 +- > tests/automake.mk | 3 +- > tests/dpif-netdev.at | 8 + > tests/drop-stats.at | 190 ++++++++++++++++++++++ > tests/ofproto-dpif.at | 2 +- > tests/testsuite.at | 1 + > tests/tunnel-push-pop.at | 28 +++- > tests/tunnel.at | 16 +- > vswitchd/vswitch.xml | 5 + > 20 files changed, 457 insertions(+), 24 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 778827f..8006724 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -407,6 +407,28 @@ enum ovs_tunnel_key_attr { > #define OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1) > > /** > + * enum xlate_error - Different types of error during translation > + */ > + > +#ifndef __KERNEL__ > +enum xlate_error { > + XLATE_OK = 0, > + XLATE_BRIDGE_NOT_FOUND, > + XLATE_RECURSION_TOO_DEEP, > + XLATE_TOO_MANY_RESUBMITS, > + XLATE_STACK_TOO_DEEP, > + XLATE_NO_RECIRCULATION_CONTEXT, > + XLATE_RECIRCULATION_CONFLICT, > + XLATE_TOO_MANY_MPLS_LABELS, > + XLATE_INVALID_TUNNEL_METADATA, > + XLATE_UNSUPPORTED_PACKET_TYPE, > + XLATE_CONGESTION_DROP, > + XLATE_FORWARDING_DISABLED, > + XLATE_MAX, > +}; > +#endif Over and over again, kernel coding style, please. > + > +/** > * enum ovs_frag_type - IPv4 and IPv6 fragment type > * @OVS_FRAG_TYPE_NONE: Packet is not a fragment. > * @OVS_FRAG_TYPE_FIRST: Packet is a fragment with offset 0. > @@ -962,6 +984,7 @@ struct check_pkt_len_arg { > * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set > * of actions if greater than the specified packet length, else execute > * another set of actions. > + * @OVS_ACTION_ATTR_DROP: Explicit drop action. > */ > > enum ovs_action_attr { > @@ -994,6 +1017,7 @@ enum ovs_action_attr { > #ifndef __KERNEL__ > OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/ > OVS_ACTION_ATTR_TUNNEL_POP, /* u32 port number. */ > + OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */ > #endif > __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted > * from userspace. */ > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 5142bad..8adeac5 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -102,6 +102,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; > > @@ -5753,7 +5764,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_, > band = &meter->bands[exceeded_band[j]]; > band->packet_count += 1; > band->byte_count += dp_packet_size(packet); > - > + COVERAGE_INC(datapath_drop_meter); > dp_packet_delete(packet); > } else { > /* Meter accepts packet. */ > @@ -6503,6 +6514,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; > } > > @@ -6623,6 +6635,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; > } > > @@ -6753,6 +6766,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++; > } > } > @@ -7022,6 +7036,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); > } > } > > @@ -7036,6 +7051,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, packets_dropped; > > switch ((enum ovs_action_attr)type) { > case OVS_ACTION_ATTR_OUTPUT: > @@ -7077,6 +7093,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; > > @@ -7086,10 +7105,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_)); This is not a tunnel push error. We literally executed all the actions without errors and that is not our fault that there are no more actions after tnl_push. We're just saving some time avoiding actual execution of a pointless action. So, this should not be accounted as error, and definitely not as a tunnel push error. > 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: > @@ -7109,7 +7134,14 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, > > dp_packet_batch_apply_cutlen(packets_); > > + packet_count = dp_packet_batch_size(packets_); > netdev_pop_header(p->port->netdev, packets_); > + packets_dropped = > + packet_count - dp_packet_batch_size(packets_); > + if (packets_dropped) { > + COVERAGE_ADD(datapath_drop_tunnel_pop_error, > + packets_dropped); > + } > if (dp_packet_batch_is_empty(packets_)) { > return; > } > @@ -7124,6 +7156,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; > > @@ -7168,6 +7205,9 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, > > return; > } > + COVERAGE_ADD(datapath_drop_lock_error, > + dp_packet_batch_size(packets_)); > + Empty line not needed here. > break; > > case OVS_ACTION_ATTR_RECIRC: > @@ -7191,6 +7231,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; > > @@ -7348,6 +7390,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_CT_CLEAR: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + case OVS_ACTION_ATTR_DROP: > case __OVS_ACTION_ATTR_MAX: > OVS_NOT_REACHED(); > } > diff --git a/lib/dpif.c b/lib/dpif.c > index c88b210..5a9ff46 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_CT_CLEAR: > case OVS_ACTION_ATTR_UNSPEC: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + 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 c98513e..7cc2220 100644 > --- a/lib/dpif.h > +++ b/lib/dpif.h > @@ -892,6 +892,8 @@ 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 *); > + New empty line is not needed here. > > /* Log functions. */ > struct vlog_module; > diff --git a/lib/odp-execute.c b/lib/odp-execute.c > index 563ad1d..3d4ad9e 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,72 @@ > #include "util.h" > #include "csum.h" > #include "conntrack.h" > +#include "openvswitch/vlog.h" > + > +VLOG_DEFINE_THIS_MODULE(odp_execute); > +COVERAGE_DEFINE(datapath_drop_sample_error); > +COVERAGE_DEFINE(datapath_drop_nsh_decap_error); > +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); Please, add a single space after ':'. > + } > +} > > /* Masked copy of an ethernet address. 'src' is already properly masked. */ > static void > @@ -621,6 +688,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_INC(datapath_drop_sample_error); > dp_packet_delete(packet); > } > return; > @@ -749,6 +817,7 @@ requires_datapath_assistance(const struct nlattr *a) > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_CT_CLEAR: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + case OVS_ACTION_ATTR_DROP: > return false; > > case OVS_ACTION_ATTR_UNSPEC: > @@ -965,6 +1034,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(datapath_drop_nsh_decap_error); > dp_packet_delete(packet); > } > } > @@ -989,6 +1059,13 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal, > } > break; > > + case OVS_ACTION_ATTR_DROP:{ > + const enum xlate_error *drop_reason = nl_attr_get(a); > + > + dp_update_drop_action_counter(*drop_reason, batch->count); dp_packet_batch_size(batch) > + dp_packet_delete_batch(batch, steal); > + 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 b9600b4..9bda91f 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -141,6 +141,7 @@ odp_action_len(uint16_t type) > case OVS_ACTION_ATTR_PUSH_NSH: return ATTR_LEN_VARIABLE; > case OVS_ACTION_ATTR_POP_NSH: return 0; > case OVS_ACTION_ATTR_CHECK_PKT_LEN: return ATTR_LEN_VARIABLE; > + case OVS_ACTION_ATTR_DROP: return sizeof(uint32_t); > > case OVS_ACTION_ATTR_UNSPEC: > case __OVS_ACTION_ATTR_MAX: > @@ -1238,6 +1239,9 @@ format_odp_action(struct ds *ds, const struct nlattr *a, > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > format_odp_check_pkt_len_action(ds, a, portno_names); > break; > + case OVS_ACTION_ATTR_DROP: > + ds_put_cstr(ds, "drop"); > + break; > case OVS_ACTION_ATTR_UNSPEC: > case __OVS_ACTION_ATTR_MAX: > default: > @@ -2575,6 +2579,7 @@ odp_actions_from_string(const char *s, const struct simap *port_names, > size_t old_size; > > if (!strcasecmp(s, "drop")) { > + nl_msg_put_u32(actions, OVS_ACTION_ATTR_DROP, XLATE_OK); > return 0; > } > > diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c > index b8bd1b8..b413768 100644 > --- a/ofproto/ofproto-dpif-ipfix.c > +++ b/ofproto/ofproto-dpif-ipfix.c > @@ -3016,6 +3016,7 @@ dpif_ipfix_read_actions(const struct flow *flow, > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > 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 9abaab6..f9ea47a 100644 > --- a/ofproto/ofproto-dpif-sflow.c > +++ b/ofproto/ofproto-dpif-sflow.c > @@ -1224,6 +1224,7 @@ dpif_sflow_read_actions(const struct flow *flow, > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_UNSPEC: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + 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 cebae7a..222c954 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -445,6 +445,12 @@ 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"; > + case XLATE_MAX: > + break; > } > return "Unknown error"; > } > @@ -6002,6 +6008,12 @@ 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_u32(odp_actions, OVS_ACTION_ATTR_DROP, error); > +} > + > +static void > put_ct_helper(struct xlate_ctx *ctx, > struct ofpbuf *odp_actions, struct ofpact_conntrack *ofc) > { > @@ -7638,8 +7650,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) > + if (!ecn_drop > && (!in_port || may_receive(in_port, &ctx))) { > const struct ofpact *ofpacts; > size_t ofpacts_len; > @@ -7671,6 +7684,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) { > @@ -7679,6 +7693,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. */ > @@ -7784,6 +7800,21 @@ exit: > ofpbuf_clear(xin->odp_actions); > } > } > + > + /* Install drop action if datapath supports explicit drop action. */ > + if (xin->odp_actions && !xin->odp_actions->size && > + ovs_explicit_drop_action_supported(ctx.xbridge->ofproto)) { > + put_drop_action(xin->odp_actions, ctx.error); > + } > + > + /* Since congestion drop and forwarding drop are not exactly > + * translation error, we are resetting the translation error. > + */ > + if (ctx.error == XLATE_CONGESTION_DROP || > + ctx.error == XLATE_FORWARDING_DISABLED) { > + ctx.error = XLATE_OK; > + } > + > return ctx.error; > } > > diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h > index f97c7c0..3426a27 100644 > --- a/ofproto/ofproto-dpif-xlate.h > +++ b/ofproto/ofproto-dpif-xlate.h > @@ -206,19 +206,6 @@ int xlate_lookup(const struct dpif_backer *, const struct flow *, > struct dpif_sflow **, struct netflow **, > ofp_port_t *ofp_in_port); > > -enum xlate_error { > - XLATE_OK = 0, > - XLATE_BRIDGE_NOT_FOUND, > - XLATE_RECURSION_TOO_DEEP, > - XLATE_TOO_MANY_RESUBMITS, > - XLATE_STACK_TOO_DEEP, > - XLATE_NO_RECIRCULATION_CONTEXT, > - XLATE_RECIRCULATION_CONFLICT, > - XLATE_TOO_MANY_MPLS_LABELS, > - XLATE_INVALID_TUNNEL_METADATA, > - XLATE_UNSUPPORTED_PACKET_TYPE, > -}; > - > const char *xlate_strerror(enum xlate_error error); > > enum xlate_error xlate_actions(struct xlate_in *, struct xlate_out *); > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 2cd786a..7d580dc 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -860,6 +860,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. > @@ -1584,6 +1590,8 @@ check_support(struct dpif_backer *backer) > backer->rt_support.max_hash_alg = check_max_dp_hash_alg(backer); > backer->rt_support.check_pkt_len = check_check_pkt_len(backer); > backer->rt_support.ct_timeout = check_ct_timeout_policy(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); > @@ -5550,6 +5558,8 @@ get_datapath_cap(const char *datapath_type, struct smap *cap) > > smap_add(cap, "check_pkt_len", s.check_pkt_len ? "true" : "false"); > smap_add(cap, "ct_timeout", s.ct_timeout ? "true" : "false"); > + smap_add(cap, "explicit_drop_action", > + s.explicit_drop_action ? "true" :"false"); > } > > /* Gets timeout policy name in 'backer' based on 'zone', 'dl_type' and > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index cd45363..bcaacef 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -199,7 +199,11 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *, > \ > /* True if the datapath supports OVS_CT_ATTR_TIMEOUT in \ > * OVS_ACTION_ATTR_CT action. */ \ > - DPIF_SUPPORT_FIELD(bool, ct_timeout, "Conntrack timeout policy") > + DPIF_SUPPORT_FIELD(bool, ct_timeout, "Conntrack timeout policy") \ > + \ > + /* 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 { > @@ -382,4 +386,6 @@ bool ofproto_dpif_ct_zone_timeout_policy_get_name( > const struct dpif_backer *backer, uint16_t zone, uint16_t dl_type, > uint8_t nw_proto, char **tp_name, bool *unwildcard); > > +bool ovs_explicit_drop_action_supported(struct ofproto_dpif *); > + > #endif /* ofproto-dpif.h */ > diff --git a/tests/automake.mk b/tests/automake.mk > index 4bf8f00..c9362ab 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -105,7 +105,8 @@ TESTSUITE_AT = \ > tests/auto-attach.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 ef521dd..0aeb4e7 100644 > --- a/tests/dpif-netdev.at > +++ b/tests/dpif-netdev.at > @@ -349,6 +349,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/read-counter datapath_drop_meter > +], [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..23df977 > --- /dev/null > +++ b/tests/drop-stats.at > @@ -0,0 +1,190 @@ > +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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > + ovs-appctl netdev-dummy/receive p1 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > + ovs-appctl netdev-dummy/receive p1 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > +], [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:212, used:0.0, actions:drop > +]) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_of_pipeline > +], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > +], [0], [ignore]) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_of_pipeline > +], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > +], [0], [ignore]) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_recursion_too_deep > +], [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)' > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_too_many_resubmit > +], [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)' > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_stack_too_deep > +], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > +], [0], [ignore]) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_too_many_mpls_labels > +], [0], [dnl > +1 > +]) > + > + > +OVS_VSWITCHD_STOP(["/|WARN|/d"]) > +AT_CLEANUP > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index 49326c5..1ba396a 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -9425,7 +9425,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 e759123..7369991 100644 > --- a/tests/testsuite.at > +++ b/tests/testsuite.at > @@ -76,3 +76,4 @@ m4_include([tests/auto-attach.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..b92c23f 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/read-counter datapath_drop_tunnel_pop_error > +], [0], [dnl > +1 > +]) > + > +AT_CHECK([ovs-appctl netdev-dummy/receive p0 'aa55aa550000001b213cab6408004503007079464000402fba600101025c0101025820000800000001c845000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_congestion > +], [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 faffb41..ce000a2 100644 > --- a/tests/tunnel.at > +++ b/tests/tunnel.at > @@ -102,8 +102,9 @@ 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], > - [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 > +AT_CHECK([tail -3 stdout], [0], > + [Final flow: unchanged > +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 > ]) > OVS_VSWITCHD_STOP(["/dropping tunnel packet marked ECN CE but is not ECN capable/d"]) > @@ -193,6 +194,17 @@ 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']) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter datapath_drop_invalid_port > +], [0], [dnl > +1 > +]) > + > OVS_VSWITCHD_STOP > AT_CLEANUP > > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index 09e7bdf..14f73cc 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -5917,6 +5917,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ > timeout policies based on connection tracking zones, as configured > through the <code>CT_Timeout_Policy</code> table. > </column> > + <column name="capabilities" key="explicit_drop_action" > + type='{"type": "boolean"}'> > + True if the datapath supports OVS_ACTION_ATTR_DROP. If false, > + explicit drop action will not be sent to the datapath. > + </column> > </group> > > <group title="Common Columns"> >
Thanks for the review Ilya, For the below comment > @@ -7086,10 +7105,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_)); This is not a tunnel push error. We literally executed all the actions without errors and that is not our fault that there are no more actions after tnl_push. We're just saving some time avoiding actual execution of a pointless action. So, this should not be accounted as error, and definitely not as a tunnel push error. I agree we need a better name .Does datapath_drop_incomplete_dp_action sound good or do you have any other suggestions in mind? Regards & Thanks Anju T -----Original Message----- From: Ilya Maximets <i.maximets@ovn.org> Sent: Friday, December 13, 2019 9:57 PM To: Anju Thomas <anju.thomas@ericsson.com>; dev@openvswitch.org Cc: i.maximets@ovn.org; echaudro@redhat.com; Rohith Basavaraja <rohith.basavaraja@gmail.com>; Keshav Gupta <keshugupta1@gmail.com> Subject: Re: [PATCH v17] Improved Packet Drop Statistics in OVS On 06.12.2019 09:49, 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 > > Co-authored-by: Rohith Basavaraja <rohith.basavaraja@gmail.com> > Co-authored-by: Keshav Gupta <keshugupta1@gmail.com> > Signed-off-by: Anju Thomas <anju.thomas@ericsson.com> > Signed-off-by: Rohith Basavaraja <rohith.basavaraja@gmail.com> > Signed-off-by: Keshav Gupta <keshugupta1@gmail.com> > Acked-by: Eelco Chaudron <echaudro@redhat.com > --- Thanks for a new version. I really hope that v18 will be the last one. This patch needs some minor rebase on top of current master. 6 more comments inline (some of them are really minor, but I decided to point to them as you'll re-spin the patch anyway). Please, fix them and send v18. Best regards, Ilya Maximets. > v17 (Ilya's comments) > 1. comments for xlate_error > 2. define xlate_error in ifndef __KERNEL__ 3. move declaration of > xlate_error after OVS_TUNNEL_KEY_ATTR_MAX 4. comment change for DROP > action 5. Add in datapath capability > > v16 (Ilya and Eelco comments) > 1. remove old declaration of xlate_error in v15 2. kernel file > formatting in openvswitch.h for xlate_error 3. remove drop_action > from odp_actions_from_string > > v15(Ilya's comments) > 1. Description in openvswitch.h > 2. Move xlate_error to openvswitch.h to not include ofproto heeader > in dp 3. Return u32 for instead of size of enum 4. Formatting 5. > Add drop-stats in testsuite.at 6. Use coverage-read-counter 7. Use > flow instead of raw packet data wherever possible 8. Change sleep to > warp > > v14 (Eelco's comments) > 1. Remove definition of dpif_show_drop_stats_support 2. Remove extra > check for drop reason in odp_execute_actions > > v13(Eelco and Illya's comments) > 1. packet_dropped changed to packets_dropped 2. Uses > dp_packet_batch_size instead of packet->size 3. Add version history > > v12(Ben's comments) > 1. new dp action in kernel block in openvswitch.h 2. change > xlate_error enum to be used as u32 3. resetting xlate error in case > of congestion drop > and forwarding disabled > --- > datapath/linux/compat/include/linux/openvswitch.h | 24 +++ > lib/dpif-netdev.c | 47 +++++- > lib/dpif.c | 7 + > lib/dpif.h | 2 + > lib/odp-execute.c | 77 +++++++++ > lib/odp-util.c | 5 + > ofproto/ofproto-dpif-ipfix.c | 1 + > ofproto/ofproto-dpif-sflow.c | 1 + > ofproto/ofproto-dpif-xlate.c | 33 +++- > ofproto/ofproto-dpif-xlate.h | 13 -- > ofproto/ofproto-dpif.c | 10 ++ > ofproto/ofproto-dpif.h | 8 +- > tests/automake.mk | 3 +- > tests/dpif-netdev.at | 8 + > tests/drop-stats.at | 190 ++++++++++++++++++++++ > tests/ofproto-dpif.at | 2 +- > tests/testsuite.at | 1 + > tests/tunnel-push-pop.at | 28 +++- > tests/tunnel.at | 16 +- > vswitchd/vswitch.xml | 5 + > 20 files changed, 457 insertions(+), 24 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 778827f..8006724 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -407,6 +407,28 @@ enum ovs_tunnel_key_attr { #define > OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1) > > /** > + * enum xlate_error - Different types of error during translation */ > + > +#ifndef __KERNEL__ > +enum xlate_error { > + XLATE_OK = 0, > + XLATE_BRIDGE_NOT_FOUND, > + XLATE_RECURSION_TOO_DEEP, > + XLATE_TOO_MANY_RESUBMITS, > + XLATE_STACK_TOO_DEEP, > + XLATE_NO_RECIRCULATION_CONTEXT, > + XLATE_RECIRCULATION_CONFLICT, > + XLATE_TOO_MANY_MPLS_LABELS, > + XLATE_INVALID_TUNNEL_METADATA, > + XLATE_UNSUPPORTED_PACKET_TYPE, > + XLATE_CONGESTION_DROP, > + XLATE_FORWARDING_DISABLED, > + XLATE_MAX, > +}; > +#endif Over and over again, kernel coding style, please. > + > +/** > * enum ovs_frag_type - IPv4 and IPv6 fragment type > * @OVS_FRAG_TYPE_NONE: Packet is not a fragment. > * @OVS_FRAG_TYPE_FIRST: Packet is a fragment with offset 0. > @@ -962,6 +984,7 @@ struct check_pkt_len_arg { > * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set > * of actions if greater than the specified packet length, else execute > * another set of actions. > + * @OVS_ACTION_ATTR_DROP: Explicit drop action. > */ > > enum ovs_action_attr { > @@ -994,6 +1017,7 @@ enum ovs_action_attr { #ifndef __KERNEL__ > OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/ > OVS_ACTION_ATTR_TUNNEL_POP, /* u32 port number. */ > + OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */ > #endif > __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted > * from userspace. */ > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index > 5142bad..8adeac5 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -102,6 +102,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; > > @@ -5753,7 +5764,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_, > band = &meter->bands[exceeded_band[j]]; > band->packet_count += 1; > band->byte_count += dp_packet_size(packet); > - > + COVERAGE_INC(datapath_drop_meter); > dp_packet_delete(packet); > } else { > /* Meter accepts packet. */ @@ -6503,6 +6514,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; > } > > @@ -6623,6 +6635,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; > } > > @@ -6753,6 +6766,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++; > } > } > @@ -7022,6 +7036,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); > } > } > > @@ -7036,6 +7051,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, packets_dropped; > > switch ((enum ovs_action_attr)type) { > case OVS_ACTION_ATTR_OUTPUT: > @@ -7077,6 +7093,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; > > @@ -7086,10 +7105,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_)); This is not a tunnel push error. We literally executed all the actions without errors and that is not our fault that there are no more actions after tnl_push. We're just saving some time avoiding actual execution of a pointless action. So, this should not be accounted as error, and definitely not as a tunnel push error. > 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: > @@ -7109,7 +7134,14 @@ dp_execute_cb(void *aux_, struct > dp_packet_batch *packets_, > > dp_packet_batch_apply_cutlen(packets_); > > + packet_count = dp_packet_batch_size(packets_); > netdev_pop_header(p->port->netdev, packets_); > + packets_dropped = > + packet_count - dp_packet_batch_size(packets_); > + if (packets_dropped) { > + COVERAGE_ADD(datapath_drop_tunnel_pop_error, > + packets_dropped); > + } > if (dp_packet_batch_is_empty(packets_)) { > return; > } > @@ -7124,6 +7156,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; > > @@ -7168,6 +7205,9 @@ dp_execute_cb(void *aux_, struct dp_packet_batch > *packets_, > > return; > } > + COVERAGE_ADD(datapath_drop_lock_error, > + dp_packet_batch_size(packets_)); > + Empty line not needed here. > break; > > case OVS_ACTION_ATTR_RECIRC: > @@ -7191,6 +7231,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; > > @@ -7348,6 +7390,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_CT_CLEAR: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + case OVS_ACTION_ATTR_DROP: > case __OVS_ACTION_ATTR_MAX: > OVS_NOT_REACHED(); > } > diff --git a/lib/dpif.c b/lib/dpif.c > index c88b210..5a9ff46 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_CT_CLEAR: > case OVS_ACTION_ATTR_UNSPEC: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + 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 c98513e..7cc2220 100644 > --- a/lib/dpif.h > +++ b/lib/dpif.h > @@ -892,6 +892,8 @@ 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 *); > + New empty line is not needed here. > > /* Log functions. */ > struct vlog_module; > diff --git a/lib/odp-execute.c b/lib/odp-execute.c index > 563ad1d..3d4ad9e 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,72 @@ > #include "util.h" > #include "csum.h" > #include "conntrack.h" > +#include "openvswitch/vlog.h" > + > +VLOG_DEFINE_THIS_MODULE(odp_execute); > +COVERAGE_DEFINE(datapath_drop_sample_error); > +COVERAGE_DEFINE(datapath_drop_nsh_decap_error); > +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); Please, add a single space after ':'. > + } > +} > > /* Masked copy of an ethernet address. 'src' is already properly > masked. */ static void @@ -621,6 +688,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_INC(datapath_drop_sample_error); > dp_packet_delete(packet); > } > return; > @@ -749,6 +817,7 @@ requires_datapath_assistance(const struct nlattr *a) > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_CT_CLEAR: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + case OVS_ACTION_ATTR_DROP: > return false; > > case OVS_ACTION_ATTR_UNSPEC: > @@ -965,6 +1034,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(datapath_drop_nsh_decap_error); > dp_packet_delete(packet); > } > } > @@ -989,6 +1059,13 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal, > } > break; > > + case OVS_ACTION_ATTR_DROP:{ > + const enum xlate_error *drop_reason = nl_attr_get(a); > + > + dp_update_drop_action_counter(*drop_reason, > + batch->count); dp_packet_batch_size(batch) > + dp_packet_delete_batch(batch, steal); > + 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 b9600b4..9bda91f > 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -141,6 +141,7 @@ odp_action_len(uint16_t type) > case OVS_ACTION_ATTR_PUSH_NSH: return ATTR_LEN_VARIABLE; > case OVS_ACTION_ATTR_POP_NSH: return 0; > case OVS_ACTION_ATTR_CHECK_PKT_LEN: return ATTR_LEN_VARIABLE; > + case OVS_ACTION_ATTR_DROP: return sizeof(uint32_t); > > case OVS_ACTION_ATTR_UNSPEC: > case __OVS_ACTION_ATTR_MAX: > @@ -1238,6 +1239,9 @@ format_odp_action(struct ds *ds, const struct nlattr *a, > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > format_odp_check_pkt_len_action(ds, a, portno_names); > break; > + case OVS_ACTION_ATTR_DROP: > + ds_put_cstr(ds, "drop"); > + break; > case OVS_ACTION_ATTR_UNSPEC: > case __OVS_ACTION_ATTR_MAX: > default: > @@ -2575,6 +2579,7 @@ odp_actions_from_string(const char *s, const struct simap *port_names, > size_t old_size; > > if (!strcasecmp(s, "drop")) { > + nl_msg_put_u32(actions, OVS_ACTION_ATTR_DROP, XLATE_OK); > return 0; > } > > diff --git a/ofproto/ofproto-dpif-ipfix.c > b/ofproto/ofproto-dpif-ipfix.c index b8bd1b8..b413768 100644 > --- a/ofproto/ofproto-dpif-ipfix.c > +++ b/ofproto/ofproto-dpif-ipfix.c > @@ -3016,6 +3016,7 @@ dpif_ipfix_read_actions(const struct flow *flow, > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > 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 9abaab6..f9ea47a 100644 > --- a/ofproto/ofproto-dpif-sflow.c > +++ b/ofproto/ofproto-dpif-sflow.c > @@ -1224,6 +1224,7 @@ dpif_sflow_read_actions(const struct flow *flow, > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_UNSPEC: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + 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 cebae7a..222c954 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -445,6 +445,12 @@ 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"; > + case XLATE_MAX: > + break; > } > return "Unknown error"; > } > @@ -6002,6 +6008,12 @@ 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_u32(odp_actions, OVS_ACTION_ATTR_DROP, error); } > + > +static void > put_ct_helper(struct xlate_ctx *ctx, > struct ofpbuf *odp_actions, struct ofpact_conntrack > *ofc) { @@ -7638,8 +7650,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) > + if (!ecn_drop > && (!in_port || may_receive(in_port, &ctx))) { > const struct ofpact *ofpacts; > size_t ofpacts_len; > @@ -7671,6 +7684,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) { > @@ -7679,6 +7693,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. */ @@ -7784,6 > +7800,21 @@ exit: > ofpbuf_clear(xin->odp_actions); > } > } > + > + /* Install drop action if datapath supports explicit drop action. */ > + if (xin->odp_actions && !xin->odp_actions->size && > + ovs_explicit_drop_action_supported(ctx.xbridge->ofproto)) { > + put_drop_action(xin->odp_actions, ctx.error); > + } > + > + /* Since congestion drop and forwarding drop are not exactly > + * translation error, we are resetting the translation error. > + */ > + if (ctx.error == XLATE_CONGESTION_DROP || > + ctx.error == XLATE_FORWARDING_DISABLED) { > + ctx.error = XLATE_OK; > + } > + > return ctx.error; > } > > diff --git a/ofproto/ofproto-dpif-xlate.h > b/ofproto/ofproto-dpif-xlate.h index f97c7c0..3426a27 100644 > --- a/ofproto/ofproto-dpif-xlate.h > +++ b/ofproto/ofproto-dpif-xlate.h > @@ -206,19 +206,6 @@ int xlate_lookup(const struct dpif_backer *, const struct flow *, > struct dpif_sflow **, struct netflow **, > ofp_port_t *ofp_in_port); > > -enum xlate_error { > - XLATE_OK = 0, > - XLATE_BRIDGE_NOT_FOUND, > - XLATE_RECURSION_TOO_DEEP, > - XLATE_TOO_MANY_RESUBMITS, > - XLATE_STACK_TOO_DEEP, > - XLATE_NO_RECIRCULATION_CONTEXT, > - XLATE_RECIRCULATION_CONFLICT, > - XLATE_TOO_MANY_MPLS_LABELS, > - XLATE_INVALID_TUNNEL_METADATA, > - XLATE_UNSUPPORTED_PACKET_TYPE, > -}; > - > const char *xlate_strerror(enum xlate_error error); > > enum xlate_error xlate_actions(struct xlate_in *, struct xlate_out > *); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index > 2cd786a..7d580dc 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -860,6 +860,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. > @@ -1584,6 +1590,8 @@ check_support(struct dpif_backer *backer) > backer->rt_support.max_hash_alg = check_max_dp_hash_alg(backer); > backer->rt_support.check_pkt_len = check_check_pkt_len(backer); > backer->rt_support.ct_timeout = check_ct_timeout_policy(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); @@ > -5550,6 +5558,8 @@ get_datapath_cap(const char *datapath_type, struct > smap *cap) > > smap_add(cap, "check_pkt_len", s.check_pkt_len ? "true" : "false"); > smap_add(cap, "ct_timeout", s.ct_timeout ? "true" : "false"); > + smap_add(cap, "explicit_drop_action", > + s.explicit_drop_action ? "true" :"false"); > } > > /* Gets timeout policy name in 'backer' based on 'zone', 'dl_type' > and diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index > cd45363..bcaacef 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -199,7 +199,11 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *, > \ > /* True if the datapath supports OVS_CT_ATTR_TIMEOUT in \ > * OVS_ACTION_ATTR_CT action. */ \ > - DPIF_SUPPORT_FIELD(bool, ct_timeout, "Conntrack timeout policy") > + DPIF_SUPPORT_FIELD(bool, ct_timeout, "Conntrack timeout policy") \ > + \ > + /* 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 { @@ -382,4 +386,6 @@ bool > ofproto_dpif_ct_zone_timeout_policy_get_name( > const struct dpif_backer *backer, uint16_t zone, uint16_t dl_type, > uint8_t nw_proto, char **tp_name, bool *unwildcard); > > +bool ovs_explicit_drop_action_supported(struct ofproto_dpif *); > + > #endif /* ofproto-dpif.h */ > diff --git a/tests/automake.mk b/tests/automake.mk index > 4bf8f00..c9362ab 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -105,7 +105,8 @@ TESTSUITE_AT = \ > tests/auto-attach.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 > ef521dd..0aeb4e7 100644 > --- a/tests/dpif-netdev.at > +++ b/tests/dpif-netdev.at > @@ -349,6 +349,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/read-counter datapath_drop_meter ], [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(f > rag=no), actions:meter(0),7 > recirc_id(0),in_port(2),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(f > rag=no), actions:8 diff --git a/tests/drop-stats.at > b/tests/drop-stats.at new file mode 100644 index 0000000..23df977 > --- /dev/null > +++ b/tests/drop-stats.at > @@ -0,0 +1,190 @@ > +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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > + ovs-appctl netdev-dummy/receive p1 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > + ovs-appctl netdev-dummy/receive p1 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > +], [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:212, used:0.0, actions:drop > +]) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_of_pipeline ], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > +], [0], [ignore]) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_of_pipeline ], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > +], [0], [ignore]) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_recursion_too_deep ], > +[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)' > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_too_many_resubmit ], > +[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)' > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_stack_too_deep ], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' > +], [0], [ignore]) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_too_many_mpls_labels ], > +[0], [dnl > +1 > +]) > + > + > +OVS_VSWITCHD_STOP(["/|WARN|/d"]) > +AT_CLEANUP > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index > 49326c5..1ba396a 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -9425,7 +9425,7 @@ > recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x8100),vlan(v > id=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]] > -dpif|DBG|skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),c > -dpif|DBG|t_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port( > -dpif|DBG|1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09/00:00:00 > -dpif|DBG|:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type( > -dpif|DBG|0x1234) > +dpif|DBG|dummy@ovs-dummy: put[[modify]] > +dpif|DBG|skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),c > +dpif|DBG|t_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port( > +dpif|DBG|1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09/00:00:00 > +dpif|DBG|:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type( > +dpif|DBG|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:0 > a/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(v > id=99,pcp=7/0x0),encap(eth_type(0x1234)), actions:drop > ]) > diff --git a/tests/testsuite.at b/tests/testsuite.at index > e759123..7369991 100644 > --- a/tests/testsuite.at > +++ b/tests/testsuite.at > @@ -76,3 +76,4 @@ m4_include([tests/auto-attach.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..b92c23f 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 > +'aa55aa550000001b213cab6408004500007079464000402fba600101025c01010258 > +20000800000001c845000054ba200000400184861e0000011e00000200004227e7540 > +0030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f20 > +2122232425262728292a2b2c2d2e2f3031323334353637']) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter datapath_drop_tunnel_pop_error ], > +[0], [dnl > +1 > +]) > + > +AT_CHECK([ovs-appctl netdev-dummy/receive p0 > +'aa55aa550000001b213cab6408004503007079464000402fba600101025c01010258 > +20000800000001c845000054ba200000400184861e0000011e00000200004227e7540 > +0030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f20 > +2122232425262728292a2b2c2d2e2f3031323334353637']) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter drop_action_congestion ], [0], [dnl > +1 > +]) > + > + > dnl Check GREL3 only accepts non-fragmented packets? > AT_CHECK([ovs-appctl netdev-dummy/receive p0 > 'aa55aa550000001b213cab6408004500007e79464000402fba550101025c010102582 > 0000800000001c8fe71d883724fbeb6f4e1494a080045000054ba200000400184861e0 > 000011e00000200004227e75400030af3195500000000f265010000000000101112131 > 415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363 > 7']) > > @@ -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,typ > e=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),ip > v4(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,typ > +e=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 faffb41..ce000a2 100644 > --- a/tests/tunnel.at > +++ b/tests/tunnel.at > @@ -102,8 +102,9 @@ 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(sr > c=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], > - [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 > +AT_CHECK([tail -3 stdout], [0], > + [Final flow: unchanged > +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 > ]) > OVS_VSWITCHD_STOP(["/dropping tunnel packet marked ECN CE but is not > ECN capable/d"]) @@ -193,6 +194,17 @@ 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))),s > et(skb_mark(0x2)),1 > ]) > + > +AT_CHECK([ovs-appctl netdev-dummy/receive p2 > +'aa55aa550001f8bc124434b6080045000054ba200000400184860101035801010370 > +01004227e75400030af3195500000000f265010000000000101112131415161718191 > +a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) > + > +ovs-appctl time/warp 5000 > + > +AT_CHECK([ > +ovs-appctl coverage/read-counter datapath_drop_invalid_port ], [0], > +[dnl > +1 > +]) > + > OVS_VSWITCHD_STOP > AT_CLEANUP > > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index > 09e7bdf..14f73cc 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -5917,6 +5917,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ > timeout policies based on connection tracking zones, as configured > through the <code>CT_Timeout_Policy</code> table. > </column> > + <column name="capabilities" key="explicit_drop_action" > + type='{"type": "boolean"}'> > + True if the datapath supports OVS_ACTION_ATTR_DROP. If false, > + explicit drop action will not be sent to the datapath. > + </column> > </group> > > <group title="Common Columns"> >
On 16.12.2019 05:07, Anju Thomas wrote: > Thanks for the review Ilya, > > For the below comment > > >> @@ -7086,10 +7105,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_)); > This is not a tunnel push error. We literally executed all the actions without errors and that is not our fault that there are no more actions after tnl_push. We're just saving some time avoiding actual execution of a pointless action. So, this should not be accounted as error, and definitely not as a tunnel push error. > > I agree we need a better name .Does datapath_drop_incomplete_dp_action sound good or do you have any other suggestions in mind? datapath_drop_last_action_tnl_push ? And we actually could just avoid counting in this case because: 1. It's a normal scenario of action execution. If the code looked like this: case OVS_ACTION_ATTR_TUNNEL_PUSH: dp_packet_batch_apply_cutlen(packets_); push_tnl_action(pmd, a, packets_) break; you most probably wouldn't noticed this case because it's perfectly normal. 2. We're not tracking similar cases for all other actions. What if push_vlan, set_ipv4_src or meter is the last action? The only reason why we have a spacial case with big comment for tunnel push is that we had dp_packet leak here and we don't really want to execute heavy push action if we're going to drop these packets anyway. Best regards, Ilya Maximets.
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h index 778827f..8006724 100644 --- a/datapath/linux/compat/include/linux/openvswitch.h +++ b/datapath/linux/compat/include/linux/openvswitch.h @@ -407,6 +407,28 @@ enum ovs_tunnel_key_attr { #define OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1) /** + * enum xlate_error - Different types of error during translation + */ + +#ifndef __KERNEL__ +enum xlate_error { + XLATE_OK = 0, + XLATE_BRIDGE_NOT_FOUND, + XLATE_RECURSION_TOO_DEEP, + XLATE_TOO_MANY_RESUBMITS, + XLATE_STACK_TOO_DEEP, + XLATE_NO_RECIRCULATION_CONTEXT, + XLATE_RECIRCULATION_CONFLICT, + XLATE_TOO_MANY_MPLS_LABELS, + XLATE_INVALID_TUNNEL_METADATA, + XLATE_UNSUPPORTED_PACKET_TYPE, + XLATE_CONGESTION_DROP, + XLATE_FORWARDING_DISABLED, + XLATE_MAX, +}; +#endif + +/** * enum ovs_frag_type - IPv4 and IPv6 fragment type * @OVS_FRAG_TYPE_NONE: Packet is not a fragment. * @OVS_FRAG_TYPE_FIRST: Packet is a fragment with offset 0. @@ -962,6 +984,7 @@ struct check_pkt_len_arg { * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set * of actions if greater than the specified packet length, else execute * another set of actions. + * @OVS_ACTION_ATTR_DROP: Explicit drop action. */ enum ovs_action_attr { @@ -994,6 +1017,7 @@ enum ovs_action_attr { #ifndef __KERNEL__ OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/ OVS_ACTION_ATTR_TUNNEL_POP, /* u32 port number. */ + OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */ #endif __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted * from userspace. */ diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 5142bad..8adeac5 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -102,6 +102,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; @@ -5753,7 +5764,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_, band = &meter->bands[exceeded_band[j]]; band->packet_count += 1; band->byte_count += dp_packet_size(packet); - + COVERAGE_INC(datapath_drop_meter); dp_packet_delete(packet); } else { /* Meter accepts packet. */ @@ -6503,6 +6514,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; } @@ -6623,6 +6635,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; } @@ -6753,6 +6766,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++; } } @@ -7022,6 +7036,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); } } @@ -7036,6 +7051,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, packets_dropped; switch ((enum ovs_action_attr)type) { case OVS_ACTION_ATTR_OUTPUT: @@ -7077,6 +7093,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; @@ -7086,10 +7105,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: @@ -7109,7 +7134,14 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, dp_packet_batch_apply_cutlen(packets_); + packet_count = dp_packet_batch_size(packets_); netdev_pop_header(p->port->netdev, packets_); + packets_dropped = + packet_count - dp_packet_batch_size(packets_); + if (packets_dropped) { + COVERAGE_ADD(datapath_drop_tunnel_pop_error, + packets_dropped); + } if (dp_packet_batch_is_empty(packets_)) { return; } @@ -7124,6 +7156,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; @@ -7168,6 +7205,9 @@ 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: @@ -7191,6 +7231,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; @@ -7348,6 +7390,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, case OVS_ACTION_ATTR_POP_NSH: case OVS_ACTION_ATTR_CT_CLEAR: case OVS_ACTION_ATTR_CHECK_PKT_LEN: + case OVS_ACTION_ATTR_DROP: case __OVS_ACTION_ATTR_MAX: OVS_NOT_REACHED(); } diff --git a/lib/dpif.c b/lib/dpif.c index c88b210..5a9ff46 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_CT_CLEAR: case OVS_ACTION_ATTR_UNSPEC: case OVS_ACTION_ATTR_CHECK_PKT_LEN: + 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 c98513e..7cc2220 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -892,6 +892,8 @@ 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 *); + /* Log functions. */ struct vlog_module; diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 563ad1d..3d4ad9e 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,72 @@ #include "util.h" #include "csum.h" #include "conntrack.h" +#include "openvswitch/vlog.h" + +VLOG_DEFINE_THIS_MODULE(odp_execute); +COVERAGE_DEFINE(datapath_drop_sample_error); +COVERAGE_DEFINE(datapath_drop_nsh_decap_error); +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 @@ -621,6 +688,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_INC(datapath_drop_sample_error); dp_packet_delete(packet); } return; @@ -749,6 +817,7 @@ requires_datapath_assistance(const struct nlattr *a) case OVS_ACTION_ATTR_POP_NSH: case OVS_ACTION_ATTR_CT_CLEAR: case OVS_ACTION_ATTR_CHECK_PKT_LEN: + case OVS_ACTION_ATTR_DROP: return false; case OVS_ACTION_ATTR_UNSPEC: @@ -965,6 +1034,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(datapath_drop_nsh_decap_error); dp_packet_delete(packet); } } @@ -989,6 +1059,13 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal, } break; + case OVS_ACTION_ATTR_DROP:{ + const enum xlate_error *drop_reason = nl_attr_get(a); + + dp_update_drop_action_counter(*drop_reason, batch->count); + dp_packet_delete_batch(batch, steal); + 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 b9600b4..9bda91f 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -141,6 +141,7 @@ odp_action_len(uint16_t type) case OVS_ACTION_ATTR_PUSH_NSH: return ATTR_LEN_VARIABLE; case OVS_ACTION_ATTR_POP_NSH: return 0; case OVS_ACTION_ATTR_CHECK_PKT_LEN: return ATTR_LEN_VARIABLE; + case OVS_ACTION_ATTR_DROP: return sizeof(uint32_t); case OVS_ACTION_ATTR_UNSPEC: case __OVS_ACTION_ATTR_MAX: @@ -1238,6 +1239,9 @@ format_odp_action(struct ds *ds, const struct nlattr *a, case OVS_ACTION_ATTR_CHECK_PKT_LEN: format_odp_check_pkt_len_action(ds, a, portno_names); break; + case OVS_ACTION_ATTR_DROP: + ds_put_cstr(ds, "drop"); + break; case OVS_ACTION_ATTR_UNSPEC: case __OVS_ACTION_ATTR_MAX: default: @@ -2575,6 +2579,7 @@ odp_actions_from_string(const char *s, const struct simap *port_names, size_t old_size; if (!strcasecmp(s, "drop")) { + nl_msg_put_u32(actions, OVS_ACTION_ATTR_DROP, XLATE_OK); return 0; } diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c index b8bd1b8..b413768 100644 --- a/ofproto/ofproto-dpif-ipfix.c +++ b/ofproto/ofproto-dpif-ipfix.c @@ -3016,6 +3016,7 @@ dpif_ipfix_read_actions(const struct flow *flow, case OVS_ACTION_ATTR_POP_NSH: case OVS_ACTION_ATTR_CHECK_PKT_LEN: 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 9abaab6..f9ea47a 100644 --- a/ofproto/ofproto-dpif-sflow.c +++ b/ofproto/ofproto-dpif-sflow.c @@ -1224,6 +1224,7 @@ dpif_sflow_read_actions(const struct flow *flow, case OVS_ACTION_ATTR_POP_NSH: case OVS_ACTION_ATTR_UNSPEC: case OVS_ACTION_ATTR_CHECK_PKT_LEN: + 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 cebae7a..222c954 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -445,6 +445,12 @@ 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"; + case XLATE_MAX: + break; } return "Unknown error"; } @@ -6002,6 +6008,12 @@ 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_u32(odp_actions, OVS_ACTION_ATTR_DROP, error); +} + +static void put_ct_helper(struct xlate_ctx *ctx, struct ofpbuf *odp_actions, struct ofpact_conntrack *ofc) { @@ -7638,8 +7650,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) + if (!ecn_drop && (!in_port || may_receive(in_port, &ctx))) { const struct ofpact *ofpacts; size_t ofpacts_len; @@ -7671,6 +7684,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) { @@ -7679,6 +7693,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. */ @@ -7784,6 +7800,21 @@ exit: ofpbuf_clear(xin->odp_actions); } } + + /* Install drop action if datapath supports explicit drop action. */ + if (xin->odp_actions && !xin->odp_actions->size && + ovs_explicit_drop_action_supported(ctx.xbridge->ofproto)) { + put_drop_action(xin->odp_actions, ctx.error); + } + + /* Since congestion drop and forwarding drop are not exactly + * translation error, we are resetting the translation error. + */ + if (ctx.error == XLATE_CONGESTION_DROP || + ctx.error == XLATE_FORWARDING_DISABLED) { + ctx.error = XLATE_OK; + } + return ctx.error; } diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h index f97c7c0..3426a27 100644 --- a/ofproto/ofproto-dpif-xlate.h +++ b/ofproto/ofproto-dpif-xlate.h @@ -206,19 +206,6 @@ int xlate_lookup(const struct dpif_backer *, const struct flow *, struct dpif_sflow **, struct netflow **, ofp_port_t *ofp_in_port); -enum xlate_error { - XLATE_OK = 0, - XLATE_BRIDGE_NOT_FOUND, - XLATE_RECURSION_TOO_DEEP, - XLATE_TOO_MANY_RESUBMITS, - XLATE_STACK_TOO_DEEP, - XLATE_NO_RECIRCULATION_CONTEXT, - XLATE_RECIRCULATION_CONFLICT, - XLATE_TOO_MANY_MPLS_LABELS, - XLATE_INVALID_TUNNEL_METADATA, - XLATE_UNSUPPORTED_PACKET_TYPE, -}; - const char *xlate_strerror(enum xlate_error error); enum xlate_error xlate_actions(struct xlate_in *, struct xlate_out *); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 2cd786a..7d580dc 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -860,6 +860,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. @@ -1584,6 +1590,8 @@ check_support(struct dpif_backer *backer) backer->rt_support.max_hash_alg = check_max_dp_hash_alg(backer); backer->rt_support.check_pkt_len = check_check_pkt_len(backer); backer->rt_support.ct_timeout = check_ct_timeout_policy(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); @@ -5550,6 +5558,8 @@ get_datapath_cap(const char *datapath_type, struct smap *cap) smap_add(cap, "check_pkt_len", s.check_pkt_len ? "true" : "false"); smap_add(cap, "ct_timeout", s.ct_timeout ? "true" : "false"); + smap_add(cap, "explicit_drop_action", + s.explicit_drop_action ? "true" :"false"); } /* Gets timeout policy name in 'backer' based on 'zone', 'dl_type' and diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index cd45363..bcaacef 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -199,7 +199,11 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *, \ /* True if the datapath supports OVS_CT_ATTR_TIMEOUT in \ * OVS_ACTION_ATTR_CT action. */ \ - DPIF_SUPPORT_FIELD(bool, ct_timeout, "Conntrack timeout policy") + DPIF_SUPPORT_FIELD(bool, ct_timeout, "Conntrack timeout policy") \ + \ + /* 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 { @@ -382,4 +386,6 @@ bool ofproto_dpif_ct_zone_timeout_policy_get_name( const struct dpif_backer *backer, uint16_t zone, uint16_t dl_type, uint8_t nw_proto, char **tp_name, bool *unwildcard); +bool ovs_explicit_drop_action_supported(struct ofproto_dpif *); + #endif /* ofproto-dpif.h */ diff --git a/tests/automake.mk b/tests/automake.mk index 4bf8f00..c9362ab 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -105,7 +105,8 @@ TESTSUITE_AT = \ tests/auto-attach.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 ef521dd..0aeb4e7 100644 --- a/tests/dpif-netdev.at +++ b/tests/dpif-netdev.at @@ -349,6 +349,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/read-counter datapath_drop_meter +], [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..23df977 --- /dev/null +++ b/tests/drop-stats.at @@ -0,0 +1,190 @@ +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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' + ovs-appctl netdev-dummy/receive p1 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' + ovs-appctl netdev-dummy/receive p1 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' +], [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:212, used:0.0, actions:drop +]) + +ovs-appctl time/warp 5000 + +AT_CHECK([ +ovs-appctl coverage/read-counter drop_action_of_pipeline +], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' +], [0], [ignore]) + +ovs-appctl time/warp 5000 + +AT_CHECK([ +ovs-appctl coverage/read-counter drop_action_of_pipeline +], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' +], [0], [ignore]) + +ovs-appctl time/warp 5000 + +AT_CHECK([ +ovs-appctl coverage/read-counter drop_action_recursion_too_deep +], [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)' + +ovs-appctl time/warp 5000 + +AT_CHECK([ +ovs-appctl coverage/read-counter drop_action_too_many_resubmit +], [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)' + +ovs-appctl time/warp 5000 + +AT_CHECK([ +ovs-appctl coverage/read-counter drop_action_stack_too_deep +], [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 'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' +], [0], [ignore]) + +ovs-appctl time/warp 5000 + +AT_CHECK([ +ovs-appctl coverage/read-counter drop_action_too_many_mpls_labels +], [0], [dnl +1 +]) + + +OVS_VSWITCHD_STOP(["/|WARN|/d"]) +AT_CLEANUP diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 49326c5..1ba396a 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -9425,7 +9425,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 e759123..7369991 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -76,3 +76,4 @@ m4_include([tests/auto-attach.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..b92c23f 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/read-counter datapath_drop_tunnel_pop_error +], [0], [dnl +1 +]) + +AT_CHECK([ovs-appctl netdev-dummy/receive p0 'aa55aa550000001b213cab6408004503007079464000402fba600101025c0101025820000800000001c845000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) + +ovs-appctl time/warp 5000 + +AT_CHECK([ +ovs-appctl coverage/read-counter drop_action_congestion +], [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 faffb41..ce000a2 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -102,8 +102,9 @@ 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], - [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 +AT_CHECK([tail -3 stdout], [0], + [Final flow: unchanged +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 ]) OVS_VSWITCHD_STOP(["/dropping tunnel packet marked ECN CE but is not ECN capable/d"]) @@ -193,6 +194,17 @@ 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']) + +ovs-appctl time/warp 5000 + +AT_CHECK([ +ovs-appctl coverage/read-counter datapath_drop_invalid_port +], [0], [dnl +1 +]) + OVS_VSWITCHD_STOP AT_CLEANUP diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 09e7bdf..14f73cc 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -5917,6 +5917,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ timeout policies based on connection tracking zones, as configured through the <code>CT_Timeout_Policy</code> table. </column> + <column name="capabilities" key="explicit_drop_action" + type='{"type": "boolean"}'> + True if the datapath supports OVS_ACTION_ATTR_DROP. If false, + explicit drop action will not be sent to the datapath. + </column> </group> <group title="Common Columns">