> -----Original Message----- > From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] > Sent: Monday, January 20, 2020 8:43 PM > To: wangyunjian <wangyunj...@huawei.com>; dev@dpdk.org; > keith.wi...@intel.com > Cc: xudingke <xudin...@huawei.com>; sta...@dpdk.org > Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] net/tap: fix memory leak when > unregister intr handler > > On 1/20/2020 8:30 AM, Yunjian Wang wrote: > > The return check of function tap_lsc_intr_handle_set() is wrong, it should > > be 0 or a positive number if success. So the intr_handle->intr_vec was not > > been freed when tap_lsc_intr_handle_set() returned a positive number. > > > > Fixes: 4870a8cdd968 ("net/tap: support Rx interrupt") > > Cc: sta...@dpdk.org > > > > Signed-off-by: Yunjian Wang <wangyunj...@huawei.com> > > --- > > drivers/net/tap/rte_eth_tap.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c > > index a13d8d50d..079310fea 100644 > > --- a/drivers/net/tap/rte_eth_tap.c > > +++ b/drivers/net/tap/rte_eth_tap.c > > @@ -1591,8 +1591,10 @@ tap_intr_handle_set(struct rte_eth_dev *dev, int > set) > > int err; > > > > err = tap_lsc_intr_handle_set(dev, set); > > - if (err) > > + if (err < 0) { > > +1 to "err < 0", it seems 'tap_lsc_intr_handle_set()' can return positive on > success for 'rte_intr_callback_unregister()' path. > > > + tap_rx_intr_vec_set(dev, 0); > > for the case "set == 1", this function may be called and it will try to free > some resources, but this may cause redundant free calls and cause segfault, > did > you able to have any chance to test this mentioned path? > What about putting a "if (!set)" before calling this function? > I have tested this mentioned path and find nothing wrong. I will send the v2 later according to Ferruh's suggestions.
Thanks, Yunjian > > return err; > > + } > > err = tap_rx_intr_vec_set(dev, set); > > if (err && set) > > tap_lsc_intr_handle_set(dev, 0); > >
