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

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 16 Feb 2017 22:58:11 +0100 (CET)
From:   Andreas Schultz <aschultz@...p.net>
To:     Or Gerlitz <ogerlitz@...lanox.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        pablo <pablo@...filter.org>, laforge <laforge@...monks.org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net/gtp: Add udp source port generation
 according to flow hash

Hi Or,

----- On Feb 16, 2017, at 3:59 PM, Or Gerlitz ogerlitz@...lanox.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@...lanox.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