| 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: Mon, 8 Aug 2016 17:13:04 +0800
From: Feng Gao <gfree.wind@...il.com>
To: Philp Prindeville <philipp@...fish-solutions.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
Hi Philip,
Do you mean like the following?
struct gre_full_hdr {
struct {
__be16 flags;
__be16 protocol;
} fixed_header;
__be16 csum;
__be16 reserved1;
__be32 key;
__be32 seq;
} __packed;
But we need struct gre_base_hdr to get the fixed header of GRE in
function __skb_flow_dissect like the following codes.
struct gre_base_hdr *hdr, _hdr;
hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
BTW, the original codes define one local stuct gre_hdr. Now I use the
unified struct gre_base_hdr instead of it.
Best Regards
Feng
On Mon, Aug 8, 2016 at 11:27 AM, Philp Prindeville
<philipp@...fish-solutions.com> wrote:
> 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