Skip to content

Commit

Permalink
ofproto: Refactor packet_out handling.
Browse files Browse the repository at this point in the history
Refactor handle_packet_out() to prepare for bundle support for packet
outs in a later patch.

Two new callbacks are introduced in ofproto-provider class:
->packet_xlate() and ->packet_execute().  ->packet_xlate() translates
the packet using the flow and actions provided by the caller, but
defers all OpenFlow-visible side-effects (stats, learn actions, actual
packet output, etc.) to be explicitly executed with the
->packet_execute() call.

Adds a new ofproto_rule_reduce_timeouts__() that must be called with
'ofproto_mutex' held.  This is used in the next patch.

Signed-off-by: Jarno Rajahalme <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
  • Loading branch information
Jarno Rajahalme committed Sep 14, 2016
1 parent 493f017 commit 1f4a893
Show file tree
Hide file tree
Showing 9 changed files with 490 additions and 182 deletions.
11 changes: 8 additions & 3 deletions ofproto/ofproto-dpif-upcall.c
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,9 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
stats.used = time_msec();
stats.tcp_flags = ntohs(upcall->flow->tcp_flags);

xlate_in_init(&xin, upcall->ofproto, upcall->flow, upcall->in_port, NULL,
xlate_in_init(&xin, upcall->ofproto,
ofproto_dpif_get_tables_version(upcall->ofproto),
upcall->flow, upcall->in_port, NULL,
stats.tcp_flags, upcall->packet, wc, odp_actions);

if (upcall->type == DPIF_UC_MISS) {
Expand Down Expand Up @@ -1908,7 +1910,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
ukey->xcache = xlate_cache_new();
}

xlate_in_init(&xin, ofproto, &flow, ofp_in_port, NULL, push.tcp_flags,
xlate_in_init(&xin, ofproto, ofproto_dpif_get_tables_version(ofproto),
&flow, ofp_in_port, NULL, push.tcp_flags,
NULL, need_revalidate ? &wc : NULL, odp_actions);
if (push.n_packets) {
xin.resubmit_stats = &push;
Expand Down Expand Up @@ -2086,7 +2089,9 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, size_t n_ops)
if (!error) {
struct xlate_in xin;

xlate_in_init(&xin, ofproto, &flow, ofp_in_port, NULL,
xlate_in_init(&xin, ofproto,
ofproto_dpif_get_tables_version(ofproto),
&flow, ofp_in_port, NULL,
push->tcp_flags, NULL, NULL, NULL);
xin.resubmit_stats = push->n_packets ? push : NULL;
xin.allow_side_effects = push->n_packets > 0;
Expand Down
23 changes: 17 additions & 6 deletions ofproto/ofproto-dpif-xlate-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@

#include "ofproto/ofproto-dpif-xlate-cache.h"

#include <errno.h>
#include <arpa/inet.h>
#include <errno.h>
#include <net/if.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <sys/socket.h>

#include "bfd.h"
#include "bitmap.h"
Expand All @@ -35,8 +35,8 @@
#include "mac-learning.h"
#include "netdev-vport.h"
#include "ofproto/ofproto-dpif-mirror.h"
#include "ofproto/ofproto-dpif.h"
#include "ofproto/ofproto-dpif-xlate.h"
#include "ofproto/ofproto-dpif.h"
#include "ofproto/ofproto-provider.h"
#include "openvswitch/dynamic-string.h"
#include "openvswitch/vlog.h"
Expand All @@ -47,12 +47,17 @@

VLOG_DEFINE_THIS_MODULE(ofproto_xlate_cache);

void
xlate_cache_init(struct xlate_cache *xcache)
{
ofpbuf_init(&xcache->entries, 120);
}

struct xlate_cache *
xlate_cache_new(void)
{
struct xlate_cache *xcache = xmalloc(sizeof *xcache);

ofpbuf_init(&xcache->entries, 512);
xlate_cache_init(xcache);
return xcache;
}

Expand Down Expand Up @@ -261,9 +266,15 @@ xlate_cache_clear(struct xlate_cache *xcache)
}

void
xlate_cache_delete(struct xlate_cache *xcache)
xlate_cache_uninit(struct xlate_cache *xcache)
{
xlate_cache_clear(xcache);
ofpbuf_uninit(&xcache->entries);
}

void
xlate_cache_delete(struct xlate_cache *xcache)
{
xlate_cache_uninit(xcache);
free(xcache);
}
2 changes: 2 additions & 0 deletions ofproto/ofproto-dpif-xlate-cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,14 @@ struct xlate_cache {
struct ofpbuf entries;
};

void xlate_cache_init(struct xlate_cache *);
struct xlate_cache *xlate_cache_new(void);
struct xc_entry *xlate_cache_add_entry(struct xlate_cache *, enum xc_type);
void xlate_push_stats_entry(struct xc_entry *, const struct dpif_flow_stats *);
void xlate_push_stats(struct xlate_cache *, const struct dpif_flow_stats *);
void xlate_cache_clear_entry(struct xc_entry *);
void xlate_cache_clear(struct xlate_cache *);
void xlate_cache_uninit(struct xlate_cache *);
void xlate_cache_delete(struct xlate_cache *);

#endif /* ofproto-dpif-xlate-cache.h */
40 changes: 22 additions & 18 deletions ofproto/ofproto-dpif-xlate.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,6 @@ struct xlate_ctx {

const struct xbridge *xbridge;

/* Flow tables version at the beginning of the translation. */
ovs_version_t tables_version;

/* Flow at the last commit. */
struct flow base_flow;

Expand Down Expand Up @@ -1432,7 +1429,7 @@ group_is_alive(const struct xlate_ctx *ctx, uint32_t group_id, int depth)
struct group_dpif *group;

group = group_dpif_lookup(ctx->xbridge->ofproto, group_id,
ctx->tables_version, false);
ctx->xin->tables_version, false);
if (group) {
return group_first_live_bucket(ctx, group, depth) != NULL;
}
Expand Down Expand Up @@ -2699,8 +2696,9 @@ compose_table_xlate(struct xlate_ctx *ctx, const struct xport *out_dev,
output.port = OFPP_TABLE;
output.max_len = 0;

return ofproto_dpif_execute_actions__(xbridge->ofproto, &flow, NULL,
&output.ofpact, sizeof output,
return ofproto_dpif_execute_actions__(xbridge->ofproto,
ctx->xin->tables_version, &flow,
NULL, &output.ofpact, sizeof output,
ctx->indentation, ctx->depth,
ctx->resubmits, packet);
}
Expand Down Expand Up @@ -2888,7 +2886,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
struct flow old_flow = ctx->xin->flow;
bool old_conntrack = ctx->conntracked;
bool old_was_mpls = ctx->was_mpls;
ovs_version_t old_version = ctx->tables_version;
ovs_version_t old_version = ctx->xin->tables_version;
struct ofpbuf old_stack = ctx->stack;
union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)];
struct ofpbuf old_action_set = ctx->action_set;
Expand All @@ -2906,7 +2904,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
clear_conntrack(flow);

/* The bridge is now known so obtain its table version. */
ctx->tables_version
ctx->xin->tables_version
= ofproto_dpif_get_tables_version(ctx->xbridge->ofproto);

if (!process_special(ctx, peer) && may_receive(peer, ctx)) {
Expand Down Expand Up @@ -2943,7 +2941,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
ctx->stack = old_stack;

/* Restore calling bridge's lookup version. */
ctx->tables_version = old_version;
ctx->xin->tables_version = old_version;

/* The peer bridge popping MPLS should have no effect on the original
* bridge. */
Expand Down Expand Up @@ -3184,7 +3182,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
ctx->table_id = table_id;

rule = rule_dpif_lookup_from_table(ctx->xbridge->ofproto,
ctx->tables_version,
ctx->xin->tables_version,
&ctx->xin->flow, ctx->wc,
ctx->xin->resubmit_stats,
&ctx->table_id, in_port,
Expand Down Expand Up @@ -3432,7 +3430,7 @@ xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)

/* Take ref only if xcache exists. */
group = group_dpif_lookup(ctx->xbridge->ofproto, group_id,
ctx->tables_version, ctx->xin->xcache);
ctx->xin->tables_version, ctx->xin->xcache);
if (!group) {
/* XXX: Should set ctx->error ? */
return true;
Expand Down Expand Up @@ -4956,12 +4954,13 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,

void
xlate_in_init(struct xlate_in *xin, struct ofproto_dpif *ofproto,
const struct flow *flow, ofp_port_t in_port,
struct rule_dpif *rule, uint16_t tcp_flags,
ovs_version_t version, const struct flow *flow,
ofp_port_t in_port, struct rule_dpif *rule, uint16_t tcp_flags,
const struct dp_packet *packet, struct flow_wildcards *wc,
struct ofpbuf *odp_actions)
{
xin->ofproto = ofproto;
xin->tables_version = version;
xin->flow = *flow;
xin->flow.in_port.ofp_port = in_port;
xin->flow.actset_output = OFPP_UNSET;
Expand Down Expand Up @@ -5314,6 +5313,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
goto exit;
}
ctx.xbridge = new_bridge;
/* The bridge is now known so obtain its table version. */
ctx.xin->tables_version
= ofproto_dpif_get_tables_version(ctx.xbridge->ofproto);
}

/* Set the thawed table id. Note: A table lookup is done only if there
Expand Down Expand Up @@ -5364,12 +5366,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
ctx.error = XLATE_NO_RECIRCULATION_CONTEXT;
goto exit;
}
/* The bridge is now known so obtain its table version. */
ctx.tables_version = ofproto_dpif_get_tables_version(ctx.xbridge->ofproto);

if (!xin->ofpacts && !ctx.rule) {
ctx.rule = rule_dpif_lookup_from_table(
ctx.xbridge->ofproto, ctx.tables_version, flow, ctx.wc,
ctx.xbridge->ofproto, ctx.xin->tables_version, flow, ctx.wc,
ctx.xin->resubmit_stats, &ctx.table_id,
flow->in_port.ofp_port, true, true, ctx.xin->xcache);
if (ctx.xin->resubmit_stats) {
Expand Down Expand Up @@ -5552,7 +5552,8 @@ xlate_resume(struct ofproto_dpif *ofproto,
flow_extract(&packet, &flow);

struct xlate_in xin;
xlate_in_init(&xin, ofproto, &flow, 0, NULL, ntohs(flow.tcp_flags),
xlate_in_init(&xin, ofproto, ofproto_dpif_get_tables_version(ofproto),
&flow, 0, NULL, ntohs(flow.tcp_flags),
&packet, NULL, odp_actions);

struct ofpact_note noop;
Expand Down Expand Up @@ -5630,7 +5631,10 @@ xlate_send_packet(const struct ofport_dpif *ofport, bool oam,

ofpact_put_OUTPUT(&ofpacts)->port = xport->ofp_port;

return ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow, NULL,
/* Actions here are not referring to anything versionable (flow tables or
* groups) so we don't need to worry about the version here. */
return ofproto_dpif_execute_actions(xport->xbridge->ofproto,
OVS_VERSION_MAX, &flow, NULL,
ofpacts.data, ofpacts.size, packet);
}

Expand Down
3 changes: 2 additions & 1 deletion ofproto/ofproto-dpif-xlate.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ struct xlate_out {

struct xlate_in {
struct ofproto_dpif *ofproto;
ovs_version_t tables_version; /* Lookup in this version. */

/* Flow to which the OpenFlow actions apply. xlate_actions() will modify
* this flow when actions change header fields. */
Expand Down Expand Up @@ -202,7 +203,7 @@ const char *xlate_strerror(enum xlate_error error);

enum xlate_error xlate_actions(struct xlate_in *, struct xlate_out *);

void xlate_in_init(struct xlate_in *, struct ofproto_dpif *,
void xlate_in_init(struct xlate_in *, struct ofproto_dpif *, ovs_version_t,
const struct flow *, ofp_port_t in_port, struct rule_dpif *,
uint16_t tcp_flags, const struct dp_packet *packet,
struct flow_wildcards *, struct ofpbuf *odp_actions);
Expand Down
Loading

0 comments on commit 1f4a893

Please sign in to comment.