[ovs-dev] [PATCH 1/3] netdev-dpdk: Fix double attaching of virtual devices.

[Ilya Maximets]

Hi.

Sorry for late response.
Comments inline.

Best regards, Ilya Maximets.

On 22.04.2017 05:01, Ben Pfaff wrote:
> On Mon, Apr 03, 2017 at 06:04:16PM +0300, Ilya Maximets wrote:
>> 'devargs' for virtual devices contains not only name but
>> also a list of arguments like this:
>>
>> 	'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap'
>> 	or
>> 	'eth_af_packet0,iface=eth0'
>>
>> We must cut off the arguments from this string before calling
>> 'rte_eth_dev_get_port_by_name()' to avoid double attaching of
>> the same device.
>>
>> CC: Ciara Loftus <ciara.loftus at intel.com>
>> Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
>> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
>> ---
>>  lib/netdev-dpdk.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index ddc651b..c8d7108 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1114,9 +1114,16 @@ static int
>>  netdev_dpdk_process_devargs(const char *devargs, char **errp)
>>  {
>>      uint8_t new_port_id = UINT8_MAX;
>> +    char *ind, *name = xstrdup(devargs);
>> +
>> +    /* Get the name from the comma separated list of arguments. */
>> +    ind = index(name, ',');
>> +    if (ind != NULL) {
>> +        *ind = '\0';
>> +    }
>>  
>>      if (!rte_eth_dev_count()
>> -            || rte_eth_dev_get_port_by_name(devargs, &new_port_id)
>> +            || rte_eth_dev_get_port_by_name(name, &new_port_id)
>>              || !rte_eth_dev_is_valid_port(new_port_id)) {
>>          /* Device not found in DPDK, attempt to attach it */
>>          if (!rte_eth_dev_attach(devargs, &new_port_id)) {
>> @@ -1129,6 +1136,7 @@ netdev_dpdk_process_devargs(const char *devargs, char **errp)
>>          }
>>      }
>>  
>> +    free(name);
>>      return new_port_id;
>>  }
> 
> Maybe this is a simpler version?

Yes. I like it. Thanks.
Sometimes it's hard to find the best library function for some job.
Looks like 'strcspn' much better than 'index'.

I'll send v2 with this change soon.
 
> --8<--------------------------cut here-------------------------->8--
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ddc651bf98a6..736b7908ee0e 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1113,10 +1113,11 @@ netdev_dpdk_lookup_by_port_id(int port_id)
>  static int
>  netdev_dpdk_process_devargs(const char *devargs, char **errp)
>  {
> +    /* Get the name up to the first comma. */
> +    char *name = xmemdup0(devargs, strcspn(devargs, ","));
>      uint8_t new_port_id = UINT8_MAX;
> -
>      if (!rte_eth_dev_count()
> -            || rte_eth_dev_get_port_by_name(devargs, &new_port_id)
> +            || rte_eth_dev_get_port_by_name(name, &new_port_id)
>              || !rte_eth_dev_is_valid_port(new_port_id)) {
>          /* Device not found in DPDK, attempt to attach it */
>          if (!rte_eth_dev_attach(devargs, &new_port_id)) {
> @@ -1129,6 +1130,7 @@ netdev_dpdk_process_devargs(const char *devargs, char **errp)
>          }
>      }
>  
> +    free(name);
>      return new_port_id;
>  }
>  
> 
> 
>