[dpdk-dev] [RFC PATCH 5/5] virtio: Extend virtio-net PMD to support container environment

[Tetsuya Mukawa]

On 2016/01/28 18:48, Xie, Huawei wrote:
> On 1/28/2016 10:47 AM, Tetsuya Mukawa wrote:
>> On 2016/01/28 0:58, Xie, Huawei wrote:
>>> On 1/21/2016 7:09 PM, Tetsuya Mukawa wrote:
>>> [snip]
>>>> +
>>>> +static int
>>>> +qtest_raw_recv(int fd, char *buf, size_t count)
>>>> +{
>>>> +	size_t len = count;
>>>> +	size_t total_len = 0;
>>>> +	int ret = 0;
>>>> +
>>>> +	while (len > 0) {
>>>> +		ret = read(fd, buf, len);
>>>> +		if (ret == (int)len)
>>>> +			break;
>>>> +		if (*(buf + ret - 1) == '\n')
>>>> +			break;
>>> The above two lines should be put after the below if block.
>> Yes, it should be so.
>>
>>>> +		if (ret == -1) {
>>>> +			if (errno == EINTR)
>>>> +				continue;
>>>> +			return ret;
>>>> +		}
>>>> +		total_len += ret;
>>>> +		buf += ret;
>>>> +		len -= ret;
>>>> +	}
>>>> +	return total_len + ret;
>>>> +}
>>>> +
>>> [snip]
>>>
>>>> +
>>>> +static void
>>>> +qtest_handle_one_message(struct qtest_session *s, char *buf)
>>>> +{
>>>> +	int ret;
>>>> +
>>>> +	if (strncmp(buf, interrupt_message, strlen(interrupt_message)) == 0) {
>>>> +		if (rte_atomic16_read(&s->enable_intr) == 0)
>>>> +			return;
>>>> +
>>>> +		/* relay interrupt to pipe */
>>>> +		ret = write(s->irqfds.writefd, "1", 1);
>>>> +		if (ret < 0)
>>>> +			rte_panic("cannot relay interrupt\n");
>>>> +	} else {
>>>> +		/* relay normal message to pipe */
>>>> +		ret = qtest_raw_send(s->msgfds.writefd, buf, strlen(buf));
>>>> +		if (ret < 0)
>>>> +			rte_panic("cannot relay normal message\n");
>>>> +	}
>>>> +}
>>>> +
>>>> +static char *
>>>> +qtest_get_next_message(char *p)
>>>> +{
>>>> +	p = strchr(p, '\n');
>>>> +	if ((p == NULL) || (*(p + 1) == '\0'))
>>>> +		return NULL;
>>>> +	return p + 1;
>>>> +}
>>>> +
>>>> +static void
>>>> +qtest_close_one_socket(int *fd)
>>>> +{
>>>> +	if (*fd > 0) {
>>>> +		close(*fd);
>>>> +		*fd = -1;
>>>> +	}
>>>> +}
>>>> +
>>>> +static void
>>>> +qtest_close_sockets(struct qtest_session *s)
>>>> +{
>>>> +	qtest_close_one_socket(&s->qtest_socket);
>>>> +	qtest_close_one_socket(&s->msgfds.readfd);
>>>> +	qtest_close_one_socket(&s->msgfds.writefd);
>>>> +	qtest_close_one_socket(&s->irqfds.readfd);
>>>> +	qtest_close_one_socket(&s->irqfds.writefd);
>>>> +	qtest_close_one_socket(&s->ivshmem_socket);
>>>> +}
>>>> +
>>>> +/*
>>>> + * This thread relays QTest response using pipe.
>>>> + * The function is needed because we need to separate IRQ message from others.
>>>> + */
>>>> +static void *
>>>> +qtest_event_handler(void *data) {
>>>> +	struct qtest_session *s = (struct qtest_session *)data;
>>>> +	char buf[1024];
>>>> +	char *p;
>>>> +	int ret;
>>>> +
>>>> +	for (;;) {
>>>> +		memset(buf, 0, sizeof(buf));
>>>> +		ret = qtest_raw_recv(s->qtest_socket, buf, sizeof(buf));
>>>> +		if (ret < 0) {
>>>> +			qtest_close_sockets(s);
>>>> +			return NULL;
>>>> +		}
>>>> +
>>>> +		/* may receive multiple messages at the same time */
>>> From the qtest_raw_recv implementation, if at some point one message is
>>> received by two qtest_raw_recv calls, then is that message discarded?
>>> We could save the last incomplete message in buffer, and combine the
>>> message received next time together.
>> I guess we don't lose replies from QEMU.
>> Please let me describe more.
>>
>> According to the qtest specification, after sending a message, we need
>> to receive a reply like below.
>> APP: ---command---> QEMU
>> APP: <-----------OK---- QEMU
>>
>> But, to handle interrupt message, we need to take care below case.
>> APP: ---command---> QEMU
>> APP: <---interrupt---- QEMU
>> APP: <-----------OK---- QEMU
>>
>> Also, we need to handle a case like multiple threads tries to send a
>> qtest message.
>> Anyway, here is current implementation.
>>
>> So far, we have 3 types of sockets.
>> 1. socket for qtest messaging.
>> 2. socket for relaying normal message.
>> 3. socket for relaying interrupt message.
>>
>> About read direction:
>> The qtest socket is only read by "qtest_event_handler". The handler may
>> receive multiple messages at once.
> I think there are two assumptions that all messages are ended with "\n"
> and the sizeof(buf) could hold the maximum length of sum of all multiple
> messages that QEMU could send at one time.
> Otherwise in the last read call of qtest_raw_receive, you might receive
> only part of the a message.

I've got your point. I will fix above case.

Thanks,
Tetsuya