[dpdk-dev] Why nothing since 1.8.0?

[Neil Horman]

On Sun, Jan 18, 2015 at 09:48:33PM +0000, O'driscoll, Tim wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > Sent: Sunday, January 18, 2015 6:25 PM
> > To: O'driscoll, Tim
> > Cc: Thomas Monjalon; dev at dpdk.org
> > Subject: Re: [dpdk-dev] Why nothing since 1.8.0?
> > 
> > On Sat, Jan 17, 2015 at 07:57:04PM +0000, O'driscoll, Tim wrote:
> > > I'm going to risk the wrath of the open source purists amongst you by top-
> > posting. The reason is that there has been lots of email on this subject, and I
> > want to summarise the key issue and clearly state our position on it.
> > Hoperfully nobody is offended by this!
> > >
> > Acutally, I am a bit upset by your doing this.  While top posting can be an
> > acceptible response form, doing so in an interleaved thread gives you the
> > opportunity to effectively rewrite the conversation on your own terms.
> > While
> > you might have summarized your position accurately in your mind, you've
> > discarded all the evidence that I presented in opposition.  I don't appreciate
> > that.  But we can rebuild from here, no worries.
> > 
> > > This thread has generated lots of debate, which is good, and there are a
> > number of items that I believe everybody agrees on (having a MAINTAINERS
> > file, better tools etc.). However, the key issue that we do not agree on is the
> > granularity of the repositories within DPDK.
> > >
> > No, thats not really the crux of the debate in my mind anymore, though that
> > is
> > certainly part of it, as it effects the convienience of developers to contribute
> > to the project.  More to the point, the area of disagreement here is the best,
> > most efficient way to integrate patches to various pieces of the dpdk that
> > balances developer convienience, effiency and transparency (I've not
> > ennumerated
> > that last part before, as I've not thought of the right word, and that may still
> > not be quite right, but more on it later).
> > 
> > > The current state is:
> > > - One master repository
> > > - A small number of sub-repositories, each with a separate
> > maintainer/SME, including: i40e, fm10k, bnx2x, docs, dts. These are used to
> > generate pull requests to the main repo.
> > >
> > No, this is not the current state.  The current state is:
> > - One master repository
> 
> We have a repository for docs, with a separate maintainer that was used to generate pull requests for 1.8. From our perspective that worked well.
> For i40e, 1.8 development was done in the main repository, and we agreed to transition to a separate repo for 2.0. 2.0 development is currently in progress.
> We haven't upstreamed anything for fm10k yet, but that will be done in its own repo from the start, for the 2.0 release.
> 
> > We have lots of cloned dpdk trees on dpdk.org, but there is no path from
> > them to
> > the master repository.  Nothing has been comitted to any of the subtrees,
> > despite having been open for a few months now.
> 
> The plan is to use the i40e and fm10k repos for 2.0 development, which is in progress.
> 
> >  I don't see any
> > documentation
> > indicating who the maintainer of each tree is, and so don't have the foggiest
> > idea who to contact if I want to get something merged to these trees.  They
> > aren't subtrees, they're just clones of the master repository.
> 
> A MAINTAINERS file to clarify responsibilities is a good idea.
> 
> > > You're proposing the following:
> > > - Remove the individual PMD repositories, and replace them with a single
> > repository containing all PMDs, plus parts of the core code that are closely
> > linked to the PMDs, with you as the maintainer and SMEs for each PMD.
> > >
> > Not necesarily me, mind you (though yes, I've volunteered).  I'm happy for
> > someone else to take on this role if they volunteer.  The point is not to have
> > a
> > separate repo to integrate patches for any one PMD (as theres no need, and
> > it
> > makes development harder).  I want one repository that I can use to target
> > development against all PMDs, just like we do with net/net-next in the linux
> > community.
> > 
> > > As I've said before, and as Venky has also explained in private email on this,
> > we do not agree with this proposed change. We believe it would be a
> > backward step, and would have an adverse impact on DPDK.
> > >
> > You've asserted that several times, but not once have you provided any
> > supporting evidence or data to back that assertion.  Conversely I've provided
> > several bits of evidence to support my assertion that using the linux
> > workflow
> > model would work perfectly well here and handle all our needs (which
> > include,
> > but are not limited to, yours).
> 
> The reason is to give as much control as possible to the people most familiar with the PMD code and the corresponding NIC hardware.
> 
> > > The key issue here is that we've deliberately tried to give as much control
> > as possible to those who are intimately familiar with a particular PMD and the
> > underlying hardware. In our view, that's just common sense. What you're
> > proposing is to take some of that control away, and give it to someone else
> > (in this case you) who isn't familiar with the details. We don't agree with that
> > approach.
> > >
> > Yes, you have proposed that.  The question is, why?  People intimately
> > familiar
> > with the code should be writing and reviewing code.  Why do they need the
> > responsibility for integration as well?  Thats really the question.  Answer
> > that, and perhaps we can make some process on this issue.  Tell me how one
> > of
> > the people most familiar with a given piece of hardware and the software
> > that
> > drives it having the added responsibility of handling the trivial operations of
> > SCM helps you, or anyone else?  I'll point out again here that there are 6 I40e
> > patches on the DPDK list, some as old as November 20th of last year.  4 have
> > been ACK'd, but none have been integrated, with no additional commentary
> > from
> > the experts who are tasked with the patch management role.  How is that
> > efficient, transparent or condusive to development?
> 
> We're doing a lot of work on i40e and fm10k at the moment. In both cases, somebody needs to fill the maintainer role. That could be someone familiar with the PMD code and NIC hardware, or it could be somebody without this depth of knowledge. The current situation is the first option. It's true that it doesn't have to be done that way, but it seems sensible to give as much control as possible to the people with the expertise in these areas. We just believe that this will allow for more efficient development as the maintainer will have a detailed understanding of what they're applying.
> 
> > > The arguments I've heard in favour of your proposal are summarised
> > below. Apologies for paraphrasing, and for any errors, but the email thread is
> > too big and too convoluted to address these all separately. I've also included
> > an explanation for each item to say why we don't believe they're sufficient to
> > justify your proposed change.
> > >
> > I thought we were doing just fine with the conversation.  The paraphrasing
> > here
> > is why I was upset by your top post.  You convieniently ignored all my
> > supporting evidence.
> > 
> > > 1. Your proposal is consistent with the Linux kernel, but current state is not.
> > > In both cases (current state and your proposal), the workflow is exactly the
> > same as the Linux kernel. The difference we're discussing is simply the
> > granularity of the repositories.
> > >
> > Thats simply not true.  My proposal is consistent with the linux kernel model,
> > but what you want in no way is.  The graunularity is diffferent as you note,
> > but
> > you're adding in this requirement whereby a developer for a given PMD
> > must be
> > the tree maintainer for a subtree solely for the purpose of maintaining that
> > PMD.  That in no way shape or form resembles the linux kernel model.  If you
> > wish to do that internally to Intel, thats great, and you have all the ability
> > to do that, but to mandate it as part of the community project is anathema to
> > the way the linux workflow works, and open source projects in general.
> 
> We're not mandating that at all. What we're saying is that this is the approach that's in place for fm10k and i40e, and that we don't agree with your proposal to change this. For other PMDs, the model could be different.
> 
> > > DPDK is much smaller than the kernel, so the granularity is naturally going
> > to be different. The kernel may combine drivers into a single repository due
> > to its size and complexity, but that doesn't mean we need to do the same for
> > DPDK.
> > >
> > Why?  You're right in the most general of senses, diffferent projects can
> > work
> > differently, but being different in one way (size/complexity), doesn't
> > mandate
> > that it be different in another way (workflow).  We can (and are) exploring
> > different ways to implement workflows here, but the question is one of
> > reason.
> > The kernel combines drivers into a single respository because its a natural
> > functional separation that allows Linus to divide the workload to
> > subordinates,
> > while still giving contributing developers a canonical place to go for their
> > development target needs.  Its efficient (As I calculated before in the
> > interleaved section below, Dave Miller integrates either from pull requests or
> > individual posts, over 1000 patches on average every 2 month release cycle),
> > fast (even though every patch goes to the mailing list, gets reviewed, and
> > acked), and convienient for all developers.
> > 
> > Conversely, the I40e driver has seen 114 patches in the last 6 month period
> > to
> > the DPDK.  What is it about dpdk PMD's that makes a process that is as
> > efficient, fast and transparent as the upstream kenrel's unacceptible to you?
> 
> I still believe the workflow and process are the same in both cases. In both cases there's a subtree to which a maintainer applies patches and then generates pull requests to the main repo. The difference in your proposal is the granularity of the subtree.
> 
> > > 2. Maintainer and SME are separate roles and should not be combined.
> > > We understand that they're separate roles. For the PMDs that we're
> > developing, the most efficient way for us to manage the work is to combine
> > these and have one of our SMEs act as maintainer as well. That's an internal
> > decision that suits the structure of our development team and the current
> > state of the i40e and fm10k PMDs. Others are obviously free to make their
> > own choices for PMDs that they're developing.
> > >
> > Thats a really interesting thing to say.  You made an internal decision,  but
> > this isn't an internal project.  This is a community project.  If you want an
> > internal tree to commit patches too, thats fine, you can and should do that
> > for
> > whatever internal workflow suits your needs, but before they get merged to
> > the
> > community project (which includes the I40e driver), they need to get posted
> > to
> > the community list to allow any and all an opportunity to review them (even
> > if
> > they choose not to, they deserve the opportunity). 
> 
> All of our work is posted to the mailing list and available to anybody in the community to review. That's been the case since the 1.7 release.
> 
> > It seems that, while you
> > haven't said it in so many words, you are looking for ways to accelerate that
> > process, and potentially cutting out the community in the process.
> 
> Absolutely not.
> 
> > Let me ask a question here: Do you intend to post all the I40e patches that
> > you
> > plan to commit to the I40e tree to the DPDK list?  Or do you plan to integrate
> > them to the tree using only internal review, and then send a pull request to
> > the
> > list?  If you are planning on doing the latter, that would explain your desires
> > to merge the tree maintainer and SME role, but would be a huge step
> > backwards
> > from all of the progress you've made toward making this project a true
> > community
> > project.  As Stephen indicates, what you're then doing is creating several
> > out-of-tree drivers, and that would be totally unacceptable.  You're of course
> > welcome to do so, but I would not accept any workflow in which changes to
> > code
> > did not have patches posted to the list.
> 
> As above, all of our work has been, and will continue to be, posted to the mailing list and is open to anybody in the community to review.
> 
> > > 3. A maintainer can handle a higher volume of patches than will be present
> > in any individual PMD.
> > > That's true, but it's also not relevant. Our goal is not to make the
> > SME/maintainer for i40e, fm10k etc. a full-time position. Our goal is to have
> > an expert in this position, so that we can move quickly and still ensure good
> > quality software.
> > >
> > Please re-read your above statement. I think you're contradicting yourself
> > 1) You agree that a tree maintainer can handle a higher volume of patches
> > than
> > any one PMD presents, implying that a tree maintainer can, when properly
> > focused, efficiently take the feedback of SME's and integrate many patches
> > quickly.
> > 
> > 2) You claim (1) is irellevant because because your goal is to put an expert in
> > the position of tree maintainer so that you can move more quickly.
> > 
> > If you agree that (1) allows for a fast, efficient and transparent workflow,
> > how
> > can you both claim it is both irrelevant and that you need a merged SME/tree
> > maintainer role to achieve the same goal?
> > 
> > Question: How exactly do you believe putting an SME in the role of tree
> > maintainer will improve code quality and make code integration faster?
> 
> I think Thomas is a good example here. Theoretically, someone in his role would not need any knowledge of DPDK, since they would just be fulfilling an SCM function. However, I believe his knowledge and understanding of DPDK have been crucial in building the community and getting the project to the stage it's at now. The same applies for i40e and fm10k maintainers. We believe that having experts in these roles is the most efficient way to make progress.
> 
> > > 4. We shouldn't give maintainer work to an SME as it detracts from their
> > other tasks.
> > > We don't anticipate a problem with workload for the people that we have
> > in these positions.
> > You're having that problem right now, you just refuse to acknoweldge it.  Let
> > me
> > present it for you:
> > http://dpdk.org/dev/patchwork/project/dpdk/list/?q=I40e
> > 
> > That shows the only 6 patches that have been posted for I40e since 1.8
> > released,
> > ranging dating back as far as November 20th.  These patches have been
> > sittting
> > on the list since then unacted upon.  If fact, taking a closer look, its a bit
> > worse than that.  The X710 patch in that list has been integrated, but a
> > version
> > different from the one in patchwork.  Its probably just an oversight, but the
> > fact remains, whoever is doing your subtree maintenence is focused on your
> > internal needs and is ignoring their community responsibilities.  Thats a
> > problem.
> 
> This is work in progress. We're confident that we'll have the necessary i40e changes in place for 2.0.
> 
> > > 5. There will be extra overhead for developers who want to implement
> > changes spanning multiple PMDs.
> > > That's true, but it's also something of a hypothetical argument. The people
> > who've spoken against your proposal on the mailing list are from Intel and
> > 6Wind, who are also the people contributing to most of these PMDs. I had a
> > quick scan of the commits to see if I could spot anything from another
> > contributor that might fall into this category, and I couldn't (admittedly it
> > wasn't an exhaustive search, which someone else is free to do if they want).
> > If this situation does arise, then Thomas has previously outlined how it can be
> > handled.
> > 
> > Its not a hypothetical argument at all.  We have this situation upstream every
> > time someone makes a patch that crosses subsystems, and its managed by
> > the
> > maintainers co-ordinating their efforts when merge time comes along.  Thats
> > why
> > its so important to find the right granularity for subtrees, so that extra
> > effort is minimized.
> > 
> > And yes, you're right, most of the contributors currently are from 6wind and
> > Intel.  The goal here is to spread that participation beyond just the two of
> > you.  If you don't have a tree-maintainer that is actively handling patches on
> > the list, getting things integrated in a timely fashion, no one is going to
> > bother participating.
> > 
> > As for handling it using the workaround thomas has proposed, I've made my
> > arguments to that effect already several times, but you've neglected to
> > summarize them here with your top post, so let me re-iterate:
> > Patch sets that cross subtrees are a challenge no matter how you slice them.
> > If
> > you have subtrees at a reasonable granularity, subtree maintainers can work
> > together to ensure that the upstream merge goes smoothly.  If you force a
> > tree
> > spanning patch to be done in the parent tree you will have merge work to do
> > for
> > each subtree that sends you a pull request.  Having a few subtree
> > maintainers
> > handle that work is preferable than having to do merge fixups in the main
> > tree.
> > The main tree should whenever possible have clean merges.
> 
> My comment was saying that we should give weight to the views of the people who are currently doing the work in these areas. These people are strongly of the opinion that it's better to continue with the current approach.
> 
> > > In terms of where we go from here, I'd suggest the following:
> > > - Thomas has already asked us about a maintainer for older Intel NICs,
> > which we're looking into. We may choose to have a single repository with a
> > single maintainer/SME for e1000/igb/ixgbe combined, which would limit the
> > overall number of repositories.
> > > - You could pursue a path of having a single repository for non-Intel NICs.
> > This would obviously need to be worked with those responsible (Stephen,
> > Sujith etc.).
> > > - As Thomas previously suggested, we should review this in future, possibly
> > after 2.0. Maybe you'll be proven right and we'll all have to apologise for
> > disagreeing!
> > 
> > It sounds like you've already made a decision.  Thats really disheartening.
> > You've gone to alot of effort to make this project more open, and I'd like to
> > encourage you further in that direction.  But your comments above really
> > make
> > me think that you're hoping to isolate your development efforts, which is a
> > step
> > in the wrong direction.
> 
> What I'm doing is stating our position on your proposal, and giving my suggestions for how we move forward. There's no decision made that I'm aware of, and we're not in a position to make a decision in isolation anyway.
> 
> I suspect this is something that we will never agree on. That's fine, as we all have different opinions, and diverse opinions make for a healthy community. In this situation, our view would be that we should continue with the current approach and review again in future, as Thomas has suggested.
> 
> 

I agree, we're not going to agree on this, and you've clearly already made your
decisions and gotten Thomas to agree, so I'm done wasting my time here.

Neil