From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v3 1/5] lib/ethdev: support for inline IPsec
	events
Date: Tue, 10 Apr 2018 11:11:43 +0200
Message-ID: <4378194.OM6g8PXIhp@xps>
References: <1519896103-32479-1-git-send-email-anoob.joseph@caviumnetworks.com>
 <1521630685-5179-1-git-send-email-anoob.joseph@caviumnetworks.com>
 <1521630685-5179-2-git-send-email-anoob.joseph@caviumnetworks.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, Akhil Goyal <akhil.goyal@nxp.com>,
 Declan Doherty <declan.doherty@intel.com>,
 Radu Nicolau <radu.nicolau@intel.com>,
 Jerin Jacob <jerin.jacob@caviumnetworks.com>,
 Narayana Prasad <narayanaprasad.athreya@caviumnetworks.com>,
 Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
To: Anoob Joseph <anoob.joseph@caviumnetworks.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com
 [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id 450C31B66B
 for <dev@dpdk.org>; Tue, 10 Apr 2018 11:11:46 +0200 (CEST)
In-Reply-To: <1521630685-5179-2-git-send-email-anoob.joseph@caviumnetworks.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi,

21/03/2018 12:11, Anoob Joseph:
> Adding support for IPsec events in rte_eth_event framework. In inline
> IPsec offload, the per packet protocol defined variables, like ESN,
> would be managed by PMD. In such cases, PMD would need IPsec events
> to notify application about various conditions like, ESN overflow.
> 
> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>

No comment about IPsec handling.

The documentation could try to better link things together, see below:

>  /**
> + * Subtypes for IPsec offload events raised by eth device.
> + */
> +enum rte_eth_event_ipsec_subtype {
> +	RTE_ETH_EVENT_IPSEC_UNKNOWN = 0,
> +			/** Unknown event type */
> +	RTE_ETH_EVENT_IPSEC_ESN_OVERFLOW,
> +			/** Sequence number overflow in security offload */
> +	RTE_ETH_EVENT_IPSEC_SA_TIME_EXPIRY,
> +			/** Soft time expiry of SA */
> +	RTE_ETH_EVENT_IPSEC_SA_BYTE_EXPIRY,
> +			/** Soft byte expiry of SA */
> +	RTE_ETH_EVENT_IPSEC_MAX
> +			/** Max value of this enum */
> +};
> +
> +/**
> + * Descriptor for IPsec event. Used by eth dev to send extra information of the
> + * event.
> + */

You could link it to the event type RTE_ETH_EVENT_IPSEC in this doxygen comment.

> +struct rte_eth_event_ipsec_desc {
> +	enum rte_eth_event_ipsec_subtype stype;

stype is not easy to read & understand. What about subtype?

> +			/** Type of IPsec event */

You could add the prefix of the events in this comment: RTE_ETH_EVENT_IPSEC_*

> +	uint64_t md;

What about metadata?

> +			/** Event specific metadata */

Could you describe what is the metadata, depending on each sub-type?

> +};
> +
> +/**
>   * The eth device event type for interrupt, and maybe others in the future.
>   */
>  enum rte_eth_event_type {
> @@ -2448,6 +2475,7 @@ enum rte_eth_event_type {
>  	RTE_ETH_EVENT_INTR_RESET,
>  			/**< reset interrupt event, sent to VF on PF reset */
>  	RTE_ETH_EVENT_VF_MBOX,  /**< message from the VF received by PF */
> +	RTE_ETH_EVENT_IPSEC,    /**< IPsec offload related event */
>  	RTE_ETH_EVENT_MACSEC,   /**< MACsec offload related event */
>  	RTE_ETH_EVENT_INTR_RMV, /**< device removal event */
>  	RTE_ETH_EVENT_NEW,      /**< port is probed */