Re: [Minios-devel] [UNIKRAFT PATCH v3 02/12] lib/uknetdev: Add alignment size for packet buffers allocations

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Costin,

Thank you for the work. Please find the comments inline.

Thanks & Regards
Sharan

On 3/3/20 3:13 PM, Costin Lupu wrote:
> Packet buffers may need different alignments, depending on their driver
> requirements. For Xen netfront driver, packet buffers have to be page aligned
> because they are saved in pages shared between backend and frontend. For KVM
> virtio net driver, word size alignments suffice. Therefore, we need to save the
> alignment size in order to support multiple drivers.
>
> This patch also squeezes a small fix for setting the max_mtu when retrieving
> device information from the virtio net driver.
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> ---
>   lib/uknetdev/include/uk/netdev_core.h | 1 +
>   plat/drivers/virtio/virtio_net.c      | 2 ++
>   2 files changed, 3 insertions(+)
>
> diff --git a/lib/uknetdev/include/uk/netdev_core.h 
> b/lib/uknetdev/include/uk/netdev_core.h
> index f073e101..268d54d7 100644
> --- a/lib/uknetdev/include/uk/netdev_core.h
> +++ b/lib/uknetdev/include/uk/netdev_core.h
> @@ -94,6 +94,7 @@ struct uk_netdev_info {
>         uint16_t max_mtu;   /**< Maximum supported MTU size. */
>         uint16_t nb_encap_tx;  /**< Number of bytes required as headroom for 
> tx. */
>         uint16_t nb_encap_rx;  /**< Number of bytes required as headroom for 
> rx. */
> +       uint16_t align;  /**< Alignment required for data address. */
Why do we need to introduce yet another align field. Each of the tx 
queue and rx queue has a `uk_netdev_queue_info` which have the align 
fields. Since packet buffers are more tied to the rxq and txq why dont 
we use those align field.
>   };
>   
>   /**
> diff --git a/plat/drivers/virtio/virtio_net.c b/plat/drivers/virtio/virtio_net.c
> index efc2cb71..9f1873c5 100644
> --- a/plat/drivers/virtio/virtio_net.c
> +++ b/plat/drivers/virtio/virtio_net.c
> @@ -1048,8 +1048,10 @@ static void virtio_net_info_get(struct uk_netdev *dev,
>   
>   	dev_info->max_rx_queues = vndev->max_vqueue_pairs;
>         dev_info->max_tx_queues = vndev->max_vqueue_pairs;
> +       dev_info->max_mtu = vndev->max_mtu;
>         dev_info->nb_encap_tx = sizeof(struct virtio_net_hdr_padded);
>         dev_info->nb_encap_rx = sizeof(struct virtio_net_hdr_padded);
> +       dev_info->align = sizeof(void *); /* word size alignment */
>   }
>   
>   static int virtio_net_start(struct uk_netdev *n)