Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On Sun, Dec 8, 2019 at 3:08 PM Eli Britstein wrote:
>
>
> On 12/5/2019 6:46 PM, Sriharsha Basavapatna wrote:
> > On Wed, Dec 4, 2019 at 8:55 PM Eli Britstein wrote:
> >>
> >> On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
> >>> On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
> Signed-off-by: Eli Britstein
> Reviewed-by: Oz Shlomo
> ---
> NEWS | 2 +
> lib/netdev-offload-dpdk-flow.c | 87
> --
> 2 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 88b818948..ca9c2b230 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.12.0
> if supported by libbpf.
> * Add option to enable, disable and query TCP sequence checking
> in
> conntrack.
> + - DPDK:
> + * Add hardware offload support for output actions.
>
> v2.12.0 - 03 Sep 2019
> -
> diff --git a/lib/netdev-offload-dpdk-flow.c
> b/lib/netdev-offload-dpdk-flow.c
> index dbbc45e99..6e7efb315 100644
> --- a/lib/netdev-offload-dpdk-flow.c
> +++ b/lib/netdev-offload-dpdk-flow.c
> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
> rte_flow_action *actions)
> } else {
> ds_put_cstr(s, " RSS = null\n");
> }
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
> +const struct rte_flow_action_count *count = actions->conf;
> +
> +ds_put_cstr(s, "rte flow count action:\n");
> +if (count) {
> +ds_put_format(s,
> + " Count: shared=%d, id=%d\n",
> + count->shared, count->id);
> +} else {
> +ds_put_cstr(s, " Count = null\n");
> +}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
> +const struct rte_flow_action_port_id *port_id = actions->conf;
> +
> +ds_put_cstr(s, "rte flow port-id action:\n");
> +if (port_id) {
> +ds_put_format(s,
> + " Port-id: original=%d, id=%d\n",
> + port_id->original, port_id->id);
> +} else {
> +ds_put_cstr(s, " Port-id = null\n");
> +}
> } else {
> ds_put_format(s, "unknown rte flow action (%d)\n",
> actions->type);
> }
> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
> *patterns,
> return 0;
> }
>
> +static void
> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
> +{
> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
> +
> +count->shared = 0;
> +/* Each flow has a single count action, so no need of id */
> +count->id = 0;
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
> +}
> +
> +static void
> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
> +struct netdev *outdev)
> +{
> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
> +
> +port_id->id = netdev_dpdk_get_port_id(outdev);
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> +}
> +
> +static int
> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
> + const struct nlattr *nla,
> + struct offload_info *info)
> +{
> +struct netdev *outdev;
> +odp_port_t port;
> +
> +port = nl_attr_get_odp_port(nla);
> +outdev = netdev_ports_get(port, info->dpif_class);
> +if (outdev == NULL) {
> +VLOG_DBG_RL(_rl,
> +"Cannot find netdev for odp port %d", port);
> +return -1;
> +}
> +if (!netdev_dpdk_flow_api_supported(outdev)) {
> +VLOG_DBG_RL(_rl,
> +"Output to %s cannot be offloaded",
> +netdev_get_name(outdev));
> +return -1;
> +}
> +
> +netdev_dpdk_flow_add_count_action(actions);
> +netdev_dpdk_flow_add_port_id_action(actions, outdev);
> +netdev_close(outdev);
> +
> +return 0;
> +}
> +
> int
> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
> struct nlattr *nl_actions,
> size_t nl_actions_len,
> - struct offload_info *info OVS_UNUSED)
> + struct
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 08.12.2019 10:38, Eli Britstein wrote:
>
> On 12/5/2019 6:46 PM, Sriharsha Basavapatna wrote:
>> On Wed, Dec 4, 2019 at 8:55 PM Eli Britstein wrote:
>>>
>>> On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
> Signed-off-by: Eli Britstein
> Reviewed-by: Oz Shlomo
> ---
>NEWS | 2 +
>lib/netdev-offload-dpdk-flow.c | 87
> --
>2 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 88b818948..ca9c2b230 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.12.0
> if supported by libbpf.
> * Add option to enable, disable and query TCP sequence checking in
> conntrack.
> + - DPDK:
> + * Add hardware offload support for output actions.
>
>v2.12.0 - 03 Sep 2019
>-
> diff --git a/lib/netdev-offload-dpdk-flow.c
> b/lib/netdev-offload-dpdk-flow.c
> index dbbc45e99..6e7efb315 100644
> --- a/lib/netdev-offload-dpdk-flow.c
> +++ b/lib/netdev-offload-dpdk-flow.c
> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
> rte_flow_action *actions)
>} else {
>ds_put_cstr(s, " RSS = null\n");
>}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
> +const struct rte_flow_action_count *count = actions->conf;
> +
> +ds_put_cstr(s, "rte flow count action:\n");
> +if (count) {
> +ds_put_format(s,
> + " Count: shared=%d, id=%d\n",
> + count->shared, count->id);
> +} else {
> +ds_put_cstr(s, " Count = null\n");
> +}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
> +const struct rte_flow_action_port_id *port_id = actions->conf;
> +
> +ds_put_cstr(s, "rte flow port-id action:\n");
> +if (port_id) {
> +ds_put_format(s,
> + " Port-id: original=%d, id=%d\n",
> + port_id->original, port_id->id);
> +} else {
> +ds_put_cstr(s, " Port-id = null\n");
> +}
>} else {
>ds_put_format(s, "unknown rte flow action (%d)\n",
> actions->type);
>}
> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
> *patterns,
>return 0;
>}
>
> +static void
> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
> +{
> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
> +
> +count->shared = 0;
> +/* Each flow has a single count action, so no need of id */
> +count->id = 0;
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
> +}
> +
> +static void
> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
> +struct netdev *outdev)
> +{
> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
> +
> +port_id->id = netdev_dpdk_get_port_id(outdev);
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> +}
> +
> +static int
> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
> + const struct nlattr *nla,
> + struct offload_info *info)
> +{
> +struct netdev *outdev;
> +odp_port_t port;
> +
> +port = nl_attr_get_odp_port(nla);
> +outdev = netdev_ports_get(port, info->dpif_class);
> +if (outdev == NULL) {
> +VLOG_DBG_RL(_rl,
> +"Cannot find netdev for odp port %d", port);
> +return -1;
> +}
> +if (!netdev_dpdk_flow_api_supported(outdev)) {
> +VLOG_DBG_RL(_rl,
> +"Output to %s cannot be offloaded",
> +netdev_get_name(outdev));
> +return -1;
> +}
> +
> +netdev_dpdk_flow_add_count_action(actions);
> +netdev_dpdk_flow_add_port_id_action(actions, outdev);
> +netdev_close(outdev);
> +
> +return 0;
> +}
> +
>int
>netdev_dpdk_flow_actions_add(struct flow_actions *actions,
> struct nlattr *nl_actions,
> size_t nl_actions_len,
> - struct offload_info *info OVS_UNUSED)
> + struct offload_info *info)
>{
>struct nlattr *nla;
>size_t left;
>
>NL_ATTR_FOR_EACH_UNSAFE (nla,
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 12/5/2019 6:46 PM, Sriharsha Basavapatna wrote:
> On Wed, Dec 4, 2019 at 8:55 PM Eli Britstein wrote:
>>
>> On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
>>> On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
NEWS | 2 +
lib/netdev-offload-dpdk-flow.c | 87
--
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/NEWS b/NEWS
index 88b818948..ca9c2b230 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,8 @@ Post-v2.12.0
if supported by libbpf.
* Add option to enable, disable and query TCP sequence checking in
conntrack.
+ - DPDK:
+ * Add hardware offload support for output actions.
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-offload-dpdk-flow.c
b/lib/netdev-offload-dpdk-flow.c
index dbbc45e99..6e7efb315 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_cstr(s, " RSS = null\n");
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
+const struct rte_flow_action_count *count = actions->conf;
+
+ds_put_cstr(s, "rte flow count action:\n");
+if (count) {
+ds_put_format(s,
+ " Count: shared=%d, id=%d\n",
+ count->shared, count->id);
+} else {
+ds_put_cstr(s, " Count = null\n");
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
+const struct rte_flow_action_port_id *port_id = actions->conf;
+
+ds_put_cstr(s, "rte flow port-id action:\n");
+if (port_id) {
+ds_put_format(s,
+ " Port-id: original=%d, id=%d\n",
+ port_id->original, port_id->id);
+} else {
+ds_put_cstr(s, " Port-id = null\n");
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n",
actions->type);
}
@@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
*patterns,
return 0;
}
+static void
+netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
+{
+struct rte_flow_action_count *count = xzalloc(sizeof *count);
+
+count->shared = 0;
+/* Each flow has a single count action, so no need of id */
+count->id = 0;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
+}
+
+static void
+netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
+struct netdev *outdev)
+{
+struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
+
+port_id->id = netdev_dpdk_get_port_id(outdev);
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+}
+
+static int
+netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
+ const struct nlattr *nla,
+ struct offload_info *info)
+{
+struct netdev *outdev;
+odp_port_t port;
+
+port = nl_attr_get_odp_port(nla);
+outdev = netdev_ports_get(port, info->dpif_class);
+if (outdev == NULL) {
+VLOG_DBG_RL(_rl,
+"Cannot find netdev for odp port %d", port);
+return -1;
+}
+if (!netdev_dpdk_flow_api_supported(outdev)) {
+VLOG_DBG_RL(_rl,
+"Output to %s cannot be offloaded",
+netdev_get_name(outdev));
+return -1;
+}
+
+netdev_dpdk_flow_add_count_action(actions);
+netdev_dpdk_flow_add_port_id_action(actions, outdev);
+netdev_close(outdev);
+
+return 0;
+}
+
int
netdev_dpdk_flow_actions_add(struct flow_actions *actions,
struct nlattr *nl_actions,
size_t nl_actions_len,
- struct offload_info *info OVS_UNUSED)
+ struct offload_info *info)
{
struct nlattr *nla;
size_t left;
NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
-VLOG_DBG_RL(_rl,
-"Unsupported action type %d", nl_attr_type(nla));
-return -1;
+
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On Wed, Dec 4, 2019 at 8:55 PM Eli Britstein wrote:
>
>
> On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
> > On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
> >> Signed-off-by: Eli Britstein
> >> Reviewed-by: Oz Shlomo
> >> ---
> >> NEWS | 2 +
> >> lib/netdev-offload-dpdk-flow.c | 87
> >> --
> >> 2 files changed, 85 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/NEWS b/NEWS
> >> index 88b818948..ca9c2b230 100644
> >> --- a/NEWS
> >> +++ b/NEWS
> >> @@ -10,6 +10,8 @@ Post-v2.12.0
> >> if supported by libbpf.
> >>* Add option to enable, disable and query TCP sequence checking in
> >> conntrack.
> >> + - DPDK:
> >> + * Add hardware offload support for output actions.
> >>
> >> v2.12.0 - 03 Sep 2019
> >> -
> >> diff --git a/lib/netdev-offload-dpdk-flow.c
> >> b/lib/netdev-offload-dpdk-flow.c
> >> index dbbc45e99..6e7efb315 100644
> >> --- a/lib/netdev-offload-dpdk-flow.c
> >> +++ b/lib/netdev-offload-dpdk-flow.c
> >> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
> >> rte_flow_action *actions)
> >> } else {
> >> ds_put_cstr(s, " RSS = null\n");
> >> }
> >> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
> >> +const struct rte_flow_action_count *count = actions->conf;
> >> +
> >> +ds_put_cstr(s, "rte flow count action:\n");
> >> +if (count) {
> >> +ds_put_format(s,
> >> + " Count: shared=%d, id=%d\n",
> >> + count->shared, count->id);
> >> +} else {
> >> +ds_put_cstr(s, " Count = null\n");
> >> +}
> >> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
> >> +const struct rte_flow_action_port_id *port_id = actions->conf;
> >> +
> >> +ds_put_cstr(s, "rte flow port-id action:\n");
> >> +if (port_id) {
> >> +ds_put_format(s,
> >> + " Port-id: original=%d, id=%d\n",
> >> + port_id->original, port_id->id);
> >> +} else {
> >> +ds_put_cstr(s, " Port-id = null\n");
> >> +}
> >> } else {
> >> ds_put_format(s, "unknown rte flow action (%d)\n",
> >> actions->type);
> >> }
> >> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
> >> *patterns,
> >> return 0;
> >> }
> >>
> >> +static void
> >> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
> >> +{
> >> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
> >> +
> >> +count->shared = 0;
> >> +/* Each flow has a single count action, so no need of id */
> >> +count->id = 0;
> >> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
> >> +}
> >> +
> >> +static void
> >> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
> >> +struct netdev *outdev)
> >> +{
> >> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
> >> +
> >> +port_id->id = netdev_dpdk_get_port_id(outdev);
> >> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> >> +}
> >> +
> >> +static int
> >> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
> >> + const struct nlattr *nla,
> >> + struct offload_info *info)
> >> +{
> >> +struct netdev *outdev;
> >> +odp_port_t port;
> >> +
> >> +port = nl_attr_get_odp_port(nla);
> >> +outdev = netdev_ports_get(port, info->dpif_class);
> >> +if (outdev == NULL) {
> >> +VLOG_DBG_RL(_rl,
> >> +"Cannot find netdev for odp port %d", port);
> >> +return -1;
> >> +}
> >> +if (!netdev_dpdk_flow_api_supported(outdev)) {
> >> +VLOG_DBG_RL(_rl,
> >> +"Output to %s cannot be offloaded",
> >> +netdev_get_name(outdev));
> >> +return -1;
> >> +}
> >> +
> >> +netdev_dpdk_flow_add_count_action(actions);
> >> +netdev_dpdk_flow_add_port_id_action(actions, outdev);
> >> +netdev_close(outdev);
> >> +
> >> +return 0;
> >> +}
> >> +
> >> int
> >> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
> >>struct nlattr *nl_actions,
> >>size_t nl_actions_len,
> >> - struct offload_info *info OVS_UNUSED)
> >> + struct offload_info *info)
> >> {
> >> struct nlattr *nla;
> >> size_t left;
> >>
> >> NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
> >> -VLOG_DBG_RL(_rl,
> >> -"Unsupported action type %d", nl_attr_type(nla));
> >> -return -1;
> >> +if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT &&
> >> +
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 12/2/2019 4:25 PM, Ilya Maximets wrote: > BTW, function netdev_dpdk_flow_api_supported() is not intended to be called > in the offloading process. It's only for initialization phase. You can see > the "/* TODO: Check if we able to offload some minimal flow. */" in the code > and that might be destructive and unwanted for offloading process. I guess the thought you had in mind when writing it was to call rte_flow_validate once or more in order to find out offload capabilities of that device, but for now it is only a comment. In the future if/when we do call rte_flow_validate, I think it should not be in this function, but in a function maybe more specific for that, like netdev_offload_dpdk_cap(), or something like that. In current code for sure, I don't see any issue to call this function in the offloading process, and not only during init. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 12/4/2019 5:42 PM, Ilya Maximets wrote:
> On 04.12.2019 16:25, Eli Britstein wrote:
>> On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
>>> On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
NEWS | 2 +
lib/netdev-offload-dpdk-flow.c | 87
--
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/NEWS b/NEWS
index 88b818948..ca9c2b230 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,8 @@ Post-v2.12.0
if supported by libbpf.
* Add option to enable, disable and query TCP sequence checking in
conntrack.
+ - DPDK:
+ * Add hardware offload support for output actions.
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-offload-dpdk-flow.c
b/lib/netdev-offload-dpdk-flow.c
index dbbc45e99..6e7efb315 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_cstr(s, " RSS = null\n");
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
+const struct rte_flow_action_count *count = actions->conf;
+
+ds_put_cstr(s, "rte flow count action:\n");
+if (count) {
+ds_put_format(s,
+ " Count: shared=%d, id=%d\n",
+ count->shared, count->id);
+} else {
+ds_put_cstr(s, " Count = null\n");
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
+const struct rte_flow_action_port_id *port_id = actions->conf;
+
+ds_put_cstr(s, "rte flow port-id action:\n");
+if (port_id) {
+ds_put_format(s,
+ " Port-id: original=%d, id=%d\n",
+ port_id->original, port_id->id);
+} else {
+ds_put_cstr(s, " Port-id = null\n");
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n",
actions->type);
}
@@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
*patterns,
return 0;
}
+static void
+netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
+{
+struct rte_flow_action_count *count = xzalloc(sizeof *count);
+
+count->shared = 0;
+/* Each flow has a single count action, so no need of id */
+count->id = 0;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
+}
+
+static void
+netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
+struct netdev *outdev)
+{
+struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
+
+port_id->id = netdev_dpdk_get_port_id(outdev);
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+}
+
+static int
+netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
+ const struct nlattr *nla,
+ struct offload_info *info)
+{
+struct netdev *outdev;
+odp_port_t port;
+
+port = nl_attr_get_odp_port(nla);
+outdev = netdev_ports_get(port, info->dpif_class);
+if (outdev == NULL) {
+VLOG_DBG_RL(_rl,
+"Cannot find netdev for odp port %d", port);
+return -1;
+}
+if (!netdev_dpdk_flow_api_supported(outdev)) {
+VLOG_DBG_RL(_rl,
+"Output to %s cannot be offloaded",
+netdev_get_name(outdev));
+return -1;
+}
+
+netdev_dpdk_flow_add_count_action(actions);
+netdev_dpdk_flow_add_port_id_action(actions, outdev);
+netdev_close(outdev);
+
+return 0;
+}
+
int
netdev_dpdk_flow_actions_add(struct flow_actions *actions,
struct nlattr *nl_actions,
size_t nl_actions_len,
- struct offload_info *info OVS_UNUSED)
+ struct offload_info *info)
{
struct nlattr *nla;
size_t left;
NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
-VLOG_DBG_RL(_rl,
-"Unsupported action type %d", nl_attr_type(nla));
-return -1;
+if
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 04.12.2019 16:25, Eli Britstein wrote:
>
> On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
>> On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
>>> Signed-off-by: Eli Britstein
>>> Reviewed-by: Oz Shlomo
>>> ---
>>> NEWS | 2 +
>>> lib/netdev-offload-dpdk-flow.c | 87
>>> --
>>> 2 files changed, 85 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/NEWS b/NEWS
>>> index 88b818948..ca9c2b230 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -10,6 +10,8 @@ Post-v2.12.0
>>> if supported by libbpf.
>>>* Add option to enable, disable and query TCP sequence checking in
>>> conntrack.
>>> + - DPDK:
>>> + * Add hardware offload support for output actions.
>>>
>>> v2.12.0 - 03 Sep 2019
>>> -
>>> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
>>> index dbbc45e99..6e7efb315 100644
>>> --- a/lib/netdev-offload-dpdk-flow.c
>>> +++ b/lib/netdev-offload-dpdk-flow.c
>>> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
>>> rte_flow_action *actions)
>>> } else {
>>> ds_put_cstr(s, " RSS = null\n");
>>> }
>>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
>>> +const struct rte_flow_action_count *count = actions->conf;
>>> +
>>> +ds_put_cstr(s, "rte flow count action:\n");
>>> +if (count) {
>>> +ds_put_format(s,
>>> + " Count: shared=%d, id=%d\n",
>>> + count->shared, count->id);
>>> +} else {
>>> +ds_put_cstr(s, " Count = null\n");
>>> +}
>>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
>>> +const struct rte_flow_action_port_id *port_id = actions->conf;
>>> +
>>> +ds_put_cstr(s, "rte flow port-id action:\n");
>>> +if (port_id) {
>>> +ds_put_format(s,
>>> + " Port-id: original=%d, id=%d\n",
>>> + port_id->original, port_id->id);
>>> +} else {
>>> +ds_put_cstr(s, " Port-id = null\n");
>>> +}
>>> } else {
>>> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
>>> }
>>> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
>>> *patterns,
>>> return 0;
>>> }
>>>
>>> +static void
>>> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
>>> +{
>>> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
>>> +
>>> +count->shared = 0;
>>> +/* Each flow has a single count action, so no need of id */
>>> +count->id = 0;
>>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
>>> +}
>>> +
>>> +static void
>>> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
>>> +struct netdev *outdev)
>>> +{
>>> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
>>> +
>>> +port_id->id = netdev_dpdk_get_port_id(outdev);
>>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
>>> +}
>>> +
>>> +static int
>>> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
>>> + const struct nlattr *nla,
>>> + struct offload_info *info)
>>> +{
>>> +struct netdev *outdev;
>>> +odp_port_t port;
>>> +
>>> +port = nl_attr_get_odp_port(nla);
>>> +outdev = netdev_ports_get(port, info->dpif_class);
>>> +if (outdev == NULL) {
>>> +VLOG_DBG_RL(_rl,
>>> +"Cannot find netdev for odp port %d", port);
>>> +return -1;
>>> +}
>>> +if (!netdev_dpdk_flow_api_supported(outdev)) {
>>> +VLOG_DBG_RL(_rl,
>>> +"Output to %s cannot be offloaded",
>>> +netdev_get_name(outdev));
>>> +return -1;
>>> +}
>>> +
>>> +netdev_dpdk_flow_add_count_action(actions);
>>> +netdev_dpdk_flow_add_port_id_action(actions, outdev);
>>> +netdev_close(outdev);
>>> +
>>> +return 0;
>>> +}
>>> +
>>> int
>>> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
>>>struct nlattr *nl_actions,
>>>size_t nl_actions_len,
>>> - struct offload_info *info OVS_UNUSED)
>>> + struct offload_info *info)
>>> {
>>> struct nlattr *nla;
>>> size_t left;
>>>
>>> NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
>>> -VLOG_DBG_RL(_rl,
>>> -"Unsupported action type %d", nl_attr_type(nla));
>>> -return -1;
>>> +if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT &&
>>> +left <= NLA_ALIGN(nla->nla_len)) {
>>> +if (netdev_dpdk_flow_add_output_action(actions, nla, info )) {
>>> +return -1;
>>> +
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
> On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
>> Signed-off-by: Eli Britstein
>> Reviewed-by: Oz Shlomo
>> ---
>> NEWS | 2 +
>> lib/netdev-offload-dpdk-flow.c | 87
>> --
>> 2 files changed, 85 insertions(+), 4 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 88b818948..ca9c2b230 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -10,6 +10,8 @@ Post-v2.12.0
>> if supported by libbpf.
>>* Add option to enable, disable and query TCP sequence checking in
>> conntrack.
>> + - DPDK:
>> + * Add hardware offload support for output actions.
>>
>> v2.12.0 - 03 Sep 2019
>> -
>> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
>> index dbbc45e99..6e7efb315 100644
>> --- a/lib/netdev-offload-dpdk-flow.c
>> +++ b/lib/netdev-offload-dpdk-flow.c
>> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
>> rte_flow_action *actions)
>> } else {
>> ds_put_cstr(s, " RSS = null\n");
>> }
>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
>> +const struct rte_flow_action_count *count = actions->conf;
>> +
>> +ds_put_cstr(s, "rte flow count action:\n");
>> +if (count) {
>> +ds_put_format(s,
>> + " Count: shared=%d, id=%d\n",
>> + count->shared, count->id);
>> +} else {
>> +ds_put_cstr(s, " Count = null\n");
>> +}
>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
>> +const struct rte_flow_action_port_id *port_id = actions->conf;
>> +
>> +ds_put_cstr(s, "rte flow port-id action:\n");
>> +if (port_id) {
>> +ds_put_format(s,
>> + " Port-id: original=%d, id=%d\n",
>> + port_id->original, port_id->id);
>> +} else {
>> +ds_put_cstr(s, " Port-id = null\n");
>> +}
>> } else {
>> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
>> }
>> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
>> *patterns,
>> return 0;
>> }
>>
>> +static void
>> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
>> +{
>> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
>> +
>> +count->shared = 0;
>> +/* Each flow has a single count action, so no need of id */
>> +count->id = 0;
>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
>> +}
>> +
>> +static void
>> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
>> +struct netdev *outdev)
>> +{
>> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
>> +
>> +port_id->id = netdev_dpdk_get_port_id(outdev);
>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
>> +}
>> +
>> +static int
>> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
>> + const struct nlattr *nla,
>> + struct offload_info *info)
>> +{
>> +struct netdev *outdev;
>> +odp_port_t port;
>> +
>> +port = nl_attr_get_odp_port(nla);
>> +outdev = netdev_ports_get(port, info->dpif_class);
>> +if (outdev == NULL) {
>> +VLOG_DBG_RL(_rl,
>> +"Cannot find netdev for odp port %d", port);
>> +return -1;
>> +}
>> +if (!netdev_dpdk_flow_api_supported(outdev)) {
>> +VLOG_DBG_RL(_rl,
>> +"Output to %s cannot be offloaded",
>> +netdev_get_name(outdev));
>> +return -1;
>> +}
>> +
>> +netdev_dpdk_flow_add_count_action(actions);
>> +netdev_dpdk_flow_add_port_id_action(actions, outdev);
>> +netdev_close(outdev);
>> +
>> +return 0;
>> +}
>> +
>> int
>> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
>>struct nlattr *nl_actions,
>>size_t nl_actions_len,
>> - struct offload_info *info OVS_UNUSED)
>> + struct offload_info *info)
>> {
>> struct nlattr *nla;
>> size_t left;
>>
>> NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
>> -VLOG_DBG_RL(_rl,
>> -"Unsupported action type %d", nl_attr_type(nla));
>> -return -1;
>> +if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT &&
>> +left <= NLA_ALIGN(nla->nla_len)) {
>> +if (netdev_dpdk_flow_add_output_action(actions, nla, info )) {
>> +return -1;
>> +}
> The check in netdev_dpdk_flow_add_output_action() is insufficient to
> decide whether the device is capable of full-offload, since it assumes
> that the output
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Oz Shlomo
> ---
> NEWS | 2 +
> lib/netdev-offload-dpdk-flow.c | 87
> --
> 2 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 88b818948..ca9c2b230 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.12.0
> if supported by libbpf.
> * Add option to enable, disable and query TCP sequence checking in
> conntrack.
> + - DPDK:
> + * Add hardware offload support for output actions.
>
> v2.12.0 - 03 Sep 2019
> -
> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
> index dbbc45e99..6e7efb315 100644
> --- a/lib/netdev-offload-dpdk-flow.c
> +++ b/lib/netdev-offload-dpdk-flow.c
> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
> rte_flow_action *actions)
> } else {
> ds_put_cstr(s, " RSS = null\n");
> }
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
> +const struct rte_flow_action_count *count = actions->conf;
> +
> +ds_put_cstr(s, "rte flow count action:\n");
> +if (count) {
> +ds_put_format(s,
> + " Count: shared=%d, id=%d\n",
> + count->shared, count->id);
> +} else {
> +ds_put_cstr(s, " Count = null\n");
> +}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
> +const struct rte_flow_action_port_id *port_id = actions->conf;
> +
> +ds_put_cstr(s, "rte flow port-id action:\n");
> +if (port_id) {
> +ds_put_format(s,
> + " Port-id: original=%d, id=%d\n",
> + port_id->original, port_id->id);
> +} else {
> +ds_put_cstr(s, " Port-id = null\n");
> +}
> } else {
> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
> }
> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
> *patterns,
> return 0;
> }
>
> +static void
> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
> +{
> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
> +
> +count->shared = 0;
> +/* Each flow has a single count action, so no need of id */
> +count->id = 0;
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
> +}
> +
> +static void
> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
> +struct netdev *outdev)
> +{
> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
> +
> +port_id->id = netdev_dpdk_get_port_id(outdev);
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> +}
> +
> +static int
> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
> + const struct nlattr *nla,
> + struct offload_info *info)
> +{
> +struct netdev *outdev;
> +odp_port_t port;
> +
> +port = nl_attr_get_odp_port(nla);
> +outdev = netdev_ports_get(port, info->dpif_class);
> +if (outdev == NULL) {
> +VLOG_DBG_RL(_rl,
> +"Cannot find netdev for odp port %d", port);
> +return -1;
> +}
> +if (!netdev_dpdk_flow_api_supported(outdev)) {
> +VLOG_DBG_RL(_rl,
> +"Output to %s cannot be offloaded",
> +netdev_get_name(outdev));
> +return -1;
> +}
> +
> +netdev_dpdk_flow_add_count_action(actions);
> +netdev_dpdk_flow_add_port_id_action(actions, outdev);
> +netdev_close(outdev);
> +
> +return 0;
> +}
> +
> int
> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
> struct nlattr *nl_actions,
> size_t nl_actions_len,
> - struct offload_info *info OVS_UNUSED)
> + struct offload_info *info)
> {
> struct nlattr *nla;
> size_t left;
>
> NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
> -VLOG_DBG_RL(_rl,
> -"Unsupported action type %d", nl_attr_type(nla));
> -return -1;
> +if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT &&
> +left <= NLA_ALIGN(nla->nla_len)) {
> +if (netdev_dpdk_flow_add_output_action(actions, nla, info )) {
> +return -1;
> +}
The check in netdev_dpdk_flow_add_output_action() is insufficient to
decide whether the device is capable of full-offload, since it assumes
that the output action is supported if netdev_dpdk type is
DPDK_DEV_ETH.
There are a couple of issues with this approach:
1. For a device that is not capable of full-offload, it results in 2
calls to the PMD for every
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 12/2/2019 4:25 PM, Ilya Maximets wrote:
> On 01.12.2019 9:29, Eli Britstein wrote:
>> On 11/30/2019 1:59 PM, Ilya Maximets wrote:
>>> On 24.11.2019 14:22, Eli Britstein wrote:
On 11/22/2019 6:19 PM, Ilya Maximets wrote:
> On 20.11.2019 16:28, Eli Britstein wrote:
>> Signed-off-by: Eli Britstein
>> Reviewed-by: Oz Shlomo
>> ---
>> NEWS | 2 +
>> lib/netdev-offload-dpdk-flow.c | 87
>> --
>> 2 files changed, 85 insertions(+), 4 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 88b818948..ca9c2b230 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -10,6 +10,8 @@ Post-v2.12.0
>>if supported by libbpf.
>> * Add option to enable, disable and query TCP sequence checking
>> in
>>conntrack.
>> + - DPDK:
>> + * Add hardware offload support for output actions.
>>
>> v2.12.0 - 03 Sep 2019
>> -
>> diff --git a/lib/netdev-offload-dpdk-flow.c
>> b/lib/netdev-offload-dpdk-flow.c
>> index dbbc45e99..6e7efb315 100644
>> --- a/lib/netdev-offload-dpdk-flow.c
>> +++ b/lib/netdev-offload-dpdk-flow.c
>> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
>> rte_flow_action *actions)
>> } else {
>> ds_put_cstr(s, " RSS = null\n");
>> }
>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
>> +const struct rte_flow_action_count *count = actions->conf;
>> +
>> +ds_put_cstr(s, "rte flow count action:\n");
>> +if (count) {
>> +ds_put_format(s,
>> + " Count: shared=%d, id=%d\n",
>> + count->shared, count->id);
>> +} else {
>> +ds_put_cstr(s, " Count = null\n");
>> +}
>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
>> +const struct rte_flow_action_port_id *port_id = actions->conf;
>> +
>> +ds_put_cstr(s, "rte flow port-id action:\n");
>> +if (port_id) {
>> +ds_put_format(s,
>> + " Port-id: original=%d, id=%d\n",
>> + port_id->original, port_id->id);
>> +} else {
>> +ds_put_cstr(s, " Port-id = null\n");
>> +}
>> } else {
>> ds_put_format(s, "unknown rte flow action (%d)\n",
>> actions->type);
>> }
>> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
>> *patterns,
>> return 0;
>> }
>>
>> +static void
>> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
>> +{
>> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
>> +
>> +count->shared = 0;
>> +/* Each flow has a single count action, so no need of id */
>> +count->id = 0;
>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
>> +}
>> +
>> +static void
>> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
>> +struct netdev *outdev)
>> +{
>> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
>> +
>> +port_id->id = netdev_dpdk_get_port_id(outdev);
>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
>> +}
>> +
>> +static int
>> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
>> + const struct nlattr *nla,
>> + struct offload_info *info)
>> +{
>> +struct netdev *outdev;
>> +odp_port_t port;
>> +
>> +port = nl_attr_get_odp_port(nla);
>> +outdev = netdev_ports_get(port, info->dpif_class);
>> +if (outdev == NULL) {
>> +VLOG_DBG_RL(_rl,
>> +"Cannot find netdev for odp port %d", port);
>> +return -1;
>> +}
>> +if (!netdev_dpdk_flow_api_supported(outdev)) {
> This doesn't look correct. I mean, maybe we need to check if port is
> really the port on a same piece of hardware. Will the flow setup fail
> in this case? Will code fallback to the MARK+RSS?
We cannot check if the port is on the same HW, and it is also wrong from
the application point of view. If the operation cannot be supported, it
is in the driver (DPDK) scope to fail it.
>>> BTW, function netdev_dpdk_flow_api_supported() is not intended to be called
>>> in the offloading process. It's only for initialization phase. You can see
>>> the "/* TODO: Check if we able to offload some minimal flow. */" in the code
>>> and that might be destructive and unwanted for offloading
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 01.12.2019 9:29, Eli Britstein wrote:
>
> On 11/30/2019 1:59 PM, Ilya Maximets wrote:
>> On 24.11.2019 14:22, Eli Britstein wrote:
>>> On 11/22/2019 6:19 PM, Ilya Maximets wrote:
On 20.11.2019 16:28, Eli Britstein wrote:
> Signed-off-by: Eli Britstein
> Reviewed-by: Oz Shlomo
> ---
>NEWS | 2 +
>lib/netdev-offload-dpdk-flow.c | 87
> --
>2 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 88b818948..ca9c2b230 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.12.0
> if supported by libbpf.
> * Add option to enable, disable and query TCP sequence checking in
> conntrack.
> + - DPDK:
> + * Add hardware offload support for output actions.
>
>v2.12.0 - 03 Sep 2019
>-
> diff --git a/lib/netdev-offload-dpdk-flow.c
> b/lib/netdev-offload-dpdk-flow.c
> index dbbc45e99..6e7efb315 100644
> --- a/lib/netdev-offload-dpdk-flow.c
> +++ b/lib/netdev-offload-dpdk-flow.c
> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
> rte_flow_action *actions)
>} else {
>ds_put_cstr(s, " RSS = null\n");
>}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
> +const struct rte_flow_action_count *count = actions->conf;
> +
> +ds_put_cstr(s, "rte flow count action:\n");
> +if (count) {
> +ds_put_format(s,
> + " Count: shared=%d, id=%d\n",
> + count->shared, count->id);
> +} else {
> +ds_put_cstr(s, " Count = null\n");
> +}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
> +const struct rte_flow_action_port_id *port_id = actions->conf;
> +
> +ds_put_cstr(s, "rte flow port-id action:\n");
> +if (port_id) {
> +ds_put_format(s,
> + " Port-id: original=%d, id=%d\n",
> + port_id->original, port_id->id);
> +} else {
> +ds_put_cstr(s, " Port-id = null\n");
> +}
>} else {
>ds_put_format(s, "unknown rte flow action (%d)\n",
> actions->type);
>}
> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
> *patterns,
>return 0;
>}
>
> +static void
> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
> +{
> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
> +
> +count->shared = 0;
> +/* Each flow has a single count action, so no need of id */
> +count->id = 0;
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
> +}
> +
> +static void
> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
> +struct netdev *outdev)
> +{
> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
> +
> +port_id->id = netdev_dpdk_get_port_id(outdev);
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> +}
> +
> +static int
> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
> + const struct nlattr *nla,
> + struct offload_info *info)
> +{
> +struct netdev *outdev;
> +odp_port_t port;
> +
> +port = nl_attr_get_odp_port(nla);
> +outdev = netdev_ports_get(port, info->dpif_class);
> +if (outdev == NULL) {
> +VLOG_DBG_RL(_rl,
> +"Cannot find netdev for odp port %d", port);
> +return -1;
> +}
> +if (!netdev_dpdk_flow_api_supported(outdev)) {
This doesn't look correct. I mean, maybe we need to check if port is
really the port on a same piece of hardware. Will the flow setup fail
in this case? Will code fallback to the MARK+RSS?
>>> We cannot check if the port is on the same HW, and it is also wrong from
>>> the application point of view. If the operation cannot be supported, it
>>> is in the driver (DPDK) scope to fail it.
>> BTW, function netdev_dpdk_flow_api_supported() is not intended to be called
>> in the offloading process. It's only for initialization phase. You can see
>> the "/* TODO: Check if we able to offload some minimal flow. */" in the code
>> and that might be destructive and unwanted for offloading process.
>
> Actually, there is no need to call it at all, as we get here only if we
> found the netdev by netdev_ports_get, which means we found a device that
> can have offloads.
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 11/30/2019 1:59 PM, Ilya Maximets wrote:
> On 24.11.2019 14:22, Eli Britstein wrote:
>> On 11/22/2019 6:19 PM, Ilya Maximets wrote:
>>> On 20.11.2019 16:28, Eli Britstein wrote:
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
NEWS | 2 +
lib/netdev-offload-dpdk-flow.c | 87
--
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/NEWS b/NEWS
index 88b818948..ca9c2b230 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,8 @@ Post-v2.12.0
if supported by libbpf.
* Add option to enable, disable and query TCP sequence checking in
conntrack.
+ - DPDK:
+ * Add hardware offload support for output actions.
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-offload-dpdk-flow.c
b/lib/netdev-offload-dpdk-flow.c
index dbbc45e99..6e7efb315 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_cstr(s, " RSS = null\n");
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
+const struct rte_flow_action_count *count = actions->conf;
+
+ds_put_cstr(s, "rte flow count action:\n");
+if (count) {
+ds_put_format(s,
+ " Count: shared=%d, id=%d\n",
+ count->shared, count->id);
+} else {
+ds_put_cstr(s, " Count = null\n");
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
+const struct rte_flow_action_port_id *port_id = actions->conf;
+
+ds_put_cstr(s, "rte flow port-id action:\n");
+if (port_id) {
+ds_put_format(s,
+ " Port-id: original=%d, id=%d\n",
+ port_id->original, port_id->id);
+} else {
+ds_put_cstr(s, " Port-id = null\n");
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n",
actions->type);
}
@@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
*patterns,
return 0;
}
+static void
+netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
+{
+struct rte_flow_action_count *count = xzalloc(sizeof *count);
+
+count->shared = 0;
+/* Each flow has a single count action, so no need of id */
+count->id = 0;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
+}
+
+static void
+netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
+struct netdev *outdev)
+{
+struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
+
+port_id->id = netdev_dpdk_get_port_id(outdev);
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+}
+
+static int
+netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
+ const struct nlattr *nla,
+ struct offload_info *info)
+{
+struct netdev *outdev;
+odp_port_t port;
+
+port = nl_attr_get_odp_port(nla);
+outdev = netdev_ports_get(port, info->dpif_class);
+if (outdev == NULL) {
+VLOG_DBG_RL(_rl,
+"Cannot find netdev for odp port %d", port);
+return -1;
+}
+if (!netdev_dpdk_flow_api_supported(outdev)) {
>>> This doesn't look correct. I mean, maybe we need to check if port is
>>> really the port on a same piece of hardware. Will the flow setup fail
>>> in this case? Will code fallback to the MARK+RSS?
>> We cannot check if the port is on the same HW, and it is also wrong from
>> the application point of view. If the operation cannot be supported, it
>> is in the driver (DPDK) scope to fail it.
> BTW, function netdev_dpdk_flow_api_supported() is not intended to be called
> in the offloading process. It's only for initialization phase. You can see
> the "/* TODO: Check if we able to offload some minimal flow. */" in the code
> and that might be destructive and unwanted for offloading process.
Actually, there is no need to call it at all, as we get here only if we
found the netdev by netdev_ports_get, which means we found a device that
can have offloads.
I enhanced for error handling of netdev_dpdk_get_port_id instead of
introducing such new code, that I think not required.
>
> You, probably, wanted something like
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 24.11.2019 14:22, Eli Britstein wrote:
>
> On 11/22/2019 6:19 PM, Ilya Maximets wrote:
>> On 20.11.2019 16:28, Eli Britstein wrote:
>>> Signed-off-by: Eli Britstein
>>> Reviewed-by: Oz Shlomo
>>> ---
>>> NEWS | 2 +
>>> lib/netdev-offload-dpdk-flow.c | 87
>>> --
>>> 2 files changed, 85 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/NEWS b/NEWS
>>> index 88b818948..ca9c2b230 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -10,6 +10,8 @@ Post-v2.12.0
>>> if supported by libbpf.
>>>* Add option to enable, disable and query TCP sequence checking in
>>> conntrack.
>>> + - DPDK:
>>> + * Add hardware offload support for output actions.
>>>
>>> v2.12.0 - 03 Sep 2019
>>> -
>>> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
>>> index dbbc45e99..6e7efb315 100644
>>> --- a/lib/netdev-offload-dpdk-flow.c
>>> +++ b/lib/netdev-offload-dpdk-flow.c
>>> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
>>> rte_flow_action *actions)
>>> } else {
>>> ds_put_cstr(s, " RSS = null\n");
>>> }
>>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
>>> +const struct rte_flow_action_count *count = actions->conf;
>>> +
>>> +ds_put_cstr(s, "rte flow count action:\n");
>>> +if (count) {
>>> +ds_put_format(s,
>>> + " Count: shared=%d, id=%d\n",
>>> + count->shared, count->id);
>>> +} else {
>>> +ds_put_cstr(s, " Count = null\n");
>>> +}
>>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
>>> +const struct rte_flow_action_port_id *port_id = actions->conf;
>>> +
>>> +ds_put_cstr(s, "rte flow port-id action:\n");
>>> +if (port_id) {
>>> +ds_put_format(s,
>>> + " Port-id: original=%d, id=%d\n",
>>> + port_id->original, port_id->id);
>>> +} else {
>>> +ds_put_cstr(s, " Port-id = null\n");
>>> +}
>>> } else {
>>> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
>>> }
>>> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
>>> *patterns,
>>> return 0;
>>> }
>>>
>>> +static void
>>> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
>>> +{
>>> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
>>> +
>>> +count->shared = 0;
>>> +/* Each flow has a single count action, so no need of id */
>>> +count->id = 0;
>>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
>>> +}
>>> +
>>> +static void
>>> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
>>> +struct netdev *outdev)
>>> +{
>>> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
>>> +
>>> +port_id->id = netdev_dpdk_get_port_id(outdev);
>>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
>>> +}
>>> +
>>> +static int
>>> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
>>> + const struct nlattr *nla,
>>> + struct offload_info *info)
>>> +{
>>> +struct netdev *outdev;
>>> +odp_port_t port;
>>> +
>>> +port = nl_attr_get_odp_port(nla);
>>> +outdev = netdev_ports_get(port, info->dpif_class);
>>> +if (outdev == NULL) {
>>> +VLOG_DBG_RL(_rl,
>>> +"Cannot find netdev for odp port %d", port);
>>> +return -1;
>>> +}
>>> +if (!netdev_dpdk_flow_api_supported(outdev)) {
>> This doesn't look correct. I mean, maybe we need to check if port is
>> really the port on a same piece of hardware. Will the flow setup fail
>> in this case? Will code fallback to the MARK+RSS?
>
> We cannot check if the port is on the same HW, and it is also wrong from
> the application point of view. If the operation cannot be supported, it
> is in the driver (DPDK) scope to fail it.
BTW, function netdev_dpdk_flow_api_supported() is not intended to be called
in the offloading process. It's only for initialization phase. You can see
the "/* TODO: Check if we able to offload some minimal flow. */" in the code
and that might be destructive and unwanted for offloading process.
You, probably, wanted something like this instead (not tested):
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 4e1c4251d..92876f3a3 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -90,6 +90,9 @@ struct netdev_flow_api {
int netdev_register_flow_api_provider(const struct netdev_flow_api *);
int netdev_unregister_flow_api_provider(const char *type);
+bool netdev_flow_api_equals(const struct netdev *,
+const
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 11/22/2019 6:19 PM, Ilya Maximets wrote:
> On 20.11.2019 16:28, Eli Britstein wrote:
>> Signed-off-by: Eli Britstein
>> Reviewed-by: Oz Shlomo
>> ---
>> NEWS | 2 +
>> lib/netdev-offload-dpdk-flow.c | 87
>> --
>> 2 files changed, 85 insertions(+), 4 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 88b818948..ca9c2b230 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -10,6 +10,8 @@ Post-v2.12.0
>> if supported by libbpf.
>>* Add option to enable, disable and query TCP sequence checking in
>> conntrack.
>> + - DPDK:
>> + * Add hardware offload support for output actions.
>>
>> v2.12.0 - 03 Sep 2019
>> -
>> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
>> index dbbc45e99..6e7efb315 100644
>> --- a/lib/netdev-offload-dpdk-flow.c
>> +++ b/lib/netdev-offload-dpdk-flow.c
>> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
>> rte_flow_action *actions)
>> } else {
>> ds_put_cstr(s, " RSS = null\n");
>> }
>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
>> +const struct rte_flow_action_count *count = actions->conf;
>> +
>> +ds_put_cstr(s, "rte flow count action:\n");
>> +if (count) {
>> +ds_put_format(s,
>> + " Count: shared=%d, id=%d\n",
>> + count->shared, count->id);
>> +} else {
>> +ds_put_cstr(s, " Count = null\n");
>> +}
>> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
>> +const struct rte_flow_action_port_id *port_id = actions->conf;
>> +
>> +ds_put_cstr(s, "rte flow port-id action:\n");
>> +if (port_id) {
>> +ds_put_format(s,
>> + " Port-id: original=%d, id=%d\n",
>> + port_id->original, port_id->id);
>> +} else {
>> +ds_put_cstr(s, " Port-id = null\n");
>> +}
>> } else {
>> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
>> }
>> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
>> *patterns,
>> return 0;
>> }
>>
>> +static void
>> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
>> +{
>> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
>> +
>> +count->shared = 0;
>> +/* Each flow has a single count action, so no need of id */
>> +count->id = 0;
>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
>> +}
>> +
>> +static void
>> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
>> +struct netdev *outdev)
>> +{
>> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
>> +
>> +port_id->id = netdev_dpdk_get_port_id(outdev);
>> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
>> +}
>> +
>> +static int
>> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
>> + const struct nlattr *nla,
>> + struct offload_info *info)
>> +{
>> +struct netdev *outdev;
>> +odp_port_t port;
>> +
>> +port = nl_attr_get_odp_port(nla);
>> +outdev = netdev_ports_get(port, info->dpif_class);
>> +if (outdev == NULL) {
>> +VLOG_DBG_RL(_rl,
>> +"Cannot find netdev for odp port %d", port);
>> +return -1;
>> +}
>> +if (!netdev_dpdk_flow_api_supported(outdev)) {
> This doesn't look correct. I mean, maybe we need to check if port is
> really the port on a same piece of hardware. Will the flow setup fail
> in this case? Will code fallback to the MARK+RSS?
We cannot check if the port is on the same HW, and it is also wrong from
the application point of view. If the operation cannot be supported, it
is in the driver (DPDK) scope to fail it.
You are right about the fallback. I'll fix it in v2.
>
>> +VLOG_DBG_RL(_rl,
>> +"Output to %s cannot be offloaded",
>> +netdev_get_name(outdev));
>> +return -1;
>> +}
>> +
>> +netdev_dpdk_flow_add_count_action(actions);
>> +netdev_dpdk_flow_add_port_id_action(actions, outdev);
>> +netdev_close(outdev);
>> +
>> +return 0;
>> +}
>> +
>> int
>> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
>>struct nlattr *nl_actions,
>>size_t nl_actions_len,
>> - struct offload_info *info OVS_UNUSED)
>> + struct offload_info *info)
>> {
>> struct nlattr *nla;
>> size_t left;
>>
>> NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
>> -VLOG_DBG_RL(_rl,
>> -"Unsupported action type %d",
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 20.11.2019 16:28, Eli Britstein wrote:
> Signed-off-by: Eli Britstein
> Reviewed-by: Oz Shlomo
> ---
> NEWS | 2 +
> lib/netdev-offload-dpdk-flow.c | 87
> --
> 2 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 88b818948..ca9c2b230 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.12.0
> if supported by libbpf.
> * Add option to enable, disable and query TCP sequence checking in
> conntrack.
> + - DPDK:
> + * Add hardware offload support for output actions.
>
> v2.12.0 - 03 Sep 2019
> -
> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
> index dbbc45e99..6e7efb315 100644
> --- a/lib/netdev-offload-dpdk-flow.c
> +++ b/lib/netdev-offload-dpdk-flow.c
> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
> rte_flow_action *actions)
> } else {
> ds_put_cstr(s, " RSS = null\n");
> }
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
> +const struct rte_flow_action_count *count = actions->conf;
> +
> +ds_put_cstr(s, "rte flow count action:\n");
> +if (count) {
> +ds_put_format(s,
> + " Count: shared=%d, id=%d\n",
> + count->shared, count->id);
> +} else {
> +ds_put_cstr(s, " Count = null\n");
> +}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
> +const struct rte_flow_action_port_id *port_id = actions->conf;
> +
> +ds_put_cstr(s, "rte flow port-id action:\n");
> +if (port_id) {
> +ds_put_format(s,
> + " Port-id: original=%d, id=%d\n",
> + port_id->original, port_id->id);
> +} else {
> +ds_put_cstr(s, " Port-id = null\n");
> +}
> } else {
> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
> }
> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
> *patterns,
> return 0;
> }
>
> +static void
> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
> +{
> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
> +
> +count->shared = 0;
> +/* Each flow has a single count action, so no need of id */
> +count->id = 0;
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
> +}
> +
> +static void
> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
> +struct netdev *outdev)
> +{
> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
> +
> +port_id->id = netdev_dpdk_get_port_id(outdev);
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> +}
> +
> +static int
> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
> + const struct nlattr *nla,
> + struct offload_info *info)
> +{
> +struct netdev *outdev;
> +odp_port_t port;
> +
> +port = nl_attr_get_odp_port(nla);
> +outdev = netdev_ports_get(port, info->dpif_class);
> +if (outdev == NULL) {
> +VLOG_DBG_RL(_rl,
> +"Cannot find netdev for odp port %d", port);
> +return -1;
> +}
> +if (!netdev_dpdk_flow_api_supported(outdev)) {
This doesn't look correct. I mean, maybe we need to check if port is
really the port on a same piece of hardware. Will the flow setup fail
in this case? Will code fallback to the MARK+RSS?
> +VLOG_DBG_RL(_rl,
> +"Output to %s cannot be offloaded",
> +netdev_get_name(outdev));
> +return -1;
> +}
> +
> +netdev_dpdk_flow_add_count_action(actions);
> +netdev_dpdk_flow_add_port_id_action(actions, outdev);
> +netdev_close(outdev);
> +
> +return 0;
> +}
> +
> int
> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
> struct nlattr *nl_actions,
> size_t nl_actions_len,
> - struct offload_info *info OVS_UNUSED)
> + struct offload_info *info)
> {
> struct nlattr *nla;
> size_t left;
>
> NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
> -VLOG_DBG_RL(_rl,
> -"Unsupported action type %d", nl_attr_type(nla));
> -return -1;
> +if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT &&
> +left <= NLA_ALIGN(nla->nla_len)) {
> +if (netdev_dpdk_flow_add_output_action(actions, nla, info )) {
> +return -1;
> +}
> +} else {
> +VLOG_DBG_RL(_rl,
> +"Unsupported action type %d", nl_attr_type(nla));
> +return -1;
> +}
> }
>
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
Bleep bloop. Greetings Eli Britstein, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: Failed to merge in the changes. Patch failed at 0001 netdev-offload-dpdk-flow: Support offload of output action The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
