openssl / technical-policies

Mirror of the repository for technical policies, governed by the OTC (OpenSSL Technical Committee)
20 stars 32 forks source link

Experimental features policy #79

Closed hlandau closed 9 months ago

hlandau commented 10 months ago

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 https://github.com/openssl/project/issues/279

levitte commented 10 months ago

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 commented 10 months ago

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.

levitte commented 10 months ago

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 commented 10 months ago

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

hlandau commented 10 months ago

Some prior art:

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

nhorman commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

Is this proposal in the OTC or OMC balliwick?

arapov commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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.

hlandau commented 9 months ago

+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 https://github.com/openssl/openssl/pull/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 commented 9 months ago

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

hlandau commented 9 months ago

Moved to https://github.com/openssl/general-policies/pull/59

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