Re: [ovs-dev] [PATCH V4 05/14] netdev-offload-dpdk: Implement HW miss packet recover for vport
On Wed, Mar 17, 2021 at 12:05 PM 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.
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
> ---
> lib/netdev-offload-dpdk.c | 150 ++
> 1 file changed, 150 insertions(+)
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index f2413f5be..c78089605 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -1588,6 +1588,155 @@ netdev_offload_dpdk_flow_flush(struct netdev *netdev)
> return 0;
> }
>
> +struct get_vport_netdev_aux {
> +struct rte_flow_tunnel *tunnel;
> +odp_port_t *odp_port;
> +struct netdev *vport;
> +};
> +
> +static bool
> +get_vxlan_netdev_cb(struct netdev *netdev,
> +odp_port_t odp_port,
> +void *aux_)
> +{
> +const struct netdev_tunnel_config *tnl_cfg;
> +struct get_vport_netdev_aux *aux = aux_;
> +
> +if (strcmp(netdev_get_type(netdev), "vxlan")) {
> + return false;
> +}
> +
> +tnl_cfg = netdev_get_tunnel_config(netdev);
> +if (!tnl_cfg) {
> +VLOG_ERR_RL(&rl, "Cannot get a tunnel config for netdev %s",
> +netdev_get_name(netdev));
> +return false;
> +}
> +
> +if (tnl_cfg->dst_port == aux->tunnel->tp_dst) {
> +/* Found the netdev. Store the results and stop the traversing. */
> +aux->vport = netdev_ref(netdev);
> +*aux->odp_port = odp_port;
> +return true;
> +}
> +
> +return false;
> +}
> +
> +static struct netdev *
> +get_vxlan_netdev(const char *dpif_type,
> + struct rte_flow_tunnel *tunnel,
> + odp_port_t *odp_port)
> +{
> +struct get_vport_netdev_aux aux = {
> +.tunnel = tunnel,
> +.odp_port = odp_port,
> +.vport = NULL,
> +};
> +
> +netdev_ports_traverse(dpif_type, get_vxlan_netdev_cb, &aux);
> +return aux.vport;
> +}
> +
> +static struct netdev *
> +get_vport_netdev(const char *dpif_type,
> + struct rte_flow_tunnel *tunnel,
> + odp_port_t *odp_port)
> +{
> +if (tunnel->type == RTE_FLOW_ITEM_TYPE_VXLAN) {
> +return get_vxlan_netdev(dpif_type, tunnel, odp_port);
> +}
> +
> +OVS_NOT_REACHED();
> +}
> +
> +static int
> +netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev,
> + struct dp_packet *packet)
> +{
> +struct rte_flow_restore_info rte_restore_info;
> +struct rte_flow_tunnel *rte_tnl;
> +struct netdev *vport_netdev;
> +struct rte_flow_error error;
> +struct pkt_metadata *md;
> +struct flow_tnl *md_tnl;
> +odp_port_t vport_odp;
> +int ret = 0;
> +
> +if (netdev_dpdk_rte_flow_get_restore_info(netdev, packet,
> + &rte_restore_info, &error)) {
> +/* This function is called for every packet, and in most cases there
> + * will be no restore info from the HW, thus error is expected.
> + */
> +(void) error;
> +return 0;
> +}
> +
> +if (!(rte_restore_info.flags & RTE_FLOW_RESTORE_INFO_TUNNEL)) {
> +return EOPNOTSUPP;
> +}
> +
> +rte_tnl = &rte_restore_info.tunnel;
> +vport_netdev = get_vport_netdev(netdev->dpif_type, rte_tnl,
> +&vport_odp);
> +if (!vport_netdev) {
> +VLOG_WARN("Could not find vport netdev");
> +return EOPNOTSUPP;
> +}
> +
> +md = &packet->md;
> +/* For tunnel recovery (RTE_FLOW_RESTORE_INFO_TUNNEL), it is possible
> + * to have the packet to still be encapsulated, or not
> + * (RTE_FLOW_RESTORE_INFO_ENCAPSULATED).
> + * In the case it is on, the packet is still encapsulated, and we do
> + * the pop in SW.
> + * In the case it is off, the packet is already decapsulated by HW, and
> + * the tunnel info is provided in the tunnel struct. For this case we
> + * take it to OVS metadata.
> + */
> +if (rte_restore_info.flags & RTE_FLOW_RESTORE_INFO_ENCAPSULATED) {
> +if (!vport_netdev->netdev_class ||
> +!vport_netdev->netdev_class->pop_header) {
> +VLOG_ERR("vport nedtdev=%s with no pop_header method",
> + netdev_get_name(vport_netdev));
> +ret = EOPNOTSUPP;
> +goto close_vport_netdev;
> +}
> +parse_tcp_flags(packet);
> +if (vport_netdev->netdev_class->pop_header(packet) == NULL) {
> +/* If there is an error with popping the header, the packet is
> + * freed. In this case it should not continue SW processing.
> + */
> +ret = -1;
> +goto close_vport_netdev;
> +}
> +} else {
> +md_tnl = &md->tunnel;
> +if (rte_tnl->is_ipv6) {
> +
Re: [ovs-dev] [PATCH V4 05/14] netdev-offload-dpdk: Implement HW miss packet recover for vport
On 3/17/2021 10:48 AM, Ivan Malov wrote:
External email: Use caution opening links or attachments
Hi Eli,
On 17/03/2021 09:35, Eli Britstein wrote:
+ parse_tcp_flags(packet);
+ if (vport_netdev->netdev_class->pop_header(packet) == NULL) {
Thank you for revising the patch series. As far as I can see, in the new
revision (patch [06/14]), parsing TCP flags is done after successful
miss recovery (which yields a decapsulated packet), and that should be
fairly correct. However, why also call parse_tcp_flags() over here,
before popping the header? This invocation doesn't make use of the
returned value...
Yes, we don't need the returned value here, but parse_tcp_flags also
sets the packet layer offsets.
In the pop_header function, for example netdev_vxlan_pop_header in
lib/netdev-native-tnl.c, it needs the offsets to be valid:
ovs_assert(packet->l3_ofs > 0);
ovs_assert(packet->l4_ofs > 0);
(Sorry if I simply misread the code).
--
Ivan M
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V4 05/14] netdev-offload-dpdk: Implement HW miss packet recover for vport
Hi Eli,
On 17/03/2021 09:35, Eli Britstein wrote:
+parse_tcp_flags(packet);
+if (vport_netdev->netdev_class->pop_header(packet) == NULL) {
Thank you for revising the patch series. As far as I can see, in the new
revision (patch [06/14]), parsing TCP flags is done after successful
miss recovery (which yields a decapsulated packet), and that should be
fairly correct. However, why also call parse_tcp_flags() over here,
before popping the header? This invocation doesn't make use of the
returned value...
(Sorry if I simply misread the code).
--
Ivan M
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V4 05/14] 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.
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
lib/netdev-offload-dpdk.c | 150 ++
1 file changed, 150 insertions(+)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index f2413f5be..c78089605 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -1588,6 +1588,155 @@ netdev_offload_dpdk_flow_flush(struct netdev *netdev)
return 0;
}
+struct get_vport_netdev_aux {
+struct rte_flow_tunnel *tunnel;
+odp_port_t *odp_port;
+struct netdev *vport;
+};
+
+static bool
+get_vxlan_netdev_cb(struct netdev *netdev,
+odp_port_t odp_port,
+void *aux_)
+{
+const struct netdev_tunnel_config *tnl_cfg;
+struct get_vport_netdev_aux *aux = aux_;
+
+if (strcmp(netdev_get_type(netdev), "vxlan")) {
+ return false;
+}
+
+tnl_cfg = netdev_get_tunnel_config(netdev);
+if (!tnl_cfg) {
+VLOG_ERR_RL(&rl, "Cannot get a tunnel config for netdev %s",
+netdev_get_name(netdev));
+return false;
+}
+
+if (tnl_cfg->dst_port == aux->tunnel->tp_dst) {
+/* Found the netdev. Store the results and stop the traversing. */
+aux->vport = netdev_ref(netdev);
+*aux->odp_port = odp_port;
+return true;
+}
+
+return false;
+}
+
+static struct netdev *
+get_vxlan_netdev(const char *dpif_type,
+ struct rte_flow_tunnel *tunnel,
+ odp_port_t *odp_port)
+{
+struct get_vport_netdev_aux aux = {
+.tunnel = tunnel,
+.odp_port = odp_port,
+.vport = NULL,
+};
+
+netdev_ports_traverse(dpif_type, get_vxlan_netdev_cb, &aux);
+return aux.vport;
+}
+
+static struct netdev *
+get_vport_netdev(const char *dpif_type,
+ struct rte_flow_tunnel *tunnel,
+ odp_port_t *odp_port)
+{
+if (tunnel->type == RTE_FLOW_ITEM_TYPE_VXLAN) {
+return get_vxlan_netdev(dpif_type, tunnel, odp_port);
+}
+
+OVS_NOT_REACHED();
+}
+
+static int
+netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev,
+ struct dp_packet *packet)
+{
+struct rte_flow_restore_info rte_restore_info;
+struct rte_flow_tunnel *rte_tnl;
+struct netdev *vport_netdev;
+struct rte_flow_error error;
+struct pkt_metadata *md;
+struct flow_tnl *md_tnl;
+odp_port_t vport_odp;
+int ret = 0;
+
+if (netdev_dpdk_rte_flow_get_restore_info(netdev, packet,
+ &rte_restore_info, &error)) {
+/* This function is called for every packet, and in most cases there
+ * will be no restore info from the HW, thus error is expected.
+ */
+(void) error;
+return 0;
+}
+
+if (!(rte_restore_info.flags & RTE_FLOW_RESTORE_INFO_TUNNEL)) {
+return EOPNOTSUPP;
+}
+
+rte_tnl = &rte_restore_info.tunnel;
+vport_netdev = get_vport_netdev(netdev->dpif_type, rte_tnl,
+&vport_odp);
+if (!vport_netdev) {
+VLOG_WARN("Could not find vport netdev");
+return EOPNOTSUPP;
+}
+
+md = &packet->md;
+/* For tunnel recovery (RTE_FLOW_RESTORE_INFO_TUNNEL), it is possible
+ * to have the packet to still be encapsulated, or not
+ * (RTE_FLOW_RESTORE_INFO_ENCAPSULATED).
+ * In the case it is on, the packet is still encapsulated, and we do
+ * the pop in SW.
+ * In the case it is off, the packet is already decapsulated by HW, and
+ * the tunnel info is provided in the tunnel struct. For this case we
+ * take it to OVS metadata.
+ */
+if (rte_restore_info.flags & RTE_FLOW_RESTORE_INFO_ENCAPSULATED) {
+if (!vport_netdev->netdev_class ||
+!vport_netdev->netdev_class->pop_header) {
+VLOG_ERR("vport nedtdev=%s with no pop_header method",
+ netdev_get_name(vport_netdev));
+ret = EOPNOTSUPP;
+goto close_vport_netdev;
+}
+parse_tcp_flags(packet);
+if (vport_netdev->netdev_class->pop_header(packet) == NULL) {
+/* If there is an error with popping the header, the packet is
+ * freed. In this case it should not continue SW processing.
+ */
+ret = -1;
+goto close_vport_netdev;
+}
+} else {
+md_tnl = &md->tunnel;
+if (rte_tnl->is_ipv6) {
+memcpy(&md_tnl->ipv6_src, &rte_tnl->ipv6.src_addr,
+ sizeof md_tnl->ipv6_src);
+memcpy(&md_tnl->ipv6_dst, &rte_tnl->ipv6.dst_addr,
+ sizeof md_tnl->ipv6_dst);
+} else {
+md_tnl->ip_src = rte_tnl->ipv4.src_addr;
+md_tnl->ip_dst = rte_tnl->ipv4.dst_addr;
+
