diff mbox series

[ovs-dev,RFC,2/4] dpif-netdev: Fix EMC key length

Message ID 1516299624-452546-3-git-send-email-yipeng1.wang@intel.com
State Superseded
Delegated to: Ian Stokes
Headers show
Series dpif-netdev: Combine CD and DFC patch for datapath refactor | expand

Commit Message

Wang, Yipeng1 Jan. 18, 2018, 6:20 p.m. UTC 
EMC's key length is not initialized when insertion. Initialize the
key length before insertion.

The code might be put in another place, for now I just put it
in dfc_lookup.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 lib/dpif-netdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Bodireddy, Bhanuprakash Feb. 20, 2018, 9:14 p.m. UTC | #1 
This fix is needed and can  be included in 1/4 in next revision.

- Bhanuprakash.

>-----Original Message-----
>From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
>bounces@openvswitch.org] On Behalf Of Yipeng Wang
>Sent: Thursday, January 18, 2018 6:20 PM
>To: dev@openvswitch.org; jan.scheurich@ericsson.com
>Cc: Tai, Charlie <charlie.tai@intel.com>
>Subject: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
>
>EMC's key length is not initialized when insertion. Initialize the key length
>before insertion.
>
>The code might be put in another place, for now I just put it in dfc_lookup.
>
>Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
>---
> lib/dpif-netdev.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index b9f4b6d..3e87992
>100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -2295,7 +2295,7 @@ dfc_insert(struct dp_netdev_pmd_thread *pmd,  }
>
> static inline struct dp_netdev_flow *
>-dfc_lookup(struct dfc_cache *cache, const struct netdev_flow_key *key,
>+dfc_lookup(struct dfc_cache *cache, struct netdev_flow_key *key,
>            bool *exact_match)
> {
>     struct dp_netdev_flow *flow;
>@@ -2317,6 +2317,7 @@ dfc_lookup(struct dfc_cache *cache, const struct
>netdev_flow_key *key,
>         /* Found a match in DFC. Insert into EMC for subsequent lookups.
>          * We use probabilistic insertion here so that mainly elephant
>          * flows enter EMC. */
>+        key->len = netdev_flow_key_size(miniflow_n_values(&key->mf));
>         emc_probabilistic_insert(&cache->emc_cache, key, flow);
>         *exact_match = false;
>         return flow;
>--
>2.7.4
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Jan Scheurich Feb. 21, 2018, 8:08 a.m. UTC | #2 
I think there is an even simpler fix for the problem:

+        /* Found a match in DFC. Insert into EMC for subsequent lookups.
+         * We use probabilistic insertion here so that mainly elephant
+         * flows enter EMC.
+         * Initialize the key's length from the matched flow. */
+        key->len = flow->cr.flow.len;
+        emc_probabilistic_insert(&cache->emc_cache, key, flow);
+        *exact_match = false;
+        return flow;
+    } else {

Please use that in the next version of 1/4.

BR, Jan

> -----Original Message-----
> From: Bodireddy, Bhanuprakash [mailto:bhanuprakash.bodireddy@intel.com]
> Sent: Tuesday, 20 February, 2018 22:14
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; dev@openvswitch.org; Jan Scheurich <jan.scheurich@ericsson.com>
> Cc: Tai, Charlie <charlie.tai@intel.com>
> Subject: RE: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
> 
> This fix is needed and can  be included in 1/4 in next revision.
> 
> - Bhanuprakash.
> 
> >-----Original Message-----
> >From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> >bounces@openvswitch.org] On Behalf Of Yipeng Wang
> >Sent: Thursday, January 18, 2018 6:20 PM
> >To: dev@openvswitch.org; jan.scheurich@ericsson.com
> >Cc: Tai, Charlie <charlie.tai@intel.com>
> >Subject: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
> >
> >EMC's key length is not initialized when insertion. Initialize the key length
> >before insertion.
> >
> >The code might be put in another place, for now I just put it in dfc_lookup.
> >
> >Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> >---
> > lib/dpif-netdev.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index b9f4b6d..3e87992
> >100644
> >--- a/lib/dpif-netdev.c
> >+++ b/lib/dpif-netdev.c
> >@@ -2295,7 +2295,7 @@ dfc_insert(struct dp_netdev_pmd_thread *pmd,  }
> >
> > static inline struct dp_netdev_flow *
> >-dfc_lookup(struct dfc_cache *cache, const struct netdev_flow_key *key,
> >+dfc_lookup(struct dfc_cache *cache, struct netdev_flow_key *key,
> >            bool *exact_match)
> > {
> >     struct dp_netdev_flow *flow;
> >@@ -2317,6 +2317,7 @@ dfc_lookup(struct dfc_cache *cache, const struct
> >netdev_flow_key *key,
> >         /* Found a match in DFC. Insert into EMC for subsequent lookups.
> >          * We use probabilistic insertion here so that mainly elephant
> >          * flows enter EMC. */
> >+        key->len = netdev_flow_key_size(miniflow_n_values(&key->mf));
> >         emc_probabilistic_insert(&cache->emc_cache, key, flow);
> >         *exact_match = false;
> >         return flow;
> >--
> >2.7.4
> >
> >_______________________________________________
> >dev mailing list
> >dev@openvswitch.org
> >https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Wang, Yipeng1 Feb. 26, 2018, 5:50 p.m. UTC | #3 
Thanks Bhanu and Jan,

For this proposed simple fix, should the exact match flow key->len not equal to megaflow length from cr.flow.len?  Since one megaflow could match many exact match flows...

Thanks
Yipeng

>-----Original Message-----
>From: Jan Scheurich [mailto:jan.scheurich@ericsson.com]
>Sent: Wednesday, February 21, 2018 12:09 AM
>To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com>; Wang, Yipeng1 <yipeng1.wang@intel.com>;
>dev@openvswitch.org
>Cc: Tai, Charlie <charlie.tai@intel.com>
>Subject: RE: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
>
>I think there is an even simpler fix for the problem:
>
>+        /* Found a match in DFC. Insert into EMC for subsequent lookups.
>+         * We use probabilistic insertion here so that mainly elephant
>+         * flows enter EMC.
>+         * Initialize the key's length from the matched flow. */
>+        key->len = flow->cr.flow.len;
>+        emc_probabilistic_insert(&cache->emc_cache, key, flow);
>+        *exact_match = false;
>+        return flow;
>+    } else {
>
>Please use that in the next version of 1/4.
>
>BR, Jan
>
>> -----Original Message-----
>> From: Bodireddy, Bhanuprakash [mailto:bhanuprakash.bodireddy@intel.com]
>> Sent: Tuesday, 20 February, 2018 22:14
>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; dev@openvswitch.org; Jan Scheurich <jan.scheurich@ericsson.com>
>> Cc: Tai, Charlie <charlie.tai@intel.com>
>> Subject: RE: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
>>
>> This fix is needed and can  be included in 1/4 in next revision.
>>
>> - Bhanuprakash.
>>
>> >-----Original Message-----
>> >From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
>> >bounces@openvswitch.org] On Behalf Of Yipeng Wang
>> >Sent: Thursday, January 18, 2018 6:20 PM
>> >To: dev@openvswitch.org; jan.scheurich@ericsson.com
>> >Cc: Tai, Charlie <charlie.tai@intel.com>
>> >Subject: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
>> >
>> >EMC's key length is not initialized when insertion. Initialize the key length
>> >before insertion.
>> >
>> >The code might be put in another place, for now I just put it in dfc_lookup.
>> >
>> >Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
>> >---
>> > lib/dpif-netdev.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index b9f4b6d..3e87992
>> >100644
>> >--- a/lib/dpif-netdev.c
>> >+++ b/lib/dpif-netdev.c
>> >@@ -2295,7 +2295,7 @@ dfc_insert(struct dp_netdev_pmd_thread *pmd,  }
>> >
>> > static inline struct dp_netdev_flow *
>> >-dfc_lookup(struct dfc_cache *cache, const struct netdev_flow_key *key,
>> >+dfc_lookup(struct dfc_cache *cache, struct netdev_flow_key *key,
>> >            bool *exact_match)
>> > {
>> >     struct dp_netdev_flow *flow;
>> >@@ -2317,6 +2317,7 @@ dfc_lookup(struct dfc_cache *cache, const struct
>> >netdev_flow_key *key,
>> >         /* Found a match in DFC. Insert into EMC for subsequent lookups.
>> >          * We use probabilistic insertion here so that mainly elephant
>> >          * flows enter EMC. */
>> >+        key->len = netdev_flow_key_size(miniflow_n_values(&key->mf));
>> >         emc_probabilistic_insert(&cache->emc_cache, key, flow);
>> >         *exact_match = false;
>> >         return flow;
>> >--
>> >2.7.4
>> >
>> >_______________________________________________
>> >dev mailing list
>> >dev@openvswitch.org
>> >https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b9f4b6d..3e87992 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2295,7 +2295,7 @@  dfc_insert(struct dp_netdev_pmd_thread *pmd,
 }
 
 static inline struct dp_netdev_flow *
-dfc_lookup(struct dfc_cache *cache, const struct netdev_flow_key *key,
+dfc_lookup(struct dfc_cache *cache, struct netdev_flow_key *key,
            bool *exact_match)
 {
     struct dp_netdev_flow *flow;
@@ -2317,6 +2317,7 @@  dfc_lookup(struct dfc_cache *cache, const struct netdev_flow_key *key,
         /* Found a match in DFC. Insert into EMC for subsequent lookups.
          * We use probabilistic insertion here so that mainly elephant
          * flows enter EMC. */
+        key->len = netdev_flow_key_size(miniflow_n_values(&key->mf));
         emc_probabilistic_insert(&cache->emc_cache, key, flow);
         *exact_match = false;
         return flow;