diff mbox

[dpdk-dev,1/5] ethdev: Rename macros of packet classification type

Message ID 1406184149-11531-2-git-send-email-helin.zhang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Zhang, Helin July 24, 2014, 6:42 a.m. UTC 
  For better understanding, 'PCTYPE' was added to the name of i40e
RSS shift macros.

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 lib/librte_ether/rte_ethdev.h | 74 +++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

Comments

Thomas Monjalon July 24, 2014, 7:48 a.m. UTC | #1 
Hi Helin,

2014-07-24 14:42, Helin Zhang:
> For better understanding, 'PCTYPE' was added to the name of i40e
> RSS shift macros.
> 
> Signed-off-by: Helin Zhang <helin.zhang@intel.com>

> -#define ETH_RSS_NONF_IPV4_UDP_SHIFT           31
> +#define ETH_PCTYPE_NONF_IPV4_UDP              31

Why is it clearer? I don't understand what means PCTYPE.
Zhang, Helin July 24, 2014, 8:14 a.m. UTC | #2 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, July 24, 2014 3:48 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/5] ethdev: Rename macros of packet
> classification type
> 
> Hi Helin,
> 
> 2014-07-24 14:42, Helin Zhang:
> > For better understanding, 'PCTYPE' was added to the name of i40e RSS
> > shift macros.
> >
> > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> 
> > -#define ETH_RSS_NONF_IPV4_UDP_SHIFT           31
> > +#define ETH_PCTYPE_NONF_IPV4_UDP              31
> 
> Why is it clearer? I don't understand what means PCTYPE.
> 
> --
> Thomas

Hi Thomas

It is 'packet classification type' defined in data sheet, and widely used. It is not just for RSS only, it can be used for flow director possibly. That's why I think it would be better to rename it with PCTYPE names. Thank you!

Regards,
Helin
Thomas Monjalon July 24, 2014, 8:19 a.m. UTC | #3 
2014-07-24 08:14, Zhang, Helin:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-07-24 14:42, Helin Zhang:
> > > For better understanding, 'PCTYPE' was added to the name of i40e RSS
> > > shift macros.
> > >
> > > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > 
> > > -#define ETH_RSS_NONF_IPV4_UDP_SHIFT           31
> > > +#define ETH_PCTYPE_NONF_IPV4_UDP              31
> > 
> > Why is it clearer? I don't understand what means PCTYPE.
> 
> It is 'packet classification type' defined in data sheet, and widely used.

Widely used? No it seems to be an i40e naming.
Which datasheet are you pointing?

> It is not just for RSS only, it can be used for flow director possibly.
> That's why I think it would be better to rename it with PCTYPE names.

At least, you should add a comment in the code to explain the meaning.

Thank you
Zhang, Helin July 24, 2014, 8:23 a.m. UTC | #4 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, July 24, 2014 4:20 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/5] ethdev: Rename macros of packet
> classification type
> 
> 2014-07-24 08:14, Zhang, Helin:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-07-24 14:42, Helin Zhang:
> > > > For better understanding, 'PCTYPE' was added to the name of i40e
> > > > RSS shift macros.
> > > >
> > > > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > >
> > > > -#define ETH_RSS_NONF_IPV4_UDP_SHIFT           31
> > > > +#define ETH_PCTYPE_NONF_IPV4_UDP              31
> > >
> > > Why is it clearer? I don't understand what means PCTYPE.
> >
> > It is 'packet classification type' defined in data sheet, and widely used.
> 
> Widely used? No it seems to be an i40e naming.
> Which datasheet are you pointing?

Sorry, it is widely used in i40e. That might not so widely for the whole DPDK.
The datasheet I mentioned is the Intel(r) 40G NIC datasheet.

> 
> > It is not just for RSS only, it can be used for flow director possibly.
> > That's why I think it would be better to rename it with PCTYPE names.
> 
> At least, you should add a comment in the code to explain the meaning.

OK, good comments!

> 
> Thank you
> --
> Thomas
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 50df654..a262463 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -346,46 +346,46 @@  struct rte_eth_rss_conf {
 #define ETH_RSS_IPV6_UDP_SHIFT                7
 #define ETH_RSS_IPV6_UDP_EX_SHIFT             8
 /* for 40G only */
-#define ETH_RSS_NONF_IPV4_UDP_SHIFT           31
-#define ETH_RSS_NONF_IPV4_TCP_SHIFT           33
-#define ETH_RSS_NONF_IPV4_SCTP_SHIFT          34
-#define ETH_RSS_NONF_IPV4_OTHER_SHIFT         35
-#define ETH_RSS_FRAG_IPV4_SHIFT               36
-#define ETH_RSS_NONF_IPV6_UDP_SHIFT           41
-#define ETH_RSS_NONF_IPV6_TCP_SHIFT           43
-#define ETH_RSS_NONF_IPV6_SCTP_SHIFT          44
-#define ETH_RSS_NONF_IPV6_OTHER_SHIFT         45
-#define ETH_RSS_FRAG_IPV6_SHIFT               46
-#define ETH_RSS_FCOE_OX_SHIFT                 48
-#define ETH_RSS_FCOE_RX_SHIFT                 49
-#define ETH_RSS_FCOE_OTHER_SHIFT              50
-#define ETH_RSS_L2_PAYLOAD_SHIFT              63
+#define ETH_PCTYPE_NONF_IPV4_UDP              31
+#define ETH_PCTYPE_NONF_IPV4_TCP              33
+#define ETH_PCTYPE_NONF_IPV4_SCTP             34
+#define ETH_PCTYPE_NONF_IPV4_OTHER            35
+#define ETH_PCTYPE_FRAG_IPV4                  36
+#define ETH_PCTYPE_NONF_IPV6_UDP              41
+#define ETH_PCTYPE_NONF_IPV6_TCP              43
+#define ETH_PCTYPE_NONF_IPV6_SCTP             44
+#define ETH_PCTYPE_NONF_IPV6_OTHER            45
+#define ETH_PCTYPE_FRAG_IPV6                  46
+#define ETH_PCTYPE_FCOE_OX                    48 /* not used */
+#define ETH_PCTYPE_FCOE_RX                    49 /* not used */
+#define ETH_PCTYPE_FCOE_OTHER                 50 /* not used */
+#define ETH_PCTYPE_L2_PAYLOAD                 63
 
 /* for 1G & 10G */
-#define ETH_RSS_IPV4                    ((uint16_t)1 << ETH_RSS_IPV4_SHIFT)
-#define ETH_RSS_IPV4_TCP                ((uint16_t)1 << ETH_RSS_IPV4_TCP_SHIFT)
-#define ETH_RSS_IPV6                    ((uint16_t)1 << ETH_RSS_IPV6_SHIFT)
-#define ETH_RSS_IPV6_EX                 ((uint16_t)1 << ETH_RSS_IPV6_EX_SHIFT)
-#define ETH_RSS_IPV6_TCP                ((uint16_t)1 << ETH_RSS_IPV6_TCP_SHIFT)
-#define ETH_RSS_IPV6_TCP_EX             ((uint16_t)1 << ETH_RSS_IPV6_TCP_EX_SHIFT)
-#define ETH_RSS_IPV4_UDP                ((uint16_t)1 << ETH_RSS_IPV4_UDP_SHIFT)
-#define ETH_RSS_IPV6_UDP                ((uint16_t)1 << ETH_RSS_IPV6_UDP_SHIFT)
-#define ETH_RSS_IPV6_UDP_EX             ((uint16_t)1 << ETH_RSS_IPV6_UDP_EX_SHIFT)
+#define ETH_RSS_IPV4                    (1 << ETH_RSS_IPV4_SHIFT)
+#define ETH_RSS_IPV4_TCP                (1 << ETH_RSS_IPV4_TCP_SHIFT)
+#define ETH_RSS_IPV6                    (1 << ETH_RSS_IPV6_SHIFT)
+#define ETH_RSS_IPV6_EX                 (1 << ETH_RSS_IPV6_EX_SHIFT)
+#define ETH_RSS_IPV6_TCP                (1 << ETH_RSS_IPV6_TCP_SHIFT)
+#define ETH_RSS_IPV6_TCP_EX             (1 << ETH_RSS_IPV6_TCP_EX_SHIFT)
+#define ETH_RSS_IPV4_UDP                (1 << ETH_RSS_IPV4_UDP_SHIFT)
+#define ETH_RSS_IPV6_UDP                (1 << ETH_RSS_IPV6_UDP_SHIFT)
+#define ETH_RSS_IPV6_UDP_EX             (1 << ETH_RSS_IPV6_UDP_EX_SHIFT)
 /* for 40G only */
-#define ETH_RSS_NONF_IPV4_UDP           ((uint64_t)1 << ETH_RSS_NONF_IPV4_UDP_SHIFT)
-#define ETH_RSS_NONF_IPV4_TCP           ((uint64_t)1 << ETH_RSS_NONF_IPV4_TCP_SHIFT)
-#define ETH_RSS_NONF_IPV4_SCTP          ((uint64_t)1 << ETH_RSS_NONF_IPV4_SCTP_SHIFT)
-#define ETH_RSS_NONF_IPV4_OTHER         ((uint64_t)1 << ETH_RSS_NONF_IPV4_OTHER_SHIFT)
-#define ETH_RSS_FRAG_IPV4               ((uint64_t)1 << ETH_RSS_FRAG_IPV4_SHIFT)
-#define ETH_RSS_NONF_IPV6_UDP           ((uint64_t)1 << ETH_RSS_NONF_IPV6_UDP_SHIFT)
-#define ETH_RSS_NONF_IPV6_TCP           ((uint64_t)1 << ETH_RSS_NONF_IPV6_TCP_SHIFT)
-#define ETH_RSS_NONF_IPV6_SCTP          ((uint64_t)1 << ETH_RSS_NONF_IPV6_SCTP_SHIFT)
-#define ETH_RSS_NONF_IPV6_OTHER         ((uint64_t)1 << ETH_RSS_NONF_IPV6_OTHER_SHIFT)
-#define ETH_RSS_FRAG_IPV6               ((uint64_t)1 << ETH_RSS_FRAG_IPV6_SHIFT)
-#define ETH_RSS_FCOE_OX                 ((uint64_t)1 << ETH_RSS_FCOE_OX_SHIFT) /* not used */
-#define ETH_RSS_FCOE_RX                 ((uint64_t)1 << ETH_RSS_FCOE_RX_SHIFT) /* not used */
-#define ETH_RSS_FCOE_OTHER              ((uint64_t)1 << ETH_RSS_FCOE_OTHER_SHIFT) /* not used */
-#define ETH_RSS_L2_PAYLOAD              ((uint64_t)1 << ETH_RSS_L2_PAYLOAD_SHIFT)
+#define ETH_RSS_NONF_IPV4_UDP           (1ULL << ETH_PCTYPE_NONF_IPV4_UDP)
+#define ETH_RSS_NONF_IPV4_TCP           (1ULL << ETH_PCTYPE_NONF_IPV4_TCP)
+#define ETH_RSS_NONF_IPV4_SCTP          (1ULL << ETH_PCTYPE_NONF_IPV4_SCTP)
+#define ETH_RSS_NONF_IPV4_OTHER         (1ULL << ETH_PCTYPE_NONF_IPV4_OTHER)
+#define ETH_RSS_FRAG_IPV4               (1ULL << ETH_PCTYPE_FRAG_IPV4)
+#define ETH_RSS_NONF_IPV6_UDP           (1ULL << ETH_PCTYPE_NONF_IPV6_UDP)
+#define ETH_RSS_NONF_IPV6_TCP           (1ULL << ETH_PCTYPE_NONF_IPV6_TCP)
+#define ETH_RSS_NONF_IPV6_SCTP          (1ULL << ETH_PCTYPE_NONF_IPV6_SCTP)
+#define ETH_RSS_NONF_IPV6_OTHER         (1ULL << ETH_PCTYPE_NONF_IPV6_OTHER)
+#define ETH_RSS_FRAG_IPV6               (1ULL << ETH_PCTYPE_FRAG_IPV6)
+#define ETH_RSS_FCOE_OX                 (1ULL << ETH_PCTYPE_FCOE_OX)
+#define ETH_RSS_FCOE_RX                 (1ULL << ETH_PCTYPE_FCOE_RX)
+#define ETH_RSS_FCOE_OTHER              (1ULL << ETH_PCTYPE_FCOE_OTHER)
+#define ETH_RSS_L2_PAYLOAD              (1ULL << ETH_PCTYPE_L2_PAYLOAD)
 
 #define ETH_RSS_IP ( \
 		ETH_RSS_IPV4 | \