Re: [dpdk-dev] Proposal for a new Committer model

[Neil Horman <nhorman@tuxdriver.com>]

On Thu, Nov 24, 2016 at 01:53:55PM +0800, Yuanhan Liu wrote:
> On Wed, Nov 23, 2016 at 03:19:19PM -0500, Neil Horman wrote:
> > On Wed, Nov 23, 2016 at 11:41:20PM +0800, Yuanhan Liu wrote:
> > > On Wed, Nov 23, 2016 at 09:11:54AM -0500, Neil Horman wrote:
> > > > > Could we define some of the potential subtrees now and look to introduce them in the this release cycle? EAL and the Core libs, as suggested by Thomas, seem like 2 obvious ones.
> > > > > 
> > > > Sure, I'd suggest the following:
> > > 
> > > I would pull the git history to see which components are in
> > > active status in last release (or even, in last few release).
> > > And try to make a sub-tree if corresponding component is hot.
> > > 
> > > # the 2nd volume shows how many patches prefixed with a related component
> > > [yliu@yliu-dev ~/dpdk]$ git log --oneline v16.07..v16.11 | awk '{print $2}' | \
> > > 		        sort | uniq -c  | sort -nr | head -30 | nl
> > >      1       52 doc:
> > >      2       40 net/ixgbe/base:
> > >      3       38 app/test:
> > >      4       37 kni:
> > >      5       27 vhost:
> > >      6       27 net/virtio:
> > >      7       27 net/mlx5:
> > >      8       26 app/testpmd:
> > >      9       25 net/i40e:
> > >     10       23 net/pcap:
> > >     11       22 net/bnxt:
> > >     12       20 net/enic:
> > >     13       18 net/qede:
> > >     14       17 net/thunderx:
> > >     15       16 net/qede/base:
> > >     16       16 eal:
> > >     17       15 net/ixgbe:
> > >     18       14 net:
> > >     19       14 crypto/qat:
> > >     20       13 scripts:
> > >     21       13 net/bnx2x:
> > >     22       12 net/i40e/base:
> > >     23       12 examples/ipsec-secgw:
> > >     24       11 mbuf:
> > >     25       11 hash:
> > >     26       10 lib:
> > >     27       10 examples/ip_pipeline:
> > >     28       10 ethdev:
> > >     29        9 pci:
> > >     30        7 net/vmxnet3:
> > >     ...
> > >     46        3 pdump:
> > >     47        3 net/virtio_user:
> > >     48        3 net/ring:
> > >     49        3 net/nfp:
> > >     50        3 net/mlx:
> > >     51        3 net/ena:
> > >     52        3 net/e1000:
> > >     53        3 net/bonding:
> > >     ...
> > >     56        2 sched:
> > >     57        2 port:
> > >     ...
> > >     65        1 timer:
> > >     66        1 remove
> > >     67        1 pmdinfogen:
> > >     68        1 net/igb:
> > >     69        1 net/enic/base:
> > >     70        1 meter:
> > >     ...
> > >     84        1 cfgfile:
> > >     85        1 app/procinfo:
> > >     86        1 app/proc_info:
> > >     87        1 acl:
> > > 
> > > Something obvious is that:
> > > 
> > > - "doc" deserves a sub-tree, and John is a perfect committer for that
> > >   if he's willing to.
> > > 
> > > - generally, I'd agree with Neil that most (if not all) pmds may need
> > >   a sub-tree. While, some others may not, for example, net/ring, net/pcap.
> > > 
> > No, thats the opposite of what I think.  I think all net pmds should flow
> > through a single subtree, all crypto pmds through another, etc.
> 
> I misunderstood it. I was think you were suggesting to create a sub-tree
> for most (or all) pmds. Some of my comments didn't apply then.
> 
> But yes, we have already done that: we have next-net and next-crypto.
> 
Yes, I'm speaking elsewhere in this thread on the topic of how that merge
process works, and how it can be improved.  But this granularity I think is
correct.

