From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pankaj Chauhan Subject: Re: [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework Date: Tue, 27 Sep 2016 16:56:12 +0530 Message-ID: References: <1473072871-16108-1-git-send-email-pankaj.chauhan@nxp.com> <1473072871-16108-2-git-send-email-pankaj.chauhan@nxp.com> <84848c11-fc26-015f-b7f8-a27d0558ef0b@nxp.com> <23bebfde-54ad-04fd-64cb-c99d7a4a78e9@nxp.com> <764c0561-2325-56e9-51e3-f75285044ee4@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: , , To: "Tan, Jianfeng" , Return-path: Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0049.outbound.protection.outlook.com [104.47.40.49]) by dpdk.org (Postfix) with ESMTP id 535FB558C for ; Tue, 27 Sep 2016 13:26:27 +0200 (CEST) In-Reply-To: <764c0561-2325-56e9-51e3-f75285044ee4@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 9/19/2016 6:12 PM, Tan, Jianfeng wrote: > Hi Pankaj, > Hi Jianfeng, Sorry for delayed reply. >>> Can we hide queues inside struct vswitch_port? I mean: >>> For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far >>> you've already stored "struct vhost_dev *" into vswitch_port.priv when >>> it's a virtual port, how about store queue_id into vswitch_port.priv >>> when it's a physical port. >>> For arp_learning switch, make (port_id, all_enabled_queues) as a >>> vswitch_port. >>> Summarize above two: we treat (port_id, all_enabled_queues[]) as a >>> vswitch_port. >>> >>> How about it? >> >> Thanks for wonderful suggestion! Yes it should be possible, instead of >> having one vs_port for physical device we could create one vs_port for >> each phys device + queue_id combination. >> >> Then we can bind the vs_ports to the cores, and do away with binding >> vdevs to the cores. >> >> In order to add extra vs_ports (port + queue_id) there are two methods: >> >> 1. vhost/main.c (or vswitch_common.c) queries the underlying switch >> about how many rx queues to support thourgh a new accessor. VMDQ will >> return the number of vmdq queues, in this case. After getting the >> number of rx queues, vhost/main.c creates the extra ports i.e one >> vs_port for each physical port and queue id combination. >> >> 2. Second method is that the switch implementation for example vmdq.c >> create the extra ports (port + queue_id) as part of vs_port->add_port >> operation for the physical port. >> >> Which method do you think we should follow? I think #1 should be done >> so that same logic can be used for other switches also. > > Although VMDQs are created at initialization, we will not connect all of > them to switch until a virtio device is added. So how about creating > extra vs_ports (port + queue_id) as part of vmdq_add_port_virtio()? > Yes we can do that, it is similar to #2 which is proposed above. But I am little bit confused regarding this approach that we were planning to take, the approach of considering port_id + queue_id as one vswitch port because Yliu mentioned in one of his comments that we should not follow this approach and keep the concept of port simple. So which approach do we take now? are we fine with keeping vswitch port simple and keeping some intelligence regarding vhost devices in the main (main.c, the generic code). > Another thing, why we need the accessor, port_start()? For physical > ports, it's initialized and started at port_init(); for virtual ports, > no need to get started, right? > Intention of port_start was to have some control when the port starts doing tx/rx. For physical port it translates to rte_eth_dev_start(), and for vhost_port it does nothing as you said. Please note that in order to support this i've moved rte_eth_dev_start() from port_init() to vs_port->port_start. Thanks, Pankaj