[PATCH net-next 0/2] lwtunnel: encap locally-generated ipv4 packets

[PATCH net-next 0/2] lwtunnel: encap locally-generated ipv4 packets

[Robert Shearman]

Locally-generated IPv4 packets, such as from applications running on
the host or traceroute/ping currently don't have lwtunnel output
redirected encap applied. However, they should do in the same way as
for forwarded packets and this patch series addresses that.

Robert Shearman (2):
  lwtunnel: set skb protocol and dev
  ipv4: apply lwtunnel encap for locally-generated packets

 net/core/lwtunnel.c | 12 ++++++++++--
 net/ipv4/route.c    |  2 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.1.4

[PATCH net-next 1/2] lwtunnel: set skb protocol and dev

[Robert Shearman]

In the locally-generated packet path skb->protocol may not be set and
this is required for the lwtunnel encap in order to get the lwtstate.

This would otherwise have been set by ip_output or ip6_output so set
skb->protocol prior to calling the lwtunnel encap
function. Additionally set skb->dev in case it is needed further down
the transmit path.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 net/core/lwtunnel.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index c240c895b319..5d6d8e3d450a 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -215,8 +215,12 @@ int lwtunnel_output6(struct sock *sk, struct sk_buff *skb)
 	struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
 	struct lwtunnel_state *lwtstate = NULL;
 
-	if (rt)
+	if (rt) {
 		lwtstate = rt->rt6i_lwtstate;
+		skb->dev = rt->dst.dev;
+	}
+
+	skb->protocol = htons(ETH_P_IPV6);
 
 	return __lwtunnel_output(sk, skb, lwtstate);
 }
@@ -227,8 +231,12 @@ int lwtunnel_output(struct sock *sk, struct sk_buff *skb)
 	struct rtable *rt = (struct rtable *)skb_dst(skb);
 	struct lwtunnel_state *lwtstate = NULL;
 
-	if (rt)
+	if (rt) {
 		lwtstate = rt->rt_lwtstate;
+		skb->dev = rt->dst.dev;
+	}
+
+	skb->protocol = htons(ETH_P_IP);
 
 	return __lwtunnel_output(sk, skb, lwtstate);
 }
-- 
2.1.4

[PATCH net-next 2/2] ipv4: apply lwtunnel encap for locally-generated packets

[Robert Shearman]

lwtunnel encap is applied for forwarded packets, but not for
locally-generated packets. This is because the output function is not
overridden in __mkroute_output, unlike it is in __mkroute_input.

The lwtunnel state is correctly set on the rth through the call to
rt_set_nexthop, so all that needs to be done is to override the dst
output function to be lwtunnel_output if there is lwtunnel state
present and it requires output redirection.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 net/ipv4/route.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 908f7ef2f12a..18fd7c9095c7 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2022,6 +2022,8 @@ add:
 	}
 
 	rt_set_nexthop(rth, fl4->daddr, res, fnhe, fi, type, 0);
+	if (lwtunnel_output_redirect(rth->rt_lwtstate))
+		rth->dst.output = lwtunnel_output;
 
 	return rth;
 }
-- 
2.1.4

Re: [PATCH net-next 0/2] lwtunnel: encap locally-generated ipv4 packets

[roopa]

On 8/3/15, 9:39 AM, Robert Shearman wrote:
> Locally-generated IPv4 packets, such as from applications running on
> the host or traceroute/ping currently don't have lwtunnel output
> redirected encap applied. However, they should do in the same way as
> for forwarded packets and this patch series addresses that.
>
> Robert Shearman (2):
>    lwtunnel: set skb protocol and dev
>    ipv4: apply lwtunnel encap for locally-generated packets
>
>   net/core/lwtunnel.c | 12 ++++++++++--
>   net/ipv4/route.c    |  2 ++
>   2 files changed, 12 insertions(+), 2 deletions(-)
>
Thanks for this patch Robert. Looks good.
I have been thinking of sending a similar patch out for this and
since i was also looking at ip fragmentation, I have a slightly 
different patch which I think should also take care of
encapsulating locally generated packets too. This patch moves the output 
redirection to after ip fragmentation.
What do you think about the below (I have briefly tested it. Was 
planning to test some more before sending it out as RFC) ?

[PATCH net-next] lwtunnel: move output redirection to after ip fragmentation

