[libvirt] [PATCH 3/6] util: define virNetDevRxFilter and basic utility functions

[John Ferlan]

On 09/24/2014 05:50 AM, Laine Stump wrote:
> This same structure will be used to retrieve RX filter info for
> interfaces on the host via netlink messages, and RX filter info for
> interfaces on the guest via the qemu "query-rx-filter" command.
> ---
>  src/libvirt_private.syms |  8 +++++++
>  src/util/virnetdev.c     | 40 +++++++++++++++++++++++++++++++++
>  src/util/virnetdev.h     | 57 +++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index bb2b9a3..e5723d2 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1595,6 +1595,14 @@ virNetDevReplaceMacAddress;
>  virNetDevReplaceNetConfig;
>  virNetDevRestoreMacAddress;
>  virNetDevRestoreNetConfig;
> +virNetDevRxFilterFree;
> +virNetDevRxFilterMulticastModeTypeFromString;
> +virNetDevRxFilterMulticastModeTypeToString;
> +virNetDevRxFilterNew;
> +virNetDevRxFilterUnicastModeTypeFromString;
> +virNetDevRxFilterUnicastModeTypeToString;
> +virNetDevRxFilterVlanModeTypeFromString;
> +virNetDevRxFilterVlanModeTypeToString;
>  virNetDevSetIPv4Address;
>  virNetDevSetMAC;
>  virNetDevSetMTU;
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 8815e18..dd1f530 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1932,3 +1932,43 @@ virNetDevGetLinkInfo(const char *ifname,
>      return 0;
>  }
>  #endif /* defined(__linux__) */
> +
> +
> +VIR_ENUM_IMPL(virNetDevRxFilterUnicastMode,
> +              VIR_NETDEV_RX_FILTER_UNICAST_MODE_LAST,
> +              "none",
> +              "normal");
> +
> +VIR_ENUM_IMPL(virNetDevRxFilterMulticastMode,
> +              VIR_NETDEV_RX_FILTER_MULTICAST_MODE_LAST,
> +              "none",
> +              "normal");
> +
> +VIR_ENUM_IMPL(virNetDevRxFilterVlanMode,
> +              VIR_NETDEV_RX_FILTER_VLAN_MODE_LAST,
> +              "none",
> +              "normal");

Is there ever a chance these could be different for one of these types?
Seems "excessive" to make 3 specific definitions when 1 generic one
could suffice (drop the "Unicast, Multicast, Vlan")

> +
> +
> +virNetDevRxFilterPtr
> +virNetDevRxFilterNew(void)
> +{
> +    virNetDevRxFilterPtr filter;
> +
> +    if (VIR_ALLOC(filter) < 0)
> +        return NULL;
> +    return filter;
> +}
> +
> +
> +void
> +virNetDevRxFilterFree(virNetDevRxFilterPtr filter)
> +{
> +    if (filter) {
> +        VIR_FREE(filter->name);
> +        VIR_FREE(filter->unicast.table);
> +        VIR_FREE(filter->multicast.table);
> +        VIR_FREE(filter->vlan.table);

I haven't peeked ahead yet, but are these tables where the allocation is
allocate space for 10 table entries, allocate entries in each array
entry... If so, then the free will need to run through the tables.

Reason why I ask is I see size_t's for each structure indicating to me
it's a array of entries.

> +        VIR_FREE(filter);
> +    }
> +}
> diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
> index 69e365e..307871c 100644
> --- a/src/util/virnetdev.h
> +++ b/src/util/virnetdev.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2007-2013 Red Hat, Inc.
> + * Copyright (C) 2007-2014 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -37,6 +37,57 @@ typedef struct ifreq virIfreq;
>  typedef void virIfreq;
>  # endif
>  
> +typedef enum {
> +   VIR_NETDEV_RX_FILTER_UNICAST_MODE_NONE = 0,
> +   VIR_NETDEV_RX_FILTER_UNICAST_MODE_NORMAL,
> +
> +   VIR_NETDEV_RX_FILTER_UNICAST_MODE_LAST
> +} virNetDevRxFilterUnicastMode;
> +VIR_ENUM_DECL(virNetDevRxFilterUnicastMode)
> +
> +typedef enum {
> +   VIR_NETDEV_RX_FILTER_MULTICAST_MODE_NONE = 0,
> +   VIR_NETDEV_RX_FILTER_MULTICAST_MODE_NORMAL,
> +
> +   VIR_NETDEV_RX_FILTER_MULTICAST_MODE_LAST
> +} virNetDevRxFilterMulticastMode;
> +VIR_ENUM_DECL(virNetDevRxFilterMulticastMode)
> +
> +typedef enum {
> +   VIR_NETDEV_RX_FILTER_VLAN_MODE_NONE = 0,
> +   VIR_NETDEV_RX_FILTER_VLAN_MODE_NORMAL,
> +
> +   VIR_NETDEV_RX_FILTER_VLAN_MODE_LAST
> +} virNetDevRxFilterVlanMode;
> +VIR_ENUM_DECL(virNetDevRxFilterVlanMode)

Same here with respect to generic rather than specific

> +
> +typedef struct _virNetDevRxFilter virNetDevRxFilter;
> +typedef virNetDevRxFilter *virNetDevRxFilterPtr;
> +struct _virNetDevRxFilter {
> +    char *name; /* the alias used by qemu, *not* name used by guest */
> +    virMacAddr mac;
> +    bool promiscuous;
> +    bool broadcastAllowed;
> +
> +    struct {
> +        int mode; /* enum virNetDevRxFilterUnicastMode */
> +        bool overflow;
> +        virMacAddrPtr table;
> +        size_t nTable;
> +    } unicast;
> +    struct {
> +        int mode; /* enum virNetDevRxFilterMulticastMode */
> +        bool overflow;
> +        virMacAddrPtr table;
> +        size_t nTable;
> +    } multicast;
> +    struct {
> +        int mode; /* enum virNetDevRxFilterVlanMode */
> +        unsigned int *table;
> +        size_t nTable;
> +    } vlan;
> +};
> +

Each of the mode's would just use the generic FilterMode and the
comments adjusted accordingly...

soft ACK with changes depending on future patches which I haven't peeked
at yet.

John

>  int virNetDevSetupControl(const char *ifname,
>                            virIfreq *ifr)
>      ATTRIBUTE_RETURN_CHECK;
> @@ -150,4 +201,8 @@ int virNetDevGetLinkInfo(const char *ifname,
>                           virInterfaceLinkPtr lnk)
>      ATTRIBUTE_NONNULL(1);
>  
> +virNetDevRxFilterPtr virNetDevRxFilterNew(void)
> +   ATTRIBUTE_RETURN_CHECK;
> +void virNetDevRxFilterFree(virNetDevRxFilterPtr filter);
> +
>  #endif /* __VIR_NETDEV_H__ */
>