I'll submit as 3 separate patches. The reducing locking is really a byproduct of the bug fix, so its unnecessary to add performance measurements to that.
I'll add performance measurements for the OVS_UNLIKELY annotations (if they do in fact help as I predict). Ryan On 6/26/14 5:48 PM, "Ethan Jackson" <[email protected]> wrote: >> This patch reduces locking when dpdk_do_tx_copy() by totalling >> the dropped packets in a local variable before adding to the >> netdev's dropped stats field. >> >> This patch also fixes a bug where rte_pktmbuf_alloc() would fail >> and packets which succeeded to allocate memory with rte_pktmbuf_alloc() >> would not be sent and leak memory. >> >> This patch also adds OVS_UNLIKELY annotations. > >These sound like three separate patches. > >How much if at all does each thing help? > >Ethan > >> >> Signed-off-by: Ryan Wilson <[email protected]> >> --- >> lib/netdev-dpdk.c | 25 ++++++++++++++----------- >> 1 file changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index 6e1d293..03f1e02 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -621,7 +621,7 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, >>struct dpif_packet **packets, >> return 0; >> } >> >> -inline static void >> +static void >> dpdk_queue_pkts(struct netdev_dpdk *dev, int qid, >> struct rte_mbuf **pkts, int cnt) >> { >> @@ -658,28 +658,25 @@ dpdk_do_tx_copy(struct netdev *netdev, struct >>dpif_packet ** pkts, int cnt) >> { >> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); >> struct rte_mbuf *mbufs[cnt]; >> - int i, newcnt = 0; >> + int dropped = 0; >> + int newcnt = 0; >> + int i; >> >> for (i = 0; i < cnt; i++) { >> int size = ofpbuf_size(&pkts[i]->ofpbuf); >> - if (size > dev->max_packet_len) { >> + if (OVS_UNLIKELY(size > dev->max_packet_len)) { >> VLOG_WARN_RL(&rl, "Too big size %d max_packet_len %d", >> (int)size , dev->max_packet_len); >> >> - ovs_mutex_lock(&dev->mutex); >> - dev->stats.tx_dropped++; >> - ovs_mutex_unlock(&dev->mutex); >> - >> + dropped++; >> continue; >> } >> >> mbufs[newcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp); >> >> if (!mbufs[newcnt]) { >> - ovs_mutex_lock(&dev->mutex); >> - dev->stats.tx_dropped++; >> - ovs_mutex_unlock(&dev->mutex); >> - return; >> + dropped += cnt - i; >> + break; >> } >> >> /* We have to do a copy for now */ >> @@ -691,6 +688,12 @@ dpdk_do_tx_copy(struct netdev *netdev, struct >>dpif_packet ** pkts, int cnt) >> newcnt++; >> } >> >> + if (OVS_UNLIKELY(dropped)) { >> + ovs_mutex_lock(&dev->mutex); >> + dev->stats.tx_dropped += dropped; >> + ovs_mutex_unlock(&dev->mutex); >> + } >> + >> dpdk_queue_pkts(dev, NON_PMD_THREAD_TX_QUEUE, mbufs, newcnt); >> dpdk_queue_flush(dev, NON_PMD_THREAD_TX_QUEUE); >> } >> -- >> 1.7.9.5 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> >>https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman >>/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbff >>g%3D%3D%0A&m=6huqi0fU1tGhzvU1sZGRtWX6KvLwJlJJbE5x6GJ6rTw%3D%0A&s=df0f1133 >>1cccb80d4028b66f173e1c52bf92c366b83d7f472d11b419f31903e4 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
