Do you mean that northd should have some kind of common code, which is used to
generate buckets?
Initially I wanted to prevent the possibility even to send such a big OF group
to OVS. For all the cases:
- a new possible functionality which doesn’t limit buckets count;
- some kind of northd bug.
This place looks to me like a "last resort" for OF group generation. All
buckets are parsed/converted to OF syntax here.
Please correct me if I’m wrong.
Regards,
Vladislav Odintsov
> On 5 Dec 2022, at 19:37, Dumitru Ceara <dce...@redhat.com> wrote:
>
> 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 <odiv...@gmail.com>
>> ---
>> 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
