Skip to content

Conversation

@hanno-becker
Copy link
Contributor

@hanno-becker hanno-becker commented Dec 7, 2025

Variant of #1348 which aims to be API-compatible so we can merge it prior to v2.


Previously, mlkem_native.h required users to set MLK_CONFIG_API_* macros
before including it, separate from the MLK_CONFIG_* macros used for the
build configuration. This distinction between internal and external
configuration it confusing.

Now, mlkem_native.h derives all API settings from the same
build configuration file used internally. Also, the configuration
file is moved from mlkem/src/config.h to mlkem/mlkem_native_config.h.

  • The legacy configuration via MLK_CONFIG_API_XXX still works for
    backwards compatibility, but will be removed in v2.

  • New Config Options:

    • MLK_CONFIG_MULTILEVEL_BUILD: Explicit flag for multi-level builds

      This makes it simpler to get headers for multi-level builds.

      In the common case, the configs used in a multi-level build are
      identical except for the setting of MLK_CONFIG_PARAMETER_SET and
      MLK_CONFIG_MULTILEVEL_{NO,WITH}_SHARED. However, the latter only
      affect the build, not the API.

      With MLK_CONFIG_MULTILEVEL_BUILD exposed as a separate option,
      mlkem_native.h can use it to determine whether to suffix the
      namespace prefix with the parameter set (512/768/1024) or not,
      so no adaptation of MLK_CONFIG_MULTILEVEL_{NO,WITH}_SHARED is
      needed. Instead, a multi-level header simply becomes:

      #define MLK_CONFIG_PARAMETER_SET 512
      #include "mlkem_native.h"
      #undef MLK_CONFIG_PARAMETER_SET
      #undef MLK_H
      
      #define MLK_CONFIG_PARAMETER_SET 768
      #include "mlkem_native.h"
      #undef MLK_CONFIG_PARAMETER_SET
      #undef MLK_H
      
      #define MLK_CONFIG_PARAMETER_SET 1024
      #include "mlkem_native.h"
      #undef MLK_CONFIG_PARAMETER_SET
      #undef MLK_H

      For backwards compatibility, MLK_CONFIG_MULTILEVEL_BUILD is not
      used in the build, which continues to detect multilevel builds
      via the existing options MLK_CONFIG_MULTILEVEL_{WITH,NO}_SHARED.

    • MLK_CONFIG_NO_SUPERCOP: We had this before in mlkem_native.h as
      MLK_CONFIG_API_NO_SUPERCOP; it's now in the config and documented.

    • MLK_CONFIG_CONSTANTS_ONLY: We had this before in mlkem_native.h as
      MLK_CONFIG_API_CONSTANTS_ONLY; it's now in the config and documented.

  • Build-Internal vs API Config Separation: Config file now guards
    build-only options with #if defined(MLK_BUILD_INTERNAL).

  • Example Directory Restructuring: All examples refactored with mlkem/
    subdirectories renamed to mlkem_native/ and CFLAGS configuration
    replaced by custom configs.
    Updated READMEs to reflect new paths and clarify which options are
    set in config files vs passed via CFLAGS.

@hanno-becker hanno-becker force-pushed the config_cleanup_api_compatible branch 3 times, most recently from ff790df to e44080d Compare December 8, 2025 05:04
@hanno-becker hanno-becker changed the title [TEST] Use a single configuration for internal and external headers Use a single configuration for internal and external headers Dec 8, 2025
@hanno-becker hanno-becker marked this pull request as ready for review December 8, 2025 05:09
@hanno-becker hanno-becker requested a review from a team as a code owner December 8, 2025 05:09
@hanno-becker hanno-becker changed the title Use a single configuration for internal and external headers Use a single configuration file for internal and external headers Dec 8, 2025
@hanno-becker hanno-becker force-pushed the config_cleanup_api_compatible branch from e44080d to a69582c Compare December 8, 2025 05:30
@hanno-becker
Copy link
Contributor Author

@davidchisnall @mkannwischer I'd be glad if you could take a look. The main goal here is to make integration simpler which @davidchisnall reported to be cumbersome in the previous split-config approach.

Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

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

Thanks @hanno-becker. Seems to work like a charm.
I have a couple of comments/questions.

@hanno-becker hanno-becker force-pushed the config_cleanup_api_compatible branch 2 times, most recently from 0128862 to 96cad0c Compare December 9, 2025 04:19
@hanno-becker hanno-becker force-pushed the config_cleanup_api_compatible branch from 96cad0c to 1ea67c4 Compare December 9, 2025 04:39
@hanno-becker hanno-becker force-pushed the config_cleanup_api_compatible branch from 1ea67c4 to 94da62b Compare December 9, 2025 04:50
Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

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

