[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT/LIBLWIP PATCH 3/5] Netif driver for libuknetdev devices



Hey Sharan,

Thanks for looking at this!

> On 11. Feb 2019, at 18:42, Sharan Santhanam <Sharan.Santhanam@xxxxxxxxx> 
> wrote:
> 
> Hello Simon,
> 
> Please find the review comments inline.
> 
> 
> Thanks & Regards
> Sharan
> 
> On 2/8/19 12:00 AM, Simon Kuenzer wrote:
>> Introduces a netif driver for libuknetdev devices. Packet buffers are
>> allocated on the heap for now. Receive interrupts are supported and
>> enabled when lwIP is operated in "threaded" mode. Zero-copy receive is
>> implemented with packet buffers having two personalities: pbuf and netbuf.
>> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
>> ---
>>  Config.uk                |  17 ++
>>  Makefile.uk              |   2 +
>>  include/netif/uknetdev.h |  80 +++++
>>  netbuf.c                 | 104 +++++++
>>  netbuf.h                 | 107 +++++++
>>  uknetdev.c               | 611 +++++++++++++++++++++++++++++++++++++++
>>  6 files changed, 921 insertions(+)
>>  create mode 100644 include/netif/uknetdev.h
>>  create mode 100644 netbuf.c
>>  create mode 100644 netbuf.h
>>  create mode 100644 uknetdev.c
>> diff --git a/Config.uk b/Config.uk
>> index dce5607..23813de 100644
>> --- a/Config.uk
>> +++ b/Config.uk
>> @@ -7,6 +7,23 @@ menuconfig LIBLWIP
>>      select HAVE_NW_STACK
>>    if LIBLWIP
>> +menu "Netif drivers"
>> +config LWIP_UKNETDEV
>> +    bool "Unikraft Netdev Ethernet (libuknetdev)"
>> +    default y
>> +    select LIBUKNETDEV
>> +    select LIBUKNETDEV_DISPATCHERTHREADS if LWIP_THREADS
>> +    help
>> +            A generic driver that operates network drivers through
>> +            libuknetdev API.
>> +            Note: When lwIP is configured with mainloop mode, receive
>> +            interrupts are not enabled by this driver (there is also no
>> +            receive interrupt handler registered).
>> +            In case threaded mode is selected and the underlying device
>> +            driver does not support receive interrupts the network
>> +            interfaces have to be polled manually (uknetdev_poll()).
>> +endmenu
>> +
>>  choice
>>      prompt "Operation mode"
>>      default LWIP_THREADS
>> diff --git a/Makefile.uk b/Makefile.uk
>> index ae3fbea..e7b7cc6 100644
>> --- a/Makefile.uk
>> +++ b/Makefile.uk
>> @@ -111,6 +111,8 @@ LIBLWIP_SRCS-$(CONFIG_LWIP_SOCKET) += 
>> $(LIBLWIP_EXTRACTED)/api/sockets.c
>>  # NETIFs
>>  
>> ################################################################################
>>  LIBLWIP_SRCS-y += $(LIBLWIP_EXTRACTED)/netif/ethernet.c
>> +LIBLWIP_SRCS-$(CONFIG_LWIP_UKNETDEV) += $(LIBLWIP_BASE)/netbuf.c|unikraft
>> +LIBLWIP_SRCS-$(CONFIG_LWIP_UKNETDEV) += $(LIBLWIP_BASE)/uknetdev.c|unikraft
>>    
>> ################################################################################
>>  # IPv4
>> diff --git a/include/netif/uknetdev.h b/include/netif/uknetdev.h
>> new file mode 100644
>> index 0000000..77aaa6d
>> --- /dev/null
>> +++ b/include/netif/uknetdev.h
>> @@ -0,0 +1,80 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2019, NEC Laboratories Europe GmbH, NEC Corporation.
>> + *                     All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. 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.
>> + * 3. Neither the name of the copyright holder 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 HOLDER 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.
>> + *
>> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>> + */
>> +
>> +#ifndef _LWIP_UKNETDEV_
>> +#define _LWIP_UKNETDEV_
>> +
>> +#include <uk/config.h>
>> +#include "lwip/opt.h"
>> +
>> +#if CONFIG_LWIP_UKNETDEV /* do not define anything if not configured for 
>> use */
>> +
>> +#include "lwip/netif.h"
>> +#include "lwip/ip_addr.h"
>> +#include "netif/ethernet.h"
>> +#include <uk/netdev.h>
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +/* Configures and brings up net device */
>> +err_t uknetdev_init(struct netif *nf);
>> +
>> +/*
>> + * Note: When threading mode is enabled, only uknetdev netifs that do not
>> + * support receive interrupts should be polled for packets.
>> + */
>> +void uknetdev_poll(struct netif *nf);
>> +
>> +#ifdef CONFIG_LWIP_NOTHREADS
>> +void uknetdev_poll_all(void);
>> +#endif /* CONFIG_LWIP_NOTHREADS */
>> +
>> +struct netif *uknetdev_addif(struct uk_netdev *n
>> +#if LWIP_IPV4
>> +                         ,
>> +                         const ip4_addr_t *ipaddr,
>> +                         const ip4_addr_t *netmask,
>> +                         const ip4_addr_t *gw
>> +#endif /* LWIP_IPV4 */
>> +    );
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif /* CONFIG_LWIP_UKNETDEV */
>> +#endif /* _LWIP_NETDEV_ */
>> diff --git a/netbuf.c b/netbuf.c
>> new file mode 100644
>> index 0000000..4878f74
>> --- /dev/null
>> +++ b/netbuf.c
>> @@ -0,0 +1,104 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2019, NEC Laboratories Europe GmbH, NEC Corporation.
>> + *                     All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. 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.
>> + * 3. Neither the name of the copyright holder 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 HOLDER 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.
>> + *
>> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>> + */
>> +
>> +#include "netbuf.h"
>> +#include <string.h>
>> +
>> +/**
>> + * This function is called by lwip when our pbuf is free'd
>> + * At this point, lwip should have no reference to the pbuf
>> + * anymore, so we can just release the encapsulating netbuf
>> + */
>> +static void _netbuf_free(struct pbuf *p)
>> +{
>> +    struct uk_netbuf *nb;
>> +
>> +    nb = lwip_pbuf_to_netbuf(p);
>> +    uk_netbuf_free_single(nb);
>> +}
>> +
>> +struct uk_netbuf *lwip_alloc_netbuf(struct uk_alloc *a, size_t alloc_size,
>> +                                size_t headroom)
>> +{
>> +    void *allocation;
>> +    struct uk_netbuf *b;
>> +    struct _netbuf_pbuf *np;
>> +
>> +    allocation = uk_malloc(a, alloc_size);
>> +    if (unlikely(!allocation))
>> +            goto err_out;
>> +
>> +    b = uk_netbuf_prepare_buf(allocation, alloc_size,
>> +                              headroom, sizeof(struct _netbuf_pbuf), NULL);
>> +    if (unlikely(!b)) {
>> +            LWIP_DEBUGF(PBUF_DEBUG,
>> +                        ("Failed to initialize netbuf with encapsulated 
>> pbuf: requested headroom: %"__PRIsz", alloc_size: %"__PRIsz"\n",
>> +                         headroom, alloc_size));
>> +            goto err_free_allocation;
>> +    }
>> +
>> +    /*
>> +     * Register allocator so that uk_netbuf_free() will
>> +     * return our memory back to this allocator when free'ing
>> +     * this netbuf
>> +     */
>> +    b->_a = a;
>> +
>> +    /* Fill-out meta data */
>> +    np = (struct _netbuf_pbuf *) uk_netbuf_get_priv(b);
>> +    memset(np, 0, sizeof(struct _netbuf_pbuf));
>> +    np->pbuf_custom.pbuf.type_internal   = PBUF_ROM;
>> +    np->pbuf_custom.pbuf.flags           = PBUF_FLAG_IS_CUSTOM;
>> +    np->pbuf_custom.pbuf.payload         = b->data;
>> +    np->pbuf_custom.pbuf.ref             = 1;
>> +    np->pbuf_custom.custom_free_function = _netbuf_free;
>> +    np->netbuf = b;
>> +
>> +    /*
>> +     * Set length of netbuf to available space so that it
>> +     * can be used as receive buffer
>> +     */
>> +    b->len = b->buflen - headroom;
>> +
>> +    LWIP_DEBUGF(PBUF_DEBUG,
>> +                ("Allocated netbuf with encapsulated pbuf %p (buflen: 
>> %"__PRIsz", headroom: %"__PRIsz")\n",
>> +                 b, b->buflen, uk_netbuf_headroom(b)));
>> +    return b;
>> +
>> +err_free_allocation:
>> +    uk_free(a, allocation);
>> +err_out:
>> +    return NULL;
>> +}
>> diff --git a/netbuf.h b/netbuf.h
>> new file mode 100644
>> index 0000000..1492065
>> --- /dev/null
>> +++ b/netbuf.h
>> @@ -0,0 +1,107 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2019, NEC Laboratories Europe GmbH, NEC Corporation.
>> + *                     All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. 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.
>> + * 3. Neither the name of the copyright holder 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 HOLDER 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.
>> + *
>> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>> + */
>> +
>> +#ifndef _LWIP_NETBUF_
>> +#define _LWIP_NETBUF_
>> +
>> +#include <uk/config.h>
>> +#include "lwip/opt.h"
>> +
>> +#if CONFIG_LWIP_UKNETDEV /* do not define anything if not configured for 
>> use */
>> +
>> +#include "lwip/pbuf.h"
>> +#include <uk/netbuf.h>
>> +#include <uk/alloc.h>
>> +#include <uk/assert.h>
>> +#include <uk/essentials.h>
>> +
>> +struct _netbuf_pbuf {
>> +    struct pbuf_custom pbuf_custom;
>> +    struct uk_netbuf *netbuf;
>> +};
>> +
>> +/**
>> + * Allocates a netbuf that embeds a pbuf. Both will point
>> + * to the same packet data buffer.
>> + * Such buffers are used to implement zero-copy receive with the
>> + * uknetdev API.
>> + * NOTE: Make sure that netbuf and pbuf meta data are kept in sync
>> + * before handing over the embedded pbuf to the network stack.
>> + */
>> +struct uk_netbuf *lwip_alloc_netbuf(struct uk_alloc *a, size_t alloc_size,
>> +                                size_t headroom);
>> +
>> +/**
>> + * Returns the reference of the embedded pbuf of a netbuf
>> + */
>> +static inline struct pbuf *lwip_netbuf_to_pbuf(struct uk_netbuf *b)
>> +{
>> +    struct _netbuf_pbuf *np;
>> +
>> +    UK_ASSERT(b);
>> +    np = (struct _netbuf_pbuf *) uk_netbuf_get_priv(b);
>> +    UK_ASSERT(np);
>> +
>> +    /*
>> +     * Sanity check: We use the PBUF_CUSTOM_FLAG as a way
>> +     * to identify lwip_alloc_netbuf() was allocating this pbuf.
>> +     * We know it may fail but it is still better than nothing.
>> +     */
>> +    UK_ASSERT(np->pbuf_custom.pbuf.flags & PBUF_FLAG_IS_CUSTOM);
>> +
>> +    return &np->pbuf_custom.pbuf;
>> +}
>> +
>> +/**
>> + * Returns the reference of the encapsulating netbuf of a pbuf
>> + */
>> +static inline struct uk_netbuf *lwip_pbuf_to_netbuf(struct pbuf *p)
>> +{
>> +    struct pbuf_custom *pc;
>> +    struct _netbuf_pbuf *np;
>> +
>> +    UK_ASSERT(p);
>> +    UK_ASSERT(p->flags & PBUF_FLAG_IS_CUSTOM);
>> +
>> +    pc = (struct pbuf_custom *) __containerof(p,  struct pbuf_custom, pbuf);
>> +    UK_ASSERT(pc);
>> +    np = (struct _netbuf_pbuf *)  __containerof(pc, struct _netbuf_pbuf,
>> +                                                pbuf_custom);
>> +    UK_ASSERT(np);
>> +    return np->netbuf;
>> +}
>> +
>> +#endif /* CONFIG_LWIP_UKNETDEV */
>> +#endif /* _LWIP_NETBUF_ */
>> diff --git a/uknetdev.c b/uknetdev.c
>> new file mode 100644
>> index 0000000..1de202d
>> --- /dev/null
>> +++ b/uknetdev.c
>> @@ -0,0 +1,611 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2019, NEC Laboratories Europe GmbH, NEC Corporation.
>> + *                     All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. 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.
>> + * 3. Neither the name of the copyright holder 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 HOLDER 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.
>> + *
>> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>> + */
>> +
>> +#include <uk/config.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +
>> +#include <uk/alloc.h>
>> +#include <uk/print.h>
>> +#include "netif/uknetdev.h"
>> +#include "netbuf.h"
>> +
>> +#include "lwip/opt.h"
>> +#include "lwip/def.h"
>> +#include "lwip/pbuf.h"
>> +#include "lwip/stats.h"
>> +#include "lwip/snmp.h"
>> +#include "lwip/sys.h"
>> +#include "lwip/tcpip.h"
>> +#include "lwip/ethip6.h"
>> +#include "netif/etharp.h"
>> +#include "netif/ethernet.h"
>> +#include <uk/arch/atomic.h>
>> +
>> +#include <uk/essentials.h>
>> +
>> +#define UKNETDEV_BPS 1000000000u
>> +#define UKNETDEV_BUFLEN 2048
>> +
>> +#define UKNETDEV_NETIF_NAME0 'e'
>> +#define UKNETDEV_NETIF_NAME1 'n'
>> +
>> +/*
>> + * Global headroom settings for buffer allocations used on receive
>> + * and transmit. We are taking the maximum of all uknetdev devices as
>> + * simplification.
>> + * TODO: A per-device setting might be more efficient but requires more data
>> + *       fields for `netif->state`. For now we point directly to the 
>> according
>> + *       `struct uk_netdev` in order to avoid another allocation for these
>> + *       per-device fields.
>> + */
>> +static uint16_t rx_headroom = ETH_PAD_SIZE;
>> +static uint16_t tx_headroom = ETH_PAD_SIZE;
>> +
>> +#define netif_to_uknetdev(nf) \
>> +    ((struct uk_netdev *) (nf)->state)
>> +
>> +static uint16_t netif_alloc_rxpkts(void *argp, struct uk_netbuf *nb[],
>> +                               uint16_t count)
>> +{
>> +    struct uk_alloc *a;
>> +    uint16_t i;
>> +
>> +    UK_ASSERT(argp);
>> +
>> +    a = (struct uk_alloc *) argp;
>> +
>> +    for (i = 0; i < count; ++i) {
>> +            nb[i] = lwip_alloc_netbuf(a, UKNETDEV_BUFLEN, rx_headroom);
>> +            if (!nb[i]) {
>> +                    /* we run out of memory */
>> +                    break;
>> +            }
>> +    }
>> +
>> +    return i;
>> +}
>> +
>> +static err_t uknetdev_output(struct netif *nf, struct pbuf *p)
>> +{
>> +    struct uk_alloc *a;
>> +    struct uk_netdev *dev;
>> +    struct pbuf *q;
>> +    struct uk_netbuf *nb;
>> +    void *allocation;
>> +    char *wpos;
>> +    int ret;
>> +
>> +    UK_ASSERT(nf);
>> +    dev = netif_to_uknetdev(nf);
>> +    UK_ASSERT(dev);
>> +
>> +    a = uk_alloc_get_default();
>> +    if (!a)
>> +            return ERR_MEM;
>> +
>> +    allocation = uk_malloc(a, UKNETDEV_BUFLEN);
>> +    if (!allocation)
>> +            return ERR_MEM;
>> +    nb = uk_netbuf_prepare_buf(allocation, UKNETDEV_BUFLEN,
>> +                               tx_headroom, 0, NULL);
>> +    UK_ASSERT(nb);
>> +    nb->_a = a; /* register allocator for free operation */
>> +
>> +    if (unlikely(p->tot_len > uk_netbuf_tailroom(nb))) {
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Cannot send %"PRIu16" bytes, too big 
>> (> %"__PRIsz")\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num,
>> +                         p->tot_len, uk_netbuf_tailroom(nb)));
>> +            uk_netbuf_free_single(nb);
>> +            return ERR_MEM;
>> +    }
>> +
>> +#if ETH_PAD_SIZE
>> +    pbuf_header(p, -ETH_PAD_SIZE); /* drop the padding word */
>> +#endif /* ETH_PAD_SIZE */
>> +    /*
>> +     * Copy pbuf to netbuf
>> +     * NOTE: Unfortunately, lwIP seems not to support zero-copy transmit,
>> +     *       yet. As long as we do not have this, we have to copy.
>> +     */
>> +    wpos = nb->data;
>> +    for (q = p; q != NULL; q = p->next) {
>> +            memcpy(wpos, q->payload, q->len);
>> +            wpos += q->len;
>> +    }
>> +    nb->len = p->tot_len;
>> +#if ETH_PAD_SIZE
>> +    pbuf_header(p, ETH_PAD_SIZE); /* reclaim the padding word */
>> +#endif /* ETH_PAD_SIZE */
>> +
>> +    /* Transmit packet */
>> +    do {
>> +            ret = uk_netdev_tx_one(dev, 0, nb);
>> +    } while (uk_netdev_status_notready(ret));
>> +    if (unlikely(ret < 0)) {
>> +#if ETH_PAD_SIZE
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Failed to send %"PRIu16" bytes\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num,
>> +                         p->tot_len - ETH_PAD_SIZE));
>> +#else /* ETH_PAD_SIZE */
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Failed to send %"PRIu16" bytes\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num,
>> +                         p->tot_len));
>> +#endif /* ETH_PAD_SIZE */
>> +            /*
>> +             * Decrease refcount again because in
>> +             * the error case the netdev did not consume the pbuf
>> +             */
>> +            uk_netbuf_free_single(nb);
>> +            return ERR_IF;
>> +    }
>> +#if ETH_PAD_SIZE
>> +    LWIP_DEBUGF(NETIF_DEBUG, ("%s: %c%c%u: Sent %"PRIu16" bytes\n",
>> +                              __func__, nf->name[0], nf->name[1], nf->num,
>> +                              p->tot_len - ETH_PAD_SIZE));
>> +#else /* ETH_PAD_SIZE */
>> +    LWIP_DEBUGF(NETIF_DEBUG, ("%s: %c%c%u: Sent %"PRIu16" bytes\n",
>> +                              __func__, nf->name[0], nf->name[1], nf->num,
>> +                              p->tot_len));
>> +#endif /* ETH_PAD_SIZE */
>> +
>> +    return ERR_OK;
>> +}
>> +
>> +static void uknetdev_input(struct uk_netdev *dev,
>> +                       uint16_t queue_id __unused, void *argp)
>> +{
>> +    struct netif *nf = (struct netif *) argp;
>> +    struct uk_netbuf *nb;
>> +    struct pbuf *p;
>> +    int ret;
>> +
>> +    UK_ASSERT(dev);
>> +    UK_ASSERT(nf);
>> +    UK_ASSERT(nf->input);
>> +
>> +    LWIP_DEBUGF(NETIF_DEBUG, ("%s: %c%c%u: Poll receive queue...\n",
>> +                              __func__, nf->name[0], nf->name[1], nf->num));
>> +    do {
>> +            ret = uk_netdev_rx_one(dev, 0, &nb);
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Input status %d (%c%c%c)\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num, ret,
>> +                         uk_netdev_status_test_set(ret,
>> +                                                   UK_NETDEV_STATUS_SUCCESS)
>> +                         ? 'S' : '-',
>> +                         uk_netdev_status_test_set(ret,
>> +                                                   UK_NETDEV_STATUS_MORE)
>> +                         ? 'M' : '-',
>> +                         uk_netdev_status_test_set(ret,
>> +                                                  UK_NETDEV_STATUS_UNDERRUN)
>> +                         ? 'U' : '-'));
>> +            if (unlikely(ret < 0)) {
>> +                    /*
>> +                     * Ouch, an error happened. We cannot recover from it
>> +                     * currently, so we will throw an error message, bring
>> +                     * the interface down, and leave our loop.
>> +                     */
>> +                    uk_pr_crit("%c%c%u: Receive error %d. Stopping 
>> interface...\n",
>> +                               nf->name[0], nf->name[1], nf->num, ret);
>> +                    netif_set_down(nf);
>> +                    break;
>> +            }
>> +            if (uk_netdev_status_notready(ret)) {
>> +                    /* No (more) packets received */
>> +                    break;
>> +            }
>> +
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Received %"PRIu16" bytes\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num,
>> +                         nb->len));
>> +
>> +            /* Send packet to lwip */
>> +#if ETH_PAD_SIZE
>> +            uk_netbuf_header(nb, ETH_PAD_SIZE);
>> +#endif /* ETH_PAD_SIZE */
>> +            p = lwip_netbuf_to_pbuf(nb);
>> +            p->payload = nb->data;
>> +            p->tot_len = p->len = nb->len;
>> +            if (unlikely(nf->input(p, nf) != ERR_OK)) {
>> +                    /*
>> +                     * Drop the packet that we could not send to the stack
>> +                     */
>> +                    uk_pr_err("%c%c%u: Failed to forward packet to lwIP\n",
>> +                              nf->name[0], nf->name[1], nf->num);
>> +                    uk_netbuf_free_single(nb);
>> +            }
>> +    } while (uk_netdev_status_more(ret));
>> +}
>> +
> 
> Would be better if poll function returns an error. The user of the poll as of 
> now is blind in case rx_one fails

