Re: [ovs-dev] [PATCH v2] ofproto-dpif-ipfix: add support for per-flow drop counters
Hi Ben,
I did some additional investigation and I finally figured out why I was unable
to see 'sample' action.
The reason was that I was using 100% sampling probability for my tests. In such
a case 'sample' action is not created.
Decreasing sampling probability solved the issue, but only partially.
Per your suggestion, my implementation considered a nested 'output' action
inside 'sample' action as 'true output' only if sampling probability was set to
100%.
It means that 'output' action in 'sample' action was never consider a 'true
output', since 'sample' action is not created when probability was set to 100%.
However, there is one additional scenario when 'sample' action is created even
though probability is set to 100% - when a slow path meter is configured.
I modified my configuration to include a slow path meter and successfully
triggered a 'sample' action with probability lower than 100%.
Therefore, it looks like you were right and this action should be handled. I'll
submit v3 soon.
Regards,
Przemek
> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Szczerbik, PrzemyslawX
> Sent: Friday, July 14, 2017 1:05 PM
> To: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v2] ofproto-dpif-ipfix: add support for per-flow
> drop counters
>
> Hi Ben,
>
> I added code to handle 'clone' action as you suggested.
> I'm still not convinced that it's possible to see 'sample' action during
> IPFIX upcall,
> so I didn't include it in this patch.
> In every test that I run a flow rule with sample action was translated to
> 'userspace' action in datapath.
>
> If you are certain that 'sample' action has to be handled I'll add it in v3.
> However, most likely I'd require some assistance on how to trigger this
> action.
>
> Let me know what you think.
>
> Regards,
> Przemek
>
> > -Original Message-
> > From: Szczerbik, PrzemyslawX
> > Sent: Friday, July 14, 2017 12:42 PM
> > To: d...@openvswitch.org
> > Cc: b...@ovn.org; Szczerbik, PrzemyslawX
> > Subject: [PATCH v2] ofproto-dpif-ipfix: add support for per-flow drop
> > counters
> >
> > Patch based on RFC 5102, section 5.10. It implements per-flow drop counters:
> > - droppedPacketDeltaCount
> > - droppedPacketTotalCount
> > - droppedOctetDeltaCount
> > - droppedOctetTotalCount
> >
> > In order to determine if packet is going to be dropped, flow actions
> > associated
> > with packet are read. If there isn't at least one OVS_ACTION_ATTR_OUTPUT
> > action
> > we assume that packet is going to be dropped. Packet can have direct
> > OVS_ACTION_ATTR_OUTPUT action or one that is nested in
> > OVS_ACTION_ATTR_CLONE
> > action.
> >
> > Signed-off-by: Przemyslaw Szczerbik
> > ---
> > v1->v2
> > * Prevent segfault when dereferencing ipfix_actions in
> > ipfix_cache_entry_init
> > * Handle OVS_ACTION_ATTR_CLONE action
> >
> > ofproto/ofproto-dpif-ipfix.c | 125
> > +++---
> > ofproto/ofproto-dpif-ipfix.h | 16 +-
> > ofproto/ofproto-dpif-upcall.c | 95
> > 3 files changed, 202 insertions(+), 34 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
> > index 13ff426..9887da1 100644
> > --- a/ofproto/ofproto-dpif-ipfix.c
> > +++ b/ofproto/ofproto-dpif-ipfix.c
> > @@ -93,6 +93,8 @@ enum dpif_ipfix_tunnel_type {
> > typedef struct ofputil_ipfix_stats ofproto_ipfix_stats;
> >
> > struct dpif_ipfix_global_stats {
> > +uint64_t dropped_packet_total_count;
> > +uint64_t dropped_octet_total_count;
> > uint64_t packet_total_count;
> > uint64_t octet_total_count;
> > uint64_t octet_total_sum_of_squares;
> > @@ -409,6 +411,8 @@ OVS_PACKED(
> > struct ipfix_data_record_aggregated_common {
> > ovs_be32 flow_start_delta_microseconds; /*
> > FLOW_START_DELTA_MICROSECONDS */
> > ovs_be32 flow_end_delta_microseconds; /*
> > FLOW_END_DELTA_MICROSECONDS */
> > +ovs_be64 dropped_packet_delta_count; /*
> > DROPPED_PACKET_DELTA_COUNT */
> > +ovs_be64 dropped_packet_total_count; /*
> > DROPPED_PACKET_TOTAL_COUNT */
> > ovs_be64 packet_delta_count; /* PACKET_DELTA_COUNT */
> > ovs_be64 packet_total_count; /* PACKET_TOTAL_COUNT */
> > /* INGRESS_UNICAST_PACKET_TOTAL_COUNT */
>
Re: [ovs-dev] [PATCH v2] ofproto-dpif-ipfix: add support for per-flow drop counters
Hi Ben,
I added code to handle 'clone' action as you suggested.
I'm still not convinced that it's possible to see 'sample' action during IPFIX
upcall, so I didn't include it in this patch.
In every test that I run a flow rule with sample action was translated to
'userspace' action in datapath.
If you are certain that 'sample' action has to be handled I'll add it in v3.
However, most likely I'd require some assistance on how to trigger this action.
Let me know what you think.
Regards,
Przemek
> -Original Message-
> From: Szczerbik, PrzemyslawX
> Sent: Friday, July 14, 2017 12:42 PM
> To: d...@openvswitch.org
> Cc: b...@ovn.org; Szczerbik, PrzemyslawX
> Subject: [PATCH v2] ofproto-dpif-ipfix: add support for per-flow drop counters
>
> Patch based on RFC 5102, section 5.10. It implements per-flow drop counters:
> - droppedPacketDeltaCount
> - droppedPacketTotalCount
> - droppedOctetDeltaCount
> - droppedOctetTotalCount
>
> In order to determine if packet is going to be dropped, flow actions
> associated
> with packet are read. If there isn't at least one OVS_ACTION_ATTR_OUTPUT
> action
> we assume that packet is going to be dropped. Packet can have direct
> OVS_ACTION_ATTR_OUTPUT action or one that is nested in
> OVS_ACTION_ATTR_CLONE
> action.
>
> Signed-off-by: Przemyslaw Szczerbik
> ---
> v1->v2
> * Prevent segfault when dereferencing ipfix_actions in ipfix_cache_entry_init
> * Handle OVS_ACTION_ATTR_CLONE action
>
> ofproto/ofproto-dpif-ipfix.c | 125
> +++---
> ofproto/ofproto-dpif-ipfix.h | 16 +-
> ofproto/ofproto-dpif-upcall.c | 95
> 3 files changed, 202 insertions(+), 34 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
> index 13ff426..9887da1 100644
> --- a/ofproto/ofproto-dpif-ipfix.c
> +++ b/ofproto/ofproto-dpif-ipfix.c
> @@ -93,6 +93,8 @@ enum dpif_ipfix_tunnel_type {
> typedef struct ofputil_ipfix_stats ofproto_ipfix_stats;
>
> struct dpif_ipfix_global_stats {
> +uint64_t dropped_packet_total_count;
> +uint64_t dropped_octet_total_count;
> uint64_t packet_total_count;
> uint64_t octet_total_count;
> uint64_t octet_total_sum_of_squares;
> @@ -409,6 +411,8 @@ OVS_PACKED(
> struct ipfix_data_record_aggregated_common {
> ovs_be32 flow_start_delta_microseconds; /*
> FLOW_START_DELTA_MICROSECONDS */
> ovs_be32 flow_end_delta_microseconds; /*
> FLOW_END_DELTA_MICROSECONDS */
> +ovs_be64 dropped_packet_delta_count; /*
> DROPPED_PACKET_DELTA_COUNT */
> +ovs_be64 dropped_packet_total_count; /*
> DROPPED_PACKET_TOTAL_COUNT */
> ovs_be64 packet_delta_count; /* PACKET_DELTA_COUNT */
> ovs_be64 packet_total_count; /* PACKET_TOTAL_COUNT */
> /* INGRESS_UNICAST_PACKET_TOTAL_COUNT */
> @@ -427,11 +431,13 @@ struct ipfix_data_record_aggregated_common {
> ovs_be64 layer2_octet_total_count; /* LAYER2_OCTET_TOTAL_COUNT */
> uint8_t flow_end_reason; /* FLOW_END_REASON */
> });
> -BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_common) ==
> 97);
> +BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_common) ==
> 113);
>
> /* Part of data record for IP aggregated elements. */
> OVS_PACKED(
> struct ipfix_data_record_aggregated_ip {
> +ovs_be64 dropped_octet_delta_count; /* DROPPED_OCTET_DELTA_COUNT
> */
> +ovs_be64 dropped_octet_total_count; /* DROPPED_OCTET_TOTAL_COUNT
> */
> ovs_be64 octet_delta_count; /* OCTET_DELTA_COUNT */
> ovs_be64 octet_total_count; /* OCTET_TOTAL_COUNT */
> ovs_be64 octet_delta_sum_of_squares; /*
> OCTET_DELTA_SUM_OF_SQUARES */
> @@ -441,7 +447,7 @@ struct ipfix_data_record_aggregated_ip {
> ovs_be64 post_mcast_octet_delta_count; /*
> POST_MCAST_OCTET_DELTA_COUNT */
> ovs_be64 post_mcast_octet_total_count; /*
> POST_MCAST_OCTET_TOTAL_COUNT */
> });
> -BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_ip) == 64);
> +BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_ip) == 80);
>
> /* Part of data record for TCP aggregated elements. */
> OVS_PACKED(
> @@ -541,6 +547,8 @@ struct ipfix_flow_cache_entry {
> /* Common aggregated elements. */
> uint64_t flow_start_timestamp_usec;
> uint64_t flow_end_timestamp_usec;
> +uint64_t dropped_packet_delta_count;
> +uint64_t dropped_packet_total_count;
> uint64_t packet_delta_count;
> uint64_t packet_total_count;
> uint64_t in_ucast_packet_total_count;
> @@ -554,6 +562,8 @@ struct ipfix_flow_cache_entry {
> uint64_t post_mcast_octet_delta_count;
> uint64_t layer2_octet_delta_count;
> uint64_t layer2_octet_total_count;
> +uint64_t dropped_octet_delta_count;
> +uint64_t dropped_octet_total_count;
> uint64_t octet_delta_count;
> uint64_t octet_total_count;
> uint64_t octet_delta_sum_of_squares; /* 0 if not IP. */
> @@ -1394,6 +1404,8 @@ ipfix_define_template_fields(enum ipfix_proto
