Re: [ovs-dev] [PATCH] ovn: Fix ACLs for child logical ports.

2015-12-11 Thread Russell Bryant
On 12/10/2015 08:51 PM, Justin Pettit wrote:
> Thanks for fixing this.  It might be nice to include a comment such as the 
> following since it's not super obvious from a quick look what's being added:
> 
> /* Add child logical port to the set of all local ports. */
> 
> Acked-by: Justin Pettit 
> 
> I'd suggest cherry-picking this to "branch-2.5", too.

Thanks for the review.  I added your suggested comment, your ACK, and
then pushed this to master and branch-2.5.

-- 
Russell Bryant
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovn: Fix ACLs for child logical ports.

2015-12-10 Thread Justin Pettit
Thanks for fixing this.  It might be nice to include a comment such as the 
following since it's not super obvious from a quick look what's being added:

/* Add child logical port to the set of all local ports. */

Acked-by: Justin Pettit 

I'd suggest cherry-picking this to "branch-2.5", too.

--Justin


> On Nov 17, 2015, at 2:00 PM, Russell Bryant  wrote:
> 
> The physical input flows for child logical ports (for the
> container-in-a-VM use case, for example) did not set a conntrack zone
> ID.  The previous code only allocated a zone ID for local VIFs and
> missed doing it for child ports.
> 
> Signed-off-by: Russell Bryant 
> ---
> ovn/controller/binding.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index 7f31b31..89dca98 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -144,7 +144,6 @@ binding_run(struct controller_ctx *ctx, const struct 
> ovsrec_bridge *br_int,
> /* We have no integration bridge, therefore no local logical ports.
>  * We'll remove our chassis from all port binding records below. */
> }
> -update_ct_zones(&lports, ct_zones, ct_zone_bitmap);
> sset_clone(&all_lports, &lports);
> 
> ovsdb_idl_txn_add_comment(
> @@ -155,6 +154,9 @@ binding_run(struct controller_ctx *ctx, const struct 
> ovsrec_bridge *br_int,
> if (sset_find_and_delete(&lports, binding_rec->logical_port) ||
> (binding_rec->parent_port && binding_rec->parent_port[0] &&
>  sset_contains(&all_lports, binding_rec->parent_port))) {
> +if (binding_rec->parent_port && binding_rec->parent_port[0]) {
> +sset_add(&all_lports, binding_rec->logical_port);
> +}
> if (binding_rec->chassis == chassis_rec) {
> continue;
> }
> @@ -173,6 +175,9 @@ binding_run(struct controller_ctx *ctx, const struct 
> ovsrec_bridge *br_int,
> SSET_FOR_EACH (name, &lports) {
> VLOG_DBG("No port binding record for lport %s", name);
> }
> +
> +update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap);
> +
> sset_destroy(&lports);
> sset_destroy(&all_lports);
> }
> -- 
> 2.5.0
> 

___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovn: Fix ACLs for child logical ports.

2015-11-29 Thread Ben Pfaff
On Tue, Nov 17, 2015 at 02:00:06PM -0800, Russell Bryant wrote:
> The physical input flows for child logical ports (for the
> container-in-a-VM use case, for example) did not set a conntrack zone
> ID.  The previous code only allocated a zone ID for local VIFs and
> missed doing it for child ports.
> 
> Signed-off-by: Russell Bryant 

Justin, are you the right person to review this?  I haven't done much
with conntrack.
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] ovn: Fix ACLs for child logical ports.

2015-11-17 Thread Russell Bryant
The physical input flows for child logical ports (for the
container-in-a-VM use case, for example) did not set a conntrack zone
ID.  The previous code only allocated a zone ID for local VIFs and
missed doing it for child ports.

Signed-off-by: Russell Bryant 
---
 ovn/controller/binding.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 7f31b31..89dca98 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -144,7 +144,6 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 /* We have no integration bridge, therefore no local logical ports.
  * We'll remove our chassis from all port binding records below. */
 }
-update_ct_zones(&lports, ct_zones, ct_zone_bitmap);
 sset_clone(&all_lports, &lports);
 
 ovsdb_idl_txn_add_comment(
@@ -155,6 +154,9 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 if (sset_find_and_delete(&lports, binding_rec->logical_port) ||
 (binding_rec->parent_port && binding_rec->parent_port[0] &&
  sset_contains(&all_lports, binding_rec->parent_port))) {
+if (binding_rec->parent_port && binding_rec->parent_port[0]) {
+sset_add(&all_lports, binding_rec->logical_port);
+}
 if (binding_rec->chassis == chassis_rec) {
 continue;
 }
@@ -173,6 +175,9 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 SSET_FOR_EACH (name, &lports) {
 VLOG_DBG("No port binding record for lport %s", name);
 }
+
+update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap);
+
 sset_destroy(&lports);
 sset_destroy(&all_lports);
 }
-- 
2.5.0

___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev