[dpdk-dev] [PATCH v4 2/2] vhost: Add VHOST PMD

Tetsuya Mukawa mukawa at igel.co.jp
Tue Nov 24 04:44:28 CET 2015


On 2015/11/24 12:40, Yuanhan Liu wrote:
> On Tue, Nov 24, 2015 at 11:48:04AM +0900, Tetsuya Mukawa wrote:
>> On 2015/11/20 20:43, Yuanhan Liu wrote:
>>> On Fri, Nov 13, 2015 at 02:20:31PM +0900, Tetsuya Mukawa wrote:
>>> ....
>>>> +static pthread_mutex_t internal_list_lock = PTHREAD_MUTEX_INITIALIZER;
>>>> +
>>>> +static rte_atomic16_t nb_started_ports;
>>>> +pthread_t session_th;
>>> static?
>> Hi Yuanhan,
>>
>> I appreciate your carefully reviewing.
>> I will fix issues you commented, and submit it again.
>>
>> I added below 2 comments.
>> Could you please check it?
>>
>>>> +
>>>> +static uint16_t
>>>> +eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>>>> +{
>>>> +	struct vhost_queue *r = q;
>>>> +	uint16_t i, nb_tx = 0;
>>>> +
>>>> +	if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
>>>> +		return 0;
>>>> +
>>>> +	rte_atomic32_set(&r->while_queuing, 1);
>>>> +
>>>> +	if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
>>>> +		goto out;
>>>> +
>>>> +	/* Enqueue packets to guest RX queue */
>>>> +	nb_tx = rte_vhost_enqueue_burst(r->device,
>>>> +			r->virtqueue_id, bufs, nb_bufs);
>>>> +
>>>> +	r->tx_pkts += nb_tx;
>>>> +	r->err_pkts += nb_bufs - nb_tx;
>>>> +
>>>> +	for (i = 0; likely(i < nb_tx); i++)
>>>> +		rte_pktmbuf_free(bufs[i]);
>>> We should free upto nb_bufs here, but not nb_tx, right?
>> I guess we don't need to free all packet buffers.
>> Could you please check l2fwd_send_burst() in l2fwd example?
>> It seems DPDK application frees packet buffers that failed to send.
> Yes, you are right. I was thinking it's just a vhost app, and forgot
> that this is for rte_eth_tx_burst, sigh ...
>
>>>> +static struct rte_driver pmd_vhost_drv = {
>>>> +	.name = "eth_vhost",
>>>> +	.type = PMD_VDEV,
>>>> +	.init = rte_pmd_vhost_devinit,
>>>> +	.uninit = rte_pmd_vhost_devuninit,
>>>> +};
>>>> +
>>>> +struct
>>>> +virtio_net *rte_eth_vhost_portid2vdev(uint16_t port_id)
>>> struct virtio_net *
>>> rte_eth_vhost_portid2vdev()
>>>
>>> BTW, why making a speical eth API for virtio? This doesn't make too much
>>> sense to me.
>> This is a kind of helper function.
> Yeah, I know that. I was thinking that an API prefixed with rte_eth_
> should be a common interface for all eth drivers. Here this one is
> for vhost PMD only, though.
>
> I then had a quick check of DPDK code, and found a similar example,
> bond, such as rte_eth_bond_create(). So, it might be okay to introduce
> PMD specific eth APIs?

Yes, I guess so.

>
> Anyway, I would suggest you to put it into another patch, so that
> it can be reworked (or even dropped) if someone else doesn't like
> it (or doesn't think it's necessary).

Sure, it's nice idea.
I will split the patch.

Tetsuya

> 	--yliu
>
>> I assume that DPDK applications want to know relation between port_id
>> and virtio device structure.
>> But, in "new" callback handler that DPDK application registers,
>> application can receive virtio device structure, but it doesn't tell
>> which port is.
>>
>> To know it, probably here are steps that DPDK application needs to do.
>>
>> 1. Store interface name that is specified when vhost pmd is invoked.
>> (For example, store information like /tmp/socket0 is for port0, and
>> /tmp/socket1 is for port1)
>> 2. Compare above interface name and dev->ifname that is stored in virtio
>> device structure, then DPDK application can know which port is.
>>
>> If DPDK application uses Port Hotplug, I guess above steps are easy.
>> But if they don't, interface name will be specified in "--vdev" EAL
>> command line option.
>> So probably it's not so easy to handle interface name in DPDK application.
>> This is why I added the function.
>>
>> Thanks,
>> Tetsuya



More information about the dev mailing list