Re: [ovs-dev] [PATCH V2 11/14] netdev-offload-dpdk: Refactor offload rule creation
On Thu, Feb 25, 2021 at 7:51 PM Eli Britstein wrote:
>
>
> On 2/25/2021 9:35 AM, Sriharsha Basavapatna wrote:
> > On Wed, Feb 24, 2021 at 3:47 PM Eli Britstein wrote:
> >>
> >> On 2/24/2021 11:55 AM, Sriharsha Basavapatna wrote:
> >>> On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
> Refactor offload rule creation as a pre-step towards tunnel matching
> that depend on the netdev.
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
> ---
> lib/netdev-offload-dpdk.c | 106 --
> 1 file changed, 45 insertions(+), 61 deletions(-)
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 493cc9159..f6e668bff 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -1009,30 +1009,6 @@ add_flow_mark_rss_actions(struct flow_actions
> *actions,
> add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
> }
>
> -static struct rte_flow *
> -netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
> - struct netdev *netdev,
> - uint32_t flow_mark)
> -{
> -struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> -const struct rte_flow_attr flow_attr = {
> -.group = 0,
> -.priority = 0,
> -.ingress = 1,
> -.egress = 0
> -};
> -struct rte_flow_error error;
> -struct rte_flow *flow;
> -
> -add_flow_mark_rss_actions(&actions, flow_mark, netdev);
> -
> -flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr,
> patterns->items,
> - &actions, &error);
> -
> -free_flow_actions(&actions);
> -return flow;
> -}
> -
> static void
> add_count_action(struct flow_actions *actions)
> {
> @@ -1509,27 +1485,48 @@ parse_flow_actions(struct netdev *netdev,
> return 0;
> }
>
> -static struct rte_flow *
> -netdev_offload_dpdk_actions(struct netdev *netdev,
> -struct flow_patterns *patterns,
> -struct nlattr *nl_actions,
> -size_t actions_len)
> +static struct ufid_to_rte_flow_data *
> +create_netdev_offload(struct netdev *netdev,
> + const ovs_u128 *ufid,
> + struct flow_patterns *flow_patterns,
> + struct flow_actions *flow_actions,
> + bool enable_full,
> + uint32_t flow_mark)
> {
> -const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer =
> 1 };
> -struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> +struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1, };
> +struct flow_actions rss_actions = { .actions = NULL, .cnt = 0 };
> +struct rte_flow_item *items = flow_patterns->items;
> +struct ufid_to_rte_flow_data *flow_data = NULL;
> +bool actions_offloaded = true;
> struct rte_flow *flow = NULL;
> struct rte_flow_error error;
> -int ret;
>
> -ret = parse_flow_actions(netdev, &actions, nl_actions, actions_len);
> -if (ret) {
> -goto out;
> +if (enable_full) {
> +flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr,
> items,
> + flow_actions, &error);
> }
> -flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr,
> patterns->items,
> - &actions, &error);
> -out:
> -free_flow_actions(&actions);
> -return flow;
> +
> +if (!flow) {
> +/* If we failed to offload the rule actions fallback to MARK+RSS
> + * actions.
> + */
> >>> A debug message might be useful here, when we fallback to mark/rss
> >>> action ?
> >> We can, but this is just a refactor commit, and this fallback is not
> >> new. Add it anyway?
> > I think it'd be useful to add a debug message here and also in
> > parse_flow_actions() to indicate that action offloading failed.
>
> For the fallback, we have this info by dpctl/dump-flows ("partial").
> Furthermore, it may flood the log, depending on PMD rte_flow support.
>
> For parse_flow_action failure, there are some prints there with the
> places of failure, to be more useful rather than just a generic failure
> message.
>
> Let's keep this commit as a refactor commit without any logic changes,
> that can be added later. What do you think?
Ok.
>
> >
> +actions_offloaded = false;
> +flow_attr.transfer =
Re: [ovs-dev] [PATCH V2 11/14] netdev-offload-dpdk: Refactor offload rule creation
On 2/25/2021 9:35 AM, Sriharsha Basavapatna wrote:
On Wed, Feb 24, 2021 at 3:47 PM Eli Britstein wrote:
On 2/24/2021 11:55 AM, Sriharsha Basavapatna wrote:
On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
Refactor offload rule creation as a pre-step towards tunnel matching
that depend on the netdev.
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
lib/netdev-offload-dpdk.c | 106 --
1 file changed, 45 insertions(+), 61 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 493cc9159..f6e668bff 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -1009,30 +1009,6 @@ add_flow_mark_rss_actions(struct flow_actions *actions,
add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
}
-static struct rte_flow *
-netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
- struct netdev *netdev,
- uint32_t flow_mark)
-{
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
-const struct rte_flow_attr flow_attr = {
-.group = 0,
-.priority = 0,
-.ingress = 1,
-.egress = 0
-};
-struct rte_flow_error error;
-struct rte_flow *flow;
-
-add_flow_mark_rss_actions(&actions, flow_mark, netdev);
-
-flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, patterns->items,
- &actions, &error);
-
-free_flow_actions(&actions);
-return flow;
-}
-
static void
add_count_action(struct flow_actions *actions)
{
@@ -1509,27 +1485,48 @@ parse_flow_actions(struct netdev *netdev,
return 0;
}
-static struct rte_flow *
-netdev_offload_dpdk_actions(struct netdev *netdev,
-struct flow_patterns *patterns,
-struct nlattr *nl_actions,
-size_t actions_len)
+static struct ufid_to_rte_flow_data *
+create_netdev_offload(struct netdev *netdev,
+ const ovs_u128 *ufid,
+ struct flow_patterns *flow_patterns,
+ struct flow_actions *flow_actions,
+ bool enable_full,
+ uint32_t flow_mark)
{
-const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1, };
+struct flow_actions rss_actions = { .actions = NULL, .cnt = 0 };
+struct rte_flow_item *items = flow_patterns->items;
+struct ufid_to_rte_flow_data *flow_data = NULL;
+bool actions_offloaded = true;
struct rte_flow *flow = NULL;
struct rte_flow_error error;
-int ret;
-ret = parse_flow_actions(netdev, &actions, nl_actions, actions_len);
-if (ret) {
-goto out;
+if (enable_full) {
+flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
+ flow_actions, &error);
}
-flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, patterns->items,
- &actions, &error);
-out:
-free_flow_actions(&actions);
-return flow;
+
+if (!flow) {
+/* If we failed to offload the rule actions fallback to MARK+RSS
+ * actions.
+ */
A debug message might be useful here, when we fallback to mark/rss action ?
We can, but this is just a refactor commit, and this fallback is not
new. Add it anyway?
I think it'd be useful to add a debug message here and also in
parse_flow_actions() to indicate that action offloading failed.
For the fallback, we have this info by dpctl/dump-flows ("partial").
Furthermore, it may flood the log, depending on PMD rte_flow support.
For parse_flow_action failure, there are some prints there with the
places of failure, to be more useful rather than just a generic failure
message.
Let's keep this commit as a refactor commit without any logic changes,
that can be added later. What do you think?
+actions_offloaded = false;
+flow_attr.transfer = 0;
+add_flow_mark_rss_actions(&rss_actions, flow_mark, netdev);
+flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
+ &rss_actions, &error);
+}
+
+if (flow) {
+flow_data = ufid_to_rte_flow_associate(ufid, netdev, flow,
+ actions_offloaded);
+VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
+ netdev_get_name(netdev), flow,
+ UUID_ARGS((struct uuid *) ufid));
+}
+
+free_flow_actions(&rss_actions);
This free is needed only when we fallback to mark/rss offload, not otherwise.
OK.
+return flow_data;
}
static struct ufid_to_rte_flow_data *
@@ -1541,9 +1538,9 @@ netdev_offload_dpdk_add_
Re: [ovs-dev] [PATCH V2 11/14] netdev-offload-dpdk: Refactor offload rule creation
On Wed, Feb 24, 2021 at 3:47 PM Eli Britstein wrote:
>
>
> On 2/24/2021 11:55 AM, Sriharsha Basavapatna wrote:
> > On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
> >> Refactor offload rule creation as a pre-step towards tunnel matching
> >> that depend on the netdev.
> >>
> >> Signed-off-by: Eli Britstein
> >> Reviewed-by: Gaetan Rivet
> >> ---
> >> lib/netdev-offload-dpdk.c | 106 --
> >> 1 file changed, 45 insertions(+), 61 deletions(-)
> >>
> >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >> index 493cc9159..f6e668bff 100644
> >> --- a/lib/netdev-offload-dpdk.c
> >> +++ b/lib/netdev-offload-dpdk.c
> >> @@ -1009,30 +1009,6 @@ add_flow_mark_rss_actions(struct flow_actions
> >> *actions,
> >> add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
> >> }
> >>
> >> -static struct rte_flow *
> >> -netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
> >> - struct netdev *netdev,
> >> - uint32_t flow_mark)
> >> -{
> >> -struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> >> -const struct rte_flow_attr flow_attr = {
> >> -.group = 0,
> >> -.priority = 0,
> >> -.ingress = 1,
> >> -.egress = 0
> >> -};
> >> -struct rte_flow_error error;
> >> -struct rte_flow *flow;
> >> -
> >> -add_flow_mark_rss_actions(&actions, flow_mark, netdev);
> >> -
> >> -flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr,
> >> patterns->items,
> >> - &actions, &error);
> >> -
> >> -free_flow_actions(&actions);
> >> -return flow;
> >> -}
> >> -
> >> static void
> >> add_count_action(struct flow_actions *actions)
> >> {
> >> @@ -1509,27 +1485,48 @@ parse_flow_actions(struct netdev *netdev,
> >> return 0;
> >> }
> >>
> >> -static struct rte_flow *
> >> -netdev_offload_dpdk_actions(struct netdev *netdev,
> >> -struct flow_patterns *patterns,
> >> -struct nlattr *nl_actions,
> >> -size_t actions_len)
> >> +static struct ufid_to_rte_flow_data *
> >> +create_netdev_offload(struct netdev *netdev,
> >> + const ovs_u128 *ufid,
> >> + struct flow_patterns *flow_patterns,
> >> + struct flow_actions *flow_actions,
> >> + bool enable_full,
> >> + uint32_t flow_mark)
> >> {
> >> -const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1
> >> };
> >> -struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> >> +struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1, };
> >> +struct flow_actions rss_actions = { .actions = NULL, .cnt = 0 };
> >> +struct rte_flow_item *items = flow_patterns->items;
> >> +struct ufid_to_rte_flow_data *flow_data = NULL;
> >> +bool actions_offloaded = true;
> >> struct rte_flow *flow = NULL;
> >> struct rte_flow_error error;
> >> -int ret;
> >>
> >> -ret = parse_flow_actions(netdev, &actions, nl_actions, actions_len);
> >> -if (ret) {
> >> -goto out;
> >> +if (enable_full) {
> >> +flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
> >> + flow_actions, &error);
> >> }
> >> -flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr,
> >> patterns->items,
> >> - &actions, &error);
> >> -out:
> >> -free_flow_actions(&actions);
> >> -return flow;
> >> +
> >> +if (!flow) {
> >> +/* If we failed to offload the rule actions fallback to MARK+RSS
> >> + * actions.
> >> + */
> > A debug message might be useful here, when we fallback to mark/rss action ?
> We can, but this is just a refactor commit, and this fallback is not
> new. Add it anyway?
I think it'd be useful to add a debug message here and also in
parse_flow_actions() to indicate that action offloading failed.
> >> +actions_offloaded = false;
> >> +flow_attr.transfer = 0;
> >> +add_flow_mark_rss_actions(&rss_actions, flow_mark, netdev);
> >> +flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
> >> + &rss_actions, &error);
> >> +}
> >> +
> >> +if (flow) {
> >> +flow_data = ufid_to_rte_flow_associate(ufid, netdev, flow,
> >> + actions_offloaded);
> >> +VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
> >> + netdev_get_name(netdev), flow,
> >> + UUID_ARGS((struct uuid *) ufid));
> >> +}
> >> +
> >> +free_flow_actions(&rss_actions);
> > This free is needed only when we fallback to mark/rss offload, not
> > otherwise.
> OK.
> >
> >> +return flow_data;
> >>
Re: [ovs-dev] [PATCH V2 11/14] netdev-offload-dpdk: Refactor offload rule creation
On 2/24/2021 11:55 AM, Sriharsha Basavapatna wrote:
On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
Refactor offload rule creation as a pre-step towards tunnel matching
that depend on the netdev.
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
lib/netdev-offload-dpdk.c | 106 --
1 file changed, 45 insertions(+), 61 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 493cc9159..f6e668bff 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -1009,30 +1009,6 @@ add_flow_mark_rss_actions(struct flow_actions *actions,
add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
}
-static struct rte_flow *
-netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
- struct netdev *netdev,
- uint32_t flow_mark)
-{
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
-const struct rte_flow_attr flow_attr = {
-.group = 0,
-.priority = 0,
-.ingress = 1,
-.egress = 0
-};
-struct rte_flow_error error;
-struct rte_flow *flow;
-
-add_flow_mark_rss_actions(&actions, flow_mark, netdev);
-
-flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, patterns->items,
- &actions, &error);
-
-free_flow_actions(&actions);
-return flow;
-}
-
static void
add_count_action(struct flow_actions *actions)
{
@@ -1509,27 +1485,48 @@ parse_flow_actions(struct netdev *netdev,
return 0;
}
-static struct rte_flow *
-netdev_offload_dpdk_actions(struct netdev *netdev,
-struct flow_patterns *patterns,
-struct nlattr *nl_actions,
-size_t actions_len)
+static struct ufid_to_rte_flow_data *
+create_netdev_offload(struct netdev *netdev,
+ const ovs_u128 *ufid,
+ struct flow_patterns *flow_patterns,
+ struct flow_actions *flow_actions,
+ bool enable_full,
+ uint32_t flow_mark)
{
-const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1, };
+struct flow_actions rss_actions = { .actions = NULL, .cnt = 0 };
+struct rte_flow_item *items = flow_patterns->items;
+struct ufid_to_rte_flow_data *flow_data = NULL;
+bool actions_offloaded = true;
struct rte_flow *flow = NULL;
struct rte_flow_error error;
-int ret;
-ret = parse_flow_actions(netdev, &actions, nl_actions, actions_len);
-if (ret) {
-goto out;
+if (enable_full) {
+flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
+ flow_actions, &error);
}
-flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, patterns->items,
- &actions, &error);
-out:
-free_flow_actions(&actions);
-return flow;
+
+if (!flow) {
+/* If we failed to offload the rule actions fallback to MARK+RSS
+ * actions.
+ */
A debug message might be useful here, when we fallback to mark/rss action ?
We can, but this is just a refactor commit, and this fallback is not
new. Add it anyway?
+actions_offloaded = false;
+flow_attr.transfer = 0;
+add_flow_mark_rss_actions(&rss_actions, flow_mark, netdev);
+flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
+ &rss_actions, &error);
+}
+
+if (flow) {
+flow_data = ufid_to_rte_flow_associate(ufid, netdev, flow,
+ actions_offloaded);
+VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
+ netdev_get_name(netdev), flow,
+ UUID_ARGS((struct uuid *) ufid));
+}
+
+free_flow_actions(&rss_actions);
This free is needed only when we fallback to mark/rss offload, not otherwise.
OK.
+return flow_data;
}
static struct ufid_to_rte_flow_data *
@@ -1541,9 +1538,9 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
struct offload_info *info)
{
struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
-struct ufid_to_rte_flow_data *flows_data = NULL;
-bool actions_offloaded = true;
-struct rte_flow *flow;
+struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+struct ufid_to_rte_flow_data *flow_data = NULL;
+int err;
if (parse_flow_match(&patterns, match)) {
VLOG_DBG_RL(&rl, "%s: matches of ufid "UUID_FMT" are not supported",
@@ -1551,28 +1548,15 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
goto out;
}
-flow = netdev_offloa
Re: [ovs-dev] [PATCH V2 11/14] netdev-offload-dpdk: Refactor offload rule creation
On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
>
> Refactor offload rule creation as a pre-step towards tunnel matching
> that depend on the netdev.
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
> ---
> lib/netdev-offload-dpdk.c | 106 --
> 1 file changed, 45 insertions(+), 61 deletions(-)
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 493cc9159..f6e668bff 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -1009,30 +1009,6 @@ add_flow_mark_rss_actions(struct flow_actions *actions,
> add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
> }
>
> -static struct rte_flow *
> -netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
> - struct netdev *netdev,
> - uint32_t flow_mark)
> -{
> -struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> -const struct rte_flow_attr flow_attr = {
> -.group = 0,
> -.priority = 0,
> -.ingress = 1,
> -.egress = 0
> -};
> -struct rte_flow_error error;
> -struct rte_flow *flow;
> -
> -add_flow_mark_rss_actions(&actions, flow_mark, netdev);
> -
> -flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr,
> patterns->items,
> - &actions, &error);
> -
> -free_flow_actions(&actions);
> -return flow;
> -}
> -
> static void
> add_count_action(struct flow_actions *actions)
> {
> @@ -1509,27 +1485,48 @@ parse_flow_actions(struct netdev *netdev,
> return 0;
> }
>
> -static struct rte_flow *
> -netdev_offload_dpdk_actions(struct netdev *netdev,
> -struct flow_patterns *patterns,
> -struct nlattr *nl_actions,
> -size_t actions_len)
> +static struct ufid_to_rte_flow_data *
> +create_netdev_offload(struct netdev *netdev,
> + const ovs_u128 *ufid,
> + struct flow_patterns *flow_patterns,
> + struct flow_actions *flow_actions,
> + bool enable_full,
> + uint32_t flow_mark)
> {
> -const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
> -struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> +struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1, };
> +struct flow_actions rss_actions = { .actions = NULL, .cnt = 0 };
> +struct rte_flow_item *items = flow_patterns->items;
> +struct ufid_to_rte_flow_data *flow_data = NULL;
> +bool actions_offloaded = true;
> struct rte_flow *flow = NULL;
> struct rte_flow_error error;
> -int ret;
>
> -ret = parse_flow_actions(netdev, &actions, nl_actions, actions_len);
> -if (ret) {
> -goto out;
> +if (enable_full) {
> +flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
> + flow_actions, &error);
> }
> -flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr,
> patterns->items,
> - &actions, &error);
> -out:
> -free_flow_actions(&actions);
> -return flow;
> +
> +if (!flow) {
> +/* If we failed to offload the rule actions fallback to MARK+RSS
> + * actions.
> + */
A debug message might be useful here, when we fallback to mark/rss action ?
> +actions_offloaded = false;
> +flow_attr.transfer = 0;
> +add_flow_mark_rss_actions(&rss_actions, flow_mark, netdev);
> +flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
> + &rss_actions, &error);
> +}
> +
> +if (flow) {
> +flow_data = ufid_to_rte_flow_associate(ufid, netdev, flow,
> + actions_offloaded);
> +VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
> + netdev_get_name(netdev), flow,
> + UUID_ARGS((struct uuid *) ufid));
> +}
> +
> +free_flow_actions(&rss_actions);
This free is needed only when we fallback to mark/rss offload, not otherwise.
> +return flow_data;
> }
>
> static struct ufid_to_rte_flow_data *
> @@ -1541,9 +1538,9 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
> struct offload_info *info)
> {
> struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
> -struct ufid_to_rte_flow_data *flows_data = NULL;
> -bool actions_offloaded = true;
> -struct rte_flow *flow;
> +struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> +struct ufid_to_rte_flow_data *flow_data = NULL;
> +int err;
>
> if (parse_flow_match(&patterns, match)) {
> VLOG_DBG_RL(&rl, "%s: matches of ufid "UUID_FMT" are not supported",
> @@ -1551,28 +1548,15 @@ netdev_offload_dpdk_add_flow(struct
[ovs-dev] [PATCH V2 11/14] netdev-offload-dpdk: Refactor offload rule creation
Refactor offload rule creation as a pre-step towards tunnel matching
that depend on the netdev.
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
lib/netdev-offload-dpdk.c | 106 --
1 file changed, 45 insertions(+), 61 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 493cc9159..f6e668bff 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -1009,30 +1009,6 @@ add_flow_mark_rss_actions(struct flow_actions *actions,
add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
}
-static struct rte_flow *
-netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
- struct netdev *netdev,
- uint32_t flow_mark)
-{
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
-const struct rte_flow_attr flow_attr = {
-.group = 0,
-.priority = 0,
-.ingress = 1,
-.egress = 0
-};
-struct rte_flow_error error;
-struct rte_flow *flow;
-
-add_flow_mark_rss_actions(&actions, flow_mark, netdev);
-
-flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, patterns->items,
- &actions, &error);
-
-free_flow_actions(&actions);
-return flow;
-}
-
static void
add_count_action(struct flow_actions *actions)
{
@@ -1509,27 +1485,48 @@ parse_flow_actions(struct netdev *netdev,
return 0;
}
-static struct rte_flow *
-netdev_offload_dpdk_actions(struct netdev *netdev,
-struct flow_patterns *patterns,
-struct nlattr *nl_actions,
-size_t actions_len)
+static struct ufid_to_rte_flow_data *
+create_netdev_offload(struct netdev *netdev,
+ const ovs_u128 *ufid,
+ struct flow_patterns *flow_patterns,
+ struct flow_actions *flow_actions,
+ bool enable_full,
+ uint32_t flow_mark)
{
-const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1, };
+struct flow_actions rss_actions = { .actions = NULL, .cnt = 0 };
+struct rte_flow_item *items = flow_patterns->items;
+struct ufid_to_rte_flow_data *flow_data = NULL;
+bool actions_offloaded = true;
struct rte_flow *flow = NULL;
struct rte_flow_error error;
-int ret;
-ret = parse_flow_actions(netdev, &actions, nl_actions, actions_len);
-if (ret) {
-goto out;
+if (enable_full) {
+flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
+ flow_actions, &error);
}
-flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, patterns->items,
- &actions, &error);
-out:
-free_flow_actions(&actions);
-return flow;
+
+if (!flow) {
+/* If we failed to offload the rule actions fallback to MARK+RSS
+ * actions.
+ */
+actions_offloaded = false;
+flow_attr.transfer = 0;
+add_flow_mark_rss_actions(&rss_actions, flow_mark, netdev);
+flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
+ &rss_actions, &error);
+}
+
+if (flow) {
+flow_data = ufid_to_rte_flow_associate(ufid, netdev, flow,
+ actions_offloaded);
+VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
+ netdev_get_name(netdev), flow,
+ UUID_ARGS((struct uuid *) ufid));
+}
+
+free_flow_actions(&rss_actions);
+return flow_data;
}
static struct ufid_to_rte_flow_data *
@@ -1541,9 +1538,9 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
struct offload_info *info)
{
struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
-struct ufid_to_rte_flow_data *flows_data = NULL;
-bool actions_offloaded = true;
-struct rte_flow *flow;
+struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+struct ufid_to_rte_flow_data *flow_data = NULL;
+int err;
if (parse_flow_match(&patterns, match)) {
VLOG_DBG_RL(&rl, "%s: matches of ufid "UUID_FMT" are not supported",
@@ -1551,28 +1548,15 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
goto out;
}
-flow = netdev_offload_dpdk_actions(netdev, &patterns, nl_actions,
- actions_len);
-if (!flow) {
-/* If we failed to offload the rule actions fallback to MARK+RSS
- * actions.
- */
-flow = netdev_offload_dpdk_mark_rss(&patterns, netdev,
-info->flow_mark);
-actions_offloaded = f
