On 7/25/2019 1:04 PM, David Marchand wrote:
> Two cores can send multi segment packets on two different pcap ports.
> Because of this, we can't have one single buffer to linearize packets.
>
> Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
> and remove eth_pcap_gather_data() when necessary (if the mbuf is
> contiguous, rte_pktmbuf_read() just points at the buffer address).
>
> With this change, we won't support mono segment mbuf larger than 16k.
>
> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
Thanks David, lgtm, only a few minor syntax issues, please check below.
<...>
> @@ -336,31 +322,27 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs,
> uint16_t nb_pkts)
> * dumper */
> for (i = 0; i < nb_pkts; i++) {
> mbuf = bufs[i];
> + len = rte_pktmbuf_pkt_len(mbuf);
> + if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
> + len > sizeof(temp_data))) {
> + PMD_LOG(ERR, "Dropping multi segment PCAP packet. Size
> (%zd) > max size (%zd).",
Can we break the line to reduce the line length:
PMD_LOG(ERR,
"Dropping multi segment PCAP packet. Size (%zd) > max size (%zd).",
> + len, sizeof(temp_data));
> + rte_pktmbuf_free(mbuf);
> + continue;
> + }
> +
> calculate_timestamp(&header.ts);
> - header.len = mbuf->pkt_len;
> + header.len = len;
> header.caplen = header.len;
> -
> - if (likely(mbuf->nb_segs == 1)) {
> - pcap_dump((u_char *)dumper, &header,
> - rte_pktmbuf_mtod(mbuf, void*));
DPDK coding convention requires a tab, instead of aligning to the parenthesis.
<...>
> + /* rte_pktmbuf_read() returns a pointer to the data directly
> + * in the mbuf (when the mbuf is contiguous) or, otherwise,
> + * a pointer to temp_data after copying into it.
> + */
> + pcap_dump((u_char *)dumper, &header,
> + rte_pktmbuf_read(mbuf, 0, len, temp_data));
Same here, not need to align to the parenthesis.
<...>
> + len = rte_pktmbuf_pkt_len(mbuf);
> + if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
> + len > sizeof(temp_data))) {
> + PMD_LOG(ERR, "Dropping multi segment PCAP packet. Size
> (%zd) > max size (%zd).",
> + len, sizeof(temp_data));
ditto
> + rte_pktmbuf_free(mbuf);
> + continue;
> }
>
> + /* rte_pktmbuf_read() returns a pointer to the data directly
> + * in the mbuf (when the mbuf is contiguous) or, otherwise,
> + * a pointer to temp_data after copying into it.
> + */
> + ret = pcap_sendpacket(pcap,
> + rte_pktmbuf_read(mbuf, 0, len, temp_data),
> + len);
ditto
