[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Vlad Zolotarov 


On 03/09/15 21:13, Ananyev, Konstantin wrote:
>
>> From: Vladislav Zolotarov [mailto:vladz at cloudius-systems.com]
>> Sent: Monday, March 09, 2015 6:22 PM
>> To: Mcnamara, John
>> Cc: dev at dpdk.org; Ananyev, Konstantin
>> Subject: RE: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>
>>
>> On Mar 9, 2015 8:01 PM, "Mcnamara, John"  wrote:
>>>> -Original Message-
>>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav Zolotarov
>>>> Sent: Monday, March 9, 2015 5:14 PM
>>>> To: Ananyev, Konstantin
>>>> Cc: dev at dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>>>
>>>> On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
>>>> 
>>>> wrote:
>>>>>>> If I remember things correctly, it considered result at right side
>>>>>>> of
>>>> '=' operator as unsigned int,
>>>>>>> and then complained that we assign it to smaller size (unsigned
>>>> short) operand.
>>>>>> If that's the case - that's a clear compiler bug.
>>>>> Might be, though if we still have to support it, there is no much
>>>>> choice
>>>> I am afraid.
>>>>
>>>> Well to begin with could anybody who has this icc thing (preferably the
>>>> latest version) post the compilation errors u are talking about
>>> Hi,
>>>
>>> For what it is worth there aren't any warnings with ICC 13 and the 1/3 
>>> patch (+ the previous patchset):
>>>
>>>$ make T=x86_64-native-linuxapp-icc install CC=icc
>>>Build complete
>>>
>>>$ git log --pretty=format:"%h - %an: %s" -4
>>>b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
>>>3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ...
>>>10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ...
>>>2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_...
>>>
>>>$ icc --version
>>>icc (ICC) 13.1.1 20130313
>> Thanks a lot, John.
> As I said my worry was about 12.*.
> icc v13* and above, are not that picky.
> After applying the patch, indeed all these lines make icc 12.1 to generate 
> warnings.
> See below for details.
> Though with icc 12.1 current dpdk.org main branch will generate ~2K unhandled 
> warnings all over the places anyway...
> Which makes me think that probably DPDK support for icc 12.* was scrapped 
> already, I just didn't notice that.

So, I suppose this concludes this specific discussion, right? ;)

> Konstantin
>
> $ icc -v
> icc version 12.1.0 (gcc version 4.5.0 compatibility)
>
> $ icc -Wp,-MD,./.ixgbe_rxtx.o.d.tmp -m64 -pthread  -march=native 
> -DRTE_MACHINE_CPU
> FLAG_SSE -DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 
> -DRTE_MACHINE_CPU
> FLAG_SSSE3 -DRTE_MACHINE_CPUFLAG_SSE4_1 -DRTE_MACHINE_CPUFLAG_SSE4_2 
> -DRTE_MACHI
> NE_CPUFLAG_AVX 
> -DRTE_COMPILE_TIME_CPUFLAGS=RTE_CPUFLAG_SSE,RTE_CPUFLAG_SSE2,RTE_
> CPUFLAG_SSE3,RTE_CPUFLAG_SSSE3,RTE_CPUFLAG_SSE4_1,RTE_CPUFLAG_SSE4_2,RTE_CPUFLAG
> _AVX  -I/local/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include -include 
> /lo
> cal/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include/rte_config.h -O3 
> -Wall
> -w2 -diag-disable 271 -diag-warning 1478 -diag-disable 13368 -diag-disable 
> 15527   -o ixgbe_rxtx.o -c 
> /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>
> 
>
> /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1035): remark 
> #2259:
> non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose 
> signif
> icant bits
>  alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
>^
>
> /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1064): remark 
> #2259: non-pointer conversion from "int" to "uint16_t={unsigned short}" may 
> lose significant bits
>  rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh;
>   ^
>
> /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1067): remark 
> #2259: non-pointer conversion from "int" to "uint16_t={unsigned short}" may 
> lose significant bits
>  rxq->rx_free_trigger = rxq->rx_free_thresh - 1;
>
> 
>

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Vladislav Zolotarov 
On Mar 9, 2015 8:01 PM, "Mcnamara, John"  wrote:
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav Zolotarov
> > Sent: Monday, March 9, 2015 5:14 PM
> > To: Ananyev, Konstantin
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >
> > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
> > 
> > wrote:
> > > > > If I remember things correctly, it considered result at right side
> > > > > of
> > '=' operator as unsigned int,
> > > > > and then complained that we assign it to smaller size (unsigned
> > short) operand.
> > > >
> > > > If that's the case - that's a clear compiler bug.
> > >
> > > Might be, though if we still have to support it, there is no much
> > > choice
> > I am afraid.
> >
> > Well to begin with could anybody who has this icc thing (preferably the
> > latest version) post the compilation errors u are talking about
>
> Hi,
>
> For what it is worth there aren't any warnings with ICC 13 and the 1/3
patch (+ the previous patchset):
>
>   $ make T=x86_64-native-linuxapp-icc install CC=icc
>   Build complete
>
>   $ git log --pretty=format:"%h - %an: %s" -4
>   b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
>   3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ...
>   10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ...
>   2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_...
>
>   $ icc --version
>   icc (ICC) 13.1.1 20130313

