All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: Petr Machata <petrm@mellanox.com>
Cc: netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	Roman Mashak <mrv@mojatatu.com>,
	jhs@mojatatu.com, xiyou.wangcong@gmail.com, davem@davemloft.net,
	jiri@mellanox.com, mlxsw@mellanox.com
Subject: Re: [PATCH net-next v2 3/6] net: sched: RED: Introduce an ECN tail-dropping mode
Date: Wed, 11 Mar 2020 18:01:38 -0700	[thread overview]
Message-ID: <7a7038ca-2f6f-30f6-e168-6a3510db0db7@gmail.com> (raw)
In-Reply-To: <87imjaxv23.fsf@mellanox.com>



On 3/11/20 5:42 PM, Petr Machata wrote:
> 
> Eric Dumazet <eric.dumazet@gmail.com> writes:
> 
>> On 3/11/20 10:33 AM, Petr Machata wrote:
>>> When the RED Qdisc is currently configured to enable ECN, the RED algorithm
>>> is used to decide whether a certain SKB should be marked. If that SKB is
>>> not ECN-capable, it is early-dropped.
>>>
>>> It is also possible to keep all traffic in the queue, and just mark the
>>> ECN-capable subset of it, as appropriate under the RED algorithm. Some
>>> switches support this mode, and some installations make use of it.
>>>
>>> To that end, add a new RED flag, TC_RED_TAILDROP. When the Qdisc is
>>> configured with this flag, non-ECT traffic is enqueued (and tail-dropped
>>> when the queue size is exhausted) instead of being early-dropped.
>>>
>>
>> I find the naming of the feature very confusing.
>>
>> When enabling this new feature, we no longer drop packets
>> that could not be CE marked.
>> Tail drop is already in the packet scheduler, you want to disable it.
>>
>>
>> How this feature has been named elsewhere ???
>> (you mentioned in your cover letter :
>> "Some switches support this mode, and some installations make use of it.")
> 
> The two interfaces that I know about are Juniper and Cumulus. I don't
> know either from direct experience, but from the manual, Cumulus seems
> to allow enablement of either ECN on its own[0], or ECN with RED[1]. (Or
> RED on its own I presume, but I couldn't actually find that.)
> 
> In Juniper likewise, "on ECN-enabled queues, the switch [...] uses the
> tail-drop algorithm to drop non-ECN-capable packets during periods of
> congestion"[2]. You need to direct non-ECT traffic to a different queue
> and configure RED on that to get the RED+ECN behavior ala Linux.
> 
> So this is unlike the RED qdisc, where RED is implied, and needs to be
> turned off again by an anti-RED flag. The logic behind the chosen flag
> name is that the opposite of early dropping is tail dropping. Note that
> Juniper actually calls it that as well.
> 
> That said, I agree that from the perspective of the qdisc itself the
> name does not make sense. We can make it "nodrop", or "nored", or maybe
> "keep_non_ect"... I guess "nored" is closest to the desired effect.

Well, red algo is still used to decide if we attempt ECN marking, so "nodrop"
seems better to me :)

> 
> [0] https://docs.cumulusnetworks.com/cumulus-linux-40/Layer-1-and-Switch-Ports/Buffer-and-Queue-Management/
> [1] https://docs.cumulusnetworks.com/version/cumulus-linux-37/Network-Solutions/RDMA-over-Converged-Ethernet-RoCE/
> [2] https://www.juniper.net/documentation/en_US/junos/topics/concept/cos-qfx-series-tail-drop-understanding.html
> 

  reply	other threads:[~2020-03-12  1:01 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-11 17:33 [PATCH net-next v2 0/6] RED: Introduce an ECN tail-dropping mode Petr Machata
2020-03-11 17:33 ` [PATCH net-next v2 1/6] selftests: qdiscs: Add TDC test for RED Petr Machata
2020-03-12  2:20   ` Roman Mashak
2020-03-11 17:33 ` [PATCH net-next v2 2/6] net: sched: Allow extending set of supported RED flags Petr Machata
2020-03-11 22:09   ` Jakub Kicinski
2020-03-12  0:12     ` Petr Machata
2020-03-11 17:33 ` [PATCH net-next v2 3/6] net: sched: RED: Introduce an ECN tail-dropping mode Petr Machata
2020-03-11 18:36   ` Eric Dumazet
2020-03-12  0:42     ` Petr Machata
2020-03-12  1:01       ` Eric Dumazet [this message]
2020-03-12  2:25         ` Jakub Kicinski
2020-03-12 10:16           ` Petr Machata
2020-03-12 10:17   ` Petr Machata
2020-03-11 17:33 ` [PATCH net-next v2 4/6] mlxsw: spectrum_qdisc: Offload RED " Petr Machata
2020-03-11 17:33 ` [PATCH net-next v2 5/6] selftests: qdiscs: RED: Add taildrop tests Petr Machata
2020-03-12  2:21   ` Roman Mashak
2020-03-11 17:33 ` [PATCH net-next v2 6/6] selftests: mlxsw: RED: Test RED ECN taildrop offload Petr Machata
2020-03-15  4:04 ` [PATCH net-next v2 0/6] RED: Introduce an ECN tail-dropping mode David Miller
2020-03-16 10:54   ` Petr Machata
2020-03-16 21:55     ` David Miller
2020-03-17 12:43       ` Petr Machata

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a7038ca-2f6f-30f6-e168-6a3510db0db7@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=jhs@mojatatu.com \
    --cc=jiri@mellanox.com \
    --cc=kuba@kernel.org \
    --cc=mlxsw@mellanox.com \
    --cc=mrv@mojatatu.com \
    --cc=netdev@vger.kernel.org \
    --cc=petrm@mellanox.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.