[ovs-dev] ofproto.at: Fix minor typo in test.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 tests/ofproto.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Sun, Jul 09, 2017 at 11:51:40PM -0700, Justin Pettit wrote:
> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> ---
>  tests/ofproto.at | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 9e6acfad653d..c7ea31a77ce9 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -3430,7 +3430,7 @@ udp,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172
>      ovs-ofctl -O OpenFlow13 add-flow br0 send_flow_rem,actions=group:1234
>      ovs-ofctl -O OpenFlow13 --strict del-groups br0 group_id=1234
>      if test X"$1" = X"OFPRR_DELETE"; then shift;
> -        echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=gropu_delete table_id=0"
> +        echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=group_delete table_id=0"

Why didn't this cause a test failure?  (Is this code never actually
executed?)

> On Jul 14, 2017, at 12:59 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Sun, Jul 09, 2017 at 11:51:40PM -0700, Justin Pettit wrote:
>> Signed-off-by: Justin Pettit <jpettit@ovn.org>
>> ---
>> tests/ofproto.at | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tests/ofproto.at b/tests/ofproto.at
>> index 9e6acfad653d..c7ea31a77ce9 100644
>> --- a/tests/ofproto.at
>> +++ b/tests/ofproto.at
>> @@ -3430,7 +3430,7 @@ udp,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172
>>     ovs-ofctl -O OpenFlow13 add-flow br0 send_flow_rem,actions=group:1234
>>     ovs-ofctl -O OpenFlow13 --strict del-groups br0 group_id=1234
>>     if test X"$1" = X"OFPRR_DELETE"; then shift;
>> -        echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=gropu_delete table_id=0"
>> +        echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=group_delete table_id=0"
> 
> Why didn't this cause a test failure?  (Is this code never actually
> executed?)

Yes, there were multiple errors in the test that prevented it from running.  It's easy to correct them, but our support for OpenFlow 1.3 doesn't include generating a flow removed due to the "group delete".  The OpenFlow 1.3 specification defines such a reason but we seem to break things into "OpenFlow 1.4+" or "Open Flow 1.0". so we don't generate it for 1.3.

Do you think we should add support for 1.3 or just remove that bit of test code?

Here's the commit that introduced that test:

	https://github.com/openvswitch/ovs/commit/cc40d06bf5ca3

--Justin

On Sat, Jul 15, 2017 at 03:02:18PM -0700, Justin Pettit wrote:
> 
> > On Jul 14, 2017, at 12:59 PM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> > On Sun, Jul 09, 2017 at 11:51:40PM -0700, Justin Pettit wrote:
> >> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> >> ---
> >> tests/ofproto.at | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/tests/ofproto.at b/tests/ofproto.at
> >> index 9e6acfad653d..c7ea31a77ce9 100644
> >> --- a/tests/ofproto.at
> >> +++ b/tests/ofproto.at
> >> @@ -3430,7 +3430,7 @@ udp,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172
> >>     ovs-ofctl -O OpenFlow13 add-flow br0 send_flow_rem,actions=group:1234
> >>     ovs-ofctl -O OpenFlow13 --strict del-groups br0 group_id=1234
> >>     if test X"$1" = X"OFPRR_DELETE"; then shift;
> >> -        echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=gropu_delete table_id=0"
> >> +        echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=group_delete table_id=0"
> > 
> > Why didn't this cause a test failure?  (Is this code never actually
> > executed?)
> 
> Yes, there were multiple errors in the test that prevented it from running.  It's easy to correct them, but our support for OpenFlow 1.3 doesn't include generating a flow removed due to the "group delete".  The OpenFlow 1.3 specification defines such a reason but we seem to break things into "OpenFlow 1.4+" or "Open Flow 1.0". so we don't generate it for 1.3.
> 
> Do you think we should add support for 1.3 or just remove that bit of test code?

I think it would be better to support OF1.3.  I don't know of a reason
that it's particularly hard.

> On Jul 15, 2017, at 3:21 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Sat, Jul 15, 2017 at 03:02:18PM -0700, Justin Pettit wrote:
>> 
>>> On Jul 14, 2017, at 12:59 PM, Ben Pfaff <blp@ovn.org> wrote:
>>> 
>>> On Sun, Jul 09, 2017 at 11:51:40PM -0700, Justin Pettit wrote:
>>>> Signed-off-by: Justin Pettit <jpettit@ovn.org>
>>>> ---
>>>> tests/ofproto.at | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> 
>>>> diff --git a/tests/ofproto.at b/tests/ofproto.at
>>>> index 9e6acfad653d..c7ea31a77ce9 100644
>>>> --- a/tests/ofproto.at
>>>> +++ b/tests/ofproto.at
>>>> @@ -3430,7 +3430,7 @@ udp,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172
>>>>    ovs-ofctl -O OpenFlow13 add-flow br0 send_flow_rem,actions=group:1234
>>>>    ovs-ofctl -O OpenFlow13 --strict del-groups br0 group_id=1234
>>>>    if test X"$1" = X"OFPRR_DELETE"; then shift;
>>>> -        echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=gropu_delete table_id=0"
>>>> +        echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=group_delete table_id=0"
>>> 
>>> Why didn't this cause a test failure?  (Is this code never actually
>>> executed?)
>> 
>> Yes, there were multiple errors in the test that prevented it from running.  It's easy to correct them, but our support for OpenFlow 1.3 doesn't include generating a flow removed due to the "group delete".  The OpenFlow 1.3 specification defines such a reason but we seem to break things into "OpenFlow 1.4+" or "Open Flow 1.0". so we don't generate it for 1.3.
>> 
>> Do you think we should add support for 1.3 or just remove that bit of test code?
> 
> I think it would be better to support OF1.3.  I don't know of a reason
> that it's particularly hard.

I sent out a patch to add support:

	https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335724.html

--Justin