DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] i40e: bug fix of compile error
@ 2014-12-01  7:33 Helin Zhang
  2014-12-01 11:12 ` Thomas Monjalon
  2014-12-03  1:13 ` [dpdk-dev] [PATCH v2] i40e: " Helin Zhang
  0 siblings, 2 replies; 7+ messages in thread
From: Helin Zhang @ 2014-12-01  7:33 UTC (permalink / raw)
  To: dev

The compile error will occur as below when set 'RTE_LIBRTE_I40E_16BYTE_RX_DESC=y'.
The changes is just to fix it.

lib/librte_pmd_i40e/i40e_rxtx.c: In function i40e_rxd_build_fdir:
lib/librte_pmd_i40e/i40e_rxtx.c:431:28: error: volatile union <anonymous> has no member named fd
lib/librte_pmd_i40e/i40e_rxtx.c:427:19: error: unused variable flexbl [-Werror=unused-variable]
lib/librte_pmd_i40e/i40e_rxtx.c:427:11: error: unused variable flexbh [-Werror=unused-variable]

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 lib/librte_pmd_i40e/i40e_rxtx.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index 2d2ef04..95666fd 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -424,13 +424,9 @@ static inline uint64_t
 i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, struct rte_mbuf *mb)
 {
 	uint64_t flags = 0;
+#ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
 	uint16_t flexbh, flexbl;
 
-#ifdef RTE_LIBRTE_I40E_16BYTE_RX_DESC
-	mb->hash.fdir.hi =
-		rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd);
-	flags |= PKT_RX_FDIR_ID;
-#else
 	flexbh = (rte_le_to_cpu_32(rxdp->wb.qword2.ext_status) >>
 		I40E_RX_DESC_EXT_STATUS_FLEXBH_SHIFT) &
 		I40E_RX_DESC_EXT_STATUS_FLEXBH_MASK;
@@ -438,22 +434,28 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, struct rte_mbuf *mb)
 		I40E_RX_DESC_EXT_STATUS_FLEXBL_SHIFT) &
 		I40E_RX_DESC_EXT_STATUS_FLEXBL_MASK;
 
-
 	if (flexbh == I40E_RX_DESC_EXT_STATUS_FLEXBH_FD_ID) {
 		mb->hash.fdir.hi =
 			rte_le_to_cpu_32(rxdp->wb.qword3.hi_dword.fd_id);
 		flags |= PKT_RX_FDIR_ID;
 	} else if (flexbh == I40E_RX_DESC_EXT_STATUS_FLEXBH_FLEX) {
 		mb->hash.fdir.hi =
-			rte_le_to_cpu_32(rxdp->wb.qword3.hi_dword.flex_bytes_hi);
+			rte_le_to_cpu_32(
+			rxdp->wb.qword3.hi_dword.flex_bytes_hi);
 		flags |= PKT_RX_FDIR_FLX;
 	}
+
 	if (flexbl == I40E_RX_DESC_EXT_STATUS_FLEXBL_FLEX) {
 		mb->hash.fdir.lo =
-			rte_le_to_cpu_32(rxdp->wb.qword3.lo_dword.flex_bytes_lo);
+			rte_le_to_cpu_32(
+			rxdp->wb.qword3.lo_dword.flex_bytes_lo);
 		flags |= PKT_RX_FDIR_FLX;
 	}
+#else
+	mb->hash.fdir.hi = rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd_id);
+	flags |= PKT_RX_FDIR_ID;
 #endif
+
 	return flags;
 }
 static inline void
-- 
1.9.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] i40e: bug fix of compile error
  2014-12-01  7:33 [dpdk-dev] [PATCH] i40e: bug fix of compile error Helin Zhang
