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

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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.

> > > 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.

> > > 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.

> > > 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.

Thanks
-- 
Thomas