* [PATCH 1/1] netfilter: gre: Use the consitent GRE and PPTP struct instead of the structures defined in netfilter
@ 2016-08-19 15:01 fgao
2016-08-19 15:03 ` Feng Gao
2016-08-22 9:22 ` Pablo Neira Ayuso
0 siblings, 2 replies; 5+ messages in thread
From: fgao @ 2016-08-19 15:01 UTC (permalink / raw)
To: pablo, kaber, netfilter-devel, philipp, netdev; +Cc: gfree.wind, Gao Feng
From: Gao Feng <fgao@ikuai8.com>
There are two structures which define the GRE header and PPTP
header. So it is unneccessary to define duplicated structures in
netfilter again.
Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
v1: Intial patch
include/linux/netfilter/nf_conntrack_proto_gre.h | 63 +-----------------------
include/uapi/linux/if_tunnel.h | 1 +
net/ipv4/netfilter/nf_nat_proto_gre.c | 15 +++---
net/netfilter/nf_conntrack_proto_gre.c | 14 +++---
4 files changed, 19 insertions(+), 74 deletions(-)
diff --git a/include/linux/netfilter/nf_conntrack_proto_gre.h b/include/linux/netfilter/nf_conntrack_proto_gre.h
index df78dc2..9c741da 100644
--- a/include/linux/netfilter/nf_conntrack_proto_gre.h
+++ b/include/linux/netfilter/nf_conntrack_proto_gre.h
@@ -2,67 +2,8 @@
#define _CONNTRACK_PROTO_GRE_H
#include <asm/byteorder.h>
-/* GRE PROTOCOL HEADER */
-
-/* GRE Version field */
-#define GRE_VERSION_1701 0x0
-#define GRE_VERSION_PPTP 0x1
-
-/* GRE Protocol field */
-#define GRE_PROTOCOL_PPTP 0x880B
-
-/* GRE Flags */
-#define GRE_FLAG_C 0x80
-#define GRE_FLAG_R 0x40
-#define GRE_FLAG_K 0x20
-#define GRE_FLAG_S 0x10
-#define GRE_FLAG_A 0x80
-
-#define GRE_IS_C(f) ((f)&GRE_FLAG_C)
-#define GRE_IS_R(f) ((f)&GRE_FLAG_R)
-#define GRE_IS_K(f) ((f)&GRE_FLAG_K)
-#define GRE_IS_S(f) ((f)&GRE_FLAG_S)
-#define GRE_IS_A(f) ((f)&GRE_FLAG_A)
-
-/* GRE is a mess: Four different standards */
-struct gre_hdr {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
- __u16 rec:3,
- srr:1,
- seq:1,
- key:1,
- routing:1,
- csum:1,
- version:3,
- reserved:4,
- ack:1;
-#elif defined(__BIG_ENDIAN_BITFIELD)
- __u16 csum:1,
- routing:1,
- key:1,
- seq:1,
- srr:1,
- rec:3,
- ack:1,
- reserved:4,
- version:3;
-#else
-#error "Adjust your <asm/byteorder.h> defines"
-#endif
- __be16 protocol;
-};
-
-/* modified GRE header for PPTP */
-struct gre_hdr_pptp {
- __u8 flags; /* bitfield */
- __u8 version; /* should be GRE_VERSION_PPTP */
- __be16 protocol; /* should be GRE_PROTOCOL_PPTP */
- __be16 payload_len; /* size of ppp payload, not inc. gre header */
- __be16 call_id; /* peer's call_id for this session */
- __be32 seq; /* sequence number. Present if S==1 */
- __be32 ack; /* seq number of highest packet received by */
- /* sender in this session */
-};
+#include <net/gre.h>
+#include <net/pptp.h>
struct nf_ct_gre {
unsigned int stream_timeout;
diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h
index 361b9f0..1b27e2c 100644
--- a/include/uapi/linux/if_tunnel.h
+++ b/include/uapi/linux/if_tunnel.h
@@ -36,6 +36,7 @@
#define GRE_IS_REC(f) ((f) & GRE_REC)
#define GRE_IS_ACK(f) ((f) & GRE_ACK)
+#define GRE_VERSION_0 __cpu_to_be16(0x0000)
#define GRE_VERSION_1 __cpu_to_be16(0x0001)
#define GRE_PROTO_PPP __cpu_to_be16(0x880b)
#define GRE_PPTP_KEY_MASK __cpu_to_be32(0xffff)
diff --git a/net/ipv4/netfilter/nf_nat_proto_gre.c b/net/ipv4/netfilter/nf_nat_proto_gre.c
index 9414923..afe81a8 100644
--- a/net/ipv4/netfilter/nf_nat_proto_gre.c
+++ b/net/ipv4/netfilter/nf_nat_proto_gre.c
@@ -88,8 +88,9 @@ gre_manip_pkt(struct sk_buff *skb,
const struct nf_conntrack_tuple *tuple,
enum nf_nat_manip_type maniptype)
{
- const struct gre_hdr *greh;
- struct gre_hdr_pptp *pgreh;
+ const struct gre_base_hdr *greh;
+ struct pptp_gre_header *pgreh;
+ u16 gre_ver;
/* pgreh includes two optional 32bit fields which are not required
* to be there. That's where the magic '8' comes from */
@@ -97,18 +98,20 @@ gre_manip_pkt(struct sk_buff *skb,
return false;
greh = (void *)skb->data + hdroff;
- pgreh = (struct gre_hdr_pptp *)greh;
+ pgreh = (struct pptp_gre_header *)greh;
/* we only have destination manip of a packet, since 'source key'
* is not present in the packet itself */
if (maniptype != NF_NAT_MANIP_DST)
return true;
- switch (greh->version) {
- case GRE_VERSION_1701:
+
+ gre_ver = ntohs(greh->flags & GRE_VERSION);
+ switch (gre_ver) {
+ case GRE_VERSION_0:
/* We do not currently NAT any GREv0 packets.
* Try to behave like "nf_nat_proto_unknown" */
break;
- case GRE_VERSION_PPTP:
+ case GRE_VERSION_1:
pr_debug("call_id -> 0x%04x\n", ntohs(tuple->dst.u.gre.key));
pgreh->call_id = tuple->dst.u.gre.key;
break;
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index a96451a..f46597f 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -192,15 +192,15 @@ static bool gre_invert_tuple(struct nf_conntrack_tuple *tuple,
static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
struct net *net, struct nf_conntrack_tuple *tuple)
{
- const struct gre_hdr_pptp *pgrehdr;
- struct gre_hdr_pptp _pgrehdr;
+ const struct pptp_gre_header *pgrehdr;
+ struct pptp_gre_header _pgrehdr;
__be16 srckey;
- const struct gre_hdr *grehdr;
- struct gre_hdr _grehdr;
+ const struct gre_base_hdr *grehdr;
+ struct gre_base_hdr _grehdr;
/* first only delinearize old RFC1701 GRE header */
grehdr = skb_header_pointer(skb, dataoff, sizeof(_grehdr), &_grehdr);
- if (!grehdr || grehdr->version != GRE_VERSION_PPTP) {
+ if (!grehdr || ntohs(grehdr->flags & GRE_VERSION) != GRE_VERSION_1) {
/* try to behave like "nf_conntrack_proto_generic" */
tuple->src.u.all = 0;
tuple->dst.u.all = 0;
@@ -212,8 +212,8 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
if (!pgrehdr)
return true;
- if (ntohs(grehdr->protocol) != GRE_PROTOCOL_PPTP) {
- pr_debug("GRE_VERSION_PPTP but unknown proto\n");
+ if (grehdr->protocol != GRE_PROTO_PPP) {
+ pr_debug("Unknown GRE proto(0x%x)\n", ntohs(grehdr->protocol));
return false;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] netfilter: gre: Use the consitent GRE and PPTP struct instead of the structures defined in netfilter
2016-08-19 15:01 [PATCH 1/1] netfilter: gre: Use the consitent GRE and PPTP struct instead of the structures defined in netfilter fgao
@ 2016-08-19 15:03 ` Feng Gao
2016-08-25 12:16 ` Pablo Neira Ayuso
2016-08-22 9:22 ` Pablo Neira Ayuso
1 sibling, 1 reply; 5+ messages in thread
From: Feng Gao @ 2016-08-19 15:03 UTC (permalink / raw)
To: Gao Feng
Cc: Pablo Neira Ayuso, Patrick McHardy,
Netfilter Developer Mailing List, Philp Prindeville,
Linux Kernel Network Developers
My email server reports the last same patch email failed to send.
So I just sent it again.
I am sorry, if anyone receives duplicated ones.
Regards
Feng
On Fri, Aug 19, 2016 at 11:01 PM, <fgao@ikuai8.com> wrote:
> From: Gao Feng <fgao@ikuai8.com>
>
> There are two structures which define the GRE header and PPTP
> header. So it is unneccessary to define duplicated structures in
> netfilter again.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
> v1: Intial patch
>
> include/linux/netfilter/nf_conntrack_proto_gre.h | 63 +-----------------------
> include/uapi/linux/if_tunnel.h | 1 +
> net/ipv4/netfilter/nf_nat_proto_gre.c | 15 +++---
> net/netfilter/nf_conntrack_proto_gre.c | 14 +++---
> 4 files changed, 19 insertions(+), 74 deletions(-)
>
> diff --git a/include/linux/netfilter/nf_conntrack_proto_gre.h b/include/linux/netfilter/nf_conntrack_proto_gre.h
> index df78dc2..9c741da 100644
> --- a/include/linux/netfilter/nf_conntrack_proto_gre.h
> +++ b/include/linux/netfilter/nf_conntrack_proto_gre.h
> @@ -2,67 +2,8 @@
> #define _CONNTRACK_PROTO_GRE_H
> #include <asm/byteorder.h>
>
> -/* GRE PROTOCOL HEADER */
> -
> -/* GRE Version field */
> -#define GRE_VERSION_1701 0x0
> -#define GRE_VERSION_PPTP 0x1
> -
> -/* GRE Protocol field */
> -#define GRE_PROTOCOL_PPTP 0x880B
> -
> -/* GRE Flags */
> -#define GRE_FLAG_C 0x80
> -#define GRE_FLAG_R 0x40
> -#define GRE_FLAG_K 0x20
> -#define GRE_FLAG_S 0x10
> -#define GRE_FLAG_A 0x80
> -
> -#define GRE_IS_C(f) ((f)&GRE_FLAG_C)
> -#define GRE_IS_R(f) ((f)&GRE_FLAG_R)
> -#define GRE_IS_K(f) ((f)&GRE_FLAG_K)
> -#define GRE_IS_S(f) ((f)&GRE_FLAG_S)
> -#define GRE_IS_A(f) ((f)&GRE_FLAG_A)
> -
> -/* GRE is a mess: Four different standards */
> -struct gre_hdr {
> -#if defined(__LITTLE_ENDIAN_BITFIELD)
> - __u16 rec:3,
> - srr:1,
> - seq:1,
> - key:1,
> - routing:1,
> - csum:1,
> - version:3,
> - reserved:4,
> - ack:1;
> -#elif defined(__BIG_ENDIAN_BITFIELD)
> - __u16 csum:1,
> - routing:1,
> - key:1,
> - seq:1,
> - srr:1,
> - rec:3,
> - ack:1,
> - reserved:4,
> - version:3;
> -#else
> -#error "Adjust your <asm/byteorder.h> defines"
> -#endif
> - __be16 protocol;
> -};
> -
> -/* modified GRE header for PPTP */
> -struct gre_hdr_pptp {
> - __u8 flags; /* bitfield */
> - __u8 version; /* should be GRE_VERSION_PPTP */
> - __be16 protocol; /* should be GRE_PROTOCOL_PPTP */
> - __be16 payload_len; /* size of ppp payload, not inc. gre header */
> - __be16 call_id; /* peer's call_id for this session */
> - __be32 seq; /* sequence number. Present if S==1 */
> - __be32 ack; /* seq number of highest packet received by */
> - /* sender in this session */
> -};
> +#include <net/gre.h>
> +#include <net/pptp.h>
>
> struct nf_ct_gre {
> unsigned int stream_timeout;
> diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h
> index 361b9f0..1b27e2c 100644
> --- a/include/uapi/linux/if_tunnel.h
> +++ b/include/uapi/linux/if_tunnel.h
> @@ -36,6 +36,7 @@
> #define GRE_IS_REC(f) ((f) & GRE_REC)
> #define GRE_IS_ACK(f) ((f) & GRE_ACK)
>
> +#define GRE_VERSION_0 __cpu_to_be16(0x0000)
> #define GRE_VERSION_1 __cpu_to_be16(0x0001)
> #define GRE_PROTO_PPP __cpu_to_be16(0x880b)
> #define GRE_PPTP_KEY_MASK __cpu_to_be32(0xffff)
> diff --git a/net/ipv4/netfilter/nf_nat_proto_gre.c b/net/ipv4/netfilter/nf_nat_proto_gre.c
> index 9414923..afe81a8 100644
> --- a/net/ipv4/netfilter/nf_nat_proto_gre.c
> +++ b/net/ipv4/netfilter/nf_nat_proto_gre.c
> @@ -88,8 +88,9 @@ gre_manip_pkt(struct sk_buff *skb,
> const struct nf_conntrack_tuple *tuple,
> enum nf_nat_manip_type maniptype)
> {
> - const struct gre_hdr *greh;
> - struct gre_hdr_pptp *pgreh;
> + const struct gre_base_hdr *greh;
> + struct pptp_gre_header *pgreh;
> + u16 gre_ver;
>
> /* pgreh includes two optional 32bit fields which are not required
> * to be there. That's where the magic '8' comes from */
> @@ -97,18 +98,20 @@ gre_manip_pkt(struct sk_buff *skb,
> return false;
>
> greh = (void *)skb->data + hdroff;
> - pgreh = (struct gre_hdr_pptp *)greh;
> + pgreh = (struct pptp_gre_header *)greh;
>
> /* we only have destination manip of a packet, since 'source key'
> * is not present in the packet itself */
> if (maniptype != NF_NAT_MANIP_DST)
> return true;
> - switch (greh->version) {
> - case GRE_VERSION_1701:
> +
> + gre_ver = ntohs(greh->flags & GRE_VERSION);
> + switch (gre_ver) {
> + case GRE_VERSION_0:
> /* We do not currently NAT any GREv0 packets.
> * Try to behave like "nf_nat_proto_unknown" */
> break;
> - case GRE_VERSION_PPTP:
> + case GRE_VERSION_1:
> pr_debug("call_id -> 0x%04x\n", ntohs(tuple->dst.u.gre.key));
> pgreh->call_id = tuple->dst.u.gre.key;
> break;
> diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
> index a96451a..f46597f 100644
> --- a/net/netfilter/nf_conntrack_proto_gre.c
> +++ b/net/netfilter/nf_conntrack_proto_gre.c
> @@ -192,15 +192,15 @@ static bool gre_invert_tuple(struct nf_conntrack_tuple *tuple,
> static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
> struct net *net, struct nf_conntrack_tuple *tuple)
> {
> - const struct gre_hdr_pptp *pgrehdr;
> - struct gre_hdr_pptp _pgrehdr;
> + const struct pptp_gre_header *pgrehdr;
> + struct pptp_gre_header _pgrehdr;
> __be16 srckey;
> - const struct gre_hdr *grehdr;
> - struct gre_hdr _grehdr;
> + const struct gre_base_hdr *grehdr;
> + struct gre_base_hdr _grehdr;
>
> /* first only delinearize old RFC1701 GRE header */
> grehdr = skb_header_pointer(skb, dataoff, sizeof(_grehdr), &_grehdr);
> - if (!grehdr || grehdr->version != GRE_VERSION_PPTP) {
> + if (!grehdr || ntohs(grehdr->flags & GRE_VERSION) != GRE_VERSION_1) {
> /* try to behave like "nf_conntrack_proto_generic" */
> tuple->src.u.all = 0;
> tuple->dst.u.all = 0;
> @@ -212,8 +212,8 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
> if (!pgrehdr)
> return true;
>
> - if (ntohs(grehdr->protocol) != GRE_PROTOCOL_PPTP) {
> - pr_debug("GRE_VERSION_PPTP but unknown proto\n");
> + if (grehdr->protocol != GRE_PROTO_PPP) {
> + pr_debug("Unknown GRE proto(0x%x)\n", ntohs(grehdr->protocol));
> return false;
> }
>
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] netfilter: gre: Use the consitent GRE and PPTP struct instead of the structures defined in netfilter
2016-08-19 15:01 [PATCH 1/1] netfilter: gre: Use the consitent GRE and PPTP struct instead of the structures defined in netfilter fgao
2016-08-19 15:03 ` Feng Gao
@ 2016-08-22 9:22 ` Pablo Neira Ayuso
1 sibling, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-22 9:22 UTC (permalink / raw)
To: fgao; +Cc: kaber, netfilter-devel, philipp, netdev, gfree.wind
On Fri, Aug 19, 2016 at 11:01:34PM +0800, fgao@ikuai8.com wrote:
> From: Gao Feng <fgao@ikuai8.com>
>
> There are two structures which define the GRE header and PPTP
> header. So it is unneccessary to define duplicated structures in
> netfilter again.
Please, split this change in smaller patches, I'd suggest one to
replace GRE_* definitions and another to use generic GRE struct
definitions, so this makes it is easier to review.
> @@ -212,8 +212,8 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
> if (!pgrehdr)
> return true;
>
> - if (ntohs(grehdr->protocol) != GRE_PROTOCOL_PPTP) {
> - pr_debug("GRE_VERSION_PPTP but unknown proto\n");
> + if (grehdr->protocol != GRE_PROTO_PPP) {
> + pr_debug("Unknown GRE proto(0x%x)\n", ntohs(grehdr->protocol));
Something is fishy here, grehdr->protocol used to have ntohs(), the
pr_debug() still has it while the branch check does not.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] netfilter: gre: Use the consitent GRE and PPTP struct instead of the structures defined in netfilter
2016-08-19 15:03 ` Feng Gao
@ 2016-08-25 12:16 ` Pablo Neira Ayuso
2016-08-25 13:38 ` Feng Gao
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-25 12:16 UTC (permalink / raw)
To: Feng Gao
Cc: Gao Feng, Patrick McHardy, Netfilter Developer Mailing List,
Philp Prindeville, Linux Kernel Network Developers
On Fri, Aug 19, 2016 at 11:03:46PM +0800, Feng Gao wrote:
> My email server reports the last same patch email failed to send.
> So I just sent it again.
>
> I am sorry, if anyone receives duplicated ones.
git am v2-1-2-net-next-netfilter-gre-Use-consistent-GRE_-macros-instead-of-ones-defined-by-netfilter..patch
-s
Applying: netfilter: gre: Use consistent GRE_* macros instead of ones
defined by netfilter.
error: patch failed: include/uapi/linux/if_tunnel.h:36
error: include/uapi/linux/if_tunnel.h: patch does not apply
It seems your base was missing this patch:
commit ab10dccb11608b96b43b557c12a5ad867723e503
Author: Gao Feng <fgao@ikuai8.com>
Date: Tue Aug 9 12:38:24 2016 +0800
rps: Inspect PPTP encapsulated by GRE to get flow hash
Since I cannot see GRE_FLAGS in your patch as context.
Please rebase and resubmit, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] netfilter: gre: Use the consitent GRE and PPTP struct instead of the structures defined in netfilter
2016-08-25 12:16 ` Pablo Neira Ayuso
@ 2016-08-25 13:38 ` Feng Gao
0 siblings, 0 replies; 5+ messages in thread
From: Feng Gao @ 2016-08-25 13:38 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Gao Feng, Patrick McHardy, Netfilter Developer Mailing List,
Philp Prindeville, Linux Kernel Network Developers
Hi Pablo,
inline
On Thu, Aug 25, 2016 at 8:16 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Aug 19, 2016 at 11:03:46PM +0800, Feng Gao wrote:
>> My email server reports the last same patch email failed to send.
>> So I just sent it again.
>>
>> I am sorry, if anyone receives duplicated ones.
>
> git am v2-1-2-net-next-netfilter-gre-Use-consistent-GRE_-macros-instead-of-ones-defined-by-netfilter..patch
> -s
> Applying: netfilter: gre: Use consistent GRE_* macros instead of ones
> defined by netfilter.
> error: patch failed: include/uapi/linux/if_tunnel.h:36
> error: include/uapi/linux/if_tunnel.h: patch does not apply
>
> It seems your base was missing this patch:
>
> commit ab10dccb11608b96b43b557c12a5ad867723e503
> Author: Gao Feng <fgao@ikuai8.com>
> Date: Tue Aug 9 12:38:24 2016 +0800
>
> rps: Inspect PPTP encapsulated by GRE to get flow hash
>
> Since I cannot see GRE_FLAGS in your patch as context.
Oh, it is possible that the patches are generated based on my local
branch which has this commit locally, not the latest net-next.
Now, I am more familiar with the rules than before.
>
> Please rebase and resubmit, thanks!
Ok, I will resubmit. Sorry for this fault.
Regards
Feng
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-08-25 13:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-19 15:01 [PATCH 1/1] netfilter: gre: Use the consitent GRE and PPTP struct instead of the structures defined in netfilter fgao
2016-08-19 15:03 ` Feng Gao
2016-08-25 12:16 ` Pablo Neira Ayuso
2016-08-25 13:38 ` Feng Gao
2016-08-22 9:22 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.