Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-04-06 Thread Roi Dayan 



On 05/04/2017 10:08, Joe Stringer wrote:

On 4 April 2017 at 21:18, Roi Dayan  wrote:



On 04/04/2017 23:53, Joe Stringer wrote:


On 3 April 2017 at 10:53, Joe Stringer  wrote:


On 3 April 2017 at 03:27, Roi Dayan  wrote:




On 29/03/2017 20:13, Joe Stringer wrote:



On 29 March 2017 at 04:50, Roi Dayan  wrote:





On 23/03/2017 09:01, Joe Stringer wrote:




I ran the make check-offloads tests on a recent net-next kernel and
it
failed, output was not as expected:

../../tests/system-offloaded-traffic.at:54
: ovs-appctl dpctl/dump-flows
|
grep "eth_type(0x0800)" | sed -e



's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
--- - 2017-03-22 16:43:37.598689692 -0700
+++



/home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
2017-03-22 16:43:37.595628000 -0700
@@ -1,3 +1,3 @@
-in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
-in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
+in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output
+in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output



Hi Joe,

can you tell me what kernel you used here?
maybe tc offloads were not supported and there was a fallback to OVS
dp.




I believe that it was a snapshot of net-next relatively recently,
01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
again with latest net-next? Or do you think there may be some
userspace dependency the test relies on?



I installed net-next kernel and make check-offloads pass for me.
The last commit I'm on is
397df70 Merge branch '40GbE' of
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
last tag is 4.11-rc3+
I'm thinking maybe the test fails for you from something else like a
second
openvswitch process running already?



I don't think that was the case, but let me try again with your latest
series and the above commit.



I saw the same behaviour with upstream net-next 397df7092a15. My host
is Ubuntu 14.04 with this kernel.

I thought it might be because I'm not running any of your hardware and
I assumed that the testsuite doesn't require hardware to run. Looking
at the test it seems that assumption was wrong, but when I tried to
configure the tc-policy to skip_hw with the following modification,
the OVSDB change didn't seem to propagate into OVS (there were no log
messages about changing the tc-policy):

diff --git a/tests/system-offloaded-traffic.at
b/tests/system-offloaded-traffic.at
index 7aec8a3f430e..3ddf23a939a8 100644
--- a/tests/system-offloaded-traffic.at
+++ b/tests/system-offloaded-traffic.at
@@ -40,6 +40,7 @@ AT_SETUP([offloads - ping between two ports -
offloads enabled])
OVS_TRAFFIC_VSWITCHD_START()

AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:tc-policy="skip_hw"])
AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])

ADD_NAMESPACES(at_ns0, at_ns1)

---

Looking again, my kernel config had CLS_FLOWER disabled so that's
probably what caused the issue. My ovs-vswitchd log from the test is
below.

2017-04-04T20:41:50.737Z|1|vlog|INFO|opened log file

/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/ovs-vswitchd.log
2017-04-04T20:41:50.737Z|2|ovs_numa|INFO|Discovered 2 CPU cores on
NUMA node 0
2017-04-04T20:41:50.737Z|3|ovs_numa|INFO|Discovered 1 NUMA nodes
and 2 CPU cores

2017-04-04T20:41:50.738Z|4|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
connecting...

2017-04-04T20:41:50.738Z|5|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
connected
2017-04-04T20:41:50.743Z|6|bridge|INFO|ovs-vswitchd (Open vSwitch)
2.7.90
2017-04-04T20:41:50.757Z|7|ofproto_dpif|INFO|system@ovs-system:
Datapath supports recirculation
2017-04-04T20:41:50.757Z|8|ofproto_dpif|INFO|system@ovs-system:
MPLS label stack length probed as 1
2017-04-04T20:41:50.757Z|9|ofproto_dpif|INFO|system@ovs-system:
Datapath supports truncate action
2017-04-04T20:41:50.757Z|00010|ofproto_dpif|INFO|system@ovs-system:
Datapath supports unique flow ids
2017-04-04T20:41:50.757Z|00011|ofproto_dpif|INFO|system@ovs-system:
Datapath does not support clone action
2017-04-04T20:41:50.757Z|00012|ofproto_dpif|INFO|system@ovs-system:
Max sample nesting level probed as 10
2017-04-04T20:41:50.757Z|00013|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_state
2017-04-04T20:41:50.757Z|00014|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_zone
2017-04-04T20:41:50.757Z|00015|ofproto_dpif|INFO|system@ovs-system:
Datapath supports

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-04-06 Thread Joe Stringer 
On 5 April 2017 at 00:38, Roi Dayan  wrote:
>
>
> On 05/04/2017 07:18, Roi Dayan wrote:
>>
>>
>>
>> On 04/04/2017 23:53, Joe Stringer wrote:
>>>
>>> On 3 April 2017 at 10:53, Joe Stringer  wrote:

 On 3 April 2017 at 03:27, Roi Dayan  wrote:
>
>
>
> On 29/03/2017 20:13, Joe Stringer wrote:
>>
>>
>> On 29 March 2017 at 04:50, Roi Dayan  wrote:
>>>
>>>
>>>
>>>
>>> On 23/03/2017 09:01, Joe Stringer wrote:



 I ran the make check-offloads tests on a recent net-next kernel
 and it
 failed, output was not as expected:

 ../../tests/system-offloaded-traffic.at:54
 : ovs-appctl
 dpctl/dump-flows |
 grep "eth_type(0x0800)" | sed -e



 's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac

 s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
 --- - 2017-03-22 16:43:37.598689692 -0700
 +++



 /home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout

 2017-03-22 16:43:37.595628000 -0700
 @@ -1,3 +1,3 @@
 -in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
 used:0.001s, actions:output
 -in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
 used:0.001s, actions:output
 +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
 bytes:882, used:0.001s, actions:output
 +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
 bytes:882, used:0.001s, actions:output

