[vlc-devel] [PATCH] use overflow block on recvmsg() to adjust mtu (was: Re: [PATCH] derive truncated packet size in..)

[Ilkka Ollakka]

On Mon, Oct 29, 2018 at 01:11:49PM -0700, Nick Briggs wrote:

Hi,

> > On Oct 29, 2018, at 4:00 AM, vlc-devel-request at videolan.org wrote:

> > That sounds actually good idea.

> > Actually, why not just use that overflow_block and reallocate new one
> > for next overflow case, that way you wouldn't need to truncate the data.

> > Also you could use same approach in linux case and don't care about
> > MSG_TRUNC flag, but just check if received data did go to overflow_block
> > area and adjust mtu accordingly.


> We can also reduce the memory usage a little knowing that the
> maximum amount of user data in a UDP packet is 65507 bytes (a little less for IPv6)
> so the sum of regular and overflow block sizes doesn't need to exceed that.


> And here's your patch reworked with my memory reduction suggestion:

I think then approach is good. Not sure if Remi or Francois have any
opinion with this?

> From: Nick Briggs <nicholas.h.briggs at gmail.com>
> Subject: [PATCH] use overflow block on recvmsg() to adjust mtu

> Add second block on recvmsg() to check if we need to adjust mtu to be
> higher. With this approach, we will do extra memcpy and allocation in
> codepaths where mtu should be increased, but not on normal codepath.

> Allocates the minimum sized overflow block to capture the maximum
> sized datagram when combined with the normal (mtu sized) block.

> This uses the same approach on all platforms and doesn't rely on
> MSG_TRUNC in linux case and something else in other cases.
> ---
>  modules/access/udp.c | 50 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 16 deletions(-)

> diff --git a/modules/access/udp.c b/modules/access/udp.c
> index 654ecf8..a947bb2 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 *overflow_block;
>  } access_sys_t;

>  /*****************************************************************************
> @@ -104,6 +105,15 @@ static int Open( vlc_object_t *p_this )
>      if( unlikely( sys == NULL ) )
>          return VLC_ENOMEM;

> +    sys->mtu = 7 * 188;
> +
> +    /* Overflow can be max theoretical datagram content less anticipated MTU,
> +     *  IPv6 headers are larger than IPv4, ignore IPv6 jumbograms
> +     */
> +    sys->overflow_block = block_Alloc(65507 - sys->mtu);
> +    if( unlikely( sys->overflow_block == NULL ) )
> +        return VLC_ENOMEM;
> +
>      p_access->p_sys = sys;

>      /* Set up p_access */
> @@ -168,8 +178,6 @@ static int Open( vlc_object_t *p_this )
>          return VLC_EGENERIC;
>      }

> -    sys->mtu = 7 * 188;
> -
>      sys->timeout = var_InheritInteger( p_access, "udp-timeout");
>      if( sys->timeout > 0)
>          sys->timeout *= 1000;
> @@ -184,6 +192,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->overflow_block )
> +        block_Release( sys->overflow_block );

>      net_Close( sys->fd );
>  }
> @@ -231,20 +241,17 @@ 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 = {
> +    struct iovec iov[] = {{
>          .iov_base = pkt->p_buffer,
>          .iov_len = sys->mtu,
> -    };
> +    },{
> +        .iov_base = sys->overflow_block->p_buffer,
> +        .iov_len = sys->overflow_block->i_buffer,
> +    }};
>      struct msghdr msg = {
> -        .msg_iov = &iov,
> -        .msg_iovlen = 1,
> -        .msg_flags = trunc_flag,
> +        .msg_iov = iov,
> +        .msg_iovlen = 2,
>      };

>      struct pollfd ufd[1];
> @@ -262,7 +269,7 @@ static block_t *BlockUDP(stream_t *access, bool *restrict eof)
>              goto skip;
>       }

> -    ssize_t len = recvmsg(sys->fd, &msg, trunc_flag);
> +    ssize_t len = recvmsg(sys->fd, &msg, 0);

>      if (len < 0)
>      {
> @@ -271,11 +278,22 @@ skip:
>          return NULL;
>      }

> -    if (msg.msg_flags & trunc_flag)
> +    /* Received more than mtu amount,
> +     * we should gather blocks and increase mtu
> +     * and allocate new overflow block.  See Open()
> +     */
> +    if (unlikely(len > sys->mtu))
>      {
> -        msg_Err(access, "%zd bytes packet truncated (MTU was %zu)",
> +        msg_Warn(access, "%zd bytes packet received (MTU was %zu), adjusting mtu",
>                  len, sys->mtu);
> -        pkt->i_flags |= BLOCK_FLAG_CORRUPTED;
> +        block_t *gather_block = sys->overflow_block;
> +
> +        sys->overflow_block = block_Alloc(65507 - len);
> +
> +        gather_block->i_buffer = len - sys->mtu;
> +        pkt->p_next = gather_block;
> +        pkt = block_ChainGather( pkt );
> +
>          sys->mtu = len;
>      }
>      else
-- 
Ilkka Ollakka
When the candles are out all women are fair.
		-- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20181031/4baa1dd7/attachment.sig>