diff mbox series

[ovs-dev,v3] learning-switch: Fix coredump of OpenFlow15 learning-switch

Message ID ADIAXQB0IweyohXK8CMnYao5.1.1680849022584.Hmail.mocan@ucloud.cn
State Accepted
Commit 70ba6e97dbd75d5c255a339568e6ff56ed8f67fc
Headers show
Series [ovs-dev,v3] learning-switch: Fix coredump of OpenFlow15 learning-switch | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Faicker Mo April 7, 2023, 6:30 a.m. UTC 
The OpenFlow15 Packet-Out message contains the match instead of the in_port.
The flow.tunnel.metadata.tab is not inited but used in the loop of tun_metadata_to_nx_match.

The coredump gdb backtrace is:
 #0  memcpy_from_metadata (dst=dst@entry=0x7ffcfac2f060, src=src@entry=0x7ffcfac30880, loc=loc@entry=0x10) at lib/tun-metadata.c:467
 #1  0x00000000004506e8 in metadata_loc_from_match_read (match=0x7ffcfac30598, is_masked=<synthetic pointer>, mask=0x7ffcfac30838, idx=0, map=0x0) at lib/tun-metadata.c:865
 #2  metadata_loc_from_match_read (is_masked=<synthetic pointer>, mask=0x7ffcfac30838, idx=0, match=0x7ffcfac30598, map=0x0) at lib/tun-metadata.c:854
 #3  tun_metadata_to_nx_match (b=b@entry=0x892260, oxm=oxm@entry=OFP15_VERSION, match=match@entry=0x7ffcfac30598) at lib/tun-metadata.c:888
 #4  0x000000000047c1f8 in nx_put_raw (b=b@entry=0x892260, oxm=oxm@entry=OFP15_VERSION, match=match@entry=0x7ffcfac30598, cookie=<optimized out>, cookie@entry=0,
     cookie_mask=<optimized out>, cookie_mask@entry=0) at lib/nx-match.c:1186
 #5  0x000000000047d693 in oxm_put_match (b=b@entry=0x892260, match=match@entry=0x7ffcfac30598, version=version@entry=OFP15_VERSION) at lib/nx-match.c:1343
 #6  0x000000000043194e in ofputil_encode_packet_out (po=po@entry=0x7ffcfac30580, protocol=<optimized out>) at lib/ofp-packet.c:1226
 #7  0x000000000040a4fe in process_packet_in (sw=sw@entry=0x891d70, oh=<optimized out>) at lib/learning-switch.c:619
 #8  0x000000000040acdc in lswitch_process_packet (msg=0x892210, sw=0x891d70) at lib/learning-switch.c:374
 #9  lswitch_run (sw=0x891d70) at lib/learning-switch.c:324
 #10 0x0000000000406f26 in main (argc=<optimized out>, argv=<optimized out>) at utilities/ovs-testcontroller.c:180

Fix that by initing the flow metadata.

Fixes: 35eb6326d5d0 ("ofp-util: Add flow metadata to ofputil_packet_out")
Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>
---
v2:
- changed the init and add test case
v3:
- better init method and test case
---
 lib/learning-switch.c    |  1 +
 tests/automake.mk        |  3 ++-
 tests/learning-switch.at | 23 +++++++++++++++++++++++
 tests/testsuite.at       |  1 +
 4 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 tests/learning-switch.at

Comments