>>>
>>> Hi Joe,
>>>
>>> can you tell me what kernel you used here?
>>> maybe tc offloads were not supported and there was a fallback to
>>> OVS dp.
>>
>>
>>
>> I believe that it was a snapshot of net-next relatively recently,
>> 01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
>> again with latest net-next? Or do you think there may be some
>> userspace dependency the test relies on?
>>
>
> I installed net-next kernel and make check-offloads pass for me.
> The last commit I'm on is
> 397df70 Merge branch '40GbE' of
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
> last tag is 4.11-rc3+
> I'm thinking maybe the test fails for you from something else like a
> second
> openvswitch process running already?


 I don't think that was the case, but let me try again with your latest
 series and the above commit.
>>>
>>>
>>> I saw the same behaviour with upstream net-next 397df7092a15. My host
>>> is Ubuntu 14.04 with this kernel.
>>>
>>> I thought it might be because I'm not running any of your hardware and
>>> I assumed that the testsuite doesn't require hardware to run. Looking
>>> at the test it seems that assumption was wrong, but when I tried to
>>> configure the tc-policy to skip_hw with the following modification,
>>> the OVSDB change didn't seem to propagate into OVS (there were no log
>>> messages about changing the tc-policy):
>>>
>>> diff --git a/tests/system-offloaded-traffic.at
>>> b/tests/system-offloaded-traffic.at
>>> index 7aec8a3f430e..3ddf23a939a8 100644
>>> --- a/tests/system-offloaded-traffic.at
>>> +++ b/tests/system-offloaded-traffic.at
>>> @@ -40,6 +40,7 @@ AT_SETUP([offloads - ping between two ports -
>>> offloads enabled])
>>> OVS_TRAFFIC_VSWITCHD_START()
>>>
>>> AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
>>> +AT_CHECK([ovs-vsctl set Open_vSwitch .
>>> other_config:tc-policy="skip_hw"])
>>> AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>>>
>>> ADD_NAMESPACES(at_ns0, at_ns1)
>>>
>>> ---
>>>
>>> Looking again, my kernel config had CLS_FLOWER disabled so that's
>>> probably what caused the issue. My ovs-vswitchd log from the test is
>>> below.
>>>
>>> 2017-04-04T20:41:50.737Z|1|vlog|INFO|opened log file
>>>
>>> /home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/ovs-vswitchd.log
>>>
>>> 2017-04-04T20:41:50.737Z|2|ovs_numa|INFO|Discovered 2 CPU cores on
>>> NUMA node 0
>>> 2017-04-04T20:41:50.737Z|3|ovs_numa|INFO|Discovered 1 NUMA nodes
>>> and 2 CPU cores
>>>
>>> 2017-04-04T20:41:50.738Z|4|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
>>>
>>> connecting...
>>>
>>> 2017-04-04T20:41:50.738Z|5|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
>>>
>>> connected
>>> 2017-04-04T20:41:50.743Z|6|bridge|INFO|ovs-vswitchd (Open vSwitch)
>>> 2.7.90
>>> 2017-04-04T20:41:50.757Z|7|ofproto_dpif|INFO|system@ovs-system:
>>> Datapath supports recirculation
>>>

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-04-05 Thread Joe Stringer 
On 4 April 2017 at 21:18, Roi Dayan  wrote:
>
>
> On 04/04/2017 23:53, Joe Stringer wrote:
>>
>> On 3 April 2017 at 10:53, Joe Stringer  wrote:
>>>
>>> On 3 April 2017 at 03:27, Roi Dayan  wrote:



 On 29/03/2017 20:13, Joe Stringer wrote:
>
>
> On 29 March 2017 at 04:50, Roi Dayan  wrote:
>>
>>
>>
>>
>> On 23/03/2017 09:01, Joe Stringer wrote:
>>>
>>>
>>>
>>> I ran the make check-offloads tests on a recent net-next kernel and
>>> it
>>> failed, output was not as expected:
>>>
>>> ../../tests/system-offloaded-traffic.at:54
>>> : ovs-appctl dpctl/dump-flows
>>> |
>>> grep "eth_type(0x0800)" | sed -e
>>>
>>>
>>>
>>> 's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
>>> s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
>>> --- - 2017-03-22 16:43:37.598689692 -0700
>>> +++
>>>
>>>
>>>
>>> /home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
>>> 2017-03-22 16:43:37.595628000 -0700
>>> @@ -1,3 +1,3 @@
>>> -in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
>>> used:0.001s, actions:output
>>> -in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
>>> used:0.001s, actions:output
>>> +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
>>> bytes:882, used:0.001s, actions:output
>>> +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
>>> bytes:882, used:0.001s, actions:output
>>>
>>
>> Hi Joe,
>>
>> can you tell me what kernel you used here?
>> maybe tc offloads were not supported and there was a fallback to OVS
>> dp.
>
>
>
> I believe that it was a snapshot of net-next relatively recently,
> 01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
> again with latest net-next? Or do you think there may be some
> userspace dependency the test relies on?
>

 I installed net-next kernel and make check-offloads pass for me.
 The last commit I'm on is
 397df70 Merge branch '40GbE' of
 git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
 last tag is 4.11-rc3+
 I'm thinking maybe the test fails for you from something else like a
 second
 openvswitch process running already?
