[ovs-dev] [PATCH 3/3] netdev-dpdk: Fix performance issues / bugs in dpdk_do_tx_copy().

[Ryan Wilson 76511]

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" <ethan at nicira.com> 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 <wryan at nicira.com>
>> ---
>>  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
>> dev at openvswitch.org
>> 
>>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