* [PATCH net-next 1/4] gre: fix handling of key 0
2012-09-25 4:12 [PATCH net-next 0/4] Tunnel related patches Stephen Hemminger
@ 2012-09-25 4:12 ` Stephen Hemminger
2012-09-25 4:12 ` [PATCH net-next 2/4] gre: remove unnecessary rcu_read_lock/unlock Stephen Hemminger
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2012-09-25 4:12 UTC (permalink / raw)
To: davem; +Cc: netdev
[-- Attachment #1: gre-key0.patch --]
[-- Type: text/plain, Size: 3885 bytes --]
GRE driver incorrectly uses zero as a flag value. Zero is a perfectly
valid value for key, and the tunnel should match packets with no key only
with tunnels created without key, and vice versa.
This is a slightly visible change since previously it might be possible to
construct a working tunnel that sent key 0 and received only because
of the key wildcard of zero. I.e the sender sent key of zero, but tunnel
was defined without key.
Note: using gre key 0 requires iproute2 utilities v3.2 or later.
The original utility code was broken as well.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/ipv4/ip_gre.c 2012-08-15 08:59:22.958704223 -0700
+++ b/net/ipv4/ip_gre.c 2012-09-12 09:40:04.420959235 -0700
@@ -214,11 +214,25 @@ static struct rtnl_link_stats64 *ipgre_g
return tot;
}
+/* Does key in tunnel parameters match packet */
+static bool ipgre_key_match(const struct ip_tunnel_parm *p,
+ __u32 flags, __be32 key)
+{
+ if (p->i_flags & GRE_KEY) {
+ if (flags & GRE_KEY)
+ return key == p->i_key;
+ else
+ return false; /* key expected, none present */
+ } else
+ return !(flags & GRE_KEY);
+}
+
/* Given src, dst and key, find appropriate for input tunnel. */
static struct ip_tunnel *ipgre_tunnel_lookup(struct net_device *dev,
__be32 remote, __be32 local,
- __be32 key, __be16 gre_proto)
+ __u32 flags, __be32 key,
+ __be16 gre_proto)
{
struct net *net = dev_net(dev);
int link = dev->ifindex;
@@ -233,10 +247,12 @@ static struct ip_tunnel *ipgre_tunnel_lo
for_each_ip_tunnel_rcu(ign->tunnels_r_l[h0 ^ h1]) {
if (local != t->parms.iph.saddr ||
remote != t->parms.iph.daddr ||
- key != t->parms.i_key ||
!(t->dev->flags & IFF_UP))
continue;
+ if (!ipgre_key_match(&t->parms, flags, key))
+ continue;
+
if (t->dev->type != ARPHRD_IPGRE &&
t->dev->type != dev_type)
continue;
@@ -257,10 +273,12 @@ static struct ip_tunnel *ipgre_tunnel_lo
for_each_ip_tunnel_rcu(ign->tunnels_r[h0 ^ h1]) {
if (remote != t->parms.iph.daddr ||
- key != t->parms.i_key ||
!(t->dev->flags & IFF_UP))
continue;
+ if (!ipgre_key_match(&t->parms, flags, key))
+ continue;
+
if (t->dev->type != ARPHRD_IPGRE &&
t->dev->type != dev_type)
continue;
@@ -283,10 +301,12 @@ static struct ip_tunnel *ipgre_tunnel_lo
if ((local != t->parms.iph.saddr &&
(local != t->parms.iph.daddr ||
!ipv4_is_multicast(local))) ||
- key != t->parms.i_key ||
!(t->dev->flags & IFF_UP))
continue;
+ if (!ipgre_key_match(&t->parms, flags, key))
+ continue;
+
if (t->dev->type != ARPHRD_IPGRE &&
t->dev->type != dev_type)
continue;
@@ -489,6 +509,7 @@ static void ipgre_err(struct sk_buff *sk
const int code = icmp_hdr(skb)->code;
struct ip_tunnel *t;
__be16 flags;
+ __be32 key = 0;
flags = p[0];
if (flags&(GRE_CSUM|GRE_KEY|GRE_SEQ|GRE_ROUTING|GRE_VERSION)) {
@@ -505,6 +526,9 @@ static void ipgre_err(struct sk_buff *sk
if (skb_headlen(skb) < grehlen)
return;
+ if (flags & GRE_KEY)
+ key = *(((__be32 *)p) + (grehlen / 4) - 1);
+
switch (type) {
default:
case ICMP_PARAMETERPROB:
@@ -535,9 +559,8 @@ static void ipgre_err(struct sk_buff *sk
rcu_read_lock();
t = ipgre_tunnel_lookup(skb->dev, iph->daddr, iph->saddr,
- flags & GRE_KEY ?
- *(((__be32 *)p) + (grehlen / 4) - 1) : 0,
- p[1]);
+ flags, key, p[1]);
+
if (t == NULL)
goto out;
@@ -642,9 +665,10 @@ static int ipgre_rcv(struct sk_buff *skb
gre_proto = *(__be16 *)(h + 2);
rcu_read_lock();
- if ((tunnel = ipgre_tunnel_lookup(skb->dev,
- iph->saddr, iph->daddr, key,
- gre_proto))) {
+ tunnel = ipgre_tunnel_lookup(skb->dev,
+ iph->saddr, iph->daddr, flags, key,
+ gre_proto);
+ if (tunnel) {
struct pcpu_tstats *tstats;
secpath_reset(skb);
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH net-next 2/4] gre: remove unnecessary rcu_read_lock/unlock
2012-09-25 4:12 [PATCH net-next 0/4] Tunnel related patches Stephen Hemminger
2012-09-25 4:12 ` [PATCH net-next 1/4] gre: fix handling of key 0 Stephen Hemminger
@ 2012-09-25 4:12 ` Stephen Hemminger
2012-09-25 4:12 ` [PATCH net-next 3/4] xfrm: remove extranous rcu_read_lock Stephen Hemminger
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2012-09-25 4:12 UTC (permalink / raw)
To: davem; +Cc: netdev
[-- Attachment #1: gre-rcu.patch --]
[-- Type: text/plain, Size: 4043 bytes --]
The gre function pointers for receive and error handling are
always called (from gre.c) with rcu_read_lock already held.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/ipv4/ip_gre.c | 19 ++++++-------------
net/ipv6/ip6_gre.c | 10 +---------
2 files changed, 7 insertions(+), 22 deletions(-)
--- a/net/ipv6/ip6_gre.c 2012-09-24 17:33:22.766821472 -0700
+++ b/net/ipv6/ip6_gre.c 2012-09-24 18:12:00.783449055 -0700
@@ -437,14 +437,12 @@ static void ip6gre_err(struct sk_buff *s
ipv6h = (const struct ipv6hdr *)skb->data;
p = (__be16 *)(skb->data + offset);
- rcu_read_lock();
-
t = ip6gre_tunnel_lookup(skb->dev, &ipv6h->daddr, &ipv6h->saddr,
flags & GRE_KEY ?
*(((__be32 *)p) + (grehlen / 4) - 1) : 0,
p[1]);
if (t == NULL)
- goto out;
+ return;
switch (type) {
__u32 teli;
@@ -489,8 +487,6 @@ static void ip6gre_err(struct sk_buff *s
else
t->err_count = 1;
t->err_time = jiffies;
-out:
- rcu_read_unlock();
}
static inline void ip6gre_ecn_decapsulate_ipv4(const struct ip6_tnl *t,
@@ -528,7 +524,7 @@ static int ip6gre_rcv(struct sk_buff *sk
__be16 gre_proto;
if (!pskb_may_pull(skb, sizeof(struct in6_addr)))
- goto drop_nolock;
+ goto drop;
ipv6h = ipv6_hdr(skb);
h = skb->data;
@@ -539,7 +535,7 @@ static int ip6gre_rcv(struct sk_buff *sk
- We do not support routing headers.
*/
if (flags&(GRE_VERSION|GRE_ROUTING))
- goto drop_nolock;
+ goto drop;
if (flags&GRE_CSUM) {
switch (skb->ip_summed) {
@@ -567,7 +563,6 @@ static int ip6gre_rcv(struct sk_buff *sk
gre_proto = *(__be16 *)(h + 2);
- rcu_read_lock();
tunnel = ip6gre_tunnel_lookup(skb->dev,
&ipv6h->saddr, &ipv6h->daddr, key,
gre_proto);
@@ -646,14 +641,11 @@ static int ip6gre_rcv(struct sk_buff *sk
netif_rx(skb);
- rcu_read_unlock();
return 0;
}
icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
drop:
- rcu_read_unlock();
-drop_nolock:
kfree_skb(skb);
return 0;
}
--- a/net/ipv4/ip_gre.c 2012-09-24 17:33:33.298715280 -0700
+++ b/net/ipv4/ip_gre.c 2012-09-24 18:11:52.383533752 -0700
@@ -557,37 +557,34 @@ static void ipgre_err(struct sk_buff *sk
break;
}
- rcu_read_lock();
t = ipgre_tunnel_lookup(skb->dev, iph->daddr, iph->saddr,
flags, key, p[1]);
if (t == NULL)
- goto out;
+ return;
if (type == ICMP_DEST_UNREACH && code == ICMP_FRAG_NEEDED) {
ipv4_update_pmtu(skb, dev_net(skb->dev), info,
t->parms.link, 0, IPPROTO_GRE, 0);
- goto out;
+ return;
}
if (type == ICMP_REDIRECT) {
ipv4_redirect(skb, dev_net(skb->dev), t->parms.link, 0,
IPPROTO_GRE, 0);
- goto out;
+ return;
}
if (t->parms.iph.daddr == 0 ||
ipv4_is_multicast(t->parms.iph.daddr))
- goto out;
+ return;
if (t->parms.iph.ttl == 0 && type == ICMP_TIME_EXCEEDED)
- goto out;
+ return;
if (time_before(jiffies, t->err_time + IPTUNNEL_ERR_TIMEO))
t->err_count++;
else
t->err_count = 1;
t->err_time = jiffies;
-out:
- rcu_read_unlock();
}
static inline void ipgre_ecn_decapsulate(const struct iphdr *iph, struct sk_buff *skb)
@@ -625,7 +622,7 @@ static int ipgre_rcv(struct sk_buff *skb
__be16 gre_proto;
if (!pskb_may_pull(skb, 16))
- goto drop_nolock;
+ goto drop;
iph = ip_hdr(skb);
h = skb->data;
@@ -636,7 +633,7 @@ static int ipgre_rcv(struct sk_buff *skb
- We do not support routing headers.
*/
if (flags&(GRE_VERSION|GRE_ROUTING))
- goto drop_nolock;
+ goto drop;
if (flags&GRE_CSUM) {
switch (skb->ip_summed) {
@@ -664,7 +661,6 @@ static int ipgre_rcv(struct sk_buff *skb
gre_proto = *(__be16 *)(h + 2);
- rcu_read_lock();
tunnel = ipgre_tunnel_lookup(skb->dev,
iph->saddr, iph->daddr, flags, key,
gre_proto);
@@ -740,14 +736,11 @@ static int ipgre_rcv(struct sk_buff *skb
netif_rx(skb);
- rcu_read_unlock();
return 0;
}
icmp_send(skb, ICMP_DEST_UNREACH, ICMP_PORT_UNREACH, 0);
drop:
- rcu_read_unlock();
-drop_nolock:
kfree_skb(skb);
return 0;
}
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH net-next 3/4] xfrm: remove extranous rcu_read_lock
2012-09-25 4:12 [PATCH net-next 0/4] Tunnel related patches Stephen Hemminger
2012-09-25 4:12 ` [PATCH net-next 1/4] gre: fix handling of key 0 Stephen Hemminger
2012-09-25 4:12 ` [PATCH net-next 2/4] gre: remove unnecessary rcu_read_lock/unlock Stephen Hemminger
@ 2012-09-25 4:12 ` Stephen Hemminger
2012-09-25 4:12 ` [PATCH net-next 4/4] tunnel: drop packet if ECN present with not-ECT Stephen Hemminger
2012-09-27 22:13 ` [PATCH net-next 0/4] Tunnel related patches David Miller
4 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2012-09-25 4:12 UTC (permalink / raw)
To: davem; +Cc: netdev
[-- Attachment #1: ipip-no-rcu.patch --]
[-- Type: text/plain, Size: 3493 bytes --]
The handlers for xfrm_tunnel are always invoked with rcu read lock
already.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/ipv4/ip_vti.c | 5 -----
net/ipv4/ipip.c | 9 +--------
net/ipv6/sit.c | 6 ------
3 files changed, 1 insertion(+), 19 deletions(-)
--- a/net/ipv4/ip_vti.c 2012-08-15 08:59:22.958704223 -0700
+++ b/net/ipv4/ip_vti.c 2012-09-24 18:01:20.521904773 -0700
@@ -304,7 +304,6 @@ static int vti_err(struct sk_buff *skb,
err = -ENOENT;
- rcu_read_lock();
t = vti_tunnel_lookup(dev_net(skb->dev), iph->daddr, iph->saddr);
if (t == NULL)
goto out;
@@ -326,7 +325,6 @@ static int vti_err(struct sk_buff *skb,
t->err_count = 1;
t->err_time = jiffies;
out:
- rcu_read_unlock();
return err;
}
@@ -336,7 +334,6 @@ static int vti_rcv(struct sk_buff *skb)
struct ip_tunnel *tunnel;
const struct iphdr *iph = ip_hdr(skb);
- rcu_read_lock();
tunnel = vti_tunnel_lookup(dev_net(skb->dev), iph->saddr, iph->daddr);
if (tunnel != NULL) {
struct pcpu_tstats *tstats;
@@ -348,10 +345,8 @@ static int vti_rcv(struct sk_buff *skb)
u64_stats_update_end(&tstats->syncp);
skb->dev = tunnel->dev;
- rcu_read_unlock();
return 1;
}
- rcu_read_unlock();
return -1;
}
--- a/net/ipv4/ipip.c 2012-09-24 17:59:26.127058210 -0700
+++ b/net/ipv4/ipip.c 2012-09-24 18:00:28.410430210 -0700
@@ -365,8 +365,6 @@ static int ipip_err(struct sk_buff *skb,
}
err = -ENOENT;
-
- rcu_read_lock();
t = ipip_tunnel_lookup(dev_net(skb->dev), iph->daddr, iph->saddr);
if (t == NULL)
goto out;
@@ -398,7 +396,7 @@ static int ipip_err(struct sk_buff *skb,
t->err_count = 1;
t->err_time = jiffies;
out:
- rcu_read_unlock();
+
return err;
}
@@ -416,13 +414,11 @@ static int ipip_rcv(struct sk_buff *skb)
struct ip_tunnel *tunnel;
const struct iphdr *iph = ip_hdr(skb);
- rcu_read_lock();
tunnel = ipip_tunnel_lookup(dev_net(skb->dev), iph->saddr, iph->daddr);
if (tunnel != NULL) {
struct pcpu_tstats *tstats;
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
- rcu_read_unlock();
kfree_skb(skb);
return 0;
}
@@ -445,11 +441,8 @@ static int ipip_rcv(struct sk_buff *skb)
ipip_ecn_decapsulate(iph, skb);
netif_rx(skb);
-
- rcu_read_unlock();
return 0;
}
- rcu_read_unlock();
return -1;
}
--- a/net/ipv6/sit.c 2012-08-15 08:59:22.986703941 -0700
+++ b/net/ipv6/sit.c 2012-09-24 18:02:28.753216804 -0700
@@ -545,7 +545,6 @@ static int ipip6_err(struct sk_buff *skb
err = -ENOENT;
- rcu_read_lock();
t = ipip6_tunnel_lookup(dev_net(skb->dev),
skb->dev,
iph->daddr,
@@ -579,7 +578,6 @@ static int ipip6_err(struct sk_buff *skb
t->err_count = 1;
t->err_time = jiffies;
out:
- rcu_read_unlock();
return err;
}
@@ -599,7 +597,6 @@ static int ipip6_rcv(struct sk_buff *skb
iph = ip_hdr(skb);
- rcu_read_lock();
tunnel = ipip6_tunnel_lookup(dev_net(skb->dev), skb->dev,
iph->saddr, iph->daddr);
if (tunnel != NULL) {
@@ -615,7 +612,6 @@ static int ipip6_rcv(struct sk_buff *skb
if ((tunnel->dev->priv_flags & IFF_ISATAP) &&
!isatap_chksrc(skb, iph, tunnel)) {
tunnel->dev->stats.rx_errors++;
- rcu_read_unlock();
kfree_skb(skb);
return 0;
}
@@ -630,12 +626,10 @@ static int ipip6_rcv(struct sk_buff *skb
netif_rx(skb);
- rcu_read_unlock();
return 0;
}
/* no tunnel matched, let upstream know, ipsec may handle it */
- rcu_read_unlock();
return 1;
out:
kfree_skb(skb);
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH net-next 4/4] tunnel: drop packet if ECN present with not-ECT
2012-09-25 4:12 [PATCH net-next 0/4] Tunnel related patches Stephen Hemminger
` (2 preceding siblings ...)
2012-09-25 4:12 ` [PATCH net-next 3/4] xfrm: remove extranous rcu_read_lock Stephen Hemminger
@ 2012-09-25 4:12 ` Stephen Hemminger
2012-09-25 21:02 ` [PATCHv3 " Stephen Hemminger
2012-09-27 22:13 ` [PATCH net-next 0/4] Tunnel related patches David Miller
4 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2012-09-25 4:12 UTC (permalink / raw)
To: davem; +Cc: netdev
[-- Attachment #1: gre-rfc-6040.patch --]
[-- Type: text/plain, Size: 7420 bytes --]
Linux tunnels were written before RFC6040 and therefore never
implemented the corner case of ECN getting set in the outer header
and the inner header not being ready for it.
Section 4.2. Default Tunnel Egress Behaviour.
The new code addresses:
o If the inner ECN field is Not-ECT, the decapsulator MUST NOT
propagate any other ECN codepoint onwards. This is because the
inner Not-ECT marking is set by transports that rely on dropped
packets as an indication of congestion and would not understand or
respond to any other ECN codepoint [RFC4774]. Specifically:
* If the inner ECN field is Not-ECT and the outer ECN field is
CE, the decapsulator MUST drop the packet.
* If the inner ECN field is Not-ECT and the outer ECN field is
Not-ECT, ECT(0), or ECT(1), the decapsulator MUST forward the
outgoing packet with the ECN field cleared to Not-ECT.
Overloads rx_frame_error to keep track of this error.
This was caught by Chris Wright while reviewing the new VXLAN tunnel.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
Note: supersedes earlier patch which only did Ipv4 GRE.
net/ipv4/ip_gre.c | 23 +++++++++++------
net/ipv4/ipip.c | 19 ++++++++------
net/ipv4/xfrm4_mode_tunnel.c | 12 +++++---
net/ipv6/ip6_gre.c | 58 +++++++++++++++++++++++--------------------
4 files changed, 67 insertions(+), 45 deletions(-)
--- a/net/ipv4/ip_gre.c 2012-09-24 18:11:52.000000000 -0700
+++ b/net/ipv4/ip_gre.c 2012-09-24 18:13:13.098719907 -0700
@@ -204,7 +204,9 @@ static struct rtnl_link_stats64 *ipgre_g
tot->rx_crc_errors = dev->stats.rx_crc_errors;
tot->rx_fifo_errors = dev->stats.rx_fifo_errors;
tot->rx_length_errors = dev->stats.rx_length_errors;
+ tot->rx_frame_errors = dev->stats.rx_frame_errors;
tot->rx_errors = dev->stats.rx_errors;
+
tot->tx_fifo_errors = dev->stats.tx_fifo_errors;
tot->tx_carrier_errors = dev->stats.tx_carrier_errors;
tot->tx_dropped = dev->stats.tx_dropped;
@@ -587,15 +589,16 @@ static void ipgre_err(struct sk_buff *sk
t->err_time = jiffies;
}
-static inline void ipgre_ecn_decapsulate(const struct iphdr *iph, struct sk_buff *skb)
+static int ipgre_ecn_decapsulate(const struct iphdr *iph, struct sk_buff *skb)
{
if (INET_ECN_is_ce(iph->tos)) {
if (skb->protocol == htons(ETH_P_IP)) {
- IP_ECN_set_ce(ip_hdr(skb));
+ return IP_ECN_set_ce(ip_hdr(skb));
} else if (skb->protocol == htons(ETH_P_IPV6)) {
- IP6_ECN_set_ce(ipv6_hdr(skb));
+ return IP6_ECN_set_ce(ipv6_hdr(skb));
}
}
+ return 1;
}
static inline u8
@@ -723,17 +726,21 @@ static int ipgre_rcv(struct sk_buff *skb
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
}
+ __skb_tunnel_rx(skb, tunnel->dev);
+
+ skb_reset_network_header(skb);
+ if (!ipgre_ecn_decapsulate(iph, skb)) {
+ ++tunnel->dev->stats.rx_frame_errors;
+ ++tunnel->dev->stats.rx_errors;
+ goto drop;
+ }
+
tstats = this_cpu_ptr(tunnel->dev->tstats);
u64_stats_update_begin(&tstats->syncp);
tstats->rx_packets++;
tstats->rx_bytes += skb->len;
u64_stats_update_end(&tstats->syncp);
- __skb_tunnel_rx(skb, tunnel->dev);
-
- skb_reset_network_header(skb);
- ipgre_ecn_decapsulate(iph, skb);
-
netif_rx(skb);
return 0;
--- a/net/ipv4/ipip.c 2012-09-24 18:12:33.000000000 -0700
+++ b/net/ipv4/ipip.c 2012-09-24 18:13:13.098719907 -0700
@@ -400,13 +400,14 @@ out:
return err;
}
-static inline void ipip_ecn_decapsulate(const struct iphdr *outer_iph,
- struct sk_buff *skb)
+static int ipip_ecn_decapsulate(const struct iphdr *outer_iph,
+ struct sk_buff *skb)
{
struct iphdr *inner_iph = ip_hdr(skb);
if (INET_ECN_is_ce(outer_iph->tos))
- IP_ECN_set_ce(inner_iph);
+ return IP_ECN_set_ce(inner_iph);
+ return 1;
}
static int ipip_rcv(struct sk_buff *skb)
@@ -430,16 +431,20 @@ static int ipip_rcv(struct sk_buff *skb)
skb->protocol = htons(ETH_P_IP);
skb->pkt_type = PACKET_HOST;
+ __skb_tunnel_rx(skb, tunnel->dev);
+
+ if (!ipip_ecn_decapsulate(iph, skb)) {
+ ++tunnel->dev->stats.rx_frame_errors;
+ ++tunnel->dev->stats.rx_errors;
+ return 0;
+ }
+
tstats = this_cpu_ptr(tunnel->dev->tstats);
u64_stats_update_begin(&tstats->syncp);
tstats->rx_packets++;
tstats->rx_bytes += skb->len;
u64_stats_update_end(&tstats->syncp);
- __skb_tunnel_rx(skb, tunnel->dev);
-
- ipip_ecn_decapsulate(iph, skb);
-
netif_rx(skb);
return 0;
}
--- a/net/ipv6/ip6_gre.c 2012-09-24 18:12:00.783449055 -0700
+++ b/net/ipv6/ip6_gre.c 2012-09-24 18:13:13.098719907 -0700
@@ -149,7 +149,9 @@ static struct rtnl_link_stats64 *ip6gre_
tot->rx_crc_errors = dev->stats.rx_crc_errors;
tot->rx_fifo_errors = dev->stats.rx_fifo_errors;
tot->rx_length_errors = dev->stats.rx_length_errors;
+ tot->rx_frame_errors = dev->stats.rx_frame_errors;
tot->rx_errors = dev->stats.rx_errors;
+
tot->tx_fifo_errors = dev->stats.tx_fifo_errors;
tot->tx_carrier_errors = dev->stats.tx_carrier_errors;
tot->tx_dropped = dev->stats.tx_dropped;
@@ -489,26 +491,28 @@ static void ip6gre_err(struct sk_buff *s
t->err_time = jiffies;
}
-static inline void ip6gre_ecn_decapsulate_ipv4(const struct ip6_tnl *t,
- const struct ipv6hdr *ipv6h, struct sk_buff *skb)
-{
- __u8 dsfield = ipv6_get_dsfield(ipv6h) & ~INET_ECN_MASK;
-
- if (t->parms.flags & IP6_TNL_F_RCV_DSCP_COPY)
- ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK, dsfield);
-
- if (INET_ECN_is_ce(dsfield))
- IP_ECN_set_ce(ip_hdr(skb));
-}
+static int ip6gre_ecn_decapsulate(const struct ip6_tnl *t,
+ const struct ipv6hdr *ipv6h,
+ struct sk_buff *skb)
+{
+ __u8 dsfield = ipv6_get_dsfield(ipv6h);
+ if (skb->protocol == htons(ETH_P_IP)) {
+ dsfield &= ~INET_ECN_MASK;
+
+ if (t->parms.flags & IP6_TNL_F_RCV_DSCP_COPY)
+ ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK,
+ dsfield);
+ if (INET_ECN_is_ce(dsfield))
+ return IP_ECN_set_ce(ip_hdr(skb));
+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ if (t->parms.flags & IP6_TNL_F_RCV_DSCP_COPY)
+ ipv6_copy_dscp(dsfield, ipv6_hdr(skb));
-static inline void ip6gre_ecn_decapsulate_ipv6(const struct ip6_tnl *t,
- const struct ipv6hdr *ipv6h, struct sk_buff *skb)
-{
- if (t->parms.flags & IP6_TNL_F_RCV_DSCP_COPY)
- ipv6_copy_dscp(ipv6_get_dsfield(ipv6h), ipv6_hdr(skb));
+ if (INET_ECN_is_ce(ipv6_get_dsfield(ipv6h)))
+ return IP6_ECN_set_ce(ipv6_hdr(skb));
+ }
- if (INET_ECN_is_ce(ipv6_get_dsfield(ipv6h)))
- IP6_ECN_set_ce(ipv6_hdr(skb));
+ return 1;
}
static int ip6gre_rcv(struct sk_buff *skb)
@@ -625,20 +629,22 @@ static int ip6gre_rcv(struct sk_buff *sk
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
}
+ __skb_tunnel_rx(skb, tunnel->dev);
+
+ skb_reset_network_header(skb);
+
+ if (!ip6gre_ecn_decapsulate(tunnel, ipv6h, skb)) {
+ ++tunnel->dev->stats.rx_frame_errors;
+ ++tunnel->dev->stats.rx_errors;
+ goto drop;
+ }
+
tstats = this_cpu_ptr(tunnel->dev->tstats);
u64_stats_update_begin(&tstats->syncp);
tstats->rx_packets++;
tstats->rx_bytes += skb->len;
u64_stats_update_end(&tstats->syncp);
- __skb_tunnel_rx(skb, tunnel->dev);
-
- skb_reset_network_header(skb);
- if (skb->protocol == htons(ETH_P_IP))
- ip6gre_ecn_decapsulate_ipv4(tunnel, ipv6h, skb);
- else if (skb->protocol == htons(ETH_P_IPV6))
- ip6gre_ecn_decapsulate_ipv6(tunnel, ipv6h, skb);
-
netif_rx(skb);
return 0;
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCHv3 net-next 4/4] tunnel: drop packet if ECN present with not-ECT
2012-09-25 4:12 ` [PATCH net-next 4/4] tunnel: drop packet if ECN present with not-ECT Stephen Hemminger
@ 2012-09-25 21:02 ` Stephen Hemminger
0 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2012-09-25 21:02 UTC (permalink / raw)
To: Chris Wright, davem; +Cc: netdev
Linux tunnels were written before RFC6040 and therefore never
implemented the corner case of ECN getting set in the outer header
and the inner header not being ready for it.
Section 4.2. Default Tunnel Egress Behaviour.
o If the inner ECN field is Not-ECT, the decapsulator MUST NOT
propagate any other ECN codepoint onwards. This is because the
inner Not-ECT marking is set by transports that rely on dropped
packets as an indication of congestion and would not understand or
respond to any other ECN codepoint [RFC4774]. Specifically:
* If the inner ECN field is Not-ECT and the outer ECN field is
CE, the decapsulator MUST drop the packet.
* If the inner ECN field is Not-ECT and the outer ECN field is
Not-ECT, ECT(0), or ECT(1), the decapsulator MUST forward the
outgoing packet with the ECN field cleared to Not-ECT.
This patch moves the ECN decap logic out of the individual tunnels
into a common place.
It also adds logging to allow detecting broken systems that
set ECN bits incorrectly when tunneling (or an intermediate
router might be changing the header).
Overloads rx_frame_error to keep track of ECN related error.
Thanks to Chris Wright who caught this while reviewing the new VXLAN
tunnel.
This code was tested by injecting faulty logic in other end GRE
to send incorrectly encapsulated packets.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
v3 - add common code and logging for ECN broken encapsulation
v2 - supersedes earlier GRE only version
include/net/inet_ecn.h | 76 +++++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/ip_gre.c | 38 ++++++++++++++----------
net/ipv4/ipip.c | 42 ++++++++++++++++-----------
net/ipv6/ip6_gre.c | 54 +++++++++++++++-------------------
4 files changed, 147 insertions(+), 63 deletions(-)
--- a/net/ipv4/ip_gre.c 2012-09-25 08:37:13.995876570 -0700
+++ b/net/ipv4/ip_gre.c 2012-09-25 11:00:46.918340181 -0700
@@ -120,6 +120,10 @@
Alexey Kuznetsov.
*/
+static bool log_ecn_error = true;
+module_param(log_ecn_error, bool, 0644);
+MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
+
static struct rtnl_link_ops ipgre_link_ops __read_mostly;
static int ipgre_tunnel_init(struct net_device *dev);
static void ipgre_tunnel_setup(struct net_device *dev);
@@ -204,7 +208,9 @@ static struct rtnl_link_stats64 *ipgre_g
tot->rx_crc_errors = dev->stats.rx_crc_errors;
tot->rx_fifo_errors = dev->stats.rx_fifo_errors;
tot->rx_length_errors = dev->stats.rx_length_errors;
+ tot->rx_frame_errors = dev->stats.rx_frame_errors;
tot->rx_errors = dev->stats.rx_errors;
+
tot->tx_fifo_errors = dev->stats.tx_fifo_errors;
tot->tx_carrier_errors = dev->stats.tx_carrier_errors;
tot->tx_dropped = dev->stats.tx_dropped;
@@ -587,17 +593,6 @@ static void ipgre_err(struct sk_buff *sk
t->err_time = jiffies;
}
-static inline void ipgre_ecn_decapsulate(const struct iphdr *iph, struct sk_buff *skb)
-{
- if (INET_ECN_is_ce(iph->tos)) {
- if (skb->protocol == htons(ETH_P_IP)) {
- IP_ECN_set_ce(ip_hdr(skb));
- } else if (skb->protocol == htons(ETH_P_IPV6)) {
- IP6_ECN_set_ce(ipv6_hdr(skb));
- }
- }
-}
-
static inline u8
ipgre_ecn_encapsulate(u8 tos, const struct iphdr *old_iph, struct sk_buff *skb)
{
@@ -620,6 +615,7 @@ static int ipgre_rcv(struct sk_buff *skb
struct ip_tunnel *tunnel;
int offset = 4;
__be16 gre_proto;
+ int err;
if (!pskb_may_pull(skb, 16))
goto drop;
@@ -723,17 +719,27 @@ static int ipgre_rcv(struct sk_buff *skb
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
}
+ __skb_tunnel_rx(skb, tunnel->dev);
+
+ skb_reset_network_header(skb);
+ err = IP_ECN_decapsulate(iph, skb);
+ if (unlikely(err)) {
+ if (log_ecn_error)
+ net_info_ratelimited("non-ECT from %pI4 with TOS=%#x\n",
+ &iph->saddr, iph->tos);
+ if (err > 1) {
+ ++tunnel->dev->stats.rx_frame_errors;
+ ++tunnel->dev->stats.rx_errors;
+ goto drop;
+ }
+ }
+
tstats = this_cpu_ptr(tunnel->dev->tstats);
u64_stats_update_begin(&tstats->syncp);
tstats->rx_packets++;
tstats->rx_bytes += skb->len;
u64_stats_update_end(&tstats->syncp);
- __skb_tunnel_rx(skb, tunnel->dev);
-
- skb_reset_network_header(skb);
- ipgre_ecn_decapsulate(iph, skb);
-
netif_rx(skb);
return 0;
--- a/net/ipv4/ipip.c 2012-09-25 08:37:14.791868701 -0700
+++ b/net/ipv4/ipip.c 2012-09-25 11:01:41.733794349 -0700
@@ -120,6 +120,10 @@
#define HASH_SIZE 16
#define HASH(addr) (((__force u32)addr^((__force u32)addr>>4))&0xF)
+static bool log_ecn_error = true;
+module_param(log_ecn_error, bool, 0644);
+MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
+
static int ipip_net_id __read_mostly;
struct ipip_net {
struct ip_tunnel __rcu *tunnels_r_l[HASH_SIZE];
@@ -400,28 +404,18 @@ out:
return err;
}
-static inline void ipip_ecn_decapsulate(const struct iphdr *outer_iph,
- struct sk_buff *skb)
-{
- struct iphdr *inner_iph = ip_hdr(skb);
-
- if (INET_ECN_is_ce(outer_iph->tos))
- IP_ECN_set_ce(inner_iph);
-}
-
static int ipip_rcv(struct sk_buff *skb)
{
struct ip_tunnel *tunnel;
const struct iphdr *iph = ip_hdr(skb);
+ int err;
tunnel = ipip_tunnel_lookup(dev_net(skb->dev), iph->saddr, iph->daddr);
if (tunnel != NULL) {
struct pcpu_tstats *tstats;
- if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
- kfree_skb(skb);
- return 0;
- }
+ if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
+ goto drop;
secpath_reset(skb);
@@ -430,21 +424,35 @@ static int ipip_rcv(struct sk_buff *skb)
skb->protocol = htons(ETH_P_IP);
skb->pkt_type = PACKET_HOST;
+ __skb_tunnel_rx(skb, tunnel->dev);
+
+ err = IP_ECN_decapsulate(iph, skb);
+ if (unlikely(err)) {
+ if (log_ecn_error)
+ net_info_ratelimited("non-ECT from %pI4 with TOS=%#x\n",
+ &iph->saddr, iph->tos);
+ if (err > 1) {
+ ++tunnel->dev->stats.rx_frame_errors;
+ ++tunnel->dev->stats.rx_errors;
+ goto drop;
+ }
+ }
+
tstats = this_cpu_ptr(tunnel->dev->tstats);
u64_stats_update_begin(&tstats->syncp);
tstats->rx_packets++;
tstats->rx_bytes += skb->len;
u64_stats_update_end(&tstats->syncp);
- __skb_tunnel_rx(skb, tunnel->dev);
-
- ipip_ecn_decapsulate(iph, skb);
-
netif_rx(skb);
return 0;
}
return -1;
+
+drop:
+ kfree_skb(skb);
+ return 0;
}
/*
--- a/net/ipv6/ip6_gre.c 2012-09-25 08:37:13.995876570 -0700
+++ b/net/ipv6/ip6_gre.c 2012-09-25 11:00:35.478454105 -0700
@@ -56,6 +56,10 @@
#include <net/ip6_tunnel.h>
+static bool log_ecn_error = true;
+module_param(log_ecn_error, bool, 0644);
+MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
+
#define IPV6_TCLASS_MASK (IPV6_FLOWINFO_MASK & ~IPV6_FLOWLABEL_MASK)
#define IPV6_TCLASS_SHIFT 20
@@ -149,7 +153,9 @@ static struct rtnl_link_stats64 *ip6gre_
tot->rx_crc_errors = dev->stats.rx_crc_errors;
tot->rx_fifo_errors = dev->stats.rx_fifo_errors;
tot->rx_length_errors = dev->stats.rx_length_errors;
+ tot->rx_frame_errors = dev->stats.rx_frame_errors;
tot->rx_errors = dev->stats.rx_errors;
+
tot->tx_fifo_errors = dev->stats.tx_fifo_errors;
tot->tx_carrier_errors = dev->stats.tx_carrier_errors;
tot->tx_dropped = dev->stats.tx_dropped;
@@ -489,28 +495,6 @@ static void ip6gre_err(struct sk_buff *s
t->err_time = jiffies;
}
-static inline void ip6gre_ecn_decapsulate_ipv4(const struct ip6_tnl *t,
- const struct ipv6hdr *ipv6h, struct sk_buff *skb)
-{
- __u8 dsfield = ipv6_get_dsfield(ipv6h) & ~INET_ECN_MASK;
-
- if (t->parms.flags & IP6_TNL_F_RCV_DSCP_COPY)
- ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK, dsfield);
-
- if (INET_ECN_is_ce(dsfield))
- IP_ECN_set_ce(ip_hdr(skb));
-}
-
-static inline void ip6gre_ecn_decapsulate_ipv6(const struct ip6_tnl *t,
- const struct ipv6hdr *ipv6h, struct sk_buff *skb)
-{
- if (t->parms.flags & IP6_TNL_F_RCV_DSCP_COPY)
- ipv6_copy_dscp(ipv6_get_dsfield(ipv6h), ipv6_hdr(skb));
-
- if (INET_ECN_is_ce(ipv6_get_dsfield(ipv6h)))
- IP6_ECN_set_ce(ipv6_hdr(skb));
-}
-
static int ip6gre_rcv(struct sk_buff *skb)
{
const struct ipv6hdr *ipv6h;
@@ -522,6 +506,7 @@ static int ip6gre_rcv(struct sk_buff *sk
struct ip6_tnl *tunnel;
int offset = 4;
__be16 gre_proto;
+ int err;
if (!pskb_may_pull(skb, sizeof(struct in6_addr)))
goto drop;
@@ -625,20 +610,29 @@ static int ip6gre_rcv(struct sk_buff *sk
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
}
+ __skb_tunnel_rx(skb, tunnel->dev);
+
+ skb_reset_network_header(skb);
+
+ err = IP6_ECN_decapsulate(ipv6h, skb);
+ if (unlikely(err)) {
+ if (log_ecn_error)
+ net_info_ratelimited("non-ECT from %pI6 with dsfield=%#x\n",
+ &ipv6h->saddr,
+ ipv6_get_dsfield(ipv6h));
+ if (err > 1) {
+ ++tunnel->dev->stats.rx_frame_errors;
+ ++tunnel->dev->stats.rx_errors;
+ goto drop;
+ }
+ }
+
tstats = this_cpu_ptr(tunnel->dev->tstats);
u64_stats_update_begin(&tstats->syncp);
tstats->rx_packets++;
tstats->rx_bytes += skb->len;
u64_stats_update_end(&tstats->syncp);
- __skb_tunnel_rx(skb, tunnel->dev);
-
- skb_reset_network_header(skb);
- if (skb->protocol == htons(ETH_P_IP))
- ip6gre_ecn_decapsulate_ipv4(tunnel, ipv6h, skb);
- else if (skb->protocol == htons(ETH_P_IPV6))
- ip6gre_ecn_decapsulate_ipv6(tunnel, ipv6h, skb);
-
netif_rx(skb);
return 0;
--- a/include/net/inet_ecn.h 2012-08-15 08:59:22.766706151 -0700
+++ b/include/net/inet_ecn.h 2012-09-25 10:50:45.408313935 -0700
@@ -15,6 +15,8 @@ enum {
INET_ECN_MASK = 3,
};
+extern int sysctl_tunnel_ecn_log;
+
static inline int INET_ECN_is_ce(__u8 dsfield)
{
return (dsfield & INET_ECN_MASK) == INET_ECN_CE;
@@ -145,4 +147,78 @@ static inline int INET_ECN_set_ce(struct
return 0;
}
+/*
+ * RFC 6080 4.2
+ * To decapsulate the inner header at the tunnel egress, a compliant
+ * tunnel egress MUST set the outgoing ECN field to the codepoint at the
+ * intersection of the appropriate arriving inner header (row) and outer
+ * header (column) in Figure 4
+ *
+ * +---------+------------------------------------------------+
+ * |Arriving | Arriving Outer Header |
+ * | Inner +---------+------------+------------+------------+
+ * | Header | Not-ECT | ECT(0) | ECT(1) | CE |
+ * +---------+---------+------------+------------+------------+
+ * | Not-ECT | Not-ECT |Not-ECT(!!!)|Not-ECT(!!!)| <drop>(!!!)|
+ * | ECT(0) | ECT(0) | ECT(0) | ECT(1) | CE |
+ * | ECT(1) | ECT(1) | ECT(1) (!) | ECT(1) | CE |
+ * | CE | CE | CE | CE(!!!)| CE |
+ * +---------+---------+------------+------------+------------+
+ *
+ * Figure 4: New IP in IP Decapsulation Behaviour
+ *
+ * returns 0 on success
+ * 1 if something is broken and should be logged (!!! above)
+ * 2 if packet should be dropped
+ */
+static inline int INET_ECN_decapsulate(struct sk_buff *skb,
+ __u8 outer, __u8 inner)
+{
+ if (INET_ECN_is_not_ect(inner)) {
+ switch (outer & INET_ECN_MASK) {
+ case INET_ECN_NOT_ECT:
+ return 0;
+ case INET_ECN_ECT_0:
+ case INET_ECN_ECT_1:
+ return 1;
+ case INET_ECN_CE:
+ return 2;
+ }
+ }
+
+ if (INET_ECN_is_ce(outer))
+ INET_ECN_set_ce(skb);
+
+ return 0;
+}
+
+static inline int IP_ECN_decapsulate(const struct iphdr *oiph,
+ struct sk_buff *skb)
+{
+ __u8 inner;
+
+ if (skb->protocol == htons(ETH_P_IP))
+ inner = ip_hdr(skb)->tos;
+ else if (skb->protocol == htons(ETH_P_IPV6))
+ inner = ipv6_get_dsfield(ipv6_hdr(skb));
+ else
+ return 0;
+
+ return INET_ECN_decapsulate(skb, oiph->tos, inner);
+}
+
+static inline int IP6_ECN_decapsulate(const struct ipv6hdr *oipv6h,
+ struct sk_buff *skb)
+{
+ __u8 inner;
+
+ if (skb->protocol == htons(ETH_P_IP))
+ inner = ip_hdr(skb)->tos;
+ else if (skb->protocol == htons(ETH_P_IPV6))
+ inner = ipv6_get_dsfield(ipv6_hdr(skb));
+ else
+ return 0;
+
+ return INET_ECN_decapsulate(skb, ipv6_get_dsfield(oipv6h), inner);
+}
#endif
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 0/4] Tunnel related patches
2012-09-25 4:12 [PATCH net-next 0/4] Tunnel related patches Stephen Hemminger
` (3 preceding siblings ...)
2012-09-25 4:12 ` [PATCH net-next 4/4] tunnel: drop packet if ECN present with not-ECT Stephen Hemminger
@ 2012-09-27 22:13 ` David Miller
4 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2012-09-27 22:13 UTC (permalink / raw)
To: shemminger; +Cc: netdev
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 24 Sep 2012 21:12:22 -0700
> These are a set of small tunnel related patches.
Series applied, with the updated version of patch #4.
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread