netdev - RE: [net-next v4 13/15] iecm: Add ethtool

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 23 Jul 2020 00:43:06 +0000
From:   "Brady, Alan" <alan.brady@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "Michael, Alice" <alice.michael@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
        "Burra, Phani R" <phani.r.burra@...el.com>,
        "Hay, Joshua A" <joshua.a.hay@...el.com>,
        "Chittim, Madhu" <madhu.chittim@...el.com>,
        "Linga, Pavan Kumar" <pavan.kumar.linga@...el.com>,
        "Skidmore, Donald C" <donald.c.skidmore@...el.com>,
        "Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
        "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
Subject: RE: [net-next v4 13/15] iecm: Add ethtool

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Tuesday, July 21, 2020 11:28 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>
> Cc: davem@...emloft.net; Michael, Alice <alice.michael@...el.com>;
> netdev@...r.kernel.org; nhorman@...hat.com; sassmann@...hat.com;
> Kirsher, Jeffrey T <jeffrey.t.kirsher@...el.com>; Brady, Alan
> <alan.brady@...el.com>; Burra, Phani R <phani.r.burra@...el.com>; Hay,
> Joshua A <joshua.a.hay@...el.com>; Chittim, Madhu
> <madhu.chittim@...el.com>; Linga, Pavan Kumar
> <pavan.kumar.linga@...el.com>; Skidmore, Donald C
> <donald.c.skidmore@...el.com>; Brandeburg, Jesse
> <jesse.brandeburg@...el.com>; Samudrala, Sridhar
> <sridhar.samudrala@...el.com>
> Subject: Re: [net-next v4 13/15] iecm: Add ethtool
> 
> On Mon, 20 Jul 2020 17:38:08 -0700 Tony Nguyen wrote:
> > +/**
> > + * iecm_set_rxfh - set the Rx flow hash indirection table
> > + * @netdev: network interface device structure
> > + * @indir: indirection table
> > + * @key: hash key
> > + * @hfunc: hash function to use
> > + *
> > + * Returns -EINVAL if the table specifies an invalid queue id,
> > +otherwise
> > + * returns 0 after programming the table.
> > + */
> > +static int iecm_set_rxfh(struct net_device *netdev, const u32 *indir,
> > +			 const u8 *key, const u8 hfunc)
> > +{
> > +	struct iecm_vport *vport = iecm_netdev_to_vport(netdev);
> > +	struct iecm_adapter *adapter;
> > +	u16 *qid_list;
> > +	u16 lut;
> > +
> > +	adapter = vport->adapter;
> > +
> > +	if (!iecm_is_cap_ena(adapter, VIRTCHNL_CAP_RSS)) {
> > +		dev_info(&adapter->pdev->dev, "RSS is not supported on this
> device\n");
> > +		return 0;
> > +	}
> > +	if (adapter->state != __IECM_UP)
> > +		return 0;
> > +
> > +	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc !=
> ETH_RSS_HASH_TOP)
> > +		return -EOPNOTSUPP;
> > +
> > +	if (key)
> > +		memcpy(adapter->rss_data.rss_key, key,
> > +		       adapter->rss_data.rss_key_size);
> > +
> > +	qid_list = kcalloc(vport->num_rxq, sizeof(u16), GFP_KERNEL);
> > +	if (!qid_list)
> > +		return -ENOMEM;
> > +
> > +	iecm_get_rx_qid_list(vport, qid_list);
> > +
> > +	if (indir) {
> > +		for (lut = 0; lut < adapter->rss_data.rss_lut_size; lut++) {
> > +			int index = indir[lut];
> > +
> > +			if (index >= vport->num_rxq) {
> > +				kfree(qid_list);
> > +				return -EINVAL;
> > +			}
> 
> Core checks this already.

Will fix.

> 
> > +			adapter->rss_data.rss_lut[lut] = qid_list[index];
> > +		}
> > +	} else {
> > +		iecm_fill_dflt_rss_lut(vport, qid_list);
> 
> indir == NULL means no change, not reset.
> 

Yes this is wrong, will fix.

> > +	}
> > +
> > +	kfree(qid_list);
> > +
> > +	return iecm_config_rss(vport);
> > +}
> > +
> > +/**
> > + * iecm_get_channels: get the number of channels supported by the
> > +device
> > + * @netdev: network interface device structure
> > + * @ch: channel information structure
> > + *
> > + * Report maximum of TX and RX. Report one extra channel to match our
> > +mailbox
> > + * Queue.
> > + */
> > +static void iecm_get_channels(struct net_device *netdev,
> > +			      struct ethtool_channels *ch) {
> > +	struct iecm_vport *vport = iecm_netdev_to_vport(netdev);
> > +
> > +	/* Report maximum channels */
> > +	ch->max_combined = IECM_MAX_Q;
> > +
> > +	ch->max_other = IECM_MAX_NONQ;
> > +	ch->other_count = IECM_MAX_NONQ;
> > +
> > +	ch->combined_count = max(vport->num_txq, vport->num_rxq);
> 
> If you allow different counts of rxq and txq - the calculation is
> 
> combined = min(rxq, txq)
> rx = rxq - combined
> tx = txq - combined
> 
> not very intuitive, but that's my interpretation of the API.
> 
> Can rxq != txq?
> 

Ultimately yes.  We're still missing some of the finer details required to manage buffers in this new split queue model so we're not quite ready to enable that in set_channels, but yes I we should certainly fix the reporting in get_channels to be accurate.  Will fix.

> > +}
> 
> > +static void iecm_get_drvinfo(struct net_device *netdev,
> > +			     struct ethtool_drvinfo *drvinfo) {
> > +	struct iecm_adapter *adapter = iecm_netdev_to_adapter(netdev);
> > +
> > +	strlcpy(drvinfo->driver, iecm_drv_name, 32);
> > +	strlcpy(drvinfo->fw_version, "N/A", 4);
> 
> I think we agreed to remove this, what happened?
> 

My sincere apologies, not intentional.  This will be gone in the next version.

-Alan