Hi Darrell,

Good catch. Thanks. One comment inline.

Best regards, Ilya Maximets.

On 16.05.2017 20:14, Darrell Ball wrote:
> I applied the following incremental to fix a missing free on error, but 
> otherwise is fine 
> Can you fold in the incremental.
> 
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 56b9d25..25d9c9b 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1128,7 +1128,7 @@ netdev_dpdk_process_devargs(const char *devargs, char 
> **errp)
>          } else {
>              /* Attach unsuccessful */
>              VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", 
> devargs);
> -            return -1;
> +            new_port_id = UINT8_MAX;

'rte_eth_dev_get_port_by_name' and 'rte_eth_dev_attach' set a valid
'new_port_id' only on success. So, we don't need to reinitialize it
here. Just removing of return should be enough.

>          }
>      }
> 
> 
> On 5/12/17, 8:10 AM, "Ilya Maximets" <[email protected]> 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 <[email protected]>
>     Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs 
> (vdevs)")
>     Signed-off-by: Ilya Maximets <[email protected]>
>     ---
>     
>      lib/netdev-dpdk.c | 5 ++++-
>      1 file changed, 4 insertions(+), 1 deletion(-)
>     
>     diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>     index 609b8da..56b9d25 100644
>     --- a/lib/netdev-dpdk.c
>     +++ b/lib/netdev-dpdk.c
>     @@ -1114,10 +1114,12 @@ 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)) {
>     @@ -1130,6 +1132,7 @@ netdev_dpdk_process_devargs(const char *devargs, 
> char **errp)
>              }
>          }
>      
>     +    free(name);
>          return new_port_id;
>      }
>      
>     -- 
>     2.7.4
>     
>     
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to