Re: RFC: mac802154 Packet Queueing and Slave Devices

[Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Sun, 2012-09-09 at 20:43 -0400, Alan Ott wrote:
> Hi,
> 
> Tony and I were recently talking about packet queueing on 802.15.4. What
> currently happens (in net/mac802154/tx.c) is that each tx packet (skb)
> is stuck on a work queue, and the worker function then sends each packet
> to the hardware driver in order.
> 
> The problem with this is that it defeats the netif flow control.

And qdisc ability to better control bufferbloat...

By the way, mac802154_tx() looks buggy :

if (!(priv->phy->channels_supported[page] & (1 << chan))) {
	WARN_ON(1);
	// Here, a kfree_skb(skb) is missing.
	return NETDEV_TX_OK;
}
if (skb_cow_head(skb, priv->hw.extra_tx_headroom)) {
	dev_kfree_skb(skb);  // should be kfree_skb(skb)
	return NETDEV_TX_OK;
}
work = kzalloc(sizeof(struct xmit_work), GFP_ATOMIC);
if (!work)
	return NETDEV_TX_BUSY;

NETDEV_TX_BUSY is going to loop. So if there is really no more memory,
its a deadlock. You should instead kfree_skb(skb) and return
NETDEV_TX_OK.

Also mac802154_wpan_xmit() returns NETDEV_TX_OK without kfree_skb(skb)
here :

if (chan == MAC802154_CHAN_NONE ||
    page >= WPAN_NUM_PAGES ||
    chan >= WPAN_NUM_CHANNELS)
	return NETDEV_TX_OK;




------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/