diff mbox series

[v2,2/3] app/testpmd: enable configuring GENEVE port

Message ID 20200827070244.32392-3-ophirmu@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series Add GENEVE protocol parsing to testpmd |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ophir Munk Aug. 27, 2020, 7:02 a.m. UTC 
  From: Ophir Munk <ophirmu@mellanox.com>

IANA has assigned port 6081 as the fixed well-known destination port for
GENEVE. Nevertheless draft-ietf-nvo3-geneve-09 recommends that
implementations make this configurable.  This commit enables specifying
any positive UDP destination port number for GENEVE.

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
 app/test-pmd/parameters.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Ferruh Yigit Sept. 14, 2020, 5:31 p.m. UTC | #1 
On 8/27/2020 8:02 AM, Ophir Munk wrote:
> From: Ophir Munk <ophirmu@mellanox.com>
> 
> IANA has assigned port 6081 as the fixed well-known destination port for
> GENEVE. Nevertheless draft-ietf-nvo3-geneve-09 recommends that
> implementations make this configurable.  This commit enables specifying
> any positive UDP destination port number for GENEVE.
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
>  app/test-pmd/parameters.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 7cb0e3d..0d135b0 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -70,7 +70,8 @@ usage(char* progname)
>  	       "--rxpt= | --rxht= | --rxwt= | --rxfreet= | "
>  	       "--txpt= | --txht= | --txwt= | --txfreet= | "
>  	       "--txrst= | --tx-offloads= | | --rx-offloads= | "
> -	       "--vxlan-gpe-port= ]\n",
> +	       "--vxlan-gpe-port= ] "
> +	       "--geneve-port= ]\n",
>  	       progname);

Hi Ophir,

Is there a command line to update the geneve port?

I can see there are some parameter for other tunneling too, but do you
find providing parameter useful? I think command line better since it is
more flexible, what do you think?
Ophir Munk Sept. 15, 2020, 8:46 a.m. UTC | #2 
Hi Ferruh,
I don't think that having the flexibility to specify the Geneve protocol in the command line will have an added value.
The reason is that any Geneve setup has a fixed protocol number for the duration of the test lifetime (fixed and synched by all the peers in the lab). The protocol number is not dynamic.
So even if we could specify the protocol in the command line - it would be a onetime setting after testpmd startup. Therefore, it is simpler to have it as parameter only.

Regards,
Ophir 

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, September 14, 2020 8:31 PM
> To: Ophir Munk <ophirmu@nvidia.com>; dev@dpdk.org; Wenzhuo Lu
> <wenzhuo.lu@intel.com>; Beilei Xing <beilei.xing@intel.com>; Bernard
> Iremonger <bernard.iremonger@intel.com>
> Cc: Ophir Munk <ophirmu@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH v2 2/3] app/testpmd: enable configuring
> GENEVE port
> 
> On 8/27/2020 8:02 AM, Ophir Munk wrote:
> > From: Ophir Munk <ophirmu@mellanox.com>
> >
> > IANA has assigned port 6081 as the fixed well-known destination port
> > for GENEVE. Nevertheless draft-ietf-nvo3-geneve-09 recommends that
> > implementations make this configurable.  This commit enables
> > specifying any positive UDP destination port number for GENEVE.
> >
> > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> > ---
> >  app/test-pmd/parameters.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 7cb0e3d..0d135b0 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -70,7 +70,8 @@ usage(char* progname)
> >  	       "--rxpt= | --rxht= | --rxwt= | --rxfreet= | "
> >  	       "--txpt= | --txht= | --txwt= | --txfreet= | "
> >  	       "--txrst= | --tx-offloads= | | --rx-offloads= | "
> > -	       "--vxlan-gpe-port= ]\n",
> > +	       "--vxlan-gpe-port= ] "
> > +	       "--geneve-port= ]\n",
> >  	       progname);
> 
> Hi Ophir,
> 
> Is there a command line to update the geneve port?
> 
> I can see there are some parameter for other tunneling too, but do you find
> providing parameter useful? I think command line better since it is more
> flexible, what do you think?
Ferruh Yigit Sept. 15, 2020, 11:07 a.m. UTC | #3 
On 9/15/2020 9:46 AM, Ophir Munk wrote:
> Hi Ferruh,
> I don't think that having the flexibility to specify the Geneve protocol in the command line will have an added value.
> The reason is that any Geneve setup has a fixed protocol number for the duration of the test lifetime (fixed and synched by all the peers in the lab). The protocol number is not dynamic.
> So even if we could specify the protocol in the command line - it would be a onetime setting after testpmd startup. Therefore, it is simpler to have it as parameter only.
> 

OK, if you think there is a value.

Overall, not specific to this patch, it bothers me not having any
guideline what to add as a parameter, and not able to say what is
implemented as parameter without searching for it ...

