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

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

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.

> >   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
is almost impossible to avoid when we have multiple trees.

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

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

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.

	--yliu

> > > 		- librte_acl
> > > 		- librte_distributor
> > > 		- librte_hash
> > > 		- librte_jobstats
> > > 		- librte_lpm
> > > 		- librte_meter
> > > 		- librte_pipeline
> > > 		- librte_power
> > > 		- librte_reorder
> > > 		- librte_ring
> > > 		- librte_sched
> > > 		- librte_table
> > > 		- librte_timer
> > > 		- librte_vhost
> > > 
> > > Thats just a rough stab mind, but perhaps it would get the ball rolling.  I'd be
> > > willing to take maintainership of one of these subtrees if there is consensus
> > > around my doing so.
> >