> > >   For those non-active pmds, I think it's okay to let the generic
> > >   pmd committer to cover them.
> > > 
> > Not sure what you're getting at here.  Low volume pms (or any library) can still
> > go through a subtree.  The goal is to fragmet the commit work so one person
> > doesn't have to do it all.
> > 
> > > - it's not that wise to me to list all the components we have so far
> > >   and make a sub-tree for each of them.
> > > 
> > I think you misunderstood the organization of my last note.  I agree with you
> > here.  When I listed the core and listed several libraries under it, my intent
> > was to create a core subtree that accepted patches for all of those libraries.
> > 
> > >   For example, some components like librte_{port, pdump, cfgfile, acl,
> > >   and etc} just have few (or even, just one) commits in last release.
> > >   It makes no sense to me to introduce a tree for each of them.
> > > 
> > Yes, this is what I was saying in my last note.
> > 
> > > Another thought is we could also create sub-trees based on category
> > > but not on components like Neil suggested, especially that EAL looks
> > > way too big to be maintained in one tree. Instead, it could be something
> > > like:
> > > 
> > > - a tree for BSD
> > > 
> > This gets tricky, because then several libraries will be covered by multiple
> > trees, and that leads to merge conflicts.
> 
> If we go that way, I meant a sub-sub-tree under EAL sub-tree. And conflicts
Hmmm.....I'm not sure we're quite there yet.  I'm not opposed to such a subtree
per-se, but I'd be somewhat curious to see how much BSD specific change or
linux-specific change was going into EAL before we broke that out separately.
Perhaps the best approach is to break out EAL and let that subtree maintainer
choose to further subdivide it as needed

> is almost impossible to avoid when we have multiple trees.
> 
I don't think thats particularly true.  Especially if the divisions are made on
a rough file basis.  A high percentage of  files should rarely if ever conflict.

Of course, there is always the possibility of a conflict, but we can minimize
it.

> > > - a tree for ARM (and some other trees for other platforms)
> > > 
> > > - a tree for mem related (mempool, mbuf, hugepage, etc)
> > > 
> > > - a tree for BUS
> > > 
> > > - ...
> > > 
> > > 
> > > Last but not the least, I think it's general good to have more and
> > > more trees in the end. But I don't think it's a good idea to go
> > > radically and create all those trees once (say in one release).
> > > 
> > > Something I would like to suggest is one or two (or a bit more) at
> > > a release. For example, if I remember them well, we have next-net
> > > tree at 16.04, and next-virtio (including vhost) at 16.07, and a
> > > recent one, next-crypto at 16.11.
> > > 
> > I'm not sure what you mean by this.
> 
> I meant we already add more and more trees, from 0 and 1, and then
> from 1 to 3 (and more), a bit slowly but not radically.
> 
Ah, you're commenting on the frequency with which we create new trees.  I tend
to think that we shouldn't really limit ourselves artificially there.  That is
to say, lets do what makes sense in terms of community organization, and in
terms of resources.  Creating new trees at release boundaries seems natural, but
lets not assert that there is a 'right' number of trees to create in any single
given release.

> >  -next trees rather by definition should e
> > rebased on a release to start at the head of thomas's tree and add commits from
> > there based on their subject area.
> 
> Yep, and that's we are doing.
> 
Ok

> And maybe we could revisit your suggested list:
> 
> > > > 	* net-pmds:
> > > > 		- all network pmds located under drivers/net
> > > > 		- librte_net
> > > > 		- libtre_ether
> > > > 		- librte_ip_frag
> > > > 		- librte_pdump
> > > > 		- librte_port
> > > > 	* crypto-pmds:
> > > > 		- all crypto pmds located under drivers/crypto
> > > > 		- librte_cryptodev
> 
> We already have the two.
> 
> > > > 	* eal:
> > > > 		- librte_eal
> 
> I think EAL deserves to have a sub-tree.
> 
> > > > 	* core:
> > > > 		- librte_cfgfile
> > > > 		- librte_cmdline
> > > > 		- librte_compat
> > > > 		- librte_kvargs
> > > > 		- librte_kni
> > > > 		- librte_compat
> > > > 	* misc:
> 
> It may be vague to define which belongs to core and which belongs to
> misc. It might be better to have a lib sub-tree, to hold all others
> that don't belong to other sub-trees.
> 
Sure, I'm not tied to any particular names, just trying to find a sane
subdivision more than anything else

Neil