[PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Or Gerlitz]

Generate the source udp header according to the flow represented by
the packet we are encapsulating, as done for other udp tunnels. This
helps on the receiver side to apply RSS spreading.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---

found in code inspection.. compile tested only


 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 bda0c64..ff1244b 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -564,6 +564,7 @@ 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;
+	__be16 src_port;
 	int err;
 
 	/* Ensure there is sufficient headroom. */
@@ -572,6 +573,8 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	skb_reset_inner_headers(skb);
 
+	src_port = udp_flow_src_port(dev_net(dev), skb, 0, 0, true);
+
 	/* PDP context lookups in gtp_build_skb_*() need rcu read-side lock. */
 	rcu_read_lock();
 	switch (proto) {
@@ -596,7 +599,7 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 				    pktinfo.iph->tos,
 				    ip4_dst_hoplimit(&pktinfo.rt->dst),
 				    0,
-				    pktinfo.gtph_port, pktinfo.gtph_port,
+				    src_port, pktinfo.gtph_port,
 				    true, false);
 		break;
 	}
-- 
2.3.7

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Andreas Schultz]

Hi Or,

----- On Feb 16, 2017, at 3:59 PM, Or Gerlitz ogerlitz@mellanox.com wrote:

> Generate the source udp header according to the flow represented by
> the packet we are encapsulating, as done for other udp tunnels. This
> helps on the receiver side to apply RSS spreading.

This might work for GTPv0-U, However, for GTPv1-U this could interfere
with error handling in the user space control process when the UDP port
extension  header is used in error indications.

3GPP TS 29.281 Rel 13, section 5.2.2.1 defines the UDP port extension and
section 7.3.1 says that the UDP source port extension can be used to 
mitigate DOS attacks. This would IMHO imply that the user space control
process needs to know the TEID to UDP source port mapping.

The other question is, on what is this actually hashing. When I understand
the code correctly, this will hash on the source/destination of the orignal
flow. I would expect that a SGSN/SGW/eNodeB would like the keep flow
processing on a per TEID base, so the port hashing should be base on the
TEID.

All together, I think adding an additional UDP source port argument to
the PDP context might be a better solution.

Andreas

> 
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> 
> found in code inspection.. compile tested only
> 
> 
> 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 bda0c64..ff1244b 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -564,6 +564,7 @@ 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;
> +	__be16 src_port;
> 	int err;
> 
> 	/* Ensure there is sufficient headroom. */
> @@ -572,6 +573,8 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct
> net_device *dev)
> 
> 	skb_reset_inner_headers(skb);
> 
> +	src_port = udp_flow_src_port(dev_net(dev), skb, 0, 0, true);
> +
> 	/* PDP context lookups in gtp_build_skb_*() need rcu read-side lock. */
> 	rcu_read_lock();
> 	switch (proto) {
> @@ -596,7 +599,7 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct
> net_device *dev)
> 				    pktinfo.iph->tos,
> 				    ip4_dst_hoplimit(&pktinfo.rt->dst),
> 				    0,
> -				    pktinfo.gtph_port, pktinfo.gtph_port,
> +				    src_port, pktinfo.gtph_port,
> 				    true, false);
> 		break;
> 	}
> --
> 2.3.7

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Or Gerlitz]

On Thu, Feb 16, 2017 at 11:58 PM, Andreas Schultz <aschultz@tpip.net> wrote:
> Hi Or,
> ----- On Feb 16, 2017, at 3:59 PM, Or Gerlitz ogerlitz@mellanox.com wrote:
>
>> Generate the source udp header according to the flow represented by
>> the packet we are encapsulating, as done for other udp tunnels. This
>> helps on the receiver side to apply RSS spreading.
>
> This might work for GTPv0-U, However, for GTPv1-U this could interfere
> with error handling in the user space control process when the UDP port
> extension  header is used in error indications.


in the document you posted there's this quote "The source IP and port
have no meaning and can change at any time" -- I assume it refers to
v0? can we identify in the kernel code that we're on v0 and have the
patch come into play?

> 3GPP TS 29.281 Rel 13, section 5.2.2.1 defines the UDP port extension and
> section 7.3.1 says that the UDP source port extension can be used to
> mitigate DOS attacks. This would IMHO imply that the user space control
> process needs to know the TEID to UDP source port mapping.

> The other question is, on what is this actually hashing. When I understand
> the code correctly, this will hash on the source/destination of the orignal
> flow. I would expect that a SGSN/SGW/eNodeB would like the keep flow
> processing on a per TEID base, so the port hashing should be base on the TEID.

is it possible for packets belonging to the same TCP session or UDP
"pseudo session" (given pair of src/dst ip/port) to be encapsulated
using different TEID?

hashing on the TEID imposes a harder requirement on the NIC HW vs.
just UDP based RSS.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Tom Herbert]