>>>
>>>
>>> I don't think that was the case, but let me try again with your latest
>>> series and the above commit.
>>
>>
>> I saw the same behaviour with upstream net-next 397df7092a15. My host
>> is Ubuntu 14.04 with this kernel.
>>
>> I thought it might be because I'm not running any of your hardware and
>> I assumed that the testsuite doesn't require hardware to run. Looking
>> at the test it seems that assumption was wrong, but when I tried to
>> configure the tc-policy to skip_hw with the following modification,
>> the OVSDB change didn't seem to propagate into OVS (there were no log
>> messages about changing the tc-policy):
>>
>> diff --git a/tests/system-offloaded-traffic.at
>> b/tests/system-offloaded-traffic.at
>> index 7aec8a3f430e..3ddf23a939a8 100644
>> --- a/tests/system-offloaded-traffic.at
>> +++ b/tests/system-offloaded-traffic.at
>> @@ -40,6 +40,7 @@ AT_SETUP([offloads - ping between two ports -
>> offloads enabled])
>> OVS_TRAFFIC_VSWITCHD_START()
>>
>> AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
>> +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:tc-policy="skip_hw"])
>> AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>>
>> ADD_NAMESPACES(at_ns0, at_ns1)
>>
>> ---
>>
>> Looking again, my kernel config had CLS_FLOWER disabled so that's
>> probably what caused the issue. My ovs-vswitchd log from the test is
>> below.
>>
>> 2017-04-04T20:41:50.737Z|1|vlog|INFO|opened log file
>>
>> /home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/ovs-vswitchd.log
>> 2017-04-04T20:41:50.737Z|2|ovs_numa|INFO|Discovered 2 CPU cores on
>> NUMA node 0
>> 2017-04-04T20:41:50.737Z|3|ovs_numa|INFO|Discovered 1 NUMA nodes
>> and 2 CPU cores
>>
>> 2017-04-04T20:41:50.738Z|4|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
>> connecting...
>>
>> 2017-04-04T20:41:50.738Z|5|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
>> connected
>> 2017-04-04T20:41:50.743Z|6|bridge|INFO|ovs-vswitchd (Open vSwitch)
>> 2.7.90
>> 2017-04-04T20:41:50.757Z|7|ofproto_dpif|INFO|system@ovs-system:
>> Datapath supports recirculation
>> 2017-04-04T20:41:50.757Z|8|ofproto_dpif|INFO|system@ovs-system:
>> MPLS label stack length probed as 1
>> 2017-04-04T20:41:50.757Z|9|ofproto_dpif|INFO|system@ovs-system:
>> Datapath supports truncate action
>>

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-04-04 Thread Roi Dayan 



On 04/04/2017 23:53, Joe Stringer wrote:

On 3 April 2017 at 10:53, Joe Stringer  wrote:

On 3 April 2017 at 03:27, Roi Dayan  wrote:



On 29/03/2017 20:13, Joe Stringer wrote:


On 29 March 2017 at 04:50, Roi Dayan  wrote:




On 23/03/2017 09:01, Joe Stringer wrote:



I ran the make check-offloads tests on a recent net-next kernel and it
failed, output was not as expected:

../../tests/system-offloaded-traffic.at:54
: ovs-appctl dpctl/dump-flows |
grep "eth_type(0x0800)" | sed -e


's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
--- - 2017-03-22 16:43:37.598689692 -0700
+++


/home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
2017-03-22 16:43:37.595628000 -0700
@@ -1,3 +1,3 @@
-in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
-in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
+in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output
+in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output



Hi Joe,

can you tell me what kernel you used here?
maybe tc offloads were not supported and there was a fallback to OVS dp.



I believe that it was a snapshot of net-next relatively recently,
01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
again with latest net-next? Or do you think there may be some
userspace dependency the test relies on?



I installed net-next kernel and make check-offloads pass for me.
The last commit I'm on is
397df70 Merge branch '40GbE' of
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
last tag is 4.11-rc3+
I'm thinking maybe the test fails for you from something else like a second
openvswitch process running already?


I don't think that was the case, but let me try again with your latest
series and the above commit.


I saw the same behaviour with upstream net-next 397df7092a15. My host
is Ubuntu 14.04 with this kernel.

I thought it might be because I'm not running any of your hardware and
I assumed that the testsuite doesn't require hardware to run. Looking
at the test it seems that assumption was wrong, but when I tried to
configure the tc-policy to skip_hw with the following modification,
the OVSDB change didn't seem to propagate into OVS (there were no log
messages about changing the tc-policy):

diff --git a/tests/system-offloaded-traffic.at
b/tests/system-offloaded-traffic.at
index 7aec8a3f430e..3ddf23a939a8 100644
--- a/tests/system-offloaded-traffic.at
+++ b/tests/system-offloaded-traffic.at
@@ -40,6 +40,7 @@ AT_SETUP([offloads - ping between two ports -
offloads enabled])
OVS_TRAFFIC_VSWITCHD_START()

AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:tc-policy="skip_hw"])
AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])

ADD_NAMESPACES(at_ns0, at_ns1)

---

Looking again, my kernel config had CLS_FLOWER disabled so that's
probably what caused the issue. My ovs-vswitchd log from the test is
below.

