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

Pankaj Chauhan pankaj.chauhan at nxp.com
Sat Sep 3 17:28:52 CEST 2016


On 9/2/2016 7:47 PM, Maxime Coquelin wrote:
>
> Hi Pankaj,
>
>   Sorry for the late review.

Hi Maxime,

No problem :) Thanks for the review, and it came just in time. I am 
almost done with testing the patchset for intel (with vmdq) and i was 
about to send the next version tomorrow.

Now that i've your review and views, i will incorporte them and send the 
tested patchset soon.

Thanks again for your time and effort for review.

>
> On 08/27/2016 06:26 PM, Pankaj Chauhan wrote:
>> Indroduce support for a generic framework for handling of switching
>> between
> s/Indroduce/Introduce/
>> physical and virtio devices. The vswitch framework introduces the
>> following
>> concept:
> Shouldn't you use vhost term instead of virtio?

Agreed, i will use vhost in next version
>>
>> 1. vswitch_dev: Vswitch device is a logical switch which can have
>> Phsyical and
> s/Phsyical/physical/
>> virtio devices. The devices are operated/used using standard DPDK API for
>> devices.
> Maybe mention that today, we don't use PMD API for vhost devices but
> use directly the vhost library API?

Agreed, will do it.
>>
>> 2. vswitch_port: Any physical or virtio device that is added to
>> vswitch. The
>> port can have it's own tx/rx functions for doing data transfer, which
>> are exposed
> s/it's/its/

sorry for typo, will fix it.
>> 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).
> Very good.
>>
>> 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 implmentation.
> s/implmentation/implementation/

