[ovs-dev] [PATCH v1] ipfix: Bug fix for not sending template packets on 32-bit OS

[William Tu]

Acked-by: William Tu <u9012063 at gmail.com>

I think it fixes the issue, although the root cause is that we are

1) Comparing signed int (last_template_set_time) and unsigned int
(export_time_sec). From the C99, the operand with signed integer type
is converted to the type of the operand with unsigned integer type, so
last_template_set_time is cast to unsigned int.
...
&& (exporter->last_template_set_time + IPFIX_TEMPLATE_INTERVAL)
                <= export_time_sec) {

And, after initial send,
2) assigning unsigned int to signed int
      exporter->last_template_set_time = export_time_sec;

I don't have a 32-bit system to test, but explicitly cast to time_t
should also solve the problem.

Regards,
William

On Fri, Jun 10, 2016 at 6:55 PM, Benli Ye <daniely at vmware.com> wrote:
> 'last_template_set_time' in truct dpif_ipfix_exporter is declared
> as time_t and time_t is long int type. If we initialize
> 'last_template_set_time' as TIME_MIN, whose value is -2147483648
> on 32-bit OS and -2^63 on 64-bit OS. There will be a problem on
> 32-bit OS when comparing 'last_template_set_time' with a unisgned int
> type variable, because type casting will happen and negative value
> could be a large positive number. Fix this problem by simply initialize
> 'last_template_set_time' as 0.
> ---
>  ofproto/ofproto-dpif-ipfix.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
> index 79ba234..b1b2237 100644
> --- a/ofproto/ofproto-dpif-ipfix.c
> +++ b/ofproto/ofproto-dpif-ipfix.c
> @@ -495,7 +495,7 @@ dpif_ipfix_exporter_init(struct dpif_ipfix_exporter *exporter)
>  {
>      exporter->collectors = NULL;
>      exporter->seq_number = 1;
> -    exporter->last_template_set_time = TIME_MIN;
> +    exporter->last_template_set_time = 0;
>      hmap_init(&exporter->cache_flow_key_map);
>      ovs_list_init(&exporter->cache_flow_start_timestamp_list);
>      exporter->cache_active_timeout = 0;
> @@ -511,7 +511,7 @@ dpif_ipfix_exporter_clear(struct dpif_ipfix_exporter *exporter)
>      collectors_destroy(exporter->collectors);
>      exporter->collectors = NULL;
>      exporter->seq_number = 1;
> -    exporter->last_template_set_time = TIME_MIN;
> +    exporter->last_template_set_time = 0;
>      exporter->cache_active_timeout = 0;
>      exporter->cache_max_flows = 0;
>  }
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev