Kevin Darbyshire-Bryant <ke...@darbyshire-bryant.me.uk> writes: > Ahoy there team Bufferboat :-) > > I’ve been looking at the new SCE related code and a few questions have fallen > out of the brain cell. > > 1) > https://github.com/dtaht/sch_cake/blob/47d821f89f39c1b2216d6f65b014f609e46fc57c/sce.h#L50 > Curious as to why htons isn’t used here as per other instances in > CAKE?
No idea about this. > 2) Immediately below that line we have a skb length guard before > trying to set ECT ultimately via ipv?_change_dsfield - we have other > places in CAKE where we play with dsfield bits that don’t have an > (obvious) guard - should they? > > 3) And saving the most curious for last, in cake_update_flowkeys we check the > protocol again (we want IPv4 packets only) but we use tc_skb_protocol(skb) > https://github.com/dtaht/sch_cake/blob/47d821f89f39c1b2216d6f65b014f609e46fc57c/sch_cake.c#L628 > > tc_skb_protocol says "/* We need to take extra care in case the skb came via > * vlan accelerated path. In that case, use skb->vlan_proto > * as the original vlan header was already stripped. > */ > if (skb_vlan_tag_present(skb)) > return skb->vlan_proto; > return skb->protocol; > " > > What??? Should cake_handle_diffserv use tc_skb_protocol too? and > INET_ECN_set_sce ?? Yup, I believe you are right about both of these; nice catch! I pushed fixes for cake_handle_diffserv(); keeping my hands off the sce bits for now ;) -Toke _______________________________________________ Cake mailing list Cake@lists.bufferbloat.net https://lists.bufferbloat.net/listinfo/cake