diff mbox series

[ovs-dev,v2] ofproto-xlate: Fix crash when forwarding packet between legacy_l3 tunnels

Message ID PR3PR07MB82929B5B3E7E656E40A6F76A9AE59@PR3PR07MB8292.eurprd07.prod.outlook.com
State Not Applicable
Headers show
Series [ovs-dev,v2] ofproto-xlate: Fix crash when forwarding packet between legacy_l3 tunnels | expand

Checks

Context Check Description
ovsrobot/apply-robot fail apply and check: fail
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Jan Scheurich April 4, 2022, 6:47 a.m. UTC 
From: Jan Scheurich <jan.scheurich@ericsson.com>

A packet received from a tunnel port with legacy_l3 packet-type (e.g.
lisp, L3 gre, gtpu) is conceptually wrapped in a dummy Ethernet header
for processing in an OF pipeline that is not packet-type-aware. Before
transmission of the packet to another legacy_l3 tunnel port, the dummy
Ethernet header is stripped again.

In ofproto-xlate, wrapping in the dummy Ethernet header is done by 
simply changing the packet_type to PT_ETH. The generation of the 
push_eth datapath action is deferred until the packet's flow changes 
need to be committed, for example at output to a normal port. The 
deferred Ethernet encapsulation is marked in the pending_encap flag.

This patch fixes a bug in the translation of the output action to a
legacy_l3 tunnel port, where the packet_type of the flow is reverted 
from PT_ETH to PT_IPV4 or PT_IPV6 (depending on the dl_type) to 
remove its Ethernet header without clearing the pending_encap flag 
if it was set. At the subsequent commit of the flow changes, the 
unexpected combination of pending_encap == true with an PT_IPV4 
or PT_IPV6 packet_type hit the OVS_NOT_REACHED() abortion clause.

The pending_encap is now cleared in this situation.

Reported-By: Dincer Beken <dbeken@blackned.de>
Signed-off-By: Jan Scheurich <jan.scheurich@ericsson.com>
Signed-off-By: Dincer Beken <dbeken@blackned.de>

v1->v2:
  A specific test has been added to tunnel-push-pop.at to verify the
  correct behavior.
---
 ofproto/ofproto-dpif-xlate.c |  4 ++++
 tests/tunnel-push-pop.at     | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)

--
2.25.1
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index cc9c1c628..1843a5d66 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4195,6 +4195,10 @@  compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         if (xport->pt_mode == NETDEV_PT_LEGACY_L3) {
             flow->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,
                                                ntohs(flow->dl_type));
+            if (ctx->pending_encap) {
+                /* The Ethernet header was not actually added yet. */
+                ctx->pending_encap = false;
+            }
         }
     }

diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 57589758f..70462d905 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -546,6 +546,28 @@  AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port  [[37]]' | sort], [0], [dnl
   port  7: rx pkts=5, bytes=434, drop=?, errs=?, frame=?, over=?, crc=?
 ])

+dnl Send out packets received from L3GRE tunnel back to L3GRE tunnel 
+AT_CHECK([ovs-ofctl del-flows int-br]) AT_CHECK([ovs-ofctl add-flow 
+int-br "in_port=7,actions=set_field:3->in_port,7"])
+AT_CHECK([ovs-vsctl -- set Interface br0 options:pcap=br0.pcap])
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 
+'aa55aa550000001b213cab6408004500007079464000402fba630101025c0101025820
+000800000001c845000054ba200000400184861e0000011e00000200004227e75400030
+af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f20212223
+2425262728292a2b2c2d2e2f3031323334353637'])
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 
+'aa55aa550000001b213cab6408004500007079464000402fba630101025c0101025820
+000800000001c845000054ba200000400184861e0000011e00000200004227e75400030
+af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f20212223
+2425262728292a2b2c2d2e2f3031323334353637'])
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 
+'aa55aa550000001b213cab6408004500007079464000402fba630101025c0101025820
+000800000001c845000054ba200000400184861e0000011e00000200004227e75400030
+af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f20212223
+2425262728292a2b2c2d2e2f3031323334353637'])
+
+ovs-appctl time/warp 1000
+
+AT_CHECK([ovs-pcap p0.pcap > p0.pcap.txt 2>&1]) AT_CHECK([tail -6 
+p0.pcap.txt], [0], [dnl
+aa55aa550000001b213cab6408004500007079464000402fba630101025c01010258200
+00800000001c845000054ba200000400184861e0000011e00000200004227e75400030a
+f3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232
+425262728292a2b2c2d2e2f3031323334353637
+001b213cab64aa55aa55000008004500007000004000402f33aa010102580101025c200
+00800000001c845000054ba200000400184861e0000011e00000200004227e75400030a
+f3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232
+425262728292a2b2c2d2e2f3031323334353637
+aa55aa550000001b213cab6408004500007079464000402fba630101025c01010258200
+00800000001c845000054ba200000400184861e0000011e00000200004227e75400030a
+f3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232
+425262728292a2b2c2d2e2f3031323334353637
+001b213cab64aa55aa55000008004500007000004000402f33aa010102580101025c200
+00800000001c845000054ba200000400184861e0000011e00000200004227e75400030a
+f3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232
+425262728292a2b2c2d2e2f3031323334353637
+aa55aa550000001b213cab6408004500007079464000402fba630101025c01010258200
+00800000001c845000054ba200000400184861e0000011e00000200004227e75400030a
+f3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232
+425262728292a2b2c2d2e2f3031323334353637
+001b213cab64aa55aa55000008004500007000004000402f33aa010102580101025c200
+00800000001c845000054ba200000400184861e0000011e00000200004227e75400030a
+f3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232
+425262728292a2b2c2d2e2f3031323334353637
+])
+
+
 dnl Check decapsulation of Geneve packet with options
 AT_CAPTURE_FILE([ofctl_monitor.log])
 AT_CHECK([ovs-ofctl monitor int-br 65534 --detach --no-chdir --pidfile 2> ofctl_monitor.log])