opnsense / tools

OPNsense release engineering toolkit
https://opnsense.org/
BSD 2-Clause "Simplified" License
260 stars 187 forks source link

WITH_OFED_EXTRA also disabled #407

Closed rileyg98 closed 2 months ago

rileyg98 commented 2 months ago

Found that WITH_OFED only configures some of the userspace diagnostics - which are very helpful for configuration, but doesn't enable the actual subnet manager (without which an Infiniband network cannot run). As such, WITH_OFED_EXTRA is also required.

I will lodge a PR for this shortly.

fichtner commented 2 months ago

I think you're looking for "opensm" specifically, right?

EDIT: Nevermind, you have it in the PR subject :)

fichtner commented 2 months ago

I get the feeling opensm is misplaced in WITH_OFED_EXTRA seeing it pulls a lot of diagnostics as well

rileyg98 commented 2 months ago

It seems so, but as I understand, ofed was upstreamed from Mellanox (now nvidia) a while back and this is how they had it set up.

What's even more odd is that the opensm service is created with ofed extras disabled, so it may be an error in upstream. I'm happy to go and lodge a bug there, as I did find that someone mentioned there were changes back in 2021 and it may have caused a bug (where someone couldn't get opensm to start).

I've been able to get it to work locally with a custom 24.1 build with the extras enabled, and it doesn't seem to impact anything - except IPoIB connections not being well understood by the GUI, but that's well out of scope of the tools repo and something I may look at fixing myself down the line in core.

Honestly this feels so much more like where a plugin or port would be better, but for some reason it's in the base freeBSD build profile.

On Mon, 6 May 2024, 5:46 pm Franco Fichtner, @.***> wrote:

I get the feeling opensm is misplaced in WITH_OFED_EXTRA seeing it pulls a lot of diagnostics as well

— Reply to this email directly, view it on GitHub https://github.com/opnsense/tools/issues/407#issuecomment-2095379687, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACSLB453CZZ7TZNJ7VGI4TTZA4YNVAVCNFSM6AAAAABHG7PNBKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJVGM3TSNRYG4 . You are receiving this because you authored the thread.Message ID: @.***>

fichtner commented 2 months ago

Thanks for the details.

I have a gut feeling that I'd rather want to change opensm to go with WITH_OFED as you mention that the extras don't build correctly... we could always let upstream fix but eventually it means handrolling their changes as well. It might not even be an upstream issue.

I've been able to get it to work locally with a custom 24.1 build with the extras enabled, and it doesn't seem to impact anything - except IPoIB connections not being well understood by the GUI, but that's well out of scope of the tools repo and something I may look at fixing myself down the line in core.

That would be highly appreciated!

Cheers, Franco

fichtner commented 2 months ago

Previous PR binary result: https://github.com/opnsense/tools/commit/8190c8665a

opensm change: https://github.com/opnsense/src/commit/a1edc286c Binary result: https://github.com/opnsense/tools/commit/de9c374e4

If that state works for you I'd avoid WITH_OFED_EXTRA for now.

Cheers, Franco

rileyg98 commented 2 months ago

I'll test that tonight and have a look!

On Mon, 6 May 2024, 7:13 pm Franco Fichtner, @.***> wrote:

Previous PR binary result: 8190c8665a https://github.com/opnsense/tools/commit/8190c8665a

opensm change: @.*** https://github.com/opnsense/src/commit/a1edc286c Binary result: de9c374e4 https://github.com/opnsense/tools/commit/de9c374e4

If that state works for you I'd avoid WITH_OFED_EXTRA for now.

Cheers, Franco

— Reply to this email directly, view it on GitHub https://github.com/opnsense/tools/issues/407#issuecomment-2095524465, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACSLB46LRYFPGZBN4XNCJCTZA5CUVAVCNFSM6AAAAABHG7PNBKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJVGUZDINBWGU . You are receiving this because you authored the thread.Message ID: @.***>

rileyg98 commented 2 months ago

The opnsense/src change you've made builds opensm just fine as part of base, and is definitely better than WITH_OFED_EXTRA (which allegedly builds "useless examples" from upstream comments, but this OpenSM is the one that's considered usable, at least versus the Windows version)

Thanks, I'll close my PR as unnecessary and close this issue. I'll open an issue in core for the IPoIB stuff once I get core straightened out in my head.

fichtner commented 2 months ago

@rileyg98 splendid, thanks for your help!