Hi Ben,

> -----Original Message-----
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, 11 July, 2017 23:46
> 
> What is your plan for handling ofpact_check__() with decap actions?
> 

In general a decap() action reveals an inner packet that requires reparsing 
before any subsequent actions can be applied. The only known exception is 
decap() on an Ethernet packet, which just changes the packet type to (1, 
ether_type). So for an Ethernet packet we could just change the flow's packet 
type accordingly.

For all other packet types, we should not accept any subsequent actions in an 
action list after decap().
We can't leave the packet_type unchanged, as that might allow false positives. 
Instead we should perhaps change the packet_type to an "undefined" value:

So here's my proposal:

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index de7c612..e0a6c7b 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -7814,7 +7814,7 @@ inconsistent_match(enum ofputil_protocol 
*usable_protocols)
     *usable_protocols &= OFPUTIL_P_OF10_ANY;
 }

-/* May modify flow_packet_type, flow->dl_type, flow->nw_proto and
+/* May modify flow->packet_type, flow->dl_type, flow->nw_proto and
  * flow->vlan_tci, caller must restore them.
  *
  * Modifies some actions, filling in fields that could not be properly set
@@ -8127,8 +8127,16 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, 
struct ofpact *a,
         return 0;

     case OFPACT_DECAP:
-        /* FIXME: The resulting packet_type is not known at flow deployment
-         * time. How can we allow actions with pre-requisites after decap? */
+        if (flow->packet_type == htonl(PT_ETH)) {
+            /* Adjust the packet_type to allow subsequent actions. */
+            flow->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,
+                                               ntohs(flow->dl_type));
+        } else {
+            /* The actual packet_type is only known after decapsulation.
+             * Do not allow subsequent actions that depend on packet headers. 
*/
+            flow->packet_type = htonl(PT_UNKNOWN);
+            flow->dl_type = OVS_BE16_MAX;
+        }
         return 0;

I haven't tested that yet, but I believe it should do the trick.

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

Reply via email to