Typo again my bad!, will fix it
>
>
>>
>> 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>
>> ---
>>  examples/vhost/Makefile         |   2 +-
>>  examples/vhost/vswitch_common.c | 467
>> ++++++++++++++++++++++++++++++++++++++++
>>  examples/vhost/vswitch_common.h | 175 +++++++++++++++
>>  3 files changed, 643 insertions(+), 1 deletion(-)
>>  create mode 100644 examples/vhost/vswitch_common.c
>>  create mode 100644 examples/vhost/vswitch_common.h
>>
>> diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
>> index e95c68a..458d166 100644
>> --- a/examples/vhost/Makefile
>> +++ b/examples/vhost/Makefile
>> @@ -48,7 +48,7 @@ else
>>  APP = vhost-switch
>>
>>  # all source are stored in SRCS-y
>> -SRCS-y := main.c
>> +SRCS-y := main.c vswitch_common.c
>>
>>  CFLAGS += -O2 -D_FILE_OFFSET_BITS=64
>>  CFLAGS += $(WERROR_FLAGS)
>> diff --git a/examples/vhost/vswitch_common.c
>> b/examples/vhost/vswitch_common.c
>> new file mode 100644
>> index 0000000..f0e07f2
>> --- /dev/null
>> +++ b/examples/vhost/vswitch_common.c
>> @@ -0,0 +1,467 @@
>> +/*-
>> + *   BSD LICENSE
>> + *
>> + *   Copyright(c) 2016 Freescale Semiconductor. All rights reserved.
>> + *   All rights reserved.
>> + *
>> + *   Redistribution and use in source and binary forms, with or without
>> + *   modification, are permitted provided that the following conditions
>> + *   are met:
>> + *
>> + *     * Redistributions of source code must retain the above copyright
>> + *       notice, this list of conditions and the following disclaimer.
>> + *     * Redistributions in binary form must reproduce the above
>> copyright
>> + *       notice, this list of conditions and the following disclaimer in
>> + *       the documentation and/or other materials provided with the
>> + *       distribution.
>> + *     * Neither the name of Freescale Semiconductor nor the names of
>> its
>> + *       contributors may be used to endorse or promote products derived
>> + *       from this software without specific prior written permission.
>> + *
>> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> FITNESS FOR
>> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
>> COPYRIGHT
>> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>> INCIDENTAL,
>> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>> USE,
>> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
>> ON ANY
>> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
>> THE USE
>> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>> DAMAGE.
>> + */
>> +
>> +#include <unistd.h>
>> +#include <rte_atomic.h>
>> +#include <rte_cycles.h>
>> +#include <rte_ethdev.h>
>> +#include <rte_log.h>
>> +#include <rte_string_fns.h>
>> +#include <rte_malloc.h>
>> +#include <rte_virtio_net.h>
>> +#include <rte_ip.h>
>> +#include <rte_tcp.h>
>> +
>> +#include <sys/queue.h>
>> +#include <strings.h>
>> +#include <errno.h>
>> +
>> +#include "vswitch_common.h"
>> +
>> +/* Meta data for vswitch. Since this is going to be used in this file
>> only it
>> + * is defined here instead of in a header file.
>> + */
>> +struct vs_mdata {
>> +    rte_spinlock_t lock;
>> +    int vswitch_dev_count;
>> +    LIST_HEAD(, vswitch_dev) vswitch_head;
> Since this is only used in this file, maybe you could just remove the
> vswitch prefix from the fields.
>> +};
>> +
>> +static struct vs_mdata *vs_mdata_g;
>> +
>> +static uint16_t vs_do_tx_phys_port(struct vswitch_port *port,
>> uint16_t tx_q,
>> +            __attribute__((unused))struct rte_mempool *mbuf_pool,
>> +                   struct rte_mbuf **pkts, uint16_t pkt_count)
>> +{
>> +    return rte_eth_tx_burst(port->port_id, tx_q, pkts, pkt_count);
>> +}
>> +
>> +
>> +static uint16_t vs_do_tx_virtio_port(struct vswitch_port *port,
>> uint16_t tx_q,
>> +            __attribute__((unused)) struct rte_mempool *mbuf_pool,
>> +                struct rte_mbuf **pkts, uint16_t pkt_count)
>> +{
>> +    return rte_vhost_enqueue_burst(port->port_id, tx_q, pkts,
>> pkt_count);
>> +}
>> +
>> +
>> +static uint16_t vs_do_rx_phys_port (struct vswitch_port *port,
>> uint16_t rx_q,
>> +            __attribute__((unused))struct rte_mempool *mbuf_pool,
>> +                 struct rte_mbuf **rx_pkts, uint16_t pkt_count)
>> +{
>> +    return rte_eth_rx_burst(port->port_id, rx_q, rx_pkts, pkt_count);
>> +}
>> +
>> +static uint16_t vs_do_rx_virtio_port (struct vswitch_port *port,
>> uint16_t rx_q,
>> +                struct rte_mempool *mbuf_pool,
>> +                struct rte_mbuf **rx_pkts, uint16_t pkt_count)
>> +{
>> +    return rte_vhost_dequeue_burst(port->port_id, rx_q, mbuf_pool,
>> +                       rx_pkts, pkt_count);
>> +}
> I would not put the above here but in dedicated file.
> Indeed, you wouldn't need to include rte_vhost & rte_eth headers here,
> and try to make it as more generic as possible.
>
> It does not represent much code, but doing so would clearly draw a line
> between the layers.

Yes i think that would be correct. There are few more functions which 
will go like get_txq/get_rxq (as you suggested later in patch).

