diff mbox

[dpdk-dev] Shared compilation issue - mempool/octeontx: add build and log infrastructure

Message ID 3e228854-d5d4-7dc1-6c25-29de7f6c37c7@caviumnetworks.com (mailing list archive)
State Not Applicable, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Santosh Shukla Oct. 14, 2017, 3:27 p.m. UTC 
  On Saturday 14 October 2017 11:37 AM, Hemant Agrawal wrote:
> Santosh/jerin,
> 	Your patch "mempool/octeontx: add build and log infrastructure" is causing shared build failure for NXP's patch "event/dpaa2: support event eth adapter", which is still under review.
> http://dpdk.org/dev/patchwork/patch/30248/
>
> In your patch, you have added mempool to be dependent on event, which is not correct.
>
> "+DEPDIRS-mempool := bus event"

Yes, Octeontx needed mbox definitions and it was defined at event area.

But You are right that adding event dependency in mempool may create issues
for future feature like you mentioned.

> Ideally, if you have common code, you should put that in bus or mempool . This way you can make event, net or sec etc dependent on bus or mempool  and not the otherway around.

Right.
Proposed change for makefile will be:


Can you give it a try and let me know your feedback.

Thanks.

Comments

Hemant Agrawal Oct. 16, 2017, 6:48 a.m. UTC | #1 
Hi Santosh,

On 10/14/2017 8:57 PM, santosh wrote:
>
> On Saturday 14 October 2017 11:37 AM, Hemant Agrawal wrote:
>> Santosh/jerin,
>> 	Your patch "mempool/octeontx: add build and log infrastructure" is causing shared build failure for NXP's patch "event/dpaa2: support event eth adapter", which is still under review.
>> http://dpdk.org/dev/patchwork/patch/30248/
>>
>> In your patch, you have added mempool to be dependent on event, which is not correct.
>>
>> "+DEPDIRS-mempool := bus event"
>
> Yes, Octeontx needed mbox definitions and it was defined at event area.
>
> But You are right that adding event dependency in mempool may create issues
> for future feature like you mentioned.
>
>> Ideally, if you have common code, you should put that in bus or mempool . This way you can make event, net or sec etc dependent on bus or mempool  and not the otherway around.
>
> Right.
> Proposed change for makefile will be:
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 3a5b22342..0467250aa 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -32,13 +32,13 @@
>  include $(RTE_SDK)/mk/rte.vars.mk
>
>  DIRS-y += bus
> -DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
> -DEPDIRS-event := bus
>  DIRS-y += mempool
> -DEPDIRS-mempool := bus event
> +DEPDIRS-mempool := bus
>  DIRS-y += net
>  DEPDIRS-net := bus mempool
>  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
>  DEPDIRS-crypto := bus mempool
> +DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
> +DEPDIRS-event := bus mempool
>
>  include $(RTE_SDK)/mk/rte.subdir.mk
>
> Can you give it a try and let me know your feedback.
>

This change breaks the octonetx compilation.
You may also have to move the the common code from event to mempool.

Regards,
Hemant

> Thanks.
>
>
Santosh Shukla Oct. 16, 2017, 6:50 a.m. UTC | #2 
On Monday 16 October 2017 12:18 PM, Hemant Agrawal wrote:
> Hi Santosh,
>
> On 10/14/2017 8:57 PM, santosh wrote:
>>
>> On Saturday 14 October 2017 11:37 AM, Hemant Agrawal wrote:
>>> Santosh/jerin,
>>>     Your patch "mempool/octeontx: add build and log infrastructure" is causing shared build failure for NXP's patch "event/dpaa2: support event eth adapter", which is still under review.
>>> http://dpdk.org/dev/patchwork/patch/30248/
>>>
>>> In your patch, you have added mempool to be dependent on event, which is not correct.
>>>
>>> "+DEPDIRS-mempool := bus event"
>>
>> Yes, Octeontx needed mbox definitions and it was defined at event area.
>>
>> But You are right that adding event dependency in mempool may create issues
>> for future feature like you mentioned.
>>
>>> Ideally, if you have common code, you should put that in bus or mempool . This way you can make event, net or sec etc dependent on bus or mempool  and not the otherway around.
>>
>> Right.
>> Proposed change for makefile will be:
>>
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index 3a5b22342..0467250aa 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -32,13 +32,13 @@
>>  include $(RTE_SDK)/mk/rte.vars.mk
>>
>>  DIRS-y += bus
>> -DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
>> -DEPDIRS-event := bus
>>  DIRS-y += mempool
>> -DEPDIRS-mempool := bus event
>> +DEPDIRS-mempool := bus
>>  DIRS-y += net
>>  DEPDIRS-net := bus mempool
>>  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
>>  DEPDIRS-crypto := bus mempool
>> +DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
>> +DEPDIRS-event := bus mempool
>>
>>  include $(RTE_SDK)/mk/rte.subdir.mk
>>
>> Can you give it a try and let me know your feedback.
>>
>
> This change breaks the octonetx compilation.
> You may also have to move the the common code from event to mempool.
>
Yes, I missed mentioning that part in earlier mail.

