netdev - Re: [PATCH] bgmac: driver for GBit MAC core on BCMA bus

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]

Date:	Sat, 5 Jan 2013 17:57:15 +0100
From:	Francois Romieu <romieu@...zoreil.com>
To:	Rafał Miłecki <zajec5@...il.com>
Cc:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] bgmac: driver for GBit MAC core on BCMA bus

Rafał Miłecki <zajec5@...il.com> :
[...]
> We really need some good explanation for NETDEV_TX_BUSY somewhere
> (documentation!).

It is already explained in Documentation/networking/driver.txt:
<snip>
1) The ndo_start_xmit method must not return NETDEV_TX_BUSY under
   any normal circumstances.  It is considered a hard error unless
   there is no way your device can tell ahead of time when it's
   transmit function will become busy.

   Instead it must maintain the queue properly.  For example,
   for a driver implementing scatter-gather this means:
</snip>

Please compare your V3:bgmac_dma_tx_add and the sample code after the
aforementioned paragraph.

> I've checked some drivers and:
> 
> 1) b44.c
> In case of:
> > if (unlikely(TX_BUFFS_AVAIL(bp) < 1)) {
> it does netif_stop_queue and returns NETDEV_TX_BUSY

The driver labels it as bug checking and it stops queueing when need
arises at the end of the xmit function.

> 2) sky2.c
> In case of:
> > if (unlikely(tx_avail(sky2) < tx_le_req(skb)))
> it just returns NETDEV_TX_BUSY

Same thing as above. Less documented though.

> 3) jme.2
> In case of:
> > if (unlikely(idx < 0)) {
> it does netif_stop_queue and returns NETDEV_TX_BUSY

The driver labels it as bug checking. It stops queueing when need
arises at the end of the xmit function (see jme_stop_queue_if_full).

> 4) de2104x.c
> In case of:
> > if (tx_free == 0) {
> it does netif_stop_queue and returns NETDEV_TX_BUSY

It stops queueing at the end of the start_xmit handler as well.

> So there are many drivers doing the same mistake I did.

After a packet has been handed to hardware, each of these drivers
checks if queueing should be disabled before returning from start_xmit.

V3 bgmac doesn't.

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html