* [PATCH net-next] net/gtp: Add udp source port generation according to flow hash @ 2017-02-16 14:59 Or Gerlitz 2017-02-16 21:58 ` Andreas Schultz 0 siblings, 1 reply; 18+ messages in thread From: Or Gerlitz @ 2017-02-16 14:59 UTC (permalink / raw) To: David S. Miller Cc: Jamal Hadi Salim, Pablo Neira Ayuso, laforge, netdev, 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-16 14:59 [PATCH net-next] net/gtp: Add udp source port generation according to flow hash Or Gerlitz @ 2017-02-16 21:58 ` Andreas Schultz 2017-02-22 21:29 ` Or Gerlitz 0 siblings, 1 reply; 18+ messages in thread From: Andreas Schultz @ 2017-02-16 21:58 UTC (permalink / raw) To: Or Gerlitz; +Cc: David S. Miller, Jamal Hadi Salim, pablo, laforge, netdev 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-16 21:58 ` Andreas Schultz @ 2017-02-22 21:29 ` Or Gerlitz 2017-02-22 21:47 ` Tom Herbert 0 siblings, 1 reply; 18+ messages in thread From: Or Gerlitz @ 2017-02-22 21:29 UTC (permalink / raw) To: Andreas Schultz Cc: Or Gerlitz, David S. Miller, Jamal Hadi Salim, pablo, laforge, netdev 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-22 21:29 ` Or Gerlitz @ 2017-02-22 21:47 ` Tom Herbert 2017-02-23 9:35 ` Andreas Schultz 2017-02-23 13:49 ` Pablo Neira Ayuso 0 siblings, 2 replies; 18+ messages in thread From: Tom Herbert @ 2017-02-22 21:47 UTC (permalink / raw) To: Or Gerlitz Cc: Andreas Schultz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, pablo, laforge, netdev 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-22 21:47 ` Tom Herbert @ 2017-02-23 9:35 ` Andreas Schultz 2017-02-23 14:00 ` Pablo Neira Ayuso 2017-02-23 13:49 ` Pablo Neira Ayuso 1 sibling, 1 reply; 18+ messages in thread From: Andreas Schultz @ 2017-02-23 9:35 UTC (permalink / raw) To: Tom Herbert Cc: Or Gerlitz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, pablo, laforge, netdev, timo lindhorst 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 9:35 ` Andreas Schultz @ 2017-02-23 14:00 ` Pablo Neira Ayuso 2017-02-23 16:35 ` Tom Herbert 0 siblings, 1 reply; 18+ messages in thread From: Pablo Neira Ayuso @ 2017-02-23 14:00 UTC (permalink / raw) To: Andreas Schultz Cc: Tom Herbert, Or Gerlitz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, laforge, netdev, timo lindhorst 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 14:00 ` Pablo Neira Ayuso @ 2017-02-23 16:35 ` Tom Herbert 2017-02-23 16:50 ` Harald Welte 0 siblings, 1 reply; 18+ messages in thread From: Tom Herbert @ 2017-02-23 16:35 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: Andreas Schultz, Or Gerlitz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, laforge, netdev, timo lindhorst 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! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 16:35 ` Tom Herbert @ 2017-02-23 16:50 ` Harald Welte 2017-02-23 17:01 ` David Miller 0 siblings, 1 reply; 18+ messages in thread From: Harald Welte @ 2017-02-23 16:50 UTC (permalink / raw) To: Tom Herbert Cc: Pablo Neira Ayuso, Andreas Schultz, Or Gerlitz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, netdev, timo lindhorst 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) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 16:50 ` Harald Welte @ 2017-02-23 17:01 ` David Miller 0 siblings, 0 replies; 18+ messages in thread From: David Miller @ 2017-02-23 17:01 UTC (permalink / raw) To: laforge Cc: tom, pablo, aschultz, gerlitz.or, ogerlitz, jhs, netdev, timo.lindhorst 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-22 21:47 ` Tom Herbert 2017-02-23 9:35 ` Andreas Schultz @ 2017-02-23 13:49 ` Pablo Neira Ayuso 2017-02-23 13:58 ` Or Gerlitz 2017-02-23 14:21 ` Andreas Schultz 1 sibling, 2 replies; 18+ messages in thread From: Pablo Neira Ayuso @ 2017-02-23 13:49 UTC (permalink / raw) To: Tom Herbert Cc: Or Gerlitz, Andreas Schultz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, laforge, netdev 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 13:49 ` Pablo Neira Ayuso @ 2017-02-23 13:58 ` Or Gerlitz 2017-02-23 14:21 ` Andreas Schultz 1 sibling, 0 replies; 18+ messages in thread From: Or Gerlitz @ 2017-02-23 13:58 UTC (permalink / raw) To: Pablo Neira Ayuso, Tom Herbert Cc: Or Gerlitz, Andreas Schultz, David S. Miller, Jamal Hadi Salim, laforge, netdev 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? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 13:49 ` Pablo Neira Ayuso 2017-02-23 13:58 ` Or Gerlitz @ 2017-02-23 14:21 ` Andreas Schultz 2017-02-23 16:42 ` Pablo Neira Ayuso 2017-02-23 16:42 ` Tom Herbert 1 sibling, 2 replies; 18+ messages in thread From: Andreas Schultz @ 2017-02-23 14:21 UTC (permalink / raw) To: pablo Cc: Tom Herbert, Or Gerlitz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, laforge, netdev ----- 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 14:21 ` Andreas Schultz @ 2017-02-23 16:42 ` Pablo Neira Ayuso 2017-02-23 17:19 ` Andreas Schultz 2017-02-23 16:42 ` Tom Herbert 1 sibling, 1 reply; 18+ messages in thread From: Pablo Neira Ayuso @ 2017-02-23 16:42 UTC (permalink / raw) To: Andreas Schultz Cc: Tom Herbert, Or Gerlitz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, laforge, netdev 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 16:42 ` Pablo Neira Ayuso @ 2017-02-23 17:19 ` Andreas Schultz 2017-02-23 17:54 ` David Miller 0 siblings, 1 reply; 18+ messages in thread From: Andreas Schultz @ 2017-02-23 17:19 UTC (permalink / raw) To: pablo Cc: Tom Herbert, Or Gerlitz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, laforge, netdev ----- 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 17:19 ` Andreas Schultz @ 2017-02-23 17:54 ` David Miller 2017-03-15 16:14 ` Or Gerlitz 0 siblings, 1 reply; 18+ messages in thread From: David Miller @ 2017-02-23 17:54 UTC (permalink / raw) To: aschultz; +Cc: pablo, tom, gerlitz.or, ogerlitz, jhs, laforge, netdev 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 17:54 ` David Miller @ 2017-03-15 16:14 ` Or Gerlitz 2017-03-15 16:25 ` Pablo Neira Ayuso 0 siblings, 1 reply; 18+ messages in thread From: Or Gerlitz @ 2017-03-15 16:14 UTC (permalink / raw) To: David Miller Cc: Andreas Schultz, Pablo Neira Ayuso, Tom Herbert, Or Gerlitz, Jamal Hadi Salim, laforge, Linux Netdev List 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-03-15 16:14 ` Or Gerlitz @ 2017-03-15 16:25 ` Pablo Neira Ayuso 0 siblings, 0 replies; 18+ messages in thread From: Pablo Neira Ayuso @ 2017-03-15 16:25 UTC (permalink / raw) To: Or Gerlitz Cc: David Miller, Andreas Schultz, Tom Herbert, Or Gerlitz, Jamal Hadi Salim, laforge, Linux Netdev List 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net/gtp: Add udp source port generation according to flow hash 2017-02-23 14:21 ` Andreas Schultz 2017-02-23 16:42 ` Pablo Neira Ayuso @ 2017-02-23 16:42 ` Tom Herbert 1 sibling, 0 replies; 18+ messages in thread From: Tom Herbert @ 2017-02-23 16:42 UTC (permalink / raw) To: Andreas Schultz Cc: pablo, Or Gerlitz, Or Gerlitz, David S. Miller, Jamal Hadi Salim, laforge, netdev 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-03-15 16:25 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-16 14:59 [PATCH net-next] net/gtp: Add udp source port generation according to flow hash Or Gerlitz 2017-02-16 21:58 ` Andreas Schultz 2017-02-22 21:29 ` Or Gerlitz 2017-02-22 21:47 ` Tom Herbert 2017-02-23 9:35 ` Andreas Schultz 2017-02-23 14:00 ` Pablo Neira Ayuso 2017-02-23 16:35 ` Tom Herbert 2017-02-23 16:50 ` Harald Welte 2017-02-23 17:01 ` David Miller 2017-02-23 13:49 ` Pablo Neira Ayuso 2017-02-23 13:58 ` Or Gerlitz 2017-02-23 14:21 ` Andreas Schultz 2017-02-23 16:42 ` Pablo Neira Ayuso 2017-02-23 17:19 ` Andreas Schultz 2017-02-23 17:54 ` David Miller 2017-03-15 16:14 ` Or Gerlitz 2017-03-15 16:25 ` Pablo Neira Ayuso 2017-02-23 16:42 ` Tom Herbert
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.