netdev - Re: [PATCH net-next 1/7] sfc: decouple TXQ type from label

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: <81283b4d-8176-3db2-dc95-87a37c06b7c0@solarflare.com>
Date:   Fri, 11 Sep 2020 18:36:04 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     <linux-net-drivers@...arflare.com>, <davem@...emloft.net>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/7] sfc: decouple TXQ type from label

On 11/09/2020 16:53, Jakub Kicinski wrote:
> On Thu, 10 Sep 2020 21:31:29 +0100 Edward Cree wrote:
>> diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
>> index 48d91b26f1a2..b0a08d9f4773 100644
>> --- a/drivers/net/ethernet/sfc/tx.c
>> +++ b/drivers/net/ethernet/sfc/tx.c
>> @@ -527,6 +527,12 @@ netdev_tx_t efx_hard_start_xmit(struct sk_buff *skb,
>>  	}
>>  
>>  	tx_queue = efx_get_tx_queue(efx, index, type);
>> +	if (WARN_ON(!tx_queue))
> _ONCE
Good catch.
>> +		/* We don't have a TXQ of the right type.
>> +		 * This should never happen, as we don't advertise offload
>> +		 * features unless we can support them.
>> +		 */
>> +		return NETDEV_TX_BUSY;
> You should probably drop this packet, right? Next time qdisc calls the
> driver it's unlikely to find a queue it needs.
Hmm, the comment at the top of efx_hard_start_xmit() claims that
 "returning anything other than NETDEV_TX_OK will cause the OS to free
 the skb".  Is that not in fact true?
Should I instead do what the error path of __efx_enqueue_skb() does -
 free the skb, kick pending TX, and return NETDEV_TX_OK?  I have to
 admit I've never 100% understood the netdev_tx_t semantics.