Thanks a lot, John.

>
> John.
> --
>
>

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Vladislav Zolotarov 
On Mar 9, 2015 8:01 PM, "Mcnamara, John"  wrote:
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav Zolotarov
> > Sent: Monday, March 9, 2015 5:14 PM
> > To: Ananyev, Konstantin
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >
> > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
> > 
> > wrote:
> > > > > If I remember things correctly, it considered result at right side
> > > > > of
> > '=' operator as unsigned int,
> > > > > and then complained that we assign it to smaller size (unsigned
> > short) operand.
> > > >
> > > > If that's the case - that's a clear compiler bug.
> > >
> > > Might be, though if we still have to support it, there is no much
> > > choice
> > I am afraid.
> >
> > Well to begin with could anybody who has this icc thing (preferably the
> > latest version) post the compilation errors u are talking about
>
> Hi,
>
> For what it is worth there aren't any warnings with ICC 13 and the 1/3
patch (+ the previous patchset):
>
>   $ make T=x86_64-native-linuxapp-icc install CC=icc
>   Build complete
>
>   $ git log --pretty=format:"%h - %an: %s" -4
>   b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
>   3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ...
>   10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ...
>   2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_...
>
>   $ icc --version
>   icc (ICC) 13.1.1 20130313

That's worth a lot to me!.. :D

>
> John.
> --
>
>

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Ananyev, Konstantin 


> -Original Message-
> From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com]
> Sent: Monday, March 09, 2015 7:32 PM
> To: Ananyev, Konstantin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
> 
> 
> On 03/09/15 21:13, Ananyev, Konstantin wrote:
> >
> >> From: Vladislav Zolotarov [mailto:vladz at cloudius-systems.com]
> >> Sent: Monday, March 09, 2015 6:22 PM
> >> To: Mcnamara, John
> >> Cc: dev at dpdk.org; Ananyev, Konstantin
> >> Subject: RE: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >>
> >>
> >> On Mar 9, 2015 8:01 PM, "Mcnamara, John"  
> >> wrote:
> >>>> -Original Message-
> >>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav 
> >>>> Zolotarov
> >>>> Sent: Monday, March 9, 2015 5:14 PM
> >>>> To: Ananyev, Konstantin
> >>>> Cc: dev at dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >>>>
> >>>> On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
> >>>> 
> >>>> wrote:
> >>>>>>> If I remember things correctly, it considered result at right side
> >>>>>>> of
> >>>> '=' operator as unsigned int,
> >>>>>>> and then complained that we assign it to smaller size (unsigned
> >>>> short) operand.
> >>>>>> If that's the case - that's a clear compiler bug.
> >>>>> Might be, though if we still have to support it, there is no much
> >>>>> choice
> >>>> I am afraid.
> >>>>
> >>>> Well to begin with could anybody who has this icc thing (preferably the
> >>>> latest version) post the compilation errors u are talking about
> >>> Hi,
> >>>
> >>> For what it is worth there aren't any warnings with ICC 13 and the 1/3 
> >>> patch (+ the previous patchset):
> >>>
> >>>$ make T=x86_64-native-linuxapp-icc install CC=icc
> >>>Build complete
> >>>
> >>>$ git log --pretty=format:"%h - %an: %s" -4
> >>>b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
> >>>3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback 
> >>> ...
> >>>10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx 
> >>> ...
> >>>2a5bc6a - Vladislav Zolotarov: ixgbe: Use the 
> >>> rte_le_to_cpu_xx()/rte_...
> >>>
> >>>$ icc --version
> >>>icc (ICC) 13.1.1 20130313
> >> Thanks a lot, John.
> > As I said my worry was about 12.*.
> > icc v13* and above, are not that picky.
> > After applying the patch, indeed all these lines make icc 12.1 to generate 
> > warnings.
> > See below for details.
> > Though with icc 12.1 current dpdk.org main branch will generate ~2K 
> > unhandled warnings all over the places anyway...
> > Which makes me think that probably DPDK support for icc 12.* was scrapped 
> > already, I just didn't notice that.
> 
> So, I suppose this concludes this specific discussion, right? ;)

