Skip to content
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

Experimental features policy #79

Closed
wants to merge 3 commits into from

Conversation

hlandau
Copy link
Member

@hlandau hlandau commented Nov 7, 2023

This is a complete strawman which I wrote just now, since my brain started writing it and it was better to get it down than lose it.

It's a complete rough draft. Let me know your thoughts. Also interested in other completely different ways we could do experimental features than specially designated build flags. Ideas welcome.

Notionally, fixes openssl/project#279

@hlandau hlandau added the policy change A change to a policy is being proposed label Nov 7, 2023
@hlandau hlandau self-assigned this Nov 7, 2023
@levitte
Copy link
Member

levitte commented Nov 7, 2023

Generally speaking, I like.

One thing that might need being expanded on: if an experimental feature includes a brand new header file, how should that be treated?

@hlandau
Copy link
Member Author

hlandau commented Nov 7, 2023

Generally speaking, I like.

One thing that might need being expanded on: if an experimental feature includes a brand new header file, how should that be treated?

Perhaps something like #include <experimental/XXX.h>. We already have internal/ etc. to scare people off trying to use our internal symbols. We can avoid copying/installing these headers if an experimental feature is not built. And the entire contents can be guarded by ifdef of course.

the time of a release or at the time of the creation of a release branch.

Nothing relating to an experimental feature shall be considered a CVE or handled
under the security process.
Copy link
Member

Choose a reason for hiding this comment

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

The security policy should be updated in this case to make it clear that issues in experimental features are outside of the threat model

@levitte
Copy link
Member

levitte commented Nov 7, 2023

Generally speaking, I like.
One thing that might need being expanded on: if an experimental feature includes a brand new header file, how should that be treated?

Perhaps something like #include <experimental/XXX.h>. We already have internal/ etc. to scare people off trying to use our internal symbols. We can avoid copying/installing these headers if an experimental feature is not built. And the entire contents can be guarded by ifdef of course.

Yes! I like!

@hlandau
Copy link
Member Author

hlandau commented Nov 7, 2023

I actually see "unstable" as more ominous than "experimental". But the name can be bikeshedded later. Updated in response to feedback.

@hlandau
Copy link
Member Author

hlandau commented Nov 7, 2023

Some prior art:

  • Google is (AIUI) known for a branchless development model where there is only master and everything is handled with feature flags, which demonstrates the viability of this model, even if we have no intention of relying solely on such a mechanism.
  • The Linux kernel has a 'staging' process whereby new drivers, filesystems, etc. can be placed in a "staging" subdirectory until they have matured. This process is sometimes used, sometimes not, not everything goes through this. They are not guaranteed to make it out of staging. It may be useful to examine their policy here.
  • LLVM accepts new target architectures but doesn't necessarily build them by default. It's entirely normal and expected for a new target architecture which is merged to remain disabled by default, but in-tree, for several releases until it is mature. It takes a lot of time and commits for a target to get to a good state so this is a practical necessity. e.g. I believe RISC-V is turned on by default now but for a long time it was not. There are also less used targets have been kept off by default for a long time.

@nhorman Given your kernel experience re: the above, your thoughts would be of interest

@nhorman
Copy link

nhorman commented Nov 7, 2023

the staging process lends itself best in the kernel to 'pluggable' features - i.e. code that compiles to modules which can be included/excluded at run time (i.e. drivers and such). In fact everything in staging is in the drivers directory in the kernel. I only point that out because, while that might be a useful model for us to follow for independently built components like provider libraries, I'm not sure what the balance of that is vs. features that require tighter build time integration to the existing code base (e.g. features that have to be linked into libcrypto/libssl directly at build time).

For features which require tighter integration in the kernel (the canonical example being the rt patchset for real-time kernels), those efforts maintained their "pending inclusion/experimental" status by just constantly rebasing on every released kernel, and did so for I don't know how many years. That works, but has obvious drawbacks in terms of work effort to keep long running experimental code up to date.

The DPDK I believe for some time allowed experimental features to be merged, while making them default-off configurable, but they found that doing that created a large amount of if-deffery that was less than ideal, especially when new features required alterations to common code, or when multiple experimental features started modifying the same common code.

