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
