issue: 4656811 Removing IPoIB support #1146

AlexanderGrissik · 2025-11-09T14:18:26Z

Description

Removing IPoIB data and control pathes.

What

IPoIB is not supported any more due to restrictions in the mlx5 driver.

Why ?

Obsolete features removal

Change type

What kind of change does this PR introduce?

Check list

Code follows the style de facto guidelines of this project
Comments have been inserted in hard to understand places
Documentation has been updated (if necessary)
Test has been added (if possible)

AlexanderGrissik · 2025-11-09T14:20:26Z

Pay attention to changes for:

RX_BUF_SIZE(mtu)
neigh_observer

greptile-apps

Greptile Overview

Greptile Summary

This PR removes IPoIB (IP over InfiniBand) support from libvma, as it's no longer supported by the mlx5 driver. The changes are comprehensive and touch 55 files, removing ~2,821 lines of code.

Key Changes

Removed entire IPoIB data and control paths: Deleted neigh_ib, qp_mgr_ib, net_device_val_ib, IPoIB_addr classes
Deleted IGMP handling: Removed igmp_handler.cpp and igmp_mgr.cpp (only needed for IPoIB multicast)
Simplified packet processing: Removed all transport type checks - code now assumes Ethernet only
Updated device validation: Replaced verify_ipoib_or_eth_qp_creation() with verify_eth_qp_creation()
Removed IPoIB-specific macros: Cleaned up IPOIB_* constants, ipoibhdr struct, GRH_HDR_LEN, etc.
Simplified flow steering: Multicast flow rules now only support Ethernet (removed IB UD flow specs)
Build configuration: Removed IBV_FLOW_SPEC_IB capability checks and IPoIB source files from Makefile

Impact

This is a breaking change that removes support for InfiniBand datagram mode interfaces. Applications using VMA on IPoIB interfaces will no longer work. The change is well-documented in the PR description as an "obsolete features removal" due to mlx5 driver restrictions.

Confidence Score: 5/5

This PR is safe to merge - it's a clean, systematic removal of obsolete IPoIB support
The removal is thorough and well-executed across all layers. All IPoIB-specific code paths have been cleanly eliminated including classes, functions, macros, and build configurations. The code now assumes Ethernet-only transport, which simplifies the codebase. No logic errors or incomplete removals detected.
No files require special attention - the removal is consistent and complete across all affected files

Important Files Changed

File Analysis

Filename	Score	Overview
src/vma/util/vtypes.h	5/5	Removed IPoIB-related macros and constants (IPOIB_HW_ADDR_*, ipoibhdr struct, GRH_HDR_LEN, etc.). Clean removal of unused definitions.
src/vma/proto/neighbour.cpp	5/5	Removed neigh_ib class and neigh_ib_val operator. Changed buffer size from IPOIB_HW_ADDR_LEN to ETH_ALEN. Only Ethernet neighbor handling remains.
src/vma/proto/L2_address.h	5/5	Removed IPoIB_addr class entirely. Only ETH_addr class remains for L2 address handling.
src/vma/dev/net_device_val.cpp	5/5	Removed net_device_val_ib class and all IPoIB device handling. Removed verify_enable_ipoib(), removed ARPHRD_INFINIBAND handling. Only Ethernet device support remains.
src/vma/dev/qp_mgr.cpp	5/5	Removed qp_mgr_ib class and all IB UD QP handling. Removed fictive AH creation in trigger_completion. Only RAW_PACKET QP support remains.
src/vma/dev/cq_mgr.cpp	5/5	Removed transport_type field and GRH_HDR_LEN handling. Removed is_ib_tcp_frame() checks. Hardcoded to ETH_HDR_LEN for prefetch calculations.
src/vma/dev/ring_slave.cpp	5/5	Removed IPoIB transport type handling in rx_process_buffer(). Removed IGMP protocol handling. Removed transport_type switch statement - now assumes Ethernet only.
src/vma/dev/rfs_mc.cpp	5/5	Removed IB multicast flow steering. Changed prepare_flow_spec() from bool to void. Only Ethernet multicast flow rules remain.

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant VMA as VMA Library
    participant NetDev as net_device_val
    participant QP as qp_mgr
    participant CQ as cq_mgr
    participant Ring as ring_slave
    participant RFS as rfs_mc/rfs_uc

    Note over VMA,RFS: IPoIB Support Removal

    App->>VMA: Initialize VMA
    VMA->>NetDev: configure()
    NetDev->>NetDev: verify_eth_qp_creation()
    Note right of NetDev: Previously: verify_ipoib_or_eth_qp_creation()<br/>Now: Only Ethernet validation

    NetDev->>NetDev: create_L2_address()
    Note right of NetDev: Only ETH_addr created<br/>IPoIB_addr class removed

    NetDev->>QP: configure()
    QP->>QP: prepare_ibv_qp()
    Note right of QP: Only IBV_QPT_RAW_PACKET<br/>IBV_QPT_UD removed

    QP->>CQ: init_rx_cq_mgr()
    CQ->>CQ: configure()
    Note right of CQ: Hardcoded ETH_HDR_LEN<br/>GRH_HDR_LEN handling removed

    App->>VMA: Multicast Join
    VMA->>RFS: attach_flow()
    RFS->>RFS: prepare_flow_spec()
    Note right of RFS: Only Ethernet MC flow rules<br/>IB MC flow specs removed

    App->>VMA: Send/Receive Data
    VMA->>Ring: rx_process_buffer()
    Ring->>Ring: Validate Ethernet header
    Note right of Ring: No IPoIB header validation<br/>No IGMP processing
    Ring->>Ring: Process IP packet
    Ring->>App: Deliver data

    Note over VMA,RFS: All IPoIB code paths eliminated

