DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH BUG 335 1/3] net/dpaa: fix compilation error with 0 headroom
@ 2019-07-25 11:06 Hemant Agrawal
  2019-07-25 11:06 ` [dpdk-dev] [PATCH BUG 335 2/3] bus/fslmc: " Hemant Agrawal
  2019-07-25 11:06 ` [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: " Hemant Agrawal
  0 siblings, 2 replies; 8+ messages in thread
From: Hemant Agrawal @ 2019-07-25 11:06 UTC (permalink / raw)
  To: dev, matan; +Cc: stable

When using RTE_PKTMBUF_HEADROOM as 0, dpaa driver throws compilation error
error "Annotation requirement is more than RTE_PKTMBUF_HEADROOM"

This patch change it into run-time check.
Reported as: https://bugs.dpdk.org/show_bug.cgi?id=335

Fixes: ff9e112d7870 ("net/dpaa: add NXP DPAA PMD driver skeleton")
Cc: stable@dpdk.org

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 drivers/net/dpaa/dpaa_ethdev.c | 10 ++++++++++
 drivers/net/dpaa/dpaa_ethdev.h |  4 ----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index adc0bd5ac..7154fb9b4 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -1469,6 +1469,16 @@ rte_dpaa_probe(struct rte_dpaa_driver *dpaa_drv __rte_unused,
 
 	PMD_INIT_FUNC_TRACE();
 
+	if ((DPAA_MBUF_HW_ANNOTATION + DPAA_FD_PTA_SIZE) >
+		RTE_PKTMBUF_HEADROOM) {
+		DPAA_PMD_ERR(
+		"RTE_PKTMBUF_HEADROOM(%d) shall be > DPAA Annotation req(%d)",
+		RTE_PKTMBUF_HEADROOM,
+		DPAA_MBUF_HW_ANNOTATION + DPAA_FD_PTA_SIZE);
+
+		return -1;
+	}
+
 	/* In case of secondary process, the device is already configured
 	 * and no further action is required, except portal initialization
 	 * and verifying secondary attachment to port name.
diff --git a/drivers/net/dpaa/dpaa_ethdev.h b/drivers/net/dpaa/dpaa_ethdev.h
index 18bc7dfa8..f63a5f164 100644
--- a/drivers/net/dpaa/dpaa_ethdev.h
+++ b/drivers/net/dpaa/dpaa_ethdev.h
@@ -22,10 +22,6 @@
 #define DPAA_MBUF_HW_ANNOTATION		64
 #define DPAA_FD_PTA_SIZE		64
 
-#if (DPAA_MBUF_HW_ANNOTATION + DPAA_FD_PTA_SIZE) > RTE_PKTMBUF_HEADROOM
-#error "Annotation requirement is more than RTE_PKTMBUF_HEADROOM"
-#endif
-
 /* mbuf->seqn will be used to store event entry index for
  * driver specific usage. For parallel mode queues, invalid
  * index will be set and for atomic mode queues, valid value
-- 
2.17.1


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

* [dpdk-dev] [PATCH BUG 335 2/3] bus/fslmc: fix compilation error with 0 headroom
  2019-07-25 11:06 [dpdk-dev] [PATCH BUG 335 1/3] net/dpaa: fix compilation error with 0 headroom Hemant Agrawal
@ 2019-07-25 11:06 ` Hemant Agrawal
  2019-07-25 11:06 ` [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: " Hemant Agrawal
  1 sibling, 0 replies; 8+ messages in thread
From: Hemant Agrawal @ 2019-07-25 11:06 UTC (permalink / raw)
  To: dev, matan; +Cc: stable

When using RTE_PKTMBUF_HEADROOM as 0, dpaa driver throws compilation error
error "Annotation requirement is more than RTE_PKTMBUF_HEADROOM"

This patch change it into run-time check.
Reported as: https://bugs.dpdk.org/show_bug.cgi?id=335

Fixes: beb2a7865dda ("bus/fslmc: define hardware annotation area size")
Cc: stable@dpdk.org

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 drivers/bus/fslmc/portal/dpaa2_hw_pvt.h |  4 ----
 drivers/net/dpaa2/dpaa2_ethdev.c        | 10 ++++++++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h b/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
index 8644761db..4bb6b26c7 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
@@ -72,10 +72,6 @@
 #define DPAA2_MBUF_HW_ANNOTATION	64
 #define DPAA2_FD_PTA_SIZE		0
 
-#if (DPAA2_MBUF_HW_ANNOTATION + DPAA2_FD_PTA_SIZE) > RTE_PKTMBUF_HEADROOM
-#error "Annotation requirement is more than RTE_PKTMBUF_HEADROOM"
-#endif
-
 /* we will re-use the HEADROOM for annotation in RX */
 #define DPAA2_HW_BUF_RESERVE	0
 #define DPAA2_PACKET_LAYOUT_ALIGN	64 /*changing from 256 */
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 03f69599c..dd6a78f9f 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -2319,6 +2319,16 @@ rte_dpaa2_probe(struct rte_dpaa2_driver *dpaa2_drv,
 	struct rte_eth_dev *eth_dev;
 	int diag;
 
+	if ((DPAA2_MBUF_HW_ANNOTATION + DPAA2_FD_PTA_SIZE) >
+		RTE_PKTMBUF_HEADROOM) {
+		DPAA2_PMD_ERR(
+		"RTE_PKTMBUF_HEADROOM(%d) shall be > DPAA2 Annotation req(%d)",
+		RTE_PKTMBUF_HEADROOM,
+		DPAA2_MBUF_HW_ANNOTATION + DPAA2_FD_PTA_SIZE);
+
+		return -1;
+	}
+
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		eth_dev = rte_eth_dev_allocate(dpaa2_dev->device.name);
 		if (!eth_dev)
-- 
2.17.1


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

* [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: fix compilation error with 0 headroom
  2019-07-25 11:06 [dpdk-dev] [PATCH BUG 335 1/3] net/dpaa: fix compilation error with 0 headroom Hemant Agrawal
  2019-07-25 11:06 ` [dpdk-dev] [PATCH BUG 335 2/3] bus/fslmc: " Hemant Agrawal
@ 2019-07-25 11:06 ` Hemant Agrawal
  2019-07-26 12:25   ` Olivier Matz
  1 sibling, 1 reply; 8+ messages in thread
From: Hemant Agrawal @ 2019-07-25 11:06 UTC (permalink / raw)
  To: dev, matan; +Cc: stable, Olivier Matz

When using RTE_PKTMBUF_HEADROOM as 0, virito ethdev driver throws
compilation error
virtio_ethdev.c:1851:2: note: in expansion of macro ‘RTE_BUILD_BUG_ON’
RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM
	< sizeof(struct virtio_net_hdr_mrg_rxbuf));

This patch change it into run-time check.
Reported as: https://bugs.dpdk.org/show_bug.cgi?id=335

Fixes: 198ab33677c9 ("net/virtio: move device initialization in a function")
Cc: stable@dpdk.org
Cc: Olivier Matz <olivier.matz@6wind.com>

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 drivers/net/virtio/virtio_ethdev.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 62c827443..3e2e8bd2a 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1848,7 +1848,14 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	struct virtio_hw *hw = eth_dev->data->dev_private;
 	int ret;
 
-	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf));
+	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
+		PMD_INIT_LOG(ERR,
+			"Not sufficient headroom required = %d, avail = %d",
+			(int)sizeof(struct virtio_net_hdr_mrg_rxbuf),
+			RTE_PKTMBUF_HEADROOM);
+
+		return -1;
+	}
 
 	eth_dev->dev_ops = &virtio_eth_dev_ops;
 
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: fix compilation error with 0 headroom
  2019-07-25 11:06 ` [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: " Hemant Agrawal
@ 2019-07-26 12:25   ` Olivier Matz
  2019-07-30 13:35     ` Maxime Coquelin
  0 siblings, 1 reply; 8+ messages in thread
From: Olivier Matz @ 2019-07-26 12:25 UTC (permalink / raw)
  To: Hemant Agrawal; +Cc: dev, matan, stable, Stephen Hemminger

Hi,

On Thu, Jul 25, 2019 at 04:36:45PM +0530, Hemant Agrawal wrote:
> When using RTE_PKTMBUF_HEADROOM as 0, virito ethdev driver throws
> compilation error
> virtio_ethdev.c:1851:2: note: in expansion of macro ‘RTE_BUILD_BUG_ON’
> RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM
> 	< sizeof(struct virtio_net_hdr_mrg_rxbuf));
> 
> This patch change it into run-time check.
> Reported as: https://bugs.dpdk.org/show_bug.cgi?id=335
> 
> Fixes: 198ab33677c9 ("net/virtio: move device initialization in a function")

I think the proper commit is:
Fixes: dec08c28c0b3 ("virtio: check packet headroom at compile time")

It looks this patch more or less reverts this old commit.
+CC Stephen


> Cc: stable@dpdk.org
> Cc: Olivier Matz <olivier.matz@6wind.com>
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 62c827443..3e2e8bd2a 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1848,7 +1848,14 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>  	struct virtio_hw *hw = eth_dev->data->dev_private;
>  	int ret;
>  
> -	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf));
> +	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
> +		PMD_INIT_LOG(ERR,
> +			"Not sufficient headroom required = %d, avail = %d",
> +			(int)sizeof(struct virtio_net_hdr_mrg_rxbuf),
> +			RTE_PKTMBUF_HEADROOM);
> +
> +		return -1;
> +	}
>  
>  	eth_dev->dev_ops = &virtio_eth_dev_ops;
>  
> -- 
> 2.17.1
> 


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

* Re: [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: fix compilation error with 0 headroom
  2019-07-26 12:25   ` Olivier Matz
@ 2019-07-30 13:35     ` Maxime Coquelin
  2019-08-01  8:09       ` Hemant Agrawal
  0 siblings, 1 reply; 8+ messages in thread
From: Maxime Coquelin @ 2019-07-30 13:35 UTC (permalink / raw)
  To: Olivier Matz, Hemant Agrawal; +Cc: dev, matan, stable, Stephen Hemminger



On 7/26/19 2:25 PM, Olivier Matz wrote:
> Hi,
> 
> On Thu, Jul 25, 2019 at 04:36:45PM +0530, Hemant Agrawal wrote:
>> When using RTE_PKTMBUF_HEADROOM as 0, virito ethdev driver throws
>> compilation error
>> virtio_ethdev.c:1851:2: note: in expansion of macro ‘RTE_BUILD_BUG_ON’
>> RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM
>> 	< sizeof(struct virtio_net_hdr_mrg_rxbuf));
>>
>> This patch change it into run-time check.
>> Reported as: https://bugs.dpdk.org/show_bug.cgi?id=335
>>
>> Fixes: 198ab33677c9 ("net/virtio: move device initialization in a function")
> 
> I think the proper commit is:
> Fixes: dec08c28c0b3 ("virtio: check packet headroom at compile time")

Indeed.

> It looks this patch more or less reverts this old commit.
> +CC Stephen

I wonder whether we could have a warning at build time so that the one
who builds DPDK is aware some driver may not be usable, in addition to
the below patch that fails virtio-net init.

Any thoughts?

Thanks,
Maxime

>> Cc: stable@dpdk.org
>> Cc: Olivier Matz <olivier.matz@6wind.com>
>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>>   drivers/net/virtio/virtio_ethdev.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>> index 62c827443..3e2e8bd2a 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -1848,7 +1848,14 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>>   	struct virtio_hw *hw = eth_dev->data->dev_private;
>>   	int ret;
>>   
>> -	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf));
>> +	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
>> +		PMD_INIT_LOG(ERR,
>> +			"Not sufficient headroom required = %d, avail = %d",
>> +			(int)sizeof(struct virtio_net_hdr_mrg_rxbuf),
>> +			RTE_PKTMBUF_HEADROOM);
>> +
>> +		return -1;
>> +	}
>>   
>>   	eth_dev->dev_ops = &virtio_eth_dev_ops;
>>   
>> -- 
>> 2.17.1
>>
> 

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

* Re: [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: fix compilation error with 0 headroom
  2019-07-30 13:35     ` Maxime Coquelin
@ 2019-08-01  8:09       ` Hemant Agrawal
  2019-08-05 13:07         ` Maxime Coquelin
  0 siblings, 1 reply; 8+ messages in thread
From: Hemant Agrawal @ 2019-08-01  8:09 UTC (permalink / raw)
  To: Maxime Coquelin, Olivier Matz; +Cc: dev, matan, stable, Stephen Hemminger

HI,
> On 7/26/19 2:25 PM, Olivier Matz wrote:
> > Hi,
> >
> > On Thu, Jul 25, 2019 at 04:36:45PM +0530, Hemant Agrawal wrote:
> >> When using RTE_PKTMBUF_HEADROOM as 0, virito ethdev driver throws
> >> compilation error
> >> virtio_ethdev.c:1851:2: note: in expansion of macro ‘RTE_BUILD_BUG_ON’
> >> RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM
> >> 	< sizeof(struct virtio_net_hdr_mrg_rxbuf));
> >>
> >> This patch change it into run-time check.
> >> Reported as:
> >>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbug
> >>
> s.dpdk.org%2Fshow_bug.cgi%3Fid%3D335&amp;data=02%7C01%7Chemant.
> agrawa
> >>
> l%40nxp.com%7C303b7755f6ff4c44721a08d714f2cbb5%7C686ea1d3bc2b4c6
> fa92c
> >>
> d99c5c301635%7C0%7C0%7C637000905339982654&amp;sdata=oQ4b4TZ36x
> WiHOtKE
> >> %2BwCZfy2ND%2FgP%2FESfGL%2FEdoJbYI%3D&amp;reserved=0
> >>
> >> Fixes: 198ab33677c9 ("net/virtio: move device initialization in a
> >> function")
> >
> > I think the proper commit is:
> > Fixes: dec08c28c0b3 ("virtio: check packet headroom at compile time")
> 
> Indeed.
> 
> > It looks this patch more or less reverts this old commit.
> > +CC Stephen
> 
> I wonder whether we could have a warning at build time so that the one who
> builds DPDK is aware some driver may not be usable, in addition to the
> below patch that fails virtio-net init.

[Hemant] I will also prefer compile time check instead of run-time check for any non-default configs.
If someone is modifying the config, he can very well disable the drivers, which don't like those settings. 

However, earlier discussion w.r.t this bug moved in other direction to make DPDK compliable for all cases and allow regress testing.

> 
> Any thoughts?
> 
> Thanks,
> Maxime
> 
> >> Cc: stable@dpdk.org
> >> Cc: Olivier Matz <olivier.matz@6wind.com>
> >>
> >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >> ---
> >>   drivers/net/virtio/virtio_ethdev.c | 9 ++++++++-
> >>   1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/virtio/virtio_ethdev.c
> >> b/drivers/net/virtio/virtio_ethdev.c
> >> index 62c827443..3e2e8bd2a 100644
> >> --- a/drivers/net/virtio/virtio_ethdev.c
> >> +++ b/drivers/net/virtio/virtio_ethdev.c
> >> @@ -1848,7 +1848,14 @@ eth_virtio_dev_init(struct rte_eth_dev
> *eth_dev)
> >>   	struct virtio_hw *hw = eth_dev->data->dev_private;
> >>   	int ret;
> >>
> >> -	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct
> virtio_net_hdr_mrg_rxbuf));
> >> +	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) >
> RTE_PKTMBUF_HEADROOM) {
> >> +		PMD_INIT_LOG(ERR,
> >> +			"Not sufficient headroom required = %d, avail = %d",
> >> +			(int)sizeof(struct virtio_net_hdr_mrg_rxbuf),
> >> +			RTE_PKTMBUF_HEADROOM);
> >> +
> >> +		return -1;
> >> +	}
> >>
> >>   	eth_dev->dev_ops = &virtio_eth_dev_ops;
> >>
> >> --
> >> 2.17.1
> >>
> >

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

