[dpdk-dev] [PATCH] pcap: fix captured frame length
On Thu, Jan 28, 2016 at 06:14:45PM +, Nicolas Pernas Maradei wrote: > Hi Dror, > > Good catch. What you are saying makes sense and it is also explained in > pcap's documentation. Was your setup unusual though? > This might sound like a silly question but I don't remember seeing that > issue and I should have since your fix is correct. > > Nico. > Applied to dpdk-next-net/rel_16_04 /Bruce
[dpdk-dev] [PATCH] pcap: fix captured frame length
Hi,
I'm just adding the Acked-by line to the patch. Apologise I missed that one
earlier.
Nico.
On 28/01/16 11:09, Dror Birkman wrote:
> The actual captured length is header.caplen, whereas header.len is
> the original length on the wire.
>
> Signed-off-by: Dror Birkman
> Acked-by: Nicolas Pernas Maradei
> ---
>
>
> Without this fix, if the captured length is smaller than the original
> length on the wire, mbuf will contain incorrect data.
>
>
> drivers/net/pcap/rte_eth_pcap.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index f9230eb..1d121f8 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -220,25 +220,25 @@ eth_pcap_rx(void *queue,
> buf_size =
> (uint16_t)(rte_pktmbuf_data_room_size(pcap_q->mb_pool) -
> RTE_PKTMBUF_HEADROOM);
>
> - if (header.len <= buf_size) {
> + if (header.caplen <= buf_size) {
> /* pcap packet will fit in the mbuf, go ahead and copy
> */
> rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
> - header.len);
> - mbuf->data_len = (uint16_t)header.len;
> + header.caplen);
> + mbuf->data_len = (uint16_t)header.caplen;
> } else {
> /* Try read jumbo frame into multi mbufs. */
> if (unlikely(eth_pcap_rx_jumbo(pcap_q->mb_pool,
> mbuf,
> packet,
> -header.len) == -1))
> +header.caplen) == -1))
> break;
> }
>
> - mbuf->pkt_len = (uint16_t)header.len;
> + mbuf->pkt_len = (uint16_t)header.caplen;
> mbuf->port = pcap_q->in_port;
> bufs[num_rx] = mbuf;
> num_rx++;
> - rx_bytes += header.len;
> + rx_bytes += header.caplen;
> }
> pcap_q->rx_pkts += num_rx;
> pcap_q->rx_bytes += rx_bytes;
[dpdk-dev] [PATCH] pcap: fix captured frame length
On Thu, Jan 28, 2016 at 06:14:45PM +, Nicolas Pernas Maradei wrote: > Hi Dror, > > Good catch. What you are saying makes sense and it is also explained in > pcap's documentation. Was your setup unusual though? > This might sound like a silly question but I don't remember seeing that > issue and I should have since your fix is correct. > > Nico. > Hi Nico, I assume from your reply that you are ok with this patch. Can you please confirm this by acking it using the proper form "Acked-by: " line. Thanks, /Bruce
[dpdk-dev] [PATCH] pcap: fix captured frame length
Hi Nicolas,
Thanks.
I tried to test my application with a pcap file I had, but got weird
results, so I wrote back the rte_mbufs in pcap format and hex compared it
to my input pcap. Only then I noticed that my original pcap had frames that
were captured with a length smaller than the frame length.
I guess you can call it pure (bad) luck :)
Dror
On Thu, Jan 28, 2016 at 8:00 PM, Nicolas Pernas Maradei <
nicolas.pernas.maradei at emutex.com> wrote:
> Hi Dror,
>
> Good catch. What you are saying makes sense and it is also explained in
> pcap's documentation. Was your setup unusual though?
> This might sound like a silly question but I don't remember seeing that
> issue and I should have since your fix is correct.
>
> Nico.
>
> On 28/01/16 08:09, Dror Birkman wrote:
>
> The actual captured length is header.caplen, whereas header.len is
> the original length on the wire.
>
> Signed-off-by: Dror Birkman lightcyber.com>
> ---
>
>
> Without this fix, if the captured length is smaller than the original
> length on the wire, mbuf will contain incorrect data.
>
>
> drivers/net/pcap/rte_eth_pcap.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index f9230eb..1d121f8 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -220,25 +220,25 @@ eth_pcap_rx(void *queue,
> buf_size =
> (uint16_t)(rte_pktmbuf_data_room_size(pcap_q->mb_pool) -
> RTE_PKTMBUF_HEADROOM);
>
> - if (header.len <= buf_size) {
> + if (header.caplen <= buf_size) {
> /* pcap packet will fit in the mbuf, go ahead and copy
> */
> rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
> - header.len);
> - mbuf->data_len = (uint16_t)header.len;
> + header.caplen);
> + mbuf->data_len = (uint16_t)header.caplen;
> } else {
> /* Try read jumbo frame into multi mbufs. */
> if (unlikely(eth_pcap_rx_jumbo(pcap_q->mb_pool,
> mbuf,
> packet,
> -header.len) == -1))
> +header.caplen) == -1))
> break;
> }
>
> - mbuf->pkt_len = (uint16_t)header.len;
> + mbuf->pkt_len = (uint16_t)header.caplen;
> mbuf->port = pcap_q->in_port;
> bufs[num_rx] = mbuf;
> num_rx++;
> - rx_bytes += header.len;
> + rx_bytes += header.caplen;
> }
> pcap_q->rx_pkts += num_rx;
> pcap_q->rx_bytes += rx_bytes;
>
>
> - no title specified @page { } table { border-collapse:collapse;
> border-spacing:0; empty-cells:show } td, th { vertical-align:top;
> font-size:12pt;} h1, h2, h3, h4, h5, h6 { clear:both } ol, ul { margin:0;
> padding:0;} li { list-style: none; margin:0; padding:0;} li span. { clear: both; line-height:0;
> width:0; height:0; margin:0; padding:0; } span.footnodeNumber {
> padding-right:1em; } span.annotation_style_by_filter { font-size:95%;
> font-family:Arial; background-color:#fff000; margin:0; border:0; padding:0;
> } * { margin:0;} .P1_borderStart { font-size:12pt; line-height:120%;
> margin-top:0cm; font-family:Liberation Serif; writing-mode:page;
> margin-left:0.3cm; margin-right:0cm; text-indent:0cm;
> background-color:transparent; padding-bottom:0.247cm;
> border-bottom-style:none; } .P1 { font-size:12pt; line-height:120%;
> font-family:Liberation Serif; writing-mode:page; margin-left:0.3cm;
> margin-right:0cm; text-indent:0cm; background-color:transparent;
> padding-bottom:0.247cm; padding-top:0cm; border-top-style:none;
> border-bottom-style:none; } .P1_borderEnd { font-size:12pt;
> line-height:120%; margin-bottom:0.247cm; font-family:Liberation Serif;
> writing-mode:page; margin-left:0.3cm; margin-right:0cm; text-indent:0cm;
> background-color:transparent; padding-top:0cm; border-top-style:none;}
> .P2_borderStart { font-size:12pt; line-height:120%; margin-top:0cm;
> font-family:Liberation Serif; writing-mode:page; margin-left:0.3cm;
> margin-right:0cm; text-indent:0cm; background-color:transparent;
> padding-bottom:0.247cm; border-bottom-style:none; } .P2 { font-size:12pt;
> line-height:120%; font-family:Liberation Serif; writing-mode:page;
> margin-left:0.3cm; margin-right:0cm; text-indent:0cm;
> background-color:transparent; padding-bottom:0.247cm; padding-top:0cm;
> border-top-style:none; border-bottom-style:none; } .P2_borderEnd {
> font-size:12pt; line-height:120%; margin-bottom:0.247cm;
> font-family:Liberation Serif; writing-mode:page; margin-left:0.3cm;
> margin-right:0cm; text-indent:0cm;
[dpdk-dev] [PATCH] pcap: fix captured frame length
Hi Dror,
Good catch. What you are saying makes sense and it is also explained in
pcap's documentation. Was your setup unusual though?
This might sound like a silly question but I don't remember seeing that
issue and I should have since your fix is correct.
Nico.
On 28/01/16 11:09, Dror Birkman wrote:
> The actual captured length is header.caplen, whereas header.len is
> the original length on the wire.
>
> Signed-off-by: Dror Birkman
> ---
>
>
> Without this fix, if the captured length is smaller than the original
> length on the wire, mbuf will contain incorrect data.
>
>
> drivers/net/pcap/rte_eth_pcap.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index f9230eb..1d121f8 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -220,25 +220,25 @@ eth_pcap_rx(void *queue,
> buf_size =
> (uint16_t)(rte_pktmbuf_data_room_size(pcap_q->mb_pool) -
> RTE_PKTMBUF_HEADROOM);
>
> - if (header.len <= buf_size) {
> + if (header.caplen <= buf_size) {
> /* pcap packet will fit in the mbuf, go ahead and copy
> */
> rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
> - header.len);
> - mbuf->data_len = (uint16_t)header.len;
> + header.caplen);
> + mbuf->data_len = (uint16_t)header.caplen;
> } else {
> /* Try read jumbo frame into multi mbufs. */
> if (unlikely(eth_pcap_rx_jumbo(pcap_q->mb_pool,
> mbuf,
> packet,
> -header.len) == -1))
> +header.caplen) == -1))
> break;
> }
>
> - mbuf->pkt_len = (uint16_t)header.len;
> + mbuf->pkt_len = (uint16_t)header.caplen;
> mbuf->port = pcap_q->in_port;
> bufs[num_rx] = mbuf;
> num_rx++;
> - rx_bytes += header.len;
> + rx_bytes += header.caplen;
> }
> pcap_q->rx_pkts += num_rx;
> pcap_q->rx_bytes += rx_bytes;
[dpdk-dev] [PATCH] pcap: fix captured frame length
The actual captured length is header.caplen, whereas header.len is
the original length on the wire.
Signed-off-by: Dror Birkman
---
Without this fix, if the captured length is smaller than the original
length on the wire, mbuf will contain incorrect data.
drivers/net/pcap/rte_eth_pcap.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index f9230eb..1d121f8 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -220,25 +220,25 @@ eth_pcap_rx(void *queue,
buf_size =
(uint16_t)(rte_pktmbuf_data_room_size(pcap_q->mb_pool) -
RTE_PKTMBUF_HEADROOM);
- if (header.len <= buf_size) {
+ if (header.caplen <= buf_size) {
/* pcap packet will fit in the mbuf, go ahead and copy
*/
rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
- header.len);
- mbuf->data_len = (uint16_t)header.len;
+ header.caplen);
+ mbuf->data_len = (uint16_t)header.caplen;
} else {
/* Try read jumbo frame into multi mbufs. */
if (unlikely(eth_pcap_rx_jumbo(pcap_q->mb_pool,
mbuf,
packet,
- header.len) == -1))
+ header.caplen) == -1))
break;
}
- mbuf->pkt_len = (uint16_t)header.len;
+ mbuf->pkt_len = (uint16_t)header.caplen;
mbuf->port = pcap_q->in_port;
bufs[num_rx] = mbuf;
num_rx++;
- rx_bytes += header.len;
+ rx_bytes += header.caplen;
}
pcap_q->rx_pkts += num_rx;
pcap_q->rx_bytes += rx_bytes;
--
2.6.3
