Re: [dpdk-dev] [PATCH v7 1/2] ethdev: add packet integrity checks

[Thomas Monjalon <thomas@monjalon.net>]

19/04/2021 10:29, Gregory Etelson:
> +Item: ``PACKET_INTEGRITY_CHECKS``
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Matches packet integrity.
> +For some devices application needs to enable integration checks in HW
> +before using this item.
> +
> +- ``level``: the encapsulation level that should be checked. level 0 means the
> +  default PMD mode (Can be inner most / outermost). value of 1 means outermost
> +  and higher value means inner header. See also RSS level.

Would be nicer to make sub-list for levels.
Please start sentences with a capital letter.

> +- ``packet_ok``: All HW packet integrity checks have passed based on the max
> +  layer of the packet.

"based on the max layer" is not clear. Do you mean all layers?

> +- ``l2_ok``: all layer 2 HW integrity checks passed.
> +- ``l3_ok``: all layer 3 HW integrity checks passed.
> +- ``l4_ok``: all layer 4 HW integrity checks passed.
> +- ``l2_crc_ok``: layer 2 crc check passed.

s/crc/CRC/

> +- ``ipv4_csum_ok``: ipv4 checksum check passed.

s/ipv4/IPv4/

> +- ``l4_csum_ok``: layer 4 checksum check passed.
> +- ``l3_len_ok``: the layer 3 len is smaller than the frame len.

s/len/length/

> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> +* **Added packet integrity match to flow rules.**
> +
> +  * Added ``RTE_FLOW_ITEM_TYPE_INTEGRITY`` flow item.
> +  * Added ``rte_flow_item_integrity`` data structure.

It should be moved with other ethdev changes.

> +
>  * **Added support for Marvell CN10K SoC drivers.**
>  
>    Added Marvell CN10K SoC support. Marvell CN10K SoC are based on Octeon 10

> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> +	/**
> +	 * [META]
> +	 *
> +	 * Matches on packet integrity.
> +	 * For some devices application needs to enable integration checks in HW
> +	 * before using this item.
> +	 *
> +	 * See struct rte_flow_item_integrity.

Better to use @see syntax.

> +	 */
> +	RTE_FLOW_ITEM_TYPE_INTEGRITY,
>  };

> +struct rte_flow_item_integrity {
> +	/**< Tunnel encapsulation level the item should apply to.
> +	 * @see rte_flow_action_rss
> +	 */
> +	uint32_t level;

missing RTE_STD_C11 here for anonymous union.

> +	union {
> +		__extension__
> +		struct {
> +			/**< The packet is valid after passing all HW checks. */
> +			uint64_t packet_ok:1;
> +			/**< L2 layer is valid after passing all HW checks. */
> +			uint64_t l2_ok:1;
> +			/**< L3 layer is valid after passing all HW checks. */
> +			uint64_t l3_ok:1;
> +			/**< L4 layer is valid after passing all HW checks. */
> +			uint64_t l4_ok:1;
> +			/**< L2 layer CRC is valid. */
> +			uint64_t l2_crc_ok:1;
> +			/**< IPv4 layer checksum is valid. */
> +			uint64_t ipv4_csum_ok:1;
> +			/**< L4 layer checksum is valid. */
> +			uint64_t l4_csum_ok:1;
> +			/**< The l3 length is smaller than the frame length. */
> +			uint64_t l3_len_ok:1;
> +			uint64_t reserved:56;
> +		};
> +		uint64_t value;
> +	};
> +};