SDFAB-155 Allow forwarding of packet-outs like regular packets

p4src/include/header.p4

@@ -19,7 +19,8 @@ header packet_in_header_t {
 header packet_out_header_t {
     PortId_t          egress_port;
     CpuLoopbackMode_t cpu_loopback_mode;
-    @padding bit<85>  pad0;
+    bit<1>            do_forwarding;
+    @padding bit<84>  pad0;
     bit<16>           ether_type;
 }
 

p4src/include/parser.p4

@@ -43,7 +43,7 @@ parser FabricIngressParser (packet_in  packet,
         transition select(tmp.ether_type) {
             ETHERTYPE_CPU_LOOPBACK_INGRESS: parse_fake_ethernet;
             ETHERTYPE_CPU_LOOPBACK_EGRESS: parse_fake_ethernet_and_accept;
-            ETHERTYPE_PACKET_OUT: parse_packet_out;
+            ETHERTYPE_PACKET_OUT: check_packet_out;
             default: parse_ethernet;
         }
     }
@@ -59,12 +59,26 @@ parser FabricIngressParser (packet_in  packet,
         transition accept;
     }
 
-    state parse_packet_out {
+    state check_packet_out {
+        packet_out_header_t tmp = packet.lookahead<packet_out_header_t>();
+        transition select(tmp.do_forwarding) {
+            0: parse_packet_out_and_accept;
+            default: strip_packet_out;
+        }
+    }
+
+    state parse_packet_out_and_accept {
+        // Will transmit over requested egress port as-is. No need to parse further.
         packet.extract(hdr.packet_out);
-        // Will send to requested egress port as-is. No need to parse further.
         transition accept;
     }
 
+    state strip_packet_out {
+        // Remove packet-out header and process as a regular packet.
+        packet.advance(ETH_HDR_BYTES * 8);
+        transition parse_ethernet;
+    }
+
     state parse_ethernet {
         packet.extract(hdr.ethernet);
         transition select(packet.lookahead<bit<16>>()) {

ptf/tests/ptf/fabric.ptf/test.py

@@ -197,6 +197,41 @@ def runTestInternal(self, ip_dst, prefix_list):
                         )
 
 
+class FabricIPv4UnicastFromPacketOutTest(IPv4UnicastTest):
+    """Packet-outs should be routed like regular packets when setting
+    packet_out_header_t.do_forwarding=1
+    """
+    @tvsetup
+    @autocleanup
+    def doRunTest(self, pkt, mac_dest, tagged2, tc_name):
+        self.runIPv4UnicastTest(
+            pkt, mac_dest, tagged1=False, tagged2=tagged2, from_packet_out=True
+        )
+
+    def runTest(self):
+        print("")
+        # Cpu port is always considered untagged.
+        for tagged2 in [False, True]:
+            for pkt_type in BASE_PKT_TYPES | GTP_PKT_TYPES:
+                tc_name = pkt_type + "_VLAN_" + str(tagged2)
+                print(
+                    "Testing {} packet, out-tagged={}...".format(pkt_type, tagged2)
+                )
+                pkt = getattr(testutils, "simple_%s_packet" % pkt_type)(
+                    eth_src=ZERO_MAC,
+                    eth_dst=ZERO_MAC,
+                    ip_src=HOST1_IPV4,
+                    ip_dst=HOST2_IPV4,
+                    pktlen=MIN_PKT_LEN
+                )
+                self.doRunTest(
+                    pkt=pkt,
+                    mac_dest=HOST2_MAC,
+                    tagged2=tagged2,
+                    tc_name=tc_name,
+                )
+
+
 class FabricIPv4UnicastDefaultRouteTest(FabricIPv4UnicastTest):
     def runTest(self):
         self.runTestInternal(DEFAULT_ROUTE_IPV4, [PREFIX_DEFAULT_ROUTE])

ptf/tests/ptf/fabric_test.py

@@ -557,7 +557,7 @@ def tearDown(self):
         self.reset_switch_info()
         P4RuntimeTest.tearDown(self)
 
-    def build_packet_out(self, pkt, port, cpu_loopback_mode=CPU_LOOPBACK_MODE_DISABLED):
+    def build_packet_out(self, pkt, port, cpu_loopback_mode=CPU_LOOPBACK_MODE_DISABLED, do_forwarding=False):
         packet_out = p4runtime_pb2.PacketOut()
         packet_out.payload = bytes(pkt)
         # egress_port
@@ -568,13 +568,17 @@ def build_packet_out(self, pkt, port, cpu_loopback_mode=CPU_LOOPBACK_MODE_DISABL
         cpu_loopback_mode_md = packet_out.metadata.add()
         cpu_loopback_mode_md.metadata_id = 2
         cpu_loopback_mode_md.value = stringify(cpu_loopback_mode, 1)
+        # do_forwarding
+        do_forwarding_md = packet_out.metadata.add()
+        do_forwarding_md.metadata_id = 3
+        do_forwarding_md.value = stringify(1 if do_forwarding else 0, 1)
         # pad0
         pad0_md = packet_out.metadata.add()
-        pad0_md.metadata_id = 3
+        pad0_md.metadata_id = 4
         pad0_md.value = stringify(0, 1)
         # ether type
         ether_type_md = packet_out.metadata.add()
-        ether_type_md.metadata_id = 4
+        ether_type_md.metadata_id = 5
         ether_type_md.value = stringify(ETH_TYPE_PACKET_OUT, 2)
         return packet_out
 
@@ -1412,6 +1416,7 @@ def runIPv4UnicastTest(
         override_eg_port=None,
         port_type1=PORT_TYPE_EDGE,
         port_type2=PORT_TYPE_EDGE,
+        from_packet_out=False,
     ):
         """
         Execute an IPv4 unicast routing test.
@@ -1442,6 +1447,7 @@ def runIPv4UnicastTest(
         :param override_eg_port: to override the default or the provided eg port
         :param port_type1: port type to be used for the programming of the ig port
         :param port_type2: port type to be used for the programming of the eg port
+        :param from_packet_out: ingress packet is a packet-out (enables do_forwarding)
         """
         if IP not in pkt or Ether not in pkt:
             self.fail("Cannot do IPv4 test with packet that is not IP")
@@ -1451,13 +1457,18 @@ def runIPv4UnicastTest(
             ig_port = self.port1
         if eg_port is None:
             eg_port = self.port2
+        if from_packet_out:
+            ig_port = self.cpu_port
+            port_type1 = PORT_TYPE_INTERNAL
 
         # If the input pkt has a VLAN tag, use that to configure tables.
         pkt_is_tagged = False
         if Dot1Q in pkt:
             vlan1 = pkt[Dot1Q].vlan
             tagged1 = True
             pkt_is_tagged = True
+            # packet-outs should be untagged
+            assert not from_packet_out
         else:
             vlan1 = VLAN_ID_1
 
@@ -1474,17 +1485,22 @@ def runIPv4UnicastTest(
         mpls_label = MPLS_LABEL_2
         if dst_ipv4 is None:
             dst_ipv4 = pkt[IP].dst
-        switch_mac = pkt[Ether].dst
+        if from_packet_out:
+            switch_mac = SWITCH_MAC
+        else:
+            switch_mac = pkt[Ether].dst
 
         # Setup ports.
         self.setup_port(ig_port, vlan1, port_type1, tagged1)
         self.setup_port(eg_port, vlan2, port_type2, tagged2)
 
         # Forwarding type -> routing v4
+        # If from_packet_out, set eth_dst to don't care. All packet-outs should
+        # be routed, independently of eth_dst.
         for eth_type in routed_eth_types:
             self.set_forwarding_type(
                 ig_port,
-                switch_mac,
+                switch_mac if not from_packet_out else None,
                 ethertype=eth_type,
                 fwd_type=FORWARDING_TYPE_UNICAST_IPV4,
             )
@@ -1504,7 +1520,9 @@ def runIPv4UnicastTest(
         if exp_pkt is None:
             # Build exp pkt using the input one.
             exp_pkt = pkt.copy() if not exp_pkt_base else exp_pkt_base
-            exp_pkt = pkt_route(exp_pkt, next_hop_mac)
+            # Route
+            exp_pkt[Ether].src = switch_mac
+            exp_pkt[Ether].dst = next_hop_mac
             if not is_next_hop_spine:
                 exp_pkt = pkt_decrement_ttl(exp_pkt)
             if tagged2 and Dot1Q not in exp_pkt:
@@ -1518,7 +1536,11 @@ def runIPv4UnicastTest(
         if no_send:
             return
 
-        self.send_packet(ig_port, pkt)
+        if from_packet_out:
+            self.send_packet_out(self.build_packet_out(
+                pkt=pkt, port=0, do_forwarding=True))
+        else:
+            self.send_packet(ig_port, pkt)
 
         verify_port = eg_port
         if override_eg_port:

src/main/java/org/stratumproject/fabric/tna/behaviour/FabricInterpreter.java

@@ -39,13 +39,16 @@
 import static org.onlab.util.ImmutableByteSequence.copyFrom;
 import static org.onosproject.net.PortNumber.CONTROLLER;
 import static org.onosproject.net.PortNumber.FLOOD;
+import static org.onosproject.net.PortNumber.TABLE;
 import static org.onosproject.net.flow.instructions.Instruction.Type.OUTPUT;
 import static org.onosproject.net.pi.model.PiPacketOperationType.PACKET_OUT;
-import static org.stratumproject.fabric.tna.behaviour.FabricTreatmentInterpreter.mapPreNextTreatment;
+import static org.stratumproject.fabric.tna.behaviour.Constants.ONE;
+import static org.stratumproject.fabric.tna.behaviour.Constants.ZERO;
 import static org.stratumproject.fabric.tna.behaviour.FabricTreatmentInterpreter.mapAclTreatment;
 import static org.stratumproject.fabric.tna.behaviour.FabricTreatmentInterpreter.mapEgressNextTreatment;
 import static org.stratumproject.fabric.tna.behaviour.FabricTreatmentInterpreter.mapForwardingTreatment;
 import static org.stratumproject.fabric.tna.behaviour.FabricTreatmentInterpreter.mapNextTreatment;
+import static org.stratumproject.fabric.tna.behaviour.FabricTreatmentInterpreter.mapPreNextTreatment;
 
 /**
  * Interpreter for fabric-tna pipeline.
@@ -176,17 +179,18 @@ public PiAction mapTreatment(TrafficTreatment treatment, PiTableId piTableId)
     }
 
     private PiPacketOperation createPiPacketOperation(
-            DeviceId deviceId, ByteBuffer data, long portNumber)
+            ByteBuffer data, long portNumber, boolean doForwarding)
             throws PiInterpreterException {
-        Collection<PiPacketMetadata> metadata = createPacketMetadata(portNumber);
+        Collection<PiPacketMetadata> metadata = createPacketMetadata(portNumber, doForwarding);
         return PiPacketOperation.builder()
                 .withType(PACKET_OUT)
                 .withData(copyFrom(data))
                 .withMetadatas(metadata)
                 .build();
     }
 
-    private Collection<PiPacketMetadata> createPacketMetadata(long portNumber)
+    private Collection<PiPacketMetadata> createPacketMetadata(
+            long portNumber, boolean doForwarding)
             throws PiInterpreterException {
         try {
             ImmutableList.Builder<PiPacketMetadata> builder = ImmutableList.builder();
@@ -200,6 +204,10 @@ private Collection<PiPacketMetadata> createPacketMetadata(long portNumber)
                     .withValue(copyFrom(CPU_LOOPBACK_MODE_DISABLED)
                             .fit(P4InfoConstants.CPU_LOOPBACK_MODE_BITWIDTH))
                     .build());
+            builder.add(PiPacketMetadata.builder()
+                    .withId(P4InfoConstants.DO_FORWARDING)
+                    .withValue(copyFrom(doForwarding ? ONE : ZERO))
+                    .build());
             builder.add(PiPacketMetadata.builder()
                     .withId(P4InfoConstants.PAD0)
                     .withValue(copyFrom(0)
@@ -220,10 +228,9 @@ private Collection<PiPacketMetadata> createPacketMetadata(long portNumber)
     @Override
     public Collection<PiPacketOperation> mapOutboundPacket(OutboundPacket packet)
             throws PiInterpreterException {
-        DeviceId deviceId = packet.sendThrough();
         TrafficTreatment treatment = packet.treatment();
 
-        // fabric.p4 supports only OUTPUT instructions.
+        // We support only OUTPUT instructions.
         List<Instructions.OutputInstruction> outInstructions = treatment
                 .allInstructions()
                 .stream()
@@ -238,18 +245,21 @@ public Collection<PiPacketOperation> mapOutboundPacket(OutboundPacket packet)
 
         ImmutableList.Builder<PiPacketOperation> builder = ImmutableList.builder();
         for (Instructions.OutputInstruction outInst : outInstructions) {
-            if (outInst.port().isLogical() && !outInst.port().equals(FLOOD)) {
-                throw new PiInterpreterException(format(
-                        "Output on logical port '%s' not supported", outInst.port()));
+            if (outInst.port().equals(TABLE)) {
+                // Logical port. Forward using the switch tables like a regular packet.
+                builder.add(createPiPacketOperation(packet.data(), 0, true));
             } else if (outInst.port().equals(FLOOD)) {
-                // Since fabric.p4 does not support flooding, we create a packet
-                // operation for each switch port.
+                // Logical port. Create a packet operation for each switch port.
                 final DeviceService deviceService = handler().get(DeviceService.class);
                 for (Port port : deviceService.getPorts(packet.sendThrough())) {
-                    builder.add(createPiPacketOperation(deviceId, packet.data(), port.number().toLong()));
+                    builder.add(createPiPacketOperation(packet.data(), port.number().toLong(), false));
                 }
+            } else if (outInst.port().isLogical()) {
+                throw new PiInterpreterException(format(
+                        "Output on logical port '%s' not supported", outInst.port()));
             } else {
-                builder.add(createPiPacketOperation(deviceId, packet.data(), outInst.port().toLong()));
+                // Send as-is to given port bypassing all switch tables.
+                builder.add(createPiPacketOperation(packet.data(), outInst.port().toLong(), false));
             }
         }
         return builder.build();

src/main/java/org/stratumproject/fabric/tna/behaviour/P4InfoConstants.java

@@ -307,6 +307,9 @@ private P4InfoConstants() {
     public static final PiPacketMetadataId CPU_LOOPBACK_MODE =
             PiPacketMetadataId.of("cpu_loopback_mode");
     public static final int CPU_LOOPBACK_MODE_BITWIDTH = 2;
+    public static final PiPacketMetadataId DO_FORWARDING =
+            PiPacketMetadataId.of("do_forwarding");
+    public static final int DO_FORWARDING_BITWIDTH = 1;
     public static final PiPacketMetadataId EGRESS_PORT =
             PiPacketMetadataId.of("egress_port");
     public static final int EGRESS_PORT_BITWIDTH = 9;
@@ -317,5 +320,5 @@ private P4InfoConstants() {
             PiPacketMetadataId.of("ingress_port");
     public static final int INGRESS_PORT_BITWIDTH = 9;
     public static final PiPacketMetadataId PAD0 = PiPacketMetadataId.of("pad0");
-    public static final int PAD0_BITWIDTH = 85;
+    public static final int PAD0_BITWIDTH = 84;
 }