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

[Pankaj Chauhan]

On 9/9/2016 2:26 PM, Tan, Jianfeng wrote:
> Hi Pankaj,
>
> Thanks for the massive and great work.

Hi Jianfeng,

Thanks for the review.
>
> On 9/5/2016 6:54 PM, Pankaj Chauhan wrote:
>> Introduce support for a generic framework for handling of switching
>> between
>> physical and vhost devices. The vswitch framework introduces the
>> following
>> concept:
>>
>> 1. vswitch_dev: Vswitch device is a logical switch which can have
>> physical and
>> virtio devices. The devices are operated/used using standard rte_eth
>> API for
>> physical devices and lib_vhost API for vhost devices, PMD API is not used
>> for vhost devices.
>>
>> 2. vswitch_port: Any physical or virtio device that is added to
>> vswitch. The
>> port can have its own tx/rx functions for doing data transfer, which
>> are exposed
>> to the framework using generic function pointers
>> (vs_port->do_tx/do_rx). This way
>> the generic code can do tx/rx without understanding the type of device
>> (Physical or
>> virtio). Similarly each port has its own functions to select tx/rx
>> queues. The framework
>> provides default tx/rx queue selection functions to the port when port
>> is added
>> (for both physical and vhost devices). But the framework allows the
>> switch implementation
>> to override the queue selection functions (vs_port->get_txq/rxq) if
>> required.
>>
>> 3. vswitch_ops: The ops is set of function pointers which are used to
>> do operations
>> like learning, unlearning, add/delete port, lookup_and_forward. The
>> user of vswitch
>> framework (vhost/main.[c,h])uses these function pointers to perform
>> above mentioned
>> operations, thus it remains agnostic of the underlying implementation.
>>
>> Different switching logics can implement their vswitch_device and
>> vswitch_ops, and
>> register with the framework. This framework makes vhost-switch
>> application scalable
>> in terms of:
>>
>> 1. Different switching logics (one of them is vmdq, vhost/vmdq.[c,h]
>> 2. Number of ports.
>> 3. Policies of selecting ports for rx and tx.
>>
>> Signed-off-by: Pankaj Chauhan <pankaj.chauhan at nxp.com>
>
> After this patch set, how's mapping relationship between cores and
> vswitch_dev? Old vhost example does not have the concept of switch, so
> each core is binded with some VDEVs. Now, we still keep original logic?
>
> (a) If yes, provided that phys device could has no direct relationship
> with vdevs in other switching logics, we may need to bind those physical
> devices to cores too? In that case, switch_worker() will receiving pkts
> from all devices (phys or virtual) and dispatch, which looks like:
>
> switch_worker() {
>     FOR each port binding to this core {
>          rx(port, pkts[], count);
>          vs_lookup_n_fwd( information_needed );
>     }
> }

Since we support only one switch device running at one time. We bind the 
ports of the switchdev to the core. But The switch might have it's own 
logic to bind the port to the core. For example
VMDQ only supports one Physical port, other switch can support more than 
one Physical port. In order to take care of that i have added two ops in 
swithdev_ops:

1. vs_sched_rx_port:

struct vswitch_port *vs_sched_rx_port(struct vswitch_dev *vs_dev, enum
                                       vswitch_port_type ptype, uint16_t 
core_id)

2. vs_sched_tx_port:

struct vswitch_port *vs_sched_tx_port(struct vswitch_dev *vs_dev, enum
                                       vswitch_port_type ptype, uint16_t
core_id)

The idea of providing these functions is that vhost/main requests the 
underlying switch implementation to schedule a port for rx or Tx, the 
current running core_id is also passed as parameter. So the switch can
take a decision which port to do rx or tx based on core id, and may be 
some other custom policy.

For example VMDQ always returns the one single Physical port in response 
to these functions called from any core. The other switch
can return the ports bound to the cores.

Similarly there are two port operations (vs_port->get_rxq and 
vs_port->get_txq), here also we pass core_id as parameter so that
the underlying switch implementation can schedule the rx or tx queue 
based on the core_id.

The above mentioned ops are used in drain_eth_rx() and 
do_drain_mbuf_table() (main.c) currently, and they leave binding of 
physical port and the queues to the underlying implementation. This
way we can accommodate VMDQ which uses only one physical port and 
rxqueues based on VMDQ, OR any other switch which uses multiple physical 
port and rx/tx queue scheduling based on some other logic.

Please suggest if this way of scheduling ports and tx/rx queues is fine 
or not?
>
> (b) If no, we may bind each core with n switches? If so, switch_worker()
> also need to be reworked to keep receiving pkts from all ports of the
> switch, and dispatch.
>
> switch_worker() {
>     FOR each switch binding to this core {
>          FOR each port of switch {
>              rx(port, pkts[], count);
>              vs_lookup_n_fwd( information_needed );
>         }
>     }
> }
Since we currently support only one switch_dev in a running instance of
vhost_switch() we can use binding of ports to core as you suggested in #(a).

>
> In all, (1) we'd better not use vdev to find phys dev in switch_worker
> any more;

I agree with you. I have tried to do following in drain_eth_rx ():


         core_id = rte_lcore_id();
         rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
                                     core_id);
         if (unlikely(!rx_port))
             goto out;

        rxq = rx_port->get_rxq(rx_port, vdev, core_id);
         rx_count = rx_port->do_rx(rx_port, rxq, NULL, pkts, 
MAX_PKT_BURST);

Here we don't assume any relation between vdev and the physical device 
or rxq.  But pass vdev to the underlying switch so that it can choose
the rxq for this vdev. In case of VMDQ it will return VMDQ queue number 
for this vdev. In case of any other switch implementation it can have 
their own logic to decide rxq.

(2) we'd better not differentiate phys device and virtual
> 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);

		vs_lookup_n_fwd(rx_port, pkts, rx_count, rxq);

}

Here i have two problems in achieving switch_worker as mentioned above:

1. while doing Rx from physical port, we need the associated vdev to 
know the VMDQ rx_q. That was the reason i kept current logic of keeping 
vdevs bound to the core in the switch_worker.  If we don't keep list of 
vdevs assigned to the core, how we' will get the rxq on phsyical device 
in case of VMDQ switch implementation.

2. drain_mbuf_table() currently assumes that there is only one physical 
device on which we have to transmit. We may have to rethink where to 
keep the tx queues (&lcore_tx_queue[core_id]), i case we have multiple 
physical ports we might have to move tx queues to each port.

Since i didn't have good solution currently for #1 and #2 mentioned 
above i kept assumption of one single physical port as it is and tried 
to integrate VMDQ in the framework with leaving these assumptions (thus
the main structure of switch_worker) as it it.

Please suggest if you have some solution to above mentioned points in 
your mind. Ideally i agree we should have switch_worker() as i mentioned 
in pseudo-code i gave above.
>
> Thanks,
> Jianfeng
>