Re: [UNIKRAFT/LWIP PATCH v4 2/5] lwipopts.h: Disable Ethernet frame padding

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Simon,

Please find the comment inline.

Thanks & Regards
Sharan

On 10/30/20 7:31 PM, Simon Kuenzer wrote:
> From: Costin Lupu <costin.lupu@xxxxxxxxx>
>
> We set ETH_PAD_SIZE to 0 considering that padding and alignment is
> handled according to each driver capabilities.
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> ---
>   include/arch/cc.h | 4 +++-
>   uknetdev.c        | 6 ++++++
>   2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/include/arch/cc.h b/include/arch/cc.h
> index a1d0c34..b4526e0 100644
> --- a/include/arch/cc.h
> +++ b/include/arch/cc.h
> @@ -51,7 +51,9 @@
>   
>   /* 32 bit checksum calculation */
>   #define LWIP_CHKSUM_ALGORITHM 3
> -#define ETH_PAD_SIZE 2
> +
> +/* Disable padding on Ethernet frames (only some uknetdev driver support it) */
> +#define ETH_PAD_SIZE 0
If we make this driver dependent, shouldn't this be driven by either 
KConfig  hidden option.
>   
>   /* rand */
>   #define LWIP_RAND() uk_swrand_randr()
> diff --git a/uknetdev.c b/uknetdev.c
> index e645ac1..ccb7368 100644
> --- a/uknetdev.c
> +++ b/uknetdev.c
> @@ -259,6 +259,12 @@ static void uknetdev_input(struct uk_netdev *dev,
>   
>   		/* Send packet to lwip */
>   #if ETH_PAD_SIZE
> +               /* If the following assertion fails, the driver most likely does
> +                * not support padded receive. In such a case ETH_PAD_SIZE has
> +                * to be set to 0.
> +                */
> +               UK_ASSERT(uk_netbuf_headroom(nb) >= ETH_PAD_SIZE);
> +
>                 uk_netbuf_header(nb, ETH_PAD_SIZE);
>   #endif /* ETH_PAD_SIZE */
>                 p = lwip_netbuf_to_pbuf(nb);