2
0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 14:25:26 +00:00

ofproto: Handle OpenFlow version mismatch for requestforward with groups.

OpenFlow 1.4+ supports a feature called requestforward.  When a controller
enables this feature, the switch sends that controller a copy of other
controllers' group and meter modification requests.  OpenFlow 1.5 supports
some group features not in OpenFlow 1.4.  When OVS attempted to forward
such requests to an OpenFlow 1.4 controller, it reported an error and
exited.  This commit fixes the problem by making OVS properly translate the
messages to OpenFlow 1.4 format.

Reported-by: Pierre Cregut <pierre.cregut@orange.com>
Tested-by: Pierre Cregut <pierre.cregut@orange.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047453.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
This commit is contained in:
Ben Pfaff
2018-09-25 14:06:37 -07:00
parent ffe6cea896
commit f836888d28
8 changed files with 166 additions and 105 deletions

View File

@@ -103,7 +103,9 @@ struct ofputil_group_mod {
void ofputil_uninit_group_mod(struct ofputil_group_mod *gm);
struct ofpbuf *ofputil_encode_group_mod(enum ofp_version ofp_version,
const struct ofputil_group_mod *gm);
const struct ofputil_group_mod *gm,
const struct ovs_list *new_buckets,
int group_existed);
enum ofperr ofputil_decode_group_mod(const struct ofp_header *,
struct ofputil_group_mod *);

View File

@@ -125,7 +125,20 @@ struct ofputil_requestforward {
};
/* reason == OFPRFR_GROUP_MOD. */
struct ofputil_group_mod *group_mod;
struct {
struct ofputil_group_mod *group_mod;
/* If nonnull, points to the full set of new buckets that resulted
* from a OFPGC15_INSERT_BUCKET or OFPGC15_REMOVE_BUCKET command.
* Needed to translate such group_mods into OpenFlow 1.1-1.4
* OFPGC11_MODIFY. */
const struct ovs_list *new_buckets;
/* If nonnegative, specifies whether the group existed before the
* command was executed. Needed to translate OVS's nonstandard
* OFPGC11_ADD_OR_MOD into a standard command. */
int group_existed;
};
};
};

View File

