[202511] loganalyzer: Remove IPFIX template size estimation, query actual size from SAI instead#4305
Closed
Pterosaur wants to merge 8 commits intosonic-net:202511from
Closed
Conversation
…tead The previous approach estimated the IPFIX template buffer size based on counter subscription counts, but the estimation could be inaccurate and result in SAI_STATUS_BUFFER_OVERFLOW errors with noisy error logs from the SDK layer. Instead of estimating, pass count=0 and list=nullptr to get_tam_tel_type_attribute() to query the required buffer size first, then allocate the exact buffer and fetch the data. This follows the standard SAI pattern for variable-length attribute queries and eliminates the unreliable estimation logic. Signed-off-by: Ze Gan <ganze718@gmail.com>
Add unit tests covering all code paths in updateTemplates() to satisfy the diff-coverage ≥80% requirement: - BufferOverflow_ThenSuccess: SAI returns BUFFER_OVERFLOW on the size query, then SUCCESS on the data fetch (happy path with templates). - Success_EmptyTemplate: SAI returns SUCCESS with count=0 on the first call (no templates available). - FirstCall_UnexpectedFailure: SAI returns an unexpected error on the size query. - SecondCall_Failure: SAI returns BUFFER_OVERFLOW on the size query but fails on the data fetch. - UnknownOID_Throws: caller passes an OID not registered in the profile. Uses the bufferorch_ut.cpp pattern: copy sai_tam_api_t, override get_tam_tel_type_attribute with a mock, restore on tear-down. Signed-off-by: Ze Gan <ganze718@gmail.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Remove unused 'tel_oid' parameter from Stub::init() to fix -Werror=unused-parameter build failure. Signed-off-by: Ze Gan <ganze718@gmail.com>
Pterosaur
force-pushed
the
backport/202511/hft-ipfix-template-size
branch
from
ed33884 to
6fc0c72
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Move #define private/protected public before all #include directives to ensure hftelprofile.h is expanded with public access, even when indirectly included via mock_orchagent_main.h -> hftelorch.h. Signed-off-by: Ze Gan <ganze718@gmail.com>
Pterosaur
force-pushed
the
backport/202511/hft-ipfix-template-size
branch
from
126d5ec to
c2771b2
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The UpdateTemplatesTest fixture assumed sai_tam_api was already initialized by other test fixtures, but execution order is not guaranteed. Initialize a default sai_tam_api_t when NULL. Signed-off-by: Ze Gan <ganze718@gmail.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Use void* intermediate cast to avoid -Wcast-align error when casting aligned unsigned char buffer to HFTelProfile* on ARM. Signed-off-by: Ze Gan <ganze718@gmail.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
|
@Pterosaur please help resolve conflicts |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
|
@Pterosaur please help again |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @Pterosaur closing this PR as a cherry pick got merged yesterday with this PR: #4369, let me know if we need anything else in 202511 for the original PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Backport of #4304 to 202511 branch.
Original PR: #4304
Summary:
Remove the IPFIX template size estimation logic in
HFTelProfile::updateTemplates()and replace it with a two-phase SAI query: first query the required buffer size (count=0, list=nullptr), then allocate and fetch the actual data. This avoids incorrect size estimates that could cause buffer overflow or truncation.Includes mock test coverage for all code paths in
updateTemplates().Conflict resolution
tests/mock_tests/Makefile.am: 202511 branch does not haveportphyattr_ut.cppandcounternameupdater_ut.cpp(present on master). Resolved by adding onlyhftelprofile_ut.cppto the existing file list.Type of change
Approach
See original PR #4304 for full details.
How did you verify/test it?
Mock tests cover all code paths in
updateTemplates():BufferOverflow_ThenSuccess: BUFFER_OVERFLOW → SUCCESS happy pathSuccess_EmptyTemplate: SUCCESS with count=0FirstCall_UnexpectedFailure: unexpected error on size querySecondCall_Failure: data fetch fails after BUFFER_OVERFLOWUnknownOID_Throws: unknown tel-type OIDAny platform specific information?
No.
Documentation
N/A