On Mon, Jul 8, 2024 at 5:01 PM Qiang Qiang45 Zhang via dev <
ovs-dev@openvswitch.org> wrote:

> When a logical switch has 2 FWGs and each FWG has 3 ports,
> Logical-Flow only has one fwg_group() action.
> Submitted-at: northd: Fix issues for Forwarding_Group by ZhangQiang3 *
> Pull Request #250 * ovn-org/ovn (github.com)<
> https://github.com/ovn-org/ovn/pull/250>
>
> From 02186da234426bc361615eb6b5142c76f296202f Mon Sep 17 00:00:00 2001
> From: zhangqiang45 zhangqian...@lenovo.com<mailto:zhangqian...@lenovo.com>
> Date: Mon, 8 Jul 2024 14:25:04 +0800
> Subject: [PATCH ovn] northd: Fix issues for Forwarding_Group     The use of
> variables from the outer loop in the inner loop causes the outer loop to
> terminate prematurely.     eg. LVS are three fwgs,
> fwg1(p1,p2,p3,p4),fwg2(p11,p22),fwg3(p31,p32),only fwg1 in logical_flows
>
> ---
>

Hello,

thank you for the fix. The commit message seems to be broken as it
includes the email header for some reason. Also could you please add
test to ovn-northd.at that ensures this won't happen in future?

northd/northd.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 6898daa00..21ab0bb91 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -7929,6 +7929,9 @@ build_fwd_group_lflows(struct ovn_datapath *od,
> struct lflow_table *lflows,
>              continue;
>          }
>
> +        ds_clear(&match);
> +        ds_clear(&actions);
> +
>          /* ARP responder for the forwarding group's virtual IP */
>          ds_put_format(&match, "arp.tpa == %s && arp.op == 1",
>                        fwd_group->vip);
> @@ -7959,9 +7962,9 @@ build_fwd_group_lflows(struct ovn_datapath *od,
> struct lflow_table *lflows,
>              ds_put_cstr(&group_ports, "liveness=\"true\",");
>          }
>          ds_put_cstr(&group_ports, "childports=");
> -        for (i = 0; i < (fwd_group->n_child_port - 1); ++i) {
> +        for (int j = 0; j < (fwd_group->n_child_port - 1); ++j) {
>

nit: Please use size_t as the "j" type.

             ds_put_format(&group_ports, "\"%s\",",
> -                         fwd_group->child_port[i]);
> +                         fwd_group->child_port[j]);
>          }
>          ds_put_format(&group_ports, "\"%s\"",
>                        fwd_group->child_port[fwd_group->n_child_port - 1]);
> --
> 2.39.3
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

amu...@redhat.com
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to