| Message ID | 20231002103347.101274-3-amusil@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Expose CT limit via DB | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Ales Musil <amusil@redhat.com> writes: > Progpagate the CT limit that is present in the DB into > datapath. The limit is currently only propagated on change > and can be overwritten by the dpctl commands. > > Signed-off-by: Ales Musil <amusil@redhat.com> > Acked-by: Simon Horman <horms@ovn.org> > --- > v3: Rebase on top of current master. > Add ack from Simon and fix the missing '.'. > Revert the unrelated change. > --- > ofproto/ofproto-dpif.c | 39 ++++++++++++++++++++++++++++++++ > ofproto/ofproto-dpif.h | 5 +++++ > ofproto/ofproto-provider.h | 5 +++++ > ofproto/ofproto.c | 12 ++++++++++ > ofproto/ofproto.h | 2 ++ > tests/system-traffic.at | 46 +++++++++++++++++++++++++++++++++++++- > vswitchd/bridge.c | 9 ++++++++ > 7 files changed, 117 insertions(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index ba5706f6a..55eaeefa3 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -220,6 +220,7 @@ static void ofproto_unixctl_init(void); > static void ct_zone_config_init(struct dpif_backer *backer); > static void ct_zone_config_uninit(struct dpif_backer *backer); > static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer); > +static void ct_zone_limits_commit(struct dpif_backer *backer); > > static inline struct ofproto_dpif * > ofproto_dpif_cast(const struct ofproto *ofproto) > @@ -513,6 +514,7 @@ type_run(const char *type) > > process_dpif_port_changes(backer); > ct_zone_timeout_policy_sweep(backer); > + ct_zone_limits_commit(backer); > > return 0; > } > @@ -5522,6 +5524,8 @@ ct_zone_config_init(struct dpif_backer *backer) > cmap_init(&backer->ct_zones); > hmap_init(&backer->ct_tps); > ovs_list_init(&backer->ct_tp_kill_list); > + ovs_list_init(&backer->ct_zone_limits_to_add); > + ovs_list_init(&backer->ct_zone_limits_to_del); > clear_existing_ct_timeout_policies(backer); > } > > @@ -5545,6 +5549,8 @@ ct_zone_config_uninit(struct dpif_backer *backer) > id_pool_destroy(backer->tp_ids); > cmap_destroy(&backer->ct_zones); > hmap_destroy(&backer->ct_tps); > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add); > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del); > } > > static void > @@ -5625,6 +5631,38 @@ ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone_id) > } > } > > +static void > +ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, > + int64_t *limit) > +{ > + struct dpif_backer *backer = shash_find_data(&all_dpif_backers, > + datapath_type); > + if (!backer) { > + return; > + } > + > + if (limit) { > + ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_add, zone_id, > + *limit, 0); > + } else { > + ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_del, zone_id, 0, 0); > + } > +} > + > +static void > +ct_zone_limits_commit(struct dpif_backer *backer) > +{ > + if (!ovs_list_is_empty(&backer->ct_zone_limits_to_add)) { > + ct_dpif_set_limits(backer->dpif, NULL, &backer->ct_zone_limits_to_add); > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add); > + } > + > + if (!ovs_list_is_empty(&backer->ct_zone_limits_to_del)) { > + ct_dpif_del_limits(backer->dpif, &backer->ct_zone_limits_to_del); > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del); > + } > +} > + > static void > get_datapath_cap(const char *datapath_type, struct smap *cap) > { > @@ -6914,4 +6952,5 @@ const struct ofproto_class ofproto_dpif_class = { > ct_flush, /* ct_flush */ > ct_set_zone_timeout_policy, > ct_del_zone_timeout_policy, > + ct_zone_limit_update, > }; > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index d8e0cd37a..b863dd6fc 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -284,6 +284,11 @@ struct dpif_backer { > feature than 'bt_support'. */ > > struct atomic_count tnl_count; > + > + struct ovs_list ct_zone_limits_to_add; /* CT zone limits queued for > + * addition into datapath. */ > + struct ovs_list ct_zone_limits_to_del; /* CT zone limt queued for > + * deletion from datapath. */ > }; > > /* All existing ofproto_backer instances, indexed by ofproto->up.type. */ > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > index 9f7b8b6e8..5604aa65b 100644 > --- a/ofproto/ofproto-provider.h > +++ b/ofproto/ofproto-provider.h > @@ -1921,6 +1921,11 @@ 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); > + > + /* Queues the CT zone limit update. In order for this change to take > + * effect it needs to be commited. */ > + void (*ct_zone_limit_update)(const char *dp_type, uint16_t zone, > + int64_t *limit); > }; > > extern const struct ofproto_class ofproto_dpif_class; > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index e78c80d11..e054753b8 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -1026,6 +1026,18 @@ ofproto_ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone_id) > > } > > +void > +ofproto_ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, > + int64_t *limit) > +{ > + datapath_type = ofproto_normalize_type(datapath_type); > + const struct ofproto_class *class = ofproto_class_find__(datapath_type); > + > + if (class && class->ct_zone_limit_update) { > + class->ct_zone_limit_update(datapath_type, zone_id, limit); > + } > +} > + > > /* Spanning Tree Protocol (STP) configuration. */ > > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h > index 8efdb20a0..700a6f087 100644 > --- a/ofproto/ofproto.h > +++ b/ofproto/ofproto.h > @@ -384,6 +384,8 @@ void ofproto_ct_set_zone_timeout_policy(const char *datapath_type, > struct simap *timeout_policy); > void ofproto_ct_del_zone_timeout_policy(const char *datapath_type, > uint16_t zone); > +void ofproto_ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, > + int64_t *limit); > void ofproto_get_datapath_cap(const char *datapath_type, > struct smap *dp_cap); > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index 418cd32fe..537db66e0 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -5195,9 +5195,53 @@ udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10. > udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 > ]) > > +dnl Test limit set via database. > +VSCTL_ADD_DATAPATH_TABLE() > + > +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=0]) I wouldn't block the series over this since the rest of the test assumes that zone=0 is okay, but I think a good follow up would be to rewrite to not use zone=0 in the system-traffic tests. Zone 0 is the default zone, and can possibly get polluted on some systems by other entries, so it probably is best to avoid it in the future. > +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=3]) > + > +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=3]) > +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl > +default limit=10 > +zone=0,limit=5,count=0 > +]) > + > +AT_CHECK([ovs-vsctl add-zone-limit $DP_TYPE zone=0 limit=3]) > +AT_CHECK([ovs-vsctl add-zone-limit $DP_TYPE zone=3 limit=3]) > + > +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl > +default limit=10 > +zone=0,limit=3,count=0 > +zone=3,limit=3,count=0]) > + > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000200080000 actions=resubmit(,0)"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000300080000 actions=resubmit(,0)"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000400080000 actions=resubmit(,0)"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000500080000 actions=resubmit(,0)"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000600080000 actions=resubmit(,0)"]) > + > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.3," | sort ], [0], [dnl > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=2),reply=(src=10.1.1.4,dst=10.1.1.3,sport=2,dport=1),zone=3 > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10.1.1.3,sport=3,dport=1),zone=3 > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 > +]) > + > +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl > +default limit=10 > +zone=0,limit=3,count=0 > +zone=3,limit=3,count=3 > +]) > + > +AT_CHECK([ovs-vsctl del-zone-limit $DP_TYPE zone=3]) > +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl > +default limit=10 > +zone=0,limit=3,count=0]) > + > OVS_TRAFFIC_VSWITCHD_STOP(["dnl > /could not create datapath/d > -/(Cannot allocate memory) on packet/d"]) > +/(Cannot allocate memory) on packet/d > +/failed to .* timeout policy/d"]) > AT_CLEANUP > > AT_SETUP([FTP - no conntrack]) > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index e9110c1d8..6cd09e145 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -157,6 +157,7 @@ struct aa_mapping { > /* Internal representation of conntrack zone configuration table in OVSDB. */ > struct ct_zone { > uint16_t zone_id; > + int64_t limit; /* Limit of allowed entries. */ > struct simap tp; /* A map from timeout policy attribute to > * timeout value. */ > struct hmap_node node; /* Node in 'struct datapath' 'ct_zones' > @@ -756,6 +757,13 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) > ofproto_ct_set_zone_timeout_policy(dp->type, ct_zone->zone_id, > &ct_zone->tp); > } > + > + int64_t desired_limit = zone_cfg->limit ? *zone_cfg->limit : -1; > + if (ct_zone->limit != desired_limit) { > + ofproto_ct_zone_limit_update(dp->type, zone_id, zone_cfg->limit); > + } > + > + ct_zone->limit = desired_limit; > ct_zone->last_used = idl_seqno; > } > > @@ -763,6 +771,7 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) > HMAP_FOR_EACH_SAFE (ct_zone, node, &dp->ct_zones) { > if (ct_zone->last_used != idl_seqno) { > ofproto_ct_del_zone_timeout_policy(dp->type, ct_zone->zone_id); > + ofproto_ct_zone_limit_update(dp->type, ct_zone->zone_id, NULL); > ct_zone_remove_and_destroy(dp, ct_zone); > } > }
On Thu, Oct 5, 2023 at 2:49 PM Aaron Conole <aconole@redhat.com> wrote: > Ales Musil <amusil@redhat.com> writes: > > > Progpagate the CT limit that is present in the DB into > > datapath. The limit is currently only propagated on change > > and can be overwritten by the dpctl commands. > > > > Signed-off-by: Ales Musil <amusil@redhat.com> > > Acked-by: Simon Horman <horms@ovn.org> > > --- > > v3: Rebase on top of current master. > > Add ack from Simon and fix the missing '.'. > > Revert the unrelated change. > > --- > > ofproto/ofproto-dpif.c | 39 ++++++++++++++++++++++++++++++++ > > ofproto/ofproto-dpif.h | 5 +++++ > > ofproto/ofproto-provider.h | 5 +++++ > > ofproto/ofproto.c | 12 ++++++++++ > > ofproto/ofproto.h | 2 ++ > > tests/system-traffic.at | 46 +++++++++++++++++++++++++++++++++++++- > > vswitchd/bridge.c | 9 ++++++++ > > 7 files changed, 117 insertions(+), 1 deletion(-) > > > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > > index ba5706f6a..55eaeefa3 100644 > > --- a/ofproto/ofproto-dpif.c > > +++ b/ofproto/ofproto-dpif.c > > @@ -220,6 +220,7 @@ static void ofproto_unixctl_init(void); > > static void ct_zone_config_init(struct dpif_backer *backer); > > static void ct_zone_config_uninit(struct dpif_backer *backer); > > static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer); > > +static void ct_zone_limits_commit(struct dpif_backer *backer); > > > > static inline struct ofproto_dpif * > > ofproto_dpif_cast(const struct ofproto *ofproto) > > @@ -513,6 +514,7 @@ type_run(const char *type) > > > > process_dpif_port_changes(backer); > > ct_zone_timeout_policy_sweep(backer); > > + ct_zone_limits_commit(backer); > > > > return 0; > > } > > @@ -5522,6 +5524,8 @@ ct_zone_config_init(struct dpif_backer *backer) > > cmap_init(&backer->ct_zones); > > hmap_init(&backer->ct_tps); > > ovs_list_init(&backer->ct_tp_kill_list); > > + ovs_list_init(&backer->ct_zone_limits_to_add); > > + ovs_list_init(&backer->ct_zone_limits_to_del); > > clear_existing_ct_timeout_policies(backer); > > } > > > > @@ -5545,6 +5549,8 @@ ct_zone_config_uninit(struct dpif_backer *backer) > > id_pool_destroy(backer->tp_ids); > > cmap_destroy(&backer->ct_zones); > > hmap_destroy(&backer->ct_tps); > > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add); > > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del); > > } > > > > static void > > @@ -5625,6 +5631,38 @@ ct_del_zone_timeout_policy(const char > *datapath_type, uint16_t zone_id) > > } > > } > > > > +static void > > +ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, > > + int64_t *limit) > > +{ > > + struct dpif_backer *backer = shash_find_data(&all_dpif_backers, > > + datapath_type); > > + if (!backer) { > > + return; > > + } > > + > > + if (limit) { > > + ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_add, zone_id, > > + *limit, 0); > > + } else { > > + ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_del, > zone_id, 0, 0); > > + } > > +} > > + > > +static void > > +ct_zone_limits_commit(struct dpif_backer *backer) > > +{ > > + if (!ovs_list_is_empty(&backer->ct_zone_limits_to_add)) { > > + ct_dpif_set_limits(backer->dpif, NULL, > &backer->ct_zone_limits_to_add); > > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add); > > + } > > + > > + if (!ovs_list_is_empty(&backer->ct_zone_limits_to_del)) { > > + ct_dpif_del_limits(backer->dpif, > &backer->ct_zone_limits_to_del); > > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del); > > + } > > +} > > + > > static void > > get_datapath_cap(const char *datapath_type, struct smap *cap) > > { > > @@ -6914,4 +6952,5 @@ const struct ofproto_class ofproto_dpif_class = { > > ct_flush, /* ct_flush */ > > ct_set_zone_timeout_policy, > > ct_del_zone_timeout_policy, > > + ct_zone_limit_update, > > }; > > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > > index d8e0cd37a..b863dd6fc 100644 > > --- a/ofproto/ofproto-dpif.h > > +++ b/ofproto/ofproto-dpif.h > > @@ -284,6 +284,11 @@ struct dpif_backer { > > feature than > 'bt_support'. */ > > > > struct atomic_count tnl_count; > > + > > + struct ovs_list ct_zone_limits_to_add; /* CT zone limits queued for > > + * addition into datapath. > */ > > + struct ovs_list ct_zone_limits_to_del; /* CT zone limt queued for > > + * deletion from datapath. > */ > > }; > > > > /* All existing ofproto_backer instances, indexed by ofproto->up.type. > */ > > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > > index 9f7b8b6e8..5604aa65b 100644 > > --- a/ofproto/ofproto-provider.h > > +++ b/ofproto/ofproto-provider.h > > @@ -1921,6 +1921,11 @@ 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); > > + > > + /* Queues the CT zone limit update. In order for this change to take > > + * effect it needs to be commited. */ > > + void (*ct_zone_limit_update)(const char *dp_type, uint16_t zone, > > + int64_t *limit); > > }; > > > > extern const struct ofproto_class ofproto_dpif_class; > > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > > index e78c80d11..e054753b8 100644 > > --- a/ofproto/ofproto.c > > +++ b/ofproto/ofproto.c > > @@ -1026,6 +1026,18 @@ ofproto_ct_del_zone_timeout_policy(const char > *datapath_type, uint16_t zone_id) > > > > } > > > > +void > > +ofproto_ct_zone_limit_update(const char *datapath_type, uint16_t > zone_id, > > + int64_t *limit) > > +{ > > + datapath_type = ofproto_normalize_type(datapath_type); > > + const struct ofproto_class *class = > ofproto_class_find__(datapath_type); > > + > > + if (class && class->ct_zone_limit_update) { > > + class->ct_zone_limit_update(datapath_type, zone_id, limit); > > + } > > +} > > + > > > > /* Spanning Tree Protocol (STP) configuration. */ > > > > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h > > index 8efdb20a0..700a6f087 100644 > > --- a/ofproto/ofproto.h > > +++ b/ofproto/ofproto.h > > @@ -384,6 +384,8 @@ void ofproto_ct_set_zone_timeout_policy(const char > *datapath_type, > > struct simap *timeout_policy); > > void ofproto_ct_del_zone_timeout_policy(const char *datapath_type, > > uint16_t zone); > > +void ofproto_ct_zone_limit_update(const char *datapath_type, uint16_t > zone_id, > > + int64_t *limit); > > void ofproto_get_datapath_cap(const char *datapath_type, > > struct smap *dp_cap); > > > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index 418cd32fe..537db66e0 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -5195,9 +5195,53 @@ > udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10. > > > udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 > > ]) > > > > +dnl Test limit set via database. > > +VSCTL_ADD_DATAPATH_TABLE() > > + > > +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=0]) > > I wouldn't block the series over this since the rest of the test assumes > that zone=0 is okay, but I think a good follow up would be to rewrite to > not use zone=0 in the system-traffic tests. Zone 0 is the default zone, > and can possibly get polluted on some systems by other entries, so it > probably is best to avoid it in the future. > That's fair, once this is merged I can do a follow up patch that will change zone 0 to something else. > > > +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=3]) > > + > > +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=3]) > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl > > +default limit=10 > > +zone=0,limit=5,count=0 > > +]) > > + > > +AT_CHECK([ovs-vsctl add-zone-limit $DP_TYPE zone=0 limit=3]) > > +AT_CHECK([ovs-vsctl add-zone-limit $DP_TYPE zone=3 limit=3]) > > + > > +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl > > +default limit=10 > > +zone=0,limit=3,count=0 > > +zone=3,limit=3,count=0]) > > + > > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 > packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000200080000 > actions=resubmit(,0)"]) > > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 > packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000300080000 > actions=resubmit(,0)"]) > > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 > packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000400080000 > actions=resubmit(,0)"]) > > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 > packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000500080000 > actions=resubmit(,0)"]) > > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 > packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000600080000 > actions=resubmit(,0)"]) > > + > > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep > "orig=.src=10\.1\.1\.3," | sort ], [0], [dnl > > > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=2),reply=(src=10.1.1.4,dst=10.1.1.3,sport=2,dport=1),zone=3 > > > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10.1.1.3,sport=3,dport=1),zone=3 > > > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 > > +]) > > + > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl > > +default limit=10 > > +zone=0,limit=3,count=0 > > +zone=3,limit=3,count=3 > > +]) > > + > > +AT_CHECK([ovs-vsctl del-zone-limit $DP_TYPE zone=3]) > > +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl > > +default limit=10 > > +zone=0,limit=3,count=0]) > > + > > OVS_TRAFFIC_VSWITCHD_STOP(["dnl > > /could not create datapath/d > > -/(Cannot allocate memory) on packet/d"]) > > +/(Cannot allocate memory) on packet/d > > +/failed to .* timeout policy/d"]) > > AT_CLEANUP > > > > AT_SETUP([FTP - no conntrack]) > > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > > index e9110c1d8..6cd09e145 100644 > > --- a/vswitchd/bridge.c > > +++ b/vswitchd/bridge.c > > @@ -157,6 +157,7 @@ struct aa_mapping { > > /* Internal representation of conntrack zone configuration table in > OVSDB. */ > > struct ct_zone { > > uint16_t zone_id; > > + int64_t limit; /* Limit of allowed entries. */ > > struct simap tp; /* A map from timeout policy attribute > to > > * timeout value. */ > > struct hmap_node node; /* Node in 'struct datapath' 'ct_zones' > > @@ -756,6 +757,13 @@ ct_zones_reconfigure(struct datapath *dp, struct > ovsrec_datapath *dp_cfg) > > ofproto_ct_set_zone_timeout_policy(dp->type, > ct_zone->zone_id, > > &ct_zone->tp); > > } > > + > > + int64_t desired_limit = zone_cfg->limit ? *zone_cfg->limit : -1; > > + if (ct_zone->limit != desired_limit) { > > + ofproto_ct_zone_limit_update(dp->type, zone_id, > zone_cfg->limit); > > + } > > + > > + ct_zone->limit = desired_limit; > > ct_zone->last_used = idl_seqno; > > } > > > > @@ -763,6 +771,7 @@ ct_zones_reconfigure(struct datapath *dp, struct > ovsrec_datapath *dp_cfg) > > HMAP_FOR_EACH_SAFE (ct_zone, node, &dp->ct_zones) { > > if (ct_zone->last_used != idl_seqno) { > > ofproto_ct_del_zone_timeout_policy(dp->type, > ct_zone->zone_id); > > + ofproto_ct_zone_limit_update(dp->type, ct_zone->zone_id, > NULL); > > ct_zone_remove_and_destroy(dp, ct_zone); > > } > > } > > Thanks, Ales
On 10/2/23 12:33, Ales Musil wrote: > Progpagate the CT limit that is present in the DB into > datapath. The limit is currently only propagated on change > and can be overwritten by the dpctl commands. > > Signed-off-by: Ales Musil <amusil@redhat.com> > Acked-by: Simon Horman <horms@ovn.org> > --- > v3: Rebase on top of current master. > Add ack from Simon and fix the missing '.'. > Revert the unrelated change. > --- > ofproto/ofproto-dpif.c | 39 ++++++++++++++++++++++++++++++++ > ofproto/ofproto-dpif.h | 5 +++++ > ofproto/ofproto-provider.h | 5 +++++ > ofproto/ofproto.c | 12 ++++++++++ > ofproto/ofproto.h | 2 ++ > tests/system-traffic.at | 46 +++++++++++++++++++++++++++++++++++++- > vswitchd/bridge.c | 9 ++++++++ > 7 files changed, 117 insertions(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index ba5706f6a..55eaeefa3 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -220,6 +220,7 @@ static void ofproto_unixctl_init(void); > static void ct_zone_config_init(struct dpif_backer *backer); > static void ct_zone_config_uninit(struct dpif_backer *backer); > static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer); > +static void ct_zone_limits_commit(struct dpif_backer *backer); > > static inline struct ofproto_dpif * > ofproto_dpif_cast(const struct ofproto *ofproto) > @@ -513,6 +514,7 @@ type_run(const char *type) > > process_dpif_port_changes(backer); > ct_zone_timeout_policy_sweep(backer); > + ct_zone_limits_commit(backer); > > return 0; > } > @@ -5522,6 +5524,8 @@ ct_zone_config_init(struct dpif_backer *backer) > cmap_init(&backer->ct_zones); > hmap_init(&backer->ct_tps); > ovs_list_init(&backer->ct_tp_kill_list); > + ovs_list_init(&backer->ct_zone_limits_to_add); > + ovs_list_init(&backer->ct_zone_limits_to_del); > clear_existing_ct_timeout_policies(backer); > } > > @@ -5545,6 +5549,8 @@ ct_zone_config_uninit(struct dpif_backer *backer) > id_pool_destroy(backer->tp_ids); > cmap_destroy(&backer->ct_zones); > hmap_destroy(&backer->ct_tps); > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add); > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del); > } > > static void > @@ -5625,6 +5631,38 @@ ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone_id) > } > } > > +static void > +ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, > + int64_t *limit) > +{ > + struct dpif_backer *backer = shash_find_data(&all_dpif_backers, > + datapath_type); > + if (!backer) { > + return; > + } > + > + if (limit) { > + ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_add, zone_id, > + *limit, 0); > + } else { > + ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_del, zone_id, 0, 0); > + } > +} > + > +static void > +ct_zone_limits_commit(struct dpif_backer *backer) > +{ > + if (!ovs_list_is_empty(&backer->ct_zone_limits_to_add)) { > + ct_dpif_set_limits(backer->dpif, NULL, &backer->ct_zone_limits_to_add); > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add); > + } > + > + if (!ovs_list_is_empty(&backer->ct_zone_limits_to_del)) { > + ct_dpif_del_limits(backer->dpif, &backer->ct_zone_limits_to_del); > + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del); > + } > +} > + > static void > get_datapath_cap(const char *datapath_type, struct smap *cap) > { > @@ -6914,4 +6952,5 @@ const struct ofproto_class ofproto_dpif_class = { > ct_flush, /* ct_flush */ > ct_set_zone_timeout_policy, > ct_del_zone_timeout_policy, > + ct_zone_limit_update, > }; > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index d8e0cd37a..b863dd6fc 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -284,6 +284,11 @@ struct dpif_backer { > feature than 'bt_support'. */ > > struct atomic_count tnl_count; > + > + struct ovs_list ct_zone_limits_to_add; /* CT zone limits queued for > + * addition into datapath. */ > + struct ovs_list ct_zone_limits_to_del; /* CT zone limt queued for > + * deletion from datapath. */ > }; > > /* All existing ofproto_backer instances, indexed by ofproto->up.type. */ > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > index 9f7b8b6e8..5604aa65b 100644 > --- a/ofproto/ofproto-provider.h > +++ b/ofproto/ofproto-provider.h > @@ -1921,6 +1921,11 @@ 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); > + > + /* Queues the CT zone limit update. In order for this change to take > + * effect it needs to be commited. */ > + void (*ct_zone_limit_update)(const char *dp_type, uint16_t zone, > + int64_t *limit); > }; > > extern const struct ofproto_class ofproto_dpif_class; > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index e78c80d11..e054753b8 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -1026,6 +1026,18 @@ ofproto_ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone_id) > > } > > +void > +ofproto_ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, > + int64_t *limit) > +{ > + datapath_type = ofproto_normalize_type(datapath_type); > + const struct ofproto_class *class = ofproto_class_find__(datapath_type); > + > + if (class && class->ct_zone_limit_update) { > + class->ct_zone_limit_update(datapath_type, zone_id, limit); > + } > +} > + > > /* Spanning Tree Protocol (STP) configuration. */ > > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h > index 8efdb20a0..700a6f087 100644 > --- a/ofproto/ofproto.h > +++ b/ofproto/ofproto.h > @@ -384,6 +384,8 @@ void ofproto_ct_set_zone_timeout_policy(const char *datapath_type, > struct simap *timeout_policy); > void ofproto_ct_del_zone_timeout_policy(const char *datapath_type, > uint16_t zone); > +void ofproto_ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, > + int64_t *limit); > void ofproto_get_datapath_cap(const char *datapath_type, > struct smap *dp_cap); > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index 418cd32fe..537db66e0 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -5195,9 +5195,53 @@ udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10. > udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 > ]) > > +dnl Test limit set via database. > +VSCTL_ADD_DATAPATH_TABLE() > + > +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=0]) > +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=3]) > + > +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=3]) > +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl > +default limit=10 > +zone=0,limit=5,count=0 > +]) > + > +AT_CHECK([ovs-vsctl add-zone-limit $DP_TYPE zone=0 limit=3]) > +AT_CHECK([ovs-vsctl add-zone-limit $DP_TYPE zone=3 limit=3]) > + > +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl > +default limit=10 > +zone=0,limit=3,count=0 > +zone=3,limit=3,count=0]) > + > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000200080000 actions=resubmit(,0)"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000300080000 actions=resubmit(,0)"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000400080000 actions=resubmit(,0)"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000500080000 actions=resubmit(,0)"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000600080000 actions=resubmit(,0)"]) > + > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.3," | sort ], [0], [dnl > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=2),reply=(src=10.1.1.4,dst=10.1.1.3,sport=2,dport=1),zone=3 > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10.1.1.3,sport=3,dport=1),zone=3 > +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 > +]) > + > +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl > +default limit=10 > +zone=0,limit=3,count=0 > +zone=3,limit=3,count=3 > +]) > + > +AT_CHECK([ovs-vsctl del-zone-limit $DP_TYPE zone=3]) > +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl > +default limit=10 > +zone=0,limit=3,count=0]) > + > OVS_TRAFFIC_VSWITCHD_STOP(["dnl > /could not create datapath/d > -/(Cannot allocate memory) on packet/d"]) > +/(Cannot allocate memory) on packet/d > +/failed to .* timeout policy/d"]) The fact that we have a timeout policy error in a pure zone limit test is not good. See below. > AT_CLEANUP > > AT_SETUP([FTP - no conntrack]) > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index e9110c1d8..6cd09e145 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -157,6 +157,7 @@ struct aa_mapping { > /* Internal representation of conntrack zone configuration table in OVSDB. */ > struct ct_zone { > uint16_t zone_id; > + int64_t limit; /* Limit of allowed entries. */ Please, indent the comment to the level of other ones. > struct simap tp; /* A map from timeout policy attribute to > * timeout value. */ > struct hmap_node node; /* Node in 'struct datapath' 'ct_zones' > @@ -756,6 +757,13 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) > ofproto_ct_set_zone_timeout_policy(dp->type, ct_zone->zone_id, > &ct_zone->tp); > } > + > + int64_t desired_limit = zone_cfg->limit ? *zone_cfg->limit : -1; > + if (ct_zone->limit != desired_limit) { > + ofproto_ct_zone_limit_update(dp->type, zone_id, zone_cfg->limit); > + } So, on creation we only update the limit if it is set in the database.... > + > + ct_zone->limit = desired_limit; > ct_zone->last_used = idl_seqno; > } > > @@ -763,6 +771,7 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) > HMAP_FOR_EACH_SAFE (ct_zone, node, &dp->ct_zones) { > if (ct_zone->last_used != idl_seqno) { > ofproto_ct_del_zone_timeout_policy(dp->type, ct_zone->zone_id); > + ofproto_ct_zone_limit_update(dp->type, ct_zone->zone_id, NULL); ....but on removal we always remove it, even if it wasn't set. The same goes for timeout policies. Before this patch, existence of the CT_Zone row was tied to existance of the timeout policy in that row. It's not he case anymore. Now we need to check what exactly changed and only add what changed and remove what wasn't set. This will save us from clearing configuration from the kernel that was never in the database, and will avoid the error in the log. Best regards, Ilya Maximets.
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index ba5706f6a..55eaeefa3 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -220,6 +220,7 @@ static void ofproto_unixctl_init(void); static void ct_zone_config_init(struct dpif_backer *backer); static void ct_zone_config_uninit(struct dpif_backer *backer); static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer); +static void ct_zone_limits_commit(struct dpif_backer *backer); static inline struct ofproto_dpif * ofproto_dpif_cast(const struct ofproto *ofproto) @@ -513,6 +514,7 @@ type_run(const char *type) process_dpif_port_changes(backer); ct_zone_timeout_policy_sweep(backer); + ct_zone_limits_commit(backer); return 0; } @@ -5522,6 +5524,8 @@ ct_zone_config_init(struct dpif_backer *backer) cmap_init(&backer->ct_zones); hmap_init(&backer->ct_tps); ovs_list_init(&backer->ct_tp_kill_list); + ovs_list_init(&backer->ct_zone_limits_to_add); + ovs_list_init(&backer->ct_zone_limits_to_del); clear_existing_ct_timeout_policies(backer); } @@ -5545,6 +5549,8 @@ ct_zone_config_uninit(struct dpif_backer *backer) id_pool_destroy(backer->tp_ids); cmap_destroy(&backer->ct_zones); hmap_destroy(&backer->ct_tps); + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add); + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del); } static void @@ -5625,6 +5631,38 @@ ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone_id) } } +static void +ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, + int64_t *limit) +{ + struct dpif_backer *backer = shash_find_data(&all_dpif_backers, + datapath_type); + if (!backer) { + return; + } + + if (limit) { + ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_add, zone_id, + *limit, 0); + } else { + ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_del, zone_id, 0, 0); + } +} + +static void +ct_zone_limits_commit(struct dpif_backer *backer) +{ + if (!ovs_list_is_empty(&backer->ct_zone_limits_to_add)) { + ct_dpif_set_limits(backer->dpif, NULL, &backer->ct_zone_limits_to_add); + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add); + } + + if (!ovs_list_is_empty(&backer->ct_zone_limits_to_del)) { + ct_dpif_del_limits(backer->dpif, &backer->ct_zone_limits_to_del); + ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del); + } +} + static void get_datapath_cap(const char *datapath_type, struct smap *cap) { @@ -6914,4 +6952,5 @@ const struct ofproto_class ofproto_dpif_class = { ct_flush, /* ct_flush */ ct_set_zone_timeout_policy, ct_del_zone_timeout_policy, + ct_zone_limit_update, }; diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index d8e0cd37a..b863dd6fc 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -284,6 +284,11 @@ struct dpif_backer { feature than 'bt_support'. */ struct atomic_count tnl_count; + + struct ovs_list ct_zone_limits_to_add; /* CT zone limits queued for + * addition into datapath. */ + struct ovs_list ct_zone_limits_to_del; /* CT zone limt queued for + * deletion from datapath. */ }; /* All existing ofproto_backer instances, indexed by ofproto->up.type. */ diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 9f7b8b6e8..5604aa65b 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -1921,6 +1921,11 @@ 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); + + /* Queues the CT zone limit update. In order for this change to take + * effect it needs to be commited. */ + void (*ct_zone_limit_update)(const char *dp_type, uint16_t zone, + int64_t *limit); }; extern const struct ofproto_class ofproto_dpif_class; diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index e78c80d11..e054753b8 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1026,6 +1026,18 @@ ofproto_ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone_id) } +void +ofproto_ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, + int64_t *limit) +{ + datapath_type = ofproto_normalize_type(datapath_type); + const struct ofproto_class *class = ofproto_class_find__(datapath_type); + + if (class && class->ct_zone_limit_update) { + class->ct_zone_limit_update(datapath_type, zone_id, limit); + } +} + /* Spanning Tree Protocol (STP) configuration. */ diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 8efdb20a0..700a6f087 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -384,6 +384,8 @@ void ofproto_ct_set_zone_timeout_policy(const char *datapath_type, struct simap *timeout_policy); void ofproto_ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone); +void ofproto_ct_zone_limit_update(const char *datapath_type, uint16_t zone_id, + int64_t *limit); void ofproto_get_datapath_cap(const char *datapath_type, struct smap *dp_cap); diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 418cd32fe..537db66e0 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -5195,9 +5195,53 @@ udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10. udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 ]) +dnl Test limit set via database. +VSCTL_ADD_DATAPATH_TABLE() + +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=0]) +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=3]) + +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=3]) +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl +default limit=10 +zone=0,limit=5,count=0 +]) + +AT_CHECK([ovs-vsctl add-zone-limit $DP_TYPE zone=0 limit=3]) +AT_CHECK([ovs-vsctl add-zone-limit $DP_TYPE zone=3 limit=3]) + +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl +default limit=10 +zone=0,limit=3,count=0 +zone=3,limit=3,count=0]) + +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000200080000 actions=resubmit(,0)"]) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000300080000 actions=resubmit(,0)"]) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000400080000 actions=resubmit(,0)"]) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000500080000 actions=resubmit(,0)"]) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4c90a0101030a0101040001000600080000 actions=resubmit(,0)"]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.3," | sort ], [0], [dnl +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=2),reply=(src=10.1.1.4,dst=10.1.1.3,sport=2,dport=1),zone=3 +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10.1.1.3,sport=3,dport=1),zone=3 +udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 +]) + +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl +default limit=10 +zone=0,limit=3,count=0 +zone=3,limit=3,count=3 +]) + +AT_CHECK([ovs-vsctl del-zone-limit $DP_TYPE zone=3]) +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl +default limit=10 +zone=0,limit=3,count=0]) + OVS_TRAFFIC_VSWITCHD_STOP(["dnl /could not create datapath/d -/(Cannot allocate memory) on packet/d"]) +/(Cannot allocate memory) on packet/d +/failed to .* timeout policy/d"]) AT_CLEANUP AT_SETUP([FTP - no conntrack]) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index e9110c1d8..6cd09e145 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -157,6 +157,7 @@ struct aa_mapping { /* Internal representation of conntrack zone configuration table in OVSDB. */ struct ct_zone { uint16_t zone_id; + int64_t limit; /* Limit of allowed entries. */ struct simap tp; /* A map from timeout policy attribute to * timeout value. */ struct hmap_node node; /* Node in 'struct datapath' 'ct_zones' @@ -756,6 +757,13 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) ofproto_ct_set_zone_timeout_policy(dp->type, ct_zone->zone_id, &ct_zone->tp); } + + int64_t desired_limit = zone_cfg->limit ? *zone_cfg->limit : -1; + if (ct_zone->limit != desired_limit) { + ofproto_ct_zone_limit_update(dp->type, zone_id, zone_cfg->limit); + } + + ct_zone->limit = desired_limit; ct_zone->last_used = idl_seqno; } @@ -763,6 +771,7 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) HMAP_FOR_EACH_SAFE (ct_zone, node, &dp->ct_zones) { if (ct_zone->last_used != idl_seqno) { ofproto_ct_del_zone_timeout_policy(dp->type, ct_zone->zone_id); + ofproto_ct_zone_limit_update(dp->type, ct_zone->zone_id, NULL); ct_zone_remove_and_destroy(dp, ct_zone); } }