Re: [dpdk-dev] [PATCH v2] librte_pmd_packet: add PMD for AF_PACKET-based virtual devices

[Neil Horman <nhorman@tuxdriver.com>]

On Wed, Oct 08, 2014 at 05:57:46PM +0200, Thomas Monjalon wrote:
> 2014-09-29 11:05, Bruce Richardson:
> > On Fri, Sep 26, 2014 at 10:08:55AM -0400, Neil Horman wrote:
> > > On Fri, Sep 26, 2014 at 11:28:05AM +0200, Thomas Monjalon wrote:
> > > > 2014-09-16 16:16, Neil Horman:
> > > > > On Fri, Sep 12, 2014 at 02:05:23PM -0400, John W. Linville wrote:
> > > > > > Ping?  Are there objections to this patch from mid-July?
> > > > > 
> > > > > Thomas, Where are you on this?  It seems like if you don't have any objections
> > > > > to this patch, it should go in, in ilght of the lack of further commentary.
> > > > 
> > > > 1) It doesn't appear as a top priority.
> > > Thats your responsibility.  Patches can't languish and rot on a list forever
> > > just because others aren't willing to test it.  If theres further testing that
> > > you feel it needs, ask. But from my read, its been tested for functionality and
> > > performance (though high performance is never expected from a AF_PACKET PMD).
> > > Given that any one PMD will not affect the performance of another in isolation,
> > > I'm not sure what more you're waiting for here.
> 
> Yes, integration of new PMD must be accelerated.
> 
> > > > 2) It's competing with pcap PMD and bifurcated PMD to come
> > > >    (http://dpdk.org/ml/archives/dev/2014-September/005379.html)
> > > Regarding the pcap PMD, so?  Its an alternate implementation that provides
> > > different features with different limitations.  The fact that they are simmilar
> > > is irrelevant.  If simmilarity was the test, then we wouldn't bother with the
> > > bifurcated driver either, because the pcap pmd already exists.
> > > 
> > > Regarding the bifurcated driver, you can't hold existing patches on the promise
> > > of another pmd thats comming at an indeterminate time in the future.  Theres no
> > > reason not to take this now and deprecate it in the future if there is
> > > sufficient overlap with the bifurcated driver, though to my point above, they
> > > still address different needs with different limitations, so I don't see doing
> > > so as necessecary.
> 
> Yes, we'll discuss it when bifurcated driver will be released.
> 
john Fastabend posted it to netdev just a few days ago. There have been some
concerns raised, which he is trying to address.  I'm watching how that goes.

> > > > 3) There is no test associated with this PMD.
> > > That would have been a great comment to make a few months back, though whats
> > > wrong with testpmd here?  That seems to be the same test that every other pmd
> > > uses. What exactly are you looking for?
> 
> I was thinking of testing behaviour with different kernel configurations and
> unit tests for --vdev options. But it's not a major blocker.
> 
Thats fine with me.  If theres a set of unit tests that you have documentation
for, I'm sure we would be happy to run them.  I presume you just want all the
pmd vdev option exercised?  Any specific sets of kernel configurations?


> > > > If one of this item becomes wrong, it should go in.
> > > 
> > > > Currently, 2 projects are being initiated for validation (dcts) and
> > > > documentation. Keeping new things outside of the DPDK core makes it
> > > > clear that they have not to be supported by dcts and doc yet.
> > > > So, it is better to have an external PMD, like memnic, acting as a
> > > > staging area.
> > > > 
> > > So, this brings up an excellent point - Validation and support.  Commonly open
> > > source projects don't provide support at the upstream HEAD. Those items are
> > > applied and inforced by distributors.  Theres no need to ensure that the
> > > upstream head is always the most performance and stable point of the tree.  Its
> > > that need that keeps the development pace slow, and creates frustrations like
> > > this one, where a patch sits unaddressed for long periods of time.  Commonly the
> > > workflow for most open source projects is for there to be a window of time where
> > > visual review and basic functional testing are sufficient for acceptance into
> > > the head of the tree.  After the development window closes there is a
> > > stabilization period where testing/validation is done to ensure that no
> > > regressions have been encountered, optionally with a -next branch temporarily
> > > being created to accept patches for upcomming future releases.  If regressions
> > > are found, its a simple matter in git to bisect back to the offending patch,
> > > allow the contributing developer an opportunity to fix the issue, or to drop the
> > > patch.  Using a workflow like this we can have a reasonable balance of needs
> > > (good patch turn around time, as well as reasonable testing).  We've discussed
> > > this when I posted the PMD_REGISTER_DRIVER patch months ago, and I thought you
> > > were going to move in the direction of this workflow.  What happened?
> 
> Yes, we are moving to a "merge window" workflow.
> 
That would be wonderful.  I think separating the integration workflow from the
test workflow is critical here to making sure that patch integration isn't
unnecessecarily delayed.

> > > > During this time, keeping this PMD separately will allow you to update it
> > > > with a maintainer account in dpdk.org. I just need your SSH public key.
> > > > 
> > > We've discussed this too, keeping PMDs maintained separately is a very bad idea.
> > > Doing so means developers have to constantly be aware of changes to the core
> > > tree and try to keep up individually.  Integrating them all means that API
> > > changes can be easily propogated to all PMD's when needed without making work
> > > for many people.  Its exactly the reason we encourage driver writers to open
> > > source drivers in Linux, because not doing so closes developers off from the
> > > free maintenence they get when optimizations are made to API's.  And if you
> > > follow the development model above, you don't need to worry about implied
> > > support, as that correctly becomes a distributor issue.
> > > 
> > > 
> > > Neil
> > 
> > While not wanting to get too involved in the discussion, I'd just like to 
> > express my support for getting this new PMD merged in.
> 
> If RedHat is committed for its maintenance, it could integrated in release 1.8.
> But I'd like it to be renamed as pmd_af_packet (or a better name) instead of
> pmd_packet.
> 
John L. is on his way to plumbers at the moment, so is unable to comment, but
I'll try to get a few cycles to change the name of the PMD around.  And yes, I
thought that maintenance was implicit.  He's the author, of course he'll take
care of it :).  And I'll be glad to help

Neil

> Thanks
> -- 
> Thomas
>