Skip to content

ipam: Fix concurrent map access to multipool map#44150

Merged
christarazi merged 1 commit intocilium:mainfrom
christarazi:pr/christarazi/fix-44107
Feb 23, 2026
Merged

ipam: Fix concurrent map access to multipool map#44150
christarazi merged 1 commit intocilium:mainfrom
christarazi:pr/christarazi/fix-44107

Conversation

@christarazi
Copy link
Copy Markdown
Member

For some reason, I forgot to acquire the mutex when accessing the
multipool map in 788d1aa ("ipam, metrics: Add new capacity
metric"). Fix it now to prevent the concurrent map access panic as seen
in #44107.

Fixes: 788d1aa ("ipam, metrics: Add new capacity
metric")
Fixes: #44107

Signed-off-by: Chris Tarazi chris@isovalent.com

@christarazi christarazi added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. area/ipam IP address management, including cloud IPAM area/multipool Affects Multi-Pool IPAM needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch needs-backport/1.19 This PR / issue needs backporting to the v1.19 branch affects/v1.16 This issue affects v1.16 branch affects/v1.17 This issue affects v1.17 branch affects/v1.18 This issue affects v1.18 branch and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Feb 3, 2026
@christarazi christarazi force-pushed the pr/christarazi/fix-44107 branch from ef8b5a5 to fc26224 Compare February 3, 2026 14:32
@christarazi christarazi marked this pull request as ready for review February 3, 2026 14:32
@christarazi christarazi requested a review from a team as a code owner February 3, 2026 14:32
@christarazi christarazi requested a review from pippolo84 February 3, 2026 14:32
@christarazi christarazi enabled auto-merge February 3, 2026 14:32
@christarazi
Copy link
Copy Markdown
Member Author

/test

@christarazi christarazi force-pushed the pr/christarazi/fix-44107 branch 2 times, most recently from fc26224 to b7da4d3 Compare February 3, 2026 15:01
@christarazi
Copy link
Copy Markdown
Member Author

/test

Copy link
Copy Markdown
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

Fix LGTM. ✔️

However, after #44030, the multipool manager has been separated from the pod IPAM allocator to reuse the manager core logic for Cilium Network Driver IPAM (see #44081 and #44124).
Also, given that the mutex should be an internal detail of the manager, I propose to move the capacity calculation in the manager like this:

diff --git a/pkg/ipam/multipool.go b/pkg/ipam/multipool.go
index 4328a97bdd..069beeb33e 100644
--- a/pkg/ipam/multipool.go
+++ b/pkg/ipam/multipool.go
@@ -116,24 +116,7 @@ func (c *multiPoolAllocator) Dump() (map[Pool]map[string]string, string) {
 }
 
 func (c *multiPoolAllocator) Capacity() uint64 {
-       c.manager.mutex.Lock()
-       defer c.manager.mutex.Unlock()
-
-       var capacity uint64
-       for _, pool := range c.manager.pools {
-               var p *cidrPool
-               switch c.family {
-               case IPv4:
-                       p = pool.v4
-               case IPv6:
-                       p = pool.v6
-               }
-               if p == nil {
-                       continue
-               }
-               capacity += uint64(p.capacity())
-       }
-       return uint64(capacity)
+       return c.manager.capacity(c.family)
 }
 
 func (c *multiPoolAllocator) RestoreFinished() {
diff --git a/pkg/ipam/multipool_manager.go b/pkg/ipam/multipool_manager.go
index 779a647c5b..ea635e2d35 100644
--- a/pkg/ipam/multipool_manager.go
+++ b/pkg/ipam/multipool_manager.go
@@ -762,3 +762,24 @@ func (m *multiPoolManager) releaseIP(ip net.IP, poolName Pool, family Family, up
        }
        return nil
 }
+
+func (m *multiPoolManager) capacity(family Family) uint64 {
+       m.mutex.Lock()
+       defer m.mutex.Unlock()
+
+       var cap uint64
+       for _, pool := range m.pools {
+               var p *cidrPool
+               switch family {
+               case IPv4:
+                       p = pool.v4
+               case IPv6:
+                       p = pool.v6
+               }
+               if p == nil {
+                       continue
+               }
+               cap += uint64(p.capacity())
+       }
+       return uint64(cap)
+}

This should allow to export multiPoolManager.capacity if needed for the Network driver too. WDYT?

@christarazi christarazi force-pushed the pr/christarazi/fix-44107 branch from b7da4d3 to 1958e36 Compare February 18, 2026 07:08
For some reason, I forgot to acquire the mutex when accessing the
multipool map in 788d1aa ("ipam, metrics: Add new capacity
metric"). Fix it now to prevent the concurrent map access panic as seen
in cilium#44107.

Fixes: 788d1aa ("ipam, metrics: Add new capacity
metric")
Fixes: cilium#44107

Co-authored-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi force-pushed the pr/christarazi/fix-44107 branch from 1958e36 to b7b5143 Compare February 20, 2026 23:36
@christarazi
Copy link
Copy Markdown
Member Author

christarazi commented Feb 20, 2026

/test

Edit: #44434

@christarazi
Copy link
Copy Markdown
Member Author

Thanks @pippolo84, fixed!

Copy link
Copy Markdown
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

Thanks! 💯

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 23, 2026
@christarazi christarazi added this pull request to the merge queue Feb 23, 2026
Merged via the queue into cilium:main with commit 45523ba Feb 23, 2026
79 checks passed
@christarazi christarazi deleted the pr/christarazi/fix-44107 branch February 23, 2026 22:26
@YutaroHayakawa YutaroHayakawa mentioned this pull request Feb 24, 2026
21 tasks
@YutaroHayakawa YutaroHayakawa added backport-pending/1.19 The backport for Cilium 1.19.x for this PR is in progress. and removed needs-backport/1.19 This PR / issue needs backporting to the v1.19 branch labels Feb 24, 2026
@github-actions github-actions bot added backport-done/1.19 The backport for Cilium 1.19.x for this PR is done. and removed backport-pending/1.19 The backport for Cilium 1.19.x for this PR is in progress. labels Mar 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affects/v1.16 This issue affects v1.16 branch affects/v1.17 This issue affects v1.17 branch affects/v1.18 This issue affects v1.18 branch area/ipam IP address management, including cloud IPAM area/multipool Affects Multi-Pool IPAM backport-done/1.19 The backport for Cilium 1.19.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: Conformance Multi Pool IPAM (ci-multi-pool)

4 participants