* Re: [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: fix compilation error with 0 headroom
  2019-08-01  8:09       ` Hemant Agrawal
@ 2019-08-05 13:07         ` Maxime Coquelin
  2019-08-05 17:26           ` Thomas Monjalon
  0 siblings, 1 reply; 8+ messages in thread
From: Maxime Coquelin @ 2019-08-05 13:07 UTC (permalink / raw)
  To: Hemant Agrawal, Olivier Matz; +Cc: dev, matan, stable, Stephen Hemminger



On 8/1/19 10:09 AM, Hemant Agrawal wrote:
> HI,
>> On 7/26/19 2:25 PM, Olivier Matz wrote:
>>> Hi,
>>>
>>> On Thu, Jul 25, 2019 at 04:36:45PM +0530, Hemant Agrawal wrote:
>>>> When using RTE_PKTMBUF_HEADROOM as 0, virito ethdev driver throws
>>>> compilation error
>>>> virtio_ethdev.c:1851:2: note: in expansion of macro ‘RTE_BUILD_BUG_ON’
>>>> RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM
>>>> 	< sizeof(struct virtio_net_hdr_mrg_rxbuf));
>>>>
>>>> This patch change it into run-time check.
>>>> Reported as:
>>>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbug
>>>>
>> s.dpdk.org%2Fshow_bug.cgi%3Fid%3D335&amp;data=02%7C01%7Chemant.
>> agrawa
>>>>
>> l%40nxp.com%7C303b7755f6ff4c44721a08d714f2cbb5%7C686ea1d3bc2b4c6
>> fa92c
>>>>
>> d99c5c301635%7C0%7C0%7C637000905339982654&amp;sdata=oQ4b4TZ36x
>> WiHOtKE
>>>> %2BwCZfy2ND%2FgP%2FESfGL%2FEdoJbYI%3D&amp;reserved=0
>>>>
>>>> Fixes: 198ab33677c9 ("net/virtio: move device initialization in a
>>>> function")
>>>
>>> I think the proper commit is:
>>> Fixes: dec08c28c0b3 ("virtio: check packet headroom at compile time")
>>
>> Indeed.
>>
>>> It looks this patch more or less reverts this old commit.
>>> +CC Stephen
>>
>> I wonder whether we could have a warning at build time so that the one who
>> builds DPDK is aware some driver may not be usable, in addition to the
>> below patch that fails virtio-net init.
> 
> [Hemant] I will also prefer compile time check instead of run-time check for any non-default configs.
> If someone is modifying the config, he can very well disable the drivers, which don't like those settings. 
> 
> However, earlier discussion w.r.t this bug moved in other direction to make DPDK compliable for all cases and allow regress testing.

Ok, I don't have a strong opinion on this, so feel free to apply this
patch as is. We can add a build-ime warning later if we find it useful.

Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

> 
>>
>> Any thoughts?
>>
>> Thanks,
>> Maxime
>>
>>>> Cc: stable@dpdk.org
>>>> Cc: Olivier Matz <olivier.matz@6wind.com>
>>>>
>>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>>> ---
>>>>   drivers/net/virtio/virtio_ethdev.c | 9 ++++++++-
>>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/virtio/virtio_ethdev.c
>>>> b/drivers/net/virtio/virtio_ethdev.c
>>>> index 62c827443..3e2e8bd2a 100644
>>>> --- a/drivers/net/virtio/virtio_ethdev.c
>>>> +++ b/drivers/net/virtio/virtio_ethdev.c
>>>> @@ -1848,7 +1848,14 @@ eth_virtio_dev_init(struct rte_eth_dev
>> *eth_dev)
>>>>   	struct virtio_hw *hw = eth_dev->data->dev_private;
>>>>   	int ret;
>>>>
>>>> -	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct
>> virtio_net_hdr_mrg_rxbuf));
>>>> +	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) >
>> RTE_PKTMBUF_HEADROOM) {
>>>> +		PMD_INIT_LOG(ERR,
>>>> +			"Not sufficient headroom required = %d, avail = %d",
>>>> +			(int)sizeof(struct virtio_net_hdr_mrg_rxbuf),
>>>> +			RTE_PKTMBUF_HEADROOM);
>>>> +
>>>> +		return -1;
>>>> +	}
>>>>
>>>>   	eth_dev->dev_ops = &virtio_eth_dev_ops;
>>>>
>>>> --
>>>> 2.17.1
>>>>
>>>

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

* Re: [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: fix compilation error with 0 headroom
  2019-08-05 13:07         ` Maxime Coquelin
@ 2019-08-05 17:26           ` Thomas Monjalon
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2019-08-05 17:26 UTC (permalink / raw)
  To: Hemant Agrawal
  Cc: dev, Maxime Coquelin, Olivier Matz, matan, stable, Stephen Hemminger

05/08/2019 15:07, Maxime Coquelin:
> On 8/1/19 10:09 AM, Hemant Agrawal wrote:
> >> On 7/26/19 2:25 PM, Olivier Matz wrote:
> >>> On Thu, Jul 25, 2019 at 04:36:45PM +0530, Hemant Agrawal wrote:
> >>>> When using RTE_PKTMBUF_HEADROOM as 0, virito ethdev driver throws
> >>>> compilation error
> >>>> virtio_ethdev.c:1851:2: note: in expansion of macro ‘RTE_BUILD_BUG_ON’
> >>>> RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM
> >>>> 	< sizeof(struct virtio_net_hdr_mrg_rxbuf));
> >>>>
> >>>> This patch change it into run-time check.
[...]
> >>>>
> >>>> Fixes: 198ab33677c9 ("net/virtio: move device initialization in a
> >>>> function")
> >>>
> >>> I think the proper commit is:
> >>> Fixes: dec08c28c0b3 ("virtio: check packet headroom at compile time")
> >>
> >> Indeed.
> >>
> >>> It looks this patch more or less reverts this old commit.
> >>> +CC Stephen
> >>
> >> I wonder whether we could have a warning at build time so that the one who
> >> builds DPDK is aware some driver may not be usable, in addition to the
> >> below patch that fails virtio-net init.
> > 
> > [Hemant] I will also prefer compile time check instead of run-time check for any non-default configs.
> > If someone is modifying the config, he can very well disable the drivers, which don't like those settings. 
> > 
> > However, earlier discussion w.r.t this bug moved in other direction to make DPDK compliable for all cases and allow regress testing.
> 
> Ok, I don't have a strong opinion on this, so feel free to apply this
> patch as is. We can add a build-ime warning later if we find it useful.
> 
> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Series applied, thanks.

I think the right fix should be a check in meson.




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

end of thread, other threads:[~2019-08-05 17:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-25 11:06 [dpdk-dev] [PATCH BUG 335 1/3] net/dpaa: fix compilation error with 0 headroom Hemant Agrawal
2019-07-25 11:06 ` [dpdk-dev] [PATCH BUG 335 2/3] bus/fslmc: " Hemant Agrawal
2019-07-25 11:06 ` [dpdk-dev] [PATCH BUG 335 3/3] net/virtio: " Hemant Agrawal
2019-07-26 12:25   ` Olivier Matz
2019-07-30 13:35     ` Maxime Coquelin
2019-08-01  8:09       ` Hemant Agrawal
2019-08-05 13:07         ` Maxime Coquelin
2019-08-05 17:26           ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).