Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CENNSO-1767] feat: pfcp requests metrics #389

Merged
merged 7 commits into from
Feb 21, 2024
Merged

[CENNSO-1767] feat: pfcp requests metrics #389

merged 7 commits into from
Feb 21, 2024

Conversation

demo-exe
Copy link
Collaborator

No description provided.

@demo-exe demo-exe requested a review from mogaika as a code owner February 18, 2024 16:39
@demo-exe demo-exe changed the title [CENNSO-1737] feat: pfcp requests metrics [CENNSO-1767][CENNSO-1768] feat: pfcp requests and session report metrics Feb 19, 2024
@demo-exe demo-exe changed the title [CENNSO-1767][CENNSO-1768] feat: pfcp requests and session report metrics [CENNSO-1767] feat: pfcp requests metrics Feb 19, 2024
upf/upf.h Outdated
// total of PFCP messages received - corrupted
UPF_PFCP_RECEIVED_CORRUPTED = 11,
// PFCP request received
// parts of program depend on order - OK then ERROR for the same type
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe

Suggested change
// parts of program depend on order - OK then ERROR for the same type
// message _ERROR counter always should be after _OK, since arithmetic used to access it

upf/upf.h Outdated
UPF_N_COUNTERS = 11,
// total of PFCP messages received - corrupted
UPF_PFCP_RECEIVED_CORRUPTED = 11,
// PFCP request received
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not only requests, maybe messages?

upf/upf.h Outdated
UPF_PFCP_ASSOCIATION_RELEASE_REQUEST_ERROR = 29,
UPF_PFCP_ASSOCIATION_RELEASE_RESPONSE_OK = 30,
UPF_PFCP_ASSOCIATION_RELEASE_RESPONSE_ERROR = 31,
// PFCP but sent the other way
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// PFCP but sent the other way

upf/upf.h Outdated
@@ -1173,4 +1270,20 @@ u8 *upf_name_to_labels (u8 *name);

__clib_export void upf_nat_get_src_port (vlib_buffer_t *b, u16 port);

always_inline void
upf_increment_counter (upf_counters_type_t counter, u32 index, u64 count)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
upf_increment_counter (upf_counters_type_t counter, u32 index, u64 count)
upf_increment_counter (upf_counters_type_t counter, u32 index, u64 increment)

return -1;
}

if (r != 0) /* if cause != 0 */
{
upf_debug ("PFCP: error response %d", r);
upf_increment_counter (UPF_PFCP_RECEIVED_CORRUPTED, 0, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels like we could use UPF_PFCP_***_ERROR here as well, since type is known and error response is sent

upf/upf.h Show resolved Hide resolved
upf/upf.h Show resolved Hide resolved
@demo-exe demo-exe requested a review from mogaika February 20, 2024 14:54
upf/upf.h Outdated
Comment on lines 526 to 529
UPF_PFCP_RX_HB_REQUEST_OK = 12,
UPF_PFCP_RX_HB_REQUEST_ERROR = 13,
UPF_PFCP_RX_HB_RESPONSE_OK = 14,
UPF_PFCP_RX_HB_RESPONSE_ERROR = 15,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I meant with RX is that each counter may have this prefix to clarify that counter is for received message. So either all, either none

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh thought that HB could be sent 2 ways, my bad
Id go without those prefixes

@demo-exe demo-exe requested a review from mogaika February 20, 2024 15:24
@korroot korroot added the enhancement New feature or request label Feb 21, 2024
@demo-exe demo-exe merged commit 8d16074 into master Feb 21, 2024
9 checks passed
@demo-exe demo-exe deleted the mzyla/1767 branch February 21, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants