Re: [dpdk-dev] [dpdk-dev v4 1/3] net/iavf: refactor for hash flow

[Jeff Guo <jia.guo@intel.com>]

hi, qi

On 7/3/2020 10:24 PM, Zhang, Qi Z wrote:
>
>> -----Original Message-----
>> From: Guo, Jia <jia.guo@intel.com>
>> Sent: Friday, July 3, 2020 11:57 AM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing
>> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
>> Cc: dev@dpdk.org; Guo, Junfeng <junfeng.guo@intel.com>; Yang, Qiming
>> <qiming.yang@intel.com>; Su, Simei <simei.su@intel.com>; Guo, Jia
>> <jia.guo@intel.com>
>> Subject: [dpdk-dev v4 1/3] net/iavf: refactor for hash flow
>>
>> Refactor hash flow by change the process of the pattern parser and the
>> action parser, and refine the lookup table for regular IP and GTPU_EH.
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>>   drivers/net/iavf/iavf_hash.c | 1658 ++++++++++++++++++++++++----------
>>   1 file changed, 1163 insertions(+), 495 deletions(-)
>>
>> diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
>> index a7691ef0c..921dc961f 100644
>> --- a/drivers/net/iavf/iavf_hash.c
>> +++ b/drivers/net/iavf/iavf_hash.c
>> @@ -24,32 +24,33 @@
>>   #include "iavf_generic_flow.h"
>>
>>   enum iavf_pattern_hint_type {
>> -	IAVF_PATTERN_HINT_NONE,
>> -	IAVF_PATTERN_HINT_IPV4,
>> -	IAVF_PATTERN_HINT_IPV4_UDP,
>> -	IAVF_PATTERN_HINT_IPV4_TCP,
>> -	IAVF_PATTERN_HINT_IPV4_SCTP,
>> -	IAVF_PATTERN_HINT_IPV6,
>> -	IAVF_PATTERN_HINT_IPV6_UDP,
>> -	IAVF_PATTERN_HINT_IPV6_TCP,
>> -	IAVF_PATTERN_HINT_IPV6_SCTP,
>> -};
>> -
>> -enum iavf_gtpu_hint {
>> -	IAVF_GTPU_HINT_DOWNLINK,
>> -	IAVF_GTPU_HINT_UPLINK,
>> -	IAVF_GTPU_HINT_NONE,
>> -};
>> +	IAVF_PHINT_NONE				= 0x00000000,
>> +	IAVF_PHINT_IPV4				= 0x00000001,
>> +	IAVF_PHINT_IPV4_UDP			= 0x00000002,
>> +	IAVF_PHINT_IPV4_TCP			= 0x00000004,
>> +	IAVF_PHINT_IPV4_SCTP			= 0x00000008,
>> +	IAVF_PHINT_IPV6				= 0x00000010,
>> +	IAVF_PHINT_IPV6_UDP			= 0x00000020,
>> +	IAVF_PHINT_IPV6_TCP			= 0x00000040,
>> +	IAVF_PHINT_IPV6_SCTP			= 0x00000080,
>> +	IAVF_PHINT_C_VLAN			= 0x00000100,
>> +	IAVF_PHINT_S_VLAN			= 0x00000200,
>> +	IAVF_PHINT_IPV4_GTPU_EH			= 0x00000400,
>> +	IAVF_PHINT_IPV4_GTPU_EH_DWNLINK		= 0x00000800,
>> +	IAVF_PHINT_IPV4_GTPU_EH_UPLINK		= 0x00001000,
>> +};
>> +
>> +#define IAVF_GTPU_EH_DWNLINK	0
>> +#define IAVF_GTPU_EH_UPLINK	1
>>
>>   struct iavf_pattern_match_type {
>> -	enum iavf_pattern_hint_type phint_type;
>> +	uint64_t pattern_hint;
>>   };
>>
>>   struct iavf_hash_match_type {
>> -	enum iavf_pattern_hint_type phint_type;
>>   	uint64_t hash_type;
>>   	struct virtchnl_proto_hdrs *proto_hdrs;
>> -	enum iavf_gtpu_hint gtpu_hint;
>> +	uint64_t pattern_hint;
>>   };
>>
>>   struct iavf_rss_meta {
>> @@ -83,42 +84,50 @@ iavf_hash_parse_pattern_action(struct iavf_adapter
>> *ad,
>>   			       void **meta,
>>   			       struct rte_flow_error *error);
>>
>> -struct iavf_pattern_match_type phint_empty = {
>> -	IAVF_PATTERN_HINT_NONE};
>> -struct iavf_pattern_match_type phint_eth_ipv4 = {
>> -	IAVF_PATTERN_HINT_IPV4};
>> -struct iavf_pattern_match_type phint_eth_ipv4_udp = {
>> -	IAVF_PATTERN_HINT_IPV4_UDP};
>> -struct iavf_pattern_match_type phint_eth_ipv4_tcp = {
>> -	IAVF_PATTERN_HINT_IPV4_TCP};
>> -struct iavf_pattern_match_type phint_eth_ipv4_sctp = {
>> -	IAVF_PATTERN_HINT_IPV4_SCTP};
>> -struct iavf_pattern_match_type phint_eth_ipv4_gtpu_eh = {
>> -	IAVF_PATTERN_HINT_IPV4_UDP};
>> -struct iavf_pattern_match_type phint_eth_ipv4_esp = {
>> -	IAVF_PATTERN_HINT_IPV4};
>> -struct iavf_pattern_match_type phint_eth_ipv4_ah = {
>> -	IAVF_PATTERN_HINT_IPV4};
>> -struct iavf_pattern_match_type phint_eth_ipv4_l2tpv3 = {
>> -	IAVF_PATTERN_HINT_IPV4};
>> -struct iavf_pattern_match_type phint_eth_ipv4_pfcp = {
>> -	IAVF_PATTERN_HINT_IPV4_UDP};
>> -struct iavf_pattern_match_type phint_eth_ipv6 = {
>> -	IAVF_PATTERN_HINT_IPV6};
>> -struct iavf_pattern_match_type phint_eth_ipv6_udp = {
>> -	IAVF_PATTERN_HINT_IPV6_UDP};
>> -struct iavf_pattern_match_type phint_eth_ipv6_tcp = {
>> -	IAVF_PATTERN_HINT_IPV6_TCP};
>> -struct iavf_pattern_match_type phint_eth_ipv6_sctp = {
>> -	IAVF_PATTERN_HINT_IPV6_SCTP};
>> -struct iavf_pattern_match_type phint_eth_ipv6_esp = {
>> -	IAVF_PATTERN_HINT_IPV6};
>> -struct iavf_pattern_match_type phint_eth_ipv6_ah = {
>> -	IAVF_PATTERN_HINT_IPV6};
>> -struct iavf_pattern_match_type phint_eth_ipv6_l2tpv3 = {
>> -	IAVF_PATTERN_HINT_IPV6};
>> -struct iavf_pattern_match_type phint_eth_ipv6_pfcp = {
>> -	IAVF_PATTERN_HINT_IPV6_UDP};
>> +static struct iavf_pattern_match_type phint_empty = {
>> +	IAVF_PHINT_NONE};
>> +static struct iavf_pattern_match_type phint_eth_ipv4 = {
>> +	IAVF_PHINT_IPV4};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_udp = {
>> +	IAVF_PHINT_IPV4_UDP};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_tcp = {
>> +	IAVF_PHINT_IPV4_TCP};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_sctp = {
>> +	IAVF_PHINT_IPV4_SCTP};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_gtpu_eh_ipv4 = {
>> +	IAVF_PHINT_IPV4};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_gtpu_eh_ipv4_udp
>> = {
>> +	IAVF_PHINT_IPV4_UDP};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_gtpu_eh_ipv4_tcp =
>> {
>> +	IAVF_PHINT_IPV4_TCP};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_esp = {
>> +	IAVF_PHINT_IPV4};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_ah = {
>> +	IAVF_PHINT_IPV4};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_l2tpv3 = {
>> +	IAVF_PHINT_IPV4};
>> +static struct iavf_pattern_match_type phint_eth_ipv4_pfcp = {
>> +	IAVF_PHINT_IPV4_UDP};
>> +static struct iavf_pattern_match_type phint_eth_vlan_ipv4 = {
>> +	IAVF_PHINT_C_VLAN};
>> +static struct iavf_pattern_match_type phint_eth_ipv6 = {
>> +	IAVF_PHINT_IPV6};
>> +static struct iavf_pattern_match_type phint_eth_ipv6_udp = {
>> +	IAVF_PHINT_IPV6_UDP};
>> +static struct iavf_pattern_match_type phint_eth_ipv6_tcp = {
>> +	IAVF_PHINT_IPV6_TCP};
>> +static struct iavf_pattern_match_type phint_eth_ipv6_sctp = {
>> +	IAVF_PHINT_IPV6_SCTP};
>> +static struct iavf_pattern_match_type phint_eth_ipv6_esp = {
>> +	IAVF_PHINT_IPV6};
>> +static struct iavf_pattern_match_type phint_eth_ipv6_ah = {
>> +	IAVF_PHINT_IPV6};
>> +static struct iavf_pattern_match_type phint_eth_ipv6_l2tpv3 = {
>> +	IAVF_PHINT_IPV6};
>> +static struct iavf_pattern_match_type phint_eth_ipv6_pfcp = {
>> +	IAVF_PHINT_IPV6_UDP};
>> +static struct iavf_pattern_match_type phint_eth_vlan_ipv6 = {
>> +	IAVF_PHINT_C_VLAN};
>>
>>   /**
>>    * Supported pattern for hash.
>> @@ -131,26 +140,28 @@ static struct iavf_pattern_match_item
>> iavf_hash_pattern_list[] = {
>>   	{iavf_pattern_eth_ipv4_udp, IAVF_INSET_NONE,
>> &phint_eth_ipv4_udp},
>>   	{iavf_pattern_eth_ipv4_tcp, IAVF_INSET_NONE, &phint_eth_ipv4_tcp},
>>   	{iavf_pattern_eth_ipv4_sctp, IAVF_INSET_NONE,
>> &phint_eth_ipv4_sctp},
>> -	{iavf_pattern_eth_ipv6, IAVF_INSET_NONE, &phint_eth_ipv6},
>>   	{iavf_pattern_eth_ipv4_gtpu_eh_ipv4, IAVF_INSET_NONE,
>> -						&phint_eth_ipv4_gtpu_eh},
>> +					&phint_eth_ipv4_gtpu_eh_ipv4},
>>   	{iavf_pattern_eth_ipv4_gtpu_eh_ipv4_udp, IAVF_INSET_NONE,
>> -						&phint_eth_ipv4_gtpu_eh},
>> +					&phint_eth_ipv4_gtpu_eh_ipv4_udp},
>>   	{iavf_pattern_eth_ipv4_gtpu_eh_ipv4_tcp, IAVF_INSET_NONE,
>> -						&phint_eth_ipv4_gtpu_eh},
>> +					&phint_eth_ipv4_gtpu_eh_ipv4_tcp},
>>   	{iavf_pattern_eth_ipv4_esp, IAVF_INSET_NONE, &phint_eth_ipv4_esp},
>>   	{iavf_pattern_eth_ipv4_ah, IAVF_INSET_NONE, &phint_eth_ipv4_ah},
>>   	{iavf_pattern_eth_ipv4_l2tpv3, IAVF_INSET_NONE,
>> -						&phint_eth_ipv4_l2tpv3},
>> +					&phint_eth_ipv4_l2tpv3},
>>   	{iavf_pattern_eth_ipv4_pfcp, IAVF_INSET_NONE,
>> &phint_eth_ipv4_pfcp},
>> +	{iavf_pattern_eth_vlan_ipv4, IAVF_INSET_NONE,
>> &phint_eth_vlan_ipv4},
>> +	{iavf_pattern_eth_ipv6, IAVF_INSET_NONE, &phint_eth_ipv6},
>>   	{iavf_pattern_eth_ipv6_udp, IAVF_INSET_NONE,
>> &phint_eth_ipv6_udp},
>>   	{iavf_pattern_eth_ipv6_tcp, IAVF_INSET_NONE, &phint_eth_ipv6_tcp},
>>   	{iavf_pattern_eth_ipv6_sctp, IAVF_INSET_NONE,
>> &phint_eth_ipv6_sctp},
>>   	{iavf_pattern_eth_ipv6_esp, IAVF_INSET_NONE, &phint_eth_ipv6_esp},
>>   	{iavf_pattern_eth_ipv6_ah, IAVF_INSET_NONE, &phint_eth_ipv6_ah},
>>   	{iavf_pattern_eth_ipv6_l2tpv3, IAVF_INSET_NONE,
>> -						&phint_eth_ipv6_l2tpv3},
>> +					&phint_eth_ipv6_l2tpv3},
>>   	{iavf_pattern_eth_ipv6_pfcp, IAVF_INSET_NONE,
>> &phint_eth_ipv6_pfcp},
>> +	{iavf_pattern_eth_vlan_ipv6, IAVF_INSET_NONE,
>> &phint_eth_vlan_ipv6},
>>   	{iavf_pattern_empty, IAVF_INSET_NONE, &phint_empty},
>>   };
>>
>> @@ -271,6 +282,10 @@ static struct iavf_pattern_match_item
>> iavf_hash_pattern_list[] = {
>>   	FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV6_SRC) | \
>>   	FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV6_DST), {BUFF_NOUSED } }
>>
>> +#define proto_hint_gtpu_eh_only { \
>> +	VIRTCHNL_PROTO_HDR_GTPU_EH, \
>> +	FIELD_FOR_PROTO_ONLY, {BUFF_NOUSED } }
>> +
>>   #define proto_hint_gtpu_up_only { \
>>   	VIRTCHNL_PROTO_HDR_GTPU_EH_PDU_UP, \
>>   	FIELD_FOR_PROTO_ONLY, {BUFF_NOUSED } }
>> @@ -295,6 +310,8 @@ static struct iavf_pattern_match_item
>> iavf_hash_pattern_list[] = {
>>   	VIRTCHNL_PROTO_HDR_PFCP, \
>>   	FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_PFCP_SEID),
>> {BUFF_NOUSED } }
>>
>> +/* ETH */
>> +
>>   struct virtchnl_proto_hdrs hdrs_hint_eth_src = {
>>   	TUNNEL_LEVEL_OUTER, PROTO_COUNT_ONE, {proto_hint_eth_src }
>>   };
>> @@ -315,6 +332,8 @@ struct virtchnl_proto_hdrs hdrs_hint_cvlan = {
>>   	TUNNEL_LEVEL_OUTER, PROTO_COUNT_ONE, {proto_hint_cvlan }
>>   };
>>
>> +/* IPV4 */
>> +
>>   struct virtchnl_proto_hdrs hdrs_hint_ipv4_src = {
>>   	TUNNEL_LEVEL_OUTER, PROTO_COUNT_ONE, {proto_hint_ipv4_src }
>>   };
>> @@ -323,46 +342,6 @@ struct virtchnl_proto_hdrs hdrs_hint_ipv4_dst = {
>>   	TUNNEL_LEVEL_OUTER, PROTO_COUNT_ONE, {proto_hint_ipv4_dst }
>>   };
>>
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv4_src_gtpu_up = {
>> -	TUNNEL_LEVEL_FIRST_INNER, PROTO_COUNT_TWO,
>> {proto_hint_gtpu_up_only,
>> -	proto_hint_ipv4_src }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv4_src_gtpu_dwn = {
>> -	TUNNEL_LEVEL_FIRST_INNER, PROTO_COUNT_TWO,
>> {proto_hint_gtpu_dwn_only,
>> -	proto_hint_ipv4_src }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv4_dst_gtpu_up = {
>> -	TUNNEL_LEVEL_FIRST_INNER, PROTO_COUNT_TWO,
>> {proto_hint_gtpu_up_only,
>> -	proto_hint_ipv4_dst }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv4_dst_gtpu_dwn = {
>> -	TUNNEL_LEVEL_FIRST_INNER, PROTO_COUNT_TWO,
>> {proto_hint_gtpu_dwn_only,
>> -	proto_hint_ipv4_dst }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv4_esp = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv4_only,
>> -	proto_hint_esp }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv4_ah = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv4_only,
>> -	proto_hint_ah }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv4_l2tpv3 = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv4_only,
>> -	proto_hint_l2tpv3 }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv4_pfcp = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv4_only,
>> -	proto_hint_pfcp }
>> -};
>> -
>>   struct virtchnl_proto_hdrs hdrs_hint_ipv4 = {
>>   	TUNNEL_LEVEL_OUTER, PROTO_COUNT_ONE, {proto_hint_ipv4 }
>>   };
>> @@ -472,34 +451,302 @@ struct virtchnl_proto_hdrs hdrs_hint_ipv4_sctp
>> = {
>>   	proto_hint_sctp }
>>   };
>>
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv6_src = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_ONE, {proto_hint_ipv6_src }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv6_dst = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_ONE, {proto_hint_ipv6_dst }
>> -};
>> -
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv6_esp = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv6_only,
>> +struct virtchnl_proto_hdrs hdrs_hint_ipv4_esp = {
>> +	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv4_only,
>>   	proto_hint_esp }
>>   };
>>
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv6_ah = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv6_only,
>> +struct virtchnl_proto_hdrs hdrs_hint_ipv4_ah = {
>> +	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv4_only,
>>   	proto_hint_ah }
>>   };
>>
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv6_l2tpv3 = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv6_only,
>> +struct virtchnl_proto_hdrs hdrs_hint_ipv4_l2tpv3 = {
>> +	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv4_only,
>>   	proto_hint_l2tpv3 }
>>   };
>>
>> -struct virtchnl_proto_hdrs hdrs_hint_ipv6_pfcp = {
>> -	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv6_only,
>> +struct virtchnl_proto_hdrs hdrs_hint_ipv4_pfcp = {
>> +	TUNNEL_LEVEL_OUTER, PROTO_COUNT_TWO, {proto_hint_ipv4_only,
>>   	proto_hint_pfcp }
>>   };
>>
>> +struct virtchnl_proto_hdrs hdrs_hint_ipv4_udp_esp = {
>> +	TUNNEL_LEVEL_OUTER, PROTO_COUNT_THREE, {proto_hint_ipv4_only,
>> +	proto_hint_udp_only, proto_hint_esp }
>> +};
>> +
>> +/* GTPU EH */
>> +
>> +struct virtchnl_proto_hdrs hdrs_hint_ipv4_udp_src_gtpu_eh = {
>> +	TUNNEL_LEVEL_FIRST_INNER, PROTO_COUNT_THREE,
>> {proto_hint_gtpu_eh_only,
>> +	proto_hint_udp_src_port}
> The counter is three, but you have only two proto layers here, anything I missed?


Oh, that is a fault when i split the patch set with 2/3, will correct it 
on next coming version. Thanks.


> btw, I don't think these code belongs to code refactor, looks like you are adding inner l4 header which is not supported previously
> would it be better just keep his patch only for code refactor, and separate new features in a new patch?


Eventually, the gtpu inner L4 header is supported previously, so no need 
to separate it here.