Re: [dpdk-dev] net/iavf: fix protocol field selector configure

2020-05-23 Thread Jeff Guo

hi, qi

On 5/23/2020 10:03 AM, Zhang, Qi Z wrote:



-Original Message-
From: Guo, Jia 
Sent: Saturday, May 23, 2020 7:18 AM
To: Xing, Beilei ; Zhang, Qi Z ;
Wu, Jingjing 
Cc: Ye, Xiaolong ; dev@dpdk.org; Guo, Jia

Subject: [dpdk-dev] net/iavf: fix protocol field selector configure

When VFs configure the rss rule by virtchnl, it need to set bit mask into the
field selector for the protocol, then PF got the configure massage and parse
the field selector to the corresponding protocol field.

Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")

Signed-off-by: Jeff Guo 
---
  drivers/net/iavf/iavf_hash.c | 474 +--
  1 file changed, 280 insertions(+), 194 deletions(-)

diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c index
af528863b..1e0ffad4c 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -43,6 +43,7 @@ struct iavf_hash_match_type {
enum iavf_pattern_hint_type phint_type;
uint64_t hash_type;
struct virtchnl_proto_hdrs *proto_hdrs;
+   uint32_t link_type;

If this is just a hint for gtpu link type.
Better to rename to "gtpu_hint"
And use the already defined enum but not uint32_t.
  



ok.



  };

  struct iavf_rss_meta {
@@ -147,8 +148,11 @@ static struct iavf_pattern_match_item
iavf_hash_pattern_list[] = {
{iavf_pattern_empty, IAVF_INSET_NONE, &phint_empty},  };

-#defineGTP_EH_PDU_LINK_UP  1
-#defineGTP_EH_PDU_LINK_DWN 0
+enum iavf_pattern_link_type {

Rename to iavf_gtpu_hint.



seem it is better.



+   IAVF_PATTERN_LINK_DOWN,
+   IAVF_PATTERN_LINK_UP,
+   IAVF_PATTERN_LINK_NONE,
+};

The configure is for GTP down/up link,
The name "xxx_LINK_DOWN", and "xxx_LINK_UP" is a little bit misleading.
Could be
IAVF_GTPU_HINT_UPLINK.
IAVF_GTPU_HINT_DOWNLINK.
IAVF_GTPU_HINT_N/A



ok, but i will choose  IAVF_GTPU_HINT_NONE but not IAVF_GTPU_HINT_N/A 
for common and it should also knowledgeable.




  #define TUNNEL_LEVEL_OUTER0
  #define TUNNEL_LEVEL_FIRST_INNER  1
@@ -160,103 +164,112 @@ static struct iavf_pattern_match_item
iavf_hash_pattern_list[] = {
  #define BUFF_NOUSED   0
  #define FIELD_FOR_PROTO_ONLY  0

+#define FIELD_SELECTOR(proto_hdr_field)(1UL << ((proto_hdr_field) % \
+(1UL << PROTO_HDR_SHIFT)))

Could be simplified to.
#define FIELD_SELECTOR(proto_hdr_field) \
(1UL << (proto_hdr_field & PROTO_HDR_FIELD_MASK))



Use the currently macro to make it simple, sounds good. Thanks.



..

Regards
Qi


Re: [dpdk-dev] net/iavf: fix protocol field selector configure

2020-05-22 Thread Zhang, Qi Z



