linux-kernel - RE: [EXTERNAL] Re: [net-next PATCH v4 03/10] octeontx2-pf: Create representor netdev

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] [day] [month] [year] [list]

Message-ID: 
 <CH0PR18MB433916E129849A20259D4568CDE22@CH0PR18MB4339.namprd18.prod.outlook.com>
Date: Mon, 13 May 2024 10:38:46 +0000
From: Geethasowjanya Akula <gakula@...vell.com>
To: Simon Horman <horms@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "davem@...emloft.net"
	<davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        Sunil Kovvuri Goutham
	<sgoutham@...vell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
        Hariprasad Kelam <hkelam@...vell.com>
Subject: RE: [EXTERNAL] Re: [net-next PATCH v4 03/10] octeontx2-pf: Create
 representor netdev



> -----Original Message-----
> From: Simon Horman <horms@...nel.org>
> Sent: Saturday, May 11, 2024 7:41 PM
> To: Geethasowjanya Akula <gakula@...vell.com>
> Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org; kuba@...nel.org;
> davem@...emloft.net; pabeni@...hat.com; edumazet@...gle.com; Sunil
> Kovvuri Goutham <sgoutham@...vell.com>; Subbaraya Sundeep Bhatta
> <sbhatta@...vell.com>; Hariprasad Kelam <hkelam@...vell.com>
> Subject: [EXTERNAL] Re: [net-next PATCH v4 03/10] octeontx2-pf: Create
> representor netdev
> 
> ----------------------------------------------------------------------
> On Tue, May 07, 2024 at 10:09:14PM +0530, Geetha sowjanya wrote:
> > Adds initial devlink support to set/get the switchdev mode.
> > Representor netdevs are created for each rvu devices when the switch
> > mode is set to 'switchdev'. These netdevs are be used to control and
> > configure VFs.
> >
> > Signed-off-by: Geetha sowjanya <gakula@...vell.com>
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> > b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> > index 33ebbcb223e1..ff4318f414f8 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> > @@ -28,6 +28,157 @@ MODULE_DESCRIPTION(DRV_STRING);
> > MODULE_LICENSE("GPL");  MODULE_DEVICE_TABLE(pci, rvu_rep_id_table);
> >
> > +static int rvu_rep_napi_init(struct otx2_nic *priv, struct
> > +netlink_ext_ack *extack) {
> > +	struct otx2_cq_poll *cq_poll = NULL;
> > +	struct otx2_qset *qset = &priv->qset;
> > +	struct otx2_hw *hw = &priv->hw;
> > +	int err = 0, qidx, vec;
> > +	char *irq_name;
> 
> Please consider using reverse xmas tree - longest line to shortest - for local
> variable declarations in new Networking code.
Will fix it in next version.
> 
> This tool can be helpful: https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__github.com_ecree-
> 2Dsolarflare_xmastree&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=UiEt_nU
> eYFctu7JVLXVlXDhTmq_EAfooaZEYInfGuEQ&m=Mq4aRWpUpar1zTktQDlrt8Jl6
> 8BXQjkee8RpXExMCFcKMalirC_mVOYEGA0fcuSf&s=Kg1fSyZvUeZAO0TSWVB5
> A1GPqngBfG82Tx1Dz46ZH8c&e=
> 
> ...
> 
> > +int rvu_rep_create(struct otx2_nic *priv, struct netlink_ext_ack
> > +*extack) {
> > +	int rep_cnt = priv->rep_cnt;
> > +	struct net_device *ndev;
> > +	struct rep_dev *rep;
> > +	int rep_id, err;
> > +	u16 pcifunc;
> > +
> > +	priv->reps = devm_kcalloc(priv->dev, rep_cnt, sizeof(struct rep_dev *),
> > +				  GFP_KERNEL);
> > +	if (!priv->reps)
> > +		return -ENOMEM;
> > +
> > +	for (rep_id = 0; rep_id < rep_cnt; rep_id++) {
> > +		ndev = alloc_etherdev(sizeof(*rep));
> > +		if (!ndev) {
> > +			NL_SET_ERR_MSG_FMT_MOD(extack, "PFVF
> representor:%d
> > +					       creation failed", rep_id);
> 
> gcc-13 seems unhappy with a string spanning multiple lines.
> I suggest living with a line longer than 80 columns in this case.
> Maybe:
> 
> 			NL_SET_ERR_MSG_FMT_MOD(extack,
> 					       "PFVF representor:%d creation
> failed",
> 					       rep_id);
> 
> > +			err = -ENOMEM;
> > +			goto exit;
> > +		}
> 
Thanks for the feedback. Will submit next version with the suggested changes.
> ...