This is the part of the previous "simple gtp improvements" series that Pablo indicated should go into net-next.
The rcu_lock removal is small correctness changes. Passing invalid to user space allows for more standards compliant handling of invalid tunnels.
-- Andreas Schultz (2): gtp: remove unnecessary rcu_read_lock gtp: let userspace handle packets for invalid tunnels
drivers/net/gtp.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)
The rcu read lock is hold by default in the ip input path. There is no need to hold it twice in the socket recv decapsulate code path.
Signed-off-by: Andreas Schultz aschultz@tpip.net --- drivers/net/gtp.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 8b6810b..5c63a37 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -184,7 +184,6 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb, sizeof(struct gtp0_header); struct gtp0_header *gtp0; struct pdp_ctx *pctx; - int ret = 0;
if (!pskb_may_pull(skb, hdrlen)) return -1; @@ -197,26 +196,19 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb, if (gtp0->type != GTP_TPDU) return 1;
- rcu_read_lock(); pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid)); if (!pctx) { netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb); - ret = -1; - goto out_rcu; + return -1; }
if (!gtp_check_src_ms(skb, pctx, hdrlen)) { netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); - ret = -1; - goto out_rcu; + return -1; } - rcu_read_unlock();
/* Get rid of the GTP + UDP headers. */ return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet); -out_rcu: - rcu_read_unlock(); - return ret; }
static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb, @@ -226,7 +218,6 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb, sizeof(struct gtp1_header); struct gtp1_header *gtp1; struct pdp_ctx *pctx; - int ret = 0;
if (!pskb_may_pull(skb, hdrlen)) return -1; @@ -254,26 +245,19 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
- rcu_read_lock(); pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid)); if (!pctx) { netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb); - ret = -1; - goto out_rcu; + return -1; }
if (!gtp_check_src_ms(skb, pctx, hdrlen)) { netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); - ret = -1; - goto out_rcu; + return -1; } - rcu_read_unlock();
/* Get rid of the GTP + UDP headers. */ return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet); -out_rcu: - rcu_read_unlock(); - return ret; }
static void gtp_encap_disable(struct gtp_dev *gtp)
enable userspace to send error replies for invalid tunnels
Acked-by: Harald Welte laforge@netfilter.org Signed-off-by: Andreas Schultz aschultz@tpip.net --- drivers/net/gtp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 5c63a37..5dd7de6 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -199,12 +199,12 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb, pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid)); if (!pctx) { netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb); - return -1; + return 1; }
if (!gtp_check_src_ms(skb, pctx, hdrlen)) { netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); - return -1; + return 1; }
/* Get rid of the GTP + UDP headers. */ @@ -248,12 +248,12 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb, pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid)); if (!pctx) { netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb); - return -1; + return 1; }
if (!gtp_check_src_ms(skb, pctx, hdrlen)) { netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); - return -1; + return 1; }
/* Get rid of the GTP + UDP headers. */
From: Andreas Schultz aschultz@tpip.net Date: Thu, 26 Jan 2017 16:21:47 +0100
This is the part of the previous "simple gtp improvements" series that Pablo indicated should go into net-next.
The rcu_lock removal is small correctness changes. Passing invalid to user space allows for more standards compliant handling of invalid tunnels.
Series applied, thanks.
-
Andreas Schultz -
David Miller