A separate vswitch_common_txrx.c will do ?
>
>> +
>> +struct vswitch_dev *vs_get_vswitch_dev(const char *name)
>> +{
>> +    struct vswitch_dev *temp, *vs_dev = NULL;
>> +
>> +    rte_spinlock_lock(&vs_mdata_g->lock);
>> +    LIST_FOREACH(temp, &vs_mdata_g->vswitch_head, list) {
>> +        if (!strncmp(temp->name, name, VSWITCH_NAME_SIZE))
>> +            vs_dev = temp;
>> +    }
>> +    rte_spinlock_unlock(&vs_mdata_g->lock);
>> +
>> +    return vs_dev;
>> +}
>> +
>> +static struct vswitch_port *vs_get_free_port(struct vswitch_dev *vs_dev)
>> +{
>> +    int i;
>> +    struct vswitch_port *vs_port = NULL;
>> +
>> +    for (i = 0; i < vs_dev->port_count; i++) {
>> +        vs_port = &vs_dev->ports[i];
>> +        if (vs_port->state == VSWITCH_PSTATE_NOT_INUSE)
>> +            return vs_port;
> What happens in case of two concurrent accesses of vs_get_free_port()?
> Shouldn't you add an intermediate state (reserved?), and protect state
> with a lock?

Yes there should be a lock, i will add it. Intermediate state for taking 
care of case where port addition fails in add_port, after taking the 
free port, correct ?

I think it would be fine if we keep a lock in vs_get_free_port() and 
vs_free_port() in that case if port addition fails we will free the port 
  atomically. Do you agree?
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +static void vs_free_port(struct vswitch_port *vs_port)
>> +{
>> +
>> +    vs_port->state = VSWITCH_PSTATE_NOT_INUSE;
>> +}
>> +
>> +static __attribute__((unused))struct vswitch_port *vs_get_port(
>> +            struct vswitch_dev *vs_dev, unsigned int port_id,
>> +            enum vswitch_port_type type)
>> +{
>> +    int i;
>> +    struct vswitch_port *vs_port = NULL;
>> +
>> +    for (i = 0; i < vs_dev->port_count; i++) {
>> +        vs_port = &vs_dev->ports[i];
>> +        if ((vs_port->port_id == port_id) && (vs_port->type == type))
>> +            return vs_port;
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +int vs_port_start(struct vswitch_port *vs_port)
>> +{
>> +    struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
>> +    int rc = 0;
>> +
>> +    if (vs_ops->port_start)
>> +        rc = vs_ops->port_start(vs_port);
>> +
>> +    if (!rc)
>> +        vs_port->state = VSWITCH_PSTATE_LEARNING;
>
>> +
>> +    return rc;
>> +}
>> +
>> +int vs_port_stop(struct vswitch_port *vs_port)
>> +{
>> +    struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
>> +    int rc = 0;
>> +
>> +    if (vs_ops->port_stop)
>> +        rc = vs_ops->port_stop(vs_port);
>> +
>> +    if (!rc)
>> +        vs_port->state = VSWITCH_PSTATE_ADDED;
>> +
>> +    return rc;
>> +}
>> +
>> +struct vswitch_port *vs_add_port(struct vswitch_dev *vs_dev, int
>> port_id,
>> +        enum vswitch_port_type type, void *priv)
>> +{
>> +    int rc = 0;
>> +    struct vswitch_port *vs_port = NULL;
>> +    struct vswitch_ops *vs_ops = vs_dev->ops;
>> +
>> +    vs_port = vs_get_free_port(vs_dev);
>> +    if (!vs_port) {
>> +        RTE_LOG(DEBUG, VHOST_CONFIG, "Failed get free port in \
>> +            vswitch %s\n", vs_dev->name);
>> +        rc = -EBUSY;
>> +        goto out;
>> +    }
>> +
>> +    vs_port->port_id = port_id;
>> +    vs_port->type = type;
>> +    vs_port->priv = priv;
>> +
>> +    switch (type) {
>> +    case VSWITCH_PTYPE_PHYS:
>> +           vs_port->do_tx = vs_do_tx_phys_port;
>> +           vs_port->do_rx = vs_do_rx_phys_port;
>> +    case VSWITCH_PTYPE_VIRTIO:
>> +           vs_port->do_tx = vs_do_tx_virtio_port;
>> +           vs_port->do_rx = vs_do_rx_virtio_port;
>
>> +    default:
>> +        RTE_LOG(DEBUG, VHOST_CONFIG, "Invalid port [id %d, type %d]",
>> +                port_id, type);
>> +           rc = -EINVAL;
>> +           goto out;
>> +    }
>> +
>> +    if (vs_ops->add_port)
>> +        rc = vs_ops->add_port(vs_port);
>> +
>> +    if (rc)
>> +        goto out;
>> +
>> +    vs_port->state = VSWITCH_PSTATE_ADDED;
>> +
>> +    rte_eth_macaddr_get(vs_port->port_id, &vs_port->mac_addr);
>> +    RTE_LOG(INFO, VHOST_PORT, "Port %u MAC: %02"PRIx8" %02"PRIx8"
>> %02"PRIx8
>> +            " %02"PRIx8" %02"PRIx8" %02"PRIx8"\n",
>> +            (unsigned)port_id,
>> +            vs_port->mac_addr.addr_bytes[0],
>> +            vs_port->mac_addr.addr_bytes[1],
>> +            vs_port->mac_addr.addr_bytes[2],
>> +            vs_port->mac_addr.addr_bytes[3],
>> +            vs_port->mac_addr.addr_bytes[4],
>> +            vs_port->mac_addr.addr_bytes[5]);
>> +
>> +    RTE_LOG(DEBUG, VHOST_CONFIG, "Added port [%d, type %d] to \
>> +            vswitch %s\n", vs_port->port_id, type, vs_dev->name);
>> +out:
>> +    if (rc){
>> +        RTE_LOG(INFO, VHOST_CONFIG, "Failed to Add port [%d, type %d]
>> to \
>> +            vswitch %s\n", port_id, type, vs_dev->name);
>> +        if (vs_port)
>> +            vs_free_port(vs_port);
>> +        vs_port = NULL;
>> +    }
>> +
>> +    return vs_port;
>> +}
>> +
>> +int vs_del_port(struct vswitch_port *vs_port)
>> +{
>> +    int rc = 0;
>> +    struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
>> +
>> +    if (vs_ops->del_port)
>> +        rc = vs_ops->del_port(vs_port);
>> +
>> +    if (!rc)
>> +        vs_port->state = VSWITCH_PSTATE_NOT_INUSE;
>> +
>> +
>> +    /*TBD:XXX: may be put a dummy function which frees all packets
>> without
>> +     * any tx/rx, NULL looks ugly!
>> +     */
>> +    vs_port->do_tx = vs_port->do_rx = NULL;
>> +
>> +    RTE_LOG(DEBUG, VHOST_CONFIG, "Removed port [%d, type %d] from \
>> +            vswitch %s\n", vs_port->port_id, vs_port->type,
>> +            vs_port->vs_dev->name);
>> +
>> +    return rc;
>> +}
>> +
>> +int vs_learn_port(struct vswitch_port *vs_port, struct rte_mbuf
>> +                **pkts, uint16_t count)
>> +{
>> +    struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
>> +    int rc;
>> +
> Maybe check if vs_ops->learn_port() is valid.

Yes, actually i was thinking that some ops like learn, unlearn, 
lookup_n_fwd are mandatory ops. Thats why i did not use check here.
But yes in case if they are mandatory i should make sure that
they are not NULL while registering the switch. Do you agree on this 
method ? if yes i will add it in next version.

>> +    rc = vs_ops->learn_port(vs_port, pkts, count);
>> +    if (!rc)
>> +        vs_port->state = VSWITCH_PSTATE_FORWARDING;
>> +
>> +    return rc;
>> +}
>> +
>> +int vs_unlearn_port(struct vswitch_port *vs_port)
>> +{
>> +    struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
>> +
> Ditto
>> +    vs_ops->unlearn_port(vs_port);
>> +    vs_port->state = VSWITCH_PSTATE_LEARNING;
>> +
>> +    return 0;
>> +}
>> +
>> +
>> +int vs_lookup_n_fwd(struct vswitch_port *vs_port, struct rte_mbuf
>> **pkts,
>> +            uint16_t count, uint16_t in_rxq)
>> +{
>> +    int rc, i;
>> +    struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
>> +
> Ditto
>> +    rc = vs_ops->lookup_n_fwd(vs_port, pkts, count, in_rxq);
>> +
>> +    if (rc) {
>> +        for (i = 0; i < count; i++)
>> +            rte_pktmbuf_free(pkts[i]);
> What happens if some packets are forwarded before error happens?

very valid point. I took shortcut because currently vmdq.c doesn't have 
any errors in this leg :), my bad!.

In this case do we move the responsibility of freeing packets to 
lookup_n_fwd() function of the switch ? what do you think ?
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>> +int vs_do_broadcast_fwd(struct vswitch_dev *vs_dev,
>> +            struct vswitch_port *in_port, struct rte_mbuf *mbuf)
>> +{
>> +    int i = 0, tx_q;
>> +    struct vswitch_port *dest_port;
>> +
>> +    for (i = 0; i < vs_dev->port_count; i++) {
>> +        dest_port = &vs_dev->ports[i];
>> +        if ((in_port->type != dest_port->type) &&
> If in_port type is Virtio, don't we want to broadcast also to other
> virtio interfaces?

A silly mistake which creeped, just because code was not tested. I 
realized this while i was testing it this week.

I meant this:

/*Do not forward on incoming port */
if ((in_port->type == dest_port->type) && (in_port->port_id == 
dest_port->port_id))
	continue;

I have changed it to above in my current code, that i will send in next 
version.
>
>> +            (in_port->port_id != dest_port->port_id)) {
>> +            if (dest_port->type == VSWITCH_PTYPE_VIRTIO)
>> +                tx_q = VIRTIO_TXQ;
>> +            else
>> +                tx_q = rte_lcore_id();
>> +            dest_port->do_tx(dest_port, tx_q, NULL, &mbuf, 1);
>> +        }
> Also, I would simplify the code to something like this,
> not taking my above remark into account:
>
> for (i = 0; i < vs_dev->port_count; i++) {
>   dest_port = &vs_dev->ports[i];
>
>   if (in_port->type == dest_port->type)
>     continue;
>
>   if (in_port->port_id == dest_port->port_id)
>     continue;
>
>   if (dest_port->type == VSWITCH_PTYPE_VIRTIO)
>     tx_q = VIRTIO_TXQ;
>   else
>     tx_q = rte_lcore_id();
>
>   dest_port->do_tx(dest_port, tx_q, NULL, &mbuf, 1);
> }
>
> And also maybe create a new ops in the port to get the Tx queue,

Very good suggestion. I've added get_txq/get_rxq functions to the port.
this way the switch implementation can decide the queue to select for 
each port. This gives the implementation of switches to have different 
type of scheduling for queues and may be support multiqueue with a queue 
scheduling logic in underlying implementation.
>
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +struct vswitch_port *vs_sched_rx_port(struct vswitch_dev *vs_dev, enum
>> +                      vswitch_port_type ptype, uint16_t core_id)
>> +{
>> +    struct vswitch_port *vs_port = NULL;
>> +    struct vswitch_ops *vs_ops = vs_dev->ops;
>> +
>> +    if (vs_ops->sched_rx_port)
>> +        vs_port = vs_ops->sched_rx_port(vs_dev, ptype, core_id);
>> +
>> +    return vs_port;
>> +}
>> +
>> +struct vswitch_port *vs_sched_tx_port(struct vswitch_dev *vs_dev, enum
>> +                      vswitch_port_type ptype, uint16_t core_id)
>> +{
>> +    struct vswitch_port *vs_port = NULL;
>> +    struct vswitch_ops *vs_ops = vs_dev->ops;
>> +
>> +    if (vs_ops->sched_tx_port)
>> +        vs_port = vs_ops->sched_tx_port(vs_dev, ptype, core_id);
>> +
>> +    return vs_port;
>> +}
>> +
>> +
>> +struct vswitch_dev *vs_register_switch(const char *name, int priv_size,
>> +                int max_ports, struct vswitch_ops *ops)
>> +{
>> +    struct vswitch_dev *vs_dev;
>> +    struct vswitch_port *vs_port;
>> +    int size, i;
>> +
>> +    size = priv_size + sizeof(struct vswitch_dev);
>> +    vs_dev = rte_malloc(NULL, size, 64);
>> +    if (!vs_dev) {
>> +        RTE_LOG(DEBUG, VHOST_CONFIG, "Failed to vswitch device\n");
>> +        goto out;
>> +    }
>> +
>> +    strncpy(vs_dev->name, name, VSWITCH_NAME_SIZE);
>> +    vs_dev->priv = (void *) (vs_dev + 1);
>> +
>> +    size = max_ports * sizeof(struct vswitch_port);
>> +    vs_dev->ports = rte_malloc(NULL, size, 64);
>> +    if (!vs_dev->ports) {
>> +        RTE_LOG(DEBUG, VHOST_CONFIG,
>> +            "Failed allocate %d ports for vswitch %s\n",
>> +            max_ports, vs_dev->name);
>> +        goto out;
>> +    }
>> +
>> +    memset(vs_dev->ports, 0, size);
>> +    for (i = 0; i < max_ports; i++)
>> +    {
>> +        vs_port = &vs_dev->ports[i];
>> +        vs_port->state = VSWITCH_PSTATE_NOT_INUSE;
>> +    }
>> +
>> +    vs_dev->port_count = max_ports;
>> +    vs_dev->ops = ops;
>> +
>> +    rte_spinlock_lock(&vs_mdata_g->lock);
>> +    LIST_INSERT_HEAD(&vs_mdata_g->vswitch_head, vs_dev, list);
>> +    vs_mdata_g->vswitch_dev_count++;
>> +    rte_spinlock_unlock(&vs_mdata_g->lock);
>> +
>> +    RTE_LOG(DEBUG, VHOST_CONFIG, "Added vswitch %s, max_ports %d\n",
>> +        vs_dev->name, vs_dev->port_count);
>> +
>> +    return vs_dev;
>> +
>> +out:
>> +    if (vs_dev && vs_dev->ports)
>> +        rte_free(vs_dev->ports);
>> +    if (vs_dev)
>> +        rte_free(vs_dev);
>> +
>> +    return NULL;
>> +}
>> +
>> +int  vs_unregister_switch(struct vswitch_dev *vs_dev)
>> +{
>> +    struct vswitch_dev *temp;
>> +    int removed = 0, rc;
>> +
>> +    rte_spinlock_lock(&vs_mdata_g->lock);
>> +    LIST_FOREACH(temp, &vs_mdata_g->vswitch_head, list) {
>> +        if (!strncmp(temp->name, vs_dev->name, VSWITCH_NAME_SIZE)){
>> +            LIST_REMOVE(temp, list);
>> +            removed = 1;
>> +        }
>> +    }
>> +    rte_spinlock_unlock(&vs_mdata_g->lock);
>> +
>> +    if (!removed) {
>> +        RTE_LOG(DEBUG, VHOST_CONFIG, "vswitch [%s] not found\n",
>> +            vs_dev->name);
>> +        rc = -ENODEV;
>> +    }
>> +
>> +    RTE_LOG(DEBUG, VHOST_CONFIG, "Unregistering and freeing vswitch
>> [%s]\n",
>> +        vs_dev->name);
>> +
>> +    if (vs_dev->ports)
>> +        rte_free(vs_dev->ports);
>> +
>> +    rte_free(vs_dev);
>> +
>> +    return rc;
>> +}
>> +
>> +int vs_switch_dev_init(struct vswitch_dev *vs_dev, uint16_t conf_flags)
>> +{
>> +    struct vswitch_ops *ops = vs_dev->ops;
>> +    int rc = 0;
>> +
>> +    if (ops->switch_init)
>> +        rc = ops->switch_init(vs_dev, conf_flags);
>> +
>> +    if (!rc)
>> +        vs_dev->conf_flags = conf_flags;
>> +
>> +    return rc;
>> +}
>> +
>> +int vs_vswitch_init(void)
>> +{
>> +    int rc = 0;
>> +
>> +    vs_mdata_g = rte_malloc(NULL, sizeof(struct vs_mdata), 64);
>> +    if (!vs_mdata_g)
>> +    {
>> +        RTE_LOG(DEBUG, VHOST_CONFIG,
>> +            "Failed to allocate mem for vhost siwtch metadata\n");
>> +        rc = -ENOMEM;
>> +           goto out;
>> +    }
> Check indent.

I'll fix it.
>
>> +
>> +    memset(vs_mdata_g, 0, sizeof(struct vs_mdata));
>> +    LIST_INIT(&vs_mdata_g->vswitch_head);
>> +    rte_spinlock_init(&vs_mdata_g->lock);
>> +
>> +out:
>> +    return rc;
>> +}
>> +
>> diff --git a/examples/vhost/vswitch_common.h
>> b/examples/vhost/vswitch_common.h
>> new file mode 100644
>> index 0000000..0496152
>> --- /dev/null
>> +++ b/examples/vhost/vswitch_common.h
>> @@ -0,0 +1,175 @@
>> +/*-
>> + *   BSD LICENSE
>> + *
>> + *   Copyright(c) 2016 Freescale Semiconductor. All rights reserved.
>> + *   All rights reserved.
>> + *
>> + *   Redistribution and use in source and binary forms, with or without
>> + *   modification, are permitted provided that the following conditions
>> + *   are met:
>> + *
>> + *     * Redistributions of source code must retain the above copyright
>> + *       notice, this list of conditions and the following disclaimer.
>> + *     * Redistributions in binary form must reproduce the above
>> copyright
>> + *       notice, this list of conditions and the following disclaimer in
>> + *       the documentation and/or other materials provided with the
>> + *       distribution.
>> + *     * Neither the name of Freescale Semiconductor nor the names of
>> its
>> + *       contributors may be used to endorse or promote products derived
>> + *       from this software without specific prior written permission.
>> + *
>> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> FITNESS FOR
>> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
>> COPYRIGHT
>> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>> INCIDENTAL,
>> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>> USE,
>> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
>> ON ANY
>> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
>> THE USE
>> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>> DAMAGE.
>> + */
>> +
>> +#ifndef __VHOST_SWITCH_COMMON_H__
>> +#define __VHOST_SWITCH_COMMON_H__
>> +
>> +#include <sys/queue.h>
>> +#include "main.h"
>> +
>> +#define VSWITCH_NAME_SIZE    32
>> +
>> +enum vswitch_port_type {
>> +    VSWITCH_PTYPE_PHYS = 1,
>> +    VSWITCH_PTYPE_VIRTIO,
>> +    VSWITCH_PTYPE_END,
>> +};
>> +enum vswitch_port_state {
>> +    VSWITCH_PSTATE_ADDED = 1,
>> +    VSWITCH_PSTATE_LEARNING,
>> +    VSWITCH_PSTATE_FORWARDING,
>> +    VSWITCH_PSTATE_NOT_INUSE,
>> +    VSWITCH_PSTATE_END,
>> +};
>> +
>> +struct vswitch_port {
>> +    enum vswitch_port_type type;
>> +    enum vswitch_port_state state;
>> +    unsigned int port_id;
>> +    struct rte_eth_conf port_conf;
>> +    struct vswitch_dev *vs_dev;    /*switch to which this port
>> belongs */
>> +    void *priv;            /*Private for port specefic data*/
>> +    struct ether_addr mac_addr;
>> +    int phys_port_rxq;
>> +    uint16_t (*do_tx) (struct vswitch_port *port, uint16_t tx_q,
>> +               __attribute__((unused))struct rte_mempool *mbuf_pool,
>> +               struct rte_mbuf **tx_pkts,    uint16_t pkt_count);
>> +    uint16_t (*do_rx) (struct vswitch_port *port, uint16_t rx_q,
>> +               __attribute__((unused))struct rte_mempool *mbuf_pool,
>> +               struct rte_mbuf **rx_pkts,    uint16_t pkt_count);
>> +};
>> +
>> +struct vswitch_dev {
>> +    char name[VSWITCH_NAME_SIZE];
>> +    void *priv;                /* Private for switch specefic
>> +                         * data */
>> +    uint64_t conf_flags;
>> +    LIST_ENTRY (vswitch_dev) list;
>> +    struct vswitch_port *ports;
>> +    int port_count;
>> +    struct vswitch_ops *ops;
>> +};
>> +
>> +/* Function typedefs */
>> +typedef int (*vs_switch_init_t) (struct vswitch_dev *, uint64_t
>> conf_flags);
>> +typedef int (*vs_add_port_t) (struct vswitch_port *vs_port);
>> +typedef int (*vs_del_port_t) (struct vswitch_port *port);
>> +typedef int (*vs_learn_port_t) (struct vswitch_port *port, struct
>> rte_mbuf
>> +                **pkts, uint16_t count);
>> +typedef int (*vs_unlearn_port_t) (struct vswitch_port *port);
>> +typedef int (*vs_lookup_n_fwd_t)(struct vswitch_port *in_port, struct
>> rte_mbuf
>> +                  **pkts, uint16_t pkt_count, uint16_t rxq);
>> +#if 0
>> +typedef uint16_t (*vs_do_tx_t) (struct vswitch_port *port, uint16_t
>> tx_q, struct
>> +               rte_mbuf **tx_pkts,    uint16_t pkt_count);
>> +
>> +typedef uint16_t (*vs_do_rx_t) (struct vswitch_port *port, uint16_t
>> rx_q, struct
>> +               rte_mbuf **rx_pkts,    uint16_t pkt_count);
>> +#endif
>> +typedef struct vswitch_port* (*vs_sched_tx_port_t)(struct vswitch_dev
>> *vs_dev,
>> +                enum vswitch_port_type ptype, uint16_t core_id);
>> +
>> +typedef struct vswitch_port* (*vs_sched_rx_port_t)(struct vswitch_dev
>> *vs_dev,
>> +                enum vswitch_port_type ptype, uint16_t core_id);
>> +
>> +typedef int (*vs_port_start_t) (struct vswitch_port *port);
>> +
>> +struct vswitch_ops {
>> +    vs_add_port_t add_port;
>> +    vs_del_port_t del_port;
>> +    vs_lookup_n_fwd_t lookup_n_fwd;
>> +    vs_learn_port_t learn_port;
>> +    vs_unlearn_port_t unlearn_port;
>> +    vs_port_start_t port_start;
>> +    vs_port_start_t port_stop;
>> +    vs_switch_init_t switch_init;
>> +    vs_sched_tx_port_t sched_tx_port;
>> +    vs_sched_rx_port_t sched_rx_port;
>> +
>> +};
>> +
>> +/*VSWITCH conf flags */
>> +#define VS_CNF_FLG_VM2VM_HARDWARE    (1 << 0)
>> +#define VS_CNF_FLG_VM2VM_SOFTWARE    (1 << 1)
>> +#define VS_CNF_FLG_PROMISCOUS_EN    (1 << 2)
>> +#define VS_CNF_FLG_JUMBO_EN        (1 << 3)
>> +#define VS_CNF_FLG_VLAN_STRIP_EN    (1 << 4)
>> +#define VS_CNF_FLG_STATS_EN        (1 << 5)
>> +
>> +/*API for vswitch implementations*/
>> +struct vswitch_dev *vs_register_switch(const char *name, int priv_size,
>> +                int max_ports, struct vswitch_ops *ops);
>> +int  vs_unregister_switch(struct vswitch_dev *dev);
>> +
>> +int vs_vswitch_init(void);
>> +
>> +
>> +/*API for user of vswitch like vhost-switch application */
>> +
>> +struct vswitch_dev *vs_get_vswitch_dev(const char *name);
>> +struct vswitch_port *vs_add_port(struct vswitch_dev *vs_dev, int
>> port_id,
>> +        enum vswitch_port_type type, void *priv);
>> +
>> +int vs_del_port(struct vswitch_port *port);
>> +
>> +int vs_switch_dev_init(struct vswitch_dev *vs_dev, uint16_t conf_flags);
>> +
>> +int vs_port_start(struct vswitch_port *vs_port);
>> +int vs_port_stop(struct vswitch_port *vs_port);
>> +int vs_learn_port(struct vswitch_port *port, struct rte_mbuf
>> +                **pkts, uint16_t count);
>> +int vs_unlearn_port(struct vswitch_port *port);
>> +int vs_lookup_n_fwd(struct vswitch_port *in_port, struct rte_mbuf
>> +                  **pkts, uint16_t count, uint16_t in_rxq);
>> +
>> +int vs_do_broadcast_fwd(struct vswitch_dev *vs_dev,
>> +            struct vswitch_port *in_port, struct rte_mbuf *mbuf);
>> +
>> +struct vswitch_port *vs_sched_tx_port(struct vswitch_dev *vs_dev, enum
>> +                vswitch_port_type ptype, uint16_t core_id);
>> +
>> +struct vswitch_port *vs_sched_rx_port(struct vswitch_dev *vs_dev, enum
>> +                vswitch_port_type ptype, uint16_t core_id);
>> +/*Extern APIs from vhost/main.c */
>> +
>> +extern struct mbuf_table *vhost_switch_get_txq(uint16_t core_id);
>> +extern int virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m);
>> +extern struct vhost_dev *find_vhost_dev(struct ether_addr *mac);
>> +void do_drain_mbuf_table(struct mbuf_table *tx_q);
>> +
>> +/* TBD:XXX: This needs to be removed here, when constructor mechanism
>> + * for registering swittches is in place
>> + */
>> +extern void vmdq_switch_impl_init(void);
>> +#endif
>> +
>>
>




More information about the dev mailing list