On 25.06.2019 13:12, Ilya Maximets wrote:
> 'netdev' datapath is implemented within ovs-vswitchd process and can
> not exist without it, so it should be gracefully terminated with a
> full cleanup of resources upon ovs-vswitchd exit.
>
> This change forces dpif cleanup for 'netdev' datapath regardless of
> passing '--cleanup' to 'ovs-appctl exit'. Such solution allowes to
> not pass this additional option everytime for userspace datapath
> installations and also allowes to not terminate system datapath in
> setups where both datapaths runs at the same time.
>
> Exception made for 'internal' ports that could have user ip/route
> configuration. These ports will not be removed without '--cleanup'.
>
> This change fixes OVS disappearing from the DPDK point of view
> (keeping HW NICs improperly configured, sudden closing of vhost-user
> connections) and will help with linux devices clearing with upcoming
> AF_XDP netdev support.
>
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
I forgot about the change log. Sorry.
Version 2:
* Internal ports removal avoided to preserve user ip/route config.
> NEWS | 3 +++
> lib/dpif-netdev.c | 1 +
> lib/dpif-netlink.c | 1 +
> lib/dpif-provider.h | 6 ++++++
> lib/dpif.c | 7 +++++++
> lib/dpif.h | 2 ++
> ofproto/ofproto-dpif.c | 8 ++++++++
> vswitchd/ovs-vswitchd.8.in | 4 ++++
> 8 files changed, 32 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index af1659ce8..2b6393803 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -27,6 +27,9 @@ Post-v2.11.0
> * New action "check_pkt_len".
> * Port configuration with "other-config:priority-tags" now has a mode
> that retains the 802.1Q header even if VLAN and priority are both
> zero.
> + * 'ovs-appctl exit' now implies cleanup of non-internal ports in
> userspace
> + datapath regardless of '--cleanup' option. Use '--cleanup' to remove
> + internal ports too.
> - OVSDB:
> * OVSDB clients can now resynchronize with clustered servers much more
> quickly after a brief disconnection, saving bandwidth and CPU time.
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 4e73f96fb..ad6ff5338 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -7411,6 +7411,7 @@ dpif_netdev_ipf_dump_done(struct dpif *dpif OVS_UNUSED,
> void *ipf_dump_ctx)
>
> const struct dpif_class dpif_netdev_class = {
> "netdev",
> + true, /* cleanup_required */
> dpif_netdev_init,
> dpif_netdev_enumerate,
> dpif_netdev_port_open_type,
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index ba80a0079..985a28426 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -3384,6 +3384,7 @@ probe_broken_meters(struct dpif *dpif)
>
> const struct dpif_class dpif_netlink_class = {
> "system",
> + false, /* cleanup_required */
> NULL, /* init */
> dpif_netlink_enumerate,
> NULL,
> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> index b2a4dff96..12898b9e3 100644
> --- a/lib/dpif-provider.h
> +++ b/lib/dpif-provider.h
> @@ -119,6 +119,12 @@ struct dpif_class {
> * the type assumed if no type is specified when opening a dpif. */
> const char *type;
>
> + /* If 'true', datapath ports should be destroyed on ofproto destruction.
> + *
> + * This is used by the vswitch at exit, so that it can clean any
> + * datapaths that can not exist without it (e.g. netdev datapath). */
> + bool cleanup_required;
> +
> /* Called when the dpif provider is registered, typically at program
> * startup. Returning an error from this function will prevent any
> * datapath with this class from being created.
> diff --git a/lib/dpif.c b/lib/dpif.c
> index a18fb1b02..c88b2106f 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -498,6 +498,13 @@ dpif_type(const struct dpif *dpif)
> return dpif->dpif_class->type;
> }
>
> +/* Checks if datapath 'dpif' requires cleanup. */
> +bool
> +dpif_cleanup_required(const struct dpif *dpif)
> +{
> + return dpif->dpif_class->cleanup_required;
> +}
> +
> /* Returns the fully spelled out name for the given datapath 'type'.
> *
> * Normalized type string can be compared with strcmp(). Unnormalized type
> diff --git a/lib/dpif.h b/lib/dpif.h
> index 883472a59..289d574a0 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -419,6 +419,8 @@ const char *dpif_name(const struct dpif *);
> const char *dpif_base_name(const struct dpif *);
> const char *dpif_type(const struct dpif *);
>
> +bool dpif_cleanup_required(const struct dpif *);
> +
> int dpif_delete(struct dpif *);
>
> /* Statistics for a dpif as a whole. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index cbeb6776f..751535249 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1965,6 +1965,7 @@ port_destruct(struct ofport *port_, bool del)
> struct ofport_dpif *port = ofport_dpif_cast(port_);
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
> const char *devname = netdev_get_name(port->up.netdev);
> + const char *netdev_type = netdev_get_type(port->up.netdev);
> char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
> const char *dp_port_name;
>
> @@ -1973,6 +1974,13 @@ port_destruct(struct ofport *port_, bool del)
> xlate_ofport_remove(port);
> xlate_txn_commit();
>
> + if (!del && strcmp(netdev_type,
> + ofproto_port_open_type(port->up.ofproto,
> "internal"))) {
> + /* Check if datapath requires removal of attached ports. Avoid
> + * removal of 'internal' ports to preserve user ip/route settings. */
> + del = dpif_cleanup_required(ofproto->backer->dpif);
> + }
> +
> dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,
> sizeof namebuf);
> if (del && dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
> diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
> index fcf22244a..a5cdc3aa4 100644
> --- a/vswitchd/ovs-vswitchd.8.in
> +++ b/vswitchd/ovs-vswitchd.8.in
> @@ -110,6 +110,10 @@ how to configure Open vSwitch.
> Causes \fBovs\-vswitchd\fR to gracefully terminate. If \fI--cleanup\fR
> is specified, release datapath resources configured by \fBovs\-vswitchd\fR.
> Otherwise, datapath flows and other resources remains undeleted.
> +Resources of datapaths that are integrated into \fBovs\-vswitchd\fR (e.g.
> +the \fBnetdev\fR datapath type) are always released regardless of
> +\fI--cleanup\fR except for ports with \fBinternal\fR type. Use
> \fI--cleanup\fR
> +to release \fBinternal\fR ports too.
> .
> .IP "\fBqos/show-types\fR \fIinterface\fR"
> Queries the interface for a list of Quality of Service types that are
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