> -Original Message-
> From: Guo, Jia 
> Sent: Saturday, May 23, 2020 7:18 AM
> To: Xing, Beilei ; Zhang, Qi Z ;
> Wu, Jingjing 
> Cc: Ye, Xiaolong ; dev@dpdk.org; Guo, Jia
> 
> Subject: [dpdk-dev] net/iavf: fix protocol field selector configure
> 
> When VFs configure the rss rule by virtchnl, it need to set bit mask into the
> field selector for the protocol, then PF got the configure massage and parse
> the field selector to the corresponding protocol field.
> 
> Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")
> 
> Signed-off-by: Jeff Guo 
> ---
>  drivers/net/iavf/iavf_hash.c | 474 +--
>  1 file changed, 280 insertions(+), 194 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c index
> af528863b..1e0ffad4c 100644
> --- a/drivers/net/iavf/iavf_hash.c
> +++ b/drivers/net/iavf/iavf_hash.c
> @@ -43,6 +43,7 @@ struct iavf_hash_match_type {
>   enum iavf_pattern_hint_type phint_type;
>   uint64_t hash_type;
>   struct virtchnl_proto_hdrs *proto_hdrs;
> + uint32_t link_type;

If this is just a hint for gtpu link type.
Better to rename to "gtpu_hint"
And use the already defined enum but not uint32_t.
 
>  };
> 
>  struct iavf_rss_meta {
> @@ -147,8 +148,11 @@ static struct iavf_pattern_match_item
> iavf_hash_pattern_list[] = {
>   {iavf_pattern_empty, IAVF_INSET_NONE, &phint_empty},  };
> 
> -#define  GTP_EH_PDU_LINK_UP  1
> -#define  GTP_EH_PDU_LINK_DWN 0
> +enum iavf_pattern_link_type {

Rename to iavf_gtpu_hint.

> + IAVF_PATTERN_LINK_DOWN,
> + IAVF_PATTERN_LINK_UP,
> + IAVF_PATTERN_LINK_NONE,
> +};

The configure is for GTP down/up link, 
The name "xxx_LINK_DOWN", and "xxx_LINK_UP" is a little bit misleading.
Could be 
IAVF_GTPU_HINT_UPLINK.
IAVF_GTPU_HINT_DOWNLINK.
IAVF_GTPU_HINT_N/A

> 
>  #define TUNNEL_LEVEL_OUTER   0
>  #define TUNNEL_LEVEL_FIRST_INNER 1
> @@ -160,103 +164,112 @@ static struct iavf_pattern_match_item
> iavf_hash_pattern_list[] = {
>  #define BUFF_NOUSED  0
>  #define FIELD_FOR_PROTO_ONLY 0
> 
> +#define FIELD_SELECTOR(proto_hdr_field)  (1UL << ((proto_hdr_field) % \
> +  (1UL << PROTO_HDR_SHIFT)))

Could be simplified to.
#define FIELD_SELECTOR(proto_hdr_field) \
(1UL << (proto_hdr_field & PROTO_HDR_FIELD_MASK))

..

Regards
Qi


[dpdk-dev] net/iavf: fix protocol field selector configure

2020-05-22 Thread Jeff Guo
When VFs configure the rss rule by virtchnl, it need to set bit mask into
the field selector for the protocol, then PF got the configure massage and
parse the field selector to the corresponding protocol field.

Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")

Signed-off-by: Jeff Guo 
---
 drivers/net/iavf/iavf_hash.c | 474 +--
 1 file changed, 280 insertions(+), 194 deletions(-)

diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index af528863b..1e0ffad4c 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -43,6 +43,7 @@ struct iavf_hash_match_type {
enum iavf_pattern_hint_type phint_type;
uint64_t hash_type;
struct virtchnl_proto_hdrs *proto_hdrs;
+   uint32_t link_type;
 };
 
 struct iavf_rss_meta {
@@ -147,8 +148,11 @@ static struct iavf_pattern_match_item 
iavf_hash_pattern_list[] = {
{iavf_pattern_empty, IAVF_INSET_NONE, &phint_empty},
 };
 
-#defineGTP_EH_PDU_LINK_UP  1
-#defineGTP_EH_PDU_LINK_DWN 0
+enum iavf_pattern_link_type {
+   IAVF_PATTERN_LINK_DOWN,
+   IAVF_PATTERN_LINK_UP,
+   IAVF_PATTERN_LINK_NONE,
+};
 
 #define TUNNEL_LEVEL_OUTER 0
 #define TUNNEL_LEVEL_FIRST_INNER   1
@@ -160,103 +164,112 @@ static struct iavf_pattern_match_item 
iavf_hash_pattern_list[] = {
 #define BUFF_NOUSED0
 #define FIELD_FOR_PROTO_ONLY   0
 
+#define FIELD_SELECTOR(proto_hdr_field)(1UL << ((proto_hdr_field) % \
+(1UL << PROTO_HDR_SHIFT)))
+
 #define proto_hint_eth_src { \
-   VIRTCHNL_PROTO_HDR_ETH, VIRTCHNL_PROTO_HDR_ETH_SRC, {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_ETH, FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_ETH_SRC), \
+   {BUFF_NOUSED } }
 
 #define proto_hint_eth_dst { \
-   VIRTCHNL_PROTO_HDR_ETH, VIRTCHNL_PROTO_HDR_ETH_DST, {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_ETH, FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_ETH_DST), \
+   {BUFF_NOUSED } }
 
 #define proto_hint_eth_only { \
VIRTCHNL_PROTO_HDR_ETH, FIELD_FOR_PROTO_ONLY, {BUFF_NOUSED } }
 
 #define proto_hint_eth { \
VIRTCHNL_PROTO_HDR_ETH, \
-   VIRTCHNL_PROTO_HDR_ETH_SRC | VIRTCHNL_PROTO_HDR_ETH_DST, \
-   {BUFF_NOUSED } }
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_ETH_SRC) | \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_ETH_DST), {BUFF_NOUSED } }
 
 #define proto_hint_svlan { \
