[net-next, 0/3] seg6: improvements to Segment Routing in IPv6

[net-next, 0/3] seg6: improvements to Segment Routing in IPv6

[Andrea Mayer]

This patchset is divided in 3 patches and it introduces some improvements
to Segment Routing in IPv6, which are:

- in function get_srh() verify the srh pointer after calling
  pskb_may_pull();

- set skb->transport_header properly after calling decap_and_validate()
  function;

- allow local packet processing for SRv6 End.DT6 behavior.

Any comments on the patchset are welcome.

Thanks.


Andrea Mayer (3):
  verify srh pointer in get_srh()
  set skb transport_header properly after decap_and_validate()
  allow local packet processing for SRv6 End.DT6 behavior

 net/ipv6/seg6_local.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

-- 
2.20.1

[net-next, 1/3] seg6: verify srh pointer in get_srh()

[Andrea Mayer]

pskb_may_pull may change pointers in header. For this reason, it is
mandatory to reload any pointer that points into skb header.

Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
 net/ipv6/seg6_local.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 9d4f75e0d33a..e187dec2eed1 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -75,12 +75,16 @@ static struct ipv6_sr_hdr *get_srh(struct sk_buff *skb)
 		return NULL;
 
 	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
-
 	len = (srh->hdrlen + 1) << 3;
 
 	if (!pskb_may_pull(skb, srhoff + len))
 		return NULL;
 
+	/* note that pskb_may_pull may change pointers in header;
+	 * for this reason it is necessary to reload them when needed.
+	 */
+	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
+
 	if (!seg6_validate_srh(srh, len))
 		return NULL;
 
-- 
2.20.1

Re: [net-next, 1/3] seg6: verify srh pointer in get_srh()

[David Miller]

From: Andrea Mayer <andrea.mayer@uniroma2.it>
Date: Wed, 13 Nov 2019 20:29:10 +0100

> pskb_may_pull may change pointers in header. For this reason, it is
> mandatory to reload any pointer that points into skb header.
> 
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>

This is a bug fix and must be separated out and submitted to 'net'.

Then you must wait until 'net' is merged into 'net-next' so that you
can cleanly resubmit the other changes in this series which add the
new features.

Actually, patch #2 looks like a bug fix as well.

Re: [net-next, 1/3] seg6: verify srh pointer in get_srh()

[Andrea Mayer]

On Thu, 14 Nov 2019 17:45:12 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Andrea Mayer <andrea.mayer@uniroma2.it>
> Date: Wed, 13 Nov 2019 20:29:10 +0100
> 
> > pskb_may_pull may change pointers in header. For this reason, it is
> > mandatory to reload any pointer that points into skb header.
> > 
> > Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> 
> This is a bug fix and must be separated out and submitted to 'net'.
> 
> Then you must wait until 'net' is merged into 'net-next' so that you
> can cleanly resubmit the other changes in this series which add the
> new features.
> 
> Actually, patch #2 looks like a bug fix as well.

Hi,
thanks for your review. I will submit the first two patches to 'net'.

Regards,
Andrea Mayer

[net-next, 2/3] seg6: set skb transport_header properly after decap_and_validate()

[Andrea Mayer]

