Re: [patch v5, kernel version 3.2.1] Source mode for macvlan interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Le vendredi 27 janvier 2012 à 10:58 +0100, Stefan Gula a écrit :
> From: Stefan Gula <steweg@xxxxxxxxx>
> 

...

> +static struct macvlan_source_list *macvlan_hash_lookup_sources_list(
> +	const struct macvlan_dev *vlan,
> +	const unsigned char *addr)
> +{
> +	struct macvlan_source_list *list;
> +	struct hlist_node *n;
> +	struct hlist_head *h = &vlan->port->vlan_source_hash[addr[5]];
> +
> +	hlist_for_each_entry_rcu(list, n, h, hlist) {

You dont hold rcu_read_lock() here, and dont need to, since you are in
the controler path, rtnl is locked :

	hlist_for_each_entry(...)

> +		if (!compare_ether_addr_64bits(list->addr, addr) &&
> +		    list->vlan == vlan)
> +			return list;
> +	}
> +	return NULL;
> +}
> +
> +static int macvlan_hash_add_sources(struct macvlan_dev *vlan,
> +				    const unsigned char *addr)
> +{
> +	struct macvlan_port *port = vlan->port;
> +	struct macvlan_source_list *list;
> +	struct hlist_head *h;
> +
> +	list = macvlan_hash_lookup_sources_list(vlan, addr);
> +	if (!list) {
> +		list = kmalloc(sizeof(*list), GFP_KERNEL);

By the way, use of GFP_KERNEL here is a proof you dont hold
rcu_read_lock().

(If you were, you would not be allowed to sleep in kmalloc())

> +		if (list) {
> +			memcpy(list->addr, addr, ETH_ALEN);
> +			list->vlan = vlan;
> +			h = &port->vlan_source_hash[addr[5]];
> +			hlist_add_head_rcu(&list->hlist, h);
> +		} else
> +			return -ENOMEM;

	if (cond) {
		foo;
		bar;
	} else {
		blip;
	}


> +	}
> +	return 0;
> +}
> +
> +static void macvlan_hash_del_sources(struct macvlan_source_list *list)
> +{
> +	hlist_del_rcu(&list->hlist);
> +	kfree_rcu(list, rcu);
> +}
> +

...

> +static int macvlan_changelink_sources(struct macvlan_dev *vlan, u32 mode,
> +				      unsigned char *addr)
> +{
> +	if (mode == MACVLAN_MACADDR_ADD)
> +		return macvlan_hash_add_sources(vlan, addr);
> +

Same codestyle here...

	if (cond) {
		foo;
	} else if (cond2) {
	} else {
		blip;
	}


> +	else if (mode == MACVLAN_MACADDR_DEL) {
> +		struct macvlan_source_list *list;
> +
> +		list = macvlan_hash_lookup_sources_list(vlan, addr);
> +		if (list)
> +			macvlan_hash_del_sources(list);
> +	} else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux Kernel Discussion]     [Ethernet Bridging]     [Linux Wireless Networking]     [Linux Bluetooth Networking]     [Linux Networking Users]     [VLAN]     [Git]     [IETF Annouce]     [Linux Assembly]     [Security]     [Bugtraq]     [Photo]     [Singles Social Networking]     [Yosemite Information]     [MIPS Linux]     [ARM Linux Kernel]     [ARM Linux]     [Linux Virtualization]     [Linux Security]     [Linux IDE]     [Linux RAID]     [Linux SCSI]     [Free Dating]

Add to Google Powered by Linux