mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-09-02 07:15:17 +00:00
Code Issues Releases Wiki Activity

dpif-netdev: Fix crash when PACKET_OUT is metered.

Browse Source 
When a PACKET_OUT has output port of OFPP_TABLE, and the rule
table includes a meter and this causes the packet to be deleted,
execute with a clone of the packet, restoring the original packet
if it is changed by the execution.

Add tests to verify the original issue is fixed, and that the fix
doesn't break tunnel processing.

Reported-by: Tony van der Peet <tony.vanderpeet@alliedtelesis.co.nz>
Signed-off-by: Tony van der Peet <tony.vanderpeet@alliedtelesis.co.nz>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
Tony van der Peet
2021-08-16 17:10:07 +12:00
committed by Ilya Maximets
parent 5c1cc74da6
commit 7e6b41ac8d
4 changed files with 101 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
lib/dp-packet.h
View File

@@ -199,6 +199,7 @@ struct dp_packet *dp_packet_clone_data_with_headroom(const void *, size_t,
void dp_packet_resize(struct dp_packet *b, size_t new_headroom,
size_t new_tailroom);
static inline void dp_packet_delete(struct dp_packet *);
static inline void dp_packet_swap(struct dp_packet *, struct dp_packet *);
static inline void *dp_packet_at(const struct dp_packet *, size_t offset,
size_t size);
@@ -256,6 +257,18 @@ dp_packet_delete(struct dp_packet *b)
}
}
/* Swaps content of two packets. */
static inline void
dp_packet_swap(struct dp_packet *a, struct dp_packet *b)
{
ovs_assert(a->source == DPBUF_MALLOC || a->source == DPBUF_STUB);
ovs_assert(b->source == DPBUF_MALLOC || b->source == DPBUF_STUB);
struct dp_packet c = *a;
*a = *b;
*b = c;
}
/* If 'b' contains at least 'offset + size' bytes of data, returns a pointer to
* byte 'offset'. Otherwise, returns a null pointer. */
static inline void *

13
lib/dpif-netdev.c
View File

@@ -4083,7 +4083,10 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute)
flow_hash_5tuple(execute->flow, 0));
}
dp_packet_batch_init_packet(&pp, execute->packet);
/* Making a copy because the packet might be stolen during the execution
* and caller might still need it. */
struct dp_packet *packet_clone = dp_packet_clone(execute->packet);
dp_packet_batch_init_packet(&pp, packet_clone);
dp_netdev_execute_actions(pmd, &pp, false, execute->flow,
execute->actions, execute->actions_len);
dp_netdev_pmd_flush_output_packets(pmd, true);
@@ -4093,6 +4096,14 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute)
dp_netdev_pmd_unref(pmd);
}
if (dp_packet_batch_size(&pp)) {
/* Packet wasn't dropped during the execution. Swapping content with
* the original packet, because the caller might expect actions to
* modify it. */
dp_packet_swap(execute->packet, packet_clone);
dp_packet_delete_batch(&pp, true);
}
return 0;
}

20
tests/ofproto-dpif.at
View File

@@ -9723,6 +9723,26 @@ OFPST_TABLE reply (OF1.3) (xid=0x2):
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([ofproto-dpif packet-out table meter drop])
OVS_VSWITCHD_START
add_of_ports br0 1 2
AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps bands=type=drop rate=1'])
AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 'in_port=1 action=meter:1,output:2'])
ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000400080000 actions=resubmit(,0)"
ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000400080000 actions=resubmit(,0)"
# Check that vswitchd hasn't crashed by dumping the meter added above
AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0 | ofctl_strip], [0], [dnl
OFPST_METER_CONFIG reply (OF1.3):
meter=1 pktps bands=
type=drop rate=1
])
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([ofproto-dpif - ICMPv6])
OVS_VSWITCHD_START
add_of_ports br0 1

56
tests/tunnel-push-pop.at
View File

@@ -595,6 +595,62 @@ OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep 50540000000a5054000000091235 | wc
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([tunnel_push_pop - packet_out debug_slow])
OVS_VSWITCHD_START(
[add-port br0 p0 dnl
-- set Interface p0 type=dummy ofport_request=1 dnl
other-config:hwaddr=aa:55:aa:55:00:00])
AT_CHECK([ovs-appctl vlog/set dpif_netdev:dbg])
AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br datapath_type=dummy])
AT_CHECK([ovs-vsctl add-port int-br t2 dnl
-- set Interface t2 type=geneve options:remote_ip=1.1.2.92 dnl
options:key=123 ofport_request=2])
dnl First setup dummy interface IP address, then add the route
dnl so that tnl-port table can get valid IP address for the device.
AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 1.1.2.88/24], [0], [OK
])
AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
])
AT_CHECK([ovs-ofctl add-flow br0 action=normal])
dnl This ARP reply from p0 has two effects:
dnl 1. The ARP cache will learn that 1.1.2.92 is at f8:bc:12:44:34:b6.
dnl 2. The br0 mac learning will learn that f8:bc:12:44:34:b6 is on p0.
AT_CHECK([
ovs-appctl netdev-dummy/receive p0 dnl
'recirc_id(0),in_port(2),dnl
eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),dnl
arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'
])
AT_CHECK([ovs-vsctl -- set Interface p0 options:tx_pcap=p0.pcap])
packet=50540000000a505400000009123
encap=f8bc124434b6aa55aa5500000800450000320000400040113406010102580101025c83a917c1001e00000000655800007b00
dnl Output to tunnel from a int-br internal port.
dnl Checking that the packet arrived and it was correctly encapsulated.
AT_CHECK([ovs-ofctl add-flow int-br "in_port=LOCAL,actions=debug_slow,output:2"])
AT_CHECK([ovs-appctl netdev-dummy/receive int-br "${packet}4"])
OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}4" | wc -l` -ge 1])
dnl Sending again to exercise the non-miss upcall path.
AT_CHECK([ovs-appctl netdev-dummy/receive int-br "${packet}4"])
OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}4" | wc -l` -ge 2])
dnl Output to tunnel from the controller.
AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out int-br CONTROLLER "debug_slow,output:2" "${packet}5"])
OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}5" | wc -l` -ge 1])
dnl Datapath actions should not have tunnel push action.
AT_CHECK([ovs-appctl dpctl/dump-flows | grep -q tnl_push], [1])
dnl There should be slow_path action instead.
AT_CHECK([ovs-appctl dpctl/dump-flows | grep -q 'slow_path(action)'], [0])
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([tunnel_push_pop - underlay bridge match])
OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy ofport_request=1 other-config:hwaddr=aa:55:aa:55:00:00])