netdev - Re: [PATCH 1/1] net: dsa: replacing the hard-coded sized array "dsa

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]
Message-ID: <547E23C1.2040007@gmail.com>
Date:	Tue, 02 Dec 2014 12:40:33 -0800
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Andrey Volkov <andrey.volkov@...vision.fr>, netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] net: dsa: replacing the hard-coded sized array "dsa_switch"
 by dynamic one

On 02/12/14 06:50, Andrey Volkov wrote:
> 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.

Sounds reasonable to me, you probably want to resubmit and trim the
"Hello" form your commit message.

> 
> 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.

> 
> Signed-off-by: Andrey Volkov <andrey.volkov@...vision.fr>                                                                                                                                                                                    
> ---                                                                                                                                                                                                                                          
>  include/net/dsa.h |    3 +--                                                                                                                                                                                                                
>  net/dsa/dsa.c     |    7 +++----                                                                                                                                                                                                            
>  2 files changed, 4 insertions(+), 6 deletions(-)                                                                                                                                                                                            
>                                                                                                                                                                                                                                              
> diff --git a/include/net/dsa.h b/include/net/dsa.h                                                                                                                                                                                           
> index ed3c34b..733db2e 100644                                                                                                                                                                                                                
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -28,7 +28,6 @@ enum dsa_tag_protocol {
>         DSA_TAG_PROTO_BRCM,
>  };
>  
> -#define DSA_MAX_SWITCHES       4
>  #define DSA_MAX_PORTS          12
>  
>  struct 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 {
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 322c778..c081a19 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -604,8 +604,6 @@ static int dsa_of_probe(struct platform_device *pdev)
>         pdev->dev.platform_data = pd;
>         pd->netdev = &ethernet_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;
> 

--
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