| 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
| ||
|
Message-ID: <bdfc02a5-b1fb-e0aa-ed0a-0214132253fa@intel.com>
Date: Mon, 14 Aug 2023 10:51:59 -0700
From: "Linga, Pavan Kumar" <pavan.kumar.linga@...el.com>
To: Simon Horman <horms@...nel.org>, Tony Nguyen <anthony.l.nguyen@...el.com>
CC: <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
<edumazet@...gle.com>, <netdev@...r.kernel.org>, Alan Brady
<alan.brady@...el.com>, <emil.s.tantilov@...el.com>,
<jesse.brandeburg@...el.com>, <sridhar.samudrala@...el.com>,
<shiraz.saleem@...el.com>, <sindhu.devale@...el.com>, <willemb@...gle.com>,
<decot@...gle.com>, <andrew@...n.ch>, <leon@...nel.org>, <mst@...hat.com>,
<simon.horman@...igine.com>, <shannon.nelson@....com>,
<stephen@...workplumber.org>, Alice Michael <alice.michael@...el.com>,
"Joshua Hay" <joshua.a.hay@...el.com>, Phani Burra <phani.r.burra@...el.com>
Subject: Re: [PATCH net-next v4 14/15] idpf: add ethtool callbacks
On 8/9/2023 3:33 AM, Simon Horman wrote:
> On Mon, Aug 07, 2023 at 05:34:15PM -0700, Tony Nguyen wrote:
>> From: Alan Brady <alan.brady@...el.com>
>>
>> Initialize all the ethtool ops that are supported by the driver and
>> add the necessary support for the ethtool callbacks. Also add
>> asynchronous link notification virtchnl support where the device
>> Control Plane sends the link status and link speed as an
>> asynchronous event message. Driver report the link speed on
>> ethtool .idpf_get_link_ksettings query.
>>
>> Introduce soft reset function which is used by some of the ethtool
>> callbacks such as .set_channels, .set_ringparam etc. to change the
>> existing queue configuration. It deletes the existing queues by sending
>> delete queues virtchnl message to the CP and calls the 'vport_stop' flow
>> which disables the queues, vport etc. New set of queues are requested to
>> the CP and reconfigure the queue context by calling the 'vport_open'
>> flow. Soft reset flow also adjusts the number of vectors associated to a
>> vport if .set_channels is called.
>>
>> Signed-off-by: Alan Brady <alan.brady@...el.com>
>> Co-developed-by: Alice Michael <alice.michael@...el.com>
>> Signed-off-by: Alice Michael <alice.michael@...el.com>
>> Co-developed-by: Joshua Hay <joshua.a.hay@...el.com>
>> Signed-off-by: Joshua Hay <joshua.a.hay@...el.com>
>> Co-developed-by: Phani Burra <phani.r.burra@...el.com>
>> Signed-off-by: Phani Burra <phani.r.burra@...el.com>
>> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@...el.com>
>> Reviewed-by: Willem de Bruijn <willemb@...gle.com>
>> Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
>> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
>> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
>
> ...
>
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
>
> ...
>
>> +/**
>> + * idpf_get_ethtool_stats - report device statistics
>> + * @netdev: network interface device structure
>> + * @stats: ethtool statistics structure
>> + * @data: pointer to data buffer
>> + *
>> + * All statistics are added to the data buffer as an array of u64.
>> + */
>> +static void idpf_get_ethtool_stats(struct net_device *netdev,
>> + struct ethtool_stats __always_unused *stats,
>> + u64 *data)
>> +{
>> + struct idpf_vport *vport = idpf_netdev_to_vport(netdev);
>> + struct idpf_vport_config *vport_config;
>> + struct page_pool_stats pp_stats = { };
>> + unsigned int total = 0;
>> + unsigned int i, j;
>> + bool is_splitq;
>> + u16 qtype;
>> +
>> + if (!vport || vport->state != __IDPF_VPORT_UP)
>> + return;
>> +
>> + rcu_read_lock();
>> +
>> + idpf_collect_queue_stats(vport);
>> + idpf_add_port_stats(vport, &data);
>> +
>> + for (i = 0; i < vport->num_txq_grp; i++) {
>> + struct idpf_txq_group *txq_grp = &vport->txq_grps[i];
>> +
>> + qtype = VIRTCHNL2_QUEUE_TYPE_TX;
>> +
>> + for (j = 0; j < txq_grp->num_txq; j++, total++) {
>> + struct idpf_queue *txq = txq_grp->txqs[j];
>> +
>> + if (!txq)
>> + idpf_add_empty_queue_stats(&data, qtype);
>> + else
>> + idpf_add_queue_stats(&data, txq);
>> + }
>> + }
>> +
>> + vport_config = vport->adapter->vport_config[vport->idx];
>> + /* It is critical we provide a constant number of stats back to
>> + * userspace regardless of how many queues are actually in use because
>> + * there is no way to inform userspace the size has changed between
>> + * ioctl calls. This will fill in any missing stats with zero.
>> + */
>> + for (; total < vport_config->max_q.max_txq; total++)
>> + idpf_add_empty_queue_stats(&data, VIRTCHNL2_QUEUE_TYPE_TX);
>> + total = 0;
>> +
>> + is_splitq = idpf_is_queue_model_split(vport->rxq_model);
>> +
>> + for (i = 0; i < vport->num_rxq_grp; i++) {
>> + struct idpf_rxq_group *rxq_grp = &vport->rxq_grps[i];
>> + u16 num_rxq;
>> +
>> + qtype = VIRTCHNL2_QUEUE_TYPE_RX;
>> +
>> + if (is_splitq)
>> + num_rxq = rxq_grp->splitq.num_rxq_sets;
>> + else
>> + num_rxq = rxq_grp->singleq.num_rxq;
>> +
>> + for (j = 0; j < num_rxq; j++, total++) {
>> + struct idpf_queue *rxq;
>> +
>> + if (is_splitq)
>> + rxq = &rxq_grp->splitq.rxq_sets[j]->rxq;
>> + else
>> + rxq = rxq_grp->singleq.rxqs[j];
>> + if (!rxq)
>
> Hi Alan, Tony, all,
>
> Here it is assumed that rxq may be NULl...
>
>> + idpf_add_empty_queue_stats(&data, qtype);
>> + else
>> + idpf_add_queue_stats(&data, rxq);
>> +
>> + /* In splitq mode, don't get page pool stats here since
>> + * the pools are attached to the buffer queues
>> + */
>> + if (is_splitq)
>> + continue;
>> +
>> + page_pool_get_stats(rxq->pp, &pp_stats);
>
> ... but here rxq is dereferenced.
>
Thanks for pointing at it. Will fix in the next revision.
> Flagged by Smatch.
>
>> + }
>> + }
>
> ...
>
Regards,
Pavan
Powered by blists - more mailing lists