Message ID | 1487599487-12574-2-git-send-email-hanxueluo@126.com (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Yuanhan Liu |
Headers | show |
Context | Check | Description |
---|---|---|
ci/checkpatch | success | coding style OK |
ci/Intel-compilation | success | Compilation OK |
On Mon, Feb 20, 2017 at 10:04:46PM +0800, hanxueluo@126.com wrote: > From: Huanle Han <hanxueluo@gmail.com> > > This commit fixs segment fault when rte_eth_dev_close() > is called on a virtio dev more than once. > Assigning zero after free to avoids freed memory to > be accessed again. Thanks for the fix! And here are few more minor nits you might want be awre of: - a fix patch needs a fixline (check http://dpdk.org/dev for howto just in case you don't know) - if it fixes a fatal bug (like this one), it should also be picked (or backported) to a specific stable release. In this case, you should add Cc: stable@dpdk.org just before your SoB (Signed-off-by). --yliu > > Signed-off-by: Huanle Han <hanxueluo@gmail.com> > --- > drivers/net/virtio/virtio_ethdev.c | 5 +++++ > lib/librte_ether/rte_ethdev.c | 2 ++ > 2 files changed, 7 insertions(+) > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index 8465e1a..b9565db 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -545,6 +545,9 @@ virtio_free_queues(struct virtio_hw *hw) > int queue_type; > uint16_t i; > > + if (hw->vqs == NULL) > + return; > + > for (i = 0; i < nr_vq; i++) { > vq = hw->vqs[i]; > if (!vq) > @@ -563,9 +566,11 @@ virtio_free_queues(struct virtio_hw *hw) > } > > rte_free(vq); > + hw->vqs[i] = NULL; > } > > rte_free(hw->vqs); > + hw->vqs = NULL; > } > > static int > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index eb0a94a..24f82dc 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -1074,8 +1074,10 @@ rte_eth_dev_close(uint8_t port_id) > dev->data->dev_started = 0; > (*dev->dev_ops->dev_close)(dev); > > + dev->data->nb_rx_queues = 0; > rte_free(dev->data->rx_queues); > dev->data->rx_queues = NULL; > + dev->data->nb_tx_queues = 0; > rte_free(dev->data->tx_queues); > dev->data->tx_queues = NULL; > } > -- > 2.7.4 >
On Wed, Feb 22, 2017 at 10:24:13AM +0800, Yuanhan Liu wrote: > On Mon, Feb 20, 2017 at 10:04:46PM +0800, hanxueluo@126.com wrote: > > From: Huanle Han <hanxueluo@gmail.com> > > > > This commit fixs segment fault when rte_eth_dev_close() > > is called on a virtio dev more than once. > > Assigning zero after free to avoids freed memory to > > be accessed again. > > Thanks for the fix! And here are few more minor nits you might want be > awre of: > > - a fix patch needs a fixline (check http://dpdk.org/dev for howto just > in case you don't know) > > - if it fixes a fatal bug (like this one), it should also be picked (or > backported) to a specific stable release. In this case, you should add > Cc: stable@dpdk.org > > just before your SoB (Signed-off-by). Applied to dpdk-next-virtio with: Fixes: 69c80d4ef89b ("net/virtio: allocate queue at init stage") Cc: stable@dpdk.org Thanks. --yliu
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 8465e1a..b9565db 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -545,6 +545,9 @@ virtio_free_queues(struct virtio_hw *hw) int queue_type; uint16_t i; + if (hw->vqs == NULL) + return; + for (i = 0; i < nr_vq; i++) { vq = hw->vqs[i]; if (!vq) @@ -563,9 +566,11 @@ virtio_free_queues(struct virtio_hw *hw) } rte_free(vq); + hw->vqs[i] = NULL; } rte_free(hw->vqs); + hw->vqs = NULL; } static int diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index eb0a94a..24f82dc 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -1074,8 +1074,10 @@ rte_eth_dev_close(uint8_t port_id) dev->data->dev_started = 0; (*dev->dev_ops->dev_close)(dev); + dev->data->nb_rx_queues = 0; rte_free(dev->data->rx_queues); dev->data->rx_queues = NULL; + dev->data->nb_tx_queues = 0; rte_free(dev->data->tx_queues); dev->data->tx_queues = NULL; }