> Regards,
> Ophir 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Monday, September 14, 2020 8:31 PM
>> To: Ophir Munk <ophirmu@nvidia.com>; dev@dpdk.org; Wenzhuo Lu
>> <wenzhuo.lu@intel.com>; Beilei Xing <beilei.xing@intel.com>; Bernard
>> Iremonger <bernard.iremonger@intel.com>
>> Cc: Ophir Munk <ophirmu@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH v2 2/3] app/testpmd: enable configuring
>> GENEVE port
>>
>> On 8/27/2020 8:02 AM, Ophir Munk wrote:
>>> From: Ophir Munk <ophirmu@mellanox.com>
>>>
>>> IANA has assigned port 6081 as the fixed well-known destination port
>>> for GENEVE. Nevertheless draft-ietf-nvo3-geneve-09 recommends that
>>> implementations make this configurable.  This commit enables
>>> specifying any positive UDP destination port number for GENEVE.
>>>
>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
>>> ---
>>>  app/test-pmd/parameters.c | 13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>> index 7cb0e3d..0d135b0 100644
>>> --- a/app/test-pmd/parameters.c
>>> +++ b/app/test-pmd/parameters.c
>>> @@ -70,7 +70,8 @@ usage(char* progname)
>>>  	       "--rxpt= | --rxht= | --rxwt= | --rxfreet= | "
>>>  	       "--txpt= | --txht= | --txwt= | --txfreet= | "
>>>  	       "--txrst= | --tx-offloads= | | --rx-offloads= | "
>>> -	       "--vxlan-gpe-port= ]\n",
>>> +	       "--vxlan-gpe-port= ] "
>>> +	       "--geneve-port= ]\n",
>>>  	       progname);
>>
>> Hi Ophir,
>>
>> Is there a command line to update the geneve port?
>>
>> I can see there are some parameter for other tunneling too, but do you find
>> providing parameter useful? I think command line better since it is more
>> flexible, what do you think?
Ophir Munk Sept. 15, 2020, 12:59 p.m. UTC | #4 
Hi Ferruh,
I added to documentation the new parameter "--geneve-port=N" (second patch in the series).
I also rebased the series and sent v3.

Regards,
Ophir

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> > I don't think that having the flexibility to specify the Geneve protocol in the
> command line will have an added value.
> > The reason is that any Geneve setup has a fixed protocol number for the
> duration of the test lifetime (fixed and synched by all the peers in the lab).
> The protocol number is not dynamic.
> > So even if we could specify the protocol in the command line - it would be
> a onetime setting after testpmd startup. Therefore, it is simpler to have it as
> parameter only.
> >
> 
> Overall, not specific to this patch, it bothers me not having any guideline
> what to add as a parameter, and not able to say what is implemented as
> parameter without searching for it ...
> 
> > Regards,
> > Ophir
> >
Ophir Munk Sept. 15, 2020, 1:19 p.m. UTC | #5 
Hi,
V4 sent with misspelling corrections.

Regards,
Ophir

> -----Original Message-----
> From: Ophir Munk
> Sent: Tuesday, September 15, 2020 3:59 PM
> To: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org; Wenzhuo Lu
> <wenzhuo.lu@intel.com>; Beilei Xing <beilei.xing@intel.com>; Bernard
> Iremonger <bernard.iremonger@intel.com>
> Cc: Ophir Munk <ophirmu@mellanox.com>; Raslan Darawsheh
> <rasland@nvidia.com>
> Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: enable configuring
> GENEVE port
> 
> Hi Ferruh,
> I added to documentation the new parameter "--geneve-port=N" (second
> patch in the series).
> I also rebased the series and sent v3.
> 
> Regards,
> Ophir
diff mbox series

Patch

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 7cb0e3d..0d135b0 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -70,7 +70,8 @@  usage(char* progname)
 	       "--rxpt= | --rxht= | --rxwt= | --rxfreet= | "
 	       "--txpt= | --txht= | --txwt= | --txfreet= | "
 	       "--txrst= | --tx-offloads= | | --rx-offloads= | "
-	       "--vxlan-gpe-port= ]\n",
+	       "--vxlan-gpe-port= ] "
+	       "--geneve-port= ]\n",
 	       progname);
 #ifdef RTE_LIBRTE_CMDLINE
 	printf("  --interactive: run in interactive mode.\n");
@@ -199,6 +200,7 @@  usage(char* progname)
 	printf("  --rx-offloads=0xXXXXXXXX: hexadecimal bitmask of RX queue offloads\n");
 	printf("  --hot-plug: enable hot plug for device.\n");
 	printf("  --vxlan-gpe-port=N: UPD port of tunnel VXLAN-GPE\n");
+	printf("  --geneve-port=N: UPD port of tunnel GENEVE\n");
 	printf("  --mlockall: lock all memory\n");
 	printf("  --no-mlockall: do not lock all memory\n");
 	printf("  --mp-alloc <native|anon|xmem|xmemhuge>: mempool allocation method.\n"
@@ -664,6 +666,7 @@  launch_args_parse(int argc, char** argv)
 		{ "rx-offloads",		1, 0, 0 },
 		{ "hot-plug",			0, 0, 0 },
 		{ "vxlan-gpe-port",		1, 0, 0 },
+		{ "geneve-port",		1, 0, 0 },
 		{ "mlockall",			0, 0, 0 },
 		{ "no-mlockall",		0, 0, 0 },
 		{ "mp-alloc",			1, 0, 0 },
@@ -1298,6 +1301,14 @@  launch_args_parse(int argc, char** argv)
 					rte_exit(EXIT_FAILURE,
 						 "vxlan-gpe-port must be >= 0\n");
 			}
+			if (!strcmp(lgopts[opt_idx].name, "geneve-port")) {
+				n = atoi(optarg);
+				if (n >= 0)
+					geneve_udp_port = (uint16_t)n;
+				else
+					rte_exit(EXIT_FAILURE,
+						 "geneve-port must be >= 0\n");
+			}
 			if (!strcmp(lgopts[opt_idx].name, "print-event"))
 				if (parse_event_printing_config(optarg, 1)) {
 					rte_exit(EXIT_FAILURE,