DPDK patches and discussions
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: "Doherty, Declan" <declan.doherty@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 0/4] Link Bonding Library
Date: Fri, 6 Jun 2014 10:54:26 -0400	[thread overview]
Message-ID: <20140606145426.GA2543@hmsreliant.think-freely.org> (raw)
In-Reply-To: <345C63BAECC1AD42A2EC8C63AFFC3ADC13D37331@IRSMSX101.ger.corp.intel.com>

On Fri, Jun 06, 2014 at 08:23:31AM +0000, Doherty, Declan wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, May 29, 2014 12:34 PM
> > To: Doherty, Declan
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 0/4] Link Bonding Library
> > 
> > On Thu, May 29, 2014 at 10:33:00AM +0000, Doherty, Declan wrote:
> > > -----Original Message-----
> > > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > > Sent: Wednesday, May 28, 2014 6:49 PM
> > > > To: Doherty, Declan
> > > > Cc: dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH 0/4] Link Bonding Library
> > > >
> > > > On Wed, May 28, 2014 at 04:32:00PM +0100, declan.doherty@intel.com
> > wrote:
> > > > > From: Declan Doherty <declan.doherty@intel.com>
> > > > >
> > > > > Initial release of Link Bonding Library (lib/librte_bond) with 
> > > > > support for bonding modes :
> > > > >  0 - Round Robin
> > > > >  1 - Active Backup
> > > > >  2 - Balance l2 / l23 / l34
> > > > >  3 - Broadcast
> > > > >
> > > > Why make this a separate library?  That requires exposure of yet 
> > > > another
> > API to applications.  Instead, why > not write a PMD that can enslave 
> > other PMD's and treat them all as a single interface?  That way this 
> > all >  > works with the existing API.
> > > >
> > > > Neil
> > >
> > > Hi Neil,
> > > the link bonding device is essentially a software PMD, and as such 
> > > supports
> > all the standard PMD APIs, the only new APIs which the link bonding 
> > library introduces  are for the control operations of the bonded 
> > device which are currently unsupported by the standard PMD API.
> > Operations such as creating, adding/removing slaves, and configuring 
> > the modes of operation of the device have no analogous APIs in the 
> > current PMD API and required new ones to be created .
> > 
> > Thats really only true in spirit, in the sense that this library 
> > transmits and receives frames like a PMD does.  In practice it doesn't 
> > work and isn't architected the same way.  You don't register the 
> > driver using the same method as the other PMDs, which means that using 
> > --vdev on the command line wont work for this type of device.  It also 
> > implies that applications have to be made specifically aware of the 
> > fact that they are using a bonded interface (i.e. they need to call 
> > the bonding setup routines to create the bond).  I would
> > recommend:
> > 
> > 1) Register the pmd using the PMD_DRIVER_REGISTER macro, like other 
> > PMD's
> > 2) Use the kvargs library to support configuration via the --vdev 
> > command line option, so bonds can be created administratively, rather 
> > than just programatically
> > 3) Separate the command api from the PMD functionality into separate 
> > libraries (use control mbufs to communicate configuration changes to 
> > the pmd).  This will allow users to dynamically load the pmd 
> > functionality (without compile or run time linking requirements), and 
> > then optionally use the programatic interface (or not if they want to 
> > save memory)
> > 
> > Regards
> > Neil
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, June 5, 2014 12:04 PM
> > To: Doherty, Declan
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 0/4] Link Bonding Library
> > 
> > On Wed, Jun 04, 2014 at 04:18:01PM +0100, declan.doherty@intel.com
> > wrote:
> > > From: Declan Doherty <declan.doherty@intel.com>
> > >
> > > v2 patch additions,
> > > fix for tx burst broadcast, incrementing the reference count on each 
> > > mbuf by the number of slaves - 1 add/remove slave behavior chnange 
> > > to fix primary slave port assignment patchcheck code fixes
> > >
> > >
> > >
> > This doesn't address any of the comments I made previously.
> > Neil
> 
> 
> Hi Neil, 
> 
> sorry for not replying regarding this earlier, in relation to your recommendations 1 and 2, I'm currently working on the implementation of both and should have an additional patch within the next couple of days to add this functionality.
> 
> Regarding your third recommendation, I have no problem splitting the library into separate API and PMD libraries although this does seem to break convention with the other non hw based pmd's, such as pmd_pcap,
I'm not at all sure what you mean here.  pmd_pcap registers a driver with the
registration mechanism, allowing for the use of the --vdev option on the command
line, same as all the other virtual drivers.  The physical drivers do the same
thing using the --whitelist and --blacklist options.  How is the bonding driver
following convention with pmd_pcap?


