[ovs-dev] [PATCH v3 0/2] conntrack : Add support for rx chceksum offload.

Chandran, Sugesh sugesh.chandran at intel.com
Mon Jul 10 23:51:51 UTC 2017


Hi Darell, Ilya

Regards
_Sugesh


> -----Original Message-----
> From: Darrell Ball [mailto:dball at vmware.com]
> Sent: Monday, July 10, 2017 5:53 PM
> To: Ilya Maximets <i.maximets at samsung.com>; ovs-dev at openvswitch.org;
> Ben Pfaff <blp at ovn.org>
> Cc: Chandran, Sugesh <sugesh.chandran at intel.com>; Traynor, Kevin
> <kevin.traynor at 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.maximets at 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.
> 



More information about the dev mailing list