Skip to content

Commit d9d85e9

Browse files
tomerdbzgalnoam
authored andcommitted
issue: 4664603 fix rx ring fast-path pointer
The m_p_rx_ring pointer serves as a performance optimization for single-ring sockets, avoiding map iteration overhead in the common case. However, the logic to maintain this pointer was duplicated across three locations with inconsistent behavior: 1. rx_add_ring_cb() - only set when map size == 1 2. rx_del_ring_cb() - conditionally nullified when removing 3. do_rings_migration_rx() - only set if previously NULL This inconsistency caused a bug where m_p_rx_ring could point to a stale ring after transitioning from single-ring to multi-ring configurations, leading to incorrect CQ arming and packet loss in multi-homed or bonded scenarios. Introduce update_rx_ring_ptr() helper to centralize and standardize the pointer maintenance logic: - Sets m_p_rx_ring when exactly one ring exists (fast path) - Nullifies m_p_rx_ring otherwise (forces map iteration) Replace all three inconsistent code blocks with calls to this helper, ensuring correct behavior during ring addition, removal, and migration. Signed-off-by: Tomer Cabouly <[email protected]>
1 parent b41b3f5 commit d9d85e9

File tree

2 files changed

+26
-11
lines changed

2 files changed

+26
-11
lines changed

src/core/sock/sockinfo.cpp

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,9 +1170,7 @@ void sockinfo::do_rings_migration_rx(resource_allocation_key &old_key)
11701170
unlock_rx_q();
11711171
m_rx_ring_map_lock.lock();
11721172
lock_rx_q();
1173-
if (!m_p_rx_ring && m_rx_ring_map.size() == 1) {
1174-
m_p_rx_ring = m_rx_ring_map.begin()->first;
1175-
}
1173+
update_rx_ring_ptr();
11761174
unlock_rx_q();
11771175
m_rx_ring_map_lock.unlock();
11781176

@@ -1530,9 +1528,7 @@ void sockinfo::rx_add_ring_cb(ring *p_ring)
15301528
* - rx_del_ring_cb()
15311529
* - do_rings_migration_rx()
15321530
*/
1533-
if (m_rx_ring_map.size() == 1) {
1534-
m_p_rx_ring = m_rx_ring_map.begin()->first;
1535-
}
1531+
update_rx_ring_ptr();
15361532

15371533
notify_epoll = true;
15381534

@@ -1620,11 +1616,7 @@ void sockinfo::rx_del_ring_cb(ring *p_ring)
16201616
delete p_ring_info;
16211617

16221618
if (m_p_rx_ring == base_ring) {
1623-
if (m_rx_ring_map.size() == 1) {
1624-
m_p_rx_ring = m_rx_ring_map.begin()->first;
1625-
} else {
1626-
m_p_rx_ring = nullptr;
1627-
}
1619+
update_rx_ring_ptr();
16281620

16291621
move_descs(base_ring, &temp_rx_reuse, &m_rx_reuse_buff.rx_reuse, true);
16301622
move_descs(base_ring, &temp_rx_reuse_global, &m_rx_reuse_buff.rx_reuse, false);
@@ -1828,6 +1820,15 @@ bool sockinfo::attach_as_uc_receiver_anyip(sa_family_t family, role_t role, bool
18281820
return ret;
18291821
}
18301822

1823+
void sockinfo::update_rx_ring_ptr()
1824+
{
1825+
if (m_rx_ring_map.size() == 1) {
1826+
m_p_rx_ring = m_rx_ring_map.begin()->first;
1827+
} else {
1828+
m_p_rx_ring = nullptr;
1829+
}
1830+
}
1831+
18311832
transport_t sockinfo::find_target_family(role_t role, const struct sockaddr *sock_addr_first,
18321833
const struct sockaddr *sock_addr_second /* = NULL */)
18331834
{

src/core/sock/sockinfo.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,20 @@ class sockinfo {
444444
int fcntl_helper(int __cmd, unsigned long int __arg, bool &bexit);
445445
bool attach_as_uc_receiver_anyip(sa_family_t family, role_t role, bool skip_rules);
446446

447+
/**
448+
* @brief Update the RX ring fast-path pointer based on ring map size.
449+
*
450+
* This method maintains the m_p_rx_ring optimization pointer for single-ring sockets.
451+
* Performance optimization: When a socket has exactly one RX ring (the common case),
452+
* m_p_rx_ring provides direct pointer access, avoiding map iteration overhead.
453+
*
454+
* Must be called after any operation that modifies m_rx_ring_map:
455+
* - Adding rings (rx_add_ring_cb)
456+
* - Removing rings (rx_del_ring_cb)
457+
* - Ring migration (do_rings_migration_rx)
458+
*/
459+
void update_rx_ring_ptr();
460+
447461
protected:
448462
dst_entry *m_p_connected_dst_entry = nullptr;
449463
sockinfo_state m_state = SOCKINFO_OPENED; // socket current state

0 commit comments

Comments
 (0)