|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [UNIKRAFT PATCH v4 04/12] plat/xen/drivers/net: Configure netfront device
Hi Sharan,
Please see inline.
On 10/22/20 2:00 PM, Sharan Santhanam wrote:
> Hello Costin,
>
> Please find some minor comments inline:
>
> Thanks & Regards
>
> Sharan
>
> On 8/13/20 10:53 AM, Costin Lupu wrote:
>> The information needed for configuring netfront devices is in the
>> Xenstore. For
>> now, we only retrieve the MAC and IP addresses from there in order to
>> initialize the device.
>>
>> This patch introduces the `netfront_dev` structure which integrates
>> both netdev
>> and Xenbus device information and also keeps the configuration
>> parameters.
>>
>> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>> Signed-off-by: Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx>
>> ---
>> plat/xen/Makefile.uk | 1 +
>> plat/xen/drivers/net/netfront.c | 100 ++++++++++++++
>> plat/xen/drivers/net/netfront.h | 66 ++++++++++
>> plat/xen/drivers/net/netfront_xb.h | 43 ++++++
>> plat/xen/drivers/net/netfront_xs.c | 205 +++++++++++++++++++++++++++++
>> 5 files changed, 415 insertions(+)
>> create mode 100644 plat/xen/drivers/net/netfront.h
>> create mode 100644 plat/xen/drivers/net/netfront_xb.h
>> create mode 100644 plat/xen/drivers/net/netfront_xs.c
>>
>> diff --git a/plat/xen/Makefile.uk b/plat/xen/Makefile.uk
>> index 339b1b21..dfca58fe 100644
>> --- a/plat/xen/Makefile.uk
>> +++ b/plat/xen/Makefile.uk
>> @@ -121,6 +121,7 @@ LIBXENNETFRONT_ASINCLUDES-y +=
>> $(LIBXENPLAT_ASINCLUDES-y)
>> LIBXENNETFRONT_CFLAGS-y += $(LIBXENPLAT_CFLAGS-y)
>> LIBXENNETFRONT_CINCLUDES-y += $(LIBXENPLAT_CINCLUDES-y)
>> LIBXENNETFRONT_SRCS-y +=
>> $(LIBXENPLAT_BASE)/drivers/net/netfront.c
>> +LIBXENNETFRONT_SRCS-y +=
>> $(LIBXENPLAT_BASE)/drivers/net/netfront_xs.c
>> endif
>> ifeq ($(CONFIG_XEN_BLKFRONT),y)
>> diff --git a/plat/xen/drivers/net/netfront.c
>> b/plat/xen/drivers/net/netfront.c
>> index b455911e..539e1cbc 100644
>> --- a/plat/xen/drivers/net/netfront.c
>> +++ b/plat/xen/drivers/net/netfront.c
>> @@ -33,21 +33,121 @@
>> * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>> */
>> +#include <uk/assert.h>
>> +#include <uk/print.h>
>> +#include <uk/alloc.h>
>> +#include <uk/netdev_driver.h>
>> #include <xenbus/xenbus.h>
>> +#include "netfront.h"
>> +#include "netfront_xb.h"
>> #define DRIVER_NAME "xen-netfront"
>> +#define to_netfront_dev(dev) \
>> + __containerof(dev, struct netfront_dev, netdev)
>> +
>> static struct uk_alloc *drv_allocator;
>> +static const void *netfront_einfo_get(struct uk_netdev *n,
>> + enum uk_netdev_einfo_type einfo_type)
>> +{
>> + struct netfront_dev *nfdev;
>> +
>> + UK_ASSERT(n != NULL);
>> +
>> + nfdev = to_netfront_dev(n);
>> + switch (einfo_type) {
>> + case UK_NETDEV_IPV4_ADDR_STR:
>> + return nfdev->econf.ipv4addr;
>> + case UK_NETDEV_IPV4_MASK_STR:
>> + return nfdev->econf.ipv4mask;
>> + case UK_NETDEV_IPV4_GW_STR:
>> + return nfdev->econf.ipv4gw;
>> + default:
>> + break;
>> + }
>> +
>> + /* type not supported */
>> + return NULL;
>> +}
>> +
>> +static const struct uk_hwaddr *netfront_mac_get(struct uk_netdev *n)
>> +{
>> + struct netfront_dev *nfdev;
>> +
>> + UK_ASSERT(n != NULL);
>> + nfdev = to_netfront_dev(n);
>> + return &nfdev->hw_addr;
>> +}
>> +
>> +static uint16_t netfront_mtu_get(struct uk_netdev *n)
>> +{
>> + struct netfront_dev *nfdev;
>> +
>> + UK_ASSERT(n != NULL);
>> + nfdev = to_netfront_dev(n);
>> + return nfdev->mtu;
>> +}
>> +
>> +static unsigned int netfront_promisc_get(struct uk_netdev *n)
>> +{
>> + struct netfront_dev *nfdev;
>> +
>> + UK_ASSERT(n != NULL);
>> + nfdev = to_netfront_dev(n);
>> + return nfdev->promisc;
>> +}
>> +
>> +static const struct uk_netdev_ops netfront_ops = {
>> + .einfo_get = netfront_einfo_get,
>> + .hwaddr_get = netfront_mac_get,
>> + .mtu_get = netfront_mtu_get,
>> + .promiscuous_get = netfront_promisc_get,
>> +};
>> static int netfront_add_dev(struct xenbus_device *xendev)
>> {
>> + struct netfront_dev *nfdev;
>> int rc = 0;
>> UK_ASSERT(xendev != NULL);
>> + nfdev = uk_calloc(drv_allocator, 1, sizeof(*nfdev));
>> + if (!nfdev) {
>> + rc = -ENOMEM;
>> + goto err_out;
>> + }
>> +
>> + nfdev->xendev = xendev;
>> + nfdev->mtu = ETH_PKT_PAYLOAD_LEN;
> Adapt it to make it compatible with latest macro.
>> +
>> + /* Xenbus initialization */
>> + rc = netfront_xb_init(nfdev, drv_allocator);
>> + if (rc) {
>> + uk_pr_err("Error initializing Xenbus data: %d\n", rc);
>> + goto err_xenbus;
>> + }
>> +
>> + /* register netdev */
>> + nfdev->netdev.ops = &netfront_ops;
>> + rc = uk_netdev_drv_register(&nfdev->netdev, drv_allocator,
>> DRIVER_NAME);
>> + if (rc < 0) {
>> + uk_pr_err("Failed to register %s device with libuknetdev\n",
>> + DRIVER_NAME);
>> + goto err_register;
>> + }
>> + nfdev->uid = rc;
>> + rc = 0;
>> +
>> +out:
>> return rc;
>> +err_register:
>> + netfront_xb_fini(nfdev, drv_allocator);
>> +err_xenbus:
>> + uk_free(drv_allocator, nfdev);
>> +err_out:
>> + goto out;
>> }
>> static int netfront_drv_init(struct uk_alloc *allocator)
>> diff --git a/plat/xen/drivers/net/netfront.h
>> b/plat/xen/drivers/net/netfront.h
>> new file mode 100644
>> index 00000000..0cc8230b
>> --- /dev/null
>> +++ b/plat/xen/drivers/net/netfront.h
>> @@ -0,0 +1,66 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
>> + * Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2020, University Politehnica of Bucharest. 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 __NETFRONT_H__
>> +#define __NETFRONT_H__
>> +
>> +#include <uk/netdev.h>
>> +
>> +struct xs_econf {
>> + char *ipv4addr;
>> + char *ipv4mask;
>> + char *ipv4gw;
>> +};
>> +
>> +struct netfront_dev {
>> + /* Xenbus device */
>> + struct xenbus_device *xendev;
>> + /* Network device */
>> + struct uk_netdev netdev;
>> +
>> + /* Configuration parameters */
>> + struct xs_econf econf;
>> +
>> + /* The netdevice identifier */
>> + uint16_t uid;
>> + /* The mtu */
>> + uint16_t mtu;
>> + /* The hw address of the netdevice */
>> + struct uk_hwaddr hw_addr;
>> + /* RX promiscuous mode. */
>> + uint8_t promisc : 1;
>> +};
>> +
>> +#endif /* __NETFRONT_H__ */
>> diff --git a/plat/xen/drivers/net/netfront_xb.h
>> b/plat/xen/drivers/net/netfront_xb.h
>> new file mode 100644
>> index 00000000..6332cf71
>> --- /dev/null
>> +++ b/plat/xen/drivers/net/netfront_xb.h
>> @@ -0,0 +1,43 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2020, University Politehnica of Bucharest. 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 __NETFRONT_XB_H__
>> +#define __NETFRONT_XB_H__
>> +
>> +#include "netfront.h"
>> +
>> +int netfront_xb_init(struct netfront_dev *netdev, struct uk_alloc *a);
>> +void netfront_xb_fini(struct netfront_dev *netdev, struct uk_alloc *a);
>> +
>> +#endif /* __NETFRONT_XB_H__ */
>> diff --git a/plat/xen/drivers/net/netfront_xs.c
>> b/plat/xen/drivers/net/netfront_xs.c
>> new file mode 100644
>> index 00000000..48f01b34
>> --- /dev/null
>> +++ b/plat/xen/drivers/net/netfront_xs.c
>> @@ -0,0 +1,205 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
>> + *
>> + * Copyright (c) 2020, University Politehnica of Bucharest. 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 <inttypes.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <uk/errptr.h>
>> +#include <uk/print.h>
>> +#include <uk/assert.h>
>> +#include <xenbus/xs.h>
>> +#include "netfront_xb.h"
>> +
>> +
>> +static int xs_read_backend_id(const char *nodename, domid_t *domid)
>> +{
>> + char path[strlen(nodename) + sizeof("/backend-id")];
>> + int value, rc;
> UK_ASSERT(nodename);
Actually this is too late to assert it here. Given that this is a static
function and we control how we call it, the assertion should actually be
made in netfront_xb_init().
>> +
>> + sprintf(path, "%s/backend-id", nodename);
> snprintf instead of sprintf
Ack.
>> +
>> + rc = xs_read_integer(XBT_NIL, path, &value);
>> + if (!rc)
>> + *domid = (domid_t) value;
>> +
>> + return rc;
>> +}
>> +
>> +static void xs_econf_fini(struct xs_econf *econf,
>> + struct uk_alloc *a)
>> +{
>> + if (econf->ipv4addr) {
>> + uk_free(a, econf->ipv4addr);
>> + econf->ipv4addr = NULL;
>> + }
>> + if (econf->ipv4mask) {
>> + uk_free(a, econf->ipv4mask);
>> + econf->ipv4mask = NULL;
>> + }
>> + if (econf->ipv4gw) {
>> + uk_free(a, econf->ipv4gw);
>> + econf->ipv4gw = NULL;
>> + }
>> +}
>> +
>> +static int xs_econf_init(struct xs_econf *econf, char *ip_str,
>> + struct uk_alloc *a)
>> +{
>> + int rc = -1;
>> + char *ip_addr = NULL, *ip_gw_str = NULL, *ip_mask_str = NULL;
>> +
>> + /* IP */
>> + ip_addr = strtok(ip_str, " ");
>> + if (ip_addr == NULL)
>> + goto out_err;
>> + econf->ipv4addr = uk_malloc(a, strlen(ip_addr) + 1);
>> + if (!econf->ipv4addr) {
>> + uk_pr_err("Error allocating ip config.\n");
>> + goto out_err;
>> + }
>> + memcpy(econf->ipv4addr, ip_addr, strlen(ip_addr) + 1);
>> +
>> + /* Mask */
>> + ip_mask_str = strtok(NULL, " ");
>> + if (ip_mask_str == NULL)
>> + goto out_err;
>> + econf->ipv4mask = uk_malloc(a, strlen(ip_mask_str) + 1);
>> + if (!econf->ipv4mask) {
>> + uk_pr_err("Error allocating ip mask config.\n");
>> + goto out_err;
>> + }
>> + memcpy(econf->ipv4mask, ip_mask_str, strlen(ip_mask_str) + 1);
>> +
>> + /* Gateway */
>> + ip_gw_str = strtok(NULL, " ");
>> + if (ip_gw_str == NULL)
>> + goto out_err;
>> + econf->ipv4gw = uk_malloc(a, strlen(ip_gw_str) + 1);
>> + if (!econf->ipv4gw) {
>> + uk_pr_err("Error allocating ip gateway config.\n");
>> + goto out_err;
>> + }
>> + memcpy(econf->ipv4gw, ip_gw_str, strlen(ip_gw_str) + 1);
>> +
>> + rc = 0;
>> +out:
>> + return rc;
>> +out_err:
>> + xs_econf_fini(econf, a);
>> + goto out;
>> +}
>> +
>> +int netfront_xb_init(struct netfront_dev *nfdev, struct uk_alloc *a)
>> +{
>> + struct xenbus_device *xendev;
>> + char *mac_str, *p, *ip_str;
>> + int rc;
>> +
>> + UK_ASSERT(nfdev != NULL);
>> +
>> + xendev = nfdev->xendev;
>> + UK_ASSERT(xendev != NULL);
>> +
>> + rc = xs_read_backend_id(xendev->nodename, &xendev->otherend_id);
>> + if (rc)
>> + goto out;
>> +
>> + /* read backend path */
>> + xendev->otherend = xs_read(XBT_NIL, xendev->nodename, "backend");
>> + if (PTRISERR(xendev->otherend)) {
>> + uk_pr_err("Error reading backend path.\n");
>> + rc = PTR2ERR(xendev->otherend);
>> + xendev->otherend = NULL;
>> + goto out;
>> + }
>> +
>> + /* read MAC address */
>> + mac_str = xs_read(XBT_NIL, xendev->nodename, "mac");
>> + if (PTRISERR(mac_str)) {
>> + uk_pr_err("Error reading MAC address.\n");
>> + rc = PTR2ERR(mac_str);
>> + goto no_conf;
>> + }
>> + uk_pr_info("\tMAC %s\n", mac_str);
>> +
>> + p = mac_str;
>> + for (int i = 0; i < UK_NETDEV_HWADDR_LEN; i++) {
>> + nfdev->hw_addr.addr_bytes[i] = (uint8_t) strtoul(p, &p, 16);
>> + p++;
>> + }
>> + free(mac_str);
>> +
>> + /* read IP address */
>> + ip_str = xs_read(XBT_NIL, xendev->otherend, "ip");
>> + if (PTRISERR(ip_str)) {
>> + uk_pr_err("Error reading IP address.\n");
>> + rc = PTR2ERR(ip_str);
>> + goto no_conf;
>> + }
>> + uk_pr_info("\tIP: %s\n", ip_str);
>> +
>> + rc = xs_econf_init(&nfdev->econf, ip_str, a);
>> + if (rc)
>> + goto no_conf;
>> + free(ip_str);
>> +
>> + /* TODO spit event channels */
>> +
>> + /* TODO netmap */
>> +
>> +out:
>> + return rc;
>> +no_conf:
>> + if (xendev->otherend) {
>> + free(xendev->otherend);
>> + xendev->otherend = NULL;
>> + }
>> + goto out;
>> +}
>> +
>> +void netfront_xb_fini(struct netfront_dev *nfdev, struct uk_alloc *a)
>> +{
>> + struct xenbus_device *xendev;
>> +
>> + UK_ASSERT(nfdev != NULL);
>> +
>> + xendev = nfdev->xendev;
>> + UK_ASSERT(xendev != NULL);
>> +
>> + xs_econf_fini(&nfdev->econf, a);
>> +
>> + if (xendev->otherend) {
>> + free(xendev->otherend);
>> + xendev->otherend = NULL;
>> + }
>> +}
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |