netdev - Re: [PATCHv4 1/1] net: forcedeth: add xmit

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] [day] [month] [year] [list]

Message-ID: <ad67f447-650d-3d09-b347-bb416bc4a246@oracle.com>
Date:   Wed, 6 Nov 2019 13:50:23 +0800
From:   Zhu Yanjun <yanjun.zhu@...cle.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     rain.1986.08.12@...il.com, davem@...emloft.net,
        netdev@...r.kernel.org
Subject: Re: [PATCHv4 1/1] net: forcedeth: add xmit_more support


On 2019/11/6 12:48, Jakub Kicinski wrote:
> On Wed, 6 Nov 2019 12:47:29 +0800, Zhu Yanjun wrote:
>> On 2019/11/6 1:48, Jakub Kicinski wrote:
>>> On Mon,  4 Nov 2019 23:26:41 -0500, Zhu Yanjun wrote:
>>>> diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
>>>> index 05d2b47..0d21ddd 100644
>>>> --- a/drivers/net/ethernet/nvidia/forcedeth.c
>>>> +++ b/drivers/net/ethernet/nvidia/forcedeth.c
>>>> @@ -2259,7 +2265,12 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>    			u64_stats_update_begin(&np->swstats_tx_syncp);
>>>>    			nv_txrx_stats_inc(stat_tx_dropped);
>>>>    			u64_stats_update_end(&np->swstats_tx_syncp);
>>>> -			return NETDEV_TX_OK;
>>>> +
>>>> +			writel(NVREG_TXRXCTL_KICK | np->txrxctl_bits,
>>>> +			       get_hwbase(dev) + NvRegTxRxControl);
>>>> +			ret = NETDEV_TX_OK;
>>>> +
>>>> +			goto dma_error;
>>> You could goto the middle of the txkick if statement here, instead of
>>> duplicating the writel()?
>> As your suggestion, the change is like this:
>>
>> @@ -2374,7 +2374,9 @@ static netdev_tx_t nv_start_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>>           spin_unlock_irqrestore(&np->lock, flags);
>>
>>    txkick:
>> -       if (netif_queue_stopped(dev) || !netdev_xmit_more()) {
>> +       if (netif_queue_stopped(dev) || !netdev_xmit_more())
>> +dma_error:
>> +       {
>>                   u32 txrxctl_kick = NVREG_TXRXCTL_KICK | np->txrxctl_bits;
>>
>>                   writel(txrxctl_kick, get_hwbase(dev) + NvRegTxRxControl);
>>
>> The opening brace on the first of the line. It conflicts with the following:
>>
>> Documentation/process/coding-style.rst:
>> "
>>     98 3) Placing Braces and Spaces
>>     99 ----------------------------
>>    100
>>    101 The other issue that always comes up in C styling is the placement of
>>    102 braces.  Unlike the indent size, there are few technical reasons to
>>    103 choose one placement strategy over the other, but the preferred
>> way, as
>>    104 shown to us by the prophets Kernighan and Ritchie, is to put the
>> opening
>>    105 brace last on the line, and put the closing brace first, thusly:
>> "
>> So I prefer to the current code style.
>>
>> Thanks for your suggestions.
> 	if (netif_queue_stopped(dev) || !netdev_xmit_more()) {
> 		u32 txrxctl_kick;
>
> txkick:
> 		txrxctl_kick = NVREG_TXRXCTL_KICK | np->txrxctl_bits;
> 		writel(txrxctl_kick, get_hwbase(dev) + NvRegTxRxControl);
> 	}

Thanks a lot. Will send a new patch with the above changes.

Zhu Yanjun

>