[PATCH net-2.6.26][VLAN]: Reduce memory consumed by vlan_group-s.

[PATCH net-2.6.26][VLAN]: Reduce memory consumed by vlan_group-s.

[Pavel Emelyanov]

Currently each vlan_groupd contains 8 pointers on arrays with 512 
pointers on struct net_device each :) Such a construction "in many 
cases ... wastes memory".

My proposal is to allow for some of these arrays pointers be NULL,
meaning that there are no devices in it. When a new device is added 
to the vlan_group, the appropriate array is allocated.

The check in vlan_group_get_device's is safe, since the pointer
vg->vlan_devices_arrays[x] can only switch from NULL to not-NULL.
The vlan_group_prealloc_vid() is guarded with rtnl lock and is
also safe.

I've checked (I hope that) all the places, that use these arrays 
and found, that the register_vlan_dev is the only place, that can
put a vlan device on an empty vlan_group.

Rough calculations shows, that after the patch a setup with a 
single vlan dev (or up to 512 vlans with sequential vids) will 
occupy approximately 8 times less memory.

The question I have is - does this patch makes sense, or a totally 
new structures are required to store the vlan_devs?

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 79504b2..edd55af 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -93,7 +93,7 @@ static inline struct net_device *vlan_group_get_device(struct vlan_group *vg,
 {
 	struct net_device **array;
 	array = vg->vlan_devices_arrays[vlan_id / VLAN_GROUP_ARRAY_PART_LEN];
-	return array[vlan_id % VLAN_GROUP_ARRAY_PART_LEN];
+	return array ? array[vlan_id % VLAN_GROUP_ARRAY_PART_LEN] : NULL;
 }
 
 static inline void vlan_group_set_device(struct vlan_group *vg,
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index dbc81b9..4ff856d 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -106,29 +106,35 @@ static void vlan_group_free(struct vlan_group *grp)
 static struct vlan_group *vlan_group_alloc(int ifindex)
 {
 	struct vlan_group *grp;
-	unsigned int size;
-	unsigned int i;
 
 	grp = kzalloc(sizeof(struct vlan_group), GFP_KERNEL);
 	if (!grp)
 		return NULL;
 
-	size = sizeof(struct net_device *) * VLAN_GROUP_ARRAY_PART_LEN;
-
-	for (i = 0; i < VLAN_GROUP_ARRAY_SPLIT_PARTS; i++) {
-		grp->vlan_devices_arrays[i] = kzalloc(size, GFP_KERNEL);
-		if (!grp->vlan_devices_arrays[i])
-			goto err;
-	}
-
 	grp->real_dev_ifindex = ifindex;
 	hlist_add_head_rcu(&grp->hlist,
 			   &vlan_group_hash[vlan_grp_hashfn(ifindex)]);
 	return grp;
+}
 
-err:
-	vlan_group_free(grp);
-	return NULL;
+static int vlan_group_prealloc_vid(struct vlan_group *vg, int vid)
+{
+	struct net_device **array;
+	unsigned int size;
+
+	ASSERT_RTNL();
+
+	array = vg->vlan_devices_arrays[vid / VLAN_GROUP_ARRAY_PART_LEN];
+	if (array != NULL)
+		return 0;
+
+	size = sizeof(struct net_device *) * VLAN_GROUP_ARRAY_PART_LEN;
+	array = kzalloc(size, GFP_KERNEL);
+	if (array == NULL)
+		return -ENOBUFS;
+
+	vg->vlan_devices_arrays[vid / VLAN_GROUP_ARRAY_PART_LEN] = array;
+	return 0;
 }
 
 static void vlan_rcu_free(struct rcu_head *rcu)
@@ -247,6 +253,10 @@ int register_vlan_dev(struct net_device *dev)
 			return -ENOBUFS;
 	}
 
+	err = vlan_group_prealloc_vid(grp, vlan_id);
+	if (err < 0)
+		goto out_free_group;
+
 	err = register_netdevice(dev);
 	if (err < 0)
 		goto out_free_group;

Re: [PATCH net-2.6.26][VLAN]: Reduce memory consumed by vlan_group-s.

[Pavel Emelyanov]

Hi, Patrick!

Did you have time to look at this patch? And besides, Alexey
Dobriyan sent some netns-related patches for netfilter... :)