This patch adds tunnel headroom in lwtstate to make
sure we account for tunnel data in mtu calculations and
moves tunnel output redirection after ip fragmentation.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
  include/net/lwtunnel.h   |    1 +
  net/ipv4/ip_output.c     |    4 ++++
  net/ipv4/route.c         |    5 +++--
  net/mpls/mpls_iptunnel.c |    1 +
  4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index 918e03c..7816805 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -18,6 +18,7 @@ struct lwtunnel_state {
         __u16           flags;
         atomic_t        refcnt;
         int             len;
+       __u16           headroom;
         __u8            data[0];
  };

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 6bf89a6..ae3119f 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -73,6 +73,7 @@
  #include <net/icmp.h>
  #include <net/checksum.h>
  #include <net/inetpeer.h>
+#include <net/lwtunnel.h>
  #include <linux/igmp.h>
  #include <linux/netfilter_ipv4.h>
  #include <linux/netfilter_bridge.h>
@@ -201,6 +202,9 @@ static int ip_finish_output2(struct sock *sk, struct 
sk_buff *skb)
                 skb = skb2;
         }

+       if (lwtunnel_output_redirect(rt->rt_lwtstate))
+               return lwtunnel_output(sk, skb);
+
         rcu_read_lock_bh();
         nexthop = (__force u32) rt_nexthop(rt, ip_hdr(skb)->daddr);
         neigh = __ipv4_neigh_lookup_noref(dev, nexthop);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d3964fa..4e07b9a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1234,6 +1234,9 @@ static unsigned int ipv4_mtu(const struct 
dst_entry *dst)

         mtu = dst->dev->mtu;

+       if (lwtunnel_output_redirect(rt->rt_lwtstate))
+               mtu -= rt->rt_lwtstate->headroom;
+
         if (unlikely(dst_metric_locked(dst, RTAX_MTU))) {
                 if (rt->rt_uses_gateway && mtu > 576)
                         mtu = 576;
@@ -1634,8 +1637,6 @@ static int __mkroute_input(struct sk_buff *skb,
         rth->dst.output = ip_output;

         rt_set_nexthop(rth, daddr, res, fnhe, res->fi, res->type, itag);
-       if (lwtunnel_output_redirect(rth->rt_lwtstate))
-               rth->dst.output = lwtunnel_output;
         skb_dst_set(skb, &rth->dst);
  out:
         err = 0;

Re: [PATCH net-next 0/2] lwtunnel: encap locally-generated ipv4 packets

[David Miller]

From: Robert Shearman <rshearma@brocade.com>
Date: Mon, 3 Aug 2015 17:39:19 +0100

> Locally-generated IPv4 packets, such as from applications running on
> the host or traceroute/ping currently don't have lwtunnel output
> redirected encap applied. However, they should do in the same way as
> for forwarded packets and this patch series addresses that.

Series applied, thanks.

Re: [PATCH net-next 0/2] lwtunnel: encap locally-generated ipv4 packets

[Robert Shearman]

On 03/08/15 22:41, roopa wrote:
> On 8/3/15, 9:39 AM, Robert Shearman wrote:
>> Locally-generated IPv4 packets, such as from applications running on
>> the host or traceroute/ping currently don't have lwtunnel output
>> redirected encap applied. However, they should do in the same way as
>> for forwarded packets and this patch series addresses that.
>>
>> Robert Shearman (2):
>>    lwtunnel: set skb protocol and dev
>>    ipv4: apply lwtunnel encap for locally-generated packets
>>
>>   net/core/lwtunnel.c | 12 ++++++++++--
>>   net/ipv4/route.c    |  2 ++
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
> Thanks for this patch Robert. Looks good.
> I have been thinking of sending a similar patch out for this and
> since i was also looking at ip fragmentation, I have a slightly
> different patch which I think should also take care of
> encapsulating locally generated packets too. This patch moves the output
> redirection to after ip fragmentation.
> What do you think about the below (I have briefly tested it. Was
> planning to test some more before sending it out as RFC) ?

I'm glad you're looking at fragmentation - this does need to be 
implemented at some point.

While it looks like fragmentation should work, the issue is that now 
post-routing netfilter modules will be presented with un-encapsulated 
packets without distinguishing them from encapsulated packets.

An example of why this is a problem is that this would prevent operators 
from implementing rules to prevent non-control IP packets being output 
onto an interface in an MPLS core, and I have seen service providers 
doing this sort of thing in the past. So I think this is a pretty big 
deal for MPLS. There are possibly other less obvious use cases that 
would be prevented by this change.

So as long as you can keep these working, I'd be fine with such an approach.

Thanks,
Rob