Hi,
The configuration of this feature is not clear to me.
Please see the comments below.
09/10/2020 15:13, nipun.gu...@nxp.com:
> From: Nipun Gupta <nipun.gu...@nxp.com>
>
> This change adds a RX offload capability and configuration to
> enable hardware to drop the packets in case of any error in the
> packets such as L3 checksum error or L4 checksum.
>
> Signed-off-by: Nipun Gupta <nipun.gu...@nxp.com>
> Signed-off-by: Rohit Raj <rohit....@nxp.com>
> Reviewed-by: Asaf Penso <as...@nvidia.com>
> ---
[...]
> +/**
> + * A structure used to enable/disable error packet drop on RX.
RX -> Rx
same for other occurences below
> + */
> +struct rte_rx_err_pkt_drop_conf {
The name should start with rte_eth_
> + /** enable/disable all RX error packet drop.
> + * 0 (default) - disable, 1 enable
> + */
> + uint32_t all:1;
> +};
I don't understand the meaning of this struct.
Is it just one bit to drop packets having an error?
How do you determine what is an error?
[...]
> @@ -1236,6 +1246,8 @@ struct rte_eth_conf {
> uint32_t dcb_capability_en;
> struct rte_fdir_conf fdir_conf; /**< FDIR configuration. DEPRECATED */
> struct rte_intr_conf intr_conf; /**< Interrupt mode configuration. */
> + struct rte_rx_err_pkt_drop_conf err_pkt_drop_conf;
> + /**< RX error packet drop configuration. */
Why a per-port configuration is needed in addition of the per-queue offload?
[...]
> @@ -1260,6 +1272,7 @@ struct rte_eth_conf {
> #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000
> #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000
> #define DEV_RX_OFFLOAD_RSS_HASH 0x00080000
> +#define DEV_RX_OFFLOAD_ERR_PKT_DROP 0x00100000
New offload names should starte with RTE_ prefix.
> +/**
> + * RX Error Drop offload config/capabilities of a device. These
> + * are valid only when RX capability DEV_RX_OFFLOAD_ERR_PKT_DROP
> + * is supported by the device.
> + */
> +#define DEV_RX_ERR_PKT_DROP_OFFLOAD_ALL 0x00000001
I don't understand the meaning.
> @@ -1411,6 +1431,8 @@ struct rte_eth_dev_info {
> /**< Device per-queue RX offload capabilities. */
> uint64_t tx_queue_offload_capa;
> /**< Device per-queue TX offload capabilities. */
> + uint64_t rx_err_drop_offload_capa;
> + /**< RX error packet drop offload capabilities. */
Why adding a new field here instead of reporting in rx_offload_capa?
