[ovs-dev] [PATCH ovn] ovn-controller: Skip vport bindings done through OVS external_ids:iface-id.

Numan Siddique numans at ovn.org
Tue Apr 7 09:21:45 UTC 2020


On Thu, Apr 2, 2020 at 2:06 PM Dumitru Ceara <dceara at redhat.com> wrote:
>
> Port bindings of type "virtual" should not have an associated OVS port
> in the integration bridge. If this is the case, it's a misconfig and
> ovn-controller should ignore it.
>
> If such a situation is detected, ovn-controller will also log a warning
> message to inform the user about the wrong configuration.
>
> Reported-at: https://bugzilla.redhat.com/1818844
> CC: Numan Siddique <nusiddiq at redhat.com>
> Fixes: 054f4c85c413 ("Add a new logical switch port type - 'virtual'")
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>

Thanks for the fix. I applied this patch to master and brach-20.03

Numan

> ---
>  controller/binding.c | 12 ++++++++++++
>  tests/ovn.at         | 20 ++++++++++++++++++++
>  2 files changed, 32 insertions(+)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 5ea12a8..20a89d0 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -447,6 +447,18 @@ is_our_chassis(const struct sbrec_chassis *chassis_rec,
>      const struct ovsrec_interface *iface_rec
>          = shash_find_data(lport_to_iface, binding_rec->logical_port);
>
> +    /* Ports of type "virtual" should never be explicitly bound to an OVS
> +     * port in the integration bridge. If that's the case, ignore the binding
> +     * and log a warning.
> +     */
> +    if (iface_rec && !strcmp(binding_rec->type, "virtual")) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +        VLOG_WARN_RL(&rl,
> +                     "Virtual port %s should not be bound to OVS port %s",
> +                     binding_rec->logical_port, iface_rec->name);
> +        return false;
> +    }
> +
>      bool our_chassis = false;
>      if (iface_rec
>          || (binding_rec->parent_port && binding_rec->parent_port[0] &&
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 0135838..e8554f6 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -14894,6 +14894,11 @@ ovs-vsctl -- add-port br-int hv1-vif2 -- \
>      options:tx_pcap=hv1/vif2-tx.pcap \
>      options:rxq_pcap=hv1/vif2-rx.pcap \
>      ofport-request=2
> +ovs-vsctl -- add-port br-int hv1-vif3 -- \
> +    set interface hv1-vif3 \
> +    options:tx_pcap=hv1/vif3-tx.pcap \
> +    options:rxq_pcap=hv1/vif3-rx.pcap \
> +    ofport-request=3
>
>  sim_add hv2
>  as hv2
> @@ -14987,6 +14992,21 @@ logical_port=sw0-vir) = x], [0], [])
>  AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \
>  logical_port=sw0-vir) = x])
>
> +# Try to bind sw0-vir directly to an OVS port. This should be ignored by
> +# ovn-controller.
> +as hv1
> +ovs-vsctl set interface hv1-vif3 external-ids:iface-id=sw0-vir
> +
> +AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding \
> +logical_port=sw0-vir) = x], [0], [])
> +
> +# Cleanup hv1-vif3.
> +as hv1
> +ovs-vsctl del-port hv1-vif3
> +
> +AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding \
> +logical_port=sw0-vir) = x], [0], [])
> +
>  # From sw0-p0 send GARP for 10.0.0.10. hv1 should claim sw0-vir
>  # and sw0-p1 should be its virtual_parent.
>  eth_src=505400000003
> --
> 1.8.3.1
>


More information about the dev mailing list