We'll post series soon.

Thanks.
Santosh Shukla Oct. 16, 2017, 1:47 p.m. UTC | #3 
Hi Hemant,


On Monday 16 October 2017 12:20 PM, santosh wrote:
> On Monday 16 October 2017 12:18 PM, Hemant Agrawal wrote:
>> Hi Santosh,
>>
>> On 10/14/2017 8:57 PM, santosh wrote:
>>> On Saturday 14 October 2017 11:37 AM, Hemant Agrawal wrote:
>>>> Santosh/jerin,
>>>>     Your patch "mempool/octeontx: add build and log infrastructure" is causing shared build failure for NXP's patch "event/dpaa2: support event eth adapter", which is still under review.
>>>> http://dpdk.org/dev/patchwork/patch/30248/
>>>>
>>>> In your patch, you have added mempool to be dependent on event, which is not correct.
>>>>
>>>> "+DEPDIRS-mempool := bus event"
>>> Yes, Octeontx needed mbox definitions and it was defined at event area.
>>>
>>> But You are right that adding event dependency in mempool may create issues
>>> for future feature like you mentioned.
>>>
>>>> Ideally, if you have common code, you should put that in bus or mempool . This way you can make event, net or sec etc dependent on bus or mempool  and not the otherway around.
>>> Right.
>>> Proposed change for makefile will be:
>>>
>>> diff --git a/drivers/Makefile b/drivers/Makefile
>>> index 3a5b22342..0467250aa 100644
>>> --- a/drivers/Makefile
>>> +++ b/drivers/Makefile
>>> @@ -32,13 +32,13 @@
>>>  include $(RTE_SDK)/mk/rte.vars.mk
>>>
>>>  DIRS-y += bus
>>> -DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
>>> -DEPDIRS-event := bus
>>>  DIRS-y += mempool
>>> -DEPDIRS-mempool := bus event
>>> +DEPDIRS-mempool := bus
>>>  DIRS-y += net
>>>  DEPDIRS-net := bus mempool
>>>  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
>>>  DEPDIRS-crypto := bus mempool
>>> +DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
>>> +DEPDIRS-event := bus mempool
>>>
>>>  include $(RTE_SDK)/mk/rte.subdir.mk
>>>
>>> Can you give it a try and let me know your feedback.
>>>
>> This change breaks the octonetx compilation.
>> You may also have to move the the common code from event to mempool.
>>
> Yes, I missed mentioning that part in earlier mail.
>
> We'll post series soon.
>
> Thanks.

Sent fix [1] and [2]. Pl. test with your series.

Thanks.

[1] http://dpdk.org/dev/patchwork/patch/30430/
[2] http://dpdk.org/dev/patchwork/patch/30431/
diff mbox

Patch

diff --git a/drivers/Makefile b/drivers/Makefile
index 3a5b22342..0467250aa 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -32,13 +32,13 @@ 
 include $(RTE_SDK)/mk/rte.vars.mk
 
 DIRS-y += bus
-DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
-DEPDIRS-event := bus
 DIRS-y += mempool
-DEPDIRS-mempool := bus event
+DEPDIRS-mempool := bus
 DIRS-y += net
 DEPDIRS-net := bus mempool
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
 DEPDIRS-crypto := bus mempool
+DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += event
+DEPDIRS-event := bus mempool
 
 include $(RTE_SDK)/mk/rte.subdir.mk