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
