[ovs-dev] [PATCH 15/31] fixup: Change from global modal behavior to L3 tunnel only mode.

Jan Scheurich jan.scheurich at ericsson.com
Wed Jun 14 22:48:50 UTC 2017


We do understand your motivation for proposing this. But such change is a significant and requires careful consideration. We need to satisfy four main requirements:



1. Legacy controllers that are not packet-type aware must not be affected by the introduction of packet_type in OVS. The externally visible behavior of OVS must not change as long as OVS used in "legacy" mode:

* Matching works exactly as without support packet_type as long as the pipeline handles only Ethernet packets.

* The controller shall not be exposed to unexpected packet_type fields in e.g. flow or packet in messages.



2. Packet type-unaware controllers shall be able to use legacy L3 tunnels as in the past (LISP OVS 2.7, or with the just merged series for L3 tunneling in user space), i.e. implicit conversion to/from Ethernet for processing in the pipeline without controller involvement.



3. Packet-type aware controllers shall be able to use versatile tunnel ports directly without implicit conversion of packet types through OVS.



4. The tunnel handling logic should be easily understandable not only for OVS experts but also for controller developers of the two camps (packet type-aware and unaware).



We think backward compatibility requirement (1) can be fulfilled even if an OVS bridge is always packet type-ware, provided that the default packet_type (0,0) is filtered away in all OF matches sent out by OVS (flow messages and packet in). (Note: It is not quite obvious that packet type-aware controllers will recognize flows if they have explicitly included a packet_type=(0,0) match in the flows they sent. But we have the reciprocal problem in our previous patch also).



Regarding your proposed renaming of the alternative bridge property: We think the name used in the OVSDB schema XML "legacy-l3-tunnel" (or perhaps "legacy-l3-tunneling"?) is more appropriate than the name "legacy-l3-pipeline" that is actually used in the code (and with negation between OVSDB and ofproto).



It seems clear that (2) and (3) together require some config knob to control the implicit packet type conversion for legacy controllers. If bridge property packet-type-aware is not available for this, we either need a different bridge property (as in your proposal) or we have to configure this per port.



We can assume that the legacy L3 tunnel handling behavior (2) is set. In your proposal this is selected through "legacy-l3-tunnel=true". Tunnel ports operating in this mode are either L2 only or L3 only and the behavior is determined through the "layer3" option per tunnel. The semantics are summarized by the table below:



layer3                    Rx from tunnel                                                  Tx to tunnel


false                      Ethernet packets are processed as is,       Ethernet packet are sent as is,
                                L3 packets are dropped.                                L3 packets are dropped.


true                       L3 packets are encapsulated in                   The Ethernet header is stripped
                                dummy Ethernet header for pipeline       before transmission to tunnel,
                                processing,                                                         L3 packets are dropped.
                                Ethernet packets are dropped.



The new L3/versatile tunnel port handling for packet-type-aware controllers is as follows:



                                Rx from tunnel                                                  Tx to tunnel



                                All packets that can be mapped to a         All packet types are sent as is w/o
                                known packet type (0,0) or (1,x) are         implicit type conversion. If the tunnel
                                processed as is.                                                 does not support the packet type as
                                Other packets are dropped.                        payload, packets are dropped.



Our understanding is that you intend to apply this behavior for "legacy-l3-tunnel=false". It is not clear from the documentation (patch 20/31) what the meaning of the "layer3" tunnel option should be in this case. You say "tunnel ports configured as layer3 can only send and receive L3 packets." without detailing what that means.



In our view there are only three possibilities that might make some sense:



1. Ignore "layer3" option when "legacy-l3-tunnel=false" and always apply the above handling of versatile tunnels. As the "layer3" option was introduced to decide the transmit behavior for versatile tunnel protocols in legacy L3 tunneling, it seems logical to not attach an artificial meaning to it in the new L3 tunneling scheme built on packet_type.



2. Apply above handling of versatile ports for "legacy-l3-tunnel=false" and "layer3=false". For "layer3=true" apply the same handling as for versatile ports but limit send and receive to L3 packets only and drop Ethernet packets. This would be consistent with the statement that the port is limited to L3 packets, but it seems to add little value to be able to "castrate" a versatile tunnel port to L3 by configuration. Have you got a use case in mind?


3. Apply above handling of versatile ports for "legacy-l3-tunnel=false" and "layer3=false". For "layer3=true" ignore that "legacy-l3-tunnel=false" and apply the same implicit conversion to/from Ethernet in the pipeline as for legacy L3 tunneling. It seems rather counter-intuitive to do this when legacy L3 tunneling is turned off.



So, from the above we would tend to favor option 1. In your code you actually implement a mixture of 2 and 3: implicitly convert to Ethernet at Rx from layer3 tunnel but transmit Ethernet packets as is to layer3 tunnel. We believe this was unintentional.



The alternative to having both bridge property "legacy-l3-tunnel" and a port property "layer3" would be to go for a single port property "tunnel-mode" with three values "layer2" (default), "layer3" and "versatile". "layer2" tunnel ports would only send and receive Ethernet packets as is. "layer3" tunnel ports would send and receive L3 packets with implicit conversion from/to Ethernet in the pipeline. Finally, "versatile" tunnel ports would send receive any type of supported packet as is without implicit conversion. In future versions of OVS the default could be changed to "versatile".



We think that this approach would also fulfill requirements 1, 2 and 3. Our feeling is that this approach would also be somewhat easier to explain to users. It would not be backward compatible to the just introduced Boolean "layer3" flag, but that has not been released yet, so perhaps it would be ok.



What do you think?



> -----Original Message-----

> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org] On Behalf Of Ben Pfaff

> Sent: Tuesday, 13 June, 2017 00:29

> To: dev at openvswitch.org

> Cc: Ben Pfaff <blp at ovn.org>

> Subject: [ovs-dev] [PATCH 15/31] fixup: Change from global modal behavior to L3 tunnel only mode.

>

> The policy originally proposed by this patch series was to make support for

> a packet type aware pipeline a property of each bridge, via

> other-config:packet-type-aware.  Global modes like this have implications

> for testing, that mean that many features need to be tested in both modes.

>

> This commit changes to a policy in which the only mode is the behavior for

> L3 tunnels.  Other than the behavior for these tunnels, bridges behave the

> same way in both modes.  This should also allow for backward compatibility

> with legacy controllers, but it should also make testing the system more

> straightforward, especially since the actual differences in behavior

> between the two modes are much smaller.

>

> With this change, OVS adopts the following behavior:

>

> - Any incoming flow that matches on a data field, that does not match on

>   packet_type, implicitly matches on packet_type(0,0).

>

> - If an outgoing flow matches on a data field and on packet_type(0,0),

>   OVS omits the match on packet_type(0,0) from the outgoing flow.

>

> This allows for a great deal of backward compatibility: only non-Ethernet

> packet_type flows are really affected on input or on output.

>

> Signed-off-by: Ben Pfaff <blp at ovn.org<mailto:blp at ovn.org>>

> ---

>  include/openvswitch/match.h      |  2 ++

>  include/openvswitch/ofp-errors.h |  3 +--

>  lib/learn.c                      |  1 +

>  lib/match.c                      | 24 ++++++++++++++++++++

>  lib/meta-flow.xml                | 49 +++++++++++++++++++---------------------

>  lib/nx-match.c                   | 28 +++++++++++++++--------

>  lib/nx-match.h                   |  4 ++--

>  lib/ofp-parse.c                  |  4 ++++

>  lib/ofp-util.c                   | 31 +++++++++++++++++++------

>  ofproto/ofproto-dpif-xlate.c     | 21 +++++++++--------

>  ofproto/ofproto-dpif-xlate.h     |  2 +-

>  ofproto/ofproto-dpif.c           |  6 ++---

>  ofproto/ofproto-provider.h       |  6 ++---

>  ofproto/ofproto.c                | 27 ++++++----------------

>  ofproto/ofproto.h                |  2 +-

>  tests/ofproto.at                 |  1 +

>  tests/ovs-ofctl.at               |  2 +-

>  vswitchd/bridge.c                | 11 +++++----

>  vswitchd/vswitch.xml             | 20 ++++++++++++----

>  19 files changed, 149 insertions(+), 95 deletions(-)

>

> diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h

> index 58ca5848b127..da2f780c3ba5 100644

> --- a/include/openvswitch/match.h

> +++ b/include/openvswitch/match.h

> @@ -23,6 +23,7 @@

>

>  struct ds;

>  struct ofputil_port_map;

> +struct mf_field;

>

>  /* A flow classification match.

>   *

> @@ -119,6 +120,7 @@ void match_set_ct_ipv6_dst_masked(struct match *, const struct in6_addr *,

>  void match_set_packet_type(struct match *, ovs_be32 packet_type);

>  void match_set_default_packet_type(struct match *);

>  bool match_has_default_packet_type(const struct match *);

> +void match_add_ethernet_prereq(struct match *, const struct mf_field *);

>

>  void match_set_skb_priority(struct match *, uint32_t skb_priority);

>  void match_set_dl_type(struct match *, ovs_be16);

> diff --git a/include/openvswitch/ofp-errors.h b/include/openvswitch/ofp-errors.h

> index 7d75386352d8..aeb58e012fd3 100644

> --- a/include/openvswitch/ofp-errors.h

> +++ b/include/openvswitch/ofp-errors.h

> @@ -311,8 +311,7 @@ enum ofperr {

>  /* ## OFPET_BAD_MATCH ## */

>  /* ## --------------- ## */

>

> -    /* NX1.0(4,264), OF1.1+(4,0).  Unsupported match type specified by the

> -     * match */

> +    /* OF1.1+(4,0).  Unsupported match type specified by the match */

>      OFPERR_OFPBMC_BAD_TYPE,

>

>      /* OF1.1+(4,1).  Length problem in match. */

> diff --git a/lib/learn.c b/lib/learn.c

> index bc5a6eb2d16f..4658b8611857 100644

> --- a/lib/learn.c

> +++ b/lib/learn.c

> @@ -139,6 +139,7 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow,

>          switch (spec->dst_type) {

>          case NX_LEARN_DST_MATCH:

>              mf_write_subfield(&spec->dst, &value, &fm->match);

> +            match_add_ethernet_prereq(&fm->match, spec->dst.field);

>              mf_vl_mff_set_tlv_bitmap(

>                  spec->dst.field, &fm->match.flow.tunnel.metadata.present.map);

>              break;

> diff --git a/lib/match.c b/lib/match.c

> index 0e23eb6e6069..6ea6cf0519ca 100644

> --- a/lib/match.c

> +++ b/lib/match.c

> @@ -502,6 +502,30 @@ match_has_default_packet_type(const struct match *match)

>              && match->wc.masks.packet_type == OVS_BE32_MAX);

