diff mbox

[ovs-dev,v2] odp-execute: Reuse rss hash in OVS_ACTION_ATTR_HASH.

Message ID 1499958423-17067-1-git-send-email-i.maximets@samsung.com
State Accepted
Headers show

Commit Message

Ilya Maximets July 13, 2017, 3:07 p.m. UTC
If RSS hash exists in a packet it can be reused instead of
5 tuple hash re-calculation in OVS_ACTION_ATTR_HASH. This
leads to increasing the performance of sending packets to
the OVS bonding in userspace datapath up to 10-15%.

Additionally fixed unit test 'select group with dp_hash
selection method' to not depend on dp_hash value.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---

Version 2:
	* Removed assumption on hash_basis value.
	* hash_finish replaced with hash_int as more appropriate.
	* Fixed 'select group with dp_hash selection method' UT.

 lib/odp-execute.c     | 11 +++++++++--
 tests/ofproto-dpif.at |  4 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

Comments

Andy Zhou July 13, 2017, 8:26 p.m. UTC | #1
On Thu, Jul 13, 2017 at 8:07 AM, Ilya Maximets <i.maximets@samsung.com> wrote:
> If RSS hash exists in a packet it can be reused instead of
> 5 tuple hash re-calculation in OVS_ACTION_ATTR_HASH. This
> leads to increasing the performance of sending packets to
> the OVS bonding in userspace datapath up to 10-15%.
>
> Additionally fixed unit test 'select group with dp_hash
> selection method' to not depend on dp_hash value.
>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>

