net: cleanup for UDP tunnel's GRO
Subsystems (2)
Paolo Abeni
maintainercover net: cleanup for UDP tunnel's GRO
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.
This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the gro_cells hooks, in both vxlan and geneve driver are removed.
Paolo Abeni (4):
udp_offload: simplify error path
udp offload: allow GRO on 0 checksum packets
vxlan: remove gro_cell support
geneve: remove gro_cell support
drivers/net/geneve.c | 12 +----------- drivers/net/vxlan.c | 9 ++------- include/net/vxlan.h | 1 - net/ipv4/udp_offload.c | 12 +++++++----- 4 files changed, 10 insertions(+), 24 deletions(-)
Signature · 1 line
1.8.3.1
Quoted 11 lines · Paolo Abeni
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via
gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the
gro_cellshooks, in both vxlan and geneve driver are removed.
I think removing the gro_cells hooks may be taking things one step too far.
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO. My concern here is that you are optimizing for one specific use case while at the same time possibly negatively impacting other use cases. It would be useful to provide some data on what the advantages and disadvantages are expected to be.
- Alex
Quoted 7 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
Quoted 6 lines · Paolo Abeni
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via
gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.
Quoted 7 lines
Quoted 5 lines
This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the
gro_cellshooks, in both vxlan and geneve driver are removed.I think removing the
gro_cellshooks may be taking things one step too far.
+1
Quoted 3 lines
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.
Regards,
Shmulik
Quoted 18 lines · Shmulik Ladkani
Quoted 7 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
Quoted 6 lines · Paolo Abeni
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via
gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.Quoted 7 lines
Quoted 5 lines
This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the
gro_cellshooks, in both vxlan and geneve driver are removed.I think removing the
gro_cellshooks may be taking things one step too far.+1
Quoted 7 lines
Quoted 3 lines
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.
Isn't this a misconfiguration? TCP should not fragment at all, not even in vxlan/geneve if one cares about performance? And UDP is not GRO'ed anyway.
Bye,
Hannes
Quoted 6 lines · On Fri, 8 Jul 2016 16:57:10 -0400 Hannes Frederic Sowa…
Quoted 5 lines · Shmulik Ladkani
Quoted 3 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
Quoted 7 lines
Quoted 3 lines
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.Isn't this a misconfiguration? TCP should not fragment at all, not even in vxlan/geneve if one cares about performance? And UDP is not GRO'ed anyway.
It's not an ideal configuration, but it is a valid one.
Imagine TCP within vxlan/geneve, that gets properly segmented and encapsulated.
The vxlan/geneve datagrams go out the wire, and these can occasionally be fragmented on the way (e.g. when we can't control the MTUs along the path, or if unable to use PMTUD for whatever reason).
At the receiving vxlan/geneve termination, these IP frags are not GROed.
Instead they get reassembled by the IP stack, then handed to UDP and to the vxlan/geneve drivers.
Quoted 1 line
From that point, GROing at the vxlan/geneve device, which aggregates
the TCP segments into a TCP super-packet still make sense and has benefits.
Regards,
Shmulik
Quoted 15 lines · Shmulik Ladkani
Quoted 6 lines · On Fri, 8 Jul 2016 16:57:10 -0400 Hannes Frederic Sowa…
Quoted 5 lines · Shmulik Ladkani
Quoted 3 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
Quoted 7 lines
Quoted 3 lines
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.Isn't this a misconfiguration? TCP should not fragment at all, not even in vxlan/geneve if one cares about performance? And UDP is not GRO'ed anyway.
Quoted 8 lines
It's not an ideal configuration, but it is a valid one.
Imagine TCP within vxlan/geneve, that gets properly segmented and encapsulated.
The vxlan/geneve datagrams go out the wire, and these can occasionally be fragmented on the way (e.g. when we can't control the MTUs along the path, or if unable to use PMTUD for whatever reason).
PMTUD doesn't work with vxlan in most situations anyway. But you can still control the mtu/mss with ip route and you don't need to modify the mid-hosts.
Quoted 8 lines
At the receiving vxlan/geneve termination, these IP frags are not GROed.
Instead they get reassembled by the IP stack, then handed to UDP and to the vxlan/geneve drivers.
From that point, GROing at the vxlan/geneve device, which aggregates the TCP segments into a TCP super-packet still make sense and has benefits.
Given the spreading with which those fragments will be send, I wonder if the GRO ontop of the tunnels will really aggregate them?
Bye,
Hannes
Quoted 23 lines · Hannes Frederic Sowa
Quoted 22 lines · Shmulik Ladkani
Quoted 7 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
Quoted 6 lines · Paolo Abeni
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via
gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.Quoted 7 lines
Quoted 5 lines
This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the
gro_cellshooks, in both vxlan and geneve driver are removed.I think removing the
gro_cellshooks may be taking things one step too far.+1
Quoted 3 lines
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
Quoted 7 lines
Quoted 3 lines
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.Isn't this a misconfiguration? TCP should not fragment at all, not even in vxlan/geneve if one cares about performance? And UDP is not GRO'ed anyway.
The problem is that the DF bit of the outer header is what matters, not the inner one. I believe the default for many of the UDP tunnels is to not set the DF bit on the outer header. The fact is fragmentation shouldn't happen, but it can and we need to keep that in mind when we work on these sort of things.
There have been a number of cases in the past with tunnels where "it works for me" has been how things have been implemented and we need to avoid that as it creates a significant amount of pain for end users when they do things and they don't work because they have strayed off of the one use case that has been tested.
- Alex
Quoted 33 lines · Alexander Duyck
Quoted 23 lines · Hannes Frederic Sowa
Quoted 22 lines · Shmulik Ladkani
Quoted 7 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
Quoted 6 lines · Paolo Abeni
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via
gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.Quoted 7 lines
Quoted 5 lines
This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the
gro_cellshooks, in both vxlan and geneve driver are removed.I think removing the
gro_cellshooks may be taking things one step too far.+1
Quoted 3 lines
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
Quoted 7 lines
Quoted 3 lines
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.Isn't this a misconfiguration? TCP should not fragment at all, not even in vxlan/geneve if one cares about performance? And UDP is not GRO'ed anyway.
Quoted 5 lines
The problem is that the DF bit of the outer header is what matters, not the inner one. I believe the default for many of the UDP tunnels is to not set the DF bit on the outer header. The fact is fragmentation shouldn't happen, but it can and we need to keep that in mind when we work on these sort of things.
"Old" tunnel protocols inherit the outer DF bit from the inner one, geneve and vxlan do not. I think we simply never should set DF bit because vxlan pmtu with source port rss perturbation breaks pmtu discovery anyway. On the other hand doing so and not having end-to-end protection of the VNI because of proposed 0 checksum is also... otherwise at least the Ethernet FCS could protect the frame.
Quoted 5 lines
There have been a number of cases in the past with tunnels where "it works for me" has been how things have been implemented and we need to avoid that as it creates a significant amount of pain for end users when they do things and they don't work because they have strayed off of the one use case that has been tested.
We certainly don't want to break fragmentation with vxlan and this patch doesn't change so.
I really do wonder if GRO on top of fragmentation does have any effect. Would be great if someone has data for that already?
Bye,
Hannes
Quoted 40 lines · Hannes Frederic Sowa
Quoted 33 lines · Alexander Duyck
Quoted 23 lines · Hannes Frederic Sowa
Quoted 22 lines · Shmulik Ladkani
Quoted 7 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
Quoted 6 lines · Paolo Abeni
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via
gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.Quoted 7 lines
Quoted 5 lines
This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the
gro_cellshooks, in both vxlan and geneve driver are removed.I think removing the
gro_cellshooks may be taking things one step too far.+1
Quoted 3 lines
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
Quoted 7 lines
Quoted 3 lines
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.Isn't this a misconfiguration? TCP should not fragment at all, not even in vxlan/geneve if one cares about performance? And UDP is not GRO'ed anyway.
Quoted 5 lines
The problem is that the DF bit of the outer header is what matters, not the inner one. I believe the default for many of the UDP tunnels is to not set the DF bit on the outer header. The fact is fragmentation shouldn't happen, but it can and we need to keep that in mind when we work on these sort of things.
Quoted 6 lines
"Old" tunnel protocols inherit the outer DF bit from the inner one, geneve and vxlan do not. I think we simply never should set DF bit because vxlan pmtu with source port rss perturbation breaks pmtu discovery anyway. On the other hand doing so and not having end-to-end protection of the VNI because of proposed 0 checksum is also... otherwise at least the Ethernet FCS could protect the frame.
The "Old" tunnel protocols can be configured to behave the same way as well. It is just that most of them default to a mode where they support getting a path MTU if I recall correctly.
Quoted 11 lines
Quoted 5 lines
There have been a number of cases in the past with tunnels where "it works for me" has been how things have been implemented and we need to avoid that as it creates a significant amount of pain for end users when they do things and they don't work because they have strayed off of the one use case that has been tested.
We certainly don't want to break fragmentation with vxlan and this patch doesn't change so.
I really do wonder if GRO on top of fragmentation does have any effect. Would be great if someone has data for that already?
I think that logic is kind of backwards. It is already there. Instead of asking people to prove that this change is invalid the onus should be on the submitter to prove the change causes no harm.
The whole argument is kind of moot anyway based on the comment from Tom. This is based around being able to aggregate frames with a zero checksum which we can already do, but it requires that the hardware has validated the inner checksum. What this patch set is doing is trying to take frames that have no checksum and force us to perform a checksum for the inner headers in software. Really we don't do that now for general GRO, why should we do it in the case of tunnels? Also I think there is a test escape for the case of a frame with an outer checksum that was not validated by hardware. In that case the checksum isn't converted until you hit the UDP handler which will then convert it to checksum complete and it then would rely on the GRO cells to merge the frames after we have already validated the checksum for the outer header.
- Alex
Quoted 49 lines · Alexander Duyck
Quoted 40 lines · Hannes Frederic Sowa
Quoted 33 lines · Alexander Duyck
Quoted 23 lines · Hannes Frederic Sowa
Quoted 22 lines · Shmulik Ladkani
Quoted 7 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
Quoted 6 lines · Paolo Abeni
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via
gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.Quoted 7 lines
Quoted 5 lines
This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the
gro_cellshooks, in both vxlan and geneve driver are removed.I think removing the
gro_cellshooks may be taking things one step too far.+1
Quoted 3 lines
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
Quoted 7 lines
Quoted 3 lines
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.Isn't this a misconfiguration? TCP should not fragment at all, not even in vxlan/geneve if one cares about performance? And UDP is not GRO'ed anyway.
Quoted 5 lines
The problem is that the DF bit of the outer header is what matters, not the inner one. I believe the default for many of the UDP tunnels is to not set the DF bit on the outer header. The fact is fragmentation shouldn't happen, but it can and we need to keep that in mind when we work on these sort of things.
Quoted 6 lines
"Old" tunnel protocols inherit the outer DF bit from the inner one, geneve and vxlan do not. I think we simply never should set DF bit because vxlan pmtu with source port rss perturbation breaks pmtu discovery anyway. On the other hand doing so and not having end-to-end protection of the VNI because of proposed 0 checksum is also... otherwise at least the Ethernet FCS could protect the frame.
Quoted 3 lines
The "Old" tunnel protocols can be configured to behave the same way as well. It is just that most of them default to a mode where they support getting a path MTU if I recall correctly.
That actually only happend very recently, about one month ago (for gre at least). The other (ipip) doesn't support that.
Quoted 9 lines
Quoted 9 lines
Quoted 5 lines
There have been a number of cases in the past with tunnels where "it works for me" has been how things have been implemented and we need to avoid that as it creates a significant amount of pain for end users when they do things and they don't work because they have strayed off of the one use case that has been tested.
We certainly don't want to break fragmentation with vxlan and this patch doesn't change so.
Quoted 6 lines
Quoted 2 lines
I really do wonder if GRO on top of fragmentation does have any effect. Would be great if someone has data for that already?
I think that logic is kind of backwards. It is already there. Instead of asking people to prove that this change is invalid the onus should be on the submitter to prove the change causes no harm.
Of course, sorry, I didn't want to make the impression others should do that. I asked because Shmulik made the impression on me he had experience with GRO+fragmentation on vxlan and/or geneve and could provide some data, maybe even just anecdotal.
Quoted 5 lines
The whole argument is kind of moot anyway based on the comment from Tom. This is based around being able to aggregate frames with a zero checksum which we can already do, but it requires that the hardware has validated the inner checksum. What this patch set is doing is trying to take frames that have no checksum and force us to perform a
Quoted 8 lines
checksum for the inner headers in software. Really we don't do that now for general GRO, why should we do it in the case of tunnels? Also I think there is a test escape for the case of a frame with an outer checksum that was not validated by hardware. In that case the checksum isn't converted until you hit the UDP handler which will then convert it to checksum complete and it then would rely on the GRO cells to merge the frames after we have already validated the checksum for the outer header.
I do agree that Tom's comment changes things a little bit, but currently I tend towards removing the check completely, which needs further benchmark data. I wonder what difference it should make if we do the checksum check only in the gro cells call and not in the regular gro call (modulo of course, if gre or ipip need RPS before).
General-GRO does compute the checksum for TCP if not pre-computed as far as I know. So basically we don't special case for tunnels, otherwise why would this check be in the udp gro handler?
Maybe it also makes sense to optimistically compare some values of the tcp header before validating the checksum, so we can make sure that we calculate the expensive checksum only if it would actually only make sense.
Alex, unfortunately I didn't understand your last two sentences.
Bye,
Hannes
Quoted 92 lines · Hannes Frederic Sowa
Quoted 49 lines · Alexander Duyck
Quoted 40 lines · Hannes Frederic Sowa
Quoted 33 lines · Alexander Duyck
Quoted 23 lines · Hannes Frederic Sowa
Quoted 22 lines · Shmulik Ladkani
Quoted 7 lines · On Fri, 8 Jul 2016 09:21:40 -0700 Alexander Duyck <[email protected]>…
Quoted 6 lines · Paolo Abeni
With udp tunnel offload in place, the kernel can do GRO for some udp tunnels at the ingress device level. Currently both the geneve and the vxlan drivers implement an additional GRO aggregation point via
gro_cells.
The latter takes effect for tunnels using zero checksum udp packets, which are currently explicitly not aggregated by the udp offload layer.Quoted 7 lines
Quoted 5 lines
This patch series adapts the udp tunnel offload to process also zero checksum udp packets, if the tunnel's socket allow it. Aggregation, if possible is always performed at the ingress device level.
Then the
gro_cellshooks, in both vxlan and geneve driver are removed.I think removing the
gro_cellshooks may be taking things one step too far.+1
Quoted 3 lines
I get that there is an impression that it is redundant but there are a number of paths that could lead to VXLAN or GENEVE frames being received that are not aggregated via GRO.
Quoted 7 lines
Quoted 3 lines
There's the case where the vxlan/geneve datagrams get IP fragmented, and IP frags are not GROed.
GRO aggregation at the vxlan/geneve level is beneficial for this case.Isn't this a misconfiguration? TCP should not fragment at all, not even in vxlan/geneve if one cares about performance? And UDP is not GRO'ed anyway.
Quoted 5 lines
The problem is that the DF bit of the outer header is what matters, not the inner one. I believe the default for many of the UDP tunnels is to not set the DF bit on the outer header. The fact is fragmentation shouldn't happen, but it can and we need to keep that in mind when we work on these sort of things.
Quoted 6 lines
"Old" tunnel protocols inherit the outer DF bit from the inner one, geneve and vxlan do not. I think we simply never should set DF bit because vxlan pmtu with source port rss perturbation breaks pmtu discovery anyway. On the other hand doing so and not having end-to-end protection of the VNI because of proposed 0 checksum is also... otherwise at least the Ethernet FCS could protect the frame.
Quoted 3 lines
The "Old" tunnel protocols can be configured to behave the same way as well. It is just that most of them default to a mode where they support getting a path MTU if I recall correctly.
That actually only happend very recently, about one month ago (for gre at least). The other (ipip) doesn't support that.
Quoted 9 lines
Quoted 9 lines
Quoted 5 lines
There have been a number of cases in the past with tunnels where "it works for me" has been how things have been implemented and we need to avoid that as it creates a significant amount of pain for end users when they do things and they don't work because they have strayed off of the one use case that has been tested.
We certainly don't want to break fragmentation with vxlan and this patch doesn't change so.
Quoted 6 lines
Quoted 2 lines
I really do wonder if GRO on top of fragmentation does have any effect. Would be great if someone has data for that already?
I think that logic is kind of backwards. It is already there. Instead of asking people to prove that this change is invalid the onus should be on the submitter to prove the change causes no harm.
Of course, sorry, I didn't want to make the impression others should do that. I asked because Shmulik made the impression on me he had experience with GRO+fragmentation on vxlan and/or geneve and could provide some data, maybe even just anecdotal.
Quoted 5 lines
The whole argument is kind of moot anyway based on the comment from Tom. This is based around being able to aggregate frames with a zero checksum which we can already do, but it requires that the hardware has validated the inner checksum. What this patch set is doing is trying to take frames that have no checksum and force us to perform a
Quoted 8 lines
checksum for the inner headers in software. Really we don't do that now for general GRO, why should we do it in the case of tunnels? Also I think there is a test escape for the case of a frame with an outer checksum that was not validated by hardware. In that case the checksum isn't converted until you hit the UDP handler which will then convert it to checksum complete and it then would rely on the GRO cells to merge the frames after we have already validated the checksum for the outer header.
Quoted 5 lines
I do agree that Tom's comment changes things a little bit, but currently I tend towards removing the check completely, which needs further benchmark data. I wonder what difference it should make if we do the checksum check only in the gro cells call and not in the regular gro call (modulo of course, if gre or ipip need RPS before).
I'm thinking the main difference is that if there is zero checksum and the hardware hasn't provided any sort of checksum I am not certain if GRO actually occurs. In the case of a checksum being present but not validated by hardware the checksum should be converted to CHECKSUM_COMPLETE once it is validated and then GRO could occur for the inner data.
Quoted 3 lines
General-GRO does compute the checksum for TCP if not pre-computed as far as I know. So basically we don't special case for tunnels, otherwise why would this check be in the udp gro handler?
So I think this might be a more recent change. I know that you used to be able to disable the Rx checksum in order to disable GRO. It seems like now if you disable it the frames are still aggregated. It might be worth while to figure out the reasoning for that change as it may be applicable to tunnels as well.
Quoted 3 lines
Maybe it also makes sense to optimistically compare some values of the tcp header before validating the checksum, so we can make sure that we calculate the expensive checksum only if it would actually only make sense.
Okay. One other thing you might look at is if you could defer validating the outer checksums until after the inner one has been validated in the case that the hardware has not validated it.
Quoted 1 line
Alex, unfortunately I didn't understand your last two sentences.
The patch set as it currently stands was only performing GRO if a checksum was not present, however if a checksum is present on the outer header then the cost checksum the UDP packet and then work back to the TCP header should be comparable to the overhead needed to just do the TCP header. Basically what I am getting at is that if you are going going to coalesce the no checksum case for tunnels without having the hardware validate the checksum is present you could probably do the same thing for tunnels if the checksum is present on the outer header.
- Alex
Quoted 11 lines · On Fri, 8 Jul 2016 19:04:27 -0400 Hannes Frederic Sowa…
Quoted 6 lines
Quoted 2 lines
I really do wonder if GRO on top of fragmentation does have any effect. Would be great if someone has data for that already?
I think that logic is kind of backwards. It is already there. Instead of asking people to prove that this change is invalid the onus should be on the submitter to prove the change causes no harm.
Of course, sorry, I didn't want to make the impression others should do that. I asked because Shmulik made the impression on me he had experience with GRO+fragmentation on vxlan and/or geneve and could provide some data, maybe even just anecdotal.
Few anecdotal updates.
I don't have ready-made data as the systems are not using this exact kind of of setup.
However, by performing some quick experimentations, it reveals that GRO on top of the tunnels, where tunnel datagrams are fragmented, has some effect. The packets indeed get aggregated, although not aggresively as in the non-fragmented case.
Whether the effect is significant depends on the system.
In a system that is very sensitive to non-aggregated skbs (due to a cpu bottleneck during further processing of the decapsulated packets), the effect of aggregation is indeed significant.
Regards,
Shmulik
Quoted 23 lines · Shmulik Ladkani
Quoted 11 lines · On Fri, 8 Jul 2016 19:04:27 -0400 Hannes Frederic Sowa…
Quoted 6 lines
Quoted 2 lines
I really do wonder if GRO on top of fragmentation does have any effect. Would be great if someone has data for that already?
I think that logic is kind of backwards. It is already there. Instead of asking people to prove that this change is invalid the onus should be on the submitter to prove the change causes no harm.
Of course, sorry, I didn't want to make the impression others should do that. I asked because Shmulik made the impression on me he had experience with GRO+fragmentation on vxlan and/or geneve and could provide some data, maybe even just anecdotal.
Few anecdotal updates.
I don't have ready-made data as the systems are not using this exact kind of of setup.
However, by performing some quick experimentations, it reveals that GRO on top of the tunnels, where tunnel datagrams are fragmented, has some effect. The packets indeed get aggregated, although not aggresively as in the non-fragmented case.
Quoted 5 lines
Whether the effect is significant depends on the system.
In a system that is very sensitive to non-aggregated skbs (due to a cpu bottleneck during further processing of the decapsulated packets), the effect of aggregation is indeed significant.
Cool, thanks. I thought it wouldn't happen because of the packet pacing. We will also do some more tests ourselves. Maybe it is time to add fragmentation support to inet_gro_receive to handle those cases much more easily without going through fragmentation engine at all, would probably speed up your usage significantly?
Talking about ip fragmentation in general, are you end-host or mid-router fragmented? Do you know if there are different characteristics if linux fragments vs. some kind of hw-router in the middle (do fragments get paced?).
Thanks,
Hannes
Quoted 30 lines · On Sat, 9 Jul 2016 11:35:03 -0400 Hannes Frederic Sowa…
Quoted 23 lines · Shmulik Ladkani
Quoted 11 lines · On Fri, 8 Jul 2016 19:04:27 -0400 Hannes Frederic Sowa…
Quoted 6 lines
Quoted 2 lines
I really do wonder if GRO on top of fragmentation does have any effect. Would be great if someone has data for that already?
I think that logic is kind of backwards. It is already there. Instead of asking people to prove that this change is invalid the onus should be on the submitter to prove the change causes no harm.
Of course, sorry, I didn't want to make the impression others should do that. I asked because Shmulik made the impression on me he had experience with GRO+fragmentation on vxlan and/or geneve and could provide some data, maybe even just anecdotal.
Few anecdotal updates.
I don't have ready-made data as the systems are not using this exact kind of of setup.
However, by performing some quick experimentations, it reveals that GRO on top of the tunnels, where tunnel datagrams are fragmented, has some effect. The packets indeed get aggregated, although not aggresively as in the non-fragmented case.
Quoted 5 lines
Whether the effect is significant depends on the system.
In a system that is very sensitive to non-aggregated skbs (due to a cpu bottleneck during further processing of the decapsulated packets), the effect of aggregation is indeed significant.
Quoted 5 lines
Cool, thanks. I thought it wouldn't happen because of the packet pacing. We will also do some more tests ourselves. Maybe it is time to add fragmentation support to
inet_gro_receiveto handle those cases much more easily without going through fragmentation engine at all, would probably speed up your usage significantly?
Indeed, that seems beneficial. I wondered about this back ago. I found it not trivial, though. Without the transport headers available per received SKB, it makes GRO complex than currently is :)
Quoted 2 lines
Talking about ip fragmentation in general, are you end-host or mid-router fragmented?
Currently dealing with end-host fragmentation. (follow the thread at [1] - usecase is better explained there)
[1] http://www.spinics.net/lists/netdev/msg385085.html
Regards,
Shmulik