[v7,0/2] app/testpmd implement VXLAN/NVGRE Encap/Decap

Message ID cover.1530099631.git.nelio.laranjeiro@6wind.com (mailing list archive)
Headers
Series app/testpmd implement VXLAN/NVGRE Encap/Decap |

Message

Nélio Laranjeiro June 27, 2018, 11:45 a.m. UTC
  This series adds an easy and maintainable configuration version support for
those two actions for 18.08 by using global variables in testpmd to store the
necessary information for the tunnel encapsulation.  Those variables are used
in conjunction of RTE_FLOW_ACTION_{VXLAN,NVGRE}_ENCAP action to create easily
the action for flows.

A common way to use it:

 set vxlan ipv4 4 4 4 127.0.0.1 128.0.0.1 11:11:11:11:11:11 22:22:22:22:22:22
 flow create 0 ingress pattern end actions vxlan_encap / queue index 0 / end

 set vxlan ipv6 4 4 4 ::1 ::2222 11:11:11:11:11:11 22:22:22:22:22:22
 flow create 0 ingress pattern end actions vxlan_encap / queue index 0 / end

 set nvgre ipv4 4 127.0.0.1 128.0.0.1 11:11:11:11:11:11 22:22:22:22:22:22
 flow create 0 ingress pattern end actions nvgre_encap / queue index 0 / end

 set nvgre ipv6 4 ::1 ::2222 11:11:11:11:11:11 22:22:22:22:22:22
 flow create 0 ingress pattern end actions nvgre_encap / queue index 0 / end

This also replace the proposal done by Mohammad Abdul Awal [1] which handles
in a more complex way for the same work.

Note this API has already a modification planned for 18.11 [2] thus those
series should have a limited life for a single release.

[1] https://dpdk.org/ml/archives/dev/2018-May/101403.html
[2] https://dpdk.org/ml/archives/dev/2018-June/103485.html

Changes in v7:

- add missing documentation added in v5 and removed in v6 by mistake.

Changes in v6:

- fix compilation under redhat 7.5 with gcc 4.8.5 20150623

Changes in v5:

- fix documentation generation.
- add more explanation on how to generate several encapsulated flows.

Changes in v4:

- fix big endian issue on vni and tni.
- add samples to the documentation.
- set the VXLAN UDP source port to 0 by default to let the driver generate it
  from the inner hash as described in the RFC 7348.
- use default rte flow mask for each item.

Changes in v3:

- support VLAN in the outer encapsulation.
- fix the documentation with missing arguments.

Changes in v2:

- add default IPv6 values for NVGRE encapsulation.
- replace VXLAN to NVGRE in comments concerning NVGRE layer.

Nelio Laranjeiro (2):
  app/testpmd: add VXLAN encap/decap support
  app/testpmd: add NVGRE encap/decap support

 app/test-pmd/cmdline.c                      | 252 ++++++++++++++++++
 app/test-pmd/cmdline_flow.c                 | 274 ++++++++++++++++++++
 app/test-pmd/testpmd.c                      |  32 +++
 app/test-pmd/testpmd.h                      |  32 +++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  82 ++++++
 5 files changed, 672 insertions(+)
  

Comments

Mohammad Abdul Awal July 2, 2018, 10:40 a.m. UTC | #1
On 27/06/2018 12:45, Nelio Laranjeiro wrote:
> This series adds an easy and maintainable configuration version support for
> those two actions for 18.08 by using global variables in testpmd to store the
> necessary information for the tunnel encapsulation.  Those variables are used
> in conjunction of RTE_FLOW_ACTION_{VXLAN,NVGRE}_ENCAP action to create easily
> the action for flows.
>
> A common way to use it:
>
>   set vxlan ipv4 4 4 4 127.0.0.1 128.0.0.1 11:11:11:11:11:11 22:22:22:22:22:22
>   flow create 0 ingress pattern end actions vxlan_encap / queue index 0 / end
>
>   set vxlan ipv6 4 4 4 ::1 ::2222 11:11:11:11:11:11 22:22:22:22:22:22
>   flow create 0 ingress pattern end actions vxlan_encap / queue index 0 / end
>
>   set nvgre ipv4 4 127.0.0.1 128.0.0.1 11:11:11:11:11:11 22:22:22:22:22:22
>   flow create 0 ingress pattern end actions nvgre_encap / queue index 0 / end
>
>   set nvgre ipv6 4 ::1 ::2222 11:11:11:11:11:11 22:22:22:22:22:22
>   flow create 0 ingress pattern end actions nvgre_encap / queue index 0 / end
>
> This also replace the proposal done by Mohammad Abdul Awal [1] which handles
> in a more complex way for the same work.
>
> Note this API has already a modification planned for 18.11 [2] thus those
> series should have a limited life for a single release.
>
> [1] https://dpdk.org/ml/archives/dev/2018-May/101403.html
> [2] https://dpdk.org/ml/archives/dev/2018-June/103485.html
>
> Changes in v7:
>
> - add missing documentation added in v5 and removed in v6 by mistake.
>
> Changes in v6:
>
> - fix compilation under redhat 7.5 with gcc 4.8.5 20150623
>
> Changes in v5:
>
> - fix documentation generation.
> - add more explanation on how to generate several encapsulated flows.
>
> Changes in v4:
>
> - fix big endian issue on vni and tni.
> - add samples to the documentation.
> - set the VXLAN UDP source port to 0 by default to let the driver generate it
>    from the inner hash as described in the RFC 7348.
> - use default rte flow mask for each item.
>
> Changes in v3:
>
> - support VLAN in the outer encapsulation.
> - fix the documentation with missing arguments.
>
> Changes in v2:
>
> - add default IPv6 values for NVGRE encapsulation.
> - replace VXLAN to NVGRE in comments concerning NVGRE layer.
>
> Nelio Laranjeiro (2):
>    app/testpmd: add VXLAN encap/decap support
>    app/testpmd: add NVGRE encap/decap support
>
>   app/test-pmd/cmdline.c                      | 252 ++++++++++++++++++
>   app/test-pmd/cmdline_flow.c                 | 274 ++++++++++++++++++++
>   app/test-pmd/testpmd.c                      |  32 +++
>   app/test-pmd/testpmd.h                      |  32 +++
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  82 ++++++
>   5 files changed, 672 insertions(+)


Hi,

I have one concern in terms of usability though.
In testpmd, the rte_flow command line options have auto-completion with 
"<item_name> <item_name_value>" format which make using the command very 
much user friendly.

For the command "set vxlan ipv4 4 4 4 127.0.0.1 128.0.0.1 
11:11:11:11:11:11 22:22:22:22:22:22", it does not look much user 
friendly to me. A user may easily lose track of sequence of 9 param 
items. It would be much user friendly if the options would be like below 
and has auto-completion.

set vxlan ip_ver <ip_ver-value> vni <vni-value> udp_src <udp_src-value> 
udp-dst <udp_dst-value> ip_src <ip_src-value> ip_dst <ip_dst-value> 
eth_src <eth_src-value> eth_dst <eth_dst-value>

This way an user may never feel confused. Can maintainers comment on 
this point please?

Regards,
Awal.
  
Ferruh Yigit July 4, 2018, 2:54 p.m. UTC | #2
On 7/2/2018 11:40 AM, Mohammad Abdul Awal wrote:
> 
> On 27/06/2018 12:45, Nelio Laranjeiro wrote:
>> This series adds an easy and maintainable configuration version support for
>> those two actions for 18.08 by using global variables in testpmd to store the
>> necessary information for the tunnel encapsulation.  Those variables are used
>> in conjunction of RTE_FLOW_ACTION_{VXLAN,NVGRE}_ENCAP action to create easily
>> the action for flows.
>>
>> A common way to use it:
>>
>>   set vxlan ipv4 4 4 4 127.0.0.1 128.0.0.1 11:11:11:11:11:11 22:22:22:22:22:22
>>   flow create 0 ingress pattern end actions vxlan_encap / queue index 0 / end
>>
>>   set vxlan ipv6 4 4 4 ::1 ::2222 11:11:11:11:11:11 22:22:22:22:22:22
>>   flow create 0 ingress pattern end actions vxlan_encap / queue index 0 / end
>>
>>   set nvgre ipv4 4 127.0.0.1 128.0.0.1 11:11:11:11:11:11 22:22:22:22:22:22
>>   flow create 0 ingress pattern end actions nvgre_encap / queue index 0 / end
>>
>>   set nvgre ipv6 4 ::1 ::2222 11:11:11:11:11:11 22:22:22:22:22:22
>>   flow create 0 ingress pattern end actions nvgre_encap / queue index 0 / end
>>
>> This also replace the proposal done by Mohammad Abdul Awal [1] which handles
>> in a more complex way for the same work.
>>
>> Note this API has already a modification planned for 18.11 [2] thus those
>> series should have a limited life for a single release.
>>
>> [1] https://dpdk.org/ml/archives/dev/2018-May/101403.html
>> [2] https://dpdk.org/ml/archives/dev/2018-June/103485.html
>>
>> Changes in v7:
>>
>> - add missing documentation added in v5 and removed in v6 by mistake.
>>
>> Changes in v6:
>>
>> - fix compilation under redhat 7.5 with gcc 4.8.5 20150623
>>
>> Changes in v5:
>>
>> - fix documentation generation.
>> - add more explanation on how to generate several encapsulated flows.
>>
>> Changes in v4:
>>
>> - fix big endian issue on vni and tni.
>> - add samples to the documentation.
>> - set the VXLAN UDP source port to 0 by default to let the driver generate it
>>    from the inner hash as described in the RFC 7348.
>> - use default rte flow mask for each item.
>>
>> Changes in v3:
>>
>> - support VLAN in the outer encapsulation.
>> - fix the documentation with missing arguments.
>>
>> Changes in v2:
>>
>> - add default IPv6 values for NVGRE encapsulation.
>> - replace VXLAN to NVGRE in comments concerning NVGRE layer.
>>
>> Nelio Laranjeiro (2):
>>    app/testpmd: add VXLAN encap/decap support
>>    app/testpmd: add NVGRE encap/decap support
>>
>>   app/test-pmd/cmdline.c                      | 252 ++++++++++++++++++
>>   app/test-pmd/cmdline_flow.c                 | 274 ++++++++++++++++++++
>>   app/test-pmd/testpmd.c                      |  32 +++
>>   app/test-pmd/testpmd.h                      |  32 +++
>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  82 ++++++
>>   5 files changed, 672 insertions(+)
> 
> 
> Hi,
> 
> I have one concern in terms of usability though.
> In testpmd, the rte_flow command line options have auto-completion with 
> "<item_name> <item_name_value>" format which make using the command very 
> much user friendly.
> 
> For the command "set vxlan ipv4 4 4 4 127.0.0.1 128.0.0.1 
> 11:11:11:11:11:11 22:22:22:22:22:22", it does not look much user 
> friendly to me. A user may easily lose track of sequence of 9 param 
> items. It would be much user friendly if the options would be like below 
> and has auto-completion.
> 
> set vxlan ip_ver <ip_ver-value> vni <vni-value> udp_src <udp_src-value> 
> udp-dst <udp_dst-value> ip_src <ip_src-value> ip_dst <ip_dst-value> 
> eth_src <eth_src-value> eth_dst <eth_dst-value>

Hi Nelio, Adrien,

I tend to agree with Awal here, this is to forget/confuse and key-value pairs
makes it easier to use.

Meanwhile this is an usability improvement and I prefer not to block this patch
for this.

What is your comment on this, how should we proceed?

Thanks,
ferruh

> 
> This way an user may never feel confused. Can maintainers comment on 
> this point please?
> 
> Regards,
> Awal.
>
  
Nélio Laranjeiro July 5, 2018, 9:37 a.m. UTC | #3
On Wed, Jul 04, 2018 at 03:54:32PM +0100, Ferruh Yigit wrote:
> On 7/2/2018 11:40 AM, Mohammad Abdul Awal wrote:
> > 
> > On 27/06/2018 12:45, Nelio Laranjeiro wrote:
> >> This series adds an easy and maintainable configuration version support for
> >> those two actions for 18.08 by using global variables in testpmd to store the
> >> necessary information for the tunnel encapsulation.  Those variables are used
> >> in conjunction of RTE_FLOW_ACTION_{VXLAN,NVGRE}_ENCAP action to create easily
> >> the action for flows.
> >>
> >> A common way to use it:
> >>
> >>   set vxlan ipv4 4 4 4 127.0.0.1 128.0.0.1 11:11:11:11:11:11 22:22:22:22:22:22
> >>   flow create 0 ingress pattern end actions vxlan_encap / queue index 0 / end
> >>
> >>   set vxlan ipv6 4 4 4 ::1 ::2222 11:11:11:11:11:11 22:22:22:22:22:22
> >>   flow create 0 ingress pattern end actions vxlan_encap / queue index 0 / end
> >>
> >>   set nvgre ipv4 4 127.0.0.1 128.0.0.1 11:11:11:11:11:11 22:22:22:22:22:22
> >>   flow create 0 ingress pattern end actions nvgre_encap / queue index 0 / end
> >>
> >>   set nvgre ipv6 4 ::1 ::2222 11:11:11:11:11:11 22:22:22:22:22:22
> >>   flow create 0 ingress pattern end actions nvgre_encap / queue index 0 / end
> >>
> >> This also replace the proposal done by Mohammad Abdul Awal [1] which handles
> >> in a more complex way for the same work.
> >>
> >> Note this API has already a modification planned for 18.11 [2] thus those
> >> series should have a limited life for a single release.
> >>
> >> [1] https://dpdk.org/ml/archives/dev/2018-May/101403.html
> >> [2] https://dpdk.org/ml/archives/dev/2018-June/103485.html
> >>
> >> Changes in v7:
> >>
> >> - add missing documentation added in v5 and removed in v6 by mistake.
> >>
> >> Changes in v6:
> >>
> >> - fix compilation under redhat 7.5 with gcc 4.8.5 20150623
> >>
> >> Changes in v5:
> >>
> >> - fix documentation generation.
> >> - add more explanation on how to generate several encapsulated flows.
> >>
> >> Changes in v4:
> >>
> >> - fix big endian issue on vni and tni.
> >> - add samples to the documentation.
> >> - set the VXLAN UDP source port to 0 by default to let the driver generate it
> >>    from the inner hash as described in the RFC 7348.
> >> - use default rte flow mask for each item.
> >>
> >> Changes in v3:
> >>
> >> - support VLAN in the outer encapsulation.
> >> - fix the documentation with missing arguments.
> >>
> >> Changes in v2:
> >>
> >> - add default IPv6 values for NVGRE encapsulation.
> >> - replace VXLAN to NVGRE in comments concerning NVGRE layer.
> >>
> >> Nelio Laranjeiro (2):
> >>    app/testpmd: add VXLAN encap/decap support
> >>    app/testpmd: add NVGRE encap/decap support
> >>
> >>   app/test-pmd/cmdline.c                      | 252 ++++++++++++++++++
> >>   app/test-pmd/cmdline_flow.c                 | 274 ++++++++++++++++++++
> >>   app/test-pmd/testpmd.c                      |  32 +++
> >>   app/test-pmd/testpmd.h                      |  32 +++
> >>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  82 ++++++
> >>   5 files changed, 672 insertions(+)
> > 
> > 
> > Hi,
> > 
> > I have one concern in terms of usability though.
> > In testpmd, the rte_flow command line options have auto-completion with 
> > "<item_name> <item_name_value>" format which make using the command very 
> > much user friendly.
> > 
> > For the command "set vxlan ipv4 4 4 4 127.0.0.1 128.0.0.1 
> > 11:11:11:11:11:11 22:22:22:22:22:22", it does not look much user 
> > friendly to me. A user may easily lose track of sequence of 9 param 
> > items. It would be much user friendly if the options would be like below 
> > and has auto-completion.
> > 
> > set vxlan ip_ver <ip_ver-value> vni <vni-value> udp_src <udp_src-value> 
> > udp-dst <udp_dst-value> ip_src <ip_src-value> ip_dst <ip_dst-value> 
> > eth_src <eth_src-value> eth_dst <eth_dst-value>
> 
> Hi Nelio, Adrien,
> 
> I tend to agree with Awal here, this is to forget/confuse and key-value pairs
> makes it easier to use.
> 
> Meanwhile this is an usability improvement and I prefer not to block this patch
> for this.
> 
> What is your comment on this, how should we proceed?
> 
> Thanks,
> ferruh

Hi,

I also agree with this proposal, I'll prepare a v8 with those fix
tokens.

> > This way an user may never feel confused. Can maintainers comment on 
> > this point please?
> > 
> > Regards,
> > Awal.

Thanks