[dpdk-dev,2/3] pcap: add support for jumbo frames

Message ID 1425044560-23397-3-git-send-email-tero.aho@coriant.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tero Aho Feb. 27, 2015, 1:42 p.m. UTC
  Extend eth_pcap_rx and eth_pcap_tx to support jumbo frames.
On receive side read large packets into multiple mbufs and
on the transmit side do the opposite.

Signed-off-by: Tero Aho <tero.aho@coriant.com>
---
 lib/librte_pmd_pcap/rte_eth_pcap.c | 88 ++++++++++++++++++++++++++++++++------
 1 file changed, 75 insertions(+), 13 deletions(-)

--
1.9.1


============================================================
The information contained in this message may be privileged
and confidential and protected from disclosure. If the reader
of this message is not the intended recipient, or an employee
or agent responsible for delivering this message to the
intended recipient, you are hereby notified that any reproduction,
dissemination or distribution of this communication is strictly
prohibited. If you have received this communication in error,
please notify us immediately by replying to the message and
deleting it from your computer. Thank you. Coriant-Tellabs
============================================================
  

Comments

John McNamara April 28, 2015, 1:52 p.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tero Aho
> Sent: Friday, February 27, 2015 1:43 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/3] pcap: add support for jumbo frames
> 
> Extend eth_pcap_rx and eth_pcap_tx to support jumbo frames.
> On receive side read large packets into multiple mbufs and on the transmit
> side do the opposite.
> 


