Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan <pankaj.chauhan@nxp.com>]

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