2017-04-04T20:41:50.737Z|1|vlog|INFO|opened log file
/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/ovs-vswitchd.log
2017-04-04T20:41:50.737Z|2|ovs_numa|INFO|Discovered 2 CPU cores on
NUMA node 0
2017-04-04T20:41:50.737Z|3|ovs_numa|INFO|Discovered 1 NUMA nodes
and 2 CPU cores
2017-04-04T20:41:50.738Z|4|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
connecting...
2017-04-04T20:41:50.738Z|5|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
connected
2017-04-04T20:41:50.743Z|6|bridge|INFO|ovs-vswitchd (Open vSwitch) 2.7.90
2017-04-04T20:41:50.757Z|7|ofproto_dpif|INFO|system@ovs-system:
Datapath supports recirculation
2017-04-04T20:41:50.757Z|8|ofproto_dpif|INFO|system@ovs-system:
MPLS label stack length probed as 1
2017-04-04T20:41:50.757Z|9|ofproto_dpif|INFO|system@ovs-system:
Datapath supports truncate action
2017-04-04T20:41:50.757Z|00010|ofproto_dpif|INFO|system@ovs-system:
Datapath supports unique flow ids
2017-04-04T20:41:50.757Z|00011|ofproto_dpif|INFO|system@ovs-system:
Datapath does not support clone action
2017-04-04T20:41:50.757Z|00012|ofproto_dpif|INFO|system@ovs-system:
Max sample nesting level probed as 10
2017-04-04T20:41:50.757Z|00013|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_state
2017-04-04T20:41:50.757Z|00014|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_zone
2017-04-04T20:41:50.757Z|00015|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_mark
2017-04-04T20:41:50.757Z|00016|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_label

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-04-04 Thread Joe Stringer 
On 3 April 2017 at 10:53, Joe Stringer  wrote:
> On 3 April 2017 at 03:27, Roi Dayan  wrote:
>>
>>
>> On 29/03/2017 20:13, Joe Stringer wrote:
>>>
>>> On 29 March 2017 at 04:50, Roi Dayan  wrote:



 On 23/03/2017 09:01, Joe Stringer wrote:
>
>
> I ran the make check-offloads tests on a recent net-next kernel and it
> failed, output was not as expected:
>
> ../../tests/system-offloaded-traffic.at:54
> : ovs-appctl dpctl/dump-flows |
> grep "eth_type(0x0800)" | sed -e
>
>
> 's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
> s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
> --- - 2017-03-22 16:43:37.598689692 -0700
> +++
>
>
> /home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
> 2017-03-22 16:43:37.595628000 -0700
> @@ -1,3 +1,3 @@
> -in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
> used:0.001s, actions:output
> -in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
> used:0.001s, actions:output
> +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
> bytes:882, used:0.001s, actions:output
> +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
> bytes:882, used:0.001s, actions:output
>

 Hi Joe,

 can you tell me what kernel you used here?
 maybe tc offloads were not supported and there was a fallback to OVS dp.
>>>
>>>
>>> I believe that it was a snapshot of net-next relatively recently,
>>> 01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
>>> again with latest net-next? Or do you think there may be some
>>> userspace dependency the test relies on?
>>>
>>
>> I installed net-next kernel and make check-offloads pass for me.
>> The last commit I'm on is
>> 397df70 Merge branch '40GbE' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
>> last tag is 4.11-rc3+
>> I'm thinking maybe the test fails for you from something else like a second
>> openvswitch process running already?
>
> I don't think that was the case, but let me try again with your latest
> series and the above commit.

I saw the same behaviour with upstream net-next 397df7092a15. My host
is Ubuntu 14.04 with this kernel.

I thought it might be because I'm not running any of your hardware and
I assumed that the testsuite doesn't require hardware to run. Looking
at the test it seems that assumption was wrong, but when I tried to
configure the tc-policy to skip_hw with the following modification,
the OVSDB change didn't seem to propagate into OVS (there were no log
messages about changing the tc-policy):

diff --git a/tests/system-offloaded-traffic.at
b/tests/system-offloaded-traffic.at
index 7aec8a3f430e..3ddf23a939a8 100644
--- a/tests/system-offloaded-traffic.at
+++ b/tests/system-offloaded-traffic.at
@@ -40,6 +40,7 @@ AT_SETUP([offloads - ping between two ports -
offloads enabled])
OVS_TRAFFIC_VSWITCHD_START()

AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:tc-policy="skip_hw"])
AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])

ADD_NAMESPACES(at_ns0, at_ns1)

---

Looking again, my kernel config had CLS_FLOWER disabled so that's
probably what caused the issue. My ovs-vswitchd log from the test is
below.

