> -----Original Message-----
> From: Akihiko Odaki <akihiko.od...@daynix.com>
> Sent: Friday, 14 April 2023 13:37
> Cc: Sriram Yagnaraman <sriram.yagnara...@est.tech>; Jason Wang
> <jasow...@redhat.com>; Dmitry Fleytman <dmitry.fleyt...@gmail.com>;
> Michael S. Tsirkin <m...@redhat.com>; Alex Bennée <alex.ben...@linaro.org>;
> Philippe Mathieu-Daudé <phi...@linaro.org>; Thomas Huth
> <th...@redhat.com>; Wainer dos Santos Moschetta
> <waine...@redhat.com>; Beraldo Leal <bl...@redhat.com>; Cleber Rosa
> <cr...@redhat.com>; Laurent Vivier <lviv...@redhat.com>; Paolo Bonzini
> <pbonz...@redhat.com>; qemu-devel@nongnu.org; Akihiko Odaki
> <akihiko.od...@daynix.com>
> Subject: [PATCH 06/40] net/net_rx_pkt: Use iovec for
> net_rx_pkt_set_protocols()
>
> igb does not properly ensure the buffer passed to
> net_rx_pkt_set_protocols() is contiguous for the entire L2/L3/L4 header.
> Allow it to pass scattered data to net_rx_pkt_set_protocols().
>
> Fixes: 3a977deebe ("Intrdocue igb device emulation")
> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
> ---
> hw/net/igb_core.c | 2 +-
> hw/net/net_rx_pkt.c | 14 +++++--------- hw/net/net_rx_pkt.h | 10 ++++++----
> hw/net/virtio-net.c | 7 +++++--
> hw/net/vmxnet3.c | 7 ++++++-
> include/net/eth.h | 6 +++---
> net/eth.c | 18 ++++++++----------
> 7 files changed, 34 insertions(+), 30 deletions(-)
>
Very nice.
Reviewed-by: Sriram Yagnaraman <sriram.yagnara...@est.tech>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> 5d4884b834..53f60fc3d3 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -1650,7 +1650,7 @@ igb_receive_internal(IGBCore *core, const struct
> iovec *iov, int iovcnt,
>
> ehdr = PKT_GET_ETH_HDR(filter_buf);
> net_rx_pkt_set_packet_type(core->rx_pkt, get_eth_packet_type(ehdr));
> - net_rx_pkt_set_protocols(core->rx_pkt, filter_buf, size);
> + net_rx_pkt_set_protocols(core->rx_pkt, iov, iovcnt, iov_ofs);
>
> queues = igb_receive_assign(core, ehdr, size, &rss_info, external_tx);
> if (!queues) {
> diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c index
> 39cdea06de..63be6e05ad 100644
> --- a/hw/net/net_rx_pkt.c
> +++ b/hw/net/net_rx_pkt.c
> @@ -103,7 +103,7 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt,
> iov, iovcnt, ploff, pkt->tot_len);
> }
>
> - eth_get_protocols(pkt->vec, pkt->vec_len, &pkt->hasip4, &pkt->hasip6,
> + eth_get_protocols(pkt->vec, pkt->vec_len, 0, &pkt->hasip4,
> + &pkt->hasip6,
> &pkt->l3hdr_off, &pkt->l4hdr_off, &pkt->l5hdr_off,
> &pkt->ip6hdr_info, &pkt->ip4hdr_info,
> &pkt->l4hdr_info);
>
> @@ -186,17 +186,13 @@ size_t net_rx_pkt_get_total_len(struct NetRxPkt
> *pkt)
> return pkt->tot_len;
> }
>
> -void net_rx_pkt_set_protocols(struct NetRxPkt *pkt, const void *data,
> - size_t len)
> +void net_rx_pkt_set_protocols(struct NetRxPkt *pkt,
> + const struct iovec *iov, size_t iovcnt,
> + size_t iovoff)
> {
> - const struct iovec iov = {
> - .iov_base = (void *)data,
> - .iov_len = len
> - };
> -
> assert(pkt);
>
> - eth_get_protocols(&iov, 1, &pkt->hasip4, &pkt->hasip6,
> + eth_get_protocols(iov, iovcnt, iovoff, &pkt->hasip4, &pkt->hasip6,
> &pkt->l3hdr_off, &pkt->l4hdr_off, &pkt->l5hdr_off,
> &pkt->ip6hdr_info, &pkt->ip4hdr_info,
> &pkt->l4hdr_info); } diff --
> git a/hw/net/net_rx_pkt.h b/hw/net/net_rx_pkt.h index
> d00b484900..a06f5c2675 100644
> --- a/hw/net/net_rx_pkt.h
> +++ b/hw/net/net_rx_pkt.h
> @@ -55,12 +55,14 @@ size_t net_rx_pkt_get_total_len(struct NetRxPkt *pkt);
> * parse and set packet analysis results
> *
> * @pkt: packet
> - * @data: pointer to the data buffer to be parsed
> - * @len: data length
> + * @iov: received data scatter-gather list
> + * @iovcnt: number of elements in iov
> + * @iovoff: data start offset in the iov
> *
> */
> -void net_rx_pkt_set_protocols(struct NetRxPkt *pkt, const void *data,
> - size_t len);
> +void net_rx_pkt_set_protocols(struct NetRxPkt *pkt,
> + const struct iovec *iov, size_t iovcnt,
> + size_t iovoff);
>
> /**
> * fetches packet analysis results
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index
> 53e1c32643..37551fd854 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1835,9 +1835,12 @@ static int virtio_net_process_rss(NetClientState
> *nc, const uint8_t *buf,
> VIRTIO_NET_HASH_REPORT_UDPv6,
> VIRTIO_NET_HASH_REPORT_UDPv6_EX
> };
> + struct iovec iov = {
> + .iov_base = (void *)buf,
> + .iov_len = size
> + };
>
> - net_rx_pkt_set_protocols(pkt, buf + n->host_hdr_len,
> - size - n->host_hdr_len);
> + net_rx_pkt_set_protocols(pkt, &iov, 1, n->host_hdr_len);
> net_rx_pkt_get_protocols(pkt, &hasip4, &hasip6, &l4hdr_proto);
> net_hash_type = virtio_net_get_hash_type(hasip4, hasip6, l4hdr_proto,
> n->rss_data.hash_types); diff
> --git
> a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 9acff310e7..05f41b6dfa
> 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2001,7 +2001,12 @@ vmxnet3_receive(NetClientState *nc, const uint8_t
> *buf, size_t size)
> get_eth_packet_type(PKT_GET_ETH_HDR(buf)));
>
> if (vmxnet3_rx_filter_may_indicate(s, buf, size)) {
> - net_rx_pkt_set_protocols(s->rx_pkt, buf, size);
> + struct iovec iov = {
> + .iov_base = (void *)buf,
> + .iov_len = size
> + };
> +
> + net_rx_pkt_set_protocols(s->rx_pkt, &iov, 1, 0);
> vmxnet3_rx_need_csum_calculate(s->rx_pkt, buf, size);
> net_rx_pkt_attach_data(s->rx_pkt, buf, size, s->rx_vlan_stripping);
> bytes_indicated = vmxnet3_indicate_packet(s) ? size : -1; diff --git
> a/include/net/eth.h b/include/net/eth.h index c5ae4493b4..9f19c3a695
> 100644
> --- a/include/net/eth.h
> +++ b/include/net/eth.h
> @@ -312,10 +312,10 @@ eth_get_l2_hdr_length(const void *p) }
>
> static inline uint32_t
> -eth_get_l2_hdr_length_iov(const struct iovec *iov, int iovcnt)
> +eth_get_l2_hdr_length_iov(const struct iovec *iov, size_t iovcnt,
> +size_t iovoff)
> {
> uint8_t p[sizeof(struct eth_header) + sizeof(struct vlan_header)];
> - size_t copied = iov_to_buf(iov, iovcnt, 0, p, ARRAY_SIZE(p));
> + size_t copied = iov_to_buf(iov, iovcnt, iovoff, p, ARRAY_SIZE(p));
>
> if (copied < ARRAY_SIZE(p)) {
> return copied;
> @@ -397,7 +397,7 @@ typedef struct eth_l4_hdr_info_st {
> bool has_tcp_data;
> } eth_l4_hdr_info;
>
> -void eth_get_protocols(const struct iovec *iov, int iovcnt,
> +void eth_get_protocols(const struct iovec *iov, size_t iovcnt, size_t
> +iovoff,
> bool *hasip4, bool *hasip6,
> size_t *l3hdr_off,
> size_t *l4hdr_off, diff --git a/net/eth.c b/net/eth.c
> index
> 70bcd8e355..d7b30df79f 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -136,7 +136,7 @@ _eth_tcp_has_data(bool is_ip4,
> return l4len > TCP_HEADER_DATA_OFFSET(tcp); }
>
> -void eth_get_protocols(const struct iovec *iov, int iovcnt,
> +void eth_get_protocols(const struct iovec *iov, size_t iovcnt, size_t
> +iovoff,
> bool *hasip4, bool *hasip6,
> size_t *l3hdr_off,
> size_t *l4hdr_off, @@ -147,26 +147,24 @@ void
> eth_get_protocols(const struct iovec *iov, int iovcnt, {
> int proto;
> bool fragment = false;
> - size_t l2hdr_len = eth_get_l2_hdr_length_iov(iov, iovcnt);
> size_t input_size = iov_size(iov, iovcnt);
> size_t copied;
> uint8_t ip_p;
>
> *hasip4 = *hasip6 = false;
> + *l3hdr_off = iovoff + eth_get_l2_hdr_length_iov(iov, iovcnt,
> + iovoff);
> l4hdr_info->proto = ETH_L4_HDR_PROTO_INVALID;
>
> - proto = eth_get_l3_proto(iov, iovcnt, l2hdr_len);
> -
> - *l3hdr_off = l2hdr_len;
> + proto = eth_get_l3_proto(iov, iovcnt, *l3hdr_off);
>
> if (proto == ETH_P_IP) {
> struct ip_header *iphdr = &ip4hdr_info->ip4_hdr;
>
> - if (input_size < l2hdr_len) {
> + if (input_size < *l3hdr_off) {
> return;
> }
>
> - copied = iov_to_buf(iov, iovcnt, l2hdr_len, iphdr, sizeof(*iphdr));
> + copied = iov_to_buf(iov, iovcnt, *l3hdr_off, iphdr,
> + sizeof(*iphdr));
> if (copied < sizeof(*iphdr) ||
> IP_HEADER_VERSION(iphdr) != IP_HEADER_VERSION_4) {
> return;
> @@ -175,17 +173,17 @@ void eth_get_protocols(const struct iovec *iov, int
> iovcnt,
> *hasip4 = true;
> ip_p = iphdr->ip_p;
> ip4hdr_info->fragment = IP4_IS_FRAGMENT(iphdr);
> - *l4hdr_off = l2hdr_len + IP_HDR_GET_LEN(iphdr);
> + *l4hdr_off = *l3hdr_off + IP_HDR_GET_LEN(iphdr);
>
> fragment = ip4hdr_info->fragment;
> } else if (proto == ETH_P_IPV6) {
> - if (!eth_parse_ipv6_hdr(iov, iovcnt, l2hdr_len, ip6hdr_info)) {
> + if (!eth_parse_ipv6_hdr(iov, iovcnt, *l3hdr_off, ip6hdr_info))
> + {
> return;
> }
>
> *hasip6 = true;
> ip_p = ip6hdr_info->l4proto;
> - *l4hdr_off = l2hdr_len + ip6hdr_info->full_hdr_len;
> + *l4hdr_off = *l3hdr_off + ip6hdr_info->full_hdr_len;
> fragment = ip6hdr_info->fragment;
> } else {
> return;
> --
> 2.40.0
