From 89108874d59364313f9e5e192ba8ca00a2771d93 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Thu, 19 Nov 2015 18:20:39 -0800 Subject: [PATCH] ofproto: Check actions also for packet outs and traces. Make the packet out and trace processing perform the same actions checks as flow mod processing does. This used to be the case before, but at some point these have diverged to perform different combinations of checks. Signed-off-by: Jarno Rajahalme Acked-by: Ben Pfaff --- ofproto/ofproto-dpif.c | 15 ++++++++++----- ofproto/ofproto-provider.h | 3 +++ ofproto/ofproto.c | 20 ++++++++++++++++---- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 5cc64cbca..49001e896 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4946,13 +4946,18 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn, int argc, goto exit; } if (enforce_consistency) { - retval = ofpacts_check_consistency(ofpacts.data, ofpacts.size, - &flow, u16_to_ofp(ofproto->up.max_ports), - 0, 0, usable_protocols); + retval = ofpacts_check_consistency(ofpacts.data, ofpacts.size, &flow, + u16_to_ofp(ofproto->up.max_ports), + 0, ofproto->up.n_tables, + usable_protocols); } else { retval = ofpacts_check(ofpacts.data, ofpacts.size, &flow, - u16_to_ofp(ofproto->up.max_ports), 0, 0, - &usable_protocols); + u16_to_ofp(ofproto->up.max_ports), 0, + ofproto->up.n_tables, &usable_protocols); + } + if (!retval) { + retval = ofproto_check_ofpacts(&ofproto->up, ofpacts.data, + ofpacts.size); } if (retval) { diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 117cd1fcc..98d439e99 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -1787,6 +1787,9 @@ void ofproto_delete_flow(struct ofproto *, const struct match *, int priority) OVS_EXCLUDED(ofproto_mutex); void ofproto_flush_flows(struct ofproto *); +enum ofperr ofproto_check_ofpacts(struct ofproto *, + const struct ofpact ofpacts[], + size_t ofpacts_len); static inline const struct rule_actions * rule_get_actions(const struct rule *rule) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index c7dd8a2c9..2cdc8d428 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -3333,7 +3333,7 @@ reject_slave_controller(struct ofconn *ofconn) * - If they use any groups, then 'ofproto' has that group configured. * * Returns 0 if successful, otherwise an OpenFlow error. */ -static enum ofperr +enum ofperr ofproto_check_ofpacts(struct ofproto *ofproto, const struct ofpact ofpacts[], size_t ofpacts_len) { @@ -3399,10 +3399,22 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh) /* Verify actions against packet, then send packet if successful. */ flow_extract(payload, &flow); flow.in_port.ofp_port = po.in_port; - error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len); + + /* Check actions like for flow mods. We pass a 'table_id' of 0 to + * ofproto_check_consistency(), which isn't strictly correct because these + * actions aren't in any table. This is OK as 'table_id' is only used to + * check instructions (e.g., goto-table), which can't appear on the action + * list of a packet-out. */ + error = ofpacts_check_consistency(po.ofpacts, po.ofpacts_len, + &flow, u16_to_ofp(p->max_ports), + 0, p->n_tables, + ofconn_get_protocol(ofconn)); if (!error) { - error = p->ofproto_class->packet_out(p, payload, &flow, - po.ofpacts, po.ofpacts_len); + error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len); + if (!error) { + error = p->ofproto_class->packet_out(p, payload, &flow, + po.ofpacts, po.ofpacts_len); + } } dp_packet_delete(payload); -- 2.20.1