[dpdk-dev] [PATCH v2 2/6] security: add support for DOCSIS protocol

[Akhil Goyal]

Hi David,

> +/** DOCSIS direction */
> +enum rte_security_docsis_direction {
> +	RTE_SECURITY_DOCSIS_UPLINK,
> +	/**< Uplink
> +	 * - Decryption, followed by CRC Verification
> +	 */
> +	RTE_SECURITY_DOCSIS_DOWNLINK,
> +	/**< Downlink
> +	 * - CRC Generation, followed by Encryption
> +	 */
> +};
> +
> +/**
> + * DOCSIS security session configuration.
> + *
> + * This structure contains data required to create a DOCSIS security session.
> + */
> +struct rte_security_docsis_xform {
> +	enum rte_security_docsis_direction direction;
> +	/** DOCSIS direction */
> +	uint16_t crc_size;
> +	/**< CRC size in bytes */
> +};
> +
>  /**
>   * Security session action type.
>   */
> @@ -325,6 +349,8 @@ enum rte_security_session_protocol {
>  	/**< MACSec Protocol */
>  	RTE_SECURITY_PROTOCOL_PDCP,
>  	/**< PDCP Protocol */
> +	RTE_SECURITY_PROTOCOL_DOCSIS,
> +	/**< DOCSIS Protocol */
>  };
> 
>  /**
> @@ -340,6 +366,7 @@ struct rte_security_session_conf {
>  		struct rte_security_ipsec_xform ipsec;
>  		struct rte_security_macsec_xform macsec;
>  		struct rte_security_pdcp_xform pdcp;
> +		struct rte_security_docsis_xform docsis;
>  	};
>  	/**< Configuration parameters for security session */
>  	struct rte_crypto_sym_xform *crypto_xform;
> @@ -355,6 +382,63 @@ struct rte_security_session {
>  	/**< Opaque user defined data */
>  };
> 
> +/**
> + * DOCSIS operation parameters
> + */
> +struct rte_security_docsis_op {
> +	struct rte_crypto_sym_op crypto_sym;
> +	/**< Symmetric crypto operation parameters */
> +
> +	struct {
> +		uint16_t offset;
> +		/**<
> +		 * Starting point for CRC processing, specified
> +		 * as the number of bytes from start of the packet in
> +		 * the source mbuf in crypto_sym
> +		 */
> +		uint16_t length;
> +		/**<
> +		 * The length, in bytes, of the source mbuf on which the
> +		 * CRC will be computed
> +		 */
> +	} crc;
> +	/**< CRC operation parameters */

As per my understanding, CRC is a kind of authentication. Can we reuse the fields of rte_crypto_sym_op
Auth.data.offset and auth.data.length. This way you can save the unnecessary 4 bytes here. Probably add
Comment in the structure definition that it can be used as offset and length for CRC.

And if you feel that reserved field is needed in near future, then you can add a proper name to it or else
You can do away with the rte_security_docsis_op itself as there will be no other fields in it.

> +
> +	uint64_t reserved;
> +	/**< Reserved for future use */
> +};
> +
> +/**
> + * Security operation types
> + */
> +enum rte_security_op_type {
> +	RTE_SECURITY_OP_TYPE_DOCSIS = 1
> +	/**< DOCSIS operation */
> +};
> +
> +/**
> + * Security operation parameters
> + *
> + * @note If the size of this struct changes, it may be also necessary to update
> + * the RTE_CRYPTO_OP_SECURITY_MAX_SZ define
> + */
> +struct rte_security_op {
> +	enum rte_security_op_type type;
> +	/**< Type of operation */
> +	RTE_STD_C11
> +	union {
> +		struct rte_security_docsis_op docsis;
> +	};
> +	/**< Parameters for security operation */
> +};
> +
> +/* Macro to check the size of a struct at compile time */
> +#define _SECURITY_STRUCT_LEN_CHECK(n, X) enum
> security_static_assert_enum_##X \
> +	{ security_static_assert_##X = (n)/((sizeof(struct X) <= (n)) ? 1 : 0) }
> +
> +/* Check the size of the rte_security_op struct */
> +_SECURITY_STRUCT_LEN_CHECK(RTE_CRYPTO_OP_SECURITY_MAX_SZ,
> rte_security_op);
> +
>  /**
>   * Create security session as specified by the session configuration
>   *
> @@ -496,12 +580,22 @@ static inline int
>  rte_security_attach_session(struct rte_crypto_op *op,
>  			    struct rte_security_session *sess)
>  {
> -	if (unlikely(op->type != RTE_CRYPTO_OP_TYPE_SYMMETRIC))
> -		return -EINVAL;
> +	struct rte_security_op *s_op;
> +	int ret = -EINVAL;
> +
> +	if (likely(op->type == RTE_CRYPTO_OP_TYPE_SYMMETRIC)) {
> +		ret = __rte_security_attach_session(op->sym, sess);
> +	} else if (op->type == RTE_CRYPTO_OP_TYPE_SECURITY) {
> +		s_op = (struct rte_security_op *)&op->security;
> +		if (s_op->type == RTE_SECURITY_OP_TYPE_DOCSIS)
> +			ret = __rte_security_attach_session(
> +						&s_op->docsis.crypto_sym,
> +						sess);
> +	}
> 
>  	op->sess_type =  RTE_CRYPTO_OP_SECURITY_SESSION;
> 
> -	return __rte_security_attach_session(op->sym, sess);
> +	return ret;
>  }
> 
>  struct rte_security_macsec_stats {
> @@ -523,6 +617,10 @@ struct rte_security_pdcp_stats {
>  	uint64_t reserved;
>  };
> 
> +struct rte_security_docsis_stats {
> +	uint64_t reserved;
> +};
> +
>  struct rte_security_stats {
>  	enum rte_security_session_protocol protocol;
>  	/**< Security protocol to be configured */
> @@ -532,6 +630,7 @@ struct rte_security_stats {
>  		struct rte_security_macsec_stats macsec;
>  		struct rte_security_ipsec_stats ipsec;
>  		struct rte_security_pdcp_stats pdcp;
> +		struct rte_security_docsis_stats docsis;
>  	};
>  };
> 
> @@ -591,6 +690,13 @@ struct rte_security_capability {
>  			/**< Capability flags, see RTE_SECURITY_PDCP_* */
>  		} pdcp;
>  		/**< PDCP capability */
> +		struct {
> +			enum rte_security_docsis_direction direction;
> +			/**< DOCSIS direction */
> +			uint16_t crc_size;
> +			/**< CRC size in bytes */
> +		} docsis;
> +		/**< DOCSIS capability */
>  	};
> 
>  	const struct rte_cryptodev_capabilities *crypto_capabilities;
> @@ -649,6 +755,10 @@ struct rte_security_capability_idx {
>  			enum rte_security_pdcp_domain domain;
>  			uint32_t capa_flags;
>  		} pdcp;
> +		struct {
> +			enum rte_security_docsis_direction direction;
> +			uint16_t crc_size;
> +		} docsis;
>  	};
>  };
> 
> --
> 2.17.1