From 9dbb9d5e94d1db5a0fb5cb3867c26d7c3d07d0c4 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 4 Aug 2010 14:08:26 -0700 Subject: [PATCH] ofproto: Avoid user->kernel->user round-trip for many controller actions. When an OpenFlow flow says to send packets to the controller, until now ofproto has executed that using dpif_execute(), which passes the packet up to the kernel. The kernel queues the packet into its "action" queue, and then later ofproto pulls the packet back down from the kernel and sends it to the controller. However, this is unnecessary. Open vSwitch can just recognize in advance that it will get the packet back and handle it directly, skipping the round trip. This commit implements this optimization. This generally affects only the first packet in a flow, since generally the rest come directly down from the kernel. It only optimizes the "easy" case where the first action in a flow is to send the packet to the controller, since this seems to be the common case in the flows that I'm looking at now. --- lib/dpif-provider.h | 8 -------- lib/dpif.h | 12 +++++++++++- ofproto/ofproto.c | 37 +++++++++++++++++++++++++++++++++++-- 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index b2f9d4bd1..1106db888 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -324,14 +324,6 @@ struct dpif_class { void (*recv_wait)(struct dpif *dpif); }; -/* Minimum number of bytes of headroom for a packet returned by the 'recv' - * member function (see above). This headroom allows "struct odp_msg" to be - * replaced by "struct ofp_packet_in" without copying the buffer. */ -#define DPIF_RECV_MSG_PADDING (sizeof(struct ofp_packet_in) \ - - sizeof(struct odp_msg)) -BUILD_ASSERT_DECL(sizeof(struct ofp_packet_in) > sizeof(struct odp_msg)); -BUILD_ASSERT_DECL(DPIF_RECV_MSG_PADDING % 4 == 0); - extern const struct dpif_class dpif_linux_class; extern const struct dpif_class dpif_netdev_class; diff --git a/lib/dpif.h b/lib/dpif.h index a63972545..1496c227f 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -18,10 +18,12 @@ #ifndef DPIF_H #define DPIF_H 1 -#include "openvswitch/datapath-protocol.h" #include #include #include +#include "openflow/openflow.h" +#include "openvswitch/datapath-protocol.h" +#include "util.h" #ifdef __cplusplus extern "C" { @@ -90,6 +92,14 @@ int dpif_execute(struct dpif *, uint16_t in_port, const union odp_action[], size_t n_actions, const struct ofpbuf *); +/* Minimum number of bytes of headroom for a packet returned by dpif_recv() + * member function. This headroom allows "struct odp_msg" to be replaced by + * "struct ofp_packet_in" without copying the buffer. */ +#define DPIF_RECV_MSG_PADDING (sizeof(struct ofp_packet_in) \ + - sizeof(struct odp_msg)) +BUILD_ASSERT_DECL(sizeof(struct ofp_packet_in) > sizeof(struct odp_msg)); +BUILD_ASSERT_DECL(DPIF_RECV_MSG_PADDING % 4 == 0); + int dpif_recv_get_mask(const struct dpif *, int *listen_mask); int dpif_recv_set_mask(struct dpif *, int listen_mask); int dpif_get_sflow_probability(const struct dpif *, uint32_t *probability); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 2e1530a3f..a9e270b9f 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1927,6 +1927,39 @@ rule_has_out_port(const struct rule *rule, uint16_t out_port) return false; } +static bool +execute_odp_actions(struct ofproto *ofproto, uint16_t in_port, + const union odp_action *actions, size_t n_actions, + const struct ofpbuf *packet) +{ + if (n_actions > 0 && actions[0].type == ODPAT_CONTROLLER) { + /* As an optimization, avoid a round-trip from userspace to kernel to + * userspace. This also avoids possibly filling up kernel packet + * buffers along the way. */ + struct ofpbuf *copy; + struct odp_msg *msg; + + copy = ofpbuf_new(DPIF_RECV_MSG_PADDING + sizeof(struct odp_msg) + + packet->size); + ofpbuf_reserve(copy, DPIF_RECV_MSG_PADDING); + msg = ofpbuf_put_uninit(copy, sizeof *msg); + msg->type = _ODPL_ACTION_NR; + msg->length = sizeof(struct odp_msg) + packet->size; + msg->port = in_port; + msg->reserved = 0; + msg->arg = actions[0].controller.arg; + ofpbuf_put(copy, packet->data, packet->size); + + send_packet_in(ofproto, copy); + + actions++; + n_actions--; + } + + return !n_actions || !dpif_execute(ofproto->dpif, in_port, + actions, n_actions, packet); +} + /* Executes the actions indicated by 'rule' on 'packet', which is in flow * 'flow' and is considered to have arrived on ODP port 'in_port'. * @@ -1969,8 +2002,8 @@ rule_execute(struct ofproto *ofproto, struct rule *rule, } /* Execute the ODP actions. */ - if (!dpif_execute(ofproto->dpif, flow->in_port, - actions, n_actions, packet)) { + if (execute_odp_actions(ofproto, flow->in_port, + actions, n_actions, packet)) { struct odp_flow_stats stats; flow_extract_stats(flow, packet, &stats); update_stats(ofproto, rule, &stats); -- 2.43.0