[ovs-dev] [RFC PATCH V4 8/9] netdev-dpdk: copy large packet to multi-seg. mbufs

Chandran, Sugesh sugesh.chandran at intel.com
Tue Dec 12 20:33:36 UTC 2017



Regards
_Sugesh


> -----Original Message-----
> From: Kavanagh, Mark B
> Sent: Tuesday, December 12, 2017 2:42 PM
> To: Chandran, Sugesh <sugesh.chandran at intel.com>; dev at openvswitch.org;
> qiudayu at chinac.com
> Cc: Stokes, Ian <ian.stokes at intel.com>; Loftus, Ciara <ciara.loftus at intel.com>;
> santosh.shukla at caviumnetworks.com
> Subject: RE: [ovs-dev][RFC PATCH V4 8/9] netdev-dpdk: copy large packet to
> multi-seg. mbufs
> 
> 
> 
> >-----Original Message-----
> >From: Chandran, Sugesh
> >Sent: Friday, December 8, 2017 6:01 PM
> >To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; dev at openvswitch.org;
> >qiudayu at chinac.com
> >Cc: Stokes, Ian <ian.stokes at intel.com>; Loftus, Ciara
> ><ciara.loftus at intel.com>; santosh.shukla at caviumnetworks.com
> >Subject: RE: [ovs-dev][RFC PATCH V4 8/9] netdev-dpdk: copy large packet
> >to multi-seg. mbufs
> >
> >
> >
> >Regards
> >_Sugesh
> >
> >
> >> -----Original Message-----
> >> From: Kavanagh, Mark B
> >> Sent: Friday, December 8, 2017 12:02 PM
> >> To: dev at openvswitch.org; qiudayu at chinac.com
> >> Cc: Stokes, Ian <ian.stokes at intel.com>; Loftus, Ciara
> ><ciara.loftus at intel.com>;
> >> santosh.shukla at caviumnetworks.com; Chandran, Sugesh
> >> <sugesh.chandran at intel.com>; Kavanagh, Mark B
> >> <mark.b.kavanagh at intel.com>
> >> Subject: [ovs-dev][RFC PATCH V4 8/9] netdev-dpdk: copy large packet
> >> to
> >multi-
> >> seg. mbufs
> >>
> >> From: Michael Qiu <qiudayu at chinac.com>
> >>
> >> Currently, packets are only copied to a single segment in the
> >> function dpdk_do_tx_copy(). This could be an issue in the case of
> >> jumbo frames, particularly when multi-segment mbufs are involved.
> >>
> >> This patch calculates the number of segments needed by a packet and
> >> copies
> >the
> >> data to each segment.
> >>
> >> Co-authored-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
> >>
> >> Signed-off-by: Michael Qiu <qiudayu at chinac.com>
> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
> >> ---
> >>  lib/netdev-dpdk.c | 56
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++----
> >>  1 file changed, 52 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> >> 8a81690..f83bb9e
> >> 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -1830,8 +1830,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 *pkts[PKT_ARRAY_SIZE];
> >> +    struct rte_mbuf *temp, *head = NULL;
> >>      uint32_t cnt = batch_cnt;
> >>      uint32_t dropped = 0;
> >> +    uint32_t i, j, nb_segs;
> >[Sugesh] This function become bit longer, can we split it ?
> 
> I'll try to split out the multi-segment code into a separate function that is
> invoked on the basis of (nb_segs > 1).
> 
> >>
> >>      if (dev->type != DPDK_DEV_VHOST) {
> >>          /* Check if QoS has been configured for this netdev. */ @@
> >> -1844,9
> >> +1846,10 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct
> >> dp_packet_batch *batch)
> >>
> >>      uint32_t txcnt = 0;
> >>
> >> -    for (uint32_t i = 0; i < cnt; i++) {
> >> +    for (i = 0; i < cnt; i++) {
> >>          struct dp_packet *packet = batch->packets[i];
> >>          uint32_t size = dp_packet_size(packet);
> >> +        uint16_t max_data_len, data_len;
> >>
> >>          if (OVS_UNLIKELY(size > dev->max_packet_len)) {
> >>              VLOG_WARN_RL(&rl, "Too big size %u max_packet_len %d",
> >> @@ -
> >> 1856,15 +1859,60 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,
> >> struct dp_packet_batch *batch)
> >>              continue;
> >>          }
> >>
> >> -        pkts[txcnt] = rte_pktmbuf_alloc(dev->mp);
> >> +        temp = pkts[txcnt] = rte_pktmbuf_alloc(dev->mp);
> >>          if (OVS_UNLIKELY(!pkts[txcnt])) {
> >>              dropped += cnt - i;
> >>              break;
> >>          }
> >>
> >> +        /* All new allocated mbuf's max data len is the same */
> >> +        max_data_len = temp->buf_len - temp->data_off;
> >> +
> >> +        /* Calculate # of output mbufs. */
> >> +        nb_segs = size / max_data_len;
> >> +        if (size % max_data_len) {
> >> +            nb_segs = nb_segs + 1;
> >> +        }
> >> +
> >> +        /* Allocate additional mbufs when multiple output mbufs required.
> >*/
> >> +        for (j = 1; j < nb_segs; j++) {
> >> +            temp->next = rte_pktmbuf_alloc(dev->mp);
> >[Sugesh] Does alloc_bulk would be better to use than multiple single
> >allocation from the pool?
> 
> Since we need to iterate over the set of allocated mbufs and link each to the
> next anyway, it's probably more straightforward with the single alloc.
[Sugesh] Sure.. that make sense.

> 
> >
> >> +            if (!temp->next) {
> >> +                rte_pktmbuf_free(pkts[txcnt]);
> >
> >> +                pkts[txcnt] = NULL;
> >> +                break;
> >> +            }
> >> +            temp = temp->next;
> >> +        }
> >>          /* We have to do a copy for now */
> >> -        memcpy(rte_pktmbuf_mtod(pkts[txcnt], void *),
> >> -               dp_packet_data(packet), size);
> >> +        rte_pktmbuf_pkt_len(pkts[txcnt]) = size;
> >> +        temp = pkts[txcnt];
> >> +
> >> +        data_len = size < max_data_len ? size: max_data_len;
> >> +        if (packet->source == DPBUF_DPDK) {
> >> +            head = &(packet->mbuf);
> >> +            while (temp && head && size > 0) {
> >> +                rte_memcpy(rte_pktmbuf_mtod(temp, void *),
> >> +                        dp_packet_data((struct dp_packet *)head),
> >data_len);
> >> +                rte_pktmbuf_data_len(temp) = data_len;
> >> +                head = head->next;
> >> +                size = size - data_len;
> >> +                data_len =  size < max_data_len ? size: max_data_len;
> >> +                temp = temp->next;
> >> +            }
> >> +        } else {
> >> +            int offset = 0;
> >> +            while (temp && size > 0) {
> >> +                memcpy(rte_pktmbuf_mtod(temp, void *),
> >> +                    dp_packet_at(packet, offset, data_len), data_len);
> >> +                rte_pktmbuf_data_len(temp) = data_len;
> >> +                temp = temp->next;
> >> +                size = size - data_len;
> >> +                offset += data_len;
> >> +                data_len = size < max_data_len ? size: max_data_len;
> >> +            }
> >> +        }
> >> +
> >>          dp_packet_set_size((struct dp_packet *)pkts[txcnt], size);
> >>          pkts[txcnt]->nb_segs = packet->mbuf.nb_segs;
> >>          pkts[txcnt]->ol_flags = packet->mbuf.ol_flags;
> >> --
> >> 1.9.3



More information about the dev mailing list