in the receive path (more precisely in ip6_rcv_core()) the
skb->transport_header is set to skb->network_header + sizeof(*hdr). As a
consequence, after routing operations, destination input expects to find
skb->transport_header correctly set to the next protocol (or extension
header) that follows the network protocol. However, decap behaviors (DX*,
DT*) remove the outer IPv6 and SRH extension and do not set again the
skb->transport_header pointer correctly. For this reason, the patch sets
the skb->transport_header to the skb->network_header + sizeof(hdr) in each
DX and DT* behavior.

Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
 net/ipv6/seg6_local.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index e187dec2eed1..948a4c2a59f2 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -340,6 +340,8 @@ static int input_action_end_dx6(struct sk_buff *skb,
 	if (!ipv6_addr_any(&slwt->nh6))
 		nhaddr = &slwt->nh6;
 
+	skb_set_transport_header(skb, sizeof(struct ipv6hdr));
+
 	seg6_lookup_nexthop(skb, nhaddr, 0);
 
 	return dst_input(skb);
@@ -369,6 +371,8 @@ static int input_action_end_dx4(struct sk_buff *skb,
 
 	skb_dst_drop(skb);
 
+	skb_set_transport_header(skb, sizeof(struct iphdr));
+
 	err = ip_route_input(skb, nhaddr, iph->saddr, 0, skb->dev);
 	if (err)
 		goto drop;
@@ -389,6 +393,8 @@ static int input_action_end_dt6(struct sk_buff *skb,
 	if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
 		goto drop;
 
+	skb_set_transport_header(skb, sizeof(struct ipv6hdr));
+
 	seg6_lookup_nexthop(skb, NULL, slwt->table);
 
 	return dst_input(skb);
-- 
2.20.1

[net-next, 3/3] seg6: allow local packet processing for SRv6 End.DT6 behavior

[Andrea Mayer]

End.DT6 behavior makes use of seg6_lookup_nexthop function which drops all
packets that are destined to be locally processed. However, DT* should be
able to delivery decapsulated packets that are destined to local addresses.
Function seg6_lookup_nexthop is also used by DX6, so in order to maintain
compatibility I created another routing helper function which is called
seg6_lookup_any_nexthop.
This function is able to take in to account both packets that have to be
processed locally and the ones that are destined to be forwarded directly
to another machine. Hence, seg6_lookup_any_nexthop is used in DT6 rather
than seg6_lookup_nexthop to allow local delivery.

Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
 net/ipv6/seg6_local.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 948a4c2a59f2..8a723ab3d29c 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -148,8 +148,9 @@ static void advance_nextseg(struct ipv6_sr_hdr *srh, struct in6_addr *daddr)
 	*daddr = *addr;
 }
 
-int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
-			u32 tbl_id)
+static int
+seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
+			u32 tbl_id, int local_delivery)
 {
 	struct net *net = dev_net(skb->dev);
 	struct ipv6hdr *hdr = ipv6_hdr(skb);
@@ -157,6 +158,7 @@ int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
 	struct dst_entry *dst = NULL;
 	struct rt6_info *rt;
 	struct flowi6 fl6;
+	int dev_flags = 0;
 
 	fl6.flowi6_iif = skb->dev->ifindex;
 	fl6.daddr = nhaddr ? *nhaddr : hdr->daddr;
@@ -181,7 +183,13 @@ int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
 		dst = &rt->dst;
 	}
 
-	if (dst && dst->dev->flags & IFF_LOOPBACK && !dst->error) {
+	/* we want to discard traffic destined for local packet processing,
+	 * if @local_delivery is set to false.
+	 */
+	if (!local_delivery)
+		dev_flags |= IFF_LOOPBACK;
+
+	if (dst && (dst->dev->flags & dev_flags) && !dst->error) {
 		dst_release(dst);
 		dst = NULL;
 	}
@@ -198,6 +206,12 @@ int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
 	return dst->error;
 }
 
+inline int seg6_lookup_nexthop(struct sk_buff *skb,
+			       struct in6_addr *nhaddr, u32 tbl_id)
+{
+	return seg6_lookup_any_nexthop(skb, nhaddr, tbl_id, false);
+}
+
 /* regular endpoint function */
 static int input_action_end(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 {
@@ -395,7 +409,7 @@ static int input_action_end_dt6(struct sk_buff *skb,
 
 	skb_set_transport_header(skb, sizeof(struct ipv6hdr));
 
-	seg6_lookup_nexthop(skb, NULL, slwt->table);
+	seg6_lookup_any_nexthop(skb, NULL, slwt->table, true);
 
 	return dst_input(skb);
 
-- 
2.20.1