[ovs-dev] tests: Add a test case for patch ports not created by OVN.

From: Numan Siddique <numans@ovn.org>

When ovn-controller creates patch ports for ovn-bridge-mappings,
it sets external_ids:ovn-localnet-port=<localnet_port_name>
in the OVS Port.  This indicates that OVN owns and manages these
patch ports.  It's possible that CMS may create patch ports
externally connecting br-int to CMS managed OVS bridges.

This patch adds a test case to ensure that ovn-controller
doesn't modify or delete these patch ports in anyway.

Reported-at: https://issues.redhat.com/browse/FDP-734
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 tests/ovn.at | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

Thanks Numan.

Acked-by: Mark Michelson <mmichels@redhat.com>

On 9/18/24 16:19, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> When ovn-controller creates patch ports for ovn-bridge-mappings,
> it sets external_ids:ovn-localnet-port=<localnet_port_name>
> in the OVS Port.  This indicates that OVN owns and manages these
> patch ports.  It's possible that CMS may create patch ports
> externally connecting br-int to CMS managed OVS bridges.
> 
> This patch adds a test case to ensure that ovn-controller
> doesn't modify or delete these patch ports in anyway.
> 
> Reported-at: https://issues.redhat.com/browse/FDP-734
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>   tests/ovn.at | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 114 insertions(+)
> 
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4b6e8132f0..e6c8421539 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -38926,3 +38926,117 @@ OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected-vif1])
>   
>   AT_CLEANUP
>   ])
> +
> +AT_SETUP([Patch ports not owned by OVN])
> +
> +AT_KEYWORDS([ovn])
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +ovs-vsctl add-br br-eth0
> +ovs-vsctl add-br br-eth1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.20
> +
> +ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl lsp-add sw0 sw0p1
> +
> +check ovn-nbctl --wait=hv sync
> +
> +check ovn-appctl -t ovn-controller debug/ignore-startup-delay
> +
> +# Waits until the OVS database contains exactly the specified patch ports.
> +# Each argument should be of the form BRIDGE PORT PEER.
> +check_patch_ports () {
> +    # Generate code to check that the set of patch ports is exactly as
> +    # specified.
> +    echo 'ovs-vsctl -f csv -d bare --no-headings --columns=name find Interface type=patch | sort' > query
> +    for patch
> +    do
> +        echo $patch
> +    done | cut -d' ' -f 2 | sort > expout
> +
> +    # Generate code to verify that the configuration of each patch
> +    # port is correct.
> +    for patch
> +    do
> +        set $patch; bridge=$1 port=$2 peer=$3
> +        echo >>query "ovs-vsctl iface-to-br $port -- get Interface $port type options"
> +        echo >>expout "$bridge
> +patch
> +{peer=$peer}"
> +    done
> +
> +    # Run the query until we get the expected result (or until a timeout).
> +    #
> +    # (We use sed to drop all "s from output because ovs-vsctl quotes some
> +    # of the port names but not others.)
> +    AT_CAPTURE_FILE([query])
> +    AT_CAPTURE_FILE([expout])
> +    AT_CAPTURE_FILE([stdout])
> +    OVS_WAIT_UNTIL([. ./query | sed 's/"//g' > stdout #"
> +                    diff -u stdout expout >/dev/null])
> +}
> +
> +# Initially there should be no patch ports.
> +check_patch_ports
> +
> +check ovn-nbctl lsp-add sw0 ln-sw0
> +check ovn-nbctl lsp-set-type ln-sw0 localnet
> +check ovn-nbctl lsp-set-addresses ln-sw0 unknown
> +check ovn-nbctl --wait=hv lsp-set-options ln-sw0 network_name=physnet1
> +
> +# There should still be no patch ports.
> +check_patch_ports
> +
> +as hv1
> +check ovs-vsctl \
> +    -- add-port br-int vif1 \
> +    -- set Interface vif1 external_ids:iface-id=sw0p1
> +
> +# ovn-controller should bind the interface.
> +wait_for_ports_up
> +
> +AS_BOX([Checking for patch ports to be created by OVN for ln-sw0])
> +check_patch_ports \
> +    'br-int  patch-br-int-to-ln-sw0  patch-ln-sw0-to-br-int' \
> +    'br-eth0 patch-ln-sw0-to-br-int  patch-br-int-to-ln-sw0'
> +
> +
> +check ovs-vsctl add-port br-int patch-br-int-to-br-eth0 \
> +   -- set interface patch-br-int-to-br-eth0 options:peer=patch-br-eth0-to-br-int \
> +   -- set interface patch-br-int-to-br-eth0 type=patch \
> +   -- add-port br-eth0 patch-br-eth0-to-br-int \
> +   -- set interface patch-br-eth0-to-br-int options:peer=patch-br-int-to-br-eth0 \
> +   -- set interface patch-br-eth0-to-br-int type=patch \
> +
> +AS_BOX([Checking for patch ports after creating manual patch ports])
> +
> +check_patch_ports \
> +    'br-int  patch-br-int-to-ln-sw0   patch-ln-sw0-to-br-int' \
> +    'br-eth0 patch-ln-sw0-to-br-int   patch-br-int-to-ln-sw0' \
> +    'br-int  patch-br-int-to-br-eth0  patch-br-eth0-to-br-int' \
> +    'br-eth0 patch-br-eth0-to-br-int  patch-br-int-to-br-eth0'
> +
> +# Delete the localnet port.  ovn-controller should not
> +# delete the manually created ones.
> +check ovn-nbctl --wait=hv lsp-del ln-sw0
> +
> +check_patch_ports \
> +    'br-int  patch-br-int-to-br-eth0  patch-br-eth0-to-br-int' \
> +    'br-eth0 patch-br-eth0-to-br-int  patch-br-int-to-br-eth0'
> +
> +# Set external_ids:ovn-localnet-port to the manually created patch port.
> +# ovn-controller should delete the patch ports now as we are setting
> +# OVN as its owner.
> +check ovs-vsctl set port patch-br-int-to-br-eth0 external_ids:ovn-localnet-port=foo
> +ovn-nbctl --wait=hv sync
> +
> +AS_BOX([Checking for patch ports after setting external_ids:ovn-localnet-port to the manually created patch port])
> +check_patch_ports
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP

On Wed, Oct 16, 2024 at 4:52 PM Mark Michelson <mmichels@redhat.com> wrote:
>
> Thanks Numan.
>
> Acked-by: Mark Michelson <mmichels@redhat.com>

Thanks.  Applied to main.

Numan

>
> On 9/18/24 16:19, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > When ovn-controller creates patch ports for ovn-bridge-mappings,
> > it sets external_ids:ovn-localnet-port=<localnet_port_name>
> > in the OVS Port.  This indicates that OVN owns and manages these
> > patch ports.  It's possible that CMS may create patch ports
> > externally connecting br-int to CMS managed OVS bridges.
> >
> > This patch adds a test case to ensure that ovn-controller
> > doesn't modify or delete these patch ports in anyway.
> >
> > Reported-at: https://issues.redhat.com/browse/FDP-734
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> >   tests/ovn.at | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 114 insertions(+)
> >
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 4b6e8132f0..e6c8421539 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -38926,3 +38926,117 @@ OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected-vif1])
> >
> >   AT_CLEANUP
> >   ])
> > +
> > +AT_SETUP([Patch ports not owned by OVN])
> > +
> > +AT_KEYWORDS([ovn])
> > +ovn_start
> > +
> > +net_add n1
> > +sim_add hv1
> > +ovs-vsctl add-br br-eth0
> > +ovs-vsctl add-br br-eth1
> > +ovs-vsctl add-br br-phys
> > +ovn_attach n1 br-phys 192.168.0.20
> > +
> > +ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1
> > +
> > +check ovn-nbctl ls-add sw0
> > +check ovn-nbctl lsp-add sw0 sw0p1
> > +
> > +check ovn-nbctl --wait=hv sync
> > +
> > +check ovn-appctl -t ovn-controller debug/ignore-startup-delay
> > +
> > +# Waits until the OVS database contains exactly the specified patch ports.
> > +# Each argument should be of the form BRIDGE PORT PEER.
> > +check_patch_ports () {
> > +    # Generate code to check that the set of patch ports is exactly as
> > +    # specified.
> > +    echo 'ovs-vsctl -f csv -d bare --no-headings --columns=name find Interface type=patch | sort' > query
> > +    for patch
> > +    do
> > +        echo $patch
> > +    done | cut -d' ' -f 2 | sort > expout
> > +
> > +    # Generate code to verify that the configuration of each patch
> > +    # port is correct.
> > +    for patch
> > +    do
> > +        set $patch; bridge=$1 port=$2 peer=$3
> > +        echo >>query "ovs-vsctl iface-to-br $port -- get Interface $port type options"
> > +        echo >>expout "$bridge
> > +patch
> > +{peer=$peer}"
> > +    done
> > +
> > +    # Run the query until we get the expected result (or until a timeout).
> > +    #
> > +    # (We use sed to drop all "s from output because ovs-vsctl quotes some
> > +    # of the port names but not others.)
> > +    AT_CAPTURE_FILE([query])
> > +    AT_CAPTURE_FILE([expout])
> > +    AT_CAPTURE_FILE([stdout])
> > +    OVS_WAIT_UNTIL([. ./query | sed 's/"//g' > stdout #"
> > +                    diff -u stdout expout >/dev/null])
> > +}
> > +
> > +# Initially there should be no patch ports.
> > +check_patch_ports
> > +
> > +check ovn-nbctl lsp-add sw0 ln-sw0
> > +check ovn-nbctl lsp-set-type ln-sw0 localnet
> > +check ovn-nbctl lsp-set-addresses ln-sw0 unknown
> > +check ovn-nbctl --wait=hv lsp-set-options ln-sw0 network_name=physnet1
> > +
> > +# There should still be no patch ports.
> > +check_patch_ports
> > +
> > +as hv1
> > +check ovs-vsctl \
> > +    -- add-port br-int vif1 \
> > +    -- set Interface vif1 external_ids:iface-id=sw0p1
> > +
> > +# ovn-controller should bind the interface.
> > +wait_for_ports_up
> > +
> > +AS_BOX([Checking for patch ports to be created by OVN for ln-sw0])
> > +check_patch_ports \
> > +    'br-int  patch-br-int-to-ln-sw0  patch-ln-sw0-to-br-int' \
> > +    'br-eth0 patch-ln-sw0-to-br-int  patch-br-int-to-ln-sw0'
> > +
> > +
> > +check ovs-vsctl add-port br-int patch-br-int-to-br-eth0 \
> > +   -- set interface patch-br-int-to-br-eth0 options:peer=patch-br-eth0-to-br-int \
> > +   -- set interface patch-br-int-to-br-eth0 type=patch \
> > +   -- add-port br-eth0 patch-br-eth0-to-br-int \
> > +   -- set interface patch-br-eth0-to-br-int options:peer=patch-br-int-to-br-eth0 \
> > +   -- set interface patch-br-eth0-to-br-int type=patch \
> > +
> > +AS_BOX([Checking for patch ports after creating manual patch ports])
> > +
> > +check_patch_ports \
> > +    'br-int  patch-br-int-to-ln-sw0   patch-ln-sw0-to-br-int' \
> > +    'br-eth0 patch-ln-sw0-to-br-int   patch-br-int-to-ln-sw0' \
> > +    'br-int  patch-br-int-to-br-eth0  patch-br-eth0-to-br-int' \
> > +    'br-eth0 patch-br-eth0-to-br-int  patch-br-int-to-br-eth0'
> > +
> > +# Delete the localnet port.  ovn-controller should not
> > +# delete the manually created ones.
> > +check ovn-nbctl --wait=hv lsp-del ln-sw0
> > +
> > +check_patch_ports \
> > +    'br-int  patch-br-int-to-br-eth0  patch-br-eth0-to-br-int' \
> > +    'br-eth0 patch-br-eth0-to-br-int  patch-br-int-to-br-eth0'
> > +
> > +# Set external_ids:ovn-localnet-port to the manually created patch port.
> > +# ovn-controller should delete the patch ports now as we are setting
> > +# OVN as its owner.
> > +check ovs-vsctl set port patch-br-int-to-br-eth0 external_ids:ovn-localnet-port=foo
> > +ovn-nbctl --wait=hv sync
> > +
> > +AS_BOX([Checking for patch ports after setting external_ids:ovn-localnet-port to the manually created patch port])
> > +check_patch_ports
> > +
> > +OVN_CLEANUP([hv1])
> > +AT_CLEANUP
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>