Previously the out_port of a flow monitor was
checked in ofmonitor_report() using ofoperation_has_out_port().
When ofoperation_has_out_port() was removed so was the call to
it in ofmonitor_report() thus flow monitor updates are longer
filtered on the out_port.
This restores filtering on the out_port by using
ofproto_rule_has_out_port to check the actions of the rule.
If the actions have been changed by a modify actions then
ofpacts_output_to_port() is also used to check the old actions.
This patch also adds a test to exercise out_ports for flow monitors.
This resolves what appears to be a regression introduced by
b20f4073eecd4761 ("ofproto: Do straightforward removal of asynchronous flow
operations.")
Signed-off-by: Simon Horman <ho...@verge.net.au>
---
ofproto/connmgr.c | 8 ++++++-
ofproto/connmgr.h | 4 +++-
ofproto/ofproto-provider.h | 2 ++
ofproto/ofproto.c | 12 ++++++-----
tests/ofproto.at | 52 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 7d313c7..260b065 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -2068,7 +2068,8 @@ void
ofmonitor_report(struct connmgr *mgr, struct rule *rule,
enum nx_flow_update_event event,
enum ofp_flow_removed_reason reason,
- const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid)
+ const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid,
+ const struct rule_actions *old_actions)
OVS_REQUIRES(ofproto_mutex)
{
enum nx_flow_monitor_flags update;
@@ -2114,6 +2115,11 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
HMAP_FOR_EACH (m, ofconn_node, &ofconn->monitors) {
if (m->flags & update
&& (m->table_id == 0xff || m->table_id == rule->table_id)
+ && (ofproto_rule_has_out_port(rule, m->out_port)
+ || (old_actions
+ && ofpacts_output_to_port(old_actions->ofpacts,
+ old_actions->ofpacts_len,
+ m->out_port)))
&& cls_rule_is_loose_match(&rule->cr, &m->match)) {
flags |= m->flags;
}
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index 75a4c06..75a1ffe 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -24,6 +24,7 @@
#include "ofp-errors.h"
#include "ofp-util.h"
#include "ofproto.h"
+#include "ofproto-provider.h"
#include "openflow/nicira-ext.h"
#include "openvswitch/types.h"
@@ -218,7 +219,8 @@ void ofmonitor_destroy(struct ofmonitor *)
void ofmonitor_report(struct connmgr *, struct rule *,
enum nx_flow_update_event, enum ofp_flow_removed_reason,
- const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid)
+ const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid,
+ const struct rule_actions *old_actions)
OVS_REQUIRES(ofproto_mutex);
void ofmonitor_flush(struct connmgr *) OVS_REQUIRES(ofproto_mutex);
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index c8d70e6..ab5cda8 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -421,6 +421,8 @@ BUILD_ASSERT_DECL(offsetof(struct rule_actions, ofpacts) %
OFPACT_ALIGNTO == 0);
const struct rule_actions *rule_actions_create(const struct ofpact *, size_t);
void rule_actions_destroy(const struct rule_actions *);
+bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t port)
+ OVS_REQUIRES(ofproto_mutex);
/* A set of rules to which an OpenFlow operation applies. */
struct rule_collection {
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 83a9c35..7b2de18 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2562,7 +2562,7 @@ rule_actions_destroy(const struct rule_actions *actions)
/* Returns true if 'rule' has an OpenFlow OFPAT_OUTPUT or OFPAT_ENQUEUE action
* that outputs to 'port' (output to OFPP_FLOOD and OFPP_ALL doesn't count). */
-static bool
+bool
ofproto_rule_has_out_port(const struct rule *rule, ofp_port_t port)
OVS_REQUIRES(ofproto_mutex)
{
@@ -4077,7 +4077,7 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod
*fm,
}
ofmonitor_report(ofproto->connmgr, rule, NXFME_ADDED, 0,
- req ? req->ofconn : NULL, req ? req->xid : 0);
+ req ? req->ofconn : NULL, req ? req->xid : 0, NULL);
return req ? send_buffered_packet(req->ofconn, fm->buffer_id, rule) : 0;
}
@@ -4167,7 +4167,6 @@ modify_flows__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
if (change_actions) {
ovsrcu_set(&rule->actions, rule_actions_create(fm->ofpacts,
fm->ofpacts_len));
- rule_actions_destroy(actions);
}
if (change_actions || reset_counters) {
@@ -4176,10 +4175,12 @@ modify_flows__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
if (event != NXFME_MODIFIED || change_actions || change_cookie) {
ofmonitor_report(ofproto->connmgr, rule, event, 0,
- req ? req->ofconn : NULL, req ? req->xid : 0);
+ req ? req->ofconn : NULL, req ? req->xid : 0,
+ change_actions ? actions : NULL);
}
if (change_actions) {
+ rule_actions_destroy(actions);
learned_cookies_inc(ofproto, rule_get_actions(rule));
learned_cookies_dec(ofproto, actions, &dead_cookies);
}
@@ -4289,7 +4290,8 @@ delete_flows__(const struct rule_collection *rules,
ofproto_rule_send_removed(rule, reason);
ofmonitor_report(ofproto->connmgr, rule, NXFME_DELETED, reason,
- req ? req->ofconn : NULL, req ? req->xid : 0);
+ req ? req->ofconn : NULL, req ? req->xid : 0,
+ NULL);
oftable_remove_rule(rule);
ofproto->ofproto_class->rule_delete(rule);
diff --git a/tests/ofproto.at b/tests/ofproto.at
index bd9c3af..8ed1571 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2250,6 +2250,58 @@ ovs-appctl -t ovs-ofctl exit
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto - flow monitoring with out_port])
+AT_KEYWORDS([monitor])
+OVS_VSWITCHD_START
+
+ovs-ofctl add-flow br0 in_port=0,dl_vlan=121,actions=output:1
+ovs-ofctl add-flow br0 in_port=0,dl_vlan=122,actions=output:1
+ovs-ofctl add-flow br0 in_port=0,dl_vlan=123,actions=output:2
+
+# Start a monitor watching the flow table and check the initial reply.
+ovs-ofctl monitor br0 watch:out_port=2 --detach --no-chdir --pidfile
>monitor.log 2>&1
+AT_CAPTURE_FILE([monitor.log])
+ovs-appctl -t ovs-ofctl ofctl/barrier
+AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
+ [NXST_FLOW_MONITOR reply:
+ event=ADDED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:2
+OFPT_BARRIER_REPLY:
+])
+
+ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
+
+# Add, modify flows and check the updates.
+ovs-ofctl mod-flows br0 dl_vlan=121,actions=drop
+ovs-ofctl mod-flows br0 dl_vlan=122,actions=output:1,output:2
+ovs-appctl -t ovs-ofctl ofctl/barrier
+
+ovs-ofctl mod-flows br0 dl_vlan=123,actions=output:1,output:2
+ovs-appctl -t ovs-ofctl ofctl/barrier
+
+ovs-ofctl mod-flows br0 dl_vlan=122,actions=output:1
+ovs-appctl -t ovs-ofctl ofctl/barrier
+ovs-ofctl mod-flows br0 dl_vlan=123,actions=output:2
+ovs-appctl -t ovs-ofctl ofctl/barrier
+
+AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
+[NXST_FLOW_MONITOR reply (xid=0x0):
+ event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=122
actions=output:1,output:2
+OFPT_BARRIER_REPLY:
+NXST_FLOW_MONITOR reply (xid=0x0):
+ event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=123
actions=output:1,output:2
+OFPT_BARRIER_REPLY:
+NXST_FLOW_MONITOR reply (xid=0x0):
+ event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=122 actions=output:1
+OFPT_BARRIER_REPLY:
+NXST_FLOW_MONITOR reply (xid=0x0):
+ event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:2
+OFPT_BARRIER_REPLY:
+])
+
+ovs-appctl -t ovs-ofctl exit
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto - flow monitoring pause and resume])
AT_KEYWORDS([monitor])
--
2.0.0.rc2
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
