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.
