[ovs-dev] [PATCH] dpif-netdev: Fix (packet) memory leaks in the slow path.

Pravin Shelar pshelar at nicira.com
Fri Sep 19 22:49:17 UTC 2014


On Fri, Sep 19, 2014 at 1:28 PM, Daniele Di Proietto
<ddiproietto at vmware.com> wrote:
> If a packet didn't match a rule in the fast path classifier its memory was
> never freed. The issue was particularly clear with DPDK devices because it was
> not possible to process more than ~250000 DPDK mbufs in the slow path.
>
> This commit fixes the problem by:
> * calling dpif_packet_delete() if the upcalls are disabled
> * passing may_steal==true to dp_netdev_execute_actions() during normal upcall
>   processing
>
> Signed-off-by: Daniele Di Proietto <ddiproietto at vmware.com>
> ---
>  lib/dpif-netdev.c | 47 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 7e27acf..8df3b81 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2576,7 +2576,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
>              /* We can't allow the packet batching in the next loop to execute
>               * the actions.  Otherwise, if there are any slow path actions,
>               * we'll send the packet up twice. */
> -            dp_netdev_execute_actions(pmd, &packets[i], 1, false, md,
> +            dp_netdev_execute_actions(pmd, &packets[i], 1, true, md,
>                                        ofpbuf_data(&actions),
>                                        ofpbuf_size(&actions));
>
> @@ -2602,6 +2602,17 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
>          ofpbuf_uninit(&actions);
>          ofpbuf_uninit(&put_actions);
>          fat_rwlock_unlock(&dp->upcall_rwlock);
> +    } else if (OVS_UNLIKELY(any_miss)) {
> +        int dropped_cnt = 0;
> +
> +        for (i = 0; i < cnt; i++) {
> +            if (OVS_UNLIKELY(!rules[i] && mfs[i])) {
> +                dpif_packet_delete(packets[i]);
> +                dropped_cnt++;
> +            }
> +        }
> +
> +        dp_netdev_count_packet(dp, DP_STAT_LOST, dropped_cnt);
>      }
>
>      n_batches = 0;
> @@ -2658,6 +2669,18 @@ dpif_netdev_register_upcall_cb(struct dpif *dpif, upcall_callback *cb,
>  }
>
>  static void
> +dp_netdev_drop_packets(struct dpif_packet ** packets, int cnt, bool may_steal)
> +{
> +    int i;
> +
> +    if (may_steal) {
> +        for (i = 0; i < cnt; i++) {
> +            dpif_packet_delete(packets[i]);
> +        }
> +    }
> +}
> +
> +static void
>  dp_execute_cb(void *aux_, struct dpif_packet **packets, int cnt,
>                struct pkt_metadata *md,
>                const struct nlattr *a, bool may_steal)
> @@ -2676,10 +2699,8 @@ dp_execute_cb(void *aux_, struct dpif_packet **packets, int cnt,
>          p = dp_netdev_lookup_port(dp, u32_to_odp(nl_attr_get_u32(a)));
>          if (OVS_LIKELY(p)) {
>              netdev_send(p->netdev, pmd->core_id, packets, cnt, may_steal);
> -        } else if (may_steal) {
> -            for (i = 0; i < cnt; i++) {
> -                dpif_packet_delete(packets[i]);
> -            }
> +        } else {
> +            dp_netdev_drop_packets(packets, cnt, may_steal);
>          }
>          break;
>
> @@ -2702,17 +2723,17 @@ dp_execute_cb(void *aux_, struct dpif_packet **packets, int cnt,
>                                           DPIF_UC_ACTION, userdata, &actions,
>                                           NULL);
>                  if (!error || error == ENOSPC) {
> -                    dp_netdev_execute_actions(pmd, &packets[i], 1, false, md,
> -                                              ofpbuf_data(&actions),
> +                    dp_netdev_execute_actions(pmd, &packets[i], 1, may_steal,
> +                                              md, ofpbuf_data(&actions),
>                                                ofpbuf_size(&actions));
> -                }
> -
> -                if (may_steal) {
> +                } else if (may_steal) {
>                      dpif_packet_delete(packets[i]);
>                  }
>              }
>              ofpbuf_uninit(&actions);
>              fat_rwlock_unlock(&dp->upcall_rwlock);
> +        } else {
> +            dp_netdev_drop_packets(packets, cnt, may_steal);
>          }
>
>          break;
> @@ -2772,11 +2793,7 @@ dp_execute_cb(void *aux_, struct dpif_packet **packets, int cnt,
>              break;
>          } else {
>              VLOG_WARN("Packet dropped. Max recirculation depth exceeded.");
> -            if (may_steal) {
> -                for (i = 0; i < cnt; i++) {
> -                    dpif_packet_delete(packets[i]);
> -                }
> -            }
> +            dp_netdev_drop_packets(packets, cnt, may_steal);
>          }
>          break;
>
You can move calls dp_netdev_drop_packets() at the bottom of dp_execute_cb()
All successful switch-case can return immediately and error cases can
drop packets if required.

otherwise looks good.

> --
> 2.1.0.rc1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list