[vlc-devel] [PATCH 1/2] access/udp: use MSG_PEEK for non-linux cases to check new size

[Nick Briggs]

Here's an alternate idea to handle the non-Linux case:

Since you're allocating the peek_block, make your iov array 2 elements, add the peek buffer as the second element, and just check if the returned length is greater than the primary block you were reading into. MSG_TRUNC will never be set. It would then mirror the logic of the Linux case with the MSG_TRUNC in the recvmsg() flags, and it doesn't depend on PEEKing the *next* block to see if it is also bigger than the expected MTU. 


Date: Fri, 26 Oct 2018 20:26:25 +0300
> From: Ilkka Ollakka <ileoo at videolan.org>
> To: vlc-devel at videolan.org
> Subject: [vlc-devel] [PATCH 1/2] access/udp: use MSG_PEEK for
> 	non-linux cases to check new size
> Message-ID: <20181026172626.17043-1-ileoo at videolan.org>
> 
> ---
> modules/access/udp.c | 48 ++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/modules/access/udp.c b/modules/access/udp.c
> index 654ecf83c3..ccae192587 100644
> --- a/modules/access/udp.c
> +++ b/modules/access/udp.c
> @@ -81,6 +81,7 @@ typedef struct
>     int fd;
>     int timeout;
>     size_t mtu;
> +    block_t *peek_block;
> } access_sys_t;
> 
> /*****************************************************************************
> @@ -104,6 +105,15 @@ static int Open( vlc_object_t *p_this )
>     if( unlikely( sys == NULL ) )
>         return VLC_ENOMEM;
> 
> +    sys->peek_block = NULL;
> +#ifdef MSG_TRUNC
> +# ifndef __linux__
> +    sys->peek_block = block_Alloc(65536);
> +    if( unlikely( sys->peek_block == NULL ) )
> +        return VLC_ENOMEM;
> +# endif
> +#endif
> +
>     p_access->p_sys = sys;
> 
>     /* Set up p_access */
> @@ -184,6 +194,8 @@ static void Close( vlc_object_t *p_this )
> {
>     stream_t     *p_access = (stream_t*)p_this;
>     access_sys_t *sys = p_access->p_sys;
> +    if( sys->peek_block )
> +        block_Release( sys->peek_block );
> 
>     net_Close( sys->fd );
> }
> @@ -231,11 +243,6 @@ static block_t *BlockUDP(stream_t *access, bool *restrict eof)
>         return NULL;
>     }
> 
> -#ifdef __linux__
> -    const int trunc_flag = MSG_TRUNC;
> -#else
> -    const int trunc_flag = 0;
> -#endif
> 
>     struct iovec iov = {
>         .iov_base = pkt->p_buffer,
> @@ -244,7 +251,6 @@ static block_t *BlockUDP(stream_t *access, bool *restrict eof)
>     struct msghdr msg = {
>         .msg_iov = &iov,
>         .msg_iovlen = 1,
> -        .msg_flags = trunc_flag,
>     };
> 
>     struct pollfd ufd[1];
> @@ -262,6 +268,15 @@ static block_t *BlockUDP(stream_t *access, bool *restrict eof)
>             goto skip;
>      }
> 
> +#ifdef __linux__
> +    /* We define MSG_TRUNC only for linux
> +     * as for other systems it's not defined flag for input
> +     */
> +    const int trunc_flag = MSG_TRUNC;
> +#else
> +    const int trunc_flag = 0;
> +#endif
> +
>     ssize_t len = recvmsg(sys->fd, &msg, trunc_flag);
> 
>     if (len < 0)
> @@ -271,14 +286,31 @@ skip:
>         return NULL;
>     }
> 
> -    if (msg.msg_flags & trunc_flag)
> +#ifdef MSG_TRUNC
> +    if (msg.msg_flags & MSG_TRUNC)
>     {
> +        pkt->i_flags |= BLOCK_FLAG_CORRUPTED;
> +        /* On linux systems MSG_TRUNC tells the available data,
> +           on other cases, I think we need to use MSG_PEEK to get how much
> +           there would be availabled in case of trunced message.
> +           In MSG_PEEK case, assumption is also, that in real world
> +           the size don't fluctuate huge amounts in streaming and the result
> +           is most likely correct mtu size if current was truncated.
> +         */
> +# ifdef __linux__
>         msg_Err(access, "%zd bytes packet truncated (MTU was %zu)",
>                 len, sys->mtu);
> -        pkt->i_flags |= BLOCK_FLAG_CORRUPTED;
>         sys->mtu = len;
> +# else
> +        size_t old_mtu = sys->mtu;
> +        sys->mtu = recv(sys->fd, sys->peek_block->p_buffer, sys->peek_block->i_buffer, MSG_PEEK);
> +
> +        msg_Err(access, "%zd bytes packet truncated (MTU was %zu)",
> +                sys->mtu, old_mtu);
> +# endif
>     }
>     else
> +#endif
>         pkt->i_buffer = len;
> 
>     return pkt;
> -- 
> 2.19.1
>