Thanks,
Pavel

> Currently each vlan_groupd contains 8 pointers on arrays with 512 
> pointers on struct net_device each :) Such a construction "in many 
> cases ... wastes memory".
> 
> My proposal is to allow for some of these arrays pointers be NULL,
> meaning that there are no devices in it. When a new device is added 
> to the vlan_group, the appropriate array is allocated.
> 
> The check in vlan_group_get_device's is safe, since the pointer
> vg->vlan_devices_arrays[x] can only switch from NULL to not-NULL.
> The vlan_group_prealloc_vid() is guarded with rtnl lock and is
> also safe.
> 
> I've checked (I hope that) all the places, that use these arrays 
> and found, that the register_vlan_dev is the only place, that can
> put a vlan device on an empty vlan_group.
> 
> Rough calculations shows, that after the patch a setup with a 
> single vlan dev (or up to 512 vlans with sequential vids) will 
> occupy approximately 8 times less memory.
> 
> The question I have is - does this patch makes sense, or a totally 
> new structures are required to store the vlan_devs?
> 
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 
> ---
> 
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 79504b2..edd55af 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -93,7 +93,7 @@ static inline struct net_device *vlan_group_get_device(struct vlan_group *vg,
>  {
>  	struct net_device **array;
>  	array = vg->vlan_devices_arrays[vlan_id / VLAN_GROUP_ARRAY_PART_LEN];
> -	return array[vlan_id % VLAN_GROUP_ARRAY_PART_LEN];
> +	return array ? array[vlan_id % VLAN_GROUP_ARRAY_PART_LEN] : NULL;
>  }
>  
>  static inline void vlan_group_set_device(struct vlan_group *vg,
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index dbc81b9..4ff856d 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -106,29 +106,35 @@ static void vlan_group_free(struct vlan_group *grp)
>  static struct vlan_group *vlan_group_alloc(int ifindex)
>  {
>  	struct vlan_group *grp;
> -	unsigned int size;
> -	unsigned int i;
>  
>  	grp = kzalloc(sizeof(struct vlan_group), GFP_KERNEL);
>  	if (!grp)
>  		return NULL;
>  
> -	size = sizeof(struct net_device *) * VLAN_GROUP_ARRAY_PART_LEN;
> -
> -	for (i = 0; i < VLAN_GROUP_ARRAY_SPLIT_PARTS; i++) {
> -		grp->vlan_devices_arrays[i] = kzalloc(size, GFP_KERNEL);
> -		if (!grp->vlan_devices_arrays[i])
> -			goto err;
> -	}
> -
>  	grp->real_dev_ifindex = ifindex;
>  	hlist_add_head_rcu(&grp->hlist,
>  			   &vlan_group_hash[vlan_grp_hashfn(ifindex)]);
>  	return grp;
> +}
>  
> -err:
> -	vlan_group_free(grp);
> -	return NULL;
> +static int vlan_group_prealloc_vid(struct vlan_group *vg, int vid)
> +{
> +	struct net_device **array;
> +	unsigned int size;
> +
> +	ASSERT_RTNL();
> +
> +	array = vg->vlan_devices_arrays[vid / VLAN_GROUP_ARRAY_PART_LEN];
> +	if (array != NULL)
> +		return 0;
> +
> +	size = sizeof(struct net_device *) * VLAN_GROUP_ARRAY_PART_LEN;
> +	array = kzalloc(size, GFP_KERNEL);
> +	if (array == NULL)
> +		return -ENOBUFS;
> +
> +	vg->vlan_devices_arrays[vid / VLAN_GROUP_ARRAY_PART_LEN] = array;
> +	return 0;
>  }
>  
>  static void vlan_rcu_free(struct rcu_head *rcu)
> @@ -247,6 +253,10 @@ int register_vlan_dev(struct net_device *dev)
>  			return -ENOBUFS;
>  	}
>  
> +	err = vlan_group_prealloc_vid(grp, vlan_id);
> +	if (err < 0)
> +		goto out_free_group;
> +
>  	err = register_netdevice(dev);
>  	if (err < 0)
>  		goto out_free_group;
> 
> 

Re: [PATCH net-2.6.26][VLAN]: Reduce memory consumed by vlan_group-s.

[Patrick McHardy]

Pavel Emelyanov wrote:
> Currently each vlan_groupd contains 8 pointers on arrays with 512 
> pointers on struct net_device each :) Such a construction "in many 
> cases ... wastes memory".
> 
> My proposal is to allow for some of these arrays pointers be NULL,
> meaning that there are no devices in it. When a new device is added 
> to the vlan_group, the appropriate array is allocated.
> 
> The check in vlan_group_get_device's is safe, since the pointer
> vg->vlan_devices_arrays[x] can only switch from NULL to not-NULL.
> The vlan_group_prealloc_vid() is guarded with rtnl lock and is
> also safe.
> 
> I've checked (I hope that) all the places, that use these arrays 
> and found, that the register_vlan_dev is the only place, that can
> put a vlan device on an empty vlan_group.
> 
> Rough calculations shows, that after the patch a setup with a 
> single vlan dev (or up to 512 vlans with sequential vids) will 
> occupy approximately 8 times less memory.
> 
> The question I have is - does this patch makes sense, or a totally 
> new structures are required to store the vlan_devs?


