Re: [PATCH v2] net: remove an assert call in eth_get_gso_type

[Peter Maydell]

On Tue, 20 Oct 2020 at 15:05, P J P <ppandit@redhat.com> wrote:
>
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> eth_get_gso_type() routine returns segmentation offload type based on
> L3 protocol type. It calls g_assert_not_reached if L3 protocol is
> unknown, making the following return statement unreachable. Remove the
> g_assert call, as it maybe triggered by a guest user.
>
> Reported-by: Gaoning Pan <pgn@zju.edu.cn>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  net/eth.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> Update v2: add qemu_log()
>   -> https://lists.nongnu.org/archive/html/qemu-devel/2020-10/msg05576.html
>
> diff --git a/net/eth.c b/net/eth.c
> index 0c1d413ee2..fd76e349eb 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -16,6 +16,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "net/eth.h"
>  #include "net/checksum.h"
>  #include "net/tap.h"
> @@ -71,9 +72,7 @@ eth_get_gso_type(uint16_t l3_proto, uint8_t *l3_hdr, 
> uint8_t l4proto)
>              return VIRTIO_NET_HDR_GSO_TCPV6 | ecn_state;
>          }
>      }
> -
> -    /* Unsupported offload */
> -    g_assert_not_reached();
> +    qemu_log("Probably not GSO frame, unknown L3 protocol: %hd\n", l3_proto);

It's generally not a good idea to use qemu_log() without a
particular mask, as then it will get printed if the user turns
on any logging but not otherwise.

If the guest must have done something wrong to get us here:
 use LOG_GUEST_ERROR
If this is some functionality we ought to implement but have
not, and so something will now be broken:
 use LOG_UNIMP
If the fallback for what happens in this situation is fine,
and maybe it's just suboptimal performance, or an unusual
case that might be interesting to know about but which
we're handling within the spec:
 consider a tracepoint instead

thanks
-- PMM