Re: [ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
Hi Eli, From what has been discussed so far it follows that JUMP from group 0 to the very same group 0 is intentional and corresponds to dp_netdev_recirculate. But could you please clarify, is OvS supposed to always use the group value of 0 with the tunnel offload model? In other words, can PMD expect the OvS to always insert "tunnel set" and "tunnel match" rules at group 0? Can the PMD always expect OvS to use priority level of 0, too? A brief comment in the code would be useful, too. -- Ivan M ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
On 3/2/2021 11:22 AM, Ivan Malov wrote: External email: Use caution opening links or attachments Hi Eli, From what has been discussed so far it follows that JUMP from group 0 to the very same group 0 is intentional and corresponds to dp_netdev_recirculate. But could you please clarify, is OvS supposed to always use the group value of 0 with the tunnel offload model? In other words, can PMD expect the OvS to always insert "tunnel set" and "tunnel match" rules at group 0? Can the PMD always expect OvS to use priority level of 0, too? Hi Ivan, Thanks for your comments. In the proposed series, yes. OVS will always insert tunnel-set and tunnel-match rules in group 0, and in priority 0. However, the PMD and DPDK in general are not OVS specific, and other applications may choose to use in other ways, so the PMD must not assume such assumptions. A brief comment in the code would be useful, too. Yes, I will add it in v3 to be sent soon. -- Ivan M ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
On 3/1/2021 1:30 PM, Sriharsha Basavapatna wrote: The details of this array is under the PMD's responsibility. So this means the PMD has to pick a range of private action values that are not defined in rte_flow.h, but what if later a new action type with the same value is added to rte_flow.h ? The other question is if the PMD could use one of the existing action types in rte_flow.h [i.e, to avoid defining its own private action types] and return it in the opaque action array ? The goal of the API is to be able for each PMD to have its own implementation, private actions or not. For the application, this is opaque, as it doesn't know the details of it. If there is a change in rte_flow.h, it means ABI change in DPDK. DPDK release policy protects us in a sense. Take this example: assume action type 100 is not yet defined in rte_flow.h and a PMD uses this value for a new private action that it defines. Later, if a new standard action type is added to rte_flow.h with the same value, then the PMD has no way to distinguish if the action is a standard action or its private action. Also, this private action type defined by some vendor's PMD could be 100 and it could be 200 in another vendor's PMD. So don't we need to ensure that the standard action types and private action types don't overlap ? One way to handle this might be to reserve a range of values in rte_flow.h as a vendor specific range, for example 100 to 200. And each PMD could define its own private action types within this range, since it is ensured that no standard action types would be defined in that range. I don't know regarding other PMDs, but at least for mlx5, the "private" are negative values, or for unsigned > 2^31, so I don't think they would be a conflict. I suppose DPDK PMDs should follow changes in rte_flow.h. Anyway, it is not related to OVS. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
On Thu, Feb 25, 2021 at 7:50 PM Eli Britstein wrote:
>
>
> On 2/25/2021 9:35 AM, Sriharsha Basavapatna wrote:
> > On Wed, Feb 24, 2021 at 1:50 PM Eli Britstein wrote:
> >>
> >> On 2/24/2021 9:52 AM, Sriharsha Basavapatna wrote:
> >>> On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
> Support tunnel pop action.
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
> ---
> Documentation/howto/dpdk.rst | 1 +
> NEWS | 1 +
> lib/netdev-offload-dpdk.c| 173 ---
> 3 files changed, 160 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index f0d45e47b..4918d80f3 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -398,6 +398,7 @@ Supported actions for hardware offload are:
> - VLAN Push/Pop (push_vlan/pop_vlan).
> - Modification of IPv6
> (set_field:->ipv6_src/ipv6_dst/mod_nw_ttl).
> - Clone/output (tnl_push and output) for encapsulating over a tunnel.
> +- Tunnel pop, for changing from a physical port to a vport.
>
> Further Reading
> ---
> diff --git a/NEWS b/NEWS
> index a7bffce97..6850d5621 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -26,6 +26,7 @@ v2.15.0 - xx xxx
> - DPDK:
> * Removed support for vhost-user dequeue zero-copy.
> * Add support for DPDK 20.11.
> + * Add hardware offload support for tunnel pop action
> (experimental).
> - Userspace datapath:
> * Add the 'pmd' option to "ovs-appctl dpctl/dump-flows", which
> restricts a flow dump to a single PMD thread if set.
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 78f866080..493cc9159 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -140,15 +140,30 @@ struct flow_actions {
> struct rte_flow_action *actions;
> int cnt;
> int current_max;
> +struct netdev *tnl_netdev;
> +/* tnl_actions is the opaque array of actions returned by the PMD.
> */
> +struct rte_flow_action *tnl_actions;
> >>> Why is this an opaque array ? Since it is struct rte_flow_action, OVS
> >>> knows the type and members. Is it opaque because the value of
> >>> rte_flow_action_type member is unknown to OVS ? Is it a private action
> >>> type and if so how does the PMD ensure that it doesn't conflict with
> >>> standard action types ?
> >> True it is not used by OVS, but that's not why it opaque. Although it is
> >> struct rte_flow_action array, the PMD may use its own private actions,
> >> not defined in rte_flow.h, thus not known to the application.
> >>
> >> The details of this array is under the PMD's responsibility.
> > So this means the PMD has to pick a range of private action values
> > that are not defined in rte_flow.h, but what if later a new action
> > type with the same value is added to rte_flow.h ?
> > The other question is if the PMD could use one of the existing action
> > types in rte_flow.h [i.e, to avoid defining its own private action
> > types] and return it in the opaque action array ?
>
> The goal of the API is to be able for each PMD to have its own
> implementation, private actions or not.
>
> For the application, this is opaque, as it doesn't know the details of it.
>
> If there is a change in rte_flow.h, it means ABI change in DPDK. DPDK
> release policy protects us in a sense.
Take this example: assume action type 100 is not yet defined in
rte_flow.h and a PMD uses this value for a new private action that it
defines. Later, if a new standard action type is added to rte_flow.h
with the same value, then the PMD has no way to distinguish if the
action is a standard action or its private action. Also, this private
action type defined by some vendor's PMD could be 100 and it could be
200 in another vendor's PMD. So don't we need to ensure that the
standard action types and private action types don't overlap ? One way
to handle this might be to reserve a range of values in rte_flow.h as
a vendor specific range, for example 100 to 200. And each PMD could
define its own private action types within this range, since it is
ensured that no standard action types would be defined in that range.
>
> >
> +uint32_t num_of_tnl_actions;
> +/* tnl_actions_pos is where the tunnel actions starts within the
> 'actions'
> + * field.
> + */
> +int tnl_actions_pos;
> >>> Names should indicate that they are private or pmd specific ?
> >>>
> >>> tnl_actions --> tnl_private_actions or tnl_pmd_actions
> >>> num_of_tnl_actions --> num_private_tnl_actions or num_pmd_tnl_actions
> >>> tnl_actions_pos --> tnl_private_actions_pos or tnl_pmd_actions_pos
> >>
Re: [ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
On 2/25/2021 9:35 AM, Sriharsha Basavapatna wrote:
On Wed, Feb 24, 2021 at 1:50 PM Eli Britstein wrote:
On 2/24/2021 9:52 AM, Sriharsha Basavapatna wrote:
On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
Support tunnel pop action.
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
Documentation/howto/dpdk.rst | 1 +
NEWS | 1 +
lib/netdev-offload-dpdk.c| 173 ---
3 files changed, 160 insertions(+), 15 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index f0d45e47b..4918d80f3 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -398,6 +398,7 @@ Supported actions for hardware offload are:
- VLAN Push/Pop (push_vlan/pop_vlan).
- Modification of IPv6 (set_field:->ipv6_src/ipv6_dst/mod_nw_ttl).
- Clone/output (tnl_push and output) for encapsulating over a tunnel.
+- Tunnel pop, for changing from a physical port to a vport.
Further Reading
---
diff --git a/NEWS b/NEWS
index a7bffce97..6850d5621 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ v2.15.0 - xx xxx
- DPDK:
* Removed support for vhost-user dequeue zero-copy.
* Add support for DPDK 20.11.
+ * Add hardware offload support for tunnel pop action (experimental).
- Userspace datapath:
* Add the 'pmd' option to "ovs-appctl dpctl/dump-flows", which
restricts a flow dump to a single PMD thread if set.
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 78f866080..493cc9159 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -140,15 +140,30 @@ struct flow_actions {
struct rte_flow_action *actions;
int cnt;
int current_max;
+struct netdev *tnl_netdev;
+/* tnl_actions is the opaque array of actions returned by the PMD. */
+struct rte_flow_action *tnl_actions;
Why is this an opaque array ? Since it is struct rte_flow_action, OVS
knows the type and members. Is it opaque because the value of
rte_flow_action_type member is unknown to OVS ? Is it a private action
type and if so how does the PMD ensure that it doesn't conflict with
standard action types ?
True it is not used by OVS, but that's not why it opaque. Although it is
struct rte_flow_action array, the PMD may use its own private actions,
not defined in rte_flow.h, thus not known to the application.
The details of this array is under the PMD's responsibility.
So this means the PMD has to pick a range of private action values
that are not defined in rte_flow.h, but what if later a new action
type with the same value is added to rte_flow.h ?
The other question is if the PMD could use one of the existing action
types in rte_flow.h [i.e, to avoid defining its own private action
types] and return it in the opaque action array ?
The goal of the API is to be able for each PMD to have its own
implementation, private actions or not.
For the application, this is opaque, as it doesn't know the details of it.
If there is a change in rte_flow.h, it means ABI change in DPDK. DPDK
release policy protects us in a sense.
+uint32_t num_of_tnl_actions;
+/* tnl_actions_pos is where the tunnel actions starts within the 'actions'
+ * field.
+ */
+int tnl_actions_pos;
Names should indicate that they are private or pmd specific ?
tnl_actions --> tnl_private_actions or tnl_pmd_actions
num_of_tnl_actions --> num_private_tnl_actions or num_pmd_tnl_actions
tnl_actions_pos --> tnl_private_actions_pos or tnl_pmd_actions_pos
OK. _pmd_
+struct ds s_tnl;
};
static void
-dump_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
+dump_flow_attr(struct ds *s, struct ds *s_extra,
+ const struct rte_flow_attr *attr,
+ struct flow_actions *flow_actions)
{
-ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s",
+if (flow_actions->num_of_tnl_actions) {
+ds_clone(s_extra, &flow_actions->s_tnl);
+}
+ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s%s",
attr->ingress ? "ingress " : "",
attr->egress ? "egress " : "", attr->priority,
attr->group,
- attr->transfer ? "transfer " : "");
+ attr->transfer ? "transfer " : "",
+ flow_actions->num_of_tnl_actions ? "tunnel_set 1 " : "");
}
/* Adds one pattern item 'field' with the 'mask' to dynamic string 's' using
@@ -395,9 +410,19 @@ dump_vxlan_encap(struct ds *s, const struct rte_flow_item
*items)
static void
dump_flow_action(struct ds *s, struct ds *s_extra,
- const struct rte_flow_action *actions)
+ struct flow_actions *flow_actions, int act_index)
{
-if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
+const struct rte_flow_action *actions = &flow_actions->actions[act_index];
+
+if (actions->type == RTE_FLOW_ACTION
Re: [ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
On Wed, Feb 24, 2021 at 1:50 PM Eli Britstein wrote:
>
>
> On 2/24/2021 9:52 AM, Sriharsha Basavapatna wrote:
> > On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
> >> Support tunnel pop action.
> >>
> >> Signed-off-by: Eli Britstein
> >> Reviewed-by: Gaetan Rivet
> >> ---
> >> Documentation/howto/dpdk.rst | 1 +
> >> NEWS | 1 +
> >> lib/netdev-offload-dpdk.c| 173 ---
> >> 3 files changed, 160 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> >> index f0d45e47b..4918d80f3 100644
> >> --- a/Documentation/howto/dpdk.rst
> >> +++ b/Documentation/howto/dpdk.rst
> >> @@ -398,6 +398,7 @@ Supported actions for hardware offload are:
> >> - VLAN Push/Pop (push_vlan/pop_vlan).
> >> - Modification of IPv6 (set_field:->ipv6_src/ipv6_dst/mod_nw_ttl).
> >> - Clone/output (tnl_push and output) for encapsulating over a tunnel.
> >> +- Tunnel pop, for changing from a physical port to a vport.
> >>
> >> Further Reading
> >> ---
> >> diff --git a/NEWS b/NEWS
> >> index a7bffce97..6850d5621 100644
> >> --- a/NEWS
> >> +++ b/NEWS
> >> @@ -26,6 +26,7 @@ v2.15.0 - xx xxx
> >> - DPDK:
> >>* Removed support for vhost-user dequeue zero-copy.
> >>* Add support for DPDK 20.11.
> >> + * Add hardware offload support for tunnel pop action (experimental).
> >> - Userspace datapath:
> >>* Add the 'pmd' option to "ovs-appctl dpctl/dump-flows", which
> >> restricts a flow dump to a single PMD thread if set.
> >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >> index 78f866080..493cc9159 100644
> >> --- a/lib/netdev-offload-dpdk.c
> >> +++ b/lib/netdev-offload-dpdk.c
> >> @@ -140,15 +140,30 @@ struct flow_actions {
> >> struct rte_flow_action *actions;
> >> int cnt;
> >> int current_max;
> >> +struct netdev *tnl_netdev;
> >> +/* tnl_actions is the opaque array of actions returned by the PMD. */
> >> +struct rte_flow_action *tnl_actions;
> > Why is this an opaque array ? Since it is struct rte_flow_action, OVS
> > knows the type and members. Is it opaque because the value of
> > rte_flow_action_type member is unknown to OVS ? Is it a private action
> > type and if so how does the PMD ensure that it doesn't conflict with
> > standard action types ?
>
> True it is not used by OVS, but that's not why it opaque. Although it is
> struct rte_flow_action array, the PMD may use its own private actions,
> not defined in rte_flow.h, thus not known to the application.
>
> The details of this array is under the PMD's responsibility.
So this means the PMD has to pick a range of private action values
that are not defined in rte_flow.h, but what if later a new action
type with the same value is added to rte_flow.h ?
The other question is if the PMD could use one of the existing action
types in rte_flow.h [i.e, to avoid defining its own private action
types] and return it in the opaque action array ?
>
> >
> >> +uint32_t num_of_tnl_actions;
> >> +/* tnl_actions_pos is where the tunnel actions starts within the
> >> 'actions'
> >> + * field.
> >> + */
> >> +int tnl_actions_pos;
> > Names should indicate that they are private or pmd specific ?
> >
> > tnl_actions --> tnl_private_actions or tnl_pmd_actions
> > num_of_tnl_actions --> num_private_tnl_actions or num_pmd_tnl_actions
> > tnl_actions_pos --> tnl_private_actions_pos or tnl_pmd_actions_pos
> OK. _pmd_
> >
> >> +struct ds s_tnl;
> >> };
> >>
> >> static void
> >> -dump_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
> >> +dump_flow_attr(struct ds *s, struct ds *s_extra,
> >> + const struct rte_flow_attr *attr,
> >> + struct flow_actions *flow_actions)
> >> {
> >> -ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s",
> >> +if (flow_actions->num_of_tnl_actions) {
> >> +ds_clone(s_extra, &flow_actions->s_tnl);
> >> +}
> >> +ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s%s",
> >> attr->ingress ? "ingress " : "",
> >> attr->egress ? "egress " : "", attr->priority,
> >> attr->group,
> >> - attr->transfer ? "transfer " : "");
> >> + attr->transfer ? "transfer " : "",
> >> + flow_actions->num_of_tnl_actions ? "tunnel_set 1 " :
> >> "");
> >> }
> >>
> >> /* Adds one pattern item 'field' with the 'mask' to dynamic string 's'
> >> using
> >> @@ -395,9 +410,19 @@ dump_vxlan_encap(struct ds *s, const struct
> >> rte_flow_item *items)
> >>
> >> static void
> >> dump_flow_action(struct ds *s, struct ds *s_extra,
> >> - const struct rte_flow_action *actions)
> >> + struct flow_actions *flow_actions, int act_index)
> >> {
> >> -if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
> >> +const str
Re: [ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
On 2/24/2021 9:52 AM, Sriharsha Basavapatna wrote:
On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
Support tunnel pop action.
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
Documentation/howto/dpdk.rst | 1 +
NEWS | 1 +
lib/netdev-offload-dpdk.c| 173 ---
3 files changed, 160 insertions(+), 15 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index f0d45e47b..4918d80f3 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -398,6 +398,7 @@ Supported actions for hardware offload are:
- VLAN Push/Pop (push_vlan/pop_vlan).
- Modification of IPv6 (set_field:->ipv6_src/ipv6_dst/mod_nw_ttl).
- Clone/output (tnl_push and output) for encapsulating over a tunnel.
+- Tunnel pop, for changing from a physical port to a vport.
Further Reading
---
diff --git a/NEWS b/NEWS
index a7bffce97..6850d5621 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ v2.15.0 - xx xxx
- DPDK:
* Removed support for vhost-user dequeue zero-copy.
* Add support for DPDK 20.11.
+ * Add hardware offload support for tunnel pop action (experimental).
- Userspace datapath:
* Add the 'pmd' option to "ovs-appctl dpctl/dump-flows", which
restricts a flow dump to a single PMD thread if set.
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 78f866080..493cc9159 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -140,15 +140,30 @@ struct flow_actions {
struct rte_flow_action *actions;
int cnt;
int current_max;
+struct netdev *tnl_netdev;
+/* tnl_actions is the opaque array of actions returned by the PMD. */
+struct rte_flow_action *tnl_actions;
Why is this an opaque array ? Since it is struct rte_flow_action, OVS
knows the type and members. Is it opaque because the value of
rte_flow_action_type member is unknown to OVS ? Is it a private action
type and if so how does the PMD ensure that it doesn't conflict with
standard action types ?
True it is not used by OVS, but that's not why it opaque. Although it is
struct rte_flow_action array, the PMD may use its own private actions,
not defined in rte_flow.h, thus not known to the application.
The details of this array is under the PMD's responsibility.
+uint32_t num_of_tnl_actions;
+/* tnl_actions_pos is where the tunnel actions starts within the 'actions'
+ * field.
+ */
+int tnl_actions_pos;
Names should indicate that they are private or pmd specific ?
tnl_actions --> tnl_private_actions or tnl_pmd_actions
num_of_tnl_actions --> num_private_tnl_actions or num_pmd_tnl_actions
tnl_actions_pos --> tnl_private_actions_pos or tnl_pmd_actions_pos
OK. _pmd_
+struct ds s_tnl;
};
static void
-dump_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
+dump_flow_attr(struct ds *s, struct ds *s_extra,
+ const struct rte_flow_attr *attr,
+ struct flow_actions *flow_actions)
{
-ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s",
+if (flow_actions->num_of_tnl_actions) {
+ds_clone(s_extra, &flow_actions->s_tnl);
+}
+ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s%s",
attr->ingress ? "ingress " : "",
attr->egress ? "egress " : "", attr->priority,
attr->group,
- attr->transfer ? "transfer " : "");
+ attr->transfer ? "transfer " : "",
+ flow_actions->num_of_tnl_actions ? "tunnel_set 1 " : "");
}
/* Adds one pattern item 'field' with the 'mask' to dynamic string 's' using
@@ -395,9 +410,19 @@ dump_vxlan_encap(struct ds *s, const struct rte_flow_item
*items)
static void
dump_flow_action(struct ds *s, struct ds *s_extra,
- const struct rte_flow_action *actions)
+ struct flow_actions *flow_actions, int act_index)
{
-if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
+const struct rte_flow_action *actions = &flow_actions->actions[act_index];
+
+if (actions->type == RTE_FLOW_ACTION_TYPE_END) {
+ds_put_cstr(s, "end");
+} else if (flow_actions->num_of_tnl_actions &&
+ act_index >= flow_actions->tnl_actions_pos &&
+ act_index < flow_actions->tnl_actions_pos +
+ flow_actions->num_of_tnl_actions) {
+/* Opaque PMD tunnel actions is skipped. */
Wouldn't it be useful to at least print the value of PMD action types ?
The only way we can print them as raw numbers. I see little added value
for this on one hand, but on the other hand it will defect the testpmd
format print, so I think better to avoid.
+return;
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
const struct rte_flow_action_mark *mark = actions->conf;
ds_put_cstr(s, "mark ");
@@ -528,6 +553,14
Re: [ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein wrote:
>
> Support tunnel pop action.
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
> ---
> Documentation/howto/dpdk.rst | 1 +
> NEWS | 1 +
> lib/netdev-offload-dpdk.c| 173 ---
> 3 files changed, 160 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index f0d45e47b..4918d80f3 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -398,6 +398,7 @@ Supported actions for hardware offload are:
> - VLAN Push/Pop (push_vlan/pop_vlan).
> - Modification of IPv6 (set_field:->ipv6_src/ipv6_dst/mod_nw_ttl).
> - Clone/output (tnl_push and output) for encapsulating over a tunnel.
> +- Tunnel pop, for changing from a physical port to a vport.
>
> Further Reading
> ---
> diff --git a/NEWS b/NEWS
> index a7bffce97..6850d5621 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -26,6 +26,7 @@ v2.15.0 - xx xxx
> - DPDK:
> * Removed support for vhost-user dequeue zero-copy.
> * Add support for DPDK 20.11.
> + * Add hardware offload support for tunnel pop action (experimental).
> - Userspace datapath:
> * Add the 'pmd' option to "ovs-appctl dpctl/dump-flows", which
> restricts a flow dump to a single PMD thread if set.
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 78f866080..493cc9159 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -140,15 +140,30 @@ struct flow_actions {
> struct rte_flow_action *actions;
> int cnt;
> int current_max;
> +struct netdev *tnl_netdev;
> +/* tnl_actions is the opaque array of actions returned by the PMD. */
> +struct rte_flow_action *tnl_actions;
Why is this an opaque array ? Since it is struct rte_flow_action, OVS
knows the type and members. Is it opaque because the value of
rte_flow_action_type member is unknown to OVS ? Is it a private action
type and if so how does the PMD ensure that it doesn't conflict with
standard action types ?
> +uint32_t num_of_tnl_actions;
> +/* tnl_actions_pos is where the tunnel actions starts within the
> 'actions'
> + * field.
> + */
> +int tnl_actions_pos;
Names should indicate that they are private or pmd specific ?
tnl_actions --> tnl_private_actions or tnl_pmd_actions
num_of_tnl_actions --> num_private_tnl_actions or num_pmd_tnl_actions
tnl_actions_pos --> tnl_private_actions_pos or tnl_pmd_actions_pos
> +struct ds s_tnl;
> };
>
> static void
> -dump_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
> +dump_flow_attr(struct ds *s, struct ds *s_extra,
> + const struct rte_flow_attr *attr,
> + struct flow_actions *flow_actions)
> {
> -ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s",
> +if (flow_actions->num_of_tnl_actions) {
> +ds_clone(s_extra, &flow_actions->s_tnl);
> +}
> +ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s%s",
>attr->ingress ? "ingress " : "",
>attr->egress ? "egress " : "", attr->priority,
> attr->group,
> - attr->transfer ? "transfer " : "");
> + attr->transfer ? "transfer " : "",
> + flow_actions->num_of_tnl_actions ? "tunnel_set 1 " : "");
> }
>
> /* Adds one pattern item 'field' with the 'mask' to dynamic string 's' using
> @@ -395,9 +410,19 @@ dump_vxlan_encap(struct ds *s, const struct
> rte_flow_item *items)
>
> static void
> dump_flow_action(struct ds *s, struct ds *s_extra,
> - const struct rte_flow_action *actions)
> + struct flow_actions *flow_actions, int act_index)
> {
> -if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
> +const struct rte_flow_action *actions =
> &flow_actions->actions[act_index];
> +
> +if (actions->type == RTE_FLOW_ACTION_TYPE_END) {
> +ds_put_cstr(s, "end");
> +} else if (flow_actions->num_of_tnl_actions &&
> + act_index >= flow_actions->tnl_actions_pos &&
> + act_index < flow_actions->tnl_actions_pos +
> + flow_actions->num_of_tnl_actions) {
> +/* Opaque PMD tunnel actions is skipped. */
Wouldn't it be useful to at least print the value of PMD action types ?
> +return;
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
> const struct rte_flow_action_mark *mark = actions->conf;
>
> ds_put_cstr(s, "mark ");
> @@ -528,6 +553,14 @@ dump_flow_action(struct ds *s, struct ds *s_extra,
> ds_put_cstr(s, "vxlan_encap / ");
> dump_vxlan_encap(s_extra, items);
> ds_put_cstr(s_extra, ";");
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_JUMP) {
> +const struct rte_flow_action_jump *jump = actions->conf;
> +
> +ds_put_cstr(s, "jump ");
> +if
[ovs-dev] [PATCH V2 10/14] netdev-offload-dpdk: Support tunnel pop action
Support tunnel pop action.
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
Documentation/howto/dpdk.rst | 1 +
NEWS | 1 +
lib/netdev-offload-dpdk.c| 173 ---
3 files changed, 160 insertions(+), 15 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index f0d45e47b..4918d80f3 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -398,6 +398,7 @@ Supported actions for hardware offload are:
- VLAN Push/Pop (push_vlan/pop_vlan).
- Modification of IPv6 (set_field:->ipv6_src/ipv6_dst/mod_nw_ttl).
- Clone/output (tnl_push and output) for encapsulating over a tunnel.
+- Tunnel pop, for changing from a physical port to a vport.
Further Reading
---
diff --git a/NEWS b/NEWS
index a7bffce97..6850d5621 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ v2.15.0 - xx xxx
- DPDK:
* Removed support for vhost-user dequeue zero-copy.
* Add support for DPDK 20.11.
+ * Add hardware offload support for tunnel pop action (experimental).
- Userspace datapath:
* Add the 'pmd' option to "ovs-appctl dpctl/dump-flows", which
restricts a flow dump to a single PMD thread if set.
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 78f866080..493cc9159 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -140,15 +140,30 @@ struct flow_actions {
struct rte_flow_action *actions;
int cnt;
int current_max;
+struct netdev *tnl_netdev;
+/* tnl_actions is the opaque array of actions returned by the PMD. */
+struct rte_flow_action *tnl_actions;
+uint32_t num_of_tnl_actions;
+/* tnl_actions_pos is where the tunnel actions starts within the 'actions'
+ * field.
+ */
+int tnl_actions_pos;
+struct ds s_tnl;
};
static void
-dump_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
+dump_flow_attr(struct ds *s, struct ds *s_extra,
+ const struct rte_flow_attr *attr,
+ struct flow_actions *flow_actions)
{
-ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s",
+if (flow_actions->num_of_tnl_actions) {
+ds_clone(s_extra, &flow_actions->s_tnl);
+}
+ds_put_format(s, "%s%spriority %"PRIu32" group %"PRIu32" %s%s",
attr->ingress ? "ingress " : "",
attr->egress ? "egress " : "", attr->priority, attr->group,
- attr->transfer ? "transfer " : "");
+ attr->transfer ? "transfer " : "",
+ flow_actions->num_of_tnl_actions ? "tunnel_set 1 " : "");
}
/* Adds one pattern item 'field' with the 'mask' to dynamic string 's' using
@@ -395,9 +410,19 @@ dump_vxlan_encap(struct ds *s, const struct rte_flow_item
*items)
static void
dump_flow_action(struct ds *s, struct ds *s_extra,
- const struct rte_flow_action *actions)
+ struct flow_actions *flow_actions, int act_index)
{
-if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
+const struct rte_flow_action *actions = &flow_actions->actions[act_index];
+
+if (actions->type == RTE_FLOW_ACTION_TYPE_END) {
+ds_put_cstr(s, "end");
+} else if (flow_actions->num_of_tnl_actions &&
+ act_index >= flow_actions->tnl_actions_pos &&
+ act_index < flow_actions->tnl_actions_pos +
+ flow_actions->num_of_tnl_actions) {
+/* Opaque PMD tunnel actions is skipped. */
+return;
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
const struct rte_flow_action_mark *mark = actions->conf;
ds_put_cstr(s, "mark ");
@@ -528,6 +553,14 @@ dump_flow_action(struct ds *s, struct ds *s_extra,
ds_put_cstr(s, "vxlan_encap / ");
dump_vxlan_encap(s_extra, items);
ds_put_cstr(s_extra, ";");
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_JUMP) {
+const struct rte_flow_action_jump *jump = actions->conf;
+
+ds_put_cstr(s, "jump ");
+if (jump) {
+ds_put_format(s, "group %"PRIu32" ", jump->group);
+}
+ds_put_cstr(s, "/ ");
} else {
ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
}
@@ -537,20 +570,21 @@ static struct ds *
dump_flow(struct ds *s, struct ds *s_extra,
const struct rte_flow_attr *attr,
const struct rte_flow_item *items,
- const struct rte_flow_action *actions)
+ struct flow_actions *flow_actions)
{
+int i;
+
if (attr) {
-dump_flow_attr(s, attr);
+dump_flow_attr(s, s_extra, attr, flow_actions);
}
ds_put_cstr(s, "pattern ");
while (items && items->type != RTE_FLOW_ITEM_TYPE_END) {
dump_flow_pattern(s, items++);
}
ds_put_cstr(s, "end actions ");
-while (actions && actions->type != RTE_FLOW_ACTION_TYPE_END) {
-dump_flow_action(
