On Sat, Jun 18, 2016 at 3:36 PM, William Tu <u9012...@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
>
> Signed-off-by: William Tu <u9012...@gmail.com>
> ---
> include/openvswitch/ofp-actions.h | 10 +++
> lib/dp-packet.c | 2 +
> lib/dp-packet.h | 67 +++++++++++++++
> lib/dpif-netdev.c | 32 +++++++-
> lib/dpif.c | 24 +++++-
> lib/dpif.h | 1 +
> lib/netdev-bsd.c | 3 +
> lib/netdev-dpdk.c | 6 ++
> lib/netdev-dummy.c | 5 ++
> lib/netdev-linux.c | 3 +
> lib/netdev.c | 6 +-
> lib/odp-execute.c | 12 +++
> lib/odp-util.c | 23 ++++++
> lib/ofp-actions.c | 106 ++++++++++++++++++++++++
> ofproto/ofproto-dpif-sflow.c | 1 +
> ofproto/ofproto-dpif-xlate.c | 57 +++++++++++++
> ofproto/ofproto-dpif.c | 58 +++++++++++++
> 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, 719 insertions(+), 4 deletions(-)
>
> diff --git a/include/openvswitch/ofp-actions.h
> b/include/openvswitch/ofp-actions.h
> index 91c7ee5..0b8ccbb 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..8e7defc 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
> @@ -168,6 +169,7 @@ dp_packet_clone_with_headroom(const struct dp_packet
> *buffer, size_t headroom)
> new_buffer->l3_ofs = buffer->l3_ofs;
> new_buffer->l4_ofs = buffer->l4_ofs;
> new_buffer->md = buffer->md;
> + new_buffer->cutlen = buffer->cutlen;
> #ifdef DPDK_NETDEV
> new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
> #else
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 118c84d..a2af649 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -60,6 +60,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 +495,34 @@ 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_HEADER_LEN) {
> + max_len = ETH_HEADER_LEN;
> + }
> +
> + if (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)
> +{
> + /* Always in valid range if user uses dp_packet_set_cutlen. */
> + return b->cutlen;
> +}
> +
> static inline void *
> dp_packet_data(const struct dp_packet *b)
> {
> @@ -567,12 +596,14 @@ enum { NETDEV_MAX_BURST = 32 }; /* Maximum number
> packets in a batch. */
>
> struct dp_packet_batch {
> int count;
> + bool trunc; /* true if the batch needs truncate. */
> struct dp_packet *packets[NETDEV_MAX_BURST];
> };
>
> static inline void dp_packet_batch_init(struct dp_packet_batch *b)
> {
> b->count = 0;
> + b->trunc = false;
> }
>
> static inline void
> @@ -585,12 +616,14 @@ dp_packet_batch_clone(struct dp_packet_batch *dst,
> dst->packets[i] = dp_packet_clone(src->packets[i]);
> }
> dst->count = src->count;
> + dst->trunc = src->trunc;
> }
>
> static inline void
> packet_batch_init_packet(struct dp_packet_batch *b, struct dp_packet *p)
> {
> b->count = 1;
> + b->trunc = false;
> b->packets[0] = p;
> }
>
> @@ -606,6 +639,40 @@ dp_packet_delete_batch(struct dp_packet_batch *batch,
> bool may_steal)
> }
> }
>
> +static inline void
> +dp_packet_batch_cutlen(struct dp_packet_batch *pktb)
This function name is not descriptive enough, can indicate that this
is actually truncating all packets from the batch.
> +{
> + int i;
> +
> + if (!pktb->trunc)
> + return;
> +
> + for (i = 0; i < pktb->count; i++) {
> + uint32_t cutlen = dp_packet_get_cutlen(pktb->packets[i]);
> +
> + if (cutlen > 0) {
> + dp_packet_set_size(pktb->packets[i],
> + dp_packet_size(pktb->packets[i]) - cutlen);
> + dp_packet_reset_cutlen(pktb->packets[i]);
> + }
> + }
> + pktb->trunc = false;
> +}
> +
> +static inline void
> +dp_packet_batch_reset_cutlen(struct dp_packet_batch *pktb)
> +{
> + int i;
> +
> + if (!pktb->trunc)
> + return;
> +
> + pktb->trunc = false;
> + for (i = 0; i < pktb->count; i++) {
> + dp_packet_reset_cutlen(pktb->packets[i]);
> + }
> +}
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 8d39d9e..90e1f31 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_batch *orig_packets_ = packets_;
> int err;
>
> if (!may_steal) {
> @@ -4064,6 +4065,9 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
> *packets_,
> packets_ = &tnl_pkt;
> }
>
> + dp_packet_batch_cutlen(packets_);
> + dp_packet_batch_reset_cutlen(orig_packets_);
> +
> err = push_tnl_action(pmd, a, packets_);
> if (!err) {
> (*depth)++;
> @@ -4076,6 +4080,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_batch *orig_packets_ = packets_;
> odp_port_t portno = u32_to_odp(nl_attr_get_u32(a));
>
> p = pmd_tx_port_cache_lookup(pmd, portno);
> @@ -4084,10 +4089,13 @@ 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;
> }
>
> + dp_packet_batch_cutlen(packets_);
> + dp_packet_batch_reset_cutlen(orig_packets_);
> +
is there any need to call dp_packet_batch_reset_cutlen() function
after dp_packet_batch_cutlen()?
> netdev_pop_header(p->netdev, packets_);
> if (!packets_->count) {
> return;
> @@ -4107,22 +4115,41 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
> *packets_,
>
> case OVS_ACTION_ATTR_USERSPACE:
> if (!fat_rwlock_tryrdlock(&dp->upcall_rwlock)) {
> + struct dp_packet_batch *orig_packets_ = packets_;
> struct dp_packet **packets = packets_->packets;
> const struct nlattr *userdata;
> + struct dp_packet_batch usr_pkt;
> struct ofpbuf actions;
> struct flow flow;
> ovs_u128 ufid;
> + bool clone = false;
> int i;
>
> userdata = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA);
> ofpbuf_init(&actions, 0);
>
> + if (packets_->trunc) {
> + if (!may_steal) {
> + dp_packet_batch_clone(&usr_pkt, packets_);
> + packets_ = &usr_pkt;
> + packets = packets_->packets;
> + clone = true;
> + }
> + dp_packet_batch_cutlen(packets_);
> + dp_packet_batch_reset_cutlen(orig_packets_);
> + }
> +
> for (i = 0; i < packets_->count; i++) {
> flow_extract(packets[i], &flow);
> dpif_flow_hash(dp->dpif, &flow, sizeof flow, &ufid);
> dp_execute_userspace_action(pmd, packets[i], may_steal,
> &flow,
> &ufid, &actions, userdata);
> }
> +
> + if (clone) {
> + dp_packet_delete_batch(packets_, true);
> + }
> +
> ofpbuf_uninit(&actions);
> fat_rwlock_unlock(&dp->upcall_rwlock);
>
> @@ -4170,6 +4197,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..b9cbadf 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);
>
> @@ -1106,7 +1107,8 @@ dpif_execute_helper_cb(void *aux_, struct
> dp_packet_batch *packets_,
> struct ofpbuf execute_actions;
> uint64_t stub[256 / 8];
> struct pkt_metadata *md = &packet->md;
> - bool dst_set;
> + bool dst_set, clone = false;
> + uint32_t cutlen = dp_packet_get_cutlen(packet);
>
> dst_set = flow_tnl_dst_is_set(&md->tunnel);
> if (dst_set) {
> @@ -1124,6 +1126,21 @@ 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 &&
> + type != OVS_ACTION_ATTR_CT &&
> + type != OVS_ACTION_ATTR_RECIRC) {
Checking for allowed action is easier to read than not allowed.
> + if (!may_steal) {
> + trunc_packet = dp_packet_clone(packet);
> + packet = trunc_packet;
> + clone = true;
> + }
> +
> + dp_packet_set_size(packet, dp_packet_size(packet) - cutlen);
> + dp_packet_reset_cutlen(orig_packet);
> + }
> +
> execute.packet = packet;
> execute.flow = aux->flow;
> execute.needs_help = false;
> @@ -1135,6 +1152,10 @@ dpif_execute_helper_cb(void *aux_, struct
> dp_packet_batch *packets_,
> if (dst_set) {
> ofpbuf_uninit(&execute_actions);
> }
> +
> + if (clone) {
> + dp_packet_delete(trunc_packet);
> + }
> break;
> }
>
> @@ -1146,6 +1167,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..2e92d97 100644
> --- a/lib/netdev-bsd.c
> +++ b/lib/netdev-bsd.c
> @@ -699,6 +699,9 @@ netdev_bsd_send(struct netdev *netdev_, int qid
> OVS_UNUSED,
> const void *data = dp_packet_data(pkts[i]);
> size_t size = dp_packet_size(pkts[i]);
>
> + /* Truncate the packet if it is configured. */
> + size -= dp_packet_get_cutlen(pkts[i]);
> +
> while (!error) {
> ssize_t retval;
> if (dev->tap_fd >= 0) {
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index fc0c8d3..8474b17 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1585,6 +1585,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);
> + }
No need to check if cutlen is positive value, subtracting zero does
not matter anyways.
>
> 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..d6e5a54 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -1170,6 +1170,9 @@ netdev_linux_send(struct netdev *netdev_, int qid
> OVS_UNUSED,
> size_t size = dp_packet_size(pkts[i]);
> ssize_t retval;
>
> + /* Truncate the packet if it is configured. */
> + size -= dp_packet_get_cutlen(pkts[i]);
> +
> if (!is_tap_netdev(netdev_)) {
> /* Use our AF_PACKET socket to send to this device. */
> struct sockaddr_ll sll;
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 4be806d..444998d 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,10 @@ (struct netdev *netdev, int qid, struct dp_packet_batch
> *batch,
> if (!error) {
> COVERAGE_INC(netdev_sent);
> }
> +
> + if (!may_steal) {
> + dp_packet_batch_reset_cutlen(batch);
> + }
This could be moved to if block above. it is not required if there is any error.
> return error;
> }
>
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 4239624..5a43904 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -503,6 +503,7 @@ requires_datapath_assistance(const struct nlattr *a)
> case OVS_ACTION_ATTR_HASH:
> case OVS_ACTION_ATTR_PUSH_MPLS:
> case OVS_ACTION_ATTR_POP_MPLS:
> + case OVS_ACTION_ATTR_TRUNC:
> return false;
>
> case OVS_ACTION_ATTR_UNSPEC:
> @@ -625,6 +626,17 @@ odp_execute_actions(void *dp, struct dp_packet_batch
> *batch, bool steal,
> }
> break;
>
> + case OVS_ACTION_ATTR_TRUNC: {
> + const struct ovs_action_trunc *trunc =
> + nl_attr_get_unspec(a, sizeof *trunc);
> +
> + batch->trunc = true;
> + for (i = 0; i < cnt; i++) {
> + dp_packet_set_cutlen(packets[i], trunc->max_len);
> + }
> + break;
> + }
> +
> case OVS_ACTION_ATTR_OUTPUT:
> case OVS_ACTION_ATTR_TUNNEL_PUSH:
> case OVS_ACTION_ATTR_TUNNEL_POP:
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 48c05f5..d7b6a2d 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -107,6 +107,7 @@ odp_action_len(uint16_t type)
>
> switch ((enum ovs_action_attr) type) {
> case OVS_ACTION_ATTR_OUTPUT: return sizeof(uint32_t);
> + case OVS_ACTION_ATTR_TRUNC: return sizeof(struct ovs_action_trunc);
> case OVS_ACTION_ATTR_TUNNEL_PUSH: return ATTR_LEN_VARIABLE;
> case OVS_ACTION_ATTR_TUNNEL_POP: return sizeof(uint32_t);
> case OVS_ACTION_ATTR_USERSPACE: return ATTR_LEN_VARIABLE;
> @@ -777,6 +778,14 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
> case OVS_ACTION_ATTR_OUTPUT:
> ds_put_format(ds, "%"PRIu32, nl_attr_get_u32(a));
> break;
> + case OVS_ACTION_ATTR_TRUNC: {
> + const struct ovs_action_trunc *trunc =
> + nl_attr_get_unspec(a, sizeof *trunc);
> +
> + ds_put_format(ds, "trunc(%"PRIu32")", trunc->max_len);
> + break;
> + }
> + break;
> case OVS_ACTION_ATTR_TUNNEL_POP:
> ds_put_format(ds, "tnl_pop(%"PRIu32")", nl_attr_get_u32(a));
> break;
> @@ -1528,6 +1537,20 @@ parse_odp_action(const char *s, const struct simap
> *port_names,
> }
> }
>
> + {
> + uint32_t max_len;
> + int n;
> +
> + if (ovs_scan(s, "trunc(%"SCNi32")%n", &max_len, &n)) {
> + struct ovs_action_trunc *trunc;
> +
> + trunc = nl_msg_put_unspec_uninit(actions,
> + OVS_ACTION_ATTR_TRUNC, sizeof *trunc);
> + trunc->max_len = max_len;
> + return n;
> + }
> + }
> +
> if (port_names) {
> int len = strcspn(s, delimiters);
> struct simap_node *node;
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index ea55896..997cc15 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -301,6 +301,9 @@ enum ofp_raw_action_type {
> /* NX1.0+(36): struct nx_action_nat, ... */
> NXAST_RAW_NAT,
>
> + /* NX1.0+(39): struct nx_action_output_trunc. */
> + NXAST_RAW_OUTPUT_TRUNC,
> +
> /* ## ------------------ ## */
> /* ## Debugging actions. ## */
> /* ## ------------------ ## */
> @@ -381,6 +384,7 @@ ofpact_next_flattened(const struct ofpact *ofpact)
> case OFPACT_CONTROLLER:
> case OFPACT_ENQUEUE:
> case OFPACT_OUTPUT_REG:
> + case OFPACT_OUTPUT_TRUNC:
> case OFPACT_BUNDLE:
> case OFPACT_SET_FIELD:
> case OFPACT_SET_VLAN_VID:
> @@ -538,6 +542,40 @@ encode_OUTPUT(const struct ofpact_output *output,
> }
>
> static char * OVS_WARN_UNUSED_RESULT
> +parse_truncate_subfield(struct ofpact_output_trunc *output_trunc,
> + const char *arg_)
> +{
> + char *key, *value;
> + char *arg = CONST_CAST(char *, arg_);
> +
> + while (ofputil_parse_key_value(&arg, &key, &value)) {
> + if (!strcmp(key, "port")) {
> + if (!ofputil_port_from_string(value, &output_trunc->port)) {
> + return xasprintf("output to unknown truncate port: %s",
> + value);
> + }
> + if (ofp_to_u16(output_trunc->port) > ofp_to_u16(OFPP_MAX)) {
> + if (output_trunc->port != OFPP_LOCAL &&
> + output_trunc->port != OFPP_IN_PORT)
> + return xasprintf("output to unsupported truncate port: %s",
> + value);
> + }
> + } else if (!strcmp(key, "max_len")) {
> + char *err;
> +
> + err = str_to_u32(value, &output_trunc->max_len);
> + if (err) {
> + return err;
> + }
> + } else {
> + return xasprintf("invalid key '%s' in output_trunc argument",
> + key);
> + }
> + }
> + return NULL;
> +}
> +
> +static char * OVS_WARN_UNUSED_RESULT
> parse_OUTPUT(const char *arg, struct ofpbuf *ofpacts,
> enum ofputil_protocol *usable_protocols OVS_UNUSED)
> {
> @@ -547,6 +585,11 @@ parse_OUTPUT(const char *arg, struct ofpbuf *ofpacts,
> output_reg = ofpact_put_OUTPUT_REG(ofpacts);
> output_reg->max_len = UINT16_MAX;
> return mf_parse_subfield(&output_reg->src, arg);
> + } else if (strstr(arg, "port") && strstr(arg, "max_len")) {
> + struct ofpact_output_trunc *output_trunc;
> +
> + output_trunc = ofpact_put_OUTPUT_TRUNC(ofpacts);
> + return parse_truncate_subfield(output_trunc, arg);
> } else {
> struct ofpact_output *output;
>
> @@ -5603,6 +5646,61 @@ parse_NAT(char *arg, struct ofpbuf *ofpacts,
> return NULL;
> }
>
> +/* Truncate output action. */
> +struct nx_action_output_trunc {
> + ovs_be16 type; /* OFPAT_VENDOR. */
> + ovs_be16 len; /* At least 16. */
> + ovs_be32 vendor; /* NX_VENDOR_ID. */
> + ovs_be16 subtype; /* NXAST_OUTPUT_TRUNC. */
> + ovs_be16 port; /* Output port */
> + ovs_be32 max_len; /* Truncate packet to size bytes */
> +};
> +OFP_ASSERT(sizeof(struct nx_action_output_trunc) == 16);
> +
> +static enum ofperr
> +decode_NXAST_RAW_OUTPUT_TRUNC(const struct nx_action_output_trunc *natrc,
> + enum ofp_version ofp_version OVS_UNUSED,
> + struct ofpbuf *out)
> +{
> + struct ofpact_output_trunc *output_trunc;
> +
> + output_trunc = ofpact_put_OUTPUT_TRUNC(out);
> + output_trunc->max_len = ntohl(natrc->max_len);
> + output_trunc->port = u16_to_ofp(ntohs(natrc->port));
> +
> + if (output_trunc->max_len < ETH_HEADER_LEN) {
> + return OFPERR_OFPBAC_BAD_ARGUMENT;
> + }
> + return 0;
> +}
> +
> +static void
> +encode_OUTPUT_TRUNC(const struct ofpact_output_trunc *output_trunc,
> + enum ofp_version ofp_version OVS_UNUSED,
> + struct ofpbuf *out)
> +{
> + struct nx_action_output_trunc *natrc = put_NXAST_OUTPUT_TRUNC(out);
> +
> + natrc->max_len = htonl(output_trunc->max_len);
> + natrc->port = htons(ofp_to_u16(output_trunc->port));
> +}
> +
> +static char * OVS_WARN_UNUSED_RESULT
> +parse_OUTPUT_TRUNC(const char *arg, struct ofpbuf *ofpacts OVS_UNUSED,
> + enum ofputil_protocol *usable_protocols OVS_UNUSED)
> +{
> + /* Disable output_trunc parsing. Expose as output(port=N,max_len=M) and
> + * reuse parse_OUTPUT to parse output_trunc action. */
> + return xasprintf("unknown action %s", arg);
> +}
> +
> +static void
> +format_OUTPUT_TRUNC(const struct ofpact_output_trunc *a, struct ds *s)
> +{
> + ds_put_format(s, "%soutput%s(port=%"PRIu16",max_len=%"PRIu32")",
> + colors.special, colors.end, a->port, a->max_len);
> +}
> +
>
> /* Meter instruction. */
>
> @@ -5997,6 +6095,7 @@ ofpact_is_set_or_move_action(const struct ofpact *a)
> case OFPACT_NOTE:
> case OFPACT_OUTPUT:
> case OFPACT_OUTPUT_REG:
> + case OFPACT_OUTPUT_TRUNC:
> case OFPACT_POP_MPLS:
> case OFPACT_POP_QUEUE:
> case OFPACT_PUSH_MPLS:
> @@ -6025,6 +6124,7 @@ ofpact_is_allowed_in_actions_set(const struct ofpact *a)
> case OFPACT_DEC_TTL:
> case OFPACT_GROUP:
> case OFPACT_OUTPUT:
> + case OFPACT_OUTPUT_TRUNC:
> case OFPACT_POP_MPLS:
> case OFPACT_PUSH_MPLS:
> case OFPACT_PUSH_VLAN:
> @@ -6249,6 +6349,7 @@ ovs_instruction_type_from_ofpact_type(enum ofpact_type
> type)
> case OFPACT_CONTROLLER:
> case OFPACT_ENQUEUE:
> case OFPACT_OUTPUT_REG:
> + case OFPACT_OUTPUT_TRUNC:
> case OFPACT_BUNDLE:
> case OFPACT_SET_VLAN_VID:
> case OFPACT_SET_VLAN_PCP:
> @@ -6677,6 +6778,10 @@ ofpact_check__(enum ofputil_protocol
> *usable_protocols, struct ofpact *a,
> case OFPACT_OUTPUT_REG:
> return mf_check_src(&ofpact_get_OUTPUT_REG(a)->src, flow);
>
> + case OFPACT_OUTPUT_TRUNC:
> + return ofpact_check_output_port(ofpact_get_OUTPUT_TRUNC(a)->port,
> + max_ports);
> +
> case OFPACT_BUNDLE:
> return bundle_check(ofpact_get_BUNDLE(a), max_ports, flow);
>
> @@ -7354,6 +7459,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact,
> ofp_port_t port)
> return port == OFPP_CONTROLLER;
>
> case OFPACT_OUTPUT_REG:
> + case OFPACT_OUTPUT_TRUNC:
> case OFPACT_BUNDLE:
> case OFPACT_SET_VLAN_VID:
> case OFPACT_SET_VLAN_PCP:
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index fbc82b7..5d26b7c 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -1140,6 +1140,7 @@ dpif_sflow_read_actions(const struct flow *flow,
> }
> break;
>
> + case OVS_ACTION_ATTR_TRUNC:
> case OVS_ACTION_ATTR_USERSPACE:
> case OVS_ACTION_ATTR_RECIRC:
> case OVS_ACTION_ATTR_HASH:
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 6013831..078c286 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3995,6 +3995,56 @@ xlate_output_reg_action(struct xlate_ctx *ctx,
> }
>
> static void
> +xlate_output_trunc_action(struct xlate_ctx *ctx,
> + ofp_port_t port, uint32_t max_len)
> +{
> + bool support_trunc = ctx->xbridge->support.trunc;
> + struct ovs_action_trunc *trunc;
> + char name[OFP_MAX_PORT_NAME_LEN];
> +
> + switch (port) {
> + case OFPP_TABLE:
> + case OFPP_NORMAL:
> + case OFPP_FLOOD:
> + case OFPP_ALL:
> + case OFPP_CONTROLLER:
> + case OFPP_NONE:
> + ofputil_port_to_string(port, name, sizeof name);
> + xlate_report(ctx, "output_trunc does not support port: %s", name);
> + break;
> + case OFPP_LOCAL:
> + case OFPP_IN_PORT:
> + default:
> + if (port != ctx->xin->flow.in_port.ofp_port) {
> + const struct xport *xport = get_ofp_port(ctx->xbridge, port);
> +
> + if (xport == NULL || xport->odp_port == ODPP_NONE) {
> + /* Since truncate happens at its following output action, if
> + * the output port is a patch port, the behavior is somehow
> + * unpredicable. For simpilicity, disallow this case. */
> + ofputil_port_to_string(port, name, sizeof name);
> + XLATE_REPORT_ERROR(ctx, "bridge %s: "
> + "output_trunc does not support port: %s",
> + ctx->xbridge->name, name);
> + break;
> + }
> +
> + trunc = nl_msg_put_unspec_uninit(ctx->odp_actions,
> + OVS_ACTION_ATTR_TRUNC,
> + sizeof *trunc);
> + trunc->max_len = max_len;
> + xlate_output_action(ctx, port, max_len, false);
> + if (!support_trunc) {
> + ctx->xout->slow |= SLOW_ACTION;
> + }
> + } else {
> + xlate_report(ctx, "skipping output to input port");
> + }
> + break;
> + }
> +}
> +
> +static void
> xlate_enqueue_action(struct xlate_ctx *ctx,
> const struct ofpact_enqueue *enqueue)
> {
> @@ -4333,6 +4383,7 @@ freeze_unroll_actions(const struct ofpact *a, const
> struct ofpact *end,
> for (; a < end; a = ofpact_next(a)) {
> switch (a->type) {
> case OFPACT_OUTPUT_REG:
> + case OFPACT_OUTPUT_TRUNC:
> case OFPACT_GROUP:
> case OFPACT_OUTPUT:
> case OFPACT_CONTROLLER:
> @@ -4584,6 +4635,7 @@ recirc_for_mpls(const struct ofpact *a, struct
> xlate_ctx *ctx)
>
> /* Output actions do not require recirculation. */
> case OFPACT_OUTPUT:
> + case OFPACT_OUTPUT_TRUNC:
> case OFPACT_ENQUEUE:
> case OFPACT_OUTPUT_REG:
> /* Set actions that don't touch L3+ fields do not require recirculation.
> */
> @@ -4933,6 +4985,11 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
> ofpacts_len,
> xlate_output_reg_action(ctx, ofpact_get_OUTPUT_REG(a));
> break;
>
> + case OFPACT_OUTPUT_TRUNC:
> + xlate_output_trunc_action(ctx, ofpact_get_OUTPUT_TRUNC(a)->port,
> + ofpact_get_OUTPUT_TRUNC(a)->max_len);
> + break;
> +
> case OFPACT_LEARN:
> xlate_learn_action(ctx, ofpact_get_LEARN(a));
> break;
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 8a3c525..8c19f91 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1212,6 +1212,63 @@ check_masked_set_action(struct dpif_backer *backer)
> return !error;
> }
>
> +/* Tests whether 'backer''s datapath supports truncation of a packet in
> + * OVS_ACTION_ATTR_TRUNC. We need to disable some features on older
> + * datapaths that don't support this feature. */
> +static bool
> +check_trunc_action(struct dpif_backer *backer)
> +{
> + struct eth_header *eth;
> + struct ofpbuf actions;
> + struct dpif_execute execute;
> + struct dp_packet packet;
> + struct ovs_action_trunc *trunc;
> + struct flow flow;
> + int error;
> +
> + /* Compose an action with output(port:1,
> + * max_len:OVS_ACTION_OUTPUT_MIN + 1).
> + * This translates to one truncate action and one output action. */
> + ofpbuf_init(&actions, 64);
> + trunc = nl_msg_put_unspec_uninit(&actions,
> + OVS_ACTION_ATTR_TRUNC, sizeof *trunc);
> +
> + trunc->max_len = ETH_HEADER_LEN + 1;
> + nl_msg_put_odp_port(&actions, OVS_ACTION_ATTR_OUTPUT, u32_to_odp(1));
> +
> + /* Compose a dummy Ethernet packet. */
> + dp_packet_init(&packet, ETH_HEADER_LEN);
> + eth = dp_packet_put_zeros(&packet, ETH_HEADER_LEN);
> + eth->eth_type = htons(0x1234);
> +
> + flow_extract(&packet, &flow);
> +
> + /* Execute the actions. On older datapaths this fails with EINVAL, on
> + * newer datapaths it succeeds. */
> + execute.actions = actions.data;
> + execute.actions_len = actions.size;
> + execute.packet = &packet;
> + execute.flow = &flow;
> + execute.needs_help = false;
> + execute.probe = true;
> + execute.mtu = 0;
> +
> + error = dpif_execute(backer->dpif, &execute);
> +
> + dp_packet_uninit(&packet);
> + ofpbuf_uninit(&actions);
> +
> + if (error) {
> + VLOG_INFO("%s: Datapath does not support truncate action",
> + dpif_name(backer->dpif));
> + } else {
> + VLOG_INFO("%s: Datapath supports truncate action",
> + dpif_name(backer->dpif));
> + }
> +
> + return !error;
> +}
> +
> #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE) \
> static bool \
> check_##NAME(struct dpif_backer *backer) \
> @@ -1263,6 +1320,7 @@ check_support(struct dpif_backer *backer)
> backer->support.odp.recirc = check_recirc(backer);
> backer->support.odp.max_mpls_depth = check_max_mpls_depth(backer);
> backer->support.masked_set_action = check_masked_set_action(backer);
> + backer->support.trunc = check_trunc_action(backer);
> backer->support.ufid = check_ufid(backer);
> backer->support.tnl_push_pop = dpif_supports_tnl_push_pop(backer->dpif);
>
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index 9e03b01..4034475 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -90,6 +90,9 @@ struct dpif_backer_support {
> /* True if the datapath supports OVS_FLOW_ATTR_UFID. */
> bool ufid;
>
> + /* True if the datapath supports OVS_ACTION_ATTR_TRUNC action. */
> + bool trunc;
> +
> /* Each member represents support for related OVS_KEY_ATTR_* fields. */
> struct odp_support odp;
> };
> diff --git a/tests/odp.at b/tests/odp.at
> index 7b94c92..e630855 100644
> --- a/tests/odp.at
> +++ b/tests/odp.at
...
> AT_SETUP([conntrack - controller])
> CHECK_CONNTRACK()
> OVS_TRAFFIC_VSWITCHD_START()
> --
I really like number of tests you have added. Can you also add test to
validate SLOW_ACTION case of truncate? You can add ovs-appctl command
to disable datapath truncate support to simulate the case.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
