| Message ID | 20151125075700.GD2142@vergenet.net |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Nov 25, 2015 at 04:57:02PM +0900, Simon Horman wrote: > On Wed, Nov 25, 2015 at 04:22:28PM +0900, Simon Horman wrote: > > On Tue, Sep 29, 2015 at 11:50:40AM -0700, Ben Pfaff wrote: > > > The OpenFlow specification implies that every flag is part of the flow > > > state, even though that isn't really meaningful for OFPFF_CHECK_OVERLAP > > > or OFPFF_RESET_COUNTS. This commit adds them to the flow state (reported > > > in flow stats replies). > > > > > > Found by OFTest. > > > > > > Signed-off-by: Ben Pfaff <blp@nicira.com> > > > > Well that is the silliest thing I have seen all day. > > Do you know of any plan to resolve this in future Open Flow versions? > > > > Reviewed-by: Simon Horman <simon.horman@netronome.com> > > Oops, I spoke slightly to soon. > > I think the test you add is slightly misleading as it seems to me > that flags have been exposed since OF1.3 rather than OF1.4. > > The following incremental change to that test, though verbose, > illustrates that. Simon, thank you. I feel like this makes you a co-author, are you willing to give me a Signed-off-by for it?
On Sat, Nov 28, 2015 at 11:56:46AM -0800, Ben Pfaff wrote: > On Wed, Nov 25, 2015 at 04:57:02PM +0900, Simon Horman wrote: > > On Wed, Nov 25, 2015 at 04:22:28PM +0900, Simon Horman wrote: > > > On Tue, Sep 29, 2015 at 11:50:40AM -0700, Ben Pfaff wrote: > > > > The OpenFlow specification implies that every flag is part of the flow > > > > state, even though that isn't really meaningful for OFPFF_CHECK_OVERLAP > > > > or OFPFF_RESET_COUNTS. This commit adds them to the flow state (reported > > > > in flow stats replies). > > > > > > > > Found by OFTest. > > > > > > > > Signed-off-by: Ben Pfaff <blp@nicira.com> > > > > > > Well that is the silliest thing I have seen all day. > > > Do you know of any plan to resolve this in future Open Flow versions? > > > > > > Reviewed-by: Simon Horman <simon.horman@netronome.com> > > > > Oops, I spoke slightly to soon. > > > > I think the test you add is slightly misleading as it seems to me > > that flags have been exposed since OF1.3 rather than OF1.4. > > > > The following incremental change to that test, though verbose, > > illustrates that. > > Simon, thank you. > > I feel like this makes you a co-author, are you willing to give me a > Signed-off-by for it? Sure, here it is: Signed-off-by: Simon Horman <simon.horman@netronome.com>
On Mon, Nov 30, 2015 at 08:39:24AM +0900, Simon Horman wrote: > On Sat, Nov 28, 2015 at 11:56:46AM -0800, Ben Pfaff wrote: > > On Wed, Nov 25, 2015 at 04:57:02PM +0900, Simon Horman wrote: > > > On Wed, Nov 25, 2015 at 04:22:28PM +0900, Simon Horman wrote: > > > > On Tue, Sep 29, 2015 at 11:50:40AM -0700, Ben Pfaff wrote: > > > > > The OpenFlow specification implies that every flag is part of the flow > > > > > state, even though that isn't really meaningful for OFPFF_CHECK_OVERLAP > > > > > or OFPFF_RESET_COUNTS. This commit adds them to the flow state (reported > > > > > in flow stats replies). > > > > > > > > > > Found by OFTest. > > > > > > > > > > Signed-off-by: Ben Pfaff <blp@nicira.com> > > > > > > > > Well that is the silliest thing I have seen all day. > > > > Do you know of any plan to resolve this in future Open Flow versions? > > > > > > > > Reviewed-by: Simon Horman <simon.horman@netronome.com> > > > > > > Oops, I spoke slightly to soon. > > > > > > I think the test you add is slightly misleading as it seems to me > > > that flags have been exposed since OF1.3 rather than OF1.4. > > > > > > The following incremental change to that test, though verbose, > > > illustrates that. > > > > Simon, thank you. > > > > I feel like this makes you a co-author, are you willing to give me a > > Signed-off-by for it? > > Sure, here it is: > > Signed-off-by: Simon Horman <simon.horman@netronome.com> Thanks! I applied this to master.
diff --git a/tests/ofproto.at b/tests/ofproto.at index bc7a53c7a5e7..30b1cfa90e4d 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -842,16 +842,32 @@ AT_CLEANUP AT_SETUP([ofproto - add-flow and flags]) OVS_VSWITCHD_START AT_CHECK([ovs-ofctl -F openflow10 add-flow br0 check_overlap,in_port=1,actions=drop]) -# Prior to OF1.4, flow dumps didn't include a "flags" field. +# Prior to OF1.3, flow dumps didn't include a "flags" field. AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip], [0], [dnl OFPST_FLOW reply: in_port=1 actions=drop ]) -# OF1.4 makes the flags visible. +AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [dnl +OFPST_FLOW reply (OF1.1): + in_port=1 actions=drop +]) +AT_CHECK([ovs-ofctl -O OpenFlow12 dump-flows br0 | ofctl_strip], [0], [dnl +OFPST_FLOW reply (OF1.2): + in_port=1 actions=drop +]) +# OF1.3 makes the flags visible. +AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 | ofctl_strip], [0], [dnl +OFPST_FLOW reply (OF1.3): + check_overlap reset_counts in_port=1 actions=drop +]) AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip], [0], [dnl OFPST_FLOW reply (OF1.4): check_overlap reset_counts in_port=1 actions=drop ]) +AT_CHECK([ovs-ofctl -O OpenFlow15 dump-flows br0 | ofctl_strip], [0], [dnl +OFPST_FLOW reply (OF1.5): + check_overlap reset_counts in_port=1 actions=drop +]) OVS_VSWITCHD_STOP AT_CLEANUP