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 <ho...@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 <ho...@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 <ho...@verge.net.au>
[b...@nicira.com changed "missing" value, removed OFPTT_ALL]
Signed-off-by: Ben Pfaff <b...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
