-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create new liboqs-cupqc-meta repo and delete old liboqs-cupqs team/repo #146
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Pravek Sharma <[email protected]>
Signed-off-by: Pravek Sharma <[email protected]>
Validation succeeded✅ The proposed configuration changes are valid!Configuration changesDirectory
Github
🔸 Please review the changes detected as they will be applied immediately once this PR is merged 🔸 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This data is presently stored in a private repo: https://github.com/praveksharma/cupqc-mlkem. The possibility of storing this data in a Nvidia repo was discussed but this solution is more feasible and straightforward to implement.
Where and when was this discussed? What are the arguments speaking against NVIDIA taking continued responsibility for the adaptation layer to their proprietary HW library? What speaks for people sorely missed to move forward OQS in general (@praveksharma ) to assume (maintenance) responsibilities of a commercial entity?
Net-net: I think merging this PR would do a severe disservice to OQS: It removes NVIDIA technical folks that could help OQS move forward and instead puts all maintenance obligations onto the liboqs-committers team (with no-one from NVIDIA present in).
Reviewing all activities, I have to ask the question whether it was NVIDIA's sole interest to push support for their proprietary API into OQS for marketing reasons alone and when done, withdraw immediately and "let the community" deal with any issues incl. maintenance? @ydoroz, can you comment?
I acknowledge that NVIDIA funds PQCA and wants their money worth of marketing exposure for that -- but for as long as nothing of this money flows into the active support of OQS development I do not think it is warranted to spend community effort and resources on code benefiting primarily the entity selling the hardware.
To make this actionable, what about selecting either of these two alternatives to resolve this:
- NVIDIA declares its willingness to support this proprietary API integration by maintaining it long-term with active participation in the OQS project, i.e., NVIDIA technical people "earn their stripes" to become "oqs-committers" and hence, support the community along with the code thus added. Advantages: More helping hands to move OQS forward & cuPQC is maintained throughout the full software stack.
- OQS removes cuPQC support from
liboqs
: Advantages: No need for this PR & no need for maintenance of this by the OQS core team & much cleaner (and in consequence, more secure) code & more time to resolve problems beleaguering the whole project and not just a single vendor's single algorithm feature.
Pravek previously explained that NVIDIA uses a mono-repo on Github for their open source activity and that doesn't match what we need for an upstream. So either the upstream (a) lives under an individual user's account (as it is now), (b) lives under the OQS org (as this PR proposes), (c) is eliminated because we refactor the code generation code to not need an upstream repo at all (we discussed previously and decided against), or (d) this feature is removed. Of those options, I think the least bad option is the code living under the OQS org (c), as this PR proposes. Colleagues from NVIDIA continue to participate in OQS and PQCA calls, and I have no concerns about maintenance at this point. I'm willing to operate under the assumption that that continues. In terms of this PR, perhaps it would be appropriate for there to continue to be a |
Could you please point to the previous explanation (if on GH) or state the mismatch again? And/or point me to the NVIDIA repo to understand the problem for myself? |
That would be a way to alleviate most of my concerns: This would clearly document who can (and should) help in case of problems with that library and OQS has a simple, pre-documented rationale to drop cuPQC support the moment NVIDIA "re-prioritizes" away (not an unheard-of thing :). |
The conversation is not on GitHub, these are separate converstions I had them them with @dstebila and @stevenireeves offline. I do not have specific repo to point to since that is contingent on Nvidia's internal processes. The problem is that unlike the current metadata repo https://github.com/praveksharma/cupqc-mlkem (and liboqs upstream repos), which has a specific directory structure, the metedata if stored by Nvidia would be a subdirectory among many. copy_from_upstream would need to updated introducing further fragmentation in logic and we would need to wait for as long as it takes Nvidia's to internally approve the repo in the first place. I believe the solution proposed by this PR has the quickest turnaround and does not burden the OQS team any more than Solution 1 that you proposed. The way I see it, when the cuPQC API is updated either someone from the liboqs-cupqc-maintainers team (which I shall create) updates the metadata repo or the integration breaks and cuPQC support is dropped until that is fixed. |
Thanks in advance for creating that @praveksharma ! Beyond the above-mentioned potential update issues created by NVIDIA I'm also (more, actually :) concerned about any challenges (questions, problems, bugs, security vulnerability reports, etc.) put forward to OQS by users of NVIDIA hardware and libcupqc as they see |
We are always eager to assist with any issues related to cuPQC integration and appreciate feedback on bugs, usability, or other problems, as it helps us improve the library. I agree with @dstebila’s recommendation to assign an NVIDIA member as a point of contact for any potential issues. In this regard, we can add Steven as one of the liboqs-cupqc-maintainers to facilitate support. |
Signed-off-by: Pravek Sharma <[email protected]>
Validation succeeded✅ The proposed configuration changes are valid!Configuration changesDirectory
Github
🔸 Please review the changes detected as they will be applied immediately once this PR is merged 🔸 |
You're completely right @baentsch! I think updating cuPQC as necessary (as with the API changes) to ensure the integration continues working is necessary for ongoing support in liboqs.
Thank you for the confirmation @ydoroz! I've created a new liboqs-cupqc-mainters team including you, @stevenireeves, and myself. |
@praveksharma Thanks for leading this. If you need something on our end, please let me know. |
@praveksharma Is this ready to merge? |
Signed-off-by: Pravek Sharma <[email protected]>
Validation succeeded✅ The proposed configuration changes are valid!Configuration changesDirectory
Github
🔸 Please review the changes detected as they will be applied immediately once this PR is merged 🔸 |
Yes, I believe the current permissions should address earlier comments. What do you think @baentsch? |
The integration of cuPQC's ML-KEM into liboqs is now complete. This PR does two things:
Unconditionally, changes to
config.yaml
mustThe following goals apply to changes to the file
config.yaml
with exceptions possible, as long as the rationale for the exception is documented by comments in the file:All the following conditions hold for permissions set in
config.yaml
: