mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 22:35:15 +00:00
Code Issues Releases Wiki Activity

ofp-actions: Don't encode variable length fields using NXAST_REG_LOAD.

Browse Source 
Currently, when using an OpenFlow 1.0 connection to encode a
tunnel metadata set field action, a series of NXAST_REG_LOADs
are emitted. The result is something like this:

actions=load:0xa->NXM_NX_TUN_METADATA0[0..63],load:0->
NXM_NX_TUN_METADATA0[64..127],load:0->NXM_NX_TUN_METADATA0[128..191],
load:0->NXM_NX_TUN_METADATA0[192..255],load:0->NXM_NX_TUN_METADATA0
[256..319],load:0->NXM_NX_TUN_METADATA0[320..383],load:0->
NXM_NX_TUN_METADATA0[384..447],load:0->NXM_NX_TUN_METADATA0[448..511],
load:0->NXM_NX_TUN_METADATA0[512..575],load:0->NXM_NX_TUN_METADATA0
[576..639],load:0->NXM_NX_TUN_METADATA0[640..703],load:0->
NXM_NX_TUN_METADATA0[704..767],load:0->NXM_NX_TUN_METADATA0[768..831],
load:0->NXM_NX_TUN_METADATA0[832..895],load:0->NXM_NX_TUN_METADATA0
[896..959],load:0->NXM_NX_TUN_METADATA0[960..991]

This happens because tunnel metadata is seen as a maximum size field
and so many loads need to be emitted to cover the entire thing. Besides
being ugly (this shows up when using ovs-ofctl in the default
configuration), it exposes the internal size of the field. While this
shouldn't be an issue since specific protocol fields (such as Geneve
options) have fixed max sizes even if the OVS implementation is extended,
it's still not a great idea.

If we instead use NXAST_REG_LOAD2 in cases where there isn't a suitable
OpenFlow alternative, both problems are avoided:

actions=set_field:0xa->tun_metadata0

This prefers NXAST_REG_LOAD2 for variable length fields since they would
all generally have the same problems. In addition, since the concept of
this type of field is fairly new, there are no backwards compatibility
issues.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
This commit is contained in:
Jesse Gross
2015-08-27 18:40:45 -07:00
parent abd0694c5d
commit 4fe04a7e6d
1 changed files with 4 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
lib/ofp-actions.c
View File

@@ -2271,11 +2271,11 @@ static void
set_field_to_nxast(const struct ofpact_set_field *sf, struct ofpbuf *openflow)
{
/* If 'sf' cannot be encoded as NXAST_REG_LOAD because it requires an
* experimenter OXM (or if it came in as NXAST_REG_LOAD2), encode as
* NXAST_REG_LOAD2. Otherwise use NXAST_REG_LOAD, which is backward
* compatible. */
* experimenter OXM or is variable length (or if it came in as
* NXAST_REG_LOAD2), encode as NXAST_REG_LOAD2. Otherwise use
* NXAST_REG_LOAD, which is backward compatible. */
if (sf->ofpact.raw == NXAST_RAW_REG_LOAD2
|| !mf_nxm_header(sf->field->id)) {
|| !mf_nxm_header(sf->field->id) || sf->field->variable_len) {
struct nx_action_reg_load2 *narl OVS_UNUSED;
size_t start_ofs = openflow->size;