Skip to content

[feg] Diameter failed connection recovery (possible fix for issue #7496)#7637

Merged
emakeev merged 1 commit intomagma:masterfrom
emakeev:feg_7496_fix
Jun 19, 2021
Merged

[feg] Diameter failed connection recovery (possible fix for issue #7496)#7637
emakeev merged 1 commit intomagma:masterfrom
emakeev:feg_7496_fix

Conversation

@emakeev
Copy link
Copy Markdown
Contributor

@emakeev emakeev commented Jun 17, 2021 •
edited
Loading

Signed-off-by: Evgeniy Makeev [email protected]

Summary

Diameter failed connection recovery (possible fix for issue #7496)

Test Plan

unit tests; try to reproduce issue #7496

From the latest PR build logs:

session_proxy    | I0618 21:53:56.243525       1 impl.go:51] Mconfig refresh succeeded from: /var/opt/magma/configs/gateway.mconfig
session_proxy    | E0618 21:53:57.407516       1 diameter_client.go:164] diameter connection 172.16.1.13:3907->172.16.1.6:3868 error: diameter error on 172.16.1.6:3868: read tcp 172.16.1.13:3907->172.16.1.6:3868: use of closed network connection
session_proxy    | I0618 21:53:57.408293       1 connection.go:180] destroyed 172.16.1.13:3907->172.16.1.6:3868 connection
session_proxy    | I0618 21:53:57.409293       1 diameter_client.go:179] diameter connection 172.16.1.13:3907->172.16.1.6:3868 is successfully recovered
session_proxy    | I0618 21:54:00.374577       1 connection_manager.go:115] ConnectionManager: enabling connections

We were able to reproduce the issue in the lab & the introduced recovery logic seemed to fix the connection (reconnect) in less than 3 seconds.

Additional Information

  • This change is backwards-breaking

@emakeev emakeev requested review from a team and uri200 June 17, 2021 19:44
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Jun 17, 2021
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 17, 2021

Thanks for opening a PR! 💯 Please note that all commits must be signed off. This is enforced by the DCO check.

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@emakeev emakeev linked an issue Jun 17, 2021 that may be closed by this pull request
[FeGW] TCP diameter Gx/Gy links failure #7496
Closed
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 17, 2021 •
edited
Loading

Codecov Report

Merging #7637 (6b932f5) into master (18e0915) will increase coverage by 27.70%.
The diff coverage is 21.56%.

❗ Current head 6b932f5 differs from pull request most recent head 6574fb4. Consider uploading reports for the commit 6574fb4 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           master    #7637       +/-   ##
===========================================
+ Coverage   34.22%   61.93%   +27.70%     
===========================================
  Files        1175      530      -645     
  Lines      105580    36236    -69344     
  Branches     1323        0     -1323     
===========================================
- Hits        36138    22443    -13695     
+ Misses      65984    10649    -55335     
+ Partials     3458     3144      -314
Flag Coverage Δ
c_cpp ?
cloud_lint 65.72% <ø> (ø)
feg-lint 56.25% <21.56%> (-0.14%) ⬇️
lte-test ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
feg/gateway/diameter/connection_manager.go 56.92% <4.54%> (-26.80%) ⬇️
feg/gateway/diameter/diameter_client.go 61.21% <34.48%> (-4.75%) ⬇️
lte/gateway/c/core/oai/lib/itti/timer.c
...re/oai/lib/pipelined_client/PipelinedClientAPI.cpp
...ore/oai/tasks/nas/ies/ApnAggregateMaximumBitRate.c
...c/core/oai/lib/sms_orc8r_client/SMSOrc8rClient.cpp
lte/gateway/c/session_manager/Monitor.h
...way/c/session_manager/OperationalStatesHandler.cpp
...e/gateway/python/magma/enodebd/tr069/spyne_mods.py
.../core/oai/tasks/nas/emm/msg/DownlinkNasTransport.c
... and 637 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18e0915...6574fb4. Read the comment docs.

@emakeev emakeev force-pushed the feg_7496_fix branch 2 times, most recently from 6b932f5 to 6574fb4 Compare June 18, 2021 17:35
@pull-request-size pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Jun 18, 2021
@emakeev emakeev enabled auto-merge (squash) June 18, 2021 20:41
@emakeev emakeev added the ready2merge This PR is ready to be merged (is approved and passes all required checks) label Jun 18, 2021
uri200
uri200 approved these changes Jun 19, 2021
View reviewed changes
@emakeev emakeev merged commit beac53c into magma:master Jun 19, 2021
@uri200 uri200 added apply-v1.4 Needs to be applied to v1.4 release branch as well apply-v1.5 Apply this commit to the v1.5 release branch as well. apply-v1.6 labels Jun 19, 2021
themarwhal pushed a commit that referenced this pull request Jun 19, 2021
@emakeev @themarwhal
[feg] Diameter failed connection recovery (possible fix for issue #7496…
dddf319 
…) (#7637)

Signed-off-by: Evgeniy Makeev <[email protected]>
themarwhal pushed a commit that referenced this pull request Jun 19, 2021
@emakeev @themarwhal
[feg] Diameter failed connection recovery (possible fix for issue #7496…
e4a2b2f 
…) (#7637)

Signed-off-by: Evgeniy Makeev <[email protected]>
@themarwhal themarwhal added the backported-v1.4 Has been backported to v1.4 release branch label Jun 19, 2021
themarwhal pushed a commit that referenced this pull request Jun 19, 2021
@emakeev @themarwhal
[feg] Diameter failed connection recovery (possible fix for issue #7496…
74f27cb 
…) (#7637)

Signed-off-by: Evgeniy Makeev <[email protected]>
rmeleromira pushed a commit to rmeleromira/magma that referenced this pull request Jul 24, 2021
@emakeev @rmeleromira
[feg] Diameter failed connection recovery (possible fix for issue mag…
7297b8c 
…ma#7496) (magma#7637)

Signed-off-by: Evgeniy Makeev <[email protected]>
Signed-off-by: Ramon Melero <[email protected]>
@emakeev emakeev deleted the feg_7496_fix branch July 28, 2021 19:55
@uri200 uri200 removed apply-v1.4 Needs to be applied to v1.4 release branch as well apply-v1.5 Apply this commit to the v1.5 release branch as well. labels Aug 23, 2021
@uri200 uri200 removed the apply-v1.6 label Aug 23, 2021
m-trojanowski pushed a commit to openEPC/magma that referenced this pull request Oct 20, 2021
@emakeev @themarwhal
[feg] Diameter failed connection recovery (possible fix for issue mag…
37b5fbf 
…ma#7496) (magma#7637)

Signed-off-by: Evgeniy Makeev <[email protected]>
m-trojanowski pushed a commit to openEPC/magma that referenced this pull request Oct 20, 2021
@emakeev @themarwhal
[feg] Diameter failed connection recovery (possible fix for issue mag…
1dca679 
…ma#7496) (magma#7637)

Signed-off-by: Evgeniy Makeev <[email protected]>
@crbertoldo crbertoldo mentioned this pull request Mar 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@uri200 uri200 uri200 approved these changes

Assignees

No one assigned

Labels

backported-v1.4 Has been backported to v1.4 release branch backported-v1.5 backported-v1.6 ready2merge This PR is ready to be merged (is approved and passes all required checks) size/L Denotes a Pull Request that changes 100-499 lines.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FeGW] TCP diameter Gx/Gy links failure

3 participants

@emakeev @uri200 @themarwhal