Thanks Tao for fixing this. I think the title can be more generic because
this problem and fix applies to all tunnel types rather than just VXLAN.

On Tue, Mar 12, 2024 at 7:04 AM Tao Liu <[email protected]> wrote:
>
> Reproduce:
>   ovs-vsctl add-port br-int p0 \
>     -- set interface p0 type=vxlan options:remote_ip=10.10.10.1
>
>   sleep 2
>
>   ovs-vsctl --if-exists del-port p0 \
>     -- add-port br-int p1 \
>     -- set interface p1 type=vxlan options:remote_ip=10.10.10.1
>   ovs-vsctl: Error detected while setting up 'p1': could not add
>   network device p1 to ofproto (File exists).
>
> vswitchd log:
>   bridge|INFO|bridge br-int: added interface p0 on port 1106
>   bridge|INFO|bridge br-int: deleted interface p0 on port 1106
>   tunnel|WARN|p1: attempting to add tunnel port with same config as port
'p0' (::->10.10.10.1, key=0, legacy_l2, dp port=122)
>   ofproto|WARN|br-int: could not add port p1 (File exists)
>   bridge|WARN|could not add network device p1 to ofproto (File exists)
>
> CallTrace:
>   bridge_reconfigure
>     bridge_del_ports
>       port_destroy
>         iface_destroy__
>           netdev_remove         <------ netdev p0 removed
>     bridge_delete_or_reconfigure_ports
>       OFPROTO_PORT_FOR_EACH
>         ofproto_port_dump_next
>           port_dump_next
>           port_query_by_name    <------ netdev_shash do not contain p0
>         ofproto_port_del        <------ p0 do not del in ofproto
>     bridge_add_ports
>       bridge_add_ports__
>         iface_create
>           iface_do_create
>             ofproto_port_add    <------ p1 add failed
>
> Fixes: fe83f81df977 ("netdev: Remove netdev from global shash when the
user is changing interface configuration.")
> Signed-off-by: Tao Liu <[email protected]>
> ---
>  ofproto/ofproto-dpif.c | 13 +++++++++----
>  tests/tunnel.at        | 12 ++++++++++++
>  2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f59d69c4d..0cac3050d 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3905,14 +3905,19 @@ port_query_by_name(const struct ofproto
*ofproto_, const char *devname,
>
>      if (sset_contains(&ofproto->ghost_ports, devname)) {
>          const char *type = netdev_get_type_from_name(devname);
> +        const struct ofport *ofport =
> +                        shash_find_data(&ofproto->up.port_by_name,
devname);
> +        if (!type && ofport && ofport->netdev) {
> +            type = netdev_get_type(ofport->netdev);
> +        }
>
>          /* We may be called before ofproto->up.port_by_name is populated
with
>           * the appropriate ofport.  For this reason, we must get the
name and
> -         * type from the netdev layer directly. */
> +         * type from the netdev layer directly.
> +         * When a port deleted, the corresponding netdev is also removed
from
> +         * netdev_shash. netdev_get_type_from_name returns NULL in such
case.
> +         * We should try to get type from ofport->netdev. */

nit: this comment is better to be moved to the above where we are trying to
get the type from ofport.

Otherwise looks good to me:
Acked-by: Han Zhou <[email protected]>
Tested-by: Han Zhou <[email protected]>

>          if (type) {
> -            const struct ofport *ofport;
> -
> -            ofport = shash_find_data(&ofproto->up.port_by_name, devname);
>              ofproto_port->ofp_port = ofport ? ofport->ofp_port :
OFPP_NONE;
>              ofproto_port->name = xstrdup(devname);
>              ofproto_port->type = xstrdup(type);
> diff --git a/tests/tunnel.at b/tests/tunnel.at
> index 71e7c2df4..9d539ee6f 100644
> --- a/tests/tunnel.at
> +++ b/tests/tunnel.at
> @@ -1269,6 +1269,18 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])]
>  AT_CLEANUP
>
> +AT_SETUP([tunnel - re-create port with different name])
> +OVS_VSWITCHD_START(
> +  [add-port br0 p0 -- set int p0 type=vxlan
options:remote_ip=10.10.10.1])
> +
> +AT_CHECK([ovs-vsctl --if-exists del-port p0 -- \
> +          add-port br0 p1 -- \
> +          set int p1 type=vxlan options:remote_ip=10.10.10.1])
> +
> +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])]
> +AT_CLEANUP
> +
>  AT_SETUP([tunnel - SRV6 basic])
>  OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy \
>                      ofport_request=1 \
> --
> 2.31.1
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to