Re: [PATCH 02/24] pcap: add pcap_iov()

[Stefano Brivio <sbrivio@redhat.com>]

On Fri,  2 Feb 2024 15:11:29 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  pcap.c | 32 ++++++++++++++++++++++++++++++++
>  pcap.h |  1 +
>  2 files changed, 33 insertions(+)
> 
> diff --git a/pcap.c b/pcap.c
> index 501d52d4992b..b002bb01314c 100644
> --- a/pcap.c
> +++ b/pcap.c
> @@ -31,6 +31,7 @@
>  #include "util.h"
>  #include "passt.h"
>  #include "log.h"
> +#include "iov.h"
>  
>  #define PCAP_VERSION_MINOR 4
>  
> @@ -130,6 +131,37 @@ void pcap_multiple(const struct iovec *iov, unsigned int n, size_t offset)
>  	}
>  }
>  
> +void pcap_iov(const struct iovec *iov, unsigned int n)
> +{
> +	struct timeval tv;
> +	struct pcap_pkthdr h;
> +	size_t len;
> +	unsigned int i;

From longest to shortest, rationale:

  https://hisham.hm/2018/06/16/when-listing-repeated-things-make-pyramids/

> +
> +	if (pcap_fd == -1)
> +		return;
> +
> +	gettimeofday(&tv, NULL);
> +
> +	len = iov_size(iov, n);
> +
> +	h.tv_sec = tv.tv_sec;
> +	h.tv_usec = tv.tv_usec;
> +	h.caplen = h.len = len;
> +
> +	if (write(pcap_fd, &h, sizeof(h)) < 0) {
> +		debug("Cannot write pcap header");
> +		return;
> +	}
> +
> +	for (i = 0; i < n; i++) {
> +		if (write(pcap_fd, iov[i].iov_base, iov[i].iov_len) < 0) {
> +			debug("Cannot log packet, iov %d length %lu\n",

%zu instead of %lu -- that guarantees the correct width for size_t on
different architectures (it's not always 'long'). And "iov %d" followed
by the length doesn't really tell that we're printing the index of the
vector, maybe something on the lines of:

			debug("Cannot log packet, length %zu, iovec index %i\n",

?
> +			      i, iov[i].iov_len);
> +		}
> +	}
> +}
> +
>  /**
>   * pcap_init() - Initialise pcap file
>   * @c:		Execution context
> diff --git a/pcap.h b/pcap.h
> index da5a7e846b72..732a0ddf14cc 100644
> --- a/pcap.h
> +++ b/pcap.h
> @@ -8,6 +8,7 @@
>  
>  void pcap(const char *pkt, size_t len);
>  void pcap_multiple(const struct iovec *iov, unsigned int n, size_t offset);
> +void pcap_iov(const struct iovec *iov, unsigned int n);
>  void pcap_init(struct ctx *c);
>  
>  #endif /* PCAP_H */

-- 
Stefano