[ovs-dev] [PATCH 3/5] ofp-actions: Translate mod_nw_ecn action to OF1.1 properly.

Jarno Rajahalme jarno at ovn.org
Thu Jul 14 10:03:41 UTC 2016


Acked-by: Jarno Rajahalme <jarno at ovn.org>

> On Jul 13, 2016, at 5:06 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> Also, translate OF1.2+ "set_field" on OXM_OF_IP_ECN properly to OF1.1
> "mod_nw_ecn".
> 
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
> NEWS                 |  3 +++
> lib/ofp-actions.c    | 28 +++++++++++++++++++++++-----
> tests/ofp-actions.at | 40 ++++++++++++++++++++++++++++++++++++++++
> tests/ovs-ofctl.at   |  4 ++--
> 4 files changed, 68 insertions(+), 7 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 3c206f7..15fa165 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -19,8 +19,11 @@ Post-v2.5.0
>        packet to size M bytes when outputting to port N.
>      * New command OFPGC_ADD_OR_MOD for OFPT_GROUP_MOD message that adds a
>        new group or modifies an existing groups
> +   - Improved OpenFlow version compatibility for actions:
>      * New OpenFlow extension to support the "group" action in OpenFlow 1.0.
>      * OpenFlow 1.0 "enqueue" action now properly translated to OpenFlow 1.1+.
> +     * OpenFlow 1.1 "mod_nw_ecn" action now properly translated to
> +       OpenFlow 1.0.
>    - ovs-ofctl:
>      * queue-get-config command now allows a queue ID to be specified.
>      * '--bundle' option can now be used with OpenFlow 1.3.
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index 4ac284f..232e728 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -356,6 +356,9 @@ static enum ofperr ofpacts_verify(const struct ofpact[], size_t ofpacts_len,
> static void ofpact_put_set_field(struct ofpbuf *openflow, enum ofp_version,
>                                  enum mf_field_id, uint64_t value);
> 
> +static void put_reg_load(struct ofpbuf *openflow,
> +                         const struct mf_subfield *, uint64_t value);
> +
> static enum ofperr ofpact_pull_raw(struct ofpbuf *, enum ofp_version,
>                                    enum ofp_raw_action_type *, uint64_t *arg);
> static void *ofpact_put_raw(struct ofpbuf *, enum ofp_version,
> @@ -1913,7 +1916,9 @@ encode_SET_IP_ECN(const struct ofpact_ecn *ip_ecn,
> {
>     uint8_t ecn = ip_ecn->ecn;
>     if (ofp_version == OFP10_VERSION) {
> -        /* XXX */
> +        struct mf_subfield dst = { .field = mf_from_id(MFF_IP_ECN),
> +                                   .ofs = 0, .n_bits = 2 };
> +        put_reg_load(out, &dst, ecn);
>     } else if (ofp_version == OFP11_VERSION) {
>         put_OFPAT11_SET_NW_ECN(out, ecn);
>     } else {
> @@ -2604,6 +2609,18 @@ ofpact_put_set_field(struct ofpbuf *openflow, enum ofp_version ofp_version,
>     pad_ofpat(openflow, start_ofs);
> }
> 
> +static void
> +put_reg_load(struct ofpbuf *openflow,
> +             const struct mf_subfield *dst, uint64_t value)
> +{
> +    ovs_assert(dst->n_bits <= 64);
> +
> +    struct nx_action_reg_load *narl = put_NXAST_REG_LOAD(openflow);
> +    narl->ofs_nbits = nxm_encode_ofs_nbits(dst->ofs, dst->n_bits);
> +    narl->dst = htonl(mf_nxm_header(dst->field->id));
> +    narl->value = htonll(value);
> +}
> +
> static bool
> next_load_segment(const struct ofpact_set_field *sf,
>                   struct mf_subfield *dst, uint64_t *value)
> @@ -2648,10 +2665,7 @@ set_field_to_nxast(const struct ofpact_set_field *sf, struct ofpbuf *openflow)
> 
>         dst.ofs = dst.n_bits = 0;
>         while (next_load_segment(sf, &dst, &value)) {
> -            struct nx_action_reg_load *narl = put_NXAST_REG_LOAD(openflow);
> -            narl->ofs_nbits = nxm_encode_ofs_nbits(dst.ofs, dst.n_bits);
> -            narl->dst = htonl(mf_nxm_header(dst.field->id));
> -            narl->value = htonll(value);
> +            put_reg_load(openflow, &dst, value);
>         }
>     }
> }
> @@ -2759,6 +2773,10 @@ 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);
> +        break;
> +
>     case MFF_TCP_SRC:
>     case MFF_UDP_SRC:
>         put_OFPAT_SET_TP_SRC(out, sf->value.be16);
> diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
> index a3b4ccf..b177893 100644
> --- a/tests/ofp-actions.at
> +++ b/tests/ofp-actions.at
> @@ -329,6 +329,9 @@ AT_DATA([test-data], [dnl
> # actions=mod_nw_tos:48
> 0007 0008 30 000000
> 
> +# actions=mod_nw_ecn:2
> +0008 0008 02 000000
> +
> # actions=mod_tp_src:80
> 0009 0008 0050 0000
> 
> @@ -768,3 +771,40 @@ OFPST_FLOW reply (OF1.3):
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> +
> +AT_SETUP([mod_nw_ecn action translation])
> +AT_KEYWORDS([ofp-actions])
> +OVS_VSWITCHD_START
> +
> +dnl OpenFlow 1.1, but no other version, has a "mod_nw_ecn" action.
> +dnl Check that we translate it properly for OF1.0 and OF1.2.
> +dnl (OF1.3+ should be the same as OF1.2.)
> +AT_CHECK([ovs-ofctl -O OpenFlow11 add-flow br0 'ip,actions=mod_nw_ecn:2'])
> +AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl
> +NXST_FLOW reply:
> + ip actions=load:0x2->NXM_NX_IP_ECN[[]]
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [dnl
> +OFPST_FLOW reply (OF1.1):
> + ip actions=mod_nw_ecn:2
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow12 dump-flows br0 | ofctl_strip], [0], [dnl
> +OFPST_FLOW reply (OF1.2):
> + ip actions=set_field:2->nw_ecn
> +])
> +
> +dnl Check that OF1.2+ set_field to set ECN is translated into the OF1.1
> +dnl mod_nw_ecn action.
> +dnl
> +dnl We don't do anything equivalent for OF1.0 reg_load because we prefer
> +dnl that anything that comes in as reg_load gets translated back to reg_load
> +dnl on output.  Perhaps this is somewhat inconsistent but it's what OVS
> +dnl has done for multiple versions.
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip,actions=set_field:2->ip_ecn'])
> +AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [dnl
> +OFPST_FLOW reply (OF1.1):
> + ip actions=mod_nw_ecn:2
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index e3d3e7a..23effd6 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -184,7 +184,7 @@ tcp,nw_src=192.168.0.3,tp_dst=80 actions=set_queue:37,output:1
> udp,nw_src=192.168.0.3,tp_dst=53 actions=pop_queue,output:1
> cookie=0x123456789abcdef hard_timeout=10 priority=60000 actions=controller
> actions=note:41.42.43,note:00.01.02.03.04.05.06.07,note
> -ip,actions=set_field:10.4.3.77->ip_src
> +ip,actions=set_field:10.4.3.77->ip_src,mod_nw_ecn:2
> sctp actions=drop
> sctp actions=drop
> in_port=0 actions=resubmit:0
> @@ -215,7 +215,7 @@ OFPT_FLOW_MOD: ADD tcp,nw_src=192.168.0.3,tp_dst=80 actions=set_queue:37,output:
> OFPT_FLOW_MOD: ADD udp,nw_src=192.168.0.3,tp_dst=53 actions=pop_queue,output:1
> OFPT_FLOW_MOD: ADD priority=60000 cookie:0x123456789abcdef hard:10 actions=CONTROLLER:65535
> OFPT_FLOW_MOD: ADD actions=note:41.42.43.00.00.00,note:00.01.02.03.04.05.06.07.00.00.00.00.00.00,note:00.00.00.00.00.00
> -OFPT_FLOW_MOD: ADD ip actions=mod_nw_src:10.4.3.77
> +OFPT_FLOW_MOD: ADD ip actions=mod_nw_src:10.4.3.77,load:0x2->NXM_NX_IP_ECN[]
> OFPT_FLOW_MOD: ADD sctp actions=drop
> OFPT_FLOW_MOD: ADD sctp actions=drop
> OFPT_FLOW_MOD: ADD in_port=0 actions=resubmit:0
> -- 
> 2.1.3
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list