netdev - Re: [RFC PATCH net-next v2 2/9] bnxt: implement queue api

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]

Message-ID: <4ea91657-2361-406d-923d-c55da89a3ff4@davidwei.uk>
Date: Sun, 5 May 2024 17:36:45 -0700
From: David Wei <dw@...idwei.uk>
To: Mina Almasry <almasrymina@...gle.com>
Cc: netdev@...r.kernel.org, Michael Chan <michael.chan@...adcom.com>,
 Pavan Chebbi <pavan.chebbi@...adcom.com>,
 Andy Gospodarek <andrew.gospodarek@...adcom.com>,
 Adrian Alvarado <adrian.alvarado@...adcom.com>,
 Shailend Chand <shailend@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>
Subject: Re: [RFC PATCH net-next v2 2/9] bnxt: implement queue api

On 2024-05-02 10:23, Mina Almasry wrote:
> On Wed, May 1, 2024 at 9:54 PM David Wei <dw@...idwei.uk> wrote:

...

>> +
>> +static int bnxt_queue_stop(struct net_device *dev, int idx, void **out_qmem)
>> +{
>> +       struct bnxt *bp = netdev_priv(dev);
>> +       struct bnxt_rx_ring_info *rxr;
>> +       struct bnxt_cp_ring_info *cpr;
>> +       int rc;
>> +
>> +       rc = bnxt_hwrm_rx_ring_reset(bp, idx);
>> +       if (rc)
>> +               return rc;
>> +
>> +       bnxt_free_one_rx_ring_skbs(bp, idx);
>> +       rxr = &bp->rx_ring[idx];
>> +       rxr->rx_prod = 0;
>> +       rxr->rx_agg_prod = 0;
>> +       rxr->rx_sw_agg_prod = 0;
>> +       rxr->rx_next_cons = 0;
>> +
>> +       cpr = &rxr->bnapi->cp_ring;
>> +       cpr->sw_stats.rx.rx_resets++;
>> +
>> +       *out_qmem = rxr;
> 
> Oh, I'm not sure you can do this, no?
> 
> rxr is a stack variable, it goes away after the function returns, no?

Not for bnxt, where rx_ring is a dynamically allocated array.

In later patches I will change how the ndos are implemented. Next series
I'll squash these intermediate patches that are now useless.

> 
> You have to kzalloc sizeof(struct bnext_rx_ring_info), no?
> 
> Other than that, the code looks very similar to what we do for GVE,
> and good to me.

Thanks.

> 
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops = {
>> +       .ndo_queue_mem_alloc    = bnxt_queue_mem_alloc,
>> +       .ndo_queue_mem_free     = bnxt_queue_mem_free,
>> +       .ndo_queue_start        = bnxt_queue_start,
>> +       .ndo_queue_stop         = bnxt_queue_stop,
>> +};
>> +
>>  static void bnxt_remove_one(struct pci_dev *pdev)
>>  {
>>         struct net_device *dev = pci_get_drvdata(pdev);
>> @@ -15275,6 +15336,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>         dev->stat_ops = &bnxt_stat_ops;
>>         dev->watchdog_timeo = BNXT_TX_TIMEOUT;
>>         dev->ethtool_ops = &bnxt_ethtool_ops;
>> +       dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops;
>>         pci_set_drvdata(pdev, dev);
>>
>>         rc = bnxt_alloc_hwrm_resources(bp);
>> --
>> 2.43.0
>>
> 
>