I believe so.
Sorry for the noise generated.
Konstantin

> 
> > Konstantin
> >
> > $ icc -v
> > icc version 12.1.0 (gcc version 4.5.0 compatibility)
> >
> > $ icc -Wp,-MD,./.ixgbe_rxtx.o.d.tmp -m64 -pthread  -march=native 
> > -DRTE_MACHINE_CPU
> > FLAG_SSE -DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 
> > -DRTE_MACHINE_CPU
> > FLAG_SSSE3 -DRTE_MACHINE_CPUFLAG_SSE4_1 -DRTE_MACHINE_CPUFLAG_SSE4_2 
> > -DRTE_MACHI
> > NE_CPUFLAG_AVX 
> > -DRTE_COMPILE_TIME_CPUFLAGS=RTE_CPUFLAG_SSE,RTE_CPUFLAG_SSE2,RTE_
> > CPUFLAG_SSE3,RTE_CPUFLAG_SSSE3,RTE_CPUFLAG_SSE4_1,RTE_CPUFLAG_SSE4_2,RTE_CPUFLAG
> > _AVX  -I/local/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include 
> > -include /lo
> > cal/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include/rte_config.h -O3 
> > -Wall
> > -w2 -diag-disable 271 -diag-warning 1478 -diag-disable 13368 -diag-disable 
> > 15527   -o ixgbe_rxtx.o -c
> /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >
> > 
> >
> > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1035): remark 
> > #2259:
> > non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose 
> > signif
> > icant bits
> >  alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
> >^
> >
> > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1064): remark 
> > #2259: non-pointer conversion from "int" to
> "uint16_t={unsigned short}" may lose significant bits
> >  rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh;
> >   ^
> >
> > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1067): remark 
> > #2259: non-pointer conversion from "int" to
> "uint16_t={unsigned short}" may lose significant bits
> >  rxq->rx_free_trigger = rxq->rx_free_thresh - 1;
> >
> > 
> >

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Ananyev, Konstantin 


> 
> From: Vladislav Zolotarov [mailto:vladz at cloudius-systems.com]
> Sent: Monday, March 09, 2015 6:22 PM
> To: Mcnamara, John
> Cc: dev at dpdk.org; Ananyev, Konstantin
> Subject: RE: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
> 
> On Mar 9, 2015 8:01 PM, "Mcnamara, John"  wrote:
> >
> > > -Original Message-
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav 
> > > Zolotarov
> > > Sent: Monday, March 9, 2015 5:14 PM
> > > To: Ananyev, Konstantin
> > > Cc: dev at dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> > >
> > > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
> > > 
> > > wrote:
> > > > > > If I remember things correctly, it considered result at right side
> > > > > > of
> > > '=' operator as unsigned int,
> > > > > > and then complained that we assign it to smaller size (unsigned
> > > short) operand.
> > > > >
> > > > > If that's the case - that's a clear compiler bug.
> > > >
> > > > Might be, though if we still have to support it, there is no much
> > > > choice
> > > I am afraid.
> > >
> > > Well to begin with could anybody who has this icc thing (preferably the
> > > latest version) post the compilation errors u are talking about
> >
> > Hi,
> >
> > For what it is worth there aren't any warnings with ICC 13 and the 1/3 
> > patch (+ the previous patchset):
> >
> > ? $ make T=x86_64-native-linuxapp-icc install CC=icc
> > ? Build complete
> >
> > ? $ git log --pretty=format:"%h - %an: %s" -4
> > ? b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
> > ? 3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ...
> > ? 10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ...
> > ? 2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_...
> >
> > ? $ icc --version
> > ? icc (ICC) 13.1.1 20130313
> Thanks a lot, John.

As I said my worry was about 12.*.
icc v13* and above, are not that picky.
After applying the patch, indeed all these lines make icc 12.1 to generate 
warnings.
See below for details.
Though with icc 12.1 current dpdk.org main branch will generate ~2K unhandled 
warnings all over the places anyway...
Which makes me think that probably DPDK support for icc 12.* was scrapped 
already, I just didn't notice that.
Konstantin

$ icc -v
icc version 12.1.0 (gcc version 4.5.0 compatibility)

