Skip to content

Commit

Permalink
ofproto: Check actions also for packet outs and traces.
Browse files Browse the repository at this point in the history
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 <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
  • Loading branch information
Jarno Rajahalme authored and jrajahalme committed Nov 20, 2015
1 parent 0a939c1 commit 8910887
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
15 changes: 10 additions & 5 deletions ofproto/ofproto-dpif.c
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions ofproto/ofproto-provider.h
Expand Up @@ -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)
Expand Down
20 changes: 16 additions & 4 deletions ofproto/ofproto.c
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 8910887

Please sign in to comment.