netdev - Re: [PATCH net-next v2] GTP: add support for flow based tunneling API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <67f7c207-a537-dd22-acd8-dcce42755d1a@norrbonn.se>
Date:   Sun, 13 Dec 2020 08:56:28 +0100
From:   Jonas Bonn <jonas@...rbonn.se>
To:     Pravin B Shelar <pbshelar@...com>, netdev@...r.kernel.org,
        pablo@...filter.org, laforge@...monks.org
Cc:     pravin.ovn@...il.com
Subject: Re: [PATCH net-next v2] GTP: add support for flow based tunneling API

Hi Pravin,

I've been thinking a bit about this and find it more and more 
interesting.  Could you post a bit of information about the ip-route 
changes you'll make in order to support GTP LWT encapsulation?  Could 
you provide an example command line?

I understand the advantages here of coupling to BPF and OVS.  How does 
storing the encapsulation parameters via ip-route compare to storing 
them as PDP contexts from the point of view of resource consumption? 
Are there are other advantages/disadvantages?

On 12/12/2020 05:40, Pravin B Shelar wrote:
> Following patch add support for flow based tunneling API
> to send and recv GTP tunnel packet over tunnel metadata API.
> This would allow this device integration with OVS or eBPF using
> flow based tunneling APIs.
> 
> Signed-off-by: Pravin B Shelar <pbshelar@...com>
> ---
> Fixed according to comments from Jonas Bonn
> ---
>   drivers/net/gtp.c                  | 514 ++++++++++++++++++++---------
>   include/uapi/linux/gtp.h           |  12 +
>   include/uapi/linux/if_link.h       |   1 +
>   include/uapi/linux/if_tunnel.h     |   1 +
>   tools/include/uapi/linux/if_link.h |   1 +
>   5 files changed, 382 insertions(+), 147 deletions(-)
> 
> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> index 4c04e271f184..0e212a70fe4b 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -21,6 +21,7 @@
>   #include <linux/file.h>
>   #include <linux/gtp.h>
>   
> +#include <net/dst_metadata.h>
>   #include <net/net_namespace.h>
>   #include <net/protocol.h>
>   #include <net/ip.h>
> @@ -73,6 +74,9 @@ struct gtp_dev {
>   	unsigned int		hash_size;
>   	struct hlist_head	*tid_hash;
>   	struct hlist_head	*addr_hash;
> +	/* Used by flow based tunnel. */
> +	bool			collect_md;
> +	struct socket		*collect_md_sock;

I'm not convinced that you need to special-case LWT in this way.  It 
should be possible to just use the regular sk1u socket.  I know that the 
sk1u socket is created in userspace and might be set up to listen on the 
wrong address, but that's a user error if they try to use that device 
with LWT.  You could easily make the sk1u socket an optional parameter 
and create it (as you do in your patch) if it's not provided.  Then 
ip_tunnel_collect_metadata() would tell you whether to get the 
encapsulaton details from the tunnel itself or whether to look up a PDP 
context.  That should suffice, right?

/Jonas