app/testpmd: set raw cmd use rte hdr struct
Checks
Commit Message
From: Xiaoyu Min <jackmin@nvidia.com>
The rte_flow_item_eth and rte_flow_item_vlan items are refined [1].
The structs do not exactly represent the packet bits captured on the wire
anymore so set raw_encap/decap commands should only copy real header
instead of the whole struct.
Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
[1]:
commit 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
items")
Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
---
app/test-pmd/cmdline_flow.c | 72 +++++++++++++++++++------------------
1 file changed, 38 insertions(+), 34 deletions(-)
Comments
On 11/3/2020 1:20 PM, Xiaoyu Min wrote:
> From: Xiaoyu Min <jackmin@nvidia.com>
>
> The rte_flow_item_eth and rte_flow_item_vlan items are refined [1].
> The structs do not exactly represent the packet bits captured on the wire
> anymore so set raw_encap/decap commands should only copy real header
> instead of the whole struct.
>
> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
>
> [1]:
> commit 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> items")
>
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
Hi Dekel, Ori,
This is second fix [1] we get related to the "rte_flow_item_xxx" size changes
[2] from you.
The assumption that 'rte_flow_item_xxx' size is same as xxx header size seems
implemented in multiple locations, and changing 'rte_flow_item_xxx' struct size
breaking those usage.
Can one of you guys spend some time to actively check all possible breakages?
They may be very hard to find unless someone explicitly check for this.
Thanks,
ferruh
[1]
first one: https://patches.dpdk.org/patch/82863/
[2]
Fixes: ad976bd40d28 ("ethdev: add extensions attributes to IPv6 item")
Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
On 11/3/2020 1:20 PM, Xiaoyu Min wrote:
> From: Xiaoyu Min <jackmin@nvidia.com>
>
> The rte_flow_item_eth and rte_flow_item_vlan items are refined [1].
> The structs do not exactly represent the packet bits captured on the wire
> anymore so set raw_encap/decap commands should only copy real header
> instead of the whole struct.
>
> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
>
> [1]:
> commit 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> items")
>
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/main, thanks.
On 11/3/2020 2:27 PM, Ferruh Yigit wrote:
> On 11/3/2020 1:20 PM, Xiaoyu Min wrote:
>> From: Xiaoyu Min <jackmin@nvidia.com>
>>
>> The rte_flow_item_eth and rte_flow_item_vlan items are refined [1].
>> The structs do not exactly represent the packet bits captured on the wire
>> anymore so set raw_encap/decap commands should only copy real header
>> instead of the whole struct.
>>
>> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
>>
>> [1]:
>> commit 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
>> items")
>>
>> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
>
>
> Hi Dekel, Ori,
>
> This is second fix [1] we get related to the "rte_flow_item_xxx" size changes
> [2] from you.
>
> The assumption that 'rte_flow_item_xxx' size is same as xxx header size seems
> implemented in multiple locations, and changing 'rte_flow_item_xxx' struct size
> breaking those usage.
>
> Can one of you guys spend some time to actively check all possible breakages?
> They may be very hard to find unless someone explicitly check for this.
>
Hi Dekel, Ori,
Is there anyone actively checking for this?
> Thanks,
> ferruh
>
>
> [1]
> first one: https://patches.dpdk.org/patch/82863/
>
>
> [2]
> Fixes: ad976bd40d28 ("ethdev: add extensions attributes to IPv6 item")
> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
Hi Ferruh,
Sorry for the late response, we are checking for additional impacts of this change.
Looks like there is another fix needed, after all checks are completed we'll submit all relevant patches.
Regards,
Dekel
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, November 9, 2020 12:05 PM
> To: Xiaoyu Min <jackmin@mellanox.com>; Ori Kam <orika@nvidia.com>;
> Wenzhuo Lu <wenzhuo.lu@intel.com>; Beilei Xing <beilei.xing@intel.com>;
> Bernard Iremonger <bernard.iremonger@intel.com>; Matan Azrad
> <matan@nvidia.com>; Dekel Peled <dekelp@nvidia.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>
> Cc: dev@dpdk.org; Jack Min <jackmin@nvidia.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Hyong Youb Kim <hyonkim@cisco.com>; John
> Daley <johndale@cisco.com>; Asaf Penso <asafp@nvidia.com>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: set raw cmd use rte hdr
> struct
>
> On 11/3/2020 2:27 PM, Ferruh Yigit wrote:
> > On 11/3/2020 1:20 PM, Xiaoyu Min wrote:
> >> From: Xiaoyu Min <jackmin@nvidia.com>
> >>
> >> The rte_flow_item_eth and rte_flow_item_vlan items are refined [1].
> >> The structs do not exactly represent the packet bits captured on the
> >> wire anymore so set raw_encap/decap commands should only copy real
> >> header instead of the whole struct.
> >>
> >> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> >>
> >> [1]:
> >> commit 09315fc83861 ("ethdev: add VLAN attributes to ethernet and
> >> VLAN
> >> items")
> >>
> >> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> >
> >
> > Hi Dekel, Ori,
> >
> > This is second fix [1] we get related to the "rte_flow_item_xxx" size
> > changes [2] from you.
> >
> > The assumption that 'rte_flow_item_xxx' size is same as xxx header
> > size seems implemented in multiple locations, and changing
> > 'rte_flow_item_xxx' struct size breaking those usage.
> >
> > Can one of you guys spend some time to actively check all possible
> breakages?
> > They may be very hard to find unless someone explicitly check for this.
> >
>
> Hi Dekel, Ori,
>
> Is there anyone actively checking for this?
>
>
> > Thanks,
> > ferruh
> >
> >
> > [1]
> > first one:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hes.dpdk.org%2Fpatch%2F82863%2F&data=04%7C01%7Cdekelp%40nvi
> dia.com
> >
> %7C3ab20dfba7264766e7e808d88496fab7%7C43083d15727340c1b7db39efd9c
> cc17a
> >
> %7C0%7C0%7C637405131289676761%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAw
> >
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda
> ta=xF
> >
> xO%2BmzQP6bjS9BE1Eg9fd1pd8VPLNHV%2FJm83cwJ2Xw%3D&reserve
> d=0
> >
> >
> > [2]
> > Fixes: ad976bd40d28 ("ethdev: add extensions attributes to IPv6 item")
> > Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> > items")
On 11/9/2020 11:14 AM, Dekel Peled wrote:
> Hi Ferruh,
>
> Sorry for the late response, we are checking for additional impacts of this change.
> Looks like there is another fix needed, after all checks are completed we'll submit all relevant patches.
>
Thanks Dekel, appreciated.
> Regards,
> Dekel
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Monday, November 9, 2020 12:05 PM
>> To: Xiaoyu Min <jackmin@mellanox.com>; Ori Kam <orika@nvidia.com>;
>> Wenzhuo Lu <wenzhuo.lu@intel.com>; Beilei Xing <beilei.xing@intel.com>;
>> Bernard Iremonger <bernard.iremonger@intel.com>; Matan Azrad
>> <matan@nvidia.com>; Dekel Peled <dekelp@nvidia.com>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>
>> Cc: dev@dpdk.org; Jack Min <jackmin@nvidia.com>; Andrew Rybchenko
>> <arybchenko@solarflare.com>; NBU-Contact-Thomas Monjalon
>> <thomas@monjalon.net>; Hyong Youb Kim <hyonkim@cisco.com>; John
>> Daley <johndale@cisco.com>; Asaf Penso <asafp@nvidia.com>
>> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: set raw cmd use rte hdr
>> struct
>>
>> On 11/3/2020 2:27 PM, Ferruh Yigit wrote:
>>> On 11/3/2020 1:20 PM, Xiaoyu Min wrote:
>>>> From: Xiaoyu Min <jackmin@nvidia.com>
>>>>
>>>> The rte_flow_item_eth and rte_flow_item_vlan items are refined [1].
>>>> The structs do not exactly represent the packet bits captured on the
>>>> wire anymore so set raw_encap/decap commands should only copy real
>>>> header instead of the whole struct.
>>>>
>>>> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
>>>>
>>>> [1]:
>>>> commit 09315fc83861 ("ethdev: add VLAN attributes to ethernet and
>>>> VLAN
>>>> items")
>>>>
>>>> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
>>>
>>>
>>> Hi Dekel, Ori,
>>>
>>> This is second fix [1] we get related to the "rte_flow_item_xxx" size
>>> changes [2] from you.
>>>
>>> The assumption that 'rte_flow_item_xxx' size is same as xxx header
>>> size seems implemented in multiple locations, and changing
>>> 'rte_flow_item_xxx' struct size breaking those usage.
>>>
>>> Can one of you guys spend some time to actively check all possible
>> breakages?
>>> They may be very hard to find unless someone explicitly check for this.
>>>
>>
>> Hi Dekel, Ori,
>>
>> Is there anyone actively checking for this?
>>
>>
>>> Thanks,
>>> ferruh
>>>
>>>
>>> [1]
>>> first one:
>>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>>
>> hes.dpdk.org%2Fpatch%2F82863%2F&data=04%7C01%7Cdekelp%40nvi
>> dia.com
>>>
>> %7C3ab20dfba7264766e7e808d88496fab7%7C43083d15727340c1b7db39efd9c
>> cc17a
>>>
>> %7C0%7C0%7C637405131289676761%7CUnknown%7CTWFpbGZsb3d8eyJWIj
>> oiMC4wLjAw
>>>
>> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda
>> ta=xF
>>>
>> xO%2BmzQP6bjS9BE1Eg9fd1pd8VPLNHV%2FJm83cwJ2Xw%3D&reserve
>> d=0
>>>
>>>
>>> [2]
>>> Fixes: ad976bd40d28 ("ethdev: add extensions attributes to IPv6 item")
>>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
>>> items")
>
@@ -24,6 +24,10 @@
#include <rte_flow.h>
#include <rte_hexdump.h>
#include <rte_vxlan.h>
+#include <rte_gre.h>
+#include <rte_mpls.h>
+#include <rte_gtp.h>
+#include <rte_geneve.h>
#include "testpmd.h"
@@ -7361,42 +7365,42 @@ cmdline_parse_inst_t cmd_flow = {
static void
update_fields(uint8_t *buf, struct rte_flow_item *item, uint16_t next_proto)
{
- struct rte_flow_item_ipv4 *ipv4;
- struct rte_flow_item_eth *eth;
- struct rte_flow_item_ipv6 *ipv6;
- struct rte_flow_item_vxlan *vxlan;
- struct rte_flow_item_vxlan_gpe *gpe;
+ struct rte_ipv4_hdr *ipv4;
+ struct rte_ether_hdr *eth;
+ struct rte_ipv6_hdr *ipv6;
+ struct rte_vxlan_hdr *vxlan;
+ struct rte_vxlan_gpe_hdr *gpe;
struct rte_flow_item_nvgre *nvgre;
uint32_t ipv6_vtc_flow;
switch (item->type) {
case RTE_FLOW_ITEM_TYPE_ETH:
- eth = (struct rte_flow_item_eth *)buf;
+ eth = (struct rte_ether_hdr *)buf;
if (next_proto)
- eth->type = rte_cpu_to_be_16(next_proto);
+ eth->ether_type = rte_cpu_to_be_16(next_proto);
break;
case RTE_FLOW_ITEM_TYPE_IPV4:
- ipv4 = (struct rte_flow_item_ipv4 *)buf;
- ipv4->hdr.version_ihl = 0x45;
- if (next_proto && ipv4->hdr.next_proto_id == 0)
- ipv4->hdr.next_proto_id = (uint8_t)next_proto;
+ ipv4 = (struct rte_ipv4_hdr *)buf;
+ ipv4->version_ihl = 0x45;
+ if (next_proto && ipv4->next_proto_id == 0)
+ ipv4->next_proto_id = (uint8_t)next_proto;
break;
case RTE_FLOW_ITEM_TYPE_IPV6:
- ipv6 = (struct rte_flow_item_ipv6 *)buf;
- if (next_proto && ipv6->hdr.proto == 0)
- ipv6->hdr.proto = (uint8_t)next_proto;
- ipv6_vtc_flow = rte_be_to_cpu_32(ipv6->hdr.vtc_flow);
+ ipv6 = (struct rte_ipv6_hdr *)buf;
+ if (next_proto && ipv6->proto == 0)
+ ipv6->proto = (uint8_t)next_proto;
+ ipv6_vtc_flow = rte_be_to_cpu_32(ipv6->vtc_flow);
ipv6_vtc_flow &= 0x0FFFFFFF; /*< reset version bits. */
ipv6_vtc_flow |= 0x60000000; /*< set ipv6 version. */
- ipv6->hdr.vtc_flow = rte_cpu_to_be_32(ipv6_vtc_flow);
+ ipv6->vtc_flow = rte_cpu_to_be_32(ipv6_vtc_flow);
break;
case RTE_FLOW_ITEM_TYPE_VXLAN:
- vxlan = (struct rte_flow_item_vxlan *)buf;
- vxlan->flags = 0x08;
+ vxlan = (struct rte_vxlan_hdr *)buf;
+ vxlan->vx_flags = 0x08;
break;
case RTE_FLOW_ITEM_TYPE_VXLAN_GPE:
- gpe = (struct rte_flow_item_vxlan_gpe *)buf;
- gpe->flags = 0x0C;
+ gpe = (struct rte_vxlan_gpe_hdr *)buf;
+ gpe->vx_flags = 0x0C;
break;
case RTE_FLOW_ITEM_TYPE_NVGRE:
nvgre = (struct rte_flow_item_nvgre *)buf;
@@ -7605,36 +7609,36 @@ cmd_set_raw_parsed(const struct buffer *in)
item->spec = flow_item_default_mask(item);
switch (item->type) {
case RTE_FLOW_ITEM_TYPE_ETH:
- size = sizeof(struct rte_flow_item_eth);
+ size = sizeof(struct rte_ether_hdr);
break;
case RTE_FLOW_ITEM_TYPE_VLAN:
- size = sizeof(struct rte_flow_item_vlan);
+ size = sizeof(struct rte_vlan_hdr);
proto = RTE_ETHER_TYPE_VLAN;
break;
case RTE_FLOW_ITEM_TYPE_IPV4:
- size = sizeof(struct rte_flow_item_ipv4);
+ size = sizeof(struct rte_ipv4_hdr);
proto = RTE_ETHER_TYPE_IPV4;
break;
case RTE_FLOW_ITEM_TYPE_IPV6:
- size = sizeof(struct rte_flow_item_ipv6);
+ size = sizeof(struct rte_ipv6_hdr);
proto = RTE_ETHER_TYPE_IPV6;
break;
case RTE_FLOW_ITEM_TYPE_UDP:
- size = sizeof(struct rte_flow_item_udp);
+ size = sizeof(struct rte_udp_hdr);
proto = 0x11;
break;
case RTE_FLOW_ITEM_TYPE_TCP:
- size = sizeof(struct rte_flow_item_tcp);
+ size = sizeof(struct rte_tcp_hdr);
proto = 0x06;
break;
case RTE_FLOW_ITEM_TYPE_VXLAN:
- size = sizeof(struct rte_flow_item_vxlan);
+ size = sizeof(struct rte_vxlan_hdr);
break;
case RTE_FLOW_ITEM_TYPE_VXLAN_GPE:
- size = sizeof(struct rte_flow_item_vxlan_gpe);
+ size = sizeof(struct rte_vxlan_gpe_hdr);
break;
case RTE_FLOW_ITEM_TYPE_GRE:
- size = sizeof(struct rte_flow_item_gre);
+ size = sizeof(struct rte_gre_hdr);
proto = 0x2F;
break;
case RTE_FLOW_ITEM_TYPE_GRE_KEY:
@@ -7642,7 +7646,7 @@ cmd_set_raw_parsed(const struct buffer *in)
proto = 0x0;
break;
case RTE_FLOW_ITEM_TYPE_MPLS:
- size = sizeof(struct rte_flow_item_mpls);
+ size = sizeof(struct rte_mpls_hdr);
proto = 0x0;
break;
case RTE_FLOW_ITEM_TYPE_NVGRE:
@@ -7650,14 +7654,14 @@ cmd_set_raw_parsed(const struct buffer *in)
proto = 0x2F;
break;
case RTE_FLOW_ITEM_TYPE_GENEVE:
- size = sizeof(struct rte_flow_item_geneve);
+ size = sizeof(struct rte_geneve_hdr);
break;
case RTE_FLOW_ITEM_TYPE_L2TPV3OIP:
- size = sizeof(struct rte_flow_item_l2tpv3oip);
+ size = sizeof(rte_be32_t);
proto = 0x73;
break;
case RTE_FLOW_ITEM_TYPE_ESP:
- size = sizeof(struct rte_flow_item_esp);
+ size = sizeof(struct rte_esp_hdr);
proto = 0x32;
break;
case RTE_FLOW_ITEM_TYPE_AH:
@@ -7665,7 +7669,7 @@ cmd_set_raw_parsed(const struct buffer *in)
proto = 0x33;
break;
case RTE_FLOW_ITEM_TYPE_GTP:
- size = sizeof(struct rte_flow_item_gtp);
+ size = sizeof(struct rte_gtp_hdr);
break;
case RTE_FLOW_ITEM_TYPE_PFCP:
size = sizeof(struct rte_flow_item_pfcp);