Re: [Minios-devel] [UNIKRAFT PATCH v4 09/11] lib/uknetdev: Promiscuous mode interfaces

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 11.10.18 16:58, Sharan Santhanam wrote:
> Hello Simon,
>
> Please find the comments inline.
>
> On 10/10/2018 02:00 PM, Simon Kuenzer wrote:
> > From: Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx>
> >
> > Add interfaces to query, enable, and disable promiscuous mode for a
> > netdev.
> >
> > 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      | 26 ++++++++++++++++++++++++++
> >   lib/uknetdev/include/uk/netdev_core.h |  9 +++++++++
> >   lib/uknetdev/netdev.c                 | 35 
> > +++++++++++++++++++++++++++++++++++
> >   4 files changed, 72 insertions(+)
> >
> > diff --git a/lib/uknetdev/exportsyms.uk b/lib/uknetdev/exportsyms.uk
> > index 8b13499..6e8b06b 100644
> > --- a/lib/uknetdev/exportsyms.uk
> > +++ b/lib/uknetdev/exportsyms.uk
> > @@ -24,3 +24,5 @@ uk_netdev_txq_configure
> >   uk_netdev_start
> >   uk_netdev_hwaddr_set
> >   uk_netdev_hwaddr_get
> > +uk_netdev_promiscuous_get
> > +uk_netdev_promiscuous_set
> > diff --git a/lib/uknetdev/include/uk/netdev.h 
> > b/lib/uknetdev/include/uk/netdev.h
> > index 423e321..b06978a 100644
> > --- a/lib/uknetdev/include/uk/netdev.h
> > +++ b/lib/uknetdev/include/uk/netdev.h
> > @@ -309,6 +309,32 @@ const struct uk_hwaddr 
> > *uk_netdev_hwaddr_get(struct uk_netdev *dev);
> >    */
> >   int uk_netdev_hwaddr_set(struct uk_netdev *dev, const struct 
> > uk_hwaddr *hwaddr);
> > +/**
> > + * Returns if promiscuous mode is enabled for an Unikraft network 
> > device.
> > + *
> > + * @param dev
> > + *   The Unikraft Network Device.
> > + * @return
> > + *   - (1): if promiscuous is enabled
> > + *   - (0): if promiscuous is disabled.
> We should rephrase it as < 0 for error, as it depend on the device driver
Yes

> > + *   - (-1): on error
> > + */
> > +int uk_netdev_promiscuous_get(struct uk_netdev *dev);
> > +
> > +/**
> > + * Enables or disables promiscuous mode for an Unikraft network device.
> > + *
> > + * @param dev
> > + *   The Unikraft Network Device.
> > + * @param mode
> > + *   - (0): disable promiscuous mode
> > + *   - (1): enable promiscuous mode
> > + * @return
> > + *   - (0): if successful.
> > + *   - (-ENOTSUP): if driver doesn't support promiscuous mode.
> > + */
> > +int uk_netdev_promiscuous_set(struct uk_netdev *dev, int mode);
> > +
> >   #ifdef __cplusplus
> >   }
> >   #endif
> > diff --git a/lib/uknetdev/include/uk/netdev_core.h 
> > b/lib/uknetdev/include/uk/netdev_core.h
> > index cca6d13..0171127 100644
> > --- a/lib/uknetdev/include/uk/netdev_core.h
> > +++ b/lib/uknetdev/include/uk/netdev_core.h
> > @@ -241,6 +241,11 @@ typedef const struct uk_hwaddr 
> > *(*uk_netdev_hwaddr_get_t)(
> >   typedef int (*uk_netdev_hwaddr_set_t)(struct uk_netdev *dev,
> >                         const struct uk_hwaddr *hwaddr);
> > +/** Driver callback type to get the current promiscuous mode */
> > +typedef int (*uk_netdev_promiscuous_get_t)(struct uk_netdev *dev);
> > +
> > +/** Driver callback type to enable or disable promiscuous mode */
> > +typedef int (*uk_netdev_promiscuous_set_t)(struct uk_netdev *dev, int 
> > mode);
> >   /**
> >    * A structure containing the functions exported by a driver.
> > @@ -250,6 +255,10 @@ struct uk_netdev_ops {
> >       uk_netdev_hwaddr_get_t          hwaddr_get;       /* recommended */
> >       uk_netdev_hwaddr_set_t          hwaddr_set;       /* optional */
> > +    /** Promiscuous mode. */
> > +    uk_netdev_promiscuous_set_t     promiscuous_set;  /* optional */
> > +    uk_netdev_promiscuous_get_t     promiscuous_get;
> > +
> >       /** 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 3fb521a..0af63ac 100644
> > --- a/lib/uknetdev/netdev.c
> > +++ b/lib/uknetdev/netdev.c
> > @@ -78,6 +78,7 @@ int uk_netdev_drv_register(struct uk_netdev *dev, 
> > struct uk_alloc *a,
> >       UK_ASSERT(dev->ops->txq_info_get);
> >       UK_ASSERT(dev->ops->txq_configure);
> >       UK_ASSERT(dev->ops->start);
> > +    UK_ASSERT(dev->ops->promiscuous_get);
> >       dev->_data = _alloc_data(a, netdev_count,  drv_name);
> >       if (!dev->_data)
> > @@ -449,3 +450,37 @@ const struct uk_hwaddr 
> > *uk_netdev_hwaddr_get(struct uk_netdev *dev)
> >       return dev->ops->hwaddr_get(dev);
> >   }
> > +
> > +int uk_netdev_promiscuous_get(struct uk_netdev *dev)
> > +{
> > +    UK_ASSERT(dev);
> > +    UK_ASSERT(dev->_data);
> > +    UK_ASSERT(dev->ops);
> > +    UK_ASSERT(dev->ops->promiscuous_get);
> > +
> > +    /* We do support retrieving of promiscuous mode
> > +     * only when device was configured
> > +     */
> > +    UK_ASSERT(dev->_data->state == UK_NETDEV_CONFIGURED ||
> > +          dev->_data->state == UK_NETDEV_RUNNING);
> > +
> > +    return dev->ops->promiscuous_get(dev);
> > +}
> > +
> > +int uk_netdev_promiscuous_set(struct uk_netdev *dev, int mode)
> > +{
> > +    UK_ASSERT(dev);
> > +    UK_ASSERT(dev->_data);
> > +    UK_ASSERT(dev->ops);
> > +
> > +    /* We do support setting of promiscuous mode
> > +     * only when device was configured
> > +     */
> > +    UK_ASSERT(dev->_data->state == UK_NETDEV_CONFIGURED
> > +          || dev->_data->state == UK_NETDEV_RUNNING);
> > +
> > +    if (unlikely(!dev->ops->promiscuous_set))
> > +        return -ENOTSUP;
> > +
> > +    return dev->ops->promiscuous_set(dev, mode ? 1 : 0);
> > +}
> Thanks & Regards
> Sharan
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel