diff mbox series

[v4,1/4] ethdev: rename memzones allocated for DMA

Message ID 20181011210251.7705-2-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal: allow hotplug to skip an already probed device |

Checks

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

Commit Message

Thomas Monjalon Oct. 11, 2018, 9:02 p.m. UTC 
  The helper rte_eth_dma_zone_reserve() is called by PMDs
when probing a new port.
It creates a new memzone with an unique name.
The name of this memzone was using the name of the driver
doing the probe.

In order to avoid assigning the driver before the end of the probing
(next patch), the driver name is removed from these memzone names.
The ethdev name (data->name) is not used because it may be too long
and may be not set at this stage of probing.

Syntax of old name: <driver>_<ring>_<port>_<queue>
Syntax of new name: eth_p<port>_q<queue>_<ring>

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_ethdev/rte_ethdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Andrew Rybchenko Oct. 12, 2018, 7:53 a.m. UTC | #1 
On 10/12/18 12:02 AM, Thomas Monjalon wrote:
> The helper rte_eth_dma_zone_reserve() is called by PMDs
> when probing a new port.
> It creates a new memzone with an unique name.
> The name of this memzone was using the name of the driver
> doing the probe.
>
> In order to avoid assigning the driver before the end of the probing
> (next patch), the driver name is removed from these memzone names.
> The ethdev name (data->name) is not used because it may be too long
> and may be not set at this stage of probing.
>
> Syntax of old name: <driver>_<ring>_<port>_<queue>
> Syntax of new name: eth_p<port>_q<queue>_<ring>
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>   lib/librte_ethdev/rte_ethdev.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index ef99f7068..ec443def5 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -3441,9 +3441,8 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
>   	char z_name[RTE_MEMZONE_NAMESIZE];
>   	const struct rte_memzone *mz;
>   
> -	snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> -		 dev->device->driver->name, ring_name,
> -		 dev->data->port_id, queue_id);
> +	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> +		 dev->data->port_id, queue_id, ring_name);
>   
>   	mz = rte_memzone_lookup(z_name);
>   	if (mz)

LGTM, but I've found more places where the pattern is duplicate
and testpmd frightens me:
  - app/test-pmd/config.c ring_dma_zone_lookup() which is used
    to look at descriptors (looks like Intel specific since has
    RTE_LIBRTE_I40E_16BYTE_RX_DESC conditional code)
  - drivers/net/bnx2x/bnx2x_rxtx.c
  - drivers/net/cxgbe/sge.c (few times)

I've not dig why bnx2x and cxgbe do not use rte_eth_dma_zone_reserve().
Thomas Monjalon Oct. 12, 2018, 4:40 p.m. UTC | #2 
12/10/2018 09:53, Andrew Rybchenko:
> On 10/12/18 12:02 AM, Thomas Monjalon wrote:
> > The helper rte_eth_dma_zone_reserve() is called by PMDs
> > when probing a new port.
> > It creates a new memzone with an unique name.
> > The name of this memzone was using the name of the driver
> > doing the probe.
> >
> > In order to avoid assigning the driver before the end of the probing
> > (next patch), the driver name is removed from these memzone names.
> > The ethdev name (data->name) is not used because it may be too long
> > and may be not set at this stage of probing.
> >
> > Syntax of old name: <driver>_<ring>_<port>_<queue>
> > Syntax of new name: eth_p<port>_q<queue>_<ring>
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >   lib/librte_ethdev/rte_ethdev.c | 5 ++---
> >   1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> > index ef99f7068..ec443def5 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -3441,9 +3441,8 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
> >   	char z_name[RTE_MEMZONE_NAMESIZE];
> >   	const struct rte_memzone *mz;
> >   
> > -	snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> > -		 dev->device->driver->name, ring_name,
> > -		 dev->data->port_id, queue_id);
> > +	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> > +		 dev->data->port_id, queue_id, ring_name);
> >   
> >   	mz = rte_memzone_lookup(z_name);
> >   	if (mz)
> 
> LGTM, but I've found more places where the pattern is duplicate
> and testpmd frightens me:
>   - app/test-pmd/config.c ring_dma_zone_lookup() which is used
>     to look at descriptors (looks like Intel specific since has
>     RTE_LIBRTE_I40E_16BYTE_RX_DESC conditional code)

