On 2/18/24 16:40, Cheng Li wrote:
Ovs ko passes odp key and packet to userspace. Next packet is
extracted into flow, which is the input for xlate to generate wc.
At last, ukey(= odp_key/wc) is installed into datapath. If the
odp_key is not consistant with packet extracted flow. The ukey
will be wrong.

commit [1] was created to fix inconsistance caused by bad tcp
header. commit [2] was cretaed to fix inconsistance caused by bad
ip header. There is no guarantee of the consistance of odp_key and
packet flow. So it is necessary to make the check to prevent from
installing wrong ukey.

[1] 1f5749c790accd98dbcafdaefc40bf5e52d7c672
[2] 79349cbab0b2a755140eedb91833ad2760520a83

Signed-off-by: Cheng Li <[email protected]>


Hi Cheng,

Do you have a concrete example of a key / kernel version / OVS version that would generate an inconsistency that is not caught by the call to odp_flow_key_to_flow()?

Thanks.
Adrián

---
  ofproto/ofproto-dpif-upcall.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index b5cbeed87..6e46e5a5a 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -66,6 +66,7 @@ COVERAGE_DEFINE(upcall_flow_limit_reduced);
  COVERAGE_DEFINE(upcall_flow_limit_scaled);
  COVERAGE_DEFINE(upcall_ukey_contention);
  COVERAGE_DEFINE(upcall_ukey_replace);
+COVERAGE_DEFINE(upcall_packet_flow_inconsistant);
/* A thread that reads upcalls from dpif, forwards each upcall's packet,
   * and possibly sets up a kernel flow as a cache. */
@@ -840,6 +841,8 @@ recv_upcalls(struct handler *handler)
      struct dpif_upcall dupcalls[UPCALL_MAX_BATCH];
      struct upcall upcalls[UPCALL_MAX_BATCH];
      struct flow flows[UPCALL_MAX_BATCH];
+    struct flow odp_flow;
+    struct flow_wildcards flow_wc;
      size_t n_upcalls, i;
n_upcalls = 0;
@@ -903,8 +906,17 @@ recv_upcalls(struct handler *handler)
          upcall->out_tun_key = dupcall->out_tun_key;
          upcall->actions = dupcall->actions;
+ /* Save odp flow before overwrite. */
+        memcpy(&odp_flow, flow, sizeof flow);
          pkt_metadata_from_flow(&dupcall->packet.md, flow);
          flow_extract(&dupcall->packet, flow);
+        flow_wildcards_init_for_packet(&flow_wc, &flow);
+        if (!flow_equal_except(&odp_flow, flow, &flow_wc)) {
+            /* If odp flow is not consistant with flow extract from packet,
+             * bad ukey/mask will be installed. */
+            COVERAGE_INC(upcall_packet_flow_inconsistant);
+            goto cleanup;
+        }
error = process_upcall(udpif, upcall,
                                 &upcall->odp_actions, &upcall->wc);

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to