2017-04-04T20:41:50.737Z|1|vlog|INFO|opened log file
/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/ovs-vswitchd.log
2017-04-04T20:41:50.737Z|2|ovs_numa|INFO|Discovered 2 CPU cores on
NUMA node 0
2017-04-04T20:41:50.737Z|3|ovs_numa|INFO|Discovered 1 NUMA nodes
and 2 CPU cores
2017-04-04T20:41:50.738Z|4|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
connecting...
2017-04-04T20:41:50.738Z|5|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
connected
2017-04-04T20:41:50.743Z|6|bridge|INFO|ovs-vswitchd (Open vSwitch) 2.7.90
2017-04-04T20:41:50.757Z|7|ofproto_dpif|INFO|system@ovs-system:
Datapath supports recirculation
2017-04-04T20:41:50.757Z|8|ofproto_dpif|INFO|system@ovs-system:
MPLS label stack length probed as 1
2017-04-04T20:41:50.757Z|9|ofproto_dpif|INFO|system@ovs-system:
Datapath supports truncate action
2017-04-04T20:41:50.757Z|00010|ofproto_dpif|INFO|system@ovs-system:
Datapath supports unique flow ids
2017-04-04T20:41:50.757Z|00011|ofproto_dpif|INFO|system@ovs-system:
Datapath does not support clone action
2017-04-04T20:41:50.757Z|00012|ofproto_dpif|INFO|system@ovs-system:
Max sample nesting level probed as 10
2017-04-04T20:41:50.757Z|00013|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_state
2017-04-04T20:41:50.757Z|00014|ofproto_dpif|INFO|system@ovs-system:
Datapath supports

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-04-03 Thread Joe Stringer 
On 3 April 2017 at 03:27, Roi Dayan  wrote:
>
>
> On 29/03/2017 20:13, Joe Stringer wrote:
>>
>> On 29 March 2017 at 04:50, Roi Dayan  wrote:
>>>
>>>
>>>
>>> On 23/03/2017 09:01, Joe Stringer wrote:


 I ran the make check-offloads tests on a recent net-next kernel and it
 failed, output was not as expected:

 ../../tests/system-offloaded-traffic.at:54
 : ovs-appctl dpctl/dump-flows |
 grep "eth_type(0x0800)" | sed -e


 's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
 s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
 --- - 2017-03-22 16:43:37.598689692 -0700
 +++


 /home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
 2017-03-22 16:43:37.595628000 -0700
 @@ -1,3 +1,3 @@
 -in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
 used:0.001s, actions:output
 -in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
 used:0.001s, actions:output
 +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
 bytes:882, used:0.001s, actions:output
 +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
 bytes:882, used:0.001s, actions:output

>>>
>>> Hi Joe,
>>>
>>> can you tell me what kernel you used here?
>>> maybe tc offloads were not supported and there was a fallback to OVS dp.
>>
>>
>> I believe that it was a snapshot of net-next relatively recently,
>> 01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
>> again with latest net-next? Or do you think there may be some
>> userspace dependency the test relies on?
>>
>
> I installed net-next kernel and make check-offloads pass for me.
> The last commit I'm on is
> 397df70 Merge branch '40GbE' of
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
> last tag is 4.11-rc3+
> I'm thinking maybe the test fails for you from something else like a second
> openvswitch process running already?

I don't think that was the case, but let me try again with your latest
series and the above commit.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-04-03 Thread Roi Dayan 



On 29/03/2017 20:13, Joe Stringer wrote:

On 29 March 2017 at 04:50, Roi Dayan  wrote:



On 23/03/2017 09:01, Joe Stringer wrote:


I ran the make check-offloads tests on a recent net-next kernel and it
failed, output was not as expected:

../../tests/system-offloaded-traffic.at:54
: ovs-appctl dpctl/dump-flows |
grep "eth_type(0x0800)" | sed -e

's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
--- - 2017-03-22 16:43:37.598689692 -0700
+++

/home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
2017-03-22 16:43:37.595628000 -0700
@@ -1,3 +1,3 @@
-in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
-in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
+in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output
+in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output



Hi Joe,

can you tell me what kernel you used here?
maybe tc offloads were not supported and there was a fallback to OVS dp.


I believe that it was a snapshot of net-next relatively recently,
01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
again with latest net-next? Or do you think there may be some
userspace dependency the test relies on?



I installed net-next kernel and make check-offloads pass for me.
The last commit I'm on is
397df70 Merge branch '40GbE' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue

last tag is 4.11-rc3+
I'm thinking maybe the test fails for you from something else like a 
second openvswitch process running already?


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-03-29 Thread Joe Stringer 
On 29 March 2017 at 04:50, Roi Dayan  wrote:
>
>
> On 23/03/2017 09:01, Joe Stringer wrote:
>>
>> I ran the make check-offloads tests on a recent net-next kernel and it
>> failed, output was not as expected:
>>
>> ../../tests/system-offloaded-traffic.at:54
>> : ovs-appctl dpctl/dump-flows |
>> grep "eth_type(0x0800)" | sed -e
>>
>> 's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
>> s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
>> --- - 2017-03-22 16:43:37.598689692 -0700
>> +++
>>
>> /home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
>> 2017-03-22 16:43:37.595628000 -0700
>> @@ -1,3 +1,3 @@
>> -in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
>> used:0.001s, actions:output
>> -in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
>> used:0.001s, actions:output
>> +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
>> bytes:882, used:0.001s, actions:output
>> +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
>> bytes:882, used:0.001s, actions:output
>>
>
> Hi Joe,
>
> can you tell me what kernel you used here?
> maybe tc offloads were not supported and there was a fallback to OVS dp.

I believe that it was a snapshot of net-next relatively recently,
01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
again with latest net-next? Or do you think there may be some
userspace dependency the test relies on?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-03-29 Thread Roi Dayan 



On 23/03/2017 09:01, Joe Stringer wrote:

I ran the make check-offloads tests on a recent net-next kernel and it
failed, output was not as expected:

../../tests/system-offloaded-traffic.at:54
: ovs-appctl dpctl/dump-flows |
grep "eth_type(0x0800)" | sed -e
's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
--- - 2017-03-22 16:43:37.598689692 -0700
+++
/home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
2017-03-22 16:43:37.595628000 -0700
@@ -1,3 +1,3 @@
-in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
-in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
+in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output
+in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output



Hi Joe,

can you tell me what kernel you used here?
maybe tc offloads were not supported and there was a fallback to OVS dp.

Thanks,
Roi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-03-23 Thread Simon Horman 
On Wed, Mar 22, 2017 at 01:10:33PM +0200, Roi Dayan wrote:
> This patch series introduces rule offload functionality to dpif-netlink
> via netdev ports new flow offloading API. The user can specify whether to
> enable rule offloading or not via OVS configuration. Netdev providers
> are able to implement netdev flow offload API in order to offload rules.
> 
> This patch series also implements one offload scheme for netdev-linux,
> using TC flower classifier, which was chosen because its sort of natural
> to state OVS DP rules for this classifier. However, the code can be
> extended to support other classifiers such as U32, eBPF, etc which support
> offload as well.
> 
> The use-case we are currently addressing is the newly sriov switchdev mode
> in the Linux kernel which was introduced in version 4.8 [1][2].
> This series was tested against sriov vfs vports representors of the
> Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.

Hi Roi,

as I stated with regards to v4 I think it would be very nice to get this
merged. Although it has a limited feature-set - compared to OvS - I believe
it provides an excellent base for further work and as the patch-set is
already large it makes sense to me not to further expand it by adding more
features to it.  Furthermore I think the patch-set should have minimal
impact on those not interested in programming flows into hardware.

Modulo resolving the build error that Joe reported:

Reviewed-by: Simon Horman 


With regards to the features provided by this patch-set I
think that some use-cases include:

* L2 forwarding
* Flow based forwarding to/from VMs
* Overlay network using VLAN or VXLAN
* Load Balancing - directed by user-installed flows
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-03-23 Thread Jiri Pirko 
Thu, Mar 23, 2017 at 08:01:35AM CET, j...@ovn.org wrote:
>On 22 March 2017 at 04:10, Roi Dayan  wrote:
>> This patch series introduces rule offload functionality to dpif-netlink
>> via netdev ports new flow offloading API. The user can specify whether to
>> enable rule offloading or not via OVS configuration. Netdev providers
>> are able to implement netdev flow offload API in order to offload rules.
>>
>> This patch series also implements one offload scheme for netdev-linux,
>> using TC flower classifier, which was chosen because its sort of natural
>> to state OVS DP rules for this classifier. However, the code can be
>> extended to support other classifiers such as U32, eBPF, etc which support
>> offload as well.
>>
>> The use-case we are currently addressing is the newly sriov switchdev mode
>> in the Linux kernel which was introduced in version 4.8 [1][2].
>> This series was tested against sriov vfs vports representors of the
>> Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.
>>
>>
>> V4->V5:
>> - Fix compat
>> - Fix VXLAN IPv6 tunnel matching
>> - Fix order of actions in dump flows
>> - Update ovs-dpctl man page about the addtion of type to dump-flows
>>
>> Travis
>> https://travis-ci.org/roidayan/ovs/builds/213735371
>> AppVeyor
>> https://ci.appveyor.com/project/roidayan/ovs/build/1.0.18
>
>Hi Roi,
>
>This series does not compile on the OVS master:
>https://travis-ci.org/joestringer/openvswitch/builds/214039943
>
>I ran the make check-offloads tests on a recent net-next kernel and it
>failed, output was not as expected:
>
>../../tests/system-offloaded-traffic.at:54: ovs-appctl dpctl/dump-flows |
>grep "eth_type(0x0800)" | sed -e
>'s/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
>s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
>--- - 2017-03-22 16:43:37.598689692 -0700
>+++
>/home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
>2017-03-22 16:43:37.595628000 -0700
>@@ -1,3 +1,3 @@
>-in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756, used:0.001s,
>actions:output
>-in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756, used:0.001s,
>actions:output
>+in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:882,
>used:0.001s, actions:output
>+in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:882,
>used:0.001s, actions:output
>
>Did you end up allowing other_config:hw-offload to be configured at
>runtime? The tests seem to be assuming this, but the documentation still
>says that OVS must be restarted to enable offloads.
>
>The testsuite is an encouraging sign. I can only imagine that it's not
>attempting to comprehensively cover OVS flow translation to the tc flower
>API, because of its simplicity. Presumably you intend to improve this over
>time though?
>
>At a high level, the functionality isn't particularly compelling at this
>stage. OVS has a huge level of programmability, and the limits applied in
>this patch restrict the offloads to quite a small subset of use cases. That
>said, I believe I've mentioned off-list a couple of times that if there is
>broad interest in the OVS community regarding this series, and that
>interest is shown (for instance, by people trying out the patches,
>providing review, etc) then that's an encouraging sign that this feature is
>providing useful functionality---and therefore should be merged.

Just for the record, I'm using this patchset as well. For testing offload
of OVS on top or rack Mellanox Spectum switches - mlxsw driver in upstream
kernel. We use TCAM to offload OVS rules to.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-03-23 Thread Roi Dayan 



On 23/03/2017 09:01, Joe Stringer wrote:

On 22 March 2017 at 04:10, Roi Dayan > wrote:

This patch series introduces rule offload functionality to dpif-netlink
via netdev ports new flow offloading API. The user can specify whether to
enable rule offloading or not via OVS configuration. Netdev providers
are able to implement netdev flow offload API in order to offload rules.

This patch series also implements one offload scheme for netdev-linux,
using TC flower classifier, which was chosen because its sort of natural
to state OVS DP rules for this classifier. However, the code can be
extended to support other classifiers such as U32, eBPF, etc which support
offload as well.

The use-case we are currently addressing is the newly sriov switchdev mode
in the Linux kernel which was introduced in version 4.8 [1][2].
This series was tested against sriov vfs vports representors of the
Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.


V4->V5:
- Fix compat
- Fix VXLAN IPv6 tunnel matching
- Fix order of actions in dump flows
- Update ovs-dpctl man page about the addtion of type to dump-flows

Travis
https://travis-ci.org/roidayan/ovs/builds/213735371
AppVeyor
https://ci.appveyor.com/project/roidayan/ovs/build/1.0.18


Hi Roi,

This series does not compile on the OVS master:
https://travis-ci.org/joestringer/openvswitch/builds/214039943


Hi Joe,

right. didn't rebase over latest master. We'll check it out.



I ran the make check-offloads tests on a recent net-next kernel and it
failed, output was not as expected:

../../tests/system-offloaded-traffic.at:54
: ovs-appctl dpctl/dump-flows |
grep "eth_type(0x0800)" | sed -e
's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
--- - 2017-03-22 16:43:37.598689692 -0700
+++
/home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
2017-03-22 16:43:37.595628000 -0700
@@ -1,3 +1,3 @@
-in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
-in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
+in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output
+in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output



we'll check it.


Did you end up allowing other_config:hw-offload to be configured at
runtime? The tests seem to be assuming this, but the documentation still
says that OVS must be restarted to enable offloads.


It's possible to change to true at runtime but not back to false because 
of the possible mixup that could be between offloaded rules and 
non-offloaded rules that we aren't handling currently. e.g. should 
probably flush all offloaded rules so they will be added back non-offloaded.

Because of this we left the comment in the doc.



The testsuite is an encouraging sign. I can only imagine that it's not
attempting to comprehensively cover OVS flow translation to the tc
flower API, because of its simplicity. Presumably you intend to improve
this over time though?


of course. this is a very simple example for a test. this example test 
should probably be improved as well.




At a high level, the functionality isn't particularly compelling at this
stage. OVS has a huge level of programmability, and the limits applied
in this patch restrict the offloads to quite a small subset of use
cases. That said, I believe I've mentioned off-list a couple of times
that if there is broad interest in the OVS community regarding this
series, and that interest is shown (for instance, by people trying out
the patches, providing review, etc) then that's an encouraging sign that
this feature is providing useful functionality---and therefore should be
merged.

For reference, the kinds of use cases that I think could make this
series more compelling include stateful connection tracking and tunnel
termination, in particular with TLV support (for Geneve, NSH).

The biggest concern from a maintainership point of view is the potential
burden on reviews and support that this may introduce. Your approach of
making this disabled by default and configurable in the database makes
this easier, as this restricts the potential impact on regular users.
I've heard arguments on both sides of this, but effectively if over time
the usefulness is low and maintenance burden is high, we'd be inclined
to remove this again; rather, if it becomes more useful over time and we
get active involvement from the community for ongoing patch review and
assisting users that run into trouble, then that would make the
maintainers job easier.



thanks for the feedback,
Roi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-03-23 Thread Joe Stringer 
On 22 March 2017 at 04:10, Roi Dayan  wrote:
> This patch series introduces rule offload functionality to dpif-netlink
> via netdev ports new flow offloading API. The user can specify whether to
> enable rule offloading or not via OVS configuration. Netdev providers
> are able to implement netdev flow offload API in order to offload rules.
>
> This patch series also implements one offload scheme for netdev-linux,
> using TC flower classifier, which was chosen because its sort of natural
> to state OVS DP rules for this classifier. However, the code can be
> extended to support other classifiers such as U32, eBPF, etc which support
> offload as well.
>
> The use-case we are currently addressing is the newly sriov switchdev mode
> in the Linux kernel which was introduced in version 4.8 [1][2].
> This series was tested against sriov vfs vports representors of the
> Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.
>
>
> V4->V5:
> - Fix compat
> - Fix VXLAN IPv6 tunnel matching
> - Fix order of actions in dump flows
> - Update ovs-dpctl man page about the addtion of type to dump-flows
>
> Travis
> https://travis-ci.org/roidayan/ovs/builds/213735371
> AppVeyor
> https://ci.appveyor.com/project/roidayan/ovs/build/1.0.18

Hi Roi,

This series does not compile on the OVS master:
https://travis-ci.org/joestringer/openvswitch/builds/214039943

I ran the make check-offloads tests on a recent net-next kernel and it
failed, output was not as expected:

../../tests/system-offloaded-traffic.at:54: ovs-appctl dpctl/dump-flows |
grep "eth_type(0x0800)" | sed -e
's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
--- - 2017-03-22 16:43:37.598689692 -0700
+++
/home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
2017-03-22 16:43:37.595628000 -0700
@@ -1,3 +1,3 @@
-in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756, used:0.001s,
actions:output
-in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756, used:0.001s,
actions:output
+in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:882,
used:0.001s, actions:output
+in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:882,
used:0.001s, actions:output

Did you end up allowing other_config:hw-offload to be configured at
runtime? The tests seem to be assuming this, but the documentation still
says that OVS must be restarted to enable offloads.

The testsuite is an encouraging sign. I can only imagine that it's not
attempting to comprehensively cover OVS flow translation to the tc flower
API, because of its simplicity. Presumably you intend to improve this over
time though?

At a high level, the functionality isn't particularly compelling at this
stage. OVS has a huge level of programmability, and the limits applied in
this patch restrict the offloads to quite a small subset of use cases. That
said, I believe I've mentioned off-list a couple of times that if there is
broad interest in the OVS community regarding this series, and that
interest is shown (for instance, by people trying out the patches,
providing review, etc) then that's an encouraging sign that this feature is
providing useful functionality---and therefore should be merged.

For reference, the kinds of use cases that I think could make this series
more compelling include stateful connection tracking and tunnel
termination, in particular with TLV support (for Geneve, NSH).

The biggest concern from a maintainership point of view is the potential
burden on reviews and support that this may introduce. Your approach of
making this disabled by default and configurable in the database makes this
easier, as this restricts the potential impact on regular users. I've heard
arguments on both sides of this, but effectively if over time the
usefulness is low and maintenance burden is high, we'd be inclined to
remove this again; rather, if it becomes more useful over time and we get
active involvement from the community for ongoing patch review and
assisting users that run into trouble, then that would make the maintainers
job easier.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

2017-03-22 Thread Roi Dayan 
This patch series introduces rule offload functionality to dpif-netlink
via netdev ports new flow offloading API. The user can specify whether to
enable rule offloading or not via OVS configuration. Netdev providers
are able to implement netdev flow offload API in order to offload rules.

This patch series also implements one offload scheme for netdev-linux,
using TC flower classifier, which was chosen because its sort of natural
to state OVS DP rules for this classifier. However, the code can be
extended to support other classifiers such as U32, eBPF, etc which support
offload as well.

The use-case we are currently addressing is the newly sriov switchdev mode
in the Linux kernel which was introduced in version 4.8 [1][2].
This series was tested against sriov vfs vports representors of the
Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.


V4->V5:
- Fix compat
- Fix VXLAN IPv6 tunnel matching
- Fix order of actions in dump flows
- Update ovs-dpctl man page about the addtion of type to dump-flows

Travis
https://travis-ci.org/roidayan/ovs/builds/213735371
AppVeyor
https://ci.appveyor.com/project/roidayan/ovs/build/1.0.18

V3->V4:
- Move declarations to the right commit with implementation
- Fix tc_get_flower flow return false success 
- Fix memory leaks - not releasing tc_transact replies
- Fix travis failure for OSX compilation
- Fix loop in dpif_netlink_flow_dump_next
- Fix declared default value for tc-policy in vswitch.xml
- Refactor loop in netdev_tc_flow_dump_next
- Add missing error checks in parse_flow_put
- Fix handling modify request where old rule is in hw and new
  rule is not supported and needs to be in sw.
- Use 2 hashmaps instead of 1 for faster reverse lookup of ufid from tc
- Init ports when enabling hw-offload after OVS is running

TODO: Fix breaking of datapath compilation
  Fix testsuite failures

Travis
https://travis-ci.org/roidayan/ovs/builds/210549325
AppVeyor
https://ci.appveyor.com/project/roidayan/ovs/build/1.0.15

V2->V3:
- Code styling fixes
- Bug fixes
- Using already available macros/functions to match current OVS code
- Refactored code according to V2 review
- Replaced bool option skip-hw for string option tc-policy
- Added hw offload tests using policy skip_hw
- Fixed most compatability compiling issues
- Travis
https://travis-ci.org/roidayan/ovs/builds/199610124
- AppVeyor
https://ci.appveyor.com/project/roidayan/ovs/build/1.0.14
- Fixed compiling with DPDK enabled

TODO:
- need to fix datapath compiling issues found in travis after adding tc
  compatability headers
- need to fix failing test cases because of get_ifindex

V1->V2:
- Added generic netdev flow offloads API.
- Implemented relevant flow API in netdev-linux (and netdev-vport).
- Added a other_config hw-offload option to enable offloading (defaults to 
false).
- Fixed coding style to conform with OVS.
- Policy removed for now. (Will be discussed how best implemented later).


Thanks,
Paul & Roi


Paul Blakey (24):
  tc: Add tc flower interface
  netdev: Adding a new netdev api to be used for offloading flows
  other-config: Add hw-offload switch to control netdev flow offloading
  other-config: Add tc-policy switch to control tc flower flag
  dpif: Save added ports in a port map for netdev flow api use
  dpif-netlink: Flush added ports using netdev flow api
  netdev-tc-offloads: Implement netdev flow flush using tc interface
  dpif-netlink: Dump netdevs flows on flow dump
  netdev-tc-offloads: Add ufid to tc/netdev map
  netdev-tc-offloads: Implement netdev flow dump api using tc interface
  dpif-netlink: Use netdev flow put api to insert a flow
  netdev-tc-offloads: Add flower mask to priority map
  netdev-tc-offloads: Implement netdev flow put using tc interface
  dpif-netlink: Use netdev flow del api to delete a flow
  netdev-tc-offloads: Implement netdev flow del using tc interface
  dpif-netlink: Use netdev flow get api to query a flow
  netdev-tc-offloads: Implement flow get using tc interface
  netdev-linux: Disallow setting policing when configured with hw
offload
  netdev-vport: Use common offloads interface
  netdev-tc-offloads: Add ingress on netdev flow api init
  dpctl: Add an option to dump only certain kinds of flows
  tests: Add system-offloads-testsuite
  compat: Add tc compatibility headers for old kernels
  netdev: Init flow api on already added ports on offload enable

 acinclude.m4 |   26 +
 configure.ac |1 +
 include/automake.mk  |1 +
 include/linux/automake.mk|4 +
 include/linux/pkt_cls.h  |  165 +
 include/linux/tc_act/tc_tunnel_key.h |   46 ++
 include/linux/tc_act/tc_vlan.h   |   40 ++
 lib/automake.mk  |4