DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Bruce Richardson" <bruce.richardson@intel.com>,
	"Gaëtan Rivet" <gaetan.rivet@6wind.com>
Cc: Neil Horman <nhorman@tuxdriver.com>,
	dev@dpdk.org, Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	techboard@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 00/13] introduce fail-safe PMD
Date: Wed, 15 Mar 2017 12:15:56 +0100	[thread overview]
Message-ID: <2220671.FCXKFdQQ2A@xps13> (raw)
In-Reply-To: <20170315032853.GA366048@bricha3-MOBL3.ger.corp.intel.com>

2017-03-15 03:28, Bruce Richardson:
> On Tue, Mar 14, 2017 at 03:49:47PM +0100, Gaëtan Rivet wrote:
> > > > 2. Bonding and link availability
> > > >
> > > >  The hot-plug functionality is not a core function of the bonding PMD.
> > > >  It is only interested in knowing if the link is active or not.
> > > >
> > > Currently, yes.  The suggestion was that you augment the bonding driver so that
> > > hot plug is a core function of bonding.
> > > 
> > > >  Adding the device persistence to the bonding PMD would mean adding the
> > > >  ability to flexibly parse device definitions to cope with plug-ins in
> > > >  evolving busses (PCI hot-plug could mean changing bus addresses), being
> > > >  able to emulate the EAL and the ether layer and to properly store the
> > > >  device configuration.  This means formally describing the life of a
> > > >  device in a DPDK application from start to finish.
> > > >
> > > Which seems to me to be exactly what your PMD does.  I don't see why its
> > > fundamentally harder to do that in an existing pmd, than it is in a new one.
> > > 
> > 
> > Indeed it does. I must emphasize the "formally describe the life of a
> > device". The hot-plug functionality goes beyong the link-level check.
> > The description of a device from a DPDK standpoint is complete in the
> > fail-safe PMD. The state-machine must be able to describe the entire
> > life of a device, from the devargs parsing to its start-up.
> > 
> > We cannot reuse the existing bonding PMD architecture for this.  We
> > would have to rewrite the bonding PMD from the ground up for the
> > hot-plug function. Because it is actually a different approach to
> > managing the slaves.
> > 
> > This is what I wanted to illustrate in [Fig. 1] and [Fig. 2]:
> > 
> > - In the bonding, the init and configuration steps are still the
> >  responsibility of the application and no one else. The bonding PMD
> >  captures the device, re-applies its configuration upon dev_configure()
> >  which is actually re-applying part of the configuration already  present
> > within the slave eth_dev (cf rte_eth_dev_config_restore).
> > 
> > - In the fail-safe, the init and configuration are both the
> >  responsibilities of the fail-safe PMD itself, not the application
> >  anymore. This handling of these responsibilities in lieu of the
> >  application is the whole point of the "deferred hot-plug" support, of
> >  proposing a simple implementation to the user.
> > 
> > This change in responsibilities is the bulk of the fail-safe code. It
> > would have to be added as-is to the bonding. Verifying the correctness
> > of the sync of the initialization phase (acceptable states of a device
> > following several events registered by the fail-safe PMD) and the
> > configuration items between the state the application believes it is in
> > and the fail-safe knows it is in, is the bulk of the fail-safe code.
> > 
> > This function is not overlapping with that of the bonding. The reason I
> > did not add this whole architecture to the bonding is that when I tried
> > to do so, I found that I only had two possibilities:
> > 
> > - The current slave handling path is kept, and we only add a new one
> >  with additional functionalities: full init and conf handling with
> >  extended parsing capabilities.
> > 
> > - The current slave handling is scraped and replaced entirely by the new
> >  slave management. The old capturing of existing device is not done
> >  anymore.
> > 
> > The first solution is not acceptable, because we effectively end-up with
> > a maintenance nightmare by having to validate two types of slaves with
> > differing capabilities, differing initialization paths and differing
> > configuration code.  This is extremely awkward and architecturally
> > unsound. This is essentially the same as having the exact code of the
> > fail-safe as an aside in the bonding, maintening exactly the same
> > breadth of code while having muddier interfaces and organization.
> > 
> > The second solution is not acceptable, because we are bending the whole
> > existing bonding API to our whim. We could just as well simply rename
> > the fail-safe PMD as bonding, add a few grouping capabilities and call
> > it a day. This is not acceptable for users.
> > 
> If the first solution is indeed not an option, why do you think this
> second one would be unacceptable for users? If the functionality remains
> the same, I don't see how it matters much for users which driver
> provides it or where the code originates.
> 
> Despite all the discussion, it still just doesn't make sense to me to
> have more than one DPDK driver to handle failover - be it link or
> device. If nothing else, it's going to be awkward to explain to users
> that if they want fail-over for when a link goes down they have to use
> driver A, but if they want fail-over when a NIC gets hotplugged they use
> driver B, and if they want both kinds of failover - which would surely
> be the expected case - they need to use both drivers. The usability is
> a problem here.

It seems everybody agrees on the need for the failsafe code.
We are just discussing the right place to implement it.

Gaetan, moving this code in the bonding PMD means replacing the bonding
API design by the failsafe design, right?
With the failsafe design in the bonding PMD, is it possible to keep other
bonding features?

In case we do not have a consensus in the following days, I suggest to add
this topic in the next techboard meeting agenda.

  reply	other threads:[~2017-03-15 11:15 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-03 15:40 [dpdk-dev] [PATCH 00/12] " Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 01/12] ethdev: save VLAN filter setting Gaetan Rivet
2017-03-03 17:33   ` Stephen Hemminger
2017-03-03 15:40 ` [dpdk-dev] [PATCH 02/12] ethdev: add flow API rule copy function Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 03/12] ethdev: add deferred intermediate device state Gaetan Rivet
2017-03-03 17:34   ` Stephen Hemminger
2017-03-03 15:40 ` [dpdk-dev] [PATCH 04/12] pci: expose device detach routine Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 05/12] pci: expose parse and probe routines Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 06/12] net/failsafe: add fail-safe PMD Gaetan Rivet
2017-03-03 17:38   ` Stephen Hemminger
2017-03-06 14:19     ` Gaëtan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 07/12] net/failsafe: add plug-in support Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 08/12] net/failsafe: add flexible device definition Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 09/12] net/failsafe: support flow API Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 10/12] net/failsafe: support offload capabilities Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 11/12] net/failsafe: add fast burst functions Gaetan Rivet
2017-03-03 15:40 ` [dpdk-dev] [PATCH 12/12] net/failsafe: support device removal Gaetan Rivet
2017-03-03 16:14 ` [dpdk-dev] [PATCH 00/12] introduce fail-safe PMD Bruce Richardson
2017-03-06 13:53   ` Gaëtan Rivet
2017-03-03 17:27 ` Stephen Hemminger
2017-03-08 15:15 ` [dpdk-dev] [PATCH v2 00/13] " Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 01/13] ethdev: save VLAN filter setting Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 02/13] ethdev: add flow API rule copy function Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 03/13] ethdev: add deferred intermediate device state Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 04/13] pci: expose device detach routine Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 05/13] pci: expose parse and probe routines Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 06/13] net/failsafe: add fail-safe PMD Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 07/13] net/failsafe: add plug-in support Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 08/13] net/failsafe: add flexible device definition Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 09/13] net/failsafe: support flow API Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 10/13] net/failsafe: support offload capabilities Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 11/13] net/failsafe: add fast burst functions Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 12/13] net/failsafe: support device removal Gaetan Rivet
2017-03-08 15:15   ` [dpdk-dev] [PATCH v2 13/13] net/failsafe: support link status change event Gaetan Rivet
2017-03-08 16:54   ` [dpdk-dev] [PATCH v2 00/13] introduce fail-safe PMD Neil Horman
2017-03-09  9:15     ` Bruce Richardson
2017-03-10  9:13       ` Gaëtan Rivet
2017-03-10 22:43         ` Neil Horman
2017-03-14 14:49           ` Gaëtan Rivet
2017-03-15  3:28             ` Bruce Richardson
2017-03-15 11:15               ` Thomas Monjalon [this message]
2017-03-15 14:25                 ` Gaëtan Rivet
2017-03-16 20:50                   ` Neil Horman
2017-03-17 10:56                     ` Gaëtan Rivet
2017-03-18 19:51                       ` Neil Horman
2017-03-20 15:00   ` Thomas Monjalon
2017-05-17 12:50     ` Ferruh Yigit
2017-05-17 16:59       ` Gaëtan Rivet
2017-03-23 13:01   ` 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=2220671.FCXKFdQQ2A@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=gaetan.rivet@6wind.com \
    --cc=nhorman@tuxdriver.com \
    --cc=techboard@dpdk.org \
    /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).