From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id D6B505A2B for ; Sat, 17 Jan 2015 20:57:08 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP; 17 Jan 2015 11:57:07 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,417,1418112000"; d="scan'208";a="513775942" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by orsmga003.jf.intel.com with ESMTP; 17 Jan 2015 11:50:40 -0800 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.28]) by IRSMSX108.ger.corp.intel.com ([169.254.11.64]) with mapi id 14.03.0195.001; Sat, 17 Jan 2015 19:57:05 +0000 From: "O'driscoll, Tim" To: Neil Horman Thread-Topic: [dpdk-dev] Why nothing since 1.8.0? Thread-Index: AQHQMDgkRICerg8+NU+uGsyD3xhjm5zAGjKAgAB5GwCAAANIgIAAWrAAgAA2YQCAAEhygIAAF++AgAAuIsCAAUKzgIABxIRw Date: Sat, 17 Jan 2015 19:57:04 +0000 Message-ID: <26FA93C7ED1EAA44AB77D62FBE1D27BA54CA4BCC@IRSMSX102.ger.corp.intel.com> References: <20150114122352.63ef79eb@urahara> <1717148.0lUn7GOqmp@xps13> <20150115130616.GA22455@hmsreliant.think-freely.org> <12253538.YOLaLVcskf@xps13> <20150115185112.GC22455@hmsreliant.think-freely.org> <26FA93C7ED1EAA44AB77D62FBE1D27BA54C9978E@IRSMSX102.ger.corp.intel.com> <20150116165119.GC27496@hmsreliant.think-freely.org> In-Reply-To: <20150116165119.GC27496@hmsreliant.think-freely.org> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] Why nothing since 1.8.0? X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 17 Jan 2015 19:57:11 -0000 I'm going to risk the wrath of the open source purists amongst you by top-p= osting. The reason is that there has been lots of email on this subject, an= d I want to summarise the key issue and clearly state our position on it. H= operfully nobody is offended by this! This thread has generated lots of debate, which is good, and there are a nu= mber 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. 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 r= equests to the main repo. You're proposing the following: - Remove the individual PMD repositories, and replace them with a single re= pository 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.=20 As I've said before, and as Venky has also explained in private email on th= is, we do not agree with this proposed change. We believe it would be a bac= kward step, and would have an adverse impact on DPDK. 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'r= e proposing is to take some of that control away, and give it to someone el= se (in this case you) who isn't familiar with the details. We don't agree w= ith that approach. The arguments I've heard in favour of your proposal are summarised below. A= pologies 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. 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 th= e same as the Linux kernel. The difference we're discussing is simply the g= ranularity of the repositories. 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 d= ue to its size and complexity, but that doesn't mean we need to do the same= for DPDK. 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 developi= ng, the most efficient way for us to manage the work is to combine these an= d have one of our SMEs act as maintainer as well. That's an internal decisi= on that suits the structure of our development team and the current state o= f the i40e and fm10k PMDs. Others are obviously free to make their own choi= ces for PMDs that they're developing. 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/ma= intainer 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. 4. We shouldn't give maintainer work to an SME as it detracts from their ot= her tasks. We don't anticipate a problem with workload for the people that we have in = these positions. 5. There will be extra overhead for developers who want to implement change= s 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 con= tributor 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. 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, whic= h we're looking into. We may choose to have a single repository with a sing= le maintainer/SME for e1000/igb/ixgbe combined, which would limit the overa= ll 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, Su= jith 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 fo= r disagreeing! Tim > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Friday, January 16, 2015 4:51 PM > To: O'driscoll, Tim > Cc: Thomas Monjalon; dev@dpdk.org > Subject: Re: [dpdk-dev] Why nothing since 1.8.0? >=20 > On Thu, Jan 15, 2015 at 09:55:00PM +0000, O'driscoll, Tim wrote: > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman > > > Sent: Thursday, January 15, 2015 6:51 PM > > > To: Thomas Monjalon > > > Cc: dev@dpdk.org > > > Subject: Re: [dpdk-dev] Why nothing since 1.8.0? > > > > > > On Thu, Jan 15, 2015 at 06:25:33PM +0100, Thomas Monjalon wrote: > > > > 2015-01-15 08:06, Neil Horman: > > > > > On Thu, Jan 15, 2015 at 10:51:38AM +0100, Thomas Monjalon wrote: > > > > > > 2015-01-15 04:27, Ouyang, Changchun: > > > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, > Helin > > > > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil > > > Horman > > > > > > > > > On Wed, Jan 14, 2015 at 12:23:52PM -0800, Stephen > Hemminger > > > wrote: > > > > > > > > > > Ok, so 1.8.0 came out almost a month ago and none of th= e > > > patches > > > > > > > > > > that were deferred waiting for the release got merged s= ince > > > then. > > > > > > > > > > Last commit in git is the 1.8.0 release. > > > > > > > > > > > > > > > > > > > > Where is the post-merge window bundle, where are the > later > > > commits? > > > > > > > > > > Lots of patches are sitting rotting in patchwork... > > > > > > > > > > > > > > > > > > +1, I've had the same questions. > > > > > > > > > Neil > > > > > > > > > > > > > > > > +1, Some patch set might be ready for being merged. > > > > > > > > > > > > > > +1, the earlier some patches are merged into mainline, and t= he > easier > > > those > > > > > > > sequent patch sets can resolve their conflicts. > > > > > > > > > > > > +1, there are some patches which are properly reviewed > > > > > > > > > > > > Reminder: sub-tree to manage specific part of DPDK can be open = on > > > request > > > > > > > > > > Ok, I think what you're saying here is you're too busy to handle = all the > > > patches > > > > > comming in at the moment. As such I'd like to propose a sub-tree > > > encompassing > > > > > all the pmds in DPDK. I would envision that including all the ac= utal > pmd's > > > in > > > > > the tree, as well as the infrastructure that is used to interface= them to > the > > > > > core (i.e. the ethdev/rte_ether library). I'll gladly maintain t= he patch > pool > > > > > and send you pull requests. > > > > > > > > The list of PMDs is increasing: > > > > librte_pmd_af_packet > > > > librte_pmd_bond > > > > librte_pmd_e1000 > > > > librte_pmd_enic > > > > librte_pmd_i40e > > > > librte_pmd_ixgbe > > > > librte_pmd_pcap > > > > librte_pmd_ring > > > > librte_pmd_virtio > > > > librte_pmd_vmxnet3 > > > > librte_pmd_xenvirt > > > > There is already some sub-trees for bnx2x, fm10k and i40e: > > > > http://dpdk.org/browse/ > > > > > > > Yes, and I've mentioned before that that is an absolutely silly way t= o > break > > > out > > > subtrees. You have to find a balance of workload distribution and > developer > > > convienience. > > > > > > I also note that these are problematic because you're not merging > anything > > > from them. Is it your intention to keep bnx2 and fm10k separate in > > > perpituity? > > > If so, thats a real problem, because then we effectively just have se= veral > out > > > of tree drivers, and thats just unacceptible. > > > > > > > > If you could set me up with a login to dpdk.org, I'd appreciate i= t. > > > > > > > > It is preferred to have 1 sub-tree per module. > > > > What do you think of managing contributions for af_packet and/or > virtio? > > > > It would make sense as virtio is a RedHat technology. > > > > Maybe it could include vhost lib and example. > > > > > > > No, for reasons I've mentioned before. If you take each pmd/library = and > > > create > > > a subtree for it, you've created the most fine grained control of sub= trees > you > > > could ask for, but you've created a nighmare of a burden on developer= s > who > > > want > > > to update any code, especially if they have patches that hit multiple > trees. > > > > > > Look at some of the stats in the dpdk tree: > > > > > > Library Commits between 1.7.0 and 1.8.0 > > > librte_acl 5 > > > librte_cfgfile 0 > > > librte_cmdline 4 > > > librte_compat 0 > > > librte_distributor 5 > > > librte_eal 125 > > > librte_ether 31 > > > librte_hash 1 > > > librte_ip_frag 5 > > > librte_ivshmem 0 > > > librte_kni 2 > > > librte_kvargs 0 > > > librte_lpm 1 > > > librte_malloc 1 > > > librte_mbuf 39 > > > librte_mempool 4 > > > librte_meter 0 > > > librte_net 4 > > > librte_pipeline 0 > > > librte_pmd_af_packet 4 > > > librte_pmd_bond 20 > > > librte_pmd_e1000 21 > > > librte_pmd_enic 12 > > > librte_pmd_i40e 90 > > > librte_pmd_ixgbe 83 > > > librte_pmd_pcap 4 > > > librte_pmd_ring 0 > > > librte_pmd_virtio 21 > > > librte_pmd_vmxnet3 21 > > > librte_pmd_xenvirt 6 > > > librte_port 6 > > > librte_power 3 > > > librte_ring 2 > > > librte_sched 1 > > > librte_table 7 > > > librte_timer 0 > > > librte_vhost 30 > > > > > > If you look at all of the pmds in the dpdk tree, we're talking about = ~300 > > > patches per release. If you look at the net-next tree for the linux = kernel, > > > Dave Miller merged 569 patches on his own (based on the following > > > command: > > > git log --pretty=3Dformat:%H v3.17..v3.18 -- drivers/net/ethernet/ > net/core/ | > > > wc > > > -l) > > > > > > And that doesn't account for the ~500 patches that come in via pull > request > > > from > > > the wireless subtree. Nor does it account for the merge window for n= et- > > > next > > > being 2 months instead of dpdk's 6 months. Theres no need in any way > for > > > 12 > > > maintainers to be twiddling their thumbs waiting on ~20 patches each, > and > > > for > > > that split, you've forced developers to potentially develop patches > against 12 > > > trees (12 being the current number of PMD's that are in the dpdk). > > > > > > The right answer here is balance. Let me split out the pmd's and eth= ernet > > > infrastructure libraries to a subtree. I'll pull in patches posted r= egarding > > > pmd's and librte_ether/ip_frag etc, and send you a pull requests afte= r > each > > > release so you get all the latest bits, and then pulls for stabilizat= ion on > each > > > -rc. I can manage 300 patches without issue, and that takes a load of= f your > > > shoulders. I'll get fm10k integrated, as well as bnx2. That gives u= s a single > > > alternate tree for developers to go to for pmd and pmd infrastructure > > > updates. > > > Its a win-win. > > > > > > Regards > > > Neil > > > > I agree with Thomas on this. The approach we've been taking for PMDs fo= r > newer Intel NICs is to have a separate sub-repository with a maintainer > who's an expert in the area. This offloads some work from Thomas, ensures > that the maintainer is very familiar with the PMD code and the correspond= ing > hardware, > As I mentioned to you in a private note, you're convoluting two roles int= o a > single one here to the detriment of both: >=20 > 1) A tree maintainer is a machine, who is responsible for the mechanicm o= f > SCM > and tree maintenence. They are responsible for merging patches, making > sure > that merged patches make it to their upstream tree, ensuring conflicts ge= t > resolved, and above all, making sure that SME's review patches before the= y > get > merged >=20 > 2) An SME (subject matter expert), is just that, someone who is intimatel= y > familiar with the hardware and software of a certain bit of a project. T= heir > only responsibility is to ensure that proposed changes are legitimate and > safe. > They review patches within their purview and provide acks to the tree > maintainer. >=20 > When you merge these roles, you take the person most responsible for the > stability of their code, and distract them with 1000 patch management > operations, and from the work of developing new code for their area of > expertise. >=20 > The separation of these roles has evolved in several communities because = of > exactly the above. Linus knows alot about the kernel, but he's never ope= ned > the > I40e datasheet, and he doesn't have to because he trusts Dave M to ensure > that > code in his pull requests is legitimate. Dave in turn relies on someone = at > Intel to ack every I40e patch on the list before he merges it. He does e= xactly > the same thing with every single other sub-component as listed in the > MAINTAINERS file. Thats how he is able to manage twice as many patches a= s > the > dpdk can in half the amount of time. If its efficiency you're after, tha= ts the > route to go. Please don't ignore all that evolutionary refinement. >=20 >=20 > >and doesn't involve too much additional work for the developers involved > (as you said, there isn't a huge volume of commits for any individual PMD= ). > Patch volume isn't where the additional effort comes in. Its in the > determination of what tree to develop against. >=20 > Lets take an example. I'm doing work on the I40e card, so I have to look= to > see > if theres a separate tree for it. I figure out there is, so I have to cl= one > that tree, and develop a patch against it. So far so good. >=20 > A few days later I notice a bug in the pcap pmd, so I want to write a pat= ch to > fix it. If we carry your model out to the point where each pmd has its o= wn > subtree, I have to go find that new tree, and clone it as well (or add a = new > remote to my existing tree), pull those changes, branch from the proper > master, > and continue my work). >=20 > If later I find a bug in the virtio pmd, I have to repeat the above proce= ss > again, cloning a new tree, or adding a new remote for that drivers tree. >=20 > It doesn't sound like alot when you're just sitting in a room talking abo= ut it, > but for the developers actually working on the code, its a significant > inconvienience, since it means that any pmd you want to touch requires yo= u > to go > through this lookup process, ensuring your branching from the right maste= r > branch in the proper tree. >=20 > Conversely, if you put all pmds in a single subtree, it doesn't matter wh= at > pmd a person is working on, they only have to clone the pmd subtree, and > they're > good to go. >=20 >=20 > > We have this in place for i40e now, and will be applying this to fm10k,= which > hasn't been upstreamed yet but will be in time for the 2.0 release. Based= on > our experiences with those, we may well look at extending the model to > include PMDs for other Intel NICs, and possibly other libraries, as well. > > > You really haven't. You have an i40e tree, but you have dozens of I40e > patches > on the list, and all of them thus far have been integrated into the main = tree. > Ditto with the bnx2x tree and others that have been separated. Please > remember > subject matter expert !=3D repo maintainer. The roles can be, and should= be, > separeated. >=20 > > As you said, there's a balance to be struck, and too many subtrees may > become unmanageable. With respect to your concern about developers > having to potentially develop patches against multiple subtrees, this has > never been raised as a concern by any of our development team. Is there > any historical data on the number of changes that would fall into this > category so we can see if it's a real problem or not? > Look at the linux kernel if you want historical data on where the balance= is. > Major subsystems has become the natural breaking point for subtree > maintenence. > Every net driver goes through Millers net or net-next tree. All scsi upd= ates > go > through Bottomleys scsi tree. FCoE changes used go through what used to > be Rob > Loves FCoE tree. It just makes sense. I'm sure there isn't recorded dat= a to > show anything more granular than that because no one ever seriously > considered > breaking out all 301 network drivers into their own subtree, because its > obviously unmanageable at that scale. Even taken all as a single unit (as > the kernel does for network drivers), a single maintainer can still handl= e the > patch volume with the right subject matter experts doing timely reviews. >=20 >=20 > The point of this very long message is that maintainers !=3D repos. Just > because > you're a subject matter expert in a given bit of hardware/pmd, in no way > means > you need to be responsible for patch merging/maintenence. Doing so just > makes > everything harder. >=20 > Neil