net/enic: fix segfault caused by changing MTU

Message ID 20211026000256.11492-1-hyonkim@cisco.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series net/enic: fix segfault caused by changing MTU |

Checks

Context Check Description
ci/iol-x86_64-unit-testing fail Testing issues
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-mellanox-Performance fail Performance Testing issues
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS

Commit Message

Hyong Youb Kim (hyonkim) Oct. 26, 2021, 12:02 a.m. UTC
  Changing MTU after the device start causes a segfault in the Rx
handler. The MTU handler (enic_set_mtu) performs the following steps.
1. Stop NIC Rx
2. Change Rx handler '(struct rte_eth_dev)->rx_pkt_burst' to
   the dummy handler and sleep a while to quiesce
3. Re-allocate/initialize Rx structures
4. Change Rx handler back to the real handler
   (e.g. enic_noscatter_recv_pkts)

enic_set_mtu does not update the recently introduced fast-path pointer
'(struct rte_eth_fp_ops)->rx_pkt_burst'. Since rte_eth_rx_burst only
uses the fast-path pointer, it keeps invoking the real Rx handler, not
the dummy one set by (2). And, (3) causes a segfault in the real Rx
handler (e.g. dereferencing freed structures).

Fix the segfault by updating the fast-path pointer as well.

Fixes: c87d435a4d79 ("ethdev: copy fast-path API into separate structure")

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic_main.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Ferruh Yigit Nov. 3, 2021, 6:41 p.m. UTC | #1
On 10/26/2021 1:02 AM, Hyong Youb Kim wrote:
> Changing MTU after the device start causes a segfault in the Rx
> handler. The MTU handler (enic_set_mtu) performs the following steps.
> 1. Stop NIC Rx
> 2. Change Rx handler '(struct rte_eth_dev)->rx_pkt_burst' to
>     the dummy handler and sleep a while to quiesce
> 3. Re-allocate/initialize Rx structures
> 4. Change Rx handler back to the real handler
>     (e.g. enic_noscatter_recv_pkts)
> 
> enic_set_mtu does not update the recently introduced fast-path pointer
> '(struct rte_eth_fp_ops)->rx_pkt_burst'. Since rte_eth_rx_burst only
> uses the fast-path pointer, it keeps invoking the real Rx handler, not
> the dummy one set by (2). And, (3) causes a segfault in the real Rx
> handler (e.g. dereferencing freed structures).
> 
> Fix the segfault by updating the fast-path pointer as well.
> 
> Fixes: c87d435a4d79 ("ethdev: copy fast-path API into separate structure")
> 
> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> Reviewed-by: John Daley <johndale@cisco.com>

Applied to dpdk-next-net/main, thanks.
  

Patch

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 21b1fffb14..42bf363529 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1665,6 +1665,7 @@  int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
 
 	/* replace Rx function with a no-op to avoid getting stale pkts */
 	eth_dev->rx_pkt_burst = enic_dummy_recv_pkts;
+	rte_eth_fp_ops[enic->port_id].rx_pkt_burst = eth_dev->rx_pkt_burst;
 	rte_mb();
 
 	/* Allow time for threads to exit the real Rx function. */
@@ -1699,6 +1700,7 @@  int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
 	/* put back the real receive function */
 	rte_mb();
 	enic_pick_rx_handler(eth_dev);
+	rte_eth_fp_ops[enic->port_id].rx_pkt_burst = eth_dev->rx_pkt_burst;
 	rte_mb();
 
 	/* restart Rx traffic */