From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <maxime.coquelin@redhat.com>
Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73])
 by dpdk.org (Postfix) with ESMTP id 6D2F51E20
 for <dev@dpdk.org>; Wed, 28 Feb 2018 09:33:56 +0100 (CET)
Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com
 [10.11.54.4])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mx1.redhat.com (Postfix) with ESMTPS id B942B402290A;
 Wed, 28 Feb 2018 08:33:55 +0000 (UTC)
Received: from [10.36.112.52] (ovpn-112-52.ams2.redhat.com [10.36.112.52])
 by smtp.corp.redhat.com (Postfix) with ESMTPS id 0915D2026E04;
 Wed, 28 Feb 2018 08:33:52 +0000 (UTC)
To: "Yang, Zhiyong" <zhiyong.yang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "yliu@fridaylinux.org" <yliu@fridaylinux.org>,
 "Tan, Jianfeng" <jianfeng.tan@intel.com>, "Bie, Tiwei"
 <tiwei.bie@intel.com>, "Wang, Zhihong" <zhihong.wang@intel.com>
Cc: "Wang, Dong1" <dong1.wang@intel.com>
References: <20180214145330.4679-1-zhiyong.yang@intel.com>
 <20180214145330.4679-4-zhiyong.yang@intel.com>
 <5780f87d-96bc-2c51-eaf8-d38ac4af7d11@redhat.com>
 <E182254E98A5DA4EB1E657AC7CB9BD2A8B055828@BGSMSX101.gar.corp.intel.com>
From: Maxime Coquelin <maxime.coquelin@redhat.com>
Message-ID: <01ac48e2-59c4-e82e-da0c-2598a2b7e1bc@redhat.com>
Date: Wed, 28 Feb 2018 09:33:51 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.6.0
MIME-Version: 1.0
In-Reply-To: <E182254E98A5DA4EB1E657AC7CB9BD2A8B055828@BGSMSX101.gar.corp.intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
 (mx1.redhat.com [10.11.55.6]); Wed, 28 Feb 2018 08:33:55 +0000 (UTC)
X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]);
 Wed, 28 Feb 2018 08:33:55 +0000 (UTC) for IP:'10.11.54.4'
 DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com'
 HELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:''
Subject: Re: [dpdk-dev] [PATCH 3/4] net/virtio-user: support server mode
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 28 Feb 2018 08:33:56 -0000



On 02/28/2018 02:53 AM, Yang, Zhiyong wrote:
> 
> 
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Wednesday, February 28, 2018 2:02 AM
>> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org;
>> yliu@fridaylinux.org; Tan, Jianfeng <jianfeng.tan@intel.com>; Bie, Tiwei
>> <tiwei.bie@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
>> Cc: Wang, Dong1 <dong1.wang@intel.com>
>> Subject: Re: [PATCH 3/4] net/virtio-user: support server mode
>>
>>
>>
>> On 02/14/2018 03:53 PM, Zhiyong Yang wrote:
>>> virtio user adds to support for server mode.
>>>
>>> Virtio user with server mode creates socket file and then starts to
>>> wait for first connection from vhost user with client mode in blocking mode.
>>>
>>> Server mode virtio user supports many times' vhost reconnections with
>>> same configurations.
>>>
>>> Support only one connection at the same time in server mode.
>>>
>>> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
>>> ---
>>>    drivers/net/virtio/virtio_ethdev.c               |  9 ++-
>>>    drivers/net/virtio/virtio_user/vhost_user.c      | 77
>> ++++++++++++++++++++--
>>>    drivers/net/virtio/virtio_user/virtio_user_dev.c | 44 +++++++++----
>>>    drivers/net/virtio/virtio_user_ethdev.c          | 81
>> ++++++++++++++++++++++--
>>>    4 files changed, 186 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_ethdev.c
>>> b/drivers/net/virtio/virtio_ethdev.c
>>> index 884f74ad0..44d037d6b 100644
>>> --- a/drivers/net/virtio/virtio_ethdev.c
>>> +++ b/drivers/net/virtio/virtio_ethdev.c
>>> @@ -1273,9 +1273,13 @@ static void
>>>    virtio_notify_peers(struct rte_eth_dev *dev)
>>>    {
>>>    	struct virtio_hw *hw = dev->data->dev_private;
>>> -	struct virtnet_rx *rxvq = dev->data->rx_queues[0];
>>> +	struct virtnet_rx *rxvq = NULL;
>> I don't think it is needed to assign to NULL here.
> 
> Ok.
>>
>>>    	struct rte_mbuf *rarp_mbuf;
>>>
>>> +	if (!dev->data->rx_queues)
>>> +		return;
>>> +
>>> +	rxvq = dev->data->rx_queues[0];
>> The above change is valid, but I think it should be in a dedicated patch, as it
>> might be backported to -stable.
>>
> Ok, it will cause crash in some cases. For example, the code goes here before memory allocation of  rxvq is done.
> 
>>>    	rarp_mbuf = rte_net_make_rarp_packet(rxvq->mpool,
>>>    			(struct ether_addr *)hw->mac_addr);
>>>    	if (rarp_mbuf == NULL) {
>>> @@ -1333,7 +1337,8 @@ virtio_interrupt_handler(void *param)
>>>
>>>    	if (isr & VIRTIO_NET_S_ANNOUNCE) {
>>>    		virtio_notify_peers(dev);
>>> -		virtio_ack_link_announce(dev);
>>> +		if (hw->cvq) > +
>> 	virtio_ack_link_announce(dev);
>> Is this change also related to server mode?
>> It may deserve a dedicated patch too.
> 
> Above changes are not related to server mode. :).
> Just looks more reasonable.
> 
> If format extra two patches,  Should I put them in this series or in another series?

Thanks. Put them in the same series, with a Fixes: line and Cc:
stable@dpdk.org if the fixed patch was introduced before v17.11.


> Thanks
> Zhiyong
> 
>>>    	}
>>>    }
>>
>> Thanks,
>> Maxime