Message ID | 1497850143-3116-6-git-send-email-qdy220091330@gmail.com |
---|---|
State | Deferred |
Headers | show |
>From: Michael Qiu [mailto:qdy220091330@gmail.com] >Sent: Monday, June 19, 2017 6:29 AM >To: dev@openvswitch.org >Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; blp@ovn.org; >dball@vmware.com; Michael Qiu <qiudayu@chinac.com> >Subject: [PATCH 5/5] lib/netdev-dpdk: copy large packet to multi-segment mbufs > >From: Michael Qiu <qiudayu@chinac.com> > >Currently, one packet is only copied to one segment >in function dpdk_do_tx_copy(), this could be an issue >when a jumbo frame comes, especially for multiple segments. > >This patch calculate the segment number needed by the packet and >copy the data to different segments. > >Signed-off-by: Michael Qiu <qiudayu@chinac.com> Hi Michael, One comment inline. Thanks, Mark >--- > lib/netdev-dpdk.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 48 insertions(+), 5 deletions(-) > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >index 0485872..38ec2ed 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -1776,14 +1776,16 @@ 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; > int dropped = 0; > int newcnt = 0; >- int i; >+ int i, j, nb_segs; > > dp_packet_batch_apply_cutlen(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", >@@ -1793,7 +1795,24 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct >dp_packet_batch *batch) > continue; > } > >- pkts[newcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp); >+ temp = pkts[newcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp); > Need to check if rte_pktmbuf_alloc() succeeded here. + >+ /* 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(pkts[newcnt]); >+ pkts[newcnt] = NULL; >+ break; >+ } >+ temp = temp->next; >+ } > > if (!pkts[newcnt]) { > dropped += batch->count - i; >@@ -1801,10 +1820,34 @@ 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(pkts[newcnt], void *), >- dp_packet_data(batch->packets[i]), size); >+ rte_pktmbuf_pkt_len(pkts[newcnt]) = size; >+ temp = pkts[newcnt]; >+ tmp_len = size < max_data_len ? size: max_data_len; >+ if (batch->packets[i]->source == DPBUF_DPDK) { Could you please add a comment explaining in which scenario this section is executed? From what I can tell, dpdk_do_tx_copy is invoked only from either netdev_dpdk_send__ and/or netdev_dpdk_vhost_send (as a result of the OUTPUT action) if one of the following two conditions are met: 1. the packet's 'source' != DPBUF_DPDK (N/A in this case) 2. 'may_steal' == false The only case I've observed in which the output action is invoked and where 'may_steal' is false is in the context of dpif_execute; however, in this case, I'm unsure if the packet's source is DPBUF_DPDK. Thanks in advance. >+ 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 { >+ 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; >+ } >+ } >+ > >- pkts[newcnt]->nb_segs = batch->packets[i]->mbuf.nb_segs; >+ pkts[newcnt]->nb_segs = nb_segs; > pkts[newcnt]->ol_flags = batch->packets[i]->mbuf.ol_flags; > pkts[newcnt]->packet_type = batch->packets[i]->mbuf.packet_type; > pkts[newcnt]->tx_offload = batch->packets[i]->mbuf.tx_offload; >-- >1.8.3.1
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 0485872..38ec2ed 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -1776,14 +1776,16 @@ 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; int dropped = 0; int newcnt = 0; - int i; + int i, j, nb_segs; dp_packet_batch_apply_cutlen(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", @@ -1793,7 +1795,24 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch *batch) continue; } - pkts[newcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp); + temp = pkts[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(pkts[newcnt]); + pkts[newcnt] = NULL; + break; + } + temp = temp->next; + } if (!pkts[newcnt]) { dropped += batch->count - i; @@ -1801,10 +1820,34 @@ 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(pkts[newcnt], void *), - dp_packet_data(batch->packets[i]), size); + rte_pktmbuf_pkt_len(pkts[newcnt]) = size; + temp = pkts[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 { + 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; + } + } + - pkts[newcnt]->nb_segs = batch->packets[i]->mbuf.nb_segs; + pkts[newcnt]->nb_segs = nb_segs; pkts[newcnt]->ol_flags = batch->packets[i]->mbuf.ol_flags; pkts[newcnt]->packet_type = batch->packets[i]->mbuf.packet_type; pkts[newcnt]->tx_offload = batch->packets[i]->mbuf.tx_offload;