| 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: Sun, 7 Aug 2016 21:27:09 -0600
From: Philp Prindeville <philipp@...fish-solutions.com>
To: Feng Gao <gfree.wind@...il.com>
Cc: fgao@...vckh6395k16k5.yundunddos.com,
"David S. Miller" <davem@...emloft.net>,
Stephen Hemminger <stephen@...workplumber.org>,
Pravin B Shelar <pshelar@...ira.com>,
Tom Herbert <tom@...bertland.com>,
Alex Duyck <aduyck@...antis.com>,
Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH v4 1/1] rps: Inspect PPTP encapsulated by GRE to get flow
hash
Feng,
An anonymous structure is defined here:
https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
i.e.:
struct gre_full_hdr {
struct gre_base_hdr;
...
so yes, I'm talking about making fixed_header be anonymous instead.
-Philip
On 08/07/2016 08:50 PM, Feng Gao wrote:
> Hi Philp,
>
> Forgive my poor English, I am not clear about the comment #1.
> "Can you make gre_base_hdr be anonymous?".
>
> +struct gre_full_hdr {
> + struct gre_base_hdr fixed_header;
>
> Do you mean make the member "fixed_header" as anonymous or not?
>
> Best Regards
> Feng
>
>
> On Mon, Aug 8, 2016 at 5:03 AM, Philp Prindeville
> <philipp@...fish-solutions.com> wrote:
>> 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