Skip to content

[lte][agw] Eliminate some of the unnecessary state syncs on s1ap#5888

Merged
ulaskozat merged 1 commit intomagma:masterfrom
ulaskozat:s1ap_reduce_write
Apr 5, 2021
Merged

[lte][agw] Eliminate some of the unnecessary state syncs on s1ap#5888
ulaskozat merged 1 commit intomagma:masterfrom
ulaskozat:s1ap_reduce_write

Conversation

@ulaskozat
Copy link
Copy Markdown
Contributor

Summary

Each message handling by default triggers state synchronization service on S1AP task. This PR eliminates such syncs for the subset of S1AP handled messages that does not make any state modifications. This is complementary to the PR #5868.

Test Plan

Integration tests for regression.
Spirent tests for performance. Attached below 3 iterations with 500 UEs, 5 UE/sec attach rate, 500kbps per UE traffic.

Screen Shot 2021-04-01 at 11 16 34 PM

Additional Information

  • This change is backwards-breaking

@ulaskozat ulaskozat added the component: agw Access gateway-related issue label Apr 2, 2021
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Apr 2, 2021
Copy link
Copy Markdown
Contributor

@ardzoht ardzoht left a comment

Choose a reason for hiding this comment

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

minor comment, overall lgtm

state = get_s1ap_state(false);
AssertFatal(state != NULL, "failed to retrieve s1ap state (was null)");

bool is_state_same = false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor nit: I feel has_state_changed is a better name for this, and it should reduce the number of the value being set to true

Copy link
Copy Markdown
Contributor Author

@ulaskozat ulaskozat Apr 2, 2021

Choose a reason for hiding this comment

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

@ardzoht I want to have the default behavior to sync the state and explicitly call out the ones that do not modify the state (at all or most of the time). This will be more robust against new additions to the code base (e.g., s1 mobility).

@ulaskozat ulaskozat merged commit b9cff0f into magma:master Apr 5, 2021
@ulaskozat ulaskozat deleted the s1ap_reduce_write branch April 5, 2021 17:01
chandra-77 pushed a commit to chandra-77/magma that referenced this pull request Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: agw Access gateway-related issue size/M Denotes a PR that changes 30-99 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants