On Mon, Nov 19, 2012 at 05:11:21PM -0800, Ben Pfaff wrote:
> On Tue, Nov 20, 2012 at 09:39:56AM +0900, Simon Horman wrote:
> > On Mon, Nov 19, 2012 at 10:13:22AM -0800, Ben Pfaff wrote:
> > > On Mon, Nov 19, 2012 at 02:59:29PM +0900, Simon Horman wrote:
> > > > Allow only OpenFlow 12 as a flow dump protocol for OpenFlow12.
> > > > Allow OpenFlow10 and NXM as flow dump protocols for other OpenFlow
> > > > versions.
> > > > This only changes the behaviour prior to this patch in the case of
> > > > OpenFlow12
> > > > which seemed broken as an attempt would be made to set NXM as the
> > > > flow dump format using an OpenFlow 1.0 message even though an OpenFlow
> > > > 1.2
> > > > connection had been negotiated..
> > > >
> > > > Signed-off-by: Simon Horman <ho...@verge.net.au>
> > >
> > > I think that this is no longer needed with the changes that I made a few
> > > patches back. set_protocol_for_flow_dump() will try to set the protocol
> > > with try_set_protocol(), which in turn will call
> > > ofputil_encode_set_protocol(), which in turn will see that the versions
> > > are incompatible, with:
> > >
> > > cur_version = ofputil_protocol_to_ofp_version(current);
> > > want_version = ofputil_protocol_to_ofp_version(want);
> > > if (cur_version != want_version) {
> > > *next = current;
> > > return NULL;
> > > }
> > >
> > > which makes try_set_protocol() return that it failed to set that
> > > protocol, which makes set_protocol_for_flow_dump() go on to the next
> > > one.
> >
> > I'm somewhat unsure how you envisage this working.
> >
> > Without this patch the code you refer to does indeed see that
> > all versions are incompatible. But in the end it runs out of options
> > and produces a fatal error.
>
> Do we just need to add OFPUTIL_P_OF12_OXM to
> ofputil_flow_dump_protocols[], at the beginning?
Sorry for missing that. Yes that seems to be sufficient.
An updated patch is below, which should apply to master.
I don't think there are any other changes but
would you like me to re-spin the series anyway?
----------------------------------------------------------------
From: Simon Horman <ho...@verge.net.au>
ofp-util: Flow Dump Protocol for OpenFlow 12
Allow only OpenFlow 12 as a flow dump protocol.
The implementation of set_protocol_for_flow_dump ensures that
this will only be selected if an OpenFlow12 connection has
been negotiated.
Signed-off-by: Simon Horman <ho...@verge.net.au>
---
v7
* As suggested by Ben Pfaff
- Vastly simplify the implementation, it is now
sufficient to simply add OFPUTIL_P_OF12_OXM to the beginning
of ofputil_flow_dump_protocols[].
v6
* No change
v5
* Rebase
v4
* Rebase
---
lib/ofp-util.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1a9d611..e335ff8 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -588,6 +588,7 @@ static const struct proto_abbrev proto_abbrevs[] = {
#define N_PROTO_ABBREVS ARRAY_SIZE(proto_abbrevs)
enum ofputil_protocol ofputil_flow_dump_protocols[] = {
+ OFPUTIL_P_OF12_OXM,
OFPUTIL_P_OF10_NXM,
OFPUTIL_P_OF10_STD,
};
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
