lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 10 Apr 2012 11:03:48 +0300
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	John Fastabend <john.r.fastabend@...el.com>
Cc:	roprabhu@...co.com, stephen.hemminger@...tta.com,
	davem@...emloft.net, hadi@...erus.ca, bhutchings@...arflare.com,
	jeffrey.t.kirsher@...el.com, netdev@...r.kernel.org,
	gregory.v.rose@...el.com, krkumar2@...ibm.com, sri@...ibm.com
Subject: Re: [net-next PATCH v1 2/7] net: addr_list: add exclusive dev_uc_add
 and dev_mc_add

On Mon, Apr 09, 2012 at 03:00:23PM -0700, John Fastabend wrote:
> This adds a dev_uc_add_excl() and dev_mc_add_excl() calls
> similar to the original dev_{uc|mc}_add() except it sets
> the global bit and returns -EEXIST for duplicat entires.
> 
> This is useful for drivers that support SR-IOV, macvlan
> devices and any other devices that need to manage the
> unicast and multicast lists.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@...el.com>
> ---
> 
>  include/linux/netdevice.h |    2 +
>  net/core/dev_addr_lists.c |   97 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 83 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 05822e5..b68a326 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2572,6 +2572,7 @@ extern int dev_addr_init(struct net_device *dev);
>  
>  /* Functions used for unicast addresses handling */
>  extern int dev_uc_add(struct net_device *dev, unsigned char *addr);
> +extern int dev_uc_add_excl(struct net_device *dev, unsigned char *addr);
>  extern int dev_uc_del(struct net_device *dev, unsigned char *addr);
>  extern int dev_uc_sync(struct net_device *to, struct net_device *from);
>  extern void dev_uc_unsync(struct net_device *to, struct net_device *from);
> @@ -2581,6 +2582,7 @@ extern void dev_uc_init(struct net_device *dev);
>  /* Functions used for multicast addresses handling */
>  extern int dev_mc_add(struct net_device *dev, unsigned char *addr);
>  extern int dev_mc_add_global(struct net_device *dev, unsigned char *addr);
> +extern int dev_mc_add_excl(struct net_device *dev, unsigned char *addr);
>  extern int dev_mc_del(struct net_device *dev, unsigned char *addr);
>  extern int dev_mc_del_global(struct net_device *dev, unsigned char *addr);
>  extern int dev_mc_sync(struct net_device *to, struct net_device *from);
> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
> index 29c07fe..5405e28 100644
> --- a/net/core/dev_addr_lists.c
> +++ b/net/core/dev_addr_lists.c
> @@ -21,12 +21,35 @@
>   * General list handling functions
>   */
>  
> +static int __hw_addr_create_ex(struct netdev_hw_addr_list *list,
> +			       unsigned char *addr, int addr_len,
> +			       unsigned char addr_type, bool global)
> +{
> +	struct netdev_hw_addr *ha;
> +	int alloc_size;
> +
> +	alloc_size = sizeof(*ha);
> +	if (alloc_size < L1_CACHE_BYTES)
> +		alloc_size = L1_CACHE_BYTES;
> +	ha = kmalloc(alloc_size, GFP_ATOMIC);
> +	if (!ha)
> +		return -ENOMEM;
> +	memcpy(ha->addr, addr, addr_len);
> +	ha->type = addr_type;
> +	ha->refcount = 1;
> +	ha->global_use = global;
> +	ha->synced = false;
> +	list_add_tail_rcu(&ha->list, &list->list);
> +	list->count++;
> +
> +	return 0;
> +}
> +
>  static int __hw_addr_add_ex(struct netdev_hw_addr_list *list,
>  			    unsigned char *addr, int addr_len,
>  			    unsigned char addr_type, bool global)
>  {
>  	struct netdev_hw_addr *ha;
> -	int alloc_size;
>  
>  	if (addr_len > MAX_ADDR_LEN)
>  		return -EINVAL;
> @@ -46,21 +69,7 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list,
>  		}
>  	}
>  
> -
> -	alloc_size = sizeof(*ha);
> -	if (alloc_size < L1_CACHE_BYTES)
> -		alloc_size = L1_CACHE_BYTES;
> -	ha = kmalloc(alloc_size, GFP_ATOMIC);
> -	if (!ha)
> -		return -ENOMEM;
> -	memcpy(ha->addr, addr, addr_len);
> -	ha->type = addr_type;
> -	ha->refcount = 1;
> -	ha->global_use = global;
> -	ha->synced = false;
> -	list_add_tail_rcu(&ha->list, &list->list);
> -	list->count++;
> -	return 0;
> +	return __hw_addr_create_ex(list, addr, addr_len, addr_type, global);
>  }
>  
>  static int __hw_addr_add(struct netdev_hw_addr_list *list, unsigned char *addr,
> @@ -377,6 +386,34 @@ EXPORT_SYMBOL(dev_addr_del_multiple);
>   */
>  
>  /**
> + *	dev_uc_add_excl - Add a global secondary unicast address
> + *	@dev: device
> + *	@addr: address to add
> + */
> +int dev_uc_add_excl(struct net_device *dev, unsigned char *addr)
> +{
> +	struct netdev_hw_addr *ha;
> +	int err;
> +
> +	netif_addr_lock_bh(dev);
> +	list_for_each_entry(ha, &dev->uc.list, list) {
> +		if (!memcmp(ha->addr, addr, dev->addr_len) &&
> +		    ha->type == NETDEV_HW_ADDR_T_UNICAST) {
> +			err = -EEXIST;
> +			goto out;
> +		}
> +	}
> +	err = __hw_addr_create_ex(&dev->uc, addr, dev->addr_len,
> +				  NETDEV_HW_ADDR_T_UNICAST, true);
> +	if (!err)
> +		__dev_set_rx_mode(dev);
> +out:
> +	netif_addr_unlock_bh(dev);
> +	return err;
> +}
> +EXPORT_SYMBOL(dev_uc_add_excl);
> +
> +/**
>   *	dev_uc_add - Add a secondary unicast address
>   *	@dev: device
>   *	@addr: address to add
> @@ -501,6 +538,34 @@ EXPORT_SYMBOL(dev_uc_init);
>   * Multicast list handling functions
>   */
>  
> +/**
> + *	dev_mc_add_excl - Add a global secondary multicast address
> + *	@dev: device
> + *	@addr: address to add
> + */
> +int dev_mc_add_excl(struct net_device *dev, unsigned char *addr)
> +{
> +	struct netdev_hw_addr *ha;
> +	int err;
> +
> +	netif_addr_lock_bh(dev);
> +	list_for_each_entry(ha, &dev->mc.list, list) {
> +		if (!memcmp(ha->addr, addr, dev->addr_len) &&
> +		    ha->type == NETDEV_HW_ADDR_T_UNICAST) {
> +			err = -EEXIST;
> +			goto out;
> +		}
> +	}
> +	err = __hw_addr_create_ex(&dev->mc, addr, dev->addr_len,
> +				  NETDEV_HW_ADDR_T_UNICAST, true);
> +	if (!err)
> +		__dev_set_rx_mode(dev);
> +out:
> +	netif_addr_unlock_bh(dev);
> +	return err;
> +}
> +EXPORT_SYMBOL(dev_mc_add_excl);
> +

Why is it a mistake to have duplicate multicast addresses?
For example macvlan floods all multicasts so it's fine,
and assuming it didn't, it might still be fine to have to
macvlans get the same multicast, no?

>  static int __dev_mc_add(struct net_device *dev, unsigned char *addr,
>  			bool global)
>  {
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