[PATCH for-4.19 0/3] Backport of series: 'sched: A few small fixes for sch_cake' from 5.1
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.
-Toke
---
Toke Høiland-Jørgensen (3): sch_cake: Simplify logic in cake_select_tin() sch_cake: Use tc_skb_protocol() helper for getting packet protocol sch_cake: Make sure we can write the IP header before changing DSCP bits
net/sched/sch_cake.c | 57 +++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 29 deletions(-)
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);
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];
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
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...
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)
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
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
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
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
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Toke Høiland-Jørgensen