On Fri, Jul 3, 2020 at 8:22 AM Toke Høiland-Jørgensen <t...@redhat.com> 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
> consistently
> + * whether VLAN acceleration is enabled or not.
> + */
> +static inline __be16 skb_protocol(const struct sk_buff *skb, bool skip_vlan)
> +{
> + unsigned int offset = skb_mac_offset(skb) + sizeof(struct ethhdr);
> + __be16 proto = 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 : proto;
> +
> + while (eth_type_vlan(proto)) {
> + vh = skb_header_pointer(skb, offset, sizeof(vhdr), &vhdr);
> + if (!vh)
> + break;
> +
> + proto = vh->h_vlan_encapsulated_proto;
> + offset += 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 <linux/ip.h>
> #include <linux/skbuff.h>
> +#include <linux/if_vlan.h>
>
> #include <net/inet_sock.h>
> #include <net/dsfield.h>
> @@ -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) <=
> 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) <=
> 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.