$ icc -Wp,-MD,./.ixgbe_rxtx.o.d.tmp -m64 -pthread  -march=native 
-DRTE_MACHINE_CPU
FLAG_SSE -DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 -DRTE_MACHINE_CPU
FLAG_SSSE3 -DRTE_MACHINE_CPUFLAG_SSE4_1 -DRTE_MACHINE_CPUFLAG_SSE4_2 -DRTE_MACHI
NE_CPUFLAG_AVX -DRTE_COMPILE_TIME_CPUFLAGS=RTE_CPUFLAG_SSE,RTE_CPUFLAG_SSE2,RTE_
CPUFLAG_SSE3,RTE_CPUFLAG_SSSE3,RTE_CPUFLAG_SSE4_1,RTE_CPUFLAG_SSE4_2,RTE_CPUFLAG
_AVX  -I/local/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include -include /lo
cal/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include/rte_config.h -O3 -Wall
-w2 -diag-disable 271 -diag-warning 1478 -diag-disable 13368 -diag-disable 
15527   -o ixgbe_rxtx.o -c 
/local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c



/local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1035): remark #2259:
non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose signif
icant bits
alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
  ^

/local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1064): remark #2259: 
non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose 
significant bits
rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh;
 ^

/local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1067): remark #2259: 
non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose 
significant bits
rxq->rx_free_trigger = rxq->rx_free_thresh - 1;

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Vladislav Zolotarov 
On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" 
wrote:
>
>
>
> > -Original Message-
> > From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com]
> > Sent: Monday, March 09, 2015 3:58 PM
> > To: Ananyev, Konstantin; Wodkowski, PawelX; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >
> >
> >
> > On 03/09/15 13:29, Ananyev, Konstantin wrote:
> > >
> > >> -Original Message-
> > >> From: Wodkowski, PawelX
> > >> Sent: Monday, March 09, 2015 11:09 AM
> > >> To: Ananyev, Konstantin; Vlad Zolotarov; dev at dpdk.org
> > >> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> > >>
> > >> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> > >>>> Sent: Monday, March 09, 2015 10:21 AM
> > >>>> To: dev at dpdk.org
> > >>>> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> > >>>>
> > >>>>  - Removed the not needed casting.
> > >>>>  - ixgbe_dev_rx_init(): shorten the lines by defining a local
alias variable to access
> > >>>> >data->dev_conf.rxmode.
> > >>>>
> > >>>> Signed-off-by: Vlad Zolotarov 
> > >>>> ---
> > >>>>lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27
---
> > >>>>1 file changed, 12 insertions(+), 15 deletions(-)
> > >>>>
> > >>>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > >>>> index 72c65df..609b5fd 100644
> > >>>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > >>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > >>>> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
> > >>>>  int diag, i;
> > >>>>
> > >>>>  /* allocate buffers in bulk directly into the S/W ring */
> > >>>> -alloc_idx = (uint16_t)(rxq->rx_free_trigger -
> > >>>> -(rxq->rx_free_thresh - 1));
> > >>>> +alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh -
1);
> > >>> I think all these extra casts came in to keep icc 12.* compiling
without warnings.
> > >>> I am agree that they are unnecessary.
> > >>> Though if we still have to support icc 12.* we either need to keep
them, or find
> > >>> some other way to keep it happy.
> > >>> Konstantin
> > >>>
> > >> What warnings icc 12.* is throwing?
> > > Try and see :)
> > >
> > >> Only warning I can think of here is
> > >> signed -> unsigned implicit cast.
> > > If I remember things correctly, it considered result at right side of
'=' operator as unsigned int,
> > > and then complained that we assign it to smaller size (unsigned
short) operand.
> >
> > If that's the case - that's a clear compiler bug.
>
> Might be, though if we still have to support it, there is no much choice
I am afraid.

Well to begin with could anybody who has this icc thing (preferably the
latest version) post the compilation errors u are talking about. Let's
continue this discussion with some specific things in hands. So far there
were a lot of "maybe"s and "as far as i remember"s. Could u, guys, pls., be
more specific so that i could address the real issues and not just your
fears? ;)

Thanks,
Vlad

>
> >
> > >
> > >> Changing '1' to '1U' helps?
> > > Don't think so, but you are welcome to try.
> > >
> > > Konstantin
> > >
> > >>
> > >> --
> > >> Pawel
>
,

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Mcnamara, John 
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladislav Zolotarov
> Sent: Monday, March 9, 2015 5:14 PM
> To: Ananyev, Konstantin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
> On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin"
> 
> wrote:
> > > > If I remember things correctly, it considered result at right side
> > > > of
> '=' operator as unsigned int,
> > > > and then complained that we assign it to smaller size (unsigned
> short) operand.
> > >
> > > If that's the case - that's a clear compiler bug.
> >
> > Might be, though if we still have to support it, there is no much
> > choice
> I am afraid.
> 
> Well to begin with could anybody who has this icc thing (preferably the
> latest version) post the compilation errors u are talking about

Hi,

For what it is worth there aren't any warnings with ICC 13 and the 1/3 patch (+ 
the previous patchset):

  $ make T=x86_64-native-linuxapp-icc install CC=icc
  Build complete

  $ git log --pretty=format:"%h - %an: %s" -4  
  b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups
  3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ...
  10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ...
  2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_...

  $ icc --version
  icc (ICC) 13.1.1 20130313

John.
--

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Vlad Zolotarov 


On 03/09/15 13:29, Ananyev, Konstantin wrote:
>
>> -Original Message-
>> From: Wodkowski, PawelX
>> Sent: Monday, March 09, 2015 11:09 AM
>> To: Ananyev, Konstantin; Vlad Zolotarov; dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>
>> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
>>>
>>>> -Original Message-
>>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>>>> Sent: Monday, March 09, 2015 10:21 AM
>>>> To: dev at dpdk.org
>>>> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>>>
>>>>  - Removed the not needed casting.
>>>>  - ixgbe_dev_rx_init(): shorten the lines by defining a local alias 
>>>> variable to access
>>>> >data->dev_conf.rxmode.
>>>>
>>>> Signed-off-by: Vlad Zolotarov 
>>>> ---
>>>>lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ---
>>>>1 file changed, 12 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
>>>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>>> index 72c65df..609b5fd 100644
>>>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>>> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>>>>int diag, i;
>>>>
>>>>/* allocate buffers in bulk directly into the S/W ring */
>>>> -  alloc_idx = (uint16_t)(rxq->rx_free_trigger -
>>>> -  (rxq->rx_free_thresh - 1));
>>>> +  alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
>>> I think all these extra casts came in to keep icc 12.* compiling without 
>>> warnings.
>>> I am agree that they are unnecessary.
>>> Though if we still have to support icc 12.* we either need to keep them, or 
>>> find
>>> some other way to keep it happy.
>>> Konstantin
>>>
>> What warnings icc 12.* is throwing?
> Try and see :)
>
>> Only warning I can think of here is
>> signed -> unsigned implicit cast.
> If I remember things correctly, it considered result at right side of '=' 
> operator as unsigned int,
> and then complained that we assign it to smaller size (unsigned short) 
> operand.

If that's the case - that's a clear compiler bug.

>
>> Changing '1' to '1U' helps?
> Don't think so, but you are welcome to try.
>
> Konstantin
>
>>
>> --
>> Pawel

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Ananyev, Konstantin 


> -Original Message-
> From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com]
> Sent: Monday, March 09, 2015 3:58 PM
> To: Ananyev, Konstantin; Wodkowski, PawelX; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
> 
> 
> On 03/09/15 13:29, Ananyev, Konstantin wrote:
> >
> >> -Original Message-
> >> From: Wodkowski, PawelX
> >> Sent: Monday, March 09, 2015 11:09 AM
> >> To: Ananyev, Konstantin; Vlad Zolotarov; dev at dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >>
> >> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
> >>>
> >>>> -Original Message-
> >>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> >>>> Sent: Monday, March 09, 2015 10:21 AM
> >>>> To: dev at dpdk.org
> >>>> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >>>>
> >>>>  - Removed the not needed casting.
> >>>>  - ixgbe_dev_rx_init(): shorten the lines by defining a local alias 
> >>>> variable to access
> >>>> >data->dev_conf.rxmode.
> >>>>
> >>>> Signed-off-by: Vlad Zolotarov 
> >>>> ---
> >>>>lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ---
> >>>>1 file changed, 12 insertions(+), 15 deletions(-)
> >>>>
> >>>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> >>>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >>>> index 72c65df..609b5fd 100644
> >>>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >>>> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
> >>>>  int diag, i;
> >>>>
> >>>>  /* allocate buffers in bulk directly into the S/W ring */
> >>>> -alloc_idx = (uint16_t)(rxq->rx_free_trigger -
> >>>> -(rxq->rx_free_thresh - 1));
> >>>> +alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
> >>> I think all these extra casts came in to keep icc 12.* compiling without 
> >>> warnings.
> >>> I am agree that they are unnecessary.
> >>> Though if we still have to support icc 12.* we either need to keep them, 
> >>> or find
> >>> some other way to keep it happy.
> >>> Konstantin
> >>>
> >> What warnings icc 12.* is throwing?
> > Try and see :)
> >
> >> Only warning I can think of here is
> >> signed -> unsigned implicit cast.
> > If I remember things correctly, it considered result at right side of '=' 
> > operator as unsigned int,
> > and then complained that we assign it to smaller size (unsigned short) 
> > operand.
> 
> If that's the case - that's a clear compiler bug.

