Message ID | 1510757966-20364-2-git-send-email-pkusunyifeng@gmail.com |
---|---|
State | Accepted |
Delegated to: | Ian Stokes |
Headers | show |
Series | [ovs-dev,1/3] dpif-netdev: Fix memory leak | expand |
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.
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
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(+)