_{55 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

tomerdbz

Amazing work Grissik. some small questions and 2 opportunities for optimizations

tomerdbz · 2025-11-10T09:09:53Z

src/vma/dev/ring_tap.h

 	void inc_cq_moderation_stats(size_t sz_data) { NOT_IN_USE(sz_data); }
-	virtual uint32_t get_underly_qpn() { return -1; }
-        virtual uint32_t get_max_inline_data() { return 0; }
+    virtual uint32_t get_max_inline_data() { return 0; }


tomerdbz · 2025-11-10T09:10:43Z

src/vma/dev/qp_mgr.cpp

 		iphdr* p_buffer_iphdr = (iphdr *)(p_mem_buf_desc->p_buffer + sizeof(*p_buffer_ethhdr));
 		memset(p_buffer_iphdr, 0, sizeof(*p_buffer_iphdr));
 		sge[0].length = sizeof(ethhdr) + sizeof(iphdr);
 		sge[0].addr = (uintptr_t)(p_mem_buf_desc->p_buffer);


do we want to remove comment in line 445?

tomerdbz · 2025-11-10T09:11:22Z

src/vma/dev/ring_slave.cpp

+		ring_logwarn("Rx buffer dropped - Invalid Ethr Type (%#x : %#x)", p_eth_h->h_proto, htons(ETH_P_IP));
 		return false;
 	}



do we want to remove comment on line 622?

tomerdbz · 2025-11-10T09:12:30Z

src/vma/util/sys_vars.h

 #define MCE_MAX_RX_PREFETCH_BYTES			(2044)
 #define MCE_RX_CQ_DRAIN_RATE_DISABLED			(0)
 #define MCE_CQ_DRAIN_INTERVAL_DISABLED			(0)
 #define MCE_CQ_ADAPTIVE_MODERATION_DISABLED		(0)


can't comment on line 641 - but I think MCE_MIN_RX_PREFETCH_BYTES should change to (ETH+IP+UDP)

I don't want to change the value as its constant today for both cases but i remove the comment becuase its incorrect even in IPoIB case

tomerdbz · 2025-11-10T09:13:42Z

src/vma/util/utils.h

-		int val_size, char *filename, char* base_ifname);
-
 #if defined(DEFINED_VERBS_VERSION) && (DEFINED_VERBS_VERSION == 2)
 int validate_raw_qp_privliges();


can't comment on line 324
is create_mgid_from_ipv4_mc_ip still relevant? why?

tomerdbz · 2025-11-10T09:20:12Z

src/vma/proto/neighbour.h

+	friend  class	neigh_eth;
 	L2_address*		m_l2_address;
 };



we don't need the abstraction of neigh_eth_val that inherits from neigh_val anymore
we can remove neigh_val and just hold neigh_eth_val (or alternatively, remove neigh_eth_val and make neigh_val concretely about eth)

tomerdbz · 2025-11-10T09:21:11Z

src/vma/proto/neighbour.h

 	rdma_cm_id*		m_cma_id;
 	sockaddr_in  		m_dst_addr;
 	sockaddr_in  		m_src_addr;
 	enum rdma_port_space 	m_rdma_port_space;


why do we need this now w/o IB? i think the member can be removed

We use rdma for neigh stuff also for ETH

tomerdbz · 2025-11-10T09:22:38Z

src/vma/proto/neighbour.h

