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, 20 Sep 2016 14:28:17 +0530 Message-ID: <9807e34b-6553-7a10-516b-2c59ad5c667d@nxp.com> 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> <20160919144303.GL23158@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: , , To: Yuanhan Liu , "Tan, Jianfeng" Return-path: Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0047.outbound.protection.outlook.com [104.47.40.47]) by dpdk.org (Postfix) with ESMTP id 88ED85589 for ; Tue, 20 Sep 2016 10:58:29 +0200 (CEST) In-Reply-To: <20160919144303.GL23158@yliu-dev.sh.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 8:13 PM, Yuanhan Liu wrote: > Firstly, sorry for being late on this discussion: I just got a chance > to follow what you guys were talking about. > > On Tue, Sep 13, 2016 at 02:51:31PM +0800, Tan, Jianfeng wrote: >>> (2) we'd better not differentiate phys device and virtual > > Agreed. > >>>> device in generic framework (it's just an attribute of vswitch_port. >>>> >>>> What do you think? >>> >>> I agree with your thought that given the current API in this patchset we >>> should aim for making switch_worker agnostic of the port type. Ideally it >>> should look something like this: >>> >>> switch_worker() { >>> >>> rx_port mask = VSWITCH_PTYPE_PHYS | VSWITCH_PTYPE_PHYS; >>> >>> rx_port = vs_sched_rx_port(vswit_dev_g, rx_port_mask, core_id) >>> rx_q = rx_port->get_rxq(vs_port, vdev, code_id); >>> rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST); >> >> 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. > > Well, note that vhost-user also supports multiple queue; it's just > haven't been enabled yet. So, storing "vdev" for virtio port and > "queue_id" for phys port doesn't make too much sense. > >> 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? > > Sorry, I don't quite like the idea. It's weird to use "vswitch_port + queue_id" > combination to represent a port. A vswitch_port should be just a port: let's > keep the logic that simple. > We wanted to take that approach to make vhost/main.c agnostic port type and have common code for rx/tx processing. The current version of patchset (v2) takes care of multiqueue, as it calls vs_port->get_txq/get_rxq to get the queue on which rx/tx has to be performed. This way the underlying switch can decide the queue based on core_id and vs_port. But in the v2 patchset we still bind vhost_dev to the cores, and pass it to vs_port->get_rxq() to get the rx_queue corresponding to vhost_dev. Jianfeng had suggested to remove vhost_dev to core binding, and bind vs_port to the cores. Creating one vswitch_port for a physical port + queue_id was a step in that direction, thus creating very generic code in vhost/main.c. YLiu/Jianfeng, Please suggest what approach we should take here? Should we keep the logic of binding vhost_dev to core (as in V2 patchset), thus leaving some intelligence about vhost_dev in vhost/main.c. Or What other options do you suggest if we want to achieve port type agnostic vhost/main.c Thanks, Pankaj