On Tue, Jun 07, 2011 at 09:25:49AM -0700, Ben Pfaff wrote:
> On Mon, Jun 06, 2011 at 06:20:02PM -0700, Jean Tourrilhes wrote:
> > Just for curiosity, I've implemented the patch along the line
> > of Ben suggestions, see below. I'm wondering if the compiler is smart
> > enough to pack bools, if not, I may use bitfields...
>
> Adding a "bool" there should not increase the size of the structure
> since there is already some padding.
>
> Bit-fields usually make code slower since the compiler isn't very good
> at optimizing access to them.
>
> Your patch looks good to me. Ethan, would you mind taking a look too,
> since you've been thinking about this? If it looks good to you then
> I'll commit it to master.
Second version with the bitfield, just because ;-)
Regards,
Jean
diff -u -p -r -x '*~' -x '*.cmd' -x '*.order' -x '*.mod' openvswitch-1.1.0-jean3/ofproto/ofproto.c openvswitch-1.1.0-jean4/ofproto/ofproto.c
--- openvswitch-1.1.0-jean3/ofproto/ofproto.c 2011-06-06 17:10:06.000000000 -0700
+++ openvswitch-1.1.0-jean4/ofproto/ofproto.c 2011-06-07 11:23:17.000000000 -0700
@@ -151,6 +151,7 @@ struct action_xlate_ctx {
tag_type tags; /* Tags associated with OFPP_NORMAL actions. */
bool may_set_up_flow; /* True ordinarily; false if the actions must
* be reassessed for every packet. */
+ bool has_resubmit; /* Rule contains resubmit action. */
uint16_t nf_output_iface; /* Output interface index for NetFlow. */
/* xlate_actions() initializes and uses these members, but the client has no
@@ -193,13 +194,14 @@ struct rule {
struct cls_rule cr; /* In owning ofproto's classifier. */
long ms_idle_timeout; /* In milliseconds from time of last use. */
long ms_hard_timeout; /* In milliseconds from time of creation. */
- bool send_flow_removed; /* Send a flow removed message? */
+ int send_flow_removed:1, /* Send a flow removed message? */
+ has_resubmit:1; /* Rule contains resubmit action. */
int n_actions; /* Number of elements in actions[]. */
union ofp_action *actions; /* OpenFlow actions. */
struct list facets; /* List of "struct facet"s. */
uint32_t limiter_id; /* Rate Limiter instance. */
- int has_limiter_id:1; /* Capacity bitfield */
+ int has_limiter_id:1; /* Capacity bitfield */
};
static struct rule *rule_from_cls_rule(const struct cls_rule *);
@@ -1702,7 +1704,7 @@ static void
facet_make_actions(struct ofproto *p, struct facet *facet,
const struct ofpbuf *packet)
{
- const struct rule *rule = facet->rule;
+ struct rule *rule = facet->rule;
struct ofpbuf *odp_actions;
struct action_xlate_ctx ctx;
@@ -1711,6 +1713,7 @@ facet_make_actions(struct ofproto *p, st
facet->tags = ctx.tags;
facet->may_install = ctx.may_set_up_flow;
facet->nf_flow.output_iface = ctx.nf_output_iface;
+ rule->has_resubmit = ctx.has_resubmit;
if (facet->actions_len != odp_actions->size
|| memcmp(facet->actions, odp_actions->data, odp_actions->size)) {
@@ -1953,6 +1956,7 @@ facet_revalidate(struct ofproto *ofproto
facet->tags = ctx.tags;
facet->nf_flow.output_iface = ctx.nf_output_iface;
facet->may_install = ctx.may_set_up_flow;
+ new_rule->has_resubmit = ctx.has_resubmit;
if (actions_changed) {
free(facet->actions);
facet->actions_len = odp_actions->size;
@@ -2355,6 +2359,7 @@ xlate_nicira_action(struct action_xlate_
case NXAST_RESUBMIT:
nar = (const struct nx_action_resubmit *) nah;
xlate_table_action(ctx, ofp_port_to_odp_port(ntohs(nar->in_port)));
+ ctx->has_resubmit = true;
break;
case NXAST_SET_TUNNEL:
@@ -2535,6 +2540,7 @@ action_xlate_ctx_init(struct action_xlat
ctx->packet = packet;
ctx->resubmit_hook = NULL;
ctx->check_special = true;
+ ctx->has_resubmit = false;
}
static void
@@ -3372,6 +3378,10 @@ flow_push_stats(struct ofproto *ofproto,
{
struct ofproto_push push;
+ if (!rule->has_resubmit) {
+ return;
+ }
+
push.packets = packets;
push.bytes = bytes;
push.used = used;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
