From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.toke.dk (mail.toke.dk [52.28.52.200]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id AB88C3BA8E for ; Tue, 1 May 2018 15:31:46 -0400 (EDT) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1525203105; bh=E5aNGD/ZUi56m0KINdzM0sINzau+2FySq7RfQveeTS8=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=Gf7kbp0Az0fwJUYIRKtr29QWHU4Zzs/CzHqiKxL02+OoX3p7m1oisHQ5xsBHRJN/O pDidnYZbSWdKasSa+IDL1M3zEbsBc6jR440FQ96HTtmFRDsLdkMcmmql+utS/aOvqM 0WcXyzapZQflDnQE9MliTtpopzVQHSM9AS+aeXF7xHArY/11HZWH4qnAsRroGXwiIP fVky/oSYaA6r776ycmNHvXbssz2V6QeyrmhTZniJqbxZTzSur/8uXPBlH/TJpLftsd IZ6NEP0382MwG47sHpiiALjdav2JrfhLUDZxIXl0fl+yFA94ZgR6TrFGz5nj2L0eMD PUO13N7nYKTJg== To: Eric Dumazet , Eric Dumazet , Dave Taht , Cong Wang Cc: Linux Kernel Network Developers , Cake List In-Reply-To: <4ec8da81-8671-f434-bada-27088b09ce7b@gmail.com> References: <20180429213439.7389-1-toke@toke.dk> <878t932ont.fsf@toke.dk> <4ec8da81-8671-f434-bada-27088b09ce7b@gmail.com> Date: Tue, 01 May 2018 21:31:46 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <871sev2mvx.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Cake] [PATCH net-next v6] Add Common Applications Kept Enhanced (cake) qdisc X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 May 2018 19:31:47 -0000 Eric Dumazet writes: > On 05/01/2018 11:53 AM, Toke H=C3=B8iland-J=C3=B8rgensen wrote: > >> *sigh* - can do, I guess. Though I'm not sure what that is going to >> accomplish, exactly? > > > I guess that nobody really wants to really review Cake if > it is a file with 2700 lines of code and hundreds of variables/tunables. > > Sure, we have big files in networking land, as a result of thousands > of changes. > > If you split it, then maybe the work can be split among reviewers as a > result. > > Or maybe David Miller can simply merge your patch as is, and hope for > the best, I really do not know. > > It seems you guys spent years/months on work on this stuff, so what is > the big deal about presenting your work in the best possible way ? I was objecting to what felt like an arbitrary moving of goal posts without an explanation. Now that you give one, that is fine of course. I'll split it an resubmit. Could you comment on specifically what you believe is broken in this, please, so I can fix it in the same iteration? +static inline struct tcphdr *cake_get_tcphdr(struct sk_buff *skb) +{ + struct ipv6hdr *ipv6h; + struct iphdr *iph; + + /* check IPv6 header size immediately, since for IPv4 we need the space + * for the TCP header anyway + */ + if (!pskb_may_pull(skb, skb_network_offset(skb) + + sizeof(struct ipv6hdr))) + return NULL; + + iph =3D ip_hdr(skb); + + if (iph->version =3D=3D 4) { + /* special-case 6in4 tunnelling, as that is a common way to get + * v6 connectivity in the home + */ + if (iph->protocol =3D=3D IPPROTO_IPV6) { + if (!pskb_may_pull(skb, (skb_network_offset(skb) + + ip_hdrlen(skb) + + sizeof(struct ipv6hdr)))) + return NULL; + + ipv6h =3D (struct ipv6hdr *)(skb_network_header(skb) + + ip_hdrlen(skb)); + + if (ipv6h->nexthdr !=3D IPPROTO_TCP) + return NULL; + + skb_set_inner_network_header(skb, + skb_network_offset(skb) + + ip_hdrlen(skb)); + skb_set_inner_transport_header(skb, + skb_inner_network_offset(skb) + + sizeof(struct ipv6hdr)); + + } else if (iph->protocol =3D=3D IPPROTO_TCP) { + /* we always set the inner headers so we can use those + * unconditionally in the filtering logic + */ + skb_set_inner_network_header(skb, + skb_network_offset(skb)); + skb_set_inner_transport_header(skb, + skb_network_offset(skb) + + ip_hdrlen(skb)); + } else { + return NULL; + } + + } else if (iph->version =3D=3D 6) { + ipv6h =3D (struct ipv6hdr *)iph; + + if (ipv6h->nexthdr !=3D IPPROTO_TCP) + return NULL; + + /* we always set the inner headers so we can use those + * unconditionally in the filtering logic + */ + skb_set_inner_network_header(skb, + skb_network_offset(skb)); + skb_set_inner_transport_header(skb, + skb_network_offset(skb) + + sizeof(struct ipv6hdr)); + + } else { + return NULL; + } + + if (!pskb_may_pull(skb, skb_inner_transport_offset(skb) + + sizeof(struct tcphdr)) || + !pskb_may_pull(skb, skb_inner_transport_offset(skb) + + inner_tcp_hdrlen(skb))) + return NULL; + + return inner_tcp_hdr(skb); +} Thanks, -Toke