[ovs-dev] [PATCHv7 2/2] ofp-actions: Add truncate action.

pravin shelar pshelar at ovn.org
Fri Jun 17 17:56:31 UTC 2016


On Tue, Jun 14, 2016 at 4:42 PM, William Tu <u9012063 at gmail.com> wrote:
> The patch adds a new action to support packet truncation.  The new action
> is formatted as 'output(port=n,max_len=m)', as output to port n, with
> packet size being MIN(original_size, m).
>
> One use case is to enable port mirroring to send smaller packets to the
> destination port so that only useful packet information is mirrored/copied,
> saving some performance overhead of copying entire packet payload.  Example
> use case is below as well as shown in the testcases:
>
>     - Output to port 1 with max_len 100 bytes.
>     - The output packet size on port 1 will be MIN(original_packet_size, 100).
>     # ovs-ofctl add-flow br0 'actions=output(port=1,max_len=100)'
>
>     - The scope of max_len is limited to output action itself.  The following
>       packet size of output:1 and output:2 will be intact.
>     # ovs-ofctl add-flow br0 \
>             'actions=output(port=1,max_len=100),output:1,output:2'
>     - The Datapath actions shows:
>     # Datapath actions: trunc(100),1,1,2
>
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/137660394
> Signed-off-by: William Tu <u9012063 at gmail.com>
> Cc: Pravin Shelar <pshelar at nicira.com>
> ---
>  include/openvswitch/ofp-actions.h |  10 +++
>  lib/dp-packet.c                   |   1 +
>  lib/dp-packet.h                   |  27 ++++++
>  lib/dpif-netdev.c                 |  33 +++++++-
>  lib/dpif.c                        |  23 ++++++
>  lib/dpif.h                        |   1 +
>  lib/netdev-bsd.c                  |   5 ++
>  lib/netdev-dpdk.c                 |   6 ++
>  lib/netdev-dummy.c                |   5 ++
>  lib/netdev-linux.c                |   5 ++
>  lib/netdev.c                      |   8 +-
>  lib/odp-execute.c                 |  11 +++
>  lib/odp-util.c                    |  23 ++++++
>  lib/ofp-actions.c                 | 107 ++++++++++++++++++++++++
>  ofproto/ofproto-dpif-sflow.c      |   1 +
>  ofproto/ofproto-dpif-xlate.c      |  57 +++++++++++++
>  ofproto/ofproto-dpif.c            |  56 +++++++++++++
>  ofproto/ofproto-dpif.h            |   3 +
>  tests/odp.at                      |   1 +
>  tests/ofp-actions.at              |   3 +
>  tests/ofproto-dpif.at             | 124 ++++++++++++++++++++++++++++
>  tests/ovs-ofctl.at                |   8 ++
>  tests/system-traffic.at           | 168 ++++++++++++++++++++++++++++++++++++++
>  23 files changed, 683 insertions(+), 3 deletions(-)
>
> diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
> index 038ef87..51ffa71 100644
> --- a/include/openvswitch/ofp-actions.h
> +++ b/include/openvswitch/ofp-actions.h
> @@ -108,6 +108,7 @@
>      OFPACT(UNROLL_XLATE,    ofpact_unroll_xlate, ofpact, "unroll_xlate") \
>      OFPACT(CT,              ofpact_conntrack,   ofpact, "ct")           \
>      OFPACT(NAT,             ofpact_nat,         ofpact, "nat")          \
> +    OFPACT(OUTPUT_TRUNC,    ofpact_output_trunc,ofpact, "output_trunc") \
>                                                                          \
>      /* Debugging actions.                                               \
>       *                                                                  \
> @@ -290,6 +291,15 @@ struct ofpact_output_reg {
>      struct mf_subfield src;
>  };
>
> +/* OFPACT_OUTPUT_TRUNC.
> + *
> + * Used for NXAST_OUTPUT_TRUNC. */
> +struct ofpact_output_trunc {
> +    struct ofpact ofpact;
> +    ofp_port_t port;            /* Output port. */
> +    uint32_t max_len;           /* Max send len. */
> +};
> +
>  /* Bundle slave choice algorithm to apply.
>   *
>   * In the descriptions below, 'slaves' is the list of possible slaves in the
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index 0c85d50..4ee2f56 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -30,6 +30,7 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, enum dp_packet_source so
>      dp_packet_reset_offsets(b);
>      pkt_metadata_init(&b->md, 0);
>      dp_packet_rss_invalidate(b);
> +    dp_packet_reset_cutlen(b);
>  }
>
>  static void
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 118c84d..a4b153b 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -19,6 +19,7 @@
>
>  #include <stddef.h>
>  #include <stdint.h>
> +#include <linux/if_ether.h>
>  #include "openvswitch/list.h"
>  #include "packets.h"
>  #include "util.h"
> @@ -60,6 +61,7 @@ struct dp_packet {
>                                      * or UINT16_MAX. */
>      uint16_t l4_ofs;               /* Transport-level header offset,
>                                        or UINT16_MAX. */
> +    uint32_t cutlen;               /* length in bytes to cut from the end. */
>      union {
>          struct pkt_metadata md;
>          uint64_t data[DP_PACKET_CONTEXT_SIZE / 8];
> @@ -494,6 +496,31 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
>  }
>  #endif
>
> +static inline void
> +dp_packet_reset_cutlen(struct dp_packet *b)
> +{
> +    b->cutlen = 0;
> +}
> +
> +static inline uint32_t
> +dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
> +{
> +    if (max_len < ETH_HLEN ||
> +        max_len >= dp_packet_size(b)) {
> +        b->cutlen = 0;
> +    }
> +    else {
> +        b->cutlen = dp_packet_size(b) - max_len;
> +    }
> +    return b->cutlen;
> +}
> +
> +static inline uint32_t
> +dp_packet_get_cutlen(struct dp_packet *b)
> +{
> +    return b->cutlen;
> +}
> +
>  static inline void *
>  dp_packet_data(const struct dp_packet *b)
>  {
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 8d39d9e..70d4cc2 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4057,6 +4057,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
>      case OVS_ACTION_ATTR_TUNNEL_PUSH:
>          if (*depth < MAX_RECIRC_DEPTH) {
>              struct dp_packet_batch tnl_pkt;
> +            struct dp_packet **orig_packets = packets_->packets;
>              int err;
>
>              if (!may_steal) {
> @@ -4064,6 +4065,19 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
>                  packets_ = &tnl_pkt;
>              }
>
> +            for (int i = 0; i < packets_->count; i++) {
> +                /* if may_steal, then opacket == packet. */
> +                struct dp_packet *orig_packet = orig_packets[i];
> +                struct dp_packet *packet = packets_->packets[i];
> +                uint32_t cutlen = dp_packet_get_cutlen(orig_packet);
> +
> +                if (cutlen > 0) {
> +                    dp_packet_set_size(packet,
> +                            dp_packet_size(packet) - cutlen);
> +                    dp_packet_reset_cutlen(orig_packet);
> +                }
> +            }
> +
Can you write function to trim batch of packets so that same function
can be used in tunnel  push and pop action.
>              err = push_tnl_action(pmd, a, packets_);
>              if (!err) {
>                  (*depth)++;
> @@ -4076,6 +4090,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
>
>      case OVS_ACTION_ATTR_TUNNEL_POP:
>          if (*depth < MAX_RECIRC_DEPTH) {
> +            struct dp_packet **orig_packets = packets_->packets;
>              odp_port_t portno = u32_to_odp(nl_attr_get_u32(a));
>
>              p = pmd_tx_port_cache_lookup(pmd, portno);
> @@ -4084,8 +4099,21 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
>                  int i;
>
>                  if (!may_steal) {
> -                   dp_packet_batch_clone(&tnl_pkt, packets_);
> -                   packets_ = &tnl_pkt;
> +                    dp_packet_batch_clone(&tnl_pkt, packets_);
> +                    packets_ = &tnl_pkt;
> +                }
> +
> +                for (int i = 0; i < packets_->count; i++) {
> +                    /* if may_steal, then opacket == packet. */
> +                    struct dp_packet *orig_packet = orig_packets[i];
> +                    struct dp_packet *packet = packets_->packets[i];
> +                    uint32_t cutlen = dp_packet_get_cutlen(orig_packet);
> +
> +                    if (cutlen > 0) {
> +                        dp_packet_set_size(packet,
> +                                dp_packet_size(packet) - cutlen);
> +                        dp_packet_reset_cutlen(orig_packet);
> +                    }
>                  }
>
>                  netdev_pop_header(p->netdev, packets_);
> @@ -4170,6 +4198,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
>      case OVS_ACTION_ATTR_SAMPLE:
>      case OVS_ACTION_ATTR_HASH:
>      case OVS_ACTION_ATTR_UNSPEC:
> +    case OVS_ACTION_ATTR_TRUNC:
>      case __OVS_ACTION_ATTR_MAX:
>          OVS_NOT_REACHED();
>      }
> diff --git a/lib/dpif.c b/lib/dpif.c
> index c4f24c7..92f37f8 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1092,6 +1092,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
>      struct dpif_execute_helper_aux *aux = aux_;
>      int type = nl_attr_type(action);
>      struct dp_packet *packet = packets_->packets[0];
> +    struct dp_packet *trunc_packet = NULL, *orig_packet;
>
>      ovs_assert(packets_->count == 1);
>
> @@ -1107,6 +1108,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
>          uint64_t stub[256 / 8];
>          struct pkt_metadata *md = &packet->md;
>          bool dst_set;
> +        uint32_t cutlen = dp_packet_get_cutlen(packet);
>
>          dst_set = flow_tnl_dst_is_set(&md->tunnel);
>          if (dst_set) {
> @@ -1124,6 +1126,18 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
>              execute.actions_len = NLA_ALIGN(action->nla_len);
>          }
>
> +        orig_packet = packet;
> +
> +        if (cutlen > 0) {
> +            if (!may_steal) {
> +               trunc_packet = dp_packet_clone(packet);
> +               packet = trunc_packet;
> +            }
> +            /* Truncation applies to the clone packet or the original
> +             * packet with may_steal == true. */
> +            dp_packet_set_size(packet, dp_packet_size(orig_packet) - cutlen);
> +        }
> +
>          execute.packet = packet;
>          execute.flow = aux->flow;
>          execute.needs_help = false;
> @@ -1135,6 +1149,14 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
>          if (dst_set) {
>              ofpbuf_uninit(&execute_actions);
>          }
> +
> +        /* Reset the truncation state so next output action is intact. */
> +        if (cutlen > 0) {
> +            dp_packet_reset_cutlen(orig_packet);
> +            if (!may_steal) {
> +                dp_packet_delete(trunc_packet);
> +            }
> +        }
>          break;
>      }
The packet trim operation is applied to CT and RECIRC action, kernel
datapath does not do that. we should not have any inconstancy in
truncate action over different datapath.


>
> @@ -1146,6 +1168,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
>      case OVS_ACTION_ATTR_SET:
>      case OVS_ACTION_ATTR_SET_MASKED:
>      case OVS_ACTION_ATTR_SAMPLE:
> +    case OVS_ACTION_ATTR_TRUNC:
>      case OVS_ACTION_ATTR_UNSPEC:
>      case __OVS_ACTION_ATTR_MAX:
>          OVS_NOT_REACHED();
> diff --git a/lib/dpif.h b/lib/dpif.h
> index 6788301..981868c 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -784,6 +784,7 @@ struct dpif_upcall {
>      size_t key_len;             /* Length of 'key' in bytes. */
>      ovs_u128 ufid;              /* Unique flow identifier for 'key'. */
>      struct nlattr *mru;         /* Maximum receive unit. */
> +    struct nlattr *cutlen;      /* Number of bytes shrink from the end. */
>
>      /* DPIF_UC_ACTION only. */
>      struct nlattr *userdata;    /* Argument to OVS_ACTION_ATTR_USERSPACE. */
> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> index 43fa982..7fc0888 100644
> --- a/lib/netdev-bsd.c
> +++ b/lib/netdev-bsd.c
> @@ -698,6 +698,11 @@ netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
>      for (i = 0; i < cnt; i++) {
>          const void *data = dp_packet_data(pkts[i]);
>          size_t size = dp_packet_size(pkts[i]);
> +        uint32_t cutlen = dp_packet_get_cutlen(pkts[i]);
> +
> +        if (cutlen > 0) {
> +            size -= cutlen;
> +        }
>
We need to check for minimum packet length as we have done in kernel
datapath. Can you add API to read truncated packet length to avoid
duplicate code?

>          while (!error) {
>              ssize_t retval;
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 19d355f..a66d4ff 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1602,6 +1602,12 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
>
>          for (i = 0; i < cnt; i++) {
>              int size = dp_packet_size(pkts[i]);
> +            uint32_t cutlen = dp_packet_get_cutlen(pkts[i]);
> +
> +            if (cutlen > 0) {
> +                size -= cutlen;
> +                dp_packet_set_size(pkts[i], size);
> +            }
>
>              if (OVS_UNLIKELY(size > dev->max_packet_len)) {
>                  if (next_tx_idx != i) {
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index aa244b6..cd90c33 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -1043,6 +1043,11 @@ netdev_dummy_send(struct netdev *netdev, int qid OVS_UNUSED,
>      for (i = 0; i < cnt; i++) {
>          const void *buffer = dp_packet_data(pkts[i]);
>          size_t size = dp_packet_size(pkts[i]);
> +        uint32_t cutlen = dp_packet_get_cutlen(pkts[i]);
> +
> +        if (cutlen > 0) {
> +            size -= cutlen;
> +        }
>
>          if (size < ETH_HEADER_LEN) {
>              error = EMSGSIZE;
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 82813ba..082ca06 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -1169,6 +1169,11 @@ netdev_linux_send(struct netdev *netdev_, int qid OVS_UNUSED,
>          const void *data = dp_packet_data(pkts[i]);
>          size_t size = dp_packet_size(pkts[i]);
>          ssize_t retval;
> +        uint32_t cutlen = dp_packet_get_cutlen(pkts[i]);
> +
> +        if (cutlen > 0) {
> +            size -= cutlen;
> +        }
>
>          if (!is_tap_netdev(netdev_)) {
>              /* Use our AF_PACKET socket to send to this device. */
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 4be806d..f285ddc 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -681,7 +681,7 @@ netdev_set_tx_multiq(struct netdev *netdev, unsigned int n_txq)
>      return error;
>  }
>
> -/* Sends 'buffers' on 'netdev'.  Returns 0 if successful (for every packet),
> +/* Sends 'batch' on 'netdev'.  Returns 0 if successful (for every packet),
>   * otherwise a positive errno value.  Returns EAGAIN without blocking if
>   * at least one the packets cannot be queued immediately.  Returns EMSGSIZE
>   * if a partial packet was transmitted or if a packet is too big or too small
> @@ -717,6 +717,12 @@ netdev_send(struct netdev *netdev, int qid, struct dp_packet_batch *batch,
>      if (!error) {
>          COVERAGE_INC(netdev_sent);
>      }
> +
> +    if (!may_steal) {
> +        for (int i = 0; i < batch->count; i++) {
> +            dp_packet_reset_cutlen(batch->packets[i]);
> +        }
> +    }
>      return error;
>  }
>



More information about the dev mailing list