[dpdk-dev] [PATCH v11 2/3] ethdev: extend RSS offload types

[Andrew Rybchenko]

On 10/9/19 9:57 AM, Simei Su wrote:
> This patch reserves several bits as input set selection from the
> high end of the 64 bits. It is combined with exisiting ETH_RSS_*
> to represent RSS types.
>
> Signed-off-by: Simei Su <simei.su at intel.com>
> Reviewed-by: Qi Zhang <qi.z.zhang at intel.com>
> Acked-by: Ori Kam <orika at mellanox.com>
> ---
>   lib/librte_ethdev/rte_ethdev.c |  5 +++++
>   lib/librte_ethdev/rte_ethdev.h | 35 +++++++++++++++++++++++++++++++++++
>   2 files changed, 40 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index af82360..69f4133 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1269,6 +1269,9 @@ struct rte_eth_dev *
>   		goto rollback;
>   	}
>   
> +	dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf =
> +		strip_out_src_dst_only(dev_conf->rx_adv_conf.rss_conf.rss_hf);
> +
>   	/* Check that device supports requested rss hash functions. */
>   	if ((dev_info.flow_type_rss_offloads |
>   	     dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
> @@ -3112,6 +3115,8 @@ struct rte_eth_dev *
>   	if (ret != 0)
>   		return ret;
>   
> +	rss_conf->rss_hf = strip_out_src_dst_only(rss_conf->rss_hf);
> +
>   	dev = &rte_eth_devices[port_id];
>   	if ((dev_info.flow_type_rss_offloads | rss_conf->rss_hf) !=
>   	    dev_info.flow_type_rss_offloads) {
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 7722f70..ef59ed5 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -505,6 +505,20 @@ struct rte_eth_rss_conf {
>   #define ETH_RSS_GENEVE             (1ULL << 20)
>   #define ETH_RSS_NVGRE              (1ULL << 21)
>   
> +/*
> + * We use the following macros to combine with above ETH_RSS_* for
> + * more specific input set selection. These bits are defined starting
> + * from the high end of the 64 bits.
> + * Note: If we use above ETH_RSS_* without SRC/DST_ONLY, it represents
> + * both SRC and DST are taken into account. If SRC_ONLY and DST_ONLY of
> + * the same level are used simultaneously, it is the same case as none of
> + * them are added.
> + */
> +#define ETH_RSS_L3_SRC_ONLY        (1ULL << 63)
> +#define ETH_RSS_L3_DST_ONLY        (1ULL << 62)
> +#define ETH_RSS_L4_SRC_ONLY        (1ULL << 61)
> +#define ETH_RSS_L4_DST_ONLY        (1ULL << 60)
> +
>   #define ETH_RSS_IP ( \
>   	ETH_RSS_IPV4 | \
>   	ETH_RSS_FRAG_IPV4 | \
> @@ -4034,6 +4048,27 @@ int rte_eth_dev_adjust_nb_rx_tx_desc(uint16_t port_id,
>   void *
>   rte_eth_dev_get_sec_ctx(uint16_t port_id);
>   
> +/**
> + * If SRC_ONLY and DST_ONLY of the same level are used
> + * simultaneously, it is the same case as none of them
> + * are added.
> + *
> + * @param rss_hf
> + *   RSS types with SRC/DST_ONLY.
> + * @return
> + *   RSS types.
> + */
> +static inline uint64_t
> +strip_out_src_dst_only(uint64_t rss_hf)

Inline function in public header without corresponding prefix is a bad idea.
Please, move it to C file and I think that inline should be removed.
Let the compiler do its job.

> +{
> +	if ((rss_hf & ETH_RSS_L3_SRC_ONLY) && (rss_hf & ETH_RSS_L3_DST_ONLY))
> +		rss_hf &= ~(ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY);
> +
> +	if ((rss_hf & ETH_RSS_L4_SRC_ONLY) && (rss_hf & ETH_RSS_L4_DST_ONLY))
> +		rss_hf &= ~(ETH_RSS_L4_SRC_ONLY | ETH_RSS_L4_DST_ONLY);
> +
> +	return rss_hf;
> +}
>   
>   #include <rte_ethdev_core.h>
>