Thanks @hanno-becker. Very nice improvement.

Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Previously, mlkem_native.h required users to set MLK_CONFIG_API_* macros
before including it, separate from the MLK_CONFIG_* macros used for the
build configuration. This distinction between internal and external
configuration it confusing.

Now, mlkem_native.h derives all API settings from the same
build configuration file used internally.

- The legacy configuration via MLK_CONFIG_API_XXX still works for
  backwards compatibility, but will be removed in v2.

- New Config Options:
  * MLK_CONFIG_MULTILEVEL_BUILD: Explicit flag for multi-level builds

    This makes it simpler to get headers for multi-level builds.

    In the common case, the configs used in a multi-level build are
    identical except for the setting of MLK_CONFIG_PARAMETER_SET and
    MLK_CONFIG_MULTILEVEL_{NO,WITH}_SHARED. However, the latter only
    affect the build, not the API.

    With MLK_CONFIG_MULTILEVEL_BUILD exposed as a separate option,
    mlkem_native.h can use it to determine whether to suffix the
    namespace prefix with the parameter set (512/768/1024) or not,
    so no adaptation of MLK_CONFIG_MULTILEVEL_{NO,WITH}_SHARED is
    needed. Instead, a multi-level header simply becomes:

    ```c
    #define MLK_CONFIG_PARAMETER_SET 512
    #include "mlkem_native.h"
    #undef MLK_CONFIG_PARAMETER_SET

    #define MLK_CONFIG_PARAMETER_SET 768
    #include "mlkem_native.h"
    #undef MLK_CONFIG_PARAMETER_SET

    #define MLK_CONFIG_PARAMETER_SET 1024
    #include "mlkem_native.h"
    #undef MLK_CONFIG_PARAMETER_SET
    ```

    For backwards compatibility, MLK_CONFIG_MULTILEVEL_BUILD is not
    used in the build, which continues to detect multilevel builds
    via the existing options MLK_CONFIG_MULTILEVEL_{WITH,NO}_SHARED.

  * MLK_CONFIG_NO_SUPERCOP: We had this before in mlkem_native.h as
    MLK_CONFIG_API_NO_SUPERCOP; it's now in the config and documented.

  * MLK_CONFIG_CONSTANTS_ONLY: We had this before in mlkem_native.h as
    MLK_CONFIG_API_CONSTANTS_ONLY; it's now in the config and documented.

- Build-Internal vs API Config Separation: Config file now guards
  build-only options with #if defined(MLK_BUILD_INTERNAL).

- Example Directory Restructuring: All examples refactored with mlkem/
  subdirectories renamed to mlkem_native/ and CFLAGS configuration
  replaced by custom configs.
  Updated READMEs to reflect new paths and clarify which options are
  set in config files vs passed via CFLAGS.

Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
@hanno-becker hanno-becker force-pushed the config_cleanup_api_compatible branch from 94da62b to f0668d2 Compare December 9, 2025 07:50
@hanno-becker
Copy link
Contributor Author

@davidchisnall This is ready for merge now. Do you have time to give it a try?

@davidchisnall
Copy link

Thanks for this, I can confirm that it works nicely for my use case. #1351 is not necessary after it. Doing the same in mldsa would be amazing.

@hanno-becker hanno-becker merged commit b9bab5b into main Dec 9, 2025
387 checks passed
@hanno-becker hanno-becker deleted the config_cleanup_api_compatible branch December 9, 2025 10:38
@hanno-becker
Copy link
Contributor Author

@davidchisnall Thank you for confirming, yes, we'll do the same for mldsa-native.

Out of interest, which repository are you targeting for your integration? https://github.com/CHERIoT-Platform/cheriot-rtos?

@davidchisnall
Copy link

Once it's a bit cleaner, I'll create a new one for PQC libraries.

@hanno-becker
Copy link
Contributor Author

Doing the same in mldsa would be amazing.

@mkannwischer is doing this here pq-code-package/mldsa-native#782. Should hopefully be ready some point today.

@hanno-becker
Copy link
Contributor Author

@davidchisnall This is merged in mldsa-native as well, now. If you are interested in mldsa-native, I assume that the memory usage is a problem there as well (both the fact that it's still statically stack-allocated, and the mere amount of RAM). Feel free to open an issue so the desire for optimizations in this regard is publicly documented. Focusing on verification first, @mkannwischer has already made significant reductions in pq-code-package/mldsa-native#751; this is still experimental, but knowing that this would be useful for your use case would provide good impetus to continue with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants