From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id BD1B71B012 for ; Thu, 4 Jan 2018 03:51:38 +0100 (CET) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Jan 2018 18:51:35 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,505,1508828400"; d="scan'208";a="17322589" Received: from debian-xvivbkq.sh.intel.com ([10.67.104.226]) by orsmga003.jf.intel.com with ESMTP; 03 Jan 2018 18:51:32 -0800 Date: Thu, 4 Jan 2018 10:51:05 +0800 From: Tiwei Bie To: Xiao Wang Cc: dev@dpdk.org, yliu@fridaylinux.org, stephen@networkplumber.org Message-ID: <20180104025105.ho6vdzpjpmqb7gwp@debian-xvivbkq.sh.intel.com> References: <1512396128-119985-3-git-send-email-xiao.w.wang@intel.com> <1515051700-117262-1-git-send-email-xiao.w.wang@intel.com> <1515051700-117262-3-git-send-email-xiao.w.wang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1515051700-117262-3-git-send-email-xiao.w.wang@intel.com> User-Agent: NeoMutt/20170609 (1.8.3) Subject: Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: support GUEST ANNOUNCE X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Jan 2018 02:51:39 -0000 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