> so would prefer not to go down that route. Also, I don't see the ability to dynamically load the pmd as justification for the use of a control mbufs interface, this would require that either additional queues be created to handle control messages, or that the data TX queues have the ability to identify and process control mbufs, and either of these options will have a performance hit,

Well, you're correct in that the driver would have to identify control messages,
but I don't see how thats a performance hit at all.  If you create a control
channel queue, then all you have to introduce to the datapath is a single branch
to see which queue is transmitting, and your done. If control mbufs are just
that hurtful to performance, it seems thats an area that really needs
improvement then.  

Another alternative is to add a control packet method to the rte_eth_dev
structure allowing you to pass control mbufs directly to a pmd out of line with
the datapath.  That could avoid any additional branches in the datapath, and
still allow you to separate user api from driver implementation.


> for the core handling the separate control messages or directly to the tx burst performance. I am not currently aware of any functional requirements or use cases for bonded devices to be used over multiple processes which is the only reason I can see for using an interface based on control mbufs,

The reason is administrative.  Its got nothing to do with how many processes
want to use an interface, it has to do with an interface knowing if its using
the bonding driver or not (it shouldn't ever have to, but with your patch set,
its required to use the bonding api to create the interface and add slaves.

> and by providing a --vdev configuration options the user can still write their applications without any knowledge of the additional link bonding programmatic interface. If in the future a use case does emerge which requires multi-process control of bonded devices, I think it would make sense then to add an additional library to provide this functionality.

How exactly does that happen?  At what point does the bonding library in its
current form register a driver with rte_ethdev without the application first
calling some function in the bonding library to do so.  Because until that
happens, --vdev is non functional for this pmd.

FWIW, the concern for me here is one of packaging.  I don't want users to have
to pull in a library that they don't absolutely need.  the way you have this put
together right now, they need the entire bonding library even if they just want
a statically configured bond interface

Neil


> 
> Regards
> Declan
> --------------------------------------------------------------
> Intel Shannon Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
> Business address: Dromore House, East Park, Shannon, Co. Clare
> 
> This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
> 
> 
> 

  reply	other threads:[~2014-06-06 14:54 UTC|newest]

Thread overview: 127+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-28 15:32 [dpdk-dev] [PATCH " declan.doherty
2014-05-28 15:32 ` [dpdk-dev] [PATCH 1/4] " declan.doherty
2014-05-28 16:54   ` Shaw, Jeffrey B
2014-05-29 13:32     ` Doherty, Declan
2014-05-28 15:32 ` [dpdk-dev] [PATCH 2/4] Link bonding unit tests declan.doherty
2014-05-28 15:32 ` [dpdk-dev] [PATCH 3/4] Link bonding integration into testpmd declan.doherty
2014-05-28 15:32 ` [dpdk-dev] [PATCH 4/4] Add Link Bonding Library to Doxygen declan.doherty
2014-05-28 17:49 ` [dpdk-dev] [PATCH 0/4] Link Bonding Library Neil Horman
2014-05-29 10:33   ` Doherty, Declan
2014-05-29 11:33     ` Neil Horman
2014-05-29  3:23 ` Cao, Waterman
2014-05-29 10:35   ` Doherty, Declan
2014-06-04 15:18 ` [dpdk-dev] [PATCH v2 " declan.doherty
2014-06-04 15:18   ` [dpdk-dev] [PATCH v2 1/4] " declan.doherty
2014-06-04 15:18     ` declan.doherty
2014-06-05 15:15     ` Stephen Hemminger
2014-06-06  9:07       ` Doherty, Declan
2014-06-06 15:13         ` Stephen Hemminger
2014-06-09 21:11     ` Eric Kinzie
2014-06-13 14:03       ` Doherty, Declan
2014-06-04 15:18   ` [dpdk-dev] [PATCH v2 2/4] Link bonding unit tests, including: - code to generate packet bursts for testing rx and tx functionality of bonded device - virtual/stubbed out ethdev for use as slave ethdev in testing - checkpack fixes declan.doherty
2014-06-04 15:18     ` declan.doherty
2014-06-04 15:18   ` [dpdk-dev] [PATCH v2 0/4] Link Bonding Library declan.doherty
2014-06-04 15:18   ` [dpdk-dev] [PATCH v2 3/4] Adding link bonding support to testpmd. - Includes the ability to create new bonded devices. - Add /remove bonding slave devices. - Interogate bonded device stats/configuration - Change bonding modes and select balance transmit polices declan.doherty
2014-06-04 15:18   ` [dpdk-dev] [PATCH v2 4/4] Add Link Bonding Library to Doxygen declan.doherty
2014-06-04 16:10   ` [dpdk-dev] [PATCH v2 0/4] Link Bonding Library Doherty, Declan
2014-06-05  8:03   ` De Lara Guarch, Pablo
2014-06-05 11:03   ` Neil Horman
2014-06-06  8:23     ` Doherty, Declan
2014-06-06 14:54       ` Neil Horman [this message]
2014-06-13 14:56         ` Doherty, Declan
2014-06-13 15:11           ` Neil Horman
2014-06-06  3:26   ` Cao, Waterman
2014-06-11 16:33   ` Thomas Monjalon
2014-06-13 14:08     ` Doherty, Declan
2014-06-13 15:15       ` Thomas Monjalon
2014-06-13 14:41 ` [dpdk-dev] [PATCH v3 0/5] Link Bonding PMD Library Declan Doherty
2014-06-13 14:41   ` [dpdk-dev] [PATCH v3 1/5] " Declan Doherty
2014-06-13 14:41   ` [dpdk-dev] [PATCH v3 2/5] Link Bonding PMD Library (librte_eal/librte_ether link bonding support changes) Declan Doherty
2014-06-13 16:08     ` Neil Horman
2014-06-13 18:34       ` Doherty, Declan
2014-06-13 19:38         ` Neil Horman
2014-06-16  8:59           ` Doherty, Declan
2014-06-16 11:07             ` Neil Horman
2014-06-16 16:17               ` Richardson, Bruce
2014-06-16 17:47                 ` Neil Horman
2014-06-16 18:07                   ` Richardson, Bruce
2014-06-16 18:09                   ` Thomas Monjalon
2014-06-13 21:59     ` Stephen Hemminger
2014-06-16  7:59       ` Doherty, Declan
2014-06-13 14:42   ` [dpdk-dev] [PATCH v3 3/5] Link Bonding PMD Library (Unit Test Suite) Declan Doherty
2014-06-13 14:42   ` [dpdk-dev] [PATCH v3 4/5] Link Bonding PMD Library (testpmd link bonding API support) Declan Doherty
2014-06-13 14:42   ` [dpdk-dev] [PATCH v3 5/5] Link Bonding PMD Library (Doxygen Additions) Declan Doherty
2014-06-13 15:20   ` [dpdk-dev] [PATCH v3 0/5] Link Bonding PMD Library Neil Horman
2014-06-16 11:18   ` [dpdk-dev] [PATCH v4 0/6] Link Bonding Library Declan Doherty
2014-06-18 16:14     ` [dpdk-dev] [PATCH v5 " Declan Doherty
2014-06-18 16:18       ` Neil Horman
2014-06-24 14:52       ` [dpdk-dev] [PATCH v6 " Declan Doherty
2014-06-24 16:03         ` [dpdk-dev] [PATCH v7 " Declan Doherty
2014-06-25 20:07           ` [dpdk-dev] [PATCH v8 " Declan Doherty
2014-06-26 16:02             ` De Lara Guarch, Pablo
2014-06-26 23:57             ` [dpdk-dev] [PATCH v9 0/5] link bonding Thomas Monjalon
2014-06-26 23:57               ` [dpdk-dev] [PATCH v9 1/5] bond: new link bonding library Thomas Monjalon
2014-06-27  0:45                 ` Thomas Monjalon
2014-06-26 23:57               ` [dpdk-dev] [PATCH v9 2/5] ethdev: add unique name to devices Thomas Monjalon
2014-06-26 23:57               ` [dpdk-dev] [PATCH v9 3/5] eal: support link bonding device initialization Thomas Monjalon
2014-06-26 23:57               ` [dpdk-dev] [PATCH v9 4/5] bond: unit tests Thomas Monjalon
2014-06-26 23:57               ` [dpdk-dev] [PATCH v9 5/5] bond: testpmd support Thomas Monjalon
2014-06-27 10:18               ` [dpdk-dev] [PATCH v10 0/5] link bonding Declan Doherty
2014-06-27 20:58                 ` Thomas Monjalon
2014-06-29 17:49                 ` [dpdk-dev] [PATCH v11 0/5] link bonding library Declan Doherty
2014-06-30  9:21                   ` Thomas Monjalon
2014-06-30  9:28                     ` Doherty, Declan
2014-07-01 22:01                       ` Thomas Monjalon
2014-06-29 17:49                 ` [dpdk-dev] [PATCH v11 1/5] bond: new " Declan Doherty
2014-06-30  9:13                   ` Thomas Monjalon
2014-06-30 22:29                   ` Robert Sanford
2014-07-01 14:16                     ` Thomas Monjalon
2014-07-01 14:19                     ` Doherty, Declan
2014-07-01 14:26                       ` Thomas Monjalon
2014-06-29 17:49                 ` [dpdk-dev] [PATCH v11 2/5] ethdev: add unique name to devices Declan Doherty
2014-06-29 17:49                 ` [dpdk-dev] [PATCH v11 3/5] eal: support link bonding device initialization Declan Doherty
2014-06-29 17:49                 ` [dpdk-dev] [PATCH v11 4/5] bond: unit tests Declan Doherty
2014-06-30  8:56                   ` Thomas Monjalon
2014-06-29 17:49                 ` [dpdk-dev] [PATCH v11 5/5] bond: testpmd support Declan Doherty
2014-06-27 10:18               ` [dpdk-dev] [PATCH v10 1/5] bond: new link bonding library Declan Doherty
2014-06-27 10:18               ` [dpdk-dev] [PATCH v10 2/5] ethdev: add unique name to devices Declan Doherty
2014-06-27 10:18               ` [dpdk-dev] [PATCH v10 3/5] eal: support link bonding device initialization Declan Doherty
2014-06-27 10:18               ` [dpdk-dev] [PATCH v10 4/5] bond: unit tests Declan Doherty
2014-06-27 10:18               ` [dpdk-dev] [PATCH v10 5/5] bond: testpmd support Declan Doherty
2014-06-25 20:07           ` [dpdk-dev] [PATCH v8 1/6] Link Bonding Library (lib/librte_pmd_bond) Declan Doherty
2014-06-25 20:07           ` [dpdk-dev] [PATCH v8 2/6] Support for unique interface naming of pmds Declan Doherty
2014-06-25 20:07           ` [dpdk-dev] [PATCH v8 3/6] EAL support for link bonding device initialization Declan Doherty
2014-06-25 20:07           ` [dpdk-dev] [PATCH v8 4/6] Link bonding Unit Tests Declan Doherty
2014-06-25 20:07           ` [dpdk-dev] [PATCH v8 5/6] testpmd link bonding additions Declan Doherty
2014-06-25 20:07           ` [dpdk-dev] [PATCH v8 6/6] Link Bonding Library doxygen additions Declan Doherty
2014-06-24 16:03         ` [dpdk-dev] [PATCH v7 1/6] Link Bonding Library (lib/librte_pmd_bond) Declan Doherty
2014-06-24 16:03         ` [dpdk-dev] [PATCH v7 2/6] Support for unique interface naming of pmds Declan Doherty
2014-06-24 16:03         ` [dpdk-dev] [PATCH v7 3/6] EAL support for link bonding device initialization Declan Doherty
2014-06-25 13:54           ` Thomas Monjalon
2014-06-25 14:41             ` Doherty, Declan
2014-06-25 16:00               ` Thomas Monjalon
2014-06-25 16:15                 ` Richardson, Bruce
2014-06-24 16:03         ` [dpdk-dev] [PATCH v7 4/6] Link bonding Unit Tests Declan Doherty
2014-06-24 16:03         ` [dpdk-dev] [PATCH v7 5/6] testpmd link bonding additions Declan Doherty
2014-06-24 16:03         ` [dpdk-dev] [PATCH v7 6/6] Link Bonding Library doxygen additions Declan Doherty
2014-06-25 13:43           ` Thomas Monjalon
2014-06-25 14:19             ` Doherty, Declan
2014-06-25 14:23               ` Thomas Monjalon
2014-06-24 14:52       ` [dpdk-dev] [PATCH v6 1/6] Link Bonding Library (lib/librte_pmd_bond) Declan Doherty
2014-06-24 14:52       ` [dpdk-dev] [PATCH v6 2/6] Support for unique interface naming of pmds Declan Doherty
2014-06-24 14:52       ` [dpdk-dev] [PATCH v6 3/6] EAL support for link bonding device initialization Declan Doherty
2014-06-24 14:52       ` [dpdk-dev] [PATCH v6 4/6] Link bonding Unit Tests Declan Doherty
2014-06-24 14:52       ` [dpdk-dev] [PATCH v6 5/6] testpmd link bonding additions Declan Doherty
2014-06-24 14:52       ` [dpdk-dev] [PATCH v6 6/6] Link Bonding Library doxygen additions Declan Doherty
2014-06-18 16:14     ` [dpdk-dev] [PATCH v5 1/6] Link Bonding Library (lib/librte_pmd_bond) Declan Doherty
2014-06-18 16:14     ` [dpdk-dev] [PATCH v5 2/6] Support for unique interface naming of pmds Declan Doherty
2014-06-18 16:14     ` [dpdk-dev] [PATCH v5 3/6] EAL support for link bonding device initialization Declan Doherty
2014-06-18 16:14     ` [dpdk-dev] [PATCH v5 4/6] Link bonding Unit Tests Declan Doherty
2014-06-18 16:14     ` [dpdk-dev] [PATCH v5 5/6] testpmd link bonding additions Declan Doherty
2014-06-18 16:14     ` [dpdk-dev] [PATCH v5 6/6] Link Bonding Library doxygen additions Declan Doherty
2014-06-16 11:18   ` [dpdk-dev] [PATCH v4 1/6] Link Bonding Library (lib/librte_pmd_bond) initial release with support for Mode 0 - Round Robin Mode 1 - Active Backup Mode 2 - Balance -> Supports 3 transmit polices (layer 2, layer 2+3, la Mode 3 - Broadcast Declan Doherty
2014-06-16 11:18   ` [dpdk-dev] [PATCH v4 2/6] Support for unique interface naming of pmds Declan Doherty
2014-06-16 11:18   ` [dpdk-dev] [PATCH v4 3/6] EAL support for link bonding device initialization Declan Doherty
2014-06-16 11:18   ` [dpdk-dev] [PATCH v4 4/6] Link bonding Unit Tests Declan Doherty
2014-06-16 11:18   ` [dpdk-dev] [PATCH v4 5/6] testpmd link bonding additions Declan Doherty
2014-06-16 11:18   ` [dpdk-dev] [PATCH v4 6/6] Link Bonding Library doxygen additions Declan Doherty

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=20140606145426.GA2543@hmsreliant.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@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).