On Mon, Jun 24, 2019 at 06:28:37PM +0300, 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. > > This 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.
Does this mean that OVS bridges or internal ports will be deleted from the system regardless of --cleanup parameter? fbl > > Signed-off-by: Ilya Maximets <[email protected]> > --- > NEWS | 2 ++ > 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 | 4 +++- > vswitchd/ovs-vswitchd.8.in | 3 +++ > 8 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index af1659ce8..dbbc3827b 100644 > --- a/NEWS > +++ b/NEWS > @@ -27,6 +27,8 @@ 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 userspace datapath cleanup regardless > + of '--cleanup' option. > - 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..f503d116a 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 must be destroyed on ofproto destruction. > + * > + * This is used by the vswitch at exit, so that it can delete 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..957f70cfa 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -678,7 +678,7 @@ close_dpif_backer(struct dpif_backer *backer, bool del) > shash_find_and_delete(&all_dpif_backers, backer->type); > free(backer->type); > free(backer->dp_version_string); > - if (del) { > + if (del || dpif_cleanup_required(backer->dpif)) { > dpif_delete(backer->dpif); > } > dpif_close(backer->dpif); > @@ -1973,6 +1973,8 @@ port_destruct(struct ofport *port_, bool del) > xlate_ofport_remove(port); > xlate_txn_commit(); > > + del = 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..b5e1c1a8f 100644 > --- a/vswitchd/ovs-vswitchd.8.in > +++ b/vswitchd/ovs-vswitchd.8.in > @@ -110,6 +110,9 @@ 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. > . > .IP "\fBqos/show-types\fR \fIinterface\fR" > Queries the interface for a list of Quality of Service types that are > -- > 2.17.1 > > _______________________________________________ > 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
