Re: [UNIKRAFT PATCH 4/4] lib/uknetdev: Introduce ioalign

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Thanks for the work

Reviewed-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>

--Sharan

On 10/30/20 2:41 PM, Simon Kuenzer wrote:
> Similar to lib/ukblkdev, we introduce a device info parameter (`struct
> uk_netdev_info`) to query alignment requirements of packet buffer areas
> for a uknetdev device: `ioalign`.
> At the same time the commit clarifies a confusion with the queue
> information parameter `nb_align`. This parameter is intended to describe
> requirements for sizing a rx/tx queue. The comment is updated and the
> `virtio-net` driver is corrected. This fixes commit a52168588.
>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> ---
>   lib/uknetdev/include/uk/netdev_core.h | 5 +++--
>   plat/drivers/virtio/virtio_net.c      | 3 +--
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/uknetdev/include/uk/netdev_core.h 
> b/lib/uknetdev/include/uk/netdev_core.h
> index e7fa97f2..df6ceebf 100644
> --- a/lib/uknetdev/include/uk/netdev_core.h
> +++ b/lib/uknetdev/include/uk/netdev_core.h
> @@ -147,6 +147,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 ioalign;  /**< Alignment in bytes for packet data buffers */
>         uint32_t features; /**< bitmap of the features supported */
>   };
>   
> @@ -156,8 +157,8 @@ struct uk_netdev_info {
>   struct uk_netdev_queue_info {
>         uint16_t nb_max;        /**< Max allowed number of descriptors. */
>         uint16_t nb_min;        /**< Min allowed number of descriptors. */
> -       uint16_t nb_align;      /**< Number should be a multiple of nb_align. */
> -       int nb_is_power_of_two; /**< Number should be a power of two. */
> +       uint16_t nb_align;      /**< Number of descriptors should be aligned. */
> +       int nb_is_power_of_two; /**< Number of descriptors should be a power of 
> two. */
>   };
>   
>   /**
> diff --git a/plat/drivers/virtio/virtio_net.c b/plat/drivers/virtio/virtio_net.c
> index 9930859a..b5979d1e 100644
> --- a/plat/drivers/virtio/virtio_net.c
> +++ b/plat/drivers/virtio/virtio_net.c
> @@ -770,7 +770,6 @@ static int virtio_netdev_rxq_info_get(struct uk_netdev *dev,
>         rxq = &vndev->rxqs[queue_id];
>         qinfo->nb_min = 1;
>         qinfo->nb_max = rxq->max_nb_desc;
> -       qinfo->nb_align = sizeof(void *); /* word size alignment */
>         qinfo->nb_is_power_of_two = 1;
>   
>   exit:
> @@ -798,7 +797,6 @@ static int virtio_netdev_txq_info_get(struct uk_netdev *dev,
>         txq = &vndev->txqs[queue_id];
>         qinfo->nb_min = 1;
>         qinfo->nb_max = txq->max_nb_desc;
> -       qinfo->nb_align = sizeof(void *); /* word size alignment */
>         qinfo->nb_is_power_of_two = 1;
>   
>   exit:
> @@ -1053,6 +1051,7 @@ static void virtio_net_info_get(struct uk_netdev *dev,
>         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->ioalign = sizeof(void *); /* word size alignment */
>         dev_info->features = UK_FEATURE_RXQ_INTR_AVAILABLE;
>   }
>