[ovs-dev] [PATCHv7 2/2] ofp-actions: Add truncate action.
William Tu
u9012063 at gmail.com
Sat Jun 18 16:34:01 UTC 2016
Hi Pravin,
Thanks for the feedback, I've submitted another version to address these.
Regards,
William
On Fri, Jun 17, 2016 at 10:56 AM, pravin shelar <pshelar at ovn.org> wrote:
> 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