The 05/11/2021 11:38, Roi Dayan wrote:
> From: Jianbo Liu <[email protected]>
>
> There is a race condidtion between purger and handler. Handler may
> create new ukey and install it while executing 'ovs-appctl
> revalidator/purge' command. However, before handler calls
> transition_ukey() in handle_upcalls(), purger may get this ukey from
> umap, then evict and delete it. This will trigger ovs_abort in
> transition_ukey() for handler because it is trying to set state to
> EVICTED or OPERATIONAL, but ukey is already in DELETED state.
> To fix this issue, purger must not delete ukey in VISIBLE state.
Friendly ping for review ...
Thanks!
Jianbo
>
> Fixes: 98bb4286970d ("tests: Add command to purge revalidators of flows.")
> Signed-off-by: Jianbo Liu <[email protected]>
> Reviewed-by: Roi Dayan <[email protected]>
> ---
> ofproto/ofproto-dpif-upcall.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index ccf97266c0b9..3add505ff652 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -327,6 +327,12 @@ struct ukey_op {
> struct dpif_op dop; /* Flow operation. */
> };
>
> +enum sweep_type {
> + PURGE_NONE,
> + PURGE_SOFT,
> + PURGE_HARD,
> +};
> +
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> static struct ovs_list all_udpifs = OVS_LIST_INITIALIZER(&all_udpifs);
>
> @@ -345,7 +351,7 @@ static unsigned long udpif_get_n_flows(struct udpif *);
> static void revalidate(struct revalidator *);
> static void revalidator_pause(struct revalidator *);
> static void revalidator_sweep(struct revalidator *);
> -static void revalidator_purge(struct revalidator *);
> +static void revalidator_purge(struct revalidator *, enum sweep_type);
> static void upcall_unixctl_show(struct unixctl_conn *conn, int argc,
> const char *argv[], void *aux);
> static void upcall_unixctl_disable_megaflows(struct unixctl_conn *, int argc,
> @@ -541,7 +547,7 @@ udpif_stop_threads(struct udpif *udpif, bool delete_flows)
>
> if (delete_flows) {
> for (i = 0; i < udpif->n_revalidators; i++) {
> - revalidator_purge(&udpif->revalidators[i]);
> + revalidator_purge(&udpif->revalidators[i], PURGE_HARD);
> }
> }
>
> @@ -2772,7 +2778,8 @@ revalidator_pause(struct revalidator *revalidator)
> }
>
> static void
> -revalidator_sweep__(struct revalidator *revalidator, bool purge)
> +revalidator_sweep__(struct revalidator *revalidator,
> + enum sweep_type sweep_type)
> {
> struct udpif *udpif;
> uint64_t dump_seq, reval_seq;
> @@ -2803,13 +2810,13 @@ revalidator_sweep__(struct revalidator *revalidator,
> bool purge)
> }
> ukey_state = ukey->state;
> if (ukey_state == UKEY_OPERATIONAL
> - || (ukey_state == UKEY_VISIBLE && purge)) {
> + || (ukey_state == UKEY_VISIBLE && sweep_type == PURGE_HARD))
> {
> struct recirc_refs recircs = RECIRC_REFS_EMPTY_INITIALIZER;
> bool seq_mismatch = (ukey->dump_seq != dump_seq
> && ukey->reval_seq != reval_seq);
> enum reval_result result;
>
> - if (purge) {
> + if (sweep_type > PURGE_NONE) {
> result = UKEY_DELETE;
> } else if (!seq_mismatch) {
> result = UKEY_KEEP;
> @@ -2856,13 +2863,13 @@ revalidator_sweep__(struct revalidator *revalidator,
> bool purge)
> static void
> revalidator_sweep(struct revalidator *revalidator)
> {
> - revalidator_sweep__(revalidator, false);
> + revalidator_sweep__(revalidator, PURGE_NONE);
> }
>
> static void
> -revalidator_purge(struct revalidator *revalidator)
> +revalidator_purge(struct revalidator *revalidator, enum sweep_type
> sweep_type)
> {
> - revalidator_sweep__(revalidator, true);
> + revalidator_sweep__(revalidator, sweep_type);
> }
>
> /* In reaction to dpif purge, purges all 'ukey's with same 'pmd_id'. */
> @@ -3056,7 +3063,7 @@ upcall_unixctl_purge(struct unixctl_conn *conn, int
> argc OVS_UNUSED,
> int n;
>
> for (n = 0; n < udpif->n_revalidators; n++) {
> - revalidator_purge(&udpif->revalidators[n]);
> + revalidator_purge(&udpif->revalidators[n], PURGE_SOFT);
> }
> }
> unixctl_command_reply(conn, "");
> --
> 2.8.0
>
--
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
