[ovs-dev] [PATCH 5/5] lib/netdev-dpdk: copy large packet to multi-segment mbufs

Michael Qiu qdy220091330 at gmail.com
Tue Nov 1 07:07:28 UTC 2016



2016/10/28 19:17, Kavanagh, Mark B :
>> Currently, one packet is only copied to one segment
>> in function dpdk_do_tx_copy(),  this could be an issue
>> when a jumboframe comes, especially for multipile segments.
> Typos - 'jumboframe', 'multipile'
>
> This patch doesn't apply cleanly - change 'mbufs' to 'pkts' as previously described to address this.

I will fix the issues, and rebase to the latest code.

>
> I haven't tested this patchset yet - will try to get to it next week.

OK, thanks.

> Thanks again,
> Mark
>
>> This patch calculate the segment number needed by the packet and
>> copy the data to different segments.
>>
>> Signed-off-by: Michael Qiu <qiudayu at chinac.com>
>> Signed-off-by: Jijiang Liu <liujijiang at chinac.com>
>> ---
>> lib/netdev-dpdk.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 48 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index ad92504..abb3b53 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1554,9 +1554,10 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch
>> *batch)
>> #endif
>>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>      struct rte_mbuf *mbufs[PKT_ARRAY_SIZE];
>> +    struct rte_mbuf *temp, *head = NULL;
>>      int dropped = 0;
>>      int newcnt = 0;
>> -    int i;
>> +    int i, j, nb_segs;
>>
>>      /* If we are on a non pmd thread we have to use the mempool mutex, because
>>       * every non pmd thread shares the same mempool cache */
>> @@ -1569,6 +1570,7 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch
>> *batch)
>>
>>      for (i = 0; i < batch->count; i++) {
>>          int size = dp_packet_size(batch->packets[i]);
>> +        int max_data_len, tmp_len;
>>
>>          if (OVS_UNLIKELY(size > dev->max_packet_len)) {
>>              VLOG_WARN_RL(&rl, "Too big size %d max_packet_len %d",
>> @@ -1578,7 +1580,24 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch
>> *batch)
>>              continue;
>>          }
>>
>> -        mbufs[newcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp);
>> +        temp = mbufs[newcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp);
>> +
>> +        /* all new allocated mbuf's max data len is the same */
>> +        max_data_len = temp->buf_len - temp->data_off;
>> +
>> +        nb_segs = size/max_data_len;
>> +        if (size % max_data_len)
>> +            nb_segs = nb_segs + 1;
>> +
>> +        for (j = 1; j < nb_segs; j++) {
>> +            temp->next = rte_pktmbuf_alloc(dev->dpdk_mp->mp);
>> +            if (!temp->next) {
>> +                rte_pktmbuf_free(mbufs[newcnt]);
>> +                mbufs[newcnt] = NULL;
>> +                break;
>> +            }
>> +            temp = temp->next;
>> +        }
> Need to terminate the mbuf chain here:
> 	+ temp->next = NULL;

No need, all mbufs' next field have been set to NULL when allocated.

>
>>          if (!mbufs[newcnt]) {
>>              dropped += batch->count - i;
>> @@ -1586,15 +1605,37 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct
>> dp_packet_batch *batch)
>>          }
>>
>>          /* We have to do a copy for now */
>> -        memcpy(rte_pktmbuf_mtod(mbufs[newcnt], void *),
>> -               dp_packet_data(batch->packets[i]), size);
>> +        rte_pktmbuf_pkt_len(mbufs[newcnt]) = size;
>> +        temp = mbufs[newcnt];
>> +        tmp_len = size < max_data_len ? size: max_data_len;
>> +        if (batch->packets[i]->source == DPBUF_DPDK) {
>> +            head = &(batch->packets[i]->mbuf);
>> +            while (temp && head && size > 0) {
>> +                rte_memcpy(rte_pktmbuf_mtod(temp, void*), dp_packet_data((struct dp_packet
>> *)head),tmp_len);
>> +                rte_pktmbuf_data_len(temp) = tmp_len;
>> +                head = head->next;
>> +                size = size - tmp_len;
>> +                tmp_len =  size < max_data_len ? size: max_data_len;
>> +                temp = temp->next;
>> +            }
>> +        } else {
> This section could use a comment or two.

OK, I will add a comment.

>
>> +            int offset = 0;
>> +            while (temp && size > 0) {
>> +                memcpy(rte_pktmbuf_mtod(temp, void *),
>> +                    dp_packet_at(batch->packets[i], offset,tmp_len), tmp_len);
>> +                rte_pktmbuf_data_len(temp) = tmp_len;
>> +                temp = temp->next;
>> +                size = size - tmp_len;
>> +                offset +=tmp_len;
>> +                tmp_len =  size < max_data_len ? size: max_data_len;
>> +            }
>> +        }
>> +
>>
>> -        mbufs[newcnt]->nb_segs = batch->packets[i]->mbuf.nb_segs;
>> +        mbufs[newcnt]->nb_segs = nb_segs;
>>          mbufs[newcnt]->ol_flags = batch->packets[i]->mbuf.ol_flags;
>>          mbufs[newcnt]->packet_type = batch->packets[i]->mbuf.packet_type;
>>          mbufs[newcnt]->tx_offload = batch->packets[i]->mbuf.tx_offload;
>> -        rte_pktmbuf_data_len(mbufs[newcnt]) = size;
>> -        rte_pktmbuf_pkt_len(mbufs[newcnt]) = size;
>>
>>          newcnt++;
>>      }
>> --
>> 1.8.3.1
>



More information about the dev mailing list