Might be, though if we still have to support it, there is no much choice I am 
afraid.

> 
> >
> >> Changing '1' to '1U' helps?
> > Don't think so, but you are welcome to try.
> >
> > Konstantin
> >
> >>
> >> --
> >> Pawel

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Vlad Zolotarov 


On 03/09/15 12:49, Ananyev, Konstantin wrote:
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>> Sent: Monday, March 09, 2015 10:21 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>
>> - Removed the not needed casting.
>> - ixgbe_dev_rx_init(): shorten the lines by defining a local alias 
>> variable to access
>>>data->dev_conf.rxmode.
>>
>> Signed-off-by: Vlad Zolotarov 
>> ---
>>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ---
>>   1 file changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> index 72c65df..609b5fd 100644
>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>>  int diag, i;
>>
>>  /* allocate buffers in bulk directly into the S/W ring */
>> -alloc_idx = (uint16_t)(rxq->rx_free_trigger -
>> -(rxq->rx_free_thresh - 1));
>> +alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
> I think all these extra casts came in to keep icc 12.* compiling without 
> warnings.
> I am agree that they are unnecessary.
> Though if we still have to support icc 12.* we either need to keep them, or 
> find
> some other way to keep it happy.
Fix icc maybe?
I'm sorry, but what do I miss here? Both alloc_idx, rxq->rx_free_trigger 
and rxq->rx_free_thresh are uint16_t

So, according to C standard the result is also uint16_t thus no casting 
is needed:

the result of a subtraction generating a negative number in an unsigned type is 
well-defined:

 1. [...] A computation involving unsigned operands can never
overflow, because a result that cannot be represented by the
resulting unsigned integer type is reduced modulo the number
that is one greater than the largest value that can be
represented by the resulting type. (ISO/IEC 9899:1999 (E) ?6.2.5/9)


Could u, pls., be more specific and send here the error generated by icc 
after my patches?

thanks,
vlad



> Konstantin
>
>>  rxep = >sw_ring[alloc_idx];
>>  diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep,
>>  rxq->rx_free_thresh);
>> @@ -1061,10 +1060,9 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>>  IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger);
>>
>>  /* update state of internal queue structure */
>> -rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_trigger +
>> -rxq->rx_free_thresh);
>> +rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh;
>>  if (rxq->rx_free_trigger >= rxq->nb_rx_desc)
>> -rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_thresh - 1);
>> +rxq->rx_free_trigger = rxq->rx_free_thresh - 1;
>>
>>  /* no errors */
>>  return 0;
>> @@ -3560,6 +3558,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>  uint32_t rxcsum;
>>  uint16_t buf_size;
>>  uint16_t i;
>> +struct rte_eth_rxmode *rx_conf = >data->dev_conf.rxmode;
>>
>>  PMD_INIT_FUNC_TRACE();
>>  hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>> @@ -3582,7 +3581,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>   * Configure CRC stripping, if any.
>>   */
>>  hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
>> -if (dev->data->dev_conf.rxmode.hw_strip_crc)
>> +if (rx_conf->hw_strip_crc)
>>  hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
>>  else
>>  hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP;
>> @@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>  /*
>>   * Configure jumbo frame support, if any.
>>   */
>> -if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
>> +if (rx_conf->jumbo_frame == 1) {
>>  hlreg0 |= IXGBE_HLREG0_JUMBOEN;
>>  maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS);
>>  maxfrs &= 0x;
>> -maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16);
>> +maxfrs |= (rx_conf->max_rx_pkt_len << 16);
>>  IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs);
>>  } else
>>  hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
>> @@ -3618,9 +3617,

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Vlad Zolotarov 
   - Removed the not needed casting.
   - ixgbe_dev_rx_init(): shorten the lines by defining a local alias variable 
to access
  >data->dev_conf.rxmode.

Signed-off-by: Vlad Zolotarov 
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ---
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 72c65df..609b5fd 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
int diag, i;

/* allocate buffers in bulk directly into the S/W ring */
-   alloc_idx = (uint16_t)(rxq->rx_free_trigger -
-   (rxq->rx_free_thresh - 1));
+   alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
rxep = >sw_ring[alloc_idx];
diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep,
rxq->rx_free_thresh);
@@ -1061,10 +1060,9 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger);

