Re: [dpdk-dev] [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test

["Li, Xiaoyun" <xiaoyun.li@intel.com>]

> -----Original Message-----
> From: oulijun <oulijun@huawei.com>
> Sent: Tuesday, March 23, 2021 22:18
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; linuxarm@openeuler.org
> Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB
> test
> 
> 
> 
> 在 2021/3/23 16:55, Li, Xiaoyun 写道:
> > Hi
> >
> >> -----Original Message-----
> >> From: Lijun Ou <oulijun@huawei.com>
> >> Sent: Friday, March 5, 2021 18:22
> >> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> >> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org;
> >> linuxarm@openeuler.org
> >> Subject: [PATCH 1/3] app/testpmd: fix forwarding configuration when
> >> DCB test
> >>
> >> From: Huisong Li <lihuisong@huawei.com>
> >>
> > The commit message is too long and redundant. Please simplify it.
> >
<snip>
> >> +static uint16_t
> >> +get_fwd_port_total_tc_num(void)
> >> +{
> >> +	struct rte_eth_dcb_info dcb_info;
> >> +	uint16_t total_tc_num = 0;
> >> +	unsigned int i;
> >> +
> >> +	for (i = 0; i < nb_fwd_ports; i++) {
> >> +		(void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info);
> >> +		total_tc_num += dcb_info.nb_tcs;
> >> +	}
> >> +
> >> +	return total_tc_num;
> >> +}
> >
> > It's only 3 lines. Is it really necessary to make it into a function which is not
> called by anyone else? A comment and the loop are enough.
> >
> To calculate the total number of TCs on all forwarding ports, we have to call
> rte_eth_dev_get_dcb_info().  However,  rte_eth_dev_get_dcb_info() has been
> called twice by dcb_fwd_config_setup() to setup dcb forwarding configuration.
> The dcb_fwd_config_setup() might be more readable if encapsulated, I think.  In
> addition, variables in "cur_fwd_config"
> are initialized more centrally. What do you think?

I'm not familiar with dcb. Why does the rxp_dcb_info get from fwd_ports_ids[0] and txp_dcb_info get from fwd_ports_ids[1]?
Is this a have-to-be? Can it be both from port 0 or last port?
If it can be port 0 or last port, you can just store the last dcb_info after your loop (loop from 0, i++ or loop from last i--).

> >> +
<snip>
> >> --
> >> 2.7.4
> >
> > .
> >