* [Cake] [PATCH for-4.19 1/3] sch_cake: Simplify logic in cake_select_tin()
2019-04-05 10:28 [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1 Toke Høiland-Jørgensen
@ 2019-04-05 10:28 ` Toke Høiland-Jørgensen
2019-04-29 12:43 ` Greg Kroah-Hartman
2019-04-05 10:28 ` [Cake] [PATCH for-4.19 2/3] sch_cake: Use tc_skb_protocol() helper for getting packet protocol Toke Høiland-Jørgensen
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-05 10:28 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David Miller, stable, cake
commit 4b454433221de445f6d3d73b0ac27b4f7da25b83 upstream.
The logic in cake_select_tin() was getting a bit hairy, and it turns out we
can simplify it quite a bit. This also allows us to get rid of one of the
two diffserv parsing functions, which has the added benefit that
already-zeroed DSCP fields won't get re-written.
Suggested-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
net/sched/sch_cake.c | 44 ++++++++++++++++----------------------------
1 file changed, 16 insertions(+), 28 deletions(-)
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 793016d722ec..640f00e9f665 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1508,20 +1508,6 @@ static unsigned int cake_drop(struct Qdisc *sch, struct sk_buff **to_free)
return idx + (tin << 16);
}
-static void cake_wash_diffserv(struct sk_buff *skb)
-{
- switch (skb->protocol) {
- case htons(ETH_P_IP):
- ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK, 0);
- break;
- case htons(ETH_P_IPV6):
- ipv6_change_dsfield(ipv6_hdr(skb), INET_ECN_MASK, 0);
- break;
- default:
- break;
- }
-}
-
static u8 cake_handle_diffserv(struct sk_buff *skb, u16 wash)
{
u8 dscp;
@@ -1553,25 +1539,27 @@ static struct cake_tin_data *cake_select_tin(struct Qdisc *sch,
{
struct cake_sched_data *q = qdisc_priv(sch);
u32 tin;
+ u8 dscp;
- if (TC_H_MAJ(skb->priority) == sch->handle &&
- TC_H_MIN(skb->priority) > 0 &&
- TC_H_MIN(skb->priority) <= q->tin_cnt) {
+ /* Tin selection: Default to diffserv-based selection, allow overriding
+ * using firewall marks or skb->priority.
+ */
+ dscp = cake_handle_diffserv(skb,
+ q->rate_flags & CAKE_FLAG_WASH);
+
+ if (q->tin_mode == CAKE_DIFFSERV_BESTEFFORT)
+ tin = 0;
+
+ else if (TC_H_MAJ(skb->priority) == sch->handle &&
+ TC_H_MIN(skb->priority) > 0 &&
+ TC_H_MIN(skb->priority) <= q->tin_cnt)
tin = q->tin_order[TC_H_MIN(skb->priority) - 1];
- if (q->rate_flags & CAKE_FLAG_WASH)
- cake_wash_diffserv(skb);
- } else if (q->tin_mode != CAKE_DIFFSERV_BESTEFFORT) {
- /* extract the Diffserv Precedence field, if it exists */
- /* and clear DSCP bits if washing */
- tin = q->tin_index[cake_handle_diffserv(skb,
- q->rate_flags & CAKE_FLAG_WASH)];
+ else {
+ tin = q->tin_index[dscp];
+
if (unlikely(tin >= q->tin_cnt))
tin = 0;
- } else {
- tin = 0;
- if (q->rate_flags & CAKE_FLAG_WASH)
- cake_wash_diffserv(skb);
}
return &q->tins[tin];
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cake] [PATCH for-4.19 1/3] sch_cake: Simplify logic in cake_select_tin()
2019-04-05 10:28 ` [Cake] [PATCH for-4.19 1/3] sch_cake: Simplify logic in cake_select_tin() Toke Høiland-Jørgensen
@ 2019-04-29 12:43 ` Greg Kroah-Hartman
2019-04-29 12:45 ` Greg Kroah-Hartman
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-29 12:43 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: David Miller, stable, cake
On Fri, Apr 05, 2019 at 12:28:22PM +0200, Toke Høiland-Jørgensen wrote:
> commit 4b454433221de445f6d3d73b0ac27b4f7da25b83 upstream.
I see no such commit in Linus's tree. What am I supposed to do with
this?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cake] [PATCH for-4.19 1/3] sch_cake: Simplify logic in cake_select_tin()
2019-04-29 12:43 ` Greg Kroah-Hartman
@ 2019-04-29 12:45 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-29 12:45 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: David Miller, stable, cake
On Mon, Apr 29, 2019 at 02:43:10PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Apr 05, 2019 at 12:28:22PM +0200, Toke Høiland-Jørgensen wrote:
> > commit 4b454433221de445f6d3d73b0ac27b4f7da25b83 upstream.
>
> I see no such commit in Linus's tree. What am I supposed to do with
> this?
Ah, these are already in the 4.19 tree now, nevermind...
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Cake] [PATCH for-4.19 2/3] sch_cake: Use tc_skb_protocol() helper for getting packet protocol
2019-04-05 10:28 [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1 Toke Høiland-Jørgensen
2019-04-05 10:28 ` [Cake] [PATCH for-4.19 1/3] sch_cake: Simplify logic in cake_select_tin() Toke Høiland-Jørgensen
@ 2019-04-05 10:28 ` Toke Høiland-Jørgensen
2019-04-05 10:28 ` [Cake] [PATCH for-4.19 3/3] sch_cake: Make sure we can write the IP header before changing DSCP bits Toke Høiland-Jørgensen
2019-04-05 11:34 ` [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1 Greg Kroah-Hartman
3 siblings, 0 replies; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-05 10:28 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David Miller, stable, cake
Commit bbd669a868bba591ffd38b7bc75a7b361bb54b04 upstream.
We shouldn't be using skb->protocol directly as that will miss cases with
hardware-accelerated VLAN tags. Use the helper instead to get the right
protocol number.
Reported-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
net/sched/sch_cake.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 640f00e9f665..de92b5d81ca6 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1512,7 +1512,7 @@ static u8 cake_handle_diffserv(struct sk_buff *skb, u16 wash)
{
u8 dscp;
- switch (skb->protocol) {
+ switch (tc_skb_protocol(skb)) {
case htons(ETH_P_IP):
dscp = ipv4_get_dsfield(ip_hdr(skb)) >> 2;
if (wash && dscp)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Cake] [PATCH for-4.19 3/3] sch_cake: Make sure we can write the IP header before changing DSCP bits
2019-04-05 10:28 [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1 Toke Høiland-Jørgensen
2019-04-05 10:28 ` [Cake] [PATCH for-4.19 1/3] sch_cake: Simplify logic in cake_select_tin() Toke Høiland-Jørgensen
2019-04-05 10:28 ` [Cake] [PATCH for-4.19 2/3] sch_cake: Use tc_skb_protocol() helper for getting packet protocol Toke Høiland-Jørgensen
@ 2019-04-05 10:28 ` Toke Høiland-Jørgensen
2019-04-05 11:34 ` [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1 Greg Kroah-Hartman
3 siblings, 0 replies; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-05 10:28 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David Miller, stable, cake
Commit bbd669a868bba591ffd38b7bc75a7b361bb54b04 upstream.
There is not actually any guarantee that the IP headers are valid before we
access the DSCP bits of the packets. Fix this using the same approach taken
in sch_dsmark.
Reported-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
net/sched/sch_cake.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index de92b5d81ca6..9fd37d91b5ed 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1510,16 +1510,27 @@ static unsigned int cake_drop(struct Qdisc *sch, struct sk_buff **to_free)
static u8 cake_handle_diffserv(struct sk_buff *skb, u16 wash)
{
+ int wlen = skb_network_offset(skb);
u8 dscp;
switch (tc_skb_protocol(skb)) {
case htons(ETH_P_IP):
+ wlen += sizeof(struct iphdr);
+ if (!pskb_may_pull(skb, wlen) ||
+ skb_try_make_writable(skb, wlen))
+ return 0;
+
dscp = ipv4_get_dsfield(ip_hdr(skb)) >> 2;
if (wash && dscp)
ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK, 0);
return dscp;
case htons(ETH_P_IPV6):
+ wlen += sizeof(struct ipv6hdr);
+ if (!pskb_may_pull(skb, wlen) ||
+ skb_try_make_writable(skb, wlen))
+ return 0;
+
dscp = ipv6_get_dsfield(ipv6_hdr(skb)) >> 2;
if (wash && dscp)
ipv6_change_dsfield(ipv6_hdr(skb), INET_ECN_MASK, 0);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1
2019-04-05 10:28 [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1 Toke Høiland-Jørgensen
` (2 preceding siblings ...)
2019-04-05 10:28 ` [Cake] [PATCH for-4.19 3/3] sch_cake: Make sure we can write the IP header before changing DSCP bits Toke Høiland-Jørgensen
@ 2019-04-05 11:34 ` Greg Kroah-Hartman
2019-04-05 12:13 ` Toke Høiland-Jørgensen
3 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-05 11:34 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: David Miller, stable, cake
On Fri, Apr 05, 2019 at 12:28:21PM +0200, Toke Høiland-Jørgensen wrote:
> Hi Greg
>
> This series contains backports for a couple of fixes to sch_cake that was just
> merged for 5.1. This series backports an earlier refactoring commit, which makes
> the fixes themselves apply cleanly from upstream.
You have read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to submit networking patches to the stable trees, right?
I suggest trying it that way first...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1
2019-04-05 11:34 ` [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1 Greg Kroah-Hartman
@ 2019-04-05 12:13 ` Toke Høiland-Jørgensen
2019-04-05 12:20 ` Greg Kroah-Hartman
0 siblings, 1 reply; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-05 12:13 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David Miller, stable, cake
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Fri, Apr 05, 2019 at 12:28:21PM +0200, Toke Høiland-Jørgensen wrote:
>> Hi Greg
>>
>> This series contains backports for a couple of fixes to sch_cake that was just
>> merged for 5.1. This series backports an earlier refactoring commit, which makes
>> the fixes themselves apply cleanly from upstream.
>
> You have read:
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to submit networking patches to the stable trees, right?
>
> I suggest trying it that way first...
Yeah, Dave already queued the original fixes up for stable, but they are
not going to apply cleanly on 4.19; hence the first patch in this
series.
I thought it was better to just include the full series with that, for
context, but maybe that was wrong? Should I just have sent the first
one? If so, feel free to just take the first patch in this series and
let the others go through the usual stable submission process...
-Toke
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1
2019-04-05 12:13 ` Toke Høiland-Jørgensen
@ 2019-04-05 12:20 ` Greg Kroah-Hartman
2019-04-05 12:43 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-05 12:20 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: David Miller, stable, cake
On Fri, Apr 05, 2019 at 02:13:18PM +0200, Toke Høiland-Jørgensen wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>
> > On Fri, Apr 05, 2019 at 12:28:21PM +0200, Toke Høiland-Jørgensen wrote:
> >> Hi Greg
> >>
> >> This series contains backports for a couple of fixes to sch_cake that was just
> >> merged for 5.1. This series backports an earlier refactoring commit, which makes
> >> the fixes themselves apply cleanly from upstream.
> >
> > You have read:
> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to submit networking patches to the stable trees, right?
> >
> > I suggest trying it that way first...
>
> Yeah, Dave already queued the original fixes up for stable, but they are
> not going to apply cleanly on 4.19; hence the first patch in this
> series.
>
> I thought it was better to just include the full series with that, for
> context, but maybe that was wrong? Should I just have sent the first
> one? If so, feel free to just take the first patch in this series and
> let the others go through the usual stable submission process...
Dave queues up and sends me the stable backports for networking code, as
the document states. If there is a special series needed for 4.19, I'm
sure he would be glad to take them. Or, I can take them directly, after
I have the 5.0 series queued up, if I get an ack from him.
But to seemingly circumvent the normal process, isn't ok, I need to know
that at least you tried :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Cake] [PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1
2019-04-05 12:20 ` Greg Kroah-Hartman
@ 2019-04-05 12:43 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-05 12:43 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David Miller, stable, cake
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Fri, Apr 05, 2019 at 02:13:18PM +0200, Toke Høiland-Jørgensen wrote:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>
>> > On Fri, Apr 05, 2019 at 12:28:21PM +0200, Toke Høiland-Jørgensen wrote:
>> >> Hi Greg
>> >>
>> >> This series contains backports for a couple of fixes to sch_cake that was just
>> >> merged for 5.1. This series backports an earlier refactoring commit, which makes
>> >> the fixes themselves apply cleanly from upstream.
>> >
>> > You have read:
>> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>> > for how to submit networking patches to the stable trees, right?
>> >
>> > I suggest trying it that way first...
>>
>> Yeah, Dave already queued the original fixes up for stable, but they are
>> not going to apply cleanly on 4.19; hence the first patch in this
>> series.
>>
>> I thought it was better to just include the full series with that, for
>> context, but maybe that was wrong? Should I just have sent the first
>> one? If so, feel free to just take the first patch in this series and
>> let the others go through the usual stable submission process...
>
> Dave queues up and sends me the stable backports for networking code,
> as the document states. If there is a special series needed for 4.19,
> I'm sure he would be glad to take them. Or, I can take them directly,
> after I have the 5.0 series queued up, if I get an ack from him.
>
> But to seemingly circumvent the normal process, isn't ok, I need to
> know that at least you tried :)
Well I did ask Dave if he wanted me to supply a backport, see
https://marc.info/?l=linux-netdev&m=155440061002032&w=2 - I just thought
sending that directly to you was the natural thing to do. Certainly
wasn't trying to circumvent process, just haven't done any backports for
the net tree before :)
Actually, looking at the log again now, I see that I got my versions
mixed up, and the backport patch is also needed for 5.0... so I guess
I'll just (re-)submit that to Dave to put into his stable queue...
-Toke
^ permalink raw reply [flat|nested] 10+ messages in thread