-   VIRTCHNL_PROTO_HDR_S_VLAN, VIRTCHNL_PROTO_HDR_S_VLAN_ID, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_S_VLAN, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_S_VLAN_ID), {BUFF_NOUSED } }
 
 #define proto_hint_cvlan { \
-   VIRTCHNL_PROTO_HDR_C_VLAN, VIRTCHNL_PROTO_HDR_C_VLAN_ID, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_C_VLAN, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_C_VLAN_ID), {BUFF_NOUSED } }
 
 #define proto_hint_ipv4_src { \
-   VIRTCHNL_PROTO_HDR_IPV4, VIRTCHNL_PROTO_HDR_IPV4_SRC, {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_IPV4, FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV4_SRC), \
+   {BUFF_NOUSED } }
 
 #define proto_hint_ipv4_dst { \
-   VIRTCHNL_PROTO_HDR_IPV4, VIRTCHNL_PROTO_HDR_IPV4_DST, {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_IPV4, FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV4_DST), \
+   {BUFF_NOUSED } }
 
 #define proto_hint_ipv4_only { \
VIRTCHNL_PROTO_HDR_IPV4, FIELD_FOR_PROTO_ONLY, {BUFF_NOUSED } }
 
 #define proto_hint_ipv4 { \
VIRTCHNL_PROTO_HDR_IPV4, \
-   VIRTCHNL_PROTO_HDR_IPV4_SRC | VIRTCHNL_PROTO_HDR_IPV4_DST, \
-   {BUFF_NOUSED } }
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV4_SRC) | \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV4_DST), {BUFF_NOUSED } }
 
 #define proto_hint_udp_src_port { \
-   VIRTCHNL_PROTO_HDR_UDP, VIRTCHNL_PROTO_HDR_UDP_SRC_PORT, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_UDP, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_UDP_SRC_PORT), {BUFF_NOUSED } }
 
 #define proto_hint_udp_dst_port { \
-   VIRTCHNL_PROTO_HDR_UDP, VIRTCHNL_PROTO_HDR_UDP_DST_PORT, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_UDP, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_UDP_DST_PORT), {BUFF_NOUSED } }
 
 #define proto_hint_udp_only { \
VIRTCHNL_PROTO_HDR_UDP, FIELD_FOR_PROTO_ONLY, {BUFF_NOUSED } }
 
 #define proto_hint_udp { \
VIRTCHNL_PROTO_HDR_UDP, \
-   VIRTCHNL_PROTO_HDR_UDP_SRC_PORT | VIRTCHNL_PROTO_HDR_UDP_DST_PORT, \
-   {BUFF_NOUSED } }
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_UDP_SRC_PORT) | \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_UDP_DST_PORT), {BUFF_NOUSED } }
 
 #define proto_hint_tcp_src_port { \
-   VIRTCHNL_PROTO_HDR_TCP, VIRTCHNL_PROTO_HDR_TCP_SRC_PORT, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_TCP, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_TCP_SRC_PORT), {BUFF_NOUSED } }
 
 #define proto_hint_tcp_dst_port { \
-   VIRTCHNL_PROTO_HDR_TCP, VIRTCHNL_PROTO_HDR_TCP_DST_PORT, \
-   {BUFF_NOUSED } }
+

[dpdk-dev] net/iavf: fix protocol field selector configure

2020-05-22 Thread Jeff Guo
When VFs configure the rss rule by virtchnl, it need to set bit mask into
the field selector for the protocol, then PF got the configure massage and
parse the field selector to the corresponding protocol field.

Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")

Signed-off-by: Jeff Guo 
---
 drivers/net/iavf/iavf_hash.c | 474 +--
 1 file changed, 280 insertions(+), 194 deletions(-)

diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index af528863b..1e0ffad4c 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -43,6 +43,7 @@ struct iavf_hash_match_type {
enum iavf_pattern_hint_type phint_type;
uint64_t hash_type;
struct virtchnl_proto_hdrs *proto_hdrs;
+   uint32_t link_type;
 };
 
 struct iavf_rss_meta {
@@ -147,8 +148,11 @@ static struct iavf_pattern_match_item 
iavf_hash_pattern_list[] = {
{iavf_pattern_empty, IAVF_INSET_NONE, &phint_empty},
 };
 
-#defineGTP_EH_PDU_LINK_UP  1
-#defineGTP_EH_PDU_LINK_DWN 0
+enum iavf_pattern_link_type {
+   IAVF_PATTERN_LINK_DOWN,
+   IAVF_PATTERN_LINK_UP,
+   IAVF_PATTERN_LINK_NONE,
+};
 
 #define TUNNEL_LEVEL_OUTER 0
 #define TUNNEL_LEVEL_FIRST_INNER   1
@@ -160,103 +164,112 @@ static struct iavf_pattern_match_item 
iavf_hash_pattern_list[] = {
 #define BUFF_NOUSED0
 #define FIELD_FOR_PROTO_ONLY   0
 
+#define FIELD_SELECTOR(proto_hdr_field)(1UL << ((proto_hdr_field) % \
+(1UL << PROTO_HDR_SHIFT)))
+
 #define proto_hint_eth_src { \
