[ovs-dev] [PATCH] dpif-netdev: Fix "execute" packet length check.

[Ben Pfaff]

On Mon, Sep 17, 2018 at 04:23:09PM +0800, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian at huawei.com>
> 
> The length check is wrong for immediate arguments to "execute" packet.
> The ethernet header length should be considered.
> 
> Signed-off-by: Yunjian Wang <wangyunjian at huawei.com>
> ---
>  lib/dpif-netdev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 9fb82cc..6522f27 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3632,9 +3632,11 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute)
>      struct dp_netdev *dp = get_dp_netdev(dpif);
>      struct dp_netdev_pmd_thread *pmd;
>      struct dp_packet_batch pp;
> +    int n_vlan = flow_count_vlan_headers(execute->flow);
>  
>      if (dp_packet_size(execute->packet) < ETH_HEADER_LEN ||
> -        dp_packet_size(execute->packet) > UINT16_MAX) {
> +        dp_packet_size(execute->packet) > UINT16_MAX +
> +        ETH_HEADER_LEN + VLAN_HEADER_LEN * n_vlan) {
>          return EINVAL;
>      }

I was taking the 64-kB limit as a limit on the size of a buffer.  This
interprets it as a MTU limit ignoring L2 headers.  Can you explain the
importance and significance of the change?

Before I read the patch in detail, I expected it to change the "<
ETH_HEADER_LEN" to something more like "< ETH_HEADER_LEN + 4 * n_vlan".
I'm still a little surprised it doesn't do that.