@ 2014-12-01 11:12 ` Thomas Monjalon
  2014-12-02  0:35   ` Zhang, Helin
  2014-12-03  1:13 ` [dpdk-dev] [PATCH v2] i40e: " Helin Zhang
  1 sibling, 1 reply; 7+ messages in thread
From: Thomas Monjalon @ 2014-12-01 11:12 UTC (permalink / raw)
  To: Helin Zhang; +Cc: dev

2014-12-01 15:33, Helin Zhang:
> The compile error will occur as below when set 'RTE_LIBRTE_I40E_16BYTE_RX_DESC=y'.
> The changes is just to fix it.
> 
> lib/librte_pmd_i40e/i40e_rxtx.c: In function i40e_rxd_build_fdir:
> lib/librte_pmd_i40e/i40e_rxtx.c:431:28: error: volatile union <anonymous> has no member named fd
> lib/librte_pmd_i40e/i40e_rxtx.c:427:19: error: unused variable flexbl [-Werror=unused-variable]
> lib/librte_pmd_i40e/i40e_rxtx.c:427:11: error: unused variable flexbh [-Werror=unused-variable]

It would be nice to reference the commit which introduced the error
and explain it a bit.

> -			rte_le_to_cpu_32(rxdp->wb.qword3.hi_dword.flex_bytes_hi);
> +			rte_le_to_cpu_32(
> +			rxdp->wb.qword3.hi_dword.flex_bytes_hi);
[...]
> -			rte_le_to_cpu_32(rxdp->wb.qword3.lo_dword.flex_bytes_lo);
> +			rte_le_to_cpu_32(
> +			rxdp->wb.qword3.lo_dword.flex_bytes_lo);

Why are you wrapping these lines (with wrong indentation)?
It makes the fix confuse.

-- 
Thomas

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] i40e: bug fix of compile error
  2014-12-01 11:12 ` Thomas Monjalon
@ 2014-12-02  0:35   ` Zhang, Helin
  2014-12-02  6:58     ` Qiu, Michael
  0 siblings, 1 reply; 7+ messages in thread
From: Zhang, Helin @ 2014-12-02  0:35 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, December 1, 2014 7:13 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] i40e: bug fix of compile error
> 
> 2014-12-01 15:33, Helin Zhang:
> > The compile error will occur as below when set
> 'RTE_LIBRTE_I40E_16BYTE_RX_DESC=y'.
> > The changes is just to fix it.
> >
> > lib/librte_pmd_i40e/i40e_rxtx.c: In function i40e_rxd_build_fdir:
> > lib/librte_pmd_i40e/i40e_rxtx.c:431:28: error: volatile union
> > <anonymous> has no member named fd
> > lib/librte_pmd_i40e/i40e_rxtx.c:427:19: error: unused variable flexbl
> > [-Werror=unused-variable]
> > lib/librte_pmd_i40e/i40e_rxtx.c:427:11: error: unused variable flexbh
> > [-Werror=unused-variable]
> 
> It would be nice to reference the commit which introduced the error and explain
> it a bit.
> 
> > -			rte_le_to_cpu_32(rxdp->wb.qword3.hi_dword.flex_bytes_hi);
> > +			rte_le_to_cpu_32(
> > +			rxdp->wb.qword3.hi_dword.flex_bytes_hi);
> [...]
> > -			rte_le_to_cpu_32(rxdp->wb.qword3.lo_dword.flex_bytes_lo);
> > +			rte_le_to_cpu_32(
> > +			rxdp->wb.qword3.lo_dword.flex_bytes_lo);
> 
> Why are you wrapping these lines (with wrong indentation)?
> It makes the fix confuse.
Sorry, it is a code style fix, as the length of the line should not be more than 80.
Do I need to split the patch or add more commit logs?

> 
> --
> Thomas

Regards,
Helin

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] i40e: bug fix of compile error
  2014-12-02  0:35   ` Zhang, Helin
@ 2014-12-02  6:58     ` Qiu, Michael
  0 siblings, 0 replies; 7+ messages in thread
From: Qiu, Michael @ 2014-12-02  6:58 UTC (permalink / raw)
  To: Zhang, Helin, Thomas Monjalon; +Cc: dev

[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]

