Re: [Minios-devel] [UNIKRAFT PATCH 02/15] plat/tap: Bus interface for the tap device

[Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>]

Hi Sharan,


Thank you very much for the patch. Please, see my comments inline.


Best,

Roxana

On 07.10.2019 12:46, Sharan Santhanam wrote:
> The tap driver registers itself with the uk_bus. The uk_bus on
> initialization provide the necessary interface to setup the driver.
> The probe callback provides necessary interface to setup the tap device.
>
> Signed-off-by: Sharan Santhanam<sharan.santhanam@xxxxxxxxx>
> ---
>   plat/drivers/tap/tap.c | 38 ++++++++++++++++++++++++++++++++++++++
>   plat/linuxu/Config.uk  |  1 +
>   2 files changed, 39 insertions(+)
>
> diff --git a/plat/drivers/tap/tap.c b/plat/drivers/tap/tap.c
> index d233c6d..ffd21be 100644
> --- a/plat/drivers/tap/tap.c
> +++ b/plat/drivers/tap/tap.c
> @@ -32,12 +32,16 @@
>    * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>    */
>   #include <errno.h>
> +#include <stdio.h>
> +#include <string.h>
>   #include <uk/alloc.h>
>   #include <uk/arch/types.h>
>   #include <uk/netdev_core.h>
>   #include <uk/netdev_driver.h>
>   #include <uk/netbuf.h>
>   #include <uk/errptr.h>
> +#include <uk/bus.h>
> +
>   
>   #define TAPDEV_MAX_CNT		 7
>   #define TAPDEV_IFNAME_BRS       255
> @@ -54,6 +58,18 @@
>   
>   #define ETH_PKT_PAYLOAD_LEN       1500
>   
> +struct tap_net_drv {
> +       struct uk_alloc *a;
> +       UK_TAILQ_HEAD(tdev_list, struct tap_net_dev) tap_dev_list;
I will introduce this list in the next patch when you introduce `struct 
tap_net_dev`. It is unknown for now.
> +       __u16 tap_dev_cnt;
> +       char **bridge_ifs;
> +};
Could you provide some comments for this members? It would increase the 
visibility.
> +
> +/**
> + * Module level variables
> + */
> +static struct tap_net_drv tap_drv = {0};
> +
>   /**
>    * Module functions
>    */
> @@ -230,3 +246,25 @@ static const struct uk_netdev_ops tap_netdev_ops = {
>         .txq_info_get = tap_netdev_txq_info_get,
>         .rxq_info_get = tap_netdev_rxq_info_get,
>   };
> +/**
> + * Register a tap driver as bus. Currently in Unikraft, the uk_bus interface
> + * provides the necessary to provide callbacks for bring a pseudo device. In 
> the
> + * future we might provide interface to support the pseudo device.
> + */
> +static int tap_drv_probe(void)
> +{
> +       return 0;
> +}
> +
> +static int tap_drv_init(struct uk_alloc *_a)
> +{
> +       tap_drv.a = _a;
> +       UK_TAILQ_INIT(&tap_drv.tap_dev_list);
We should move it in the next patch.
> +       return 0;
> +}
> +
> +static struct uk_bus tap_bus = {
> +       .init = tap_drv_init,
> +       .probe = tap_drv_probe,
> +};
> +UK_BUS_REGISTER(&tap_bus);
> diff --git a/plat/linuxu/Config.uk b/plat/linuxu/Config.uk
> index 896e9ea..d85b6fd 100644
> --- a/plat/linuxu/Config.uk
> +++ b/plat/linuxu/Config.uk
> @@ -22,6 +22,7 @@ if (PLAT_LINUXU)
>         default y if LIBUKNETDEV
>         default n
>         depends on LIBUKNETDEV
> +       select LIBUKBUS
>         help
>                 Enable drivers to support tap device on the linuxu platform. The
>                 driver implements the uknetdev interface and provides an 
> interface
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel