On Mon, Apr 24, 2023 at 02:01:03PM +0800, Wan Junjie via dev wrote:
> ovs-save will do replace-flows for ovs restart. For some reason, ovs
> could have an invalid action, like invalid meter. Then adding flows
> will be stopped at the position of the first invalid flow. This could
> happen when ovs restart and vm related resource allocatting or destructing
> at the same time.
>
> This patch will skip the invalid flows and continue the process so
> all valid flows will be added during the ovs restart. An destructed vm
> will not affect those normal vm traffic.
>
> Signed-off-by: Wan Junjie <wanjun...@bytedance.com>
Hi Wan Junjie,
thanks for your patch.
> ---
> include/openvswitch/vconn.h | 2 +-
> lib/vconn.c | 23 ++++++++++++++---------
> utilities/ovs-ofctl.c | 19 +++++++++++++------
> 3 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
> index 5f69c732b..781c59075 100644
> --- a/include/openvswitch/vconn.h
> +++ b/include/openvswitch/vconn.h
> @@ -57,7 +57,7 @@ int vconn_recv_xid(struct vconn *, ovs_be32 xid, struct
> ofpbuf **);
> int vconn_transact(struct vconn *, struct ofpbuf *, struct ofpbuf **);
> int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf
> **);
> int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list
> *requests,
> - struct ofpbuf **replyp);
> + struct ovs_list *replies);
> int vconn_transact_multipart(struct vconn *, struct ovs_list *request,
> struct ovs_list *replies);
>
> diff --git a/lib/vconn.c b/lib/vconn.c
> index b55676227..a5f6177ec 100644
> --- a/lib/vconn.c
> +++ b/lib/vconn.c
> @@ -926,23 +926,28 @@ vconn_transact_noreply(struct vconn *vconn, struct
> ofpbuf *request,
> /* vconn_transact_noreply() for a list of "struct ofpbuf"s, sent one by one.
> * All of the requests on 'requests' are always destroyed, regardless of the
> * return value. */
> -int
> -vconn_transact_multiple_noreply(struct vconn *vconn, struct ovs_list
> *requests,
> - struct ofpbuf **replyp)
> -{
> +int vconn_transact_multiple_noreply(struct vconn *vconn,
> + struct ovs_list *requests,
> + struct ovs_list *replies) {
> struct ofpbuf *request;
> + int error = 0;
>
> LIST_FOR_EACH_POP (request, list_node, requests) {
> - int error;
> + struct ofpbuf *reply;
> +
> + error = vconn_transact_noreply(vconn, request, &reply);
>
nit: no blank line here please
> - error = vconn_transact_noreply(vconn, request, replyp);
> - if (error || *replyp) {
> - ofpbuf_list_delete(requests);
> + if (error && !reply) {
> return error;
> }
> + if (reply) {
> + ovs_list_push_back(replies, &reply->list_node);
> + }
> }
>
> - *replyp = NULL;
> + if (!ovs_list_is_empty(replies)) {
> + return error;
If there has been a reply then we get here.
And return the most recent value of error which,
by my reading, may not relate to any of the replies.
Is this intentional?
> + }
> return 0;
> }
>
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 24d0941cf..a84551672 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -715,18 +715,25 @@ dump_trivial_transaction(const char *vconn_name, enum
> ofpraw ofpraw)
> static void
> transact_multiple_noreply(struct vconn *vconn, struct ovs_list *requests)
> {
> + struct ovs_list replies;
> struct ofpbuf *reply;
>
> - run(vconn_transact_multiple_noreply(vconn, requests, &reply),
> + ovs_list_init(&replies);
> +
> + run(vconn_transact_multiple_noreply(vconn, requests, &replies),
> "talking to %s", vconn_get_name(vconn));
> - if (reply) {
> +
nit: no blank line here please
> + if (ovs_list_is_empty(&replies)) {
> + return;
> + }
> +
> + LIST_FOR_EACH_POP (reply, list_node, &replies) {
> ofp_print(stderr, reply->data, reply->size,
> ports_to_show(vconn_get_name(vconn)),
> - tables_to_show(vconn_get_name(vconn)),
> - verbosity + 2);
> - exit(1);
> + tables_to_show(vconn_get_name(vconn)), verbosity + 2);
nit: the change to the line immediately above seems to be whitepace only.
I don't think it belongs in this patch.
> + ofpbuf_delete(reply);
> }
> - ofpbuf_delete(reply);
> + exit(1);
> }
>
> /* Frees the error messages as they are printed. */
...
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