On 12/2/2014 8:36 AM, Zhang, Helin wrote:
>
>> -----Original Message-----
>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>> Sent: Monday, December 1, 2014 7:13 PM
>> To: Zhang, Helin
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] i40e: bug fix of compile error
>>
>> 2014-12-01 15:33, Helin Zhang:
>>> The compile error will occur as below when set
>> 'RTE_LIBRTE_I40E_16BYTE_RX_DESC=y'.
>>> The changes is just to fix it.
>>>
>>> lib/librte_pmd_i40e/i40e_rxtx.c: In function i40e_rxd_build_fdir:
>>> lib/librte_pmd_i40e/i40e_rxtx.c:431:28: error: volatile union
>>> <anonymous> has no member named fd
>>> lib/librte_pmd_i40e/i40e_rxtx.c:427:19: error: unused variable flexbl
>>> [-Werror=unused-variable]
>>> lib/librte_pmd_i40e/i40e_rxtx.c:427:11: error: unused variable flexbh
>>> [-Werror=unused-variable]
>> It would be nice to reference the commit which introduced the error and explain
>> it a bit.
>>
>>> -			rte_le_to_cpu_32(rxdp->wb.qword3.hi_dword.flex_bytes_hi);
>>> +			rte_le_to_cpu_32(
>>> +			rxdp->wb.qword3.hi_dword.flex_bytes_hi);
>> [...]
>>> -			rte_le_to_cpu_32(rxdp->wb.qword3.lo_dword.flex_bytes_lo);
>>> +			rte_le_to_cpu_32(
>>> +			rxdp->wb.qword3.lo_dword.flex_bytes_lo);
>> Why are you wrapping these lines (with wrong indentation)?
>> It makes the fix confuse.
> Sorry, it is a code style fix, as the length of the line should not be more than 80.
> Do I need to split the patch or add more commit logs?

I think this coding style fix is not good enough,

		mb->hash.fdir.hi = rte_le_to_cpu_32(rxdp->wb.qword3
						    .hi_dword.fd_id);

This would be better :)

See the attached diff file, so sorry I indeed do not know how to add
diff file in the email(except git send-email), So I just attached it.

>
>> --
>> Thomas
> Regards,
> Helin
>


[-- Attachment #2: diff_code_style_fix --]
[-- Type: text/plain, Size: 1471 bytes --]

diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index 2d2ef04..e0264fc 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -427,8 +427,7 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, struct rte_mbuf *mb)
 	uint16_t flexbh, flexbl;
 
 #ifdef RTE_LIBRTE_I40E_16BYTE_RX_DESC