Its an improvement, so I've applied it, thanks.

Re: [PATCH net-2.6.26][VLAN]: Reduce memory consumed by vlan_group-s.

[Pavel Emelyanov]

Hi, Patrick.

> Its an improvement, so I've applied it, thanks.

I noticed you merged some netfilter patches to net-2.6.26, but what 
are your plans about merging vlan-related patches to it? 

I ask because I have some patches with vlan netnsization, but I can't
just send them to net-2.6.26, since they will produce conflicts with 
this patch and maybe more you have for vlan, which are not at Dave yet.

Thanks,
Pavel

Re: [PATCH net-2.6.26][VLAN]: Reduce memory consumed by vlan_group-s.

[Patrick McHardy]

Pavel Emelyanov wrote:
> Hi, Patrick.
> 
>> Its an improvement, so I've applied it, thanks.
> 
> I noticed you merged some netfilter patches to net-2.6.26, but what 
> are your plans about merging vlan-related patches to it? 
> 
> I ask because I have some patches with vlan netnsization, but I can't
> just send them to net-2.6.26, since they will produce conflicts with 
> this patch and maybe more you have for vlan, which are not at Dave yet.
> 

I have some unfinished patches so I didn't send it yet, but I'll
push just this patch to Dave later today so you can merge your
work.

Re: [PATCH net-2.6.26][VLAN]: Reduce memory consumed by vlan_group-s.

[Pavel Emelyanov]

Patrick McHardy wrote:
> Pavel Emelyanov wrote:
>> Hi, Patrick.
>>
>>> Its an improvement, so I've applied it, thanks.
>> I noticed you merged some netfilter patches to net-2.6.26, but what 
>> are your plans about merging vlan-related patches to it? 
>>
>> I ask because I have some patches with vlan netnsization, but I can't
>> just send them to net-2.6.26, since they will produce conflicts with 
>> this patch and maybe more you have for vlan, which are not at Dave yet.
>>
> 
> I have some unfinished patches so I didn't send it yet, but I'll
> push just this patch to Dave later today so you can merge your
> work.
> 

Oh, this is not in a hurry, even if you decide to delay this patch
this is OK to me (vlan netnsization will get more testing then ;) ).

I only asked to correct my plans about netnsization development.

Thanks,
Pavel