| 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 09:20:19 -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
No, I was referring to anonymous structures, which is a feature of C11.
Please see the link I sent.
On 08/08/2016 03:13 AM, Feng Gao wrote:
> 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