[dpdk-dev] [PATCH v7 1/2] net/i40e: queue region set and flush

Wu, Jingjing jingjing.wu at intel.com
Fri Sep 29 14:22:49 CEST 2017


> +	/* update the local VSI info with updated queue map */
> +	(void)rte_memcpy(&vsi->info.tc_mapping, &ctxt.info.tc_mapping,
> +					sizeof(vsi->info.tc_mapping));
> +	(void)rte_memcpy(&vsi->info.queue_mapping,
> +			&ctxt.info.queue_mapping,
> +			sizeof(vsi->info.queue_mapping));
(void) can be omitted.

[......]
> +static int
> +i40e_queue_region_display_all_info(struct i40e_pf *pf, uint16_t port_id)
> +{
Not only the function name changes from "get info" to "display info". You may also
need to change the command name.

But, what I like is changing the keep the "get", but change the function
to return the a struct which contains the info, and user can call it. Otherwise it is
only used for show.

[.....]
> +enum rte_pmd_i40e_queue_region_op {
> +	RTE_PMD_I40E_REGION_UNDEFINED,
> +	RTE_PMD_I40E_QUEUE_REGION_SET,      /**< add queue region set */
> +	RTE_PMD_I40E_REGION_FLOWTYPE_SET,   /**< add pf region pctype set */
> +	/*** add queue region user priority set */

I think you missed my comments in last mail. NOT "***" but "**", please read
http://www.dpdk.org/doc/guides/contributing/documentation.html#doxygen-guidelines

> +	RTE_PMD_I40E_USER_PRIORITY_REGION_SET,
> +	/**
> +	 * ALL configuration about queue region from up layer
> +	 * at first will only keep in DPDK softwarestored in driver,
> +	 * only after " FLUSH_ON ", it commit all configuration to HW.
> +	 * Because PMD had to set hardware configuration at a time, so
> +	 * it will record all up layer command at first.
> +	 */
> +	RTE_PMD_I40E_RSS_QUEUE_REGION_ALL_FLUSH_ON,
> +	/**
> +	 * "FLUSH_OFF " is just clean all configuration about queue
> +	 * region just now, and restore all to DPDK i40e driver default
> +	 * config when start up.
> +	 */
> +	RTE_PMD_I40E_RSS_QUEUE_REGION_ALL_FLUSH_OFF,
> +	RTE_PMD_I40E_RSS_QUEUE_REGION_INFO_GET,
> +	RTE_PMD_I40E_QUEUE_REGION_OP_MAX
> +};
> +
>  #define RTE_PMD_I40E_DDP_NAME_SIZE 32
> 
>  /**
> @@ -146,6 +173,27 @@ struct rte_pmd_i40e_ptype_mapping {
>  };
> 
>  /**
> + * Queue region related information.
> + */
> +struct rte_i40e_rss_region_conf {
> +	/*** the region id for this configuration */



> +	uint8_t region_id;
> +	/** the pctype or hardware flowtype of packet,
> +	 * the specific index for each type has been defined
> +	 * in file i40e_type.h as enum i40e_filter_pctype.
> +	 */
> +	uint8_t hw_flowtype;
> +	/*** the start queue index for this region */
> +	uint8_t queue_start_index;
> +	/*** the total queue number of this queue region */
> +	uint8_t queue_num;
> +	/*** the packet's user priority for this region */
> +	uint8_t user_priority;
> +	 /*** Option types of queue region */
> +	enum rte_pmd_i40e_queue_region_op op;

The same ***  ->  **

Thanks
Jingjing



More information about the dev mailing list