Simon Horman April 7, 2023, 1:13 p.m. UTC | #1 
On Fri, Apr 07, 2023 at 02:30:22PM +0800, Faicker Mo wrote:
> The OpenFlow15 Packet-Out message contains the match instead of the in_port.
> The flow.tunnel.metadata.tab is not inited but used in the loop of tun_metadata_to_nx_match.
> 
> The coredump gdb backtrace is:
>  #0  memcpy_from_metadata (dst=dst@entry=0x7ffcfac2f060, src=src@entry=0x7ffcfac30880, loc=loc@entry=0x10) at lib/tun-metadata.c:467
>  #1  0x00000000004506e8 in metadata_loc_from_match_read (match=0x7ffcfac30598, is_masked=<synthetic pointer>, mask=0x7ffcfac30838, idx=0, map=0x0) at lib/tun-metadata.c:865
>  #2  metadata_loc_from_match_read (is_masked=<synthetic pointer>, mask=0x7ffcfac30838, idx=0, match=0x7ffcfac30598, map=0x0) at lib/tun-metadata.c:854
>  #3  tun_metadata_to_nx_match (b=b@entry=0x892260, oxm=oxm@entry=OFP15_VERSION, match=match@entry=0x7ffcfac30598) at lib/tun-metadata.c:888
>  #4  0x000000000047c1f8 in nx_put_raw (b=b@entry=0x892260, oxm=oxm@entry=OFP15_VERSION, match=match@entry=0x7ffcfac30598, cookie=<optimized out>, cookie@entry=0,
>      cookie_mask=<optimized out>, cookie_mask@entry=0) at lib/nx-match.c:1186
>  #5  0x000000000047d693 in oxm_put_match (b=b@entry=0x892260, match=match@entry=0x7ffcfac30598, version=version@entry=OFP15_VERSION) at lib/nx-match.c:1343
>  #6  0x000000000043194e in ofputil_encode_packet_out (po=po@entry=0x7ffcfac30580, protocol=<optimized out>) at lib/ofp-packet.c:1226
>  #7  0x000000000040a4fe in process_packet_in (sw=sw@entry=0x891d70, oh=<optimized out>) at lib/learning-switch.c:619
>  #8  0x000000000040acdc in lswitch_process_packet (msg=0x892210, sw=0x891d70) at lib/learning-switch.c:374
>  #9  lswitch_run (sw=0x891d70) at lib/learning-switch.c:324
>  #10 0x0000000000406f26 in main (argc=<optimized out>, argv=<optimized out>) at utilities/ovs-testcontroller.c:180
> 
> Fix that by initing the flow metadata.
> 
> Fixes: 35eb6326d5d0 ("ofp-util: Add flow metadata to ofputil_packet_out")
> Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Ilya Maximets April 25, 2023, 9:50 p.m. UTC | #2 
On 4/7/23 15:13, Simon Horman wrote:
> On Fri, Apr 07, 2023 at 02:30:22PM +0800, Faicker Mo wrote:
>> The OpenFlow15 Packet-Out message contains the match instead of the in_port.
>> The flow.tunnel.metadata.tab is not inited but used in the loop of tun_metadata_to_nx_match.
>>
>> The coredump gdb backtrace is:
>>  #0  memcpy_from_metadata (dst=dst@entry=0x7ffcfac2f060, src=src@entry=0x7ffcfac30880, loc=loc@entry=0x10) at lib/tun-metadata.c:467
>>  #1  0x00000000004506e8 in metadata_loc_from_match_read (match=0x7ffcfac30598, is_masked=<synthetic pointer>, mask=0x7ffcfac30838, idx=0, map=0x0) at lib/tun-metadata.c:865
>>  #2  metadata_loc_from_match_read (is_masked=<synthetic pointer>, mask=0x7ffcfac30838, idx=0, match=0x7ffcfac30598, map=0x0) at lib/tun-metadata.c:854
>>  #3  tun_metadata_to_nx_match (b=b@entry=0x892260, oxm=oxm@entry=OFP15_VERSION, match=match@entry=0x7ffcfac30598) at lib/tun-metadata.c:888
>>  #4  0x000000000047c1f8 in nx_put_raw (b=b@entry=0x892260, oxm=oxm@entry=OFP15_VERSION, match=match@entry=0x7ffcfac30598, cookie=<optimized out>, cookie@entry=0,
>>      cookie_mask=<optimized out>, cookie_mask@entry=0) at lib/nx-match.c:1186
>>  #5  0x000000000047d693 in oxm_put_match (b=b@entry=0x892260, match=match@entry=0x7ffcfac30598, version=version@entry=OFP15_VERSION) at lib/nx-match.c:1343
>>  #6  0x000000000043194e in ofputil_encode_packet_out (po=po@entry=0x7ffcfac30580, protocol=<optimized out>) at lib/ofp-packet.c:1226
>>  #7  0x000000000040a4fe in process_packet_in (sw=sw@entry=0x891d70, oh=<optimized out>) at lib/learning-switch.c:619
>>  #8  0x000000000040acdc in lswitch_process_packet (msg=0x892210, sw=0x891d70) at lib/learning-switch.c:374
>>  #9  lswitch_run (sw=0x891d70) at lib/learning-switch.c:324
>>  #10 0x0000000000406f26 in main (argc=<optimized out>, argv=<optimized out>) at utilities/ovs-testcontroller.c:180
>>
>> Fix that by initing the flow metadata.
>>
>> Fixes: 35eb6326d5d0 ("ofp-util: Add flow metadata to ofputil_packet_out")
>> Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>
> 
> Reviewed-by: Simon Horman <simon.horman@corigine.com>

Thanks!  Applied and backported down to 2.17.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 8102475ca..cdf42935c 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -569,6 +569,7 @@  process_packet_in(struct lswitch *sw, const struct ofp_header *oh)
     }
 
     /* Prepare packet_out in case we need one. */
+    match_init_catchall(&po.flow_metadata);
     po.buffer_id = buffer_id;
     if (buffer_id == UINT32_MAX) {
         po.packet = dp_packet_data(&pkt);
diff --git a/tests/automake.mk b/tests/automake.mk
index 86e496a5b..720c94449 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -110,7 +110,8 @@  TESTSUITE_AT = \
 	tests/mcast-snooping.at \
 	tests/packet-type-aware.at \
 	tests/nsh.at \
-	tests/drop-stats.at
+	tests/drop-stats.at \
+	tests/learning-switch.at
 
 EXTRA_DIST += $(FUZZ_REGRESSION_TESTS)
 FUZZ_REGRESSION_TESTS = \
diff --git a/tests/learning-switch.at b/tests/learning-switch.at
new file mode 100644
index 000000000..ac2fc1b80
--- /dev/null
+++ b/tests/learning-switch.at
@@ -0,0 +1,23 @@ 
+AT_BANNER([learning switch])
+
+### -----------------------------------------------------------------
+###   learning switch OpenFlow15 test case
+### -----------------------------------------------------------------
+
+AT_SETUP([learning switch - OpenFlow15])
+dnl Start ovs-testcontroller
+AT_CHECK([ovs-testcontroller --no-chdir --detach punix:controller --pidfile -v ptcp:], [0], [ignore])
+dnl Start ovs
+OVS_VSWITCHD_START([dnl
+    set bridge br0 datapath_type=dummy \
+        protocols=OpenFlow15 -- \
+    add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1 -- \
+    set-controller br0 tcp:127.0.0.1:6653])
+AT_CHECK([
+    ovs-appctl netdev-dummy/receive p1 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637
+], [0], [ignore])
+AT_CHECK([kill `cat ovs-testcontroller.pid`])
+
+OVS_WAIT_UNTIL([! test -e controller])
+OVS_VSWITCHD_STOP(["/cannot find route for controller/d"])
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index cf4e3eadf..9d77a9f51 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -77,3 +77,4 @@  m4_include([tests/packet-type-aware.at])
 m4_include([tests/nsh.at])
 m4_include([tests/drop-stats.at])
 m4_include([tests/pytest.at])
+m4_include([tests/learning-switch.at])