@@ -65,7 +65,7 @@ ofputil_encode_bundle_msgs(const struct ofputil_bundle_msg *bms,
request = ofputil_encode_flow_mod(&bms[i].fm, protocol);
break;
case OFPTYPE_GROUP_MOD:
request = ofputil_encode_group_mod(version, &bms[i].gm);
request = ofputil_encode_group_mod(version, &bms[i].gm, NULL, -1);
break;
case OFPTYPE_PACKET_OUT:
request = ofputil_encode_packet_out(&bms[i].po, protocol);

View File

@@ -1916,93 +1916,6 @@ ofputil_uninit_group_mod(struct ofputil_group_mod *gm)
ofputil_group_properties_destroy(&gm->props);
}
static struct ofpbuf *
ofputil_encode_ofp11_group_mod(enum ofp_version ofp_version,
const struct ofputil_group_mod *gm)
{
struct ofpbuf *b;
struct ofp11_group_mod *ogm;
size_t start_ogm;
struct ofputil_bucket *bucket;
b = ofpraw_alloc(OFPRAW_OFPT11_GROUP_MOD, ofp_version, 0);
start_ogm = b->size;
ofpbuf_put_zeros(b, sizeof *ogm);
LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
ofputil_put_ofp11_bucket(bucket, b, ofp_version);
}
ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
ogm->command = htons(gm->command);
ogm->type = gm->type;
ogm->group_id = htonl(gm->group_id);
return b;
}
static struct ofpbuf *
ofputil_encode_ofp15_group_mod(enum ofp_version ofp_version,
const struct ofputil_group_mod *gm)
{
struct ofpbuf *b;
struct ofp15_group_mod *ogm;
size_t start_ogm;
struct ofputil_bucket *bucket;
struct id_pool *bucket_ids = NULL;
b = ofpraw_alloc((ofp_version == OFP10_VERSION
? OFPRAW_NXT_GROUP_MOD
: OFPRAW_OFPT15_GROUP_MOD), ofp_version, 0);
start_ogm = b->size;
ofpbuf_put_zeros(b, sizeof *ogm);
LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
uint32_t bucket_id;
/* Generate a bucket id if none was supplied */
if (bucket->bucket_id > OFPG15_BUCKET_MAX) {
if (!bucket_ids) {
const struct ofputil_bucket *bkt;
bucket_ids = id_pool_create(0, OFPG15_BUCKET_MAX + 1);
/* Mark all bucket_ids that are present in gm
* as used in the pool. */
LIST_FOR_EACH_REVERSE (bkt, list_node, &gm->buckets) {
if (bkt == bucket) {
break;
}
if (bkt->bucket_id <= OFPG15_BUCKET_MAX) {
id_pool_add(bucket_ids, bkt->bucket_id);
}
}
}
if (!id_pool_alloc_id(bucket_ids, &bucket_id)) {
OVS_NOT_REACHED();
}
} else {
bucket_id = bucket->bucket_id;
}
ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version);
}
ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
ogm->command = htons(gm->command);
ogm->type = gm->type;
ogm->group_id = htonl(gm->group_id);
ogm->command_bucket_id = htonl(gm->command_bucket_id);
ogm->bucket_array_len = htons(b->size - start_ogm - sizeof *ogm);
/* Add group properties */
if (gm->props.selection_method[0]) {
ofputil_put_group_prop_ntr_selection_method(ofp_version, &gm->props, b);
}
id_pool_destroy(bucket_ids);
return b;
}
static void
bad_group_cmd(enum ofp15_group_mod_command cmd)
{
@@ -2060,11 +1973,137 @@ bad_group_cmd(enum ofp15_group_mod_command cmd)
}
static struct ofpbuf *
ofputil_encode_ofp11_group_mod(enum ofp_version ofp_version,
const struct ofputil_group_mod *gm,
const struct ovs_list *new_buckets,
int group_existed)
{
struct ofpbuf *b;
struct ofp11_group_mod *ogm;
size_t start_ogm;
struct ofputil_bucket *bucket;
b = ofpraw_alloc(OFPRAW_OFPT11_GROUP_MOD, ofp_version, 0);
start_ogm = b->size;
ofpbuf_put_zeros(b, sizeof *ogm);
uint16_t command = gm->command;
const struct ovs_list *buckets = &gm->buckets;
switch (gm->command) {
case OFPGC15_INSERT_BUCKET:
case OFPGC15_REMOVE_BUCKET:
if (!new_buckets) {
bad_group_cmd(gm->command);
}
command = OFPGC11_MODIFY;
buckets = new_buckets;
break;
case OFPGC11_ADD_OR_MOD:
if (group_existed >= 0) {
command = group_existed ? OFPGC11_MODIFY : OFPGC11_ADD;
}
break;
default:
break;
}
LIST_FOR_EACH (bucket, list_node, buckets) {
ofputil_put_ofp11_bucket(bucket, b, ofp_version);
}
ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
ogm->command = htons(command);
ogm->type = gm->type;
ogm->group_id = htonl(gm->group_id);
return b;
}
static struct ofpbuf *
ofputil_encode_ofp15_group_mod(enum ofp_version ofp_version,
const struct ofputil_group_mod *gm,
int group_existed)
{
struct ofpbuf *b;
struct ofp15_group_mod *ogm;
size_t start_ogm;
struct ofputil_bucket *bucket;
struct id_pool *bucket_ids = NULL;
b = ofpraw_alloc((ofp_version == OFP10_VERSION
? OFPRAW_NXT_GROUP_MOD
: OFPRAW_OFPT15_GROUP_MOD), ofp_version, 0);
start_ogm = b->size;
ofpbuf_put_zeros(b, sizeof *ogm);
LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
uint32_t bucket_id;
/* Generate a bucket id if none was supplied */
if (bucket->bucket_id > OFPG15_BUCKET_MAX) {
if (!bucket_ids) {
const struct ofputil_bucket *bkt;
bucket_ids = id_pool_create(0, OFPG15_BUCKET_MAX + 1);
/* Mark all bucket_ids that are present in gm
* as used in the pool. */
LIST_FOR_EACH_REVERSE (bkt, list_node, &gm->buckets) {
if (bkt == bucket) {
break;
}
if (bkt->bucket_id <= OFPG15_BUCKET_MAX) {
id_pool_add(bucket_ids, bkt->bucket_id);
}
}
}
if (!id_pool_alloc_id(bucket_ids, &bucket_id)) {
OVS_NOT_REACHED();
}
} else {
bucket_id = bucket->bucket_id;
}
ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version);
}
ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
ogm->command = htons(gm->command != OFPGC11_ADD_OR_MOD || group_existed < 0
? gm->command
: group_existed ? OFPGC11_MODIFY : OFPGC11_ADD);
ogm->type = gm->type;
ogm->group_id = htonl(gm->group_id);
ogm->command_bucket_id = htonl(gm->command_bucket_id);
ogm->bucket_array_len = htons(b->size - start_ogm - sizeof *ogm);
/* Add group properties */
if (gm->props.selection_method[0]) {
ofputil_put_group_prop_ntr_selection_method(ofp_version, &gm->props, b);
}
id_pool_destroy(bucket_ids);
return b;
}
/* Converts abstract group mod 'gm' into a message for OpenFlow version
* 'ofp_version' and returns the message. */
* 'ofp_version' and returns the message.
*
* If 'new_buckets' is nonnull, it should point to the full set of new buckets
* that resulted from a OFPGC15_INSERT_BUCKET or OFPGC15_REMOVE_BUCKET command.
* It is needed to translate such group_mods into OpenFlow 1.1-1.4
* OFPGC11_MODIFY. If it is null but needed for translation, then encoding the
* group_mod will print an error on stderr and exit().
*
* If 'group_existed' is nonnegative, it should specify whether the group
* existed before the command was executed. If it is nonnegative, then it is
* used to translate OVS's nonstandard OFPGC11_ADD_OR_MOD into a standard
* command. If it is negative, then OFPGC11_ADD_OR_MOD will be left as is. */
struct ofpbuf *
ofputil_encode_group_mod(enum ofp_version ofp_version,
const struct ofputil_group_mod *gm)
const struct ofputil_group_mod *gm,
const struct ovs_list *new_buckets,
int group_existed)
{
switch (ofp_version) {
@@ -2072,15 +2111,13 @@ ofputil_encode_group_mod(enum ofp_version ofp_version,
case OFP12_VERSION:
case OFP13_VERSION:
case OFP14_VERSION:
if (gm->command > OFPGC11_DELETE && gm->command != OFPGC11_ADD_OR_MOD) {
bad_group_cmd(gm->command);
}
return ofputil_encode_ofp11_group_mod(ofp_version, gm);
return ofputil_encode_ofp11_group_mod(ofp_version, gm,
new_buckets, group_existed);
case OFP10_VERSION:
case OFP15_VERSION:
case OFP16_VERSION:
return ofputil_encode_ofp15_group_mod(ofp_version, gm);
return ofputil_encode_ofp15_group_mod(ofp_version, gm, group_existed);
default:
OVS_NOT_REACHED();

View File

@@ -796,7 +796,8 @@ ofputil_encode_requestforward(const struct ofputil_requestforward *rf,
switch (rf->reason) {
case OFPRFR_GROUP_MOD:
inner = ofputil_encode_group_mod(ofp_version, rf->group_mod);
inner = ofputil_encode_group_mod(ofp_version, rf->group_mod,
rf->new_buckets, rf->group_existed);
break;
case OFPRFR_METER_MOD:
@@ -829,6 +830,9 @@ enum ofperr
ofputil_decode_requestforward(const struct ofp_header *outer,
struct ofputil_requestforward *rf)
{
rf->new_buckets = NULL;
rf->group_existed = -1;
struct ofpbuf b = ofpbuf_const_initializer(outer, ntohs(outer->length));
/* Skip past outer message. */
@@ -920,6 +924,7 @@ ofputil_destroy_requestforward(struct ofputil_requestforward *rf)
case OFPRFR_GROUP_MOD:
ofputil_uninit_group_mod(rf->group_mod);
free(rf->group_mod);
/* 'rf' does not own rf->new_buckets. */
break;
case OFPRFR_METER_MOD:

View File

@@ -7393,10 +7393,13 @@ ofproto_group_mod_finish(struct ofproto *ofproto,
remove_groups_postponed(&ogm->old_groups);
if (req) {
struct ofputil_requestforward rf;
rf.xid = req->request->xid;
rf.reason = OFPRFR_GROUP_MOD;
rf.group_mod = &ogm->gm;
struct ofputil_requestforward rf = {
.xid = req->request->xid,
.reason = OFPRFR_GROUP_MOD,
.group_mod = &ogm->gm,
.new_buckets = new_group ? &new_group->buckets : NULL,
.group_existed = group_collection_n(&ogm->old_groups) > 0,
};
connmgr_send_requestforward(ofproto->connmgr, req->ofconn, &rf);
}
}

View File

@@ -777,7 +777,7 @@ add_flow_mod(struct ofputil_flow_mod *fm, struct ovs_list *msgs)
static struct ofpbuf *
encode_group_mod(const struct ofputil_group_mod *gm)
{
return ofputil_encode_group_mod(OFP13_VERSION, gm);
return ofputil_encode_group_mod(OFP13_VERSION, gm, NULL, -1);
}
static void

View File

@@ -2956,7 +2956,8 @@ bundle_group_mod__(const char *remote, struct ofputil_group_mod *gms,
for (i = 0; i < n_gms; i++) {
struct ofputil_group_mod *gm = &gms[i];
struct ofpbuf *request = ofputil_encode_group_mod(version, gm);
struct ofpbuf *request = ofputil_encode_group_mod(version, gm,
NULL, -1);
ovs_list_push_back(&requests, &request->list_node);
ofputil_uninit_group_mod(gm);
@@ -2989,7 +2990,7 @@ ofctl_group_mod__(const char *remote, struct ofputil_group_mod *gms,
for (i = 0; i < n_gms; i++) {
gm = &gms[i];
request = ofputil_encode_group_mod(version, gm);
request = ofputil_encode_group_mod(version, gm, NULL, -1);
transact_noreply(vconn, request);
ofputil_uninit_group_mod(gm);
}