diff mbox

[ovs-dev,2/3] ofproto: Probe for sample nesting level.

Message ID 1489099810-68023-2-git-send-email-azhou@ovn.org
State Accepted
Headers show

Commit Message

Andy Zhou March 9, 2017, 10:50 p.m. UTC
Add logics to detect the max level of nesting allowed by the
sample action implemented in the datapath.

Future patch allows xlate code to generate different odp actions
based on this information.

Signed-off-by: Andy Zhou <azhou@ovn.org>
---
 include/openvswitch/flow.h |  3 +++
 lib/dpif.c                 |  2 +-
 ofproto/ofproto-dpif.c     | 60 ++++++++++++++++++++++++++++++++++++++++++++++
 ofproto/ofproto-dpif.h     |  3 +++
 4 files changed, 67 insertions(+), 1 deletion(-)

Comments

Joe Stringer March 11, 2017, 12:48 a.m. UTC | #1
On 9 March 2017 at 14:50, Andy Zhou <azhou@ovn.org> wrote:
> Add logics to detect the max level of nesting allowed by the
> sample action implemented in the datapath.
>
> Future patch allows xlate code to generate different odp actions
> based on this information.
>
> Signed-off-by: Andy Zhou <azhou@ovn.org>

Acked-by: Joe Stringer <joe@ovn.org>

Couple of minor comments below..

<snip>

> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 5289693..4b88d4b 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -983,6 +983,65 @@ check_max_mpls_depth(struct dpif_backer *backer)
>      return n;
>  }
>
> +static void
> +add_sample_actions(struct ofpbuf *actions, int nesting)
> +{
> +    if (nesting == 0) {
> +        nl_msg_put_odp_port(actions, OVS_ACTION_ATTR_OUTPUT, u32_to_odp(1));
> +        return;
> +    }
> +
> +    size_t start, actions_start;
> +
> +    start = nl_msg_start_nested(actions, OVS_ACTION_ATTR_SAMPLE);
> +    actions_start = nl_msg_start_nested(actions, OVS_SAMPLE_ATTR_ACTIONS);
> +    add_sample_actions(actions, nesting - 1);
> +    nl_msg_end_nested(actions, actions_start);
> +    nl_msg_put_u32(actions, OVS_SAMPLE_ATTR_PROBABILITY, UINT32_MAX);
> +    nl_msg_end_nested(actions, start);
> +}
> +
> +/* Tests the nested sample actions levels supported by 'backer''s datapath.
> + *
> + * Returns the number of nested sample actions accepted by the datapath
> + * Otherwise returns the number of MPLS push actions supported by
> + * the datapath. */

Huh? What does MPLS have to do with sampling?

> +static size_t
> +check_max_sample_nesting(struct dpif_backer *backer)
> +{
> +    struct odputil_keybuf keybuf;
> +    struct ofpbuf key;
> +    struct flow flow;
> +    int n;
> +
> +    struct odp_flow_key_parms odp_parms = {
> +        .flow = &flow,
> +    };
> +
> +    memset(&flow, 0, sizeof flow);
> +    ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> +    odp_flow_key_from_flow(&odp_parms, &key);
> +
> +    /* OVS datapath has always supported at least 3 nested leves.  */

*levels
Andy Zhou March 11, 2017, 1:19 a.m. UTC | #2
On Fri, Mar 10, 2017 at 4:48 PM, Joe Stringer <joe@ovn.org> wrote:
> On 9 March 2017 at 14:50, Andy Zhou <azhou@ovn.org> wrote:
>> Add logics to detect the max level of nesting allowed by the
>> sample action implemented in the datapath.
>>
>> Future patch allows xlate code to generate different odp actions
>> based on this information.
>>
>> Signed-off-by: Andy Zhou <azhou@ovn.org>
>
> Acked-by: Joe Stringer <joe@ovn.org>
>
> Couple of minor comments below..
>
> <snip>
>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 5289693..4b88d4b 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -983,6 +983,65 @@ check_max_mpls_depth(struct dpif_backer *backer)
>>      return n;
>>  }
>>
>> +static void
>> +add_sample_actions(struct ofpbuf *actions, int nesting)
>> +{
>> +    if (nesting == 0) {
>> +        nl_msg_put_odp_port(actions, OVS_ACTION_ATTR_OUTPUT, u32_to_odp(1));
>> +        return;
>> +    }
>> +
>> +    size_t start, actions_start;
>> +
>> +    start = nl_msg_start_nested(actions, OVS_ACTION_ATTR_SAMPLE);
>> +    actions_start = nl_msg_start_nested(actions, OVS_SAMPLE_ATTR_ACTIONS);
>> +    add_sample_actions(actions, nesting - 1);
>> +    nl_msg_end_nested(actions, actions_start);
>> +    nl_msg_put_u32(actions, OVS_SAMPLE_ATTR_PROBABILITY, UINT32_MAX);
>> +    nl_msg_end_nested(actions, start);
>> +}
>> +
>> +/* Tests the nested sample actions levels supported by 'backer''s datapath.
>> + *
>> + * Returns the number of nested sample actions accepted by the datapath
>> + * Otherwise returns the number of MPLS push actions supported by
>> + * the datapath. */
>
> Huh? What does MPLS have to do with sampling?
Cut-and-paste error.  Will update the comment.
>
>> +static size_t
>> +check_max_sample_nesting(struct dpif_backer *backer)
>> +{
>> +    struct odputil_keybuf keybuf;
>> +    struct ofpbuf key;
>> +    struct flow flow;
>> +    int n;
>> +
>> +    struct odp_flow_key_parms odp_parms = {
>> +        .flow = &flow,
>> +    };
>> +
>> +    memset(&flow, 0, sizeof flow);
>> +    ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
>> +    odp_flow_key_from_flow(&odp_parms, &key);
>> +
>> +    /* OVS datapath has always supported at least 3 nested leves.  */
>
> *levels
Will fix. Thanks.
diff mbox

Patch

diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index 5cd78e4..6606d3c 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -61,6 +61,9 @@  const char *flow_tun_flag_to_string(uint32_t flags);
 /* Maximum number of supported MPLS labels. */
 #define FLOW_MAX_MPLS_LABELS 3
 
+/* Maximum number of supported SAMPLE action nesting. */
+#define FLOW_MAX_SAMPLE_NESTING 10
+
 /*
  * A flow in the network.
  *
diff --git a/lib/dpif.c b/lib/dpif.c
index dad0fcd..1760de8 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -916,7 +916,7 @@  dpif_probe_feature(struct dpif *dpif, const char *name,
                           nl_actions, nl_actions_size,
                           ufid, NON_PMD_CORE_ID, NULL);
     if (error) {
-        if (error != EINVAL) {
+        if (error != EINVAL && error != EOVERFLOW) {
             VLOG_WARN("%s: %s flow probe failed (%s)",
                       dpif_name(dpif), name, ovs_strerror(error));
         }
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5289693..4b88d4b 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -983,6 +983,65 @@  check_max_mpls_depth(struct dpif_backer *backer)
     return n;
 }
 
+static void
+add_sample_actions(struct ofpbuf *actions, int nesting)
+{
+    if (nesting == 0) {
+        nl_msg_put_odp_port(actions, OVS_ACTION_ATTR_OUTPUT, u32_to_odp(1));
+        return;
+    }
+
+    size_t start, actions_start;
+
+    start = nl_msg_start_nested(actions, OVS_ACTION_ATTR_SAMPLE);
+    actions_start = nl_msg_start_nested(actions, OVS_SAMPLE_ATTR_ACTIONS);
+    add_sample_actions(actions, nesting - 1);
+    nl_msg_end_nested(actions, actions_start);
+    nl_msg_put_u32(actions, OVS_SAMPLE_ATTR_PROBABILITY, UINT32_MAX);
+    nl_msg_end_nested(actions, start);
+}
+
+/* Tests the nested sample actions levels supported by 'backer''s datapath.
+ *
+ * Returns the number of nested sample actions accepted by the datapath
+ * Otherwise returns the number of MPLS push actions supported by
+ * the datapath. */
+static size_t
+check_max_sample_nesting(struct dpif_backer *backer)
+{
+    struct odputil_keybuf keybuf;
+    struct ofpbuf key;
+    struct flow flow;
+    int n;
+
+    struct odp_flow_key_parms odp_parms = {
+        .flow = &flow,
+    };
+
+    memset(&flow, 0, sizeof flow);
+    ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
+    odp_flow_key_from_flow(&odp_parms, &key);
+
+    /* OVS datapath has always supported at least 3 nested leves.  */
+    for (n = 3; n < FLOW_MAX_SAMPLE_NESTING; n++) {
+        struct ofpbuf actions;
+        bool ok;
+
+        ofpbuf_init(&actions, 300);
+        add_sample_actions(&actions, n);
+        ok = dpif_probe_feature(backer->dpif, "Sample action nesting", &key,
+                                &actions, NULL);
+        ofpbuf_uninit(&actions);
+        if (!ok) {
+            break;
+        }
+    }
+
+    VLOG_INFO("%s: Max sample nesting level probed as %d",
+              dpif_name(backer->dpif), n);
+    return n;
+}
+
 /* Tests whether 'backer''s datapath supports masked data in
  * OVS_ACTION_ATTR_SET actions.  We need to disable some features on older
  * datapaths that don't support this feature. */
@@ -1203,6 +1262,7 @@  check_support(struct dpif_backer *backer)
     backer->support.ufid = check_ufid(backer);
     backer->support.tnl_push_pop = dpif_supports_tnl_push_pop(backer->dpif);
     backer->support.clone = check_clone(backer);
+    backer->support.sample_nesting = check_max_sample_nesting(backer);
 
     /* Flow fields. */
     backer->support.odp.ct_state = check_ct_state(backer);
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 533fadf..cc514d2 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -168,6 +168,9 @@  struct dpif_backer_support {
 
     /* True if the datapath supports OVS_ACTION_ATTR_CLONE action. */
     bool clone;
+
+    /* Maximum level of nesting allowed by OVS_ACTION_ATTR_SAMPLE action.  */
+    size_t sample_nesting;
 };
 
 /* Reasons that we might need to revalidate every datapath flow, and