netdev - Re: [PATCH net-next 2/7] net: dsa: Add devlink regions support to 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: <20200816215009.m7ovmuwjme3lrl4g@skbuf>
Date:   Mon, 17 Aug 2020 00:50:09 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>, Chris Healy <cphealy@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>
Subject: Re: [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA

On Sun, Aug 16, 2020 at 09:43:11PM +0200, Andrew Lunn wrote:
> Allow DSA drivers to make use of devlink regions, via simple wrappers.
> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
>  include/net/dsa.h |  6 ++++++
>  net/dsa/dsa.c     | 16 ++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 63ff6f717307..8963440ec7f8 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -660,6 +660,12 @@ void dsa_devlink_resource_occ_get_register(struct dsa_switch *ds,
>  					   void *occ_get_priv);
>  void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
>  					     u64 resource_id);
> +struct devlink_region *
> +dsa_devlink_region_create(struct dsa_switch *ds,
> +			  const struct devlink_region_ops *ops,
> +			  u32 region_max_snapshots, u64 region_size);
> +void dsa_devlink_region_destroy(struct devlink_region *region);
> +
>  struct dsa_port *dsa_port_from_netdev(struct net_device *netdev);
>  
>  struct dsa_devlink_priv {
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 86351da4e202..fea2efe5fe68 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -412,6 +412,22 @@ void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
>  }
>  EXPORT_SYMBOL_GPL(dsa_devlink_resource_occ_get_unregister);
>  
> +struct devlink_region *
> +dsa_devlink_region_create(struct dsa_switch *ds,
> +			  const struct devlink_region_ops *ops,
> +			  u32 region_max_snapshots, u64 region_size)
> +{
> +	return devlink_region_create(ds->devlink, ops, region_max_snapshots,
> +				     region_size);
> +}
> +EXPORT_SYMBOL_GPL(dsa_devlink_region_create);
> +
> +void dsa_devlink_region_destroy(struct devlink_region *region)
> +{
> +	devlink_region_destroy(region);
> +}
> +EXPORT_SYMBOL_GPL(dsa_devlink_region_destroy);
> +
>  struct dsa_port *dsa_port_from_netdev(struct net_device *netdev)
>  {
>  	if (!netdev || !dsa_slave_dev_check(netdev))
> -- 
> 2.28.0
> 

Could we perhaps open-code these from the drivers themselves? There's
hardly any added value in DSA providing a "helper" for creation of
devlink resources (regions, shared buffers, etc).

Take the ocelot/felix driver for example. It is a DSA driver whose core
functionality is provided by drivers/net/ethernet/mscc/ocelot*.c, which
is non-DSA code. If it were to implement devlink regions, presumably
that code would live in drivers/net/ethernet/mscc/ and not in
drivers/net/dsa/.

Thanks,
-Vladimir