net: dsa: replacing the hard-coded sized array "dsa_switch" by dynamic one
Subsystems (3)
PATCH 1/1 net: dsa: replacing the hard-coded sized array "dsa_switch" by dynamic one
Hello,
In time of developing one of our devices (with huge, more then 6, number of onboard switches), I've bumped with this ancient, I hope, restriction in the 'struct dsa_switch_tree' definition. So this simple patch remove this restriction and make dsa_switch_tree more scalable for the "usual" 1-2 switches configuration too.
P.S. I've plans to fix hardcoded number of ports too, but it is not so easy as with number of switches. So if someone have any objections/suggestions I'll happy to discuss them.
---
include/net/dsa.h | 3 +-- net/dsa/dsa.c | 7 +++---- 2 files changed, 4 insertions(+), 6 deletions(-)
include/net/dsa.h +1 -2
@@ -28,7 +28,6 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_BRCM, }; -#define DSA_MAX_SWITCHES 4#define DSA_MAX_PORTS 12struct dsa_chip_data {
@@ -117,7 +116,7 @@ struct dsa_switch_tree {/* * Data for the individual switch chips. */ -struct dsa_switch *ds[DSA_MAX_SWITCHES ]; +struct dsa_switch *ds[]; };struct dsa_switch {
net/dsa/dsa.c +3 -4
@@ -604,8 +604,6 @@ static int dsa_of_probe(struct platform_device *pdev) pdev->dev.platform_data = pd; pd->netdev = ðernet_dev->dev; pd->nr_chips =of_get_child_count (np); -if (pd->nr_chips >DSA_MAX_SWITCHES ) - pd->nr_chips =DSA_MAX_SWITCHES ; pd->chip =kcalloc (pd->nr_chips,sizeof (struct dsa_chip_data ),GFP_KERNEL );
@@ -717,7 +715,7 @@ static int dsa_probe(struct platform_device *pdev) pd = pdev->dev.platform_data; } -if (pd ==NULL || pd->netdev ==NULL ) +if (pd ==NULL || pd->netdev ==NULL || pd->nr_chips ==0 )return -EINVAL ; dev =dev_to_net_device (pd->netdev);
@@ -732,7 +730,8 @@ static int dsa_probe(struct platform_device *pdev) goto out; } - dst =kzalloc (sizeof (*dst),GFP_KERNEL ); + dst =kzalloc (sizeof (*dst) + +sizeof (struct dsa_switch *) * pd->nr_chips,GFP_KERNEL );if (dst ==NULL ) {dev_put (dev); ret = -ENOMEM ;
Quoted 6 lines · Andrey Volkov
Hello,
In time of developing one of our devices (with huge, more then 6, number of onboard switches), I've bumped with this ancient, I hope, restriction in the 'struct
dsa_switch_tree' definition. So this simple patch remove this restriction and makedsa_switch_treemore scalable for the "usual" 1-2 switches configuration too.
Sounds reasonable to me, you probably want to resubmit and trim the "Hello" form your commit message.
Quoted 3 lines
P.S. I've plans to fix hardcoded number of ports too, but it is not so easy as with number of switches. So if someone have any objections/suggestions I'll happy to discuss them.
I think the number of ports in a switch is something that should come from the switch driver, and eventually intersected with what the platform configuration has provided.
The difficulty is in case of sparse port number allocation because you still want to allocate e.g: 6 ports even though Port 0 and 5 are used, I don't think we want to introduce a logical to physical mapping, that would be too error prone.
Quoted 61 lines
Signed-off-by Andrey Volkov <[email protected]>---
include/net/dsa.h | 3 +-- net/dsa/dsa.c | 7 +++---- 2 files changed, 4 insertions(+), 6 deletions(-)include/net/dsa.h +1
-2@@ -28,7 +28,6 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_BRCM, }; -#define DSA_MAX_SWITCHES 4#define DSA_MAX_PORTS 12struct dsa_chip_data {@@ -117,7 +116,7 @@ struct dsa_switch_tree {/* * Data for the individual switch chips. */ -struct dsa_switch *ds[DSA_MAX_SWITCHES ]; +struct dsa_switch *ds[]; };struct dsa_switch {net/dsa/dsa.c +3
-4@@ -604,8 +604,6 @@ static int dsa_of_probe(struct platform_device *pdev) pdev->dev.platform_data = pd; pd->netdev = ðernet_dev->dev; pd->nr_chips =of_get_child_count (np); -if (pd->nr_chips >DSA_MAX_SWITCHES ) - pd->nr_chips =DSA_MAX_SWITCHES ; pd->chip =kcalloc (pd->nr_chips,sizeof (struct dsa_chip_data ),GFP_KERNEL );@@ -717,7 +715,7 @@ static int dsa_probe(struct platform_device *pdev) pd = pdev->dev.platform_data; } -if (pd ==NULL || pd->netdev ==NULL ) +if (pd ==NULL || pd->netdev ==NULL || pd->nr_chips ==0 )return -EINVAL ; dev =dev_to_net_device (pd->netdev);@@ -732,7 +730,8 @@ static int dsa_probe(struct platform_device *pdev) goto out; } - dst =kzalloc (sizeof (*dst),GFP_KERNEL ); + dst =kzalloc (sizeof (*dst) + +sizeof (struct dsa_switch *) * pd->nr_chips,GFP_KERNEL );if (dst ==NULL ) {dev_put (dev); ret = -ENOMEM ;
PATCH v2 1/1 net: dsa: replacing the hard-coded sized array "dsa_switch" by dynamic one
Replacing the hard-coded sized array "dsa_switch" by dynamic one and remove useless DSA_MAX_SWITCHES define.
This patch also make dsa_switch_tree scalable from the "usual" 1-2 up to 10th switches configuration.
--- include/net/dsa.h | 3 +-- net/dsa/dsa.c | 7 +++---- 2 files changed, 4 insertions(+), 6 deletions(-)
include/net/dsa.h +1 -2
@@ -28,7 +28,6 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_BRCM, }; -#define DSA_MAX_SWITCHES 4#define DSA_MAX_PORTS 12struct dsa_chip_data {
@@ -117,7 +116,7 @@ struct dsa_switch_tree {/* * Data for the individual switch chips. */ -struct dsa_switch *ds[DSA_MAX_SWITCHES ]; +struct dsa_switch *ds[]; };struct dsa_switch {
net/dsa/dsa.c +3 -4
@@ -604,8 +604,6 @@ static int dsa_of_probe(struct platform_device *pdev) pdev->dev.platform_data = pd; pd->netdev = ðernet_dev->dev; pd->nr_chips =of_get_child_count (np); -if (pd->nr_chips >DSA_MAX_SWITCHES ) - pd->nr_chips =DSA_MAX_SWITCHES ; pd->chip =kcalloc (pd->nr_chips,sizeof (struct dsa_chip_data ),GFP_KERNEL );
@@ -717,7 +715,7 @@ static int dsa_probe(struct platform_device *pdev) pd = pdev->dev.platform_data; } -if (pd ==NULL || pd->netdev ==NULL ) +if (pd ==NULL || pd->netdev ==NULL || pd->nr_chips ==0 )return -EINVAL ; dev =dev_to_net_device (pd->netdev);
@@ -732,7 +730,8 @@ static int dsa_probe(struct platform_device *pdev) goto out; } - dst =kzalloc (sizeof (*dst),GFP_KERNEL ); + dst =kzalloc (sizeof (*dst) + +sizeof (struct dsa_switch *) * pd->nr_chips,GFP_KERNEL );if (dst ==NULL ) {dev_put (dev); ret = -ENOMEM ;
David Miller
maintainerFrom: Andrey Volkov <[email protected]>
Date: Thu, 04 Dec 2014 16:54:32 +0100
net/dsa/dsa.c static int dsa_probe(struct platform_device *pdev) · line 734
Quoted 7 lines
net/dsa/dsa.c +2
-1@@ -732,7 +730,8 @@ static int dsa_probe(struct platform_device *pdev) goto out; } - dst =kzalloc (sizeof (*dst),GFP_KERNEL ); + dst =kzalloc (sizeof (*dst) + +sizeof (struct dsa_switch *) * pd->nr_chips,GFP_KERNEL );
Your email client has completely corrupted your patch. For one thing it has transformed TAB characters into spaces.
Please read Documentation/email-clients.txt, email the patch to yourself, and then try to successfully apply the patch you receive in that email. If it doesn't work for you, it won't work for us either.
Do not resubmit this patch until your have fully resolved this, thanks.
Quoted 7 lines · Florian Fainelli
Quoted 6 lines · Andrey Volkov
Hello,
In time of developing one of our devices (with huge, more then 6, number of onboard switches), I've bumped with this ancient, I hope, restriction in the 'struct
dsa_switch_tree' definition. So this simple patch remove this restriction and makedsa_switch_treemore scalable for the "usual" 1-2 switches configuration too.
Quoted 8 lines
Sounds reasonable to me, you probably want to resubmit and trim the "Hello" form your commit message.
Quoted 2 lines
P.S. I've plans to fix hardcoded number of ports too, but it is not so easy as with number of switches. So if someone have any objections/suggestions I'll happy to discuss them.I think the number of ports in a switch is something that should come from the switch driver, and eventually intersected with what the platform configuration has provided.
Yes it's exactly what I've in my mind. Also in our project I need to handle currently unsupported case:
when switches combined into the stacks by more than one hardwired links, i.e. some ports must be configured as part of 'hard-coded' trunks.
Quoted 4 lines
The difficulty is in case of sparse port number allocation because you still want to allocate e.g: 6 ports even though Port 0 and 5 are used, I don't think we want to introduce a logical to physical mapping, that would be too error prone.
Agree.
Quoted 65 lines
Quoted 52 lines
Signed-off-by Andrey Volkov <[email protected]>---
include/net/dsa.h | 3 +-- net/dsa/dsa.c | 7 +++---- 2 files changed, 4 insertions(+), 6 deletions(-)include/net/dsa.h +1
-2@@ -28,7 +28,6 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_BRCM, }; -#define DSA_MAX_SWITCHES 4#define DSA_MAX_PORTS 12struct dsa_chip_data {@@ -117,7 +116,7 @@ struct dsa_switch_tree {/* * Data for the individual switch chips. */ -struct dsa_switch *ds[DSA_MAX_SWITCHES ]; +struct dsa_switch *ds[]; };struct dsa_switch {net/dsa/dsa.c +1
-3@@ -604,8 +604,6 @@ static int dsa_of_probe(struct platform_device *pdev) pdev->dev.platform_data = pd; pd->netdev = ðernet_dev->dev; pd->nr_chips =of_get_child_count (np); -if (pd->nr_chips >DSA_MAX_SWITCHES ) - pd->nr_chips =DSA_MAX_SWITCHES ; pd->chip =kcalloc (pd->nr_chips,sizeof (struct dsa_chip_data ),GFP_KERNEL );@@ -717,7 +715,7 @@ static int dsa_probe(struct platform_device *pdev) pd = pdev->dev.platform_data; } -if (pd ==NULL || pd->netdev ==NULL ) +if (pd ==NULL || pd->netdev ==NULL || pd->nr_chips ==0 )return -EINVAL ; dev =dev_to_net_device (pd->netdev);net/dsa/dsa.c static int dsa_probe(struct platform_device *pdev) · line 737
Quoted 9 lines
net/dsa/dsa.c +2
-1@@ -732,7 +730,8 @@ static int dsa_probe(struct platform_device *pdev) goto out; } - dst =kzalloc (sizeof (*dst),GFP_KERNEL ); + dst =kzalloc (sizeof (*dst) + +sizeof (struct dsa_switch *) * pd->nr_chips,GFP_KERNEL );if (dst ==NULL ) {dev_put (dev); ret = -ENOMEM ;--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html