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

Remove unused memory allocations associated with bgp #14075

Merged
merged 5 commits into from
Jul 21, 2023

Conversation

donaldsharp
Copy link
Member

See individual Commits, but we were seeing issues at scale with bgp taking up too much memory when you have something like 4k peers. More commits to probably come.

This was only ever being allocated and de-allocated.
Let's save 65k per peer

Signed-off-by: Donald Sharp <[email protected]>
This is never used.  Free up another 65k of stream data
never used per peer.

Signed-off-by: Donald Sharp <[email protected]>
It is never used.

Signed-off-by: Donald Sharp <[email protected]>
The peer->ibuf_scratch was allocating 65535 * 10 bytes
for scratch space to hold data incoming from a read
from a peer.  When you have 4k peers this is 262,1400,000
or 262 mb of data.  Which is crazy large.  Especially
since the i/o pthread is reading per peer without
any chance of having the data interfere with other reads.

Signed-off-by: Donald Sharp <[email protected]>
@github-actions github-actions bot added size/L and removed size/M labels Jul 21, 2023
Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

Amazing!

@ton31337
Copy link
Member

@Mergifyio backport dev/9.0

@mergify
Copy link

mergify bot commented Jul 21, 2023

backport dev/9.0

✅ Backports have been created

@ton31337 ton31337 added this to the 9.0 milestone Jul 21, 2023
@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Jul 21, 2023

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-13077/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-13077/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-13077/artifact/TOPO9U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-13077/artifact/TOPO9U18AMD64/TopotestDetails/

Successful on other platforms/tests
  • Addresssanitizer topotests part 1
  • Static analyzer (clang)
  • Topotests Ubuntu 18.04 i386 part 4
  • Topotests Ubuntu 18.04 amd64 part 1
  • Topotests debian 10 amd64 part 2
  • Topotests debian 10 amd64 part 7
  • Topotests Ubuntu 18.04 arm8 part 0
  • Topotests Ubuntu 18.04 amd64 part 8
  • Debian 9 deb pkg check
  • Addresssanitizer topotests part 8
  • Topotests Ubuntu 18.04 amd64 part 6
  • Topotests Ubuntu 18.04 arm8 part 5
  • Ubuntu 18.04 deb pkg check
  • Ubuntu 20.04 deb pkg check
  • Addresssanitizer topotests part 6
  • Debian 10 deb pkg check
  • Topotests debian 10 amd64 part 1
  • Addresssanitizer topotests part 5
  • Addresssanitizer topotests part 4
  • Addresssanitizer topotests part 0
  • Topotests debian 10 amd64 part 3
  • Topotests debian 10 amd64 part 4
  • Topotests Ubuntu 18.04 i386 part 9
  • Topotests Ubuntu 18.04 arm8 part 3
  • Topotests Ubuntu 18.04 arm8 part 2
  • Topotests debian 10 amd64 part 0
  • Addresssanitizer topotests part 9
  • Topotests Ubuntu 18.04 i386 part 6
  • Topotests debian 10 amd64 part 5
  • Topotests Ubuntu 18.04 arm8 part 7
  • Topotests Ubuntu 18.04 i386 part 7
  • Topotests Ubuntu 18.04 i386 part 2
  • Topotests Ubuntu 18.04 i386 part 1
  • Addresssanitizer topotests part 7
  • Topotests Ubuntu 18.04 arm8 part 8
  • Topotests Ubuntu 18.04 amd64 part 5
  • Topotests debian 10 amd64 part 6
  • Topotests Ubuntu 18.04 arm8 part 1
  • Topotests Ubuntu 18.04 arm8 part 6
  • Addresssanitizer topotests part 3
  • Topotests Ubuntu 18.04 amd64 part 7
  • Topotests Ubuntu 18.04 i386 part 5
  • Topotests Ubuntu 18.04 i386 part 0
  • Topotests Ubuntu 18.04 amd64 part 2
  • Topotests Ubuntu 18.04 i386 part 8
  • Topotests Ubuntu 18.04 i386 part 3
  • Topotests Ubuntu 18.04 amd64 part 4
  • CentOS 7 rpm pkg check
  • Topotests debian 10 amd64 part 8
  • Topotests Ubuntu 18.04 arm8 part 4
  • Addresssanitizer topotests part 2
  • Topotests Ubuntu 18.04 arm8 part 9
  • Topotests Ubuntu 18.04 amd64 part 3
  • Topotests debian 10 amd64 part 9
  • Topotests Ubuntu 18.04 amd64 part 0

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-13078/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

@ton31337 ton31337 merged commit 202c73c into FRRouting:master Jul 21, 2023
ton31337 added a commit that referenced this pull request Jul 22, 2023
Remove unused memory allocations associated with bgp (backport #14075)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants