mbox series

[ovs-dev,V3,00/12] netdev datapath offload: Support IPv6 and VXLAN encap

Message ID 20200621111924.12397-1-elibr@mellanox.com
Headers show
Series netdev datapath offload: Support IPv6 and VXLAN encap | expand

Message

Eli Britstein June 21, 2020, 11:19 a.m. UTC
This patch set includes additional offloads - IPv6 and VXLAN encap, and
enhanced logging to increase debugability.

Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
                 TCP/UDP ports, set IPv6 and encap actions
                 (clone/output).
Patch #9:        Bug fix of partial offloads.
Patches #10-#11: Enhance log prints for debugability.
Patch #12:       Fix Ethernet matching for type only.

v2-v1:
- Removed redundant out label.
- Added a patch to fix dl_type match only.
v3-v2:
- Rebased, and more elaboration in #7 commit message.

Travis:
v1: https://travis-ci.org/github/elibritstein/OVS/builds/688413350
v2: https://travis-ci.org/github/elibritstein/OVS/builds/691375847
v3: https://travis-ci.org/github/elibritstein/OVS/builds/700534550


Eli Britstein (10):
  netdev-offload-dpdk: Remove pre-validate of patterns function
  netdev-offload-dpdk: Add IPv6 pattern matching
  netdev-offload-dpdk: Support offload of set IPv6 actions
  netdev-offload-dpdk: Support partial TCP/UDP port matching
  netdev-offload-dpdk: Support offload of clone tnl_push/output actions
  netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
  dpif-netdev: Don't use zero flow mark
  dpif-netdev: Add mega ufid in flow add log
  netdev-offload-dpdk: Add testpmd log commands
  netdev-offload-dpdk: Fix Ethernet matching for type only

Ilya Maximets (2):
  netdev: Allow storing dpif type into netdev structure.
  netdev-offload: Use dpif type instead of class.

 Documentation/howto/dpdk.rst  |   4 +-
 NEWS                          |   3 +
 lib/dpif-netdev.c             |  26 +-
 lib/dpif-netlink.c            |  23 +-
 lib/dpif.c                    |  21 +-
 lib/netdev-offload-dpdk.c     | 649 +++++++++++++++++++++++++++++++++++-------
 lib/netdev-offload-tc.c       |   3 +-
 lib/netdev-offload.c          |  52 ++--
 lib/netdev-offload.h          |  16 +-
 lib/netdev-provider.h         |   3 +-
 lib/netdev.c                  |  16 ++
 lib/netdev.h                  |   2 +
 ofproto/ofproto-dpif-upcall.c |   5 +-
 tests/dpif-netdev.at          |  20 +-
 tests/ofproto-macros.at       |   3 +-
 15 files changed, 657 insertions(+), 189 deletions(-)

Comments

Ilya Maximets June 24, 2020, 10:37 a.m. UTC | #1
On 6/21/20 1:19 PM, Eli Britstein wrote:
> This patch set includes additional offloads - IPv6 and VXLAN encap, and
> enhanced logging to increase debugability.
> 
> Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
>                  TCP/UDP ports, set IPv6 and encap actions
>                  (clone/output).
> Patch #9:        Bug fix of partial offloads.
> Patches #10-#11: Enhance log prints for debugability.
> Patch #12:       Fix Ethernet matching for type only.
> 
> v2-v1:
> - Removed redundant out label.
> - Added a patch to fix dl_type match only.
> v3-v2:
> - Rebased, and more elaboration in #7 commit message.

Hi.

I noticed that you didn't include Acked-by tags from Harsha.
Was it intentional? i.e. if there was any significant changes during
rebase process?

I'd like to have these patches acked by Harsha before applying, so
my question if tags from v2 are valid for v3 and can I use them?
Harsha?  Eli?

Best regards, Ilya Maximets.

> 
> Travis:
> v1: https://travis-ci.org/github/elibritstein/OVS/builds/688413350
> v2: https://travis-ci.org/github/elibritstein/OVS/builds/691375847
> v3: https://travis-ci.org/github/elibritstein/OVS/builds/700534550
> 
> 
> Eli Britstein (10):
>   netdev-offload-dpdk: Remove pre-validate of patterns function
>   netdev-offload-dpdk: Add IPv6 pattern matching
>   netdev-offload-dpdk: Support offload of set IPv6 actions
>   netdev-offload-dpdk: Support partial TCP/UDP port matching
>   netdev-offload-dpdk: Support offload of clone tnl_push/output actions
>   netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
>   dpif-netdev: Don't use zero flow mark
>   dpif-netdev: Add mega ufid in flow add log
>   netdev-offload-dpdk: Add testpmd log commands
>   netdev-offload-dpdk: Fix Ethernet matching for type only
> 
> Ilya Maximets (2):
>   netdev: Allow storing dpif type into netdev structure.
>   netdev-offload: Use dpif type instead of class.
> 
>  Documentation/howto/dpdk.rst  |   4 +-
>  NEWS                          |   3 +
>  lib/dpif-netdev.c             |  26 +-
>  lib/dpif-netlink.c            |  23 +-
>  lib/dpif.c                    |  21 +-
>  lib/netdev-offload-dpdk.c     | 649 +++++++++++++++++++++++++++++++++++-------
>  lib/netdev-offload-tc.c       |   3 +-
>  lib/netdev-offload.c          |  52 ++--
>  lib/netdev-offload.h          |  16 +-
>  lib/netdev-provider.h         |   3 +-
>  lib/netdev.c                  |  16 ++
>  lib/netdev.h                  |   2 +
>  ofproto/ofproto-dpif-upcall.c |   5 +-
>  tests/dpif-netdev.at          |  20 +-
>  tests/ofproto-macros.at       |   3 +-
>  15 files changed, 657 insertions(+), 189 deletions(-)
>
Eli Britstein June 24, 2020, 10:47 a.m. UTC | #2
On 6/24/2020 1:37 PM, Ilya Maximets wrote:
> On 6/21/20 1:19 PM, Eli Britstein wrote:
>> This patch set includes additional offloads - IPv6 and VXLAN encap, and
>> enhanced logging to increase debugability.
>>
>> Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
>>                   TCP/UDP ports, set IPv6 and encap actions
>>                   (clone/output).
>> Patch #9:        Bug fix of partial offloads.
>> Patches #10-#11: Enhance log prints for debugability.
>> Patch #12:       Fix Ethernet matching for type only.
>>
>> v2-v1:
>> - Removed redundant out label.
>> - Added a patch to fix dl_type match only.
>> v3-v2:
>> - Rebased, and more elaboration in #7 commit message.
> Hi.
>
> I noticed that you didn't include Acked-by tags from Harsha.
> Was it intentional? i.e. if there was any significant changes during
> rebase process?
>
> I'd like to have these patches acked by Harsha before applying, so
> my question if tags from v2 are valid for v3 and can I use them?
> Harsha?  Eli?

No. It was by mistake. Sorry. The rebase had only one minor conflict in

[PATCH V3 06/12] netdev-offload: Use dpif type instead of class.

The conflict was due to previous commit [1].

As I will have to rebase again for sure to add testpmd prints for VLANs, 
as [2] was merged, will it be OK I'll add the Acked-by then?

[1] 191536574e3b ("netdev-offload: Implement terse dump support").