>  }

>

> +/* A match on 'field' is being added to or has been added to 'match'.  If

> + * 'field' is a data field, and 'match' does not already match on packet_type,

> + * this function make it match on the Ethernet packet_type.

> + *

> + * This function is useful because OpenFlow implicitly applies to Ethernet

> + * packets when there's no explicit packet_type, but matching on a metadata

> + * field doesn't imply anything about the packet_type and falsely inferring

> + * that it does can cause harm.  A flow that matches only on metadata fields,

> + * for example, should be able to match more than just Ethernet flows.  There

> + * are also important reasons that a catch-all match (one with no field matches

> + * at all) should not imply a packet_type(0,0) match.  For example, a "flow

> + * dump" request that matches on no fields should return every flow in the

> + * switch, not just the flows that match on Ethernet.  As a second example,

> + * OpenFlow 1.2+ special-cases "table miss" flows, that is catch-all flows with

> + * priority 0, and inferring a match on packet_type(0,0) causes such a flow not

> + * to be a table miss flow.  */

> +void

> +match_add_ethernet_prereq(struct match *match, const struct mf_field *field)

> +{

> +    if (field->prereqs != MFP_NONE) {

> +        match_set_default_packet_type(match);

> +    }

> +}

> +

>  void

>  match_set_dl_type(struct match *match, ovs_be16 dl_type)

>  {

> diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml

> index 283a1cb0bcb8..0c93c88c5939 100644

> --- a/lib/meta-flow.xml

> +++ b/lib/meta-flow.xml

> @@ -26,19 +26,25 @@

>      networking technology in use are called called <dfn>root fields</dfn>.

>      Open vSwitch 2.7 and earlier considered Ethernet fields to be root fields,

>      and this remains the default mode of operation for Open vSwitch bridges.

> -    In this mode, when a packet is received from a non-Ethernet interfaces,

> -    such as a layer-3 LISP or GRE tunnel, Open vSwitch force-fits it to this

> -    Ethernet-centric point of view by pretending that an Ethernet header is

> -    present whose Ethernet type that indicates the packet's actual type (and

> +    When a packet is received from a non-Ethernet interfaces, such as a layer-3

> +    LISP or GRE tunnel, Open vSwitch 2.7 and earlier force-fit the packet to

> +    this Ethernet-centric point of view by pretending that an Ethernet header

> +    is present whose Ethernet type that indicates the packet's actual type (and

>      whose source and destination addresses are all-zero).

>    </p>

>

>    <p>

> -    Open vSwitch 2.8 and later supports the ``packet type-aware pipeline''

> -    concept introduced in OpenFlow 1.5.  A bridge configured to be packet

> -    type-aware can handle packets of multiple networking technologies, such as

> -    Ethernet, IP, ARP, MPLS, or NSH in parallel.  Such a bridge does not have

> -    any root fields.

> +    Open vSwitch 2.8 and later implement the ``packet type-aware pipeline''

> +    concept introduced in OpenFlow 1.5.  Such a pipeline does not have any root

> +    fields.  Instead, a new metadata field, <ref field="packet_type"/>,

> +    indicates the basic type of the packet, which can be Ethernet, IPv4, IPv6,

> +    or another type.  For backward compatibility, by default Open vSwitch 2.8

> +    imitates the behavior of Open vSwitch 2.7 and earlier.  Later versions of

> +    Open vSwitch may change the default, and in the meantime controllers can

> +    turn off this legacy behavior by setting

> +    <code>other-config:legacy-l3-tunnel</code> to <code>false</code> in the

> +    <code>Bridge</code> table.  (See <code>ovs-vwitchd.conf.db</code>(5) for

> +    more information.)

>    </p>

>

>    <p>

> @@ -332,14 +338,6 @@ tcp,tp_src=0x07c0/0xfff0

>      <dt><code>mplsm</code></dt>  <dd><code>eth_type=0x8848</code></dd>

>    </dl>

>

> -  <p>

> -    These shorthand notations continue to work in packet type-aware bridges.

> -    The absence of a packet_type match implies

> -    <code>packet_type=ethernet</code>, so that shorthands match on Ethernet

> -    packets with the implied eth_type. Please note that the shorthand

> -    <code>ip</code> does not match packets of packet_type (1,0x800) for IPv4.

> -  </p>

> -

>

>    <h2>Evolution of OpenFlow Fields</h2>

>

> @@ -2374,15 +2372,14 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)

>        </diagram>

>

>        <p>

> -        When the OVS bridge is configured to be packet type-aware, <ref

> -        field="packet_type"/> is a pre-requisite for all data field matches.

> -        As a special case to simplify the common case, and for backward

> -        compatibility, when no such match is supplied, Open vSwitch acts as

> -        though a match on <code>(0,0)</code> (Ethernet) had been supplied.

> -      </p>

> -

> -      <p>

> -        Open vSwitch allows any table to match this field.

> +        Matching on <ref field="packet_type"/> is a pre-requisite for matching

