From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd43.google.com (mail-io1-xd43.google.com [IPv6:2607:f8b0:4864:20::d43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id AB3693B2A4 for ; Fri, 3 Jul 2020 15:19:50 -0400 (EDT) Received: by mail-io1-xd43.google.com with SMTP id q8so33413626iow.7 for ; Fri, 03 Jul 2020 12:19:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=2pwLCKZCNv25/ycwjdAenhsrMK+b58nPM8d8dSR+7SE=; b=DtdrgqErUK3c5YjV64C8OWLvykRDZ389aTxOkt4cPA5HWPnnaXriMqwZm+6fSYW3EZ tMfobZWhFCjhG9sOKz+XBDT3zo4tUWF3fFpB4KhvmGen6ePaVOWj6ROSf7tVGCeHRFMt LMb2X69plIZImrQ3rJV6/0JYoPhYjhygfdt5THmirUmilFkTjs7483srvU6033PA+TBS s5nw0IIncBmz/oEv2M6EizhWJV3qcybzlZknoL+8TsJzO3cvnDSod4KY5/zONSXBgqk5 vBzdv0q1yDpcCM78Kob3gAB4kZq/lhc+SklertUTjo8j/yGsgUPhqyXGD09/O7zxPOBK 18aQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=2pwLCKZCNv25/ycwjdAenhsrMK+b58nPM8d8dSR+7SE=; b=eWkcjF7gHkPtqAXEUfYBDqWqtkKZbmYXKSHBsG/JH/ZsmqCpRE/Z182osIcIq06A9F buKxoyVIruMDBKKEDIH2YrJycT2jHCozWyf42gwFijB1S3fqHB86vOHUb/TwvM05SvSF d1VWRmhYvop5H7y70emyK9nCprTZcVGU//3nGRY0/tffJ2PIJjsDlYJ35EzWbvCx9CNk JK7HXN4yfYMROY3GUf3PK1ZnHHW3PAzWSb1VWS7l4G0U1j9lETAW0NfpREJubxG4d+if 8WXKcKINlh6V3ILSprLfH4WHVNipztACd2BrgAy3Eb8urW/COlD41TU6LK9xWBVAeldv ntYQ== X-Gm-Message-State: AOAM531lc4jJNJmM4dMimYKbbZ3M47r55qQewB7LnecztOaNWjel916K jCesCbPC2F7kOfpRxv+xSwLnpk1wS+ziNDwI7HI= X-Google-Smtp-Source: ABdhPJyFDPIYWvawKCXN60JR2HLAbtjgphufK6rP/A31cYE1M/JyM3hpW6jQFAUhpk18/xzki6rcZ7MULLMjbs5hYw0= X-Received: by 2002:a02:1a08:: with SMTP id 8mr39596917jai.124.1593803990094; Fri, 03 Jul 2020 12:19:50 -0700 (PDT) MIME-Version: 1.0 References: <20200703152239.471624-1-toke@redhat.com> In-Reply-To: <20200703152239.471624-1-toke@redhat.com> From: Cong Wang Date: Fri, 3 Jul 2020 12:19:38 -0700 Message-ID: To: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= Cc: David Miller , Linux Kernel Network Developers , Cake List , Davide Caratti , Jiri Pirko , Jamal Hadi Salim , Ilya Ponetayev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Fri, 03 Jul 2020 16:20:14 -0400 Subject: Re: [Cake] [PATCH net v2] sched: consistently handle layer3 header accesses in the presence of VLANs 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: Fri, 03 Jul 2020 19:19:50 -0000 On Fri, Jul 3, 2020 at 8:22 AM Toke H=C3=B8iland-J=C3=B8rgensen wrote: > diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h > index b05e855f1ddd..d0c1cb0d264d 100644 > --- a/include/linux/if_vlan.h > +++ b/include/linux/if_vlan.h > @@ -308,6 +308,35 @@ static inline bool eth_type_vlan(__be16 ethertype) > } > } > > +/* A getter for the SKB protocol field which will handle VLAN tags consi= stently > + * whether VLAN acceleration is enabled or not. > + */ > +static inline __be16 skb_protocol(const struct sk_buff *skb, bool skip_v= lan) > +{ > + unsigned int offset =3D skb_mac_offset(skb) + sizeof(struct ethhd= r); > + __be16 proto =3D skb->protocol; > + struct vlan_hdr vhdr, *vh; > + > + if (!skip_vlan) > + /* VLAN acceleration strips the VLAN header from the skb = and > + * moves it to skb->vlan_proto > + */ > + return skb_vlan_tag_present(skb) ? skb->vlan_proto : prot= o; > + > + while (eth_type_vlan(proto)) { > + vh =3D skb_header_pointer(skb, offset, sizeof(vhdr), &vhd= r); > + if (!vh) > + break; > + > + proto =3D vh->h_vlan_encapsulated_proto; > + offset +=3D sizeof(vhdr); > + } > + > + return proto; > +} > + > + > + Just nit: too many newlines here. Please run checkpatch.pl. > static inline bool vlan_hw_offload_capable(netdev_features_t features, > __be16 proto) > { > diff --git a/include/net/inet_ecn.h b/include/net/inet_ecn.h > index 0f0d1efe06dd..82763ba597f2 100644 > --- a/include/net/inet_ecn.h > +++ b/include/net/inet_ecn.h > @@ -4,6 +4,7 @@ > > #include > #include > +#include > > #include > #include > @@ -172,7 +173,7 @@ static inline void ipv6_copy_dscp(unsigned int dscp, = struct ipv6hdr *inner) > > static inline int INET_ECN_set_ce(struct sk_buff *skb) > { > - switch (skb->protocol) { > + switch (skb_protocol(skb, true)) { > case cpu_to_be16(ETH_P_IP): > if (skb_network_header(skb) + sizeof(struct iphdr) <=3D > skb_tail_pointer(skb)) > @@ -191,7 +192,7 @@ static inline int INET_ECN_set_ce(struct sk_buff *skb= ) > > static inline int INET_ECN_set_ect1(struct sk_buff *skb) > { > - switch (skb->protocol) { > + switch (skb_protocol(skb, true)) { > case cpu_to_be16(ETH_P_IP): > if (skb_network_header(skb) + sizeof(struct iphdr) <=3D > skb_tail_pointer(skb)) These two helpers are called by non-net_sched too, are you sure your change is correct for them too? For example, IP6_ECN_decapsulate() uses skb->protocol then calls INET_ECN_decapsulate() which calls the above, after your change they use skb_protocol(). This looks inconsistent to me. Thanks.