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.

Sign up for GitHub

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

Jump to bottom

Keepalive logic in zmq-cpiface #93

Merged
merged 10 commits into from
Jul 12, 2020
Merged

Keepalive logic in zmq-cpiface #93

merged 10 commits into from
Jul 12, 2020

Conversation

ajamshed
Copy link
Contributor

Cleaned up the code.
Also, made sure that cpiface self-restarts for non-k8s setup as well.

@ajamshed ajamshed mentioned this pull request Jul 11, 2020
Merged
@ajamshed ajamshed requested a review from krsna1729 July 11, 2020 07:09
@ajamshed ajamshed changed the title Thakurajay l master Keepalive logic in zmq-cpiface Jul 11, 2020
ajamshed
ajamshed commented Jul 11, 2020
View reviewed changes
@ajamshed
Copy link
Contributor Author

ajamshed commented Jul 11, 2020 •
edited
Loading

This PR is not ready yet. Sessions need to be deleted before restart.
Done.

@ajamshed ajamshed added the wip label Jul 11, 2020
@krsna1729 krsna1729 marked this pull request as draft July 11, 2020 18:25
@krsna1729 krsna1729 removed the wip label Jul 11, 2020
@ajamshed ajamshed added wip and removed wip labels Jul 11, 2020
@ajamshed ajamshed marked this pull request as ready for review July 11, 2020 20:29
@ajamshed ajamshed force-pushed the thakurajayL-master branch from c3a4da0 to 7995ab7 Compare July 11, 2020 20:34
thakurajayL
thakurajayL previously approved these changes Jul 12, 2020
View reviewed changes
krsna1729
krsna1729 reviewed Jul 12, 2020
View reviewed changes
@ajamshed ajamshed force-pushed the thakurajayL-master branch from d8ba216 to 4154220 Compare July 12, 2020 05:56
krsna1729
krsna1729 approved these changes Jul 12, 2020
View reviewed changes
Copy link
Member

@krsna1729 krsna1729 left a comment

Choose a reason for hiding this comment

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

Hopefully you are still able to rebase up4 branch once this merges.

@krsna1729
Copy link
Member

@ajamshed check the jenkins failure

@ajamshed
Copy link
Contributor Author

Retest this please.

@thakurajayL
Copy link
Contributor

Should we have similar code i case pfcp endpoint at (upf) detects Central Plane down ?

@ajamshed
Copy link
Contributor Author

Should we have similar code i case pfcp endpoint at (upf) detects Central Plane down ?

We should first rebase. Then we will need to add keepalive/heartbeat logic to both zmq-cpiface.cc as well as pfcpiface.go.

@thakurajayL
Copy link
Contributor

Should we have similar code i case pfcp endpoint at (upf) detects Central Plane down ?

We should first rebase. Then we will need to add keepalive/heartbeat logic to both zmq-cpiface.cc as well as pfcpiface.go.

I am not sure. Let's forget about other things. This fix is proposed for master branch as of now. Do i have to do any action ? I dont see Git asking me to do rebase.

@ajamshed
Copy link
Contributor Author

Should we have similar code i case pfcp endpoint at (upf) detects Central Plane down ?

We should first rebase. Then we will need to add keepalive/heartbeat logic to both zmq-cpiface.cc as well as pfcpiface.go.

I am not sure. Let's forget about other things. This fix is proposed for master branch as of now. Do i have to do any action ? I dont see Git asking me to do rebase.

No other action needed. The PR should be merged to the master once the Jenkins test passes. I have already sent a message to @yoooou for help. Merging similar changes to the topics/up4-bess branch will require additional work since we need to clear more table entries (PDRLookup, FARLookup, Counters) before the upf restarts.

@yoooou
Copy link

yoooou commented Jul 12, 2020

retest this please

1 similar comment
@yoooou
Copy link

yoooou commented Jul 12, 2020

retest this please

@ajamshed ajamshed merged commit af1023d into master Jul 12, 2020
@ajamshed ajamshed deleted the thakurajayL-master branch July 12, 2020 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krsna1729 krsna1729 krsna1729 approved these changes

@thakurajayL thakurajayL thakurajayL left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ajamshed @krsna1729 @thakurajayL @yoooou