On 12/2/22 18:31, Vladislav Odintsov wrote: > It is possible to send OpenFlow group_mod message to OVS to create a > group with any number of buckets: > > ovs-ofctl dump-groups br-int > NXST_GROUP_DESC reply (xid=0x2): > > group_id=4,type=select,selection_method=dp_hash,bucket=bucket_id:0,weight:100,actions=load:0x1->OXM_OF_PKT_REG4[48..63],resubmit(,20),...bucket=bucket_id:10000,... > > This patch introduces a limit of buckets that may be requested to 1024. > In case the limit is reached, ovn-controller will write WARN log about > this fact.
Isn't it simpler to just limit the number of buckets in northd instead? What is the downside of doing that instead? Thanks, Dumitru > > Signed-off-by: Vladislav Odintsov <[email protected]> > --- > lib/actions.c | 40 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 38 insertions(+), 2 deletions(-) > > diff --git a/lib/actions.c b/lib/actions.c > index adbb42db4..4322556bf 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -44,6 +44,9 @@ > #include "controller/lflow.h" > > VLOG_DEFINE_THIS_MODULE(actions); > + > +#define MAX_BUCKETS_PER_GROUP 1024 > + > > /* Prototypes for functions to be defined by each action. */ > #define OVNACT(ENUM, STRUCT) \ > @@ -1371,7 +1374,18 @@ encode_ct_lb(const struct ovnact_ct_lb *cl, > BUILD_ASSERT(MFF_LOG_CT_ZONE < MFF_REG0 + FLOW_N_REGS); > BUILD_ASSERT(MFF_LOG_DNAT_ZONE >= MFF_REG0); > BUILD_ASSERT(MFF_LOG_DNAT_ZONE < MFF_REG0 + FLOW_N_REGS); > - for (size_t bucket_id = 0; bucket_id < cl->n_dsts; bucket_id++) { > + > + int n_buckets; > + bool group_overflow = false; > + if (cl->n_dsts > MAX_BUCKETS_PER_GROUP) { > + n_buckets = MAX_BUCKETS_PER_GROUP; > + group_overflow = true; > + } > + else { > + n_buckets = cl->n_dsts; > + } > + > + for (size_t bucket_id = 0; bucket_id < n_buckets; bucket_id++) { > const struct ovnact_ct_lb_dst *dst = &cl->dsts[bucket_id]; > char ip_addr[INET6_ADDRSTRLEN]; > if (dst->family == AF_INET) { > @@ -1405,6 +1419,12 @@ encode_ct_lb(const struct ovnact_ct_lb *cl, > /* Create an action to set the group. */ > og = ofpact_put_GROUP(ofpacts); > og->group_id = table_id; > + > + if (group_overflow) { > + VLOG_WARN("OF group id '%d' is desired to have more than " > + "MAX_BUCKETS_PER_GROUP (%d) buckets. Limited to maximum.", > + table_id, MAX_BUCKETS_PER_GROUP); > + } > } > > static void > @@ -1542,7 +1562,17 @@ encode_SELECT(const struct ovnact_select *select, > > struct mf_subfield sf = expr_resolve_field(&select->res_field); > > - for (size_t bucket_id = 0; bucket_id < select->n_dsts; bucket_id++) { > + int n_buckets; > + bool group_overflow = false; > + if (select->n_dsts > MAX_BUCKETS_PER_GROUP) { > + n_buckets = MAX_BUCKETS_PER_GROUP; > + group_overflow = true; > + } > + else { > + n_buckets = select->n_dsts; > + } > + > + for (size_t bucket_id = 0; bucket_id < n_buckets; bucket_id++) { > const struct ovnact_select_dst *dst = &select->dsts[bucket_id]; > ds_put_format(&ds, ",bucket=bucket_id=%"PRIuSIZE",weight:%"PRIu16 > ",actions=", bucket_id, dst->weight); > @@ -1561,6 +1591,12 @@ encode_SELECT(const struct ovnact_select *select, > /* Create an action to set the group. */ > og = ofpact_put_GROUP(ofpacts); > og->group_id = table_id; > + > + if (group_overflow) { > + VLOG_WARN("OF group id '%d' is desired to have more than " > + "MAX_BUCKETS_PER_GROUP (%d) buckets. Limited to maximum.", > + table_id, MAX_BUCKETS_PER_GROUP); > + } > } > > static void _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