> +        on any data field, but for backward compatibility, when a match on a

> +        data field is present without a <ref field="packet_type"/> match, Open

> +        vSwitch acts as though a match on <code>(0,0)</code> (Ethernet) had

> +        been supplied.  Similarly, when Open vSwitch sends flow match

> +        information to a controller, e.g. in a reply to a request to dump the

> +        flow table, Open vSwitch omits a match on packet type (0,0) if it would

> +        be implied by a data field match.

>        </p>

>      </field>

>

> diff --git a/lib/nx-match.c b/lib/nx-match.c

> index daed882852aa..cb0cad8458b9 100644

> --- a/lib/nx-match.c

> +++ b/lib/nx-match.c

> @@ -561,6 +561,8 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict,

>                  free(err_str);

>                  return OFPERR_OFPBMC_BAD_VALUE;

>              }

> +

> +            match_add_ethernet_prereq(match, field);

>          }

>

>          if (error) {

> @@ -775,6 +777,7 @@ oxm_pull_field_array(const void *fields_data, size_t fields_len,

>

>  struct nxm_put_ctx {

>      struct ofpbuf *output;

> +    bool implied_ethernet;

>  };

>

>  void

> @@ -795,6 +798,9 @@ nxm_put__(struct nxm_put_ctx *ctx,

>            const void *value, const void *mask, size_t n_bytes)

>  {

>      nxm_put_entry_raw(ctx->output, field, version, value, mask, n_bytes);

> +    if (!ctx->implied_ethernet && mf_from_id(field)->prereqs != MFP_NONE) {

> +        ctx->implied_ethernet = true;

> +    }

>  }

>

>  static void

> @@ -1021,7 +1027,7 @@ nx_put_raw(struct ofpbuf *b, enum ofp_version oxm, const struct match *match,

>

>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 39);

>

> -    struct nxm_put_ctx ctx = { .output = b };

> +    struct nxm_put_ctx ctx = { .output = b, .implied_ethernet = false };

>

>      /* OpenFlow Packet Type. Must be first. */

>      if (match->wc.masks.packet_type && !match_has_default_packet_type(match)) {

> @@ -1206,6 +1212,16 @@ nx_put_raw(struct ofpbuf *b, enum ofp_version oxm, const struct match *match,

>          }

>      }

>

> +    if (match_has_default_packet_type(match) && !ctx.implied_ethernet) {

> +        uint64_t pt_stub[16 / 8];

> +        struct ofpbuf pt;

> +        ofpbuf_use_stack(&pt, pt_stub, sizeof pt_stub);

> +        nxm_put_entry_raw(&pt, MFF_PACKET_TYPE, oxm, &flow->packet_type,

> +                          NULL, sizeof flow->packet_type);

> +

> +        ofpbuf_insert(b, start_len, pt.data, pt.size);

> +    }

> +

>      match_len = b->size - start_len;

>      return match_len;

>  }

> @@ -2103,15 +2119,12 @@ oxm_writable_fields(void)

>  /* Returns a bitmap of fields that can be encoded in OXM and that can be

>   * matched in a flow table.  */

>  struct mf_bitmap

> -oxm_matchable_fields(bool packet_type_aware)

> +oxm_matchable_fields(void)

>  {

>      struct mf_bitmap b = MF_BITMAP_INITIALIZER;

>      int i;

>

>      for (i = 0; i < MFF_N_IDS; i++) {

> -        if (i == MFF_PACKET_TYPE && !packet_type_aware) {

> -            continue;

> -        }

>          if (mf_oxm_header(i, 0)) {

>              bitmap_set1(b.bm, i);

>          }

> @@ -2122,15 +2135,12 @@ oxm_matchable_fields(bool packet_type_aware)

>  /* Returns a bitmap of fields that can be encoded in OXM and that can be

>   * matched in a flow table with an arbitrary bitmask.  */

>  struct mf_bitmap

> -oxm_maskable_fields(bool packet_type_aware)

> +oxm_maskable_fields(void)

>  {

>      struct mf_bitmap b = MF_BITMAP_INITIALIZER;

>      int i;

>

>      for (i = 0; i < MFF_N_IDS; i++) {

> -        if (i == MFF_PACKET_TYPE && !packet_type_aware) {

> -            continue;

> -        }

>          if (mf_oxm_header(i, 0) && mf_from_id(i)->maskable == MFM_FULLY) {

>              bitmap_set1(b.bm, i);

>          }

> diff --git a/lib/nx-match.h b/lib/nx-match.h

> index c6f2b26db8ea..e304072d3532 100644

> --- a/lib/nx-match.h

> +++ b/lib/nx-match.h

> @@ -149,8 +149,8 @@ ovs_be64 oxm_bitmap_from_mf_bitmap(const struct mf_bitmap *, enum ofp_version);

>  struct mf_bitmap oxm_bitmap_to_mf_bitmap(ovs_be64 oxm_bitmap,

>                                           enum ofp_version);

>  struct mf_bitmap oxm_writable_fields(void);

> -struct mf_bitmap oxm_matchable_fields(bool packet_type_aware);

> -struct mf_bitmap oxm_maskable_fields(bool packet_type_aware);

> +struct mf_bitmap oxm_matchable_fields(void);

> +struct mf_bitmap oxm_maskable_fields(void);

>

>  /* Dealing with the 'ofs_nbits' members in several Nicira extensions. */

>

> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c

> index 8467be7fe2b5..8e2448b20dbd 100644

> --- a/lib/ofp-parse.c

> +++ b/lib/ofp-parse.c

> @@ -251,6 +251,7 @@ parse_field(const struct mf_field *mf, const char *s,

>      error = mf_parse(mf, s, port_map, &value, &mask);

>      if (!error) {

>          *usable_protocols &= mf_set(mf, &value, &mask, match, &error);

> +        match_add_ethernet_prereq(match, mf);

>      }

>      return error;

>  }

> @@ -297,6 +298,8 @@ parse_subfield(const char *name, const char *str_value, struct match *match,

>          bitwise_copy(&val, size, 0, &value, size, sf.ofs, sf.n_bits);

>          bitwise_one (               &mask,  size, sf.ofs, sf.n_bits);

>          *usable_protocols &= mf_set(field, &value, &mask, match, &error);

> +

> +        match_add_ethernet_prereq(match, sf.field);

>      }

>      return error;

>  }

> @@ -416,6 +419,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,

>              if (p->nw_proto) {

>                  match_set_nw_proto(&fm->match, p->nw_proto);

>              }

> +            match_set_default_packet_type(&fm->match);

>          } else if (!strcmp(name, "eth")) {

>              match_set_packet_type(&fm->match, htonl(PT_ETH));

>          } else if (fields & F_FLAGS && !strcmp(name, "send_flow_rem")) {

> diff --git a/lib/ofp-util.c b/lib/ofp-util.c

> index 941000c24745..344b66a30d0c 100644

> --- a/lib/ofp-util.c

> +++ b/lib/ofp-util.c

> @@ -161,6 +161,19 @@ ofputil_match_from_ofp10_match(const struct ofp10_match *ofmatch,

>      ofputil_wildcard_from_ofpfw10(ofpfw, &match->wc);

>      memset(&match->tun_md, 0, sizeof match->tun_md);

>

> +    /* If any fields, except in_port, are matched, then we also need to match

> +     * on the Ethernet packet_type. */

> +    const uint32_t ofpfw_data_bits = (OFPFW10_NW_TOS | OFPFW10_NW_PROTO

> +                                      | OFPFW10_TP_SRC | OFPFW10_TP_DST

> +                                      | OFPFW10_DL_SRC | OFPFW10_DL_DST

> +                                      | OFPFW10_DL_TYPE

> +                                      | OFPFW10_DL_VLAN | OFPFW10_DL_VLAN_PCP);

> +    if ((ofpfw & ofpfw_data_bits) != ofpfw_data_bits

> +        || ofputil_wcbits_to_netmask(ofpfw >> OFPFW10_NW_SRC_SHIFT)

> +        || ofputil_wcbits_to_netmask(ofpfw >> OFPFW10_NW_DST_SHIFT)) {

> +        match_set_default_packet_type(match);

> +    }

> +

>      /* Initialize most of match->flow. */

>      match->flow.nw_src = ofmatch->nw_src;

>      match->flow.nw_dst = ofmatch->nw_dst;

> @@ -328,6 +341,7 @@ ofputil_match_from_ofp11_match(const struct ofp11_match *ofmatch,

>      bool ipv4, arp, rarp;

>

>      match_init_catchall(match);

> +    match->flow.tunnel.metadata.tab = NULL;

>

>      if (!(wc & OFPFW11_IN_PORT)) {

>          ofp_port_t ofp_port;

> @@ -340,10 +354,13 @@ ofputil_match_from_ofp11_match(const struct ofp11_match *ofmatch,

>          match_set_in_port(match, ofp_port);

>      }

>

> -    match_set_dl_src_masked(match, ofmatch->dl_src,

> -                            eth_addr_invert(ofmatch->dl_src_mask));

> -    match_set_dl_dst_masked(match, ofmatch->dl_dst,

> -                            eth_addr_invert(ofmatch->dl_dst_mask));

> +    struct eth_addr dl_src_mask = eth_addr_invert(ofmatch->dl_src_mask);

> +    struct eth_addr dl_dst_mask = eth_addr_invert(ofmatch->dl_dst_mask);

> +    if (!eth_addr_is_zero(dl_src_mask) || !eth_addr_is_zero(dl_dst_mask)) {

> +        match_set_dl_src_masked(match, ofmatch->dl_src, dl_src_mask);

> +        match_set_dl_dst_masked(match, ofmatch->dl_dst, dl_dst_mask);

> +        match_set_default_packet_type(match);

> +    }

>

>      if (!(wc & OFPFW11_DL_VLAN)) {

>          if (ofmatch->dl_vlan == htons(OFPVID11_NONE)) {

> @@ -375,11 +392,13 @@ ofputil_match_from_ofp11_match(const struct ofp11_match *ofmatch,

>                  }

>              }

>          }

> +        match_set_default_packet_type(match);

>      }

>

>      if (!(wc & OFPFW11_DL_TYPE)) {

>          match_set_dl_type(match,

>                            ofputil_dl_type_from_openflow(ofmatch->dl_type));

> +        match_set_default_packet_type(match);

>      }

>

>      ipv4 = match->flow.dl_type == htons(ETH_TYPE_IP);

> @@ -7688,9 +7707,7 @@ ofputil_normalize_match__(struct match *match, bool may_log)

>

>      /* Figure out what fields may be matched. */

>      /* Check the packet_type first and extract dl_type. */

> -    if (wc.masks.packet_type == 0 ||

> -        (wc.masks.packet_type == OVS_BE32_MAX &&

> -         match->flow.packet_type == htonl(PT_ETH))) {

> +    if (wc.masks.packet_type == 0 || match_has_default_packet_type(match)) {

>          may_match = MAY_ETHER;

>          dl_type = match->flow.dl_type;

>      } else if (wc.masks.packet_type == OVS_BE32_MAX &&

> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c

> index 149b47cca9a9..d18d58c87c20 100644

> --- a/ofproto/ofproto-dpif-xlate.c

> +++ b/ofproto/ofproto-dpif-xlate.c

> @@ -108,7 +108,7 @@ struct xbridge {

>

>      bool has_in_band;             /* Bridge has in band control? */

>      bool forward_bpdu;            /* Bridge forwards STP BPDUs? */

> -    bool packet_type_aware;       /* Bridge is packet-type-aware? */

> +    bool legacy_l3_pipeline;      /* Treat L3 tunnel packets as L2? */

>

>      /* Datapath feature support. */

>      struct dpif_backer_support support;

> @@ -556,7 +556,7 @@ static void xlate_xbridge_set(struct xbridge *, struct dpif *,

>                                const struct dpif_ipfix *,

>                                const struct netflow *,

>                                bool forward_bpdu, bool has_in_band,

> -                              bool packet_type_aware,

> +                              bool legacy_l3_pipeline,

>                                const struct dpif_backer_support *);

>  static void xlate_xbundle_set(struct xbundle *xbundle,

>                                enum port_vlan_mode vlan_mode,

> @@ -817,7 +817,7 @@ xlate_xbridge_set(struct xbridge *xbridge,

>                    const struct dpif_ipfix *ipfix,

>                    const struct netflow *netflow,

>                    bool forward_bpdu, bool has_in_band,

> -                  bool packet_type_aware,

> +                  bool legacy_l3_pipeline,

>                    const struct dpif_backer_support *support)

>  {

>      if (xbridge->ml != ml) {

> @@ -863,7 +863,7 @@ xlate_xbridge_set(struct xbridge *xbridge,

>      xbridge->dpif = dpif;

>      xbridge->forward_bpdu = forward_bpdu;

>      xbridge->has_in_band = has_in_band;

> -    xbridge->packet_type_aware = packet_type_aware;

> +    xbridge->legacy_l3_pipeline = legacy_l3_pipeline;

>      xbridge->support = *support;

>  }

>

> @@ -954,7 +954,7 @@ xlate_xbridge_copy(struct xbridge *xbridge)

>                        xbridge->rstp, xbridge->ms, xbridge->mbridge,

>                        xbridge->sflow, xbridge->ipfix, xbridge->netflow,

>                        xbridge->forward_bpdu, xbridge->has_in_band,

> -                      xbridge->packet_type_aware, &xbridge->support);

> +                      xbridge->legacy_l3_pipeline, &xbridge->support);

>      LIST_FOR_EACH (xbundle, list_node, &xbridge->xbundles) {

>          xlate_xbundle_copy(new_xbridge, xbundle);

>      }

> @@ -1106,7 +1106,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,

>                    const struct dpif_ipfix *ipfix,

>                    const struct netflow *netflow,

>                    bool forward_bpdu, bool has_in_band,

> -                  bool packet_type_aware,

> +                  bool legacy_l3_pipeline,

>                    const struct dpif_backer_support *support)

>  {

>      struct xbridge *xbridge;

> @@ -1125,7 +1125,8 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,

>      xbridge->name = xstrdup(name);

>

>      xlate_xbridge_set(xbridge, dpif, ml, stp, rstp, ms, mbridge, sflow, ipfix,

> -                      netflow, forward_bpdu, has_in_band, packet_type_aware, support);

> +                      netflow, forward_bpdu, has_in_band, legacy_l3_pipeline,

> +                      support);

>  }

>

>  static void

> @@ -3356,9 +3357,9 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,

>          return;

>      }

>

> -    /* In a bridge that is not packet type-aware, convert the packet to what

> -     * the output port expects. */

> -    if (!ctx->xbridge->packet_type_aware) {

> +    /* Honor the legacy_l3_pipeline setting, by treating an L3 packet as

> +     * L2.  */

> +    if (!ctx->xbridge->legacy_l3_pipeline) {

>          if (flow->packet_type == htonl(PT_ETH) && xport->is_layer3 ) {

>              /* Ethernet packet to L3 outport -> pop Ethernet header. */

>              flow->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,

> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h

> index 69eda23c5a37..6b9c6504e894 100644

> --- a/ofproto/ofproto-dpif-xlate.h

> +++ b/ofproto/ofproto-dpif-xlate.h

> @@ -166,7 +166,7 @@ void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *,

>                         const struct mbridge *, const struct dpif_sflow *,

>                         const struct dpif_ipfix *, const struct netflow *,

>                         bool forward_bpdu, bool has_in_band,

> -                       bool packet_type_aware,

> +                       bool legacy_l3_pipeline,

>                         const struct dpif_backer_support *support);

>  void xlate_remove_ofproto(struct ofproto_dpif *);

>

> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c

> index 098db18c7ffa..81c8576efce0 100644

> --- a/ofproto/ofproto-dpif.c

> +++ b/ofproto/ofproto-dpif.c

> @@ -446,7 +446,7 @@ type_run(const char *type)

>                                ofproto->netflow,

>                                ofproto->up.forward_bpdu,

>                                connmgr_has_in_band(ofproto->up.connmgr),

> -                              ofproto->up.packet_type_aware,

> +                              ofproto->up.legacy_l3_pipeline,

>                                &ofproto->backer->support);

>

>              HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {

> @@ -1793,7 +1793,7 @@ set_tables_version(struct ofproto *ofproto_, ovs_version_t version)

>  }

>

>  static void

> -packet_type_aware_changed(struct ofproto *ofproto_)

> +legacy_l3_pipeline_changed(struct ofproto *ofproto_)

>  {

>      struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);

>

> @@ -5652,7 +5652,7 @@ const struct ofproto_class ofproto_dpif_class = {

>      flush,

>      query_tables,

>      set_tables_version,

> -    packet_type_aware_changed,

> +    legacy_l3_pipeline_changed,

>      port_alloc,

>      port_construct,

>      port_destruct,

> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h

> index 749f61b34f8b..7ff2d28a5306 100644

> --- a/ofproto/ofproto-provider.h

> +++ b/ofproto/ofproto-provider.h

> @@ -85,7 +85,7 @@ struct ofproto {

>      char *serial_desc;          /* Serial number (NULL for default). */

>      char *dp_desc;              /* Datapath description (NULL for default). */

>      enum ofputil_frag_handling frag_handling;

> -    bool packet_type_aware;     /* Supports packet-type aware pipeline */

> +    bool legacy_l3_pipeline;    /* Treat L3 tunnel packets as L2? */

>

>      /* Datapath. */

>      struct hmap ports;          /* Contains "struct ofport"s. */

> @@ -933,8 +933,8 @@ struct ofproto_class {

>       * can be triggered. */

>      void (*set_tables_version)(struct ofproto *ofproto, ovs_version_t version);

>

> -    /* Called when 'ofproto->packet_type_aware' has changed. */

> -    void (*packet_type_aware_changed)(struct ofproto *ofproto);

> +    /* Called when 'ofproto->legacy_l3_pipeline' has changed. */

> +    void (*legacy_l3_pipeline_changed)(struct ofproto *ofproto);

>

>  /* ## ---------------- ## */

>  /* ## ofport Functions ## */

> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c

> index 11a26af9f1bb..0f9d99b50cac 100644

> --- a/ofproto/ofproto.c

> +++ b/ofproto/ofproto.c

> @@ -785,12 +785,12 @@ ofproto_set_dp_desc(struct ofproto *p, const char *dp_desc)

>  }

>

>  void

> -ofproto_set_packet_type_aware(struct ofproto *p, bool pta)

> +ofproto_set_legacy_l3_pipeline(struct ofproto *p, bool llp)

>  {

> -    if (pta != p->packet_type_aware) {

> -        p->packet_type_aware = pta;

> -        if (p->ofproto_class->packet_type_aware_changed) {

> -            p->ofproto_class->packet_type_aware_changed(p);

> +    if (llp != p->legacy_l3_pipeline) {

> +        p->legacy_l3_pipeline = llp;

> +        if (p->ofproto_class->legacy_l3_pipeline_changed) {

> +            p->ofproto_class->legacy_l3_pipeline_changed(p);

>          }

>      }

>  }

> @@ -3219,8 +3219,8 @@ query_tables(struct ofproto *ofproto,

>               struct ofputil_table_stats **statsp)

>  {

>      struct mf_bitmap rw_fields = oxm_writable_fields();

> -    struct mf_bitmap match = oxm_matchable_fields(ofproto->packet_type_aware);

> -    struct mf_bitmap mask = oxm_maskable_fields(ofproto->packet_type_aware);

> +    struct mf_bitmap match = oxm_matchable_fields();

> +    struct mf_bitmap mask = oxm_maskable_fields();

>

>      struct ofputil_table_features *features;

>      struct ofputil_table_stats *stats;

> @@ -4762,19 +4762,6 @@ add_flow_init(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,

>          return OFPERR_OFPBRC_EPERM;

>      }

>

> -    if (ofproto->packet_type_aware) {

> -        if (!match.wc.masks.packet_type) {

> -            /* Add default match on packet_type Ethernet.*/

> -            match.flow.packet_type = htonl(PT_ETH);

> -            match.wc.masks.packet_type = OVS_BE32_MAX;

> -        }

> -    } else {

> -        if (match.wc.masks.packet_type) {

> -            /* Match on packet_type not allowed. */

> -            return OFPERR_OFPBMC_BAD_TYPE;

> -        }

> -    }

> -

>      if (!ofm->temp_rule) {

>          cls_rule_init(&cr, &match, fm->priority);

>

> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h

> index 02f81ccf06c6..c93b5103d088 100644

> --- a/ofproto/ofproto.h

> +++ b/ofproto/ofproto.h

> @@ -325,7 +325,7 @@ void ofproto_set_threads(int n_handlers, int n_revalidators);

>  void ofproto_type_set_config(const char *type,

>                               const struct smap *other_config);

>  void ofproto_set_dp_desc(struct ofproto *, const char *dp_desc);

> -void ofproto_set_packet_type_aware(struct ofproto *p, bool pta);

> +void ofproto_set_legacy_l3_pipeline(struct ofproto *p, bool llp);

>  int ofproto_set_snoops(struct ofproto *, const struct sset *snoops);

>  int ofproto_set_netflow(struct ofproto *,

>                          const struct netflow_options *nf_options);

> diff --git a/tests/ofproto.at b/tests/ofproto.at

> index 76a33b6902aa..25aa00aff4a2 100644

> --- a/tests/ofproto.at

> +++ b/tests/ofproto.at

> @@ -2390,6 +2390,7 @@ metadata in_port in_port_oxm pkt_mark ct_mark ct_label reg0 reg1 reg2 reg3 reg4

>      matching:

>        dp_hash: arbitrary mask

>        recirc_id: exact match or wildcard

> +      packet_type: exact match or wildcard

>        conj_id: exact match or wildcard

>        tun_id: arbitrary mask

>        tun_src: arbitrary mask

> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at

> index 52eaf0320cd5..6d2ae1d26bea 100644

> --- a/tests/ovs-ofctl.at

> +++ b/tests/ovs-ofctl.at

> @@ -2107,7 +2107,7 @@ OXM_OF_VLAN_VID_W(0123/1123)

>  nx_pull_match() returned error OFPBMC_BAD_PREREQ

>  OXM_OF_VLAN_VID(1123)

>  OXM_OF_VLAN_VID(1123)

> -<any>

> +OXM_OF_PACKET_TYPE(00000000)

>  OXM_OF_VLAN_VID_W(1103/1f0f)

>  OXM_OF_VLAN_VID_W(1103/1f0f), OXM_OF_VLAN_PCP(01)

>  OXM_OF_VLAN_VID_W(1000/1000)

> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c

> index ac9550f37fc7..da533ccb5bde 100644

> --- a/vswitchd/bridge.c

> +++ b/vswitchd/bridge.c

> @@ -254,7 +254,7 @@ static void bridge_configure_ipfix(struct bridge *);

>  static void bridge_configure_spanning_tree(struct bridge *);

>  static void bridge_configure_tables(struct bridge *);

>  static void bridge_configure_dp_desc(struct bridge *);

> -static void bridge_configure_packet_type_aware(struct bridge *);

> +static void bridge_configure_legacy_l3_pipeline(struct bridge *);

>  static void bridge_configure_aa(struct bridge *);

>  static void bridge_aa_refresh_queued(struct bridge *);

>  static bool bridge_aa_need_refresh(struct bridge *);

> @@ -709,7 +709,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)

>          bridge_configure_tables(br);

>          bridge_configure_dp_desc(br);

>          bridge_configure_aa(br);

> -        bridge_configure_packet_type_aware(br);

> +        bridge_configure_legacy_l3_pipeline(br);

>      }

>      free(managers);

>

> @@ -3804,10 +3804,11 @@ bridge_configure_dp_desc(struct bridge *br)

>  }

>

>  static void

> -bridge_configure_packet_type_aware(struct bridge *br)

> +bridge_configure_legacy_l3_pipeline(struct bridge *br)

>  {

> -    ofproto_set_packet_type_aware(br->ofproto,

> -            smap_get_bool(&br->cfg->other_config, "packet-type-aware", false));

> +    bool legacy_l3_pipeline = smap_get_bool(&br->cfg->other_config,

> +                                            "legacy-l3-pipeline", true);

> +    ofproto_set_legacy_l3_pipeline(br->ofproto, !legacy_l3_pipeline);

>  }

>

>  static struct aa_mapping *

> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml

> index 299c6817c283..dc97d75dc113 100644

> --- a/vswitchd/vswitch.xml

> +++ b/vswitchd/vswitch.xml

> @@ -855,12 +855,22 @@

>          ID, the default queue is used instead.

>        </column>

>

> -      <column name="other_config" key="packet-type-aware"

> +      <column name="other_config" key="legacy-l3-tunnel"

>                type='{"type": "boolean"}'>

> -        If set to <code>true</code>, the bridge supports the packet type

> -        awareness feature introduced in OpenFlow version 1.5. The bridge can

> -        switch Ethernet and non-Ethernet packets and a controller must consider

> -        the type and match on the packet_type match field as appropriate.

> +        <p>

> +          If set to <code>true</code>, the Open vSwitch pipeline imitates the

> +          legacy behavior from Open vSwitch 2.7 and earlier that treats packets

> +          received on L3 tunnels as Ethernet packets, by synthesizing an

> +          Ethernet header whose Ethertype is the packet's type and all-zero

> +          source and destination.  If set to <code>false</code>, the pipeline

> +          handles L3 tunnel packets using the OpenFlow 1.5+

> +          <code>packet_type</code> field.

> +        </p>

> +

> +        <p>

> +          The default in Open vSwitch 2.8 is <code>true</code>.  Later versions

> +          of Open vSwitch may default to <code>false</code>.

> +        </p>

>        </column>

>

>        <column name="protocols">

> --

> 2.10.2

>

> _______________________________________________

> dev mailing list

> dev at openvswitch.org<mailto:dev at openvswitch.org>

> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list