[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