On Wed, Feb 22, 2017 at 1:29 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Thu, Feb 16, 2017 at 11:58 PM, Andreas Schultz <aschultz@tpip.net> wrote:
>> Hi Or,
>> ----- On Feb 16, 2017, at 3:59 PM, Or Gerlitz ogerlitz@mellanox.com wrote:
>>
>>> Generate the source udp header according to the flow represented by
>>> the packet we are encapsulating, as done for other udp tunnels. This
>>> helps on the receiver side to apply RSS spreading.
>>
>> This might work for GTPv0-U, However, for GTPv1-U this could interfere
>> with error handling in the user space control process when the UDP port
>> extension  header is used in error indications.
>
>
> in the document you posted there's this quote "The source IP and port
> have no meaning and can change at any time" -- I assume it refers to
> v0? can we identify in the kernel code that we're on v0 and have the
> patch come into play?
>
>> 3GPP TS 29.281 Rel 13, section 5.2.2.1 defines the UDP port extension and
>> section 7.3.1 says that the UDP source port extension can be used to
>> mitigate DOS attacks. This would IMHO imply that the user space control
>> process needs to know the TEID to UDP source port mapping.
>
>> The other question is, on what is this actually hashing. When I understand
>> the code correctly, this will hash on the source/destination of the orignal
>> flow. I would expect that a SGSN/SGW/eNodeB would like the keep flow
>> processing on a per TEID base, so the port hashing should be base on the TEID.
>
> is it possible for packets belonging to the same TCP session or UDP
> "pseudo session" (given pair of src/dst ip/port) to be encapsulated
> using different TEID?
>
> hashing on the TEID imposes a harder requirement on the NIC HW vs.
> just UDP based RSS.

This shouldn't be taken as a HW requirement and it's unlikely we'd add
explicit GTP support in flow_dissector. If we can't get entropy in the
UDP source port then IPv6 flow label is a potential alternative (so
that should be supported in NICs for RSS).

I'll also reiterate my previous point about the need for GTP testing--
in order for us to be able to evaluate the GTP datapath for things
like performance or how they withstand against DDOS we really need an
easy way to isolate the datapath.

Tom

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Andreas Schultz]

Hi Tom,

----- On Feb 22, 2017, at 10:47 PM, Tom Herbert tom@herbertland.com wrote:

> On Wed, Feb 22, 2017 at 1:29 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Thu, Feb 16, 2017 at 11:58 PM, Andreas Schultz <aschultz@tpip.net> wrote:
>>> Hi Or,
>>> ----- On Feb 16, 2017, at 3:59 PM, Or Gerlitz ogerlitz@mellanox.com wrote:
>>>
>>>> Generate the source udp header according to the flow represented by
>>>> the packet we are encapsulating, as done for other udp tunnels. This
>>>> helps on the receiver side to apply RSS spreading.
>>>
>>> This might work for GTPv0-U, However, for GTPv1-U this could interfere
>>> with error handling in the user space control process when the UDP port
>>> extension  header is used in error indications.
>>
>>
>> in the document you posted there's this quote "The source IP and port
>> have no meaning and can change at any time" -- I assume it refers to
>> v0? can we identify in the kernel code that we're on v0 and have the
>> patch come into play?
>>
>>> 3GPP TS 29.281 Rel 13, section 5.2.2.1 defines the UDP port extension and
>>> section 7.3.1 says that the UDP source port extension can be used to
>>> mitigate DOS attacks. This would IMHO imply that the user space control
>>> process needs to know the TEID to UDP source port mapping.
>>
>>> The other question is, on what is this actually hashing. When I understand
>>> the code correctly, this will hash on the source/destination of the orignal
>>> flow. I would expect that a SGSN/SGW/eNodeB would like the keep flow
>>> processing on a per TEID base, so the port hashing should be base on the TEID.
>>
>> is it possible for packets belonging to the same TCP session or UDP
>> "pseudo session" (given pair of src/dst ip/port) to be encapsulated
>> using different TEID?
>>
>> hashing on the TEID imposes a harder requirement on the NIC HW vs.
>> just UDP based RSS.
> 
> This shouldn't be taken as a HW requirement and it's unlikely we'd add
> explicit GTP support in flow_dissector. If we can't get entropy in the
> UDP source port then IPv6 flow label is a potential alternative (so
> that should be supported in NICs for RSS).
> 
> I'll also reiterate my previous point about the need for GTP testing--
> in order for us to be able to evaluate the GTP datapath for things
> like performance or how they withstand against DDOS we really need an
> easy way to isolate the datapath.

GTP as specified is very unsecure by definition. It is meant to be run
only on *private* mobile carrier and intra mobile carrier EPC networks.
Running it openly on the public internet would be extremly foolish.

There are some mechanisms in GTPv1-C on how to handle overload and
more extensive mechanisms in GTPv2-C for overload handling. The basic
guiding principle is to simply drop any traffic that it can't handle.

Anyhow, I havn't seen anything in 3GPP or GSMA documents that deals
with DDOS.

There are guidelines like the GSMA's IR.88 that describe how the intra
carrier roaming should work and what security measures should be
implemented.

Traffic coming in at Gi/SGi or form the UE could create a DDOS on tunnel.
However, on the UE side you still have the RAN (eNODE, SGSN, S-GW) or
an ePDG that has to apply QoS and thereby limit traffic. On the Gi/SGi
side side you have the PCEF that does the same.

So in a complete 3GPP node (GGSN, P-GW) that uses the GTP tunnel
implementation, malicious traffic should be blocked before it can reach
the tunnel.

And as I stated before, the GTP tunnel module is not supposed to be
use without any of those components. So the DDOS concern should not
be handled at the tunnel level.

Andreas

> 
> Tom

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Pablo Neira Ayuso]

On Thu, Feb 23, 2017 at 10:35:56AM +0100, Andreas Schultz wrote:
> ----- On Feb 22, 2017, at 10:47 PM, Tom Herbert tom@herbertland.com wrote:
[...]
> > This shouldn't be taken as a HW requirement and it's unlikely we'd add
> > explicit GTP support in flow_dissector. If we can't get entropy in the
> > UDP source port then IPv6 flow label is a potential alternative (so
> > that should be supported in NICs for RSS).
> > 
> > I'll also reiterate my previous point about the need for GTP testing--
> > in order for us to be able to evaluate the GTP datapath for things
> > like performance or how they withstand against DDOS we really need an
> > easy way to isolate the datapath.
> 
> GTP as specified is very unsecure by definition. It is meant to be run
> only on *private* mobile carrier and intra mobile carrier EPC networks.
> Running it openly on the public internet would be extremly foolish.
> 
> There are some mechanisms in GTPv1-C on how to handle overload and
> more extensive mechanisms in GTPv2-C for overload handling. The basic
> guiding principle is to simply drop any traffic that it can't handle.
> 
> Anyhow, I havn't seen anything in 3GPP or GSMA documents that deals
> with DDOS.
> 
> There are guidelines like the GSMA's IR.88 that describe how the intra
> carrier roaming should work and what security measures should be
> implemented.
> 
> Traffic coming in at Gi/SGi or form the UE could create a DDOS on tunnel.
> However, on the UE side you still have the RAN (eNODE, SGSN, S-GW) or
> an ePDG that has to apply QoS and thereby limit traffic. On the Gi/SGi
> side side you have the PCEF that does the same.
> 
> So in a complete 3GPP node (GGSN, P-GW) that uses the GTP tunnel
> implementation, malicious traffic should be blocked before it can reach
> the tunnel.
> 
> And as I stated before, the GTP tunnel module is not supposed to be
> use without any of those components. So the DDOS concern should not
> be handled at the tunnel level.

I think that Tom's point is that this tunnel driver will have to deal
with DDOS scenarios anyway, because reality is that you can't always
block it before it can reach the tunnel.

Or's patch helps us deal with this scenario.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Tom Herbert]

On Thu, Feb 23, 2017 at 6:00 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Feb 23, 2017 at 10:35:56AM +0100, Andreas Schultz wrote:
>> ----- On Feb 22, 2017, at 10:47 PM, Tom Herbert tom@herbertland.com wrote:
> [...]
>> > This shouldn't be taken as a HW requirement and it's unlikely we'd add
>> > explicit GTP support in flow_dissector. If we can't get entropy in the
>> > UDP source port then IPv6 flow label is a potential alternative (so
>> > that should be supported in NICs for RSS).
>> >
>> > I'll also reiterate my previous point about the need for GTP testing--
>> > in order for us to be able to evaluate the GTP datapath for things
>> > like performance or how they withstand against DDOS we really need an
>> > easy way to isolate the datapath.
>>
>> GTP as specified is very unsecure by definition. It is meant to be run
>> only on *private* mobile carrier and intra mobile carrier EPC networks.
>> Running it openly on the public internet would be extremly foolish.
>>
>> There are some mechanisms in GTPv1-C on how to handle overload and
>> more extensive mechanisms in GTPv2-C for overload handling. The basic
>> guiding principle is to simply drop any traffic that it can't handle.
>>
>> Anyhow, I havn't seen anything in 3GPP or GSMA documents that deals
>> with DDOS.
>>
>> There are guidelines like the GSMA's IR.88 that describe how the intra
>> carrier roaming should work and what security measures should be
>> implemented.
>>
>> Traffic coming in at Gi/SGi or form the UE could create a DDOS on tunnel.
>> However, on the UE side you still have the RAN (eNODE, SGSN, S-GW) or
>> an ePDG that has to apply QoS and thereby limit traffic. On the Gi/SGi
>> side side you have the PCEF that does the same.
>>
>> So in a complete 3GPP node (GGSN, P-GW) that uses the GTP tunnel
>> implementation, malicious traffic should be blocked before it can reach
>> the tunnel.
>>
>> And as I stated before, the GTP tunnel module is not supposed to be
>> use without any of those components. So the DDOS concern should not
>> be handled at the tunnel level.
>
> I think that Tom's point is that this tunnel driver will have to deal
> with DDOS scenarios anyway, because reality is that you can't always
> block it before it can reach the tunnel.
>
Right, if only we had a dime for every time someone thought their
perimeter security was sufficient only to be proven wrong!

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Harald Welte]