-	mb->hash.fdir.hi =
-		rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd);
+	mb->hash.fdir.hi = rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd);
 	flags |= PKT_RX_FDIR_ID;
 #else
 	flexbh = (rte_le_to_cpu_32(rxdp->wb.qword2.ext_status) >>
@@ -440,17 +439,17 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, struct rte_mbuf *mb)
 
 
 	if (flexbh == I40E_RX_DESC_EXT_STATUS_FLEXBH_FD_ID) {
-		mb->hash.fdir.hi =
-			rte_le_to_cpu_32(rxdp->wb.qword3.hi_dword.fd_id);
+		mb->hash.fdir.hi = rte_le_to_cpu_32(rxdp->wb.qword3
+						    .hi_dword.fd_id);
 		flags |= PKT_RX_FDIR_ID;
 	} else if (flexbh == I40E_RX_DESC_EXT_STATUS_FLEXBH_FLEX) {
-		mb->hash.fdir.hi =
-			rte_le_to_cpu_32(rxdp->wb.qword3.hi_dword.flex_bytes_hi);
+		mb->hash.fdir.hi = rte_le_to_cpu_32(rxdp->wb.qword3
+						    .hi_dword.flex_bytes_hi);
 		flags |= PKT_RX_FDIR_FLX;
 	}
 	if (flexbl == I40E_RX_DESC_EXT_STATUS_FLEXBL_FLEX) {
-		mb->hash.fdir.lo =
-			rte_le_to_cpu_32(rxdp->wb.qword3.lo_dword.flex_bytes_lo);
+		mb->hash.fdir.lo = rte_le_to_cpu_32(rxdp->wb.qword3
+						    .lo_dword.flex_bytes_lo);
 		flags |= PKT_RX_FDIR_FLX;
 	}
 #endif

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [dpdk-dev] [PATCH v2] i40e: fix of compile error
  2014-12-01  7:33 [dpdk-dev] [PATCH] i40e: bug fix of compile error Helin Zhang
  2014-12-01 11:12 ` Thomas Monjalon
@ 2014-12-03  1:13 ` Helin Zhang
  2014-12-03  7:47   ` Wu, Jingjing
  1 sibling, 1 reply; 7+ messages in thread
From: Helin Zhang @ 2014-12-03  1:13 UTC (permalink / raw)
  To: dev

The compile error will occur as below when set 'RTE_LIBRTE_I40E_16BYTE_RX_DESC=y'.
'fd_id' should be used to replace 'fd', as 'fd' is not defined in that structure
at all. In addition, local variable of 'flexbl' and 'flexbh' must be used only if
32 bytes RX descriptor is selected.

error logs:
lib/librte_pmd_i40e/i40e_rxtx.c: In function i40e_rxd_build_fdir:
lib/librte_pmd_i40e/i40e_rxtx.c:431:28: error: volatile union <anonymous> has no member named fd
lib/librte_pmd_i40e/i40e_rxtx.c:427:19: error: unused variable flexbl [-Werror=unused-variable]
lib/librte_pmd_i40e/i40e_rxtx.c:427:11: error: unused variable flexbh [-Werror=unused-variable]

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 lib/librte_pmd_i40e/i40e_rxtx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

v2 changes:
* Removed the changes for code style fix, and kept it for compile error fix only.
* Re-word the commit logs.

diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index 2d2ef04..63c872d 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -424,13 +424,9 @@ static inline uint64_t
 i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, struct rte_mbuf *mb)
 {
 	uint64_t flags = 0;
+#ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
 	uint16_t flexbh, flexbl;
 
-#ifdef RTE_LIBRTE_I40E_16BYTE_RX_DESC
-	mb->hash.fdir.hi =
-		rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd);
-	flags |= PKT_RX_FDIR_ID;
-#else
 	flexbh = (rte_le_to_cpu_32(rxdp->wb.qword2.ext_status) >>
 		I40E_RX_DESC_EXT_STATUS_FLEXBH_SHIFT) &
 		I40E_RX_DESC_EXT_STATUS_FLEXBH_MASK;
@@ -453,6 +449,10 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, struct rte_mbuf *mb)
 			rte_le_to_cpu_32(rxdp->wb.qword3.lo_dword.flex_bytes_lo);
 		flags |= PKT_RX_FDIR_FLX;
 	}
+#else
+	mb->hash.fdir.hi =
+		rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd_id);
+	flags |= PKT_RX_FDIR_ID;
 #endif
 	return flags;
 }
-- 
1.9.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH v2] i40e: fix of compile error
  2014-12-03  1:13 ` [dpdk-dev] [PATCH v2] i40e: " Helin Zhang
@ 2014-12-03  7:47   ` Wu, Jingjing
  2014-12-03 10:03     ` Thomas Monjalon
  0 siblings, 1 reply; 7+ messages in thread
From: Wu, Jingjing @ 2014-12-03  7:47 UTC (permalink / raw)
  To: Zhang, Helin, dev



> -----Original Message-----
> From: Zhang, Helin
> Sent: Wednesday, December 03, 2014 9:13 AM
> To: dev@dpdk.org
> Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Zhang, Helin
> Subject: [PATCH v2] i40e: fix of compile error
> 
> The compile error will occur as below when set
> 'RTE_LIBRTE_I40E_16BYTE_RX_DESC=y'.
> 'fd_id' should be used to replace 'fd', as 'fd' is not defined in that structure at
> all. In addition, local variable of 'flexbl' and 'flexbh' must be used only if
> 32 bytes RX descriptor is selected.
> 
> error logs:
> lib/librte_pmd_i40e/i40e_rxtx.c: In function i40e_rxd_build_fdir:
> lib/librte_pmd_i40e/i40e_rxtx.c:431:28: error: volatile union <anonymous>
> has no member named fd
> lib/librte_pmd_i40e/i40e_rxtx.c:427:19: error: unused variable flexbl [-
> Werror=unused-variable]
> lib/librte_pmd_i40e/i40e_rxtx.c:427:11: error: unused variable flexbh [-
> Werror=unused-variable]
> 
> Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> ---
>  lib/librte_pmd_i40e/i40e_rxtx.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> v2 changes:
> * Removed the changes for code style fix, and kept it for compile error fix
> only.
> * Re-word the commit logs.
> 
> diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
> index 2d2ef04..63c872d 100644
> --- a/lib/librte_pmd_i40e/i40e_rxtx.c
> +++ b/lib/librte_pmd_i40e/i40e_rxtx.c
> @@ -424,13 +424,9 @@ static inline uint64_t  i40e_rxd_build_fdir(volatile
> union i40e_rx_desc *rxdp, struct rte_mbuf *mb)  {
>  	uint64_t flags = 0;
> +#ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
>  	uint16_t flexbh, flexbl;
> 
> -#ifdef RTE_LIBRTE_I40E_16BYTE_RX_DESC
> -	mb->hash.fdir.hi =
> -		rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd);
> -	flags |= PKT_RX_FDIR_ID;
> -#else
>  	flexbh = (rte_le_to_cpu_32(rxdp->wb.qword2.ext_status) >>
>  		I40E_RX_DESC_EXT_STATUS_FLEXBH_SHIFT) &
>  		I40E_RX_DESC_EXT_STATUS_FLEXBH_MASK;
> @@ -453,6 +449,10 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc
> *rxdp, struct rte_mbuf *mb)
>  			rte_le_to_cpu_32(rxdp-
> >wb.qword3.lo_dword.flex_bytes_lo);
>  		flags |= PKT_RX_FDIR_FLX;
>  	}
> +#else
> +	mb->hash.fdir.hi =
> +		rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd_id);
> +	flags |= PKT_RX_FDIR_ID;
>  #endif
>  	return flags;
>  }
> --
> 1.9.3

Acked-by: Jingjing Wu <jingjing.wu@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH v2] i40e: fix of compile error
  2014-12-03  7:47   ` Wu, Jingjing
@ 2014-12-03 10:03     ` Thomas Monjalon
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2014-12-03 10:03 UTC (permalink / raw)
  To: Zhang, Helin; +Cc: dev

> > The compile error will occur as below when set
> > 'RTE_LIBRTE_I40E_16BYTE_RX_DESC=y'.
> > 'fd_id' should be used to replace 'fd', as 'fd' is not defined in that structure at
> > all. In addition, local variable of 'flexbl' and 'flexbh' must be used only if
> > 32 bytes RX descriptor is selected.
> > 
> > error logs:
> > lib/librte_pmd_i40e/i40e_rxtx.c: In function i40e_rxd_build_fdir:
> > lib/librte_pmd_i40e/i40e_rxtx.c:431:28: error: volatile union <anonymous>
> > has no member named fd
> > lib/librte_pmd_i40e/i40e_rxtx.c:427:19: error: unused variable flexbl [-
> > Werror=unused-variable]
> > lib/librte_pmd_i40e/i40e_rxtx.c:427:11: error: unused variable flexbh [-
> > Werror=unused-variable]
> > 
> > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> 
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>

Applied

Thanks 
-- 
Thomas

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-12-03 10:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-01  7:33 [dpdk-dev] [PATCH] i40e: bug fix of compile error Helin Zhang
2014-12-01 11:12 ` Thomas Monjalon
2014-12-02  0:35   ` Zhang, Helin
2014-12-02  6:58     ` Qiu, Michael
2014-12-03  1:13 ` [dpdk-dev] [PATCH v2] i40e: " Helin Zhang
2014-12-03  7:47   ` Wu, Jingjing
2014-12-03 10:03     ` Thomas Monjalon

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