* [PATCH] sch_sfq: avoid giving spurious NET_XMIT_CN signals
@ 2011-05-23 21:02 Eric Dumazet
2011-05-23 21:36 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: Eric Dumazet @ 2011-05-23 21:02 UTC (permalink / raw)
To: David Miller
Cc: netdev, Patrick McHardy, Jarek Poplawski, Jamal Hadi Salim,
Stephen Hemminger
While chasing a possible net_sched bug, I found that IP fragments have
litle chance to pass a congestioned SFQ qdisc :
- Say SFQ qdisc is full because one flow is non responsive.
- ip_fragment() wants to send two fragments belonging to an idle flow.
- sfq_enqueue() queues first packet, but see queue limit reached :
- sfq_enqueue() drops one packet from 'big consumer', and returns
NET_XMIT_CN.
- ip_fragment() cancel remaining fragments.
This patch restores fairness, making sure we return NET_XMIT_CN only if
we dropped a packet from the same flow.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Patrick McHardy <kaber@trash.net>
CC: Jarek Poplawski <jarkao2@gmail.com>
CC: Jamal Hadi Salim <hadi@cyberus.ca>
CC: Stephen Hemminger <shemminger@vyatta.com>
---
net/sched/sch_sfq.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 7ef87f9..b1d00f8 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -361,7 +361,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
{
struct sfq_sched_data *q = qdisc_priv(sch);
unsigned int hash;
- sfq_index x;
+ sfq_index x, qlen;
struct sfq_slot *slot;
int uninitialized_var(ret);
@@ -405,8 +405,12 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
if (++sch->q.qlen <= q->limit)
return NET_XMIT_SUCCESS;
+ qlen = slot->qlen;
sfq_drop(sch);
- return NET_XMIT_CN;
+ /* Return Congestion Notification only if we dropped a packet
+ * from this flow.
+ */
+ return (qlen != slot->qlen) ? NET_XMIT_CN : NET_XMIT_SUCCESS;
}
static struct sk_buff *
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] sch_sfq: avoid giving spurious NET_XMIT_CN signals
2011-05-23 21:02 [PATCH] sch_sfq: avoid giving spurious NET_XMIT_CN signals Eric Dumazet
@ 2011-05-23 21:36 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2011-05-23 21:36 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, kaber, jarkao2, hadi, shemminger
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 23 May 2011 23:02:42 +0200
> While chasing a possible net_sched bug, I found that IP fragments have
> litle chance to pass a congestioned SFQ qdisc :
>
> - Say SFQ qdisc is full because one flow is non responsive.
> - ip_fragment() wants to send two fragments belonging to an idle flow.
> - sfq_enqueue() queues first packet, but see queue limit reached :
> - sfq_enqueue() drops one packet from 'big consumer', and returns
> NET_XMIT_CN.
> - ip_fragment() cancel remaining fragments.
>
> This patch restores fairness, making sure we return NET_XMIT_CN only if
> we dropped a packet from the same flow.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Looks good, applied, thanks Eric.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-05-23 21:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-23 21:02 [PATCH] sch_sfq: avoid giving spurious NET_XMIT_CN signals Eric Dumazet
2011-05-23 21:36 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.