[dpdk-dev,v2] virtio: Fix crash issue for secondary process

Message ID 1427438150-6673-1-git-send-email-changchun.ouyang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Ouyang Changchun March 27, 2015, 6:35 a.m. UTC
  It needs Rx function even in the case of secondary process, and it also needs check if
it supports mergeable feature or not.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---

Changes in v2:
  -- Check if it supports mergeable or not for the secondary process.

 lib/librte_pmd_virtio/virtio_ethdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
  

Comments

Huawei Xie March 27, 2015, 7:37 a.m. UTC | #1
On 3/27/2015 2:35 PM, Ouyang, Changchun wrote:
> It needs Rx function even in the case of secondary process, and it also needs check if
> it supports mergeable feature or not.
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
>
> Changes in v2:
>   -- Check if it supports mergeable or not for the secondary process.
>
>  lib/librte_pmd_virtio/virtio_ethdev.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
> index 603be2d..0156b56 100644
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -1115,8 +1115,13 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>  	eth_dev->dev_ops = &virtio_eth_dev_ops;
>  	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
>  
> -	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> +		if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF))
> +			eth_dev->rx_pkt_burst = &virtio_recv_mergeable_pkts;
> +		else
> +			eth_dev->rx_pkt_burst = &virtio_recv_pkts;
>  		return 0;
> +	}
>  
>  	/* Allocate memory for storing MAC addresses */
>  	eth_dev->data->mac_addrs = rte_zmalloc("virtio", ETHER_ADDR_LEN, 0);
Acked-by: Huawei Xie <huawei.xie@intel.com>
  
Thomas Monjalon March 27, 2015, 8:57 a.m. UTC | #2
2015-03-27 14:35, Ouyang Changchun:
> It needs Rx function even in the case of secondary process, and it also needs check if
> it supports mergeable feature or not.
> 
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
> 
> Changes in v2:
>   -- Check if it supports mergeable or not for the secondary process.

> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -1115,8 +1115,13 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>  	eth_dev->dev_ops = &virtio_eth_dev_ops;
>  	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
>  
> -	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> +		if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF))
> +			eth_dev->rx_pkt_burst = &virtio_recv_mergeable_pkts;
> +		else
> +			eth_dev->rx_pkt_burst = &virtio_recv_pkts;

You are duplicating code, making it error prone for later maintenance.
Please merge primary and secondary cases above the if() block.

>  		return 0;
> +	}
>  
>  	/* Allocate memory for storing MAC addresses */
>  	eth_dev->data->mac_addrs = rte_zmalloc("virtio", ETHER_ADDR_LEN, 0);
>
  

Patch

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 603be2d..0156b56 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -1115,8 +1115,13 @@  eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	eth_dev->dev_ops = &virtio_eth_dev_ops;
 	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
 
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF))
+			eth_dev->rx_pkt_burst = &virtio_recv_mergeable_pkts;
+		else
+			eth_dev->rx_pkt_burst = &virtio_recv_pkts;
 		return 0;
+	}
 
 	/* Allocate memory for storing MAC addresses */
 	eth_dev->data->mac_addrs = rte_zmalloc("virtio", ETHER_ADDR_LEN, 0);