Skip to content

Fix EFA interface count for instances where MaximumNetworkCards exceeds EFA limit#8692

Open
kprahulraj wants to merge 1 commit intoeksctl-io:mainfrom
kprahulraj:main
Open

Fix EFA interface count for instances where MaximumNetworkCards exceeds EFA limit#8692
kprahulraj wants to merge 1 commit intoeksctl-io:mainfrom
kprahulraj:main

Conversation

@kprahulraj
Copy link
Collaborator

Description

Fixes #8685

We tried to set EFA's by using MaximumNetworkCards. This worked for all previous EFA-capable instances in cases where MaximumEfaInterfaces = MaximumNetworkCards, but starts to break in the latest instance types.

To fix this using NetworkInfo.EfaInfo.MaximumEfaInterfaces to determine the EFA interface count, with fallback to MaximumNetworkCards when EfaInfo is not populated.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

Copy link

@mselim00 mselim00 left a comment

Choose a reason for hiding this comment

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

this PR lgtm in terms of the fix intended for p6-b200, but it's worth pointing out:

most EFA-enabled instance types work with an efa interface on device index 0 for each network card. as far as I know, there's 4 special cases we have to handle, and those are enumerated in https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/efa-acc-inst-types.html

considering each one:

  1. p5[e]?.48xlarge
    Supports 32 network cards, and 32 EFA interfaces. Assigning an efa interface to the 0th index of each card works as we currently do, but is not the recommendation. we should instead assign efa-only interfaces, and the one on network card index 0 should use device index 1

  2. p6-b200.48xlarge
    Supports 8 network cards and 8 EFA interfaces.
    Same recommendation as p5's

  3. p6e-gb200.36xlarge
    Supports 17 network cards but only 16 EFA interfaces.
    Even though there are 16 slots where an EFA interface is supported, the recommendation is to only allocate either 8 or 4 (200 gbps vs 400 gbps / interface bandwith). the example recommendation for 4 allocates device index 0 of network cards indexed 1, 5, 9, 13. These interfaces would need to be efa-only, ENA would not be supported for those

  4. p6-b300.48xlarge
    Supports 17 network cards but only 16 EFA interfaces.
    Similar to p6e-gb200, except in this case the layout does support utilizing each EFA interface without competing for the same bandwidth, it's just that the first network card does not support any. This is the usecase that the PR currently supports.


the takeaway from this is (separate from this PR):

  1. we should find a way to switch to using efa-only instead of efa interface types - this is a breaking change though, as some users do require multiple interfaces available on host networking
  2. we should add in an override strategy for instance types like p6e-gb200.36xlarge, where the optimal allocation is not immediately self-evident from the API response.

},
},
expectedNetworkInterfaces: 17, // 1 ENA (card 0) + 16 EFA (cards 1-16)
expectedInterfaceType: "efa",

Choose a reason for hiding this comment

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

nit: this is misleading

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.

[Bug] EKSCTL can not support p6-b300.48xlarge instance group and throw EFA interface count 17 error

2 participants