Regards _Sugesh
> -----Original Message----- > From: Kavanagh, Mark B > Sent: Tuesday, December 12, 2017 2:42 PM > To: Chandran, Sugesh <[email protected]>; [email protected]; > [email protected] > Cc: Stokes, Ian <[email protected]>; Loftus, Ciara > <[email protected]>; > [email protected] > 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 <[email protected]>; [email protected]; > >[email protected] > >Cc: Stokes, Ian <[email protected]>; Loftus, Ciara > ><[email protected]>; [email protected] > >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: [email protected]; [email protected] > >> Cc: Stokes, Ian <[email protected]>; Loftus, Ciara > ><[email protected]>; > >> [email protected]; Chandran, Sugesh > >> <[email protected]>; Kavanagh, Mark B > >> <[email protected]> > >> Subject: [ovs-dev][RFC PATCH V4 8/9] netdev-dpdk: copy large packet > >> to > >multi- > >> seg. mbufs > >> > >> From: Michael Qiu <[email protected]> > >> > >> 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 <[email protected]> > >> > >> Signed-off-by: Michael Qiu <[email protected]> > >> Signed-off-by: Mark Kavanagh <[email protected]> > >> --- > >> 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
