Re: [Cake] [PATCH net-next 1/5] sch_cake: fix IP protocol handling in the presence of VLAN tags

[Davide Caratti <dcaratti@redhat.com>]

hello,

my 2 cents:

On Thu, 2020-06-25 at 21:53 +0200, Toke Høiland-Jørgensen wrote:
> I think it depends a little on the use case; some callers actually care
> about the VLAN tags themselves and handle that specially (e.g.,
> act_csum).

I remember taht something similar was discussed about 1 year ago [1].

> Whereas others (e.g., sch_dsmark) probably will have the same
> issue.

I'd say that the issue "propagates" to all qdiscs that mangle the ECN-CE
bit (i.e., calling INET_ECN_set_ce() [2]), most notably all the RED
variants and "codel/fq_codel".

>  I guess I can trying going through them all and figuring out if
> there's a more generic solution.

For sch_cake, I think that the qdisc shouldn't look at the IP header when
it schedules packets having a VLAN tag.

Probably, when tc_skb_protocol() returns ETH_P_8021Q or ETH_P_8021AD, we
should look at the VLAN priority (PCP) bits (and that's something that
cake doesn't do currently - but I have a small patch in my stash that
implements this: please let me know if you are interested in seeing it :)
).

Then, to ensure that the IP precedence is respected, even with different
VLAN tags, users should explicitly configure TC filters that "map" the
DSCP value to a PCP value. This would ensure that configured priority is
respected by the scheduler, and would also be flexible enough to allow
different "mappings". 

Sure, my proposal does not cover the problem of mangling the CE bit inside
VLAN-tagged packets, i.e. if we should understand if qdiscs should allow
it or not.

WDYT?

thank you in advance!
-- 
davide


[1] https://lore.kernel.org/netdev/CAM_iQpUmuHH8S35ERuJ-sFS=17aa-C8uHSWF-WF7toANX2edCQ@mail.gmail.com/#t
[2] https://elixir.bootlin.com/linux/latest/C/ident/INET_ECN_set_ce