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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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;
>>       }