Hi Tom,

On Thu, Feb 23, 2017 at 08:35:13AM -0800, Tom Herbert wrote:
> On Thu, Feb 23, 2017 at 6:00 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Feb 23, 2017 at 10:35:56AM +0100, Andreas Schultz wrote:
> >> ----- On Feb 22, 2017, at 10:47 PM, Tom Herbert tom@herbertland.com wrote:
> >> So in a complete 3GPP node (GGSN, P-GW) that uses the GTP tunnel
> >> implementation, malicious traffic should be blocked before it can reach
> >> the tunnel.
> >>
> >> And as I stated before, the GTP tunnel module is not supposed to be
> >> use without any of those components. So the DDOS concern should not
> >> be handled at the tunnel level.
> >
> > I think that Tom's point is that this tunnel driver will have to deal
> > with DDOS scenarios anyway, because reality is that you can't always
> > block it before it can reach the tunnel.
> >
> Right, if only we had a dime for every time someone thought their
> perimeter security was sufficient only to be proven wrong!

Yes and no.  Welcome to the cellular world. Everything is designed in
a way that it assumes everyone can be trusted and that none of those
interfaces (except the radio interface) are ever exposed to the public.

There is really very little one can do to change that world.  It's like
the Internet in the early 1990ies, and they are reluctant to learn.  And
whenever a new system is designed (like the step from MAP to DIAMETER),
they make damn sure that all the security issues are inherited from the
previous standards to ensure interoperability ;)

I understand and support the motivation to design robust systsems even
in the presence of broken/ignorant specs, but I think this is one of the
situations where it is useless (and IMHO impossible) to do anything
about it.

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

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[David Miller]

From: Harald Welte <laforge@gnumonks.org>
Date: Thu, 23 Feb 2017 17:50:51 +0100

> I understand and support the motivation to design robust systsems even
> in the presence of broken/ignorant specs, but I think this is one of the
> situations where it is useless (and IMHO impossible) to do anything
> about it.

I think avoiding trying to do something reasonable about this and just
saying "this is how cellular networks are" is not acceptable.

We know how to properly strengthen tunnelling implementations in the
kernel against DDoS attachs, GTP can be treated similarly.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Pablo Neira Ayuso]

On Wed, Feb 22, 2017 at 01:47:17PM -0800, Tom Herbert wrote:
> On Wed, Feb 22, 2017 at 1:29 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > On Thu, Feb 16, 2017 at 11:58 PM, Andreas Schultz <aschultz@tpip.net> wrote:
> >> Hi Or,
> >> ----- On Feb 16, 2017, at 3:59 PM, Or Gerlitz ogerlitz@mellanox.com wrote:
> >>
> >>> Generate the source udp header according to the flow represented by
> >>> the packet we are encapsulating, as done for other udp tunnels. This
> >>> helps on the receiver side to apply RSS spreading.
> >>
> >> This might work for GTPv0-U, However, for GTPv1-U this could interfere
> >> with error handling in the user space control process when the UDP port
> >> extension  header is used in error indications.
> >
> >
> > in the document you posted there's this quote "The source IP and port
> > have no meaning and can change at any time" -- I assume it refers to
> > v0? can we identify in the kernel code that we're on v0 and have the
> > patch come into play?
> >
> >> 3GPP TS 29.281 Rel 13, section 5.2.2.1 defines the UDP port extension and
> >> section 7.3.1 says that the UDP source port extension can be used to
> >> mitigate DOS attacks. This would IMHO imply that the user space control
> >> process needs to know the TEID to UDP source port mapping.
> >
> >> The other question is, on what is this actually hashing. When I understand
> >> the code correctly, this will hash on the source/destination of the orignal
> >> flow. I would expect that a SGSN/SGW/eNodeB would like the keep flow
> >> processing on a per TEID base, so the port hashing should be base on the TEID.
> >
> > is it possible for packets belonging to the same TCP session or UDP
> > "pseudo session" (given pair of src/dst ip/port) to be encapsulated
> > using different TEID?
> >
> > hashing on the TEID imposes a harder requirement on the NIC HW vs.
> > just UDP based RSS.
> 
> This shouldn't be taken as a HW requirement and it's unlikely we'd add
> explicit GTP support in flow_dissector. If we can't get entropy in the
> UDP source port then IPv6 flow label is a potential alternative (so
> that should be supported in NICs for RSS).