From what I see there is no access to rte_device.driver here,
except one in exit function.

>   - drivers/net/bnx2x/bnx2x_rxtx.c
>   - drivers/net/cxgbe/sge.c (few times)

In bnx2x and cxgbe, it is accessed after probing (queue setup or configure steps).

> I've not dig why bnx2x and cxgbe do not use rte_eth_dma_zone_reserve().

Yes may be interesting to standardize them.
I will add them to this patch.
Andrew Rybchenko Oct. 12, 2018, 4:42 p.m. UTC | #3 
On 10/12/18 7:40 PM, Thomas Monjalon wrote:
> 12/10/2018 09:53, Andrew Rybchenko:
>> On 10/12/18 12:02 AM, Thomas Monjalon wrote:
>>> The helper rte_eth_dma_zone_reserve() is called by PMDs
>>> when probing a new port.
>>> It creates a new memzone with an unique name.
>>> The name of this memzone was using the name of the driver
>>> doing the probe.
>>>
>>> In order to avoid assigning the driver before the end of the probing
>>> (next patch), the driver name is removed from these memzone names.
>>> The ethdev name (data->name) is not used because it may be too long
>>> and may be not set at this stage of probing.
>>>
>>> Syntax of old name: <driver>_<ring>_<port>_<queue>
>>> Syntax of new name: eth_p<port>_q<queue>_<ring>
>>>
>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>> ---
>>>    lib/librte_ethdev/rte_ethdev.c | 5 ++---
>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>> index ef99f7068..ec443def5 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>> @@ -3441,9 +3441,8 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
>>>    	char z_name[RTE_MEMZONE_NAMESIZE];
>>>    	const struct rte_memzone *mz;
>>>    
>>> -	snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
>>> -		 dev->device->driver->name, ring_name,
>>> -		 dev->data->port_id, queue_id);
>>> +	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
>>> +		 dev->data->port_id, queue_id, ring_name);
>>>    
>>>    	mz = rte_memzone_lookup(z_name);
>>>    	if (mz)
>> LGTM, but I've found more places where the pattern is duplicate
>> and testpmd frightens me:
>>    - app/test-pmd/config.c ring_dma_zone_lookup() which is used
>>      to look at descriptors (looks like Intel specific since has
>>      RTE_LIBRTE_I40E_16BYTE_RX_DESC conditional code)
> >From what I see there is no access to rte_device.driver here,
> except one in exit function.

Yes, but testpmd will fail to find the memzone and command to
take a look at descriptors will be broken.

