[ovs-dev] [PATCH] ofp-util: Use table_id in OF1.2 Flow Remove Messages

Ben Pfaff blp at nicira.com
Mon Oct 1 17:01:15 UTC 2012


On Mon, Oct 01, 2012 at 04:51:49PM +0900, Simon Horman wrote:
> Previously this field was ignored on decode and
> set to zero on encode
> 
> Also add OFPTT_ALL and OFPTT_MAX and use OFPTT_ALL in the one location
> where it seems appropriate.
> 
> Signed-off-by: Simon Horman <horms at verge.net.au>

Thanks for adding this support.

Using a constant like OFPTT_ALL is a good idea, but it is more widely
useful than your patch shows; try "grep"ping for 255 and 0xff and you
will see more instances of its use for table IDs.  I'd rather try to
use it everywhere applicable, instead of just in one place, so I
dropped that change.

I think that it's a good idea to try to distinguish table 0 from
"message didn't have a table ID", so I changed the "default" value for
decode to 255, which isn't otherwise a valid value.

I also added a comment on the table_id member to reflect that.

I've appended what I'm applying.

Thank you!

--8<--------------------------cut here-------------------------->8--

From: Simon Horman <horms at verge.net.au>
Date: Mon, 1 Oct 2012 16:51:49 +0900
Subject: [PATCH] ofp-util: Use table_id in OF1.1 and OF1.2 Flow Remove Messages

Previously this field was ignored on decode and
set to zero on encode

Signed-off-by: Simon Horman <horms at verge.net.au>
[blp at nicira.com changed "missing" value, removed OFPTT_ALL]
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-print.c    |    4 ++++
 lib/ofp-util.c     |    6 ++++--
 lib/ofp-util.h     |    1 +
 ofproto/ofproto.c  |    1 +
 tests/ofp-print.at |    2 +-
 5 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 6789625..9123f2f 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -843,6 +843,10 @@ ofp_print_flow_removed(struct ds *string, const struct ofp_header *oh)
     ds_put_format(string, " reason=%s",
                   ofp_flow_removed_reason_to_string(fr.reason));
 
+    if (fr.table_id != 255) {
+        ds_put_format(string, " table_id=%"PRIu8, fr.table_id);
+    }
+
     if (fr.cookie != htonll(0)) {
         ds_put_format(string, " cookie:0x%"PRIx64, ntohll(fr.cookie));
     }
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index c78199f..7edae87 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1913,7 +1913,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr,
         fr->priority = ntohs(ofr->priority);
         fr->cookie = ofr->cookie;
         fr->reason = ofr->reason;
-        /* XXX: ofr->table_id is ignored */
+        fr->table_id = ofr->table_id;
         fr->duration_sec = ntohl(ofr->duration_sec);
         fr->duration_nsec = ntohl(ofr->duration_nsec);
         fr->idle_timeout = ntohs(ofr->idle_timeout);
@@ -1929,6 +1929,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr,
         fr->priority = ntohs(ofr->priority);
         fr->cookie = ofr->cookie;
         fr->reason = ofr->reason;
+        fr->table_id = 255;
         fr->duration_sec = ntohl(ofr->duration_sec);
         fr->duration_nsec = ntohl(ofr->duration_nsec);
         fr->idle_timeout = ntohs(ofr->idle_timeout);
@@ -1952,6 +1953,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr,
         fr->priority = ntohs(nfr->priority);
         fr->cookie = nfr->cookie;
         fr->reason = nfr->reason;
+        fr->table_id = 255;
         fr->duration_sec = ntohl(nfr->duration_sec);
         fr->duration_nsec = ntohl(nfr->duration_nsec);
         fr->idle_timeout = ntohs(nfr->idle_timeout);
@@ -1985,7 +1987,7 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr,
         ofr->cookie = fr->cookie;
         ofr->priority = htons(fr->priority);
         ofr->reason = fr->reason;
-        ofr->table_id = 0;
+        ofr->table_id = fr->table_id;
         ofr->duration_sec = htonl(fr->duration_sec);
         ofr->duration_nsec = htonl(fr->duration_nsec);
         ofr->idle_timeout = htons(fr->idle_timeout);
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 38e7b02..8ef2206 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -243,6 +243,7 @@ struct ofputil_flow_removed {
     uint16_t priority;
     ovs_be64 cookie;
     uint8_t reason;             /* One of OFPRR_*. */
+    uint8_t table_id;           /* 255 if message didn't include table ID. */
     uint32_t duration_sec;
     uint32_t duration_nsec;
     uint16_t idle_timeout;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 7c92d71..52203fd 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3225,6 +3225,7 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t reason)
     fr.priority = rule->cr.priority;
     fr.cookie = rule->flow_cookie;
     fr.reason = reason;
+    fr.table_id = rule->table_id;
     calc_flow_duration__(rule->created, time_msec(),
                          &fr.duration_sec, &fr.duration_nsec);
     fr.idle_timeout = rule->idle_timeout;
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index c2eb002..fee3d28 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -376,7 +376,7 @@ AT_CHECK([ovs-ofctl ofp-print "\
 80 00 01 05 00 00 00 01 00 98 96 80 00 3c 00 78 \
 00 00 00 00 00 12 d6 87 00 00 00 00 6f 68 ba 66 \
 00 01 00 0a 80 00 0c 02 10 09 00 00 00 00 00 00"], [0], [dnl
-OFPT_FLOW_REMOVED (OF1.2) (xid=0x0): dl_vlan=9 reason=hard cookie:0xfedcba9876543210 duration1.01s idle60 hard120 pkts1234567 bytes1869134438
+OFPT_FLOW_REMOVED (OF1.2) (xid=0x0): dl_vlan=9 reason=hard table_id=5 cookie:0xfedcba9876543210 duration1.01s idle60 hard120 pkts1234567 bytes1869134438
 ])
 AT_CLEANUP
 
-- 
1.7.2.5




More information about the dev mailing list