RE: [PATCH 03/40] igb: Fix Rx packet type encoding

[Sriram Yagnaraman]

> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Friday, 14 April 2023 13:37
> Cc: Sriram Yagnaraman <sriram.yagnaraman@est.tech>; Jason Wang
> <jasowang@redhat.com>; Dmitry Fleytman <dmitry.fleytman@gmail.com>;
> Michael S. Tsirkin <mst@redhat.com>; Alex Bennée <alex.bennee@linaro.org>;
> Philippe Mathieu-Daudé <philmd@linaro.org>; Thomas Huth
> <thuth@redhat.com>; Wainer dos Santos Moschetta
> <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>; Cleber Rosa
> <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>; Paolo Bonzini
> <pbonzini@redhat.com>; qemu-devel@nongnu.org; Akihiko Odaki
> <akihiko.odaki@daynix.com>
> Subject: [PATCH 03/40] igb: Fix Rx packet type encoding
> 
> igb's advanced descriptor uses a packet type encoding different from one used
> in e1000e's extended descriptor. Fix the logic to encode Rx packet type
> accordingly.
> 
> Fixes: 3a977deebe ("Intrdocue igb device emulation")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  hw/net/igb_core.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> 464a41d0aa..55de212447 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -1227,7 +1227,6 @@ igb_build_rx_metadata(IGBCore *core,
>      struct virtio_net_hdr *vhdr;
>      bool hasip4, hasip6;
>      EthL4HdrProto l4hdr_proto;
> -    uint32_t pkt_type;
> 
>      *status_flags = E1000_RXD_STAT_DD;
> 
> @@ -1266,28 +1265,29 @@ igb_build_rx_metadata(IGBCore *core,
>          trace_e1000e_rx_metadata_ack();
>      }
> 
> -    if (hasip6 && (core->mac[RFCTL] & E1000_RFCTL_IPV6_DIS)) {
> -        trace_e1000e_rx_metadata_ipv6_filtering_disabled();
> -        pkt_type = E1000_RXD_PKT_MAC;
> -    } else if (l4hdr_proto == ETH_L4_HDR_PROTO_TCP ||
> -               l4hdr_proto == ETH_L4_HDR_PROTO_UDP) {
> -        pkt_type = hasip4 ? E1000_RXD_PKT_IP4_XDP :
> E1000_RXD_PKT_IP6_XDP;
> -    } else if (hasip4 || hasip6) {
> -        pkt_type = hasip4 ? E1000_RXD_PKT_IP4 : E1000_RXD_PKT_IP6;
> -    } else {
> -        pkt_type = E1000_RXD_PKT_MAC;
> -    }
> +    if (pkt_info) {
> +        *pkt_info = rss_info->enabled ? rss_info->type : 0;
> 
> -    trace_e1000e_rx_metadata_pkt_type(pkt_type);
> +        if (hasip4) {
> +            *pkt_info |= BIT(4);

DPDK seems to care about the packet type. 😊
Would it make sense to introduce a new set of macros similar to E1000_RXD_PKT* 
for igb instead of these magic numbers?
In any case, 
Reviewed-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>

> +        }
> 
> -    if (pkt_info) {
> -        if (rss_info->enabled) {
> -            *pkt_info = rss_info->type;
> +        if (hasip6) {
> +            *pkt_info |= BIT(6);
>          }
> 
> -        *pkt_info |= (pkt_type << 4);
> -    } else {
> -        *status_flags |= E1000_RXD_PKT_TYPE(pkt_type);
> +        switch (l4hdr_proto) {
> +        case ETH_L4_HDR_PROTO_TCP:
> +            *pkt_info |= BIT(8);
> +            break;
> +
> +        case ETH_L4_HDR_PROTO_UDP:
> +            *pkt_info |= BIT(9);
> +            break;
> +
> +        default:
> +            break;
> +        }
>      }
> 
>      if (hdr_info) {
> --
> 2.40.0