Hi Team, Could someone please look into this and merge if everything looks good ?
Thanks, Naveen > On 30-Mar-2023, at 1:47 PM, Naveen Yerramneni <[email protected]> > wrote: > > Add OpenFlow extn to set conntrack entries limit per zone. > This extn will be used in future to set the zone level limit for > drop zones used by OVN. > > Signed-off-by: Naveen Yerramneni <[email protected]> > Reviewed-by: Simon Horman <[email protected]> > --- > Notes: > v1 -> v2 > - Fix memory leak and added logs > v2 -> v3 > - Addressed nits > v3 -> v4 > - Updated change description > > NEWS | 2 ++ > include/openflow/nicira-ext.h | 10 ++++++++++ > include/openvswitch/ofp-msgs.h | 4 ++++ > lib/ofp-bundle.c | 1 + > lib/ofp-print.c | 11 +++++++++++ > lib/rconn.c | 1 + > ofproto/ofproto-dpif.c | 21 +++++++++++++++++++++ > ofproto/ofproto-provider.h | 4 ++++ > ofproto/ofproto.c | 25 +++++++++++++++++++++++++ > tests/ofp-print.at | 10 ++++++++++ > tests/ovs-ofctl.at | 12 ++++++++++++ > utilities/ovs-ofctl.8.in | 5 +++++ > utilities/ovs-ofctl.c | 34 ++++++++++++++++++++++++++++++++++ > 13 files changed, 140 insertions(+) > > diff --git a/NEWS b/NEWS > index fe6055a27..f6ae60856 100644 > --- a/NEWS > +++ b/NEWS > @@ -32,6 +32,8 @@ v3.1.0 - xx xxx xxxx > - OpenFlow: > * New OpenFlow extension NXT_CT_FLUSH to flush connections matching > the specified fields. > + * New OpenFlow extension NXT_CT_SET_ZONE_LIMIT to set conntrack table > + limit at zone level. > - ovs-ctl: > * New option '--dump-hugepages' to include hugepages in core dumps. This > can assist with postmortem analysis involving DPDK, but may also > produce > diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h > index 768775898..0f93ea21c 100644 > --- a/include/openflow/nicira-ext.h > +++ b/include/openflow/nicira-ext.h > @@ -1101,4 +1101,14 @@ struct nx_ct_flush { > }; > OFP_ASSERT(sizeof(struct nx_ct_flush) == 8); > > +/* NXT_CT_SET_ZONE_LIMIT. > + * > + * Sets connection tracking table zone limit. */ > +struct nx_ct_zone_limit { > + uint8_t zero[2]; /* Must be zero. */ > + ovs_be16 zone_id; /* Connection tracking zone. */ > + ovs_be32 limit; /* Drop limit. */ > +}; > +OFP_ASSERT(sizeof(struct nx_ct_zone_limit) == 8); > + > #endif /* openflow/nicira-ext.h */ > diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h > index 708427fc0..a9518557e 100644 > --- a/include/openvswitch/ofp-msgs.h > +++ b/include/openvswitch/ofp-msgs.h > @@ -518,6 +518,9 @@ enum ofpraw { > /* NXT 1.0+ (32): struct nx_ct_flush, uint8_t[8][]. */ > OFPRAW_NXT_CT_FLUSH, > > + /* NXT 1.0+ (35): struct nx_ct_zone_limit. */ > + OFPRAW_NXT_CT_SET_ZONE_LIMIT, > + > /* NXST 1.0+ (3): void. */ > OFPRAW_NXST_IPFIX_BRIDGE_REQUEST, > > @@ -776,6 +779,7 @@ enum ofptype { > OFPTYPE_IPFIX_FLOW_STATS_REPLY, /* OFPRAW_NXST_IPFIX_FLOW_REPLY */ > OFPTYPE_CT_FLUSH_ZONE, /* OFPRAW_NXT_CT_FLUSH_ZONE. */ > OFPTYPE_CT_FLUSH, /* OFPRAW_NXT_CT_FLUSH. */ > + OFPTYPE_CT_SET_ZONE_LIMIT, /* OFPRAW_NXT_CT_SET_ZONE_LIMIT. */ > > /* Flow monitor extension. */ > OFPTYPE_FLOW_MONITOR_CANCEL, /* OFPRAW_NXT_FLOW_MONITOR_CANCEL. > diff --git a/lib/ofp-bundle.c b/lib/ofp-bundle.c > index 941a8370e..3ed1f30d8 100644 > --- a/lib/ofp-bundle.c > +++ b/lib/ofp-bundle.c > @@ -293,6 +293,7 @@ ofputil_is_bundlable(enum ofptype type) > case OFPTYPE_IPFIX_FLOW_STATS_REPLY: > case OFPTYPE_CT_FLUSH_ZONE: > case OFPTYPE_CT_FLUSH: > + case OFPTYPE_CT_SET_ZONE_LIMIT: > break; > } > > diff --git a/lib/ofp-print.c b/lib/ofp-print.c > index 874079b84..8a64b72c0 100644 > --- a/lib/ofp-print.c > +++ b/lib/ofp-print.c > @@ -967,6 +967,15 @@ ofp_print_nxt_ct_flush(struct ds *string, const struct > ofp_header *oh) > return 0; > } > > +static enum ofperr > +ofp_print_nxt_ct_set_zone_limit(struct ds *string, > + const struct nx_ct_zone_limit *nzl) > +{ > + ds_put_format(string, " zone_id=%"PRIu16, ntohs(nzl->zone_id)); > + ds_put_format(string, " limit=%"PRIu32, ntohl(nzl->limit)); > + return 0; > +} > + > static enum ofperr > ofp_to_string__(const struct ofp_header *oh, > const struct ofputil_port_map *port_map, > @@ -1204,6 +1213,8 @@ ofp_to_string__(const struct ofp_header *oh, > return ofp_print_nxt_ct_flush_zone(string, ofpmsg_body(oh)); > case OFPTYPE_CT_FLUSH: > return ofp_print_nxt_ct_flush(string, oh); > + case OFPTYPE_CT_SET_ZONE_LIMIT: > + return ofp_print_nxt_ct_set_zone_limit(string, ofpmsg_body(oh)); > } > > return 0; > diff --git a/lib/rconn.c b/lib/rconn.c > index 4afa21515..91c982d98 100644 > --- a/lib/rconn.c > +++ b/lib/rconn.c > @@ -1427,6 +1427,7 @@ is_admitted_msg(const struct ofpbuf *b) > case OFPTYPE_IPFIX_FLOW_STATS_REPLY: > case OFPTYPE_CT_FLUSH_ZONE: > case OFPTYPE_CT_FLUSH: > + case OFPTYPE_CT_SET_ZONE_LIMIT: > default: > return true; > } > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index f87e27a8c..b0a66ef10 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -5631,6 +5631,26 @@ ct_del_zone_timeout_policy(const char *datapath_type, > uint16_t zone_id) > } > } > > +static void > +ct_set_zone_limit(const struct ofproto *ofproto_, const uint16_t zone_id, > + const uint32_t limit) > +{ > + struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); > + struct ovs_list zone_limits = OVS_LIST_INITIALIZER(&zone_limits); > + > + ct_dpif_push_zone_limit(&zone_limits, zone_id, limit, 0); > + int err = ct_dpif_set_limits(ofproto->backer->dpif, NULL, &zone_limits); > + if (err) { > + VLOG_ERR_RL(&rl, "failed to set zone limit id=%"PRIu16", " > + "limit=%"PRIu32" (%s)", zone_id, limit, > + ovs_strerror(err)); > + } else { > + VLOG_DBG("configured zone limit for zone=%"PRIu16", limit=%"PRIu32"", > + zone_id, limit); > + } > + ct_dpif_free_zone_limits(&zone_limits); > +} > + > static void > get_datapath_cap(const char *datapath_type, struct smap *cap) > { > @@ -6920,4 +6940,5 @@ const struct ofproto_class ofproto_dpif_class = { > ct_flush, /* ct_flush */ > ct_set_zone_timeout_policy, > ct_del_zone_timeout_policy, > + ct_set_zone_limit, /* ct_set_zone_limit */ > }; > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > index a84ddc1d0..c66623637 100644 > --- a/ofproto/ofproto-provider.h > +++ b/ofproto/ofproto-provider.h > @@ -1916,6 +1916,10 @@ struct ofproto_class { > /* Deletes the timeout policy associated with 'zone' in datapath type > * 'dp_type'. */ > void (*ct_del_zone_timeout_policy)(const char *dp_type, uint16_t zone); > + > + /* Sets conntrack zone limit */ > + void (*ct_set_zone_limit)(const struct ofproto *, const uint16_t zone, > + const uint32_t limit); > }; > > extern const struct ofproto_class ofproto_dpif_class; > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index e4a1bee76..e8e884937 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -966,6 +966,28 @@ handle_nxt_ct_flush(struct ofconn *ofconn, const struct > ofp_header *oh) > return 0; > } > > +static enum ofperr > +handle_nxt_ct_set_zone_limit(struct ofconn *ofconn, > + const struct ofp_header *oh) > +{ > + struct ofproto *ofproto = ofconn_get_ofproto(ofconn); > + const struct nx_ct_zone_limit *nzl = ofpmsg_body(oh); > + > + if (!is_all_zeros(nzl->zero, sizeof nzl->zero)) { > + return OFPERR_NXBRC_MUST_BE_ZERO; > + } > + > + uint16_t zone_id = ntohs(nzl->zone_id); > + uint32_t limit = ntohl(nzl->limit); > + if (ofproto->ofproto_class->ct_set_zone_limit) { > + ofproto->ofproto_class->ct_set_zone_limit(ofproto, zone_id, limit); > + } else { > + return EOPNOTSUPP; > + } > + > + return 0; > +} > + > void > ofproto_set_flow_restore_wait(bool flow_restore_wait_db) > { > @@ -8814,6 +8836,9 @@ handle_single_part_openflow(struct ofconn *ofconn, > const struct ofp_header *oh, > case OFPTYPE_CT_FLUSH: > return handle_nxt_ct_flush(ofconn, oh); > > + case OFPTYPE_CT_SET_ZONE_LIMIT: > + return handle_nxt_ct_set_zone_limit(ofconn, oh); > + > case OFPTYPE_HELLO: > case OFPTYPE_ERROR: > case OFPTYPE_FEATURES_REPLY: > diff --git a/tests/ofp-print.at b/tests/ofp-print.at > index 14aa55416..5c45e1ec6 100644 > --- a/tests/ofp-print.at > +++ b/tests/ofp-print.at > @@ -4181,3 +4181,13 @@ AT_CHECK([ovs-ofctl ofp-print "\ > 00 00 00 14 00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 00 00 00 00 \ > " | grep -q OFPBPC_BAD_VALUE], [0]) > AT_CLEANUP > + > +AT_SETUP([NXT_CT_SET_ZONE_LIMIT]) > +AT_KEYWORDS([ofp-print]) > +AT_CHECK([ovs-ofctl ofp-print "\ > +01 04 00 18 00 00 00 03 00 00 23 20 00 00 00 23 \ > +00 00 00 12 00 01 86 a0 \ > +"], [0], [dnl > +NXT_CT_SET_ZONE_LIMIT (xid=0x3): zone_id=18 limit=100000 > +]) > +AT_CLEANUP > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at > index 8531b2e2e..8a17d3609 100644 > --- a/tests/ovs-ofctl.at > +++ b/tests/ovs-ofctl.at > @@ -3309,3 +3309,15 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: <all>" > ovs-vswitchd.log]) > > OVS_VSWITCHD_STOP > AT_CLEANUP > + > + > +AT_SETUP([ovs-ofctl ct-set-zone-limit]) > +OVS_VSWITCHD_START > + > +AT_CHECK([ovs-appctl vlog/set ct_dpif:dbg]) > +AT_CHECK([ovs-ofctl ct-set-zone-limit br0 1 200000]) > + > +OVS_WAIT_UNTIL([grep -q "ofproto_dpif|DBG|.*zone=1.*limit=200000" > ovs-vswitchd.log]) > +AT_CHECK([grep -q "ofproto_dpif|DBG|.*zone=1.*limit=200000" > ovs-vswitchd.log]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in > index 0a611b2ee..8a6d5a3db 100644 > --- a/utilities/ovs-ofctl.8.in > +++ b/utilities/ovs-ofctl.8.in > @@ -326,6 +326,11 @@ An example of an IPv6 TCP \fIct-[orig|reply]-tuple\fR: > .IP > This command uses an Open vSwitch extension that is only in Open vSwitch 3.1 > and later. > +.IP "\fBct\-set\-zone\-limit \fIswitch zone limit\fR > +Set the connection tracking entries limit in \fIzone\fR on \fIswitch\fR. > +.IP > +This command uses an Open vSwitch extension that is only in Open > +vSwitch 3.1 and later. > . > .SS "OpenFlow Switch Flow Table Commands" > . > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c > index eabec18a3..1464827bb 100644 > --- a/utilities/ovs-ofctl.c > +++ b/utilities/ovs-ofctl.c > @@ -489,6 +489,8 @@ usage(void) > " ct-flush SWITCH [ZONE] [CT_ORIG_TUPLE [CT_REPLY_TUPLE]]\n" > " flush conntrack entries specified\n" > " by CT_ORIG/REPLY_TUPLE and ZONE\n" > + " ct-set-zone-limit SWITCH ZONE LIMIT set conntrack entries\n" > + " limit for the ZONE\n" > "\nFor OpenFlow switches and controllers:\n" > " probe TARGET probe whether TARGET is up\n" > " ping TARGET [N] latency of N-byte echos\n" > @@ -3098,6 +3100,35 @@ ofctl_ct_flush(struct ovs_cmdl_context *ctx) > vconn_close(vconn); > } > > +static void > +ofctl_ct_set_zone_limit(struct ovs_cmdl_context *ctx) > +{ > + uint16_t zone_id; > + uint32_t limit; > + > + char *error = str_to_u16(ctx->argv[2], "zone_id", &zone_id); > + if (error) { > + ovs_fatal(0, "%s", error); > + } > + error = str_to_u32(ctx->argv[3], &limit); > + if (error) { > + ovs_fatal(0, "%s", error); > + } > + > + struct vconn *vconn; > + open_vconn(ctx->argv[1], &vconn); > + enum ofp_version version = vconn_get_version(vconn); > + > + struct ofpbuf *msg = ofpraw_alloc(OFPRAW_NXT_CT_SET_ZONE_LIMIT, version, > + 0); > + struct nx_ct_zone_limit *nzl = ofpbuf_put_zeros(msg, sizeof *nzl); > + nzl->zone_id = htons(zone_id); > + nzl->limit = htonl(limit); > + > + transact_noreply(vconn, msg); > + vconn_close(vconn); > +} > + > static void > ofctl_dump_ipfix_flow(struct ovs_cmdl_context *ctx) > { > @@ -5114,6 +5145,9 @@ static const struct ovs_cmdl_command all_commands[] = { > { "ct-flush", "switch [zone=N] [ct-orig-tuple [ct-reply-tuple]]", > 1, 4, ofctl_ct_flush, OVS_RO }, > > + { "ct-set-zone-limit", "switch zone limit", > + 3, 3, ofctl_ct_set_zone_limit, OVS_RO }, > + > { "ofp-parse", "file", > 1, 1, ofctl_ofp_parse, OVS_RW }, > { "ofp-parse-pcap", "pcap", > -- > 2.22.3 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
