On Mon, Mar 19, 2018 at 12:52 PM, <[email protected]> wrote:
>
> From: Numan Siddique <[email protected]>
>
> ovn-controller is not considering Port_Binding option "requested-chassis"
> when adding flows in table 0 and table 65. This patch adds this check.

Hi Numan,

The check looks good, but sorry that I didn't understand the problem. E.g.
there are 2 host A and B, and lsp1 is set as iface-id on both hosts. When
requested-chassis for lsp1 is A, then in SB DB the port-binding should be
on A. Then in physical.c when processing the port binding for lsp1, it
should always point to host A. So how could the VIF on B be reachable?
Could you help explain a little bit?

Thanks,
Han
>
> Reported-by: Marcin Mirecki <[email protected]>
> Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/345266.html
>
> Signed-off-by: Numan Siddique <[email protected]>
> ---
>  ovn/controller/physical.c | 11 +++++++++++
>  tests/ovn.at              | 32 +++++++++++++++++++++++++++-----
>  2 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 5a80e2cda..84113ebd2 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -466,6 +466,17 @@ consider_port_binding(struct controller_ctx *ctx,
>      } else {
>          ofport = u16_to_ofp(simap_get(&localvif_to_ofport,
>                                        binding->logical_port));
> +        const char *requested_chassis = smap_get(&binding->options,
> +                                                 "requested-chassis");
> +        if (ofport && requested_chassis &&
> +            strcmp(requested_chassis, chassis->name) &&
> +            strcmp(requested_chassis, chassis->hostname)) {
> +            /* Even though there is an ofport for this port_binding, it
is
> +             * requested on a different chassis. So ignore this ofport.
> +             */
> +            ofport = 0;
> +        }
> +
>          if ((!strcmp(binding->type, "localnet")
>              || !strcmp(binding->type, "l2gateway"))
>              && ofport && binding->tag) {
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 5f985f345..75b32ef1a 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -9112,13 +9112,15 @@ sim_add hv1
>  as hv1
>  ovs-vsctl add-br br-phys
>  ovn_attach n1 br-phys 192.168.0.11
> -ovs-vsctl -- add-port br-int hv1-vif0
> +ovs-vsctl -- add-port br-int hv1-vif0 -- \
> +set Interface hv1-vif0 ofport-request=1
>
>  sim_add hv2
>  as hv2
>  ovs-vsctl add-br br-phys
>  ovn_attach n1 br-phys 192.168.0.12
> -ovs-vsctl -- add-port br-int hv2-vif0
> +ovs-vsctl -- add-port br-int hv2-vif0 -- \
> +set Interface hv2-vif0 ofport-request=1
>
>  # Allow only chassis hv1 to bind logical port lsp0.
>  ovn-nbctl lsp-set-options lsp0 requested-chassis=hv1
> @@ -9138,7 +9140,11 @@ ovs-vsctl set interface hv2-vif0
external-ids:iface-id=lsp0
>  OVS_WAIT_UNTIL([test 1 = $(grep -c "Not claiming lport lsp0"
hv2/ovn-controller.log)])
>  AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding
logical_port=lsp0) = x], [0], [])
>
> -# (2) Chassis hv1 should bind lsp0 when physical to logical mapping
exists on hv1.
> +# (2) Chassis hv2 should not add flows in OFTABLE_PHY_TO_LOG and
OFTABLE_LOG_TO_PHY tables.
> +AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=0 | grep in_port=1],
[1], [])
> +AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=65 | grep output],
[1], [])
> +
> +# (3) Chassis hv1 should bind lsp0 when physical to logical mapping
exists on hv1.
>  echo "verifying that hv1 binds lsp0 when hv1 physical/logical mapping is
added"
>  as hv1
>  ovs-vsctl set interface hv1-vif0 external-ids:iface-id=lsp0
> @@ -9146,7 +9152,12 @@ ovs-vsctl set interface hv1-vif0
external-ids:iface-id=lsp0
>  OVS_WAIT_UNTIL([test 1 = $(grep -c "Claiming lport lsp0"
hv1/ovn-controller.log)])
>  AT_CHECK([test $(ovn-sbctl --bare --columns chassis find port_binding
logical_port=lsp0) = "$hv1_uuid"], [0], [])
>
> -# (3) Chassis hv1 should release lsp0 binding and chassis hv2 should
bind lsp0 when
> +# (4) Chassis hv1 should add flows in OFTABLE_PHY_TO_LOG and
OFTABLE_LOG_TO_PHY tables.
> +as hv1 ovs-ofctl dump-flows br-int
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1],
[0], [ignore])
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep
actions=output:1], [0], [ignore])
> +
> +# (5) Chassis hv1 should release lsp0 binding and chassis hv2 should
bind lsp0 when
>  # the requested chassis for lsp0 is changed from hv1 to hv2.
>  echo "verifying that lsp0 binding moves when requested-chassis is
changed"
>
> @@ -9154,6 +9165,13 @@ ovn-nbctl lsp-set-options lsp0
requested-chassis=hv2
>  OVS_WAIT_UNTIL([test 1 = $(grep -c "Releasing lport lsp0 from this
chassis" hv1/ovn-controller.log)])
>  OVS_WAIT_UNTIL([test $(ovn-sbctl --bare --columns chassis find
port_binding logical_port=lsp0) = "$hv2_uuid"])
>
> +# (6) Chassis hv2 should add flows and hv1 should not.
> +AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=0 | grep in_port=1],
[0], [ignore])
> +AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=65 | grep
actions=output:1], [0], [ignore])
> +
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1],
[1], [])
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep output],
[1], [])
> +
>  OVN_CLEANUP([hv1],[hv2])
>
>  AT_CLEANUP
> @@ -9170,7 +9188,7 @@ sim_add hv1
>  as hv1
>  ovs-vsctl add-br br-phys
>  ovn_attach n1 br-phys 192.168.0.11
> -ovs-vsctl -- add-port br-int hv1-vif0
> +ovs-vsctl -- add-port br-int hv1-vif0 -- set Interface hv1-vif0
ofport-request=1
>
>  hv1_hostname=$(ovn-sbctl --bare --columns hostname find Chassis name=hv1)
>  echo "hv1_hostname=${hv1_hostname}"
> @@ -9181,11 +9199,15 @@ hv1_uuid=$(ovn-sbctl --bare --columns _uuid find
Chassis name=hv1)
>  echo "hv1_uuid=${hv1_uuid}"
>  OVS_WAIT_UNTIL([test 1 = $(grep -c "Claiming lport lsp0"
hv1/ovn-controller.log)])
>  AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding
logical_port=lsp0) = x"$hv1_uuid"], [0], [])
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1],
[0], [ignore])
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep
actions=output:1], [0], [ignore])
>
>  ovn-nbctl --wait=hv --timeout=3 lsp-set-options lsp0
requested-chassis=non-existant-chassis
>  OVS_WAIT_UNTIL([test 1 = $(grep -c "Releasing lport lsp0 from this
chassis" hv1/ovn-controller.log)])
>  ovn-nbctl --wait=hv --timeout=3 sync
>  AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding
logical_port=lsp0) = x], [0], [])
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1],
[1], [])
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep output],
[1], [])
>
>  OVN_CLEANUP([hv1])
>
> --
> 2.14.3
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to