RE: [RFC PATCH 1/3] net: add headers for TLS/DTLS packets

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Friday, August 11, 2023 8:17 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>
> Subject: [RFC PATCH 1/3] net: add headers for TLS/DTLS packets
> 
> From: Akhil Goyal <gakhil@marvell.com>
> 
> Added TLS and DTLS packet headers for L4 security applications.
> 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> ---
>  doc/api/doxy-api-index.md |  2 ++
>  lib/net/meson.build       |  2 ++
>  lib/net/rte_dtls.h        | 61 +++++++++++++++++++++++++++++++++++++++
>  lib/net/rte_tls.h         | 48 ++++++++++++++++++++++++++++++
>  4 files changed, 113 insertions(+)
>  create mode 100644 lib/net/rte_dtls.h
>  create mode 100644 lib/net/rte_tls.h
> 
> diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
> index fdeda13932..03e2445bb1 100644
> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -128,6 +128,8 @@ The public API headers are grouped by topics:
>    [eCPRI](@ref rte_ecpri.h),
>    [PDCP hdr](@ref rte_pdcp_hdr.h),
>    [PDCP](@ref rte_pdcp.h),
> +  [TLS](@ref rte_tls.h),
> +  [DTLS](@ref rte_dtls.h),
>    [L2TPv2](@ref rte_l2tpv2.h),
>    [PPP](@ref rte_ppp.h),
>    [IB](@ref rte_ib.h)
> diff --git a/lib/net/meson.build b/lib/net/meson.build
> index b1bc27bad5..0b69138949 100644
> --- a/lib/net/meson.build
> +++ b/lib/net/meson.build
> @@ -5,6 +5,8 @@ headers = files(
>          'rte_ip.h',
>          'rte_tcp.h',
>          'rte_udp.h',
> +        'rte_tls.h',
> +        'rte_dtls.h',
>          'rte_esp.h',
>          'rte_sctp.h',
>          'rte_icmp.h',
> diff --git a/lib/net/rte_dtls.h b/lib/net/rte_dtls.h
> new file mode 100644
> index 0000000000..1455c07a92
> --- /dev/null
> +++ b/lib/net/rte_dtls.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2023 Marvell.
> + */
> +
> +#ifndef RTE_DTLS_H
> +#define RTE_DTLS_H
> +
> +/**
> + * @file
> + *
> + * Datagram transport layer security(DTLS) related defines.
> + */
> +
> +#include <rte_byteorder.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#define RTE_DTLS_TYPE_INVALID	0 /**< Invalid DTLS message type. */
> +#define RTE_DTLS_TYPE_CCS	20 /**< Change cipher message. */

I don't see the "CCS" acronym in the RFC, suggest to make more accurate/verbose;
- Rename to RTE_DTLS_TYPE_CHANGE_CIPHER (or RTE_DTLS_TYPE_CHANGE_CIPHER_SPEC if preferred)
- Reword description to "change cipher" to "change cipher spec message."


> +#define RTE_DTLS_TYPE_ALERT	21 /**< Alert message. */
> +#define RTE_DTLS_TYPE_HANDSHAKE	22 /**< Handshake message for
> DTLS. */
> +#define RTE_DTLS_TYPE_APPDATA	23 /**< DTLS application data message.
> */
> +#define RTE_DTLS_TYPE_HEARTBEAT	24 /**< DTLS 1.3 heartbeat message. */
> +#define RTE_DTLS_TYPE_CIPHERTEXT_WITH_CID	25 /**< DTLS 1.3
> ciphertext with CID message. */
> +#define RTE_DTLS_TYPE_ACK	26 /**< DTLS 1.3 ACK message. */
> +#define RTE_DTLS_TYPE_MAX	255 /**< Maximum value as DTLS
> content type. */
> +
> +#define RTE_DTLS_VERSION_1_2	0xFEFD /**< DTLS 1.2 version. 1's
> complement of 1.2. */
> +#define RTE_DTLS_VERSION_1_3	0xFEFC /**< DTLS 1.3 version. 1's
> complement of 1.3. */
>
> +
> +/**
> + * DTLS Header
> + */
> +__extension__
> +struct rte_dtls_hdr {
> +	/** Content type of DTLS packet. Defined as RTE_DTLS_TYPE_*. */
> +	uint8_t type;
> +	/** DTLS Version defined as RTE_DTLS_VERSION*. */
> +	rte_be16_t version;

(same comment on be16_t vs struct as in TLS version below, no rework needed)

> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +	/** The sequence number for the DTLS record. */
> +	uint64_t sequence_number : 48;
> +	/** A counter value that is incremented on every cipher state change.
> */
> +	uint64_t epoch : 16;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +	/** A counter value that is incremented on every cipher state change.
> */
> +	uint64_t epoch : 16;
> +	/** The sequence number for the DTLS record. */
> +	uint64_t sequence_number : 48;
> +#endif
> +	/** The length (in bytes) of the following DTLS packet. */
> +	rte_be16_t length;
> +} __rte_packed;
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* RTE_DTLS_H */
> diff --git a/lib/net/rte_tls.h b/lib/net/rte_tls.h
> new file mode 100644
> index 0000000000..d708d06014
> --- /dev/null
> +++ b/lib/net/rte_tls.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2023 Marvell.
> + */
> +
> +#ifndef RTE_TLS_H
> +#define RTE_TLS_H
> +
> +/**
> + * @file
> + *
> + * Transport layer security(TLS) related defines.
> + */
> +
> +#include <rte_byteorder.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#define RTE_TLS_TYPE_INVALID	0 /**< Invalid TLS message type. */
> +#define RTE_TLS_TYPE_CCS	20 /**< Change cipher message. */
> +#define RTE_TLS_TYPE_ALERT	21 /**< Alert message. */
> +#define RTE_TLS_TYPE_HANDSHAKE	22 /**< Handshake message for TLS. */
> +#define RTE_TLS_TYPE_APPDATA	23 /**< TLS application data message. */
> +#define RTE_TLS_TYPE_HEARTBEAT	24 /**< TLS 1.3 heartbeat message. */
> +#define RTE_TLS_TYPE_MAX	255 /**< Maximum value as TLS content type.
> */
> +
> +#define RTE_TLS_VERSION_1_2	0x0303 /**< TLS 1.2 version. */
> +#define RTE_TLS_VERSION_1_3	0x0304 /**< TLS 1.3 version. */
> +
> +/**
> + * TLS Header
> + */
> +__extension__
> +struct rte_tls_hdr {
> +	/** Content type of TLS packet. Defined as RTE_TLS_TYPE_*. */
> +	uint8_t type;
> +	/** TLS Version defined as RTE_TLS_VERSION*. */
> +	rte_be16_t version;

In the RFC, version is defined as "struct ProtocolVersion" with two uint8's?
https://www.rfc-editor.org/rfc/rfc5246.html#appendix-A.1
With correct endianness handling this is the same, but the struct with is simpler  to RFC,
while rte_be16_t is easier to ensure single load/store at code level.
No need to change, just pointing out the different impl (but same effect) as RFC.

> +	/** The length (in bytes) of the following TLS packet. */
> +	rte_be16_t length;
> +} __rte_packed;
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* RTE_TLS_H */
> --
> 2.25.1