mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-22 01:51:26 +00:00
Code Issues Releases Wiki Activity

ipfix: Trigger revalidation if ipfix options changes.

Browse Source 
ipfix cfg creation/deleting triggers revalidation. But this does
not cover the case where ipfix options changes, which also suppose
to trigger revalidation.

Fixes: a9f5ee1199e1 ("ofproto-dpif: Trigger revalidation when ipfix config set.")
Signed-off-by: lic121 <lic121@chinatelecom.cn>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
lic121 2022-04-30 04:47:03 +00:00 committed by Ilya Maximets
parent 4e1e1e189f
commit f1c51be500
4 changed files with 54 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
ofproto/ofproto-dpif-ipfix.c
View File

@ -926,17 +926,21 @@ dpif_ipfix_bridge_exporter_destroy(struct dpif_ipfix_bridge_exporter *exporter)
static void
dpif_ipfix_bridge_exporter_set_options(
struct dpif_ipfix_bridge_exporter *exporter,
const struct ofproto_ipfix_bridge_exporter_options *options)
const struct ofproto_ipfix_bridge_exporter_options *options,
bool *options_changed)
{
bool options_changed;
if (!options || sset_is_empty(&options->targets)) {
/* No point in doing any work if there are no targets. */
dpif_ipfix_bridge_exporter_clear(exporter);
if (exporter->options) {
dpif_ipfix_bridge_exporter_clear(exporter);
*options_changed = true;
} else {
*options_changed = false;
}
return;
}
options_changed = (
*options_changed = (
!exporter->options
|| !ofproto_ipfix_bridge_exporter_options_equal(
options, exporter->options));
@ -945,7 +949,7 @@ dpif_ipfix_bridge_exporter_set_options(
* shortchanged in collectors (which indicates that opening one or
* more of the configured collectors failed, so that we should
* retry). */
if (options_changed
if (*options_changed
|| collectors_count(exporter->exporter.collectors)
< sset_count(&options->targets)) {
if (!dpif_ipfix_exporter_set_options(
@ -957,7 +961,7 @@ dpif_ipfix_bridge_exporter_set_options(
}
/* Avoid reconfiguring if options didn't change. */
if (!options_changed) {
if (!*options_changed) {
return;
}
@ -1015,17 +1019,21 @@ dpif_ipfix_flow_exporter_destroy(struct dpif_ipfix_flow_exporter *exporter)
static bool
dpif_ipfix_flow_exporter_set_options(
struct dpif_ipfix_flow_exporter *exporter,
const struct ofproto_ipfix_flow_exporter_options *options)
const struct ofproto_ipfix_flow_exporter_options *options,
bool *options_changed)
{
bool options_changed;
if (sset_is_empty(&options->targets)) {
/* No point in doing any work if there are no targets. */
dpif_ipfix_flow_exporter_clear(exporter);
if (exporter->options) {
dpif_ipfix_flow_exporter_clear(exporter);
*options_changed = true;
} else {
*options_changed = false;
}
return true;
}
options_changed = (
*options_changed = (
!exporter->options
|| !ofproto_ipfix_flow_exporter_options_equal(
options, exporter->options));
@ -1034,7 +1042,7 @@ dpif_ipfix_flow_exporter_set_options(
* shortchanged in collectors (which indicates that opening one or
* more of the configured collectors failed, so that we should
* retry). */
if (options_changed
if (*options_changed
|| collectors_count(exporter->exporter.collectors)
< sset_count(&options->targets)) {
if (!dpif_ipfix_exporter_set_options(
@ -1046,7 +1054,7 @@ dpif_ipfix_flow_exporter_set_options(
}
/* Avoid reconfiguring if options didn't change. */
if (!options_changed) {
if (!*options_changed) {
return true;
}
@ -1069,7 +1077,7 @@ remove_flow_exporter(struct dpif_ipfix *di,
free(node);
}
void
bool
dpif_ipfix_set_options(
struct dpif_ipfix *di,
const struct ofproto_ipfix_bridge_exporter_options *bridge_exporter_options,
@ -1077,16 +1085,19 @@ dpif_ipfix_set_options(
size_t n_flow_exporters_options) OVS_EXCLUDED(mutex)
{
int i;
bool beo_changed, feo_changed, entry_changed;
struct ofproto_ipfix_flow_exporter_options *options;
struct dpif_ipfix_flow_exporter_map_node *node;
ovs_mutex_lock(&mutex);
dpif_ipfix_bridge_exporter_set_options(&di->bridge_exporter,
bridge_exporter_options);
bridge_exporter_options,
&beo_changed);
/* Add new flow exporters and update current flow exporters. */
options = (struct ofproto_ipfix_flow_exporter_options *)
flow_exporters_options;
feo_changed = false;
for (i = 0; i < n_flow_exporters_options; i++) {
node = dpif_ipfix_find_flow_exporter_map_node(
di, options->collector_set_id);
@ -1095,10 +1106,14 @@ dpif_ipfix_set_options(
dpif_ipfix_flow_exporter_init(&node->exporter);
hmap_insert(&di->flow_exporter_map, &node->node,
hash_int(options->collector_set_id, 0));
feo_changed = true;
}
if (!dpif_ipfix_flow_exporter_set_options(&node->exporter, options)) {
if (!dpif_ipfix_flow_exporter_set_options(&node->exporter,
options,
&entry_changed)) {
remove_flow_exporter(di, node);
}
feo_changed = entry_changed ? true : feo_changed;
options++;
}
@ -1117,10 +1132,12 @@ dpif_ipfix_set_options(
}
if (i == n_flow_exporters_options) { /* Not found. */
remove_flow_exporter(di, node);
feo_changed = true;
}
}
ovs_mutex_unlock(&mutex);
return beo_changed || feo_changed;
}
struct dpif_ipfix *

2
ofproto/ofproto-dpif-ipfix.h
View File

@ -48,7 +48,7 @@ bool dpif_ipfix_get_bridge_exporter_output_sampling(const struct dpif_ipfix *);
bool dpif_ipfix_get_flow_exporter_tunnel_sampling(const struct dpif_ipfix *,
const uint32_t);
bool dpif_ipfix_is_tunnel_port(const struct dpif_ipfix *, odp_port_t);
void dpif_ipfix_set_options(
bool dpif_ipfix_set_options(
struct dpif_ipfix *,
const struct ofproto_ipfix_bridge_exporter_options *,
const struct ofproto_ipfix_flow_exporter_options *, size_t);

7
ofproto/ofproto-dpif.c
View File

@ -2339,6 +2339,7 @@ set_ipfix(
struct dpif_ipfix *di = ofproto->ipfix;
bool has_options = bridge_exporter_options || flow_exporters_options;
bool new_di = false;
bool options_changed = false;
if (has_options && !di) {
di = ofproto->ipfix = dpif_ipfix_create();
@ -2348,7 +2349,7 @@ set_ipfix(
if (di) {
/* Call set_options in any case to cleanly flush the flow
* caches in the last exporters that are to be destroyed. */
dpif_ipfix_set_options(
options_changed = dpif_ipfix_set_options(
di, bridge_exporter_options, flow_exporters_options,
n_flow_exporters_options);
@ -2365,9 +2366,7 @@ set_ipfix(
ofproto->ipfix = NULL;
}
/* TODO: need to consider ipfix option changes more than
* enable/disable */
if (new_di || !ofproto->ipfix) {
if (new_di || options_changed) {
ofproto->backer->need_revalidate = REV_RECONFIGURE;
}
}

17
tests/ofproto-dpif.at
View File

@ -7629,13 +7629,28 @@ dnl configure bridge IPFIX and ensure that sample action generation works at the
dnl datapath level.
AT_SETUP([ofproto-dpif - Bridge IPFIX sanity check])
OVS_VSWITCHD_START
dnl first revalidation triggered by add interface
AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
1
])
add_of_ports br0 1 2 3
AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
2
])
dnl Sample every packet using bridge-based sampling.
AT_CHECK([ovs-vsctl -- set bridge br0 ipfix=@fix -- \
--id=@fix create ipfix targets=\"127.0.0.1:4739\" \
sampling=1], [0], [ignore])
sampling=2], [0], [ignore])
AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
3
])
AT_CHECK([ovs-vsctl set ipfix `ovs-vsctl get bridge br0 ipfix` sampling=1], [0])
AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
4
])
dnl Send some packets that should be sampled.
for i in `seq 1 3`; do
AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800)'])