From patchwork Fri Feb 17 01:11:06 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Zhou X-Patchwork-Id: 728969 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3vPZjX20N7z9s7q for ; Fri, 17 Feb 2017 12:11:28 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id B7D6BBD3; Fri, 17 Feb 2017 01:11:24 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 29BE5B88 for ; Fri, 17 Feb 2017 01:11:23 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f68.google.com (mail-pg0-f68.google.com [74.125.83.68]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 994A81D0 for ; Fri, 17 Feb 2017 01:11:20 +0000 (UTC) Received: by mail-pg0-f68.google.com with SMTP id 5so3354509pgj.0 for ; Thu, 16 Feb 2017 17:11:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=ddsUEwJZ/Gccn8V4cUlG0JMFtClxJVvuekYyf/jGQ4I=; b=mx5z2aVuc5jr3xyP1/uAc/2xaZJzT9QHT7WRDbClj/+ZZloqDJ4xdUAJKviUI0RkVR lY7EBTMspRDZV7hUeZaDPUID9nEbyiH53nfRcAz02i3pIiNbQ+VgurqHgWpIJ8btJtOf 5FNZnEkCf8C/7GRQDWUGO7wPF/AZE78gRUaMMvolW7nZnIxenyS6cZgpDqt9/ebChoOu fNdi0vYKWdjF6+0hdhFeN9J4txr7vvNJs1n82sw2580ewvvMqusvBSBNul6mdAQQsYL5 C14R43Cmc2Qrv7O9UL/ut04BGPVc/qvEMHkn5Ft2OVB4eKmdYlcGI42RmBfZo/K0pAPg LpyQ== X-Gm-Message-State: AMke39klERKdLtYHid/I6VLIOUSw1UlGEqGGc58fgtqiJyto/olDsjcmpi+SZbVlgwtkHg== X-Received: by 10.98.73.130 with SMTP id r2mr6125231pfi.182.1487293880165; Thu, 16 Feb 2017 17:11:20 -0800 (PST) Received: from ubuntu.localdomain ([208.91.1.34]) by smtp.gmail.com with ESMTPSA id m6sm15631546pfg.126.2017.02.16.17.11.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 16 Feb 2017 17:11:19 -0800 (PST) From: Andy Zhou To: dev@openvswitch.org Date: Thu, 16 Feb 2017 17:11:06 -0800 Message-Id: <1487293868-21605-1-git-send-email-azhou@ovn.org> X-Mailer: git-send-email 1.9.1 X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH 1/3] ofproto-dpif: Enhance execute_controller_action(). X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Allow execute_controller_action() to accept actions encoded with nested netlink attributes. execute_controller_action() can be called during 'xlate_actions'. It tries executes all actions translated so far to get the current packet that needs to be sent to the controller. This works fine until when the action is enclosed within a nested netlink message, and the action translation has not finished yet. For example; A, clone(B, controller, C) In this case, we can not execute 'clone' since its translation has not be finished (missing C), However, A still needs to be executed before the packet can be sent to the controller. This solution is to make a copy of the odp actions translated so far, and 'fix up' the copy so that it can be executed. The original odp actions are left intact so that xlate can continue. Signed-off-by: Andy Zhou Acked-by: Jarno Rajahalme --- ofproto/ofproto-dpif-xlate.c | 149 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 144 insertions(+), 5 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 503a347..c4ca5d2 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3805,13 +3805,150 @@ flood_packets(struct xlate_ctx *ctx, bool all) ctx->nf_output_iface = NF_OUT_FLOOD; } +/* Copy and reformat a partially xlated odp actions to a new + * odp actions list in 'b', so that the new actions list + * can be executed by odp_execute_actions. + * + * When xlate using nested odp actions, such as sample and clone, + * The nested action created by nl_msg_start_nested() may not + * have been properly closed yet, thus can not be executed + * directly. + * + * Since unclosed nested action has to be last action, it can be + * fixed by skip the outer header, and treat the actions within + * as if they are outside the nested attribute. Since the effect + * of executing them on packet is the same. + * + * As an optimization, a fully closed 'sample' or 'clone' action + * is skipped since their execution has no effect to the packet. + * + * Returns true if success. 'b' contains the new actions list. + * The caller is responsible for dispose 'b'. + * + * Returns false if error, 'b' has been freed already. */ +static bool +xlate_fixup_actions(struct ofpbuf *b, const struct nlattr *actions, + size_t actions_len) +{ + const struct nlattr *a; + unsigned int left; + + NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) { + int type = nl_attr_type(a); + + switch ((enum ovs_action_attr) type) { + case OVS_ACTION_ATTR_HASH: + case OVS_ACTION_ATTR_PUSH_VLAN: + case OVS_ACTION_ATTR_POP_VLAN: + case OVS_ACTION_ATTR_PUSH_MPLS: + case OVS_ACTION_ATTR_POP_MPLS: + case OVS_ACTION_ATTR_SET: + case OVS_ACTION_ATTR_SET_MASKED: + case OVS_ACTION_ATTR_TRUNC: + case OVS_ACTION_ATTR_OUTPUT: + case OVS_ACTION_ATTR_TUNNEL_PUSH: + case OVS_ACTION_ATTR_TUNNEL_POP: + case OVS_ACTION_ATTR_USERSPACE: + case OVS_ACTION_ATTR_RECIRC: + case OVS_ACTION_ATTR_CT: + ofpbuf_put(b, a, nl_attr_len_pad(a, left)); + break; + + case OVS_ACTION_ATTR_CLONE: + /* If the clone action has been fully xlated, it can + * be skipped, since any actions executed within clone + * do not affect the current packet. + * + * When xlating actions wihtin clone, the clone action, + * because it is an nested netlink attribute, do not have + * a vlaid 'nla_len'; it will be zero instead. Skip + * the clone heaer to find the start of the actions + * enclosed. Treat those actions as if they are written + * outside of clone. */ + if (!a->nla_len) { + bool ok; + if (left < NLA_HDRLEN) { + goto error; + } + + ok = xlate_fixup_actions(b, nl_attr_get_unspec(a, 0), + left - NLA_HDRLEN); + if (!ok) { + goto error; + } + } + break; + + case OVS_ACTION_ATTR_SAMPLE: + if (!a->nla_len) { + bool ok; + if (left < NLA_HDRLEN) { + goto error; + } + const struct nlattr *attr = nl_attr_get_unspec(a, 0); + left -= NLA_HDRLEN; + + while (left > 0 && + nl_attr_type(attr) != OVS_SAMPLE_ATTR_ACTIONS) { + /* Only OVS_SAMPLE_ATTR_ACTIONS can have unclosed + * nested netlink attribute. */ + if (!attr->nla_len) { + goto error; + } + + left -= NLA_ALIGN(attr->nla_len); + attr = nl_attr_next(attr); + } + + if (left < NLA_HDRLEN) { + goto error; + } + + ok = xlate_fixup_actions(b, nl_attr_get_unspec(attr, 0), + left - NLA_HDRLEN); + if (!ok) { + goto error; + } + } + break; + + case OVS_ACTION_ATTR_UNSPEC: + case __OVS_ACTION_ATTR_MAX: + OVS_NOT_REACHED(); + } + } + + return true; + +error: + ofpbuf_delete(b); + return false; +} + +static bool +xlate_execute_odp_actions(struct dp_packet *packet, + const struct nlattr *actions, int actions_len) +{ + struct dp_packet_batch batch; + struct ofpbuf *b = ofpbuf_new(actions_len); + + if (!xlate_fixup_actions(b, actions, actions_len)) { + return false; + } + + dp_packet_batch_init_packet(&batch, packet); + odp_execute_actions(NULL, &batch, false, b->data, b->size, NULL); + ofpbuf_delete(b); + + return true; +} + static void execute_controller_action(struct xlate_ctx *ctx, int len, enum ofp_packet_in_reason reason, uint16_t controller_id, const uint8_t *userdata, size_t userdata_len) { - struct dp_packet_batch batch; struct dp_packet *packet; ctx->xout->slow |= SLOW_CONTROLLER; @@ -3825,10 +3962,12 @@ execute_controller_action(struct xlate_ctx *ctx, int len, } packet = dp_packet_clone(ctx->xin->packet); - dp_packet_batch_init_packet(&batch, packet); - odp_execute_actions(NULL, &batch, false, - ctx->odp_actions->data, ctx->odp_actions->size, NULL); - + if (!xlate_execute_odp_actions(packet, ctx->odp_actions->data, + ctx->odp_actions->size)) { + xlate_report_error(ctx, "Failed to execute controller action"); + dp_packet_delete(packet); + return; + } /* A packet sent by an action in a table-miss rule is considered an * explicit table miss. OpenFlow before 1.3 doesn't have that concept so * it will get translated back to OFPR_ACTION for those versions. */