Re: [RFC PATCH v2 00/10] udp: implement GRO support
On Tue, Oct 23, 2018 at 02:22:12PM +0200, Paolo Abeni wrote: > On Tue, 2018-10-23 at 14:10 +0200, Steffen Klassert wrote: > > > Some quick benchmark numbers with UDP packet forwarding > > (1460 byte packets) through two gateways: > > > > net-next: 16.4 Gbps > > > > net-next + UDP GRO: 20.3 Gbps > > uhmmm... what do you think about this speed-up ? skb_segment() burns a lot of cycles. If I do the same test with TCP and disable HW TSO, throughput drops also down to similar values. In case of software segmentation, the skb chain appropach is likely faster because packets are not mangled. So no need to allocate skbs, no new checksum calculations, less memcpy etc. If we have an early route lookup in GRO, we could have a good guess on the offload capabilities of the outgoing device. So in case that software segmentation is likely, use the skb chaining method. If HW segmentation is likely, merge IP packets. The chaining method might be also faster on non UDP GRO enabled sockets. I'll try to implement the skb chaining method on top of this to see what we get from that.
Re: [RFC PATCH v2 00/10] udp: implement GRO support
Hi,
On Tue, 2018-10-23 at 14:10 +0200, Steffen Klassert wrote:
> On Fri, Oct 19, 2018 at 04:25:10PM +0200, Paolo Abeni wrote:
> > This series implements GRO support for UDP sockets, as the RX counterpart
> > of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
> > The core functionality is implemented by the second patch, introducing a new
> > sockopt to enable UDP_GRO, while patch 3 implements support for passing the
> > segment size to the user space via a new cmsg.
> > UDP GRO performs a socket lookup for each ingress packets and aggregate
> > datagram
> > directed to UDP GRO enabled sockets with constant l4 tuple.
> >
> > UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables
> > NAT
> > rules, and that could potentially confuse existing applications.
> >
> > The solution adopted here is to de-segment the GRO packet before enqueuing
> > as needed. Since we must cope with packet reinsertion after de-segmentation,
> > the relevant code is factored-out in ipv4 and ipv6 specific helpers and
> > exposed
> > to UDP usage.
> >
> > While the current code can probably be improved, this safeguard
> > ,implemented in
> > the patches 4-7, allows future enachements to enable UDP GSO offload on more
> > virtual devices eventually even on forwarded packets.
>
> I was curious what I get with this when doing packet forwarding.
> So I added forwarding support with the patch below (on top of
> your patchset). While the packet processing could work the way
> I do it in this patch, I'm not sure about the way I enable
> UDP GRO on forwarding. Maybe there it a better way than just
> do UDP GRO if forwarding is enabled on the receiving device.
My idea/hope is slightly different: ensure that UDP GRO + UDP input
path + UDP desegmentation on socket enqueue is safe and as fast as
plain UDP input path and then enable UDP GRO without socket lookup, for
each incoming frame (!!!).
If we land on the input path on a non UDP GRO enabled socket, the
packet is de-segment before enqueuing.
Socket lookup would be needed only if tunnels are enabled, to check if
the ingress frame match a tunnel. We will use UDP tunnel GRO in that
case.
> Some quick benchmark numbers with UDP packet forwarding
> (1460 byte packets) through two gateways:
>
> net-next: 16.4 Gbps
>
> net-next + UDP GRO: 20.3 Gbps
uhmmm... what do you think about this speed-up ?
I hoped that without the socket lookup we can get measurably better
figures.
Cheers,
Paolo
Re: [RFC PATCH v2 00/10] udp: implement GRO support
On Fri, Oct 19, 2018 at 04:25:10PM +0200, Paolo Abeni wrote:
> This series implements GRO support for UDP sockets, as the RX counterpart
> of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
> The core functionality is implemented by the second patch, introducing a new
> sockopt to enable UDP_GRO, while patch 3 implements support for passing the
> segment size to the user space via a new cmsg.
> UDP GRO performs a socket lookup for each ingress packets and aggregate
> datagram
> directed to UDP GRO enabled sockets with constant l4 tuple.
>
> UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables NAT
> rules, and that could potentially confuse existing applications.
>
> The solution adopted here is to de-segment the GRO packet before enqueuing
> as needed. Since we must cope with packet reinsertion after de-segmentation,
> the relevant code is factored-out in ipv4 and ipv6 specific helpers and
> exposed
> to UDP usage.
>
> While the current code can probably be improved, this safeguard ,implemented
> in
> the patches 4-7, allows future enachements to enable UDP GSO offload on more
> virtual devices eventually even on forwarded packets.
I was curious what I get with this when doing packet forwarding.
So I added forwarding support with the patch below (on top of
your patchset). While the packet processing could work the way
I do it in this patch, I'm not sure about the way I enable
UDP GRO on forwarding. Maybe there it a better way than just
do UDP GRO if forwarding is enabled on the receiving device.
Some quick benchmark numbers with UDP packet forwarding
(1460 byte packets) through two gateways:
net-next: 16.4 Gbps
net-next + UDP GRO: 20.3 Gbps
Subject: [PATCH RFC] udp: Allow gro for the forwarding path.
This patch adds a early route lookup to inet_gro_receive()
in case forwarding is enabled on the receiving device.
To be forwarded packets are allowed to enter the UDP
GRO handlers then.
Signed-off-by: Steffen Klassert
---
include/linux/netdevice.h | 2 +-
include/net/dst.h | 1 +
net/core/dev.c| 6 --
net/ipv4/af_inet.c| 12
net/ipv4/route.c | 1 +
net/ipv4/udp_offload.c| 11 +--
6 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dc1d9ed33b31..2eb3fa960ad4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2304,7 +2304,7 @@ struct napi_gro_cb {
/* Number of gro_receive callbacks this packet already went through */
u8 recursion_counter:4;
- /* 1 bit hole */
+ u8 is_fwd:1;
/* used to support CHECKSUM_COMPLETE for tunneling protocols */
__wsum csum;
diff --git a/include/net/dst.h b/include/net/dst.h
index 6cf0870414c7..01bdf825a6f9 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -54,6 +54,7 @@ struct dst_entry {
#define DST_XFRM_TUNNEL0x0020
#define DST_XFRM_QUEUE 0x0040
#define DST_METADATA 0x0080
+#define DST_FWD0x0100
/* A non-zero value of dst->obsolete forces by-hand validation
* of the route entry. Positive values are set by the generic
diff --git a/net/core/dev.c b/net/core/dev.c
index 022ad73d6253..c6aaffb99456 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5387,8 +5387,10 @@ static struct list_head *gro_list_prepare(struct
napi_struct *napi,
diffs = (unsigned long)p->dev ^ (unsigned long)skb->dev;
diffs |= p->vlan_tci ^ skb->vlan_tci;
- diffs |= skb_metadata_dst_cmp(p, skb);
- diffs |= skb_metadata_differs(p, skb);
+ if (!NAPI_GRO_CB(p)->is_fwd) {
+ diffs |= skb_metadata_dst_cmp(p, skb);
+ diffs |= skb_metadata_differs(p, skb);
+ }
if (maclen == ETH_HLEN)
diffs |= compare_ether_header(skb_mac_header(p),
skb_mac_header(skb));
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1fbe2f815474..6e4e8588c0b1 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1479,8 +1479,20 @@ struct sk_buff *inet_gro_receive(struct list_head *head,
struct sk_buff *skb)
NAPI_GRO_CB(p)->flush_id = flush_id;
else
NAPI_GRO_CB(p)->flush_id |= flush_id;
+
+ NAPI_GRO_CB(skb)->is_fwd = NAPI_GRO_CB(p)->is_fwd;
+
+ goto found;
}
+ if (IN_DEV_FORWARD(__in_dev_get_rcu(skb->dev))) {
+ if (!ip_route_input_noref(skb, iph->daddr, iph->saddr, iph->tos,
+ skb->dev)) {
+ if ((skb_dst(skb)->flags & DST_FWD))
+ NAPI_GRO_CB(skb)->is_fwd = 1;
+ }
+ }
+found:
NAPI_GRO_CB(skb)->is_atomic = !!(iph->frag_off &
Re: [RFC PATCH v2 00/10] udp: implement GRO support
Hi all,
On Sun, 2018-10-21 at 16:05 -0400, Willem de Bruijn wrote:
> On Fri, Oct 19, 2018 at 10:30 AM Paolo Abeni wrote:
> >
> > This series implements GRO support for UDP sockets, as the RX counterpart
> > of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
> > The core functionality is implemented by the second patch, introducing a new
> > sockopt to enable UDP_GRO, while patch 3 implements support for passing the
> > segment size to the user space via a new cmsg.
> > UDP GRO performs a socket lookup for each ingress packets and aggregate
> > datagram
> > directed to UDP GRO enabled sockets with constant l4 tuple.
> >
> > UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables
> > NAT
> > rules, and that could potentially confuse existing applications.
>
> Good catch.
>
> > The solution adopted here is to de-segment the GRO packet before enqueuing
> > as needed. Since we must cope with packet reinsertion after de-segmentation,
> > the relevant code is factored-out in ipv4 and ipv6 specific helpers and
> > exposed
> > to UDP usage.
> >
> > While the current code can probably be improved, this safeguard
> > ,implemented in
> > the patches 4-7, allows future enachements to enable UDP GSO offload on more
> > virtual devices eventually even on forwarded packets.
> >
> > The last 4 for patches implement some performance and functional self-tests,
> > re-using the existing udpgso infrastructure. The problematic scenario
> > described
> > above is explicitly tested.
>
> This looks awesome! Impressive testing, too.
>
> A few comments in the individual patches, mostly minor.
Thank you for the in-depth review! (in the WE ;)
I'll try to address the comments on each patch individually.
In the end I rushed a bit this RFC, because the misdirection issue (and
the tentative fix) bothered me more than a bit: I wanted to check I was
not completely out-of-track.
Cheers,
Paolo
Re: [RFC PATCH v2 00/10] udp: implement GRO support
On Fri, Oct 19, 2018 at 10:30 AM Paolo Abeni wrote:
>
> This series implements GRO support for UDP sockets, as the RX counterpart
> of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
> The core functionality is implemented by the second patch, introducing a new
> sockopt to enable UDP_GRO, while patch 3 implements support for passing the
> segment size to the user space via a new cmsg.
> UDP GRO performs a socket lookup for each ingress packets and aggregate
> datagram
> directed to UDP GRO enabled sockets with constant l4 tuple.
>
> UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables NAT
> rules, and that could potentially confuse existing applications.
Good catch.
> The solution adopted here is to de-segment the GRO packet before enqueuing
> as needed. Since we must cope with packet reinsertion after de-segmentation,
> the relevant code is factored-out in ipv4 and ipv6 specific helpers and
> exposed
> to UDP usage.
>
> While the current code can probably be improved, this safeguard ,implemented
> in
> the patches 4-7, allows future enachements to enable UDP GSO offload on more
> virtual devices eventually even on forwarded packets.
>
> The last 4 for patches implement some performance and functional self-tests,
> re-using the existing udpgso infrastructure. The problematic scenario
> described
> above is explicitly tested.
This looks awesome! Impressive testing, too.
A few comments in the individual patches, mostly minor.
