On 1/6/23 20:15, Mark Michelson wrote:
For the code change itself,
Acked-by: Mark Michelson <[email protected]>
I have a note about the commit message.
On 12/19/22 11:18, Adrian Moreno wrote:
The default zero value can lead to sampling errors if the pipeline sets
an the input port to OFP_NONE during flow processing.
The commit message seems to contradict the code change. I'm guessing it should
be something more like:
"The default zero value can lead to sampling errors. To avoid this, the pipeline
sets the input port to OFP_NONE during flow processing."
The original version makes it sound like using OFP_NONE would cause sampling
errors. Does my updated language make more sense?
Sorry for the confusion. I think the double use of OFP_NONE makes this sentence
unreadable. What I meant by "if the pipeline sets an the input port to OFP_NONE
during flow processing" is that, sometimes OVN will decide to add an action that
sets the MFF_IN_PORT field to OFP_NONE (e.g: Table 64 in physical.c). If a
sample action with the default zero value is executed after OVN has set the
MFF_IN_PORT to OFP_NONE, the sampling fails.
But, to be completely fair, the default zero value is wrong in itself (as in, a
sample will be emitted with misleading data). So I could just rephrase it as:
"The default zero value would likely match an existing openflow port and end up
generating a sample with wrong output interface information. Since in this case
we're sampling in the middle of the pipeline, the correct value for sampling
port is OFP_NONE."
What do you think?
Fixes: a42c808f30b4 ("northd: add drop sampling")
Signed-off-by: Adrian Moreno <[email protected]>
---
controller/physical.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/controller/physical.c b/controller/physical.c
index 2444d3ebd..d715c2bfa 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -843,6 +843,7 @@ put_drop(const struct physical_debug *debug, uint8_t
table_id,
os->collector_set_id = debug->collector_set_id;
os->obs_domain_id = (debug->obs_domain_id << 24);
os->obs_point_id = table_id;
+ os->sampling_port = OFPP_NONE;
}
}
--
Adrián Moreno
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
