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

Message ID 1537172589-19628-1-git-send-email-wangyunjian@huawei.com
State New
Headers show
Series
  • [ovs-dev] dpif-netdev: Fix "execute" packet length check.
Related show

Commit Message

wangyunjian Sept. 17, 2018, 8:23 a.m.
From: Yunjian Wang <wangyunjian@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@huawei.com>
---
 lib/dpif-netdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Ben Pfaff Sept. 18, 2018, 4:39 a.m. | #1
On Mon, Sep 17, 2018 at 04:23:09PM +0800, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian@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@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.

Patch

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;
     }