[PATCH 1/5] gtp: set initial MTU

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 1/5] gtp: set initial MTU
@ 2020-12-02 12:33 Jonas Bonn
  2020-12-02 12:33 ` [PATCH 2/5] gtp: include role in link info Jonas Bonn
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Jonas Bonn @ 2020-12-02 12:33 UTC (permalink / raw)
  To: netdev, pablo; +Cc: 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		= &gtp_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] 6+ messages in thread

* [PATCH 2/5] gtp: include role in link info
  2020-12-02 12:33 [PATCH 1/5] gtp: set initial MTU Jonas Bonn
@ 2020-12-02 12:33 ` Jonas Bonn
  2020-12-03  8:52   ` Jeremy Sowden
  2020-12-02 12:33 ` [PATCH 3/5] gtp: really check namespaces before xmit Jonas Bonn
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Jonas Bonn @ 2020-12-02 12:33 UTC (permalink / raw)
  To: netdev, pablo; +Cc: 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..096aaf1c867e 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] 6+ messages in thread

* Re: [PATCH 2/5] gtp: include role in link info
  2020-12-02 12:33 ` [PATCH 2/5] gtp: include role in link info Jonas Bonn
@ 2020-12-03  8:52   ` Jeremy Sowden
  0 siblings, 0 replies; 6+ messages in thread
From: Jeremy Sowden @ 2020-12-03  8:52 UTC (permalink / raw)
  To: Jonas Bonn; +Cc: netdev, pablo, laforge

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On 2020-12-02, at 13:33:42 +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>
> ---
>  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..096aaf1c867e 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 */

You have a '+' at the end of l.781 and another at the beginning of
l.782.

>  }
>
>  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
>
>

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/5] gtp: really check namespaces before xmit
  2020-12-02 12:33 [PATCH 1/5] gtp: set initial MTU Jonas Bonn
  2020-12-02 12:33 ` [PATCH 2/5] gtp: include role in link info Jonas Bonn
@ 2020-12-02 12:33 ` Jonas Bonn
  2020-12-02 12:33 ` [PATCH 4/5] gtp: drop unnecessary call to skb_dst_drop Jonas Bonn
  2020-12-02 12:33 ` [PATCH 5/5] gtp: set device type Jonas Bonn
  3 siblings, 0 replies; 6+ messages in thread
From: Jonas Bonn @ 2020-12-02 12:33 UTC (permalink / raw)
  To: netdev, pablo; +Cc: 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 096aaf1c867e..e053f86f43f3 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] 6+ messages in thread

* [PATCH 4/5] gtp: drop unnecessary call to skb_dst_drop
  2020-12-02 12:33 [PATCH 1/5] gtp: set initial MTU Jonas Bonn
  2020-12-02 12:33 ` [PATCH 2/5] gtp: include role in link info Jonas Bonn
  2020-12-02 12:33 ` [PATCH 3/5] gtp: really check namespaces before xmit Jonas Bonn
@ 2020-12-02 12:33 ` Jonas Bonn
  2020-12-02 12:33 ` [PATCH 5/5] gtp: set device type Jonas Bonn
  3 siblings, 0 replies; 6+ messages in thread
From: Jonas Bonn @ 2020-12-02 12:33 UTC (permalink / raw)
  To: netdev, pablo; +Cc: 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 e053f86f43f3..c19465458187 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] 6+ messages in thread

* [PATCH 5/5] gtp: set device type
  2020-12-02 12:33 [PATCH 1/5] gtp: set initial MTU Jonas Bonn
                   ` (2 preceding siblings ...)
  2020-12-02 12:33 ` [PATCH 4/5] gtp: drop unnecessary call to skb_dst_drop Jonas Bonn
@ 2020-12-02 12:33 ` Jonas Bonn
  3 siblings, 0 replies; 6+ messages in thread
From: Jonas Bonn @ 2020-12-02 12:33 UTC (permalink / raw)
  To: netdev, pablo; +Cc: 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 c19465458187..6de38e06588d 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		= &gtp_netdev_ops;
 	dev->needs_free_netdev	= true;
+	SET_NETDEV_DEVTYPE(dev, &gtp_type);
 
 	dev->hard_header_len = 0;
 	dev->addr_len = 0;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-12-03  9:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-02 12:33 [PATCH 1/5] gtp: set initial MTU Jonas Bonn
2020-12-02 12:33 ` [PATCH 2/5] gtp: include role in link info Jonas Bonn
2020-12-03  8:52   ` Jeremy Sowden
2020-12-02 12:33 ` [PATCH 3/5] gtp: really check namespaces before xmit Jonas Bonn
2020-12-02 12:33 ` [PATCH 4/5] gtp: drop unnecessary call to skb_dst_drop Jonas Bonn
2020-12-02 12:33 ` [PATCH 5/5] gtp: set device type Jonas Bonn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).