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

Darrell Ball dball at vmware.com
Mon Jul 10 16:53:19 UTC 2017



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.

    
    Best regards, Ilya Maximets.
    



More information about the dev mailing list