[ovs-dev] [PATCH V6 00/13] Netdev vxlan-decap offload

Ferriter, Cian cian.ferriter at intel.com
Wed Jun 23 12:37:39 UTC 2021


Hi all,

As part of rebasing our AVX512 DPIF on this patchset, I tested this patchset with partial HWOL and I'm seeing strange behaviour.

I'll report back more detailed findings soon, just wanted to mention this here as soon as I found the issue.

Thanks,
Cian

> -----Original Message-----
> From: dev <ovs-dev-bounces at openvswitch.org> On Behalf Of Ilya Maximets
> Sent: Tuesday 22 June 2021 16:55
> To: Eli Britstein <elibr at nvidia.com>; dev at openvswitch.org; Ilya Maximets <i.maximets at ovn.org>
> Cc: Ivan.Malov at oktetlabs.ru; Ameer Mahagneh <ameerm at nvidia.com>; Majd Dibbiny <majd at nvidia.com>
> Subject: Re: [ovs-dev] [PATCH V6 00/13] Netdev vxlan-decap offload
> 
> On 4/4/21 11:54 AM, Eli Britstein wrote:
> > VXLAN decap in OVS-DPDK configuration consists of two flows:
> > F1: in_port(ens1f0),eth(),ipv4(),udp(), actions:tnl_pop(vxlan_sys_4789)
> > F2: tunnel(),in_port(vxlan_sys_4789),eth(),ipv4(), actions:ens1f0_0
> >
> > F1 is a classification flow. It has outer headers matches and it
> > classifies the packet as a VXLAN packet, and using tnl_pop action the
> > packet continues processing in F2.
> > F2 is a flow that has matches on tunnel metadata as well as on the inner
> > packet headers (as any other flow).
> >
> > In order to fully offload VXLAN decap path, both F1 and F2 should be
> > offloaded. As there are more than one flow in HW, it is possible that
> > F1 is done by HW but F2 is not. Packet is received by SW, and should be
> > processed starting from F2 as F1 was already done by HW.
> > Rte_flows are applicable only on physical port IDs. Keeping the original
> > physical in port on which the packet is received on enables applying
> > vport flows (e.g. F2) on that physical port.
> >
> > This patch-set makes use of [1] introduced in DPDK 20.11, that adds API
> > for tunnel offloads.
> >
> > Note that MLX5 PMD has a bug that the tnl_pop private actions must be
> > first. In OVS it is not.
> > Fixing this issue is scheduled for 21.05 (and stable 20.11.2).
> > Meanwhile, tests were done with a workaround for it [2].
> >
> > v2-v1:
> > - Tracking original in_port, and applying vport on that physical port instead of all PFs.
> > v3-v2:
> > - Traversing ports using a new API instead of flow_dump.
> > - Refactor packet state recover logic, with bug fix for error pop_header.
> > - One ref count for netdev in non-tunnel case.
> > - Rename variables, comments, rebase.
> > v4-v3:
> > - Extract orig_in_port from physdev for flow modify.
> > - Miss handling fixes.
> > v5-v4:
> > - Drop refactor offload rule creation commit.
> > - Comment about setting in_port in restore.
> > - Refactor vports flow offload commit.
> > v6-v5:
> > - Fixed duplicate netdev ref bug.
> >
> > Travis:
> > v1: https://travis-ci.org/github/elibritstein/OVS/builds/756418552
> > v2: https://travis-ci.org/github/elibritstein/OVS/builds/758382963
> > v3: https://travis-ci.org/github/elibritstein/OVS/builds/761089087
> > v4: https://travis-ci.org/github/elibritstein/OVS/builds/763146966
> > v5: https://travis-ci.org/github/elibritstein/OVS/builds/765271879
> > v6: https://travis-ci.org/github/elibritstein/OVS/builds/765816800
> >
> > GitHub Actions:
> > v1: https://github.com/elibritstein/OVS/actions/runs/515334647
> > v2: https://github.com/elibritstein/OVS/actions/runs/554986007
> > v3: https://github.com/elibritstein/OVS/actions/runs/613226225
> > v4: https://github.com/elibritstein/OVS/actions/runs/658415274
> > v5: https://github.com/elibritstein/OVS/actions/runs/704357369
> > v6: https://github.com/elibritstein/OVS/actions/runs/716304028
> >
> > [1] https://mails.dpdk.org/archives/dev/2020-October/187314.html
> > [2] https://github.com/elibritstein/dpdk-stable/pull/1
> >
> >
> > Eli Britstein (10):
> >   netdev-offload: Add HW miss packet state recover API
> >   netdev-dpdk: Introduce DPDK tunnel APIs
> >   netdev-offload: Introduce an API to traverse ports
> >   netdev-dpdk: Add flow_api support for netdev vxlan vports
> >   netdev-offload-dpdk: Implement HW miss packet recover for vport
> >   dpif-netdev: Add HW miss packet state recover logic
> >   netdev-offload-dpdk: Change log rate limits
> >   netdev-offload-dpdk: Support tunnel pop action
> >   netdev-offload-dpdk: Support vports flows offload
> >   netdev-dpdk-offload: Add vxlan pattern matching function
> >
> > Ilya Maximets (2):
> >   netdev-offload: Allow offloading to netdev without ifindex.
> >   netdev-offload: Disallow offloading to unrelated tunneling vports.
> >
> > Sriharsha Basavapatna (1):
> >   dpif-netdev: Provide orig_in_port in metadata for tunneled packets
> >
> >  Documentation/howto/dpdk.rst  |   1 +
> >  NEWS                          |   2 +
> >  lib/dpif-netdev.c             |  97 +++--
> >  lib/netdev-dpdk.c             | 118 ++++++
> >  lib/netdev-dpdk.h             | 106 ++++-
> >  lib/netdev-offload-dpdk.c     | 704 +++++++++++++++++++++++++++++++---
> >  lib/netdev-offload-provider.h |   5 +
> >  lib/netdev-offload-tc.c       |   8 +
> >  lib/netdev-offload.c          |  47 ++-
> >  lib/netdev-offload.h          |  10 +
> >  lib/packets.h                 |   8 +-
> >  11 files changed, 1022 insertions(+), 84 deletions(-)
> >
> 
> Hi.  I reviewed the whole series and it looks mostly OK to me.
> I made a several changes along the way and below you may find a diff
> of what I changed.  In short:
> 
>  - Some style fixes: style of function prototypes and a way how long
>    function calls wrapped.  Added names to uint32_t arguments to
>    function prototypes.
> 
>  - Clarified hw_miss_packet_recover() API.  It needs a note that it
>    takes ownership of a packet on error.  Fixed 'return -1' which
>    doesn't comply with the API definition (should return positive
>    errno).
> 
>  - Moved NEWS updates to correct place.
> 
>  - Added a packet drop counter.
> 
>  - Refactored changes in dfc_processing().  Basically, restored it
>    to look mostly like it was before, because LIKELY/UNLIKELY markers
>    make no sense for non-offloading cases where they both should be
>    'unlikely'.  Also, old way of writing this part allows following
>    optimization:
> 
> https://patchwork.ozlabs.org/project/openvswitch/patch/c0b99b4b9be6c290a6aa3cb00049fac4ebfac5d8.161839
> 0390.git.bnemeth at redhat.com/
> 
>  - And the only functional change that I made is that I guarded
>    actual offloading support with 'ifdef DPDK_EXPERIMENTAL_API',
>    because they doesn't make sense to me if packet restoration
>    API is not available.  I also added this information to the
>    NEWS file.  I think that we should not try to offload TUNNEL_POP
>    if we can't restore the tunnel metadata.  And if we can't have
>    the first flow in HW, there is no point adding the second one.
> 
> Complete branch with ready-to-push patches available here:
>   https://github.com/igsilya/ovs/tree/tmp-vxlan-decap
> 
> Diff below is a diff with this v6 patch-set.  Please, take a look/test.
> I'll wait for Ack on changes below or comments, if I broke something,
> before pushing this to the main repo.
> 
> Best regards, Ilya Maximets.
> 
> The diff:
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index 4918d80f3..36314c06a 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -398,7 +398,7 @@ Supported actions for hardware offload are:
>  - VLAN Push/Pop (push_vlan/pop_vlan).
>  - Modification of IPv6 (set_field:<ADDR>->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.
> +- Tunnel pop, for packets received on physical ports.
> 
>  Further Reading
>  ---------------
> diff --git a/NEWS b/NEWS
> index e49f2955d..10b3ab053 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -17,6 +17,10 @@ Post-v2.15.0
>       * OVS validated with DPDK 20.11.1. It is recommended to use this version
>         until further releases.
>       * New debug appctl command 'dpdk/get-malloc-stats'.
> +     * Add hardware offload support for tunnel pop action (experimental).
> +       Available only if DPDK experimantal APIs enabled during the build.
> +     * Add hardware offload support for VXLAN flows (experimental).
> +       Available only if DPDK experimantal APIs enabled during the build.
>     - ovsdb-tool:
>       * New option '--election-timer' to the 'create-cluster' command to set the
>         leader election timer during cluster creation.
> @@ -44,8 +48,6 @@ v2.15.0 - 15 Feb 2021
>     - DPDK:
>       * Removed support for vhost-user dequeue zero-copy.
>       * Add support for DPDK 20.11.
> -     * Add hardware offload support for tunnel pop action (experimental).
> -     * Add hardware offload support for VXLAN flows (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/dpif-netdev.c b/lib/dpif-netdev.c
> index 1bd828f82..8766a00ea 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -114,6 +114,7 @@ COVERAGE_DEFINE(datapath_drop_invalid_port);
>  COVERAGE_DEFINE(datapath_drop_invalid_bond);
>  COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
>  COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
> +COVERAGE_DEFINE(datapath_drop_hw_miss_recover);
> 
>  /* Protects against changes to 'dp_netdevs'. */
>  static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
> @@ -7068,9 +7069,8 @@ smc_lookup_batch(struct dp_netdev_pmd_thread *pmd,
>      pmd_perf_update_counter(&pmd->perf_stats, PMD_STAT_SMC_HIT, n_smc_hit);
>  }
> 
> -static struct tx_port *
> -pmd_send_port_cache_lookup(const struct dp_netdev_pmd_thread *pmd,
> -                           odp_port_t port_no);
> +static struct tx_port * pmd_send_port_cache_lookup(
> +    const struct dp_netdev_pmd_thread *pmd, odp_port_t port_no);
> 
>  static inline int
>  dp_netdev_hw_flow(const struct dp_netdev_pmd_thread *pmd,
> @@ -7081,17 +7081,13 @@ dp_netdev_hw_flow(const struct dp_netdev_pmd_thread *pmd,
>      struct tx_port *p;
>      uint32_t mark;
> 
> -    if (!netdev_is_flow_api_enabled() || *recirc_depth_get() != 0) {
> -        *flow = NULL;
> -        return 0;
> -    }
> -
>      /* Restore the packet if HW processing was terminated before completion. */
>      p = pmd_send_port_cache_lookup(pmd, port_no);
>      if (OVS_LIKELY(p)) {
>          int err = netdev_hw_miss_packet_recover(p->port->netdev, packet);
> 
> -        if (err != 0 && err != EOPNOTSUPP) {
> +        if (err && err != EOPNOTSUPP) {
> +            COVERAGE_INC(datapath_drop_hw_miss_recover);
>              return -1;
>          }
>      }
> @@ -7168,25 +7164,28 @@ dfc_processing(struct dp_netdev_pmd_thread *pmd,
>              pkt_metadata_init(&packet->md, port_no);
>          }
> 
> -        if (OVS_UNLIKELY(dp_netdev_hw_flow(pmd, port_no, packet, &flow))) {
> -            /* Packet restoration failed. Its mbuf was freed, do not continue
> -             * processing.
> -             */
> -            continue;
> -        } else if (OVS_LIKELY(flow)) {
> -            tcp_flags = parse_tcp_flags(packet);
> -            if (OVS_LIKELY(batch_enable)) {
> -                dp_netdev_queue_batches(packet, flow, tcp_flags, batches,
> -                                        n_batches);
> -            } else {
> -                /* Flow batching should be performed only after fast-path
> -                 * processing is also completed for packets with emc miss
> -                 * or else it will result in reordering of packets with
> -                 * same datapath flows. */
> -                packet_enqueue_to_flow_map(packet, flow, tcp_flags, flow_map,
> -                                           map_cnt++);
> +        if (netdev_is_flow_api_enabled() && *recirc_depth_get() == 0) {
> +            if (OVS_UNLIKELY(dp_netdev_hw_flow(pmd, port_no, packet, &flow))) {
> +                /* Packet restoration failed and it was dropped, do not
> +                 * continue processing.
> +                 */
> +                continue;
> +            }
> +            if (OVS_LIKELY(flow)) {
> +                tcp_flags = parse_tcp_flags(packet);
> +                if (OVS_LIKELY(batch_enable)) {
> +                    dp_netdev_queue_batches(packet, flow, tcp_flags, batches,
> +                                            n_batches);
> +                } else {
> +                    /* Flow batching should be performed only after fast-path
> +                     * processing is also completed for packets with emc miss
> +                     * or else it will result in reordering of packets with
> +                     * same datapath flows. */
> +                    packet_enqueue_to_flow_map(packet, flow, tcp_flags,
> +                                               flow_map, map_cnt++);
> +                }
> +                continue;
>              }
> -            continue;
>          }
> 
>          miniflow_extract(packet, &key->mf);
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 6e35d0574..bc5485d60 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -5365,11 +5365,11 @@ netdev_dpdk_rte_flow_get_restore_info(struct netdev *netdev,
>  }
> 
>  int
> -netdev_dpdk_rte_flow_tunnel_action_decap_release
> -    (struct netdev *netdev,
> -     struct rte_flow_action *actions,
> -     uint32_t num_of_actions,
> -     struct rte_flow_error *error)
> +netdev_dpdk_rte_flow_tunnel_action_decap_release(
> +    struct netdev *netdev,
> +    struct rte_flow_action *actions,
> +    uint32_t num_of_actions,
> +    struct rte_flow_error *error)
>  {
>      struct netdev_dpdk *dev;
>      int ret;
> diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
> index 3b9bf8681..7b77ed8e0 100644
> --- a/lib/netdev-dpdk.h
> +++ b/lib/netdev-dpdk.h
> @@ -53,33 +53,28 @@ netdev_dpdk_get_port_id(struct netdev *netdev);
> 
>  #ifdef ALLOW_EXPERIMENTAL_API
> 
> -int
> -netdev_dpdk_rte_flow_tunnel_decap_set(struct netdev *,
> +int netdev_dpdk_rte_flow_tunnel_decap_set(struct netdev *,
> +                                          struct rte_flow_tunnel *,
> +                                          struct rte_flow_action **,
> +                                          uint32_t *num_of_actions,
> +                                          struct rte_flow_error *);
> +int netdev_dpdk_rte_flow_tunnel_match(struct netdev *,
>                                        struct rte_flow_tunnel *,
> -                                      struct rte_flow_action **,
> -                                      uint32_t *,
> -                                      struct rte_flow_error *);
> -int
> -netdev_dpdk_rte_flow_tunnel_match(struct netdev *,
> -                                  struct rte_flow_tunnel *,
> -                                  struct rte_flow_item **,
> -                                  uint32_t *,
> -                                  struct rte_flow_error *);
> -int
> -netdev_dpdk_rte_flow_get_restore_info(struct netdev *,
> -                                      struct dp_packet *,
> -                                      struct rte_flow_restore_info *,
> +                                      struct rte_flow_item **,
> +                                      uint32_t *num_of_items,
>                                        struct rte_flow_error *);
> -int
> -netdev_dpdk_rte_flow_tunnel_action_decap_release(struct netdev *,
> -                                                 struct rte_flow_action *,
> -                                                 uint32_t,
> -                                                 struct rte_flow_error *);
> -int
> -netdev_dpdk_rte_flow_tunnel_item_release(struct netdev *,
> -                                         struct rte_flow_item *,
> -                                         uint32_t,
> -                                         struct rte_flow_error *);
> +int netdev_dpdk_rte_flow_get_restore_info(struct netdev *,
> +                                          struct dp_packet *,
> +                                          struct rte_flow_restore_info *,
> +                                          struct rte_flow_error *);
> +int netdev_dpdk_rte_flow_tunnel_action_decap_release(struct netdev *,
> +                                                     struct rte_flow_action *,
> +                                                     uint32_t num_of_actions,
> +                                                     struct rte_flow_error *);
> +int netdev_dpdk_rte_flow_tunnel_item_release(struct netdev *,
> +                                             struct rte_flow_item *,
> +                                             uint32_t num_of_items,
> +                                             struct rte_flow_error *);
> 
>  #else
> 
> @@ -92,14 +87,13 @@ set_error(struct rte_flow_error *error, enum rte_flow_error_type type)
>  }
> 
>  static inline int
> -netdev_dpdk_rte_flow_tunnel_decap_set(struct netdev *netdev OVS_UNUSED,
> -                                      struct rte_flow_tunnel *tunnel,
> -                                      struct rte_flow_action **actions,
> -                                      uint32_t *num_of_actions OVS_UNUSED,
> -                                      struct rte_flow_error *error)
> +netdev_dpdk_rte_flow_tunnel_decap_set(
> +    struct netdev *netdev OVS_UNUSED,
> +    struct rte_flow_tunnel *tunnel OVS_UNUSED,
> +    struct rte_flow_action **actions OVS_UNUSED,
> +    uint32_t *num_of_actions OVS_UNUSED,
> +    struct rte_flow_error *error)
>  {
> -    (void) tunnel;
> -    (void) actions;
>      set_error(error, RTE_FLOW_ERROR_TYPE_ACTION);
>      return -1;
>  }
> @@ -116,34 +110,34 @@ netdev_dpdk_rte_flow_tunnel_match(struct netdev *netdev OVS_UNUSED,
>  }
> 
>  static inline int
> -netdev_dpdk_rte_flow_get_restore_info(struct netdev *netdev OVS_UNUSED,
> -                                      struct dp_packet *p OVS_UNUSED,
> -                                      struct rte_flow_restore_info *info,
> -                                      struct rte_flow_error *error)
> +netdev_dpdk_rte_flow_get_restore_info(
> +    struct netdev *netdev OVS_UNUSED,
> +    struct dp_packet *p OVS_UNUSED,
> +    struct rte_flow_restore_info *info OVS_UNUSED,
> +    struct rte_flow_error *error)
>  {
> -    (void) info;
>      set_error(error, RTE_FLOW_ERROR_TYPE_ATTR);
>      return -1;
>  }
> 
>  static inline int
> -netdev_dpdk_rte_flow_tunnel_action_decap_release
> -    (struct netdev *netdev OVS_UNUSED,
> -     struct rte_flow_action *actions OVS_UNUSED,
> -     uint32_t num_of_actions OVS_UNUSED,
> -     struct rte_flow_error *error)
> +netdev_dpdk_rte_flow_tunnel_action_decap_release(
> +    struct netdev *netdev OVS_UNUSED,
> +    struct rte_flow_action *actions OVS_UNUSED,
> +    uint32_t num_of_actions OVS_UNUSED,
> +    struct rte_flow_error *error)
>  {
>      set_error(error, RTE_FLOW_ERROR_TYPE_NONE);
>      return 0;
>  }
> 
>  static inline int
> -netdev_dpdk_rte_flow_tunnel_item_release(struct netdev *netdev OVS_UNUSED,
> -                                         struct rte_flow_item *items,
> -                                         uint32_t num_of_items OVS_UNUSED,
> -                                         struct rte_flow_error *error)
> +netdev_dpdk_rte_flow_tunnel_item_release(
> +    struct netdev *netdev OVS_UNUSED,
> +    struct rte_flow_item *items OVS_UNUSED,
> +    uint32_t num_of_items OVS_UNUSED,
> +    struct rte_flow_error *error)
>  {
> -    (void) items;
>      set_error(error, RTE_FLOW_ERROR_TYPE_NONE);
>      return 0;
>  }
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 52a74a707..363f32f71 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -459,7 +459,7 @@ dump_flow_action(struct ds *s, struct ds *s_extra,
>                 act_index >= flow_actions->tnl_pmd_actions_pos &&
>                 act_index < flow_actions->tnl_pmd_actions_pos +
>                             flow_actions->tnl_pmd_actions_cnt) {
> -        /* Opaque PMD tunnel actions is skipped. */
> +        /* Opaque PMD tunnel actions are skipped. */
>          return;
>      } else if (actions->type == RTE_FLOW_ACTION_TYPE_MARK) {
>          const struct rte_flow_action_mark *mark = actions->conf;
> @@ -792,9 +792,9 @@ free_flow_actions(struct flow_actions *actions)
>      for (i = 0; i < actions->cnt; i++) {
>          if (actions->tnl_pmd_actions_cnt &&
>              i == actions->tnl_pmd_actions_pos) {
> -            if (netdev_dpdk_rte_flow_tunnel_action_decap_release
> -                    (actions->tnl_netdev, actions->tnl_pmd_actions,
> -                     actions->tnl_pmd_actions_cnt, &error)) {
> +            if (netdev_dpdk_rte_flow_tunnel_action_decap_release(
> +                    actions->tnl_netdev, actions->tnl_pmd_actions,
> +                    actions->tnl_pmd_actions_cnt, &error)) {
>                  VLOG_DBG_RL(&rl, "%s: "
>                              "netdev_dpdk_rte_flow_tunnel_action_decap_release "
>                              "failed: %d (%s).",
> @@ -1009,7 +1009,7 @@ parse_vxlan_match(struct flow_patterns *patterns,
>      return 0;
>  }
> 
> -static int
> +static int OVS_UNUSED
>  parse_flow_tnl_match(struct netdev *tnldev,
>                       struct flow_patterns *patterns,
>                       odp_port_t orig_in_port,
> @@ -1031,7 +1031,7 @@ parse_flow_tnl_match(struct netdev *tnldev,
> 
>  static int
>  parse_flow_match(struct netdev *netdev,
> -                 odp_port_t orig_in_port,
> +                 odp_port_t orig_in_port OVS_UNUSED,
>                   struct flow_patterns *patterns,
>                   struct match *match)
>  {
> @@ -1045,10 +1045,12 @@ parse_flow_match(struct netdev *netdev,
>      }
> 
>      patterns->physdev = netdev;
> +#ifdef ALLOW_EXPERIMENTAL_API /* Packet restoration API required. */
>      if (netdev_vport_is_vport_class(netdev->netdev_class) &&
>          parse_flow_tnl_match(netdev, patterns, orig_in_port, match)) {
>          return -1;
>      }
> +#endif
>      memset(&consumed_masks->in_port, 0, sizeof consumed_masks->in_port);
>      /* recirc id must be zero. */
>      if (match->wc.masks.recirc_id & match->flow.recirc_id) {
> @@ -1679,7 +1681,7 @@ add_jump_action(struct flow_actions *actions, uint32_t group)
>      add_flow_action(actions, RTE_FLOW_ACTION_TYPE_JUMP, jump);
>  }
> 
> -static int
> +static int OVS_UNUSED
>  add_tnl_pop_action(struct netdev *netdev,
>                     struct flow_actions *actions,
>                     const struct nlattr *nla)
> @@ -1767,10 +1769,12 @@ parse_flow_actions(struct netdev *netdev,
>                                      clone_actions_len)) {
>                  return -1;
>              }
> +#ifdef ALLOW_EXPERIMENTAL_API /* Packet restoration API required. */
>          } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_TUNNEL_POP) {
>              if (add_tnl_pop_action(netdev, actions, nla)) {
>                  return -1;
>              }
> +#endif
>          } else {
>              VLOG_DBG_RL(&rl, "Unsupported action type %d", nl_attr_type(nla));
>              return -1;
> @@ -2067,7 +2071,7 @@ get_vxlan_netdev_cb(struct netdev *netdev,
>      struct get_vport_netdev_aux *aux = aux_;
> 
>      if (strcmp(netdev_get_type(netdev), "vxlan")) {
> -       return false;
> +        return false;
>      }
> 
>      tnl_cfg = netdev_get_tunnel_config(netdev);
> @@ -2121,18 +2125,16 @@ netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev,
>      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)) {
> +                                              &rte_restore_info, NULL)) {
>          /* 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;
>      }
> 
> @@ -2144,25 +2146,25 @@ netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev,
>      vport_netdev = get_vport_netdev(netdev->dpif_type, rte_tnl,
>                                      &vport_odp);
>      if (!vport_netdev) {
> -        VLOG_WARN("Could not find vport netdev");
> +        VLOG_WARN_RL(&rl, "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).
> +     * to have the packet to still be encapsulated, or not.  This is reflected
> +     * by the RTE_FLOW_RESTORE_INFO_ENCAPSULATED flag.
>       * 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
> +     * 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));
> +            VLOG_ERR_RL(&rl, "vport nedtdev=%s with no pop_header method",
> +                        netdev_get_name(vport_netdev));
>              ret = EOPNOTSUPP;
>              goto close_vport_netdev;
>          }
> @@ -2171,7 +2173,7 @@ netdev_offload_dpdk_hw_miss_packet_recover(struct netdev *netdev,
>              /* If there is an error with popping the header, the packet is
>               * freed. In this case it should not continue SW processing.
>               */
> -            ret = -1;
> +            ret = EINVAL;
>              goto close_vport_netdev;
>          }
>      } else {
> diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
> index f24c7dd19..348ca7081 100644
> --- a/lib/netdev-offload-provider.h
> +++ b/lib/netdev-offload-provider.h
> @@ -89,7 +89,8 @@ struct netdev_flow_api {
> 
>      /* Recover the packet state (contents and data) for continued processing
>       * in software.
> -     * Return 0 if successful, otherwise returns a positive errno value. */
> +     * Return 0 if successful, otherwise returns a positive errno value and
> +     * takes ownership of a packet if errno != EOPNOTSUPP. */
>      int (*hw_miss_packet_recover)(struct netdev *, struct dp_packet *);
> 
>      /* Initializies the netdev flow api.
> ---
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list