Re: [UNIKRAFT/LIBLWIP PATCH v3 2/3] lib/lwip: Fetch features supported on the netdev

[Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>]

Hi Sharan,

As discussed off-list, we will do a minor modification while
upstreaming: UK_CTASSERT should be used instead of UK_ASSERT, see
comment below.

otherwise, looks good to me.

Reviewed-by: Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>

On Tue, 2020-07-21 at 17:32 +0200, Sharan Santhanam wrote:
> Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> ---
>  uknetdev.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/uknetdev.c b/uknetdev.c
> index 7047352..fe679e9 100644
> --- a/uknetdev.c
> +++ b/uknetdev.c
> @@ -62,6 +62,10 @@
>  #define UKNETDEV_NETIF_NAME0 'e'
>  #define UKNETDEV_NETIF_NAME1 'n'
>  
> +struct lwip_netdev_data {
> +     uint32_t features;
> +};
> +
>  /*
>   * Global headroom settings for buffer allocations used on receive
>   * and transmit. We are taking the maximum of all uknetdev devices
> as
> @@ -372,6 +376,7 @@ err_t uknetdev_init(struct netif *nf)
>       struct uk_netdev_rxqueue_conf rxq_conf;
>       struct uk_netdev_txqueue_conf txq_conf;
>       struct uk_netdev_info info;
> +     struct lwip_netdev_data *lwip_data;
>       const struct uk_hwaddr *hwaddr;
>       unsigned int i;
>       int ret;
> @@ -379,6 +384,21 @@ err_t uknetdev_init(struct netif *nf)
>       UK_ASSERT(nf);
>       dev = netif_to_uknetdev(nf);
>       UK_ASSERT(dev);
> +#if CONFIG_UK_NETDEV_SCRATCH_SIZE < CONFIG_LWIP_UKNETDEV_SCRATCH 
> +/**
> + * CONFIG_UK_NETDEV_SCRATCH_SIZE is configured as the max of all
> scratch pad
> + * requirements by the Makefile.uk macro uknetdev_scratch_mem. This
> value
> + * should atleast be greater CONFIG_LWIP_UKNETDEV_SCRATCH
> + */
> +#error "Insufficient Scratch memory"
> +#endif
> +
> +     /**
> +      * Make sure the CONFIG_LWIP_UKNETDEV_SCRATCH is still
> sufficient to
> +      * store lwip_data.
> +      */
> +     UK_ASSERT(sizeof(*lwip_data) <=
> CONFIG_LWIP_UKNETDEV_SCRATCH);

this can be done at compile time, we should use UK_CTASSERT.

> +     lwip_data = (struct lwip_netdev_data *)dev->scratch_pad;
>  
>       LWIP_ASSERT("uknetdev needs an input callback (netif_input
> or tcpip_input)",
>                   nf->input != NULL);
> @@ -411,6 +431,7 @@ err_t uknetdev_init(struct netif *nf)
>       uk_netdev_info_get(dev, &info);
>       if (!info.max_rx_queues || !info.max_tx_queues)
>               return ERR_IF;
> +     lwip_data->features = info.features;
>  
>       /*
>        * Update our global (rx|tx)_headroom setting that we use
> for