* [PATCH/RFC 1/8] odp: select group action attributes
2014-09-18 1:52 [PATCH/RFC 0/8] Open vSwtich ODP Select Group Action Simon Horman
@ 2014-09-18 1:52 ` Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 2/8] netlink: Allow suppression of warnings for duplicate attributes Simon Horman
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-09-18 1:52 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
This is the core of a proposed ODP select group action.
It models OpenFlow select groups without any extensions.
Further work:
We believe there is scope to add OVS_SELECT_GROUP_ATTR_* attributes
to supply parameters to the selection method: for example which
selection algorithm to use. This relates to a proposed
Open Flow extension that we have made.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
datapath/linux/compat/include/linux/openvswitch.h | 31 +++++++++++++++++++++++
lib/dpif-netdev.c | 1 +
lib/dpif.c | 1 +
lib/odp-execute.c | 1 +
lib/odp-util.c | 2 ++
5 files changed, 36 insertions(+)
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index 6910dc4..91ff939 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -510,6 +510,35 @@ enum ovs_sample_attr {
#define OVS_SAMPLE_ATTR_MAX (__OVS_SAMPLE_ATTR_MAX - 1)
/**
+ * enum ovs_bucket_attr - Bucket for * %OVS_ACTION_ATTR_SELECT_GROUP action.
+ * @OVS_BUCKET_ATTR_WEIGHT. Relative weight of bucket.
+ * @OVS_BUCKET_ATTR_ACTIONS. Set of actions to execute.
+ */
+enum ovs_bucket_attr {
+ OVS_BUCKET_ATTR_UNSPEC,
+ OVS_BUCKET_ATTR_WEIGHT, /* u16. Relative weight of bucket. */
+ OVS_BUCKET_ATTR_ACTIONS, /* Nested OVS_BUCKET_ATTR_* attributes. */
+ __OVS_BUCKET_ATTR_MAX,
+};
+
+#define OVS_BUCKET_ATTR_MAX (__OVS_BUCKET_ATTR_MAX - 1)
+
+/**
+ * enum ovs_select_group_attr - Attributes for * %OVS_ACTION_ATTR_SELECT_GROUP action.
+ * @OVS_SELECT_GROUP_ATTR_BUCKET. A bucket whose actions will be executed
+ * if the bucket is selected. One ore more buckets may be present.
+ *
+ * Selects a bucket and executes its actions.
+ */
+enum ovs_select_group_attr {
+ OVS_SELECT_GROUP_ATTR_UNSPEC,
+ OVS_SELECT_GROUP_ATTR_BUCKET, /* Nested OVS_BUCKET_ATTR_* attributes. */
+ __OVS_SELECT_GROUP_ATTR_MAX,
+};
+
+#define OVS_SELECT_GROUP_ATTR_MAX (__OVS_SELECT_GROUP_ATTR_MAX - 1)
+
+/**
* enum ovs_userspace_attr - Attributes for %OVS_ACTION_ATTR_USERSPACE action.
* @OVS_USERSPACE_ATTR_PID: u32 Netlink PID to which the %OVS_PACKET_CMD_ACTION
* message should be sent. Required.
@@ -609,6 +638,7 @@ struct ovs_action_hash {
* indicate the new packet contents. This could potentially still be
* %ETH_P_MPLS if the resulting MPLS label stack is not empty. If there
* is no MPLS label stack, as determined by ethertype, no action is taken.
+ * @OVS_ACTION_ATTR_SELECT_GROUP: Select a bucket and execute its actions.
*
* Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
* fields within a header are modifiable, e.g. the IPv4 protocol and fragment
@@ -631,6 +661,7 @@ enum ovs_action_attr {
* data immediately followed by a mask.
* The data must be zero for the unmasked
* bits. */
+ OVS_ACTION_ATTR_SELECT_GROUP, /* Nested OVS_SELECT_GROUP_*. */
__OVS_ACTION_ATTR_MAX
};
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 409c9bf..bfcfd8c 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2560,6 +2560,7 @@ dp_execute_cb(void *aux_, struct dpif_packet **packets, int cnt,
case OVS_ACTION_ATTR_SET:
case OVS_ACTION_ATTR_SET_MASKED:
case OVS_ACTION_ATTR_SAMPLE:
+ case OVS_ACTION_ATTR_SELECT_GROUP:
case OVS_ACTION_ATTR_UNSPEC:
case __OVS_ACTION_ATTR_MAX:
OVS_NOT_REACHED();
diff --git a/lib/dpif.c b/lib/dpif.c
index bf2c5f9..90b561f 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1043,6 +1043,7 @@ dpif_execute_helper_cb(void *aux_, struct dpif_packet **packets, int cnt,
case OVS_ACTION_ATTR_SET:
case OVS_ACTION_ATTR_SET_MASKED:
case OVS_ACTION_ATTR_SAMPLE:
+ case OVS_ACTION_ATTR_SELECT_GROUP:
case OVS_ACTION_ATTR_UNSPEC:
case __OVS_ACTION_ATTR_MAX:
OVS_NOT_REACHED();
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index e4bee18..c0ba868 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -518,6 +518,7 @@ odp_execute_actions__(void *dp, struct dpif_packet **packets, int cnt,
}
break;
+ case OVS_ACTION_ATTR_SELECT_GROUP:
case OVS_ACTION_ATTR_UNSPEC:
case __OVS_ACTION_ATTR_MAX:
OVS_NOT_REACHED();
diff --git a/lib/odp-util.c b/lib/odp-util.c
index d205473..77b456f 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -83,6 +83,7 @@ odp_action_len(uint16_t type)
case OVS_ACTION_ATTR_HASH: return sizeof(struct ovs_action_hash);
case OVS_ACTION_ATTR_SET: return -2;
case OVS_ACTION_ATTR_SET_MASKED: return -2;
+ case OVS_ACTION_ATTR_SELECT_GROUP: return -2;
case OVS_ACTION_ATTR_SAMPLE: return -2;
case OVS_ACTION_ATTR_UNSPEC:
@@ -581,6 +582,7 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
case OVS_ACTION_ATTR_SAMPLE:
format_odp_sample_action(ds, a);
break;
+ case OVS_ACTION_ATTR_SELECT_GROUP:
case OVS_ACTION_ATTR_UNSPEC:
case __OVS_ACTION_ATTR_MAX:
default:
--
2.0.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH/RFC 2/8] netlink: Allow suppression of warnings for duplicate attributes
2014-09-18 1:52 [PATCH/RFC 0/8] Open vSwtich ODP Select Group Action Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 1/8] odp: select group action attributes Simon Horman
@ 2014-09-18 1:52 ` Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 3/8] odp-util: formatting of datapath select group action Simon Horman
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-09-18 1:52 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
Add a multiple field to struct nl_policy which if set suppresses
warning of duplicate attributes in nl_parse_nested().
As is the case without this patch only the last occurrence of an
attribute is stored in attrs by nl_parse_nested(). As such
if the multiple field of struct nl_policy is set then it
is up to the caller to parse the message to extract all the attributes.
This is in preparation for allowing multiple OVS_SELECT_GROUP_ATTR_BUCKET
attributes in a nested OVS_ACTION_ATTR_SELECT_GROUP attribute.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
lib/netlink.c | 2 +-
lib/netlink.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/netlink.c b/lib/netlink.c
index 24b2168..bc30248 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -743,7 +743,7 @@ nl_policy_parse(const struct ofpbuf *msg, size_t nla_offset,
if (!nl_attr_validate(nla, e)) {
return false;
}
- if (attrs[type]) {
+ if (attrs[type] && !e->multiple) {
VLOG_DBG_RL(&rl, "duplicate attr %"PRIu16, type);
}
attrs[type] = nla;
diff --git a/lib/netlink.h b/lib/netlink.h
index f9234da..b0a72fd 100644
--- a/lib/netlink.h
+++ b/lib/netlink.h
@@ -195,6 +195,7 @@ struct nl_policy
enum nl_attr_type type;
size_t min_len, max_len;
bool optional;
+ bool multiple;
};
#define NL_POLICY_FOR(TYPE) \
--
2.0.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH/RFC 3/8] odp-util: formatting of datapath select group action
2014-09-18 1:52 [PATCH/RFC 0/8] Open vSwtich ODP Select Group Action Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 1/8] odp: select group action attributes Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 2/8] netlink: Allow suppression of warnings for duplicate attributes Simon Horman
@ 2014-09-18 1:52 ` Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 4/8] datapath: execution of " Simon Horman
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-09-18 1:52 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
Allow formatting of select group action. This is used
when pretty-printing datapath flows. Subsequent patches
will add support for the select group action to the datapath
and ovs-vswtichd.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
lib/odp-util.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 77b456f..4c8dd39 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -182,6 +182,71 @@ format_odp_sample_action(struct ds *ds, const struct nlattr *attr)
ds_put_format(ds, "))");
}
+static bool
+format_odp_bucket(struct ds *ds, const struct nlattr *attr)
+{
+ static const struct nl_policy ovs_sample_policy[] = {
+ [OVS_BUCKET_ATTR_WEIGHT] = { .type = NL_A_U16 },
+ [OVS_BUCKET_ATTR_ACTIONS] = { .type = NL_A_NESTED }
+ };
+ struct nlattr *a[ARRAY_SIZE(ovs_sample_policy)];
+ const struct nlattr *nla_acts;
+ int len;
+
+ ds_put_cstr(ds, "bucket");
+
+ if (!nl_parse_nested(attr, ovs_sample_policy, a, ARRAY_SIZE(a))) {
+ ds_put_cstr(ds, "(error)");
+ return false;
+ }
+
+ ds_put_format(ds, "(weight=%d,",
+ nl_attr_get_u16(a[OVS_BUCKET_ATTR_WEIGHT]));
+
+ ds_put_cstr(ds, "actions(");
+ nla_acts = nl_attr_get(a[OVS_SAMPLE_ATTR_ACTIONS]);
+ len = nl_attr_get_size(a[OVS_SAMPLE_ATTR_ACTIONS]);
+ format_odp_actions(ds, nla_acts, len);
+ ds_put_format(ds, "))");
+
+ return true;
+}
+
+static void
+format_odp_select_group_action(struct ds *ds, const struct nlattr *attr)
+{
+ static const struct nl_policy ovs_sample_policy[] = {
+ [OVS_SELECT_GROUP_ATTR_BUCKET] = { .type = NL_A_NESTED,
+ .multiple = true }
+ };
+ struct nlattr *a[ARRAY_SIZE(ovs_sample_policy)];
+ struct nlattr *nla;
+ struct ofpbuf buf;
+ size_t left;
+
+ ds_put_cstr(ds, "select_group");
+
+ if (!nl_parse_nested(attr, ovs_sample_policy, a, ARRAY_SIZE(a))) {
+ ds_put_cstr(ds, "(error)");
+ return;
+ }
+
+ ds_put_cstr(ds, "(actions(");
+
+ nl_attr_get_nested(attr, &buf);
+ NL_ATTR_FOR_EACH (nla, left, ofpbuf_data(&buf), ofpbuf_size(&buf))
+ {
+ uint16_t type = nl_attr_type(nla);
+
+ ovs_assert(type == OVS_SELECT_GROUP_ATTR_BUCKET);
+ if (!format_odp_bucket(ds, nla)) {
+ break;
+ }
+ }
+
+ ds_put_format(ds, "))");
+}
+
static const char *
slow_path_reason_to_string(uint32_t reason)
{
@@ -583,6 +648,8 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
format_odp_sample_action(ds, a);
break;
case OVS_ACTION_ATTR_SELECT_GROUP:
+ format_odp_select_group_action(ds, a);
+ break;
case OVS_ACTION_ATTR_UNSPEC:
case __OVS_ACTION_ATTR_MAX:
default:
--
2.0.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH/RFC 4/8] datapath: execution of select group action
2014-09-18 1:52 [PATCH/RFC 0/8] Open vSwtich ODP Select Group Action Simon Horman
` (2 preceding siblings ...)
2014-09-18 1:52 ` [PATCH/RFC 3/8] odp-util: formatting of datapath select group action Simon Horman
@ 2014-09-18 1:52 ` Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 5/8] datapath: Move last_action() helper to datapath.h Simon Horman
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-09-18 1:52 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
Allow execution of select group action in the datapath.
A subsequent patch will add validation and copying of
the select group action in the datapath.
The selection algorithm used is based on the RSS hash.
This was chosen because it resembles the algorithm currently
used by the implementation of select groups in ovs-vswitchd.
It may well be that in this case it is more efficient to handle
things in ovs-vswitchd, avoiding the cost of hashing on each packet.
Or that the hashing mechanism used can be optimised somehow. However,
we would like to avoid focusing on these questions of this particular
implementation.
The purpose of this patch is to form part of a prototype for a select
group action. The current Open Flow specification allows for the
implementation to select an algorithm. And we have made a separate
proposal to allow the selection algorithm to be configured via Open Flow.
Thus the algorithm and used and its implementation are not central
to the prototype.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
datapath/actions.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/datapath/actions.c b/datapath/actions.c
index 8d18848..51ca40b 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -809,6 +809,72 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
return 0;
}
+const struct nlattr *bucket_actions(const struct nlattr *attr)
+{
+ const struct nlattr *a;
+ int rem;
+
+ for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
+ a = nla_next(a, &rem)) {
+ if (nla_type(a) == OVS_BUCKET_ATTR_ACTIONS) {
+ return a;
+ }
+ }
+
+ return NULL;
+}
+
+static u16 bucket_weight(const struct nlattr *attr)
+{
+ const struct nlattr *weight;
+
+ /* validate_and_copy_bucket() ensures that the first
+ * attribute is OVS_BUCKET_ATTR_WEIGHT */
+ weight = nla_data(attr);
+ BUG_ON(nla_type(weight) != OVS_BUCKET_ATTR_WEIGHT);
+ return nla_get_u16(weight);
+}
+
+static int select_group(struct datapath *dp, struct sk_buff *skb,
+ const struct nlattr *attr)
+{
+ const struct nlattr *best_bucket = NULL;
+ const struct nlattr *acts_list;
+ const struct nlattr *bucket;
+ struct sk_buff *sample_skb;
+ u32 best_score = 0;
+ u32 basis;
+ u32 i = 0;
+ int rem;
+
+ basis = skb_get_hash(skb);
+
+ /* Only possible type of attributes is OVS_SELECT_GROUP_ATTR_BUCKET */
+ for (bucket = nla_data(attr), rem = nla_len(attr); rem > 0;
+ bucket = nla_next(bucket, &rem)) {
+ uint16_t weight = bucket_weight(bucket);
+ // XXX: This hashing seems expensive
+ u32 score = (jhash_1word(i, basis) & 0xffff) * weight;
+
+ if (score >= best_score) {
+ best_bucket = bucket;
+ best_score = score;
+ }
+ i++;
+ }
+
+ acts_list = bucket_actions(best_bucket);
+
+ /* A select group action is always the final action so
+ * there is no need to clone the skb in case of side effects.
+ * Instead just take a reference to it which will be released
+ * by do_execute_actions(). */
+ skb_get(skb);
+
+ return do_execute_actions(dp, skb, nla_data(acts_list),
+ nla_len(acts_list));
+}
+
static void execute_hash(struct sk_buff *skb, const struct nlattr *attr)
{
struct sw_flow_key *key = OVS_CB(skb)->pkt_key;
@@ -986,6 +1052,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
case OVS_ACTION_ATTR_SAMPLE:
err = sample(dp, skb, a);
break;
+
+ case OVS_ACTION_ATTR_SELECT_GROUP:
+ err = select_group(dp, skb, a);
+ break;
}
if (unlikely(err)) {
--
2.0.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH/RFC 5/8] datapath: Move last_action() helper to datapath.h
2014-09-18 1:52 [PATCH/RFC 0/8] Open vSwtich ODP Select Group Action Simon Horman
` (3 preceding siblings ...)
2014-09-18 1:52 ` [PATCH/RFC 4/8] datapath: execution of " Simon Horman
@ 2014-09-18 1:52 ` Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 6/8] datapath: validation of select group action Simon Horman
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-09-18 1:52 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
This is in preparation for using last_action() from
more than one C file as part of supporting an odp select group action.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
datapath/actions.c | 6 ------
datapath/datapath.h | 5 +++++
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index 51ca40b..9d27234 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -752,11 +752,6 @@ static int output_userspace(struct datapath *dp, struct sk_buff *skb,
return ovs_dp_upcall(dp, skb, &upcall);
}
-static bool last_action(const struct nlattr *a, int rem)
-{
- return a->nla_len == rem;
-}
-
static int sample(struct datapath *dp, struct sk_buff *skb,
const struct nlattr *attr)
{
@@ -841,7 +836,6 @@ static int select_group(struct datapath *dp, struct sk_buff *skb,
const struct nlattr *best_bucket = NULL;
const struct nlattr *acts_list;
const struct nlattr *bucket;
- struct sk_buff *sample_skb;
u32 best_score = 0;
u32 basis;
u32 i = 0;
diff --git a/datapath/datapath.h b/datapath/datapath.h
index c5d3c86..74a15e6 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -209,4 +209,9 @@ do { \
if (net_ratelimit()) \
pr_info("netlink: " fmt, ##__VA_ARGS__); \
} while (0)
+
+static inline bool last_action(const struct nlattr *a, int rem)
+{
+ return a->nla_len == rem;
+}
#endif /* datapath.h */
--
2.0.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH/RFC 6/8] datapath: validation of select group action
2014-09-18 1:52 [PATCH/RFC 0/8] Open vSwtich ODP Select Group Action Simon Horman
` (4 preceding siblings ...)
2014-09-18 1:52 ` [PATCH/RFC 5/8] datapath: Move last_action() helper to datapath.h Simon Horman
@ 2014-09-18 1:52 ` Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 7/8] ofproto: translate datapath " Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 8/8] hack: ofproto: enable odp select action Simon Horman
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-09-18 1:52 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
Allow validation and copying of select group actions.
This completes the prototype select group action implementation
in the datapath. Subsequent patches will add support to ovs-vswtichd.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
datapath/flow_netlink.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 6c74841..90eddba 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -1497,6 +1497,94 @@ static int validate_and_copy_sample(const struct nlattr *attr,
return 0;
}
+static int validate_and_copy_bucket(const struct nlattr *attr,
+ const struct sw_flow_key *key, int depth,
+ struct sw_flow_actions **sfa,
+ __be16 eth_type, __be16 vlan_tci)
+{
+ const struct nlattr *attrs[OVS_BUCKET_ATTR_MAX + 1];
+ const struct nlattr *weight, *actions;
+ const struct nlattr *a;
+ int rem, start, err, st_acts;
+
+ memset(attrs, 0, sizeof(attrs));
+ nla_for_each_nested(a, attr, rem) {
+ int type = nla_type(a);
+ if (!type || type > OVS_BUCKET_ATTR_MAX || attrs[type])
+ return -EINVAL;
+ attrs[type] = a;
+ }
+ if (rem)
+ return -EINVAL;
+
+ weight = attrs[OVS_BUCKET_ATTR_WEIGHT];
+ if (!weight || nla_len(weight) != sizeof(u16))
+ return -EINVAL;
+
+ actions = attrs[OVS_BUCKET_ATTR_ACTIONS];
+ if (!actions || (nla_len(actions) && nla_len(actions) < NLA_HDRLEN))
+ return -EINVAL;
+
+ /* validation done, copy sample action. */
+ start = add_nested_action_start(sfa, OVS_SELECT_GROUP_ATTR_BUCKET);
+ if (start < 0)
+ return start;
+ err = add_action(sfa, OVS_BUCKET_ATTR_WEIGHT,
+ nla_data(weight), sizeof(u16));
+ if (err)
+ return err;
+ st_acts = add_nested_action_start(sfa, OVS_SAMPLE_ATTR_ACTIONS);
+ if (st_acts < 0)
+ return st_acts;
+
+ err = __ovs_nla_copy_actions(actions, key, depth + 1, sfa,
+ eth_type, vlan_tci);
+ if (err)
+ return err;
+
+ add_nested_action_end(*sfa, st_acts);
+ add_nested_action_end(*sfa, start);
+
+ return 0;
+}
+
+static int validate_and_copy_select_group(const struct nlattr *attr,
+ const struct sw_flow_key *key,
+ int depth,
+ struct sw_flow_actions **sfa,
+ __be16 eth_type, __be16 vlan_tci)
+{
+ bool have_bucket = false;
+ const struct nlattr *a;
+ int rem, start, err;
+
+ start = add_nested_action_start(sfa, OVS_ACTION_ATTR_SELECT_GROUP);
+ if (start < 0)
+ return start;
+
+ nla_for_each_nested(a, attr, rem) {
+ int type = nla_type(a);
+
+ if (!type || type > OVS_SAMPLE_ATTR_MAX)
+ return -EINVAL;
+
+ /* Only possible type is OVS_SELECT_GROUP_ATTR_BUCKET */
+ if ((nla_len(a) && nla_len(a) < NLA_HDRLEN))
+ return -EINVAL;
+ err = validate_and_copy_bucket(a, key, depth, sfa,
+ eth_type, vlan_tci);
+ if (err < 0)
+ return err;
+ have_bucket = true;
+ }
+ if (rem || !have_bucket)
+ return -EINVAL;
+
+ add_nested_action_end(*sfa, start);
+
+ return 0;
+}
+
static int validate_tp_port(const struct sw_flow_key *flow_key,
__be16 eth_type)
{
@@ -1750,6 +1838,7 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
[OVS_ACTION_ATTR_POP_VLAN] = 0,
[OVS_ACTION_ATTR_SET] = (u32)-1,
[OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
+ [OVS_ACTION_ATTR_SELECT_GROUP] = (u32)-1,
[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
};
const struct ovs_action_push_vlan *vlan;
@@ -1856,6 +1945,19 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
skip_copy = true;
break;
+ case OVS_ACTION_ATTR_SELECT_GROUP:
+ /* Nothing may come after a select group */
+ if (!last_action(a, rem))
+ return -EINVAL;
+
+ err = validate_and_copy_select_group(a, key, depth,
+ sfa, eth_type,
+ vlan_tci);
+ if (err)
+ return err;
+ skip_copy = true;
+ break;
+
default:
return -EINVAL;
}
--
2.0.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH/RFC 7/8] ofproto: translate datapath select group action
2014-09-18 1:52 [PATCH/RFC 0/8] Open vSwtich ODP Select Group Action Simon Horman
` (5 preceding siblings ...)
2014-09-18 1:52 ` [PATCH/RFC 6/8] datapath: validation of select group action Simon Horman
@ 2014-09-18 1:52 ` Simon Horman
2014-09-18 1:52 ` [PATCH/RFC 8/8] hack: ofproto: enable odp select action Simon Horman
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-09-18 1:52 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
This add support for the select group action to ovs-vswtichd.
This new feature is currently disabled in xlate_select_group()
because ctx->xbridge->dp_select_group is always false.
This patch is a prototype and has several limitations:
* It assumes that no actions follow a select group action
because the resulting packet after a select group action may
differ depending on the bucket used. It may be possible
to address this problem using recirculation. Or to not use
the datapath select group in such situations. In any case
this patch does not solve this problem or even prevent it
from occurring.
* If recirculation occurs inside more than one bucket then
it seems that separate recirculation ids would be required
for each occurrence. This patch does not solve this problem or even
prevent it from occurring.
* No attempt is made to handle per-bucket statistics.
This would most likely require further datapath modifications.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
ofproto/ofproto-dpif-xlate.c | 108 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 107 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 6e33a27..b08a821 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -103,6 +103,9 @@ struct xbridge {
* False if the datapath supports only 8-byte (or shorter) userdata. */
bool variable_length_userdata;
+ /* True if datapath supports select group action */
+ bool dp_select_group;
+
/* Number of MPLS label stack entries that the datapath supports
* in matches. */
size_t max_mpls_depth;
@@ -329,6 +332,9 @@ static void xlate_report(struct xlate_ctx *, const char *);
static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
uint8_t table_id, bool may_packet_in,
bool honor_table_miss);
+static void xlate_group_stats(struct xlate_ctx *, struct group_dpif *,
+ struct ofputil_bucket *);
+static void xlate_group_bucket(struct xlate_ctx *, struct ofputil_bucket *);
static bool input_vid_is_valid(uint16_t vid, struct xbundle *, bool warn);
static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid);
static void output_normal(struct xlate_ctx *, const struct xbundle *,
@@ -2353,6 +2359,61 @@ fix_sflow_action(struct xlate_ctx *ctx)
ctx->sflow_odp_port, ctx->sflow_n_outputs, cookie);
}
+/* Compose bucket for SELECT_GROUP action. */
+static void
+compose_bucket_action(struct xlate_ctx *ctx, struct group_dpif *group,
+ struct ofputil_bucket *bucket)
+{
+ size_t bucket_offset, actions_offset;
+ struct ofpbuf *odp_actions = ctx->xout->odp_actions;
+
+ bucket_offset = nl_msg_start_nested(odp_actions,
+ OVS_SELECT_GROUP_ATTR_BUCKET);
+
+ nl_msg_put_u16(odp_actions, OVS_BUCKET_ATTR_WEIGHT, bucket->weight);
+
+ actions_offset = nl_msg_start_nested(odp_actions, OVS_BUCKET_ATTR_ACTIONS);
+
+ xlate_group_bucket(ctx, bucket);
+ xlate_group_stats(ctx, group, bucket);
+
+ nl_msg_end_nested(odp_actions, actions_offset);
+ nl_msg_end_nested(odp_actions, bucket_offset);
+
+ ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
+ odp_actions, &ctx->xout->wc);
+}
+
+/* Compose SELECT_GROUP action.
+ * May be called multiple times to add multiple buckets.
+ * The first call should pass SIZE_MAX as the value for offset_
+ * and subsequent calls should pass returned by the previous call.
+ * The last call should pass NULL as the bucket parameter
+ * and previous calls should pass a valid bucket to add to the
+ * select group. */
+static size_t
+compose_select_group_action(struct xlate_ctx *ctx, struct group_dpif *group,
+ struct ofputil_bucket *bucket,
+ const size_t offset_)
+{
+ size_t offset = offset_;
+
+ if (bucket != NULL) {
+ if (offset == SIZE_MAX) {
+ offset = nl_msg_start_nested(ctx->xout->odp_actions,
+ OVS_ACTION_ATTR_SELECT_GROUP);
+ }
+
+ compose_bucket_action(ctx, group, bucket);
+ } else {
+ if (offset != SIZE_MAX) {
+ nl_msg_end_nested(ctx->xout->odp_actions, offset);
+ }
+ }
+
+ return offset;
+}
+
static enum slow_path_reason
process_special(struct xlate_ctx *ctx, const struct flow *flow,
const struct xport *xport, const struct ofpbuf *packet)
@@ -2775,7 +2836,40 @@ xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif *group)
}
static void
-xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
+xlate_select_group_datapath(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+ struct flow_wildcards *wc = &ctx->xout->wc;
+ struct ofputil_bucket *bucket;
+ const struct list *buckets;
+ struct flow old_flow = ctx->xin->flow;
+ size_t offset = SIZE_MAX;
+
+ group_dpif_get_buckets(group, &buckets);
+ LIST_FOR_EACH (bucket, list_node, buckets) {
+ if (bucket_is_alive(ctx, bucket, 0)) {
+ offset = compose_select_group_action(ctx, group, bucket, offset);
+
+ /* Roll back flow to previous state.
+ *
+ * This allows composition of the actions for subsequent
+ * buckets in such a way that they apply to the state of the
+ * packet present before the select group action.
+ *
+ * XXX: Assumes no actions follow the select group action.
+ * Handle with recirculation?
+ */
+ ctx->xin->flow = old_flow;
+ }
+ }
+
+ if (offset != SIZE_MAX) {
+ memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
+ compose_select_group_action(ctx, group, NULL, offset);
+ }
+}
+
+static void
+xlate_select_group_userspace(struct xlate_ctx *ctx, struct group_dpif *group)
{
struct flow_wildcards *wc = &ctx->xout->wc;
struct ofputil_bucket *bucket;
@@ -2800,6 +2894,16 @@ xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
}
static void
+xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+ if (ctx->xbridge->dp_select_group) {
+ xlate_select_group_datapath(ctx, group);
+ } else {
+ xlate_select_group_userspace(ctx, group);
+ }
+}
+
+static void
xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
{
ctx->in_group = true;
@@ -2990,6 +3094,8 @@ compose_recirculate_action(struct xlate_ctx *ctx,
ofpacts_len = ofpacts_base_len -
((uint8_t *)ofpact_current - (uint8_t *)ofpacts_base);
+ /* XXX: If recirculation occurs inside buckets of a select group action
+ * then multiple recirculation ids may be required. */
if (ctx->rule) {
id = rule_dpif_get_recirc_id(ctx->rule);
} else {
--
2.0.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH/RFC 8/8] hack: ofproto: enable odp select action
2014-09-18 1:52 [PATCH/RFC 0/8] Open vSwtich ODP Select Group Action Simon Horman
` (6 preceding siblings ...)
2014-09-18 1:52 ` [PATCH/RFC 7/8] ofproto: translate datapath " Simon Horman
@ 2014-09-18 1:52 ` Simon Horman
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-09-18 1:52 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
This is a quick hack to enable the datapath group select action.
It is in lieu of some combination of:
* probing
* run-time configuration by the end-use.
* run-time heuristic to use the action as appropriate
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
ofproto/ofproto-dpif-xlate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b08a821..eca617d 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2896,7 +2896,7 @@ xlate_select_group_userspace(struct xlate_ctx *ctx, struct group_dpif *group)
static void
xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
{
- if (ctx->xbridge->dp_select_group) {
+ if (true /*ctx->xbridge->dp_select_group*/ ) {
xlate_select_group_datapath(ctx, group);
} else {
xlate_select_group_userspace(ctx, group);
--
2.0.1
^ permalink raw reply related [flat|nested] 9+ messages in thread