Re: [dpdk-dev] [PATCH v2 00/26] net/ena: new features and fixes

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 6/6/2018 12:46 PM, Michał Krawczyk wrote:
> Hi Ferruh,
> 
> 2018-06-05 18:42 GMT+02:00 Ferruh Yigit <ferruh.yigit@intel.com>:
>>
>> On 6/4/2018 1:09 PM, Michal Krawczyk wrote:
>>> The ENA driver was updated with the new features and few fixes and minor
>>> changes are introduced.
>>> First of all, the communication layer which is delivered by vendor was
>>> updated - the version in the HEAD is a bit outdated now. ENA is able to
>>> communicate with the driver through Admin queue by using admin interrupts
>>> instead of polling.
>>> Admin interrupts are also used for handling AENQ events, which are used for
>>> the following new features:
>>>   - LSC handlers
>>>   - watchdog and device rest
>>>   - monitoring the admin queue
>>>   - handling ENA notifications (getting hints from device)
>>> For the watchdog and admin queue monitoring, the timers had to be used, so
>>> the makefile was modified to do not cut out the librte_timer.
>>>
>>> From other fixes and changes:
>>>   - legacy LLQ was removed which is now deprecated API
>>>   - Rx out of order completion was added to enable cleaning up packets out
>>>     of order
>>>   - Tx mbufs are now linearized if they exceed supported number of segments
>>>   - pass information about maximum number of Tx and Rx descriptors
>>>   - the IO queue number is now taking into consideration maximum number of
>>>     sq and cq
>>>   - Tx id requested for sending is now being validated and the reset is
>>>     being triggered if it is invalid
>>>   - branch predictioning was added for better performance
>>>   - error checking and returned values were fixed
>>>   - macros for allocating memory in communication layer were fixed
>>>   - information about numa mode is now being passed to the NIC
>>>
>>> ---
>>> v2:
>>> * Rebased on top of dpdk-next-net
>>> * Added link speed patch
>>> * Added fix when allocating coherent memory in the PMD
>>
>> Hi Michał,
>>
>> I am getting build error for ICC [1] and shared library [2], can you please check?
>>
>>
>> [1]
>> .../drivers/net/ena/base/ena_com.c(323): error #592: variable "flags" is used
>> before its value is set
>>         ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags);
>>         ^
>> .../drivers/net/ena/base/ena_com.c(534): error #3656: variable "flags" may be
>> used before its value is set
>>                   ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags);
>>                   ^
>> .../drivers/net/ena/base/ena_com.c(589): error #592: variable "flags" is used
>> before its value is set
>>                 ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags);
>>                 ^
>> .../drivers/net/ena/base/ena_com.c(634): error #592: variable "flags" is used
>> before its value is set
>>         ENA_SPINLOCK_LOCK(mmio_read->lock, flags);
>>         ^
>> .../drivers/net/ena/base/ena_com.c(1297): error #592: variable "flags" is used
>> before its value is set
>>         ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags);
>>         ^
>> .../drivers/net/ena/base/ena_com.c(1341): error #592: variable "flags" is used
>> before its value is set
>>         ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags);
>>         ^
>>
> 
> That should be easy to fix - I prepared the patch but I want to send
> it with second fix together.
> 
>>
>>
>> [2]
>> ena_ethdev.o: In function `eth_ena_pci_probe':
>> .../drivers/net/ena/ena_ethdev.c:(.text+0x6de): undefined reference to
>> `rte_timer_subsystem_init'
>> .../drivers/net/ena/ena_ethdev.c:(.text+0x6ea): undefined reference to
>> `rte_timer_init'
>> ena_ethdev.o: In function `eth_ena_pci_remove':
>> .../drivers/net/ena/ena_ethdev.c:(.text+0x854): undefined reference to
>> `rte_timer_stop_sync'
>> ena_ethdev.o: In function `ena_start':
>> .../drivers/net/ena/ena_ethdev.c:(.text+0x1ff6): undefined reference to
>> `rte_timer_reset'
>> ena_ethdev.o: In function `ena_stop':
>> .../drivers/net/ena/ena_ethdev.c:(.text+0x21a1): undefined reference to
>> `rte_timer_stop_sync'
>> ena_ethdev.o: In function `ena_close':
>> .../drivers/net/ena/ena_ethdev.c:(.text+0x21d8): undefined reference to
>> `rte_timer_stop_sync'
>>
> 
> Strange that this happens, it should be fixed in the patch:
>   mk: link librte_timer with --whole-archive
> which seems to be no longer needed, because similar commit is already
> applied (eb54ef42b02f94c4093556fdd6b51e2d7fd0df47). That should
> resolve the issue with linking (at least for gcc it is helping) by
> linking timer library with --whole-archive.
> Could you, please, send me build logs so I could look closer if it is
> properly built with ICC as I don't have access to it?

Note "--whole-archive" is related to the static library build, but the above
error is related to shared build, via "CONFIG_RTE_BUILD_SHARED_LIB=y" option.

Adding librte_timer as a dependent library should fix the issue, like:
 diff --git a/drivers/net/ena/Makefile b/drivers/net/ena/Makefile
 index 43339f3b9..ff9ce315b 100644
 --- a/drivers/net/ena/Makefile
 +++ b/drivers/net/ena/Makefile
 @@ -58,5 +58,6 @@ CFLAGS += $(INCLUDES)
  LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
  LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
  LDLIBS += -lrte_bus_pci
 +LDLIBS += -lrte_timer

 include $(RTE_SDK)/mk/rte.lib.mk


> 
> Thanks,
> Michał
>