Thanks for address my concern. I am glad to hear the change did not
impact the performance.
Darrell Ball July 18, 2017, 3:48 a.m. UTC | #2
On 7/13/17, 8:07 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ilya Maximets" <ovs-dev-bounces@openvswitch.org on behalf of i.maximets@samsung.com> wrote:

    If RSS hash exists in a packet it can be reused instead of
    5 tuple hash re-calculation in OVS_ACTION_ATTR_HASH. This
    leads to increasing the performance of sending packets to
    the OVS bonding in userspace datapath up to 10-15%.
    
    Additionally fixed unit test 'select group with dp_hash
    selection method' to not depend on dp_hash value.
    
    Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
    ---
    
    Version 2:
    	* Removed assumption on hash_basis value.
    	* hash_finish replaced with hash_int as more appropriate.
    	* Fixed 'select group with dp_hash selection method' UT.
    
     lib/odp-execute.c     | 11 +++++++++--
     tests/ofproto-dpif.at |  4 ++--
     2 files changed, 11 insertions(+), 4 deletions(-)
    
    diff --git a/lib/odp-execute.c b/lib/odp-execute.c
    index d656334..03120bf 100644
    --- a/lib/odp-execute.c
    +++ b/lib/odp-execute.c
    @@ -646,8 +646,15 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
                     uint32_t hash;
     
                     DP_PACKET_BATCH_FOR_EACH (packet, batch) {
    -                    flow_extract(packet, &flow);
    -                    hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
    +                    /* RSS hash can be used here instead of 5tuple for
    +                     * performance reasons. */
    +                    if (dp_packet_rss_valid(packet)) {
    +                        hash = dp_packet_get_rss_hash(packet);
    +                        hash = hash_int(hash, hash_act->hash_basis);
    +                    } else {
    +                        flow_extract(packet, &flow);
    +                        hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
    +                    }

Presently, OVS does not have configurable hashing fields for bonds, although this seems to be asked for.
Also, OVS does not have symmetrical hashing for bonding, as exists for the multipath action.
If/when these features are added, taking the RSS of various input interfaces to hash across the outgoing members of
a given bond would not automatically work since a flexible hash algorithm would not be easily configured the same
across different input devices and also enforcing symmetry would be similarly difficult.
Potentially, we could also make these features mutually exclusive with using the RSS hash as is done here.

This patch does offer some performance gain, so we could revisit as needed in the above cases ?

                         packet->md.dp_hash = hash;
                     }
                 } else {
    diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
    index 8373f90..83c72cf 100644
    --- a/tests/ofproto-dpif.at
    +++ b/tests/ofproto-dpif.at
    @@ -491,10 +491,10 @@ for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do
         AT_CHECK([ovs-appctl netdev-dummy/receive p1 $pkt])
     done
     
    -AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | strip_ufid | strip_used | sort], [0], [dnl
    +AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | sed 's/\(actions:1\)[[01]]/\1X/' | strip_ufid | strip_used | sort], [0], [dnl
     flow-dump from non-dpdk interfaces:
     recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(src=192.168.0.1,frag=no), packets:15, bytes:630, used:0.0s, actions:hash(hash_l4(0)),recirc(0x2)
    -recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:11
    +recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:1X
     ])
     
     OVS_VSWITCHD_STOP
    -- 
    2.7.4
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=yWYQbl5I3JFWIb4JPXMrvVVnm9euTwjLosajStrcJLk&s=qqJkG2Wj8m7F1VTeYznSaFdi8ju8_aLcznAmZzHGNmA&e=
Ilya Maximets July 18, 2017, 6:19 a.m. UTC | #3
On 18.07.2017 06:48, Darrell Ball wrote:
> 
> 
> On 7/13/17, 8:07 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ilya Maximets" <ovs-dev-bounces@openvswitch.org on behalf of i.maximets@samsung.com> wrote:
> 
>     If RSS hash exists in a packet it can be reused instead of
>     5 tuple hash re-calculation in OVS_ACTION_ATTR_HASH. This
>     leads to increasing the performance of sending packets to
>     the OVS bonding in userspace datapath up to 10-15%.
>     
>     Additionally fixed unit test 'select group with dp_hash
>     selection method' to not depend on dp_hash value.
>     
>     Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>     ---
>     
>     Version 2:
>     	* Removed assumption on hash_basis value.
>     	* hash_finish replaced with hash_int as more appropriate.
>     	* Fixed 'select group with dp_hash selection method' UT.
>     
>      lib/odp-execute.c     | 11 +++++++++--
>      tests/ofproto-dpif.at |  4 ++--
>      2 files changed, 11 insertions(+), 4 deletions(-)
>     
>     diff --git a/lib/odp-execute.c b/lib/odp-execute.c
>     index d656334..03120bf 100644
>     --- a/lib/odp-execute.c
>     +++ b/lib/odp-execute.c
>     @@ -646,8 +646,15 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
>                      uint32_t hash;
>      
>                      DP_PACKET_BATCH_FOR_EACH (packet, batch) {
>     -                    flow_extract(packet, &flow);
>     -                    hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
>     +                    /* RSS hash can be used here instead of 5tuple for
>     +                     * performance reasons. */
>     +                    if (dp_packet_rss_valid(packet)) {
>     +                        hash = dp_packet_get_rss_hash(packet);
>     +                        hash = hash_int(hash, hash_act->hash_basis);
>     +                    } else {
>     +                        flow_extract(packet, &flow);
>     +                        hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
>     +                    }
> 
> Presently, OVS does not have configurable hashing fields for bonds, although this seems to be asked for.
> Also, OVS does not have symmetrical hashing for bonding, as exists for the multipath action.
> If/when these features are added, taking the RSS of various input interfaces to hash across the outgoing members of
> a given bond would not automatically work since a flexible hash algorithm would not be easily configured the same
> across different input devices and also enforcing symmetry would be similarly difficult.
> Potentially, we could also make these features mutually exclusive with using the RSS hash as is done here.
> 
> This patch does offer some performance gain, so we could revisit as needed in the above cases ?

For configurable hashing fields, I think, there should be different OVS_HASH_ALG type.
Symmetric hashing is also not required for bonding to work correctly.
From the other side kernel datapath uses exactly same thing to execute OVS_ACTION_ATTR_HASH.
skb_get_hash() is used there which is equal to RSS hash if it available.
So, this patch just unifies kernel and userspace ways to execute hash actions.
If some additional characteristics will be required we will modify both datapaths accordingly.

Best regards, Ilya Maximets.

>                          packet->md.dp_hash = hash;
>                      }
>                  } else {
>     diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>     index 8373f90..83c72cf 100644
>     --- a/tests/ofproto-dpif.at
>     +++ b/tests/ofproto-dpif.at
>     @@ -491,10 +491,10 @@ for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do
>          AT_CHECK([ovs-appctl netdev-dummy/receive p1 $pkt])
>      done
>      
>     -AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | strip_ufid | strip_used | sort], [0], [dnl
>     +AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | sed 's/\(actions:1\)[[01]]/\1X/' | strip_ufid | strip_used | sort], [0], [dnl
>      flow-dump from non-dpdk interfaces:
>      recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(src=192.168.0.1,frag=no), packets:15, bytes:630, used:0.0s, actions:hash(hash_l4(0)),recirc(0x2)
>     -recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:11
>     +recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:1X
>      ])
>      
>      OVS_VSWITCHD_STOP
>     -- 
>     2.7.4
>     
>     _______________________________________________
>     dev mailing list
>     dev@openvswitch.org
>     https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=yWYQbl5I3JFWIb4JPXMrvVVnm9euTwjLosajStrcJLk&s=qqJkG2Wj8m7F1VTeYznSaFdi8ju8_aLcznAmZzHGNmA&e= 
>     
>
Darrell Ball July 18, 2017, 6:42 p.m. UTC | #4
On 7/17/17, 11:19 PM, "Ilya Maximets" <i.maximets@samsung.com> wrote:

    On 18.07.2017 06:48, Darrell Ball wrote:
    > 

    > 

    > On 7/13/17, 8:07 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ilya Maximets" <ovs-dev-bounces@openvswitch.org on behalf of i.maximets@samsung.com> wrote:

    > 

    >     If RSS hash exists in a packet it can be reused instead of

    >     5 tuple hash re-calculation in OVS_ACTION_ATTR_HASH. This

    >     leads to increasing the performance of sending packets to

    >     the OVS bonding in userspace datapath up to 10-15%.

    >     

    >     Additionally fixed unit test 'select group with dp_hash

    >     selection method' to not depend on dp_hash value.

    >     

    >     Signed-off-by: Ilya Maximets <i.maximets@samsung.com>

    >     ---

    >     

    >     Version 2:

    >     	* Removed assumption on hash_basis value.

    >     	* hash_finish replaced with hash_int as more appropriate.

    >     	* Fixed 'select group with dp_hash selection method' UT.

    >     

    >      lib/odp-execute.c     | 11 +++++++++--

    >      tests/ofproto-dpif.at |  4 ++--

    >      2 files changed, 11 insertions(+), 4 deletions(-)

    >     

    >     diff --git a/lib/odp-execute.c b/lib/odp-execute.c

    >     index d656334..03120bf 100644

    >     --- a/lib/odp-execute.c

    >     +++ b/lib/odp-execute.c

    >     @@ -646,8 +646,15 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,

    >                      uint32_t hash;

    >      

    >                      DP_PACKET_BATCH_FOR_EACH (packet, batch) {

    >     -                    flow_extract(packet, &flow);

    >     -                    hash = flow_hash_5tuple(&flow, hash_act->hash_basis);

    >     +                    /* RSS hash can be used here instead of 5tuple for

    >     +                     * performance reasons. */

    >     +                    if (dp_packet_rss_valid(packet)) {

    >     +                        hash = dp_packet_get_rss_hash(packet);

    >     +                        hash = hash_int(hash, hash_act->hash_basis);

    >     +                    } else {

    >     +                        flow_extract(packet, &flow);

    >     +                        hash = flow_hash_5tuple(&flow, hash_act->hash_basis);

    >     +                    }

    > 

    > Presently, OVS does not have configurable hashing fields for bonds, although this seems to be asked for.

    > Also, OVS does not have symmetrical hashing for bonding, as exists for the multipath action.

    > If/when these features are added, taking the RSS of various input interfaces to hash across the outgoing members of

    > a given bond would not automatically work since a flexible hash algorithm would not be easily configured the same

    > across different input devices and also enforcing symmetry would be similarly difficult.

    > Potentially, we could also make these features mutually exclusive with using the RSS hash as is done here.

    > 

    > This patch does offer some performance gain, so we could revisit as needed in the above cases ?

    
    For configurable hashing fields, I think, there should be different OVS_HASH_ALG type.

Right now the only algorithm is OVS_HASH_ALG_L4
So, if/when the hash algorithm becomes configurable, then others would come in or maybe field specifiers. 

    Symmetric hashing is also not required for bonding to work correctly.

Symmetric hashing is a specific feature that requires the hash used to transmit the packet to be same on each end.
This would require matching configuration on each end. However, using the RSS hashes of the input 
interfaces going to the bond on each end is going to be hard to get to work since we don’t control them today in a
manner we would need to for this to work.
We could add code that tries to make them the same, but it will not work in some cases.

    From the other side kernel datapath uses exactly same thing to execute OVS_ACTION_ATTR_HASH.
    skb_get_hash() is used there which is equal to RSS hash if it available.
    So, this patch just unifies kernel and userspace ways to execute hash actions.

We don’t control the associated kernel side configuration through OVS APIs today and it is not clear to me
that would happen any time soon, so kernel side may be a moot point in near future.

But being similar now is good.

    If some additional characteristics will be required we will modify both datapaths accordingly.

A feature can exist in one datapath and not the other.
There are lots of such cases and this may be another one.


    
    Best regards, Ilya Maximets.
    
    >                          packet->md.dp_hash = hash;

    >                      }

    >                  } else {

    >     diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at

    >     index 8373f90..83c72cf 100644

    >     --- a/tests/ofproto-dpif.at

    >     +++ b/tests/ofproto-dpif.at

    >     @@ -491,10 +491,10 @@ for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do

    >          AT_CHECK([ovs-appctl netdev-dummy/receive p1 $pkt])

    >      done

    >      

    >     -AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | strip_ufid | strip_used | sort], [0], [dnl

    >     +AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | sed 's/\(actions:1\)[[01]]/\1X/' | strip_ufid | strip_used | sort], [0], [dnl

    >      flow-dump from non-dpdk interfaces:

    >      recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(src=192.168.0.1,frag=no), packets:15, bytes:630, used:0.0s, actions:hash(hash_l4(0)),recirc(0x2)

    >     -recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:11

    >     +recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:1X

    >      ])

    >      

    >      OVS_VSWITCHD_STOP

    >     -- 

    >     2.7.4

    >     

    >     _______________________________________________

    >     dev mailing list

    >     dev@openvswitch.org

    >     https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=yWYQbl5I3JFWIb4JPXMrvVVnm9euTwjLosajStrcJLk&s=qqJkG2Wj8m7F1VTeYznSaFdi8ju8_aLcznAmZzHGNmA&e= 

    >     

    >
