mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 14:25:26 +00:00
Code Issues Releases Wiki Activity

dpif: Factor 'type' and 'error' out of individual dpif_op members.

Browse Source 
I'd like to change ->dpif_flow_put() and ->dpif_execute() in the dpif
provider to take the structures of the same names as parameters, instead of
passing them discrete parameters, because this seems like a more sensible
way to do things internally than to have two different ways to pass the
parameters.  It might even simplify code slightly.  But ->flow_put() and
->execute() wouldn't want the 'type' (because it's implied by the function
being called) or 'error' (because it would be the same as the return
value).  Although of course they could just ignore those members, it seems
slightly cleaner to omit them entirely, as this change allows.

Signed-off-by: Ben Pfaff <blp@nicira.com>
This commit is contained in:
Ben Pfaff
2011-12-26 14:17:55 -08:00
parent 8338659aa5
commit c2b565b54e
5 changed files with 36 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
lib/dpif-linux.c
View File

@@ -873,7 +873,7 @@ dpif_linux_execute(struct dpif *dpif_,
} }
static void static void
dpif_linux_operate(struct dpif *dpif_, union dpif_op **ops, size_t n_ops) dpif_linux_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
{ {
struct dpif_linux *dpif = dpif_linux_cast(dpif_); struct dpif_linux *dpif = dpif_linux_cast(dpif_);
struct nl_transaction **txnsp; struct nl_transaction **txnsp;
@@ -883,10 +883,10 @@ dpif_linux_operate(struct dpif *dpif_, union dpif_op **ops, size_t n_ops)
txns = xmalloc(n_ops * sizeof *txns); txns = xmalloc(n_ops * sizeof *txns);
for (i = 0; i < n_ops; i++) { for (i = 0; i < n_ops; i++) {
struct nl_transaction *txn = &txns[i]; struct nl_transaction *txn = &txns[i];
union dpif_op *op = ops[i]; struct dpif_op *op = ops[i];
if (op->type == DPIF_OP_FLOW_PUT) { if (op->type == DPIF_OP_FLOW_PUT) {
struct dpif_flow_put *put = &op->flow_put; struct dpif_flow_put *put = &op->u.flow_put;
struct dpif_linux_flow request; struct dpif_linux_flow request;
dpif_linux_init_flow_put(dpif_, put->flags, put->key, put->key_len, dpif_linux_init_flow_put(dpif_, put->flags, put->key, put->key_len,
@@ -898,7 +898,7 @@ dpif_linux_operate(struct dpif *dpif_, union dpif_op **ops, size_t n_ops)
txn->request = ofpbuf_new(1024); txn->request = ofpbuf_new(1024);
dpif_linux_flow_to_ofpbuf(&request, txn->request); dpif_linux_flow_to_ofpbuf(&request, txn->request);
} else if (op->type == DPIF_OP_EXECUTE) { } else if (op->type == DPIF_OP_EXECUTE) {
struct dpif_execute *execute = &op->execute; struct dpif_execute *execute = &op->u.execute;
txn->request = dpif_linux_encode_execute( txn->request = dpif_linux_encode_execute(
dpif->dp_ifindex, execute->key, execute->key_len, dpif->dp_ifindex, execute->key, execute->key_len,
@@ -919,10 +919,10 @@ dpif_linux_operate(struct dpif *dpif_, union dpif_op **ops, size_t n_ops)
for (i = 0; i < n_ops; i++) { for (i = 0; i < n_ops; i++) {
struct nl_transaction *txn = &txns[i]; struct nl_transaction *txn = &txns[i];
union dpif_op *op = ops[i]; struct dpif_op *op = ops[i];
if (op->type == DPIF_OP_FLOW_PUT) { if (op->type == DPIF_OP_FLOW_PUT) {
struct dpif_flow_put *put = &op->flow_put; struct dpif_flow_put *put = &op->u.flow_put;
int error = txn->error; int error = txn->error;
if (!error && put->stats) { if (!error && put->stats) {
@@ -933,11 +933,9 @@ dpif_linux_operate(struct dpif *dpif_, union dpif_op **ops, size_t n_ops)
dpif_linux_flow_get_stats(&reply, put->stats); dpif_linux_flow_get_stats(&reply, put->stats);
} }
} }
put->error = error; op->error = error;
} else if (op->type == DPIF_OP_EXECUTE) { } else if (op->type == DPIF_OP_EXECUTE) {
struct dpif_execute *execute = &op->execute; op->error = txn->error;
execute->error = txn->error;
} else { } else {
NOT_REACHED(); NOT_REACHED();
} }

2
lib/dpif-provider.h
View File

@@ -299,7 +299,7 @@ struct dpif_class {
* *
* This function is optional. It is only worthwhile to implement it if * This function is optional. It is only worthwhile to implement it if
* 'dpif' can perform operations in batch faster than individually. */ * 'dpif' can perform operations in batch faster than individually. */
void (*operate)(struct dpif *dpif, union dpif_op **ops, size_t n_ops); void (*operate)(struct dpif *dpif, struct dpif_op **ops, size_t n_ops);
/* Enables or disables receiving packets with dpif_recv() for 'dpif'. /* Enables or disables receiving packets with dpif_recv() for 'dpif'.
* Turning packet receive off and then back on is allowed to change Netlink * Turning packet receive off and then back on is allowed to change Netlink

24
lib/dpif.c
View File

@@ -984,7 +984,7 @@ dpif_execute(struct dpif *dpif,
* This function exists because some datapaths can perform batched operations * This function exists because some datapaths can perform batched operations
* faster than individual operations. */ * faster than individual operations. */
void void
dpif_operate(struct dpif *dpif, union dpif_op **ops, size_t n_ops) dpif_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops)
{ {
size_t i; size_t i;
@@ -994,25 +994,25 @@ dpif_operate(struct dpif *dpif, union dpif_op **ops, size_t n_ops)
} }
for (i = 0; i < n_ops; i++) { for (i = 0; i < n_ops; i++) {
union dpif_op *op = ops[i]; struct dpif_op *op = ops[i];
struct dpif_flow_put *put; struct dpif_flow_put *put;
struct dpif_execute *execute; struct dpif_execute *execute;
switch (op->type) { switch (op->type) {
case DPIF_OP_FLOW_PUT: case DPIF_OP_FLOW_PUT:
put = &op->flow_put; put = &op->u.flow_put;
put->error = dpif_flow_put(dpif, put->flags, op->error = dpif_flow_put(dpif, put->flags,
put->key, put->key_len, put->key, put->key_len,
put->actions, put->actions_len, put->actions, put->actions_len,
put->stats); put->stats);
break; break;
case DPIF_OP_EXECUTE: case DPIF_OP_EXECUTE:
execute = &op->execute; execute = &op->u.execute;
execute->error = dpif_execute(dpif, execute->key, execute->key_len, op->error = dpif_execute(dpif, execute->key, execute->key_len,
execute->actions, execute->actions,
execute->actions_len, execute->actions_len,
execute->packet); execute->packet);
break; break;
default: default:

20
lib/dpif.h
View File

@@ -181,8 +181,6 @@ enum dpif_op_type {
}; };
struct dpif_flow_put { struct dpif_flow_put {
enum dpif_op_type type; /* Always DPIF_OP_FLOW_PUT. */
/* Input. */ /* Input. */
enum dpif_flow_put_flags flags; /* DPIF_FP_*. */ enum dpif_flow_put_flags flags; /* DPIF_FP_*. */
const struct nlattr *key; /* Flow to put. */ const struct nlattr *key; /* Flow to put. */
@@ -192,30 +190,26 @@ struct dpif_flow_put {
/* Output. */ /* Output. */
struct dpif_flow_stats *stats; /* Optional flow statistics. */ struct dpif_flow_stats *stats; /* Optional flow statistics. */
int error; /* 0 or positive errno value. */
}; };
struct dpif_execute { struct dpif_execute {
enum dpif_op_type type; /* Always DPIF_OP_EXECUTE. */
/* Input. */
const struct nlattr *key; /* Partial flow key (only for metadata). */ const struct nlattr *key; /* Partial flow key (only for metadata). */
size_t key_len; /* Length of 'key' in bytes. */ size_t key_len; /* Length of 'key' in bytes. */
const struct nlattr *actions; /* Actions to execute on packet. */ const struct nlattr *actions; /* Actions to execute on packet. */
size_t actions_len; /* Length of 'actions' in bytes. */ size_t actions_len; /* Length of 'actions' in bytes. */
const struct ofpbuf *packet; /* Packet to execute. */ const struct ofpbuf *packet; /* Packet to execute. */
/* Output. */
int error; /* 0 or positive errno value. */
}; };
union dpif_op { struct dpif_op {
enum dpif_op_type type; enum dpif_op_type type;
struct dpif_flow_put flow_put; int error;
struct dpif_execute execute; union {
struct dpif_flow_put flow_put;
struct dpif_execute execute;
} u;
}; };
void dpif_operate(struct dpif *, union dpif_op **ops, size_t n_ops); void dpif_operate(struct dpif *, struct dpif_op **ops, size_t n_ops);
/* Upcalls. */ /* Upcalls. */

18
ofproto/ofproto-dpif.c
View File

@@ -2413,7 +2413,7 @@ struct flow_miss {
}; };
struct flow_miss_op { struct flow_miss_op {
union dpif_op dpif_op; struct dpif_op dpif_op;
struct subfacet *subfacet; struct subfacet *subfacet;
}; };
@@ -2587,9 +2587,9 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
} }
op = &ops[(*n_ops)++]; op = &ops[(*n_ops)++];
execute = &op->dpif_op.execute; execute = &op->dpif_op.u.execute;
op->subfacet = subfacet; op->subfacet = subfacet;
execute->type = DPIF_OP_EXECUTE; op->dpif_op.type = DPIF_OP_EXECUTE;
execute->key = miss->key; execute->key = miss->key;
execute->key_len = miss->key_len; execute->key_len = miss->key_len;
execute->actions = (facet->may_install execute->actions = (facet->may_install
@@ -2602,10 +2602,10 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
if (facet->may_install && subfacet->key_fitness != ODP_FIT_TOO_LITTLE) { if (facet->may_install && subfacet->key_fitness != ODP_FIT_TOO_LITTLE) {
struct flow_miss_op *op = &ops[(*n_ops)++]; struct flow_miss_op *op = &ops[(*n_ops)++];
struct dpif_flow_put *put = &op->dpif_op.flow_put; struct dpif_flow_put *put = &op->dpif_op.u.flow_put;
op->subfacet = subfacet; op->subfacet = subfacet;
put->type = DPIF_OP_FLOW_PUT; op->dpif_op.type = DPIF_OP_FLOW_PUT;
put->flags = DPIF_FP_CREATE | DPIF_FP_MODIFY; put->flags = DPIF_FP_CREATE | DPIF_FP_MODIFY;
put->key = miss->key; put->key = miss->key;
put->key_len = miss->key_len; put->key_len = miss->key_len;
@@ -2687,7 +2687,7 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall *upcalls,
struct dpif_upcall *upcall; struct dpif_upcall *upcall;
struct flow_miss *miss, *next_miss; struct flow_miss *miss, *next_miss;
struct flow_miss_op flow_miss_ops[FLOW_MISS_MAX_BATCH * 2]; struct flow_miss_op flow_miss_ops[FLOW_MISS_MAX_BATCH * 2];
union dpif_op *dpif_ops[FLOW_MISS_MAX_BATCH * 2]; struct dpif_op *dpif_ops[FLOW_MISS_MAX_BATCH * 2];
struct hmap todo; struct hmap todo;
size_t n_ops; size_t n_ops;
size_t i; size_t i;
@@ -2758,11 +2758,10 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall *upcalls,
for (i = 0; i < n_ops; i++) { for (i = 0; i < n_ops; i++) {
struct flow_miss_op *op = &flow_miss_ops[i]; struct flow_miss_op *op = &flow_miss_ops[i];
struct dpif_execute *execute; struct dpif_execute *execute;
struct dpif_flow_put *put;
switch (op->dpif_op.type) { switch (op->dpif_op.type) {
case DPIF_OP_EXECUTE: case DPIF_OP_EXECUTE:
execute = &op->dpif_op.execute; execute = &op->dpif_op.u.execute;
if (op->subfacet->actions != execute->actions) { if (op->subfacet->actions != execute->actions) {
free((struct nlattr *) execute->actions); free((struct nlattr *) execute->actions);
} }
@@ -2770,8 +2769,7 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall *upcalls,
break; break;
case DPIF_OP_FLOW_PUT: case DPIF_OP_FLOW_PUT:
put = &op->dpif_op.flow_put; if (!op->dpif_op.error) {
if (!put->error) {
op->subfacet->installed = true; op->subfacet->installed = true;
} }
break; break;