[2] 029273855939 ("netdev-offload-dpdk: Support offload of VLAN PUSH/POP 
actions.")

>
> Best regards, Ilya Maximets.
>
>> Travis:
>> v1: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F688413350&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=PjzU%2Fiwtc7nhzTJ99oELQhi4lDyZqDwHwGuId27W4dc%3D&reserved=0
>> v2: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F691375847&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=SoYpbcTbkl872Mz9q%2Fg%2FrBLBOv37qCYwEXXJ%2BcXblj4%3D&reserved=0
>> v3: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F700534550&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=2ZIo6xz56YdJWFxuQO9iqTygcLnwPuhbGo28m3y8EC0%3D&reserved=0
>>
>>
>> Eli Britstein (10):
>>    netdev-offload-dpdk: Remove pre-validate of patterns function
>>    netdev-offload-dpdk: Add IPv6 pattern matching
>>    netdev-offload-dpdk: Support offload of set IPv6 actions
>>    netdev-offload-dpdk: Support partial TCP/UDP port matching
>>    netdev-offload-dpdk: Support offload of clone tnl_push/output actions
>>    netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
>>    dpif-netdev: Don't use zero flow mark
>>    dpif-netdev: Add mega ufid in flow add log
>>    netdev-offload-dpdk: Add testpmd log commands
>>    netdev-offload-dpdk: Fix Ethernet matching for type only
>>
>> Ilya Maximets (2):
>>    netdev: Allow storing dpif type into netdev structure.
>>    netdev-offload: Use dpif type instead of class.
>>
>>   Documentation/howto/dpdk.rst  |   4 +-
>>   NEWS                          |   3 +
>>   lib/dpif-netdev.c             |  26 +-
>>   lib/dpif-netlink.c            |  23 +-
>>   lib/dpif.c                    |  21 +-
>>   lib/netdev-offload-dpdk.c     | 649 +++++++++++++++++++++++++++++++++++-------
>>   lib/netdev-offload-tc.c       |   3 +-
>>   lib/netdev-offload.c          |  52 ++--
>>   lib/netdev-offload.h          |  16 +-
>>   lib/netdev-provider.h         |   3 +-
>>   lib/netdev.c                  |  16 ++
>>   lib/netdev.h                  |   2 +
>>   ofproto/ofproto-dpif-upcall.c |   5 +-
>>   tests/dpif-netdev.at          |  20 +-
>>   tests/ofproto-macros.at       |   3 +-
>>   15 files changed, 657 insertions(+), 189 deletions(-)
>>
Ilya Maximets June 24, 2020, 2:16 p.m. UTC | #3
On 6/24/20 12:47 PM, Eli Britstein wrote:
> 
> On 6/24/2020 1:37 PM, Ilya Maximets wrote:
>> On 6/21/20 1:19 PM, Eli Britstein wrote:
>>> This patch set includes additional offloads - IPv6 and VXLAN encap, and
>>> enhanced logging to increase debugability.
>>>
>>> Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
>>>                   TCP/UDP ports, set IPv6 and encap actions
>>>                   (clone/output).
>>> Patch #9:        Bug fix of partial offloads.
>>> Patches #10-#11: Enhance log prints for debugability.
>>> Patch #12:       Fix Ethernet matching for type only.
>>>
>>> v2-v1:
>>> - Removed redundant out label.
>>> - Added a patch to fix dl_type match only.
>>> v3-v2:
>>> - Rebased, and more elaboration in #7 commit message.
>> Hi.
>>
>> I noticed that you didn't include Acked-by tags from Harsha.
>> Was it intentional? i.e. if there was any significant changes during
>> rebase process?
>>
>> I'd like to have these patches acked by Harsha before applying, so
>> my question if tags from v2 are valid for v3 and can I use them?
>> Harsha?  Eli?
> 
> No. It was by mistake. Sorry. The rebase had only one minor conflict in
> 
> [PATCH V3 06/12] netdev-offload: Use dpif type instead of class.
> 
> The conflict was due to previous commit [1].
> 
> As I will have to rebase again for sure to add testpmd prints for VLANs, as [2] was merged, will it be OK I'll add the Acked-by then?

Sure.  I think it should be OK for patches without any functional
changes.  Current patches doesn't apply starting from the second one
due to merged patch [2], so they need a rebase anyway.

Regarding the testpmd related patch itself, I'm not sure about it.
It might be hard to support in terms that we will need to track
testpmd changes and it's also hard to tell for which testpmd version
these cmdlines should be applicable.

Another point is that it's actually printing of the same information
twice.  And also lots of extra printing code.
Maybe, if you need these cmdlines, it's better to write a helper script
to convert existing output to testpmd cmdlines?  Or even parse offloaded
and non-offloaded flows from the output of dump-flows.

> 
> [1] 191536574e3b ("netdev-offload: Implement terse dump support").
> 
> [2] 029273855939 ("netdev-offload-dpdk: Support offload of VLAN PUSH/POP actions.")
> 
>>
>> Best regards, Ilya Maximets.
>>
>>> Travis:
>>> v1: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F688413350&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=PjzU%2Fiwtc7nhzTJ99oELQhi4lDyZqDwHwGuId27W4dc%3D&reserved=0
>>> v2: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F691375847&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=SoYpbcTbkl872Mz9q%2Fg%2FrBLBOv37qCYwEXXJ%2BcXblj4%3D&reserved=0
>>> v3: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F700534550&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=2ZIo6xz56YdJWFxuQO9iqTygcLnwPuhbGo28m3y8EC0%3D&reserved=0
>>>
>>>
>>> Eli Britstein (10):
>>>    netdev-offload-dpdk: Remove pre-validate of patterns function
>>>    netdev-offload-dpdk: Add IPv6 pattern matching
>>>    netdev-offload-dpdk: Support offload of set IPv6 actions
>>>    netdev-offload-dpdk: Support partial TCP/UDP port matching
>>>    netdev-offload-dpdk: Support offload of clone tnl_push/output actions
>>>    netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
>>>    dpif-netdev: Don't use zero flow mark
>>>    dpif-netdev: Add mega ufid in flow add log
>>>    netdev-offload-dpdk: Add testpmd log commands
>>>    netdev-offload-dpdk: Fix Ethernet matching for type only
>>>
>>> Ilya Maximets (2):
>>>    netdev: Allow storing dpif type into netdev structure.
>>>    netdev-offload: Use dpif type instead of class.
>>>
>>>   Documentation/howto/dpdk.rst  |   4 +-
>>>   NEWS                          |   3 +
>>>   lib/dpif-netdev.c             |  26 +-
>>>   lib/dpif-netlink.c            |  23 +-
>>>   lib/dpif.c                    |  21 +-
>>>   lib/netdev-offload-dpdk.c     | 649 +++++++++++++++++++++++++++++++++++-------
>>>   lib/netdev-offload-tc.c       |   3 +-
>>>   lib/netdev-offload.c          |  52 ++--
>>>   lib/netdev-offload.h          |  16 +-
>>>   lib/netdev-provider.h         |   3 +-
>>>   lib/netdev.c                  |  16 ++
>>>   lib/netdev.h                  |   2 +
>>>   ofproto/ofproto-dpif-upcall.c |   5 +-
>>>   tests/dpif-netdev.at          |  20 +-
>>>   tests/ofproto-macros.at       |   3 +-
>>>   15 files changed, 657 insertions(+), 189 deletions(-)
>>>
Ilya Maximets June 24, 2020, 2:28 p.m. UTC | #4
On 6/24/20 4:16 PM, Ilya Maximets wrote:
> On 6/24/20 12:47 PM, Eli Britstein wrote:
>>
>> On 6/24/2020 1:37 PM, Ilya Maximets wrote:
>>> On 6/21/20 1:19 PM, Eli Britstein wrote:
>>>> This patch set includes additional offloads - IPv6 and VXLAN encap, and
>>>> enhanced logging to increase debugability.
>>>>
>>>> Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
>>>>                   TCP/UDP ports, set IPv6 and encap actions
>>>>                   (clone/output).
>>>> Patch #9:        Bug fix of partial offloads.
>>>> Patches #10-#11: Enhance log prints for debugability.
>>>> Patch #12:       Fix Ethernet matching for type only.
>>>>
>>>> v2-v1:
>>>> - Removed redundant out label.
>>>> - Added a patch to fix dl_type match only.
>>>> v3-v2:
>>>> - Rebased, and more elaboration in #7 commit message.
>>> Hi.
>>>
>>> I noticed that you didn't include Acked-by tags from Harsha.
>>> Was it intentional? i.e. if there was any significant changes during
>>> rebase process?
>>>
>>> I'd like to have these patches acked by Harsha before applying, so
>>> my question if tags from v2 are valid for v3 and can I use them?
>>> Harsha?  Eli?
>>
>> No. It was by mistake. Sorry. The rebase had only one minor conflict in
>>
>> [PATCH V3 06/12] netdev-offload: Use dpif type instead of class.
>>
>> The conflict was due to previous commit [1].
>>
>> As I will have to rebase again for sure to add testpmd prints for VLANs, as [2] was merged, will it be OK I'll add the Acked-by then?
> 
> Sure.  I think it should be OK for patches without any functional
> changes.  Current patches doesn't apply starting from the second one
> due to merged patch [2], so they need a rebase anyway.
> 
> Regarding the testpmd related patch itself, I'm not sure about it.
> It might be hard to support in terms that we will need to track
> testpmd changes and it's also hard to tell for which testpmd version
> these cmdlines should be applicable.
> 
> Another point is that it's actually printing of the same information
> twice.  And also lots of extra printing code.
> Maybe, if you need these cmdlines, it's better to write a helper script
> to convert existing output to testpmd cmdlines?  Or even parse offloaded
> and non-offloaded flows from the output of dump-flows.

Anoher option is to just drop current format entirely and use format
of testpmd cmdlines instead.  Current format is not standardized and
doesn't comply with OVS flow dumps format anyway.

What do you think?

> 
>>
>> [1] 191536574e3b ("netdev-offload: Implement terse dump support").
>>
>> [2] 029273855939 ("netdev-offload-dpdk: Support offload of VLAN PUSH/POP actions.")
>>
>>>
>>> Best regards, Ilya Maximets.
>>>
>>>> Travis:
>>>> v1: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F688413350&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=PjzU%2Fiwtc7nhzTJ99oELQhi4lDyZqDwHwGuId27W4dc%3D&reserved=0
>>>> v2: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F691375847&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=SoYpbcTbkl872Mz9q%2Fg%2FrBLBOv37qCYwEXXJ%2BcXblj4%3D&reserved=0
>>>> v3: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F700534550&data=02%7C01%7Celibr%40mellanox.com%7C6f3ba17b2ba0437c191708d8182aa79e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637285918771531067&sdata=2ZIo6xz56YdJWFxuQO9iqTygcLnwPuhbGo28m3y8EC0%3D&reserved=0
>>>>
>>>>
>>>> Eli Britstein (10):
>>>>    netdev-offload-dpdk: Remove pre-validate of patterns function
>>>>    netdev-offload-dpdk: Add IPv6 pattern matching
>>>>    netdev-offload-dpdk: Support offload of set IPv6 actions
>>>>    netdev-offload-dpdk: Support partial TCP/UDP port matching
>>>>    netdev-offload-dpdk: Support offload of clone tnl_push/output actions
>>>>    netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
>>>>    dpif-netdev: Don't use zero flow mark
>>>>    dpif-netdev: Add mega ufid in flow add log
>>>>    netdev-offload-dpdk: Add testpmd log commands
>>>>    netdev-offload-dpdk: Fix Ethernet matching for type only
>>>>
>>>> Ilya Maximets (2):
>>>>    netdev: Allow storing dpif type into netdev structure.
>>>>    netdev-offload: Use dpif type instead of class.
>>>>
>>>>   Documentation/howto/dpdk.rst  |   4 +-
>>>>   NEWS                          |   3 +
>>>>   lib/dpif-netdev.c             |  26 +-
>>>>   lib/dpif-netlink.c            |  23 +-
>>>>   lib/dpif.c                    |  21 +-
>>>>   lib/netdev-offload-dpdk.c     | 649 +++++++++++++++++++++++++++++++++++-------
>>>>   lib/netdev-offload-tc.c       |   3 +-
>>>>   lib/netdev-offload.c          |  52 ++--
>>>>   lib/netdev-offload.h          |  16 +-
>>>>   lib/netdev-provider.h         |   3 +-
>>>>   lib/netdev.c                  |  16 ++
>>>>   lib/netdev.h                  |   2 +
>>>>   ofproto/ofproto-dpif-upcall.c |   5 +-
>>>>   tests/dpif-netdev.at          |  20 +-
>>>>   tests/ofproto-macros.at       |   3 +-
>>>>   15 files changed, 657 insertions(+), 189 deletions(-)
>>>>
>
Eli Britstein June 25, 2020, 5:37 a.m. UTC | #5
On 6/24/2020 5:28 PM, Ilya Maximets wrote:
> On 6/24/20 4:16 PM, Ilya Maximets wrote:
>> On 6/24/20 12:47 PM, Eli Britstein wrote:
>>> On 6/24/2020 1:37 PM, Ilya Maximets wrote:
>>>> On 6/21/20 1:19 PM, Eli Britstein wrote:
>>>>> This patch set includes additional offloads - IPv6 and VXLAN encap, and
>>>>> enhanced logging to increase debugability.
>>>>>
>>>>> Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
>>>>>                    TCP/UDP ports, set IPv6 and encap actions
>>>>>                    (clone/output).
>>>>> Patch #9:        Bug fix of partial offloads.
>>>>> Patches #10-#11: Enhance log prints for debugability.
>>>>> Patch #12:       Fix Ethernet matching for type only.
>>>>>
>>>>> v2-v1:
>>>>> - Removed redundant out label.
>>>>> - Added a patch to fix dl_type match only.
>>>>> v3-v2:
>>>>> - Rebased, and more elaboration in #7 commit message.
>>>> Hi.
>>>>
>>>> I noticed that you didn't include Acked-by tags from Harsha.
>>>> Was it intentional? i.e. if there was any significant changes during
>>>> rebase process?
>>>>
>>>> I'd like to have these patches acked by Harsha before applying, so
>>>> my question if tags from v2 are valid for v3 and can I use them?
>>>> Harsha?  Eli?
>>> No. It was by mistake. Sorry. The rebase had only one minor conflict in
>>>
>>> [PATCH V3 06/12] netdev-offload: Use dpif type instead of class.
>>>
>>> The conflict was due to previous commit [1].
>>>
>>> As I will have to rebase again for sure to add testpmd prints for VLANs, as [2] was merged, will it be OK I'll add the Acked-by then?
>> Sure.  I think it should be OK for patches without any functional
>> changes.  Current patches doesn't apply starting from the second one
>> due to merged patch [2], so they need a rebase anyway.
OK. No functional changes from v2 (or from v1, except the additional 
dl_type patch). Could you please review the rest of the series and 
gather comments before to apply and not just rebase? Thanks.
>>
>> Regarding the testpmd related patch itself, I'm not sure about it.
>> It might be hard to support in terms that we will need to track
>> testpmd changes and it's also hard to tell for which testpmd version
>> these cmdlines should be applicable.
Testpmd format doesn't change frequently, if any. Also, a specific OVS 
version implies specific DPDK version. In the case of changed format, 
we'll need to fix.
>>
>> Another point is that it's actually printing of the same information
>> twice.  And also lots of extra printing code.
>> Maybe, if you need these cmdlines, it's better to write a helper script
>> to convert existing output to testpmd cmdlines?  Or even parse offloaded
>> and non-offloaded flows from the output of dump-flows.
> Anoher option is to just drop current format entirely and use format
> of testpmd cmdlines instead.  Current format is not standardized and
> doesn't comply with OVS flow dumps format anyway.
>
> What do you think?

I kept the old format for backward compatibility, as well as some 
testpmd require more than one command. For example, "vxlan_encap" in 
"flow create", but it won't work properly in real testpmd if there was 
no "set vxlan" to define the encap header.

I'll try to do all in testpmd format and drop the current format.

>
>>> [1] 191536574e3b ("netdev-offload: Implement terse dump support").
>>>
>>> [2] 029273855939 ("netdev-offload-dpdk: Support offload of VLAN PUSH/POP actions.")
>>>
>>>> Best regards, Ilya Maximets.
>>>>
>>>>> Travis:
>>>>> v1: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F688413350&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=6iuCat3IMKBtQAMILf4uam1BioABbJ%2BG2estCEaelXg%3D&reserved=0
>>>>> v2: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F691375847&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=ybvwkH1KMHBmUZewFiM1wnz0mPlFUNu6%2FATfLTiaBTs%3D&reserved=0
>>>>> v3: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F700534550&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=LBRQrqZyYceBOuJS4A4ZbbpvLxIhz0dnsE2S46Mn5JY%3D&reserved=0
>>>>>
>>>>>
>>>>> Eli Britstein (10):
>>>>>     netdev-offload-dpdk: Remove pre-validate of patterns function
>>>>>     netdev-offload-dpdk: Add IPv6 pattern matching
>>>>>     netdev-offload-dpdk: Support offload of set IPv6 actions
>>>>>     netdev-offload-dpdk: Support partial TCP/UDP port matching
>>>>>     netdev-offload-dpdk: Support offload of clone tnl_push/output actions
>>>>>     netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
>>>>>     dpif-netdev: Don't use zero flow mark
>>>>>     dpif-netdev: Add mega ufid in flow add log
>>>>>     netdev-offload-dpdk: Add testpmd log commands
>>>>>     netdev-offload-dpdk: Fix Ethernet matching for type only
>>>>>
>>>>> Ilya Maximets (2):
>>>>>     netdev: Allow storing dpif type into netdev structure.
>>>>>     netdev-offload: Use dpif type instead of class.
>>>>>
>>>>>    Documentation/howto/dpdk.rst  |   4 +-
>>>>>    NEWS                          |   3 +
>>>>>    lib/dpif-netdev.c             |  26 +-
>>>>>    lib/dpif-netlink.c            |  23 +-
>>>>>    lib/dpif.c                    |  21 +-
>>>>>    lib/netdev-offload-dpdk.c     | 649 +++++++++++++++++++++++++++++++++++-------
>>>>>    lib/netdev-offload-tc.c       |   3 +-
>>>>>    lib/netdev-offload.c          |  52 ++--
>>>>>    lib/netdev-offload.h          |  16 +-
>>>>>    lib/netdev-provider.h         |   3 +-
>>>>>    lib/netdev.c                  |  16 ++
>>>>>    lib/netdev.h                  |   2 +
>>>>>    ofproto/ofproto-dpif-upcall.c |   5 +-
>>>>>    tests/dpif-netdev.at          |  20 +-
>>>>>    tests/ofproto-macros.at       |   3 +-
>>>>>    15 files changed, 657 insertions(+), 189 deletions(-)
>>>>>
Eli Britstein June 29, 2020, 2:56 p.m. UTC | #6
Hi

I have rebased, changed the order of commits to do testpmd format first 
and addressed the comments.

I also added the acked-by of Harsha (though a bit changed since v2). 
Thanks again.

As far as I see the only open issue is if to drop the comment about 
XL710 or not.

Please address it and if there are any other comments before I'll post v4.

Thanks,

Eli

On 6/25/2020 8:37 AM, Eli Britstein wrote:
>
> On 6/24/2020 5:28 PM, Ilya Maximets wrote:
>> On 6/24/20 4:16 PM, Ilya Maximets wrote:
>>> On 6/24/20 12:47 PM, Eli Britstein wrote:
>>>> On 6/24/2020 1:37 PM, Ilya Maximets wrote:
>>>>> On 6/21/20 1:19 PM, Eli Britstein wrote:
>>>>>> This patch set includes additional offloads - IPv6 and VXLAN 
>>>>>> encap, and
>>>>>> enhanced logging to increase debugability.
>>>>>>
>>>>>> Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
>>>>>>                    TCP/UDP ports, set IPv6 and encap actions
>>>>>>                    (clone/output).
>>>>>> Patch #9:        Bug fix of partial offloads.
>>>>>> Patches #10-#11: Enhance log prints for debugability.
>>>>>> Patch #12:       Fix Ethernet matching for type only.
>>>>>>
>>>>>> v2-v1:
>>>>>> - Removed redundant out label.
>>>>>> - Added a patch to fix dl_type match only.
>>>>>> v3-v2:
>>>>>> - Rebased, and more elaboration in #7 commit message.
>>>>> Hi.
>>>>>
>>>>> I noticed that you didn't include Acked-by tags from Harsha.
>>>>> Was it intentional? i.e. if there was any significant changes during
>>>>> rebase process?
>>>>>
>>>>> I'd like to have these patches acked by Harsha before applying, so
>>>>> my question if tags from v2 are valid for v3 and can I use them?
>>>>> Harsha?  Eli?
>>>> No. It was by mistake. Sorry. The rebase had only one minor 
>>>> conflict in
>>>>
>>>> [PATCH V3 06/12] netdev-offload: Use dpif type instead of class.
>>>>
>>>> The conflict was due to previous commit [1].
>>>>
>>>> As I will have to rebase again for sure to add testpmd prints for 
>>>> VLANs, as [2] was merged, will it be OK I'll add the Acked-by then?
>>> Sure.  I think it should be OK for patches without any functional
>>> changes.  Current patches doesn't apply starting from the second one
>>> due to merged patch [2], so they need a rebase anyway.
> OK. No functional changes from v2 (or from v1, except the additional 
> dl_type patch). Could you please review the rest of the series and 
> gather comments before to apply and not just rebase? Thanks.
>>>
>>> Regarding the testpmd related patch itself, I'm not sure about it.
>>> It might be hard to support in terms that we will need to track
>>> testpmd changes and it's also hard to tell for which testpmd version
>>> these cmdlines should be applicable.
> Testpmd format doesn't change frequently, if any. Also, a specific OVS 
> version implies specific DPDK version. In the case of changed format, 
> we'll need to fix.
>>>
>>> Another point is that it's actually printing of the same information
>>> twice.  And also lots of extra printing code.
>>> Maybe, if you need these cmdlines, it's better to write a helper script
>>> to convert existing output to testpmd cmdlines?  Or even parse 
>>> offloaded
>>> and non-offloaded flows from the output of dump-flows.
>> Anoher option is to just drop current format entirely and use format
>> of testpmd cmdlines instead.  Current format is not standardized and
>> doesn't comply with OVS flow dumps format anyway.
>>
>> What do you think?
>
> I kept the old format for backward compatibility, as well as some 
> testpmd require more than one command. For example, "vxlan_encap" in 
> "flow create", but it won't work properly in real testpmd if there was 
> no "set vxlan" to define the encap header.
>
> I'll try to do all in testpmd format and drop the current format.
>
>>
>>>> [1] 191536574e3b ("netdev-offload: Implement terse dump support").
>>>>
>>>> [2] 029273855939 ("netdev-offload-dpdk: Support offload of VLAN 
>>>> PUSH/POP actions.")
>>>>
>>>>> Best regards, Ilya Maximets.
>>>>>
>>>>>> Travis:
>>>>>> v1: 
>>>>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F688413350&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=6iuCat3IMKBtQAMILf4uam1BioABbJ%2BG2estCEaelXg%3D&reserved=0
>>>>>> v2: 
>>>>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F691375847&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=ybvwkH1KMHBmUZewFiM1wnz0mPlFUNu6%2FATfLTiaBTs%3D&reserved=0
>>>>>> v3: 
>>>>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F700534550&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=LBRQrqZyYceBOuJS4A4ZbbpvLxIhz0dnsE2S46Mn5JY%3D&reserved=0
>>>>>>
>>>>>>
>>>>>> Eli Britstein (10):
>>>>>>     netdev-offload-dpdk: Remove pre-validate of patterns function
>>>>>>     netdev-offload-dpdk: Add IPv6 pattern matching
>>>>>>     netdev-offload-dpdk: Support offload of set IPv6 actions
>>>>>>     netdev-offload-dpdk: Support partial TCP/UDP port matching
>>>>>>     netdev-offload-dpdk: Support offload of clone tnl_push/output 
>>>>>> actions
>>>>>>     netdev-offload-dpdk: Support tnl/push using vxlan encap 
>>>>>> attribute
>>>>>>     dpif-netdev: Don't use zero flow mark
>>>>>>     dpif-netdev: Add mega ufid in flow add log
>>>>>>     netdev-offload-dpdk: Add testpmd log commands
>>>>>>     netdev-offload-dpdk: Fix Ethernet matching for type only
>>>>>>
>>>>>> Ilya Maximets (2):
>>>>>>     netdev: Allow storing dpif type into netdev structure.
>>>>>>     netdev-offload: Use dpif type instead of class.
>>>>>>
>>>>>>    Documentation/howto/dpdk.rst  |   4 +-
>>>>>>    NEWS                          |   3 +
>>>>>>    lib/dpif-netdev.c             |  26 +-
>>>>>>    lib/dpif-netlink.c            |  23 +-
>>>>>>    lib/dpif.c                    |  21 +-
>>>>>>    lib/netdev-offload-dpdk.c     | 649 
>>>>>> +++++++++++++++++++++++++++++++++++-------
>>>>>>    lib/netdev-offload-tc.c       |   3 +-
>>>>>>    lib/netdev-offload.c          |  52 ++--
>>>>>>    lib/netdev-offload.h          |  16 +-
>>>>>>    lib/netdev-provider.h         |   3 +-
>>>>>>    lib/netdev.c                  |  16 ++
>>>>>>    lib/netdev.h                  |   2 +
>>>>>>    ofproto/ofproto-dpif-upcall.c |   5 +-
>>>>>>    tests/dpif-netdev.at          |  20 +-
>>>>>>    tests/ofproto-macros.at       |   3 +-
>>>>>>    15 files changed, 657 insertions(+), 189 deletions(-)
>>>>>>
Ilya Maximets June 29, 2020, 6:23 p.m. UTC | #7
On 6/29/20 4:56 PM, Eli Britstein wrote:
> Hi
> 
> I have rebased, changed the order of commits to do testpmd format first and addressed the comments.
> 
> I also added the acked-by of Harsha (though a bit changed since v2). Thanks again.
> 
> As far as I see the only open issue is if to drop the comment about XL710 or not.
> 
> Please address it and if there are any other comments before I'll post v4.

I replied about XL710 workaround.
Please, take a look.  I have no other comments for v3 right now, so, please,
send v4 once agreed on what to do with XL710 issue.

Best regards, Ilya Maximets.

> 
> Thanks,
> 
> Eli
> 
> On 6/25/2020 8:37 AM, Eli Britstein wrote:
>>
>> On 6/24/2020 5:28 PM, Ilya Maximets wrote:
>>> On 6/24/20 4:16 PM, Ilya Maximets wrote:
>>>> On 6/24/20 12:47 PM, Eli Britstein wrote:
>>>>> On 6/24/2020 1:37 PM, Ilya Maximets wrote:
>>>>>> On 6/21/20 1:19 PM, Eli Britstein wrote:
>>>>>>> This patch set includes additional offloads - IPv6 and VXLAN encap, and
>>>>>>> enhanced logging to increase debugability.
>>>>>>>
>>>>>>> Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
>>>>>>>                    TCP/UDP ports, set IPv6 and encap actions
>>>>>>>                    (clone/output).
>>>>>>> Patch #9:        Bug fix of partial offloads.
>>>>>>> Patches #10-#11: Enhance log prints for debugability.
>>>>>>> Patch #12:       Fix Ethernet matching for type only.
>>>>>>>
>>>>>>> v2-v1:
>>>>>>> - Removed redundant out label.
>>>>>>> - Added a patch to fix dl_type match only.
>>>>>>> v3-v2:
>>>>>>> - Rebased, and more elaboration in #7 commit message.
>>>>>> Hi.
>>>>>>
>>>>>> I noticed that you didn't include Acked-by tags from Harsha.
>>>>>> Was it intentional? i.e. if there was any significant changes during
>>>>>> rebase process?
>>>>>>
>>>>>> I'd like to have these patches acked by Harsha before applying, so
>>>>>> my question if tags from v2 are valid for v3 and can I use them?
>>>>>> Harsha?  Eli?
>>>>> No. It was by mistake. Sorry. The rebase had only one minor conflict in
>>>>>
>>>>> [PATCH V3 06/12] netdev-offload: Use dpif type instead of class.
>>>>>
>>>>> The conflict was due to previous commit [1].
>>>>>
>>>>> As I will have to rebase again for sure to add testpmd prints for VLANs, as [2] was merged, will it be OK I'll add the Acked-by then?
>>>> Sure.  I think it should be OK for patches without any functional
>>>> changes.  Current patches doesn't apply starting from the second one
>>>> due to merged patch [2], so they need a rebase anyway.
>> OK. No functional changes from v2 (or from v1, except the additional dl_type patch). Could you please review the rest of the series and gather comments before to apply and not just rebase? Thanks.
>>>>
>>>> Regarding the testpmd related patch itself, I'm not sure about it.
>>>> It might be hard to support in terms that we will need to track
>>>> testpmd changes and it's also hard to tell for which testpmd version
>>>> these cmdlines should be applicable.
>> Testpmd format doesn't change frequently, if any. Also, a specific OVS version implies specific DPDK version. In the case of changed format, we'll need to fix.
>>>>
>>>> Another point is that it's actually printing of the same information
>>>> twice.  And also lots of extra printing code.
>>>> Maybe, if you need these cmdlines, it's better to write a helper script
>>>> to convert existing output to testpmd cmdlines?  Or even parse offloaded
>>>> and non-offloaded flows from the output of dump-flows.
>>> Anoher option is to just drop current format entirely and use format
>>> of testpmd cmdlines instead.  Current format is not standardized and
>>> doesn't comply with OVS flow dumps format anyway.
>>>
>>> What do you think?
>>
>> I kept the old format for backward compatibility, as well as some testpmd require more than one command. For example, "vxlan_encap" in "flow create", but it won't work properly in real testpmd if there was no "set vxlan" to define the encap header.
>>
>> I'll try to do all in testpmd format and drop the current format.
>>
>>>
>>>>> [1] 191536574e3b ("netdev-offload: Implement terse dump support").
>>>>>
>>>>> [2] 029273855939 ("netdev-offload-dpdk: Support offload of VLAN PUSH/POP actions.")
>>>>>
>>>>>> Best regards, Ilya Maximets.
>>>>>>
>>>>>>> Travis:
>>>>>>> v1: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F688413350&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=6iuCat3IMKBtQAMILf4uam1BioABbJ%2BG2estCEaelXg%3D&reserved=0
>>>>>>> v2: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F691375847&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=ybvwkH1KMHBmUZewFiM1wnz0mPlFUNu6%2FATfLTiaBTs%3D&reserved=0
>>>>>>> v3: https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F700534550&data=02%7C01%7Celibr%40mellanox.com%7Cdaa680f281e44d447bc708d8184ad3d9%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637286056953162106&sdata=LBRQrqZyYceBOuJS4A4ZbbpvLxIhz0dnsE2S46Mn5JY%3D&reserved=0
>>>>>>>
>>>>>>>
>>>>>>> Eli Britstein (10):
>>>>>>>     netdev-offload-dpdk: Remove pre-validate of patterns function
>>>>>>>     netdev-offload-dpdk: Add IPv6 pattern matching
>>>>>>>     netdev-offload-dpdk: Support offload of set IPv6 actions
>>>>>>>     netdev-offload-dpdk: Support partial TCP/UDP port matching
>>>>>>>     netdev-offload-dpdk: Support offload of clone tnl_push/output actions
>>>>>>>     netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
>>>>>>>     dpif-netdev: Don't use zero flow mark
>>>>>>>     dpif-netdev: Add mega ufid in flow add log
>>>>>>>     netdev-offload-dpdk: Add testpmd log commands
>>>>>>>     netdev-offload-dpdk: Fix Ethernet matching for type only
>>>>>>>
>>>>>>> Ilya Maximets (2):
>>>>>>>     netdev: Allow storing dpif type into netdev structure.
>>>>>>>     netdev-offload: Use dpif type instead of class.
>>>>>>>
>>>>>>>    Documentation/howto/dpdk.rst  |   4 +-
>>>>>>>    NEWS                          |   3 +
>>>>>>>    lib/dpif-netdev.c             |  26 +-
>>>>>>>    lib/dpif-netlink.c            |  23 +-
>>>>>>>    lib/dpif.c                    |  21 +-
>>>>>>>    lib/netdev-offload-dpdk.c     | 649 +++++++++++++++++++++++++++++++++++-------
>>>>>>>    lib/netdev-offload-tc.c       |   3 +-
>>>>>>>    lib/netdev-offload.c          |  52 ++--
>>>>>>>    lib/netdev-offload.h          |  16 +-
>>>>>>>    lib/netdev-provider.h         |   3 +-
>>>>>>>    lib/netdev.c                  |  16 ++
>>>>>>>    lib/netdev.h                  |   2 +
>>>>>>>    ofproto/ofproto-dpif-upcall.c |   5 +-
>>>>>>>    tests/dpif-netdev.at          |  20 +-
>>>>>>>    tests/ofproto-macros.at       |   3 +-
>>>>>>>    15 files changed, 657 insertions(+), 189 deletions(-)
>>>>>>>