| Message ID | 1564658780-26495-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn] ovn-northd: Use HMAP_FOR_EACH when adding multicast lflows | expand |
On Thu, Aug 1, 2019 at 5:09 PM Dumitru Ceara <dceara@redhat.com> wrote: > There's no need to use HMAP_FOR_EACH_SAFE when we add the logical flows > that correspond to IGMP groups as we don't modify the hashmap in the > loop. > > Also, we should reinitialize match and actions only if the flow will be > added. > > Fixes: ddc64665b678 ("OVN: Add ovn-northd IGMP support") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > Acked-by: Numan Siddique <nusiddiq@redhat.com> > --- > northd/ovn-northd.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index dabd422..880773b 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -5258,12 +5258,9 @@ build_lswitch_flows(struct hmap *datapaths, struct > hmap *ports, > > /* Ingress table 17: Add IP multicast flows learnt from IGMP > * (priority 90). */ > - struct ovn_igmp_group *igmp_group, *next_igmp_group; > - > - HMAP_FOR_EACH_SAFE (igmp_group, next_igmp_group, hmap_node, > igmp_groups) { > - ds_clear(&match); > - ds_clear(&actions); > + struct ovn_igmp_group *igmp_group; > > + HMAP_FOR_EACH (igmp_group, hmap_node, igmp_groups) { > if (!igmp_group->datapath) { > continue; > } > @@ -5275,6 +5272,9 @@ build_lswitch_flows(struct hmap *datapaths, struct > hmap *ports, > } > mcast_info->active_flows++; > > + ds_clear(&match); > + ds_clear(&actions); > + > ds_put_format(&match, "eth.mcast && ip4 && ip4.dst == %s ", > igmp_group->mcgroup.name); > ds_put_format(&actions, "outport = \"%s\"; output; ", > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
I applied the change to master. On 8/5/19 9:08 AM, Numan Siddique wrote: > On Thu, Aug 1, 2019 at 5:09 PM Dumitru Ceara <dceara@redhat.com> wrote: > >> There's no need to use HMAP_FOR_EACH_SAFE when we add the logical flows >> that correspond to IGMP groups as we don't modify the hashmap in the >> loop. >> >> Also, we should reinitialize match and actions only if the flow will be >> added. >> >> Fixes: ddc64665b678 ("OVN: Add ovn-northd IGMP support") >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> > > Acked-by: Numan Siddique <nusiddiq@redhat.com> > > > >> --- >> northd/ovn-northd.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >> index dabd422..880773b 100644 >> --- a/northd/ovn-northd.c >> +++ b/northd/ovn-northd.c >> @@ -5258,12 +5258,9 @@ build_lswitch_flows(struct hmap *datapaths, struct >> hmap *ports, >> >> /* Ingress table 17: Add IP multicast flows learnt from IGMP >> * (priority 90). */ >> - struct ovn_igmp_group *igmp_group, *next_igmp_group; >> - >> - HMAP_FOR_EACH_SAFE (igmp_group, next_igmp_group, hmap_node, >> igmp_groups) { >> - ds_clear(&match); >> - ds_clear(&actions); >> + struct ovn_igmp_group *igmp_group; >> >> + HMAP_FOR_EACH (igmp_group, hmap_node, igmp_groups) { >> if (!igmp_group->datapath) { >> continue; >> } >> @@ -5275,6 +5272,9 @@ build_lswitch_flows(struct hmap *datapaths, struct >> hmap *ports, >> } >> mcast_info->active_flows++; >> >> + ds_clear(&match); >> + ds_clear(&actions); >> + >> ds_put_format(&match, "eth.mcast && ip4 && ip4.dst == %s ", >> igmp_group->mcgroup.name); >> ds_put_format(&actions, "outport = \"%s\"; output; ", >> -- >> 1.8.3.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index dabd422..880773b 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -5258,12 +5258,9 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, /* Ingress table 17: Add IP multicast flows learnt from IGMP * (priority 90). */ - struct ovn_igmp_group *igmp_group, *next_igmp_group; - - HMAP_FOR_EACH_SAFE (igmp_group, next_igmp_group, hmap_node, igmp_groups) { - ds_clear(&match); - ds_clear(&actions); + struct ovn_igmp_group *igmp_group; + HMAP_FOR_EACH (igmp_group, hmap_node, igmp_groups) { if (!igmp_group->datapath) { continue; } @@ -5275,6 +5272,9 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, } mcast_info->active_flows++; + ds_clear(&match); + ds_clear(&actions); + ds_put_format(&match, "eth.mcast && ip4 && ip4.dst == %s ", igmp_group->mcgroup.name); ds_put_format(&actions, "outport = \"%s\"; output; ",
There's no need to use HMAP_FOR_EACH_SAFE when we add the logical flows that correspond to IGMP groups as we don't modify the hashmap in the loop. Also, we should reinitialize match and actions only if the flow will be added. Fixes: ddc64665b678 ("OVN: Add ovn-northd IGMP support") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- northd/ovn-northd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)