Darrell Ball July 20, 2017, 12:56 a.m. UTC | #5
-----Original Message-----
From: Darrell Ball <dball@vmware.com>

Date: Tuesday, July 18, 2017 at 11:42 AM
To: Ilya Maximets <i.maximets@samsung.com>, "ovs-dev@openvswitch.org" <ovs-dev@openvswitch.org>, Andy Zhou <azhou@ovn.org>
Cc: Heetae Ahn <heetae82.ahn@samsung.com>
Subject: Re: [ovs-dev] [PATCH v2] odp-execute: Reuse rss hash in OVS_ACTION_ATTR_HASH.

    
    
    On 7/17/17, 11:19 PM, "Ilya Maximets" <i.maximets@samsung.com> wrote:
    
        On 18.07.2017 06:48, Darrell Ball wrote:
        > 

        > 

        > On 7/13/17, 8:07 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ilya Maximets" <ovs-dev-bounces@openvswitch.org on behalf of i.maximets@samsung.com> wrote:

        > 

        >     If RSS hash exists in a packet it can be reused instead of

        >     5 tuple hash re-calculation in OVS_ACTION_ATTR_HASH. This

        >     leads to increasing the performance of sending packets to

        >     the OVS bonding in userspace datapath up to 10-15%.

        >     

        >     Additionally fixed unit test 'select group with dp_hash

        >     selection method' to not depend on dp_hash value.

        >     

        >     Signed-off-by: Ilya Maximets <i.maximets@samsung.com>

        >     ---

        >     

        >     Version 2:

        >     	* Removed assumption on hash_basis value.

        >     	* hash_finish replaced with hash_int as more appropriate.

        >     	* Fixed 'select group with dp_hash selection method' UT.

        >     

        >      lib/odp-execute.c     | 11 +++++++++--

        >      tests/ofproto-dpif.at |  4 ++--

        >      2 files changed, 11 insertions(+), 4 deletions(-)

        >     

        >     diff --git a/lib/odp-execute.c b/lib/odp-execute.c

        >     index d656334..03120bf 100644

        >     --- a/lib/odp-execute.c

        >     +++ b/lib/odp-execute.c

        >     @@ -646,8 +646,15 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,

        >                      uint32_t hash;

        >      

        >                      DP_PACKET_BATCH_FOR_EACH (packet, batch) {

        >     -                    flow_extract(packet, &flow);

        >     -                    hash = flow_hash_5tuple(&flow, hash_act->hash_basis);

        >     +                    /* RSS hash can be used here instead of 5tuple for

        >     +                     * performance reasons. */

        >     +                    if (dp_packet_rss_valid(packet)) {

        >     +                        hash = dp_packet_get_rss_hash(packet);

        >     +                        hash = hash_int(hash, hash_act->hash_basis);

        >     +                    } else {

        >     +                        flow_extract(packet, &flow);

        >     +                        hash = flow_hash_5tuple(&flow, hash_act->hash_basis);

        >     +                    }

        > 

        > Presently, OVS does not have configurable hashing fields for bonds, although this seems to be asked for.

        > Also, OVS does not have symmetrical hashing for bonding, as exists for the multipath action.

        > If/when these features are added, taking the RSS of various input interfaces to hash across the outgoing members of

        > a given bond would not automatically work since a flexible hash algorithm would not be easily configured the same

        > across different input devices and also enforcing symmetry would be similarly difficult.

        > Potentially, we could also make these features mutually exclusive with using the RSS hash as is done here.

        > 

        > This patch does offer some performance gain, so we could revisit as needed in the above cases ?

        
        For configurable hashing fields, I think, there should be different OVS_HASH_ALG type.
    
    Right now the only algorithm is OVS_HASH_ALG_L4
    So, if/when the hash algorithm becomes configurable, then others would come in or maybe field specifiers. 
    
        Symmetric hashing is also not required for bonding to work correctly.
    
    Symmetric hashing is a specific feature that requires the hash used to transmit the packet to be same on each end.
    This would require matching configuration on each end. However, using the RSS hashes of the input 
    interfaces going to the bond on each end is going to be hard to get to work since we don’t control them today in a
    manner we would need to for this to work.
    We could add code that tries to make them the same, but it will not work in some cases.
    
        From the other side kernel datapath uses exactly same thing to execute OVS_ACTION_ATTR_HASH.
        skb_get_hash() is used there which is equal to RSS hash if it available.
        So, this patch just unifies kernel and userspace ways to execute hash actions.
    
    We don’t control the associated kernel side configuration through OVS APIs today and it is not clear to me
    that would happen any time soon, so kernel side may be a moot point in near future.
    
    But being similar now is good.
    
        If some additional characteristics will be required we will modify both datapaths accordingly.
    
    A feature can exist in one datapath and not the other.
    There are lots of such cases and this may be another one.

In future, if a configurable hash field feature for bonding is done for userspace
datapath, we would need to bypass the RSS hash usage and calculate a single
tx hash regardless of the input interface, as there is no reasonable way to make sure
any possible input interface is using the same RSS hash.
In this case, we can simply bypass the RSS hash entirely for bonding and run a hash
calculation similarly to how we do it today for OVS_HASH_ALG_L4.
I think this would be fine.

Acked-by: Darrell Ball <dlu998@gmail.com>




    
    
        
        Best regards, Ilya Maximets.
        
        >                          packet->md.dp_hash = hash;

        >                      }

        >                  } else {

        >     diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at

        >     index 8373f90..83c72cf 100644

        >     --- a/tests/ofproto-dpif.at

        >     +++ b/tests/ofproto-dpif.at

        >     @@ -491,10 +491,10 @@ for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do

        >          AT_CHECK([ovs-appctl netdev-dummy/receive p1 $pkt])

        >      done

        >      

        >     -AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | strip_ufid | strip_used | sort], [0], [dnl

        >     +AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | sed 's/\(actions:1\)[[01]]/\1X/' | strip_ufid | strip_used | sort], [0], [dnl

        >      flow-dump from non-dpdk interfaces:

        >      recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(src=192.168.0.1,frag=no), packets:15, bytes:630, used:0.0s, actions:hash(hash_l4(0)),recirc(0x2)

        >     -recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:11

        >     +recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:1X

        >      ])

        >      

        >      OVS_VSWITCHD_STOP

        >     -- 

        >     2.7.4

        >     

        >     _______________________________________________

        >     dev mailing list

        >     dev@openvswitch.org

        >     https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=yWYQbl5I3JFWIb4JPXMrvVVnm9euTwjLosajStrcJLk&s=qqJkG2Wj8m7F1VTeYznSaFdi8ju8_aLcznAmZzHGNmA&e= 

        >     

        >
