[ovs-dev] [PATCH V2 1/3] ofproto-dpif: Move send_packet() to ofproto-dpif-xlate module.
Alex Wang
alexw at nicira.com
Fri Sep 27 22:39:57 UTC 2013
This commit moves the main logic of send_packet() function into
the ofproto-dpif-xlate module. Also, the xlate_actions() function
in ofproto-dpif-xlate module is adjusted to guarantee thread safety.
Signed-off-by: Alex Wang <alexw at nicira.com>
---
v1 -> v2:
- change xlate_actions to a thread safe version and thread unsafe version
---
ofproto/ofproto-dpif-upcall.c | 4 +-
ofproto/ofproto-dpif-xlate.c | 85 +++++++++++++++++++++++++++++++++++++----
ofproto/ofproto-dpif-xlate.h | 5 ++-
ofproto/ofproto-dpif.c | 71 ++++++++--------------------------
ofproto/ofproto-dpif.h | 1 +
5 files changed, 101 insertions(+), 65 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 180b87e..33c75b7 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -230,7 +230,7 @@ udpif_wait(struct udpif *udpif)
}
/* Notifies 'udpif' that something changed which may render previous
- * xlate_actions() results invalid. */
+ * xlate_actions_safe() results invalid. */
void
udpif_revalidate(struct udpif *udpif)
{
@@ -731,7 +731,7 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls)
miss->stats.tcp_flags, NULL);
xin.may_learn = true;
xin.resubmit_stats = &miss->stats;
- xlate_actions(&xin, &miss->xout);
+ xlate_actions_safe(&xin, &miss->xout);
flow_wildcards_or(&miss->xout.wc, &miss->xout.wc, &wc);
rule_dpif_unref(rule);
}
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a5b6814..020588f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -192,6 +192,8 @@ static struct hmap xports = HMAP_INITIALIZER(&xports);
static bool may_receive(const struct xport *, struct xlate_ctx *);
static void do_xlate_actions(const struct ofpact *, size_t ofpacts_len,
struct xlate_ctx *);
+static void xlate_actions__(struct xlate_in *, struct xlate_out *)
+ OVS_REQ_RDLOCK(xlate_rwlock);
static void xlate_normal(struct xlate_ctx *);
static void xlate_report(struct xlate_ctx *, const char *);
static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
@@ -2434,7 +2436,7 @@ xlate_actions_for_side_effects(struct xlate_in *xin)
{
struct xlate_out xout;
- xlate_actions(xin, &xout);
+ xlate_actions_safe(xin, &xout);
xlate_out_uninit(&xout);
}
@@ -2515,13 +2517,31 @@ actions_output_to_local_port(const struct xlate_ctx *ctx)
return false;
}
+/* Thread safe call to xlate_actions__(). */
+void
+xlate_actions_safe(struct xlate_in *xin, struct xlate_out *xout)
+{
+ ovs_rwlock_rdlock(&xlate_rwlock);
+ xlate_actions__(xin, xout);
+ ovs_rwlock_unlock(&xlate_rwlock);
+}
+
+/* Thread unsafe call to xlate_actions__().
+ * Caller must acquire rdlock first. */
+void
+xlate_actions_unsafe(struct xlate_in *xin, struct xlate_out *xout)
+{
+ xlate_actions__(xin, xout);
+}
+
/* Translates the 'ofpacts_len' bytes of "struct ofpacts" starting at 'ofpacts'
* into datapath actions in 'odp_actions', using 'ctx'.
*
* The caller must take responsibility for eventually freeing 'xout', with
* xlate_out_uninit(). */
-void
-xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
+static void
+xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
+ OVS_REQ_RDLOCK(xlate_rwlock)
{
struct flow_wildcards *wc = &xout->wc;
struct flow *flow = &xin->flow;
@@ -2537,8 +2557,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
COVERAGE_INC(xlate_actions);
- ovs_rwlock_rdlock(&xlate_rwlock);
-
/* Flow initialization rules:
* - 'base_flow' must match the kernel's view of the packet at the
* time that action processing starts. 'flow' represents any
@@ -2690,7 +2708,60 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
memset(&wc->masks.regs, 0, sizeof wc->masks.regs);
out:
- ovs_rwlock_unlock(&xlate_rwlock);
-
rule_actions_unref(actions);
}
+
+/* Sends 'packet' out 'ofport'.
+ * May modify 'packet'.
+ * Returns 0 if successful, otherwise a positive errno value. */
+int
+xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
+{
+ uint64_t odp_actions_stub[1024 / 8];
+ struct xport *xport;
+ struct ofpbuf key, odp_actions;
+ struct dpif_flow_stats stats;
+ struct odputil_keybuf keybuf;
+ struct ofpact_output output;
+ struct xlate_out xout;
+ struct xlate_in xin;
+ struct flow flow;
+ union flow_in_port in_port_;
+ int error;
+
+ ovs_rwlock_rdlock(&xlate_rwlock);
+ xport = xport_lookup(ofport);
+ if (!xport) {
+ error = EINVAL;
+ goto out;
+ }
+
+ ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub);
+ ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
+
+ /* Use OFPP_NONE as the in_port to avoid special packet processing. */
+ in_port_.ofp_port = OFPP_NONE;
+ flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
+ odp_flow_key_from_flow(&key, &flow, ofp_port_to_odp_port(xport->xbridge, OFPP_LOCAL));
+ dpif_flow_stats_extract(&flow, packet, time_msec(), &stats);
+
+ ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
+ output.port = xport->ofp_port;
+ output.max_len = 0;
+
+ xlate_in_init(&xin, xport->xbridge->ofproto, &flow, NULL, 0, packet);
+ xin.ofpacts_len = sizeof output;
+ xin.ofpacts = &output.ofpact;
+ xin.resubmit_stats = &stats;
+ xlate_actions_unsafe(&xin, &xout);
+
+ error = dpif_execute(xport->xbridge->dpif,
+ key.data, key.size,
+ xout.odp_actions.data, xout.odp_actions.size,
+ packet);
+ xlate_out_uninit(&xout);
+
+out:
+ ovs_rwlock_unlock(&xlate_rwlock);
+ return error;
+}
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index a54a9e4..9e7a853 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -146,12 +146,15 @@ int xlate_receive(const struct dpif_backer *, struct ofpbuf *packet,
struct ofproto_dpif **, odp_port_t *odp_in_port)
OVS_EXCLUDED(xlate_rwlock);
-void xlate_actions(struct xlate_in *, struct xlate_out *)
+void xlate_actions_safe(struct xlate_in *, struct xlate_out *)
OVS_EXCLUDED(xlate_rwlock);
+void xlate_actions_unsafe(struct xlate_in *, struct xlate_out *)
+ OVS_REQ_RDLOCK(xlate_rwlock);
void xlate_in_init(struct xlate_in *, struct ofproto_dpif *,
const struct flow *, struct rule_dpif *,
uint8_t tcp_flags, const struct ofpbuf *packet);
void xlate_out_uninit(struct xlate_out *);
void xlate_actions_for_side_effects(struct xlate_in *);
void xlate_out_copy(struct xlate_out *dst, const struct xlate_out *src);
+int xlate_send_packet(const struct ofport_dpif *, struct ofpbuf *);
#endif /* ofproto-dpif-xlate.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 80874b8..42f3d98 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -540,9 +540,6 @@ static int expire(struct dpif_backer *);
/* NetFlow. */
static void send_netflow_active_timeouts(struct ofproto_dpif *);
-/* Utilities. */
-static int send_packet(const struct ofport_dpif *, struct ofpbuf *packet);
-
/* Global variables. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -2018,7 +2015,7 @@ send_bpdu_cb(struct ofpbuf *pkt, int port_num, void *ofproto_)
VLOG_WARN_RL(&rl, "%s: cannot send BPDU on port %d "
"with unknown MAC", ofproto->up.name, port_num);
} else {
- send_packet(ofport, pkt);
+ ofproto_dpif_send_packet(ofport, pkt);
}
}
ofpbuf_delete(pkt);
@@ -2614,7 +2611,7 @@ send_pdu_cb(void *port_, const void *pdu, size_t pdu_size)
pdu_size);
memcpy(packet_pdu, pdu, pdu_size);
- send_packet(port, &packet);
+ ofproto_dpif_send_packet(port, &packet);
ofpbuf_uninit(&packet);
} else {
VLOG_ERR_RL(&rl, "port %s: cannot obtain Ethernet address of iface "
@@ -2651,7 +2648,7 @@ bundle_send_learning_packets(struct ofbundle *bundle)
LIST_FOR_EACH (learning_packet, list_node, &packets) {
int ret;
- ret = send_packet(learning_packet->private_p, learning_packet);
+ ret = ofproto_dpif_send_packet(learning_packet->private_p, learning_packet);
if (ret) {
error = ret;
n_errors++;
@@ -2873,7 +2870,7 @@ port_run_fast(struct ofport_dpif *ofport)
ofpbuf_init(&packet, 0);
cfm_compose_ccm(ofport->cfm, &packet, ofport->up.pp.hw_addr);
- send_packet(ofport, &packet);
+ ofproto_dpif_send_packet(ofport, &packet);
ofpbuf_uninit(&packet);
}
@@ -2882,7 +2879,7 @@ port_run_fast(struct ofport_dpif *ofport)
ofpbuf_init(&packet, 0);
bfd_put_packet(ofport->bfd, &packet, ofport->up.pp.hw_addr);
- send_packet(ofport, &packet);
+ ofproto_dpif_send_packet(ofport, &packet);
ofpbuf_uninit(&packet);
}
}
@@ -4200,7 +4197,7 @@ facet_check_consistency(struct facet *facet)
/* Check the datapath actions for consistency. */
rule_dpif_lookup(facet->ofproto, &facet->flow, NULL, &rule);
xlate_in_init(&xin, facet->ofproto, &facet->flow, rule, 0, NULL);
- xlate_actions(&xin, &xout);
+ xlate_actions_safe(&xin, &xout);
rule_dpif_unref(rule);
ok = ofpbuf_equal(&facet->xout.odp_actions, &xout.odp_actions)
@@ -4284,7 +4281,7 @@ facet_revalidate(struct facet *facet)
* emit a NetFlow expiration and, if so, we need to have the old state
* around to properly compose it. */
xlate_in_init(&xin, ofproto, &facet->flow, new_rule, 0, NULL);
- xlate_actions(&xin, &xout);
+ xlate_actions_safe(&xin, &xout);
flow_wildcards_or(&xout.wc, &xout.wc, &wc);
/* A facet's slow path reason should only change under dramatic
@@ -4909,7 +4906,7 @@ rule_dpif_execute(struct rule_dpif *rule, const struct flow *flow,
xlate_in_init(&xin, ofproto, flow, rule, stats.tcp_flags, packet);
xin.resubmit_stats = &stats;
- xlate_actions(&xin, &xout);
+ xlate_actions_safe(&xin, &xout);
execute_odp_actions(ofproto, flow, xout.odp_actions.data,
xout.odp_actions.size, packet);
@@ -4945,55 +4942,19 @@ rule_modify_actions(struct rule *rule_, bool reset_counters)
/* Sends 'packet' out 'ofport'.
* May modify 'packet'.
* Returns 0 if successful, otherwise a positive errno value. */
-static int
-send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
+int
+ofproto_dpif_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
- uint64_t odp_actions_stub[1024 / 8];
- struct ofpbuf key, odp_actions;
- struct dpif_flow_stats stats;
- struct odputil_keybuf keybuf;
- struct ofpact_output output;
- struct xlate_out xout;
- struct xlate_in xin;
- struct flow flow;
- union flow_in_port in_port_;
int error;
- ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub);
- ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
-
- /* Use OFPP_NONE as the in_port to avoid special packet processing. */
- in_port_.ofp_port = OFPP_NONE;
- flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
- odp_flow_key_from_flow(&key, &flow, ofp_port_to_odp_port(ofproto,
- OFPP_LOCAL));
- dpif_flow_stats_extract(&flow, packet, time_msec(), &stats);
-
- ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
- output.port = ofport->up.ofp_port;
- output.max_len = 0;
-
- xlate_in_init(&xin, ofproto, &flow, NULL, 0, packet);
- xin.ofpacts_len = sizeof output;
- xin.ofpacts = &output.ofpact;
- xin.resubmit_stats = &stats;
- xlate_actions(&xin, &xout);
-
- error = dpif_execute(ofproto->backer->dpif,
- key.data, key.size,
- xout.odp_actions.data, xout.odp_actions.size,
- packet);
- xlate_out_uninit(&xout);
+ error = xlate_send_packet(ofport, packet);
- if (error) {
- VLOG_WARN_RL(&rl, "%s: failed to send packet on port %s (%s)",
- ofproto->up.name, netdev_get_name(ofport->up.netdev),
- ovs_strerror(error));
+ if (!error) {
+ ofproto->stats.tx_packets++;
+ ofproto->stats.tx_bytes += packet->size;
}
- ofproto->stats.tx_packets++;
- ofproto->stats.tx_bytes += packet->size;
return error;
}
@@ -5075,7 +5036,7 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet,
xin.ofpacts_len = ofpacts_len;
xin.ofpacts = ofpacts;
- xlate_actions(&xin, &xout);
+ xlate_actions_safe(&xin, &xout);
dpif_execute(ofproto->backer->dpif, key.data, key.size,
xout.odp_actions.data, xout.odp_actions.size, packet);
xlate_out_uninit(&xout);
@@ -5495,7 +5456,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
trace.xin.resubmit_hook = trace_resubmit;
trace.xin.report_hook = trace_report;
- xlate_actions(&trace.xin, &trace.xout);
+ xlate_actions_safe(&trace.xin, &trace.xout);
flow_wildcards_or(&trace.xout.wc, &trace.xout.wc, &wc);
ds_put_char(ds, '\n');
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 14a9669..0863efd 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -95,6 +95,7 @@ bool vsp_adjust_flow(const struct ofproto_dpif *, struct flow *);
void ofproto_dpif_send_packet_in(struct ofproto_dpif *,
struct ofputil_packet_in *pin);
+int ofproto_dpif_send_packet(const struct ofport_dpif *, struct ofpbuf *);
void ofproto_dpif_flow_mod(struct ofproto_dpif *, struct ofputil_flow_mod *);
struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t);
--
1.7.9.5
More information about the dev
mailing list