| Message ID | 20240319155753.429593-1-amusil@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] controller: Fix ofctrl memory usage underflow. | 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 |
Thanks Ales, looks good to me. Acked-by: Mark Michelson <mmichels@redhat.com> On 3/19/24 11:57, Ales Musil wrote: > The memory usage would be increased for size of sb_addrset_ref > struct, but decreased for the size of the struct + the name. > That would slowly lead to underflows in some cases. > > Reported-at: https://issues.redhat.com/browse/FDP-507 > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > controller/ofctrl.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > index f14cd79a8..0ef3b8366 100644 > --- a/controller/ofctrl.c > +++ b/controller/ofctrl.c > @@ -1112,6 +1112,12 @@ sb_to_flow_size(const struct sb_to_flow *stf) > return sizeof *stf; > } > > +static size_t > +sb_addrset_ref_size(const struct sb_addrset_ref *sar) > +{ > + return sizeof *sar + strlen(sar->name) + 1; > +} > + > static struct sb_to_flow * > sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) > { > @@ -1181,8 +1187,8 @@ link_flow_to_sb(struct ovn_desired_flow_table *flow_table, > } > if (!found) { > sar = xmalloc(sizeof *sar); > - mem_stats.sb_flow_ref_usage += sizeof *sar; > sar->name = xstrdup(as_info->name); > + mem_stats.sb_flow_ref_usage += sb_addrset_ref_size(sar); > hmap_init(&sar->as_ip_to_flow_map); > ovs_list_insert(&stf->addrsets, &sar->list_node); > } > @@ -1568,7 +1574,7 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, > free(itfn); > } > hmap_destroy(&sar->as_ip_to_flow_map); > - mem_stats.sb_flow_ref_usage -= (sizeof *sar + strlen(sar->name) + 1); > + mem_stats.sb_flow_ref_usage -= sb_addrset_ref_size(sar); > free(sar->name); > free(sar); > }
On Wed, Mar 20, 2024 at 12:48 PM Mark Michelson <mmichels@redhat.com> wrote: > > Thanks Ales, looks good to me. > > Acked-by: Mark Michelson <mmichels@redhat.com> > Thanks Ales and Mark. I applied to main and backported down to branch-23.06. Han > On 3/19/24 11:57, Ales Musil wrote: > > The memory usage would be increased for size of sb_addrset_ref > > struct, but decreased for the size of the struct + the name. > > That would slowly lead to underflows in some cases. > > > > Reported-at: https://issues.redhat.com/browse/FDP-507 > > Signed-off-by: Ales Musil <amusil@redhat.com> > > --- > > controller/ofctrl.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > > index f14cd79a8..0ef3b8366 100644 > > --- a/controller/ofctrl.c > > +++ b/controller/ofctrl.c > > @@ -1112,6 +1112,12 @@ sb_to_flow_size(const struct sb_to_flow *stf) > > return sizeof *stf; > > } > > > > +static size_t > > +sb_addrset_ref_size(const struct sb_addrset_ref *sar) > > +{ > > + return sizeof *sar + strlen(sar->name) + 1; > > +} > > + > > static struct sb_to_flow * > > sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) > > { > > @@ -1181,8 +1187,8 @@ link_flow_to_sb(struct ovn_desired_flow_table *flow_table, > > } > > if (!found) { > > sar = xmalloc(sizeof *sar); > > - mem_stats.sb_flow_ref_usage += sizeof *sar; > > sar->name = xstrdup(as_info->name); > > + mem_stats.sb_flow_ref_usage += sb_addrset_ref_size(sar); > > hmap_init(&sar->as_ip_to_flow_map); > > ovs_list_insert(&stf->addrsets, &sar->list_node); > > } > > @@ -1568,7 +1574,7 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, > > free(itfn); > > } > > hmap_destroy(&sar->as_ip_to_flow_map); > > - mem_stats.sb_flow_ref_usage -= (sizeof *sar + strlen(sar->name) + 1); > > + mem_stats.sb_flow_ref_usage -= sb_addrset_ref_size(sar); > > free(sar->name); > > free(sar); > > } > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/controller/ofctrl.c b/controller/ofctrl.c index f14cd79a8..0ef3b8366 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -1112,6 +1112,12 @@ sb_to_flow_size(const struct sb_to_flow *stf) return sizeof *stf; } +static size_t +sb_addrset_ref_size(const struct sb_addrset_ref *sar) +{ + return sizeof *sar + strlen(sar->name) + 1; +} + static struct sb_to_flow * sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) { @@ -1181,8 +1187,8 @@ link_flow_to_sb(struct ovn_desired_flow_table *flow_table, } if (!found) { sar = xmalloc(sizeof *sar); - mem_stats.sb_flow_ref_usage += sizeof *sar; sar->name = xstrdup(as_info->name); + mem_stats.sb_flow_ref_usage += sb_addrset_ref_size(sar); hmap_init(&sar->as_ip_to_flow_map); ovs_list_insert(&stf->addrsets, &sar->list_node); } @@ -1568,7 +1574,7 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, free(itfn); } hmap_destroy(&sar->as_ip_to_flow_map); - mem_stats.sb_flow_ref_usage -= (sizeof *sar + strlen(sar->name) + 1); + mem_stats.sb_flow_ref_usage -= sb_addrset_ref_size(sar); free(sar->name); free(sar); }
The memory usage would be increased for size of sb_addrset_ref struct, but decreased for the size of the struct + the name. That would slowly lead to underflows in some cases. Reported-at: https://issues.redhat.com/browse/FDP-507 Signed-off-by: Ales Musil <amusil@redhat.com> --- controller/ofctrl.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)