* [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash @ 2016-08-03 14:52 fgao 2016-08-03 16:15 ` Tom Herbert 2016-08-03 20:43 ` Philip Prindeville 0 siblings, 2 replies; 8+ messages in thread From: fgao @ 2016-08-03 14:52 UTC (permalink / raw) To: davem, philipp, stephen, pshelar, tom, aduyck, netdev Cc: gfree.wind, Gao Feng From: Gao Feng <fgao@ikuai8.com> The PPTP is encapsulated by GRE header with that GRE_VERSION bits must contain one. But current GRE RPS needs the GRE_VERSION must be zero. So RPS does not work for PPTP traffic. In my test environment, there are four MIPS cores, and all traffic are passed through by PPTP. As a result, only one core is 100% busy while other three cores are very idle. After this patch, the usage of four cores are balanced well. Signed-off-by: Gao Feng <fgao@ikuai8.com> --- v3: 1) Move struct pptp_gre_header defination into new file pptp.h 2) Use sizeof GRE and PPTP type instead of literal value; 3) Remove strict flag check for PPTP to robust; 4) Consolidate the codes again; v2: Update according to Tom and Philp's advice. 1) Consolidate the codes with GRE version 0 path; 2) Use PPP_PROTOCOL to get ppp protol; 3) Set the FLOW_DIS_ENCAPSULATION flag; v1: Intial patch drivers/net/ppp/pptp.c | 36 +---------- include/net/pptp.h | 40 ++++++++++++ include/uapi/linux/if_tunnel.h | 7 +- net/core/flow_dissector.c | 141 +++++++++++++++++++++++++---------------- 4 files changed, 134 insertions(+), 90 deletions(-) create mode 100644 include/net/pptp.h diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c index ae0905e..3e68dbc 100644 --- a/drivers/net/ppp/pptp.c +++ b/drivers/net/ppp/pptp.c @@ -37,6 +37,7 @@ #include <net/icmp.h> #include <net/route.h> #include <net/gre.h> +#include <net/pptp.h> #include <linux/uaccess.h> @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly; static const struct ppp_channel_ops pptp_chan_ops; static const struct proto_ops pptp_ops; -#define PPP_LCP_ECHOREQ 0x09 -#define PPP_LCP_ECHOREP 0x0A -#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) - -#define MISSING_WINDOW 20 -#define WRAPPED(curseq, lastseq)\ - ((((curseq) & 0xffffff00) == 0) &&\ - (((lastseq) & 0xffffff00) == 0xffffff00)) - -#define PPTP_GRE_PROTO 0x880B -#define PPTP_GRE_VER 0x1 - -#define PPTP_GRE_FLAG_C 0x80 -#define PPTP_GRE_FLAG_R 0x40 -#define PPTP_GRE_FLAG_K 0x20 -#define PPTP_GRE_FLAG_S 0x10 -#define PPTP_GRE_FLAG_A 0x80 - -#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) -#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) -#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) -#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) -#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) - -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) -struct pptp_gre_header { - u8 flags; - u8 ver; - __be16 protocol; - __be16 payload_len; - __be16 call_id; - __be32 seq; - __be32 ack; -} __packed; - static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr) { struct pppox_sock *sock; diff --git a/include/net/pptp.h b/include/net/pptp.h new file mode 100644 index 0000000..301d3e2 --- /dev/null +++ b/include/net/pptp.h @@ -0,0 +1,40 @@ +#ifndef _NET_PPTP_H +#define _NET_PPTP_H + +#define PPP_LCP_ECHOREQ 0x09 +#define PPP_LCP_ECHOREP 0x0A +#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) + +#define MISSING_WINDOW 20 +#define WRAPPED(curseq, lastseq)\ + ((((curseq) & 0xffffff00) == 0) &&\ + (((lastseq) & 0xffffff00) == 0xffffff00)) + +#define PPTP_GRE_PROTO 0x880B +#define PPTP_GRE_VER 0x1 + +#define PPTP_GRE_FLAG_C 0x80 +#define PPTP_GRE_FLAG_R 0x40 +#define PPTP_GRE_FLAG_K 0x20 +#define PPTP_GRE_FLAG_S 0x10 +#define PPTP_GRE_FLAG_A 0x80 + +#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) +#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) +#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) +#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) +#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) + +#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) +struct pptp_gre_header { + u8 flags; + u8 ver; + __be16 protocol; + __be16 payload_len; + __be16 call_id; + __be32 seq; + __be32 ack; +} __packed; + + +#endif diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h index 1046f55..7d889db 100644 --- a/include/uapi/linux/if_tunnel.h +++ b/include/uapi/linux/if_tunnel.h @@ -24,9 +24,14 @@ #define GRE_SEQ __cpu_to_be16(0x1000) #define GRE_STRICT __cpu_to_be16(0x0800) #define GRE_REC __cpu_to_be16(0x0700) -#define GRE_FLAGS __cpu_to_be16(0x00F8) +#define GRE_ACK __cpu_to_be16(0x0080) +#define GRE_FLAGS __cpu_to_be16(0x0078) #define GRE_VERSION __cpu_to_be16(0x0007) +#define GRE_VERSION_1 __cpu_to_be16(0x0001) +#define GRE_PROTO_PPP __cpu_to_be16(0x880b) + + struct ip_tunnel_parm { char name[IFNAMSIZ]; int link; diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c index 61ad43f..52b7c3c 100644 --- a/net/core/flow_dissector.c +++ b/net/core/flow_dissector.c @@ -6,6 +6,8 @@ #include <linux/if_vlan.h> #include <net/ip.h> #include <net/ipv6.h> +#include <net/gre.h> +#include <net/pptp.h> #include <linux/igmp.h> #include <linux/icmp.h> #include <linux/sctp.h> @@ -338,71 +340,102 @@ mpls: ip_proto_again: switch (ip_proto) { case IPPROTO_GRE: { - struct gre_hdr { - __be16 flags; - __be16 proto; - } *hdr, _hdr; + struct gre_base_hdr *hdr, _hdr; hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr); if (!hdr) goto out_bad; - /* - * Only look inside GRE if version zero and no - * routing - */ - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) - break; - proto = hdr->proto; - nhoff += 4; - if (hdr->flags & GRE_CSUM) - nhoff += 4; - if (hdr->flags & GRE_KEY) { - const __be32 *keyid; - __be32 _keyid; + /* Only look inside GRE without routing */ + if (!(hdr->flags & GRE_ROUTING)) { + int offset = 0; - keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid), - data, hlen, &_keyid); + proto = hdr->protocol; - if (!keyid) - goto out_bad; + if (hdr->flags & GRE_VERSION) { + /* Maybe PPTP in GRE */ + if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY) && + (hdr->flags & GRE_VERSION) == GRE_VERSION_1)) + break; + } - if (dissector_uses_key(flow_dissector, - FLOW_DISSECTOR_KEY_GRE_KEYID)) { - key_keyid = skb_flow_dissector_target(flow_dissector, - FLOW_DISSECTOR_KEY_GRE_KEYID, - target_container); - key_keyid->keyid = *keyid; + offset += sizeof(struct gre_base_hdr); + + if (hdr->flags & GRE_CSUM) + offset += sizeof(__be32); + + if (hdr->flags & GRE_KEY) { + const __be32 *keyid; + __be32 _keyid; + + keyid = __skb_header_pointer(skb, nhoff + offset, sizeof(_keyid), + data, hlen, &_keyid); + + if (!keyid) + goto out_bad; + + if (dissector_uses_key(flow_dissector, + FLOW_DISSECTOR_KEY_GRE_KEYID)) { + key_keyid = skb_flow_dissector_target(flow_dissector, + FLOW_DISSECTOR_KEY_GRE_KEYID, + target_container); + key_keyid->keyid = *keyid; + } + offset += sizeof(_keyid); } - nhoff += 4; - } - if (hdr->flags & GRE_SEQ) - nhoff += 4; - if (proto == htons(ETH_P_TEB)) { - const struct ethhdr *eth; - struct ethhdr _eth; - - eth = __skb_header_pointer(skb, nhoff, - sizeof(_eth), - data, hlen, &_eth); - if (!eth) - goto out_bad; - proto = eth->h_proto; - nhoff += sizeof(*eth); - - /* Cap headers that we access via pointers at the - * end of the Ethernet header as our maximum alignment - * at that point is only 2 bytes. - */ - if (NET_IP_ALIGN) - hlen = nhoff; - } - key_control->flags |= FLOW_DIS_ENCAPSULATION; - if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) - goto out_good; + if (hdr->flags & GRE_SEQ) + offset += sizeof(((struct pptp_gre_header *)0)->seq); + + if (hdr->flags & GRE_ACK) + offset += sizeof(((struct pptp_gre_header *)0)->ack); + + if (proto == GRE_PROTO_PPP) { + u8 _ppp_hdr[PPP_HDRLEN]; + u8 *ppp_hdr; + + ppp_hdr = skb_header_pointer(skb, nhoff + offset, + sizeof(_ppp_hdr), _ppp_hdr); + if (!ppp_hdr) + goto out_bad; + + proto = PPP_PROTOCOL(ppp_hdr); + if (proto == PPP_IP) + proto = htons(ETH_P_IP); + else if (proto == PPP_IPV6) + proto = htons(ETH_P_IPV6); + else + break; + + offset += PPP_HDRLEN; + } else if (proto == htons(ETH_P_TEB)) { + const struct ethhdr *eth; + struct ethhdr _eth; + + eth = __skb_header_pointer(skb, nhoff + offset, + sizeof(_eth), + data, hlen, &_eth); + if (!eth) + goto out_bad; + proto = eth->h_proto; + offset += sizeof(*eth); + + /* Cap headers that we access via pointers at the + * end of the Ethernet header as our maximum alignment + * at that point is only 2 bytes. + */ + if (NET_IP_ALIGN) + hlen = (nhoff + offset); + } - goto again; + nhoff += offset; + key_control->flags |= FLOW_DIS_ENCAPSULATION; + if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) + goto out_good; + + goto again; + } + break; } case NEXTHDR_HOP: case NEXTHDR_ROUTING: -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash 2016-08-03 14:52 [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash fgao @ 2016-08-03 16:15 ` Tom Herbert 2016-08-03 23:47 ` Feng Gao 2016-08-03 20:43 ` Philip Prindeville 1 sibling, 1 reply; 8+ messages in thread From: Tom Herbert @ 2016-08-03 16:15 UTC (permalink / raw) To: Gao Feng Cc: David S. Miller, Philp Prindeville, Stephen Hemminger, Pravin B Shelar, Alex Duyck, Linux Kernel Network Developers, Feng Gao On Wed, Aug 3, 2016 at 7:52 AM, <fgao@ikuai8.com> wrote: > From: Gao Feng <fgao@ikuai8.com> > > The PPTP is encapsulated by GRE header with that GRE_VERSION bits > must contain one. But current GRE RPS needs the GRE_VERSION must be > zero. So RPS does not work for PPTP traffic. > > In my test environment, there are four MIPS cores, and all traffic > are passed through by PPTP. As a result, only one core is 100% busy > while other three cores are very idle. After this patch, the usage > of four cores are balanced well. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> > --- > v3: 1) Move struct pptp_gre_header defination into new file pptp.h > 2) Use sizeof GRE and PPTP type instead of literal value; > 3) Remove strict flag check for PPTP to robust; > 4) Consolidate the codes again; > v2: Update according to Tom and Philp's advice. > 1) Consolidate the codes with GRE version 0 path; > 2) Use PPP_PROTOCOL to get ppp protol; > 3) Set the FLOW_DIS_ENCAPSULATION flag; > v1: Intial patch > > drivers/net/ppp/pptp.c | 36 +---------- > include/net/pptp.h | 40 ++++++++++++ > include/uapi/linux/if_tunnel.h | 7 +- > net/core/flow_dissector.c | 141 +++++++++++++++++++++++++---------------- > 4 files changed, 134 insertions(+), 90 deletions(-) > create mode 100644 include/net/pptp.h > > diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c > index ae0905e..3e68dbc 100644 > --- a/drivers/net/ppp/pptp.c > +++ b/drivers/net/ppp/pptp.c > @@ -37,6 +37,7 @@ > #include <net/icmp.h> > #include <net/route.h> > #include <net/gre.h> > +#include <net/pptp.h> > > #include <linux/uaccess.h> > > @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly; > static const struct ppp_channel_ops pptp_chan_ops; > static const struct proto_ops pptp_ops; > > -#define PPP_LCP_ECHOREQ 0x09 > -#define PPP_LCP_ECHOREP 0x0A > -#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) > - > -#define MISSING_WINDOW 20 > -#define WRAPPED(curseq, lastseq)\ > - ((((curseq) & 0xffffff00) == 0) &&\ > - (((lastseq) & 0xffffff00) == 0xffffff00)) > - > -#define PPTP_GRE_PROTO 0x880B > -#define PPTP_GRE_VER 0x1 > - > -#define PPTP_GRE_FLAG_C 0x80 > -#define PPTP_GRE_FLAG_R 0x40 > -#define PPTP_GRE_FLAG_K 0x20 > -#define PPTP_GRE_FLAG_S 0x10 > -#define PPTP_GRE_FLAG_A 0x80 > - > -#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) > -#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) > -#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) > -#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) > -#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) > - > -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) > -struct pptp_gre_header { > - u8 flags; > - u8 ver; > - __be16 protocol; > - __be16 payload_len; > - __be16 call_id; > - __be32 seq; > - __be32 ack; > -} __packed; > - > static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr) > { > struct pppox_sock *sock; > diff --git a/include/net/pptp.h b/include/net/pptp.h > new file mode 100644 > index 0000000..301d3e2 > --- /dev/null > +++ b/include/net/pptp.h > @@ -0,0 +1,40 @@ > +#ifndef _NET_PPTP_H > +#define _NET_PPTP_H > + > +#define PPP_LCP_ECHOREQ 0x09 > +#define PPP_LCP_ECHOREP 0x0A > +#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) > + > +#define MISSING_WINDOW 20 > +#define WRAPPED(curseq, lastseq)\ > + ((((curseq) & 0xffffff00) == 0) &&\ > + (((lastseq) & 0xffffff00) == 0xffffff00)) > + > +#define PPTP_GRE_PROTO 0x880B > +#define PPTP_GRE_VER 0x1 > + > +#define PPTP_GRE_FLAG_C 0x80 > +#define PPTP_GRE_FLAG_R 0x40 > +#define PPTP_GRE_FLAG_K 0x20 > +#define PPTP_GRE_FLAG_S 0x10 > +#define PPTP_GRE_FLAG_A 0x80 > + > +#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) > +#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) > +#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) > +#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) > +#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) > + > +#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) > +struct pptp_gre_header { > + u8 flags; > + u8 ver; > + __be16 protocol; > + __be16 payload_len; > + __be16 call_id; > + __be32 seq; > + __be32 ack; > +} __packed; > + > + > +#endif > diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h > index 1046f55..7d889db 100644 > --- a/include/uapi/linux/if_tunnel.h > +++ b/include/uapi/linux/if_tunnel.h > @@ -24,9 +24,14 @@ > #define GRE_SEQ __cpu_to_be16(0x1000) > #define GRE_STRICT __cpu_to_be16(0x0800) > #define GRE_REC __cpu_to_be16(0x0700) > -#define GRE_FLAGS __cpu_to_be16(0x00F8) > +#define GRE_ACK __cpu_to_be16(0x0080) > +#define GRE_FLAGS __cpu_to_be16(0x0078) > #define GRE_VERSION __cpu_to_be16(0x0007) > > +#define GRE_VERSION_1 __cpu_to_be16(0x0001) > +#define GRE_PROTO_PPP __cpu_to_be16(0x880b) > + > + > struct ip_tunnel_parm { > char name[IFNAMSIZ]; > int link; > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c > index 61ad43f..52b7c3c 100644 > --- a/net/core/flow_dissector.c > +++ b/net/core/flow_dissector.c > @@ -6,6 +6,8 @@ > #include <linux/if_vlan.h> > #include <net/ip.h> > #include <net/ipv6.h> > +#include <net/gre.h> > +#include <net/pptp.h> > #include <linux/igmp.h> > #include <linux/icmp.h> > #include <linux/sctp.h> > @@ -338,71 +340,102 @@ mpls: > ip_proto_again: > switch (ip_proto) { > case IPPROTO_GRE: { > - struct gre_hdr { > - __be16 flags; > - __be16 proto; > - } *hdr, _hdr; > + struct gre_base_hdr *hdr, _hdr; > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr); > if (!hdr) > goto out_bad; > - /* > - * Only look inside GRE if version zero and no > - * routing > - */ > - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) > - break; > > - proto = hdr->proto; > - nhoff += 4; > - if (hdr->flags & GRE_CSUM) > - nhoff += 4; > - if (hdr->flags & GRE_KEY) { > - const __be32 *keyid; > - __be32 _keyid; > + /* Only look inside GRE without routing */ > + if (!(hdr->flags & GRE_ROUTING)) { > + int offset = 0; > > - keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid), > - data, hlen, &_keyid); > + proto = hdr->protocol; > > - if (!keyid) > - goto out_bad; > + if (hdr->flags & GRE_VERSION) { > + /* Maybe PPTP in GRE */ > + if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY) && > + (hdr->flags & GRE_VERSION) == GRE_VERSION_1)) > + break; > + } > > - if (dissector_uses_key(flow_dissector, > - FLOW_DISSECTOR_KEY_GRE_KEYID)) { > - key_keyid = skb_flow_dissector_target(flow_dissector, > - FLOW_DISSECTOR_KEY_GRE_KEYID, > - target_container); > - key_keyid->keyid = *keyid; > + offset += sizeof(struct gre_base_hdr); > + > + if (hdr->flags & GRE_CSUM) > + offset += sizeof(__be32); > + > + if (hdr->flags & GRE_KEY) { > + const __be32 *keyid; > + __be32 _keyid; > + > + keyid = __skb_header_pointer(skb, nhoff + offset, sizeof(_keyid), > + data, hlen, &_keyid); > + > + if (!keyid) > + goto out_bad; > + > + if (dissector_uses_key(flow_dissector, > + FLOW_DISSECTOR_KEY_GRE_KEYID)) { > + key_keyid = skb_flow_dissector_target(flow_dissector, > + FLOW_DISSECTOR_KEY_GRE_KEYID, > + target_container); > + key_keyid->keyid = *keyid; > + } > + offset += sizeof(_keyid); > } > - nhoff += 4; > - } > - if (hdr->flags & GRE_SEQ) > - nhoff += 4; > - if (proto == htons(ETH_P_TEB)) { > - const struct ethhdr *eth; > - struct ethhdr _eth; > - > - eth = __skb_header_pointer(skb, nhoff, > - sizeof(_eth), > - data, hlen, &_eth); > - if (!eth) > - goto out_bad; > - proto = eth->h_proto; > - nhoff += sizeof(*eth); > - > - /* Cap headers that we access via pointers at the > - * end of the Ethernet header as our maximum alignment > - * at that point is only 2 bytes. > - */ > - if (NET_IP_ALIGN) > - hlen = nhoff; > - } > > - key_control->flags |= FLOW_DIS_ENCAPSULATION; > - if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) > - goto out_good; > + if (hdr->flags & GRE_SEQ) > + offset += sizeof(((struct pptp_gre_header *)0)->seq); > + > + if (hdr->flags & GRE_ACK) > + offset += sizeof(((struct pptp_gre_header *)0)->ack); > + > + if (proto == GRE_PROTO_PPP) { > + u8 _ppp_hdr[PPP_HDRLEN]; > + u8 *ppp_hdr; > + > + ppp_hdr = skb_header_pointer(skb, nhoff + offset, > + sizeof(_ppp_hdr), _ppp_hdr); > + if (!ppp_hdr) > + goto out_bad; > + > + proto = PPP_PROTOCOL(ppp_hdr); > + if (proto == PPP_IP) > + proto = htons(ETH_P_IP); > + else if (proto == PPP_IPV6) > + proto = htons(ETH_P_IPV6); > + else > + break; > + > + offset += PPP_HDRLEN; > + } else if (proto == htons(ETH_P_TEB)) { > + const struct ethhdr *eth; > + struct ethhdr _eth; > + > + eth = __skb_header_pointer(skb, nhoff + offset, > + sizeof(_eth), > + data, hlen, &_eth); > + if (!eth) > + goto out_bad; > + proto = eth->h_proto; > + offset += sizeof(*eth); > + > + /* Cap headers that we access via pointers at the > + * end of the Ethernet header as our maximum alignment > + * at that point is only 2 bytes. > + */ > + if (NET_IP_ALIGN) > + hlen = (nhoff + offset); > + } > > - goto again; > + nhoff += offset; > + key_control->flags |= FLOW_DIS_ENCAPSULATION; > + if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) > + goto out_good; > + > + goto again; > + } > + break; > } > case NEXTHDR_HOP: > case NEXTHDR_ROUTING: > -- > 1.9.1 > HI Feng, Please be careful about order of processing GRE options, keyid must be handled first. GRE_ACK looks like the only new field that needs to be considered for v1. Also, the keyid in v1 is split into two 16 bit fields; the first is payload length which is not usable for entropy, but the second (Call ID) does look useful for that. I would suggest the IPPROTO_GRE could look something like: diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c index 61ad43f..736b531 100644 --- a/net/core/flow_dissector.c +++ b/net/core/flow_dissector.c @@ -342,15 +342,20 @@ ip_proto_again: __be16 flags; __be16 proto; } *hdr, _hdr; + u8 gre_ver; hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr); if (!hdr) goto out_bad; + + /* Only look inside GRE for versions 0 and 1 */ + gre_ver = hdr->flags & GRE_VERSION; + if (gre_ver > 1) + break; /* - * Only look inside GRE if version zero and no - * routing + * Only look inside GRE if no routing */ - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) + if (hdr->flags & GRE_ROUTING) break; proto = hdr->proto; @@ -372,30 +377,62 @@ ip_proto_again: key_keyid = skb_flow_dissector_target(flow_dissector, FLOW_DISSECTOR_KEY_GRE_KEYID, target_container); - key_keyid->keyid = *keyid; + if (gre_ver == 0) + key_keyid->keyid = *keyid; + else + key_keyid->keyid = *keyid & htonl(0xffff); } nhoff += 4; } if (hdr->flags & GRE_SEQ) nhoff += 4; - if (proto == htons(ETH_P_TEB)) { - const struct ethhdr *eth; - struct ethhdr _eth; - - eth = __skb_header_pointer(skb, nhoff, - sizeof(_eth), - data, hlen, &_eth); - if (!eth) - goto out_bad; - proto = eth->h_proto; - nhoff += sizeof(*eth); - - /* Cap headers that we access via pointers at the - * end of the Ethernet header as our maximum alignment - * at that point is only 2 bytes. - */ - if (NET_IP_ALIGN) - hlen = nhoff; + if (gre_ver == 0) { + if (proto == htons(ETH_P_TEB)) { + const struct ethhdr *eth; + struct ethhdr _eth; + + eth = __skb_header_pointer(skb, nhoff, + sizeof(_eth), + data, hlen, &_eth); + if (!eth) + goto out_bad; + proto = eth->h_proto; + nhoff += sizeof(*eth); + + /* Cap headers that we access via pointers at the + * end of the Ethernet header as our maximum alignment + * at that point is only 2 bytes. + */ + if (NET_IP_ALIGN) + hlen = nhoff; + } + } else { /* Version 1 */ + if (hdr->flags & GRE_ACK) + offset += 4; + + if (proto == GRE_PROTO_PPP) { + u8 _ppp_hdr[PPP_HDRLEN]; + u8 *ppp_hdr; + + ppp_hdr = skb_header_pointer(skb, nhoff + offset, + sizeof(_ppp_hdr), _ppp_hdr); + if (!ppp_hdr) + goto out_bad; + + switch (PPP_PROTOCOL(ppp_hdr)); + case PPP_IP: + proto = htons(ETH_P_IP); + break; + case PPP_IPV6: + proto = htons(ETH_P_IPV6); + break; + default: + /* Could probably catch some more like MPLS */ + break; + } + + offset += PPP_HDRLEN; + } } key_control->flags |= FLOW_DIS_ENCAPSULATION; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash 2016-08-03 16:15 ` Tom Herbert @ 2016-08-03 23:47 ` Feng Gao 0 siblings, 0 replies; 8+ messages in thread From: Feng Gao @ 2016-08-03 23:47 UTC (permalink / raw) To: Tom Herbert Cc: Gao Feng, David S. Miller, Philp Prindeville, Stephen Hemminger, Pravin B Shelar, Alex Duyck, Linux Kernel Network Developers Hi Tom, inline comments On Thu, Aug 4, 2016 at 12:15 AM, Tom Herbert <tom@herbertland.com> wrote: > On Wed, Aug 3, 2016 at 7:52 AM, <fgao@ikuai8.com> wrote: >> From: Gao Feng <fgao@ikuai8.com> >> >> The PPTP is encapsulated by GRE header with that GRE_VERSION bits >> must contain one. But current GRE RPS needs the GRE_VERSION must be >> zero. So RPS does not work for PPTP traffic. >> >> In my test environment, there are four MIPS cores, and all traffic >> are passed through by PPTP. As a result, only one core is 100% busy >> while other three cores are very idle. After this patch, the usage >> of four cores are balanced well. >> >> Signed-off-by: Gao Feng <fgao@ikuai8.com> >> --- >> v3: 1) Move struct pptp_gre_header defination into new file pptp.h >> 2) Use sizeof GRE and PPTP type instead of literal value; >> 3) Remove strict flag check for PPTP to robust; >> 4) Consolidate the codes again; >> v2: Update according to Tom and Philp's advice. >> 1) Consolidate the codes with GRE version 0 path; >> 2) Use PPP_PROTOCOL to get ppp protol; >> 3) Set the FLOW_DIS_ENCAPSULATION flag; >> v1: Intial patch >> >> drivers/net/ppp/pptp.c | 36 +---------- >> include/net/pptp.h | 40 ++++++++++++ >> include/uapi/linux/if_tunnel.h | 7 +- >> net/core/flow_dissector.c | 141 +++++++++++++++++++++++++---------------- >> 4 files changed, 134 insertions(+), 90 deletions(-) >> create mode 100644 include/net/pptp.h >> >> diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c >> index ae0905e..3e68dbc 100644 >> --- a/drivers/net/ppp/pptp.c >> +++ b/drivers/net/ppp/pptp.c >> @@ -37,6 +37,7 @@ >> #include <net/icmp.h> >> #include <net/route.h> >> #include <net/gre.h> >> +#include <net/pptp.h> >> >> #include <linux/uaccess.h> >> >> @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly; >> static const struct ppp_channel_ops pptp_chan_ops; >> static const struct proto_ops pptp_ops; >> >> -#define PPP_LCP_ECHOREQ 0x09 >> -#define PPP_LCP_ECHOREP 0x0A >> -#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >> - >> -#define MISSING_WINDOW 20 >> -#define WRAPPED(curseq, lastseq)\ >> - ((((curseq) & 0xffffff00) == 0) &&\ >> - (((lastseq) & 0xffffff00) == 0xffffff00)) >> - >> -#define PPTP_GRE_PROTO 0x880B >> -#define PPTP_GRE_VER 0x1 >> - >> -#define PPTP_GRE_FLAG_C 0x80 >> -#define PPTP_GRE_FLAG_R 0x40 >> -#define PPTP_GRE_FLAG_K 0x20 >> -#define PPTP_GRE_FLAG_S 0x10 >> -#define PPTP_GRE_FLAG_A 0x80 >> - >> -#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >> -#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >> -#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >> -#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >> -#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >> - >> -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >> -struct pptp_gre_header { >> - u8 flags; >> - u8 ver; >> - __be16 protocol; >> - __be16 payload_len; >> - __be16 call_id; >> - __be32 seq; >> - __be32 ack; >> -} __packed; >> - >> static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr) >> { >> struct pppox_sock *sock; >> diff --git a/include/net/pptp.h b/include/net/pptp.h >> new file mode 100644 >> index 0000000..301d3e2 >> --- /dev/null >> +++ b/include/net/pptp.h >> @@ -0,0 +1,40 @@ >> +#ifndef _NET_PPTP_H >> +#define _NET_PPTP_H >> + >> +#define PPP_LCP_ECHOREQ 0x09 >> +#define PPP_LCP_ECHOREP 0x0A >> +#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >> + >> +#define MISSING_WINDOW 20 >> +#define WRAPPED(curseq, lastseq)\ >> + ((((curseq) & 0xffffff00) == 0) &&\ >> + (((lastseq) & 0xffffff00) == 0xffffff00)) >> + >> +#define PPTP_GRE_PROTO 0x880B >> +#define PPTP_GRE_VER 0x1 >> + >> +#define PPTP_GRE_FLAG_C 0x80 >> +#define PPTP_GRE_FLAG_R 0x40 >> +#define PPTP_GRE_FLAG_K 0x20 >> +#define PPTP_GRE_FLAG_S 0x10 >> +#define PPTP_GRE_FLAG_A 0x80 >> + >> +#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >> +#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >> +#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >> +#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >> +#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >> + >> +#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >> +struct pptp_gre_header { >> + u8 flags; >> + u8 ver; >> + __be16 protocol; >> + __be16 payload_len; >> + __be16 call_id; >> + __be32 seq; >> + __be32 ack; >> +} __packed; >> + >> + >> +#endif >> diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h >> index 1046f55..7d889db 100644 >> --- a/include/uapi/linux/if_tunnel.h >> +++ b/include/uapi/linux/if_tunnel.h >> @@ -24,9 +24,14 @@ >> #define GRE_SEQ __cpu_to_be16(0x1000) >> #define GRE_STRICT __cpu_to_be16(0x0800) >> #define GRE_REC __cpu_to_be16(0x0700) >> -#define GRE_FLAGS __cpu_to_be16(0x00F8) >> +#define GRE_ACK __cpu_to_be16(0x0080) >> +#define GRE_FLAGS __cpu_to_be16(0x0078) >> #define GRE_VERSION __cpu_to_be16(0x0007) >> >> +#define GRE_VERSION_1 __cpu_to_be16(0x0001) >> +#define GRE_PROTO_PPP __cpu_to_be16(0x880b) >> + >> + >> struct ip_tunnel_parm { >> char name[IFNAMSIZ]; >> int link; >> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c >> index 61ad43f..52b7c3c 100644 >> --- a/net/core/flow_dissector.c >> +++ b/net/core/flow_dissector.c >> @@ -6,6 +6,8 @@ >> #include <linux/if_vlan.h> >> #include <net/ip.h> >> #include <net/ipv6.h> >> +#include <net/gre.h> >> +#include <net/pptp.h> >> #include <linux/igmp.h> >> #include <linux/icmp.h> >> #include <linux/sctp.h> >> @@ -338,71 +340,102 @@ mpls: >> ip_proto_again: >> switch (ip_proto) { >> case IPPROTO_GRE: { >> - struct gre_hdr { >> - __be16 flags; >> - __be16 proto; >> - } *hdr, _hdr; >> + struct gre_base_hdr *hdr, _hdr; >> >> hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr); >> if (!hdr) >> goto out_bad; >> - /* >> - * Only look inside GRE if version zero and no >> - * routing >> - */ >> - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) >> - break; >> >> - proto = hdr->proto; >> - nhoff += 4; >> - if (hdr->flags & GRE_CSUM) >> - nhoff += 4; >> - if (hdr->flags & GRE_KEY) { >> - const __be32 *keyid; >> - __be32 _keyid; >> + /* Only look inside GRE without routing */ >> + if (!(hdr->flags & GRE_ROUTING)) { >> + int offset = 0; >> >> - keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid), >> - data, hlen, &_keyid); >> + proto = hdr->protocol; >> >> - if (!keyid) >> - goto out_bad; >> + if (hdr->flags & GRE_VERSION) { >> + /* Maybe PPTP in GRE */ >> + if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY) && >> + (hdr->flags & GRE_VERSION) == GRE_VERSION_1)) >> + break; >> + } >> >> - if (dissector_uses_key(flow_dissector, >> - FLOW_DISSECTOR_KEY_GRE_KEYID)) { >> - key_keyid = skb_flow_dissector_target(flow_dissector, >> - FLOW_DISSECTOR_KEY_GRE_KEYID, >> - target_container); >> - key_keyid->keyid = *keyid; >> + offset += sizeof(struct gre_base_hdr); >> + >> + if (hdr->flags & GRE_CSUM) >> + offset += sizeof(__be32); >> + >> + if (hdr->flags & GRE_KEY) { >> + const __be32 *keyid; >> + __be32 _keyid; >> + >> + keyid = __skb_header_pointer(skb, nhoff + offset, sizeof(_keyid), >> + data, hlen, &_keyid); >> + >> + if (!keyid) >> + goto out_bad; >> + >> + if (dissector_uses_key(flow_dissector, >> + FLOW_DISSECTOR_KEY_GRE_KEYID)) { >> + key_keyid = skb_flow_dissector_target(flow_dissector, >> + FLOW_DISSECTOR_KEY_GRE_KEYID, >> + target_container); >> + key_keyid->keyid = *keyid; >> + } >> + offset += sizeof(_keyid); >> } >> - nhoff += 4; >> - } >> - if (hdr->flags & GRE_SEQ) >> - nhoff += 4; >> - if (proto == htons(ETH_P_TEB)) { >> - const struct ethhdr *eth; >> - struct ethhdr _eth; >> - >> - eth = __skb_header_pointer(skb, nhoff, >> - sizeof(_eth), >> - data, hlen, &_eth); >> - if (!eth) >> - goto out_bad; >> - proto = eth->h_proto; >> - nhoff += sizeof(*eth); >> - >> - /* Cap headers that we access via pointers at the >> - * end of the Ethernet header as our maximum alignment >> - * at that point is only 2 bytes. >> - */ >> - if (NET_IP_ALIGN) >> - hlen = nhoff; >> - } >> >> - key_control->flags |= FLOW_DIS_ENCAPSULATION; >> - if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >> - goto out_good; >> + if (hdr->flags & GRE_SEQ) >> + offset += sizeof(((struct pptp_gre_header *)0)->seq); >> + >> + if (hdr->flags & GRE_ACK) >> + offset += sizeof(((struct pptp_gre_header *)0)->ack); >> + >> + if (proto == GRE_PROTO_PPP) { >> + u8 _ppp_hdr[PPP_HDRLEN]; >> + u8 *ppp_hdr; >> + >> + ppp_hdr = skb_header_pointer(skb, nhoff + offset, >> + sizeof(_ppp_hdr), _ppp_hdr); >> + if (!ppp_hdr) >> + goto out_bad; >> + >> + proto = PPP_PROTOCOL(ppp_hdr); >> + if (proto == PPP_IP) >> + proto = htons(ETH_P_IP); >> + else if (proto == PPP_IPV6) >> + proto = htons(ETH_P_IPV6); >> + else >> + break; >> + >> + offset += PPP_HDRLEN; >> + } else if (proto == htons(ETH_P_TEB)) { >> + const struct ethhdr *eth; >> + struct ethhdr _eth; >> + >> + eth = __skb_header_pointer(skb, nhoff + offset, >> + sizeof(_eth), >> + data, hlen, &_eth); >> + if (!eth) >> + goto out_bad; >> + proto = eth->h_proto; >> + offset += sizeof(*eth); >> + >> + /* Cap headers that we access via pointers at the >> + * end of the Ethernet header as our maximum alignment >> + * at that point is only 2 bytes. >> + */ >> + if (NET_IP_ALIGN) >> + hlen = (nhoff + offset); >> + } >> >> - goto again; >> + nhoff += offset; >> + key_control->flags |= FLOW_DIS_ENCAPSULATION; >> + if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >> + goto out_good; >> + >> + goto again; >> + } >> + break; >> } >> case NEXTHDR_HOP: >> case NEXTHDR_ROUTING: >> -- >> 1.9.1 >> > > HI Feng, > > Please be careful about order of processing GRE options, keyid must be > handled first. Thanks your reminder. But I think the keyid should be processed secondly and I did it in v4 patch. Because the keyid option is processed secondly in original codes. > GRE_ACK looks like the only new field that needs to be > considered for v1. Also, the keyid in v1 is split into two 16 bit > fields; the first is payload length which is not usable for entropy, > but the second (Call ID) does look useful for that. I would suggest > the IPPROTO_GRE could look something like: Ok, I get it. > > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c > index 61ad43f..736b531 100644 > --- a/net/core/flow_dissector.c > +++ b/net/core/flow_dissector.c > @@ -342,15 +342,20 @@ ip_proto_again: > __be16 flags; > __be16 proto; > } *hdr, _hdr; > + u8 gre_ver; > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), > data, hlen, &_hdr); > if (!hdr) > goto out_bad; > + > + /* Only look inside GRE for versions 0 and 1 */ > + gre_ver = hdr->flags & GRE_VERSION; > + if (gre_ver > 1) > + break; > /* > - * Only look inside GRE if version zero and no > - * routing > + * Only look inside GRE if no routing > */ > - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) > + if (hdr->flags & GRE_ROUTING) > break; > > proto = hdr->proto; > @@ -372,30 +377,62 @@ ip_proto_again: > key_keyid = > skb_flow_dissector_target(flow_dissector, > > FLOW_DISSECTOR_KEY_GRE_KEYID, > > target_container); > - key_keyid->keyid = *keyid; > + if (gre_ver == 0) > + key_keyid->keyid = *keyid; > + else > + key_keyid->keyid = *keyid & > htonl(0xffff); > } > nhoff += 4; > } > if (hdr->flags & GRE_SEQ) > nhoff += 4; > - if (proto == htons(ETH_P_TEB)) { > - const struct ethhdr *eth; > - struct ethhdr _eth; > - > - eth = __skb_header_pointer(skb, nhoff, > - sizeof(_eth), > - data, hlen, &_eth); > - if (!eth) > - goto out_bad; > - proto = eth->h_proto; > - nhoff += sizeof(*eth); > - > - /* Cap headers that we access via pointers at the > - * end of the Ethernet header as our maximum alignment > - * at that point is only 2 bytes. > - */ > - if (NET_IP_ALIGN) > - hlen = nhoff; > + if (gre_ver == 0) { > + if (proto == htons(ETH_P_TEB)) { > + const struct ethhdr *eth; > + struct ethhdr _eth; > + > + eth = __skb_header_pointer(skb, nhoff, > + sizeof(_eth), > + data, hlen, &_eth); > + if (!eth) > + goto out_bad; > + proto = eth->h_proto; > + nhoff += sizeof(*eth); > + > + /* Cap headers that we access via > pointers at the > + * end of the Ethernet header as our > maximum alignment > + * at that point is only 2 bytes. > + */ > + if (NET_IP_ALIGN) > + hlen = nhoff; > + } > + } else { /* Version 1 */ > + if (hdr->flags & GRE_ACK) > + offset += 4; > + > + if (proto == GRE_PROTO_PPP) { > + u8 _ppp_hdr[PPP_HDRLEN]; > + u8 *ppp_hdr; > + > + ppp_hdr = skb_header_pointer(skb, > nhoff + offset, > + sizeof(_ppp_hdr), _ppp_hdr); > + if (!ppp_hdr) > + goto out_bad; > + > + switch (PPP_PROTOCOL(ppp_hdr)); > + case PPP_IP: > + proto = htons(ETH_P_IP); > + break; > + case PPP_IPV6: > + proto = htons(ETH_P_IPV6); > + break; > + default: > + /* Could probably catch some > more like MPLS */ > + break; > + } > + > + offset += PPP_HDRLEN; > + } > } > > key_control->flags |= FLOW_DIS_ENCAPSULATION; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash 2016-08-03 14:52 [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash fgao 2016-08-03 16:15 ` Tom Herbert @ 2016-08-03 20:43 ` Philip Prindeville 2016-08-03 23:58 ` Feng Gao 1 sibling, 1 reply; 8+ messages in thread From: Philip Prindeville @ 2016-08-03 20:43 UTC (permalink / raw) To: fgao Cc: David S. Miller, Stephen Hemminger, Pravin B Shelar, Tom Herbert, Alex Duyck, netdev, gfree.wind [-- Attachment #1: Type: text/plain, Size: 10411 bytes --] Inline… > On Aug 3, 2016, at 8:52 AM, fgao@48lvckh6395k16k5.yundunddos.com wrote: > > From: Gao Feng <fgao@ikuai8.com> > > The PPTP is encapsulated by GRE header with that GRE_VERSION bits > must contain one. But current GRE RPS needs the GRE_VERSION must be > zero. So RPS does not work for PPTP traffic. > > In my test environment, there are four MIPS cores, and all traffic > are passed through by PPTP. As a result, only one core is 100% busy > while other three cores are very idle. After this patch, the usage > of four cores are balanced well. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> > --- > v3: 1) Move struct pptp_gre_header defination into new file pptp.h > 2) Use sizeof GRE and PPTP type instead of literal value; > 3) Remove strict flag check for PPTP to robust; > 4) Consolidate the codes again; > v2: Update according to Tom and Philp's advice. > 1) Consolidate the codes with GRE version 0 path; > 2) Use PPP_PROTOCOL to get ppp protol; > 3) Set the FLOW_DIS_ENCAPSULATION flag; > v1: Intial patch > > drivers/net/ppp/pptp.c | 36 +---------- > include/net/pptp.h | 40 ++++++++++++ > include/uapi/linux/if_tunnel.h | 7 +- > net/core/flow_dissector.c | 141 +++++++++++++++++++++++++---------------- > 4 files changed, 134 insertions(+), 90 deletions(-) > create mode 100644 include/net/pptp.h > > diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c > index ae0905e..3e68dbc 100644 > --- a/drivers/net/ppp/pptp.c > +++ b/drivers/net/ppp/pptp.c > @@ -37,6 +37,7 @@ > #include <net/icmp.h> > #include <net/route.h> > #include <net/gre.h> > +#include <net/pptp.h> > > #include <linux/uaccess.h> > > @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly; > static const struct ppp_channel_ops pptp_chan_ops; > static const struct proto_ops pptp_ops; > > -#define PPP_LCP_ECHOREQ 0x09 > -#define PPP_LCP_ECHOREP 0x0A > -#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) > - > -#define MISSING_WINDOW 20 > -#define WRAPPED(curseq, lastseq)\ > - ((((curseq) & 0xffffff00) == 0) &&\ > - (((lastseq) & 0xffffff00) == 0xffffff00)) > - > -#define PPTP_GRE_PROTO 0x880B > -#define PPTP_GRE_VER 0x1 > - > -#define PPTP_GRE_FLAG_C 0x80 > -#define PPTP_GRE_FLAG_R 0x40 > -#define PPTP_GRE_FLAG_K 0x20 > -#define PPTP_GRE_FLAG_S 0x10 > -#define PPTP_GRE_FLAG_A 0x80 > - > -#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) > -#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) > -#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) > -#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) > -#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) > - > -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) > -struct pptp_gre_header { > - u8 flags; > - u8 ver; > - __be16 protocol; > - __be16 payload_len; > - __be16 call_id; > - __be32 seq; > - __be32 ack; > -} __packed; > - > static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr) > { > struct pppox_sock *sock; > diff --git a/include/net/pptp.h b/include/net/pptp.h > new file mode 100644 > index 0000000..301d3e2 > --- /dev/null > +++ b/include/net/pptp.h > @@ -0,0 +1,40 @@ > +#ifndef _NET_PPTP_H > +#define _NET_PPTP_H > + > +#define PPP_LCP_ECHOREQ 0x09 > +#define PPP_LCP_ECHOREP 0x0A > +#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) > + > +#define MISSING_WINDOW 20 > +#define WRAPPED(curseq, lastseq)\ > + ((((curseq) & 0xffffff00) == 0) &&\ > + (((lastseq) & 0xffffff00) == 0xffffff00)) > + > +#define PPTP_GRE_PROTO 0x880B > +#define PPTP_GRE_VER 0x1 What about macros for accessing the lower 3 bits of the version? > + > +#define PPTP_GRE_FLAG_C 0x80 > +#define PPTP_GRE_FLAG_R 0x40 > +#define PPTP_GRE_FLAG_K 0x20 > +#define PPTP_GRE_FLAG_S 0x10 > +#define PPTP_GRE_FLAG_A 0x80 > + > +#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) > +#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) > +#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) > +#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) > +#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) > + > +#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) > +struct pptp_gre_header { > + u8 flags; > + u8 ver; > + __be16 protocol; > + __be16 payload_len; > + __be16 call_id; > + __be32 seq; > + __be32 ack; > +} __packed; What about a definition of a V0 (RFC-1701) packet? We’re handling both, so it makes sense to define both. > + > + > +#endif > diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h > index 1046f55..7d889db 100644 > --- a/include/uapi/linux/if_tunnel.h > +++ b/include/uapi/linux/if_tunnel.h > @@ -24,9 +24,14 @@ > #define GRE_SEQ __cpu_to_be16(0x1000) > #define GRE_STRICT __cpu_to_be16(0x0800) > #define GRE_REC __cpu_to_be16(0x0700) > -#define GRE_FLAGS __cpu_to_be16(0x00F8) > +#define GRE_ACK __cpu_to_be16(0x0080) > +#define GRE_FLAGS __cpu_to_be16(0x0078) > #define GRE_VERSION __cpu_to_be16(0x0007) > > +#define GRE_VERSION_1 __cpu_to_be16(0x0001) > +#define GRE_PROTO_PPP __cpu_to_be16(0x880b) > + > + > struct ip_tunnel_parm { > char name[IFNAMSIZ]; > int link; > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c > index 61ad43f..52b7c3c 100644 > --- a/net/core/flow_dissector.c > +++ b/net/core/flow_dissector.c > @@ -6,6 +6,8 @@ > #include <linux/if_vlan.h> > #include <net/ip.h> > #include <net/ipv6.h> > +#include <net/gre.h> > +#include <net/pptp.h> > #include <linux/igmp.h> > #include <linux/icmp.h> > #include <linux/sctp.h> > @@ -338,71 +340,102 @@ mpls: > ip_proto_again: > switch (ip_proto) { > case IPPROTO_GRE: { > - struct gre_hdr { > - __be16 flags; > - __be16 proto; > - } *hdr, _hdr; > + struct gre_base_hdr *hdr, _hdr; > > hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr); > if (!hdr) > goto out_bad; > - /* > - * Only look inside GRE if version zero and no > - * routing > - */ > - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) > - break; > > - proto = hdr->proto; > - nhoff += 4; > - if (hdr->flags & GRE_CSUM) > - nhoff += 4; > - if (hdr->flags & GRE_KEY) { > - const __be32 *keyid; > - __be32 _keyid; > + /* Only look inside GRE without routing */ > + if (!(hdr->flags & GRE_ROUTING)) { > + int offset = 0; > > - keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid), > - data, hlen, &_keyid); > + proto = hdr->protocol; > > - if (!keyid) > - goto out_bad; > + if (hdr->flags & GRE_VERSION) { > + /* Maybe PPTP in GRE */ > + if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY) && > + (hdr->flags & GRE_VERSION) == GRE_VERSION_1)) > + break; > + } > > - if (dissector_uses_key(flow_dissector, > - FLOW_DISSECTOR_KEY_GRE_KEYID)) { > - key_keyid = skb_flow_dissector_target(flow_dissector, > - FLOW_DISSECTOR_KEY_GRE_KEYID, > - target_container); > - key_keyid->keyid = *keyid; > + offset += sizeof(struct gre_base_hdr); > + > + if (hdr->flags & GRE_CSUM) > + offset += sizeof(__be32); This doesn’t tell me as much as taking the sizeof() of the particular field (by name) in the packet that you’re skipping. Best way to do this is naming the field in the structure… > + > + if (hdr->flags & GRE_KEY) { > + const __be32 *keyid; > + __be32 _keyid; > + > + keyid = __skb_header_pointer(skb, nhoff + offset, sizeof(_keyid), > + data, hlen, &_keyid); > + > + if (!keyid) > + goto out_bad; > + > + if (dissector_uses_key(flow_dissector, > + FLOW_DISSECTOR_KEY_GRE_KEYID)) { > + key_keyid = skb_flow_dissector_target(flow_dissector, > + FLOW_DISSECTOR_KEY_GRE_KEYID, > + target_container); > + key_keyid->keyid = *keyid; > + } > + offset += sizeof(_keyid); Same issue here. > } > - nhoff += 4; > - } > - if (hdr->flags & GRE_SEQ) > - nhoff += 4; > - if (proto == htons(ETH_P_TEB)) { > - const struct ethhdr *eth; > - struct ethhdr _eth; > - > - eth = __skb_header_pointer(skb, nhoff, > - sizeof(_eth), > - data, hlen, &_eth); > - if (!eth) > - goto out_bad; > - proto = eth->h_proto; > - nhoff += sizeof(*eth); > - > - /* Cap headers that we access via pointers at the > - * end of the Ethernet header as our maximum alignment > - * at that point is only 2 bytes. > - */ > - if (NET_IP_ALIGN) > - hlen = nhoff; > - } > > - key_control->flags |= FLOW_DIS_ENCAPSULATION; > - if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) > - goto out_good; > + if (hdr->flags & GRE_SEQ) > + offset += sizeof(((struct pptp_gre_header *)0)->seq); > + > + if (hdr->flags & GRE_ACK) > + offset += sizeof(((struct pptp_gre_header *)0)->ack); Much better. -Philip > + > + if (proto == GRE_PROTO_PPP) { > + u8 _ppp_hdr[PPP_HDRLEN]; > + u8 *ppp_hdr; > + > + ppp_hdr = skb_header_pointer(skb, nhoff + offset, > + sizeof(_ppp_hdr), _ppp_hdr); > + if (!ppp_hdr) > + goto out_bad; > + > + proto = PPP_PROTOCOL(ppp_hdr); > + if (proto == PPP_IP) > + proto = htons(ETH_P_IP); > + else if (proto == PPP_IPV6) > + proto = htons(ETH_P_IPV6); > + else > + break; > + > + offset += PPP_HDRLEN; > + } else if (proto == htons(ETH_P_TEB)) { > + const struct ethhdr *eth; > + struct ethhdr _eth; > + > + eth = __skb_header_pointer(skb, nhoff + offset, > + sizeof(_eth), > + data, hlen, &_eth); > + if (!eth) > + goto out_bad; > + proto = eth->h_proto; > + offset += sizeof(*eth); > + > + /* Cap headers that we access via pointers at the > + * end of the Ethernet header as our maximum alignment > + * at that point is only 2 bytes. > + */ > + if (NET_IP_ALIGN) > + hlen = (nhoff + offset); > + } > > - goto again; > + nhoff += offset; > + key_control->flags |= FLOW_DIS_ENCAPSULATION; > + if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) > + goto out_good; > + > + goto again; > + } > + break; > } > case NEXTHDR_HOP: > case NEXTHDR_ROUTING: > -- > 1.9.1 > [-- Attachment #2: Message signed with OpenPGP using GPGMail --] [-- Type: application/pgp-signature, Size: 496 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash 2016-08-03 20:43 ` Philip Prindeville @ 2016-08-03 23:58 ` Feng Gao 2016-08-04 0:33 ` Philp Prindeville 0 siblings, 1 reply; 8+ messages in thread From: Feng Gao @ 2016-08-03 23:58 UTC (permalink / raw) To: Philip Prindeville Cc: fgao, David S. Miller, Stephen Hemminger, Pravin B Shelar, Tom Herbert, Alex Duyck, Linux Kernel Network Developers inline comment. There are two comments that I am not clear. Best Regards Feng On Thu, Aug 4, 2016 at 4:43 AM, Philip Prindeville <philipp@redfish-solutions.com> wrote: > Inline… > >> On Aug 3, 2016, at 8:52 AM, fgao@48lvckh6395k16k5.yundunddos.com wrote: >> >> From: Gao Feng <fgao@ikuai8.com> >> >> The PPTP is encapsulated by GRE header with that GRE_VERSION bits >> must contain one. But current GRE RPS needs the GRE_VERSION must be >> zero. So RPS does not work for PPTP traffic. >> >> In my test environment, there are four MIPS cores, and all traffic >> are passed through by PPTP. As a result, only one core is 100% busy >> while other three cores are very idle. After this patch, the usage >> of four cores are balanced well. >> >> Signed-off-by: Gao Feng <fgao@ikuai8.com> >> --- >> v3: 1) Move struct pptp_gre_header defination into new file pptp.h >> 2) Use sizeof GRE and PPTP type instead of literal value; >> 3) Remove strict flag check for PPTP to robust; >> 4) Consolidate the codes again; >> v2: Update according to Tom and Philp's advice. >> 1) Consolidate the codes with GRE version 0 path; >> 2) Use PPP_PROTOCOL to get ppp protol; >> 3) Set the FLOW_DIS_ENCAPSULATION flag; >> v1: Intial patch >> >> drivers/net/ppp/pptp.c | 36 +---------- >> include/net/pptp.h | 40 ++++++++++++ >> include/uapi/linux/if_tunnel.h | 7 +- >> net/core/flow_dissector.c | 141 +++++++++++++++++++++++++---------------- >> 4 files changed, 134 insertions(+), 90 deletions(-) >> create mode 100644 include/net/pptp.h >> >> diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c >> index ae0905e..3e68dbc 100644 >> --- a/drivers/net/ppp/pptp.c >> +++ b/drivers/net/ppp/pptp.c >> @@ -37,6 +37,7 @@ >> #include <net/icmp.h> >> #include <net/route.h> >> #include <net/gre.h> >> +#include <net/pptp.h> >> >> #include <linux/uaccess.h> >> >> @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly; >> static const struct ppp_channel_ops pptp_chan_ops; >> static const struct proto_ops pptp_ops; >> >> -#define PPP_LCP_ECHOREQ 0x09 >> -#define PPP_LCP_ECHOREP 0x0A >> -#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >> - >> -#define MISSING_WINDOW 20 >> -#define WRAPPED(curseq, lastseq)\ >> - ((((curseq) & 0xffffff00) == 0) &&\ >> - (((lastseq) & 0xffffff00) == 0xffffff00)) >> - >> -#define PPTP_GRE_PROTO 0x880B >> -#define PPTP_GRE_VER 0x1 >> - >> -#define PPTP_GRE_FLAG_C 0x80 >> -#define PPTP_GRE_FLAG_R 0x40 >> -#define PPTP_GRE_FLAG_K 0x20 >> -#define PPTP_GRE_FLAG_S 0x10 >> -#define PPTP_GRE_FLAG_A 0x80 >> - >> -#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >> -#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >> -#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >> -#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >> -#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >> - >> -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >> -struct pptp_gre_header { >> - u8 flags; >> - u8 ver; >> - __be16 protocol; >> - __be16 payload_len; >> - __be16 call_id; >> - __be32 seq; >> - __be32 ack; >> -} __packed; >> - >> static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr) >> { >> struct pppox_sock *sock; >> diff --git a/include/net/pptp.h b/include/net/pptp.h >> new file mode 100644 >> index 0000000..301d3e2 >> --- /dev/null >> +++ b/include/net/pptp.h >> @@ -0,0 +1,40 @@ >> +#ifndef _NET_PPTP_H >> +#define _NET_PPTP_H >> + >> +#define PPP_LCP_ECHOREQ 0x09 >> +#define PPP_LCP_ECHOREP 0x0A >> +#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >> + >> +#define MISSING_WINDOW 20 >> +#define WRAPPED(curseq, lastseq)\ >> + ((((curseq) & 0xffffff00) == 0) &&\ >> + (((lastseq) & 0xffffff00) == 0xffffff00)) >> + >> +#define PPTP_GRE_PROTO 0x880B >> +#define PPTP_GRE_VER 0x1 > > What about macros for accessing the lower 3 bits of the version? There is already one macro "GRE_VERSION" as the mask to get version. > > >> + >> +#define PPTP_GRE_FLAG_C 0x80 >> +#define PPTP_GRE_FLAG_R 0x40 >> +#define PPTP_GRE_FLAG_K 0x20 >> +#define PPTP_GRE_FLAG_S 0x10 >> +#define PPTP_GRE_FLAG_A 0x80 >> + >> +#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >> +#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >> +#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >> +#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >> +#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >> + >> +#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >> +struct pptp_gre_header { >> + u8 flags; >> + u8 ver; >> + __be16 protocol; >> + __be16 payload_len; >> + __be16 call_id; >> + __be32 seq; >> + __be32 ack; >> +} __packed; > > > What about a definition of a V0 (RFC-1701) packet? We’re handling both, so it makes sense to define both. I don't get you. The struct "gre_base_hdr" is defined in gre.h. Do you mean define them in same file ? > > >> + >> + >> +#endif >> diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h >> index 1046f55..7d889db 100644 >> --- a/include/uapi/linux/if_tunnel.h >> +++ b/include/uapi/linux/if_tunnel.h >> @@ -24,9 +24,14 @@ >> #define GRE_SEQ __cpu_to_be16(0x1000) >> #define GRE_STRICT __cpu_to_be16(0x0800) >> #define GRE_REC __cpu_to_be16(0x0700) >> -#define GRE_FLAGS __cpu_to_be16(0x00F8) >> +#define GRE_ACK __cpu_to_be16(0x0080) >> +#define GRE_FLAGS __cpu_to_be16(0x0078) >> #define GRE_VERSION __cpu_to_be16(0x0007) >> >> +#define GRE_VERSION_1 __cpu_to_be16(0x0001) >> +#define GRE_PROTO_PPP __cpu_to_be16(0x880b) >> + >> + >> struct ip_tunnel_parm { >> char name[IFNAMSIZ]; >> int link; >> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c >> index 61ad43f..52b7c3c 100644 >> --- a/net/core/flow_dissector.c >> +++ b/net/core/flow_dissector.c >> @@ -6,6 +6,8 @@ >> #include <linux/if_vlan.h> >> #include <net/ip.h> >> #include <net/ipv6.h> >> +#include <net/gre.h> >> +#include <net/pptp.h> >> #include <linux/igmp.h> >> #include <linux/icmp.h> >> #include <linux/sctp.h> >> @@ -338,71 +340,102 @@ mpls: >> ip_proto_again: >> switch (ip_proto) { >> case IPPROTO_GRE: { >> - struct gre_hdr { >> - __be16 flags; >> - __be16 proto; >> - } *hdr, _hdr; >> + struct gre_base_hdr *hdr, _hdr; >> >> hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr); >> if (!hdr) >> goto out_bad; >> - /* >> - * Only look inside GRE if version zero and no >> - * routing >> - */ >> - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) >> - break; >> >> - proto = hdr->proto; >> - nhoff += 4; >> - if (hdr->flags & GRE_CSUM) >> - nhoff += 4; >> - if (hdr->flags & GRE_KEY) { >> - const __be32 *keyid; >> - __be32 _keyid; >> + /* Only look inside GRE without routing */ >> + if (!(hdr->flags & GRE_ROUTING)) { >> + int offset = 0; >> >> - keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid), >> - data, hlen, &_keyid); >> + proto = hdr->protocol; >> >> - if (!keyid) >> - goto out_bad; >> + if (hdr->flags & GRE_VERSION) { >> + /* Maybe PPTP in GRE */ >> + if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY) && >> + (hdr->flags & GRE_VERSION) == GRE_VERSION_1)) >> + break; >> + } >> >> - if (dissector_uses_key(flow_dissector, >> - FLOW_DISSECTOR_KEY_GRE_KEYID)) { >> - key_keyid = skb_flow_dissector_target(flow_dissector, >> - FLOW_DISSECTOR_KEY_GRE_KEYID, >> - target_container); >> - key_keyid->keyid = *keyid; >> + offset += sizeof(struct gre_base_hdr); >> + >> + if (hdr->flags & GRE_CSUM) >> + offset += sizeof(__be32); > > > This doesn’t tell me as much as taking the sizeof() of the particular field (by name) in the packet that you’re skipping. Best way to do this is naming the field in the structure… > > >> + >> + if (hdr->flags & GRE_KEY) { >> + const __be32 *keyid; >> + __be32 _keyid; >> + >> + keyid = __skb_header_pointer(skb, nhoff + offset, sizeof(_keyid), >> + data, hlen, &_keyid); >> + >> + if (!keyid) >> + goto out_bad; >> + >> + if (dissector_uses_key(flow_dissector, >> + FLOW_DISSECTOR_KEY_GRE_KEYID)) { >> + key_keyid = skb_flow_dissector_target(flow_dissector, >> + FLOW_DISSECTOR_KEY_GRE_KEYID, >> + target_container); >> + key_keyid->keyid = *keyid; >> + } >> + offset += sizeof(_keyid); > > > Same issue here. > > >> } >> - nhoff += 4; >> - } >> - if (hdr->flags & GRE_SEQ) >> - nhoff += 4; >> - if (proto == htons(ETH_P_TEB)) { >> - const struct ethhdr *eth; >> - struct ethhdr _eth; >> - >> - eth = __skb_header_pointer(skb, nhoff, >> - sizeof(_eth), >> - data, hlen, &_eth); >> - if (!eth) >> - goto out_bad; >> - proto = eth->h_proto; >> - nhoff += sizeof(*eth); >> - >> - /* Cap headers that we access via pointers at the >> - * end of the Ethernet header as our maximum alignment >> - * at that point is only 2 bytes. >> - */ >> - if (NET_IP_ALIGN) >> - hlen = nhoff; >> - } >> >> - key_control->flags |= FLOW_DIS_ENCAPSULATION; >> - if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >> - goto out_good; >> + if (hdr->flags & GRE_SEQ) >> + offset += sizeof(((struct pptp_gre_header *)0)->seq); >> + >> + if (hdr->flags & GRE_ACK) >> + offset += sizeof(((struct pptp_gre_header *)0)->ack); > > > Much better. > > -Philip > > >> + >> + if (proto == GRE_PROTO_PPP) { >> + u8 _ppp_hdr[PPP_HDRLEN]; >> + u8 *ppp_hdr; >> + >> + ppp_hdr = skb_header_pointer(skb, nhoff + offset, >> + sizeof(_ppp_hdr), _ppp_hdr); >> + if (!ppp_hdr) >> + goto out_bad; >> + >> + proto = PPP_PROTOCOL(ppp_hdr); >> + if (proto == PPP_IP) >> + proto = htons(ETH_P_IP); >> + else if (proto == PPP_IPV6) >> + proto = htons(ETH_P_IPV6); >> + else >> + break; >> + >> + offset += PPP_HDRLEN; >> + } else if (proto == htons(ETH_P_TEB)) { >> + const struct ethhdr *eth; >> + struct ethhdr _eth; >> + >> + eth = __skb_header_pointer(skb, nhoff + offset, >> + sizeof(_eth), >> + data, hlen, &_eth); >> + if (!eth) >> + goto out_bad; >> + proto = eth->h_proto; >> + offset += sizeof(*eth); >> + >> + /* Cap headers that we access via pointers at the >> + * end of the Ethernet header as our maximum alignment >> + * at that point is only 2 bytes. >> + */ >> + if (NET_IP_ALIGN) >> + hlen = (nhoff + offset); >> + } >> >> - goto again; >> + nhoff += offset; >> + key_control->flags |= FLOW_DIS_ENCAPSULATION; >> + if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >> + goto out_good; >> + >> + goto again; >> + } >> + break; >> } >> case NEXTHDR_HOP: >> case NEXTHDR_ROUTING: >> -- >> 1.9.1 >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash 2016-08-03 23:58 ` Feng Gao @ 2016-08-04 0:33 ` Philp Prindeville 2016-08-04 7:37 ` Feng Gao 0 siblings, 1 reply; 8+ messages in thread From: Philp Prindeville @ 2016-08-04 0:33 UTC (permalink / raw) To: Feng Gao Cc: fgao, David S. Miller, Stephen Hemminger, Pravin B Shelar, Tom Herbert, Alex Duyck, Linux Kernel Network Developers Inline On 08/03/2016 05:58 PM, Feng Gao wrote: > inline comment. > There are two comments that I am not clear. > > Best Regards > Feng > > On Thu, Aug 4, 2016 at 4:43 AM, Philip Prindeville > <philipp@redfish-solutions.com> wrote: >> Inline… >> >>> On Aug 3, 2016, at 8:52 AM, fgao@48lvckh6395k16k5.yundunddos.com wrote: >>> >>> From: Gao Feng <fgao@ikuai8.com> >>> >>> The PPTP is encapsulated by GRE header with that GRE_VERSION bits >>> must contain one. But current GRE RPS needs the GRE_VERSION must be >>> zero. So RPS does not work for PPTP traffic. >>> >>> In my test environment, there are four MIPS cores, and all traffic >>> are passed through by PPTP. As a result, only one core is 100% busy >>> while other three cores are very idle. After this patch, the usage >>> of four cores are balanced well. >>> >>> Signed-off-by: Gao Feng <fgao@ikuai8.com> >>> --- >>> v3: 1) Move struct pptp_gre_header defination into new file pptp.h >>> 2) Use sizeof GRE and PPTP type instead of literal value; >>> 3) Remove strict flag check for PPTP to robust; >>> 4) Consolidate the codes again; >>> v2: Update according to Tom and Philp's advice. >>> 1) Consolidate the codes with GRE version 0 path; >>> 2) Use PPP_PROTOCOL to get ppp protol; >>> 3) Set the FLOW_DIS_ENCAPSULATION flag; >>> v1: Intial patch >>> >>> drivers/net/ppp/pptp.c | 36 +---------- >>> include/net/pptp.h | 40 ++++++++++++ >>> include/uapi/linux/if_tunnel.h | 7 +- >>> net/core/flow_dissector.c | 141 +++++++++++++++++++++++++---------------- >>> 4 files changed, 134 insertions(+), 90 deletions(-) >>> create mode 100644 include/net/pptp.h >>> >>> diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c >>> index ae0905e..3e68dbc 100644 >>> --- a/drivers/net/ppp/pptp.c >>> +++ b/drivers/net/ppp/pptp.c >>> @@ -37,6 +37,7 @@ >>> #include <net/icmp.h> >>> #include <net/route.h> >>> #include <net/gre.h> >>> +#include <net/pptp.h> >>> >>> #include <linux/uaccess.h> >>> >>> @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly; >>> static const struct ppp_channel_ops pptp_chan_ops; >>> static const struct proto_ops pptp_ops; >>> >>> -#define PPP_LCP_ECHOREQ 0x09 >>> -#define PPP_LCP_ECHOREP 0x0A >>> -#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >>> - >>> -#define MISSING_WINDOW 20 >>> -#define WRAPPED(curseq, lastseq)\ >>> - ((((curseq) & 0xffffff00) == 0) &&\ >>> - (((lastseq) & 0xffffff00) == 0xffffff00)) >>> - >>> -#define PPTP_GRE_PROTO 0x880B >>> -#define PPTP_GRE_VER 0x1 >>> - >>> -#define PPTP_GRE_FLAG_C 0x80 >>> -#define PPTP_GRE_FLAG_R 0x40 >>> -#define PPTP_GRE_FLAG_K 0x20 >>> -#define PPTP_GRE_FLAG_S 0x10 >>> -#define PPTP_GRE_FLAG_A 0x80 >>> - >>> -#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >>> -#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >>> -#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >>> -#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >>> -#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >>> - >>> -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >>> -struct pptp_gre_header { >>> - u8 flags; >>> - u8 ver; >>> - __be16 protocol; >>> - __be16 payload_len; >>> - __be16 call_id; >>> - __be32 seq; >>> - __be32 ack; >>> -} __packed; >>> - >>> static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr) >>> { >>> struct pppox_sock *sock; >>> diff --git a/include/net/pptp.h b/include/net/pptp.h >>> new file mode 100644 >>> index 0000000..301d3e2 >>> --- /dev/null >>> +++ b/include/net/pptp.h >>> @@ -0,0 +1,40 @@ >>> +#ifndef _NET_PPTP_H >>> +#define _NET_PPTP_H >>> + >>> +#define PPP_LCP_ECHOREQ 0x09 >>> +#define PPP_LCP_ECHOREP 0x0A >>> +#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >>> + >>> +#define MISSING_WINDOW 20 >>> +#define WRAPPED(curseq, lastseq)\ >>> + ((((curseq) & 0xffffff00) == 0) &&\ >>> + (((lastseq) & 0xffffff00) == 0xffffff00)) >>> + >>> +#define PPTP_GRE_PROTO 0x880B >>> +#define PPTP_GRE_VER 0x1 >> What about macros for accessing the lower 3 bits of the version? > There is already one macro "GRE_VERSION" as the mask to get version. Yup, sorry. Missed that. > >> >>> + >>> +#define PPTP_GRE_FLAG_C 0x80 >>> +#define PPTP_GRE_FLAG_R 0x40 >>> +#define PPTP_GRE_FLAG_K 0x20 >>> +#define PPTP_GRE_FLAG_S 0x10 >>> +#define PPTP_GRE_FLAG_A 0x80 >>> + >>> +#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >>> +#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >>> +#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >>> +#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >>> +#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >>> + >>> +#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >>> +struct pptp_gre_header { >>> + u8 flags; >>> + u8 ver; >>> + __be16 protocol; >>> + __be16 payload_len; >>> + __be16 call_id; >>> + __be32 seq; >>> + __be32 ack; >>> +} __packed; >> >> What about a definition of a V0 (RFC-1701) packet? We’re handling both, so it makes sense to define both. > I don't get you. The struct "gre_base_hdr" is defined in gre.h. Do you > mean define them in same file ? Sorry, I phrased that poorly. Yes, they're both defined (in different headers)... but when you're parsing the v0 header you're not referencing the gre_base_hdr members to calculate your offsets. -Philip > >> >>> + >>> + >>> +#endif >>> diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h >>> index 1046f55..7d889db 100644 >>> --- a/include/uapi/linux/if_tunnel.h >>> +++ b/include/uapi/linux/if_tunnel.h >>> @@ -24,9 +24,14 @@ >>> #define GRE_SEQ __cpu_to_be16(0x1000) >>> #define GRE_STRICT __cpu_to_be16(0x0800) >>> #define GRE_REC __cpu_to_be16(0x0700) >>> -#define GRE_FLAGS __cpu_to_be16(0x00F8) >>> +#define GRE_ACK __cpu_to_be16(0x0080) >>> +#define GRE_FLAGS __cpu_to_be16(0x0078) >>> #define GRE_VERSION __cpu_to_be16(0x0007) >>> >>> +#define GRE_VERSION_1 __cpu_to_be16(0x0001) >>> +#define GRE_PROTO_PPP __cpu_to_be16(0x880b) >>> + >>> + >>> struct ip_tunnel_parm { >>> char name[IFNAMSIZ]; >>> int link; >>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c >>> index 61ad43f..52b7c3c 100644 >>> --- a/net/core/flow_dissector.c >>> +++ b/net/core/flow_dissector.c >>> @@ -6,6 +6,8 @@ >>> #include <linux/if_vlan.h> >>> #include <net/ip.h> >>> #include <net/ipv6.h> >>> +#include <net/gre.h> >>> +#include <net/pptp.h> >>> #include <linux/igmp.h> >>> #include <linux/icmp.h> >>> #include <linux/sctp.h> >>> @@ -338,71 +340,102 @@ mpls: >>> ip_proto_again: >>> switch (ip_proto) { >>> case IPPROTO_GRE: { >>> - struct gre_hdr { >>> - __be16 flags; >>> - __be16 proto; >>> - } *hdr, _hdr; >>> + struct gre_base_hdr *hdr, _hdr; >>> >>> hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr); >>> if (!hdr) >>> goto out_bad; >>> - /* >>> - * Only look inside GRE if version zero and no >>> - * routing >>> - */ >>> - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) >>> - break; >>> >>> - proto = hdr->proto; >>> - nhoff += 4; >>> - if (hdr->flags & GRE_CSUM) >>> - nhoff += 4; >>> - if (hdr->flags & GRE_KEY) { >>> - const __be32 *keyid; >>> - __be32 _keyid; >>> + /* Only look inside GRE without routing */ >>> + if (!(hdr->flags & GRE_ROUTING)) { >>> + int offset = 0; >>> >>> - keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid), >>> - data, hlen, &_keyid); >>> + proto = hdr->protocol; >>> >>> - if (!keyid) >>> - goto out_bad; >>> + if (hdr->flags & GRE_VERSION) { >>> + /* Maybe PPTP in GRE */ >>> + if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY) && >>> + (hdr->flags & GRE_VERSION) == GRE_VERSION_1)) >>> + break; >>> + } >>> >>> - if (dissector_uses_key(flow_dissector, >>> - FLOW_DISSECTOR_KEY_GRE_KEYID)) { >>> - key_keyid = skb_flow_dissector_target(flow_dissector, >>> - FLOW_DISSECTOR_KEY_GRE_KEYID, >>> - target_container); >>> - key_keyid->keyid = *keyid; >>> + offset += sizeof(struct gre_base_hdr); >>> + >>> + if (hdr->flags & GRE_CSUM) >>> + offset += sizeof(__be32); >> >> This doesn’t tell me as much as taking the sizeof() of the particular field (by name) in the packet that you’re skipping. Best way to do this is naming the field in the structure… >> >> >>> + >>> + if (hdr->flags & GRE_KEY) { >>> + const __be32 *keyid; >>> + __be32 _keyid; >>> + >>> + keyid = __skb_header_pointer(skb, nhoff + offset, sizeof(_keyid), >>> + data, hlen, &_keyid); >>> + >>> + if (!keyid) >>> + goto out_bad; >>> + >>> + if (dissector_uses_key(flow_dissector, >>> + FLOW_DISSECTOR_KEY_GRE_KEYID)) { >>> + key_keyid = skb_flow_dissector_target(flow_dissector, >>> + FLOW_DISSECTOR_KEY_GRE_KEYID, >>> + target_container); >>> + key_keyid->keyid = *keyid; >>> + } >>> + offset += sizeof(_keyid); >> >> Same issue here. >> >> >>> } >>> - nhoff += 4; >>> - } >>> - if (hdr->flags & GRE_SEQ) >>> - nhoff += 4; >>> - if (proto == htons(ETH_P_TEB)) { >>> - const struct ethhdr *eth; >>> - struct ethhdr _eth; >>> - >>> - eth = __skb_header_pointer(skb, nhoff, >>> - sizeof(_eth), >>> - data, hlen, &_eth); >>> - if (!eth) >>> - goto out_bad; >>> - proto = eth->h_proto; >>> - nhoff += sizeof(*eth); >>> - >>> - /* Cap headers that we access via pointers at the >>> - * end of the Ethernet header as our maximum alignment >>> - * at that point is only 2 bytes. >>> - */ >>> - if (NET_IP_ALIGN) >>> - hlen = nhoff; >>> - } >>> >>> - key_control->flags |= FLOW_DIS_ENCAPSULATION; >>> - if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >>> - goto out_good; >>> + if (hdr->flags & GRE_SEQ) >>> + offset += sizeof(((struct pptp_gre_header *)0)->seq); >>> + >>> + if (hdr->flags & GRE_ACK) >>> + offset += sizeof(((struct pptp_gre_header *)0)->ack); >> >> Much better. >> >> -Philip >> >> >>> + >>> + if (proto == GRE_PROTO_PPP) { >>> + u8 _ppp_hdr[PPP_HDRLEN]; >>> + u8 *ppp_hdr; >>> + >>> + ppp_hdr = skb_header_pointer(skb, nhoff + offset, >>> + sizeof(_ppp_hdr), _ppp_hdr); >>> + if (!ppp_hdr) >>> + goto out_bad; >>> + >>> + proto = PPP_PROTOCOL(ppp_hdr); >>> + if (proto == PPP_IP) >>> + proto = htons(ETH_P_IP); >>> + else if (proto == PPP_IPV6) >>> + proto = htons(ETH_P_IPV6); >>> + else >>> + break; >>> + >>> + offset += PPP_HDRLEN; >>> + } else if (proto == htons(ETH_P_TEB)) { >>> + const struct ethhdr *eth; >>> + struct ethhdr _eth; >>> + >>> + eth = __skb_header_pointer(skb, nhoff + offset, >>> + sizeof(_eth), >>> + data, hlen, &_eth); >>> + if (!eth) >>> + goto out_bad; >>> + proto = eth->h_proto; >>> + offset += sizeof(*eth); >>> + >>> + /* Cap headers that we access via pointers at the >>> + * end of the Ethernet header as our maximum alignment >>> + * at that point is only 2 bytes. >>> + */ >>> + if (NET_IP_ALIGN) >>> + hlen = (nhoff + offset); >>> + } >>> >>> - goto again; >>> + nhoff += offset; >>> + key_control->flags |= FLOW_DIS_ENCAPSULATION; >>> + if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >>> + goto out_good; >>> + >>> + goto again; >>> + } >>> + break; >>> } >>> case NEXTHDR_HOP: >>> case NEXTHDR_ROUTING: >>> -- >>> 1.9.1 >>> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash 2016-08-04 0:33 ` Philp Prindeville @ 2016-08-04 7:37 ` Feng Gao 2016-08-04 13:11 ` Feng Gao 0 siblings, 1 reply; 8+ messages in thread From: Feng Gao @ 2016-08-04 7:37 UTC (permalink / raw) To: Philp Prindeville Cc: fgao, David S. Miller, Stephen Hemminger, Pravin B Shelar, Tom Herbert, Alex Duyck, Linux Kernel Network Developers Hi Tom & Philp, The v4 patch is sent already. Could you help review again please? Tom, I follow your modification. Philp, I define one new struct gre_full_hdr which contains the completed gre header, for example, csum, key, and so on. And these members are not defined in gre_base_hdr. It is only used to offset the sizeof type. BTW, I find the struct and macro about pptp and gre are redundant. I want to refactor them in other patches. On Thu, Aug 4, 2016 at 8:33 AM, Philp Prindeville <philipp@redfish-solutions.com> wrote: > Inline > > > > On 08/03/2016 05:58 PM, Feng Gao wrote: >> >> inline comment. >> There are two comments that I am not clear. >> >> Best Regards >> Feng >> >> On Thu, Aug 4, 2016 at 4:43 AM, Philip Prindeville >> <philipp@redfish-solutions.com> wrote: >>> >>> Inline… >>> >>>> On Aug 3, 2016, at 8:52 AM, fgao@48lvckh6395k16k5.yundunddos.com wrote: >>>> >>>> From: Gao Feng <fgao@ikuai8.com> >>>> >>>> The PPTP is encapsulated by GRE header with that GRE_VERSION bits >>>> must contain one. But current GRE RPS needs the GRE_VERSION must be >>>> zero. So RPS does not work for PPTP traffic. >>>> >>>> In my test environment, there are four MIPS cores, and all traffic >>>> are passed through by PPTP. As a result, only one core is 100% busy >>>> while other three cores are very idle. After this patch, the usage >>>> of four cores are balanced well. >>>> >>>> Signed-off-by: Gao Feng <fgao@ikuai8.com> >>>> --- >>>> v3: 1) Move struct pptp_gre_header defination into new file pptp.h >>>> 2) Use sizeof GRE and PPTP type instead of literal value; >>>> 3) Remove strict flag check for PPTP to robust; >>>> 4) Consolidate the codes again; >>>> v2: Update according to Tom and Philp's advice. >>>> 1) Consolidate the codes with GRE version 0 path; >>>> 2) Use PPP_PROTOCOL to get ppp protol; >>>> 3) Set the FLOW_DIS_ENCAPSULATION flag; >>>> v1: Intial patch >>>> >>>> drivers/net/ppp/pptp.c | 36 +---------- >>>> include/net/pptp.h | 40 ++++++++++++ >>>> include/uapi/linux/if_tunnel.h | 7 +- >>>> net/core/flow_dissector.c | 141 >>>> +++++++++++++++++++++++++---------------- >>>> 4 files changed, 134 insertions(+), 90 deletions(-) >>>> create mode 100644 include/net/pptp.h >>>> >>>> diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c >>>> index ae0905e..3e68dbc 100644 >>>> --- a/drivers/net/ppp/pptp.c >>>> +++ b/drivers/net/ppp/pptp.c >>>> @@ -37,6 +37,7 @@ >>>> #include <net/icmp.h> >>>> #include <net/route.h> >>>> #include <net/gre.h> >>>> +#include <net/pptp.h> >>>> >>>> #include <linux/uaccess.h> >>>> >>>> @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly; >>>> static const struct ppp_channel_ops pptp_chan_ops; >>>> static const struct proto_ops pptp_ops; >>>> >>>> -#define PPP_LCP_ECHOREQ 0x09 >>>> -#define PPP_LCP_ECHOREP 0x0A >>>> -#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >>>> - >>>> -#define MISSING_WINDOW 20 >>>> -#define WRAPPED(curseq, lastseq)\ >>>> - ((((curseq) & 0xffffff00) == 0) &&\ >>>> - (((lastseq) & 0xffffff00) == 0xffffff00)) >>>> - >>>> -#define PPTP_GRE_PROTO 0x880B >>>> -#define PPTP_GRE_VER 0x1 >>>> - >>>> -#define PPTP_GRE_FLAG_C 0x80 >>>> -#define PPTP_GRE_FLAG_R 0x40 >>>> -#define PPTP_GRE_FLAG_K 0x20 >>>> -#define PPTP_GRE_FLAG_S 0x10 >>>> -#define PPTP_GRE_FLAG_A 0x80 >>>> - >>>> -#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >>>> -#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >>>> -#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >>>> -#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >>>> -#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >>>> - >>>> -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >>>> -struct pptp_gre_header { >>>> - u8 flags; >>>> - u8 ver; >>>> - __be16 protocol; >>>> - __be16 payload_len; >>>> - __be16 call_id; >>>> - __be32 seq; >>>> - __be32 ack; >>>> -} __packed; >>>> - >>>> static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr) >>>> { >>>> struct pppox_sock *sock; >>>> diff --git a/include/net/pptp.h b/include/net/pptp.h >>>> new file mode 100644 >>>> index 0000000..301d3e2 >>>> --- /dev/null >>>> +++ b/include/net/pptp.h >>>> @@ -0,0 +1,40 @@ >>>> +#ifndef _NET_PPTP_H >>>> +#define _NET_PPTP_H >>>> + >>>> +#define PPP_LCP_ECHOREQ 0x09 >>>> +#define PPP_LCP_ECHOREP 0x0A >>>> +#define SC_RCV_BITS >>>> (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >>>> + >>>> +#define MISSING_WINDOW 20 >>>> +#define WRAPPED(curseq, lastseq)\ >>>> + ((((curseq) & 0xffffff00) == 0) &&\ >>>> + (((lastseq) & 0xffffff00) == 0xffffff00)) >>>> + >>>> +#define PPTP_GRE_PROTO 0x880B >>>> +#define PPTP_GRE_VER 0x1 >>> >>> What about macros for accessing the lower 3 bits of the version? >> >> There is already one macro "GRE_VERSION" as the mask to get version. > > > Yup, sorry. Missed that. > > > >> >>> >>>> + >>>> +#define PPTP_GRE_FLAG_C 0x80 >>>> +#define PPTP_GRE_FLAG_R 0x40 >>>> +#define PPTP_GRE_FLAG_K 0x20 >>>> +#define PPTP_GRE_FLAG_S 0x10 >>>> +#define PPTP_GRE_FLAG_A 0x80 >>>> + >>>> +#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >>>> +#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >>>> +#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >>>> +#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >>>> +#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >>>> + >>>> +#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >>>> +struct pptp_gre_header { >>>> + u8 flags; >>>> + u8 ver; >>>> + __be16 protocol; >>>> + __be16 payload_len; >>>> + __be16 call_id; >>>> + __be32 seq; >>>> + __be32 ack; >>>> +} __packed; >>> >>> >>> What about a definition of a V0 (RFC-1701) packet? We’re handling both, >>> so it makes sense to define both. >> >> I don't get you. The struct "gre_base_hdr" is defined in gre.h. Do you >> mean define them in same file ? > > > Sorry, I phrased that poorly. Yes, they're both defined (in different > headers)... but when you're parsing the v0 header you're not referencing the > gre_base_hdr members to calculate your offsets. > > -Philip > > > >> >>> >>>> + >>>> + >>>> +#endif >>>> diff --git a/include/uapi/linux/if_tunnel.h >>>> b/include/uapi/linux/if_tunnel.h >>>> index 1046f55..7d889db 100644 >>>> --- a/include/uapi/linux/if_tunnel.h >>>> +++ b/include/uapi/linux/if_tunnel.h >>>> @@ -24,9 +24,14 @@ >>>> #define GRE_SEQ __cpu_to_be16(0x1000) >>>> #define GRE_STRICT __cpu_to_be16(0x0800) >>>> #define GRE_REC __cpu_to_be16(0x0700) >>>> -#define GRE_FLAGS __cpu_to_be16(0x00F8) >>>> +#define GRE_ACK __cpu_to_be16(0x0080) >>>> +#define GRE_FLAGS __cpu_to_be16(0x0078) >>>> #define GRE_VERSION __cpu_to_be16(0x0007) >>>> >>>> +#define GRE_VERSION_1 __cpu_to_be16(0x0001) >>>> +#define GRE_PROTO_PPP __cpu_to_be16(0x880b) >>>> + >>>> + >>>> struct ip_tunnel_parm { >>>> char name[IFNAMSIZ]; >>>> int link; >>>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c >>>> index 61ad43f..52b7c3c 100644 >>>> --- a/net/core/flow_dissector.c >>>> +++ b/net/core/flow_dissector.c >>>> @@ -6,6 +6,8 @@ >>>> #include <linux/if_vlan.h> >>>> #include <net/ip.h> >>>> #include <net/ipv6.h> >>>> +#include <net/gre.h> >>>> +#include <net/pptp.h> >>>> #include <linux/igmp.h> >>>> #include <linux/icmp.h> >>>> #include <linux/sctp.h> >>>> @@ -338,71 +340,102 @@ mpls: >>>> ip_proto_again: >>>> switch (ip_proto) { >>>> case IPPROTO_GRE: { >>>> - struct gre_hdr { >>>> - __be16 flags; >>>> - __be16 proto; >>>> - } *hdr, _hdr; >>>> + struct gre_base_hdr *hdr, _hdr; >>>> >>>> hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), >>>> data, hlen, &_hdr); >>>> if (!hdr) >>>> goto out_bad; >>>> - /* >>>> - * Only look inside GRE if version zero and no >>>> - * routing >>>> - */ >>>> - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) >>>> - break; >>>> >>>> - proto = hdr->proto; >>>> - nhoff += 4; >>>> - if (hdr->flags & GRE_CSUM) >>>> - nhoff += 4; >>>> - if (hdr->flags & GRE_KEY) { >>>> - const __be32 *keyid; >>>> - __be32 _keyid; >>>> + /* Only look inside GRE without routing */ >>>> + if (!(hdr->flags & GRE_ROUTING)) { >>>> + int offset = 0; >>>> >>>> - keyid = __skb_header_pointer(skb, nhoff, >>>> sizeof(_keyid), >>>> - data, hlen, &_keyid); >>>> + proto = hdr->protocol; >>>> >>>> - if (!keyid) >>>> - goto out_bad; >>>> + if (hdr->flags & GRE_VERSION) { >>>> + /* Maybe PPTP in GRE */ >>>> + if (!(proto == GRE_PROTO_PPP && >>>> (hdr->flags & GRE_KEY) && >>>> + (hdr->flags & GRE_VERSION) == >>>> GRE_VERSION_1)) >>>> + break; >>>> + } >>>> >>>> - if (dissector_uses_key(flow_dissector, >>>> - >>>> FLOW_DISSECTOR_KEY_GRE_KEYID)) { >>>> - key_keyid = >>>> skb_flow_dissector_target(flow_dissector, >>>> - >>>> FLOW_DISSECTOR_KEY_GRE_KEYID, >>>> - >>>> target_container); >>>> - key_keyid->keyid = *keyid; >>>> + offset += sizeof(struct gre_base_hdr); >>>> + >>>> + if (hdr->flags & GRE_CSUM) >>>> + offset += sizeof(__be32); >>> >>> >>> This doesn’t tell me as much as taking the sizeof() of the particular >>> field (by name) in the packet that you’re skipping. Best way to do this is >>> naming the field in the structure… >>> >>> >>>> + >>>> + if (hdr->flags & GRE_KEY) { >>>> + const __be32 *keyid; >>>> + __be32 _keyid; >>>> + >>>> + keyid = __skb_header_pointer(skb, nhoff + >>>> offset, sizeof(_keyid), >>>> + data, hlen, >>>> &_keyid); >>>> + >>>> + if (!keyid) >>>> + goto out_bad; >>>> + >>>> + if (dissector_uses_key(flow_dissector, >>>> + >>>> FLOW_DISSECTOR_KEY_GRE_KEYID)) { >>>> + key_keyid = >>>> skb_flow_dissector_target(flow_dissector, >>>> + >>>> FLOW_DISSECTOR_KEY_GRE_KEYID, >>>> + >>>> target_container); >>>> + key_keyid->keyid = *keyid; >>>> + } >>>> + offset += sizeof(_keyid); >>> >>> >>> Same issue here. >>> >>> >>>> } >>>> - nhoff += 4; >>>> - } >>>> - if (hdr->flags & GRE_SEQ) >>>> - nhoff += 4; >>>> - if (proto == htons(ETH_P_TEB)) { >>>> - const struct ethhdr *eth; >>>> - struct ethhdr _eth; >>>> - >>>> - eth = __skb_header_pointer(skb, nhoff, >>>> - sizeof(_eth), >>>> - data, hlen, &_eth); >>>> - if (!eth) >>>> - goto out_bad; >>>> - proto = eth->h_proto; >>>> - nhoff += sizeof(*eth); >>>> - >>>> - /* Cap headers that we access via pointers at the >>>> - * end of the Ethernet header as our maximum >>>> alignment >>>> - * at that point is only 2 bytes. >>>> - */ >>>> - if (NET_IP_ALIGN) >>>> - hlen = nhoff; >>>> - } >>>> >>>> - key_control->flags |= FLOW_DIS_ENCAPSULATION; >>>> - if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >>>> - goto out_good; >>>> + if (hdr->flags & GRE_SEQ) >>>> + offset += sizeof(((struct pptp_gre_header >>>> *)0)->seq); >>>> + >>>> + if (hdr->flags & GRE_ACK) >>>> + offset += sizeof(((struct pptp_gre_header >>>> *)0)->ack); >>> >>> >>> Much better. >>> >>> -Philip >>> >>> >>>> + >>>> + if (proto == GRE_PROTO_PPP) { >>>> + u8 _ppp_hdr[PPP_HDRLEN]; >>>> + u8 *ppp_hdr; >>>> + >>>> + ppp_hdr = skb_header_pointer(skb, nhoff + >>>> offset, >>>> + >>>> sizeof(_ppp_hdr), _ppp_hdr); >>>> + if (!ppp_hdr) >>>> + goto out_bad; >>>> + >>>> + proto = PPP_PROTOCOL(ppp_hdr); >>>> + if (proto == PPP_IP) >>>> + proto = htons(ETH_P_IP); >>>> + else if (proto == PPP_IPV6) >>>> + proto = htons(ETH_P_IPV6); >>>> + else >>>> + break; >>>> + >>>> + offset += PPP_HDRLEN; >>>> + } else if (proto == htons(ETH_P_TEB)) { >>>> + const struct ethhdr *eth; >>>> + struct ethhdr _eth; >>>> + >>>> + eth = __skb_header_pointer(skb, nhoff + >>>> offset, >>>> + sizeof(_eth), >>>> + data, hlen, >>>> &_eth); >>>> + if (!eth) >>>> + goto out_bad; >>>> + proto = eth->h_proto; >>>> + offset += sizeof(*eth); >>>> + >>>> + /* Cap headers that we access via pointers >>>> at the >>>> + * end of the Ethernet header as our >>>> maximum alignment >>>> + * at that point is only 2 bytes. >>>> + */ >>>> + if (NET_IP_ALIGN) >>>> + hlen = (nhoff + offset); >>>> + } >>>> >>>> - goto again; >>>> + nhoff += offset; >>>> + key_control->flags |= FLOW_DIS_ENCAPSULATION; >>>> + if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >>>> + goto out_good; >>>> + >>>> + goto again; >>>> + } >>>> + break; >>>> } >>>> case NEXTHDR_HOP: >>>> case NEXTHDR_ROUTING: >>>> -- >>>> 1.9.1 >>>> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash 2016-08-04 7:37 ` Feng Gao @ 2016-08-04 13:11 ` Feng Gao 0 siblings, 0 replies; 8+ messages in thread From: Feng Gao @ 2016-08-04 13:11 UTC (permalink / raw) To: Philp Prindeville Cc: fgao, David S. Miller, Stephen Hemminger, Pravin B Shelar, Tom Herbert, Alex Duyck, Linux Kernel Network Developers The link is https://patchwork.ozlabs.org/patch/655695/ Best Regards Feng On Thu, Aug 4, 2016 at 3:37 PM, Feng Gao <gfree.wind@gmail.com> wrote: > Hi Tom & Philp, > > The v4 patch is sent already. > Could you help review again please? > > Tom, > I follow your modification. > > Philp, > I define one new struct gre_full_hdr which contains the completed gre > header, for example, csum, key, and so on. > And these members are not defined in gre_base_hdr. > It is only used to offset the sizeof type. > > BTW, I find the struct and macro about pptp and gre are redundant. > I want to refactor them in other patches. > > On Thu, Aug 4, 2016 at 8:33 AM, Philp Prindeville > <philipp@redfish-solutions.com> wrote: >> Inline >> >> >> >> On 08/03/2016 05:58 PM, Feng Gao wrote: >>> >>> inline comment. >>> There are two comments that I am not clear. >>> >>> Best Regards >>> Feng >>> >>> On Thu, Aug 4, 2016 at 4:43 AM, Philip Prindeville >>> <philipp@redfish-solutions.com> wrote: >>>> >>>> Inline… >>>> >>>>> On Aug 3, 2016, at 8:52 AM, fgao@48lvckh6395k16k5.yundunddos.com wrote: >>>>> >>>>> From: Gao Feng <fgao@ikuai8.com> >>>>> >>>>> The PPTP is encapsulated by GRE header with that GRE_VERSION bits >>>>> must contain one. But current GRE RPS needs the GRE_VERSION must be >>>>> zero. So RPS does not work for PPTP traffic. >>>>> >>>>> In my test environment, there are four MIPS cores, and all traffic >>>>> are passed through by PPTP. As a result, only one core is 100% busy >>>>> while other three cores are very idle. After this patch, the usage >>>>> of four cores are balanced well. >>>>> >>>>> Signed-off-by: Gao Feng <fgao@ikuai8.com> >>>>> --- >>>>> v3: 1) Move struct pptp_gre_header defination into new file pptp.h >>>>> 2) Use sizeof GRE and PPTP type instead of literal value; >>>>> 3) Remove strict flag check for PPTP to robust; >>>>> 4) Consolidate the codes again; >>>>> v2: Update according to Tom and Philp's advice. >>>>> 1) Consolidate the codes with GRE version 0 path; >>>>> 2) Use PPP_PROTOCOL to get ppp protol; >>>>> 3) Set the FLOW_DIS_ENCAPSULATION flag; >>>>> v1: Intial patch >>>>> >>>>> drivers/net/ppp/pptp.c | 36 +---------- >>>>> include/net/pptp.h | 40 ++++++++++++ >>>>> include/uapi/linux/if_tunnel.h | 7 +- >>>>> net/core/flow_dissector.c | 141 >>>>> +++++++++++++++++++++++++---------------- >>>>> 4 files changed, 134 insertions(+), 90 deletions(-) >>>>> create mode 100644 include/net/pptp.h >>>>> >>>>> diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c >>>>> index ae0905e..3e68dbc 100644 >>>>> --- a/drivers/net/ppp/pptp.c >>>>> +++ b/drivers/net/ppp/pptp.c >>>>> @@ -37,6 +37,7 @@ >>>>> #include <net/icmp.h> >>>>> #include <net/route.h> >>>>> #include <net/gre.h> >>>>> +#include <net/pptp.h> >>>>> >>>>> #include <linux/uaccess.h> >>>>> >>>>> @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly; >>>>> static const struct ppp_channel_ops pptp_chan_ops; >>>>> static const struct proto_ops pptp_ops; >>>>> >>>>> -#define PPP_LCP_ECHOREQ 0x09 >>>>> -#define PPP_LCP_ECHOREP 0x0A >>>>> -#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >>>>> - >>>>> -#define MISSING_WINDOW 20 >>>>> -#define WRAPPED(curseq, lastseq)\ >>>>> - ((((curseq) & 0xffffff00) == 0) &&\ >>>>> - (((lastseq) & 0xffffff00) == 0xffffff00)) >>>>> - >>>>> -#define PPTP_GRE_PROTO 0x880B >>>>> -#define PPTP_GRE_VER 0x1 >>>>> - >>>>> -#define PPTP_GRE_FLAG_C 0x80 >>>>> -#define PPTP_GRE_FLAG_R 0x40 >>>>> -#define PPTP_GRE_FLAG_K 0x20 >>>>> -#define PPTP_GRE_FLAG_S 0x10 >>>>> -#define PPTP_GRE_FLAG_A 0x80 >>>>> - >>>>> -#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >>>>> -#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >>>>> -#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >>>>> -#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >>>>> -#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >>>>> - >>>>> -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >>>>> -struct pptp_gre_header { >>>>> - u8 flags; >>>>> - u8 ver; >>>>> - __be16 protocol; >>>>> - __be16 payload_len; >>>>> - __be16 call_id; >>>>> - __be32 seq; >>>>> - __be32 ack; >>>>> -} __packed; >>>>> - >>>>> static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr) >>>>> { >>>>> struct pppox_sock *sock; >>>>> diff --git a/include/net/pptp.h b/include/net/pptp.h >>>>> new file mode 100644 >>>>> index 0000000..301d3e2 >>>>> --- /dev/null >>>>> +++ b/include/net/pptp.h >>>>> @@ -0,0 +1,40 @@ >>>>> +#ifndef _NET_PPTP_H >>>>> +#define _NET_PPTP_H >>>>> + >>>>> +#define PPP_LCP_ECHOREQ 0x09 >>>>> +#define PPP_LCP_ECHOREP 0x0A >>>>> +#define SC_RCV_BITS >>>>> (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP) >>>>> + >>>>> +#define MISSING_WINDOW 20 >>>>> +#define WRAPPED(curseq, lastseq)\ >>>>> + ((((curseq) & 0xffffff00) == 0) &&\ >>>>> + (((lastseq) & 0xffffff00) == 0xffffff00)) >>>>> + >>>>> +#define PPTP_GRE_PROTO 0x880B >>>>> +#define PPTP_GRE_VER 0x1 >>>> >>>> What about macros for accessing the lower 3 bits of the version? >>> >>> There is already one macro "GRE_VERSION" as the mask to get version. >> >> >> Yup, sorry. Missed that. >> >> >> >>> >>>> >>>>> + >>>>> +#define PPTP_GRE_FLAG_C 0x80 >>>>> +#define PPTP_GRE_FLAG_R 0x40 >>>>> +#define PPTP_GRE_FLAG_K 0x20 >>>>> +#define PPTP_GRE_FLAG_S 0x10 >>>>> +#define PPTP_GRE_FLAG_A 0x80 >>>>> + >>>>> +#define PPTP_GRE_IS_C(f) ((f)&PPTP_GRE_FLAG_C) >>>>> +#define PPTP_GRE_IS_R(f) ((f)&PPTP_GRE_FLAG_R) >>>>> +#define PPTP_GRE_IS_K(f) ((f)&PPTP_GRE_FLAG_K) >>>>> +#define PPTP_GRE_IS_S(f) ((f)&PPTP_GRE_FLAG_S) >>>>> +#define PPTP_GRE_IS_A(f) ((f)&PPTP_GRE_FLAG_A) >>>>> + >>>>> +#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header)) >>>>> +struct pptp_gre_header { >>>>> + u8 flags; >>>>> + u8 ver; >>>>> + __be16 protocol; >>>>> + __be16 payload_len; >>>>> + __be16 call_id; >>>>> + __be32 seq; >>>>> + __be32 ack; >>>>> +} __packed; >>>> >>>> >>>> What about a definition of a V0 (RFC-1701) packet? We’re handling both, >>>> so it makes sense to define both. >>> >>> I don't get you. The struct "gre_base_hdr" is defined in gre.h. Do you >>> mean define them in same file ? >> >> >> Sorry, I phrased that poorly. Yes, they're both defined (in different >> headers)... but when you're parsing the v0 header you're not referencing the >> gre_base_hdr members to calculate your offsets. >> >> -Philip >> >> >> >>> >>>> >>>>> + >>>>> + >>>>> +#endif >>>>> diff --git a/include/uapi/linux/if_tunnel.h >>>>> b/include/uapi/linux/if_tunnel.h >>>>> index 1046f55..7d889db 100644 >>>>> --- a/include/uapi/linux/if_tunnel.h >>>>> +++ b/include/uapi/linux/if_tunnel.h >>>>> @@ -24,9 +24,14 @@ >>>>> #define GRE_SEQ __cpu_to_be16(0x1000) >>>>> #define GRE_STRICT __cpu_to_be16(0x0800) >>>>> #define GRE_REC __cpu_to_be16(0x0700) >>>>> -#define GRE_FLAGS __cpu_to_be16(0x00F8) >>>>> +#define GRE_ACK __cpu_to_be16(0x0080) >>>>> +#define GRE_FLAGS __cpu_to_be16(0x0078) >>>>> #define GRE_VERSION __cpu_to_be16(0x0007) >>>>> >>>>> +#define GRE_VERSION_1 __cpu_to_be16(0x0001) >>>>> +#define GRE_PROTO_PPP __cpu_to_be16(0x880b) >>>>> + >>>>> + >>>>> struct ip_tunnel_parm { >>>>> char name[IFNAMSIZ]; >>>>> int link; >>>>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c >>>>> index 61ad43f..52b7c3c 100644 >>>>> --- a/net/core/flow_dissector.c >>>>> +++ b/net/core/flow_dissector.c >>>>> @@ -6,6 +6,8 @@ >>>>> #include <linux/if_vlan.h> >>>>> #include <net/ip.h> >>>>> #include <net/ipv6.h> >>>>> +#include <net/gre.h> >>>>> +#include <net/pptp.h> >>>>> #include <linux/igmp.h> >>>>> #include <linux/icmp.h> >>>>> #include <linux/sctp.h> >>>>> @@ -338,71 +340,102 @@ mpls: >>>>> ip_proto_again: >>>>> switch (ip_proto) { >>>>> case IPPROTO_GRE: { >>>>> - struct gre_hdr { >>>>> - __be16 flags; >>>>> - __be16 proto; >>>>> - } *hdr, _hdr; >>>>> + struct gre_base_hdr *hdr, _hdr; >>>>> >>>>> hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), >>>>> data, hlen, &_hdr); >>>>> if (!hdr) >>>>> goto out_bad; >>>>> - /* >>>>> - * Only look inside GRE if version zero and no >>>>> - * routing >>>>> - */ >>>>> - if (hdr->flags & (GRE_VERSION | GRE_ROUTING)) >>>>> - break; >>>>> >>>>> - proto = hdr->proto; >>>>> - nhoff += 4; >>>>> - if (hdr->flags & GRE_CSUM) >>>>> - nhoff += 4; >>>>> - if (hdr->flags & GRE_KEY) { >>>>> - const __be32 *keyid; >>>>> - __be32 _keyid; >>>>> + /* Only look inside GRE without routing */ >>>>> + if (!(hdr->flags & GRE_ROUTING)) { >>>>> + int offset = 0; >>>>> >>>>> - keyid = __skb_header_pointer(skb, nhoff, >>>>> sizeof(_keyid), >>>>> - data, hlen, &_keyid); >>>>> + proto = hdr->protocol; >>>>> >>>>> - if (!keyid) >>>>> - goto out_bad; >>>>> + if (hdr->flags & GRE_VERSION) { >>>>> + /* Maybe PPTP in GRE */ >>>>> + if (!(proto == GRE_PROTO_PPP && >>>>> (hdr->flags & GRE_KEY) && >>>>> + (hdr->flags & GRE_VERSION) == >>>>> GRE_VERSION_1)) >>>>> + break; >>>>> + } >>>>> >>>>> - if (dissector_uses_key(flow_dissector, >>>>> - >>>>> FLOW_DISSECTOR_KEY_GRE_KEYID)) { >>>>> - key_keyid = >>>>> skb_flow_dissector_target(flow_dissector, >>>>> - >>>>> FLOW_DISSECTOR_KEY_GRE_KEYID, >>>>> - >>>>> target_container); >>>>> - key_keyid->keyid = *keyid; >>>>> + offset += sizeof(struct gre_base_hdr); >>>>> + >>>>> + if (hdr->flags & GRE_CSUM) >>>>> + offset += sizeof(__be32); >>>> >>>> >>>> This doesn’t tell me as much as taking the sizeof() of the particular >>>> field (by name) in the packet that you’re skipping. Best way to do this is >>>> naming the field in the structure… >>>> >>>> >>>>> + >>>>> + if (hdr->flags & GRE_KEY) { >>>>> + const __be32 *keyid; >>>>> + __be32 _keyid; >>>>> + >>>>> + keyid = __skb_header_pointer(skb, nhoff + >>>>> offset, sizeof(_keyid), >>>>> + data, hlen, >>>>> &_keyid); >>>>> + >>>>> + if (!keyid) >>>>> + goto out_bad; >>>>> + >>>>> + if (dissector_uses_key(flow_dissector, >>>>> + >>>>> FLOW_DISSECTOR_KEY_GRE_KEYID)) { >>>>> + key_keyid = >>>>> skb_flow_dissector_target(flow_dissector, >>>>> + >>>>> FLOW_DISSECTOR_KEY_GRE_KEYID, >>>>> + >>>>> target_container); >>>>> + key_keyid->keyid = *keyid; >>>>> + } >>>>> + offset += sizeof(_keyid); >>>> >>>> >>>> Same issue here. >>>> >>>> >>>>> } >>>>> - nhoff += 4; >>>>> - } >>>>> - if (hdr->flags & GRE_SEQ) >>>>> - nhoff += 4; >>>>> - if (proto == htons(ETH_P_TEB)) { >>>>> - const struct ethhdr *eth; >>>>> - struct ethhdr _eth; >>>>> - >>>>> - eth = __skb_header_pointer(skb, nhoff, >>>>> - sizeof(_eth), >>>>> - data, hlen, &_eth); >>>>> - if (!eth) >>>>> - goto out_bad; >>>>> - proto = eth->h_proto; >>>>> - nhoff += sizeof(*eth); >>>>> - >>>>> - /* Cap headers that we access via pointers at the >>>>> - * end of the Ethernet header as our maximum >>>>> alignment >>>>> - * at that point is only 2 bytes. >>>>> - */ >>>>> - if (NET_IP_ALIGN) >>>>> - hlen = nhoff; >>>>> - } >>>>> >>>>> - key_control->flags |= FLOW_DIS_ENCAPSULATION; >>>>> - if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >>>>> - goto out_good; >>>>> + if (hdr->flags & GRE_SEQ) >>>>> + offset += sizeof(((struct pptp_gre_header >>>>> *)0)->seq); >>>>> + >>>>> + if (hdr->flags & GRE_ACK) >>>>> + offset += sizeof(((struct pptp_gre_header >>>>> *)0)->ack); >>>> >>>> >>>> Much better. >>>> >>>> -Philip >>>> >>>> >>>>> + >>>>> + if (proto == GRE_PROTO_PPP) { >>>>> + u8 _ppp_hdr[PPP_HDRLEN]; >>>>> + u8 *ppp_hdr; >>>>> + >>>>> + ppp_hdr = skb_header_pointer(skb, nhoff + >>>>> offset, >>>>> + >>>>> sizeof(_ppp_hdr), _ppp_hdr); >>>>> + if (!ppp_hdr) >>>>> + goto out_bad; >>>>> + >>>>> + proto = PPP_PROTOCOL(ppp_hdr); >>>>> + if (proto == PPP_IP) >>>>> + proto = htons(ETH_P_IP); >>>>> + else if (proto == PPP_IPV6) >>>>> + proto = htons(ETH_P_IPV6); >>>>> + else >>>>> + break; >>>>> + >>>>> + offset += PPP_HDRLEN; >>>>> + } else if (proto == htons(ETH_P_TEB)) { >>>>> + const struct ethhdr *eth; >>>>> + struct ethhdr _eth; >>>>> + >>>>> + eth = __skb_header_pointer(skb, nhoff + >>>>> offset, >>>>> + sizeof(_eth), >>>>> + data, hlen, >>>>> &_eth); >>>>> + if (!eth) >>>>> + goto out_bad; >>>>> + proto = eth->h_proto; >>>>> + offset += sizeof(*eth); >>>>> + >>>>> + /* Cap headers that we access via pointers >>>>> at the >>>>> + * end of the Ethernet header as our >>>>> maximum alignment >>>>> + * at that point is only 2 bytes. >>>>> + */ >>>>> + if (NET_IP_ALIGN) >>>>> + hlen = (nhoff + offset); >>>>> + } >>>>> >>>>> - goto again; >>>>> + nhoff += offset; >>>>> + key_control->flags |= FLOW_DIS_ENCAPSULATION; >>>>> + if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) >>>>> + goto out_good; >>>>> + >>>>> + goto again; >>>>> + } >>>>> + break; >>>>> } >>>>> case NEXTHDR_HOP: >>>>> case NEXTHDR_ROUTING: >>>>> -- >>>>> 1.9.1 >>>>> >> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-04 13:23 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-03 14:52 [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash fgao 2016-08-03 16:15 ` Tom Herbert 2016-08-03 23:47 ` Feng Gao 2016-08-03 20:43 ` Philip Prindeville 2016-08-03 23:58 ` Feng Gao 2016-08-04 0:33 ` Philp Prindeville 2016-08-04 7:37 ` Feng Gao 2016-08-04 13:11 ` Feng Gao
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.