According to specs, section 4.4.2.3 Encapsulated T-PDU, TS 29.281.

"The UDP Source Port is a locally allocated port number at the sending
GTP-U entity."

Older specs that refer to GTP-U such as TS 09.60 and TS 29.060 also
state the same.

So Or patch looks fine to me.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Or Gerlitz]

On 2/23/2017 3:49 PM, Pablo Neira Ayuso wrote:
> "The UDP Source Port is a locally allocated port number at the sending
> GTP-U entity."

and the proposed patch goes in that direction, agree?

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Andreas Schultz]

----- On Feb 23, 2017, at 2:49 PM, pablo pablo@netfilter.org wrote:

> On Wed, Feb 22, 2017 at 01:47:17PM -0800, Tom Herbert wrote:
>> On Wed, Feb 22, 2017 at 1:29 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> > On Thu, Feb 16, 2017 at 11:58 PM, Andreas Schultz <aschultz@tpip.net> wrote:
>> >> Hi Or,
>> >> ----- On Feb 16, 2017, at 3:59 PM, Or Gerlitz ogerlitz@mellanox.com wrote:
>> >>
>> >>> Generate the source udp header according to the flow represented by
>> >>> the packet we are encapsulating, as done for other udp tunnels. This
>> >>> helps on the receiver side to apply RSS spreading.
>> >>
>> >> This might work for GTPv0-U, However, for GTPv1-U this could interfere
>> >> with error handling in the user space control process when the UDP port
>> >> extension  header is used in error indications.
>> >
>> >
>> > in the document you posted there's this quote "The source IP and port
>> > have no meaning and can change at any time" -- I assume it refers to
>> > v0? can we identify in the kernel code that we're on v0 and have the
>> > patch come into play?
>> >
>> >> 3GPP TS 29.281 Rel 13, section 5.2.2.1 defines the UDP port extension and
>> >> section 7.3.1 says that the UDP source port extension can be used to
>> >> mitigate DOS attacks. This would IMHO imply that the user space control
>> >> process needs to know the TEID to UDP source port mapping.
>> >
>> >> The other question is, on what is this actually hashing. When I understand
>> >> the code correctly, this will hash on the source/destination of the orignal
>> >> flow. I would expect that a SGSN/SGW/eNodeB would like the keep flow
>> >> processing on a per TEID base, so the port hashing should be base on the TEID.
>> >
>> > is it possible for packets belonging to the same TCP session or UDP
>> > "pseudo session" (given pair of src/dst ip/port) to be encapsulated
>> > using different TEID?
>> >
>> > hashing on the TEID imposes a harder requirement on the NIC HW vs.
>> > just UDP based RSS.
>> 
>> This shouldn't be taken as a HW requirement and it's unlikely we'd add
>> explicit GTP support in flow_dissector. If we can't get entropy in the
>> UDP source port then IPv6 flow label is a potential alternative (so
>> that should be supported in NICs for RSS).
> 
> According to specs, section 4.4.2.3 Encapsulated T-PDU, TS 29.281.
> 
> "The UDP Source Port is a locally allocated port number at the sending
> GTP-U entity."
> 
> Older specs that refer to GTP-U such as TS 09.60 and TS 29.060 also
> state the same.

It is absolutely valid the choose any sending port you want. I only
think you should know which port you did send on.

TS 29.281, Sect. 5.2.2.1 defines the UDP port extension to be used
in error indications. It provides the UDP source port of a G-PDU
that triggered an error.

If the send side does not know which port it uses to send, how
can it use this indication to correlate an error? That's the reason
I thought it would be better to add the UDP source port to the
PDP context and allow the control path to assign the source port
on context creation.

Of course, this header is optional and the receiving side is not
required to use it.

About the RSS spreading in the receive side, I would think that
a receiver would prefer to process all packets that belong to a
give TEID with the same receive instance. So keeping the UDP
source port for a given TEID stable would be beneficial. As far
as I understand it, the hash used in the patch uses the source
and destination values from the original flow. This would mean
that GTP packets that belong to the same TEID would end up with
different UDP source ports.

So what about this as a compromise, we dd a UDP source port field
to the PDP context, it defaults to 0 (zero), the control instance
can optionally initialize this field, when we hit the xmit code
and the port is non zero, use that value, if it is zero use the hash?

Regards
Andreas

> 
> So Or patch looks fine to me.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Pablo Neira Ayuso]

