On Mon, Feb 06, 2012 at 02:59:12PM -0800, Ethan Jackson wrote:
> In learning switch, should OFPUTIL_NXT_FLOW_AGE return an error as
> per the spec?
learning-switch.c implements a controller (that implements a
MAC-learning switch), so it shouldn't get NXT_FLOW_AGE requests at
all.
> > + cookie=0x0, duration=2.88s, table=0, n_packets=1, n_bytes=60,
> > idle_timeout=5,hard_timeout=10,idle_age=2,hard_age=7,priority=65535,tcp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168
>
> This particular case looked very strange to me for a few seconds
> because the hard_age is larger than the duration which shouldn't be
> possible. Of course, this is only testing the parser, but perhaps it
> makes sense to use a smaller value in the test just so it looks
> normal.
OK, I changed the hard_age to 2.
> Also in the unit tests you've added, you check that the duration,
> idle_age, and hard_age are increasing. Does it make sense to also
> check that at each step duration >= hard_age >= idle_age?
You're right, we can check more invariants here. The one you suggest
isn't always true, though, because "mod-flows" does not reset the
idle_age to 0. Also, we need to make sure that duration, hard_age,
and idle_age are calculated relative to the same value of the current
time (which is a good idea anyway).
I applied this:
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 4937040..2712d61 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1045,10 +1045,10 @@ ofp_print_flow_stats_reply(struct ds *string, const
struct ofp_header *oh)
ds_put_format(string, "hard_timeout=%"PRIu16",", fs.hard_timeout);
}
if (fs.idle_age >= 0) {
- ds_put_format(string, "idle_age=%"PRIu16",", fs.idle_age);
+ ds_put_format(string, "idle_age=%d,", fs.idle_age);
}
if (fs.hard_age >= 0 && fs.hard_age != fs.duration_sec) {
- ds_put_format(string, "hard_age=%"PRIu16",", fs.hard_age);
+ ds_put_format(string, "hard_age=%d,", fs.hard_age);
}
cls_rule_format(&fs.rule, string);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3cf6b07..473ae41 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2174,9 +2174,10 @@ handle_port_stats_request(struct ofconn *ofconn,
}
static void
-calc_flow_duration__(long long int start, uint32_t *sec, uint32_t *nsec)
+calc_flow_duration__(long long int start, long long int now,
+ uint32_t *sec, uint32_t *nsec)
{
- long long int msecs = time_msec() - start;
+ long long int msecs = now - start;
*sec = msecs / 1000;
*nsec = (msecs % 1000) * (1000 * 1000);
}
@@ -2337,11 +2338,11 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t
table_id,
return 0;
}
-/* Returns the number of whole seconds since 'start'. */
+/* Returns 'age_ms' (a duration in milliseconds), converted to seconds and
+ * forced into the range of a uint16_t. */
static int
-age_secs(long long int start)
+age_secs(long long int age_ms)
{
- long long int age_ms = time_msec() - start;
return (age_ms < 0 ? 0
: age_ms >= UINT16_MAX * 1000 ? UINT16_MAX
: (unsigned int) age_ms / 1000);
@@ -2372,17 +2373,18 @@ handle_flow_stats_request(struct ofconn *ofconn,
ofputil_start_stats_reply(osm, &replies);
LIST_FOR_EACH (rule, ofproto_node, &rules) {
+ long long int now = time_msec();
struct ofputil_flow_stats fs;
fs.rule = rule->cr;
fs.cookie = rule->flow_cookie;
fs.table_id = rule->table_id;
- calc_flow_duration__(rule->created, &fs.duration_sec,
+ calc_flow_duration__(rule->created, now, &fs.duration_sec,
&fs.duration_nsec);
fs.idle_timeout = rule->idle_timeout;
fs.hard_timeout = rule->hard_timeout;
- fs.idle_age = age_secs(rule->used);
- fs.hard_age = age_secs(rule->modified);
+ fs.idle_age = age_secs(now - rule->used);
+ fs.hard_age = age_secs(now - rule->modified);
ofproto->ofproto_class->rule_get_stats(rule, &fs.packet_count,
&fs.byte_count);
fs.actions = rule->actions;
@@ -2942,7 +2944,8 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t
reason)
fr.rule = rule->cr;
fr.cookie = rule->flow_cookie;
fr.reason = reason;
- calc_flow_duration__(rule->created, &fr.duration_sec, &fr.duration_nsec);
+ calc_flow_duration__(rule->created, time_msec(),
+ &fr.duration_sec, &fr.duration_nsec);
fr.idle_timeout = rule->idle_timeout;
rule->ofproto->ofproto_class->rule_get_stats(rule, &fr.packet_count,
&fr.byte_count);
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index facba2b..2595900 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -823,7 +823,7 @@ a8 00 01 00 00 0c 01 06 00 00 12 02 00 00 00 00 \
a8 00 01 00 00 10 04 c0 a8 00 02 00 00 0c 01 06 \
00 00 12 02 09 e3 00 00 14 02 00 00 00 00 00 00 \
00 00 00 08 00 01 00 00 00 88 00 00 00 00 00 02 \
-34 73 bc 00 ff ff 00 05 00 0a 00 4c 00 03 00 08 \
+34 73 bc 00 ff ff 00 05 00 0a 00 4c 00 03 00 03 \
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 \
00 00 00 00 00 00 00 3c 00 00 00 02 00 03 00 00 \
02 06 50 54 00 00 00 06 00 00 04 06 50 54 00 00 \
@@ -923,7 +923,7 @@ ff ff 00 18 00 00 23 20 00 07 00 1f 00 01 00 04 \
cookie=0x0, duration=3.84s, table=0, n_packets=1, n_bytes=60,
idle_timeout=5,idle_age=2,priority=65535,tcp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,tp_src=2532,tp_dst=0
actions=output:1
cookie=0x0, duration=2.872s, table=0, n_packets=1, n_bytes=60,
idle_timeout=5,idle_age=4,priority=65535,tcp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,nw_tos=0,tp_src=0,tp_dst=2533
actions=output:3
cookie=0x0, duration=4.756s, table=0, n_packets=1, n_bytes=60,
idle_timeout=5,idle_age=0,priority=65535,tcp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,tp_src=2531,tp_dst=0
actions=output:1
- cookie=0x0, duration=2.88s, table=0, n_packets=1, n_bytes=60,
idle_timeout=5,hard_timeout=10,idle_age=2,hard_age=7,priority=65535,tcp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,tp_src=2533,tp_dst=0
actions=output:1
+ cookie=0x0, duration=2.88s, table=0, n_packets=1, n_bytes=60,
idle_timeout=5,hard_timeout=10,idle_age=2,priority=65535,tcp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,tp_src=2533,tp_dst=0
actions=output:1
cookie=0x0, duration=5.672s, table=0, n_packets=1, n_bytes=60,
idle_timeout=5,priority=65535,tcp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,tp_src=2530,tp_dst=0
actions=output:1
cookie=0x0, duration=1.04s, table=0, n_packets=1, n_bytes=60,
idle_timeout=5,priority=65535,tcp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,nw_tos=0,tp_src=0,tp_dst=2535
actions=output:3
cookie=0x0, duration=1.952s, table=0, n_packets=1, n_bytes=60,
idle_timeout=5,priority=65535,tcp,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,nw_tos=0,tp_src=0,tp_dst=2534
actions=output:3
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index b8f48de..f5c1358 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1088,11 +1088,10 @@ get_ages duration3 hard3 idle3
# occasionally.
ovs-appctl time/warp 10000
ovs-appctl netdev-dummy/receive br0
'in_port(0),eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:05),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no),tcp(src=80,dst=1234)'
-ovs-ofctl dump-flows br0
-get_ages duration4 hard4 idle4
ovs-appctl time/warp 1000
ovs-appctl time/warp 1000
ovs-appctl time/warp 1000
+get_ages duration4 hard4 idle4
printf "duration: %4s => %4s => %4s => %4s\n" $duration1 $duration2 $duration3
$duration4
printf "hard_age: %4s => %4s => %4s => %4s\n" $hard1 $hard2 $hard3 $hard4
@@ -1116,5 +1115,13 @@ AT_CHECK([test $idle1 -lt $idle2])
AT_CHECK([test $idle2 -lt $idle3])
AT_CHECK([test $idle3 -gt $idle4])
+# Check some invariant relationships.
+AT_CHECK([test $duration1 = $idle1])
+AT_CHECK([test $duration2 = $idle2])
+AT_CHECK([test $duration3 = $idle3])
+AT_CHECK([test $idle3 -gt $hard3])
+AT_CHECK([test $idle4 -lt $hard4])
+AT_CHECK([test $hard4 -lt $duration4])
+
OVS_VSWITCHD_STOP
AT_CLEANUP
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
