Hi Pravin,
On 12/12/2020 06:51, Pravin Shelar wrote:
On Fri, Dec 11, 2020 at 4:29 AM Jonas Bonn <jo...@norrbonn.se> wrote:
This patch adds support for handling IPv6. Both the GTP tunnel and the
tunneled packets may be IPv6; as they constitute independent streams,
both v4-over-v6 and v6-over-v4 are supported, as well.
This patch includes only the driver functionality for IPv6 support. A
follow-on patch will add support for configuring the tunnels with IPv6
addresses.
Signed-off-by: Jonas Bonn <jo...@norrbonn.se>
---
drivers/net/gtp.c | 330 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 269 insertions(+), 61 deletions(-)
+ /* Get IP version of _inner_ packet */
+ ipver = inner_ip_hdr(skb)->version;
+
+ switch (ipver) {
+ case 4:
+ skb_set_inner_protocol(skb, cpu_to_be16(ETH_P_IP));
+ r = gtp_check_ms_ipv4(skb, pctx, hdrlen, role);
I don't see a need to set inner header on receive path, we are any
ways removing outer header from this packet in same function.
+ break;
+ case 6:
+ skb_set_inner_protocol(skb, cpu_to_be16(ETH_P_IPV6));
+ r = gtp_check_ms_ipv6(skb, pctx, hdrlen, role);
+ break;
+ }
+
+ if (!r) {
netdev_dbg(pctx->dev, "No PDP ctx for this MS\n");
return 1;
}
@@ -193,6 +256,8 @@ static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb,
!net_eq(sock_net(pctx->sk),
dev_net(pctx->dev))))
return -1;
+ skb->protocol = skb->inner_protocol;
+
iptunnel_pull_header() can set the protocol, so it would be better to
pass the correct inner protocol.
Yes, your comments above are correct. I'll fix that.
netdev_dbg(pctx->dev, "forwarding packet from GGSN to uplink\n");
/* Now that the UDP and the GTP header have been removed, set up the
@@ -201,7 +266,7 @@ static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb,
*/
skb_reset_network_header(skb);
- skb->dev = pctx->dev;
+ __skb_tunnel_rx(skb, pctx->dev, sock_net(pctx->sk));
No need to call skb_tunnel_rx() given iptunnel_pull_header() function
is already called and it does take care of clearing the context.
Right. The only difference seems to be that __skb_tunnel_rx also does:
skb->dev = dev;
iptunnel_pull_header excludes that. I can't see that setting the
skb->dev will actually change anything for this driver, but it was there
previously. Thoughts?
+static struct dst_entry *gtp_get_v6_dst(struct sk_buff *skb,
+ struct net_device *dev,
+ struct pdp_ctx *pctx,
+ struct in6_addr *saddr)
+{
+ const struct sock *sk = pctx->sk;
+ struct dst_entry *dst = NULL;
+ struct flowi6 fl6;
+
+ memset(&fl6, 0, sizeof(fl6));
+ fl6.flowi6_mark = skb->mark;
+ fl6.flowi6_proto = IPPROTO_UDP;
+ fl6.daddr = pctx->peer_addr;
+
+ dst = ipv6_stub->ipv6_dst_lookup_flow(sock_net(sk), sk, &fl6, NULL);
+ if (IS_ERR(dst)) {
+ netdev_dbg(pctx->dev, "no route to %pI6\n", &fl6.daddr);
+ return ERR_PTR(-ENETUNREACH);
+ }
+ if (dst->dev == pctx->dev) {
+ netdev_dbg(pctx->dev, "circular route to %pI6\n", &fl6.daddr);
+ dst_release(dst);
+ return ERR_PTR(-ELOOP);
+ }
+
+ *saddr = fl6.saddr;
+
+ return dst;
+}
+
IPv6 related functionality needs to be protected by IS_ENABLED(CONFIG_IPV6).
Yes, you're probably right. Given that IPv6 isn't really optional in
contexts where this driver is relevant, however, I'm almost inclined to
switch this around and make the driver depend on the availability of IPv6...
/Jonas
