[ovs-dev,2/3] dpif: Fix memory leak

Valgrind complains in test 2322 (ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR):

31,584 (26,496 direct, 5,088 indirect) bytes in 48 blocks are definitely
lost in loss record 422 of 427
   by 0x5165F4: xmalloc (util.c:120)
   by 0x466194: dp_packet_new (dp-packet.c:138)
   by 0x466194: dp_packet_new_with_headroom (dp-packet.c:148)
   by 0x46621B: dp_packet_clone_data_with_headroom (dp-packet.c:210)
   by 0x46621B: dp_packet_clone_with_headroom (dp-packet.c:170)
   by 0x49DD46: dp_packet_batch_clone (dp-packet.h:789)
   by 0x49DD46: odp_execute_clone (odp-execute.c:616)
   by 0x49DD46: odp_execute_actions (odp-execute.c:795)
   by 0x471663: dpif_execute_with_help (dpif.c:1296)
   by 0x473795: dpif_operate (dpif.c:1411)
   by 0x473E20: dpif_execute.part.21 (dpif.c:1320)
   by 0x428D38: packet_execute (ofproto-dpif.c:4682)
   by 0x41EB51: ofproto_packet_out_finish (ofproto.c:3540)
   by 0x41EB51: handle_packet_out (ofproto.c:3581)
   by 0x4233DA: handle_openflow__ (ofproto.c:8044)
   by 0x4233DA: handle_openflow (ofproto.c:8219)
   by 0x4514AA: ofconn_run (connmgr.c:1437)
   by 0x4514AA: connmgr_run (connmgr.c:363)
   by 0x41C8B5: ofproto_run (ofproto.c:1813)
   by 0x40B103: bridge_run__ (bridge.c:2919)
   by 0x4103B3: bridge_run (bridge.c:2977)
   by 0x406F14: main (ovs-vswitchd.c:119)

the parameter dp_packet_batch is leaked when 'may_steal' is true.

When dpif_execute_helper_cb is passed with a true 'may_steal', it
is supposed to take the ownership of dp_packet_batch and release
it when done.

Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
---
 lib/dpif.c | 1 +
 1 file changed, 1 insertion(+)

On 11/15/2017 6:59 AM, Yifeng Sun wrote:
> Valgrind complains in test 2322 (ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR):
>
> 31,584 (26,496 direct, 5,088 indirect) bytes in 48 blocks are definitely
> lost in loss record 422 of 427
>     by 0x5165F4: xmalloc (util.c:120)
>     by 0x466194: dp_packet_new (dp-packet.c:138)
>     by 0x466194: dp_packet_new_with_headroom (dp-packet.c:148)
>     by 0x46621B: dp_packet_clone_data_with_headroom (dp-packet.c:210)
>     by 0x46621B: dp_packet_clone_with_headroom (dp-packet.c:170)
>     by 0x49DD46: dp_packet_batch_clone (dp-packet.h:789)
>     by 0x49DD46: odp_execute_clone (odp-execute.c:616)
>     by 0x49DD46: odp_execute_actions (odp-execute.c:795)
>     by 0x471663: dpif_execute_with_help (dpif.c:1296)
>     by 0x473795: dpif_operate (dpif.c:1411)
>     by 0x473E20: dpif_execute.part.21 (dpif.c:1320)
>     by 0x428D38: packet_execute (ofproto-dpif.c:4682)
>     by 0x41EB51: ofproto_packet_out_finish (ofproto.c:3540)
>     by 0x41EB51: handle_packet_out (ofproto.c:3581)
>     by 0x4233DA: handle_openflow__ (ofproto.c:8044)
>     by 0x4233DA: handle_openflow (ofproto.c:8219)
>     by 0x4514AA: ofconn_run (connmgr.c:1437)
>     by 0x4514AA: connmgr_run (connmgr.c:363)
>     by 0x41C8B5: ofproto_run (ofproto.c:1813)
>     by 0x40B103: bridge_run__ (bridge.c:2919)
>     by 0x4103B3: bridge_run (bridge.c:2977)
>     by 0x406F14: main (ovs-vswitchd.c:119)
>
> the parameter dp_packet_batch is leaked when 'may_steal' is true.
>
> When dpif_execute_helper_cb is passed with a true 'may_steal', it
> is supposed to take the ownership of dp_packet_batch and release
> it when done.
>
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>

This patch fixes the memory leak mentioned above.

Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
> ---
>   lib/dpif.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 3f0742d382ed..310dec14655e 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1279,6 +1279,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
>       case __OVS_ACTION_ATTR_MAX:
>           OVS_NOT_REACHED();
>       }
> +    dp_packet_delete_batch(packets_, may_steal);
>   }
>   
>   /* Executes 'execute' by performing most of the actions in userspace and

On Wed, Nov 15, 2017 at 06:59:25AM -0800, Yifeng Sun wrote:
> Valgrind complains in test 2322 (ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR):
> 
> 31,584 (26,496 direct, 5,088 indirect) bytes in 48 blocks are definitely
> lost in loss record 422 of 427
>    by 0x5165F4: xmalloc (util.c:120)
>    by 0x466194: dp_packet_new (dp-packet.c:138)
>    by 0x466194: dp_packet_new_with_headroom (dp-packet.c:148)
>    by 0x46621B: dp_packet_clone_data_with_headroom (dp-packet.c:210)
>    by 0x46621B: dp_packet_clone_with_headroom (dp-packet.c:170)
>    by 0x49DD46: dp_packet_batch_clone (dp-packet.h:789)
>    by 0x49DD46: odp_execute_clone (odp-execute.c:616)
>    by 0x49DD46: odp_execute_actions (odp-execute.c:795)
>    by 0x471663: dpif_execute_with_help (dpif.c:1296)
>    by 0x473795: dpif_operate (dpif.c:1411)
>    by 0x473E20: dpif_execute.part.21 (dpif.c:1320)
>    by 0x428D38: packet_execute (ofproto-dpif.c:4682)
>    by 0x41EB51: ofproto_packet_out_finish (ofproto.c:3540)
>    by 0x41EB51: handle_packet_out (ofproto.c:3581)
>    by 0x4233DA: handle_openflow__ (ofproto.c:8044)
>    by 0x4233DA: handle_openflow (ofproto.c:8219)
>    by 0x4514AA: ofconn_run (connmgr.c:1437)
>    by 0x4514AA: connmgr_run (connmgr.c:363)
>    by 0x41C8B5: ofproto_run (ofproto.c:1813)
>    by 0x40B103: bridge_run__ (bridge.c:2919)
>    by 0x4103B3: bridge_run (bridge.c:2977)
>    by 0x406F14: main (ovs-vswitchd.c:119)
> 
> the parameter dp_packet_batch is leaked when 'may_steal' is true.
> 
> When dpif_execute_helper_cb is passed with a true 'may_steal', it
> is supposed to take the ownership of dp_packet_batch and release
> it when done.
> 
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>

Thanks, I applied this to master and backported as far as 2.4.