Hum... I actually took the same function signature for polling as I saw it on 
lwip code: slipif_poll() and netif_poll(). In this sense it is inline with what 
someone would expect from lwip. At the same time, as long we do not have net if 
remove support, a user can’t really do much in error cases.
How would you proceed?

>> +void uknetdev_poll(struct netif *nf)
>> +{
>> +    struct uk_netdev *dev;
>> +
>> +    UK_ASSERT(nf);
>> +    /*
>> +     * TODO: Unfortunately, checking the interface name is a weak sanity
>> +     * check that uknetdev_poll() is called on a netif that is driven by
>> +     * this driver...
>> +     */
>> +    UK_ASSERT(nf->name[0] == UKNETDEV_NETIF_NAME0);
>> +    UK_ASSERT(nf->name[1] == UKNETDEV_NETIF_NAME1);
>> +
>> +    dev = netif_to_uknetdev(nf);
>> +    UK_ASSERT(dev);
>> +
>> +    uknetdev_input(dev, 0, nf);
>> +}
>> +
>> +#ifdef CONFIG_LWIP_NOTHREADS
>> +void uknetdev_poll_all(void)
>> +{
>> +    struct netif *nf;
>> +
>> +    /*
>> +     * TODO: We are going through all netifs and check the interface name.
>> +     * This way we are figuring out that the netif is provided by our
>> +     * driver. Probably we should find a better and more stable solution at
>> +     * some point...
>> +     */
>> +    NETIF_FOREACH(nf) {
>> +            if (nf->name[0] == UKNETDEV_NETIF_NAME0
>> +                && nf->name[1] == UKNETDEV_NETIF_NAME1)
>> +                    uknetdev_poll(nf);
>> +    }
>> +}
>> +
>> +#else /* CONFIG_LWIP_NOTHREADS */
>> +
>> +static void uknetdev_updown(struct netif *nf)
>> +{
>> +    struct uk_netdev *dev;
>> +    int ret;
>> +
>> +    UK_ASSERT(nf);
>> +    dev = netif_to_uknetdev(nf);
>> +    UK_ASSERT(dev);
>> +
>> +    /* Enable and disable interrupts according to netif's up/down status */
>> +    if (nf->flags & NETIF_FLAG_UP) {
>> +            ret = uk_netdev_rxq_intr_enable(dev, 0);
>> +            if (ret < 0) {
>> +                    LWIP_DEBUGF(NETIF_DEBUG,
>> +                                ("%s: %c%c%u: Failed to enable rx 
>> interrrupt mode on netdev %u\n",
>> +                                 __func__, nf->name[0], nf->name[1],
>> +                                 nf->num, uk_netdev_id_get(dev)));
>> +            } else {
>> +                    LWIP_DEBUGF(NETIF_DEBUG,
>> +                                ("%s: %c%c%u: Enabled rx interrupt mode on 
>> netdev %u\n",
>> +                                 __func__, nf->name[0], nf->name[1],
>> +                                 nf->num, uk_netdev_id_get(dev)));
>> +            }
>> +
>> +            if (ret == 1) {
>> +                    /*
>> +                     * uk_netdev_rxq_intr_enable() told us that we need to
>> +                     * flush the receieve queue before interrupts are
>> +                     * enabled. For this purpose we do an initial poll.
>> +                     */
>> +                    uknetdev_poll(nf);
>> +            }
>> +    } else {
>> +            uk_netdev_rxq_intr_disable(dev, 0);
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Disabled rx interrupts on netdev 
>> %u\n",
>> +                         __func__, nf->name[0], nf->name[1],
>> +                         nf->num, uk_netdev_id_get(dev)));
>> +
>> +    }
>> +}
>> +#endif /* CONFIG_LWIP_NOTHREADS */
>> +
>> +err_t uknetdev_init(struct netif *nf)
>> +{
>> +    struct uk_alloc *a = NULL;
>> +    struct uk_netdev *dev;
>> +    struct uk_netdev_conf dev_conf;
>> +    struct uk_netdev_rxqueue_conf rxq_conf;
>> +    struct uk_netdev_txqueue_conf txq_conf;
>> +    struct uk_netdev_info info;
>> +    const struct uk_hwaddr *hwaddr;
>> +    unsigned int i;
>> +    int ret;
>> +
>> +    UK_ASSERT(nf);
>> +    dev = netif_to_uknetdev(nf);
>> +    UK_ASSERT(dev);
>> +
>> +    LWIP_ASSERT("uknetdev needs an input callback (netif_input or 
>> tcpip_input)",
>> +                nf->input != NULL);
>> +
>> +    /* Netdev has to be in unconfigured state */
>> +    if (uk_netdev_state_get(dev) != UK_NETDEV_UNCONFIGURED) {
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: Netdev %u not in uncofigured state\n",
>> +                         __func__, uk_netdev_id_get(dev)));
>> +            return ERR_ISCONN;
>> +    }
>> +
>> +    /* Interface name, the interface number (nf->num) is assigned by lwip */
>> +    nf->name[0] = UKNETDEV_NETIF_NAME0;
>> +    nf->name[1] = UKNETDEV_NETIF_NAME1;
>> +
>> +    /*
>> +     * Bring up uknetdev
>> +     * Note: We use the default allocator for setting up the rx/tx queues
>> +     */
>> +    /* TODO: In case the device initialization should happen manually before
>> +     *       attaching to lwip, we require another init function that skips
>> +     *       this initialization steps.
>> +     */
>> +    a = uk_alloc_get_default();
>> +    if (!a)
>> +            return ERR_MEM;
>> +
>> +    /* Get device information */
>> +    uk_netdev_info_get(dev, &info);
>> +    if (!info.max_rx_queues || !info.max_tx_queues)
>> +            return ERR_IF;
>> +
>> +    /*
>> +     * Update our global (rx|tx)_headroom setting that we use for
>> +     * buffer allocations
>> +     */
>> +    rx_headroom = (rx_headroom < info.nb_encap_rx)
>> +                  ? info.nb_encap_rx : rx_headroom;
>> +    tx_headroom = (tx_headroom < info.nb_encap_tx)
>> +                  ? info.nb_encap_tx : tx_headroom;
>> +    LWIP_DEBUGF(NETIF_DEBUG,
>> +                ("%s: %c%c%u: Need headroom rx:%"PRIu16", tx:%"PRIu16"\n",
>> +                 __func__, nf->name[0], nf->name[1], nf->num,
>> +                 info.nb_encap_rx, info.nb_encap_tx));
>> +
>> +    /*
>> +     * Device configuration,
>> +     * we want to use just one queue for each direction
>> +     */
>> +    dev_conf.nb_rx_queues = 1;
>> +    dev_conf.nb_tx_queues = 1;
>> +    ret = uk_netdev_configure(dev, &dev_conf);
>> +    if (ret < 0) {
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Failed to configure netdev %u\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num,
>> +                         uk_netdev_id_get(dev)));
>> +            return ERR_IF;
>> +    }
>> +
>> +    /*
>> +     * Receive queue,
>> +     * use driver default descriptors
>> +     */
>> +    rxq_conf.a = a;
>> +    rxq_conf.alloc_rxpkts = netif_alloc_rxpkts;
>> +    rxq_conf.alloc_rxpkts_argp = a;
>> +#ifdef CONFIG_LWIP_NOTHREADS
>> +    /*
>> +     * In mainloop mode, we will not use interrupts.
>> +     */
>> +    rxq_conf.callback = NULL;
>> +    rxq_conf.callback_cookie = NULL;
>> +#else  /* CONFIG_LWIP_NOTHREADS */
>> +    rxq_conf.callback = uknetdev_input;
>> +    rxq_conf.callback_cookie = nf;
>> +#ifdef CONFIG_LIBUKNETDEV_DISPATCHERTHREADS
>> +    rxq_conf.s = uk_sched_get_default();
>> +    if (!rxq_conf.s)
>> +            return ERR_IF;
>> +
>> +#endif /* CONFIG_LIBUKNETDEV_DISPATCHERTHREADS */
>> +#endif /* CONFIG_LWIP_NOTHREADS */
>> +    ret = uk_netdev_rxq_configure(dev, 0, 0, &rxq_conf);
>> +    if (ret < 0) {
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Failed to configure rx queue of 
>> netdev %u\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num,
>> +                         uk_netdev_id_get(dev)));
>> +            return ERR_IF;
>> +    }
>> +
>> +    /*
>> +     * Transmit queue,
>> +     * use driver default descriptors
>> +     */
>> +    txq_conf.a = a;
>> +    ret = uk_netdev_txq_configure(dev, 0, 0, &txq_conf);
>> +    if (ret < 0) {
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Failed to configure tx queue of 
>> netdev %u\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num,
>> +                         uk_netdev_id_get(dev)));
>> +            return ERR_IF;
>> +    }
>> +
>> +    /* Start interface */
>> +    ret = uk_netdev_start(dev);
>> +    if (ret < 0) {
>> +            LWIP_DEBUGF(NETIF_DEBUG,
>> +                        ("%s: %c%c%u: Failed to start netdev %u\n",
>> +                         __func__, nf->name[0], nf->name[1], nf->num,
>> +                         uk_netdev_id_get(dev)));
>> +            return ERR_IF;
>> +    }
>> +
>> +    /* Driver callbacks */
>> +#if LWIP_IPV4
>> +    nf->output = etharp_output;
>> +#endif /* LWIP_IPV4 */
>> +#if LWIP_IPV6
>> +    nf->output_ip6 = ethip6_output;
>> +#endif /* LWIP_IPV6 */
>> +    nf->linkoutput = uknetdev_output;
>> +
>> +    /* TODO: Set remove callback */
>> +
>> +    /* Device capabilities */
>> +    netif_set_flags(nf, (NETIF_FLAG_BROADCAST
>> +                         | NETIF_FLAG_ETHARP
>> +                         | NETIF_FLAG_LINK_UP));
>> +    LWIP_DEBUGF(NETIF_DEBUG,
>> +                ("%s: %c%c%u: flags: %"PRIx8"\n",
>> +                 __func__, nf->name[0], nf->name[1], nf->num, nf->flags));
>> +
>> +#if LWIP_CHECKSUM_CTRL_PER_NETIF
>> +    /*
>> +     * Checksum settings
>> +     * TODO: libuknetdev does not support checksum capabilities yet.
>> +     *       Because of this, we need to calculate the checksum for every
>> +     *       outgoing packet in software. We assume that we receive packets
>> +     *       from a virtual interface, so the host was doing a check for us
>> +     *       already. In case of guest-to-guest communication, the checksum
>> +     *       field may be incorrect because the other guest expects that the
>> +     *       host is offloading the calculation to hardware as soon as a
>> +     *       packet leaves the physical host machine. At this point, the
>> +     *       best we can do is not to check any checksums on incoming
>> +     *       traffic and assume everything is fine.
>> +     */
>> +    NETIF_SET_CHECKSUM_CTRL(nf, (NETIF_CHECKSUM_GEN_IP
>> +                                 | NETIF_CHECKSUM_GEN_UDP
>> +                                 | NETIF_CHECKSUM_GEN_TCP
>> +                                 | NETIF_CHECKSUM_GEN_ICMP
>> +                                 | NETIF_CHECKSUM_GEN_ICMP6));
>> +    LWIP_DEBUGF(NETIF_DEBUG,
>> +                ("%s: %c%c%u: chksum_flags: %"PRIx16"\n",
>> +                 __func__, nf->name[0], nf->name[1], nf->num,
>> +                 nf->chksum_flags));
>> +#endif /* LWIP_CHECKSUM_CTRL_PER_NETIF */
>> +
>> +    /* MAC address */
>> +    UK_ASSERT(NETIF_MAX_HWADDR_LEN >= UK_NETDEV_HWADDR_LEN);
>> +    hwaddr = uk_netdev_hwaddr_get(dev);
>> +    UK_ASSERT(hwaddr);
>> +    nf->hwaddr_len = UK_NETDEV_HWADDR_LEN;
>> +    for (i = 0; i < UK_NETDEV_HWADDR_LEN; ++i)
>> +            nf->hwaddr[i] = hwaddr->addr_bytes[i];
>> +#if UK_NETDEV_HWADDR_LEN == 6
>> +    LWIP_DEBUGF(NETIF_DEBUG,
>> +                ("%s: %c%c%u: Hardware address: 
>> %02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8"\n",
>> +                 __func__, nf->name[0], nf->name[1], nf->num,
>> +                 nf->hwaddr[0], nf->hwaddr[1], nf->hwaddr[2],
>> +                 nf->hwaddr[3], nf->hwaddr[4], nf->hwaddr[5]));
>> +#else /* UK_NETDEV_HWADDR_LEN */
>> +    LWIP_DEBUGF(NETIF_DEBUG,
>> +                ("%s: %c%c%u: Hardware address set\n",
>> +                 __func__, nf->name[0], nf->name[1], nf->num));
>> +#endif /* UK_NETDEV_HWADDR_LEN */
>> +
>> +    /* Maximum transfer unit */
>> +    nf->mtu = uk_netdev_mtu_get(dev);
>> +    UK_ASSERT(nf->mtu);
>> +    LWIP_DEBUGF(NETIF_DEBUG,
>> +                ("%s: %c%c%u: MTU: %u\n",
>> +                 __func__, nf->name[0], nf->name[1], nf->num,
>> +                 nf->mtu));
>> +
>> +#ifndef CONFIG_LWIP_NOTHREADS
>> +    /*
>> +     * We will use the status update callback to enable and disabled
>> +     * receive queue interrupts
>> +     */
>> +    netif_set_status_callback(nf, uknetdev_updown);
>> +#endif /* !CONFIG_LWIP_NOTHREADS */
>> +
>> +    /*
>> +     * Initialize the snmp variables and counters inside the struct netif.
>> +     * The last argument is the link speed, in units of bits per second.
>> +     */
>> +    NETIF_INIT_SNMP(nf, snmp_ifType_ethernet_csmacd, UKNETDEV_BPS);
>> +    LWIP_DEBUGF(NETIF_DEBUG,
>> +                ("%s: %c%c%u: Link speed: %"PRIu32" bps\n",
>> +                 __func__, nf->name[0], nf->name[1], nf->num,
>> +                 UKNETDEV_BPS));
>> +
>> +    return ERR_OK;
>> +}
>> +
>> +#if CONFIG_LWIP_NOTHREADS
>> +#define NETIF_INPUT ethernet_input
>> +#else /* CONFIG_LWIP_NOTHREADS */
>> +#define NETIF_INPUT tcpip_input
>> +#endif /*CONFIG_LWIP_NOTHREADS */
>> +
>> +struct netif *uknetdev_addif(struct uk_netdev *n
>> +#if LWIP_IPV4
>> +                         ,
>> +                         const ip4_addr_t *ipaddr,
>> +                         const ip4_addr_t *netmask,
>> +                         const ip4_addr_t *gw
>> +#endif /* LWIP_IPV4 */
>> +    )
>> +{
>> +    /*
>> +     * This pointer and UK_READ_ONCE on it is an ugly workaround
>> +     * against a pretty weird problem. Without it, the last
>> +     * parameter of netif_add passed as NULL. It seems to be a
>> +     * build time problem because:
>> +     *
>> +     * - Moving "input" parameter to the first positions helps
>> +     * - Removing one parameter helps. Seems does not matter which
>> +     * - An extra parameter added after "input" works well. And
>> +     *   input is still NULL in this case
>> +     * - Swapping "init" and "input" helps
>> +     */
>> +    static const void *pethernet_input = NETIF_INPUT;
>> +    struct netif *nf;
>> +    struct netif *ret;
>> +
>> +    nf = mem_calloc(1, sizeof(*nf));
>> +    if (!nf)
>> +            return NULL;
>> +
>> +    ret = netif_add(nf,
>> +#if LWIP_IPV4
>> +                    ipaddr, netmask, gw,
>> +#endif /* LWIP_IPV4 */
>> +                    n, uknetdev_init, UK_READ_ONCE(pethernet_input));
>> +    UK_ASSERT(nf->input);
>> +
>> +    if (!ret) {
>> +            mem_free(nf);
>> +            return NULL;
>> +    }
>> +
>> +    return ret;
>> +}

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.