diff mbox series

[v3] vdpa/sfc: make MCDI memzone name unique

Message ID 20220214105148.18414-1-asaini@xilinx.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers show
Series [v3] vdpa/sfc: make MCDI memzone name unique | expand

Checks

Context Check Description
ci/iol-x86_64-unit-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/Intel-compilation fail Compilation issues
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/checkpatch success coding style OK

Commit Message

abhimanyu.saini@xilinx.com Feb. 14, 2022, 10:51 a.m. UTC
From: Abhimanyu Saini <asaini@xilinx.com>

Buffer for MCDI channel is allocated using rte_memzone_reserve_aligned
with zone name 'mcdi'. Since multiple MCDI channels are needed to
support multiple VF(s) and rte_memzone_reserve_aligned expects unique
zone names, append PCI address to zone name to make it unique.

Signed-off-by: Abhimanyu Saini <asaini@xilinx.com>
---
v2:
  - Formatting changes
v3:
  - Formatting changes

 drivers/vdpa/sfc/sfc_vdpa_hw.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Maxime Coquelin Feb. 15, 2022, 10:56 a.m. UTC | #1
On 2/14/22 11:51, abhimanyu.saini@xilinx.com wrote:
> From: Abhimanyu Saini <asaini@xilinx.com>
> 
> Buffer for MCDI channel is allocated using rte_memzone_reserve_aligned
> with zone name 'mcdi'. Since multiple MCDI channels are needed to
> support multiple VF(s) and rte_memzone_reserve_aligned expects unique
> zone names, append PCI address to zone name to make it unique.
> 
> Signed-off-by: Abhimanyu Saini <asaini@xilinx.com>
> ---
> v2:
>    - Formatting changes
> v3:
>    - Formatting changes
> 
>   drivers/vdpa/sfc/sfc_vdpa_hw.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vdpa/sfc/sfc_vdpa_hw.c b/drivers/vdpa/sfc/sfc_vdpa_hw.c
> index fd1fee7..a7018b1 100644
> --- a/drivers/vdpa/sfc/sfc_vdpa_hw.c
> +++ b/drivers/vdpa/sfc/sfc_vdpa_hw.c
> @@ -25,21 +25,30 @@
>   {
>   	uint64_t mcdi_iova;
>   	size_t mcdi_buff_size;
> +	char mz_name[RTE_MEMZONE_NAMESIZE];
>   	const struct rte_memzone *mz = NULL;
>   	int numa_node = sva->pdev->device.numa_node;
>   	int ret;
>   
>   	mcdi_buff_size = RTE_ALIGN_CEIL(len, PAGE_SIZE);
> +	ret = snprintf(mz_name, RTE_MEMZONE_NAMESIZE, "%s_%s",
> +		       sva->pdev->name, name);
> +	if (ret < 0 || ret >= RTE_MEMZONE_NAMESIZE) {

 From the man page:
"
The functions snprintf() and vsnprintf() do not write more than size 
bytes (including the terminating null byte ('\0')).
"

you might want to pass RTE_MEMZONE_NAMESIZE - 1 as size arg to snprintf,
so you can just check ret >= 0?

> +		sfc_vdpa_err(sva, "%s_%s too long to fit in mz_name",
> +			     sva->pdev->name, name);
> +		return -EINVAL;
> +	}
>   
> -	sfc_vdpa_log_init(sva, "name=%s, len=%zu", name, len);
> +	sfc_vdpa_log_init(sva, "name=%s, len=%zu", mz_name, len);
>   
> -	mz = rte_memzone_reserve_aligned(name, mcdi_buff_size,
> +	mz = rte_memzone_reserve_aligned(mz_name, mcdi_buff_size,
>   					 numa_node,
>   					 RTE_MEMZONE_IOVA_CONTIG,
>   					 PAGE_SIZE);
>   	if (mz == NULL) {
>   		sfc_vdpa_err(sva, "cannot reserve memory for %s: len=%#x: %s",
> -			     name, (unsigned int)len, rte_strerror(rte_errno));
> +			     mz_name, (unsigned int)len,
> +			     rte_strerror(rte_errno));
>   		return -ENOMEM;
>   	}
>
Maxime Coquelin Feb. 15, 2022, 10:59 a.m. UTC | #2
On 2/15/22 11:56, Maxime Coquelin wrote:
> 
> 
> On 2/14/22 11:51, abhimanyu.saini@xilinx.com wrote:
>> From: Abhimanyu Saini <asaini@xilinx.com>
>>
>> Buffer for MCDI channel is allocated using rte_memzone_reserve_aligned
>> with zone name 'mcdi'. Since multiple MCDI channels are needed to
>> support multiple VF(s) and rte_memzone_reserve_aligned expects unique
>> zone names, append PCI address to zone name to make it unique.
>>
>> Signed-off-by: Abhimanyu Saini <asaini@xilinx.com>
>> ---
>> v2:
>>    - Formatting changes
>> v3:
>>    - Formatting changes
>>
>>   drivers/vdpa/sfc/sfc_vdpa_hw.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/vdpa/sfc/sfc_vdpa_hw.c 
>> b/drivers/vdpa/sfc/sfc_vdpa_hw.c
>> index fd1fee7..a7018b1 100644
>> --- a/drivers/vdpa/sfc/sfc_vdpa_hw.c
>> +++ b/drivers/vdpa/sfc/sfc_vdpa_hw.c
>> @@ -25,21 +25,30 @@
>>   {
>>       uint64_t mcdi_iova;
>>       size_t mcdi_buff_size;
>> +    char mz_name[RTE_MEMZONE_NAMESIZE];
>>       const struct rte_memzone *mz = NULL;
>>       int numa_node = sva->pdev->device.numa_node;
>>       int ret;
>>       mcdi_buff_size = RTE_ALIGN_CEIL(len, PAGE_SIZE);
>> +    ret = snprintf(mz_name, RTE_MEMZONE_NAMESIZE, "%s_%s",
>> +               sva->pdev->name, name);
>> +    if (ret < 0 || ret >= RTE_MEMZONE_NAMESIZE) {
> 
>  From the man page:
> "
> The functions snprintf() and vsnprintf() do not write more than size 
> bytes (including the terminating null byte ('\0')).
> "
> 
> you might want to pass RTE_MEMZONE_NAMESIZE - 1 as size arg to snprintf,
> so you can just check ret >= 0?

Sorry, I misread, the return value can indeed be greater than size:
"
If the output  was  truncated  due to this limit, then the return value
is the number of characters (excluding the terminating null byte)
which would have been written to the final string if enough space had
been available.  Thus, a return value of size or more  means
that the output was truncated.
"

>> +        sfc_vdpa_err(sva, "%s_%s too long to fit in mz_name",
>> +                 sva->pdev->name, name);
>> +        return -EINVAL;
>> +    }
>> -    sfc_vdpa_log_init(sva, "name=%s, len=%zu", name, len);
>> +    sfc_vdpa_log_init(sva, "name=%s, len=%zu", mz_name, len);
>> -    mz = rte_memzone_reserve_aligned(name, mcdi_buff_size,
>> +    mz = rte_memzone_reserve_aligned(mz_name, mcdi_buff_size,
>>                        numa_node,
>>                        RTE_MEMZONE_IOVA_CONTIG,
>>                        PAGE_SIZE);
>>       if (mz == NULL) {
>>           sfc_vdpa_err(sva, "cannot reserve memory for %s: len=%#x: %s",
>> -                 name, (unsigned int)len, rte_strerror(rte_errno));
>> +                 mz_name, (unsigned int)len,
>> +                 rte_strerror(rte_errno));
>>           return -ENOMEM;
>>       }
Maxime Coquelin Feb. 15, 2022, 12:21 p.m. UTC | #3
On 2/14/22 11:51, abhimanyu.saini@xilinx.com wrote:
> From: Abhimanyu Saini <asaini@xilinx.com>
> 
> Buffer for MCDI channel is allocated using rte_memzone_reserve_aligned
> with zone name 'mcdi'. Since multiple MCDI channels are needed to
> support multiple VF(s) and rte_memzone_reserve_aligned expects unique
> zone names, append PCI address to zone name to make it unique.
> 
> Signed-off-by: Abhimanyu Saini <asaini@xilinx.com>
> ---
> v2:
>    - Formatting changes
> v3:
>    - Formatting changes
> 
>   drivers/vdpa/sfc/sfc_vdpa_hw.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 

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

Thanks,
Maxime
Maxime Coquelin Feb. 17, 2022, 8:54 a.m. UTC | #4
On 2/14/22 11:51, abhimanyu.saini@xilinx.com wrote:
> From: Abhimanyu Saini <asaini@xilinx.com>
> 
> Buffer for MCDI channel is allocated using rte_memzone_reserve_aligned
> with zone name 'mcdi'. Since multiple MCDI channels are needed to
> support multiple VF(s) and rte_memzone_reserve_aligned expects unique
> zone names, append PCI address to zone name to make it unique.
> 
> Signed-off-by: Abhimanyu Saini <asaini@xilinx.com>
> ---
> v2:
>    - Formatting changes
> v3:
>    - Formatting changes
> 
>   drivers/vdpa/sfc/sfc_vdpa_hw.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vdpa/sfc/sfc_vdpa_hw.c b/drivers/vdpa/sfc/sfc_vdpa_hw.c
> index fd1fee7..a7018b1 100644
> --- a/drivers/vdpa/sfc/sfc_vdpa_hw.c
> +++ b/drivers/vdpa/sfc/sfc_vdpa_hw.c
> @@ -25,21 +25,30 @@
>   {
>   	uint64_t mcdi_iova;
>   	size_t mcdi_buff_size;
> +	char mz_name[RTE_MEMZONE_NAMESIZE];
>   	const struct rte_memzone *mz = NULL;
>   	int numa_node = sva->pdev->device.numa_node;
>   	int ret;
>   
>   	mcdi_buff_size = RTE_ALIGN_CEIL(len, PAGE_SIZE);
> +	ret = snprintf(mz_name, RTE_MEMZONE_NAMESIZE, "%s_%s",
> +		       sva->pdev->name, name);
> +	if (ret < 0 || ret >= RTE_MEMZONE_NAMESIZE) {
> +		sfc_vdpa_err(sva, "%s_%s too long to fit in mz_name",
> +			     sva->pdev->name, name);
> +		return -EINVAL;
> +	}
>   
> -	sfc_vdpa_log_init(sva, "name=%s, len=%zu", name, len);
> +	sfc_vdpa_log_init(sva, "name=%s, len=%zu", mz_name, len);
>   
> -	mz = rte_memzone_reserve_aligned(name, mcdi_buff_size,
> +	mz = rte_memzone_reserve_aligned(mz_name, mcdi_buff_size,
>   					 numa_node,
>   					 RTE_MEMZONE_IOVA_CONTIG,
>   					 PAGE_SIZE);
>   	if (mz == NULL) {
>   		sfc_vdpa_err(sva, "cannot reserve memory for %s: len=%#x: %s",
> -			     name, (unsigned int)len, rte_strerror(rte_errno));
> +			     mz_name, (unsigned int)len,
> +			     rte_strerror(rte_errno));
>   		return -ENOMEM;
>   	}
>   


Applied to dpdk-next-virtio/main.

Thanks,
Maxime
diff mbox series

Patch

diff --git a/drivers/vdpa/sfc/sfc_vdpa_hw.c b/drivers/vdpa/sfc/sfc_vdpa_hw.c
index fd1fee7..a7018b1 100644
--- a/drivers/vdpa/sfc/sfc_vdpa_hw.c
+++ b/drivers/vdpa/sfc/sfc_vdpa_hw.c
@@ -25,21 +25,30 @@ 
 {
 	uint64_t mcdi_iova;
 	size_t mcdi_buff_size;
+	char mz_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz = NULL;
 	int numa_node = sva->pdev->device.numa_node;
 	int ret;
 
 	mcdi_buff_size = RTE_ALIGN_CEIL(len, PAGE_SIZE);
+	ret = snprintf(mz_name, RTE_MEMZONE_NAMESIZE, "%s_%s",
+		       sva->pdev->name, name);
+	if (ret < 0 || ret >= RTE_MEMZONE_NAMESIZE) {
+		sfc_vdpa_err(sva, "%s_%s too long to fit in mz_name",
+			     sva->pdev->name, name);
+		return -EINVAL;
+	}
 
-	sfc_vdpa_log_init(sva, "name=%s, len=%zu", name, len);
+	sfc_vdpa_log_init(sva, "name=%s, len=%zu", mz_name, len);
 
-	mz = rte_memzone_reserve_aligned(name, mcdi_buff_size,
+	mz = rte_memzone_reserve_aligned(mz_name, mcdi_buff_size,
 					 numa_node,
 					 RTE_MEMZONE_IOVA_CONTIG,
 					 PAGE_SIZE);
 	if (mz == NULL) {
 		sfc_vdpa_err(sva, "cannot reserve memory for %s: len=%#x: %s",
-			     name, (unsigned int)len, rte_strerror(rte_errno));
+			     mz_name, (unsigned int)len,
+			     rte_strerror(rte_errno));
 		return -ENOMEM;
 	}