On 7/12/24 19:06, Adrian Moreno wrote:
> When the flow translation results in a datapath action list whose last
> action is an "observational" action, i.e: one generated for IPFIX,
> sFlow or local sampling applications, the packet is actually going to be
> dropped (and observed).
>
> In that case, add an explicit drop action so that drop statistics remain
> accurate. This behavior is controlled by a configurable boolean knob
> called "explicit_sampled_drops"
>
> Combine the "optimizations" and other odp_actions "tweaks" into a single
> function.
>
> Signed-off-by: Adrian Moreno <[email protected]>
> ---
> NEWS | 5 +
> ofproto/ofproto-dpif-xlate.c | 64 ++++++++++---
> ofproto/ofproto-dpif-xlate.h | 4 +
> ofproto/ofproto-dpif.c | 6 ++
> ofproto/ofproto-dpif.h | 2 +
> ofproto/ofproto-provider.h | 4 +
> ofproto/ofproto.c | 10 ++
> ofproto/ofproto.h | 2 +
> tests/drop-stats.at | 173 +++++++++++++++++++++++++++++++++++
> tests/ofproto-dpif.at | 49 ++++++++++
> vswitchd/bridge.c | 4 +-
> vswitchd/vswitch.xml | 26 +++++-
> 12 files changed, 333 insertions(+), 16 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index c27727c0d..4eb73aada 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -40,6 +40,11 @@ Post-v3.3.0
> allows samples to be emitted locally (instead of via IPFIX) in a
> datapath-specific manner. The Linux kernel datapath is the first to
> support this feature by using the new datapath "psample" action.
> + - OVSDB:
We normally use OVSDB for changes in the database server, not the vswitch
schema. Just remove it.
> + * A new configuration knob "other_config:explicit-sampled-drops" in the
> + Open_vSwitch table controls whether an explicit drop action shall be
> + added at the end of datapath flows whose last action is an
> + observability-driven sample action.
>
>
> v3.3.0 - 16 Feb 2024
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 0a0964348..e5234b950 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3415,6 +3415,7 @@ compose_sample_action(struct xlate_ctx *ctx,
> struct ofproto *ofproto = &ctx->xin->ofproto->up;
> uint32_t meter_id = ofproto->slowpath_meter_id;
> size_t cookie_offset = 0;
> + size_t observe_offset;
>
> /* The meter action is only used to throttle userspace actions.
> * If they are not needed and the sampling rate is 100%, avoid generating
> @@ -3432,6 +3433,7 @@ compose_sample_action(struct xlate_ctx *ctx,
> }
>
> if (args->psample) {
> + observe_offset = ctx->odp_actions->size;
> odp_put_psample_action(ctx->odp_actions,
> args->psample->group_id,
> (void *) &args->psample->cookie,
> @@ -3443,6 +3445,7 @@ compose_sample_action(struct xlate_ctx *ctx,
> nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER,
> meter_id);
> }
>
> + observe_offset = ctx->odp_actions->size;
> odp_port_t odp_port = ofp_port_to_odp_port(
> ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
> uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port);
> @@ -3457,6 +3460,9 @@ compose_sample_action(struct xlate_ctx *ctx,
> if (is_sample) {
> nl_msg_end_nested(ctx->odp_actions, actions_offset);
> nl_msg_end_nested(ctx->odp_actions, sample_offset);
> + ctx->xout->last_observe_offset = sample_offset;
> + } else {
> + ctx->xout->last_observe_offset = observe_offset;
> }
>
> return cookie_offset;
> @@ -8066,12 +8072,16 @@ xlate_wc_finish(struct xlate_ctx *ctx)
> }
> }
>
> -/* This will optimize the odp actions generated. For now, it will remove
> - * trailing clone actions that are unnecessary. */
> +/* This will tweak the odp actions generated. For now, it will:
> + * - Remove trailing clone actions that are unnecessary.
> + * - Add an explicit drop action if the action list is empty.
> + * - Add an explicit drop action if the last action is an observability
> + * sample. This tweak is controlled by a configurable knob. */
> static void
> -xlate_optimize_odp_actions(struct xlate_in *xin)
> +xlate_tweak_odp_actions(struct xlate_ctx *ctx)
> {
> - struct ofpbuf *actions = xin->odp_actions;
> + uint32_t last_observe_offset = ctx->xout->last_observe_offset;
> + struct ofpbuf *actions = ctx->xin->odp_actions;
> struct nlattr *last_action = NULL;
> struct nlattr *a;
> int left;
> @@ -8085,11 +8095,28 @@ xlate_optimize_odp_actions(struct xlate_in *xin)
> last_action = a;
> }
>
> + if (!last_action) {
> + if (ovs_explicit_drop_action_supported(ctx->xbridge->ofproto)) {
> + put_drop_action(actions, XLATE_OK);
> + }
> + return;
> + }
> +
> /* Remove the trailing clone() action, by directly embedding the nested
> * actions. */
> - if (last_action && nl_attr_type(last_action) == OVS_ACTION_ATTR_CLONE) {
> + if (nl_attr_type(last_action) == OVS_ACTION_ATTR_CLONE) {
> void *dest;
>
> + if (last_observe_offset != UINT32_MAX &&
> + (unsigned char *) actions->data + last_observe_offset >
> + (unsigned char *) last_action) {
> + /* The last sample is inside the trailing clone.
> + * Adjust its offset. */
> + last_observe_offset -= (unsigned char *)
> nl_attr_get(last_action) -
> + (unsigned char *) last_action;
> + ctx->xout->last_observe_offset = last_observe_offset;
> + }
> +
> nl_msg_reset_size(actions,
> (unsigned char *) last_action -
> (unsigned char *) actions->data);
> @@ -8097,6 +8124,16 @@ xlate_optimize_odp_actions(struct xlate_in *xin)
> dest = nl_msg_put_uninit(actions, nl_attr_get_size(last_action));
> memmove(dest, nl_attr_get(last_action),
> nl_attr_get_size(last_action));
> }
> +
> + /* If the last action of the list is an observability action, add an
> + * explicit drop action so that drop statistics remain reliable. */
> + if (ctx->xbridge->ofproto->explicit_sampled_drops &&
> + ovs_explicit_drop_action_supported(ctx->xbridge->ofproto) &&
> + last_observe_offset != UINT32_MAX &&
> + (unsigned char *) last_action == (unsigned char *) actions->data +
> + last_observe_offset) {
> + put_drop_action(actions, XLATE_OK);
> + }
> }
>
> /* Translates the flow, actions, or rule in 'xin' into datapath actions in
> @@ -8113,6 +8150,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
> *xout)
> *xout = (struct xlate_out) {
> .slow = 0,
> .recircs = RECIRC_REFS_EMPTY_INITIALIZER,
> + .last_observe_offset = UINT32_MAX,
> };
>
> struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
> @@ -8541,17 +8579,15 @@ exit:
> xout->slow = 0;
> if (xin->odp_actions) {
> ofpbuf_clear(xin->odp_actions);
> + /* Make the drop explicit if the datapath supports it. */
> + if (ovs_explicit_drop_action_supported(ctx.xbridge->ofproto)) {
> + put_drop_action(xin->odp_actions, ctx.error);
> + }
> }
> } else {
> - /* In the non-error case, see if we can further optimize the datapath
> - * rules by removing redundant (clone) actions. */
> - xlate_optimize_odp_actions(xin);
> - }
> -
> - /* 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);
> + /* In the non-error case, see if we can further optimize or tweak
> + * datapath actions. */
> + xlate_tweak_odp_actions(&ctx);
> }
>
> /* Since congestion drop and forwarding drop are not exactly
> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
> index 08f9397d8..d973a634a 100644
> --- a/ofproto/ofproto-dpif-xlate.h
> +++ b/ofproto/ofproto-dpif-xlate.h
> @@ -61,6 +61,10 @@ struct xlate_out {
>
> /* Recirc action IDs on which references are held. */
> struct recirc_refs recircs;
> +
> + /* Keep track of the last action whose purpose is purely observational.
> + * e.g: IPFIX, sFlow, local sampling. */
> + uint32_t last_observe_offset;
> };
>
> struct xlate_in {
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 21deb7788..9e46f31e2 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1820,6 +1820,7 @@ construct(struct ofproto *ofproto_)
> ofproto->change_seq = 0;
> ofproto->ams_seq = seq_create();
> ofproto->ams_seqno = seq_read(ofproto->ams_seq);
> + ofproto->explicit_sampled_drops = false;
>
>
> SHASH_FOR_EACH_SAFE (node, &init_ofp_ports) {
> @@ -2092,6 +2093,11 @@ run(struct ofproto *ofproto_)
> }
> }
> }
> +
> + if (ofproto->explicit_sampled_drops != ofproto_explicit_sampled_drops) {
> + ofproto->explicit_sampled_drops = ofproto_explicit_sampled_drops;
> + ofproto->backer->need_revalidate = REV_RECONFIGURE;
> + }
> return 0;
> }
>
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index d7b7d5f8c..9bb82e831 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -365,6 +365,8 @@ struct ofproto_dpif {
>
> bool is_controller_connected; /* True if any controller admitted this
> * switch connection. */
> + bool explicit_sampled_drops; /* If explicit drop actions must added
> after
> + * trailing sample actions. */
> };
>
> struct ofproto_dpif *ofproto_dpif_lookup_by_name(const char *name);
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 85991554c..cce90066b 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -550,6 +550,10 @@ extern unsigned ofproto_offloaded_stats_delay;
> * ofproto-dpif implementation. */
> extern uint32_t n_handlers, n_revalidators;
>
> +/* If an explicit datapath drop action shall be added after trailing sample
> + * actions coming from IPFIX / sFlow / local sampling. */
> +extern bool ofproto_explicit_sampled_drops;
> +
> static inline struct rule *rule_from_cls_rule(const struct cls_rule *);
>
> void ofproto_rule_expire(struct rule *rule, uint8_t reason)
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 8c1efe4bf..6dfd94f3e 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -312,6 +312,7 @@ unsigned ofproto_max_idle = OFPROTO_MAX_IDLE_DEFAULT;
> unsigned ofproto_max_revalidator = OFPROTO_MAX_REVALIDATOR_DEFAULT;
> unsigned ofproto_min_revalidate_pps = OFPROTO_MIN_REVALIDATE_PPS_DEFAULT;
> unsigned ofproto_offloaded_stats_delay = OFPROTO_OFFLOADED_STATS_DELAY;
> +bool ofproto_explicit_sampled_drops = OFPROTO_EXPLICIT_SAMPLED_DROPS_DEFAULT;
>
> uint32_t n_handlers, n_revalidators;
>
> @@ -737,6 +738,15 @@ ofproto_set_offloaded_stats_delay(unsigned
> offloaded_stats_delay)
> ofproto_offloaded_stats_delay = offloaded_stats_delay;
> }
>
> +/* Set if an explicit datapath drop action shall be added after trailing
> sample
> + * actions coming from IPFIX / sFlow / local sampling. */
> +void
> +ofproto_set_explicit_sampled_drops(bool explicit_sampled_drops)
> +{
> + ofproto_explicit_sampled_drops = explicit_sampled_drops;
> +}
> +
> +
> /* If forward_bpdu is true, the NORMAL action will forward frames with
> * reserved (e.g. STP) destination Ethernet addresses. if forward_bpdu is
> false,
> * the NORMAL action will drop these frames. */
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index f1ff80e52..fcf8e201d 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -326,6 +326,7 @@ int ofproto_port_dump_done(struct ofproto_port_dump *);
> #define OFPROTO_MAX_REVALIDATOR_DEFAULT 500 /* ms */
> #define OFPROTO_MIN_REVALIDATE_PPS_DEFAULT 5
> #define OFPROTO_OFFLOADED_STATS_DELAY 2000 /* ms */
> +#define OFPROTO_EXPLICIT_SAMPLED_DROPS_DEFAULT false
>
> const char *ofproto_port_open_type(const struct ofproto *,
> const char *port_type);
> @@ -398,6 +399,7 @@ void ofproto_ct_zone_limit_protection_update(const char
> *datapath_type,
> bool protected);
> void ofproto_get_datapath_cap(const char *datapath_type,
> struct smap *dp_cap);
> +void ofproto_set_explicit_sampled_drops(bool explicit_sampled_drops);
>
> /* Configuration of ports. */
> void ofproto_port_unregister(struct ofproto *, ofp_port_t ofp_port);
> diff --git a/tests/drop-stats.at b/tests/drop-stats.at
> index 1d3af98da..19e4d67a1 100644
> --- a/tests/drop-stats.at
> +++ b/tests/drop-stats.at
> @@ -191,3 +191,176 @@ ovs-appctl coverage/read-counter
> drop_action_too_many_mpls_labels
>
> OVS_VSWITCHD_STOP(["/|WARN|/d"])
> AT_CLEANUP
> +
> +AT_SETUP([drop-stats - bridge sampling])
> +
> +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
> +])
Don't put multiple things into one AT_CHECK.
> +
> +AT_CHECK([ovs-vsctl -- set bridge br0 ipfix=@fix -- \
> + --id=@fix create ipfix targets=\"127.0.0.1:4739\" \
> + sampling=1],
> + [0], [ignore])
> +
> +for i in $(seq 1 3); do
> +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])
Maybe deine this packet as a macro with a join?
> +done
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed
> 's/used:[[0-9]].[[0-9]]*s/used:0.0/' | sort], [0], [flow-dump from the main
> thread:
maybe break the line with dnl. Also there is strip_used macro.
> +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:userspace(pid=0,ipfix(output_port=4294967295))
> +])
> +
> +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
> +
> +AT_CHECK([ovs-appctl coverage/read-counter drop_action_of_pipeline], [0],
> [dnl
> +0
> +])
> +
> +dnl Now activate explicit sampled drops.
> +AT_CHECK([ovs-vsctl set Open_vSwitch .
> other_config:explicit-sampled-drops=true])
> +AT_CHECK([ovs-appctl revalidator/purge])
Can we wait instead of purge?
> +
> +for i in $(seq 1 3); do
> +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])
> +done
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed
> 's/used:[[0-9]].[[0-9]]*s/used:0.0/' | sort], [0], [flow-dump from the main
> thread:
> +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:userspace(pid=0,ipfix(output_port=4294967295)),drop
> +])
Same here.
> +
> +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
> +
> +AT_CHECK([ovs-appctl coverage/read-counter drop_action_of_pipeline], [0],
> [dnl
> +3
> +])
> +
> +OVS_VSWITCHD_STOP(["/sending to collector failed/d"])
> +AT_CLEANUP
> +
> +AT_SETUP([drop-stats - sampling action])
Same for this test as well.
> +
> +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 -- \
> + add-port br0 p3 -- set Interface p3 type=dummy ofport_request=3])
add_of_br br0 1 2 3
> +
> +AT_DATA([flows.txt], [dnl
> +table=0,in_port=1,actions=sample(probability=65535,collector_set_id=1)
> +table=0,in_port=2,actions=sample(probability=32767,collector_set_id=1),load:0->reg0
> +table=0,in_port=3,actions=clone(sample(probability=65535,collector_set_id=1))
> +])
> +
> +AT_CHECK([
> + ovs-ofctl del-flows br0
> + ovs-ofctl -Oopenflow13 add-flows br0 flows.txt
> +])
> +
> +AT_CHECK([ovs-vsctl --id=@br0 get Bridge br0 \
> + -- --id=@ipfix create IPFIX targets=\"127.0.0.1:4739\" \
> + -- create Flow_Sample_Collector_Set id=1 bridge=@br0 \
> + ipfix=@ipfix],
> + [0], [ignore])
> +
> +for i in $(seq 1 3); do
> +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])
> +done
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed
> 's/used:[[0-9]].[[0-9]]*s/used:0.0/' | sort], [0], [flow-dump from the main
> thread:
> +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, dnl
> +actions:userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=0,obs_point_id=0,output_port=4294967295))
> +])
> +
> +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
> +
> +AT_CHECK([ovs-appctl coverage/read-counter drop_action_of_pipeline], [0],
> [dnl
> +0
> +])
> +
> +dnl Now activate explicit sampled drops.
> +AT_CHECK([ovs-vsctl set Open_vSwitch .
> other_config:explicit-sampled-drops=true])
> +AT_CHECK([ovs-appctl revalidator/purge])
wait?
> +
> +for i in $(seq 1 3); do
> +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])
> +done
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed
> 's/used:[[0-9]].[[0-9]]*s/used:0.0/' | sort], [0], [flow-dump from the main
> thread:
> +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, dnl
> +actions:userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=0,obs_point_id=0,output_port=4294967295)),drop
> +])
> +
> +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
> +
> +AT_CHECK([ovs-appctl coverage/read-counter drop_action_of_pipeline], [0],
> [dnl
> +3
> +])
> +
> +AT_CHECK([ovs-appctl dpctl/del-flows])
> +
> +for i in $(seq 1 3); do
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p2 \
> +
> 'in_port(2),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])
> +done
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed
> 's/used:[[0-9]].[[0-9]]*s/used:0.0/' | sort], [0], [flow-dump from the main
> thread:
> +recirc_id(0),in_port(2),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:2, bytes:212, used:0.0, dnl
> +actions:sample(sample=50.0%,actions(userspace(pid=0,flow_sample(probability=32767,collector_set_id=1,obs_domain_id=0,obs_point_id=0,output_port=4294967295)))),drop
> +])
> +
> +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
> +
> +AT_CHECK([ovs-appctl coverage/read-counter drop_action_of_pipeline], [0],
> [dnl
> +6
> +])
> +
> +AT_CHECK([ovs-appctl dpctl/del-flows])
> +
> +for i in $(seq 1 3); do
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p3 \
> +
> 'in_port(3),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])
> +done
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed
> 's/used:[[0-9]].[[0-9]]*s/used:0.0/' | sort], [0], [flow-dump from the main
> thread:
> +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:2, bytes:212, used:0.0, dnl
> +actions:userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=0,obs_point_id=0,output_port=4294967295)),drop
> +])
> +
> +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
> +
> +AT_CHECK([ovs-appctl coverage/read-counter drop_action_of_pipeline], [0],
> [dnl
> +9
> +])
Why we need to do the same thing 3 times?
> +
> +OVS_VSWITCHD_STOP(["/sending to collector failed/d"])
> +AT_CLEANUP
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 5778e9fd3..f65ace850 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -12335,3 +12335,52 @@ Datapath actions: EXPECTED_ACT
>
> OVS_VSWITCHD_STOP("/Enabling an unsupported feature is very dangerous/d")
> AT_CLEANUP
> +
> +AT_SETUP([ofproto-dpif - Local sampling - drop])
> +OVS_VSWITCHD_START
> +add_of_ports br0 1 2
> +
> +AT_CHECK([ovs-appctl dpif/set-dp-features --force br0 psample true], [0],
> [ignore])
> +
> +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \
> + -- create Flow_Sample_Collector_Set id=1 bridge=@br0
> local_group_id=42],
> + [0], [ignore])
> +
> +AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=slowpath pktps stats
> bands=type=drop rate=2'])
> +
> +AT_DATA([flows.txt], [dnl
> +in_port=1,
> actions=sample(probability=32767,collector_set_id=1,obs_domain_id=100,obs_point_id=200)
> +in_port=2,
> actions=sample(probability=65535,collector_set_id=1,obs_domain_id=100,obs_point_id=200)
> +])
> +
> +AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt])
> +
> +m4_define([TRACE_PKT], [m4_join([,],
> + [eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800)],
> + [ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no)],
> + [icmp(type=8,code=0)])])
> +
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1) TRACE_PKT'], [0],
> [stdout])
> +AT_CHECK([tail -1 stdout], [0], [dnl
> +Datapath actions:
> sample(sample=50.0%,actions(psample(group=42,cookie=0x64000000c8)))
> +])
> +
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2) TRACE_PKT'], [0],
> [stdout])
> +AT_CHECK([tail -1 stdout], [0], [dnl
> +Datapath actions: psample(group=42,cookie=0x64000000c8)
> +])
> +
> +AT_CHECK([ovs-vsctl set Open_vSwitch .
> other_config:explicit-sampled-drops=true])
> +
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1) TRACE_PKT'], [0],
> [stdout])
> +AT_CHECK([tail -1 stdout], [0], [dnl
> +Datapath actions:
> sample(sample=50.0%,actions(psample(group=42,cookie=0x64000000c8))),drop
> +])
> +
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2) TRACE_PKT'], [0],
> [stdout])
> +AT_CHECK([tail -1 stdout], [0], [dnl
> +Datapath actions: psample(group=42,cookie=0x64000000c8),drop
> +])
> +
> +OVS_VSWITCHD_STOP("/Enabling an unsupported feature is very dangerous/d")
> +AT_CLEANUP
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index c5399d18c..aa1de7d47 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -889,7 +889,9 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
> *ovs_cfg)
> ofproto_set_threads(
> smap_get_int(&ovs_cfg->other_config, "n-handler-threads", 0),
> smap_get_int(&ovs_cfg->other_config, "n-revalidator-threads", 0));
> -
> + ofproto_set_explicit_sampled_drops(
> + smap_get_bool(&ovs_cfg->other_config, "explicit-sampled-drops",
> + OFPROTO_EXPLICIT_SAMPLED_DROPS_DEFAULT));
> /* Destroy "struct bridge"s, "struct port"s, and "struct iface"s
> according
> * to 'ovs_cfg', with only very minimal configuration otherwise.
> *
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 56ac961fc..793d1d755 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -869,6 +869,30 @@
> The feature is considered experimental.
> </p>
> </column>
> +
> + <column name="other_config" key="explicit-sampled-drops"
> + type='{"type": "boolean"}'>
> + <p>
> + When a flow is installed in the datapath with an empty action list,
> + it indicates an implicit "drop" action. Most datapaths report this
Double the space.
> + for event for statistics and monitoring (in datapath-specific ways).
> + </p>
> + <p>
> + However, if any of the per-bridge or per-flow sampling
> functionalities
> + are enabled (e.g: sFlow, IPFIX, local sampling), the action list
> might
> + not be empty, but contain an action to implement such functionality.
> + This makes the datapaths not report the packet drop.
> + </p>
> + <p>
> + This knob makes Open vSwitch detect when the last datapath action
> + comes from these sampling features and add an explicit drop action
> at
> + the end to keep drop statistics accurate.
> + </p>
> + <p>
> + The default value is <code>false</code>.
> + </p>
> + </column>
> +
> </group>
> <group title="Status">
> <column name="next_cfg">
> @@ -904,7 +928,7 @@
> case when statistics gathering is not useful. Set this value to
> <code>true</code> to enable populating the <ref
> column="statistics"/>
> column or to <code>false</code> to explicitly disable it.
> - </column>
> + </column>
>
Seems unrelated.
> <column name="statistics" key="cpu"
> type='{"type": "integer", "minInteger": 1}'>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
