* [PATCH net-next 1/4] geneve: implement geneve_get_sk_family helper
2016-02-18 10:22 [PATCH net-next 0/4] iptunnel: scrub packet in iptunnel_pull_header Jiri Benc
@ 2016-02-18 10:22 ` Jiri Benc
2016-02-18 14:44 ` John W. Linville
2016-02-18 10:22 ` [PATCH net-next 2/4] geneve: move geneve device lookup before iptunnel_pull_header Jiri Benc
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Jiri Benc @ 2016-02-18 10:22 UTC (permalink / raw)
To: netdev; +Cc: Jesse Gross, John W. Linville, Pravin B Shelar
Similarly to the existing vxlan_get_sk_family.
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
drivers/net/geneve.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 6f208132a574..f09de1e30955 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -110,6 +110,11 @@ static __be64 vni_to_tunnel_id(const __u8 *vni)
#endif
}
+static sa_family_t geneve_get_sk_family(struct geneve_sock *gs)
+{
+ return gs->sock->sk->sk_family;
+}
+
static struct geneve_dev *geneve_lookup(struct geneve_sock *gs,
__be32 addr, u8 vni[])
{
@@ -165,16 +170,13 @@ static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
static u8 zero_vni[3];
u8 *vni;
int err = 0;
- sa_family_t sa_family;
#if IS_ENABLED(CONFIG_IPV6)
struct ipv6hdr *ip6h = NULL;
struct in6_addr addr6;
static struct in6_addr zero_addr6;
#endif
- sa_family = gs->sock->sk->sk_family;
-
- if (sa_family == AF_INET) {
+ if (geneve_get_sk_family(gs) == AF_INET) {
iph = ip_hdr(skb); /* outer IP header... */
if (gs->collect_md) {
@@ -188,7 +190,7 @@ static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
geneve = geneve_lookup(gs, addr, vni);
#if IS_ENABLED(CONFIG_IPV6)
- } else if (sa_family == AF_INET6) {
+ } else if (geneve_get_sk_family(gs) == AF_INET6) {
ip6h = ipv6_hdr(skb); /* outer IPv6 header... */
if (gs->collect_md) {
@@ -213,7 +215,7 @@ static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
(gnvh->oam ? TUNNEL_OAM : 0) |
(gnvh->critical ? TUNNEL_CRIT_OPT : 0);
- tun_dst = udp_tun_rx_dst(skb, sa_family, flags,
+ tun_dst = udp_tun_rx_dst(skb, geneve_get_sk_family(gs), flags,
vni_to_tunnel_id(gnvh->vni),
gnvh->opt_len * 4);
if (!tun_dst)
@@ -392,7 +394,7 @@ static void geneve_notify_add_rx_port(struct geneve_sock *gs)
struct net_device *dev;
struct sock *sk = gs->sock->sk;
struct net *net = sock_net(sk);
- sa_family_t sa_family = sk->sk_family;
+ sa_family_t sa_family = geneve_get_sk_family(gs);
__be16 port = inet_sk(sk)->inet_sport;
int err;
@@ -553,7 +555,7 @@ static void geneve_notify_del_rx_port(struct geneve_sock *gs)
struct net_device *dev;
struct sock *sk = gs->sock->sk;
struct net *net = sock_net(sk);
- sa_family_t sa_family = sk->sk_family;
+ sa_family_t sa_family = geneve_get_sk_family(gs);
__be16 port = inet_sk(sk)->inet_sport;
rcu_read_lock();
@@ -596,7 +598,7 @@ static struct geneve_sock *geneve_find_sock(struct geneve_net *gn,
list_for_each_entry(gs, &gn->sock_list, list) {
if (inet_sk(gs->sock->sk)->inet_sport == dst_port &&
- inet_sk(gs->sock->sk)->sk.sk_family == family) {
+ geneve_get_sk_family(gs) == family) {
return gs;
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/4] geneve: implement geneve_get_sk_family helper
2016-02-18 10:22 ` [PATCH net-next 1/4] geneve: implement geneve_get_sk_family helper Jiri Benc
@ 2016-02-18 14:44 ` John W. Linville
2016-02-18 15:24 ` Jiri Benc
0 siblings, 1 reply; 8+ messages in thread
From: John W. Linville @ 2016-02-18 14:44 UTC (permalink / raw)
To: Jiri Benc; +Cc: netdev, Jesse Gross, Pravin B Shelar
On Thu, Feb 18, 2016 at 11:22:49AM +0100, Jiri Benc wrote:
> Similarly to the existing vxlan_get_sk_family.
>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
> ---
> drivers/net/geneve.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 6f208132a574..f09de1e30955 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -110,6 +110,11 @@ static __be64 vni_to_tunnel_id(const __u8 *vni)
> #endif
> }
>
> +static sa_family_t geneve_get_sk_family(struct geneve_sock *gs)
> +{
> + return gs->sock->sk->sk_family;
> +}
> +
Should this be inline?
> static struct geneve_dev *geneve_lookup(struct geneve_sock *gs,
> __be32 addr, u8 vni[])
> {
> @@ -165,16 +170,13 @@ static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
> static u8 zero_vni[3];
> u8 *vni;
> int err = 0;
> - sa_family_t sa_family;
> #if IS_ENABLED(CONFIG_IPV6)
> struct ipv6hdr *ip6h = NULL;
> struct in6_addr addr6;
> static struct in6_addr zero_addr6;
> #endif
>
> - sa_family = gs->sock->sk->sk_family;
> -
> - if (sa_family == AF_INET) {
> + if (geneve_get_sk_family(gs) == AF_INET) {
> iph = ip_hdr(skb); /* outer IP header... */
>
> if (gs->collect_md) {
> @@ -188,7 +190,7 @@ static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
>
> geneve = geneve_lookup(gs, addr, vni);
> #if IS_ENABLED(CONFIG_IPV6)
> - } else if (sa_family == AF_INET6) {
> + } else if (geneve_get_sk_family(gs) == AF_INET6) {
> ip6h = ipv6_hdr(skb); /* outer IPv6 header... */
>
> if (gs->collect_md) {
> @@ -213,7 +215,7 @@ static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
> (gnvh->oam ? TUNNEL_OAM : 0) |
> (gnvh->critical ? TUNNEL_CRIT_OPT : 0);
>
> - tun_dst = udp_tun_rx_dst(skb, sa_family, flags,
> + tun_dst = udp_tun_rx_dst(skb, geneve_get_sk_family(gs), flags,
> vni_to_tunnel_id(gnvh->vni),
> gnvh->opt_len * 4);
> if (!tun_dst)
Can we count on GCC to inline geneve_get_sk_family on its own?
Otherwise, we are calling the same function as many as three times
on receive.
> @@ -392,7 +394,7 @@ static void geneve_notify_add_rx_port(struct geneve_sock *gs)
> struct net_device *dev;
> struct sock *sk = gs->sock->sk;
> struct net *net = sock_net(sk);
> - sa_family_t sa_family = sk->sk_family;
> + sa_family_t sa_family = geneve_get_sk_family(gs);
> __be16 port = inet_sk(sk)->inet_sport;
> int err;
>
> @@ -553,7 +555,7 @@ static void geneve_notify_del_rx_port(struct geneve_sock *gs)
> struct net_device *dev;
> struct sock *sk = gs->sock->sk;
> struct net *net = sock_net(sk);
> - sa_family_t sa_family = sk->sk_family;
> + sa_family_t sa_family = geneve_get_sk_family(gs);
> __be16 port = inet_sk(sk)->inet_sport;
>
> rcu_read_lock();
Then on these three you are caching the results of the function
call instead...
> @@ -596,7 +598,7 @@ static struct geneve_sock *geneve_find_sock(struct geneve_net *gn,
>
> list_for_each_entry(gs, &gn->sock_list, list) {
> if (inet_sk(gs->sock->sk)->inet_sport == dst_port &&
> - inet_sk(gs->sock->sk)->sk.sk_family == family) {
> + geneve_get_sk_family(gs) == family) {
> return gs;
> }
> }
And back to calling it each time (as necessary inside the
list_for_each_entry).
So, it seems like geneve_get_sk_family needs to be inline -- maybe GCC
is doing that on its own? FWIW, I probably would cache the results
inside of geneve_rx like you have done in geneve_notify_add_rx_port
and geneve_notify_del_rx_port.
Thanks for the attention to geneve!
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/4] geneve: implement geneve_get_sk_family helper
2016-02-18 14:44 ` John W. Linville
@ 2016-02-18 15:24 ` Jiri Benc
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-18 15:24 UTC (permalink / raw)
To: John W. Linville; +Cc: netdev, Jesse Gross, Pravin B Shelar
On Thu, 18 Feb 2016 09:44:10 -0500, John W. Linville wrote:
> On Thu, Feb 18, 2016 at 11:22:49AM +0100, Jiri Benc wrote:
> > +static sa_family_t geneve_get_sk_family(struct geneve_sock *gs)
> > +{
> > + return gs->sock->sk->sk_family;
> > +}
> > +
>
> Should this be inline?
AFAIK the kernel policy is to not add inlines in .c files, instead let
the compiler do its job.
> Can we count on GCC to inline geneve_get_sk_family on its own?
Yes.
> Otherwise, we are calling the same function as many as three times
> on receive.
[...]
> Then on these three you are caching the results of the function
> call instead...
The reason I removed the sa_family local variable from geneve_rx is the
next patch in the set where I need to put part of the family references
into a separate function.
I kept the rest of the file as it was - I thought about removing the
local variables in geneve_notify_add/del_rx_port, too, but it would
lead to ugly long lines here:
for_each_netdev_rcu(net, dev) {
if (dev->netdev_ops->ndo_add_geneve_port)
dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
port);
}
Thus, the local variable makes sense in the these two functions, makes
the code more comprehensible.
> > @@ -596,7 +598,7 @@ static struct geneve_sock *geneve_find_sock(struct geneve_net *gn,
> >
> > list_for_each_entry(gs, &gn->sock_list, list) {
> > if (inet_sk(gs->sock->sk)->inet_sport == dst_port &&
> > - inet_sk(gs->sock->sk)->sk.sk_family == family) {
> > + geneve_get_sk_family(gs) == family) {
> > return gs;
> > }
> > }
>
> And back to calling it each time (as necessary inside the
> list_for_each_entry).
Just replacing the direct sk_family dereference with the helper, no
real change here. No need to introduce a local variable here, this is
still short enough.
> So, it seems like geneve_get_sk_family needs to be inline -- maybe GCC
> is doing that on its own? FWIW, I probably would cache the results
> inside of geneve_rx like you have done in geneve_notify_add_rx_port
> and geneve_notify_del_rx_port.
Then I would just have to remove or duplicate the local variable in the
next patch. Not worth it, I think.
Thanks for review,
Jiri
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 2/4] geneve: move geneve device lookup before iptunnel_pull_header
2016-02-18 10:22 [PATCH net-next 0/4] iptunnel: scrub packet in iptunnel_pull_header Jiri Benc
2016-02-18 10:22 ` [PATCH net-next 1/4] geneve: implement geneve_get_sk_family helper Jiri Benc
@ 2016-02-18 10:22 ` Jiri Benc
2016-02-18 10:22 ` [PATCH net-next 3/4] vxlan: move vxlan " Jiri Benc
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-18 10:22 UTC (permalink / raw)
To: netdev; +Cc: Jesse Gross, John W. Linville, Pravin B Shelar
This is in preparation for iptunnel_pull_header calling skb_scrub_packet.
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
drivers/net/geneve.c | 76 ++++++++++++++++++++++++++++++----------------------
1 file changed, 44 insertions(+), 32 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index f09de1e30955..4ceccf871b3f 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -158,55 +158,60 @@ static inline struct genevehdr *geneve_hdr(const struct sk_buff *skb)
return (struct genevehdr *)(udp_hdr(skb) + 1);
}
-/* geneve receive/decap routine */
-static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
+static struct geneve_dev *geneve_lookup_skb(struct geneve_sock *gs,
+ struct sk_buff *skb)
{
- struct genevehdr *gnvh = geneve_hdr(skb);
- struct metadata_dst *tun_dst = NULL;
- struct geneve_dev *geneve = NULL;
- struct pcpu_sw_netstats *stats;
- struct iphdr *iph = NULL;
+ u8 *vni;
__be32 addr;
static u8 zero_vni[3];
- u8 *vni;
- int err = 0;
#if IS_ENABLED(CONFIG_IPV6)
- struct ipv6hdr *ip6h = NULL;
- struct in6_addr addr6;
static struct in6_addr zero_addr6;
#endif
if (geneve_get_sk_family(gs) == AF_INET) {
+ struct iphdr *iph;
+
iph = ip_hdr(skb); /* outer IP header... */
if (gs->collect_md) {
vni = zero_vni;
addr = 0;
} else {
- vni = gnvh->vni;
-
+ vni = geneve_hdr(skb)->vni;
addr = iph->saddr;
}
- geneve = geneve_lookup(gs, addr, vni);
+ return geneve_lookup(gs, addr, vni);
#if IS_ENABLED(CONFIG_IPV6)
} else if (geneve_get_sk_family(gs) == AF_INET6) {
+ struct ipv6hdr *ip6h;
+ struct in6_addr addr6;
+
ip6h = ipv6_hdr(skb); /* outer IPv6 header... */
if (gs->collect_md) {
vni = zero_vni;
addr6 = zero_addr6;
} else {
- vni = gnvh->vni;
-
+ vni = geneve_hdr(skb)->vni;
addr6 = ip6h->saddr;
}
- geneve = geneve6_lookup(gs, addr6, vni);
+ return geneve6_lookup(gs, addr6, vni);
#endif
}
- if (!geneve)
- goto drop;
+ return NULL;
+}
+
+/* geneve receive/decap routine */
+static void geneve_rx(struct geneve_dev *geneve, struct geneve_sock *gs,
+ struct sk_buff *skb)
+{
+ struct genevehdr *gnvh = geneve_hdr(skb);
+ struct metadata_dst *tun_dst = NULL;
+ struct pcpu_sw_netstats *stats;
+ int err = 0;
+ void *oiph;
if (ip_tunnel_collect_metadata() || gs->collect_md) {
__be16 flags;
@@ -243,25 +248,27 @@ static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
if (ether_addr_equal(eth_hdr(skb)->h_source, geneve->dev->dev_addr))
goto drop;
+ oiph = skb_network_header(skb);
skb_reset_network_header(skb);
- if (iph)
- err = IP_ECN_decapsulate(iph, skb);
+ if (geneve_get_sk_family(gs) == AF_INET)
+ err = IP_ECN_decapsulate(oiph, skb);
#if IS_ENABLED(CONFIG_IPV6)
- if (ip6h)
- err = IP6_ECN_decapsulate(ip6h, skb);
+ else
+ err = IP6_ECN_decapsulate(oiph, skb);
#endif
if (unlikely(err)) {
if (log_ecn_error) {
- if (iph)
+ if (geneve_get_sk_family(gs) == AF_INET)
net_info_ratelimited("non-ECT from %pI4 "
"with TOS=%#x\n",
- &iph->saddr, iph->tos);
+ &((struct iphdr *)oiph)->saddr,
+ ((struct iphdr *)oiph)->tos);
#if IS_ENABLED(CONFIG_IPV6)
- if (ip6h)
+ else
net_info_ratelimited("non-ECT from %pI6\n",
- &ip6h->saddr);
+ &((struct ipv6hdr *)oiph)->saddr);
#endif
}
if (err > 1) {
@@ -323,6 +330,7 @@ static void geneve_uninit(struct net_device *dev)
static int geneve_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
{
struct genevehdr *geneveh;
+ struct geneve_dev *geneve;
struct geneve_sock *gs;
int opts_len;
@@ -338,16 +346,20 @@ static int geneve_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
if (unlikely(geneveh->proto_type != htons(ETH_P_TEB)))
goto error;
+ gs = rcu_dereference_sk_user_data(sk);
+ if (!gs)
+ goto drop;
+
+ geneve = geneve_lookup_skb(gs, skb);
+ if (!geneve)
+ goto drop;
+
opts_len = geneveh->opt_len * 4;
if (iptunnel_pull_header(skb, GENEVE_BASE_HLEN + opts_len,
htons(ETH_P_TEB)))
goto drop;
- gs = rcu_dereference_sk_user_data(sk);
- if (!gs)
- goto drop;
-
- geneve_rx(gs, skb);
+ geneve_rx(geneve, gs, skb);
return 0;
drop:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next 3/4] vxlan: move vxlan device lookup before iptunnel_pull_header
2016-02-18 10:22 [PATCH net-next 0/4] iptunnel: scrub packet in iptunnel_pull_header Jiri Benc
2016-02-18 10:22 ` [PATCH net-next 1/4] geneve: implement geneve_get_sk_family helper Jiri Benc
2016-02-18 10:22 ` [PATCH net-next 2/4] geneve: move geneve device lookup before iptunnel_pull_header Jiri Benc
@ 2016-02-18 10:22 ` Jiri Benc
2016-02-18 10:22 ` [PATCH net-next 4/4] iptunnel: scrub packet in iptunnel_pull_header Jiri Benc
2016-02-18 19:35 ` [PATCH net-next 0/4] " David Miller
4 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-18 10:22 UTC (permalink / raw)
To: netdev; +Cc: Jesse Gross, John W. Linville, Pravin B Shelar
This is in preparation for iptunnel_pull_header calling skb_scrub_packet.
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
drivers/net/vxlan.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 3a84680b5117..b43981416fb9 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1187,22 +1187,16 @@ out:
unparsed->vx_flags &= ~VXLAN_GBP_USED_BITS;
}
-static void vxlan_rcv(struct vxlan_sock *vs, struct sk_buff *skb,
- struct vxlan_metadata *md, __be32 vni,
+static void vxlan_rcv(struct vxlan_dev *vxlan, struct vxlan_sock *vs,
+ struct sk_buff *skb, struct vxlan_metadata *md,
struct metadata_dst *tun_dst)
{
struct iphdr *oip = NULL;
struct ipv6hdr *oip6 = NULL;
- struct vxlan_dev *vxlan;
struct pcpu_sw_netstats *stats;
union vxlan_addr saddr;
int err = 0;
- /* Is this VNI defined? */
- vxlan = vxlan_vs_find_vni(vs, vni);
- if (!vxlan)
- goto drop;
-
skb_reset_mac_header(skb);
skb_scrub_packet(skb, !net_eq(vxlan->net, dev_net(vxlan->dev)));
skb->protocol = eth_type_trans(skb, vxlan->dev);
@@ -1281,6 +1275,7 @@ drop:
static int vxlan_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
{
struct metadata_dst *tun_dst = NULL;
+ struct vxlan_dev *vxlan;
struct vxlan_sock *vs;
struct vxlanhdr unparsed;
struct vxlan_metadata _md;
@@ -1302,13 +1297,17 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
unparsed.vx_flags &= ~VXLAN_HF_VNI;
unparsed.vx_vni &= ~VXLAN_VNI_MASK;
- if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB)))
- goto drop;
-
vs = rcu_dereference_sk_user_data(sk);
if (!vs)
goto drop;
+ vxlan = vxlan_vs_find_vni(vs, vxlan_vni(vxlan_hdr(skb)->vx_vni));
+ if (!vxlan)
+ goto drop;
+
+ if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB)))
+ goto drop;
+
if (vxlan_collect_metadata(vs)) {
tun_dst = udp_tun_rx_dst(skb, vxlan_get_sk_family(vs), TUNNEL_KEY,
vxlan_vni(vxlan_hdr(skb)->vx_vni),
@@ -1343,7 +1342,7 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
goto drop;
}
- vxlan_rcv(vs, skb, md, vxlan_vni(vxlan_hdr(skb)->vx_vni), tun_dst);
+ vxlan_rcv(vxlan, vs, skb, md, tun_dst);
return 0;
drop:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next 4/4] iptunnel: scrub packet in iptunnel_pull_header
2016-02-18 10:22 [PATCH net-next 0/4] iptunnel: scrub packet in iptunnel_pull_header Jiri Benc
` (2 preceding siblings ...)
2016-02-18 10:22 ` [PATCH net-next 3/4] vxlan: move vxlan " Jiri Benc
@ 2016-02-18 10:22 ` Jiri Benc
2016-02-18 19:35 ` [PATCH net-next 0/4] " David Miller
4 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-18 10:22 UTC (permalink / raw)
To: netdev; +Cc: Jesse Gross, John W. Linville, Pravin B Shelar
Part of skb_scrub_packet was open coded in iptunnel_pull_header. Let it call
skb_scrub_packet directly instead.
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
drivers/net/geneve.c | 4 ++--
drivers/net/vxlan.c | 4 ++--
include/net/ip_tunnels.h | 3 ++-
net/ipv4/ip_gre.c | 2 +-
net/ipv4/ip_tunnel_core.c | 8 +++-----
net/ipv4/ipip.c | 2 +-
net/ipv6/sit.c | 2 +-
7 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 4ceccf871b3f..dfbe3ca687f7 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -237,7 +237,6 @@ static void geneve_rx(struct geneve_dev *geneve, struct geneve_sock *gs,
}
skb_reset_mac_header(skb);
- skb_scrub_packet(skb, !net_eq(geneve->net, dev_net(geneve->dev)));
skb->protocol = eth_type_trans(skb, geneve->dev);
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
@@ -356,7 +355,8 @@ static int geneve_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
opts_len = geneveh->opt_len * 4;
if (iptunnel_pull_header(skb, GENEVE_BASE_HLEN + opts_len,
- htons(ETH_P_TEB)))
+ htons(ETH_P_TEB),
+ !net_eq(geneve->net, dev_net(geneve->dev))))
goto drop;
geneve_rx(geneve, gs, skb);
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index b43981416fb9..56cb9d23a6f3 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1198,7 +1198,6 @@ static void vxlan_rcv(struct vxlan_dev *vxlan, struct vxlan_sock *vs,
int err = 0;
skb_reset_mac_header(skb);
- skb_scrub_packet(skb, !net_eq(vxlan->net, dev_net(vxlan->dev)));
skb->protocol = eth_type_trans(skb, vxlan->dev);
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
@@ -1305,7 +1304,8 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
if (!vxlan)
goto drop;
- if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB)))
+ if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB),
+ !net_eq(vxlan->net, dev_net(vxlan->dev))))
goto drop;
if (vxlan_collect_metadata(vs)) {
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 87408ab80856..4dd616376fec 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -270,7 +270,8 @@ static inline u8 ip_tunnel_ecn_encap(u8 tos, const struct iphdr *iph,
return INET_ECN_encapsulate(tos, inner);
}
-int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto);
+int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto,
+ bool xnet);
void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
__be32 src, __be32 dst, u8 proto,
u8 tos, u8 ttl, __be16 df, bool xnet);
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 917c2c1bfadd..12071e28d958 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -238,7 +238,7 @@ static int parse_gre_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
return -EINVAL;
}
}
- return iptunnel_pull_header(skb, hdr_len, tpi->proto);
+ return iptunnel_pull_header(skb, hdr_len, tpi->proto, false);
}
static void ipgre_err(struct sk_buff *skb, u32 info,
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index a6e58b6141cd..eaca2449a09a 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -86,7 +86,8 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
}
EXPORT_SYMBOL_GPL(iptunnel_xmit);
-int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto)
+int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto,
+ bool xnet)
{
if (unlikely(!pskb_may_pull(skb, hdr_len)))
return -ENOMEM;
@@ -109,13 +110,10 @@ int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto)
skb->protocol = inner_proto;
}
- nf_reset(skb);
- secpath_reset(skb);
skb_clear_hash_if_not_l4(skb);
- skb_dst_drop(skb);
skb->vlan_tci = 0;
skb_set_queue_mapping(skb, 0);
- skb->pkt_type = PACKET_HOST;
+ skb_scrub_packet(skb, xnet);
return 0;
}
EXPORT_SYMBOL_GPL(iptunnel_pull_header);
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 6ec5b42fd172..ec51d02166de 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -195,7 +195,7 @@ static int ipip_rcv(struct sk_buff *skb)
if (tunnel) {
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
goto drop;
- if (iptunnel_pull_header(skb, 0, tpi.proto))
+ if (iptunnel_pull_header(skb, 0, tpi.proto, false))
goto drop;
return ip_tunnel_rcv(tunnel, skb, &tpi, NULL, log_ecn_error);
}
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 0625ac6356b5..f45b8ffc2840 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -740,7 +740,7 @@ static int ipip_rcv(struct sk_buff *skb)
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
goto drop;
- if (iptunnel_pull_header(skb, 0, tpi.proto))
+ if (iptunnel_pull_header(skb, 0, tpi.proto, false))
goto drop;
return ip_tunnel_rcv(tunnel, skb, &tpi, NULL, log_ecn_error);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/4] iptunnel: scrub packet in iptunnel_pull_header
2016-02-18 10:22 [PATCH net-next 0/4] iptunnel: scrub packet in iptunnel_pull_header Jiri Benc
` (3 preceding siblings ...)
2016-02-18 10:22 ` [PATCH net-next 4/4] iptunnel: scrub packet in iptunnel_pull_header Jiri Benc
@ 2016-02-18 19:35 ` David Miller
4 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2016-02-18 19:35 UTC (permalink / raw)
To: jbenc; +Cc: netdev, jesse, linville, pshelar
From: Jiri Benc <jbenc@redhat.com>
Date: Thu, 18 Feb 2016 11:22:48 +0100
> As every IP tunnel has to scrub skb on decapsulation, iptunnel_pull_header
> tried to do that and open coded part of skb_scrub_packet. Various tunneling
> protocols (VXLAN, Geneve) then called full skb_scrub_packet on their own,
> duplicating part of the scrubbing already done.
>
> Consolidate the code, calling skb_scrub_packet from iptunnel_pull_header.
> This will allow additional cleanups in VXLAN code, as the packet is scrubbed
> early during rx processing after this patchset and VXLAN can start filling
> out skb fields earlier.
>
> The full picture of vxlan cleanup patches can be seen at:
> https://github.com/jbenc/linux-vxlan/commits/master
Very nice cleanup, patch applied.
^ permalink raw reply [flat|nested] 8+ messages in thread