On Fri, Mar 10, 2017 at 10:13:32AM +0100, Gaëtan Rivet wrote:

On Thu, Mar 09, 2017 at 09:15:14AM +0000, Bruce Richardson wrote:

quoted

On Wed, Mar 08, 2017 at 11:54:02AM -0500, Neil Horman wrote:

quoted

On Wed, Mar 08, 2017 at 04:15:33PM +0100, Gaetan Rivet wrote:

quoted

This PMD intercepts and manages Ethernet device removal events issued by
slave PMDs and re-initializes them transparently when brought back so that
existing applications do not need to be modified to benefit from true
hot-plugging support.

The stacked PMD approach shares many similarities with the bonding PMD but
with a different purpose. While bonding provides the ability to group
several links into a single logical device for enhanced throughput and
supports fail-over at link level, this one manages the sudden disappearance
of the underlying device; it guarantees applications face a valid device in
working order at all times.

Why not just add this feature to the bonding pmd then?  A bond is perfectly
capable of handling the trivial case of a single underlying device, and adding
an option to make the underly slave 'persistent' seem both much simpler in terms
of implementation and code size, than adding an entire new pmd, along with its
supporting code.

Neil

@Neil
I don't know if you saw my answer to Bruce on the matter [1], it
partially adresses your point.

I did, and I think it asserts your points, but doesn't really address mine.  See
below. 

quoted

+1
I don't like the idea of having multiple PMDs in DPDK to handle
combining multiple other devices into one.

/Bruce

I understand the concern. Let's first put aside for the moment the link
grouping, which is only part of the fail-safe PMD function.

The fail-safe PMD at its core, provides an alternative paradigm, a new
proposal for a hot-plug functionality in a lightweight form-factor from a
user standpoint.

Ok, but lets be clear here, you're duplicating alot of functionality.  And for
that privlidge, there will be an additional 4000 lines of code to maintain.

The central question that I would like to tackle is this: why should we
require from our users declaring a bonding device to have hot-plug support?

We'll, strictly speaking, I suppose we don't have to require it.  But by that
same token, we don't need to do it in a separate PMD either, there are lots of
other options.

I took some time to illustrate a few modes of operation:

Fig. 1

   .-------------.
   | application |
   `------.------'
          |
     .----'-----.---------. <------ init, conf, Rx/Tx
     |          |         |
     |      .---|--.------|--. <--- conf, link check, Rx/Tx
     |      |   |  |      |  |
     v      |   v  v      v  v
.---------. | .-------. .------.
| bonding | | | ixgbe | | mlx4 |
`----.----' | `-------' `------'
     |      |
     `------'

Typical link fail-over.


Fig. 2

 .-------------.
 | application |
 `------.------'
        | <-------- init, conf, Rx/Tx
        v
  .-----------.
  | fail-safe |
  `-----.-----'
        |
    .---'----. <--- init, conf, dev check, Rx/Tx
    |        |
    v        v
.-------. .------.
| ixgbe | | mlx4 |
`-------' `------'

Typical automatic hot-plug handling with device fail-over.


Fig. 3

   .-------------.
   | application |
   `------.------'
          |
     .----'-----.-------------. <---------- init, conf, Rx/Tx
     |          |             |
     |      .---|--.----------|--. <------- conf, link check, Rx/Tx
     |      |   |  |          |  |
     v      |   v  v          v  v
.---------. | .-----------. .-----------.
| bonding | | | fail-safe | | fail-safe |
`----.----' | `-----.-----' `-----.-----'
     |      |       |             | <------ init, conf, dev check, Rx/Tx
     `------'       v             v
                .-------.      .------.
                | ixgbe |      | mlx4 |
                `-------'      `------'

Combination to provide link fail-over with automatic hot-plug handling.


