linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next] udp: allow forwarding of plain (non-fraglisted) UDP GRO packets
@ 2021-01-13 10:32 Alexander Lobakin
  2021-01-13 18:37 ` Paolo Abeni
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander Lobakin @ 2021-01-13 10:32 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Alexander Duyck, Eric Dumazet, Edward Cree, Willem de Bruijn,
	Steffen Klassert, Dongseok Yi, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Alexander Lobakin, netdev, linux-kernel

Commit 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.") actually
not only added a support for fraglisted UDP GRO, but also tweaked
some logics the way that non-fraglisted UDP GRO started to work for
forwarding too.
Commit 2e4ef10f5850 ("net: add GSO UDP L4 and GSO fraglists to the
list of software-backed types") added GSO UDP L4 to the list of
software GSO to allow virtual netdevs to forward them as is up to
the real drivers.

Tests showed that currently forwarding and NATing of plain UDP GRO
packets are performed fully correctly, regardless if the target
netdevice has a support for hardware/driver GSO UDP L4 or not.
Add the last element and allow to form plain UDP GRO packets if
there is no socket -> we are on forwarding path.

Plain UDP GRO forwarding even shows better performance than fraglisted
UDP GRO in some cases due to not wasting one skbuff_head per every
segment.

Since v1 [0]:
 - drop redundant 'if (sk)' check (Alexander Duyck);
 - add a ref in the commit message to one more commit that was
   an important step for UDP GRO forwarding.

[0] https://lore.kernel.org/netdev/20210112211536.261172-1-alobakin@pm.me

Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 net/ipv4/udp_offload.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index ff39e94781bf..6f5237a0d181 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -460,12 +460,12 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 	if (skb->dev->features & NETIF_F_GRO_FRAGLIST)
 		NAPI_GRO_CB(skb)->is_flist = sk ? !udp_sk(sk)->gro_enabled: 1;
 
-	if ((sk && udp_sk(sk)->gro_enabled) || NAPI_GRO_CB(skb)->is_flist) {
+	if (!sk || udp_sk(sk)->gro_enabled || NAPI_GRO_CB(skb)->is_flist) {
 		pp = call_gro_receive(udp_gro_receive_segment, head, skb);
 		return pp;
 	}
 
-	if (!sk || NAPI_GRO_CB(skb)->encap_mark ||
+	if (NAPI_GRO_CB(skb)->encap_mark ||
 	    (skb->ip_summed != CHECKSUM_PARTIAL &&
 	     NAPI_GRO_CB(skb)->csum_cnt == 0 &&
 	     !NAPI_GRO_CB(skb)->csum_valid) ||
-- 
2.30.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 net-next] udp: allow forwarding of plain (non-fraglisted) UDP GRO packets
  2021-01-13 10:32 [PATCH v2 net-next] udp: allow forwarding of plain (non-fraglisted) UDP GRO packets Alexander Lobakin
@ 2021-01-13 18:37 ` Paolo Abeni
  2021-01-14 10:45   ` Alexander Lobakin
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Abeni @ 2021-01-13 18:37 UTC (permalink / raw)
  To: Alexander Lobakin, David S. Miller, Jakub Kicinski
  Cc: Alexander Duyck, Eric Dumazet, Edward Cree, Willem de Bruijn,
	Steffen Klassert, Dongseok Yi, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel

On Wed, 2021-01-13 at 10:32 +0000, Alexander Lobakin wrote:
> Commit 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.") actually
> not only added a support for fraglisted UDP GRO, but also tweaked
> some logics the way that non-fraglisted UDP GRO started to work for
> forwarding too.
> Commit 2e4ef10f5850 ("net: add GSO UDP L4 and GSO fraglists to the
> list of software-backed types") added GSO UDP L4 to the list of
> software GSO to allow virtual netdevs to forward them as is up to
> the real drivers.
> 
> Tests showed that currently forwarding and NATing of plain UDP GRO
> packets are performed fully correctly, regardless if the target
> netdevice has a support for hardware/driver GSO UDP L4 or not.
> Add the last element and allow to form plain UDP GRO packets if
> there is no socket -> we are on forwarding path.

If I read correctly, the above will make UDP GRO in the forwarding path
always enabled (admin can't disable that, if forwarding is enabled).

UDP GRO can introduce measurable latency for UDP packets staging in the
napi GRO hash (no push flag for UDP ;).

Currently the admin (for fraglist) or the application (for socket-based 
"plain" GRO) have to explicitly enable the feature, but this change
will impact every user.

I think we need at lest an explict switch for this.

Cheers,

Paolo


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 net-next] udp: allow forwarding of plain (non-fraglisted) UDP GRO packets
  2021-01-13 18:37 ` Paolo Abeni
@ 2021-01-14 10:45   ` Alexander Lobakin
  0 siblings, 0 replies; 3+ messages in thread
From: Alexander Lobakin @ 2021-01-14 10:45 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Alexander Lobakin, David S. Miller, Jakub Kicinski,
	Alexander Duyck, Eric Dumazet, Edward Cree, Willem de Bruijn,
	Steffen Klassert, Dongseok Yi, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel

From: Paolo Abeni <pabeni@redhat.com>
Date: Wed, 13 Jan 2021 19:37:18 +0100

> On Wed, 2021-01-13 at 10:32 +0000, Alexander Lobakin wrote:
>> Commit 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.") actually
>> not only added a support for fraglisted UDP GRO, but also tweaked
>> some logics the way that non-fraglisted UDP GRO started to work for
>> forwarding too.
>> Commit 2e4ef10f5850 ("net: add GSO UDP L4 and GSO fraglists to the
>> list of software-backed types") added GSO UDP L4 to the list of
>> software GSO to allow virtual netdevs to forward them as is up to
>> the real drivers.
>>
>> Tests showed that currently forwarding and NATing of plain UDP GRO
>> packets are performed fully correctly, regardless if the target
>> netdevice has a support for hardware/driver GSO UDP L4 or not.
>> Add the last element and allow to form plain UDP GRO packets if
>> there is no socket -> we are on forwarding path.
>
> If I read correctly, the above will make UDP GRO in the forwarding path
> always enabled (admin can't disable that, if forwarding is enabled).
>
> UDP GRO can introduce measurable latency for UDP packets staging in the
> napi GRO hash (no push flag for UDP ;).
>
> Currently the admin (for fraglist) or the application (for socket-based
> "plain" GRO) have to explicitly enable the feature, but this change
> will impact every user.
>
> I think we need at lest an explict switch for this.

Fraglist UDP GRO is controlled by netdev feature / ethtool, plain UDO
GRO is controlled by the sockopt. Regarding that we have no sock on
forwarding, what kind of switch should we introduce here? One more
netdev feature, smth like NETIF_F_GRO_UDP_L4?

> Cheers,
>
> Paolo

Thanks,
Al


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-14 10:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 10:32 [PATCH v2 net-next] udp: allow forwarding of plain (non-fraglisted) UDP GRO packets Alexander Lobakin
2021-01-13 18:37 ` Paolo Abeni
2021-01-14 10:45   ` Alexander Lobakin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).