/* update state of internal queue structure */
-   rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_trigger +
-   rxq->rx_free_thresh);
+   rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh;
if (rxq->rx_free_trigger >= rxq->nb_rx_desc)
-   rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_thresh - 1);
+   rxq->rx_free_trigger = rxq->rx_free_thresh - 1;

/* no errors */
return 0;
@@ -3560,6 +3558,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
uint32_t rxcsum;
uint16_t buf_size;
uint16_t i;
+   struct rte_eth_rxmode *rx_conf = >data->dev_conf.rxmode;

PMD_INIT_FUNC_TRACE();
hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -3582,7 +3581,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 * Configure CRC stripping, if any.
 */
hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
-   if (dev->data->dev_conf.rxmode.hw_strip_crc)
+   if (rx_conf->hw_strip_crc)
hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
else
hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP;
@@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
/*
 * Configure jumbo frame support, if any.
 */
-   if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
+   if (rx_conf->jumbo_frame == 1) {
hlreg0 |= IXGBE_HLREG0_JUMBOEN;
maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS);
maxfrs &= 0x;
-   maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16);
+   maxfrs |= (rx_conf->max_rx_pkt_len << 16);
IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs);
} else
hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
@@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 * Reset crc_len in case it was changed after queue setup by a
 * call to configure.
 */
-   rxq->crc_len = (uint8_t)
-   ((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 :
-   ETHER_CRC_LEN);
+   rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN;

/* Setup the Base and Length of the Rx Descriptor Rings */
bus_addr = rxq->rx_ring_phys_addr;
@@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
/*
 * Configure Header Split
 */
-   if (dev->data->dev_conf.rxmode.header_split) {
+   if (rx_conf->header_split) {
if (hw->mac.type == ixgbe_mac_82599EB) {
/* Must setup the PSRTYPE register */
uint32_t psrtype;
@@ -3648,7 +3645,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
IXGBE_PSRTYPE_IPV6HDR;
IXGBE_WRITE_REG(hw, 
IXGBE_PSRTYPE(rxq->reg_idx), psrtype);
}
-   srrctl = ((dev->data->dev_conf.rxmode.split_hdr_size <<
+   srrctl = ((rx_conf->split_hdr_size <<
IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) &
IXGBE_SRRCTL_BSIZEHDR_MASK);
srrctl |= IXGBE_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS;
@@ -3699,7 +3696,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 */
rxcsum = IXGBE_READ_REG(hw, IXGBE_RXCSUM);
rxcsum |= IXGBE_RXCSUM_PCSD;
-   if (dev->data->dev_conf.rxmode.hw_ip_checksum)
+   if (rx_conf->hw_ip_checksum)
rxcsum |= IXGBE_RXCSUM_IPPCSE;
else
rxcsum &= ~IXGBE_RXCSUM_IPPCSE;
@@ -3709,7 +3706,7 @@ ixgbe_dev_rx_init(struct

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Pawel Wodkowski 
On 2015-03-09 11:49, Ananyev, Konstantin wrote:
>
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>> Sent: Monday, March 09, 2015 10:21 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>
>> - Removed the not needed casting.
>> - ixgbe_dev_rx_init(): shorten the lines by defining a local alias 
>> variable to access
>>>data->dev_conf.rxmode.
>>
>> Signed-off-by: Vlad Zolotarov 
>> ---
>>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ---
>>   1 file changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> index 72c65df..609b5fd 100644
>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>>  int diag, i;
>>
>>  /* allocate buffers in bulk directly into the S/W ring */
>> -alloc_idx = (uint16_t)(rxq->rx_free_trigger -
>> -(rxq->rx_free_thresh - 1));
>> +alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
>
> I think all these extra casts came in to keep icc 12.* compiling without 
> warnings.
> I am agree that they are unnecessary.
> Though if we still have to support icc 12.* we either need to keep them, or 
> find
> some other way to keep it happy.
> Konstantin
>

What warnings icc 12.* is throwing? Only warning I can think of here is 
signed -> unsigned implicit cast. Changing '1' to '1U' helps?


-- 
Pawel

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Ananyev, Konstantin 


> -Original Message-
> From: Wodkowski, PawelX
> Sent: Monday, March 09, 2015 11:09 AM
> To: Ananyev, Konstantin; Vlad Zolotarov; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
> >
> >
> >> -Original Message-
> >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> >> Sent: Monday, March 09, 2015 10:21 AM
> >> To: dev at dpdk.org
> >> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >>
> >> - Removed the not needed casting.
> >> - ixgbe_dev_rx_init(): shorten the lines by defining a local alias 
> >> variable to access
> >>>data->dev_conf.rxmode.
> >>
> >> Signed-off-by: Vlad Zolotarov 
> >> ---
> >>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ---
> >>   1 file changed, 12 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> >> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> index 72c65df..609b5fd 100644
> >> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> >> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
> >>int diag, i;
> >>
> >>/* allocate buffers in bulk directly into the S/W ring */
> >> -  alloc_idx = (uint16_t)(rxq->rx_free_trigger -
> >> -  (rxq->rx_free_thresh - 1));
> >> +  alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
> >
> > I think all these extra casts came in to keep icc 12.* compiling without 
> > warnings.
> > I am agree that they are unnecessary.
> > Though if we still have to support icc 12.* we either need to keep them, or 
> > find
> > some other way to keep it happy.
> > Konstantin
> >
> 
> What warnings icc 12.* is throwing? 

Try and see :)

>Only warning I can think of here is
> signed -> unsigned implicit cast. 

If I remember things correctly, it considered result at right side of '=' 
operator as unsigned int,
and then complained that we assign it to smaller size (unsigned short) operand. 
 

>Changing '1' to '1U' helps?

Don't think so, but you are welcome to try.

Konstantin

> 
> 
> --
> Pawel

[dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

2015-03-09 Thread Ananyev, Konstantin 


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
> Sent: Monday, March 09, 2015 10:21 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> 
>- Removed the not needed casting.
>- ixgbe_dev_rx_init(): shorten the lines by defining a local alias 
> variable to access
>   >data->dev_conf.rxmode.
> 
> Signed-off-by: Vlad Zolotarov 
> ---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ---
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 72c65df..609b5fd 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>   int diag, i;
> 
>   /* allocate buffers in bulk directly into the S/W ring */
> - alloc_idx = (uint16_t)(rxq->rx_free_trigger -
> - (rxq->rx_free_thresh - 1));
> + alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);

I think all these extra casts came in to keep icc 12.* compiling without 
warnings.
I am agree that they are unnecessary.
Though if we still have to support icc 12.* we either need to keep them, or find
some other way to keep it happy.
Konstantin  

>   rxep = >sw_ring[alloc_idx];
>   diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep,
>   rxq->rx_free_thresh);
> @@ -1061,10 +1060,9 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>   IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger);
> 
>   /* update state of internal queue structure */
> - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_trigger +
> - rxq->rx_free_thresh);
> + rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh;
>   if (rxq->rx_free_trigger >= rxq->nb_rx_desc)
> - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_thresh - 1);
> + rxq->rx_free_trigger = rxq->rx_free_thresh - 1;
> 
>   /* no errors */
>   return 0;
> @@ -3560,6 +3558,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>   uint32_t rxcsum;
>   uint16_t buf_size;
>   uint16_t i;
> + struct rte_eth_rxmode *rx_conf = >data->dev_conf.rxmode;
> 
>   PMD_INIT_FUNC_TRACE();
>   hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> @@ -3582,7 +3581,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>* Configure CRC stripping, if any.
>*/
>   hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> - if (dev->data->dev_conf.rxmode.hw_strip_crc)
> + if (rx_conf->hw_strip_crc)
>   hlreg0 |= IXGBE_HLREG0_RXCRCSTRP;
>   else
>   hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP;
> @@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>   /*
>* Configure jumbo frame support, if any.
>*/
> - if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
> + if (rx_conf->jumbo_frame == 1) {
>   hlreg0 |= IXGBE_HLREG0_JUMBOEN;
>   maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS);
>   maxfrs &= 0x;
> - maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16);
> + maxfrs |= (rx_conf->max_rx_pkt_len << 16);
>   IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs);
>   } else
>   hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
> @@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>* Reset crc_len in case it was changed after queue setup by a
>* call to configure.
>*/
> - rxq->crc_len = (uint8_t)
> - ((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 :
> - ETHER_CRC_LEN);
> + rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN;
> 
>   /* Setup the Base and Length of the Rx Descriptor Rings */
>   bus_addr = rxq->rx_ring_phys_addr;
> @@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>   /*
>* Configure Header Split
>*/
> - if (dev->data->dev_conf.rxmode.header_split) {
> + if (rx_conf->header_split) {
>   if (hw->mac.type == ixgbe_mac_82599EB) {
>   /* Must setup the PSRTYPE register */
>   uint32_t psrtype;
>