Andy Zhou July 20, 2017, 11:25 p.m. UTC | #6
> Acked-by: Darrell Ball <dlu998@gmail.com>

Thanks llya for the patch and Darrell for the review!
Pushed to master.
diff mbox

Patch

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index d656334..03120bf 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -646,8 +646,15 @@  odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
                 uint32_t hash;
 
                 DP_PACKET_BATCH_FOR_EACH (packet, batch) {
-                    flow_extract(packet, &flow);
-                    hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
+                    /* RSS hash can be used here instead of 5tuple for
+                     * performance reasons. */
+                    if (dp_packet_rss_valid(packet)) {
+                        hash = dp_packet_get_rss_hash(packet);
+                        hash = hash_int(hash, hash_act->hash_basis);
+                    } else {
+                        flow_extract(packet, &flow);
+                        hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
+                    }
                     packet->md.dp_hash = hash;
                 }
             } else {
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 8373f90..83c72cf 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -491,10 +491,10 @@  for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do
     AT_CHECK([ovs-appctl netdev-dummy/receive p1 $pkt])
 done
 
-AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | strip_ufid | strip_used | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/dp_hash(.*\/0x1)/dp_hash(0xXXXX\/0x1)/' | sed 's/\(actions:1\)[[01]]/\1X/' | strip_ufid | strip_used | sort], [0], [dnl
 flow-dump from non-dpdk interfaces:
 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(src=192.168.0.1,frag=no), packets:15, bytes:630, used:0.0s, actions:hash(hash_l4(0)),recirc(0x2)
-recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:11
+recirc_id(0x2),dp_hash(0xXXXX/0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:15, bytes:630, used:0.0s, actions:1X
 ])
 
 OVS_VSWITCHD_STOP