diff mbox

[ovs-dev,1/2] ofproto-dpif-upcall: Don't delete modified ukeys.

Message ID 1452196067-27545-1-git-send-email-joe@ovn.org
State Accepted
Headers show

Commit Message

Joe Stringer Jan. 7, 2016, 7:47 p.m. UTC
If revalidation returns the result UKEY_DELETE, then both the ukey and
its corresponding flow should be deleted. However, if revalidation
returns UKEY_MODIFY, the ukey itself should be modified in-place and
should not be deleted.

Fix this by only applying the ukey deletion to ukeys whose datapath
operations delete a flow.

This may fix statistics accounting issues in rare cases involving
OpenFlow rule modification where actions are updated but flows remain
the same.

Found by inspection.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 ofproto/ofproto-dpif-upcall.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jarno Rajahalme Jan. 7, 2016, 9:49 p.m. UTC | #1
Acked-by: Jarno Rajahalme <jarno@ovn.org>

> On Jan 7, 2016, at 11:47 AM, Joe Stringer <joe@ovn.org> wrote:
> 
> If revalidation returns the result UKEY_DELETE, then both the ukey and
> its corresponding flow should be deleted. However, if revalidation
> returns UKEY_MODIFY, the ukey itself should be modified in-place and
> should not be deleted.
> 
> Fix this by only applying the ukey deletion to ukeys whose datapath
> operations delete a flow.
> 
> This may fix statistics accounting issues in rare cases involving
> OpenFlow rule modification where actions are updated but flows remain
> the same.
> 
> Found by inspection.
> 
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> ofproto/ofproto-dpif-upcall.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index d1e941a5dcfb..69a04c9a50ca 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -2053,7 +2053,9 @@ push_ukey_ops(struct udpif *udpif, struct umap *umap,
>     push_ukey_ops__(udpif, ops, n_ops);
>     ovs_mutex_lock(&umap->mutex);
>     for (i = 0; i < n_ops; i++) {
> -        ukey_delete(umap, ops[i].ukey);
> +        if (ops[i].dop.type == DPIF_OP_FLOW_DEL) {
> +            ukey_delete(umap, ops[i].ukey);
> +        }
>     }
>     ovs_mutex_unlock(&umap->mutex);
> }
> -- 
> 2.1.4
>
Joe Stringer Jan. 8, 2016, 12:04 a.m. UTC | #2
On 7 January 2016 at 13:49, Jarno Rajahalme <jarno@ovn.org> wrote:
> Acked-by: Jarno Rajahalme <jarno@ovn.org>

Thanks, I applied this to master and branch-2.5.
diff mbox

Patch

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index d1e941a5dcfb..69a04c9a50ca 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2053,7 +2053,9 @@  push_ukey_ops(struct udpif *udpif, struct umap *umap,
     push_ukey_ops__(udpif, ops, n_ops);
     ovs_mutex_lock(&umap->mutex);
     for (i = 0; i < n_ops; i++) {
-        ukey_delete(umap, ops[i].ukey);
+        if (ops[i].dop.type == DPIF_OP_FLOW_DEL) {
+            ukey_delete(umap, ops[i].ukey);
+        }
     }
     ovs_mutex_unlock(&umap->mutex);
 }