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 |
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
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
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 --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;
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(-)