DPDK patches and discussions
 help / color / mirror / Atom feed
From: Nithin Dabilpuram <nithind1988@gmail.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: dev@dpdk.org, Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Jingjing Wu <jingjing.wu@intel.com>,
	Bernard Iremonger <bernard.iremonger@intel.com>
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface
Date: Mon, 20 May 2019 18:20:53 +0530	[thread overview]
Message-ID: <20190520125053.GA1399@gmail.com> (raw)
In-Reply-To: <3849744.ELThORf4eq@xps>

On Fri, May 17, 2019 at 10:59:38AM +0200, Thomas Monjalon wrote:
> 17/05/2019 10:55, Nithin Dabilpuram:
> > On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote:
> > > 15/05/2019 08:52, Nithin Dabilpuram:
> > > > Hi Thomas,
> > > > On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote:
> > > > > Hi,
> > > > > 
> > > > > 13/05/2019 13:21, Nithin Dabilpuram:
> > > > > > With the latest published interface of
> > > > > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(),
> > > > > > rte_eth_dev_close() would cleanup all the data structures of
> > > > > > port's eth dev leaving the device common resource intact
> > > > > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags.
> > > > > > So "port detach" (~hotplug remove) should be able to work,
> > > > > > with device identifier like "port attach" as eth_dev could have
> > > > > > been closed already and rte_eth_devices[port_id] reused.
> > > > > 
> > > > > "port attach" uses devargs as identifier because there
> > > > > is no port id before creating it. But "detach port" uses
> > > > > logically the port id to close.
> > > > 
> > > > But if "port close" was already called on that port,
> > > > eth_dev->state would be set as RTE_ETH_DEV_UNUSED and
> > > > that port id could be reused.
> > > > So after "port close" if we call "port detach", isn't it
> > > > incorrect to use the same port id ?
> > > 
> > > Yes it is incorrect to close a port which is already closed :)
> > > 
> > > > > > This change alters "port detach" cmdline interface to
> > > > > > work with device identifier like "port attach".
> > > > > 
> > > > > The word "port" means an ethdev port, so it should be
> > > > > referenced with a port id.
> > > > > If you want to close an EAL rte_device, then you should
> > > > > rename the command.
> > > > > But testpmd purpose should be to work with ethdev ports only.
> > > > 
> > > > Renaming the command to "detach <identifier>" ?
> > > 
> > > Yes something like that.
> > > But why do you want to manage rte_device in testpmd?
> > > Being able to close ports in not enough?
> > > Please describe a scenario.
> > >
> > 
> > We just want to support testing hotplug detach along with
> > hotplug attach from testpmd. Currently there is no way to detach
> > if we close the port first.
> 
> OK
So can I send next revision with command renamed to "detach <identifier>" ?
> 
> > Another reason is that in our new PMD, for detaching one specific port,
> > we need more than one try as the PMD might return -EAGAIN.
> > So with the current "port detach" implementation, after closing the port,
> > if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to
> > try it again.
> 
> This is a bug.
> Should we catch -EAGAIN somewhere?

It is already caught in local_dev_remove() and
rte_dev_remove() fails. Only problem as I said below is
in testpmd if first call to detach_port_device() i.e handler of "port detach", 
rte_dev_remove() returns -EAGAIN and PMD cleaned up the resources partially like eth_dev
resources, the second time call cannot work port_id will not be valid anymore.

> 
> 

  reply	other threads:[~2019-05-20 12:51 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-13 11:21 Nithin Dabilpuram
2019-05-13 11:21 ` Nithin Dabilpuram
2019-05-14 15:39 ` Thomas Monjalon
2019-05-14 15:39   ` Thomas Monjalon
2019-05-15  6:52   ` Nithin Dabilpuram
2019-05-15  6:52     ` Nithin Dabilpuram
2019-05-15  7:27     ` Thomas Monjalon
2019-05-15  7:27       ` Thomas Monjalon
2019-05-17  8:55       ` Nithin Dabilpuram
2019-05-17  8:59         ` Thomas Monjalon
2019-05-20 12:50           ` Nithin Dabilpuram [this message]
2019-05-29  8:16             ` Nithin Dabilpuram
2019-06-25  4:24               ` Nithin Dabilpuram
2019-07-02 15:58               ` Yigit, Ferruh
2019-07-03  5:05                 ` Nithin Dabilpuram
2019-07-10 13:07 ` [dpdk-dev] [PATCH v2] app/testpmd: add device related cmds Nithin Dabilpuram
2019-07-16 18:30   ` Ferruh Yigit
2019-07-17  8:08     ` [dpdk-dev] [EXT] " Nithin Kumar Dabilpuram
2019-07-17 12:30 ` [dpdk-dev] [PATCH v3] " Nithin Dabilpuram
2019-07-17 16:51   ` Ferruh Yigit
2019-07-18  5:27     ` [dpdk-dev] [EXT] " Nithin Kumar Dabilpuram
2019-07-19 19:00       ` Ferruh Yigit
2019-07-22  6:01         ` Hemant Agrawal
2019-07-22  6:15           ` Nithin Kumar Dabilpuram
2019-07-22 16:04             ` Ferruh Yigit
2019-07-17 16:54   ` [dpdk-dev] " Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190520125053.GA1399@gmail.com \
    --to=nithind1988@gmail.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=jingjing.wu@intel.com \
    --cc=thomas@monjalon.net \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).