On 17.05.2017 08:53, Ilya Maximets wrote:
> 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.
I just found that this change also was mistakenly moved to the third patch.
Oh.. Kind of bad luck with splitting of this patches.
Also, I think, I'll keep this reinitialization just to make code more clear.
>> }
>> }
>>
>>
>> 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
