From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot0-f181.google.com (mail-ot0-f181.google.com [74.125.82.181]) by dpdk.org (Postfix) with ESMTP id EE6B011C5 for ; Thu, 23 Mar 2017 14:32:16 +0100 (CET) Received: by mail-ot0-f181.google.com with SMTP id i1so187233453ota.3 for ; Thu, 23 Mar 2017 06:32:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Z29ywpiChCFoTryDWSaeVmrHzh1ZYQifEQg+LmIToQo=; b=go5GoH1sugOOzV+06LCxjjNwB48fPYS7BGk3sWjtvjyi0hC+UBCYDa46+AOUWxgAYb 8YeSXjvAtP1lxK6XtZU+2YIvKOal6PLgtBlzuHBv3u91LMpzSWRCa+E3JC+frqhJhiJR Tp7amOd4NzLa41PuRTVZEVybWZoElYMtEsN/YiIF5Ndc0wca9Awxu9jxCahQv1C99Nwm ERpAzMp9kk/1dadXY3L7qhS5EctymPqoGzuS8jtKuYtikeSlej1U4JFxnRcp9RK3s8+p PENT1ocrN6iUtW8IGVo1ZsJ4eO3/fUgSfjDrWQ/2+F9Z5JrOzIg52dtQtgQfJPxiYYeT RLMw== X-Gm-Message-State: AFeK/H1VOZzlW5G87rlZwMcz7t0RkBsD51LEQy7rih5R4gGC7s0taQ6mumt1aQGKYvPKFx3db/qyXk/yw69xLy50 X-Received: by 10.157.39.37 with SMTP id r34mr1285459ota.120.1490275935640; Thu, 23 Mar 2017 06:32:15 -0700 (PDT) MIME-Version: 1.0 Received: by 10.202.94.86 with HTTP; Thu, 23 Mar 2017 06:32:14 -0700 (PDT) In-Reply-To: <20170323113716.57e27591@glumotte.dev.6wind.com> References: <20170309205119.28170-1-bmcfall@redhat.com> <20170315180226.5999-1-bmcfall@redhat.com> <20170315180226.5999-2-bmcfall@redhat.com> <20170323113716.57e27591@glumotte.dev.6wind.com> From: Billy McFall Date: Thu, 23 Mar 2017 09:32:14 -0400 Message-ID: To: Olivier MATZ Cc: thomas.monjalon@6wind.com, wenzhuo.lu@intel.com, dev@dpdk.org Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v7 1/3] ethdev: new API to free consumed buffers in Tx ring 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, 23 Mar 2017 13:32:17 -0000 Thank you for your comments. See inline. On Thu, Mar 23, 2017 at 6:37 AM, Olivier MATZ wrote: > Hi Billy, > > On Wed, 15 Mar 2017 14:02:24 -0400, Billy McFall > wrote: > > Add a new API to force free consumed buffers on Tx ring. API will return > > the number of packets freed (0-n) or error code if feature not supported > > (-ENOTSUP) or input invalid (-ENODEV). > > > > Signed-off-by: Billy McFall > > --- > > doc/guides/conf.py | 7 +++++-- > > doc/guides/nics/features/default.ini | 4 +++- > > doc/guides/prog_guide/poll_mode_drv.rst | 28 > ++++++++++++++++++++++++++++ > > doc/guides/rel_notes/release_17_05.rst | 7 ++++++- > > lib/librte_ether/rte_ethdev.c | 14 ++++++++++++++ > > lib/librte_ether/rte_ethdev.h | 31 > +++++++++++++++++++++++++++++++ > > 6 files changed, 87 insertions(+), 4 deletions(-) > > > > [...] > > > --- a/doc/guides/prog_guide/poll_mode_drv.rst > > +++ b/doc/guides/prog_guide/poll_mode_drv.rst > > @@ -249,6 +249,34 @@ One descriptor in the TX ring is used as a sentinel > to avoid a hardware race con > > > > When configuring for DCB operation, at port initialization, both > the number of transmit queues and the number of receive queues must be set > to 128. > > > > +Free Tx mbuf on Demand > > +~~~~~~~~~~~~~~~~~~~~~~ > > + > > +Many of the drivers don't release the mbuf back to the mempool, or > local cache, immediately after the packet has been > > +transmitted. > > +Instead, they leave the mbuf in their Tx ring and either perform a bulk > release when the ``tx_rs_thresh`` has been > > +crossed or free the mbuf when a slot in the Tx ring is needed. > > + > > +An application can request the driver to release used mbufs with the > ``rte_eth_tx_done_cleanup()`` API. > > +This API requests the driver to release mbufs that are no longer in > use, independent of whether or not the > > +``tx_rs_thresh`` has been crossed. > > +There are two scenarios when an application may want the mbuf released > immediately: > > + > > +* When a given packet needs to be sent to multiple destination > interfaces (either for Layer 2 flooding or Layer 3 > > + multi-cast). > > + One option is to make a copy of the packet or a copy of the header > portion that needs to be manipulated. > > + A second option is to transmit the packet and then poll the > ``rte_eth_tx_done_cleanup()`` API until the reference > > + count on the packet is decremented. > > + Then the same packet can be transmitted to the next destination > interface. > > By reading this paragraph, it's not so clear to me that the packet > that will be transmitted on all interfaces will be different from > one port to another. > > Maybe it could be reworded to insist on that? > > What if I add the following sentence: Then the same packet can be transmitted to the next destination interface. + The application is still responsible for managing any packet manipulations needed between the different destination + interfaces, but a packet copy can be avoided. > > > + > > +* If an application is designed to make multiple runs, like a packet > generator, and one run has completed. > > + The application may want to reset to a clean state. > > I'd reword into: > > Some applications are designed to make multiple runs, like a packet > generator. > Between each run, the application may want to reset to a clean state. > > What do you mean by "clean state"? All mbufs returned into the mempools? > Why would a packet generator need that? For performance? > > Reworded as you suggested, then attempted to explain a 'clean state'. Also reworded the last sentence a little. + * Some applications are designed to make multiple runs, like a packet generator. + For performance reasons and consistency between runs, the application may want to reset back to an initial state + between each run, where all mbufs are returned to the mempool. + In this case, it can call the ``rte_eth_tx_done_cleanup()`` API for each destination interface it has been using + to request it to release of all its used mbufs. > Also, do we want to ensure that all packets are actually transmitted? > Added an additional sentence to indicate that this API doesn't manage whether or not the packet has been transmitted. Then the same packet can be transmitted to the next destination interface. The application is still responsible for managing any packet manipulations needed between the different destination interface, but a packet copy can be avoided. + This API is independent of whether the packet was transmitted or dropped, only that the mbuf is no longer in use by + the interface. > Can we do that with this API or should we use another API like > rte_eth_tx_descriptor_status() [1] ? > > [1] http://dpdk.org/dev/patchwork/patch/21549/ > > I read through this patch. This API doesn't indicate if the packet was transmitted or dropped (I think that is what you were asking). This API could be used by the application to determine if the mbuf has been freed, as opposed to polling the rte_mbuf_refcnt_read() for a change in value. Did I miss your point? > > Thanks, > Olivier > Thanks, Billy McFall