Re: [Minios-devel] [UNIKRAFT PATCH v4 08/11] lib/uknetdev: Hardware address interfaces

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Simon,

This patch looks good.

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

On 10/10/2018 02:00 PM, Simon Kuenzer wrote:
> From: Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx>
>
> Add a getter and a setter for a netdev's hardware address (MAC). This
> API interface is optional for a driver to implement. By making this
> optional we could support network devices that operate on L3 in the
> future.
>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Signed-off-by: Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx>
> ---
>   lib/uknetdev/exportsyms.uk            |  2 ++
>   lib/uknetdev/include/uk/netdev.h      | 25 +++++++++++++++++++++++
>   lib/uknetdev/include/uk/netdev_core.h | 22 ++++++++++++++++++++
>   lib/uknetdev/netdev.c                 | 38 +++++++++++++++++++++++++++++++++++
>   4 files changed, 87 insertions(+)
>
> diff --git a/lib/uknetdev/exportsyms.uk b/lib/uknetdev/exportsyms.uk
> index c733d81..8b13499 100644
> --- a/lib/uknetdev/exportsyms.uk
> +++ b/lib/uknetdev/exportsyms.uk
> @@ -22,3 +22,5 @@ uk_netdev_configure
>   uk_netdev_rxq_configure
>   uk_netdev_txq_configure
>   uk_netdev_start
> +uk_netdev_hwaddr_set
> +uk_netdev_hwaddr_get
> diff --git a/lib/uknetdev/include/uk/netdev.h b/lib/uknetdev/include/uk/netdev.h
> index 7223ba7..423e321 100644
> --- a/lib/uknetdev/include/uk/netdev.h
> +++ b/lib/uknetdev/include/uk/netdev.h
> @@ -284,6 +284,31 @@ int uk_netdev_txq_configure(struct uk_netdev *dev, 
> uint16_t queue_id,
>    */
>   int uk_netdev_start(struct uk_netdev *dev);
>   
> +/**
> + * Returns the hardware address of the Unikraft network device.
> + *
> + * @param dev
> + *   The Unikraft Network Device.
> + * @return
> + *   - (NULL): no hardware address available
> + *   - (struct uk_hwaddr *): Reference to hardware address
> + */
> +const struct uk_hwaddr *uk_netdev_hwaddr_get(struct uk_netdev *dev);
> +
> +/**
> + * Set the hardware address.
> + *
> + * @param dev
> + *   The Unikraft Network Device.
> + * @param hwaddr
> + *   Reference to new hardware address.
> + * @return
> + *   - (0): if successful.
> + *   - (-ENOTSUP): if hardware doesn't support.
> + *   - (-EINVAL): if address is invalid.
> + */
> +int uk_netdev_hwaddr_set(struct uk_netdev *dev, const struct uk_hwaddr 
> *hwaddr);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/uknetdev/include/uk/netdev_core.h 
> b/lib/uknetdev/include/uk/netdev_core.h
> index 02c5ba0..cca6d13 100644
> --- a/lib/uknetdev/include/uk/netdev_core.h
> +++ b/lib/uknetdev/include/uk/netdev_core.h
> @@ -69,6 +69,15 @@ struct uk_netdev;
>   UK_TAILQ_HEAD(uk_netdev_list, struct uk_netdev);
>   
>   /**
> + * A structure used for Ethernet hardware addresses
> + */
> +#define UK_NETDEV_HWADDR_LEN 6 /**< Length of Ethernet address. */
> +
> +struct uk_hwaddr {
> +       uint8_t addr_bytes[UK_NETDEV_HWADDR_LEN];
> +} __packed;
> +
> +/**
>    * A structure used to describe network device capabilities.
>    */
>   struct uk_netdev_info {
> @@ -224,10 +233,23 @@ typedef struct uk_netdev_rx_queue * 
> (*uk_netdev_rxq_configure_t)(
>   /** Driver callback type to start a configured Unikraft network device. */
>   typedef int  (*uk_netdev_start_t)(struct uk_netdev *dev);
>   
> +/** Driver callback type to get the hardware address. */
> +typedef const struct uk_hwaddr *(*uk_netdev_hwaddr_get_t)(
> +       struct uk_netdev *dev);
> +
> +/** Driver callback type to set the hardware address. */
> +typedef int (*uk_netdev_hwaddr_set_t)(struct uk_netdev *dev,
> +                                     const struct uk_hwaddr *hwaddr);
> +
> +
>   /**
>    * A structure containing the functions exported by a driver.
>    */
>   struct uk_netdev_ops {
> +       /** Set/Get hardware address. */
> +       uk_netdev_hwaddr_get_t          hwaddr_get;       /* recommended */
> +       uk_netdev_hwaddr_set_t          hwaddr_set;       /* optional */
> +
>         /** Device/driver capabilities and info. */
>         uk_netdev_info_get_t            info_get;
>         uk_netdev_txq_info_get_t        txq_info_get;
> diff --git a/lib/uknetdev/netdev.c b/lib/uknetdev/netdev.c
> index 3eea5ed..3fb521a 100644
> --- a/lib/uknetdev/netdev.c
> +++ b/lib/uknetdev/netdev.c
> @@ -411,3 +411,41 @@ int uk_netdev_start(struct uk_netdev *dev)
>         }
>         return ret;
>   }
> +
> +int uk_netdev_hwaddr_set(struct uk_netdev *dev,
> +                        const struct uk_hwaddr *hwaddr)
> +{
> +       UK_ASSERT(dev);
> +       UK_ASSERT(dev->_data);
> +       UK_ASSERT(dev->ops);
> +       UK_ASSERT(hwaddr);
> +
> +       /* We do support changing of hwaddr
> +        * only when device was configured
> +        */
> +       UK_ASSERT(dev->_data->state == UK_NETDEV_CONFIGURED
> +                 || dev->_data->state == UK_NETDEV_RUNNING);
> +
> +       if (dev->ops->hwaddr_set == NULL)
> +               return -ENOTSUP;
> +
> +       return dev->ops->hwaddr_set(dev, hwaddr);
> +}
> +
> +const struct uk_hwaddr *uk_netdev_hwaddr_get(struct uk_netdev *dev)
> +{
> +       UK_ASSERT(dev);
> +       UK_ASSERT(dev->_data);
> +       UK_ASSERT(dev->ops);
> +
> +       /* We do support retrieving of hwaddr
> +        * only when device was configured
> +        */
> +       UK_ASSERT(dev->_data->state == UK_NETDEV_CONFIGURED
> +                 || dev->_data->state == UK_NETDEV_RUNNING);
> +
> +       if (!dev->ops->hwaddr_get)
> +               return NULL;
> +
> +       return dev->ops->hwaddr_get(dev);
> +}
Thanks & Regards
Sharan

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel