Fill ipfix_message components

[ghost]

We had several intermediate plugins that were running perfectly fine, also when they were chained. Last week, we found out however that adding the ODIP plugin to the plugin chain made IPFIXcol crash. The problem was actually rather simple: in March 2015, some components were added to struct ipfix_message, such as metadata. Our plugins were developed before those components were introduced and completely ignored those components, also when duplicating and editing IPFIX messages. As such, metadata, for example, was not available in IPFIX messages anymore, while the ODIP plugin expects them to be there: SEGFAULT.

To solve or work-around this, I was wondering what would be a good approach. Some ideas:

• Add a check to intermediate_process.c, right after calling intermediate_process_message, and draw an error if the new components are missing.
• Split the functionality of the function message_create_from_mem. Funny enough, this function accepts some (older) ipfix_message components (i.e., input_info and source_status) as arguments, while the new components (e.g., metadata) seem to have been forgotten. Regarding a functional split, we could think about a method that duplicates an IPFIX message without data (i.e., without (option) template sets and data sets) and one that includes data. The split could also be achieved using function arguments, of course.

Any other ideas are welcome.

[thorgrin]

We have introduced IPFIXCOL_API_VERSIONin api.h for this reason. Older plugins that did not have this defined to correct version should not be used. It this check broken?

You need to update the older plugins to use the new ipfix_message structure correctly. Does the code also crash when you set the metadata and other new components to NULL? If it does not, then we can add the first check that you suggest.

The reason the message_create_from_mem does not take metadata as an argument is that the metadata is computed elsewhere (in preprocessor). However, it should at least set it to NULL. We can extend the function to take metadata as an argument and just assign it to the ipfix_message. Would that help? To be more general, which functions does your plugins use and what is the expected behaviour of those functions?

[ghost]

Yes, I believe the API version check is broken. All our plugins have the constant defined in the same way as plugins developed and maintained by CESNET, but the discrepancy was never revealed before.

The code does indeed also crash when the metadata and other new components are set to NULL, for as far as they can be set to NULL.

Although it would be great to have functions like message_create_from_mem to set all the required components, I believe this is only a 'temporary' solution. The most important thing to fix is that this kind of errors is properly detected the next time we make any changes to the data structures.

[thorgrin]

The point of the API version is that it is increased whenever the API changes and is no longer compatible. All plugins must be checked and the API version can be increased to match only after the plugin is adjusted to the new API. However, I agree that this is mainly a check that prevents loading old plugins to newer collector. Recompilation of these plugins increases the API automatically.

You can adapt the API version mechanism so that the version number is not taken from api.h, but rather set manually. Then, when the number increases, you will not be able to run the plugin until it is properly checked. Would that work for you? Basically it would require to set ipfixcol_api_version = N where N is the version number of ipfixcol for which it is created. Just call this after IPFIXCOL_API_VERSION; which declares the variable.

As for the message_create_from_mem function, I've checked its usage and we can probably move the code that fills the metadata to this function.
Another way to make your plugins compatible might be to copy the metadata using message_copy_metadata function. The live_profile member of ipfix_message should work when set to NULL.

[ghost]

Extending the API version check from our plugins would certainly work and I'll update our code soon. Thanks for hinting in that direction.

As for the message components, what about plugin_id and plugin_status?