[v3] vdpa/sfc: make MCDI memzone name unique
Checks
Commit Message
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
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;
> }
>
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;
>> }
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
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
@@ -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;
}