On Thu, Feb 23, 2017 at 03:21:13PM +0100, Andreas Schultz wrote:
> ----- On Feb 23, 2017, at 2:49 PM, pablo pablo@netfilter.org wrote:
[...]
> > According to specs, section 4.4.2.3 Encapsulated T-PDU, TS 29.281.
> > 
> > "The UDP Source Port is a locally allocated port number at the sending
> > GTP-U entity."
> > 
> > Older specs that refer to GTP-U such as TS 09.60 and TS 29.060 also
> > state the same.
> 
> It is absolutely valid the choose any sending port you want. I only
> think you should know which port you did send on.
> 
> TS 29.281, Sect. 5.2.2.1 defines the UDP port extension to be used
> in error indications. It provides the UDP source port of a G-PDU
> that triggered an error.
> 
> If the send side does not know which port it uses to send, how
> can it use this indication to correlate an error? That's the reason
> I thought it would be better to add the UDP source port to the
> PDP context and allow the control path to assign the source port
> on context creation.
> 
> Of course, this header is optional and the receiving side is not
> required to use it.
> 
> About the RSS spreading in the receive side, I would think that
> a receiver would prefer to process all packets that belong to a
> give TEID with the same receive instance. So keeping the UDP
> source port for a given TEID stable would be beneficial. As far
> as I understand it, the hash used in the patch uses the source
> and destination values from the original flow. This would mean
> that GTP packets that belong to the same TEID would end up with
> different UDP source ports.

The receiver needs to scale up, and that happens if packets are
distributed between CPUs in a way that make sense. I don't think it
makes sense to pass packets that belong to the same tunnel to the same
CPU, this is exactly the scenario that makes DDOS easier to trigger.

> So what about this as a compromise, we dd a UDP source port field
> to the PDP context, it defaults to 0 (zero), the control instance
> can optionally initialize this field, when we hit the xmit code
> and the port is non zero, use that value, if it is zero use the hash?

You want to use this for your VRF concept? I would like that you take
the time to explain us your usecases. How are you going to use this
mapping between tunnels and UDP source ports? An explaination would be
better than searching at some optional (corner case) extension in the
specs whose usage is questionable.

In any case, I think we want a good default behaviour, and I think
Or's patch provides it.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Andreas Schultz]

----- On Feb 23, 2017, at 5:42 PM, pablo pablo@netfilter.org wrote:

> On Thu, Feb 23, 2017 at 03:21:13PM +0100, Andreas Schultz wrote:
>> ----- On Feb 23, 2017, at 2:49 PM, pablo pablo@netfilter.org wrote:
> [...]
>> > According to specs, section 4.4.2.3 Encapsulated T-PDU, TS 29.281.
>> > 
>> > "The UDP Source Port is a locally allocated port number at the sending
>> > GTP-U entity."
>> > 
>> > Older specs that refer to GTP-U such as TS 09.60 and TS 29.060 also
>> > state the same.
>> 
>> It is absolutely valid the choose any sending port you want. I only
>> think you should know which port you did send on.
>> 
>> TS 29.281, Sect. 5.2.2.1 defines the UDP port extension to be used
>> in error indications. It provides the UDP source port of a G-PDU
>> that triggered an error.
>> 
>> If the send side does not know which port it uses to send, how
>> can it use this indication to correlate an error? That's the reason
>> I thought it would be better to add the UDP source port to the
>> PDP context and allow the control path to assign the source port
>> on context creation.
>> 
>> Of course, this header is optional and the receiving side is not
>> required to use it.
>> 
>> About the RSS spreading in the receive side, I would think that
>> a receiver would prefer to process all packets that belong to a
>> give TEID with the same receive instance. So keeping the UDP
>> source port for a given TEID stable would be beneficial. As far
>> as I understand it, the hash used in the patch uses the source
>> and destination values from the original flow. This would mean
>> that GTP packets that belong to the same TEID would end up with
>> different UDP source ports.
> 
> The receiver needs to scale up, and that happens if packets are
> distributed between CPUs in a way that make sense. I don't think it
> makes sense to pass packets that belong to the same tunnel to the same
> CPU, this is exactly the scenario that makes DDOS easier to trigger.

When we are talking about the xmit path, then currently none of the
receivers we are talking to is going to be Linux and we have no
idea how they will behave nor do we have any influence on them. Do
we really need to make assumptions about other vendors implementations?

Traces on live GRX networks show that about 90% of the SGSN/S-GW
that would talk to us always use the default GTP-U port as source
port. Some multi chassis GSN's seem to assign source port ranges to
chassis, but that has nothing todo with DDOS protection.

So, even when do the randomization on TX, it won't help our receiver
scale up. We have to deal what the other (100% non Linux side is
going to throw at us).

