On Mon, Nov 12, 2012 at 09:57:26AM -0800, Ben Pfaff wrote: > On Wed, Nov 07, 2012 at 05:03:03PM +0900, Simon Horman wrote: > > This enables the use of the OpenFlow 12 flow format. > > > > Signed-off-by: Simon Horman <ho...@verge.net.au> > > Based on "grep", I'm nervous that we're missing important handling for > OFPUTIL_P_OF12 in a number of places. For example, it looks like > ofputil_usable_protocols(), ofputil_encode_set_protocol(), and > ofputil_flow_mod_usable_protocols() don't have any provisions for > OF1.2. Do any of the later patches in this series fix those up? If so, > then I guess we should try to fold them into this patch. If not, then > we should probably fix them.
I believe that the version of ofputil_encode_set_protocol() in the master branch already handles OFPUTIL_P_OF12. And I believe that folding in a subsequent patch resolves the rest of the problems that you raise. ---------------------------------------------------------------- ofp-util: Allow use of OpenFlow 12 flow format This enables the use of the OpenFlow 12 flow format. Signed-off-by: Simon Horman <ho...@verge.net.au> --- v5 * As suggested by Ben Pfaff: - Fold "ofp-util: Open Flow 1.1 and 1.2 flow format capabilities" into this patch v4 * Rebase --- lib/ofp-util.c | 32 ++++++++++++++++---------------- lib/ofp-util.h | 7 +++++-- tests/learn.at | 2 +- tests/ovs-ofctl.at | 2 +- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/lib/ofp-util.c b/lib/ofp-util.c index dea4aea..32244ce 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -726,7 +726,7 @@ ofputil_protocol_to_string(enum ofputil_protocol protocol) return "OpenFlow10+table_id"; case OFPUTIL_P_OF12: - return NULL; + return "OpenFlow12"; } /* Check abbreviations. */ @@ -972,27 +972,27 @@ ofputil_usable_protocols(const struct match *match) /* NXM and OF1.1+ supports bitwise matching on ethernet addresses. */ if (!eth_mask_is_exact(wc->masks.dl_src) && !eth_addr_is_zero(wc->masks.dl_src)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_NXM_ANY_OR_OF11_PLUS; } if (!eth_mask_is_exact(wc->masks.dl_dst) && !eth_addr_is_zero(wc->masks.dl_dst)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_NXM_ANY_OR_OF11_PLUS; } /* NXM and OF1.1+ support matching metadata. */ if (wc->masks.metadata != htonll(0)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_NXM_ANY_OR_OF11_PLUS; } - /* Only NXM supports matching ARP hardware addresses. */ + /* NXM and OF1.2 support matching ARP hardware addresses. */ if (!eth_addr_is_zero(wc->masks.arp_sha) || !eth_addr_is_zero(wc->masks.arp_tha)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_NXM_ANY_OR_OF12; } - /* Only NXM supports matching IPv6 traffic. */ + /* NXM and OF1.2 support matching IPv6 traffic. */ if (match->flow.dl_type == htons(ETH_TYPE_IPV6)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_NXM_ANY_OR_OF12; } /* Only NXM supports matching registers. */ @@ -1010,14 +1010,14 @@ ofputil_usable_protocols(const struct match *match) return OFPUTIL_P_NXM_ANY; } - /* Only NXM supports matching IPv6 flow label. */ + /* NXM and OF1.2 support matching IPv6 flow label. */ if (wc->masks.ipv6_label) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_NXM_ANY_OR_OF12; } - /* Only NXM supports matching IP ECN bits. */ + /* NXM and OF1.2 support matching IP ECN bits. */ if (wc->masks.nw_tos & IP_ECN_MASK) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_NXM_ANY_OR_OF12; } /* Only NXM supports matching IP TTL/hop limit. */ @@ -1025,9 +1025,9 @@ ofputil_usable_protocols(const struct match *match) return OFPUTIL_P_NXM_ANY; } - /* Only NXM supports non-CIDR IPv4 address masks. */ + /* NXM and OF1.1+ support non-CIDR IPv4 address masks. */ if (!ip_is_cidr(wc->masks.nw_src) || !ip_is_cidr(wc->masks.nw_dst)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_NXM_ANY_OR_OF11_PLUS; } /* Only NXM supports bitwise matching on transport port. */ @@ -1597,9 +1597,9 @@ ofputil_flow_mod_usable_protocols(const struct ofputil_flow_mod *fms, usable_protocols &= OFPUTIL_P_TID; } - /* Matching of the cookie is only supported through NXM. */ + /* Matching of the cookie is only supporte through NXM and OF1.1+. */ if (fm->cookie_mask != htonll(0)) { - usable_protocols &= OFPUTIL_P_NXM_ANY; + usable_protocols &= OFPUTIL_P_NXM_ANY_OR_OF11_PLUS; } } assert(usable_protocols); diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 4bd5a00..fa5df68 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -72,10 +72,13 @@ enum ofputil_protocol { OFPUTIL_P_OF12 = 1 << 4, /* OpenFlow 1.2 flow format. */ /* All protocols. */ -#define OFPUTIL_P_ANY (OFPUTIL_P_OF10_ANY | OFPUTIL_P_NXM_ANY) +#define OFPUTIL_P_ANY (OFPUTIL_P_OF10_ANY | OFPUTIL_P_NXM_ANY | OFPUTIL_P_OF12) /* Protocols in which a specific table may be specified in flow_mods. */ -#define OFPUTIL_P_TID (OFPUTIL_P_OF10_TID | OFPUTIL_P_NXM_TID) +#define OFPUTIL_P_TID (OFPUTIL_P_OF10_TID | OFPUTIL_P_NXM_TID | OFPUTIL_P_OF12) + +#define OFPUTIL_P_NXM_ANY_OR_OF12 (OFPUTIL_P_NXM_ANY | OFPUTIL_P_OF12) +#define OFPUTIL_P_NXM_ANY_OR_OF11_PLUS OFPUTIL_P_NXM_ANY_OR_OF12 }; /* Protocols to use for flow dumps, from most to least preferred. */ diff --git a/tests/learn.at b/tests/learn.at index adb67a4..78d0b59 100644 --- a/tests/learn.at +++ b/tests/learn.at @@ -24,7 +24,7 @@ table=0 actions=learn(table=1,hard_timeout=10, NXM_OF_VLAN_TCI[0..11],output:NXM table=1 priority=0 actions=flood ]]) AT_CHECK([ovs-ofctl parse-flows flows.txt], [0], -[[usable protocols: OpenFlow10+table_id,NXM+table_id +[[usable protocols: OpenFlow10+table_id,NXM+table_id,OpenFlow12 chosen protocol: OpenFlow10+table_id OFPT_FLOW_MOD (xid=0x1): ADD table:255 actions=learn(table=1,in_port=99,NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],load:NXM_OF_IN_PORT[]->NXM_NX_REG1[16..31]) OFPT_FLOW_MOD (xid=0x2): ADD table:255 actions=learn(table=1,NXM_OF_VLAN_TCI[0..11],NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],output:NXM_OF_IN_PORT[]) diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 958982f..d8017af 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -1866,7 +1866,7 @@ AT_CHECK([ovs-ofctl -F openflow10 add-flow dummy tun_id=123,actions=drop], [1], [], [ovs-ofctl: none of the usable flow formats (NXM) is among the allowed flow formats (OpenFlow10) ]) AT_CHECK([ovs-ofctl -F openflow10 add-flow dummy metadata=123,actions=drop], - [1], [], [ovs-ofctl: none of the usable flow formats (NXM) is among the allowed flow formats (OpenFlow10) + [1], [], [ovs-ofctl: none of the usable flow formats (NXM,OpenFlow12) is among the allowed flow formats (OpenFlow10) ]) AT_CLEANUP -- 1.7.10.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev