[v4,4/8] net/netvsc: fix invalid rte_free on dev_close
diff mbox series

Message ID 20200331171404.23596-5-stephen@networkplumber.org
State Accepted, archived
Delegated to: Ferruh Yigit
Headers show
Series
  • net/netvsc: bug fixes
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger March 31, 2020, 5:14 p.m. UTC
The netvsc PMD was putting the mac address in private data but the
core rte_ethdev doesn't allow that it. It has to be in rte_malloc'd
memory or the a message will be printed on shutdown/close.
 EAL: Invalid memory

Fixes: f8279f47dd89 ("net/netvsc: fix crash in secondary process")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/netvsc/hn_ethdev.c | 16 ++++++++++------
 drivers/net/netvsc/hn_var.h    |  2 --
 2 files changed, 10 insertions(+), 8 deletions(-)

Comments

Ferruh Yigit April 6, 2020, 4 p.m. UTC | #1
On 3/31/2020 6:14 PM, Stephen Hemminger wrote:
> The netvsc PMD was putting the mac address in private data but the
> core rte_ethdev doesn't allow that it. It has to be in rte_malloc'd
> memory or the a message will be printed on shutdown/close.

It doesn't have to be rte_malloc'ed. Setting 'eth_dev->data->mac_addrs' to null
in 'hn_dev_close()' should fix it.

But dynamically allocating it also works if that is what preferred.

>  EAL: Invalid memory
> 
> Fixes: f8279f47dd89 ("net/netvsc: fix crash in secondary process")
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/netvsc/hn_ethdev.c | 16 ++++++++++------
>  drivers/net/netvsc/hn_var.h    |  2 --
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
> index ac6610838008..05f1a25a1abc 100644
> --- a/drivers/net/netvsc/hn_ethdev.c
> +++ b/drivers/net/netvsc/hn_ethdev.c
> @@ -134,8 +134,6 @@ eth_dev_vmbus_allocate(struct rte_vmbus_device *dev, size_t private_data_size)
>  static void
>  eth_dev_vmbus_release(struct rte_eth_dev *eth_dev)
>  {
> -	/* mac_addrs must not be freed alone because part of dev_private */
> -	eth_dev->data->mac_addrs = NULL;
>  	/* free ether device */
>  	rte_eth_dev_release_port(eth_dev);
>  
> @@ -937,9 +935,6 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
>  	eth_dev->tx_pkt_burst = &hn_xmit_pkts;
>  	eth_dev->rx_pkt_burst = &hn_recv_pkts;
>  
> -	/* Since Hyper-V only supports one MAC address, just use local data */
> -	eth_dev->data->mac_addrs = &hv->mac_addr;
> -
>  	/*
>  	 * for secondary processes, we don't initialize any further as primary
>  	 * has already done this work.
> @@ -947,6 +942,15 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>  		return 0;
>  
> +	/* Since Hyper-V only supports one MAC address */
> +	eth_dev->data->mac_addrs = rte_calloc("hv_mac", HN_MAX_MAC_ADDRS,
> +					      sizeof(struct rte_ether_addr), 0);
> +	if (eth_dev->data->mac_addrs == NULL) {
> +		PMD_INIT_LOG(ERR,
> +			     "Failed to allocate memory store MAC addresses");
> +		return -ENOMEM;
> +	}
> +
>  	hv->vmbus = vmbus;
>  	hv->rxbuf_res = &vmbus->resource[HV_RECV_BUF_MAP];
>  	hv->chim_res  = &vmbus->resource[HV_SEND_BUF_MAP];
> @@ -989,7 +993,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
>  	if (err)
>  		goto failed;
>  
> -	err = hn_rndis_get_eaddr(hv, hv->mac_addr.addr_bytes);
> +	err = hn_rndis_get_eaddr(hv, eth_dev->data->mac_addrs->addr_bytes);
>  	if (err)
>  		goto failed;
>  
> diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
> index 822d737bd3cc..b4c61717379f 100644
> --- a/drivers/net/netvsc/hn_var.h
> +++ b/drivers/net/netvsc/hn_var.h
> @@ -139,8 +139,6 @@ struct hn_data {
>  	uint8_t		rss_key[40];
>  	uint16_t	rss_ind[128];
>  
> -	struct rte_ether_addr mac_addr;
> -
>  	struct rte_eth_dev_owner owner;
>  	struct rte_intr_handle vf_intr;
>  
>

Patch
diff mbox series

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index ac6610838008..05f1a25a1abc 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -134,8 +134,6 @@  eth_dev_vmbus_allocate(struct rte_vmbus_device *dev, size_t private_data_size)
 static void
 eth_dev_vmbus_release(struct rte_eth_dev *eth_dev)
 {
-	/* mac_addrs must not be freed alone because part of dev_private */
-	eth_dev->data->mac_addrs = NULL;
 	/* free ether device */
 	rte_eth_dev_release_port(eth_dev);
 
@@ -937,9 +935,6 @@  eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	eth_dev->tx_pkt_burst = &hn_xmit_pkts;
 	eth_dev->rx_pkt_burst = &hn_recv_pkts;
 
-	/* Since Hyper-V only supports one MAC address, just use local data */
-	eth_dev->data->mac_addrs = &hv->mac_addr;
-
 	/*
 	 * for secondary processes, we don't initialize any further as primary
 	 * has already done this work.
@@ -947,6 +942,15 @@  eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	/* Since Hyper-V only supports one MAC address */
+	eth_dev->data->mac_addrs = rte_calloc("hv_mac", HN_MAX_MAC_ADDRS,
+					      sizeof(struct rte_ether_addr), 0);
+	if (eth_dev->data->mac_addrs == NULL) {
+		PMD_INIT_LOG(ERR,
+			     "Failed to allocate memory store MAC addresses");
+		return -ENOMEM;
+	}
+
 	hv->vmbus = vmbus;
 	hv->rxbuf_res = &vmbus->resource[HV_RECV_BUF_MAP];
 	hv->chim_res  = &vmbus->resource[HV_SEND_BUF_MAP];
@@ -989,7 +993,7 @@  eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	if (err)
 		goto failed;
 
-	err = hn_rndis_get_eaddr(hv, hv->mac_addr.addr_bytes);
+	err = hn_rndis_get_eaddr(hv, eth_dev->data->mac_addrs->addr_bytes);
 	if (err)
 		goto failed;
 
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index 822d737bd3cc..b4c61717379f 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -139,8 +139,6 @@  struct hn_data {
 	uint8_t		rss_key[40];
 	uint16_t	rss_ind[128];
 
-	struct rte_ether_addr mac_addr;
-
 	struct rte_eth_dev_owner owner;
 	struct rte_intr_handle vf_intr;