[ovs-dev] [PATCH v3] ipfix: add support for exporting ipfix statistics

[Ben Pfaff]

On Fri, Jun 10, 2016 at 04:08:47PM +0000, Daniel Ye wrote:
> Hi Ben,
> 
> Your suspect is absolutely right, it’s a bug on 32-bit x86. This bug is related to time_t struct, because time_t is a long int type.
> See detail information about the code below:
> ————————————————————
> static void
> dpif_ipfix_exporter_init(struct dpif_ipfix_exporter *exporter)
> {
>     exporter->collectors = NULL;
>     exporter->seq_number = 1;
>     exporter->last_template_set_time = TIME_MIN; ———> For 32-bit x86, TIME_MIN is -2147483648, for 64-bit OS, TIME_MIN is -2^63
>     hmap_init(&exporter->cache_flow_key_map);
>     ovs_list_init(&exporter->cache_flow_start_timestamp_list);
>     exporter->cache_active_timeout = 0;
>     exporter->cache_max_flows = 0;
> }
> … …
>         if (!template_msg_sent
>             && (exporter->last_template_set_time + IPFIX_TEMPLATE_INTERVAL)
>                 <= export_time_sec) { ———> export_time_sec is signed int. Because of type casting, ‘<=‘ will never be true on 32-bit x86 OS.
>             ipfix_send_template_msgs(exporter, export_time_sec,
>                                      entry->flow_key.obs_domain_id);
>             exporter->last_template_set_time = export_time_sec;
>             template_msg_sent = true;
>         }
> ————————————————————
> Because of the analysis above, IPFIX template packet will never be sent on 32-bit x86 OS. That’s why the test case failed. I have addressed this
> issue in patch v5. Do I need a separate patch to address this issue?

Good catch!

It sounds like this fixes an existing bug in released versions of Open
vSwitch.  If so, please break it out as a separate patch so that we can
apply it to older versions as well.

Thanks,

Ben.