| 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
| ||
|
Date: Thu, 4 Aug 2016 07:47:25 +0800
From: Feng Gao <gfree.wind@...il.com>
To: Tom Herbert <tom@...bertland.com>
Cc: Gao Feng <fgao@...ai8.com>,
"David S. Miller" <davem@...emloft.net>,
Philp Prindeville <philipp@...fish-solutions.com>,
Stephen Hemminger <stephen@...workplumber.org>,
Pravin B Shelar <pshelar@...ira.com>,
Alex Duyck <aduyck@...antis.com>,
Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash
Hi Tom,
inline comments
On Thu, Aug 4, 2016 at 12:15 AM, Tom Herbert <tom@...bertland.com> wrote:
> On Wed, Aug 3, 2016 at 7:52 AM, <fgao@...ai8.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>
>> ---
>> 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;
Powered by blists - more mailing lists