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

Sucheta Chakraborty
2014-03-26T07:25:56Z

> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Tuesday, March 25, 2014 12:25 AM
> To: Sucheta Chakraborty
> Cc: netdev; Dept-HSG Linux NIC Dev; gregory.v.rose@intel.com; linux-
> net-drivers@solarflare.com; Ariel Elior; amirv@mellanox.com;
> mkubecek@suse.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.