netdev - RE: [RFC 04/20] RDMA/irdma: Add driver framework definitions

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]

Date:   Thu, 26 Sep 2019 19:51:12 +0000
From:   "Saleem, Shiraz" <shiraz.saleem@...el.com>
To:     Leon Romanovsky <leon@...nel.org>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:     "dledford@...hat.com" <dledford@...hat.com>,
        "jgg@...lanox.com" <jgg@...lanox.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "Ismail, Mustafa" <mustafa.ismail@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
Subject: RE: [RFC 04/20] RDMA/irdma: Add driver framework definitions

> Subject: Re: [RFC 04/20] RDMA/irdma: Add driver framework definitions
> 

<...............>

> > +/**
> > + * i40iw_l2param_change - handle qs handles for QoS and MSS change
> > + * @ldev: LAN device information
> > + * @client: client for parameter change
> > + * @params: new parameters from L2
> > + */
> > +static void i40iw_l2param_change(struct i40e_info *ldev,
> > +				 struct i40e_client *client,
> > +				 struct i40e_params *params)
> > +{
> > +	struct irdma_l2params *l2params;
> > +	struct l2params_work *work;
> > +	struct irdma_device *iwdev;
> > +	struct irdma_handler *hdl;
> > +	int i;
> > +
> > +	hdl = irdma_find_handler(ldev->pcidev);
> > +	if (!hdl)
> > +		return;
> > +
> > +	iwdev = (struct irdma_device *)((u8 *)hdl + sizeof(*hdl));
> > +
> > +	if (atomic_read(&iwdev->params_busy))
> > +		return;
> > +	work = kzalloc(sizeof(*work), GFP_KERNEL);
> > +	if (!work)
> > +		return;
> > +
> > +	atomic_inc(&iwdev->params_busy);
> 
> Changing parameters through workqueue and perform locking with atomic_t,
> exciting.
> Please do proper locking scheme and better to avoid workqueue at all.

Hmmm....Yes, this is buggy. Will come up with a better solution.

> 
> <...>
> 
> > +/* client interface functions */
> > +static const struct i40e_client_ops i40e_ops = {
> > +	.open = i40iw_open,
> > +	.close = i40iw_close,
> > +	.l2_param_change = i40iw_l2param_change };
> > +
> > +static struct i40e_client i40iw_client = {
> > +	.name = "irdma",
> > +	.ops = &i40e_ops,
> > +	.version.major = I40E_CLIENT_VERSION_MAJOR,
> > +	.version.minor = I40E_CLIENT_VERSION_MINOR,
> > +	.version.build = I40E_CLIENT_VERSION_BUILD,
> > +	.type = I40E_CLIENT_IWARP,
> > +};
> > +
> > +int i40iw_probe(struct platform_device *pdev) {
> > +	struct i40e_peer_dev_platform_data *pdata =
> > +		dev_get_platdata(&pdev->dev);
> > +	struct i40e_info *ldev;
> > +
> > +	if (!pdata)
> > +		return -EINVAL;
> > +
> > +	ldev = pdata->ldev;
> > +
> > +	if (ldev->version.major != I40E_CLIENT_VERSION_MAJOR ||
> > +	    ldev->version.minor != I40E_CLIENT_VERSION_MINOR) {
> > +		pr_err("version mismatch:\n");
> > +		pr_err("expected major ver %d, caller specified major ver %d\n",
> > +		       I40E_CLIENT_VERSION_MAJOR, ldev->version.major);
> > +		pr_err("expected minor ver %d, caller specified minor ver %d\n",
> > +		       I40E_CLIENT_VERSION_MINOR, ldev->version.minor);
> > +		return -EINVAL;
> > +	}
> 
> This is can't be in upstream code, we don't support out-of-tree modules,
> everything else will have proper versions.
> 

OK.