Hi Xiao,
On Wed, Jan 03, 2018 at 11:41:40PM -0800, Xiao Wang wrote:
[...]
> +static int
> +virtio_dev_pause(struct rte_eth_dev *dev)
> +{
> + struct virtio_hw *hw = dev->data->dev_private;
> +
> + if (hw->started == 0)
> + return -1;
> + hw->started = 0;
> + /*
> + * Prevent the worker thread from touching queues to avoid contention,
> + * 1 ms should be enough for the ongoing Tx function to finish.
> + */
> + rte_delay_ms(1);
> + return 0;
> +}
> +
> +static void
> +virtio_dev_resume(struct rte_eth_dev *dev)
> +{
> + struct virtio_hw *hw = dev->data->dev_private;
> +
> + hw->started = 1;
> +}
Based on your current implementation, hw->state_lock needs to
be held during a call of virtio_dev_pause()..virtio_dev_resume().
So I think the code would be more readable and much easier to
use if we take the lock in virtio_dev_pause() and release the
lock in virtio_dev_resume().
> +
> +static void
> +virtio_notify_peers(struct rte_eth_dev *dev)
> +{
> + struct virtio_hw *hw = dev->data->dev_private;
> + struct virtnet_tx *txvq = dev->data->tx_queues[0];
> + struct virtnet_rx *rxvq = dev->data->rx_queues[0];
> +
> + hw->rarp_buf[0] = rte_mbuf_raw_alloc(rxvq->mpool);
> + if (hw->rarp_buf[0] == NULL) {
> + PMD_DRV_LOG(ERR, "first mbuf allocate failed");
> + return;
> + }
> +
> + if (make_rarp_packet(hw->rarp_buf[0],
> + (struct ether_addr *)hw->mac_addr)) {
> + rte_pktmbuf_free(hw->rarp_buf[0]);
> + return;
> + }
> +
> + /* If virtio port just stopped, no need to send RARP */
> + if (virtio_dev_pause(dev) < 0) {
> + rte_pktmbuf_free(hw->rarp_buf[0]);
> + return;
> + }
> +
> + dev->tx_pkt_burst(txvq, hw->rarp_buf, 1);
You have already provided virtio_dev_pause()/virtio_dev_resume().
I think you can also make this part generic and provide an inject
function, e.g.:
uint16_t
virtio_inject_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t
nb_pkts)
{
......
txvq->inject_pkts = tx_pkts;
nb_tx = dev->tx_pkt_burst(txvq, tx_pkts, nb_pkts);
txvq->inject_pkts = NULL;
return nb_tx;
}
And you can introduce virtio_dev_pause()/virtio_dev_resume()/
virtio_injec... in a separate patch. And introduce the GUEST
ANNOUNCE support in the third patch.
Thanks,
Tiwei
