diff mbox

[ovs-dev,v2] dpif-netdev: Fix insertion probability

Message ID 1498230465-12337-1-git-send-email-ciara.loftus@intel.com
State Superseded
Headers show

Commit Message

Ciara Loftus June 23, 2017, 3:07 p.m. UTC 
emc_conditional_insert uses pmd->last_cycles and the packet's RSS hash
to generate a random number used to determine whether or not an emc
entry should be inserted. This works for single-packet bursts as
last_cycles is updated for each burst. However, for bursts > 1 packet,
where the packets in the batch generate the same RSS hash,
pmd->last_cycles remains constant for the entire burst also, and thus
cannot be used as a random number for each packet in the burst.

This commit replaces the use of pmd->last_cycles with random_uint32()
for this purpose and subsequently fixes the behavior of the
emc_insert_inv_prob setting for high-throughput (large bursts)
single-flow cases.

Fixes: 4c30b24602c3 ("dpif-netdev: Conditional EMC insert")
Reported-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
v2:
- Remove unnecessary ORing in random number calcuation as per Ben's
  suggestion.
---
 lib/dpif-netdev.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Darrell Ball June 23, 2017, 3:19 p.m. UTC | #1 
Thanks for the V2

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

On 6/23/17, 8:07 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ciara Loftus" <ovs-dev-bounces@openvswitch.org on behalf of ciara.loftus@intel.com> wrote:

    emc_conditional_insert uses pmd->last_cycles and the packet's RSS hash
    to generate a random number used to determine whether or not an emc
    entry should be inserted. This works for single-packet bursts as
    last_cycles is updated for each burst. However, for bursts > 1 packet,
    where the packets in the batch generate the same RSS hash,
    pmd->last_cycles remains constant for the entire burst also, and thus
    cannot be used as a random number for each packet in the burst.
    
    This commit replaces the use of pmd->last_cycles with random_uint32()
    for this purpose and subsequently fixes the behavior of the
    emc_insert_inv_prob setting for high-throughput (large bursts)
    single-flow cases.
    
    Fixes: 4c30b24602c3 ("dpif-netdev: Conditional EMC insert")
    Reported-by: Kevin Traynor <ktraynor@redhat.com>
    Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
    ---
    v2:
    - Remove unnecessary ORing in random number calcuation as per Ben's
      suggestion.
    ---
     lib/dpif-netdev.c | 6 +-----
     1 file changed, 1 insertion(+), 5 deletions(-)
    
    diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
    index 4e29085..bf489fc 100644
    --- a/lib/dpif-netdev.c
    +++ b/lib/dpif-netdev.c
    @@ -2073,11 +2073,7 @@ emc_probabilistic_insert(struct dp_netdev_pmd_thread *pmd,
         uint32_t min;
         atomic_read_relaxed(&pmd->dp->emc_insert_min, &min);
     
    -#ifdef DPDK_NETDEV
    -    if (min && (key->hash ^ (uint32_t) pmd->last_cycles) <= min) {
    -#else
    -    if (min && (key->hash ^ random_uint32()) <= min) {
    -#endif
    +    if (min && (random_uint32() <= min)) {
             emc_insert(&pmd->flow_cache, key, flow);
         }
     }
    -- 
    2.4.11
    
    _______________________________________________
    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=JrQoU6MRrRUr-vSHj8aty_n1K3x38W60xK_NbbUQcQU&s=4ZmKaLYcaKABSRAGlOcM6BHYVkO2fhhXFQyBer4MRxU&e=
Darrell Ball June 23, 2017, 3:27 p.m. UTC | #2 
Oops, one suggestion I forgot per coding standards

Use

+    if (min && random_uint32() <= min) {

instead of

+    if (min && (random_uint32() <= min)) {

Darrell

On 6/23/17, 8:19 AM, "ovs-dev-bounces@openvswitch.org on behalf of Darrell Ball" <ovs-dev-bounces@openvswitch.org on behalf of dball@vmware.com> wrote:

    Thanks for the V2
    
    Acked-by: Darrell Ball <dlu998@gmail.com>
    
    On 6/23/17, 8:07 AM, "ovs-dev-bounces@openvswitch.org on behalf of Ciara Loftus" <ovs-dev-bounces@openvswitch.org on behalf of ciara.loftus@intel.com> wrote:
    
        emc_conditional_insert uses pmd->last_cycles and the packet's RSS hash
        to generate a random number used to determine whether or not an emc
        entry should be inserted. This works for single-packet bursts as
        last_cycles is updated for each burst. However, for bursts > 1 packet,
        where the packets in the batch generate the same RSS hash,
        pmd->last_cycles remains constant for the entire burst also, and thus
        cannot be used as a random number for each packet in the burst.
        
        This commit replaces the use of pmd->last_cycles with random_uint32()
        for this purpose and subsequently fixes the behavior of the
        emc_insert_inv_prob setting for high-throughput (large bursts)
        single-flow cases.
        
        Fixes: 4c30b24602c3 ("dpif-netdev: Conditional EMC insert")
        Reported-by: Kevin Traynor <ktraynor@redhat.com>
        Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
        ---
        v2:
        - Remove unnecessary ORing in random number calcuation as per Ben's
          suggestion.
        ---
         lib/dpif-netdev.c | 6 +-----
         1 file changed, 1 insertion(+), 5 deletions(-)
        
        diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
        index 4e29085..bf489fc 100644
        --- a/lib/dpif-netdev.c
        +++ b/lib/dpif-netdev.c
        @@ -2073,11 +2073,7 @@ emc_probabilistic_insert(struct dp_netdev_pmd_thread *pmd,
             uint32_t min;
             atomic_read_relaxed(&pmd->dp->emc_insert_min, &min);
         
        -#ifdef DPDK_NETDEV
        -    if (min && (key->hash ^ (uint32_t) pmd->last_cycles) <= min) {
        -#else
        -    if (min && (key->hash ^ random_uint32()) <= min) {
        -#endif
        +    if (min && (random_uint32() <= min)) {
                 emc_insert(&pmd->flow_cache, key, flow);
             }
         }
        -- 
        2.4.11
        
        _______________________________________________
        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=JrQoU6MRrRUr-vSHj8aty_n1K3x38W60xK_NbbUQcQU&s=4ZmKaLYcaKABSRAGlOcM6BHYVkO2fhhXFQyBer4MRxU&e= 
        
    
    _______________________________________________
    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=8WLbj6U88nUf_t6V6rYhdh9Kg9vv9SxBe5SqrK-UuPk&s=NVYZEPuUQ0phyjFsGeage-_jQNL2YpH5DZeuvyTzghI&e=
diff mbox

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4e29085..bf489fc 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2073,11 +2073,7 @@  emc_probabilistic_insert(struct dp_netdev_pmd_thread *pmd,
     uint32_t min;
     atomic_read_relaxed(&pmd->dp->emc_insert_min, &min);
 
-#ifdef DPDK_NETDEV
-    if (min && (key->hash ^ (uint32_t) pmd->last_cycles) <= min) {
-#else
-    if (min && (key->hash ^ random_uint32()) <= min) {
-#endif
+    if (min && (random_uint32() <= min)) {
         emc_insert(&pmd->flow_cache, key, flow);
     }
 }