netdev - RE: [RFC v2 1/1] net: Add support to configure SR-IOV VF minimum and maximum Tx rate through ip tool.

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: <290BEC245535C3449143D5D84900EEF78CDFF12B@avmb3.qlogic.org>
Date:	Wed, 26 Mar 2014 07:25:29 +0000
From:	Sucheta Chakraborty <sucheta.chakraborty@...gic.com>
To:	Ben Hutchings <ben@...adent.org.uk>
CC:	netdev <netdev@...r.kernel.org>,
	Dept-HSG Linux NIC Dev <Dept-HSGLinuxNICDev@...gic.com>,
	"gregory.v.rose@...el.com" <gregory.v.rose@...el.com>,
	"linux-net-drivers@...arflare.com" <linux-net-drivers@...arflare.com>,
	Ariel Elior <Ariel.Elior@...gic.com>,
	"amirv@...lanox.com" <amirv@...lanox.com>,
	"mkubecek@...e.cz" <mkubecek@...e.cz>
Subject: RE: [RFC v2 1/1] net: Add support to configure SR-IOV VF minimum
 and maximum Tx rate through ip tool.

> -----Original Message-----
> From: Ben Hutchings [mailto:ben@...adent.org.uk]
> Sent: Tuesday, March 25, 2014 12:25 AM
> To: Sucheta Chakraborty
> Cc: netdev; Dept-HSG Linux NIC Dev; gregory.v.rose@...el.com; linux-
> net-drivers@...arflare.com; Ariel Elior; amirv@...lanox.com;
> mkubecek@...e.cz
> Subject: Re: [RFC v2 1/1] net: Add support to configure SR-IOV VF
> minimum and maximum Tx rate through ip tool.
> 
> On Mon, 2014-03-24 at 00:57 -0400, Sucheta Chakraborty wrote:
> [...]
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> [...]
> > +		case IFLA_VF_RATE: {
> > +			struct ifla_vf_rate *ivt;
> > +			struct ifla_vf_info ivf;
> >  			ivt = nla_data(vf);
> >  			err = -EOPNOTSUPP;
> > -			if (ops->ndo_set_vf_tx_rate)
> > -				err = ops->ndo_set_vf_tx_rate(dev, ivt->vf,
> > -							      ivt->rate);
> > +			if ((ivt->min_tx_rate == -1 ||
> > +			     ivt->max_tx_rate == -1) &&
> > +			    ops->ndo_get_vf_config)
> > +				err = ops->ndo_get_vf_config(dev, ivt->vf,
> > +							     &ivf);
> > +			else
> > +				err = 0;
> > +			if (err)
> > +				break;
> > +			if (ivt->min_tx_rate == -1)
> > +				ivt->min_tx_rate = ivf.min_tx_rate;
> > +			if (ivt->max_tx_rate == -1)
> > +				ivt->max_tx_rate = ivf.max_tx_rate;
> [...]
> 
> This is modifying the the content of the netlink skb, which I think is
> not allowed.  I think you need to use local variables for this instead.
> 
> Also, this special-casing of -1 isn't documented anywhere.  Is it even
> necessary?  If userland needs to set just one limit, it can read the
> existing limits and set both.
> 
> Ben.
> 
> --
> Ben Hutchings
> Everything should be made as simple as possible, but not simpler.
>                                                            - Albert
> Einstein

Thanks Ben for your feedback.

I have used (-1)  to differentiate if user does not want to set particular field.
For example: in case if user sets max_tx_rate and does not set min_tx_rate, then, we send max_tx_rate as set by user and min_tx_rate as -1.

Are you suggesting that in above scenario if user does not set min_tx_rate, then user space should get that value from driver and send same value to driver interface?

Also, user may want to set value 0 which should be differentiated from not setting any value.

Thanks,
Sucheta.