May be it is already broken etc. I think someone from Intel should
comment it.
Andrew Rybchenko Oct. 12, 2018, 4:46 p.m. UTC | #4 
On 10/12/18 7:42 PM, Andrew Rybchenko wrote:
> On 10/12/18 7:40 PM, Thomas Monjalon wrote:
>> 12/10/2018 09:53, Andrew Rybchenko:
>>> On 10/12/18 12:02 AM, Thomas Monjalon wrote:
>>>> The helper rte_eth_dma_zone_reserve() is called by PMDs
>>>> when probing a new port.
>>>> It creates a new memzone with an unique name.
>>>> The name of this memzone was using the name of the driver
>>>> doing the probe.
>>>>
>>>> In order to avoid assigning the driver before the end of the probing
>>>> (next patch), the driver name is removed from these memzone names.
>>>> The ethdev name (data->name) is not used because it may be too long
>>>> and may be not set at this stage of probing.
>>>>
>>>> Syntax of old name: <driver>_<ring>_<port>_<queue>
>>>> Syntax of new name: eth_p<port>_q<queue>_<ring>
>>>>
>>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>>> ---
>>>>    lib/librte_ethdev/rte_ethdev.c | 5 ++---
>>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_ethdev.c 
>>>> b/lib/librte_ethdev/rte_ethdev.c
>>>> index ef99f7068..ec443def5 100644
>>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>>> @@ -3441,9 +3441,8 @@ rte_eth_dma_zone_reserve(const struct 
>>>> rte_eth_dev *dev, const char *ring_name,
>>>>        char z_name[RTE_MEMZONE_NAMESIZE];
>>>>        const struct rte_memzone *mz;
>>>>    -    snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
>>>> -         dev->device->driver->name, ring_name,
>>>> -         dev->data->port_id, queue_id);
>>>> +    snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
>>>> +         dev->data->port_id, queue_id, ring_name);
>>>>           mz = rte_memzone_lookup(z_name);
>>>>        if (mz)
>>> LGTM, but I've found more places where the pattern is duplicate
>>> and testpmd frightens me:
>>>    - app/test-pmd/config.c ring_dma_zone_lookup() which is used
>>>      to look at descriptors (looks like Intel specific since has
>>>      RTE_LIBRTE_I40E_16BYTE_RX_DESC conditional code)
>> >From what I see there is no access to rte_device.driver here,
>> except one in exit function.
>
> Yes, but testpmd will fail to find the memzone and command to
> take a look at descriptors will be broken.
>
> May be it is already broken etc. I think someone from Intel should
> comment it.

