From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuanhan Liu Subject: Re: [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework Date: Mon, 19 Sep 2016 22:43:03 +0800 Message-ID: <20160919144303.GL23158@yliu-dev.sh.intel.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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Pankaj Chauhan , dev@dpdk.org, hemant.agrawal@nxp.com, maxime.coquelin@redhat.com To: "Tan, Jianfeng" Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id C499D2B84 for ; Mon, 19 Sep 2016 16:42:33 +0200 (CEST) Content-Disposition: inline In-Reply-To: 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" 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. --yliu