HI, jingjing
I will rework code as your comments in v8.
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, September 29, 2017 8:23 PM
> To: Zhao1, Wei <wei.zh...@intel.com>; dev@dpdk.org
> Cc: Zhao1, Wei <wei.zh...@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v7 1/2] net/i40e: queue region set and flush
>
>
> > + /* 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#doxyg
> en-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
