linux-kernel - Re: [PATCH V6 net-next 06/11] net: hibmcge: Implement .ndo_start

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: <55fbd21f-ac81-465f-b164-7e5b6c0ca7d9@huawei.com>
Date: Tue, 3 Sep 2024 22:41:57 +0800
From: Jijie Shao <shaojijie@...wei.com>
To: Paolo Abeni <pabeni@...hat.com>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>
CC: <shaojijie@...wei.com>, <shenjian15@...wei.com>,
	<wangpeiyang1@...wei.com>, <liuyonglong@...wei.com>, <chenhao418@...wei.com>,
	<sudongming1@...wei.com>, <xujunsheng@...wei.com>, <shiyongbang@...wei.com>,
	<libaihan@...wei.com>, <andrew@...n.ch>, <jdamato@...tly.com>,
	<horms@...nel.org>, <jonathan.cameron@...wei.com>,
	<shameerali.kolothum.thodi@...wei.com>, <salil.mehta@...wei.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V6 net-next 06/11] net: hibmcge: Implement .ndo_start_xmit
 function


on 2024/9/3 21:31, Paolo Abeni wrote:
> On 8/30/24 14:15, Jijie Shao wrote:
>> +netdev_tx_t hbg_net_start_xmit(struct sk_buff *skb, struct 
>> net_device *net_dev)
>> +{
>> +    struct hbg_ring *ring = netdev_get_tx_ring(net_dev);
>> +    struct hbg_priv *priv = netdev_priv(net_dev);
>> +    /* This smp_load_acquire() pairs with smp_store_release() in
>> +     * hbg_tx_buffer_recycle() called in tx interrupt handle process.
>> +     */
>> +    u32 ntc = smp_load_acquire(&ring->ntc);
>> +    struct hbg_buffer *buffer;
>> +    struct hbg_tx_desc tx_desc;
>> +    u32 ntu = ring->ntu;
>> +
>> +    if (unlikely(!hbg_nic_is_open(priv))) {
>> +        dev_kfree_skb_any(skb);
>> +        return NETDEV_TX_OK;
>> +    }
>> +
>> +    if (unlikely(!skb->len ||
>> +             skb->len > hbg_spec_max_frame_len(priv, HBG_DIR_TX))) {
>> +        dev_kfree_skb_any(skb);
>> +        net_dev->stats.tx_errors++;
>> +        return NETDEV_TX_OK;
>> +    }
>> +
>> +    if (unlikely(hbg_queue_is_full(ntc, ntu, ring) ||
>> +             hbg_fifo_is_full(ring->priv, ring->dir))) {
>> +        netif_stop_queue(net_dev);
>> +        return NETDEV_TX_BUSY;
>> +    }
>> +
>> +    buffer = &ring->queue[ntu];
>> +    buffer->skb = skb;
>> +    buffer->skb_len = skb->len;
>> +    if (unlikely(hbg_dma_map(buffer))) {
>> +        dev_kfree_skb_any(skb);
>> +        return NETDEV_TX_OK;
>> +    }
>> +
>> +    buffer->state = HBG_TX_STATE_START;
>> +    hbg_init_tx_desc(buffer, &tx_desc);
>> +    hbg_hw_set_tx_desc(priv, &tx_desc);
>> +
>> +    /* This smp_store_release() pairs with smp_load_acquire() in
>> +     * hbg_tx_buffer_recycle() called in tx interrupt handle process.
>> +     */
>> +    smp_store_release(&ring->ntu, hbg_queue_next_prt(ntu, ring));
>
> Here you should probably check for netif_txq_maybe_stop()
>
>> +    net_dev->stats.tx_bytes += skb->len;
>> +    net_dev->stats.tx_packets++;
>
> Try to avoid 'dev->stats' usage. Instead you could use per napi stats 
> accounting (no contention).

use dev_sw_netstats_tx_add() or dev_sw_netstats_rx_add() ?

>
> Side note: 'net_dev' is quite an unusual variable name for a network 
> device.

sure, 'dev' is ok.
	
	Thanks,
	Jijie Shao