On 1/20/22 11:31, Eelco Chaudron wrote:


On 20 Jan 2022, at 11:06, Adrian Moreno wrote:

On 1/20/22 10:13, Eelco Chaudron wrote:


On 17 Jan 2022, at 10:27, Adrian Moreno wrote:

We are currently requiring in_port to be a valid port number for ipfix
sampling even if the sampling is done on the output port (egress).

This restriction is not really needed and can affect pipelines that
intentionally set the in_port to OFPP_NONE during flow processing. For
instance, OVN does this, see:

cfa547821 Fix ovn-controller generated packets from getting dropped for
reject ACL action.

This patch skips ipfix sampling only if both (ingress and egress) ports
are invalid.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2016346
Signed-off-by: Adrian Moreno <amore...@redhat.com>

Adrian, the change looks good to me. Maybe you could add a test case for this 
specific configuration, i.e., ingress and egress only?


I agree we need a test. I'm not very familiar with the unit test system so I 
was looking into how to verify what datapath flows are being programmed. Any 
suggestions/pointers?

My first idea would be to use “ovs-appctl ofproto/trace” which will give you 
the megaflow and datapath actions, but the existing tests are probably better.
The “ofproto-dpif - Bridge IPFIX sanity check” (and the others) just use 
dpctl/dump-flows, guess you can use those as a baseline.


Thanks, I'll take a look.


Cheers,

Eelco

---
   ofproto/ofproto-dpif-xlate.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index cafcd014a..189276bc1 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3272,7 +3272,9 @@ compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t 
output_odp_port)
       struct dpif_ipfix *ipfix = ctx->xbridge->ipfix;
       odp_port_t tunnel_out_port = ODPP_NONE;

-    if (!ipfix || ctx->xin->flow.in_port.ofp_port == OFPP_NONE) {
+    if (!ipfix ||
+        (output_odp_port == ODPP_NONE &&
+         ctx->xin->flow.in_port.ofp_port == OFPP_NONE)) {
           return;
       }

--
2.34.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


--
Adrián Moreno


--
Adrián Moreno

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to