Usually the table id in flow mods is 255, which means that goto table
instruction cannot be checked before the table is picked (for flow add),
or the rules to be modified are found (flow mod).
Move goto table checking from decode (ofp-util) to actions checking
(ofp-actions), and postpone the action checking until the table in
which the actions are added is known.
This fixes OFPBRC_BAD_TABLE_ID errors for flow adds that specify the table
id as 255, and have a goto table instruction.
Signed-off-by: Jarno Rajahalme <jarno.rajahalme@nsn.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
enum ofperr
ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
unsigned int instructions_len,
enum ofperr
ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
unsigned int instructions_len,
struct ofpbuf *ofpacts)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
struct ofpbuf *ofpacts)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
oigt = instruction_get_OFPIT11_GOTO_TABLE(
insts[OVSINST_OFPIT11_GOTO_TABLE]);
oigt = instruction_get_OFPIT11_GOTO_TABLE(
insts[OVSINST_OFPIT11_GOTO_TABLE]);
- if (table_id >= oigt->table_id) {
- error = OFPERR_OFPBRC_BAD_TABLE_ID;
- goto exit;
- }
ogt = ofpact_put_GOTO_TABLE(ofpacts);
ogt->table_id = oigt->table_id;
}
ogt = ofpact_put_GOTO_TABLE(ofpacts);
ogt->table_id = oigt->table_id;
}
\f
/* May modify flow->dl_type, caller must restore it. */
static enum ofperr
\f
/* May modify flow->dl_type, caller must restore it. */
static enum ofperr
-ofpact_check__(const struct ofpact *a, struct flow *flow, ofp_port_t max_ports)
+ofpact_check__(const struct ofpact *a, struct flow *flow, ofp_port_t max_ports,
+ uint8_t table_id)
{
const struct ofpact_enqueue *enqueue;
{
const struct ofpact_enqueue *enqueue;
case OFPACT_CLEAR_ACTIONS:
case OFPACT_WRITE_METADATA:
case OFPACT_CLEAR_ACTIONS:
case OFPACT_WRITE_METADATA:
- case OFPACT_GOTO_TABLE:
return 0;
case OFPACT_METER: {
return 0;
case OFPACT_METER: {
+
+ case OFPACT_GOTO_TABLE:
+ if (ofpact_get_GOTO_TABLE(a)->table_id <= table_id) {
+ return OFPERR_OFPBRC_BAD_TABLE_ID;
+ }
+ return 0;
+
default:
NOT_REACHED();
}
default:
NOT_REACHED();
}
* May temporarily modify 'flow', but restores the changes before returning. */
enum ofperr
ofpacts_check(const struct ofpact ofpacts[], size_t ofpacts_len,
* May temporarily modify 'flow', but restores the changes before returning. */
enum ofperr
ofpacts_check(const struct ofpact ofpacts[], size_t ofpacts_len,
- struct flow *flow, ofp_port_t max_ports)
+ struct flow *flow, ofp_port_t max_ports, uint8_t table_id)
{
const struct ofpact *a;
ovs_be16 dl_type = flow->dl_type;
enum ofperr error = 0;
OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
{
const struct ofpact *a;
ovs_be16 dl_type = flow->dl_type;
enum ofperr error = 0;
OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
- error = ofpact_check__(a, flow, max_ports);
+ error = ofpact_check__(a, flow, max_ports, table_id);
struct ofpbuf *ofpacts);
enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
unsigned int instructions_len,
struct ofpbuf *ofpacts);
enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
unsigned int instructions_len,
struct ofpbuf *ofpacts);
enum ofperr ofpacts_check(const struct ofpact[], size_t ofpacts_len,
struct ofpbuf *ofpacts);
enum ofperr ofpacts_check(const struct ofpact[], size_t ofpacts_len,
- struct flow *, ofp_port_t max_ports);
+ struct flow *, ofp_port_t max_ports,
+ uint8_t table_id);
enum ofperr ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len);
/* Converting ofpacts to OpenFlow. */
enum ofperr ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len);
/* Converting ofpacts to OpenFlow. */
fm->ofpacts = ofpbuf_steal_data(&ofpacts);
err = ofpacts_check(fm->ofpacts, fm->ofpacts_len, &fm->match.flow,
fm->ofpacts = ofpbuf_steal_data(&ofpacts);
err = ofpacts_check(fm->ofpacts, fm->ofpacts_len, &fm->match.flow,
if (err) {
exit(EXIT_FAILURE);
}
if (err) {
exit(EXIT_FAILURE);
}
- error = ofpacts_pull_openflow11_instructions(&b, b.size, ofm->table_id,
- ofpacts);
+ error = ofpacts_pull_openflow11_instructions(&b, b.size, ofpacts);
if (error) {
return error;
}
if (error) {
return error;
}
}
if (ofpacts_pull_openflow11_instructions(msg, length - sizeof *ofs -
}
if (ofpacts_pull_openflow11_instructions(msg, length - sizeof *ofs -
- padded_match_len,
- ofs->table_id, ofpacts)) {
+ padded_match_len, ofpacts)) {
VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
return EINVAL;
}
VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
return EINVAL;
}
enum { OFPROTO_POSTPONE = 1 << 16 };
BUILD_ASSERT_DECL(OFPROTO_POSTPONE < OFPERR_OFS);
enum { OFPROTO_POSTPONE = 1 << 16 };
BUILD_ASSERT_DECL(OFPROTO_POSTPONE < OFPERR_OFS);
-int ofproto_flow_mod(struct ofproto *, const struct ofputil_flow_mod *);
+int ofproto_flow_mod(struct ofproto *, struct ofputil_flow_mod *);
void ofproto_add_flow(struct ofproto *, const struct match *,
unsigned int priority,
const struct ofpact *ofpacts, size_t ofpacts_len);
void ofproto_add_flow(struct ofproto *, const struct match *,
unsigned int priority,
const struct ofpact *ofpacts, size_t ofpacts_len);
/* OpenFlow. */
static enum ofperr add_flow(struct ofproto *, struct ofconn *,
/* OpenFlow. */
static enum ofperr add_flow(struct ofproto *, struct ofconn *,
- const struct ofputil_flow_mod *,
+ struct ofputil_flow_mod *,
const struct ofp_header *);
static void delete_flow__(struct rule *, struct ofopgroup *,
enum ofp_flow_removed_reason);
static bool handle_openflow(struct ofconn *, const struct ofpbuf *);
static enum ofperr handle_flow_mod__(struct ofproto *, struct ofconn *,
const struct ofp_header *);
static void delete_flow__(struct rule *, struct ofopgroup *,
enum ofp_flow_removed_reason);
static bool handle_openflow(struct ofconn *, const struct ofpbuf *);
static enum ofperr handle_flow_mod__(struct ofproto *, struct ofconn *,
- const struct ofputil_flow_mod *,
+ struct ofputil_flow_mod *,
const struct ofp_header *);
static void calc_duration(long long int start, long long int now,
uint32_t *sec, uint32_t *nsec);
const struct ofp_header *);
static void calc_duration(long long int start, long long int now,
uint32_t *sec, uint32_t *nsec);
*
* This is a helper function for in-band control and fail-open. */
int
*
* This is a helper function for in-band control and fail-open. */
int
-ofproto_flow_mod(struct ofproto *ofproto, const struct ofputil_flow_mod *fm)
+ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm)
{
return handle_flow_mod__(ofproto, NULL, fm, NULL);
}
{
return handle_flow_mod__(ofproto, NULL, fm, NULL);
}
-/* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are
- * appropriate for a packet with the prerequisites satisfied by 'flow'.
+/* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are appropriate
+ * for a packet with the prerequisites satisfied by 'flow' in table 'table_id'.
* 'flow' may be temporarily modified, but is restored at return.
*/
static enum ofperr
ofproto_check_ofpacts(struct ofproto *ofproto,
const struct ofpact ofpacts[], size_t ofpacts_len,
* 'flow' may be temporarily modified, but is restored at return.
*/
static enum ofperr
ofproto_check_ofpacts(struct ofproto *ofproto,
const struct ofpact ofpacts[], size_t ofpacts_len,
+ struct flow *flow, uint8_t table_id)
{
enum ofperr error;
uint32_t mid;
{
enum ofperr error;
uint32_t mid;
- error = ofpacts_check(ofpacts, ofpacts_len, flow, ofproto->max_ports);
+ error = ofpacts_check(ofpacts, ofpacts_len, flow, ofproto->max_ports,
+ table_id);
if (error) {
return error;
}
if (error) {
return error;
}
/* Verify actions against packet, then send packet if successful. */
in_port_.ofp_port = po.in_port;
flow_extract(payload, 0, 0, NULL, &in_port_, &flow);
/* Verify actions against packet, then send packet if successful. */
in_port_.ofp_port = po.in_port;
flow_extract(payload, 0, 0, NULL, &in_port_, &flow);
- error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len, &flow);
+ error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len, &flow, 0);
if (!error) {
error = p->ofproto_class->packet_out(p, payload, &flow,
po.ofpacts, po.ofpacts_len);
if (!error) {
error = p->ofproto_class->packet_out(p, payload, &flow,
po.ofpacts, po.ofpacts_len);
* if any. */
static enum ofperr
add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
* if any. */
static enum ofperr
add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
- const struct ofputil_flow_mod *fm, const struct ofp_header *request)
+ struct ofputil_flow_mod *fm, const struct ofp_header *request)
{
struct oftable *table;
struct ofopgroup *group;
{
struct oftable *table;
struct ofopgroup *group;
return OFPERR_OFPBRC_EPERM;
}
return OFPERR_OFPBRC_EPERM;
}
+ /* Verify actions. */
+ error = ofproto_check_ofpacts(ofproto, fm->ofpacts, fm->ofpacts_len,
+ &fm->match.flow, table_id);
+ if (error) {
+ return error;
+ }
+
/* Allocate new rule and initialize classifier rule. */
rule = ofproto->ofproto_class->rule_alloc();
if (!rule) {
/* Allocate new rule and initialize classifier rule. */
rule = ofproto->ofproto_class->rule_alloc();
if (!rule) {
* Returns 0 on success, otherwise an OpenFlow error code. */
static enum ofperr
modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
* Returns 0 on success, otherwise an OpenFlow error code. */
static enum ofperr
modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
- const struct ofputil_flow_mod *fm,
- const struct ofp_header *request,
+ struct ofputil_flow_mod *fm, const struct ofp_header *request,
struct list *rules)
{
struct ofopgroup *group;
struct list *rules)
{
struct ofopgroup *group;
+ /* Verify actions. */
+ error = ofpacts_check(fm->ofpacts, fm->ofpacts_len, &fm->match.flow,
+ ofproto->max_ports, rule->table_id);
+ if (error) {
+ return error;
+ }
+
actions_changed = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
rule->ofpacts, rule->ofpacts_len);
actions_changed = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
rule->ofpacts, rule->ofpacts_len);
static enum ofperr
modify_flows_add(struct ofproto *ofproto, struct ofconn *ofconn,
static enum ofperr
modify_flows_add(struct ofproto *ofproto, struct ofconn *ofconn,
- const struct ofputil_flow_mod *fm,
- const struct ofp_header *request)
+ struct ofputil_flow_mod *fm, const struct ofp_header *request)
{
if (fm->cookie_mask != htonll(0) || fm->new_cookie == htonll(UINT64_MAX)) {
return 0;
{
if (fm->cookie_mask != htonll(0) || fm->new_cookie == htonll(UINT64_MAX)) {
return 0;
* if any. */
static enum ofperr
modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
* if any. */
static enum ofperr
modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
- const struct ofputil_flow_mod *fm,
+ struct ofputil_flow_mod *fm,
const struct ofp_header *request)
{
struct list rules;
const struct ofp_header *request)
{
struct list rules;
* if any. */
static enum ofperr
modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
* if any. */
static enum ofperr
modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
- const struct ofputil_flow_mod *fm,
+ struct ofputil_flow_mod *fm,
const struct ofp_header *request)
{
struct list rules;
const struct ofp_header *request)
{
struct list rules;
ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
error = ofputil_decode_flow_mod(&fm, oh, ofconn_get_protocol(ofconn),
&ofpacts);
ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
error = ofputil_decode_flow_mod(&fm, oh, ofconn_get_protocol(ofconn),
&ofpacts);
- if (!error) {
- error = ofproto_check_ofpacts(ofproto, fm.ofpacts, fm.ofpacts_len,
- &fm.match.flow);
- }
-
if (!error) {
error = handle_flow_mod__(ofproto, ofconn, &fm, oh);
}
if (!error) {
error = handle_flow_mod__(ofproto, ofconn, &fm, oh);
}
static enum ofperr
handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
static enum ofperr
handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
- const struct ofputil_flow_mod *fm,
- const struct ofp_header *oh)
+ struct ofputil_flow_mod *fm, const struct ofp_header *oh)
{
if (ofproto->n_pending >= 50) {
ovs_assert(!list_is_empty(&ofproto->pending));
{
if (ofproto->n_pending >= 50) {
ovs_assert(!list_is_empty(&ofproto->pending));
/* Convert to ofpacts. */
ofpbuf_init(&ofpacts, 0);
size = of11_in.size;
/* Convert to ofpacts. */
ofpbuf_init(&ofpacts, 0);
size = of11_in.size;
- error = ofpacts_pull_openflow11_instructions(
- &of11_in, of11_in.size, table_id ? atoi(table_id) : 0,
- &ofpacts);
+ error = ofpacts_pull_openflow11_instructions(&of11_in, of11_in.size,
+ &ofpacts);
+ if (!error) {
+ /* Verify actions. */
+ struct flow flow;
+ memset(&flow, 0, sizeof flow);
+ error = ofpacts_check(ofpacts.data, ofpacts.size, &flow, OFPP_MAX,
+ table_id ? atoi(table_id) : 0);
+ }
if (error) {
printf("bad OF1.1 instructions: %s\n\n", ofperr_get_name(error));
ofpbuf_uninit(&ofpacts);
if (error) {
printf("bad OF1.1 instructions: %s\n\n", ofperr_get_name(error));
ofpbuf_uninit(&ofpacts);