| Message ID | 20170221192253.8607-1-e@erig.me |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Feb 21, 2017 at 02:22:53PM -0500, Eric Garver wrote: > When using set_field for nw_ecn with OF1.0 or OF1.1, you get an error > instead of a proper translation. This use to work before 4b684612d900 > ("ofp-actions: Translate mod_nw_ecn action to OF1.1 properly.") because > it would fallback to using NXM. > > e.g. > > [root@rhel7 ~]# ovs-ofctl -O OpenFlow10 add-flow br0 'ip actions=set_field:2->nw_ecn' > OFPT_ERROR (xid=0x4): OFPBAC_BAD_ARGUMENT > NXT_FLOW_MOD (xid=0x4): > ... > > $ ovs-ofctl -O OpenFlow11 add-flow br0 'ip actions=set_field:2->nw_ecn' > ovs-ofctl: none of the usable flow formats (NXM,OXM) is among the > allowed flow formats (OpenFlow11) > > Fixes: 4b684612d900 ("ofp-actions: Translate mod_nw_ecn action to OF1.1 properly.") > Signed-off-by: Eric Garver <e@erig.me> Thanks a lot! I applied this to master and branch-2.7.
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h index d5c097179d22..d8cbd1bccb60 100644 --- a/include/openvswitch/meta-flow.h +++ b/include/openvswitch/meta-flow.h @@ -1223,6 +1223,7 @@ enum OVS_PACKED_ENUM mf_field_id { * Access: read/write. * NXM: NXM_NX_IP_ECN(28) since v1.4. * OXM: OXM_OF_IP_ECN(9) since OF1.2 and v1.7. + * OF1.1: exact match. */ MFF_IP_ECN, diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 11ab6921763b..767ab1760bb6 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -2804,9 +2804,11 @@ set_field_to_legacy_openflow(const struct ofpact_set_field *sf, put_OFPAT_SET_NW_TOS(out, ofp_version, sf->value->u8 << 2); break; - case MFF_IP_ECN: - put_OFPAT11_SET_NW_ECN(out, sf->value->u8); + case MFF_IP_ECN: { + struct ofpact_ecn ip_ecn = { .ecn = sf->value->u8 }; + encode_SET_IP_ECN(&ip_ecn, ofp_version, out); break; + } case MFF_TCP_SRC: case MFF_UDP_SRC: diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index 6384c48d9ed6..58d540afed62 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -837,6 +837,14 @@ AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [dnl OFPST_FLOW reply (OF1.1): ip actions=mod_nw_ecn:2 ]) + +dnl Check that OF1.2+ set_field to set ECN is translated for earlier OF +dnl versions. +AT_CHECK([ovs-ofctl del-flows br0]) +AT_CHECK([ovs-ofctl -O OpenFlow10 add-flow br0 'ip,actions=set_field:2->ip_ecn']) +AT_CHECK([ovs-ofctl del-flows br0]) +AT_CHECK([ovs-ofctl -O OpenFlow11 add-flow br0 'ip,actions=set_field:2->ip_ecn']) + OVS_VSWITCHD_STOP AT_CLEANUP diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 7e267354ea7c..354878f61384 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -119,7 +119,7 @@ for test_case in \ 'ipv6,nw_tos=0xf0 NXM,OXM' \ 'ip,ip_dscp=0x3c any' \ 'ipv6,ip_dscp=0x3c NXM,OXM' \ - 'ip,nw_ecn=1 NXM,OXM' \ + 'ip,nw_ecn=1 NXM,OXM,OpenFlow11' \ 'ipv6,nw_ecn=1 NXM,OXM' \ 'ip,nw_ttl=5 NXM,OXM' \ 'ipv6,nw_ttl=5 NXM,OXM' \
When using set_field for nw_ecn with OF1.0 or OF1.1, you get an error instead of a proper translation. This use to work before 4b684612d900 ("ofp-actions: Translate mod_nw_ecn action to OF1.1 properly.") because it would fallback to using NXM. e.g. [root@rhel7 ~]# ovs-ofctl -O OpenFlow10 add-flow br0 'ip actions=set_field:2->nw_ecn' OFPT_ERROR (xid=0x4): OFPBAC_BAD_ARGUMENT NXT_FLOW_MOD (xid=0x4): ... $ ovs-ofctl -O OpenFlow11 add-flow br0 'ip actions=set_field:2->nw_ecn' ovs-ofctl: none of the usable flow formats (NXM,OXM) is among the allowed flow formats (OpenFlow11) Fixes: 4b684612d900 ("ofp-actions: Translate mod_nw_ecn action to OF1.1 properly.") Signed-off-by: Eric Garver <e@erig.me> --- include/openvswitch/meta-flow.h | 1 + lib/ofp-actions.c | 6 ++++-- tests/ofp-actions.at | 8 ++++++++ tests/ovs-ofctl.at | 2 +- 4 files changed, 14 insertions(+), 3 deletions(-)