>> So what about this as a compromise, we dd a UDP source port field
>> to the PDP context, it defaults to 0 (zero), the control instance
>> can optionally initialize this field, when we hit the xmit code
>> and the port is non zero, use that value, if it is zero use the hash?
> 
> You want to use this for your VRF concept? I would like that you take
> the time to explain us your usecases.

I only want the normal multi APN per GGSN/P-GW setup that every mobile
carrier on this world is running on the big commercial vendor boxes, nothing
more and nothing less.

I have tried to explain this multiple times already, but it seems I
failed every time to put it in intelligible form. The last attempt
was here: http://marc.info/?l=linux-netdev&m=148700022003294&w=2

> How are you going to use this mapping between tunnels and UDP source ports?

There is no mapping between tunnels and UDP source ports. UDP source
ports do not matter to the receiving side at all in GTP tunnels. There
is one error handling case that can benefit from knowing the source port
and I think that case shouldn't be discarded lightly.

A GTP-U tunnel is only defined by its destination IP, destination UDP port
(which is constant 2152) and it's TEID. This also means that a GTP tunnel
is a one-way construct. Only the GTP-C instance knows that a PDP context
is actually two GTP-U tunnels, one in each direction.

We had the discussion if the source IP does play a role in this. The
3GPP specifications do not make a 100% clear statement for GTP-U on that.
So the case can be made that the GTP-U tunnel pair should be symmetric
(the destination IP of one tunnel is the source of the other). There
are various other statements in the documents that imply that this is
not always the case.

I thought we had come to the conclusion (with Harald) to make the reverse
GSN peer filter an option. Then every control path implementation has
the choice to use it or not. The same should IMHO apply to the source port
randomization.

For GTP-C the situation is clear, the control instance has to accept
traffic of a given TEID from any source, otherwise certain handover
procedures would not work.

> An explaination would be better than searching at some optional
> (corner case) extension in the specs whose usage is questionable.
> 
> In any case, I think we want a good default behaviour, and I think
> Or's patch provides it.

Andreas

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[David Miller]

From: Andreas Schultz <aschultz@tpip.net>
Date: Thu, 23 Feb 2017 18:19:16 +0100 (CET)

> When we are talking about the xmit path, then currently none of the
> receivers we are talking to is going to be Linux and we have no
> idea how they will behave nor do we have any influence on them. Do
> we really need to make assumptions about other vendors implementations?
> 
> Traces on live GRX networks show that about 90% of the SGSN/S-GW
> that would talk to us always use the default GTP-U port as source
> port. Some multi chassis GSN's seem to assign source port ranges to
> chassis, but that has nothing todo with DDOS protection.

This is exactly what other UDP tunnel implementations did before
flow separation was prevelant.

I don't see the point of any of this discussion discouraging the
enablement of proper flow separation.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Or Gerlitz]

On Thu, Feb 23, 2017 at 7:54 PM, David Miller <davem@davemloft.net> wrote:
> From: Andreas Schultz <aschultz@tpip.net>
> Date: Thu, 23 Feb 2017 18:19:16 +0100 (CET)
>
>> When we are talking about the xmit path, then currently none of the
>> receivers we are talking to is going to be Linux and we have no
>> idea how they will behave nor do we have any influence on them. Do
>> we really need to make assumptions about other vendors implementations?
>>
>> Traces on live GRX networks show that about 90% of the SGSN/S-GW
>> that would talk to us always use the default GTP-U port as source
>> port. Some multi chassis GSN's seem to assign source port ranges to
>> chassis, but that has nothing todo with DDOS protection.
>
> This is exactly what other UDP tunnel implementations did before
> flow separation was prevelant.
>
> I don't see the point of any of this discussion discouraging the
> enablement of proper flow separation.

Hi Dave,

So where do we  go from here? should I resubmit the patch?

Or.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Pablo Neira Ayuso]

On Wed, Mar 15, 2017 at 06:14:02PM +0200, Or Gerlitz wrote:
> On Thu, Feb 23, 2017 at 7:54 PM, David Miller <davem@davemloft.net> wrote:
> > From: Andreas Schultz <aschultz@tpip.net>
> > Date: Thu, 23 Feb 2017 18:19:16 +0100 (CET)
> >
> >> When we are talking about the xmit path, then currently none of the
> >> receivers we are talking to is going to be Linux and we have no
> >> idea how they will behave nor do we have any influence on them. Do
> >> we really need to make assumptions about other vendors implementations?
> >>
> >> Traces on live GRX networks show that about 90% of the SGSN/S-GW
> >> that would talk to us always use the default GTP-U port as source
> >> port. Some multi chassis GSN's seem to assign source port ranges to
> >> chassis, but that has nothing todo with DDOS protection.
> >
> > This is exactly what other UDP tunnel implementations did before
> > flow separation was prevelant.
> >
> > I don't see the point of any of this discussion discouraging the
> > enablement of proper flow separation.
> 
> Hi Dave,
> 
> So where do we  go from here? should I resubmit the patch?

