From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 63F55A04E6;
	Wed, 18 Nov 2020 01:34:58 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id D4A79567C;
	Wed, 18 Nov 2020 01:34:55 +0100 (CET)
Received: from mail-ot1-f68.google.com (mail-ot1-f68.google.com
 [209.85.210.68]) by dpdk.org (Postfix) with ESMTP id AE41137B1
 for <dev@dpdk.org>; Wed, 18 Nov 2020 01:34:52 +0100 (CET)
Received: by mail-ot1-f68.google.com with SMTP id 79so93173otc.7
 for <dev@dpdk.org>; Tue, 17 Nov 2020 16:34:52 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google;
 h=mime-version:references:in-reply-to:from:date:message-id:subject:to
 :cc; bh=iAW/ENTPVsNCLNHqtoAa22WgLwH6AicpPeLX5hgIEPg=;
 b=KSq1TkyTzWJuihXSZpOvCNhAc7B8pU4OIOQJHdvcUygcouwUeOMAHX8jWUu2LJ+Was
 cEM0ibC1ApFnUnE4DAkwWtrMMb3eI+pns7r791qEdlZYFkmskktK+K9uCNWGwFUK6b5R
 YFPaL7xnFOirwqc+Iz3NHEim2PJo0/AHUb/2M=
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=iAW/ENTPVsNCLNHqtoAa22WgLwH6AicpPeLX5hgIEPg=;
 b=adeldJ/OL3FRf1Sd8sNEx5u3VIpoqNAhLroIbsumcmxtrS7hsLXK0Q1m68DTGpszwz
 sq8YmI+hUmj45UPC6QfF+29RhCFibl3eIIMHDb+7s4/SIthve8LZ66WcOmF/D7YHkT+O
 QBFN1PBT+Tpkk2SGWHDTMexhy83uihc10ArK/LXIzUDUhQwMOK+zFuGLTaKQ1zgShGHt
 pxne/IhSl+zV2kvpX0ZhMI3DZPNFGBm1d3CuemmIRSI9jY5VW8enKnQAd4KizuuiPRqa
 JLweRinQS5QXAEkKi8YsGHf3lqX/t0jLTJo4B9mSxiegCk/0+n4Ez5yO1mlW1pUwAr94
 45HQ==
X-Gm-Message-State: AOAM532ELA65U9P3gpjZYJtSHIjVtNUxX1sGizJ/j6Pfsrq694z+VnAj
 EnktjBG0BpR0gHG0wqG/Dx0Slmt44IQVjSSmkfhSFA==
X-Google-Smtp-Source: ABdhPJx5bPUNGcfpVBpeXCBlzDzlVfubFgrjMz0cN8FcZ7OG1XXsS9PH5Gmf5Q2kiKf9n+nL4z7WbMXHyLBrmpMW6us=
X-Received: by 2002:a9d:12f7:: with SMTP id g110mr4599009otg.95.1605659691823; 
 Tue, 17 Nov 2020 16:34:51 -0800 (PST)
MIME-Version: 1.0
References: <cover.1605493464.git.jackmin@nvidia.com>
 <141eb0140301108b1320ecfd93c89e48b02cd546.1605493464.git.jackmin@nvidia.com>
 <b18c18eb-ad02-39f1-3dda-43d05d0b1a8c@intel.com>
In-Reply-To: <b18c18eb-ad02-39f1-3dda-43d05d0b1a8c@intel.com>
From: Ajit Khaparde <ajit.khaparde@broadcom.com>
Date: Tue, 17 Nov 2020 16:34:35 -0800
Message-ID: <CACZ4nhtvgDXgyYZB=5-jfmDOT1VFPATQBvci6jn=TEZod0HxkA@mail.gmail.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Xiaoyu Min <jackmin@mellanox.com>,
 Somnath Kotur <somnath.kotur@broadcom.com>, 
 dpdk-dev <dev@dpdk.org>, Xiaoyu Min <jackmin@nvidia.com>
Content-Type: text/plain; charset="UTF-8"
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
Subject: Re: [dpdk-dev] [PATCH 3/5] net/bnxt: fix protocol size for VXLAN
 encap copy
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Nov 16, 2020 at 8:13 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 11/16/2020 7:55 AM, Xiaoyu Min wrote:
> > From: Xiaoyu Min <jackmin@nvidia.com>
> >
> > The rte_flow_item_eth and rte_flow_item_vlan items are refined.
> > The structs do not exactly represent the packet bits captured on the
> > wire anymore so should only copy real header instead of the whole struct.
> >
> > Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> >
> > Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> items")
> >
> > Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
>
> <...>
>
> > @@ -1726,7 +1727,7 @@ ulp_rte_vxlan_encap_act_handler(const struct
> rte_flow_action *action_item,
> >               BNXT_TF_DBG(ERR, "vxlan encap does not have vni\n");
> >               return BNXT_TF_RC_ERROR;
> >       }
> > -     vxlan_size = sizeof(struct rte_flow_item_vxlan);
> > +     vxlan_size = sizeof(struct rte_vxlan_hdr);
> >       /* copy the vxlan details */
> >       memcpy(&vxlan_spec, item->spec, vxlan_size);
> >       vxlan_spec.flags = 0x08;
> >
>
> 'vxlan_size' seems used both to copy rt_flow_item [1] and to header [2].
> Also
> ''vxlan_size' is used to copy the 'vxlan_spec'.
>
> Since both "struct rte_flow_item_vxlan" & "struct rte_vxlan_hdr" size is
> same,
> this should work fine, but I guess it may be broken if sizes of those two
> structures changes in the future.
>
> [1]
> memcpy(&vxlan_spec, item->spec, vxlan_size);
>
> [2]
> ulp_encap_buffer_copy(buff, (const uint8_t *)&vxlan_spec,
>         vxlan_size, ULP_BUFFER_ALIGN_8_BYTE);
>
Also, I feel rte_flow_item_vxlan is a control plane structure while
rte_vlan_hdr is more for dataplane.
It's better to keep them separate to allow refining them independently.