[ovs-dev] [PATCH v2] lib/netdev-dpdk: increase ring name length for dpdkr ports

[Traynor, Kevin]

> -----Original Message-----
> From: Mauricio Vásquez [mailto:mauricio.vasquezbernal at studenti.polito.it]
> Sent: Thursday, January 14, 2016 9:15 PM
> To: Traynor, Kevin
> Cc: dev at openvswitch.org; aconole at 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 <kevin.traynor at intel.com> wrote:
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Mauricio
> >> Vasquez
> >> B
> >> Sent: Sunday, January 10, 2016 6:28 PM
> >> To: dev at openvswitch.org
> >> Cc: aconole at 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 9999 as is, so it would be just
> > to catch
> > someone using magic numbers.
> >
> >>
> >> Signed-off-by: Mauricio Vasquez B
> >> <mauricio.vasquezbernal at studenti.polito.it>
> >> ---
> >> 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 at openvswitch.org
> >> http://openvswitch.org/mailman/listinfo/dev
> >