-   VIRTCHNL_PROTO_HDR_ETH, VIRTCHNL_PROTO_HDR_ETH_SRC, {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_ETH, FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_ETH_SRC), \
+   {BUFF_NOUSED } }
 
 #define proto_hint_eth_dst { \
-   VIRTCHNL_PROTO_HDR_ETH, VIRTCHNL_PROTO_HDR_ETH_DST, {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_ETH, FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_ETH_DST), \
+   {BUFF_NOUSED } }
 
 #define proto_hint_eth_only { \
VIRTCHNL_PROTO_HDR_ETH, FIELD_FOR_PROTO_ONLY, {BUFF_NOUSED } }
 
 #define proto_hint_eth { \
VIRTCHNL_PROTO_HDR_ETH, \
-   VIRTCHNL_PROTO_HDR_ETH_SRC | VIRTCHNL_PROTO_HDR_ETH_DST, \
-   {BUFF_NOUSED } }
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_ETH_SRC) | \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_ETH_DST), {BUFF_NOUSED } }
 
 #define proto_hint_svlan { \
-   VIRTCHNL_PROTO_HDR_S_VLAN, VIRTCHNL_PROTO_HDR_S_VLAN_ID, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_S_VLAN, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_S_VLAN_ID), {BUFF_NOUSED } }
 
 #define proto_hint_cvlan { \
-   VIRTCHNL_PROTO_HDR_C_VLAN, VIRTCHNL_PROTO_HDR_C_VLAN_ID, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_C_VLAN, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_C_VLAN_ID), {BUFF_NOUSED } }
 
 #define proto_hint_ipv4_src { \
-   VIRTCHNL_PROTO_HDR_IPV4, VIRTCHNL_PROTO_HDR_IPV4_SRC, {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_IPV4, FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV4_SRC), \
+   {BUFF_NOUSED } }
 
 #define proto_hint_ipv4_dst { \
-   VIRTCHNL_PROTO_HDR_IPV4, VIRTCHNL_PROTO_HDR_IPV4_DST, {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_IPV4, FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV4_DST), \
+   {BUFF_NOUSED } }
 
 #define proto_hint_ipv4_only { \
VIRTCHNL_PROTO_HDR_IPV4, FIELD_FOR_PROTO_ONLY, {BUFF_NOUSED } }
 
 #define proto_hint_ipv4 { \
VIRTCHNL_PROTO_HDR_IPV4, \
-   VIRTCHNL_PROTO_HDR_IPV4_SRC | VIRTCHNL_PROTO_HDR_IPV4_DST, \
-   {BUFF_NOUSED } }
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV4_SRC) | \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_IPV4_DST), {BUFF_NOUSED } }
 
 #define proto_hint_udp_src_port { \
-   VIRTCHNL_PROTO_HDR_UDP, VIRTCHNL_PROTO_HDR_UDP_SRC_PORT, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_UDP, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_UDP_SRC_PORT), {BUFF_NOUSED } }
 
 #define proto_hint_udp_dst_port { \
-   VIRTCHNL_PROTO_HDR_UDP, VIRTCHNL_PROTO_HDR_UDP_DST_PORT, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_UDP, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_UDP_DST_PORT), {BUFF_NOUSED } }
 
 #define proto_hint_udp_only { \
VIRTCHNL_PROTO_HDR_UDP, FIELD_FOR_PROTO_ONLY, {BUFF_NOUSED } }
 
 #define proto_hint_udp { \
VIRTCHNL_PROTO_HDR_UDP, \
-   VIRTCHNL_PROTO_HDR_UDP_SRC_PORT | VIRTCHNL_PROTO_HDR_UDP_DST_PORT, \
-   {BUFF_NOUSED } }
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_UDP_SRC_PORT) | \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_UDP_DST_PORT), {BUFF_NOUSED } }
 
 #define proto_hint_tcp_src_port { \
-   VIRTCHNL_PROTO_HDR_TCP, VIRTCHNL_PROTO_HDR_TCP_SRC_PORT, \
-   {BUFF_NOUSED } }
+   VIRTCHNL_PROTO_HDR_TCP, \
+   FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_TCP_SRC_PORT), {BUFF_NOUSED } }
 
 #define proto_hint_tcp_dst_port { \
-   VIRTCHNL_PROTO_HDR_TCP, VIRTCHNL_PROTO_HDR_TCP_DST_PORT, \
-   {BUFF_NOUSED } }
+