> +eth_pcap_tx_jumbo(pcap_t *pcap,
> +                 struct rte_mbuf *mbuf) {
> +       u_char data[ETHER_MAX_JUMBO_FRAME_LEN];
> +       uint16_t data_len = 0;
> +
> +       while (mbuf != NULL) {
> +               /* There is no writev style function in libpcap, */
> +               /* we unfortunately have to copy data to a buffer. */
> +               rte_memcpy(data + data_len, rte_pktmbuf_mtod(mbuf, void
> *),
> +                          mbuf->data_len);
> +               data_len += mbuf->data_len;
> +               mbuf = mbuf->next;
> +       }

Hi Tero,

Not sure if it is possible in practice to overflow data[ETHER_MAX_JUMBO_FRAME_LEN] but there should probably be a check.

John.
--
  
Maxim Uvarov June 10, 2015, 9:08 a.m. UTC | #2
Patch looks good for me. But in latest dpdk file path changed, but it 
applies with small reject.

Reviewed-by: Maxim Uvarov <maxim.uvarov@linaro.org>

Maxim.

On 02/27/15 16:42, Tero Aho wrote:
> Extend eth_pcap_rx and eth_pcap_tx to support jumbo frames.
> On receive side read large packets into multiple mbufs and
> on the transmit side do the opposite.
>
> Signed-off-by: Tero Aho <tero.aho@coriant.com>
>
> ---
> lib/librte_pmd_pcap/rte_eth_pcap.c | 88 ++++++++++++++++++++++++++++++++------
>   1 file changed, 75 insertions(+), 13 deletions(-)
>
> --
> 1.9.1
>
>
> ============================================================
> The information contained in this message may be privileged
> and confidential and protected from disclosure. If the reader
> of this message is not the intended recipient, or an employee
> or agent responsible for delivering this message to the
> intended recipient, you are hereby notified that any reproduction,
> dissemination or distribution of this communication is strictly
> prohibited. If you have received this communication in error,
> please notify us immediately by replying to the message and
> deleting it from your computer. Thank you. Coriant-Tellabs
> ============================================================
>
> diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
> index 289af28..3f23f0a 100644
> --- a/lib/librte_pmd_pcap/rte_eth_pcap.c
> +++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
> @@ -52,7 +52,7 @@
>   #include <pcap.h>
>
>   #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
> -#define RTE_ETH_PCAP_SNAPLEN 4096
> +#define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
>   #define RTE_ETH_PCAP_PROMISC 1
>   #define RTE_ETH_PCAP_TIMEOUT -1
>   #define ETH_PCAP_RX_PCAP_ARG  "rx_pcap"
> @@ -132,6 +132,43 @@ static struct rte_eth_link pmd_link = {
>                  .link_status = 0
>   };
>
> +/*
> + * Helper function to read mbuf chain.
> + */
> +static int
> +eth_pcap_rx_jumbo(struct rte_mempool *mb_pool,
> +                 struct rte_mbuf *mbuf,
> +                 const u_char *data,
> +                 uint16_t data_len)
> +{
> +       struct rte_mbuf *m = mbuf;
> +       /* copy first segment */
> +       uint16_t len = rte_pktmbuf_tailroom(mbuf);
> +       rte_memcpy(rte_pktmbuf_append(mbuf, len), data, len);
> +       data_len -= len;
> +       data += len;
> +
> +       while (data_len > 0) {
> +               /* allocate next mbuf and point to that */
> +               m->next = rte_pktmbuf_alloc(mb_pool);
> +               if (unlikely(m->next == NULL))
> +                       return -1;
> +               m = m->next;
> +
> +               /* no headroom needed in chained mbufs */
> +               rte_pktmbuf_prepend(m, rte_pktmbuf_headroom(m));
> +               m->data_len = m->pkt_len = 0;
> +
> +               /* copy next segment */
> +               len = RTE_MIN(rte_pktmbuf_tailroom(m), data_len);
> +               rte_memcpy(rte_pktmbuf_append(m, len), data, len);
> +
> +               mbuf->nb_segs++;
> +               data_len -= len;
> +               data += len;
> +       }
> +       return mbuf->nb_segs;
> +}
>
>   static uint16_t
>   eth_pcap_rx(void *queue,
> @@ -173,17 +210,18 @@ eth_pcap_rx(void *queue,
>                          rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
>                                          header.len);
>                          mbuf->data_len = (uint16_t)header.len;
> -                       mbuf->pkt_len = mbuf->data_len;
> -                       mbuf->port = pcap_q->in_port;
> -                       bufs[num_rx] = mbuf;
> -                       num_rx++;
>                  } else {
> -                       /* pcap packet will not fit in the mbuf, so drop packet */
> -                       RTE_LOG(ERR, PMD,
> -                                       "PCAP packet %d bytes will not fit in mbuf (%d bytes)\n",
> -                                       header.len, buf_size);
> -                       rte_pktmbuf_free(mbuf);
> +                       /* pcap packet will not fit in a single mbuf */
> +                       if (unlikely(eth_pcap_rx_jumbo(pcap_q->mb_pool, mbuf,
> +                                       packet, header.len) == -1)) {
> +                               rte_pktmbuf_free(mbuf);
> +                               break;
> +                       }
>                  }
> +               mbuf->pkt_len = (uint16_t)header.len;
> +               mbuf->port = pcap_q->in_port;
> +               bufs[num_rx] = mbuf;
> +               num_rx++;
>          }
>          pcap_q->rx_pkts += num_rx;
>          return num_rx;
> @@ -241,6 +279,27 @@ eth_pcap_tx_dumper(void *queue,
>   }
>
>   /*
> + * Helper function to write mbuf chain.
> + */
> +static uint16_t
> +eth_pcap_tx_jumbo(pcap_t *pcap,
> +                 struct rte_mbuf *mbuf)
> +{
> +       u_char data[ETHER_MAX_JUMBO_FRAME_LEN];
> +       uint16_t data_len = 0;
> +
> +       while (mbuf != NULL) {
> +               /* There is no writev style function in libpcap, */
> +               /* we unfortunately have to copy data to a buffer. */
> +               rte_memcpy(data + data_len, rte_pktmbuf_mtod(mbuf, void *),
> +                          mbuf->data_len);
> +               data_len += mbuf->data_len;
> +               mbuf = mbuf->next;
> +       }
> +       return pcap_sendpacket(pcap, data, data_len);
> +}
> +
> +/*
>    * Callback to handle sending packets through a real NIC.
>    */
>   static uint16_t
> @@ -259,9 +318,12 @@ eth_pcap_tx(void *queue,
>
>          for (i = 0; i < nb_pkts; i++) {
>                  mbuf = bufs[i];
> -               ret = pcap_sendpacket(tx_queue->pcap,
> -                               rte_pktmbuf_mtod(mbuf, u_char *),
> -                               mbuf->data_len);
> +               if (likely(mbuf->nb_segs == 1))
> +                       ret = pcap_sendpacket(tx_queue->pcap,
> +                                             rte_pktmbuf_mtod(mbuf, u_char *),
> +                                             mbuf->data_len);
> +               else
> +                       ret = eth_pcap_tx_jumbo(tx_queue->pcap, mbuf);
>                  if (unlikely(ret != 0))
>                          break;
>                  num_tx++;
  
John McNamara June 10, 2015, 9:43 a.m. UTC | #3
> -----Original Message-----

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maxim Uvarov

> Sent: Wednesday, June 10, 2015 10:08 AM

> To: Tero Aho; dev@dpdk.org

> Subject: Re: [dpdk-dev] [dpdk-dev,2/3] pcap: add support for jumbo frames

> 

> Patch looks good for me. But in latest dpdk file path changed, but it

> applies with small reject.


Hi,

Thanks for the feedback.

I'll rebase that part of the patchset and resubmit it.

I'll review the status of the other 2 patches in the patchset as well.

John.
--
  

Patch

diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
index 289af28..3f23f0a 100644
--- a/lib/librte_pmd_pcap/rte_eth_pcap.c
+++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
@@ -52,7 +52,7 @@ 
 #include <pcap.h>

 #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
-#define RTE_ETH_PCAP_SNAPLEN 4096
+#define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
 #define RTE_ETH_PCAP_PROMISC 1
 #define RTE_ETH_PCAP_TIMEOUT -1
 #define ETH_PCAP_RX_PCAP_ARG  "rx_pcap"
@@ -132,6 +132,43 @@  static struct rte_eth_link pmd_link = {
                .link_status = 0
 };

+/*
+ * Helper function to read mbuf chain.
+ */
+static int
+eth_pcap_rx_jumbo(struct rte_mempool *mb_pool,
+                 struct rte_mbuf *mbuf,
+                 const u_char *data,
+                 uint16_t data_len)
+{
+       struct rte_mbuf *m = mbuf;
+       /* copy first segment */
+       uint16_t len = rte_pktmbuf_tailroom(mbuf);
+       rte_memcpy(rte_pktmbuf_append(mbuf, len), data, len);
+       data_len -= len;
+       data += len;
+
+       while (data_len > 0) {
+               /* allocate next mbuf and point to that */
+               m->next = rte_pktmbuf_alloc(mb_pool);
+               if (unlikely(m->next == NULL))
+                       return -1;
+               m = m->next;
+
+               /* no headroom needed in chained mbufs */
+               rte_pktmbuf_prepend(m, rte_pktmbuf_headroom(m));
+               m->data_len = m->pkt_len = 0;
+
+               /* copy next segment */
+               len = RTE_MIN(rte_pktmbuf_tailroom(m), data_len);
+               rte_memcpy(rte_pktmbuf_append(m, len), data, len);
+
+               mbuf->nb_segs++;
+               data_len -= len;
+               data += len;
+       }
+       return mbuf->nb_segs;
+}

 static uint16_t
 eth_pcap_rx(void *queue,
@@ -173,17 +210,18 @@  eth_pcap_rx(void *queue,
                        rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
                                        header.len);
                        mbuf->data_len = (uint16_t)header.len;
-                       mbuf->pkt_len = mbuf->data_len;
-                       mbuf->port = pcap_q->in_port;
-                       bufs[num_rx] = mbuf;
-                       num_rx++;
                } else {
-                       /* pcap packet will not fit in the mbuf, so drop packet */
-                       RTE_LOG(ERR, PMD,
-                                       "PCAP packet %d bytes will not fit in mbuf (%d bytes)\n",
-                                       header.len, buf_size);
-                       rte_pktmbuf_free(mbuf);
+                       /* pcap packet will not fit in a single mbuf */
+                       if (unlikely(eth_pcap_rx_jumbo(pcap_q->mb_pool, mbuf,
+                                       packet, header.len) == -1)) {
+                               rte_pktmbuf_free(mbuf);
+                               break;
+                       }
                }
+               mbuf->pkt_len = (uint16_t)header.len;
+               mbuf->port = pcap_q->in_port;
+               bufs[num_rx] = mbuf;
+               num_rx++;
        }
        pcap_q->rx_pkts += num_rx;
        return num_rx;
@@ -241,6 +279,27 @@  eth_pcap_tx_dumper(void *queue,
 }

 /*
+ * Helper function to write mbuf chain.
+ */
+static uint16_t
+eth_pcap_tx_jumbo(pcap_t *pcap,
+                 struct rte_mbuf *mbuf)
+{
+       u_char data[ETHER_MAX_JUMBO_FRAME_LEN];
+       uint16_t data_len = 0;
+
+       while (mbuf != NULL) {
+               /* There is no writev style function in libpcap, */
+               /* we unfortunately have to copy data to a buffer. */
+               rte_memcpy(data + data_len, rte_pktmbuf_mtod(mbuf, void *),
+                          mbuf->data_len);
+               data_len += mbuf->data_len;
+               mbuf = mbuf->next;
+       }
+       return pcap_sendpacket(pcap, data, data_len);
+}
+
+/*
  * Callback to handle sending packets through a real NIC.
  */
 static uint16_t
@@ -259,9 +318,12 @@  eth_pcap_tx(void *queue,

        for (i = 0; i < nb_pkts; i++) {
                mbuf = bufs[i];
-               ret = pcap_sendpacket(tx_queue->pcap,
-                               rte_pktmbuf_mtod(mbuf, u_char *),
-                               mbuf->data_len);
+               if (likely(mbuf->nb_segs == 1))
+                       ret = pcap_sendpacket(tx_queue->pcap,
+                                             rte_pktmbuf_mtod(mbuf, u_char *),
+                                             mbuf->data_len);
+               else
+                       ret = eth_pcap_tx_jumbo(tx_queue->pcap, mbuf);
                if (unlikely(ret != 0))
                        break;
                num_tx++;