I would think, off the top of my head, for 'uncoupled' code (features which build to their own independent dso's or applications), a staging model would work quite well for us. For that class of work, I would think a promotion from staging to mainline inclusion would be pretty straightforward. For core features of libcrypto/libssl, it may be worth considering some other approach that attempts to both avoid ifdef gymnastics, and attempts to minimize upkeep efforts. What that amounts to, i'm not sure. As you note llvm just merges experimantal arch support and configures them to be default off, which may be sufficient for us, but I think llvm enjoys having a very well defined interface for porting to a new arch, and I'm not sure we fully enjoy that same luxury when adding features to our code base.

If we expect the number of experimental features to be small, the ifdef overhead is probably manageable. If however, we expect it to grow large (to the point where we might expect complex interactions in apis that are common to multiple features), That might require a more complex mechanism. What that amounts too I don't know yet, but I'll think on it some.

@t8m
Copy link
Member

t8m commented Nov 8, 2023

I do not think we should use this widely but there might be features which are not implementable without serious refactoring or things where a big set of new APIs are needed to be added but we are not sure if these APIs are right. It might then make sense to mark these new APIs experimental to be able to change them without the need to deprecate them and wait 5 years. Because unless these APIs get into an official release the input we get on their usability is very limited.

@hlandau
Copy link
Member Author

hlandau commented Nov 8, 2023

I do not think we should use this widely but there might be features which are not implementable without serious refactoring or things where a big set of new APIs are needed to be added but we are not sure if these APIs are right. It might then make sense to mark these new APIs experimental to be able to change them without the need to deprecate them and wait 5 years. Because unless these APIs get into an official release the input we get on their usability is very limited.

Right. The idea is to only use this where the alternatives don't really work. Most features won't need to use this.

@arapov
Copy link
Member

arapov commented Nov 8, 2023

Good stuff. This will allow us to have the code merged early, eliminating the need for rebasing and additional reviews. In a perfect scenario, it would also enable the features to be tested and soaked before being included in releases.

@hlandau
Copy link
Member Author

hlandau commented Nov 8, 2023

Is this proposal in the OTC or OMC balliwick?

@arapov
Copy link
Member

arapov commented Nov 8, 2023

Is this proposal in the OTC or OMC balliwick?

OTC to approve and agree; OMC to accept OTC's proposal and give it a green light in case there are no concerns.

@hlandau
Copy link
Member Author

hlandau commented Nov 8, 2023

There is another standing policy issue that we could consider addressing here: the implementation of cryptographic primitives and protocols that are not yet national or international standards, but which are credibly on track to attain such status. As I have mentioned before, it makes it hard to ship new protocols on time if we only start after everything is finalised, especially given the slow nature of the IETF standards process.

I'm not going to add that to this for now as I expect it to be more contentious. My inclination would be that such things should be doable as experimental features if a) a specification is being developed as part of a national or international standards process, and b) OTC (OMC?) agrees that it would be in the interests of the project/our mission to begin development on it as an experimental feature. So basically we allow the OTC approval part of the above proposal to take precedence over our usual requirement for a ratified standard, so long as something is being developed as part of a standards process.

But there are a lot of different criteria you could come up with for a revised policy in that area and I remain open to suggestions. This also only affects primitives/protocols which are significant enough to need an experimental feature; if something can be done entirely in a feature branch, I don't even see a need for any kind of approval for a not-yet-standard protocol, it just precludes it from merging yet.

@mattcaswell
Copy link
Member

With TLSv1.3 we actually started development long before the standard was published. Although the release didn't ship until afterwards. So there is a precedent here.

@t8m
Copy link
Member

t8m commented Nov 8, 2023

We could even leave it out for OTC and OMC decision whether something is eligible for experimental feature and require both OTC and OMC approvals.

Copy link
Member

@slontis slontis left a comment

Choose a reason for hiding this comment

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

+1 to this.. (I did mention this kind of thing previously in an OTC meeting).
Is there a particular experimental feature you had in mind that you would use this for?

@hlandau
Copy link
Member Author

hlandau commented Nov 13, 2023

+1 to this.. (I did mention this kind of thing previously in an OTC meeting). Is there a particular experimental feature you had in mind that you would use this for?

The poster boy right now for this is (IMO) QLOG openssl/openssl#22037

It's still an I-D and the format is still changing in breaking ways. For example the above PR actually implements 0.3 because qvis doesn't support 0.4 yet. At the same time the point of this is to improve our own productivity and not having it in tree undermines that. So I think it would be ideal to put this in as an experimental feature with the understanding it's going to change its output in incompatible ways until it's an RFC.

(QLOG isn't a cryptographic primitive or protocol so IMO our policy on standards etc. doesn't preclude us from putting it in, any more than it precludes us from logging using printf() — but if we are going to put it in we need a strategy for the fact that its output is going to change in breaking ways. The alternative would be to have if (qlog_version > ...) { path A } else { path B } like constructs throughout our QLOG code and maintain those essentially forever, which is IMO maintainable but perhaps not ideal.)

@hlandau
Copy link
Member Author

hlandau commented Nov 14, 2023

Removing strawman from the title as this seems to be being received positively

@hlandau hlandau changed the title Strawman experimental features policy Experimental features policy Nov 14, 2023
@hlandau
Copy link
Member Author

hlandau commented Nov 21, 2023

Moved to openssl/general-policies#59

Of course feedback is still welcomed from anyone, which should be posted there from now on.

@hlandau hlandau closed this Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
policy change A change to a policy is being proposed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review an experimental feature policy
7 participants