Hi Sugesh

For future reference, the naming guidelines are actually pretty flexible in 
this regard.

Note that the cover letter will not get merged anyways, so it is moot.

Can you

1) Split out all the changes to the dp-packet module as Patch 1.
You can say the new ‘*_bad’ APIs will get their first user in Patch 2
2) Add me as co-author as the additions b/w V2 and V4 were significant.

Thanks Darrell





On 7/10/17, 4:51 PM, "Chandran, Sugesh" <sugesh.chand...@intel.com> wrote:

    Hi Darell, Ilya
    
    Regards
    _Sugesh
    
    
    > -----Original Message-----
    > From: Darrell Ball [mailto:db...@vmware.com]
    > Sent: Monday, July 10, 2017 5:53 PM
    > To: Ilya Maximets <i.maxim...@samsung.com>; ovs-dev@openvswitch.org;
    > Ben Pfaff <b...@ovn.org>
    > Cc: Chandran, Sugesh <sugesh.chand...@intel.com>; Traynor, Kevin
    > <kevin.tray...@intel.com>
    > Subject: Re: [ovs-dev] [PATCH v3 0/2] conntrack : Add support for rx
    > chceksum offload.
    > 
    > 
    > 
    > On 7/10/17, 1:37 AM, "Ilya Maximets" <i.maxim...@samsung.com> wrote:
    > 
    >     > ‘chceksum’ is misspelled
    >     >
    >     > Since these patches really only affect ‘dpdk’, the module name 
‘dpdk’
    > may more accurately
    >     > reflect the real effect of these patches.
    > 
    >     Please, don't do that. Only patches that changes lib/dpdk.{c,h} should
    >     have 'dpdk' prefix in subject line. All other patches should have 
proper
    >     module name according to code they're changing.
    > 
    >     I wanted to rise this issue many times ago. So, maybe it's time.
    >     There are many places where changes made to improve the DPDK-enabled
    >     datapath, but the most of changes are generic and doesn't have many
    >     DPDK-related code. Such patches doesn't need to have 'dpdk' as a 
prefix.
    >     This only makes a mess from the git history and you can never say for
    >     sure what module was changed in a particular patch by looking only on 
its
    >     subject.
    > 
    > These changes affect only the dpdk datapath.
    > I gave a full response to Sugesh.
    > 
    > 
    >     IMHO, patches should have prefixes according to modules they're 
changing
    >     like it is described in contribution guide. Generic changes should be
    >     reviewed by not only people interested in DPDK. Addition of such
    >     misleading prefixes forces them to miss maybe important generic 
changes.
    > 
    > In this case, the module name is misleading since the changes affect much
    > more than just conntrack; that is the point.
    > The changes affect generic checksum offloading by virtue of changes to dp-
    > packet.
    > These changes are in fact specific to dpdk.
    > 
    > 
    >     From the other side, many people adds 'dpdk' prefix to patches 
targeted
    >     to 'netdev-dpdk' which is not right too.
    >     All patches should have the right prefix according to the module they 
are
    >     trying to change. That is my point of view.
    > 
    > 
    >     In this particular case patches actually adds generic functionality
    >     which can be used even without DPDK. For example, if we'll implement
    >     checksum offloading for netdev-linux (not so hard). DPDK already
    > mentioned
    >     in commit message as the target and there is no need for misleading
    > prefixes.
    > 
    > That is not correct.
    > Here in the present, these changes are specific to dpdk. We work with the
    > present not one possible hypotectical future.
    > ‘IF’, in future, other code is changed that allows sharing of some code
    > changes beyond dpdk, then discussion of netdev-linux becomes relevant. It
    > is not relevant now.
    > 
    > 
    [Sugesh]I sent out the v4 patch already with same conntrack subject line 
considering that’s the guideline.
    I don’t mind changing that to dpdk, if we are following the practice to 
keep subject line based on the patch specific to what.
    
    >     Best regards, Ilya Maximets.
    > 
    
    

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to