Skip to content

Commit

Permalink
ofctrl: Do not try to program long flows
Browse files Browse the repository at this point in the history
If the flow size is bigger than UINT16_MAX it doesn't
fit into openflow message. Programming of such flow will
fail which results in disconnect of ofctrl. After reconnect
we program everything from scratch, in case the long flow still
remains the cycle continues. This causes the node to be almost
unusable as there will be massive traffic disruptions.

To prevent that check if the flow is within the allowed size.
If not log the flow that would trigger this problem and do not program
it. This isn't a self-healing process, but the chance of this happening
are very slim. Also, any flow that is bigger than allowed size is OVN
bug, and it should be fixed.

Reported-at: https://bugzilla.redhat.com/1955167
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
  • Loading branch information
almusil authored and putnopvut committed Sep 14, 2023
1 parent dd89c87 commit 1821ab7
Showing 1 changed file with 41 additions and 4 deletions.
45 changes: 41 additions & 4 deletions controller/ofctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <config.h>
#include "bitmap.h"
#include "byte-order.h"
#include "coverage.h"
#include "dirs.h"
#include "dp-packet.h"
#include "flow.h"
Expand Down Expand Up @@ -55,6 +56,8 @@

VLOG_DEFINE_THIS_MODULE(ofctrl);

COVERAGE_DEFINE(ofctrl_msg_too_long);

/* An OpenFlow flow. */
struct ovn_flow {
/* Key. */
Expand Down Expand Up @@ -1769,6 +1772,18 @@ ovn_flow_log(const struct ovn_flow *f, const char *action)
}
}

static void
ovn_flow_log_size_err(const struct ovn_flow *f)
{
COVERAGE_INC(ofctrl_msg_too_long);

static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);

char *s = ovn_flow_to_string(f);
VLOG_ERR_RL(&rl, "The FLOW_MOD message is too big: %s", s);
free(s);
}

static void
ovn_flow_uninit(struct ovn_flow *f)
{
Expand Down Expand Up @@ -1888,15 +1903,27 @@ encode_bundle_add(struct ofpbuf *msg, struct ofputil_bundle_ctrl_msg *bc)
return ofputil_encode_bundle_add(OFP15_VERSION, &bam);
}

static void
static bool
add_flow_mod(struct ofputil_flow_mod *fm,
struct ofputil_bundle_ctrl_msg *bc,
struct ovs_list *msgs)
{
struct ofpbuf *msg = encode_flow_mod(fm);
struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc);

uint32_t flow_mod_len = msg->size;
uint32_t bundle_len = bundle_msg->size;

ofpbuf_delete(msg);

if (flow_mod_len > UINT16_MAX || bundle_len > UINT16_MAX) {
ofpbuf_delete(bundle_msg);

return false;
}

ovs_list_push_back(msgs, &bundle_msg->list_node);
return true;
}

/* group_table. */
Expand Down Expand Up @@ -2235,7 +2262,10 @@ installed_flow_add(struct ovn_flow *d,
.new_cookie = htonll(d->cookie),
.command = OFPFC_ADD,
};
add_flow_mod(&fm, bc, msgs);

if (!add_flow_mod(&fm, bc, msgs)) {
ovn_flow_log_size_err(d);
}
}

static void
Expand All @@ -2259,14 +2289,18 @@ installed_flow_mod(struct ovn_flow *i, struct ovn_flow *d,
/* Use OFPFC_ADD so that cookie can be updated. */
fm.command = OFPFC_ADD;
}
add_flow_mod(&fm, bc, msgs);
bool result = add_flow_mod(&fm, bc, msgs);

/* Replace 'i''s actions and cookie by 'd''s. */
mem_stats.installed_flow_usage -= i->ofpacts_len - d->ofpacts_len;
free(i->ofpacts);
i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len);
i->ofpacts_len = d->ofpacts_len;
i->cookie = d->cookie;

if (!result) {
ovn_flow_log_size_err(i);
}
}

static void
Expand All @@ -2280,7 +2314,10 @@ installed_flow_del(struct ovn_flow *i,
.table_id = i->table_id,
.command = OFPFC_DELETE_STRICT,
};
add_flow_mod(&fm, bc, msgs);

if (!add_flow_mod(&fm, bc, msgs)) {
ovn_flow_log_size_err(i);
}
}

static void
Expand Down

0 comments on commit 1821ab7

Please sign in to comment.