DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: Ajit Khaparde <ajit.khaparde@broadcom.com>,
	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>
Subject: Re: [dpdk-dev] [PATCH 3/5] net/bnxt: fix protocol size for VXLAN encap copy
Date: Wed, 18 Nov 2020 09:37:07 +0300
Message-ID: <34f0ab63-ca71-85ea-2fc6-840b1c030ffb@oktetlabs.ru> (raw)
In-Reply-To: <CACZ4nhtvgDXgyYZB=5-jfmDOT1VFPATQBvci6jn=TEZod0HxkA@mail.gmail.com>

On 11/18/20 3:34 AM, Ajit Khaparde wrote:
> 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.

I disagree with the point. It is typical duplication which
should be fixed. VXLAN item should consist of rte_vxlan_hdr
plus extra non header fields. In fact, similar thing should
be done for Ethernet flow item spec.

These items are used for VXLAN encap action and in current
state of definitions it is tricky to use VXLAN and Ethernet
items to construct header to be prepended. It is required
to copy field by field since it is bad to rely on the fact
that flow item starts from protocol header if it is not
specified in corresponding C type definition.

  reply	other threads:[~2020-11-18  6:37 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-16  7:55 [dpdk-dev] [PATCH 0/5] fix protocol size calculation Xiaoyu Min
2020-11-16  7:55 ` [dpdk-dev] [PATCH 1/5] net/mlx5: fix protocol size for raw encap judgement Xiaoyu Min
2020-11-17 13:25   ` Matan Azrad
2020-11-22 14:21     ` Thomas Monjalon
2020-11-22 15:32   ` Thomas Monjalon
2020-11-22 16:04     ` Matan Azrad
2020-11-23  7:54       ` Ori Kam
2020-11-23  8:12         ` Thomas Monjalon
2020-11-16  7:55 ` [dpdk-dev] [PATCH 2/5] app/flow-perf: fix protocol size for raw encap Xiaoyu Min
2020-11-16  7:55 ` [dpdk-dev] [PATCH 3/5] net/bnxt: fix protocol size for VXLAN encap copy Xiaoyu Min
2020-11-16 16:12   ` Ferruh Yigit
2020-11-18  0:34     ` Ajit Khaparde
2020-11-18  6:37       ` Andrew Rybchenko [this message]
2020-11-18 17:44         ` Ajit Khaparde
2020-11-16  7:55 ` [dpdk-dev] [PATCH 4/5] net/iavf: fix protocol size for virtchnl copy Xiaoyu Min
2020-11-16 16:23   ` Ferruh Yigit
2020-11-22 13:28     ` Jack Min
2020-11-22 14:15       ` Thomas Monjalon
2020-11-23 10:02         ` Ferruh Yigit
2020-11-23 10:49   ` Ferruh Yigit
2020-11-24 21:58     ` Thomas Monjalon
2020-11-27  1:17   ` Xing, Beilei
2020-11-16  7:55 ` [dpdk-dev] [PATCH 5/5] net/softnic: update headers size calculation Xiaoyu Min
2020-11-16 16:27   ` Ferruh Yigit
2020-11-16 19:09     ` Dekel Peled
2020-11-17 10:30       ` Ferruh Yigit
2020-11-17 12:41         ` Dekel Peled
2020-11-17 15:43           ` Singh, Jasvinder
2020-11-18  2:23   ` Zhou, JunX W
2020-11-22 16:11 ` [dpdk-dev] [PATCH 0/5] fix protocol " Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=34f0ab63-ca71-85ea-2fc6-840b1c030ffb@oktetlabs.ru \
    --to=andrew.rybchenko@oktetlabs.ru \
    --cc=ajit.khaparde@broadcom.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jackmin@mellanox.com \
    --cc=jackmin@nvidia.com \
    --cc=somnath.kotur@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git