[dpdk-dev] [PATCH v3] examples/l2fwd: add cmdline option for forwarding port info

[Sunil Kumar Kori]

>-----Original Message-----
>From: Andrzej Ostruszka [C] <aostruszka at marvell.com>
>Sent: Monday, April 27, 2020 3:06 PM
>To: Sunil Kumar Kori <skori at marvell.com>; Pavan Nikhilesh Bhagavatula
><pbhagavatula at marvell.com>; Jerin Jacob Kollanukkaran
><jerinj at marvell.com>; thomas at monjalon.net; John McNamara
><john.mcnamara at intel.com>; Marko Kovacevic
><marko.kovacevic at intel.com>; Ori Kam <orika at mellanox.com>; Bruce
>Richardson <bruce.richardson at intel.com>; Radu Nicolau
><radu.nicolau at intel.com>; Akhil Goyal <akhil.goyal at nxp.com>; Tomasz
>Kantecki <tomasz.kantecki at intel.com>
>Cc: dev at dpdk.org; Vamsi Krishna Attunuru <vattunuru at marvell.com>
>Subject: Re: [dpdk-dev] [PATCH v3] examples/l2fwd: add cmdline option for
>forwarding port info
>
>On 27/04/2020 11:19, Sunil Kumar Kori wrote:
>>> -----Original Message-----
>>> From: pbhagavatula at marvell.com <pbhagavatula at marvell.com>
>[...]
>>> @@ -67,6 +68,15 @@ static uint32_t l2fwd_enabled_port_mask = 0;
>>> /* list of enabled ports */
>>> static uint32_t l2fwd_dst_ports[RTE_MAX_ETHPORTS];
>>>
>>> +struct port_pair_params {
>>> +#define NUM_PORTS	2
>>> +	uint16_t port[NUM_PORTS];
>>> +} __rte_cache_aligned;
>>
>> Is there any specific reason to use this syntax to declare two ports
>> instead of following struct port_pair_params {
>>       uint16_t port1;
>>       uint16_t port2;
>> };
>
>Initially it was so, but I made a comment that this leads to code duplication in
>check_port_pair_config() (same checks for port1 and port2
>- now handled via loop).  I still stand by this comment :)
>
Okay, I think having for two variables only is not a big deal. if it is some higher number then it look fine.
IMO, it is more readable when using suggested way. You can take on this.  

>With regards
>Andrzej Ostruszka