IIRC this patch didn't get into the merge window in time, so it's
reasonable to resubmit I think.

You may want to add this to the patch:

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks Or.

Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash

[Tom Herbert]

On Thu, Feb 23, 2017 at 6:21 AM, Andreas Schultz <aschultz@tpip.net> wrote:
> ----- On Feb 23, 2017, at 2:49 PM, pablo pablo@netfilter.org wrote:
>
>> On Wed, Feb 22, 2017 at 01:47:17PM -0800, Tom Herbert wrote:
>>> On Wed, Feb 22, 2017 at 1:29 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>> > On Thu, Feb 16, 2017 at 11:58 PM, Andreas Schultz <aschultz@tpip.net> wrote:
>>> >> Hi Or,
>>> >> ----- On Feb 16, 2017, at 3:59 PM, Or Gerlitz ogerlitz@mellanox.com wrote:
>>> >>
>>> >>> Generate the source udp header according to the flow represented by
>>> >>> the packet we are encapsulating, as done for other udp tunnels. This
>>> >>> helps on the receiver side to apply RSS spreading.
>>> >>
>>> >> This might work for GTPv0-U, However, for GTPv1-U this could interfere
>>> >> with error handling in the user space control process when the UDP port
>>> >> extension  header is used in error indications.
>>> >
>>> >
>>> > in the document you posted there's this quote "The source IP and port
>>> > have no meaning and can change at any time" -- I assume it refers to
>>> > v0? can we identify in the kernel code that we're on v0 and have the
>>> > patch come into play?
>>> >
>>> >> 3GPP TS 29.281 Rel 13, section 5.2.2.1 defines the UDP port extension and
>>> >> section 7.3.1 says that the UDP source port extension can be used to
>>> >> mitigate DOS attacks. This would IMHO imply that the user space control
>>> >> process needs to know the TEID to UDP source port mapping.
>>> >
>>> >> The other question is, on what is this actually hashing. When I understand
>>> >> the code correctly, this will hash on the source/destination of the orignal
>>> >> flow. I would expect that a SGSN/SGW/eNodeB would like the keep flow
>>> >> processing on a per TEID base, so the port hashing should be base on the TEID.
>>> >
>>> > is it possible for packets belonging to the same TCP session or UDP
>>> > "pseudo session" (given pair of src/dst ip/port) to be encapsulated
>>> > using different TEID?
>>> >
>>> > hashing on the TEID imposes a harder requirement on the NIC HW vs.
>>> > just UDP based RSS.
>>>
>>> This shouldn't be taken as a HW requirement and it's unlikely we'd add
>>> explicit GTP support in flow_dissector. If we can't get entropy in the
>>> UDP source port then IPv6 flow label is a potential alternative (so
>>> that should be supported in NICs for RSS).
>>
>> According to specs, section 4.4.2.3 Encapsulated T-PDU, TS 29.281.
>>
>> "The UDP Source Port is a locally allocated port number at the sending
>> GTP-U entity."
>>
>> Older specs that refer to GTP-U such as TS 09.60 and TS 29.060 also
>> state the same.
>
> It is absolutely valid the choose any sending port you want. I only
> think you should know which port you did send on.
>
> TS 29.281, Sect. 5.2.2.1 defines the UDP port extension to be used
> in error indications. It provides the UDP source port of a G-PDU
> that triggered an error.
>
> If the send side does not know which port it uses to send, how
> can it use this indication to correlate an error? That's the reason
> I thought it would be better to add the UDP source port to the
> PDP context and allow the control path to assign the source port
> on context creation.
>
> Of course, this header is optional and the receiving side is not
> required to use it.
>
> About the RSS spreading in the receive side, I would think that
> a receiver would prefer to process all packets that belong to a
> give TEID with the same receive instance. So keeping the UDP
> source port for a given TEID stable would be beneficial. As far
> as I understand it, the hash used in the patch uses the source
> and destination values from the original flow. This would mean
> that GTP packets that belong to the same TEID would end up with
> different UDP source ports.
>
> So what about this as a compromise, we dd a UDP source port field
> to the PDP context, it defaults to 0 (zero), the control instance
> can optionally initialize this field, when we hit the xmit code
> and the port is non zero, use that value, if it is zero use the hash?
>
Sounds reasonable. It is typical in other UDP encapsulations to allow
the UDP sort port to be configured like this for static tunnels at
least. If you need to set the port based on a hash over atypical
values (like TEID) I suggest you still do jhash with randomized keys
to minimize information leakage.

> Regards
> Andreas
>
>>
>> So Or patch looks fine to me.