Re: [dpdk-dev] [PATCH v5 1/3] app/testpmd: add GENEVE parsing

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 10/8/2020 9:44 AM, Ophir Munk wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, October 7, 2020 7:25 PM
>> To: Ophir Munk <ophirmu@nvidia.com>; dev@dpdk.org; Wenzhuo Lu
> <...>
>> Only when user set via "--geneve-port=N", it is the port for testpmd to parse
>> (same for 'geneve_udp_port' global variable), but when user give command
>> "port config N add geneve X" it is to configure the NIC to offload the parsing.
>> This is too confusing, user can't know (or remember) this without checking
>> the source code.
>>
>> Can we rename the command and variable to highlight that it is for testpmd
>> to parse, I think that will be enough, instead of trying to merge them, which
>> is hard as you described above.
>>
>> What do you think for "--testpmd-geneve-port=N" parameter and
>> 'testpmd_geneve_udp_port' variable name?
> 
> I am suggesting two options:
> 1. "--geneve-port-identifier=N" and "geneve_udp_port_identifier" as variable name.
> 2. "--geneve-parsed-port=N" and "geneve_udp_port" as variable name.
> Can you select one of them?
> 

I think 'identifier' doesn't add any more clarification, lets go with (2). Thanks.

>> We can also update the documentation to say this is only the port testpmd
>> uses for parsing, HW may be configured to use another port.
>>
> 
> Will update documentation.
> 
> <...>
>>> Done in v6. I added other missing tunnel protocols (in alphabetical order)
>> such as "gtp". Since it is more than geneve I added it to the 3rd (cleanup)
>> commit.
>>>
>>
>> Would you mind adding the 'geneve' with the patch adding 'geneve' support
>> (1/3), and add the other missing ones in the patch 3/3 ?
> 
> Will update accordingly
>