Potentially the following drivers may use it:
$ git grep -w \"rx_ring\" drivers/net/
drivers/net/avf/avf_rxtx.c:380: mz = rte_eth_dma_zone_reserve(dev, 
"rx_ring", queue_idx,
drivers/net/axgbe/axgbe_rxtx.c:91:      dma = 
rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size, 12
drivers/net/cxgbe/sge.c:1878:            fwevtq ? "fwq_ring" : "rx_ring",
drivers/net/e1000/em_rxtx.c:1437:       rz = 
rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, rsize,
drivers/net/e1000/igb_rxtx.c:1734:      rz = 
rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size,
drivers/net/fm10k/fm10k_ethdev.c:1878:  mz = 
rte_eth_dma_zone_reserve(dev, "rx_ring", queue_id,
drivers/net/i40e/i40e_rxtx.c:1853:      rz = 
rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
drivers/net/ixgbe/ixgbe_rxtx.c:2966:    rz = 
rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
drivers/net/nfp/nfp_net.c:1516: tz = rte_eth_dma_zone_reserve(dev, 
"rx_ring", queue_idx,
Thomas Monjalon Oct. 12, 2018, 5:18 p.m. UTC | #5 
12/10/2018 18:46, Andrew Rybchenko:
> On 10/12/18 7:42 PM, Andrew Rybchenko wrote:
> > On 10/12/18 7:40 PM, Thomas Monjalon wrote:
> >> 12/10/2018 09:53, Andrew Rybchenko:
> >>> On 10/12/18 12:02 AM, Thomas Monjalon wrote:
> >>>> The helper rte_eth_dma_zone_reserve() is called by PMDs
> >>>> when probing a new port.
> >>>> It creates a new memzone with an unique name.
> >>>> The name of this memzone was using the name of the driver
> >>>> doing the probe.
> >>>>
> >>>> In order to avoid assigning the driver before the end of the probing
> >>>> (next patch), the driver name is removed from these memzone names.
> >>>> The ethdev name (data->name) is not used because it may be too long
> >>>> and may be not set at this stage of probing.
> >>>>
> >>>> Syntax of old name: <driver>_<ring>_<port>_<queue>
> >>>> Syntax of new name: eth_p<port>_q<queue>_<ring>
> >>>>
> >>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> >>>> ---
> >>>>    lib/librte_ethdev/rte_ethdev.c | 5 ++---
> >>>>    1 file changed, 2 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/lib/librte_ethdev/rte_ethdev.c 
> >>>> b/lib/librte_ethdev/rte_ethdev.c
> >>>> index ef99f7068..ec443def5 100644
> >>>> --- a/lib/librte_ethdev/rte_ethdev.c
> >>>> +++ b/lib/librte_ethdev/rte_ethdev.c
> >>>> @@ -3441,9 +3441,8 @@ rte_eth_dma_zone_reserve(const struct 
> >>>> rte_eth_dev *dev, const char *ring_name,
> >>>>        char z_name[RTE_MEMZONE_NAMESIZE];
> >>>>        const struct rte_memzone *mz;
> >>>>    -    snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> >>>> -         dev->device->driver->name, ring_name,
> >>>> -         dev->data->port_id, queue_id);
> >>>> +    snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> >>>> +         dev->data->port_id, queue_id, ring_name);
> >>>>           mz = rte_memzone_lookup(z_name);
> >>>>        if (mz)
> >>> LGTM, but I've found more places where the pattern is duplicate
> >>> and testpmd frightens me:
> >>>    - app/test-pmd/config.c ring_dma_zone_lookup() which is used
> >>>      to look at descriptors (looks like Intel specific since has
> >>>      RTE_LIBRTE_I40E_16BYTE_RX_DESC conditional code)
> >> >From what I see there is no access to rte_device.driver here,
> >> except one in exit function.
> >
> > Yes, but testpmd will fail to find the memzone and command to
> > take a look at descriptors will be broken.
> >
> > May be it is already broken etc. I think someone from Intel should
> > comment it.
> 
> Potentially the following drivers may use it:
> $ git grep -w \"rx_ring\" drivers/net/
> drivers/net/avf/avf_rxtx.c:380: mz = rte_eth_dma_zone_reserve(dev, 
> "rx_ring", queue_idx,
> drivers/net/axgbe/axgbe_rxtx.c:91:      dma = 
> rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size, 12
> drivers/net/cxgbe/sge.c:1878:            fwevtq ? "fwq_ring" : "rx_ring",
> drivers/net/e1000/em_rxtx.c:1437:       rz = 
> rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, rsize,
> drivers/net/e1000/igb_rxtx.c:1734:      rz = 
> rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size,
> drivers/net/fm10k/fm10k_ethdev.c:1878:  mz = 
> rte_eth_dma_zone_reserve(dev, "rx_ring", queue_id,
> drivers/net/i40e/i40e_rxtx.c:1853:      rz = 
> rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
> drivers/net/ixgbe/ixgbe_rxtx.c:2966:    rz = 
> rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
> drivers/net/nfp/nfp_net.c:1516: tz = rte_eth_dma_zone_reserve(dev, 
> "rx_ring", queue_idx,

Excuse me, I really don't understand what you mean.
Thomas Monjalon Oct. 12, 2018, 5:21 p.m. UTC | #6 
12/10/2018 19:18, Thomas Monjalon:
> 12/10/2018 18:46, Andrew Rybchenko:
> > On 10/12/18 7:42 PM, Andrew Rybchenko wrote:
> > > On 10/12/18 7:40 PM, Thomas Monjalon wrote:
> > >> 12/10/2018 09:53, Andrew Rybchenko:
> > >>> On 10/12/18 12:02 AM, Thomas Monjalon wrote:
> > >>>> The helper rte_eth_dma_zone_reserve() is called by PMDs
> > >>>> when probing a new port.
> > >>>> It creates a new memzone with an unique name.
> > >>>> The name of this memzone was using the name of the driver
> > >>>> doing the probe.
> > >>>>
> > >>>> In order to avoid assigning the driver before the end of the probing
> > >>>> (next patch), the driver name is removed from these memzone names.
> > >>>> The ethdev name (data->name) is not used because it may be too long
> > >>>> and may be not set at this stage of probing.
> > >>>>
> > >>>> Syntax of old name: <driver>_<ring>_<port>_<queue>
> > >>>> Syntax of new name: eth_p<port>_q<queue>_<ring>
> > >>>>
> > >>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > >>>> ---
> > >>>>    lib/librte_ethdev/rte_ethdev.c | 5 ++---
> > >>>>    1 file changed, 2 insertions(+), 3 deletions(-)
> > >>>>
> > >>>> diff --git a/lib/librte_ethdev/rte_ethdev.c 
> > >>>> b/lib/librte_ethdev/rte_ethdev.c
> > >>>> index ef99f7068..ec443def5 100644
> > >>>> --- a/lib/librte_ethdev/rte_ethdev.c
> > >>>> +++ b/lib/librte_ethdev/rte_ethdev.c
> > >>>> @@ -3441,9 +3441,8 @@ rte_eth_dma_zone_reserve(const struct 
> > >>>> rte_eth_dev *dev, const char *ring_name,
> > >>>>        char z_name[RTE_MEMZONE_NAMESIZE];
> > >>>>        const struct rte_memzone *mz;
> > >>>>    -    snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> > >>>> -         dev->device->driver->name, ring_name,
> > >>>> -         dev->data->port_id, queue_id);
> > >>>> +    snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> > >>>> +         dev->data->port_id, queue_id, ring_name);
> > >>>>           mz = rte_memzone_lookup(z_name);
> > >>>>        if (mz)
> > >>> LGTM, but I've found more places where the pattern is duplicate
> > >>> and testpmd frightens me:
> > >>>    - app/test-pmd/config.c ring_dma_zone_lookup() which is used
> > >>>      to look at descriptors (looks like Intel specific since has
> > >>>      RTE_LIBRTE_I40E_16BYTE_RX_DESC conditional code)
> > >> >From what I see there is no access to rte_device.driver here,
> > >> except one in exit function.
> > >
> > > Yes, but testpmd will fail to find the memzone and command to
> > > take a look at descriptors will be broken.
> > >
> > > May be it is already broken etc. I think someone from Intel should
> > > comment it.
> > 
> > Potentially the following drivers may use it:
> > $ git grep -w \"rx_ring\" drivers/net/
> > drivers/net/avf/avf_rxtx.c:380: mz = rte_eth_dma_zone_reserve(dev, 
> > "rx_ring", queue_idx,
> > drivers/net/axgbe/axgbe_rxtx.c:91:      dma = 
> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size, 12
> > drivers/net/cxgbe/sge.c:1878:            fwevtq ? "fwq_ring" : "rx_ring",
> > drivers/net/e1000/em_rxtx.c:1437:       rz = 
> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, rsize,
> > drivers/net/e1000/igb_rxtx.c:1734:      rz = 
> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size,
> > drivers/net/fm10k/fm10k_ethdev.c:1878:  mz = 
> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_id,
> > drivers/net/i40e/i40e_rxtx.c:1853:      rz = 
> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
> > drivers/net/ixgbe/ixgbe_rxtx.c:2966:    rz = 
> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
> > drivers/net/nfp/nfp_net.c:1516: tz = rte_eth_dma_zone_reserve(dev, 
> > "rx_ring", queue_idx,
> 
> Excuse me, I really don't understand what you mean.

Oh, got it!
You mean the call to rte_memzone_lookup() must expect the new memzone name.
So I must update it here too, right?
Andrew Rybchenko Oct. 12, 2018, 5:51 p.m. UTC | #7 
On October 12, 2018 20:21:32 Thomas Monjalon <thomas@monjalon.net> wrote:

> 12/10/2018 19:18, Thomas Monjalon:
>> 12/10/2018 18:46, Andrew Rybchenko:
>> > On 10/12/18 7:42 PM, Andrew Rybchenko wrote:
>> > > On 10/12/18 7:40 PM, Thomas Monjalon wrote:
>> > >> 12/10/2018 09:53, Andrew Rybchenko:
>> > >>> On 10/12/18 12:02 AM, Thomas Monjalon wrote:
>> > >>>> The helper rte_eth_dma_zone_reserve() is called by PMDs
>> > >>>> when probing a new port.
>> > >>>> It creates a new memzone with an unique name.
>> > >>>> The name of this memzone was using the name of the driver
>> > >>>> doing the probe.
>> > >>>>
>> > >>>> In order to avoid assigning the driver before the end of the probing
>> > >>>> (next patch), the driver name is removed from these memzone names.
>> > >>>> The ethdev name (data->name) is not used because it may be too long
>> > >>>> and may be not set at this stage of probing.
>> > >>>>
>> > >>>> Syntax of old name: <driver>_<ring>_<port>_<queue>
>> > >>>> Syntax of new name: eth_p<port>_q<queue>_<ring>
>> > >>>>
>> > >>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>> > >>>> ---
>> > >>>>    lib/librte_ethdev/rte_ethdev.c | 5 ++---
>> > >>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>> > >>>>
>> > >>>> diff --git a/lib/librte_ethdev/rte_ethdev.c
>> > >>>> b/lib/librte_ethdev/rte_ethdev.c
>> > >>>> index ef99f7068..ec443def5 100644
>> > >>>> --- a/lib/librte_ethdev/rte_ethdev.c
>> > >>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> > >>>> @@ -3441,9 +3441,8 @@ rte_eth_dma_zone_reserve(const struct
>> > >>>> rte_eth_dev *dev, const char *ring_name,
>> > >>>>        char z_name[RTE_MEMZONE_NAMESIZE];
>> > >>>>        const struct rte_memzone *mz;
>> > >>>>    -    snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
>> > >>>> -         dev->device->driver->name, ring_name,
>> > >>>> -         dev->data->port_id, queue_id);
>> > >>>> +    snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
>> > >>>> +         dev->data->port_id, queue_id, ring_name);
>> > >>>>           mz = rte_memzone_lookup(z_name);
>> > >>>>        if (mz)
>> > >>> LGTM, but I've found more places where the pattern is duplicate
>> > >>> and testpmd frightens me:
>> > >>>    - app/test-pmd/config.c ring_dma_zone_lookup() which is used
>> > >>>      to look at descriptors (looks like Intel specific since has
>> > >>>      RTE_LIBRTE_I40E_16BYTE_RX_DESC conditional code)
>> > >> >From what I see there is no access to rte_device.driver here,
>> > >> except one in exit function.
>> > >
>> > > Yes, but testpmd will fail to find the memzone and command to
>> > > take a look at descriptors will be broken.
>> > >
>> > > May be it is already broken etc. I think someone from Intel should
>> > > comment it.
>> >
>> > Potentially the following drivers may use it:
>> > $ git grep -w \"rx_ring\" drivers/net/
>> > drivers/net/avf/avf_rxtx.c:380: mz = rte_eth_dma_zone_reserve(dev,
>> > "rx_ring", queue_idx,
>> > drivers/net/axgbe/axgbe_rxtx.c:91:      dma =
>> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size, 12
>> > drivers/net/cxgbe/sge.c:1878:            fwevtq ? "fwq_ring" : "rx_ring",
>> > drivers/net/e1000/em_rxtx.c:1437:       rz =
>> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, rsize,
>> > drivers/net/e1000/igb_rxtx.c:1734:      rz =
>> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size,
>> > drivers/net/fm10k/fm10k_ethdev.c:1878:  mz =
>> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_id,
>> > drivers/net/i40e/i40e_rxtx.c:1853:      rz =
>> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
>> > drivers/net/ixgbe/ixgbe_rxtx.c:2966:    rz =
>> > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
>> > drivers/net/nfp/nfp_net.c:1516: tz = rte_eth_dma_zone_reserve(dev,
>> > "rx_ring", queue_idx,
>>
>> Excuse me, I really don't understand what you mean.
>
> Oh, got it!
> You mean the call to rte_memzone_lookup() must expect the new memzone name.
> So I must update it here too, right?
Yes
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index ef99f7068..ec443def5 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -3441,9 +3441,8 @@  rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 	char z_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
 
-	snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
-		 dev->device->driver->name, ring_name,
-		 dev->data->port_id, queue_id);
+	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
+		 dev->data->port_id, queue_id, ring_name);
 
 	mz = rte_memzone_lookup(z_name);
 	if (mz)