netdev - Re: [PATCH 1/6] mac802154: Immediately retry sending failed packets

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

Message-ID: <515B3F78.2020301@signal11.us>
Date:	Tue, 02 Apr 2013 16:28:40 -0400
From:	Alan Ott <alan@...nal11.us>
To:	Alexander Smirnov <alex.bluesman.smirnov@...il.com>
CC:	Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	linux-zigbee-devel <linux-zigbee-devel@...ts.sourceforge.net>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] mac802154: Immediately retry sending failed packets

On 04/02/2013 03:11 PM, Alexander Smirnov wrote:
> 2013/4/2 Alan Ott <alan@...nal11.us <mailto:alan@...nal11.us>>
> 
>     When ops->xmit() fails, immediately retry. Previously the packet was
>     sent
>     to the back of the workqueue.
> 
>     Signed-off-by: Alan Ott <alan@...nal11.us <mailto:alan@...nal11.us>>
>     ---
>      net/mac802154/tx.c | 17 ++++++++---------
>      1 file changed, 8 insertions(+), 9 deletions(-)
> 
>     diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
>     index 4e09d07..fbf937c 100644
>     --- a/net/mac802154/tx.c
>     +++ b/net/mac802154/tx.c
>     @@ -59,19 +59,18 @@ static void mac802154_xmit_worker(struct
>     work_struct *work)
>                     }
>             }
> 
>     -       res = xw->priv->ops->xmit(&xw->priv->hw, xw->skb);
>     +       do {
>     +               res = xw->priv->ops->xmit(&xw->priv->hw, xw->skb);
>     +               if (res && ++xw->xmit_attempts >=
>     MAC802154_MAX_XMIT_ATTEMPTS) {
>     +                       pr_debug("transmission failed for %d times",
>     +                                MAC802154_MAX_XMIT_ATTEMPTS);
>     +                       break;
>     +               }
>     +       } while (res);
> 
>  
> 
> IIRC this 802.15.4 stack uses single-thread-work-queue and all RX/TX
> are performed by using it.

Hi Alexander,

Yes, that is true. As is currently implemented, the driver xmit()
functions are called from a workqueue and block until the packet is sent.

> Doing TX retry in the way you proposed -
> it's possible that you will block other packets pending in this
> queue.

Yes. Since sending data is a blocking operation, any time spent sending
(or re-sending) is blocking.

As it was before this patch series, with the buffer (workqueue) growing
arbitrarily large, doing retry by putting a packet at the end of the
workqueue was largely useless because by the time it came to retry it,
any state associated with it (with respect to fragmentation/reassembly)
had expired.

Keep in mind that with the netif stop/wake code, putting retries at the
end of the workqueue or doing them immediately is basically the same
thing, since the workqueue is no longer the packet queue (and will
ideally only have 0 or 1 packets in it). The workqueue (with these
patches) only serves to lift the driver xmit() calls out of atomic
context, allowing them to block.

However, it is easy to envision one process clogging up the works with
retries by sending many packets to an unavailable address.

What do you recommend doing here instead?

> Despite on Linux is already 'slow' system to provide
> real-time for specific 802.15.4 features, I think it's not a good
> idea to increase nodes communication latency.

With the transmit buffer length increased (and actually being used),
maybe the packets with realtime requirements can be given a higher
priority to deal with these requirements.

Alan.

> 
> 
>      out:
>             mutex_unlock(&xw->priv->phy->pib_lock);
> 
>     -       if (res) {
>     -               if (xw->xmit_attempts++ < MAC802154_MAX_XMIT_ATTEMPTS) {
>     -                       queue_work(xw->priv->dev_workqueue, &xw->work);
>     -                       return;
>     -               } else
>     -                       pr_debug("transmission failed for %d times",
>     -                                MAC802154_MAX_XMIT_ATTEMPTS);
>     -       }
> 
>             dev_kfree_skb(xw->skb);
> 
>     --
>     1.7.11.2
> 
> 

--
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