Re: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length for dpdkr ports
I sent this patch as part of a patch series:
http://openvswitch.org/pipermail/dev/2016-January/064968.html.
Thank you very much.
On 15 January 2016 at 10:17, Traynor, Kevin wrote:
>
> > -Original Message-
> > From: Mauricio Vásquez [mailto:mauricio.vasquezber...@studenti.polito.it
> ]
> > Sent: Thursday, January 14, 2016 9:15 PM
> > To: Traynor, Kevin
> > Cc: dev@openvswitch.org; acon...@bytheb.org
> > Subject: Re: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name
> length
> > for dpdkr ports
> >
> > Hello Kevin,
> >
> > It can only work up to 255 actually, notice that the port number is
> > first parsed and saved in the variable "uint8_t client_id", then the
> > type of this variable should be changed too. Additionally, the port
> > number in ovs is saved in the member "int user_port_id;" of the
> > dpdk_ring structure.
>
> Hi - yes, you're right! I had just checked the length of the buffer, not
> the
> client_id.
>
> >
> > My proposal would be to change the type of both variables to 'unsigned
> > int' and add some error handling in the case overflow happens during
> > the parsing.
> >
> > What do you think?
>
> That sounds good. I'm not really sure how likely it is that someone would
> decide to use a very large number and ring_client.c is only a test program
> but if it's simple to put in the check it's no harm to have it as good
> reference
> code.
>
> >
> >
> > On 11/01/2016, Traynor, Kevin wrote:
> > >
> > >> -Original Message-
> > >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Mauricio
> > >> Vasquez
> > >> B
> > >> Sent: Sunday, January 10, 2016 6:28 PM
> > >> To: dev@openvswitch.org
> > >> Cc: acon...@bytheb.org
> > >> Subject: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name
> length
> > >> for
> > >> dpdkr ports
> > >>
> > >> A ring name length of 10 characters is not enough for dpdkr ports
> > >> starting from dpdkr10, then it is increased to RTE_RING_NAMESIZE
> > >> characters.
> > >
> > > Looks good to me. There's some existing headroom for name length in
> > > ring_client.c
> > > but you may want to also increase it as part of this change now that
> even
> > > larger
> > > numbers are possible? It should work up to as is, so it would be
> just
> > > to catch
> > > someone using magic numbers.
> > >
> > >>
> > >> Signed-off-by: Mauricio Vasquez B
> > >>
> > >> ---
> > >> v2:
> > >> - Use RTE_RING_NAMESIZE instead of a numerical constant.
> > >>
> > >> lib/netdev-dpdk.c | 6 +++---
> > >> 1 file changed, 3 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > >> index b209df2..90512aa 100644
> > >> --- a/lib/netdev-dpdk.c
> > >> +++ b/lib/netdev-dpdk.c
> > >> @@ -1921,7 +1921,7 @@ dpdk_ring_create(const char dev_name[], unsigned
> > >> int
> > >> port_no,
> > >> unsigned int *eth_port_id)
> > >> {
> > >> struct dpdk_ring *ivshmem;
> > >> -char ring_name[10];
> > >> +char ring_name[RTE_RING_NAMESIZE];
> > >> int err;
> > >>
> > >> ivshmem = dpdk_rte_mzalloc(sizeof *ivshmem);
> > >> @@ -1930,7 +1930,7 @@ dpdk_ring_create(const char dev_name[], unsigned
> > >> int
> > >> port_no,
> > >> }
> > >>
> > >> /* XXX: Add support for multiquque ring. */
> > >> -err = snprintf(ring_name, 10, "%s_tx", dev_name);
> > >> +err = snprintf(ring_name, sizeof(ring_name), "%s_tx", dev_name);
> > >> if (err < 0) {
> > >> return -err;
> > >> }
> > >> @@ -1943,7 +1943,7 @@ dpdk_ring_create(const char dev_name[], unsigned
> > >> int
> > >> port_no,
> > >> return ENOMEM;
> > >> }
> > >>
> > >> -err = snprintf(ring_name, 10, "%s_rx", dev_name);
> > >> +err = snprintf(ring_name, sizeof(ring_name), "%s_rx", dev_name);
> > >> if (err < 0) {
> > >> return -err;
> > >> }
> > >> --
> > >> 1.9.1
> > >>
> > >> ___
> > >> dev mailing list
> > >> dev@openvswitch.org
> > >> http://openvswitch.org/mailman/listinfo/dev
> > >
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length for dpdkr ports
> -Original Message-
> From: Mauricio Vásquez [mailto:mauricio.vasquezber...@studenti.polito.it]
> Sent: Thursday, January 14, 2016 9:15 PM
> To: Traynor, Kevin
> Cc: dev@openvswitch.org; acon...@bytheb.org
> Subject: Re: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length
> for dpdkr ports
>
> Hello Kevin,
>
> It can only work up to 255 actually, notice that the port number is
> first parsed and saved in the variable "uint8_t client_id", then the
> type of this variable should be changed too. Additionally, the port
> number in ovs is saved in the member "int user_port_id;" of the
> dpdk_ring structure.
Hi - yes, you're right! I had just checked the length of the buffer, not the
client_id.
>
> My proposal would be to change the type of both variables to 'unsigned
> int' and add some error handling in the case overflow happens during
> the parsing.
>
> What do you think?
That sounds good. I'm not really sure how likely it is that someone would
decide to use a very large number and ring_client.c is only a test program
but if it's simple to put in the check it's no harm to have it as good reference
code.
>
>
> On 11/01/2016, Traynor, Kevin wrote:
> >
> >> -Original Message-
> >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Mauricio
> >> Vasquez
> >> B
> >> Sent: Sunday, January 10, 2016 6:28 PM
> >> To: dev@openvswitch.org
> >> Cc: acon...@bytheb.org
> >> Subject: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length
> >> for
> >> dpdkr ports
> >>
> >> A ring name length of 10 characters is not enough for dpdkr ports
> >> starting from dpdkr10, then it is increased to RTE_RING_NAMESIZE
> >> characters.
> >
> > Looks good to me. There's some existing headroom for name length in
> > ring_client.c
> > but you may want to also increase it as part of this change now that even
> > larger
> > numbers are possible? It should work up to as is, so it would be just
> > to catch
> > someone using magic numbers.
> >
> >>
> >> Signed-off-by: Mauricio Vasquez B
> >>
> >> ---
> >> v2:
> >> - Use RTE_RING_NAMESIZE instead of a numerical constant.
> >>
> >> lib/netdev-dpdk.c | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >> index b209df2..90512aa 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -1921,7 +1921,7 @@ dpdk_ring_create(const char dev_name[], unsigned
> >> int
> >> port_no,
> >> unsigned int *eth_port_id)
> >> {
> >> struct dpdk_ring *ivshmem;
> >> -char ring_name[10];
> >> +char ring_name[RTE_RING_NAMESIZE];
> >> int err;
> >>
> >> ivshmem = dpdk_rte_mzalloc(sizeof *ivshmem);
> >> @@ -1930,7 +1930,7 @@ dpdk_ring_create(const char dev_name[], unsigned
> >> int
> >> port_no,
> >> }
> >>
> >> /* XXX: Add support for multiquque ring. */
> >> -err = snprintf(ring_name, 10, "%s_tx", dev_name);
> >> +err = snprintf(ring_name, sizeof(ring_name), "%s_tx", dev_name);
> >> if (err < 0) {
> >> return -err;
> >> }
> >> @@ -1943,7 +1943,7 @@ dpdk_ring_create(const char dev_name[], unsigned
> >> int
> >> port_no,
> >> return ENOMEM;
> >> }
> >>
> >> -err = snprintf(ring_name, 10, "%s_rx", dev_name);
> >> +err = snprintf(ring_name, sizeof(ring_name), "%s_rx", dev_name);
> >> if (err < 0) {
> >> return -err;
> >> }
> >> --
> >> 1.9.1
> >>
> >> ___
> >> dev mailing list
> >> dev@openvswitch.org
> >> http://openvswitch.org/mailman/listinfo/dev
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length for dpdkr ports
Hello Kevin,
It can only work up to 255 actually, notice that the port number is
first parsed and saved in the variable "uint8_t client_id", then the
type of this variable should be changed too. Additionally, the port
number in ovs is saved in the member "int user_port_id;" of the
dpdk_ring structure.
My proposal would be to change the type of both variables to 'unsigned
int' and add some error handling in the case overflow happens during
the parsing.
What do you think?
On 11/01/2016, Traynor, Kevin wrote:
>
>> -Original Message-
>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Mauricio
>> Vasquez
>> B
>> Sent: Sunday, January 10, 2016 6:28 PM
>> To: dev@openvswitch.org
>> Cc: acon...@bytheb.org
>> Subject: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length
>> for
>> dpdkr ports
>>
>> A ring name length of 10 characters is not enough for dpdkr ports
>> starting from dpdkr10, then it is increased to RTE_RING_NAMESIZE
>> characters.
>
> Looks good to me. There's some existing headroom for name length in
> ring_client.c
> but you may want to also increase it as part of this change now that even
> larger
> numbers are possible? It should work up to as is, so it would be just
> to catch
> someone using magic numbers.
>
>>
>> Signed-off-by: Mauricio Vasquez B
>>
>> ---
>> v2:
>> - Use RTE_RING_NAMESIZE instead of a numerical constant.
>>
>> lib/netdev-dpdk.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index b209df2..90512aa 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1921,7 +1921,7 @@ dpdk_ring_create(const char dev_name[], unsigned
>> int
>> port_no,
>> unsigned int *eth_port_id)
>> {
>> struct dpdk_ring *ivshmem;
>> -char ring_name[10];
>> +char ring_name[RTE_RING_NAMESIZE];
>> int err;
>>
>> ivshmem = dpdk_rte_mzalloc(sizeof *ivshmem);
>> @@ -1930,7 +1930,7 @@ dpdk_ring_create(const char dev_name[], unsigned
>> int
>> port_no,
>> }
>>
>> /* XXX: Add support for multiquque ring. */
>> -err = snprintf(ring_name, 10, "%s_tx", dev_name);
>> +err = snprintf(ring_name, sizeof(ring_name), "%s_tx", dev_name);
>> if (err < 0) {
>> return -err;
>> }
>> @@ -1943,7 +1943,7 @@ dpdk_ring_create(const char dev_name[], unsigned
>> int
>> port_no,
>> return ENOMEM;
>> }
>>
>> -err = snprintf(ring_name, 10, "%s_rx", dev_name);
>> +err = snprintf(ring_name, sizeof(ring_name), "%s_rx", dev_name);
>> if (err < 0) {
>> return -err;
>> }
>> --
>> 1.9.1
>>
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length for dpdkr ports
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Mauricio Vasquez
> B
> Sent: Sunday, January 10, 2016 6:28 PM
> To: dev@openvswitch.org
> Cc: acon...@bytheb.org
> Subject: [ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length for
> dpdkr ports
>
> A ring name length of 10 characters is not enough for dpdkr ports
> starting from dpdkr10, then it is increased to RTE_RING_NAMESIZE
> characters.
Looks good to me. There's some existing headroom for name length in
ring_client.c
but you may want to also increase it as part of this change now that even larger
numbers are possible? It should work up to as is, so it would be just to
catch
someone using magic numbers.
>
> Signed-off-by: Mauricio Vasquez B
> ---
> v2:
> - Use RTE_RING_NAMESIZE instead of a numerical constant.
>
> lib/netdev-dpdk.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index b209df2..90512aa 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1921,7 +1921,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
> port_no,
> unsigned int *eth_port_id)
> {
> struct dpdk_ring *ivshmem;
> -char ring_name[10];
> +char ring_name[RTE_RING_NAMESIZE];
> int err;
>
> ivshmem = dpdk_rte_mzalloc(sizeof *ivshmem);
> @@ -1930,7 +1930,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
> port_no,
> }
>
> /* XXX: Add support for multiquque ring. */
> -err = snprintf(ring_name, 10, "%s_tx", dev_name);
> +err = snprintf(ring_name, sizeof(ring_name), "%s_tx", dev_name);
> if (err < 0) {
> return -err;
> }
> @@ -1943,7 +1943,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
> port_no,
> return ENOMEM;
> }
>
> -err = snprintf(ring_name, 10, "%s_rx", dev_name);
> +err = snprintf(ring_name, sizeof(ring_name), "%s_rx", dev_name);
> if (err < 0) {
> return -err;
> }
> --
> 1.9.1
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length for dpdkr ports
A ring name length of 10 characters is not enough for dpdkr ports
starting from dpdkr10, then it is increased to RTE_RING_NAMESIZE
characters.
Signed-off-by: Mauricio Vasquez B
---
v2:
- Use RTE_RING_NAMESIZE instead of a numerical constant.
lib/netdev-dpdk.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index b209df2..90512aa 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1921,7 +1921,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
port_no,
unsigned int *eth_port_id)
{
struct dpdk_ring *ivshmem;
-char ring_name[10];
+char ring_name[RTE_RING_NAMESIZE];
int err;
ivshmem = dpdk_rte_mzalloc(sizeof *ivshmem);
@@ -1930,7 +1930,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
port_no,
}
/* XXX: Add support for multiquque ring. */
-err = snprintf(ring_name, 10, "%s_tx", dev_name);
+err = snprintf(ring_name, sizeof(ring_name), "%s_tx", dev_name);
if (err < 0) {
return -err;
}
@@ -1943,7 +1943,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
port_no,
return ENOMEM;
}
-err = snprintf(ring_name, 10, "%s_rx", dev_name);
+err = snprintf(ring_name, sizeof(ring_name), "%s_rx", dev_name);
if (err < 0) {
return -err;
}
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
