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

  On 3/21/2015 5:59 AM, Thomas Monjalon wrote:

2015-03-19 09:45, Ouyang Changchun:


It definitely needs Rx function even in the case of secondary process, so put
the assignment a bit earlier to make sure of it.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com><mailto:changchun.ouyang@intel.com>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

uninitialized, even like eth_dev->data->mac_addrs isn't allocated., also

like mergeable, feature, etc.

Secondary process will not work unless they never touch those fields.



Prefer we have a clean fix. Customer could apply that one line of fix if

they need this urgently.

I am wondering whether other PMDS have the same issue for the second process.





                hw->vtnet_hdr_size = sizeof(struct virtio_net_hdr_mrg_rxbuf);
-       } else {
-               eth_dev->rx_pkt_burst = &virtio_recv_pkts;
+       } else
                hw->vtnet_hdr_size = sizeof(struct virtio_net_hdr);
-       }

        /* Copy the permanent MAC address to: virtio_hw */
        virtio_get_hwaddr(hw);
  

On 3/23/2015 10:33 PM, Xie, Huawei wrote:
> On 3/21/2015 5:59 AM, Thomas Monjalon wrote:
>
> 2015-03-19 09:45, Ouyang Changchun:
>
>
> It definitely needs Rx function even in the case of secondary process, so put
> the assignment a bit earlier to make sure of it.
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com><mailto:changchun.ouyang@intel.com>
> ---
>  lib/librte_pmd_virtio/virtio_ethdev.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
> index 603be2d..ad24cf2 100644
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -1113,6 +1113,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>         RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr));
>
>         eth_dev->dev_ops = &virtio_eth_dev_ops;
> +       eth_dev->rx_pkt_burst = &virtio_recv_pkts;
>         eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
>
>         if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> @@ -1148,10 +1149,8 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>         if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
>                 eth_dev->rx_pkt_burst = &virtio_recv_mergeable_pkts;
>
>
>
> Why the mergeable buffers case is not handled for secondary processes?
>
> Forgot to CC my comment to dpdk.org.
> There are many parts of eth_dev for the secondary process still
>
> uninitialized, even like eth_dev->data->mac_addrs isn't allocated., also
>
> like mergeable, feature, etc.
>
> Secondary process will not work unless they never touch those fields.
>

This comment is not right, but there is indeed a possible minor issue here.
eth_dev_data array is shared across multiple processes.
The eth_dev_data is the same for the same eth device unless the port id
allocated to it is the same, i.e, the devices are scanned in the same
order in all processes.
For instance, if we blacklist a device in one process, the port id  will
be then different.


>
> Prefer we have a clean fix. Customer could apply that one line of fix if
>
> they need this urgently.
>
> I am wondering whether other PMDS have the same issue for the second process.
>
>
>
>
>
>                 hw->vtnet_hdr_size = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> -       } else {
> -               eth_dev->rx_pkt_burst = &virtio_recv_pkts;
> +       } else
>                 hw->vtnet_hdr_size = sizeof(struct virtio_net_hdr);
> -       }
>
>         /* Copy the permanent MAC address to: virtio_hw */
>         virtio_get_hwaddr(hw);
>
>
>
>
>
>
>
>
>