You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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?
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.
Activity
thorgrin commentedon Oct 5, 2015
We have introduced
IPFIXCOL_API_VERSIONinapi.hfor 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_messagestructure correctly. Does the code also crash when you set themetadataand 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_memdoes not takemetadataas 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?thorgrin commentedon Oct 8, 2015
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 = Nwhere N is the version number of ipfixcol for which it is created. Just call this afterIPFIXCOL_API_VERSION;which declares the variable.As for the
message_create_from_memfunction, 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_metadatafunction. Thelive_profilemember ofipfix_messageshould work when set to NULL.