| Message ID | 20201211122612.869225-8-jonas@norrbonn.se (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | gtp: IPv6 support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | CHECK: Alignment should match open parenthesis |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Fri, Dec 11, 2020 at 4:29 AM Jonas Bonn <jonas@norrbonn.se> wrote: > > All GTP traffic is currently sent from the same source port. This makes > everything look like one big flow which is difficult to balance across > network resources. > > From 3GPP TS 29.281: > "...the UDP Source Port or the Flow Label field... should be set dynamically > by the sending GTP-U entity to help balancing the load in the transport > network." > > Signed-off-by: Jonas Bonn <jonas@norrbonn.se> > --- > drivers/net/gtp.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c > index 4a3a52970856..236ebbcb37bf 100644 > --- a/drivers/net/gtp.c > +++ b/drivers/net/gtp.c > @@ -477,7 +477,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev) > __be32 saddr; > struct iphdr *iph; > int headroom; > - __be16 port; > + __be16 sport, port; > int r; > > /* Read the IP destination address and resolve the PDP context. > @@ -527,6 +527,10 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev) > return -EMSGSIZE; > } > > + sport = udp_flow_src_port(sock_net(pctx->sk), skb, > + 0, USHRT_MAX, > + true); > + why use_eth is true for this is L3 GTP devices, Am missing something? > /* Ensure there is sufficient headroom. */ > r = skb_cow_head(skb, headroom); > if (unlikely(r)) > @@ -545,7 +549,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev) > iph->tos, > ip4_dst_hoplimit(&rt->dst), > 0, > - port, port, > + sport, port, > !net_eq(sock_net(pctx->sk), > dev_net(pctx->dev)), > false); > -- > 2.27.0 >
On 12/12/2020 06:35, Pravin Shelar wrote: > On Fri, Dec 11, 2020 at 4:29 AM Jonas Bonn <jonas@norrbonn.se> wrote: >> /* Read the IP destination address and resolve the PDP context. >> @@ -527,6 +527,10 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev) >> return -EMSGSIZE; >> } >> >> + sport = udp_flow_src_port(sock_net(pctx->sk), skb, >> + 0, USHRT_MAX, >> + true); >> + > why use_eth is true for this is L3 GTP devices, Am missing something? No, you are right. Will fix. /Jonas
Hi Jonas, On Fri, Dec 11, 2020 at 01:26:07PM +0100, Jonas Bonn wrote: > From 3GPP TS 29.281: > "...the UDP Source Port or the Flow Label field... should be set dynamically > by the sending GTP-U entity to help balancing the load in the transport > network." You unfortuantely didn't specifiy which 3GPP release you are referring to. At least in V15.7.0 (2020-01) Release 15 I can only find: "For the messages described below, the UDP Source Port (except as specified for the Echo Response message) may be allocated either statically or dynamically by the sending GTP-U entity. NOTE: Dynamic allocation of the UDP source port can help balancing the load in the network, depending on network deployments and network node implementations." For GTPv0, TS 29.060 states: "The UDP Source Port is a locally allocated port number at the sending GSN/RNC." unfortuantely it doesn't say if it's a locally allocated number globally for that entire GSN/RNC, or it's dynamic per flow or per packet. As I'm aware of a lot of very tight packet filtering between GSNs, I would probably not go for fully dynamic source port allocation without some kind of way how the user (GTP-control instance) being able to decide on that policy.
Hi Harald, On 12/12/2020 11:07, Harald Welte wrote: > Hi Jonas, > > On Fri, Dec 11, 2020 at 01:26:07PM +0100, Jonas Bonn wrote: >> From 3GPP TS 29.281: >> "...the UDP Source Port or the Flow Label field... should be set dynamically >> by the sending GTP-U entity to help balancing the load in the transport >> network." > > You unfortuantely didn't specifiy which 3GPP release you are referring to. > Sorry about that. I'm looking at v16.1.0. > At least in V15.7.0 (2020-01) Release 15 I can only find: > > "For the messages described below, the UDP Source Port (except as > specified for the Echo Response message) may be allocated either > statically or dynamically by the sending GTP-U entity. NOTE: Dynamic > allocation of the UDP source port can help balancing the load in the > network, depending on network deployments and network node > implementations." > > For GTPv0, TS 29.060 states: > > "The UDP Source Port is a locally allocated port number at the sending > GSN/RNC." > > unfortuantely it doesn't say if it's a locally allocated number globally > for that entire GSN/RNC, or it's dynamic per flow or per packet. I could interpret that to mean that the receiving end shouldn't be too bothered about what port a packet happens to come from... That said, dynamic per flow is almost certainly what we want here as this is mostly about being able to trigger receive side scaling for network cards that can't look at inner headers. > > As I'm aware of a lot of very tight packet filtering between GSNs, > I would probably not go for fully dynamic source port allocation > without some kind of way how the user (GTP-control instance) being > able to decide on that policy. > Sure... sounds reasonable. A flag on the link setup indicating dynamic source port yes/no should be sufficient, right? /Jonas
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 4a3a52970856..236ebbcb37bf 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -477,7 +477,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev) __be32 saddr; struct iphdr *iph; int headroom; - __be16 port; + __be16 sport, port; int r; /* Read the IP destination address and resolve the PDP context. @@ -527,6 +527,10 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev) return -EMSGSIZE; } + sport = udp_flow_src_port(sock_net(pctx->sk), skb, + 0, USHRT_MAX, + true); + /* Ensure there is sufficient headroom. */ r = skb_cow_head(skb, headroom); if (unlikely(r)) @@ -545,7 +549,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev) iph->tos, ip4_dst_hoplimit(&rt->dst), 0, - port, port, + sport, port, !net_eq(sock_net(pctx->sk), dev_net(pctx->dev)), false);
All GTP traffic is currently sent from the same source port. This makes everything look like one big flow which is difficult to balance across network resources. From 3GPP TS 29.281: "...the UDP Source Port or the Flow Label field... should be set dynamically by the sending GTP-U entity to help balancing the load in the transport network." Signed-off-by: Jonas Bonn <jonas@norrbonn.se> --- drivers/net/gtp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)