RE: [PATCH v5 6/7] test/security: add ESN and anti-replay cases for inline

[Anoob Joseph <anoobj@marvell.com>]

Hi Akhil,

Please see inline.

Thanks,
Anoob

> Subject: [PATCH v5 6/7] test/security: add ESN and anti-replay cases for inline
> 
> Added cases to test anti replay for inline IPsec processing with and without
> extended sequence number support.
> 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> ---
>  app/test/test_security_inline_proto.c | 308
> ++++++++++++++++++++++++++
>  1 file changed, 308 insertions(+)
> 
> diff --git a/app/test/test_security_inline_proto.c
> b/app/test/test_security_inline_proto.c
> index 055b753634..009405f403 100644
> --- a/app/test/test_security_inline_proto.c
> +++ b/app/test/test_security_inline_proto.c
> @@ -1091,6 +1091,136 @@ test_ipsec_inline_proto_all(const struct
> ipsec_test_flags *flags)
>  		return TEST_SKIPPED;
>  }
> 
> +static int
> +test_ipsec_inline_proto_process_with_esn(struct ipsec_test_data td[],
> +		struct ipsec_test_data res_d[],
> +		int nb_pkts,
> +		bool silent,
> +		const struct ipsec_test_flags *flags) {
> +	struct rte_security_session_conf sess_conf = {0};
> +	struct ipsec_test_data *res_d_tmp = NULL;
> +	struct rte_crypto_sym_xform cipher = {0};
> +	struct rte_crypto_sym_xform auth = {0};
> +	struct rte_crypto_sym_xform aead = {0};
> +	struct rte_mbuf *rx_pkt = NULL;
> +	struct rte_mbuf *tx_pkt = NULL;
> +	int nb_rx, nb_sent;
> +	struct rte_security_session *ses;
> +	struct rte_security_ctx *ctx;
> +	uint32_t ol_flags;
> +	int i, ret;
> +
> +	if (td[0].aead) {
> +		sess_conf.crypto_xform = &aead;
> +	} else {
> +		if (td[0].ipsec_xform.direction ==
> +				RTE_SECURITY_IPSEC_SA_DIR_EGRESS) {
> +			sess_conf.crypto_xform = &cipher;
> +			sess_conf.crypto_xform->type =
> RTE_CRYPTO_SYM_XFORM_CIPHER;
> +			sess_conf.crypto_xform->next = &auth;
> +			sess_conf.crypto_xform->next->type =
> RTE_CRYPTO_SYM_XFORM_AUTH;
> +		} else {
> +			sess_conf.crypto_xform = &auth;
> +			sess_conf.crypto_xform->type =
> RTE_CRYPTO_SYM_XFORM_AUTH;
> +			sess_conf.crypto_xform->next = &cipher;
> +			sess_conf.crypto_xform->next->type =
> RTE_CRYPTO_SYM_XFORM_CIPHER;
> +		}
> +	}
> +
> +	/* Create Inline IPsec session. */
> +	ret = create_inline_ipsec_session(&td[0], port_id, &ses, &ctx,
> +					  &ol_flags, flags, &sess_conf);
> +	if (ret)
> +		return ret;
> +
> +	if (td[0].ipsec_xform.direction ==
> RTE_SECURITY_IPSEC_SA_DIR_INGRESS)
> +		create_default_flow(port_id);

[Anoob] If rte_flow creation fails, then the test should be skipped. I see that  create_default_flow() is not returning error in case flow_validate() or flow_create() fails. IMO, it should be fixed.

> +
> +	for (i = 0; i < nb_pkts; i++) {
> +		tx_pkt = init_packet(mbufpool, td[i].input_text.data,
> +					td[i].input_text.len);
> +		if (tx_pkt == NULL) {
> +			ret = TEST_FAILED;
> +			goto out;
> +		}
> +
> +		if
> (test_ipsec_pkt_update(rte_pktmbuf_mtod_offset(tx_pkt,
> +					uint8_t *, RTE_ETHER_HDR_LEN),
> flags)) {
> +			ret = TEST_FAILED;
> +			goto out;
> +		}
> +
> +		if (td[i].ipsec_xform.direction ==
> +				RTE_SECURITY_IPSEC_SA_DIR_EGRESS) {
> +			if (flags->antireplay) {
> +				sess_conf.ipsec.esn.value =
> +						td[i].ipsec_xform.esn.value;
> +				ret = rte_security_session_update(ctx, ses,
> +						&sess_conf);
> +				if (ret) {

[Anoob] ret should be set as TEST_SKIPPED.
 
> +					printf("Could not update ESN in
> session\n");
> +					rte_pktmbuf_free(tx_pkt);
> +					goto out;
> +				}
> +			}
> +			if (ol_flags &
> RTE_SECURITY_TX_OLOAD_NEED_MDATA)
> +				rte_security_set_pkt_metadata(ctx, ses,
> +						tx_pkt, NULL);
> +			tx_pkt->ol_flags |=
> RTE_MBUF_F_TX_SEC_OFFLOAD;
> +		}
> +		/* Send packet to ethdev for inline IPsec processing. */
> +		nb_sent = rte_eth_tx_burst(port_id, 0, &tx_pkt, 1);
> +		if (nb_sent != 1) {
> +			printf("\nUnable to TX packets");
> +			rte_pktmbuf_free(tx_pkt);
> +			ret = TEST_FAILED;
> +			goto out;
> +		}
> +
> +		rte_pause();
> +
> +		/* Receive back packet on loopback interface. */
> +		do {
> +			rte_delay_ms(1);
> +			nb_rx = rte_eth_rx_burst(port_id, 0, &rx_pkt, 1);
> +		} while (nb_rx == 0);
> +
> +		rte_pktmbuf_adj(rx_pkt, RTE_ETHER_HDR_LEN);
> +
> +		if (res_d != NULL)
> +			res_d_tmp = &res_d[i];
> +
> +		ret = test_ipsec_post_process(rx_pkt, &td[i],
> +					      res_d_tmp, silent, flags);
> +		if (ret != TEST_SUCCESS) {
> +			rte_pktmbuf_free(rx_pkt);
> +			goto out;
> +		}
> +
> +		ret = test_ipsec_stats_verify(ctx, ses, flags,
> +					td->ipsec_xform.direction);
> +		if (ret != TEST_SUCCESS) {
> +			rte_pktmbuf_free(rx_pkt);
> +			goto out;
> +		}
> +
> +		rte_pktmbuf_free(rx_pkt);
> +		rx_pkt = NULL;
> +		tx_pkt = NULL;
> +		res_d_tmp = NULL; 

[Anoob] Why do we need to set res_d_tmp to NULL?

> +	}
> +
> +out:
> +	if (td->ipsec_xform.direction ==
> RTE_SECURITY_IPSEC_SA_DIR_INGRESS)
> +		destroy_default_flow(port_id);
> +
> +	/* Destroy session so that other cases can create the session again */
> +	rte_security_session_destroy(ctx, ses);
> +	ses = NULL;
> +
> +	return ret;
> +}
> 

<snip>