On Thu, Jul 04, 2013 at 12:34:46PM +0900, Joe Stringer wrote:
> On Sat, Jun 29, 2013 at 8:27 AM, Ben Pfaff <[email protected]> wrote:
> > @@ -1514,7 +1593,17 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
> >
> > /* Translate the message. */
> > fm->priority = ntohs(ofm->priority);
> > - if (ofm->command == OFPFC_ADD) {
> > + if (oh->version == OFP11_VERSION
> > + && (ofm->command == OFPFC_MODIFY ||
> > + ofm->command == OFPFC_MODIFY_STRICT)
> > + && ofm->cookie_mask == htonll(0)) {
> > + /* In OpenFlow 1.1 only, a "modify" or "modify-strict" that
> > does
> > + * not match on the cookie is treated as an "add" if there is
> > no
> > + * match. */
> > + fm->cookie = htonll(0);
> > + fm->cookie_mask = htonll(0);
> > + fm->new_cookie = ofm->cookie;
> > + } else if (ofm->command == OFPFC_ADD) {
> > fm->cookie = htonll(0);
> > fm->cookie_mask = htonll(0);
> > fm->new_cookie = ofm->cookie;
>
> Is this flowmod init code duplicated just to keep the OF1.1 comment
> separate? The first two if statements here are combined in the similar
> encode_flow_mod() code.
I went back and forth on modifying this code a few times. I think in
the last iteration I must not have noticed the conditional statements
were the same.
> Otherwise, the tests seem to check the OF1.1-specific behaviour, and
> they pass, so it looks pretty good to me.
Thanks.
I'm going to apply this soon with the following folded in to eliminate
duplicate code:
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 0878e3f..8921592 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1593,20 +1593,17 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
/* Translate the message. */
fm->priority = ntohs(ofm->priority);
- if (oh->version == OFP11_VERSION
- && (ofm->command == OFPFC_MODIFY ||
- ofm->command == OFPFC_MODIFY_STRICT)
- && ofm->cookie_mask == htonll(0)) {
+ if (ofm->command == OFPFC_ADD
+ || (oh->version == OFP11_VERSION
+ && (ofm->command == OFPFC_MODIFY ||
+ ofm->command == OFPFC_MODIFY_STRICT)
+ && ofm->cookie_mask == htonll(0))) {
/* In OpenFlow 1.1 only, a "modify" or "modify-strict" that does
* not match on the cookie is treated as an "add" if there is no
* match. */
fm->cookie = htonll(0);
fm->cookie_mask = htonll(0);
fm->new_cookie = ofm->cookie;
- } else if (ofm->command == OFPFC_ADD) {
- fm->cookie = htonll(0);
- fm->cookie_mask = htonll(0);
- fm->new_cookie = ofm->cookie;
} else {
fm->cookie = ofm->cookie;
fm->cookie_mask = ofm->cookie_mask;
@@ -1658,7 +1655,6 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
fm->cookie = htonll(0);
fm->cookie_mask = htonll(0);
fm->new_cookie = ofm->cookie;
- fm->modify_cookie = fm->new_cookie != htonll(UINT64_MAX);
fm->idle_timeout = ntohs(ofm->idle_timeout);
fm->hard_timeout = ntohs(ofm->hard_timeout);
fm->buffer_id = ntohl(ofm->buffer_id);
@@ -1690,7 +1686,6 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
}
fm->priority = ntohs(nfm->priority);
fm->new_cookie = nfm->cookie;
- fm->modify_cookie = fm->new_cookie != htonll(UINT64_MAX);
fm->idle_timeout = ntohs(nfm->idle_timeout);
fm->hard_timeout = ntohs(nfm->hard_timeout);
fm->buffer_id = ntohl(nfm->buffer_id);
@@ -1712,6 +1707,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
: OFPERR_OFPFMFC_TABLE_FULL);
}
+ fm->modify_cookie = fm->new_cookie != htonll(UINT64_MAX);
if (protocol & OFPUTIL_P_TID) {
fm->command = command & 0xff;
fm->table_id = command >> 8;
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
