> +     /* 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

Reply via email to