Re: [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 
>> Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs 
>> (vdevs)")
>> Signed-off-by: Ilya Maximets 
>> ---
>>  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, _port_id)
>> +|| rte_eth_dev_get_port_by_name(name, _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, _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, _port_id)
> +|| rte_eth_dev_get_port_by_name(name, _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, _port_id)) {
> @@ -1129,6 +1130,7 @@ netdev_dpdk_process_devargs(const char *devargs, char 
> **errp)
>  }
>  }
>  
> +free(name);
>  return new_port_id;
>  }
>  
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Ben Pfaff]

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 
> Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
> Signed-off-by: Ilya Maximets 
> ---
>  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, _port_id)
> +|| rte_eth_dev_get_port_by_name(name, _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, _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?

--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, _port_id)
+|| rte_eth_dev_get_port_by_name(name, _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, _port_id)) {
@@ -1129,6 +1130,7 @@ netdev_dpdk_process_devargs(const char *devargs, char 
**errp)
 }
 }
 
+free(name);
 return new_port_id;
 }
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Aaron Conole]

Ilya Maximets  writes:

> '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 
> Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
> Signed-off-by: Ilya Maximets 
> ---

Acked-by: Aaron Conole 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Ilya Maximets]

'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 
Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
Signed-off-by: Ilya Maximets 
---
 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, _port_id)
+|| rte_eth_dev_get_port_by_name(name, _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, _port_id)) {
@@ -1129,6 +1136,7 @@ netdev_dpdk_process_devargs(const char *devargs, char 
**errp)
 }
 }
 
+free(name);
 return new_port_id;
 }
 
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev