* [PATCH net-next v2 00/12] gtp: IPv6 support
@ 2020-12-11 12:26 Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 01/12] gtp: set initial MTU Jonas Bonn
` (11 more replies)
0 siblings, 12 replies; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
This series adds IPv6 support to the GTP tunneling driver. This comes
by way of a series of fixes that lay the groundwork for IPv6, along with
some performance work including:
- support for GSO and GRO
- cleanups that align the GTP driver a bit closer to the bareudp and
geneve drivers, including use of more generic helpers where possible
This is v2. v1 was just the five patches at the bottom of this series
that I had hoped would get a quicker review given that they are mostly
trivial. The one comment I did get has been addressed here.
Changed in v2:
- added patches 6-12
- addressed comment from Jeremy Sowden on patch 2/5
Jonas Bonn (12):
gtp: set initial MTU
gtp: include role in link info
gtp: really check namespaces before xmit
gtp: drop unnecessary call to skb_dst_drop
gtp: set device type
gtp: rework IPv4 functionality
gtp: use ephemeral source port
gtp: set dev features to enable GSO
gtp: support GRO
gtp: add IPv6 support
gtp: netlink update for ipv6
gtp: add dst_cache to tunnels
drivers/net/Kconfig | 1 +
drivers/net/gtp.c | 774 +++++++++++++++++++++++++++++----------
include/uapi/linux/gtp.h | 2 +
3 files changed, 591 insertions(+), 186 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v2 01/12] gtp: set initial MTU
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 9:38 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 02/12] gtp: include role in link info Jonas Bonn
` (10 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
The GTP link is brought up with a default MTU of zero. This can lead to
some rather unexpected behaviour for users who are more accustomed to
interfaces coming online with reasonable defaults.
This patch sets an initial MTU for the GTP link of 1500 less worst-case
tunnel overhead.
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 4c04e271f184..5a048f050a9c 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -612,11 +612,16 @@ static const struct net_device_ops gtp_netdev_ops = {
static void gtp_link_setup(struct net_device *dev)
{
+ unsigned int max_gtp_header_len = sizeof(struct iphdr) +
+ sizeof(struct udphdr) +
+ sizeof(struct gtp0_header);
+
dev->netdev_ops = >p_netdev_ops;
dev->needs_free_netdev = true;
dev->hard_header_len = 0;
dev->addr_len = 0;
+ dev->mtu = ETH_DATA_LEN - max_gtp_header_len;
/* Zero header length. */
dev->type = ARPHRD_NONE;
@@ -626,11 +631,7 @@ static void gtp_link_setup(struct net_device *dev)
dev->features |= NETIF_F_LLTX;
netif_keep_dst(dev);
- /* Assume largest header, ie. GTPv0. */
- dev->needed_headroom = LL_MAX_HEADER +
- sizeof(struct iphdr) +
- sizeof(struct udphdr) +
- sizeof(struct gtp0_header);
+ dev->needed_headroom = LL_MAX_HEADER + max_gtp_header_len;
}
static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize);
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 02/12] gtp: include role in link info
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 01/12] gtp: set initial MTU Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 9:40 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 03/12] gtp: really check namespaces before xmit Jonas Bonn
` (9 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
Querying link info for the GTP interface doesn't reveal in which "role" the
device is set to operate. Include this information in the info query
result.
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 5a048f050a9c..5682d3ba7aa5 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -728,7 +728,8 @@ static int gtp_validate(struct nlattr *tb[], struct nlattr *data[],
static size_t gtp_get_size(const struct net_device *dev)
{
- return nla_total_size(sizeof(__u32)); /* IFLA_GTP_PDP_HASHSIZE */
+ return nla_total_size(sizeof(__u32)) + /* IFLA_GTP_PDP_HASHSIZE */
+ nla_total_size(sizeof(__u32)); /* IFLA_GTP_ROLE */
}
static int gtp_fill_info(struct sk_buff *skb, const struct net_device *dev)
@@ -737,6 +738,8 @@ static int gtp_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (nla_put_u32(skb, IFLA_GTP_PDP_HASHSIZE, gtp->hash_size))
goto nla_put_failure;
+ if (nla_put_u32(skb, IFLA_GTP_ROLE, gtp->role))
+ goto nla_put_failure;
return 0;
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 03/12] gtp: really check namespaces before xmit
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 01/12] gtp: set initial MTU Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 02/12] gtp: include role in link info Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 9:41 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 04/12] gtp: drop unnecessary call to skb_dst_drop Jonas Bonn
` (8 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
Blindly assuming that packet transmission crosses namespaces results in
skb marks being lost in the single namespace case.
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 5682d3ba7aa5..e4e57c0552ee 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -592,7 +592,9 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
ip4_dst_hoplimit(&pktinfo.rt->dst),
0,
pktinfo.gtph_port, pktinfo.gtph_port,
- true, false);
+ !net_eq(sock_net(pktinfo.pctx->sk),
+ dev_net(dev)),
+ false);
break;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 04/12] gtp: drop unnecessary call to skb_dst_drop
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (2 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 03/12] gtp: really check namespaces before xmit Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 9:50 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 05/12] gtp: set device type Jonas Bonn
` (7 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
The call to skb_dst_drop() is already done as part of udp_tunnel_xmit().
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index e4e57c0552ee..04d9de385549 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -515,8 +515,6 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev,
goto err_rt;
}
- skb_dst_drop(skb);
-
/* This is similar to tnl_update_pmtu(). */
df = iph->frag_off;
if (df) {
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 05/12] gtp: set device type
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (3 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 04/12] gtp: drop unnecessary call to skb_dst_drop Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 9:51 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 06/12] gtp: rework IPv4 functionality Jonas Bonn
` (6 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
Set the devtype to 'gtp' when setting up the link.
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 04d9de385549..a1bb02818977 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -610,6 +610,10 @@ static const struct net_device_ops gtp_netdev_ops = {
.ndo_get_stats64 = dev_get_tstats64,
};
+static const struct device_type gtp_type = {
+ .name = "gtp",
+};
+
static void gtp_link_setup(struct net_device *dev)
{
unsigned int max_gtp_header_len = sizeof(struct iphdr) +
@@ -618,6 +622,7 @@ static void gtp_link_setup(struct net_device *dev)
dev->netdev_ops = >p_netdev_ops;
dev->needs_free_netdev = true;
+ SET_NETDEV_DEVTYPE(dev, >p_type);
dev->hard_header_len = 0;
dev->addr_len = 0;
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 06/12] gtp: rework IPv4 functionality
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (4 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 05/12] gtp: set device type Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 07/12] gtp: use ephemeral source port Jonas Bonn
` (5 subsequent siblings)
11 siblings, 0 replies; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
This patch does some cleanup work in the IPv4 functionality to lay the
groundwork for adding support for IPv6. The form of these changes is
largely borrowed from the bareudp and geneve drivers, so there shouldn't
be anything here that looks unnecessarily unfamiliar.
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 204 +++++++++++++++++++++-------------------------
1 file changed, 92 insertions(+), 112 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index a1bb02818977..4a3a52970856 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -25,6 +25,7 @@
#include <net/protocol.h>
#include <net/ip.h>
#include <net/udp.h>
+#include <net/ip_tunnels.h>
#include <net/udp_tunnel.h>
#include <net/icmp.h>
#include <net/xfrm.h>
@@ -381,18 +382,36 @@ static void gtp_dev_uninit(struct net_device *dev)
free_percpu(dev->tstats);
}
-static struct rtable *ip4_route_output_gtp(struct flowi4 *fl4,
- const struct sock *sk,
- __be32 daddr)
+static struct rtable *gtp_get_v4_rt(struct sk_buff *skb,
+ struct net_device *dev,
+ struct pdp_ctx *pctx,
+ __be32 *saddr)
{
- memset(fl4, 0, sizeof(*fl4));
- fl4->flowi4_oif = sk->sk_bound_dev_if;
- fl4->daddr = daddr;
- fl4->saddr = inet_sk(sk)->inet_saddr;
- fl4->flowi4_tos = RT_CONN_FLAGS(sk);
- fl4->flowi4_proto = sk->sk_protocol;
-
- return ip_route_output_key(sock_net(sk), fl4);
+ const struct sock *sk = pctx->sk;
+ struct rtable *rt = NULL;
+ struct flowi4 fl4;
+
+ memset(&fl4, 0, sizeof(fl4));
+ fl4.flowi4_oif = sk->sk_bound_dev_if;
+ fl4.daddr = pctx->peer_addr_ip4.s_addr;
+ fl4.saddr = inet_sk(sk)->inet_saddr;
+ fl4.flowi4_tos = RT_CONN_FLAGS(sk);
+ fl4.flowi4_proto = sk->sk_protocol;
+
+ rt = ip_route_output_key(sock_net(sk), &fl4);
+ if (IS_ERR(rt)) {
+ netdev_dbg(pctx->dev, "no route to %pI4\n", &fl4.daddr);
+ return ERR_PTR(-ENETUNREACH);
+ }
+ if (rt->dst.dev == dev) {
+ netdev_dbg(pctx->dev, "circular route to %pI4\n", &fl4.daddr);
+ ip_rt_put(rt);
+ return ERR_PTR(-ELOOP);
+ }
+
+ *saddr = fl4.saddr;
+
+ return rt;
}
static inline void gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx)
@@ -435,54 +454,31 @@ static inline void gtp1_push_header(struct sk_buff *skb, struct pdp_ctx *pctx)
*/
}
-struct gtp_pktinfo {
- struct sock *sk;
- struct iphdr *iph;
- struct flowi4 fl4;
- struct rtable *rt;
- struct pdp_ctx *pctx;
- struct net_device *dev;
- __be16 gtph_port;
-};
-
-static void gtp_push_header(struct sk_buff *skb, struct gtp_pktinfo *pktinfo)
+static void gtp_push_header(struct sk_buff *skb, struct pdp_ctx *pctx,
+ __be16 *port)
{
- switch (pktinfo->pctx->gtp_version) {
+ switch (pctx->gtp_version) {
case GTP_V0:
- pktinfo->gtph_port = htons(GTP0_PORT);
- gtp0_push_header(skb, pktinfo->pctx);
+ *port = htons(GTP0_PORT);
+ gtp0_push_header(skb, pctx);
break;
case GTP_V1:
- pktinfo->gtph_port = htons(GTP1U_PORT);
- gtp1_push_header(skb, pktinfo->pctx);
+ *port = htons(GTP1U_PORT);
+ gtp1_push_header(skb, pctx);
break;
}
}
-static inline void gtp_set_pktinfo_ipv4(struct gtp_pktinfo *pktinfo,
- struct sock *sk, struct iphdr *iph,
- struct pdp_ctx *pctx, struct rtable *rt,
- struct flowi4 *fl4,
- struct net_device *dev)
-{
- pktinfo->sk = sk;
- pktinfo->iph = iph;
- pktinfo->pctx = pctx;
- pktinfo->rt = rt;
- pktinfo->fl4 = *fl4;
- pktinfo->dev = dev;
-}
-
-static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev,
- struct gtp_pktinfo *pktinfo)
+static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
{
struct gtp_dev *gtp = netdev_priv(dev);
struct pdp_ctx *pctx;
struct rtable *rt;
- struct flowi4 fl4;
+ __be32 saddr;
struct iphdr *iph;
- __be16 df;
- int mtu;
+ int headroom;
+ __be16 port;
+ int r;
/* Read the IP destination address and resolve the PDP context.
* Prepend PDP header with TEI/TID from PDP ctx.
@@ -500,102 +496,86 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev,
}
netdev_dbg(dev, "found PDP context %p\n", pctx);
- rt = ip4_route_output_gtp(&fl4, pctx->sk, pctx->peer_addr_ip4.s_addr);
+ rt = gtp_get_v4_rt(skb, dev, pctx, &saddr);
if (IS_ERR(rt)) {
- netdev_dbg(dev, "no route to SSGN %pI4\n",
- &pctx->peer_addr_ip4.s_addr);
- dev->stats.tx_carrier_errors++;
- goto err;
+ if (PTR_ERR(rt) == -ENETUNREACH)
+ dev->stats.tx_carrier_errors++;
+ else if (PTR_ERR(rt) == -ELOOP)
+ dev->stats.collisions++;
+ return PTR_ERR(rt);
}
- if (rt->dst.dev == dev) {
- netdev_dbg(dev, "circular route to SSGN %pI4\n",
- &pctx->peer_addr_ip4.s_addr);
- dev->stats.collisions++;
- goto err_rt;
- }
+ headroom = sizeof(struct iphdr) + sizeof(struct udphdr);
- /* This is similar to tnl_update_pmtu(). */
- df = iph->frag_off;
- if (df) {
- mtu = dst_mtu(&rt->dst) - dev->hard_header_len -
- sizeof(struct iphdr) - sizeof(struct udphdr);
- switch (pctx->gtp_version) {
- case GTP_V0:
- mtu -= sizeof(struct gtp0_header);
- break;
- case GTP_V1:
- mtu -= sizeof(struct gtp1_header);
- break;
- }
- } else {
- mtu = dst_mtu(&rt->dst);
+ switch (pctx->gtp_version) {
+ case GTP_V0:
+ headroom += sizeof(struct gtp0_header);
+ break;
+ case GTP_V1:
+ headroom += sizeof(struct gtp1_header);
+ break;
}
- rt->dst.ops->update_pmtu(&rt->dst, NULL, skb, mtu, false);
+ r = skb_tunnel_check_pmtu(skb, &rt->dst, headroom,
+ netif_is_any_bridge_port(dev));
+ if (r < 0) {
+ ip_rt_put(rt);
+ return r;
+ } else if (r) {
+ netif_rx(skb);
+ ip_rt_put(rt);
+ return -EMSGSIZE;
+ }
- if (!skb_is_gso(skb) && (iph->frag_off & htons(IP_DF)) &&
- mtu < ntohs(iph->tot_len)) {
- netdev_dbg(dev, "packet too big, fragmentation needed\n");
- memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
- icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
- htonl(mtu));
+ /* Ensure there is sufficient headroom. */
+ r = skb_cow_head(skb, headroom);
+ if (unlikely(r))
goto err_rt;
- }
- gtp_set_pktinfo_ipv4(pktinfo, pctx->sk, iph, pctx, rt, &fl4, dev);
- gtp_push_header(skb, pktinfo);
+ skb_reset_inner_headers(skb);
+
+ gtp_push_header(skb, pctx, &port);
+
+ iph = ip_hdr(skb);
+ netdev_dbg(dev, "gtp -> IP src: %pI4 dst: %pI4\n",
+ &iph->saddr, &iph->daddr);
+
+ udp_tunnel_xmit_skb(rt, pctx->sk, skb,
+ saddr, pctx->peer_addr_ip4.s_addr,
+ iph->tos,
+ ip4_dst_hoplimit(&rt->dst),
+ 0,
+ port, port,
+ !net_eq(sock_net(pctx->sk),
+ dev_net(pctx->dev)),
+ false);
return 0;
err_rt:
ip_rt_put(rt);
-err:
return -EBADMSG;
}
static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
{
unsigned int proto = ntohs(skb->protocol);
- struct gtp_pktinfo pktinfo;
int err;
- /* Ensure there is sufficient headroom. */
- if (skb_cow_head(skb, dev->needed_headroom))
+ if (proto != ETH_P_IP && proto != ETH_P_IPV6) {
+ err = -ENOTSUPP;
goto tx_err;
-
- skb_reset_inner_headers(skb);
+ }
/* PDP context lookups in gtp_build_skb_*() need rcu read-side lock. */
rcu_read_lock();
- switch (proto) {
- case ETH_P_IP:
- err = gtp_build_skb_ip4(skb, dev, &pktinfo);
- break;
- default:
- err = -EOPNOTSUPP;
- break;
- }
+
+ err = gtp_xmit_ip4(skb, dev);
+
rcu_read_unlock();
if (err < 0)
goto tx_err;
- switch (proto) {
- case ETH_P_IP:
- netdev_dbg(pktinfo.dev, "gtp -> IP src: %pI4 dst: %pI4\n",
- &pktinfo.iph->saddr, &pktinfo.iph->daddr);
- udp_tunnel_xmit_skb(pktinfo.rt, pktinfo.sk, skb,
- pktinfo.fl4.saddr, pktinfo.fl4.daddr,
- pktinfo.iph->tos,
- ip4_dst_hoplimit(&pktinfo.rt->dst),
- 0,
- pktinfo.gtph_port, pktinfo.gtph_port,
- !net_eq(sock_net(pktinfo.pctx->sk),
- dev_net(dev)),
- false);
- break;
- }
-
return NETDEV_TX_OK;
tx_err:
dev->stats.tx_errors++;
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 07/12] gtp: use ephemeral source port
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (5 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 06/12] gtp: rework IPv4 functionality Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 5:35 ` Pravin Shelar
2020-12-12 10:07 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 08/12] gtp: set dev features to enable GSO Jonas Bonn
` (4 subsequent siblings)
11 siblings, 2 replies; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
All GTP traffic is currently sent from the same source port. This makes
everything look like one big flow which is difficult to balance across
network resources.
From 3GPP TS 29.281:
"...the UDP Source Port or the Flow Label field... should be set dynamically
by the sending GTP-U entity to help balancing the load in the transport
network."
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 4a3a52970856..236ebbcb37bf 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -477,7 +477,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
__be32 saddr;
struct iphdr *iph;
int headroom;
- __be16 port;
+ __be16 sport, port;
int r;
/* Read the IP destination address and resolve the PDP context.
@@ -527,6 +527,10 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
return -EMSGSIZE;
}
+ sport = udp_flow_src_port(sock_net(pctx->sk), skb,
+ 0, USHRT_MAX,
+ true);
+
/* Ensure there is sufficient headroom. */
r = skb_cow_head(skb, headroom);
if (unlikely(r))
@@ -545,7 +549,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
iph->tos,
ip4_dst_hoplimit(&rt->dst),
0,
- port, port,
+ sport, port,
!net_eq(sock_net(pctx->sk),
dev_net(pctx->dev)),
false);
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 08/12] gtp: set dev features to enable GSO
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (6 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 07/12] gtp: use ephemeral source port Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 5:31 ` Pravin Shelar
2020-12-11 12:26 ` [PATCH net-next v2 09/12] gtp: support GRO Jonas Bonn
` (3 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 236ebbcb37bf..7bbeec173113 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -536,7 +536,11 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
if (unlikely(r))
goto err_rt;
- skb_reset_inner_headers(skb);
+ r = udp_tunnel_handle_offloads(skb, true);
+ if (unlikely(r))
+ goto err_rt;
+
+ skb_set_inner_protocol(skb, skb->protocol);
gtp_push_header(skb, pctx, &port);
@@ -618,6 +622,8 @@ static void gtp_link_setup(struct net_device *dev)
dev->priv_flags |= IFF_NO_QUEUE;
dev->features |= NETIF_F_LLTX;
+ dev->hw_features |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
+ dev->features |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
netif_keep_dst(dev);
dev->needed_headroom = LL_MAX_HEADER + max_gtp_header_len;
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 09/12] gtp: support GRO
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (7 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 08/12] gtp: set dev features to enable GSO Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-11 15:43 ` kernel test robot
2020-12-11 12:26 ` [PATCH net-next v2 10/12] gtp: add IPv6 support Jonas Bonn
` (2 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
This patch implements GRO callbacks for UDP-tunneled GTP traffic.
iperf3 numbers
Without GRO for GTP tunnels:
Accepted connection from 172.99.2.1, port 48783
[ 5] local 172.99.0.1 port 5201 connected to 172.99.2.1 port 46095
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 563 MBytes 576306 KBytes/sec
[ 5] 1.00-2.00 sec 681 MBytes 697814 KBytes/sec
[ 5] 2.00-3.00 sec 677 MBytes 693612 KBytes/sec
[ 5] 3.00-4.00 sec 679 MBytes 695690 KBytes/sec
[ 5] 4.00-5.00 sec 683 MBytes 699521 KBytes/sec
[ 5] 5.00-6.00 sec 682 MBytes 698922 KBytes/sec
[ 5] 6.00-7.00 sec 683 MBytes 699820 KBytes/sec
[ 5] 7.00-8.00 sec 682 MBytes 698052 KBytes/sec
[ 5] 8.00-9.00 sec 683 MBytes 699245 KBytes/sec
[ 5] 9.00-10.00 sec 683 MBytes 699554 KBytes/sec
[ 5] 10.00-10.00 sec 616 KBytes 687914 KBytes/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate
[ 5] 0.00-10.00 sec 6.54 GBytes 685853 KBytes/sec receiver
With GRO for GTP tunnels:
Accepted connection from 172.99.2.1, port 40847
[ 5] local 172.99.0.1 port 5201 connected to 172.99.2.1 port 55053
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 989 MBytes 1012640 KBytes/sec
[ 5] 1.00-2.00 sec 1.23 GBytes 1291408 KBytes/sec
[ 5] 2.00-3.00 sec 1.26 GBytes 1320197 KBytes/sec
[ 5] 3.00-4.00 sec 1.29 GBytes 1350097 KBytes/sec
[ 5] 4.00-5.00 sec 1.23 GBytes 1284512 KBytes/sec
[ 5] 5.00-6.00 sec 1.26 GBytes 1326329 KBytes/sec
[ 5] 6.00-7.00 sec 1.28 GBytes 1338620 KBytes/sec
[ 5] 7.00-8.00 sec 1.28 GBytes 1346391 KBytes/sec
[ 5] 8.00-9.00 sec 1.30 GBytes 1366394 KBytes/sec
[ 5] 9.00-10.00 sec 1.26 GBytes 1323848 KBytes/sec
[ 5] 10.00-10.00 sec 384 KBytes 1113043 KBytes/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate
[ 5] 0.00-10.00 sec 12.4 GBytes 1296036 KBytes/sec receiver
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 126 insertions(+)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 7bbeec173113..86639fae8d45 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -361,6 +361,128 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
return ret;
}
+static int gtp_gro_complete(struct sock *sk, struct sk_buff * skb, int nhoff)
+{
+ size_t hdrlen;
+ char* gtphdr = skb->data + nhoff;
+ u8 version;
+ __be16 type;
+ struct packet_offload *ptype;
+ uint8_t ipver;
+ int err;
+
+ version = *gtphdr >> 5;
+ switch (version) {
+ case GTP_V0:
+ hdrlen = sizeof(struct gtp0_header);
+ break;
+ case GTP_V1:
+ hdrlen = sizeof(struct gtp1_header);
+ if (*gtphdr & GTP1_F_MASK)
+ hdrlen += 4;
+ break;
+ }
+
+ skb_set_inner_network_header(skb, nhoff + hdrlen);
+
+ ipver = inner_ip_hdr(skb)->version;
+ switch (ipver) {
+ case 4:
+ type = cpu_to_be16(ETH_P_IP);
+ break;
+ case 6:
+ type = cpu_to_be16(ETH_P_IPV6);
+ break;
+ default:
+ goto out;
+ }
+
+ rcu_read_lock();
+ ptype = gro_find_complete_by_type(type);
+ if (!ptype)
+ goto out_unlock;
+
+ err = ptype->callbacks.gro_complete(skb, nhoff + hdrlen);
+
+ skb_set_inner_mac_header(skb, nhoff + hdrlen);
+
+out_unlock:
+ rcu_read_unlock();
+out:
+
+ return err;
+
+}
+
+static struct sk_buff *gtp_gro_receive(struct sock *sk,
+ struct list_head *head,
+ struct sk_buff *skb)
+{
+ size_t off, hdrlen;
+ char* gtphdr;
+ u8 version;
+ struct sk_buff *pp = NULL;
+ __be16 type;
+ struct packet_offload *ptype;
+
+ off = skb_gro_offset(skb);
+
+ gtphdr = skb_gro_header_fast(skb, off);
+ if (skb_gro_header_hard(skb, off+1)) {
+ gtphdr = skb_gro_header_slow(skb, off+1, off);
+ if (unlikely(!gtphdr))
+ goto out;
+ }
+
+ version = *gtphdr >> 5;
+ switch (version) {
+ case GTP_V0:
+ hdrlen = sizeof(struct gtp0_header);
+ break;
+ case GTP_V1:
+ hdrlen = sizeof(struct gtp1_header);
+ if (*gtphdr & GTP1_F_MASK)
+ hdrlen += 4;
+ break;
+ }
+
+ gtphdr = skb_gro_header_fast(skb, off);
+ if (skb_gro_header_hard(skb, off+hdrlen)) {
+ gtphdr = skb_gro_header_slow(skb, off+hdrlen, off);
+ if (unlikely(!gtphdr))
+ goto out;
+ }
+
+ skb_set_inner_network_header(skb, off + hdrlen);
+
+ switch(inner_ip_hdr(skb)->version) {
+ case 4:
+ type = cpu_to_be16(ETH_P_IP);
+ break;
+ case 6:
+ type = cpu_to_be16(ETH_P_IPV6);
+ break;
+ default:
+ goto out;
+ }
+
+ rcu_read_lock();
+ ptype = gro_find_receive_by_type(type);
+ if (!ptype)
+ goto out_unlock;
+
+ skb_gro_pull(skb, hdrlen);
+ skb_gro_postpull_rcsum(skb, gtphdr, hdrlen);
+
+ pp = call_gro_receive(ptype->callbacks.gro_receive, head, skb);
+
+out_unlock:
+ rcu_read_unlock();
+out:
+
+ return pp;
+}
+
static int gtp_dev_init(struct net_device *dev)
{
struct gtp_dev *gtp = netdev_priv(dev);
@@ -622,7 +744,9 @@ static void gtp_link_setup(struct net_device *dev)
dev->priv_flags |= IFF_NO_QUEUE;
dev->features |= NETIF_F_LLTX;
+ dev->hw_features |= NETIF_F_RXCSUM;
dev->hw_features |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
+ dev->features |= NETIF_F_RXCSUM;
dev->features |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
netif_keep_dst(dev);
@@ -818,6 +942,8 @@ static struct sock *gtp_encap_enable_socket(int fd, int type,
tuncfg.encap_type = type;
tuncfg.encap_rcv = gtp_encap_recv;
tuncfg.encap_destroy = gtp_encap_destroy;
+ tuncfg.gro_receive = gtp_gro_receive;
+ tuncfg.gro_complete = gtp_gro_complete;
setup_udp_tunnel_sock(sock_net(sock->sk), sock, &tuncfg);
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 10/12] gtp: add IPv6 support
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (8 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 09/12] gtp: support GRO Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 5:51 ` Pravin Shelar
2020-12-12 11:22 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 11/12] gtp: netlink update for ipv6 Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 12/12] gtp: add dst_cache to tunnels Jonas Bonn
11 siblings, 2 replies; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
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 <jonas@norrbonn.se>
---
drivers/net/gtp.c | 330 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 269 insertions(+), 61 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 86639fae8d45..4c902bffefa3 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -3,6 +3,7 @@
*
* (C) 2012-2014 by sysmocom - s.f.m.c. GmbH
* (C) 2016 by Pablo Neira Ayuso <pablo@netfilter.org>
+ * (C) 2020 by Jonas Bonn <jonas@norrbonn.se>
*
* Author: Harald Welte <hwelte@sysmocom.de>
* Pablo Neira Ayuso <pablo@netfilter.org>
@@ -20,6 +21,7 @@
#include <linux/net.h>
#include <linux/file.h>
#include <linux/gtp.h>
+#include <linux/ipv6.h>
#include <net/net_namespace.h>
#include <net/protocol.h>
@@ -33,6 +35,11 @@
#include <net/netns/generic.h>
#include <net/gtp.h>
+#define PDP_F_PEER_V6 (1 << 0)
+#define PDP_F_MS_V6 (1 << 1)
+
+#define ipv4(in6addr) ((in6addr)->s6_addr32[3])
+
/* An active session for the subscriber. */
struct pdp_ctx {
struct hlist_node hlist_tid;
@@ -49,10 +56,10 @@ struct pdp_ctx {
} v1;
} u;
u8 gtp_version;
- u16 af;
+ u16 flags;
- struct in_addr ms_addr_ip4;
- struct in_addr peer_addr_ip4;
+ struct in6_addr ms_addr;
+ struct in6_addr peer_addr;
struct sock *sk;
struct net_device *dev;
@@ -97,9 +104,23 @@ static inline u32 gtp1u_hashfn(u32 tid)
return jhash_1word(tid, gtp_h_initval);
}
+static inline u32 ip_hashfn(struct in6_addr *ip)
+{
+ return __ipv6_addr_jhash(ip, gtp_h_initval);
+}
+
+static inline u32 ipv6_hashfn(struct in6_addr *ip)
+{
+ return ip_hashfn(ip);
+}
+
static inline u32 ipv4_hashfn(__be32 ip)
{
- return jhash_1word((__force u32)ip, gtp_h_initval);
+ struct in6_addr addr;
+
+ ipv6_addr_set_v4mapped(ip, &addr);
+
+ return ipv6_hashfn(&addr);
}
/* Resolve a PDP context structure based on the 64bit TID. */
@@ -134,23 +155,42 @@ static struct pdp_ctx *gtp1_pdp_find(struct gtp_dev *gtp, u32 tid)
return NULL;
}
-/* Resolve a PDP context based on IPv4 address of MS. */
-static struct pdp_ctx *ipv4_pdp_find(struct gtp_dev *gtp, __be32 ms_addr)
+static struct pdp_ctx *ip_pdp_find(struct gtp_dev *gtp,
+ struct in6_addr *ms_addr)
{
struct hlist_head *head;
struct pdp_ctx *pdp;
- head = >p->addr_hash[ipv4_hashfn(ms_addr) % gtp->hash_size];
+ head = >p->addr_hash[ipv6_hashfn(ms_addr) % gtp->hash_size];
hlist_for_each_entry_rcu(pdp, head, hlist_addr) {
- if (pdp->af == AF_INET &&
- pdp->ms_addr_ip4.s_addr == ms_addr)
+ if (ipv6_addr_equal(&pdp->ms_addr, ms_addr))
return pdp;
}
return NULL;
}
+/* Resolve a PDP context based on IPv6 address of MS. */
+static struct pdp_ctx *ipv6_pdp_find(struct gtp_dev *gtp,
+ struct in6_addr *ms_addr)
+{
+ return ip_pdp_find(gtp, ms_addr);
+}
+
+/* Resolve a PDP context based on IPv4 address of MS. */
+static struct pdp_ctx *ipv4_pdp_find(struct gtp_dev *gtp, __be32 ms_addr)
+{
+ struct in6_addr addr;
+
+ ipv6_addr_set_v4mapped(ms_addr, &addr);
+
+ return ip_pdp_find(gtp, &addr);
+}
+
+/* Check if the inner IP address in this packet is assigned to any
+ * existing mobile subscriber.
+ */
static bool gtp_check_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx,
unsigned int hdrlen, unsigned int role)
{
@@ -162,28 +202,51 @@ static bool gtp_check_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx,
iph = (struct iphdr *)(skb->data + hdrlen);
if (role == GTP_ROLE_SGSN)
- return iph->daddr == pctx->ms_addr_ip4.s_addr;
+ return iph->daddr == ipv4(&pctx->ms_addr);
else
- return iph->saddr == pctx->ms_addr_ip4.s_addr;
+ return iph->saddr == ipv4(&pctx->ms_addr);
}
-/* Check if the inner IP address in this packet is assigned to any
- * existing mobile subscriber.
- */
-static bool gtp_check_ms(struct sk_buff *skb, struct pdp_ctx *pctx,
- unsigned int hdrlen, unsigned int role)
+static bool gtp_check_ms_ipv6(struct sk_buff *skb, struct pdp_ctx *pctx,
+ unsigned int hdrlen, unsigned int role)
{
- switch (ntohs(skb->protocol)) {
- case ETH_P_IP:
- return gtp_check_ms_ipv4(skb, pctx, hdrlen, role);
- }
- return false;
+ struct ipv6hdr *iph;
+
+ if (!pskb_may_pull(skb, hdrlen + sizeof(struct ipv6hdr)))
+ return false;
+
+ iph = (struct ipv6hdr *)(skb->data + hdrlen);
+
+ if (role == GTP_ROLE_SGSN)
+ return ipv6_addr_equal(&iph->daddr, &pctx->ms_addr);
+ else
+ return ipv6_addr_equal(&iph->saddr, &pctx->ms_addr);
}
static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb,
unsigned int hdrlen, unsigned int role)
{
- if (!gtp_check_ms(skb, pctx, hdrlen, role)) {
+ uint8_t ipver;
+ int r;
+
+ if (!pskb_may_pull(skb, hdrlen + 1))
+ return false;
+
+ /* 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);
+ 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;
+
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));
dev_sw_netstats_rx_add(pctx->dev, skb->len);
@@ -220,7 +285,9 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
if (!pskb_may_pull(skb, hdrlen))
return -1;
- gtp0 = (struct gtp0_header *)(skb->data + sizeof(struct udphdr));
+ skb_set_inner_network_header(skb, skb_transport_offset(skb) + hdrlen);
+
+ gtp0 = (struct gtp0_header *)&udp_hdr(skb)[1];
if ((gtp0->flags >> 5) != GTP_V0)
return 1;
@@ -247,7 +314,9 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
if (!pskb_may_pull(skb, hdrlen))
return -1;
- gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
+ skb_set_inner_network_header(skb, skb_transport_offset(skb) + hdrlen);
+
+ gtp1 = (struct gtp1_header *)&udp_hdr(skb)[1];
if ((gtp1->flags >> 5) != GTP_V1)
return 1;
@@ -264,12 +333,10 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
if (gtp1->flags & GTP1_F_MASK)
hdrlen += 4;
- /* Make sure the header is larger enough, including extensions. */
+ /* Make sure the header is large enough, including extensions. */
if (!pskb_may_pull(skb, hdrlen))
return -1;
- gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
-
pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
if (!pctx) {
netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
@@ -515,7 +582,7 @@ static struct rtable *gtp_get_v4_rt(struct sk_buff *skb,
memset(&fl4, 0, sizeof(fl4));
fl4.flowi4_oif = sk->sk_bound_dev_if;
- fl4.daddr = pctx->peer_addr_ip4.s_addr;
+ fl4.daddr = ipv4(&pctx->peer_addr);
fl4.saddr = inet_sk(sk)->inet_saddr;
fl4.flowi4_tos = RT_CONN_FLAGS(sk);
fl4.flowi4_proto = sk->sk_protocol;
@@ -536,6 +603,36 @@ static struct rtable *gtp_get_v4_rt(struct sk_buff *skb,
return rt;
}
+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;
+}
+
static inline void gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx)
{
int payload_len = skb->len;
@@ -591,10 +688,9 @@ static void gtp_push_header(struct sk_buff *skb, struct pdp_ctx *pctx,
}
}
-static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
+static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev,
+ struct pdp_ctx* pctx)
{
- struct gtp_dev *gtp = netdev_priv(dev);
- struct pdp_ctx *pctx;
struct rtable *rt;
__be32 saddr;
struct iphdr *iph;
@@ -602,22 +698,6 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
__be16 sport, port;
int r;
- /* Read the IP destination address and resolve the PDP context.
- * Prepend PDP header with TEI/TID from PDP ctx.
- */
- iph = ip_hdr(skb);
- if (gtp->role == GTP_ROLE_SGSN)
- pctx = ipv4_pdp_find(gtp, iph->saddr);
- else
- pctx = ipv4_pdp_find(gtp, iph->daddr);
-
- if (!pctx) {
- netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n",
- &iph->daddr);
- return -ENOENT;
- }
- netdev_dbg(dev, "found PDP context %p\n", pctx);
-
rt = gtp_get_v4_rt(skb, dev, pctx, &saddr);
if (IS_ERR(rt)) {
if (PTR_ERR(rt) == -ENETUNREACH)
@@ -671,7 +751,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
&iph->saddr, &iph->daddr);
udp_tunnel_xmit_skb(rt, pctx->sk, skb,
- saddr, pctx->peer_addr_ip4.s_addr,
+ saddr, ipv4(&pctx->peer_addr),
iph->tos,
ip4_dst_hoplimit(&rt->dst),
0,
@@ -686,9 +766,130 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
return -EBADMSG;
}
+static int gtp_xmit_ip6(struct sk_buff *skb, struct net_device *dev,
+ struct pdp_ctx* pctx)
+{
+ struct dst_entry *dst;
+ struct in6_addr saddr;
+ struct ipv6hdr *iph;
+ int headroom;
+ __be16 sport, port;
+ int r;
+
+ dst = gtp_get_v6_dst(skb, dev, pctx, &saddr);
+ if (IS_ERR(dst)) {
+ if (PTR_ERR(dst) == -ENETUNREACH)
+ dev->stats.tx_carrier_errors++;
+ else if (PTR_ERR(dst) == -ELOOP)
+ dev->stats.collisions++;
+ return PTR_ERR(dst);
+ }
+
+ headroom = sizeof(struct ipv6hdr) + sizeof(struct udphdr);
+
+ switch (pctx->gtp_version) {
+ case GTP_V0:
+ headroom += sizeof(struct gtp0_header);
+ break;
+ case GTP_V1:
+ headroom += sizeof(struct gtp1_header);
+ break;
+ }
+
+ sport = udp_flow_src_port(sock_net(pctx->sk), skb,
+ 0, USHRT_MAX,
+ true);
+
+ r = skb_tunnel_check_pmtu(skb, dst, headroom,
+ netif_is_any_bridge_port(dev));
+ if (r < 0) {
+ dst_release(dst);
+ return r;
+ } else if (r) {
+ netif_rx(skb);
+ dst_release(dst);
+ return -EMSGSIZE;
+ }
+
+ skb_scrub_packet(skb, !net_eq(sock_net(pctx->sk), dev_net(pctx->dev)));
+
+ /* Ensure there is sufficient headroom. */
+ r = skb_cow_head(skb, dev->needed_headroom);
+ if (unlikely(r))
+ goto free_dst;
+
+ r = udp_tunnel_handle_offloads(skb, true);
+ if (unlikely(r))
+ goto free_dst;
+
+ skb_set_inner_protocol(skb, skb->protocol);
+
+ gtp_push_header(skb, pctx, &port);
+
+ iph = ipv6_hdr(skb);
+ netdev_dbg(dev, "gtp -> IP src: %pI6 dst: %pI6\n",
+ &iph->saddr, &iph->daddr);
+
+ udp_tunnel6_xmit_skb(dst, pctx->sk, skb,
+ skb->dev,
+ &saddr, &pctx->peer_addr,
+ 0,
+ ip6_dst_hoplimit(dst),
+ 0,
+ sport, port,
+ false);
+
+ return 0;
+
+free_dst:
+ dst_release(dst);
+ return -EBADMSG;
+}
+
+static struct pdp_ctx *pdp_find(struct sk_buff *skb, struct net_device *dev)
+{
+ struct gtp_dev *gtp = netdev_priv(dev);
+ unsigned int proto = ntohs(skb->protocol);
+ struct pdp_ctx* pctx = NULL;
+
+ switch (proto) {
+ case ETH_P_IP: {
+ __be32 addr;
+ struct iphdr *iph = ip_hdr(skb);
+ addr = (gtp->role == GTP_ROLE_SGSN) ? iph->saddr : iph->daddr;
+ pctx = ipv4_pdp_find(gtp, addr);
+
+ if (!pctx) {
+ netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n",
+ &addr);
+ }
+
+ break;
+ }
+ case ETH_P_IPV6: {
+ struct in6_addr* addr;
+ struct ipv6hdr *iph = ipv6_hdr(skb);
+ addr = (gtp->role == GTP_ROLE_SGSN) ? &iph->saddr : &iph->daddr;
+ pctx = ipv6_pdp_find(gtp, addr);
+
+ if (!pctx) {
+ netdev_dbg(dev, "no PDP ctx found for %pI6, skip\n",
+ addr);
+ }
+
+ break;
+ }
+ default:
+ break;
+ }
+
+ return pctx;
+}
+
static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
{
unsigned int proto = ntohs(skb->protocol);
+ struct pdp_ctx *pctx;
int err;
if (proto != ETH_P_IP && proto != ETH_P_IPV6) {
@@ -699,7 +900,17 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
/* PDP context lookups in gtp_build_skb_*() need rcu read-side lock. */
rcu_read_lock();
- err = gtp_xmit_ip4(skb, dev);
+ pctx = pdp_find(skb, dev);
+ if (!pctx) {
+ err = -ENOENT;
+ rcu_read_unlock();
+ goto tx_err;
+ }
+
+ if (pctx->flags & PDP_F_PEER_V6)
+ err = gtp_xmit_ip6(skb, dev, pctx);
+ else
+ err = gtp_xmit_ip4(skb, dev, pctx);
rcu_read_unlock();
@@ -726,7 +937,7 @@ static const struct device_type gtp_type = {
static void gtp_link_setup(struct net_device *dev)
{
- unsigned int max_gtp_header_len = sizeof(struct iphdr) +
+ unsigned int max_gtp_header_len = sizeof(struct ipv6hdr) +
sizeof(struct udphdr) +
sizeof(struct gtp0_header);
@@ -1023,11 +1234,8 @@ static struct gtp_dev *gtp_find_dev(struct net *src_net, struct nlattr *nla[])
static void ipv4_pdp_fill(struct pdp_ctx *pctx, struct genl_info *info)
{
pctx->gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]);
- pctx->af = AF_INET;
- pctx->peer_addr_ip4.s_addr =
- nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]);
- pctx->ms_addr_ip4.s_addr =
- nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
+ ipv4(&pctx->peer_addr) = nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]);
+ ipv4(&pctx->ms_addr) = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
switch (pctx->gtp_version) {
case GTP_V0:
@@ -1127,13 +1335,13 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
switch (pctx->gtp_version) {
case GTP_V0:
netdev_dbg(dev, "GTPv0-U: new PDP ctx id=%llx ssgn=%pI4 ms=%pI4 (pdp=%p)\n",
- pctx->u.v0.tid, &pctx->peer_addr_ip4,
- &pctx->ms_addr_ip4, pctx);
+ pctx->u.v0.tid, &ipv4(&pctx->peer_addr),
+ &ipv4(&pctx->ms_addr), pctx);
break;
case GTP_V1:
netdev_dbg(dev, "GTPv1-U: new PDP ctx id=%x/%x ssgn=%pI4 ms=%pI4 (pdp=%p)\n",
pctx->u.v1.i_tei, pctx->u.v1.o_tei,
- &pctx->peer_addr_ip4, &pctx->ms_addr_ip4, pctx);
+ &ipv4(&pctx->peer_addr), &ipv4(&pctx->ms_addr), pctx);
break;
}
@@ -1315,8 +1523,8 @@ static int gtp_genl_fill_info(struct sk_buff *skb, u32 snd_portid, u32 snd_seq,
if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) ||
nla_put_u32(skb, GTPA_LINK, pctx->dev->ifindex) ||
- nla_put_be32(skb, GTPA_PEER_ADDRESS, pctx->peer_addr_ip4.s_addr) ||
- nla_put_be32(skb, GTPA_MS_ADDRESS, pctx->ms_addr_ip4.s_addr))
+ nla_put_be32(skb, GTPA_PEER_ADDRESS, ipv4(&pctx->peer_addr)) ||
+ nla_put_be32(skb, GTPA_MS_ADDRESS, ipv4(&pctx->ms_addr)))
goto nla_put_failure;
switch (pctx->gtp_version) {
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 11/12] gtp: netlink update for ipv6
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (9 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 10/12] gtp: add IPv6 support Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-12 11:25 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 12/12] gtp: add dst_cache to tunnels Jonas Bonn
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
This patch adds the netlink changes required to support IPv6.
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/gtp.c | 84 ++++++++++++++++++++++++++++++----------
include/uapi/linux/gtp.h | 2 +
2 files changed, 65 insertions(+), 21 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 4c902bffefa3..40bbbe8cfad6 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1231,11 +1231,26 @@ static struct gtp_dev *gtp_find_dev(struct net *src_net, struct nlattr *nla[])
return gtp;
}
-static void ipv4_pdp_fill(struct pdp_ctx *pctx, struct genl_info *info)
+static void pdp_fill(struct pdp_ctx *pctx, struct genl_info *info)
{
pctx->gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]);
- ipv4(&pctx->peer_addr) = nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]);
- ipv4(&pctx->ms_addr) = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
+ pctx->flags = 0;
+
+ if (info->attrs[GTPA_PEER_IPV6]) {
+ pctx->flags |= PDP_F_PEER_V6;
+ pctx->peer_addr = nla_get_in6_addr(info->attrs[GTPA_PEER_IPV6]);
+ } else
+ ipv6_addr_set_v4mapped(
+ nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]),
+ &pctx->peer_addr);
+
+ if (info->attrs[GTPA_MS_IPV6]) {
+ pctx->flags |= PDP_F_MS_V6;
+ pctx->ms_addr = nla_get_in6_addr(info->attrs[GTPA_MS_IPV6]);
+ } else
+ ipv6_addr_set_v4mapped(
+ nla_get_be32(info->attrs[GTPA_MS_ADDRESS]),
+ &pctx->ms_addr);
switch (pctx->gtp_version) {
case GTP_V0:
@@ -1263,13 +1278,20 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
u32 hash_ms, hash_tid = 0;
unsigned int version;
bool found = false;
- __be32 ms_addr;
- ms_addr = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
- hash_ms = ipv4_hashfn(ms_addr) % gtp->hash_size;
+ if (info->attrs[GTPA_MS_IPV6]) {
+ struct in6_addr ms_addr_v6;
+ ms_addr_v6 = nla_get_in6_addr(info->attrs[GTPA_MS_IPV6]);
+ hash_ms = ipv6_hashfn(&ms_addr_v6) % gtp->hash_size;
+ pctx = ipv6_pdp_find(gtp, &ms_addr_v6);
+ } else {
+ __be32 ms_addr;
+ ms_addr = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
+ hash_ms = ipv4_hashfn(ms_addr) % gtp->hash_size;
+ pctx = ipv4_pdp_find(gtp, ms_addr);
+ }
version = nla_get_u32(info->attrs[GTPA_VERSION]);
- pctx = ipv4_pdp_find(gtp, ms_addr);
if (pctx)
found = true;
if (version == GTP_V0)
@@ -1292,7 +1314,7 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
if (!pctx)
pctx = pctx_tid;
- ipv4_pdp_fill(pctx, info);
+ pdp_fill(pctx, info);
if (pctx->gtp_version == GTP_V0)
netdev_dbg(dev, "GTPv0-U: update tunnel id = %llx (pdp %p)\n",
@@ -1312,7 +1334,7 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
sock_hold(sk);
pctx->sk = sk;
pctx->dev = gtp->dev;
- ipv4_pdp_fill(pctx, info);
+ pdp_fill(pctx, info);
atomic_set(&pctx->tx_seq, 0);
switch (pctx->gtp_version) {
@@ -1334,14 +1356,14 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
switch (pctx->gtp_version) {
case GTP_V0:
- netdev_dbg(dev, "GTPv0-U: new PDP ctx id=%llx ssgn=%pI4 ms=%pI4 (pdp=%p)\n",
- pctx->u.v0.tid, &ipv4(&pctx->peer_addr),
- &ipv4(&pctx->ms_addr), pctx);
+ netdev_dbg(dev, "GTPv0-U: new PDP ctx id=%llx ssgn=%pI6 ms=%pI6 (pdp=%p)\n",
+ pctx->u.v0.tid, &pctx->peer_addr,
+ &pctx->ms_addr, pctx);
break;
case GTP_V1:
- netdev_dbg(dev, "GTPv1-U: new PDP ctx id=%x/%x ssgn=%pI4 ms=%pI4 (pdp=%p)\n",
+ netdev_dbg(dev, "GTPv1-U: new PDP ctx id=%x/%x ssgn=%pI6 ms=%pI6 (pdp=%p)\n",
pctx->u.v1.i_tei, pctx->u.v1.o_tei,
- &ipv4(&pctx->peer_addr), &ipv4(&pctx->ms_addr), pctx);
+ &pctx->peer_addr, &pctx->ms_addr, pctx);
break;
}
@@ -1374,9 +1396,13 @@ static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
int err;
if (!info->attrs[GTPA_VERSION] ||
- !info->attrs[GTPA_LINK] ||
- !info->attrs[GTPA_PEER_ADDRESS] ||
- !info->attrs[GTPA_MS_ADDRESS])
+ !info->attrs[GTPA_LINK])
+ return -EINVAL;
+
+ if (!info->attrs[GTPA_PEER_ADDRESS] == !info->attrs[GTPA_PEER_IPV6])
+ return -EINVAL;
+
+ if (!info->attrs[GTPA_MS_ADDRESS] == !info->attrs[GTPA_MS_IPV6])
return -EINVAL;
version = nla_get_u32(info->attrs[GTPA_VERSION]);
@@ -1439,7 +1465,11 @@ static struct pdp_ctx *gtp_find_pdp_by_link(struct net *net,
if (!gtp)
return ERR_PTR(-ENODEV);
- if (nla[GTPA_MS_ADDRESS]) {
+ if (nla[GTPA_MS_IPV6]) {
+ struct in6_addr ip = nla_get_in6_addr(nla[GTPA_MS_IPV6]);
+
+ return ipv6_pdp_find(gtp, &ip);
+ } else if (nla[GTPA_MS_ADDRESS]) {
__be32 ip = nla_get_be32(nla[GTPA_MS_ADDRESS]);
return ipv4_pdp_find(gtp, ip);
@@ -1522,9 +1552,19 @@ static int gtp_genl_fill_info(struct sk_buff *skb, u32 snd_portid, u32 snd_seq,
goto nlmsg_failure;
if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) ||
- nla_put_u32(skb, GTPA_LINK, pctx->dev->ifindex) ||
- nla_put_be32(skb, GTPA_PEER_ADDRESS, ipv4(&pctx->peer_addr)) ||
- nla_put_be32(skb, GTPA_MS_ADDRESS, ipv4(&pctx->ms_addr)))
+ nla_put_u32(skb, GTPA_LINK, pctx->dev->ifindex))
+ goto nla_put_failure;
+
+ if ((pctx->flags & PDP_F_PEER_V6) &&
+ nla_put_in6_addr(skb, GTPA_PEER_IPV6, &pctx->peer_addr))
+ goto nla_put_failure;
+ else if (nla_put_be32(skb, GTPA_PEER_ADDRESS, ipv4(&pctx->peer_addr)))
+ goto nla_put_failure;
+
+ if ((pctx->flags & PDP_F_MS_V6) &&
+ nla_put_in6_addr(skb, GTPA_MS_IPV6, &pctx->ms_addr))
+ goto nla_put_failure;
+ else if (nla_put_be32(skb, GTPA_MS_ADDRESS, ipv4(&pctx->ms_addr)))
goto nla_put_failure;
switch (pctx->gtp_version) {
@@ -1660,6 +1700,8 @@ static const struct nla_policy gtp_genl_policy[GTPA_MAX + 1] = {
[GTPA_TID] = { .type = NLA_U64, },
[GTPA_PEER_ADDRESS] = { .type = NLA_U32, },
[GTPA_MS_ADDRESS] = { .type = NLA_U32, },
+ [GTPA_PEER_IPV6] = { .len = sizeof(struct in6_addr), },
+ [GTPA_MS_IPV6] = { .len = sizeof(struct in6_addr), },
[GTPA_FLOW] = { .type = NLA_U16, },
[GTPA_NET_NS_FD] = { .type = NLA_U32, },
[GTPA_I_TEI] = { .type = NLA_U32, },
diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h
index 79f9191bbb24..5fe0ca6a917e 100644
--- a/include/uapi/linux/gtp.h
+++ b/include/uapi/linux/gtp.h
@@ -30,6 +30,8 @@ enum gtp_attrs {
GTPA_I_TEI, /* for GTPv1 only */
GTPA_O_TEI, /* for GTPv1 only */
GTPA_PAD,
+ GTPA_PEER_IPV6,
+ GTPA_MS_IPV6,
__GTPA_MAX,
};
#define GTPA_MAX (__GTPA_MAX + 1)
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v2 12/12] gtp: add dst_cache to tunnels
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
` (10 preceding siblings ...)
2020-12-11 12:26 ` [PATCH net-next v2 11/12] gtp: netlink update for ipv6 Jonas Bonn
@ 2020-12-11 12:26 ` Jonas Bonn
2020-12-11 17:25 ` kernel test robot
11 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-11 12:26 UTC (permalink / raw)
To: netdev; +Cc: pablo, laforge, Jonas Bonn
Destination caching on a per-tunnel basis is a performance win, so we
enable this unconditionally for the module.
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
drivers/net/Kconfig | 1 +
drivers/net/gtp.c | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 260f9f46668b..f79277222125 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -276,6 +276,7 @@ config GTP
tristate "GPRS Tunneling Protocol datapath (GTP-U)"
depends on INET
select NET_UDP_TUNNEL
+ select DST_CACHE
help
This allows one to create gtp virtual interfaces that provide
the GPRS Tunneling Protocol datapath (GTP-U). This tunneling protocol
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 40bbbe8cfad6..6708738681d2 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -64,6 +64,8 @@ struct pdp_ctx {
struct sock *sk;
struct net_device *dev;
+ struct dst_cache dst_cache;
+
atomic_t tx_seq;
struct rcu_head rcu_head;
};
@@ -577,9 +579,15 @@ static struct rtable *gtp_get_v4_rt(struct sk_buff *skb,
__be32 *saddr)
{
const struct sock *sk = pctx->sk;
+ struct dst_cache *dst_cache;
struct rtable *rt = NULL;
struct flowi4 fl4;
+ dst_cache = (struct dst_cache *)&pctx->dst_cache;
+ rt = dst_cache_get_ip4(dst_cache, saddr);
+ if (rt)
+ return rt;
+
memset(&fl4, 0, sizeof(fl4));
fl4.flowi4_oif = sk->sk_bound_dev_if;
fl4.daddr = ipv4(&pctx->peer_addr);
@@ -600,6 +608,8 @@ static struct rtable *gtp_get_v4_rt(struct sk_buff *skb,
*saddr = fl4.saddr;
+ dst_cache_set_ip4(dst_cache, &rt->dst, *saddr);
+
return rt;
}
@@ -610,8 +620,14 @@ static struct dst_entry *gtp_get_v6_dst(struct sk_buff *skb,
{
const struct sock *sk = pctx->sk;
struct dst_entry *dst = NULL;
+ struct dst_cache *dst_cache;
struct flowi6 fl6;
+ dst_cache = (struct dst_cache *)&pctx->dst_cache;
+ dst = dst_cache_get_ip6(dst_cache, saddr);
+ if (dst)
+ return dst;
+
memset(&fl6, 0, sizeof(fl6));
fl6.flowi6_mark = skb->mark;
fl6.flowi6_proto = IPPROTO_UDP;
@@ -630,6 +646,8 @@ static struct dst_entry *gtp_get_v6_dst(struct sk_buff *skb,
*saddr = fl6.saddr;
+ dst_cache_set_ip6(dst_cache, dst, saddr);
+
return dst;
}
@@ -1236,6 +1254,8 @@ static void pdp_fill(struct pdp_ctx *pctx, struct genl_info *info)
pctx->gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]);
pctx->flags = 0;
+ dst_cache_reset(&pctx->dst_cache);
+
if (info->attrs[GTPA_PEER_IPV6]) {
pctx->flags |= PDP_F_PEER_V6;
pctx->peer_addr = nla_get_in6_addr(info->attrs[GTPA_PEER_IPV6]);
@@ -1331,6 +1351,11 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
if (pctx == NULL)
return ERR_PTR(-ENOMEM);
+ if (dst_cache_init(&pctx->dst_cache, GFP_ATOMIC)) {
+ kfree(pctx);
+ return ERR_PTR(-ENOBUFS);
+ }
+
sock_hold(sk);
pctx->sk = sk;
pctx->dev = gtp->dev;
@@ -1374,6 +1399,8 @@ static void pdp_context_free(struct rcu_head *head)
{
struct pdp_ctx *pctx = container_of(head, struct pdp_ctx, rcu_head);
+ dst_cache_destroy(&pctx->dst_cache);
+
sock_put(pctx->sk);
kfree(pctx);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 09/12] gtp: support GRO
2020-12-11 12:26 ` [PATCH net-next v2 09/12] gtp: support GRO Jonas Bonn
@ 2020-12-11 15:43 ` kernel test robot
0 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2020-12-11 15:43 UTC (permalink / raw)
To: Jonas Bonn, netdev
Cc: kbuild-all, clang-built-linux, pablo, laforge, Jonas Bonn
[-- Attachment #1: Type: text/plain, Size: 4992 bytes --]
Hi Jonas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jonas-Bonn/gtp-IPv6-support/20201211-203639
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 91163f82143630a9629a8bf0227d49173697c69c
config: mips-randconfig-r026-20201209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5ff35356f1af2bb92785b38c657463924d9ec386)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/0day-ci/linux/commit/de5669628a8f684dd7ed378aaf2a997221d243fa
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jonas-Bonn/gtp-IPv6-support/20201211-203639
git checkout de5669628a8f684dd7ed378aaf2a997221d243fa
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/gtp.c:402:2: warning: variable 'err' is used uninitialized whenever 'if' condition is true
if (!ptype)
^~~~~~~~~~~
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( , ## __VA_ARGS__) ) )
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) (cond) : __trace_if_value(cond))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/gtp.c:413:9: note: uninitialized use occurs here
return err;
^~~
drivers/net/gtp.c:402:2: note: remove the 'if' if its condition is always false
if (!ptype)
^~~~~~~~~~~
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( , ## __VA_ARGS__) ) )
^
>> drivers/net/gtp.c:396:2: warning: variable 'err' is used uninitialized whenever switch default is taken
default:
^~~~~~~
drivers/net/gtp.c:413:9: note: uninitialized use occurs here
return err;
^~~
drivers/net/gtp.c:372:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
fatal error: error in backend: Nested variants found in inline asm string: ' .set push
.set mips64r2
.if ( 0x00 ) != -1)) 0x00 ) != -1)) : ($( static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = $( .func = __func__, .file = "arch/mips/include/asm/atomic.h", .line = 153, $); 0x00 ) != -1)) : $))) ) && ( 0 ); .set push; .set mips64r2; .rept 1; sync 0x00; .endr; .set pop; .else; ; .endif
1: ll $1, $2 # atomic_fetch_add
addu $0, $1, $3
sc $0, $2
beqz $0, 1b
.set pop
move $0, $1
'
clang-12: error: clang frontend command failed with exit code 70 (use -v to see invocation)
clang version 12.0.0 (git://gitmirror/llvm_project 5ff35356f1af2bb92785b38c657463924d9ec386)
Target: mipsel-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/cross/clang-5ff35356f1/bin
clang-12: note: diagnostic msg:
Makefile arch drivers include kernel scripts source usr
vim +402 drivers/net/gtp.c
363
364 static int gtp_gro_complete(struct sock *sk, struct sk_buff * skb, int nhoff)
365 {
366 size_t hdrlen;
367 char* gtphdr = skb->data + nhoff;
368 u8 version;
369 __be16 type;
370 struct packet_offload *ptype;
371 uint8_t ipver;
372 int err;
373
374 version = *gtphdr >> 5;
375 switch (version) {
376 case GTP_V0:
377 hdrlen = sizeof(struct gtp0_header);
378 break;
379 case GTP_V1:
380 hdrlen = sizeof(struct gtp1_header);
381 if (*gtphdr & GTP1_F_MASK)
382 hdrlen += 4;
383 break;
384 }
385
386 skb_set_inner_network_header(skb, nhoff + hdrlen);
387
388 ipver = inner_ip_hdr(skb)->version;
389 switch (ipver) {
390 case 4:
391 type = cpu_to_be16(ETH_P_IP);
392 break;
393 case 6:
394 type = cpu_to_be16(ETH_P_IPV6);
395 break;
> 396 default:
397 goto out;
398 }
399
400 rcu_read_lock();
401 ptype = gro_find_complete_by_type(type);
> 402 if (!ptype)
403 goto out_unlock;
404
405 err = ptype->callbacks.gro_complete(skb, nhoff + hdrlen);
406
407 skb_set_inner_mac_header(skb, nhoff + hdrlen);
408
409 out_unlock:
410 rcu_read_unlock();
411 out:
412
413 return err;
414
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26882 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 09/12] gtp: support GRO
@ 2020-12-11 15:43 ` kernel test robot
0 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2020-12-11 15:43 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5125 bytes --]
Hi Jonas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jonas-Bonn/gtp-IPv6-support/20201211-203639
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 91163f82143630a9629a8bf0227d49173697c69c
config: mips-randconfig-r026-20201209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5ff35356f1af2bb92785b38c657463924d9ec386)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/0day-ci/linux/commit/de5669628a8f684dd7ed378aaf2a997221d243fa
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jonas-Bonn/gtp-IPv6-support/20201211-203639
git checkout de5669628a8f684dd7ed378aaf2a997221d243fa
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/gtp.c:402:2: warning: variable 'err' is used uninitialized whenever 'if' condition is true
if (!ptype)
^~~~~~~~~~~
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( , ## __VA_ARGS__) ) )
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) (cond) : __trace_if_value(cond))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/gtp.c:413:9: note: uninitialized use occurs here
return err;
^~~
drivers/net/gtp.c:402:2: note: remove the 'if' if its condition is always false
if (!ptype)
^~~~~~~~~~~
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( , ## __VA_ARGS__) ) )
^
>> drivers/net/gtp.c:396:2: warning: variable 'err' is used uninitialized whenever switch default is taken
default:
^~~~~~~
drivers/net/gtp.c:413:9: note: uninitialized use occurs here
return err;
^~~
drivers/net/gtp.c:372:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
fatal error: error in backend: Nested variants found in inline asm string: ' .set push
.set mips64r2
.if ( 0x00 ) != -1)) 0x00 ) != -1)) : ($( static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = $( .func = __func__, .file = "arch/mips/include/asm/atomic.h", .line = 153, $); 0x00 ) != -1)) : $))) ) && ( 0 ); .set push; .set mips64r2; .rept 1; sync 0x00; .endr; .set pop; .else; ; .endif
1: ll $1, $2 # atomic_fetch_add
addu $0, $1, $3
sc $0, $2
beqz $0, 1b
.set pop
move $0, $1
'
clang-12: error: clang frontend command failed with exit code 70 (use -v to see invocation)
clang version 12.0.0 (git://gitmirror/llvm_project 5ff35356f1af2bb92785b38c657463924d9ec386)
Target: mipsel-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/cross/clang-5ff35356f1/bin
clang-12: note: diagnostic msg:
Makefile arch drivers include kernel scripts source usr
vim +402 drivers/net/gtp.c
363
364 static int gtp_gro_complete(struct sock *sk, struct sk_buff * skb, int nhoff)
365 {
366 size_t hdrlen;
367 char* gtphdr = skb->data + nhoff;
368 u8 version;
369 __be16 type;
370 struct packet_offload *ptype;
371 uint8_t ipver;
372 int err;
373
374 version = *gtphdr >> 5;
375 switch (version) {
376 case GTP_V0:
377 hdrlen = sizeof(struct gtp0_header);
378 break;
379 case GTP_V1:
380 hdrlen = sizeof(struct gtp1_header);
381 if (*gtphdr & GTP1_F_MASK)
382 hdrlen += 4;
383 break;
384 }
385
386 skb_set_inner_network_header(skb, nhoff + hdrlen);
387
388 ipver = inner_ip_hdr(skb)->version;
389 switch (ipver) {
390 case 4:
391 type = cpu_to_be16(ETH_P_IP);
392 break;
393 case 6:
394 type = cpu_to_be16(ETH_P_IPV6);
395 break;
> 396 default:
397 goto out;
398 }
399
400 rcu_read_lock();
401 ptype = gro_find_complete_by_type(type);
> 402 if (!ptype)
403 goto out_unlock;
404
405 err = ptype->callbacks.gro_complete(skb, nhoff + hdrlen);
406
407 skb_set_inner_mac_header(skb, nhoff + hdrlen);
408
409 out_unlock:
410 rcu_read_unlock();
411 out:
412
413 return err;
414
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 26882 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 12/12] gtp: add dst_cache to tunnels
2020-12-11 12:26 ` [PATCH net-next v2 12/12] gtp: add dst_cache to tunnels Jonas Bonn
@ 2020-12-11 17:25 ` kernel test robot
0 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2020-12-11 17:25 UTC (permalink / raw)
To: Jonas Bonn, netdev
Cc: kbuild-all, clang-built-linux, pablo, laforge, Jonas Bonn
[-- Attachment #1: Type: text/plain, Size: 5157 bytes --]
Hi Jonas,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jonas-Bonn/gtp-IPv6-support/20201211-203639
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 91163f82143630a9629a8bf0227d49173697c69c
config: mips-randconfig-r026-20201209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5ff35356f1af2bb92785b38c657463924d9ec386)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/0day-ci/linux/commit/a8ea47bc0c1903be018f4d566bb96146c156ddce
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jonas-Bonn/gtp-IPv6-support/20201211-203639
git checkout a8ea47bc0c1903be018f4d566bb96146c156ddce
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
drivers/net/gtp.c:471:2: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!ptype)
^~~~~~~~~~~
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/gtp.c:482:9: note: uninitialized use occurs here
return err;
^~~
drivers/net/gtp.c:471:2: note: remove the 'if' if its condition is always false
if (!ptype)
^~~~~~~~~~~
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/net/gtp.c:465:2: warning: variable 'err' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
default:
^~~~~~~
drivers/net/gtp.c:482:9: note: uninitialized use occurs here
return err;
^~~
drivers/net/gtp.c:441:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
>> drivers/net/gtp.c:627:8: error: implicit declaration of function 'dst_cache_get_ip6' [-Werror,-Wimplicit-function-declaration]
dst = dst_cache_get_ip6(dst_cache, saddr);
^
drivers/net/gtp.c:627:8: note: did you mean 'dst_cache_get_ip4'?
include/net/dst_cache.h:33:16: note: 'dst_cache_get_ip4' declared here
struct rtable *dst_cache_get_ip4(struct dst_cache *dst_cache, __be32 *saddr);
^
>> drivers/net/gtp.c:627:6: warning: incompatible integer to pointer conversion assigning to 'struct dst_entry *' from 'int' [-Wint-conversion]
dst = dst_cache_get_ip6(dst_cache, saddr);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/gtp.c:649:2: error: implicit declaration of function 'dst_cache_set_ip6' [-Werror,-Wimplicit-function-declaration]
dst_cache_set_ip6(dst_cache, dst, saddr);
^
3 warnings and 2 errors generated.
vim +/dst_cache_get_ip6 +627 drivers/net/gtp.c
615
616 static struct dst_entry *gtp_get_v6_dst(struct sk_buff *skb,
617 struct net_device *dev,
618 struct pdp_ctx *pctx,
619 struct in6_addr *saddr)
620 {
621 const struct sock *sk = pctx->sk;
622 struct dst_entry *dst = NULL;
623 struct dst_cache *dst_cache;
624 struct flowi6 fl6;
625
626 dst_cache = (struct dst_cache *)&pctx->dst_cache;
> 627 dst = dst_cache_get_ip6(dst_cache, saddr);
628 if (dst)
629 return dst;
630
631 memset(&fl6, 0, sizeof(fl6));
632 fl6.flowi6_mark = skb->mark;
633 fl6.flowi6_proto = IPPROTO_UDP;
634 fl6.daddr = pctx->peer_addr;
635
636 dst = ipv6_stub->ipv6_dst_lookup_flow(sock_net(sk), sk, &fl6, NULL);
637 if (IS_ERR(dst)) {
638 netdev_dbg(pctx->dev, "no route to %pI6\n", &fl6.daddr);
639 return ERR_PTR(-ENETUNREACH);
640 }
641 if (dst->dev == pctx->dev) {
642 netdev_dbg(pctx->dev, "circular route to %pI6\n", &fl6.daddr);
643 dst_release(dst);
644 return ERR_PTR(-ELOOP);
645 }
646
647 *saddr = fl6.saddr;
648
> 649 dst_cache_set_ip6(dst_cache, dst, saddr);
650
651 return dst;
652 }
653
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26882 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 12/12] gtp: add dst_cache to tunnels
@ 2020-12-11 17:25 ` kernel test robot
0 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2020-12-11 17:25 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5274 bytes --]
Hi Jonas,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jonas-Bonn/gtp-IPv6-support/20201211-203639
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 91163f82143630a9629a8bf0227d49173697c69c
config: mips-randconfig-r026-20201209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5ff35356f1af2bb92785b38c657463924d9ec386)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/0day-ci/linux/commit/a8ea47bc0c1903be018f4d566bb96146c156ddce
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jonas-Bonn/gtp-IPv6-support/20201211-203639
git checkout a8ea47bc0c1903be018f4d566bb96146c156ddce
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
drivers/net/gtp.c:471:2: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!ptype)
^~~~~~~~~~~
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/gtp.c:482:9: note: uninitialized use occurs here
return err;
^~~
drivers/net/gtp.c:471:2: note: remove the 'if' if its condition is always false
if (!ptype)
^~~~~~~~~~~
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/net/gtp.c:465:2: warning: variable 'err' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
default:
^~~~~~~
drivers/net/gtp.c:482:9: note: uninitialized use occurs here
return err;
^~~
drivers/net/gtp.c:441:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
>> drivers/net/gtp.c:627:8: error: implicit declaration of function 'dst_cache_get_ip6' [-Werror,-Wimplicit-function-declaration]
dst = dst_cache_get_ip6(dst_cache, saddr);
^
drivers/net/gtp.c:627:8: note: did you mean 'dst_cache_get_ip4'?
include/net/dst_cache.h:33:16: note: 'dst_cache_get_ip4' declared here
struct rtable *dst_cache_get_ip4(struct dst_cache *dst_cache, __be32 *saddr);
^
>> drivers/net/gtp.c:627:6: warning: incompatible integer to pointer conversion assigning to 'struct dst_entry *' from 'int' [-Wint-conversion]
dst = dst_cache_get_ip6(dst_cache, saddr);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/gtp.c:649:2: error: implicit declaration of function 'dst_cache_set_ip6' [-Werror,-Wimplicit-function-declaration]
dst_cache_set_ip6(dst_cache, dst, saddr);
^
3 warnings and 2 errors generated.
vim +/dst_cache_get_ip6 +627 drivers/net/gtp.c
615
616 static struct dst_entry *gtp_get_v6_dst(struct sk_buff *skb,
617 struct net_device *dev,
618 struct pdp_ctx *pctx,
619 struct in6_addr *saddr)
620 {
621 const struct sock *sk = pctx->sk;
622 struct dst_entry *dst = NULL;
623 struct dst_cache *dst_cache;
624 struct flowi6 fl6;
625
626 dst_cache = (struct dst_cache *)&pctx->dst_cache;
> 627 dst = dst_cache_get_ip6(dst_cache, saddr);
628 if (dst)
629 return dst;
630
631 memset(&fl6, 0, sizeof(fl6));
632 fl6.flowi6_mark = skb->mark;
633 fl6.flowi6_proto = IPPROTO_UDP;
634 fl6.daddr = pctx->peer_addr;
635
636 dst = ipv6_stub->ipv6_dst_lookup_flow(sock_net(sk), sk, &fl6, NULL);
637 if (IS_ERR(dst)) {
638 netdev_dbg(pctx->dev, "no route to %pI6\n", &fl6.daddr);
639 return ERR_PTR(-ENETUNREACH);
640 }
641 if (dst->dev == pctx->dev) {
642 netdev_dbg(pctx->dev, "circular route to %pI6\n", &fl6.daddr);
643 dst_release(dst);
644 return ERR_PTR(-ELOOP);
645 }
646
647 *saddr = fl6.saddr;
648
> 649 dst_cache_set_ip6(dst_cache, dst, saddr);
650
651 return dst;
652 }
653
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 26882 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 08/12] gtp: set dev features to enable GSO
2020-12-11 12:26 ` [PATCH net-next v2 08/12] gtp: set dev features to enable GSO Jonas Bonn
@ 2020-12-12 5:31 ` Pravin Shelar
2020-12-12 7:50 ` Jonas Bonn
0 siblings, 1 reply; 36+ messages in thread
From: Pravin Shelar @ 2020-12-12 5:31 UTC (permalink / raw)
To: Jonas Bonn; +Cc: Linux Kernel Network Developers, Pablo Neira Ayuso, laforge
On Fri, Dec 11, 2020 at 4:28 AM Jonas Bonn <jonas@norrbonn.se> wrote:
>
> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
> ---
> drivers/net/gtp.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> index 236ebbcb37bf..7bbeec173113 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -536,7 +536,11 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> if (unlikely(r))
> goto err_rt;
>
> - skb_reset_inner_headers(skb);
> + r = udp_tunnel_handle_offloads(skb, true);
> + if (unlikely(r))
> + goto err_rt;
> +
> + skb_set_inner_protocol(skb, skb->protocol);
>
This should be skb_set_inner_ipproto(), since GTP is L3 protocol.
> gtp_push_header(skb, pctx, &port);
>
> @@ -618,6 +622,8 @@ static void gtp_link_setup(struct net_device *dev)
>
> dev->priv_flags |= IFF_NO_QUEUE;
> dev->features |= NETIF_F_LLTX;
> + dev->hw_features |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
> + dev->features |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
> netif_keep_dst(dev);
>
> dev->needed_headroom = LL_MAX_HEADER + max_gtp_header_len;
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 07/12] gtp: use ephemeral source port
2020-12-11 12:26 ` [PATCH net-next v2 07/12] gtp: use ephemeral source port Jonas Bonn
@ 2020-12-12 5:35 ` Pravin Shelar
2020-12-12 7:49 ` Jonas Bonn
2020-12-12 10:07 ` Harald Welte
1 sibling, 1 reply; 36+ messages in thread
From: Pravin Shelar @ 2020-12-12 5:35 UTC (permalink / raw)
To: Jonas Bonn; +Cc: Linux Kernel Network Developers, Pablo Neira Ayuso, laforge
On Fri, Dec 11, 2020 at 4:29 AM Jonas Bonn <jonas@norrbonn.se> wrote:
>
> All GTP traffic is currently sent from the same source port. This makes
> everything look like one big flow which is difficult to balance across
> network resources.
>
> From 3GPP TS 29.281:
> "...the UDP Source Port or the Flow Label field... should be set dynamically
> by the sending GTP-U entity to help balancing the load in the transport
> network."
>
> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
> ---
> drivers/net/gtp.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> index 4a3a52970856..236ebbcb37bf 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -477,7 +477,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> __be32 saddr;
> struct iphdr *iph;
> int headroom;
> - __be16 port;
> + __be16 sport, port;
> int r;
>
> /* Read the IP destination address and resolve the PDP context.
> @@ -527,6 +527,10 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> return -EMSGSIZE;
> }
>
> + sport = udp_flow_src_port(sock_net(pctx->sk), skb,
> + 0, USHRT_MAX,
> + true);
> +
why use_eth is true for this is L3 GTP devices, Am missing something?
> /* Ensure there is sufficient headroom. */
> r = skb_cow_head(skb, headroom);
> if (unlikely(r))
> @@ -545,7 +549,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> iph->tos,
> ip4_dst_hoplimit(&rt->dst),
> 0,
> - port, port,
> + sport, port,
> !net_eq(sock_net(pctx->sk),
> dev_net(pctx->dev)),
> false);
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 10/12] gtp: add IPv6 support
2020-12-11 12:26 ` [PATCH net-next v2 10/12] gtp: add IPv6 support Jonas Bonn
@ 2020-12-12 5:51 ` Pravin Shelar
2020-12-12 7:05 ` Jonas Bonn
2020-12-12 11:22 ` Harald Welte
1 sibling, 1 reply; 36+ messages in thread
From: Pravin Shelar @ 2020-12-12 5:51 UTC (permalink / raw)
To: Jonas Bonn; +Cc: Linux Kernel Network Developers, Pablo Neira Ayuso, laforge
On Fri, Dec 11, 2020 at 4:29 AM Jonas Bonn <jonas@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 <jonas@norrbonn.se>
> ---
> drivers/net/gtp.c | 330 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 269 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> index 86639fae8d45..4c902bffefa3 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -3,6 +3,7 @@
> *
> * (C) 2012-2014 by sysmocom - s.f.m.c. GmbH
> * (C) 2016 by Pablo Neira Ayuso <pablo@netfilter.org>
> + * (C) 2020 by Jonas Bonn <jonas@norrbonn.se>
> *
> * Author: Harald Welte <hwelte@sysmocom.de>
> * Pablo Neira Ayuso <pablo@netfilter.org>
> @@ -20,6 +21,7 @@
> #include <linux/net.h>
> #include <linux/file.h>
> #include <linux/gtp.h>
> +#include <linux/ipv6.h>
>
> #include <net/net_namespace.h>
> #include <net/protocol.h>
> @@ -33,6 +35,11 @@
> #include <net/netns/generic.h>
> #include <net/gtp.h>
>
> +#define PDP_F_PEER_V6 (1 << 0)
> +#define PDP_F_MS_V6 (1 << 1)
> +
> +#define ipv4(in6addr) ((in6addr)->s6_addr32[3])
> +
> /* An active session for the subscriber. */
> struct pdp_ctx {
> struct hlist_node hlist_tid;
> @@ -49,10 +56,10 @@ struct pdp_ctx {
> } v1;
> } u;
> u8 gtp_version;
> - u16 af;
> + u16 flags;
>
> - struct in_addr ms_addr_ip4;
> - struct in_addr peer_addr_ip4;
> + struct in6_addr ms_addr;
> + struct in6_addr peer_addr;
>
> struct sock *sk;
> struct net_device *dev;
> @@ -97,9 +104,23 @@ static inline u32 gtp1u_hashfn(u32 tid)
> return jhash_1word(tid, gtp_h_initval);
> }
>
> +static inline u32 ip_hashfn(struct in6_addr *ip)
> +{
> + return __ipv6_addr_jhash(ip, gtp_h_initval);
> +}
> +
> +static inline u32 ipv6_hashfn(struct in6_addr *ip)
> +{
> + return ip_hashfn(ip);
> +}
> +
> static inline u32 ipv4_hashfn(__be32 ip)
> {
> - return jhash_1word((__force u32)ip, gtp_h_initval);
> + struct in6_addr addr;
> +
> + ipv6_addr_set_v4mapped(ip, &addr);
> +
> + return ipv6_hashfn(&addr);
> }
>
> /* Resolve a PDP context structure based on the 64bit TID. */
> @@ -134,23 +155,42 @@ static struct pdp_ctx *gtp1_pdp_find(struct gtp_dev *gtp, u32 tid)
> return NULL;
> }
>
> -/* Resolve a PDP context based on IPv4 address of MS. */
> -static struct pdp_ctx *ipv4_pdp_find(struct gtp_dev *gtp, __be32 ms_addr)
> +static struct pdp_ctx *ip_pdp_find(struct gtp_dev *gtp,
> + struct in6_addr *ms_addr)
> {
> struct hlist_head *head;
> struct pdp_ctx *pdp;
>
> - head = >p->addr_hash[ipv4_hashfn(ms_addr) % gtp->hash_size];
> + head = >p->addr_hash[ipv6_hashfn(ms_addr) % gtp->hash_size];
>
> hlist_for_each_entry_rcu(pdp, head, hlist_addr) {
> - if (pdp->af == AF_INET &&
> - pdp->ms_addr_ip4.s_addr == ms_addr)
> + if (ipv6_addr_equal(&pdp->ms_addr, ms_addr))
> return pdp;
> }
>
> return NULL;
> }
>
> +/* Resolve a PDP context based on IPv6 address of MS. */
> +static struct pdp_ctx *ipv6_pdp_find(struct gtp_dev *gtp,
> + struct in6_addr *ms_addr)
> +{
> + return ip_pdp_find(gtp, ms_addr);
> +}
> +
> +/* Resolve a PDP context based on IPv4 address of MS. */
> +static struct pdp_ctx *ipv4_pdp_find(struct gtp_dev *gtp, __be32 ms_addr)
> +{
> + struct in6_addr addr;
> +
> + ipv6_addr_set_v4mapped(ms_addr, &addr);
> +
> + return ip_pdp_find(gtp, &addr);
> +}
> +
> +/* Check if the inner IP address in this packet is assigned to any
> + * existing mobile subscriber.
> + */
> static bool gtp_check_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx,
> unsigned int hdrlen, unsigned int role)
> {
> @@ -162,28 +202,51 @@ static bool gtp_check_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx,
> iph = (struct iphdr *)(skb->data + hdrlen);
>
> if (role == GTP_ROLE_SGSN)
> - return iph->daddr == pctx->ms_addr_ip4.s_addr;
> + return iph->daddr == ipv4(&pctx->ms_addr);
> else
> - return iph->saddr == pctx->ms_addr_ip4.s_addr;
> + return iph->saddr == ipv4(&pctx->ms_addr);
> }
>
> -/* Check if the inner IP address in this packet is assigned to any
> - * existing mobile subscriber.
> - */
> -static bool gtp_check_ms(struct sk_buff *skb, struct pdp_ctx *pctx,
> - unsigned int hdrlen, unsigned int role)
> +static bool gtp_check_ms_ipv6(struct sk_buff *skb, struct pdp_ctx *pctx,
> + unsigned int hdrlen, unsigned int role)
> {
> - switch (ntohs(skb->protocol)) {
> - case ETH_P_IP:
> - return gtp_check_ms_ipv4(skb, pctx, hdrlen, role);
> - }
> - return false;
> + struct ipv6hdr *iph;
> +
> + if (!pskb_may_pull(skb, hdrlen + sizeof(struct ipv6hdr)))
> + return false;
> +
> + iph = (struct ipv6hdr *)(skb->data + hdrlen);
> +
> + if (role == GTP_ROLE_SGSN)
> + return ipv6_addr_equal(&iph->daddr, &pctx->ms_addr);
> + else
> + return ipv6_addr_equal(&iph->saddr, &pctx->ms_addr);
> }
>
> static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb,
> unsigned int hdrlen, unsigned int role)
> {
> - if (!gtp_check_ms(skb, pctx, hdrlen, role)) {
> + uint8_t ipver;
> + int r;
> +
> + if (!pskb_may_pull(skb, hdrlen + 1))
> + return false;
> +
> + /* 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.
> 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.
> dev_sw_netstats_rx_add(pctx->dev, skb->len);
>
> @@ -220,7 +285,9 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
> if (!pskb_may_pull(skb, hdrlen))
> return -1;
>
> - gtp0 = (struct gtp0_header *)(skb->data + sizeof(struct udphdr));
> + skb_set_inner_network_header(skb, skb_transport_offset(skb) + hdrlen);
> +
> + gtp0 = (struct gtp0_header *)&udp_hdr(skb)[1];
>
> if ((gtp0->flags >> 5) != GTP_V0)
> return 1;
> @@ -247,7 +314,9 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
> if (!pskb_may_pull(skb, hdrlen))
> return -1;
>
> - gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
> + skb_set_inner_network_header(skb, skb_transport_offset(skb) + hdrlen);
> +
> + gtp1 = (struct gtp1_header *)&udp_hdr(skb)[1];
>
> if ((gtp1->flags >> 5) != GTP_V1)
> return 1;
> @@ -264,12 +333,10 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
> if (gtp1->flags & GTP1_F_MASK)
> hdrlen += 4;
>
> - /* Make sure the header is larger enough, including extensions. */
> + /* Make sure the header is large enough, including extensions. */
> if (!pskb_may_pull(skb, hdrlen))
> return -1;
>
> - gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
> -
> pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
> if (!pctx) {
> netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
> @@ -515,7 +582,7 @@ static struct rtable *gtp_get_v4_rt(struct sk_buff *skb,
>
> memset(&fl4, 0, sizeof(fl4));
> fl4.flowi4_oif = sk->sk_bound_dev_if;
> - fl4.daddr = pctx->peer_addr_ip4.s_addr;
> + fl4.daddr = ipv4(&pctx->peer_addr);
> fl4.saddr = inet_sk(sk)->inet_saddr;
> fl4.flowi4_tos = RT_CONN_FLAGS(sk);
> fl4.flowi4_proto = sk->sk_protocol;
> @@ -536,6 +603,36 @@ static struct rtable *gtp_get_v4_rt(struct sk_buff *skb,
> return rt;
> }
>
> +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).
> static inline void gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx)
> {
> int payload_len = skb->len;
> @@ -591,10 +688,9 @@ static void gtp_push_header(struct sk_buff *skb, struct pdp_ctx *pctx,
> }
> }
>
> -static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> +static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev,
> + struct pdp_ctx* pctx)
> {
> - struct gtp_dev *gtp = netdev_priv(dev);
> - struct pdp_ctx *pctx;
> struct rtable *rt;
> __be32 saddr;
> struct iphdr *iph;
> @@ -602,22 +698,6 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> __be16 sport, port;
> int r;
>
> - /* Read the IP destination address and resolve the PDP context.
> - * Prepend PDP header with TEI/TID from PDP ctx.
> - */
> - iph = ip_hdr(skb);
> - if (gtp->role == GTP_ROLE_SGSN)
> - pctx = ipv4_pdp_find(gtp, iph->saddr);
> - else
> - pctx = ipv4_pdp_find(gtp, iph->daddr);
> -
> - if (!pctx) {
> - netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n",
> - &iph->daddr);
> - return -ENOENT;
> - }
> - netdev_dbg(dev, "found PDP context %p\n", pctx);
> -
> rt = gtp_get_v4_rt(skb, dev, pctx, &saddr);
> if (IS_ERR(rt)) {
> if (PTR_ERR(rt) == -ENETUNREACH)
> @@ -671,7 +751,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> &iph->saddr, &iph->daddr);
>
> udp_tunnel_xmit_skb(rt, pctx->sk, skb,
> - saddr, pctx->peer_addr_ip4.s_addr,
> + saddr, ipv4(&pctx->peer_addr),
> iph->tos,
> ip4_dst_hoplimit(&rt->dst),
> 0,
> @@ -686,9 +766,130 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> return -EBADMSG;
> }
>
> +static int gtp_xmit_ip6(struct sk_buff *skb, struct net_device *dev,
> + struct pdp_ctx* pctx)
> +{
> + struct dst_entry *dst;
> + struct in6_addr saddr;
> + struct ipv6hdr *iph;
> + int headroom;
> + __be16 sport, port;
> + int r;
> +
> + dst = gtp_get_v6_dst(skb, dev, pctx, &saddr);
> + if (IS_ERR(dst)) {
> + if (PTR_ERR(dst) == -ENETUNREACH)
> + dev->stats.tx_carrier_errors++;
> + else if (PTR_ERR(dst) == -ELOOP)
> + dev->stats.collisions++;
> + return PTR_ERR(dst);
> + }
> +
> + headroom = sizeof(struct ipv6hdr) + sizeof(struct udphdr);
> +
> + switch (pctx->gtp_version) {
> + case GTP_V0:
> + headroom += sizeof(struct gtp0_header);
> + break;
> + case GTP_V1:
> + headroom += sizeof(struct gtp1_header);
> + break;
> + }
> +
> + sport = udp_flow_src_port(sock_net(pctx->sk), skb,
> + 0, USHRT_MAX,
> + true);
> +
> + r = skb_tunnel_check_pmtu(skb, dst, headroom,
> + netif_is_any_bridge_port(dev));
> + if (r < 0) {
> + dst_release(dst);
> + return r;
> + } else if (r) {
> + netif_rx(skb);
> + dst_release(dst);
> + return -EMSGSIZE;
> + }
> +
> + skb_scrub_packet(skb, !net_eq(sock_net(pctx->sk), dev_net(pctx->dev)));
> +
> + /* Ensure there is sufficient headroom. */
> + r = skb_cow_head(skb, dev->needed_headroom);
> + if (unlikely(r))
> + goto free_dst;
> +
> + r = udp_tunnel_handle_offloads(skb, true);
> + if (unlikely(r))
> + goto free_dst;
> +
> + skb_set_inner_protocol(skb, skb->protocol);
> +
> + gtp_push_header(skb, pctx, &port);
> +
> + iph = ipv6_hdr(skb);
> + netdev_dbg(dev, "gtp -> IP src: %pI6 dst: %pI6\n",
> + &iph->saddr, &iph->daddr);
> +
> + udp_tunnel6_xmit_skb(dst, pctx->sk, skb,
> + skb->dev,
> + &saddr, &pctx->peer_addr,
> + 0,
> + ip6_dst_hoplimit(dst),
> + 0,
> + sport, port,
> + false);
> +
> + return 0;
> +
> +free_dst:
> + dst_release(dst);
> + return -EBADMSG;
> +}
> +
> +static struct pdp_ctx *pdp_find(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct gtp_dev *gtp = netdev_priv(dev);
> + unsigned int proto = ntohs(skb->protocol);
> + struct pdp_ctx* pctx = NULL;
> +
> + switch (proto) {
> + case ETH_P_IP: {
> + __be32 addr;
> + struct iphdr *iph = ip_hdr(skb);
> + addr = (gtp->role == GTP_ROLE_SGSN) ? iph->saddr : iph->daddr;
> + pctx = ipv4_pdp_find(gtp, addr);
> +
> + if (!pctx) {
> + netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n",
> + &addr);
> + }
> +
> + break;
> + }
> + case ETH_P_IPV6: {
> + struct in6_addr* addr;
> + struct ipv6hdr *iph = ipv6_hdr(skb);
> + addr = (gtp->role == GTP_ROLE_SGSN) ? &iph->saddr : &iph->daddr;
> + pctx = ipv6_pdp_find(gtp, addr);
> +
> + if (!pctx) {
> + netdev_dbg(dev, "no PDP ctx found for %pI6, skip\n",
> + addr);
> + }
> +
> + break;
> + }
> + default:
> + break;
> + }
> +
> + return pctx;
> +}
> +
> static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> unsigned int proto = ntohs(skb->protocol);
> + struct pdp_ctx *pctx;
> int err;
>
> if (proto != ETH_P_IP && proto != ETH_P_IPV6) {
> @@ -699,7 +900,17 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> /* PDP context lookups in gtp_build_skb_*() need rcu read-side lock. */
> rcu_read_lock();
>
> - err = gtp_xmit_ip4(skb, dev);
> + pctx = pdp_find(skb, dev);
> + if (!pctx) {
> + err = -ENOENT;
> + rcu_read_unlock();
> + goto tx_err;
> + }
> +
> + if (pctx->flags & PDP_F_PEER_V6)
> + err = gtp_xmit_ip6(skb, dev, pctx);
> + else
> + err = gtp_xmit_ip4(skb, dev, pctx);
>
> rcu_read_unlock();
>
> @@ -726,7 +937,7 @@ static const struct device_type gtp_type = {
>
> static void gtp_link_setup(struct net_device *dev)
> {
> - unsigned int max_gtp_header_len = sizeof(struct iphdr) +
> + unsigned int max_gtp_header_len = sizeof(struct ipv6hdr) +
> sizeof(struct udphdr) +
> sizeof(struct gtp0_header);
>
> @@ -1023,11 +1234,8 @@ static struct gtp_dev *gtp_find_dev(struct net *src_net, struct nlattr *nla[])
> static void ipv4_pdp_fill(struct pdp_ctx *pctx, struct genl_info *info)
> {
> pctx->gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]);
> - pctx->af = AF_INET;
> - pctx->peer_addr_ip4.s_addr =
> - nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]);
> - pctx->ms_addr_ip4.s_addr =
> - nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
> + ipv4(&pctx->peer_addr) = nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]);
> + ipv4(&pctx->ms_addr) = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
>
> switch (pctx->gtp_version) {
> case GTP_V0:
> @@ -1127,13 +1335,13 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
> switch (pctx->gtp_version) {
> case GTP_V0:
> netdev_dbg(dev, "GTPv0-U: new PDP ctx id=%llx ssgn=%pI4 ms=%pI4 (pdp=%p)\n",
> - pctx->u.v0.tid, &pctx->peer_addr_ip4,
> - &pctx->ms_addr_ip4, pctx);
> + pctx->u.v0.tid, &ipv4(&pctx->peer_addr),
> + &ipv4(&pctx->ms_addr), pctx);
> break;
> case GTP_V1:
> netdev_dbg(dev, "GTPv1-U: new PDP ctx id=%x/%x ssgn=%pI4 ms=%pI4 (pdp=%p)\n",
> pctx->u.v1.i_tei, pctx->u.v1.o_tei,
> - &pctx->peer_addr_ip4, &pctx->ms_addr_ip4, pctx);
> + &ipv4(&pctx->peer_addr), &ipv4(&pctx->ms_addr), pctx);
> break;
> }
>
> @@ -1315,8 +1523,8 @@ static int gtp_genl_fill_info(struct sk_buff *skb, u32 snd_portid, u32 snd_seq,
>
> if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) ||
> nla_put_u32(skb, GTPA_LINK, pctx->dev->ifindex) ||
> - nla_put_be32(skb, GTPA_PEER_ADDRESS, pctx->peer_addr_ip4.s_addr) ||
> - nla_put_be32(skb, GTPA_MS_ADDRESS, pctx->ms_addr_ip4.s_addr))
> + nla_put_be32(skb, GTPA_PEER_ADDRESS, ipv4(&pctx->peer_addr)) ||
> + nla_put_be32(skb, GTPA_MS_ADDRESS, ipv4(&pctx->ms_addr)))
> goto nla_put_failure;
>
> switch (pctx->gtp_version) {
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 10/12] gtp: add IPv6 support
2020-12-12 5:51 ` Pravin Shelar
@ 2020-12-12 7:05 ` Jonas Bonn
2020-12-12 11:05 ` Harald Welte
0 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-12 7:05 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers, Pablo Neira Ayuso, laforge
Hi Pravin,
On 12/12/2020 06:51, Pravin Shelar wrote:
> On Fri, Dec 11, 2020 at 4:29 AM Jonas Bonn <jonas@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 <jonas@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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 07/12] gtp: use ephemeral source port
2020-12-12 5:35 ` Pravin Shelar
@ 2020-12-12 7:49 ` Jonas Bonn
0 siblings, 0 replies; 36+ messages in thread
From: Jonas Bonn @ 2020-12-12 7:49 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers, Pablo Neira Ayuso, laforge
On 12/12/2020 06:35, Pravin Shelar wrote:
> On Fri, Dec 11, 2020 at 4:29 AM Jonas Bonn <jonas@norrbonn.se> wrote:
>> /* Read the IP destination address and resolve the PDP context.
>> @@ -527,6 +527,10 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
>> return -EMSGSIZE;
>> }
>>
>> + sport = udp_flow_src_port(sock_net(pctx->sk), skb,
>> + 0, USHRT_MAX,
>> + true);
>> +
> why use_eth is true for this is L3 GTP devices, Am missing something?
No, you are right. Will fix.
/Jonas
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 08/12] gtp: set dev features to enable GSO
2020-12-12 5:31 ` Pravin Shelar
@ 2020-12-12 7:50 ` Jonas Bonn
2020-12-13 18:25 ` Pravin Shelar
0 siblings, 1 reply; 36+ messages in thread
From: Jonas Bonn @ 2020-12-12 7:50 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers, Pablo Neira Ayuso, laforge
On 12/12/2020 06:31, Pravin Shelar wrote:
> On Fri, Dec 11, 2020 at 4:28 AM Jonas Bonn <jonas@norrbonn.se> wrote:
>>
>> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
>> ---
>> drivers/net/gtp.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
>> index 236ebbcb37bf..7bbeec173113 100644
>> --- a/drivers/net/gtp.c
>> +++ b/drivers/net/gtp.c
>> @@ -536,7 +536,11 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
>> if (unlikely(r))
>> goto err_rt;
>>
>> - skb_reset_inner_headers(skb);
>> + r = udp_tunnel_handle_offloads(skb, true);
>> + if (unlikely(r))
>> + goto err_rt;
>> +
>> + skb_set_inner_protocol(skb, skb->protocol);
>>
> This should be skb_set_inner_ipproto(), since GTP is L3 protocol.
I think you're right, but I barely see the point of this. It all ends
up in the same place, as far as I can tell. Is this supposed to be some
sort of safeguard against trying to offload tunnels-in-tunnels?
/Jonas
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 01/12] gtp: set initial MTU
2020-12-11 12:26 ` [PATCH net-next v2 01/12] gtp: set initial MTU Jonas Bonn
@ 2020-12-12 9:38 ` Harald Welte
0 siblings, 0 replies; 36+ messages in thread
From: Harald Welte @ 2020-12-12 9:38 UTC (permalink / raw)
To: Jonas Bonn; +Cc: netdev, pablo
On Fri, Dec 11, 2020 at 01:26:01PM +0100, Jonas Bonn wrote:
> The GTP link is brought up with a default MTU of zero. This can lead to
> some rather unexpected behaviour for users who are more accustomed to
> interfaces coming online with reasonable defaults.
>
> This patch sets an initial MTU for the GTP link of 1500 less worst-case
> tunnel overhead.
Thanks, LGTM. I would probably have gone to a #define or a 'const' variable,
but I guess compilers should be smart enough to figure out that this is
static at compile time even the way you wrote it.
Acked-by: Harald Welte <laforge@gnumonks.org>
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 02/12] gtp: include role in link info
2020-12-11 12:26 ` [PATCH net-next v2 02/12] gtp: include role in link info Jonas Bonn
@ 2020-12-12 9:40 ` Harald Welte
0 siblings, 0 replies; 36+ messages in thread
From: Harald Welte @ 2020-12-12 9:40 UTC (permalink / raw)
To: Jonas Bonn; +Cc: netdev, pablo
Hi Jonas,
On Fri, Dec 11, 2020 at 01:26:02PM +0100, Jonas Bonn wrote:
> Querying link info for the GTP interface doesn't reveal in which "role" the
> device is set to operate. Include this information in the info query
> result.
>
> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
Acked-by: Harald Welte <laforge@gnumonks.org>
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 03/12] gtp: really check namespaces before xmit
2020-12-11 12:26 ` [PATCH net-next v2 03/12] gtp: really check namespaces before xmit Jonas Bonn
@ 2020-12-12 9:41 ` Harald Welte
0 siblings, 0 replies; 36+ messages in thread
From: Harald Welte @ 2020-12-12 9:41 UTC (permalink / raw)
To: Jonas Bonn; +Cc: netdev, pablo
On Fri, Dec 11, 2020 at 01:26:03PM +0100, Jonas Bonn wrote:
> Blindly assuming that packet transmission crosses namespaces results in
> skb marks being lost in the single namespace case.
>
> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
Acked-by: Harald Welte <laforge@gnumonks.org>
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 04/12] gtp: drop unnecessary call to skb_dst_drop
2020-12-11 12:26 ` [PATCH net-next v2 04/12] gtp: drop unnecessary call to skb_dst_drop Jonas Bonn
@ 2020-12-12 9:50 ` Harald Welte
2020-12-12 11:38 ` Jonas Bonn
0 siblings, 1 reply; 36+ messages in thread
From: Harald Welte @ 2020-12-12 9:50 UTC (permalink / raw)
To: Jonas Bonn; +Cc: netdev, pablo
On Fri, Dec 11, 2020 at 01:26:04PM +0100, Jonas Bonn wrote:
> The call to skb_dst_drop() is already done as part of udp_tunnel_xmit().
I must be blind, can you please point out where exactly this happens?
I don't see any skb_dst_drop in udp_tunnel_xmit_skb, and
in iptunnel_xmit() there's only a skb_dst_set (which doesn't call skb_dst_drop internally)
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 05/12] gtp: set device type
2020-12-11 12:26 ` [PATCH net-next v2 05/12] gtp: set device type Jonas Bonn
@ 2020-12-12 9:51 ` Harald Welte
0 siblings, 0 replies; 36+ messages in thread
From: Harald Welte @ 2020-12-12 9:51 UTC (permalink / raw)
To: Jonas Bonn; +Cc: netdev, pablo
On Fri, Dec 11, 2020 at 01:26:05PM +0100, Jonas Bonn wrote:
> Set the devtype to 'gtp' when setting up the link.
>
> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
Acked-by: Harald Welte <laforge@gnumonks.org>
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 07/12] gtp: use ephemeral source port
2020-12-11 12:26 ` [PATCH net-next v2 07/12] gtp: use ephemeral source port Jonas Bonn
2020-12-12 5:35 ` Pravin Shelar
@ 2020-12-12 10:07 ` Harald Welte
2020-12-12 13:40 ` Jonas Bonn
1 sibling, 1 reply; 36+ messages in thread
From: Harald Welte @ 2020-12-12 10:07 UTC (permalink / raw)
To: Jonas Bonn; +Cc: netdev, pablo
Hi Jonas,
On Fri, Dec 11, 2020 at 01:26:07PM +0100, Jonas Bonn wrote:
> From 3GPP TS 29.281:
> "...the UDP Source Port or the Flow Label field... should be set dynamically
> by the sending GTP-U entity to help balancing the load in the transport
> network."
You unfortuantely didn't specifiy which 3GPP release you are referring to.
At least in V15.7.0 (2020-01) Release 15 I can only find:
"For the messages described below, the UDP Source Port (except as
specified for the Echo Response message) may be allocated either
statically or dynamically by the sending GTP-U entity. NOTE: Dynamic
allocation of the UDP source port can help balancing the load in the
network, depending on network deployments and network node
implementations."
For GTPv0, TS 29.060 states:
"The UDP Source Port is a locally allocated port number at the sending
GSN/RNC."
unfortuantely it doesn't say if it's a locally allocated number globally
for that entire GSN/RNC, or it's dynamic per flow or per packet.
As I'm aware of a lot of very tight packet filtering between GSNs,
I would probably not go for fully dynamic source port allocation
without some kind of way how the user (GTP-control instance) being
able to decide on that policy.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 10/12] gtp: add IPv6 support
2020-12-12 7:05 ` Jonas Bonn
@ 2020-12-12 11:05 ` Harald Welte
0 siblings, 0 replies; 36+ messages in thread
From: Harald Welte @ 2020-12-12 11:05 UTC (permalink / raw)
To: Jonas Bonn
Cc: Pravin Shelar, Linux Kernel Network Developers, Pablo Neira Ayuso
Hi Jonas,
On Sat, Dec 12, 2020 at 08:05:40AM +0100, Jonas Bonn wrote:
> Yes, you're probably right. Given that IPv6 isn't really optional in
> contexts where this driver is relevant, [...]
I strongly contest this statement. GTP is used in a lot of legacy contexts
without any IPv6 requirements whatsoever. _particularly_ so on the outer/
transport level, where even GSMA IR.34 in still states:
> The IPX Provider's and Service Provider's networks must support IPv4
> addressing and routing. IPv6 addressing and routing is recommended.
So there is (still) no requirement for IPv6 on the transport level
between cellular operators.
The fact that this gtp module has existed until today with pure IPv4
support has something to say about that.
I'm of course all in support of finally getting IPv6 support merged (via
your patches!) - but I see absolutely no reason why a GTP kernel module
would have a mandatory dependency on IPv6.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 10/12] gtp: add IPv6 support
2020-12-11 12:26 ` [PATCH net-next v2 10/12] gtp: add IPv6 support Jonas Bonn
2020-12-12 5:51 ` Pravin Shelar
@ 2020-12-12 11:22 ` Harald Welte
2020-12-12 13:59 ` Jonas Bonn
1 sibling, 1 reply; 36+ messages in thread
From: Harald Welte @ 2020-12-12 11:22 UTC (permalink / raw)
To: Jonas Bonn; +Cc: netdev, pablo
Hi Jonas,
thanks again for your patches, they are very much appreciated.
However, I don't think that it is "that easy".
PDP contexts (at least) in GPRS/EDGE and UMTS come in three flavors:
* IPv4 only
* IPv6 only
* IPv4v6 (i.e. both an IPv4 and an IPv6 address within the same tunnel)
See for example osmo-ggsn at https://git.osmocom.org/osmo-ggsn
for an userspace implementation that covers all three cases,
as well as a related automatic test suite containing cases
for all three flavors at
https://git.osmocom.org/osmo-ttcn3-hacks/tree/ggsn_tests
If I read your patch correctly
On Fri, Dec 11, 2020 at 01:26:10PM +0100, Jonas Bonn wrote:
> - struct in_addr ms_addr_ip4;
> - struct in_addr peer_addr_ip4;
> + struct in6_addr ms_addr;
> + struct in6_addr peer_addr;
this simply replaces the (inner) IPv4 "MS addr" with an IPv6 "MS addr".
Sure, it is an improvement over v4-only. But IMHO any follow-up
change to introduce v4v6 PDP contexts would require significant changes,
basically re-introducing the ms_add_ip4 member which you are removing here.
Therefore, I argue very much in favor of intrducing proper IPv6 support
(including v4v6) in one go.
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 11/12] gtp: netlink update for ipv6
2020-12-11 12:26 ` [PATCH net-next v2 11/12] gtp: netlink update for ipv6 Jonas Bonn
@ 2020-12-12 11:25 ` Harald Welte
0 siblings, 0 replies; 36+ messages in thread
From: Harald Welte @ 2020-12-12 11:25 UTC (permalink / raw)
To: Jonas Bonn; +Cc: netdev, pablo
On Fri, Dec 11, 2020 at 01:26:11PM +0100, Jonas Bonn wrote:
> This patch adds the netlink changes required to support IPv6.
See my related comment to the other IPv6 patch in this series.
It is not legal to assume that v4/v6 are an either-or decision,
but it can be either v4-only, v6-only or v4 and v6 in the same PDP context.
For the "peer" (outer) address, I think it is correct to assume only either v4 or v6.
But for the inner "ms" address, it is not.
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 04/12] gtp: drop unnecessary call to skb_dst_drop
2020-12-12 9:50 ` Harald Welte
@ 2020-12-12 11:38 ` Jonas Bonn
0 siblings, 0 replies; 36+ messages in thread
From: Jonas Bonn @ 2020-12-12 11:38 UTC (permalink / raw)
To: Harald Welte; +Cc: netdev, pablo
On 12/12/2020 10:50, Harald Welte wrote:
> On Fri, Dec 11, 2020 at 01:26:04PM +0100, Jonas Bonn wrote:
>> The call to skb_dst_drop() is already done as part of udp_tunnel_xmit().
>
> I must be blind, can you please point out where exactly this happens?
It's in skb_scrub_packet() which is called by iptunnel_xmit().
/Jonas
>
> I don't see any skb_dst_drop in udp_tunnel_xmit_skb, and
> in iptunnel_xmit() there's only a skb_dst_set (which doesn't call skb_dst_drop internally)
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 07/12] gtp: use ephemeral source port
2020-12-12 10:07 ` Harald Welte
@ 2020-12-12 13:40 ` Jonas Bonn
0 siblings, 0 replies; 36+ messages in thread
From: Jonas Bonn @ 2020-12-12 13:40 UTC (permalink / raw)
To: Harald Welte; +Cc: netdev, pablo
Hi Harald,
On 12/12/2020 11:07, Harald Welte wrote:
> Hi Jonas,
>
> On Fri, Dec 11, 2020 at 01:26:07PM +0100, Jonas Bonn wrote:
>> From 3GPP TS 29.281:
>> "...the UDP Source Port or the Flow Label field... should be set dynamically
>> by the sending GTP-U entity to help balancing the load in the transport
>> network."
>
> You unfortuantely didn't specifiy which 3GPP release you are referring to.
>
Sorry about that. I'm looking at v16.1.0.
> At least in V15.7.0 (2020-01) Release 15 I can only find:
>
> "For the messages described below, the UDP Source Port (except as
> specified for the Echo Response message) may be allocated either
> statically or dynamically by the sending GTP-U entity. NOTE: Dynamic
> allocation of the UDP source port can help balancing the load in the
> network, depending on network deployments and network node
> implementations."
>
> For GTPv0, TS 29.060 states:
>
> "The UDP Source Port is a locally allocated port number at the sending
> GSN/RNC."
>
> unfortuantely it doesn't say if it's a locally allocated number globally
> for that entire GSN/RNC, or it's dynamic per flow or per packet.
I could interpret that to mean that the receiving end shouldn't be too
bothered about what port a packet happens to come from... That said,
dynamic per flow is almost certainly what we want here as this is mostly
about being able to trigger receive side scaling for network cards that
can't look at inner headers.
>
> As I'm aware of a lot of very tight packet filtering between GSNs,
> I would probably not go for fully dynamic source port allocation
> without some kind of way how the user (GTP-control instance) being
> able to decide on that policy.
>
Sure... sounds reasonable. A flag on the link setup indicating dynamic
source port yes/no should be sufficient, right?
/Jonas
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 10/12] gtp: add IPv6 support
2020-12-12 11:22 ` Harald Welte
@ 2020-12-12 13:59 ` Jonas Bonn
0 siblings, 0 replies; 36+ messages in thread
From: Jonas Bonn @ 2020-12-12 13:59 UTC (permalink / raw)
To: Harald Welte; +Cc: netdev, pablo
Hi Harald,
On 12/12/2020 12:22, Harald Welte wrote:
> Hi Jonas,
>
> thanks again for your patches, they are very much appreciated.
>
> However, I don't think that it is "that easy".
>
> PDP contexts (at least) in GPRS/EDGE and UMTS come in three flavors:
> * IPv4 only
> * IPv6 only
> * IPv4v6 (i.e. both an IPv4 and an IPv6 address within the same tunnel)
>
> See for example osmo-ggsn at https://git.osmocom.org/osmo-ggsn
> for an userspace implementation that covers all three cases,
> as well as a related automatic test suite containing cases
> for all three flavors at
> https://git.osmocom.org/osmo-ttcn3-hacks/tree/ggsn_tests
>
> If I read your patch correctly
>
> On Fri, Dec 11, 2020 at 01:26:10PM +0100, Jonas Bonn wrote:
>> - struct in_addr ms_addr_ip4;
>> - struct in_addr peer_addr_ip4;
>> + struct in6_addr ms_addr;
>> + struct in6_addr peer_addr;
>
> this simply replaces the (inner) IPv4 "MS addr" with an IPv6 "MS addr".
Yes, that's correct. It's currently an either-or proposition for the UE
address.
I actually wanted to proceed a bit carefully here because this patch
series is already quite large. I do plan on adding support for multiple
MS addresses, but already just juggling mixed inner and outer tunnels
seemed challenging enough to review.
>
> Sure, it is an improvement over v4-only. But IMHO any follow-up
> change to introduce v4v6 PDP contexts would require significant changes,
> basically re-introducing the ms_add_ip4 member which you are removing here.
I think we ultimately need to take the MS (UE address) member out of the
PDP context struct altogether. A single PDP context can apply to even
more than two addresses as the UE can/should be provisioned with
multiple IPv6 addresses (as I understand it, superficially).
>
> Therefore, I argue very much in favor of intrducing proper IPv6 support
> (including v4v6) in one go.
For provisioning contexts via Netlink, I agree that we should try to
avoid pathological intermediate steps. For the actual packet handling
of outer and inner IPv6 tunnels, I think it's moot whether or not the
PDP contexts support multiple addresses or not: the subsequent step to
extend to multiple IP's is a bit of churn, but doesn't immediately seem
overly intrusive.
Anyway, I'll look into making this change...
Thanks for the review!
/Jonas
>
> Regards,
> Harald
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v2 08/12] gtp: set dev features to enable GSO
2020-12-12 7:50 ` Jonas Bonn
@ 2020-12-13 18:25 ` Pravin Shelar
0 siblings, 0 replies; 36+ messages in thread
From: Pravin Shelar @ 2020-12-13 18:25 UTC (permalink / raw)
To: Jonas Bonn; +Cc: Linux Kernel Network Developers, Pablo Neira Ayuso, laforge
On Fri, Dec 11, 2020 at 11:50 PM Jonas Bonn <jonas@norrbonn.se> wrote:
>
>
>
> On 12/12/2020 06:31, Pravin Shelar wrote:
> > On Fri, Dec 11, 2020 at 4:28 AM Jonas Bonn <jonas@norrbonn.se> wrote:
> >>
> >> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
> >> ---
> >> drivers/net/gtp.c | 8 +++++++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> >> index 236ebbcb37bf..7bbeec173113 100644
> >> --- a/drivers/net/gtp.c
> >> +++ b/drivers/net/gtp.c
> >> @@ -536,7 +536,11 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> >> if (unlikely(r))
> >> goto err_rt;
> >>
> >> - skb_reset_inner_headers(skb);
> >> + r = udp_tunnel_handle_offloads(skb, true);
> >> + if (unlikely(r))
> >> + goto err_rt;
> >> +
> >> + skb_set_inner_protocol(skb, skb->protocol);
> >>
> > This should be skb_set_inner_ipproto(), since GTP is L3 protocol.
>
> I think you're right, but I barely see the point of this. It all ends
> up in the same place, as far as I can tell. Is this supposed to be some
> sort of safeguard against trying to offload tunnels-in-tunnels?
>
In UDP offload this flag is used to process segments. With correct
flag GTP offload handling can directly jump to L3 processing.
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2020-12-13 18:27 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 12:26 [PATCH net-next v2 00/12] gtp: IPv6 support Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 01/12] gtp: set initial MTU Jonas Bonn
2020-12-12 9:38 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 02/12] gtp: include role in link info Jonas Bonn
2020-12-12 9:40 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 03/12] gtp: really check namespaces before xmit Jonas Bonn
2020-12-12 9:41 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 04/12] gtp: drop unnecessary call to skb_dst_drop Jonas Bonn
2020-12-12 9:50 ` Harald Welte
2020-12-12 11:38 ` Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 05/12] gtp: set device type Jonas Bonn
2020-12-12 9:51 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 06/12] gtp: rework IPv4 functionality Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 07/12] gtp: use ephemeral source port Jonas Bonn
2020-12-12 5:35 ` Pravin Shelar
2020-12-12 7:49 ` Jonas Bonn
2020-12-12 10:07 ` Harald Welte
2020-12-12 13:40 ` Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 08/12] gtp: set dev features to enable GSO Jonas Bonn
2020-12-12 5:31 ` Pravin Shelar
2020-12-12 7:50 ` Jonas Bonn
2020-12-13 18:25 ` Pravin Shelar
2020-12-11 12:26 ` [PATCH net-next v2 09/12] gtp: support GRO Jonas Bonn
2020-12-11 15:43 ` kernel test robot
2020-12-11 15:43 ` kernel test robot
2020-12-11 12:26 ` [PATCH net-next v2 10/12] gtp: add IPv6 support Jonas Bonn
2020-12-12 5:51 ` Pravin Shelar
2020-12-12 7:05 ` Jonas Bonn
2020-12-12 11:05 ` Harald Welte
2020-12-12 11:22 ` Harald Welte
2020-12-12 13:59 ` Jonas Bonn
2020-12-11 12:26 ` [PATCH net-next v2 11/12] gtp: netlink update for ipv6 Jonas Bonn
2020-12-12 11:25 ` Harald Welte
2020-12-11 12:26 ` [PATCH net-next v2 12/12] gtp: add dst_cache to tunnels Jonas Bonn
2020-12-11 17:25 ` kernel test robot
2020-12-11 17:25 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.