Re: [ovs-dev] [PATCH 14/25] netdev-offload-dpdk: Implement HW miss packet recover for vport
On 7/11/2020 9:36 PM, William Tu wrote:
Hi Eli,
Thanks for the patch, very interesting work.
I'm trying to understand the patch. some questions below:
On Mon, Jan 20, 2020 at 7:09 AM Eli Britstein wrote:
A miss in virtual port offloads means the flow with tnl_pop was
offloaded, but not the following one. Recover the state and continue
with SW processing.
Why do we have a miss in virtual port offloads? Isn't we only
offloading to physical port or uplink?
at patch 25/25, you mentioned
"For virtual port (as "vxlan"), HW rules match tunnel properties
(outer header) and inner packet fields, and with a decap action. The
rules are placed on all uplinks as they are the potential for the origin
of the traffic."
Yes, the offloads are only for physical ports. Regarding misses, please
see below.
Co-authored-by: Eli Britstein
Signed-off-by: Ophir Munk
Reviewed-by: Roni Bar Yanai
Signed-off-by: Eli Britstein
---
lib/netdev-offload-dpdk.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index fc890b915..c4d77c115 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -385,7 +385,6 @@ put_flow_miss_ctx_id(uint32_t flow_ctx_id)
put_context_data_by_id(_miss_ctx_md, flow_ctx_id);
}
-OVS_UNUSED
static int
find_flow_miss_ctx(int flow_ctx_id, struct flow_miss_ctx *ctx)
{
@@ -1769,10 +1768,43 @@ out:
return ret;
}
+static int
+netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev,
+ uint32_t flow_miss_ctx_id,
+ struct dp_packet *packet)
+{
+struct flow_miss_ctx flow_miss_ctx;
+struct netdev *vport_netdev;
+
+if (find_flow_miss_ctx(flow_miss_ctx_id, _miss_ctx)) {
+return -1;
+}
+
+if (flow_miss_ctx.vport != ODPP_NONE) {
+vport_netdev = netdev_ports_get(flow_miss_ctx.vport,
+netdev->dpif_type);
+if (vport_netdev) {
+pkt_metadata_init(>md, flow_miss_ctx.vport);
+if (vport_netdev->netdev_class->pop_header) {
+vport_netdev->netdev_class->pop_header(packet);
IIUC, we need to pop header here because now we translate
tnl_pop to mark + jump.
So the outer tunnel header does not get pop/unset in hardware,
so at SW side before upcall to OVS, we need to pop here, right?
The HW offload flows follow the SW model. Once we have more than one
flow to process a packet in HW, we are exposed to misses, in case the
packet doesn't complete its full path in HW, but only part of it. In
this case, the first flow (the classification flow) is hit, but the
vport's one may not exist. We should recover the packet as if it was
processed in SW from the beginning, to maintain correctness of OVS. For
example counters - the HW already counted in the first flow. We should
not count it again in SW.
Please note that we would like to progress with an approach to enhance
DPDK and put some of the logic inside the PMD. See:
Link to RFC: http://mails.dpdk.org/archives/dev/2020-June/169656.html
Link to patchset:
http://mails.dpdk.org/archives/dev/2020-June/171590.html
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 14/25] netdev-offload-dpdk: Implement HW miss packet recover for vport
Hi Eli,
Thanks for the patch, very interesting work.
I'm trying to understand the patch. some questions below:
On Mon, Jan 20, 2020 at 7:09 AM Eli Britstein wrote:
>
> A miss in virtual port offloads means the flow with tnl_pop was
> offloaded, but not the following one. Recover the state and continue
> with SW processing.
Why do we have a miss in virtual port offloads? Isn't we only
offloading to physical port or uplink?
at patch 25/25, you mentioned
"For virtual port (as "vxlan"), HW rules match tunnel properties
(outer header) and inner packet fields, and with a decap action. The
rules are placed on all uplinks as they are the potential for the origin
of the traffic."
>
> Co-authored-by: Eli Britstein
> Signed-off-by: Ophir Munk
> Reviewed-by: Roni Bar Yanai
> Signed-off-by: Eli Britstein
> ---
> lib/netdev-offload-dpdk.c | 34 +-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index fc890b915..c4d77c115 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -385,7 +385,6 @@ put_flow_miss_ctx_id(uint32_t flow_ctx_id)
> put_context_data_by_id(_miss_ctx_md, flow_ctx_id);
> }
>
> -OVS_UNUSED
> static int
> find_flow_miss_ctx(int flow_ctx_id, struct flow_miss_ctx *ctx)
> {
> @@ -1769,10 +1768,43 @@ out:
> return ret;
> }
>
> +static int
> +netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev,
> + uint32_t flow_miss_ctx_id,
> + struct dp_packet *packet)
> +{
> +struct flow_miss_ctx flow_miss_ctx;
> +struct netdev *vport_netdev;
> +
> +if (find_flow_miss_ctx(flow_miss_ctx_id, _miss_ctx)) {
> +return -1;
> +}
> +
> +if (flow_miss_ctx.vport != ODPP_NONE) {
> +vport_netdev = netdev_ports_get(flow_miss_ctx.vport,
> +netdev->dpif_type);
> +if (vport_netdev) {
> +pkt_metadata_init(>md, flow_miss_ctx.vport);
> +if (vport_netdev->netdev_class->pop_header) {
> +vport_netdev->netdev_class->pop_header(packet);
IIUC, we need to pop header here because now we translate
tnl_pop to mark + jump.
So the outer tunnel header does not get pop/unset in hardware,
so at SW side before upcall to OVS, we need to pop here, right?
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 14/25] netdev-offload-dpdk: Implement HW miss packet recover for vport
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. checkpatch: ERROR: Author should not be also be co-author. Lines checked: 76, Warnings: 0, Errors: 1 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
[ovs-dev] [PATCH 14/25] netdev-offload-dpdk: Implement HW miss packet recover for vport
A miss in virtual port offloads means the flow with tnl_pop was
offloaded, but not the following one. Recover the state and continue
with SW processing.
Co-authored-by: Eli Britstein
Signed-off-by: Ophir Munk
Reviewed-by: Roni Bar Yanai
Signed-off-by: Eli Britstein
---
lib/netdev-offload-dpdk.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index fc890b915..c4d77c115 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -385,7 +385,6 @@ put_flow_miss_ctx_id(uint32_t flow_ctx_id)
put_context_data_by_id(_miss_ctx_md, flow_ctx_id);
}
-OVS_UNUSED
static int
find_flow_miss_ctx(int flow_ctx_id, struct flow_miss_ctx *ctx)
{
@@ -1769,10 +1768,43 @@ out:
return ret;
}
+static int
+netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev,
+ uint32_t flow_miss_ctx_id,
+ struct dp_packet *packet)
+{
+struct flow_miss_ctx flow_miss_ctx;
+struct netdev *vport_netdev;
+
+if (find_flow_miss_ctx(flow_miss_ctx_id, _miss_ctx)) {
+return -1;
+}
+
+if (flow_miss_ctx.vport != ODPP_NONE) {
+vport_netdev = netdev_ports_get(flow_miss_ctx.vport,
+netdev->dpif_type);
+if (vport_netdev) {
+pkt_metadata_init(>md, flow_miss_ctx.vport);
+if (vport_netdev->netdev_class->pop_header) {
+vport_netdev->netdev_class->pop_header(packet);
+dp_packet_reset_offload(packet);
+packet->md.in_port.odp_port = flow_miss_ctx.vport;
+} else {
+VLOG_ERR("vport nedtdev=%s with no pop_header method",
+ netdev_get_name(vport_netdev));
+}
+netdev_close(vport_netdev);
+}
+}
+
+return 0;
+}
+
const struct netdev_flow_api netdev_offload_dpdk = {
.type = "dpdk_flow_api",
.flow_put = netdev_offload_dpdk_flow_put,
.flow_del = netdev_offload_dpdk_flow_del,
.init_flow_api = netdev_offload_dpdk_init_flow_api,
.flow_get = netdev_offload_dpdk_flow_get,
+.hw_miss_packet_recover = netdev_offload_dpdk_hw_miss_packet_recover,
};
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
