| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e185b3b9-77cc-bb8f-eaea-04a801859179@redfish-solutions.com>
Date: Sun, 7 Aug 2016 15:03:13 -0600
From: Philp Prindeville <philipp@...fish-solutions.com>
To: fgao@...vckh6395k16k5.yundunddos.com, davem@...emloft.net,
stephen@...workplumber.org, pshelar@...ira.com,
tom@...bertland.com, aduyck@...antis.com, netdev@...r.kernel.org
Cc: gfree.wind@...il.com
Subject: Re: [PATCH v4 1/1] rps: Inspect PPTP encapsulated by GRE to get flow
hash
Inline...
On 08/04/2016 01:06 AM, fgao@...vckh6395k16k5.yundunddos.com wrote:
> From: Gao Feng <fgao@...ai8.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@...ai8.com>
> ---
> v4: 1) Define struct gre_full_hdr, and use sizeof its member directly;
> 2) Move version and routing check ahead;
> 3) Only PPTP in GRE check the ack flag;
> 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/gre.h | 10 +++-
> include/net/pptp.h | 40 +++++++++++++++
> include/uapi/linux/if_tunnel.h | 7 ++-
> net/core/flow_dissector.c | 113 ++++++++++++++++++++++++++++-------------
> 5 files changed, 135 insertions(+), 71 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/gre.h b/include/net/gre.h
> index 7a54a31..c469dcc 100644
> --- a/include/net/gre.h
> +++ b/include/net/gre.h
> @@ -7,9 +7,17 @@
> struct gre_base_hdr {
> __be16 flags;
> __be16 protocol;
> -};
> +} __packed;
> #define GRE_HEADER_SECTION 4
>
> +struct gre_full_hdr {
> + struct gre_base_hdr fixed_header;
Can you make gre_base_hdr be anonymous?
> + __be16 csum;
> + __be16 reserved1;
> + __be32 key;
> + __be32 seq;
> +} __packed;
> +
> #define GREPROTO_CISCO 0
> #define GREPROTO_PPTP 1
> #define GREPROTO_MAX 2
> 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..1186712 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,32 +340,42 @@ 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;
> + u16 gre_ver;
> + int offset = 0;
>
> 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))
> +
> + /* Only look inside GRE without routing */
> + if (hdr->flags & GRE_ROUTING)
> break;
>
> - proto = hdr->proto;
> - nhoff += 4;
> + /* Only look inside GRE for version 0 and 1 */
> + gre_ver = ntohs(hdr->flags & GRE_VERSION);
> + if (gre_ver > 1)
> + break;
> +
> + proto = hdr->protocol;
> + if (gre_ver) {
> + /* Version1 must be PPTP, and check the flags */
> + if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY)))
> + break;
> + }
> +
> + offset += sizeof(struct gre_base_hdr);
> +
> if (hdr->flags & GRE_CSUM)
> - nhoff += 4;
> + offset += sizeof(((struct gre_full_hdr *)0)->csum) +
> + sizeof(((struct gre_full_hdr *)0)->reserved1);
> +
> if (hdr->flags & GRE_KEY) {
> const __be32 *keyid;
> __be32 _keyid;
>
> - keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid),
> + keyid = __skb_header_pointer(skb, nhoff + offset, sizeof(_keyid),
> data, hlen, &_keyid);
> -
> if (!keyid)
> goto out_bad;
>
> @@ -372,32 +384,65 @@ 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);
Would be good to have a #define'd constant for the keyid mask...
Otherwise, looks good.
-Philip
> }
> - nhoff += 4;
> + offset += sizeof(((struct gre_full_hdr *)0)->key);
> }
> +
> 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)
> + offset += sizeof(((struct pptp_gre_header *)0)->seq);
> +
> + if (gre_ver == 0) {
> + 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);
> + }
> + } else { /* version 1, must be PPTP */
> + u8 _ppp_hdr[PPP_HDRLEN];
> + u8 *ppp_hdr;
> +
> + if (hdr->flags & GRE_ACK)
> + offset += sizeof(((struct pptp_gre_header *)0)->ack);
> +
> + ppp_hdr = skb_header_pointer(skb, nhoff + offset,
> + sizeof(_ppp_hdr), _ppp_hdr);
> + if (!ppp_hdr)
> 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;
> +
> + 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;
> }
>
> + nhoff += offset;
> key_control->flags |= FLOW_DIS_ENCAPSULATION;
> if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
> goto out_good;
Powered by blists - more mailing lists