| Message ID | SEZPR03MB74027DE77AA2C328B293173CAFA52@SEZPR03MB7402.apcprd03.prod.outlook.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [ovs-dev] 回复: [External] Re: [PATCH ovn] northd: Fix issues for Forwarding_Group | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Thu, Jul 11, 2024 at 5:41 AM Qiang Qiang45 Zhang <zhangqiang45@lenovo.com> wrote: > Test cases have been added. > > > > > > *发件人:* Ales Musil <amusil@redhat.com> > *发送时间:* 2024年7月10日 15:02 > *收件人:* Qiang Qiang45 Zhang <zhangqiang45@lenovo.com> > *抄送:* dev@openvswitch.org > *主题:* [External] Re: [ovs-dev] [PATCH ovn] northd: Fix issues for > Forwarding_Group > > > > > > > > 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 zhangqiang45@lenovo.com<mailto:zhangqiang45@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 > dev@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/> > > amusil@redhat.com > > <https://red.ht/sig> > > > Hi, the email format is super broken, the test looks fine, however the "j" in the loop is still int instead of size_t. Could you please change that and take a look at https://docs.ovn.org/en/latest/internals/contributing/submitting-patches.html how to properly send a patch email. Thanks, Ales
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]); }
Test cases have been added. 发件人: Ales Musil <amusil@redhat.com> 发送时间: 2024年7月10日 15:02 收件人: Qiang Qiang45 Zhang <zhangqiang45@lenovo.com> 抄送: dev@openvswitch.org 主题: [External] Re: [ovs-dev] [PATCH ovn] northd: Fix issues for Forwarding_Group On Mon, Jul 8, 2024 at 5:01 PM Qiang Qiang45 Zhang via dev <ovs-dev@openvswitch.org<mailto: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<http://github.com/>)<https://github.com/ovn-org/ovn/pull/250> From 02186da234426bc361615eb6b5142c76f296202f Mon Sep 17 00:00:00 2001 From: zhangqiang45 zhangqiang45@lenovo.com<mailto:zhangqiang45@lenovo.com><mailto:zhangqiang45@lenovo.com<mailto:zhangqiang45@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<http://ovn-northd.at/> that ensures this won't happen in future? northd/northd.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) ds_put_format(&group_ports, "\"%s\"", fwd_group->child_port[fwd_group->n_child_port - 1]); -- 2.39.3