sdnfv / openNetVM

A high performance container-based NFV platform from GW and UCR.
http://sdnfv.github.io/onvm/
Other
263 stars 135 forks source link

Update advanced rings usage #198

Closed dennisafa closed 4 years ago

dennisafa commented 4 years ago

Bug Report

We currently have an example of advanced rings being used in the scaling example. The custom loop (used to poll on packets from the RX ring) updates stats incorrectly.

Current Behavior The current advanced rings example updates the NF stats after enqueueing packets onto its TX ring, as seen here.
The TX thread that is running will also update the stats after dequeueing packets from its TX ring, found here. This is problematic as it updates two stats at once. In addition, if the advanced rings NF is trying to send packets to another NF, and does NOT enqueue at least 32 packets into its TX ring, some stats will not update, because: 1) TX thread will call onvm_pkt_process_tx_batch 2) onvm_pkt_process_tx_batch will call onvm_pkt_enqueue_nf 3) onvm_pkt_enqueue_nf will call onvm_pkt_flush_nf_queue ONLY if the total number of packets in its TX ring is 32 (which is the maximum amount we burst from the TX ring anyway) 4) onvm_pkt_flush_nf_queue does the job of updating the destination NF's rx_drop, tx_drop, rx, and tx stats.

Expected behavior/code This is messy because we have different threads touching the same stats. We need to either have the advanced rings loop update all stats, or the manager.

Steps to reproduce Read through this issue: https://github.com/sdnfv/openNetVM/issues/192 The problem is described thoroughly along with replication.

Environment

Possible Solution

Easier way is to have the manager update stats. To do this, update the scaling example to not use stats, and make it clear that all packets need to be enqueued onto the TX ring to be processed. Also need to update the docs. Or, the harder way: have the advanced rings update all of its stats along with properly handling outgoing packets. This lets advanced rings be completely in control of its RX/TX rings. Might be a good chunk of work.

kevindweb commented 4 years ago

I think this was completed when #212 was merged. If so I'll go ahead and close it