diff mbox series

[net-next,v2,08/12] gtp: set dev features to enable GSO

Message ID 20201211122612.869225-9-jonas@norrbonn.se (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series gtp: IPv6 support | expand

Checks

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 WARNING: Missing commit description - Add an appropriate one
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Jonas Bonn Dec. 11, 2020, 12:26 p.m. UTC 
Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
---
 drivers/net/gtp.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Pravin Shelar Dec. 12, 2020, 5:31 a.m. UTC | #1 
On Fri, Dec 11, 2020 at 4:28 AM Jonas Bonn <jonas@norrbonn.se> wrote:
>
> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
> ---
>  drivers/net/gtp.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> index 236ebbcb37bf..7bbeec173113 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -536,7 +536,11 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
>         if (unlikely(r))
>                 goto err_rt;
>
> -       skb_reset_inner_headers(skb);
> +       r = udp_tunnel_handle_offloads(skb, true);
> +       if (unlikely(r))
> +               goto err_rt;
> +
> +       skb_set_inner_protocol(skb, skb->protocol);
>
This should be skb_set_inner_ipproto(), since GTP is L3 protocol.


>         gtp_push_header(skb, pctx, &port);
>
> @@ -618,6 +622,8 @@ static void gtp_link_setup(struct net_device *dev)
>
>         dev->priv_flags |= IFF_NO_QUEUE;
>         dev->features   |= NETIF_F_LLTX;
> +       dev->hw_features |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
> +       dev->features   |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
>         netif_keep_dst(dev);
>
>         dev->needed_headroom    = LL_MAX_HEADER + max_gtp_header_len;
> --
> 2.27.0
>
Jonas Bonn Dec. 12, 2020, 7:50 a.m. UTC | #2 
On 12/12/2020 06:31, Pravin Shelar wrote:
> On Fri, Dec 11, 2020 at 4:28 AM Jonas Bonn <jonas@norrbonn.se> wrote:
>>
>> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
>> ---
>>   drivers/net/gtp.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
>> index 236ebbcb37bf..7bbeec173113 100644
>> --- a/drivers/net/gtp.c
>> +++ b/drivers/net/gtp.c
>> @@ -536,7 +536,11 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
>>          if (unlikely(r))
>>                  goto err_rt;
>>
>> -       skb_reset_inner_headers(skb);
>> +       r = udp_tunnel_handle_offloads(skb, true);
>> +       if (unlikely(r))
>> +               goto err_rt;
>> +
>> +       skb_set_inner_protocol(skb, skb->protocol);
>>
> This should be skb_set_inner_ipproto(), since GTP is L3 protocol.

I think you're right, but I barely see the point of this.  It all ends 
up in the same place, as far as I can tell.  Is this supposed to be some 
sort of safeguard against trying to offload tunnels-in-tunnels?

/Jonas
Pravin Shelar Dec. 13, 2020, 6:25 p.m. UTC | #3 
On Fri, Dec 11, 2020 at 11:50 PM Jonas Bonn <jonas@norrbonn.se> wrote:
>
>
>
> On 12/12/2020 06:31, Pravin Shelar wrote:
> > On Fri, Dec 11, 2020 at 4:28 AM Jonas Bonn <jonas@norrbonn.se> wrote:
> >>
> >> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
> >> ---
> >>   drivers/net/gtp.c | 8 +++++++-
> >>   1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> >> index 236ebbcb37bf..7bbeec173113 100644
> >> --- a/drivers/net/gtp.c
> >> +++ b/drivers/net/gtp.c
> >> @@ -536,7 +536,11 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
> >>          if (unlikely(r))
> >>                  goto err_rt;
> >>
> >> -       skb_reset_inner_headers(skb);
> >> +       r = udp_tunnel_handle_offloads(skb, true);
> >> +       if (unlikely(r))
> >> +               goto err_rt;
> >> +
> >> +       skb_set_inner_protocol(skb, skb->protocol);
> >>
> > This should be skb_set_inner_ipproto(), since GTP is L3 protocol.
>
> I think you're right, but I barely see the point of this.  It all ends
> up in the same place, as far as I can tell.  Is this supposed to be some
> sort of safeguard against trying to offload tunnels-in-tunnels?
>

In UDP offload this flag is used to process segments. With correct
flag GTP offload handling can directly jump to L3 processing.
diff mbox series

Patch

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 236ebbcb37bf..7bbeec173113 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -536,7 +536,11 @@  static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
 	if (unlikely(r))
 		goto err_rt;
 
-	skb_reset_inner_headers(skb);
+	r = udp_tunnel_handle_offloads(skb, true);
+	if (unlikely(r))
+		goto err_rt;
+
+	skb_set_inner_protocol(skb, skb->protocol);
 
 	gtp_push_header(skb, pctx, &port);
 
@@ -618,6 +622,8 @@  static void gtp_link_setup(struct net_device *dev)
 
 	dev->priv_flags	|= IFF_NO_QUEUE;
 	dev->features	|= NETIF_F_LLTX;
+	dev->hw_features |= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
+	dev->features	|= NETIF_F_SG | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM;
 	netif_keep_dst(dev);
 
 	dev->needed_headroom	= LL_MAX_HEADER + max_gtp_header_len;