Re: [Qemu-devel] [PATCH] slirp: fix clearing ifq_so from pending packets

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

Hi Sam, thanks for this patch :)

On 08/25/2017 07:37 PM, Samuel Thibault wrote:
> The if_fastq and if_batchq contain not only packets, but queues of packets
> for the same socket. When sofree frees a socket, it thus has to clear ifq_so
> from all the packets from the queues, not only the first.
> 
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

so let's raise this to:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   slirp/socket.c | 39 +++++++++++++++++++++++----------------
>   1 file changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/slirp/socket.c b/slirp/socket.c
> index ecec0295a9..cb7b5b608d 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -59,6 +59,27 @@ socreate(Slirp *slirp)
>     return(so);
>   }
>   
> +/*
> + * Remove references to so from the given message queue.
> + */
> +static void
> +soqfree(struct socket *so, struct quehead *qh)
> +{
> +    struct mbuf *ifq;
> +
> +    for (ifq = (struct mbuf *) qh->qh_link;
> +             (struct quehead *) ifq != qh;
> +             ifq = ifq->ifq_next) {
> +        if (ifq->ifq_so == so) {
> +            struct mbuf *ifm;
> +            ifq->ifq_so = NULL;
> +            for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
> +                ifm->ifq_so = NULL;
> +            }
> +        }
> +    }
> +}
> +
>   /*
>    * remque and free a socket, clobber cache
>    */
> @@ -66,23 +87,9 @@ void
>   sofree(struct socket *so)
>   {
>     Slirp *slirp = so->slirp;
> -  struct mbuf *ifm;
>   
> -  for (ifm = (struct mbuf *) slirp->if_fastq.qh_link;
> -       (struct quehead *) ifm != &slirp->if_fastq;
> -       ifm = ifm->ifq_next) {
> -    if (ifm->ifq_so == so) {
> -      ifm->ifq_so = NULL;
> -    }
> -  }
> -
> -  for (ifm = (struct mbuf *) slirp->if_batchq.qh_link;
> -       (struct quehead *) ifm != &slirp->if_batchq;
> -       ifm = ifm->ifq_next) {
> -    if (ifm->ifq_so == so) {
> -      ifm->ifq_so = NULL;
> -    }
> -  }
> +  soqfree(so, &slirp->if_fastq);
> +  soqfree(so, &slirp->if_batchq);
>   
>     if (so->so_emu==EMU_RSH && so->extra) {
>   	sofree(so->extra);
>