diff mbox

[ovs-dev] ofproto-dpif-upcall: Fix UFID usage with flow_modify.

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

Commit Message

Joe Stringer May 13, 2016, 9:17 p.m. UTC
As per the delete_op_init{,__}() functions, the UFID should only be
passed down if ukey->ufid_present is set. Otherwise it is possible to
request a flow modification only using a UFID in a datapath that doesn't
support UFID, which will fail.

Fixes: 43b2f131a229 ("ofproto: Allow in-place modifications of datapath flows.")
Signed-off-by: Joe Stringer <joe@ovn.org>
---
 ofproto/ofproto-dpif-upcall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ben Pfaff May 15, 2016, 4:41 a.m. UTC | #1
On Fri, May 13, 2016 at 02:17:12PM -0700, Joe Stringer wrote:
> As per the delete_op_init{,__}() functions, the UFID should only be
> passed down if ukey->ufid_present is set. Otherwise it is possible to
> request a flow modification only using a UFID in a datapath that doesn't
> support UFID, which will fail.
> 
> Fixes: 43b2f131a229 ("ofproto: Allow in-place modifications of datapath flows.")
> Signed-off-by: Joe Stringer <joe@ovn.org>

...

> -    op->dop.u.flow_put.ufid = &ukey->ufid;
> +    op->dop.u.flow_put.ufid = ukey->ufid_present ? &ukey->ufid: NULL;

Usually we'd put a space before the colon.

Acked-by: Ben Pfaff <blp@ovn.org>
Joe Stringer May 17, 2016, 6:08 p.m. UTC | #2
On 14 May 2016 at 21:41, Ben Pfaff <blp@ovn.org> wrote:
> On Fri, May 13, 2016 at 02:17:12PM -0700, Joe Stringer wrote:
>> As per the delete_op_init{,__}() functions, the UFID should only be
>> passed down if ukey->ufid_present is set. Otherwise it is possible to
>> request a flow modification only using a UFID in a datapath that doesn't
>> support UFID, which will fail.
>>
>> Fixes: 43b2f131a229 ("ofproto: Allow in-place modifications of datapath flows.")
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>
> ...
>
>> -    op->dop.u.flow_put.ufid = &ukey->ufid;
>> +    op->dop.u.flow_put.ufid = ukey->ufid_present ? &ukey->ufid: NULL;
>
> Usually we'd put a space before the colon.
>
> Acked-by: Ben Pfaff <blp@ovn.org>

Thanks, I made this amendment and applied the patch to master and branch-2.5.
diff mbox

Patch

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 0543c78e8790..2280e240ee76 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1930,7 +1930,7 @@  modify_op_init(struct ukey_op *op, struct udpif_key *ukey)
     op->dop.u.flow_put.key_len = ukey->key_len;
     op->dop.u.flow_put.mask = ukey->mask;
     op->dop.u.flow_put.mask_len = ukey->mask_len;
-    op->dop.u.flow_put.ufid = &ukey->ufid;
+    op->dop.u.flow_put.ufid = ukey->ufid_present ? &ukey->ufid: NULL;
     op->dop.u.flow_put.pmd_id = ukey->pmd_id;
     op->dop.u.flow_put.stats = NULL;
     ukey_get_actions(ukey, &op->dop.u.flow_put.actions,