Skip to content

Fix SR-IOV configuration generation and PCI address mapping - v1.3.5#23

Merged
johnlam90 merged 2 commits into
mainfrom
fix/vanilla-non-sriov-non-dpdk
May 28, 2025
Merged

Fix SR-IOV configuration generation and PCI address mapping - v1.3.5#23
johnlam90 merged 2 commits into
mainfrom
fix/vanilla-non-sriov-non-dpdk

Conversation

@johnlam90

@johnlam90 johnlam90 commented May 28, 2025

Copy link
Copy Markdown
Owner

Summary

This PR fixes critical issues in the AWS Multi-ENI Controller's SR-IOV configuration generation and PCI address mapping logic, bumping the version to v1.3.5.

Issues Fixed

Fixes #22 - ENI Manager incorrectly generates SR-IOV config for regular ENI configurations

Issue 1: Incorrect SR-IOV Configuration Generation

Problem: Regular ENI configurations (Case 1) were incorrectly generating SR-IOV device plugin configuration when they should only perform basic ENI attachment.

Solution: Modified the updateSRIOVConfiguration method to only process interfaces where dpdkPCIAddress is explicitly specified.

Issue 2: PCI Address Mapping Mismatch

Problem: Interface-to-NodeENI mapping was prioritizing device index over PCI address, causing incorrect associations:

  • PCI 0000:00:08.0 incorrectly mapped to sriov_kernel_1 (should be sriov_kernel_2)
  • PCI 0000:00:07.0 incorrectly mapped to sriov_kernel_2 (should be sriov_kernel_1)

Solution: Modified findNodeENIForInterface method to prioritize PCI address matching for SR-IOV configurations.

Changes Made

Core Fixes

  • pkg/eni-manager/coordinator/manager.go:
    • Fixed SR-IOV configuration generation logic to check for dpdkPCIAddress presence
    • Prioritized PCI address matching over device index matching for SR-IOV interfaces
    • Added detailed logging for interface mapping decisions

Testing

  • test/integration/regular_eni_test.go:
    • Added TestRegularENINoSRIOVConfiguration to verify Case 1 behavior
    • Added TestPCIAddressMapping to verify correct PCI address mapping
    • Comprehensive test coverage for all three use cases

Version Updates

  • Version bumped to v1.3.5 across all files:

    • cmd/main.go
    • charts/aws-multi-eni-controller/Chart.yaml
    • deploy/deployment.yaml
    • deploy/eni-manager-daemonset.yaml
    • docs/documentation.html
  • Updated CHANGELOG.md with detailed v1.3.5 release notes

Use Cases Verified

Case 1: Regular ENI (Basic Configuration)

  • NodeENI spec: No DPDK fields specified
  • Behavior: Does NOT generate SR-IOV configuration ✓
  • Behavior: Only performs basic ENI attachment ✓

Case 2: Regular SR-IOV (Non-DPDK SR-IOV)

  • NodeENI spec: enableDPDK: false and dpdkPCIAddress specified
  • Behavior: Generates correct SR-IOV configuration ✓
  • Behavior: Maps PCI addresses correctly ✓

Case 3: SR-IOV with DPDK Binding

  • NodeENI spec: enableDPDK: true with DPDK fields
  • Behavior: Handled by DPDK coordinator ✓
  • Behavior: Maps PCI addresses correctly ✓

Testing Results

# Test Results
✓ TestRegularENINoSRIOVConfiguration PASSED
✓ TestPCIAddressMapping PASSED
✓ All existing tests continue to pass
✓ Build successful for both controller and eni-manager

Expected Impact

Before Fix:

  • Regular ENI configurations incorrectly triggered SR-IOV operations
  • PCI address mismatches in SR-IOV configurations
  • Unnecessary resource consumption and potential conflicts

After Fix:

  • Regular ENI configurations work as intended (basic ENI only)
  • SR-IOV configurations correctly map PCI addresses to resource names
  • Cleaner separation between regular ENI and SR-IOV functionality
  • Improved reliability and performance

Verification Steps

  1. Regular ENI Test: Deploy NodeENI without DPDK fields

    • Verify no pcidp configuration generated
    • Verify no SR-IOV pod restarts
  2. SR-IOV Mapping Test: Deploy NodeENI with dpdkPCIAddress

    • Verify correct PCI address to resource name mapping
    • Check /etc/pcidp/config.json for accurate configuration

Breaking Changes

None. This is a bug fix that preserves all existing functionality while correcting incorrect behavior.

Checklist


John Lam added 2 commits May 27, 2025 22:43
- Fix ENI Manager incorrectly generating SR-IOV config for regular ENI configurations
- Implement proper PCI address mapping for SR-IOV configurations
- Prioritize PCI address matching over device index for SR-IOV interfaces
- Add comprehensive test coverage for interface-to-NodeENI mapping
- Resolve PCI address mismatches in SR-IOV device plugin configuration

Fixes #22
Deletes the binary executable file 'eni-manager' from version control.
Binary files should not typically be tracked in Git repositories.
@johnlam90 johnlam90 merged commit ab7d466 into main May 28, 2025
11 of 12 checks passed
@johnlam90 johnlam90 deleted the fix/vanilla-non-sriov-non-dpdk branch May 28, 2025 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix: ENI Manager incorrectly generates SR-IOV config for regular ENI configurations

1 participant