On Wed, Jan 15, 2014 at 04:13:21PM +0900, Simon Horman wrote:
> This is an proposed enhancement to
> "Implement OpenFlow support for MPLS, for up to 3 labels."
>
> It is in preparation for not adding flows to the datapath
> which have more MPLS LSEs than the datapath supports.
>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
Thanks a lot!
I think that every iteration of the array probed the same depth, so I
folded this in:
- flow_set_mpls_bos(&flow, ARRAY_SIZE(flow.mpls_lse) - 1, 1);
+ flow_set_mpls_bos(&flow, i, 1);
I decided that probing from 0 up, instead of from the max down, was a
slight improvement, because if you run into a funny error along the
way you can always return the best you've seen so far instead of
failing all the way back to 0.
I decided that DPIF_FP_CREATE | DPIF_FP_MODIFY made more sense than
just DPIF_FP_CREATE, because I wouldn't want some leftover flow from a
previous run to cause a problem.
I decided that EEXIST was actually a form of success: it means we hit
some overlapping flow, which in turn means that the flow we're trying
to add is OK.
I decided that 'n' was a better name for the variable than 'i' ;-)
Here's what I'm folding in:
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 1fe5957..b9811b5 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -981,47 +981,41 @@ check_variable_length_userdata(struct dpif_backer *backer)
static size_t
check_max_mpls_depth(struct dpif_backer *backer)
{
- size_t i;
struct flow flow;
- int error;
+ size_t n;
- /* If ARRAY_SIZE(flow.mpls_lse) becomes large then it might
- * make more sense to do a binary search here. */
- for (i = ARRAY_SIZE(flow.mpls_lse) - 1; i > 0; i--) {
+ for (n = 0; n < ARRAY_SIZE(flow.mpls_lse); n++) {
struct odputil_keybuf keybuf;
struct ofpbuf key;
+ int error;
memset(&flow, 0, sizeof flow);
flow.dl_type = htons(ETH_TYPE_MPLS);
- flow_set_mpls_bos(&flow, ARRAY_SIZE(flow.mpls_lse) - 1, 1);
+ flow_set_mpls_bos(&flow, n, 1);
ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
odp_flow_key_from_flow(&key, &flow, 0);
- error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE, key.data, key.size,
- NULL, 0, NULL, 0, NULL);
- if (!error) {
- error = dpif_flow_del(backer->dpif, key.data, key.size, NULL);
- if (error) {
- goto err;
+ error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
+ key.data, key.size, NULL, 0, NULL, 0, NULL);
+ if (error && error != EEXIST) {
+ if (error != EINVAL) {
+ VLOG_WARN("%s: MPLS stack length feature probe failed (%s)",
+ dpif_name(backer->dpif), ovs_strerror(error));
}
- i++;
break;
- } else if (error != EINVAL) {
- goto err;
+ }
+
+ error = dpif_flow_del(backer->dpif, key.data, key.size, NULL);
+ if (error) {
+ VLOG_WARN("%s: failed to delete MPLS feature probe flow",
+ dpif_name(backer->dpif));
}
}
VLOG_INFO("%s: MPLS label stack length probed as %"PRIuSIZE,
- dpif_name(backer->dpif), i);
- return i;
-
-err:
- /* Something odd happened. We're not sure how large an
- * MPLS label stack the datapath supports. Default to 0. */
- VLOG_WARN("%s: mpls stack length feature probe failed (%s)",
- dpif_name(backer->dpif), ovs_strerror(error));
- return 0;
+ dpif_name(backer->dpif), n);
+ return n;
}
static int
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