Fig. 4

   .-------------.
   | application |
   `------.------'
          |
     .----'-----.-------------. <---------- init, conf, Rx/Tx
     |          |             |
     |      .---|--.----------|--. <------- conf, link check, Rx/Tx
     |      |   |  |          |  |
     v      |   v  v          v  v
.---------. | .-----------. .-----------.
| bonding | | | fail-safe | | fail-safe |
`----.----' | `-----.-----' `-----.-----'
     |      |       |             | <------- init, conf, dev check, Rx/Tx
     `------'       |             |
             .------'---.     .---'------.
             |          |     |          |
             v          v     v          v
      .--------. .--------. .--------. .--------.
      | mlx4 1 | | mlx4 2 | | mlx4 1 | | mlx4 2 |
      | port 1 | | port 1 | | port 2 | | port 2 |
      `--------' `--------' `--------' `--------'

Complex use case with link fail-over at port level and automatic hot-plug
handling with device fail-over.

Yes, I think we all understand the purpose of your PMD - its there to provide a
placeholder device that can respond to application requests in a sane manner,
until such time as real hardware is put in its place via a hot plug/failure.  Thats all
well and good, I'm saying there are better ways to go about this that can
provide the same functionality without having to add an extra 4k lines of code
to the project, many of which already exist.

1. LSC vs. RMV

 A link status change is a valid state for a device. It calls for
 specific responses, e.g. a link switch in a bonding device, without
 losing the general configuration of the port.

 The removal of a device calls for more than pausing operations and
 switching an active device. The party responsible for initializing the
 device should take care of closing it properly. If this party also
 wants to be able to restore the device if it was plugged back in, it
 would need be able to initialize it back and reconfigure its previous
 state.

 As we can see that in [Fig. 1], this responsibility lies upon the
 application.

Again, yes, I think we all see the benefit of centralizing hot plug operations,
no one is disagreing with that, its the code/functional duplication that is
concerning.

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.

 All of this hot-plug support, in order for the bonding to be aware of
 the status of some of its links. This seems like scope-creep.

But one of your primary use cases is bonding.  Its the nature of code to add
features as time goes on.

3. Fail-safe and hot-plug support

 An attach / detach (pseudo-hotplug) API exists in DPDK. The main  problem
of this API is that it does not offer reacting to a device  plug-out, only
triggering a device detaching from an application. This  is a completely
different approach from an application standpoint.

 The fail-safe PMD offers an out-of-the-box implementation of a newly
 proposed hot-plug API [2]. It allows a seamless integration to users
 for device removal support in their applications, without requiring
 evolutions [Fig. 2]. This flexibility allows it to be used as part of
 a bond [Fig. 3], [Fig. 4], while current bonding does not allow for
 detaching devices, even if it were to be considered hot-plug. There is
 no reason however to require declaring a bonding device to be able to
 use it, from a user perspective this seems backward.

 Both bonding and fail-safe PMDs deal with enslaving devices and
 acting upon their state changing. The bonding function performs at
 link level, while the hot-plug function deals with the device itself.
 I do not think this similarity justify merging both functions.
 Maintenance would be easier with clear, simpler functions implemented
 in separate PMDs.

My suggestion to you would be to re-architect this in the following way:

1) Augment the null pmd to accept arbitrary parameters.  Paremeters which are
not explicitly recognized are stored.  Create an api call via the ethdev api to
retrieve said arguments.

2) Augment the bonding driver so that, upon configuration, null drivers can be
added to the bond, and marked as replaceable in some fashion

3) Port your hotplug implementation into the bonding code (or make it a separate
library, not sure which yet).  On a hot plug event trigger, react by querying
the replaceable interfaces to see if the hot plugged nic is meant to replace an
existing one.  If a match is found, detach the null pmd instance, and add the
hot plugged interface.

The intent here is to provide you with the functionality that you have (which is
a good feature), without having to support an entire new pmd that replicates a
great deal of existing code an functionality.

Regards
Neil

[1]: http://dpdk.org/ml/archives/dev/2017-March/059446.html
[2]: http://dpdk.org/ml/archives/dev/2017-March/059217.html

-- 
Gaëtan Rivet
6WIND