+	friend class neigh_eth;
 };

 /* neigh_entry inherits from cache_entry_subject where


can't comment on line 157
do we really need now?
EV_TIMEOUT_EXPIRED, // For IB MC join

Removing IPoIB data and control pathes. IPoIB is not supported any more due to restrictions in the mlx5 driver. Signed-off-by: Alexander Grissik <[email protected]>

greptile-apps · 2025-11-11T08:10:57Z

Greptile Overview

Greptile Summary

This PR comprehensively removes IPoIB (InfiniBand over IP) support from libvma, removing ~2,800 lines of code across 55 files. The change simplifies the codebase by eliminating transport type abstractions and IB-specific data/control paths.

Key Changes:

Removed entire IPoIB class hierarchy (neigh_ib, neigh_ib_val, net_device_val_ib, qp_mgr_ib, wqe_send_ib_handler)
Eliminated transport type enums and conditional branching throughout packet processing paths
Removed IGMP manager and handler used for IPoIB multicast
Simplified device validation to ETH-only (removed pkey, umcast, datagram mode checks)
Cleaned up L2 address handling to only support Ethernet MAC addresses (removed 20-byte IPoIB addresses)
Removed IB-specific verbs functions (priv_ibv_find_pkey_index, priv_ibv_modify_qp_from_err_to_init_ud)
Removed VMA_IPOIB environment variable and related system configuration

Justification: The PR description states IPoIB is no longer supported due to restrictions in the mlx5 driver, making this legacy code removal appropriate.

Issues Found:

One minor syntax error: double semicolon in dst_entry.cpp:394

Confidence Score: 4/5

This PR is safe to merge after fixing the minor syntax error
The changes are well-structured and systematic, removing a complete subsystem cleanly. The only issue found is a trivial syntax error (double semicolon) that needs correction. The removal is comprehensive and consistent across all layers. Score is 4/5 due to the syntax error.
src/vma/proto/dst_entry.cpp requires fixing the double semicolon on line 394

Important Files Changed

File Analysis

Filename	Score	Overview
src/vma/proto/dst_entry.cpp	4/5	Removed IPoIB-specific header configuration and transport type branching logic, simplified to ETH-only path with minor typo (double semicolon)
src/vma/proto/neighbour.cpp	5/5	Removed entire `neigh_ib` class implementation (~600 lines), cleaned up IPoIB-specific neighbor management logic
src/vma/dev/net_device_val.cpp	5/5	Removed `net_device_val_ib` class implementation and IPoIB validation logic, simplified QP verification to ETH-only
src/vma/dev/qp_mgr.cpp	5/5	Removed `qp_mgr_ib` implementation including pkey handling, AH creation, and IB-specific QP transitions
src/vma/dev/cq_mgr.cpp	5/5	Removed transport type branching, simplified to ETH-only header offset calculations and packet processing
src/vma/dev/ring_slave.cpp	5/5	Removed IGMP packet handling, IPoIB header validation, and transport type conditionals in packet processing

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant Ring as ring_slave
    participant CQ as cq_mgr
    participant QP as qp_mgr
    participant Dev as net_device_val_eth
    participant Neigh as neigh_eth
    
    Note over App,Neigh: Before: IPoIB + ETH Support
    App->>Dev: create_ring()
    Dev->>Dev: verify_bond_ipoib_or_eth_qp_creation()
    alt IPoIB Device
        Dev->>QP: create qp_mgr_ib
        QP->>QP: update_pkey_index()
        QP->>QP: modify_qp_to_ready_state()
    else ETH Device
        Dev->>QP: create qp_mgr_eth
    end
    
    App->>Ring: rx_process_buffer()
    Ring->>Ring: validate transport (IB/ETH)
    alt IPoIB
        Ring->>Ring: validate ipoibhdr
        Ring->>CQ: process with GRH_HDR_LEN offset
    else ETH
        Ring->>Ring: validate ethhdr
        Ring->>CQ: process with ETH_HDR_LEN offset
    end
    
    Note over App,Neigh: After: ETH Only Support
    App->>Dev: create_ring()
    Dev->>Dev: verify_bond_eth_qp_creation()
    Dev->>QP: create qp_mgr_eth
    
    App->>Ring: rx_process_buffer()
    Ring->>Ring: validate ethhdr only
    Ring->>CQ: process with ETH_HDR_LEN offset
    
    Note over Dev,Neigh: Removed Components
    Note over Dev: net_device_val_ib class
    Note over QP: qp_mgr_ib class
    Note over Neigh: neigh_ib, neigh_ib_val classes
    Note over Ring: IGMP handler, IPoIB validation

greptile-apps

_{55 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2025-11-11T08:10:57Z

src/vma/proto/dst_entry.cpp

-	if(m_p_net_dev_val)
-		return(m_p_net_dev_val->get_transport_type());
-	return VMA_TRANSPORT_UNKNOWN;
+	return conf_l2_hdr_and_snd_wqe_eth();;


syntax: double semicolon (typo)

Suggested change

return conf_l2_hdr_and_snd_wqe_eth();;

return conf_l2_hdr_and_snd_wqe_eth();

AlexanderGrissik requested a review from tomerdbz November 9, 2025 14:19

greptile-apps bot reviewed Nov 9, 2025

View reviewed changes

tomerdbz requested changes Nov 10, 2025

View reviewed changes

issue: 4656811 Removing IPoIB support

0918e8c

Removing IPoIB data and control pathes. IPoIB is not supported any more due to restrictions in the mlx5 driver. Signed-off-by: Alexander Grissik <[email protected]>

AlexanderGrissik force-pushed the remove_ib_rfs branch from 585c6c6 to 0918e8c Compare November 11, 2025 08:03

greptile-apps bot reviewed Nov 11, 2025

View reviewed changes

	return conf_l2_hdr_and_snd_wqe_eth();;
	return conf_l2_hdr_and_snd_wqe_eth();

issue: 4656811 Removing IPoIB support #1146

Are you sure you want to change the base?

issue: 4656811 Removing IPoIB support #1146

Uh oh!

Conversation

AlexanderGrissik commented Nov 9, 2025

Description

What

Why ?

Change type

Check list

Uh oh!

AlexanderGrissik commented Nov 9, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Key Changes

Impact

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

tomerdbz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Nov 11, 2025

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants