[RFC PATCH] lro: ip fragment checking

[Jan-Bernd Themann]

Currently there is no checking in the LRO receive path whether
TCP packets are ip fragmented. We should not consider
those packets for aggregation.
I'm not sure if this checking is actually required. Does anyone
know if it is possible to get fragmented TCP packets without
the tcp stack changing the MSS size?
This patch introduces explicit checking. Any objections?

Regards,

Jan-Bernd

---



net/ipv4/inet_lro.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/inet_lro.c b/net/ipv4/inet_lro.c
index cfd034a..1f9159d 100644
--- a/net/ipv4/inet_lro.c
+++ b/net/ipv4/inet_lro.c
@@ -64,6 +64,9 @@ static int lro_tcp_ip_check(struct iphdr *iph, struct tcphdr *tcph,
if (iph->ihl != IPH_LEN_WO_OPTIONS)
return -1;

+ if (iph->frag_off & IP_MF)
+ return -1;
+
if (tcph->cwr || tcph->ece || tcph->urg || !tcph->ack
|| tcph->rst || tcph->syn || tcph->fin)
return -1;
--
1.5.5




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Ben Hutchings]

Post by Jan-Bernd Themann
Currently there is no checking in the LRO receive path whether
TCP packets are ip fragmented. We should not consider
those packets for aggregation.
I'm not sure if this checking is actually required. Does anyone
know if it is possible to get fragmented TCP packets without
the tcp stack changing the MSS size?
This patch introduces explicit checking. Any objections?

LRO depends on the hardware performing TCP checksum offload, and the TCP
checksum cannot be verified for IP fragments in isolation. So I think
drivers should not be passing fragments into inet_lro or should reject
them in its get_frag_header() or get_skb_header() method. Certainly sfc
doesn't pass fragments into inet_lro because they have not been
checksummed.

Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Brandeburg, Jesse]

Post by Ben Hutchings

Post by Jan-Bernd Themann
Currently there is no checking in the LRO receive path whether
TCP packets are ip fragmented. We should not consider
those packets for aggregation.
I'm not sure if this checking is actually required. Does anyone
know if it is possible to get fragmented TCP packets without
the tcp stack changing the MSS size?
This patch introduces explicit checking. Any objections?

LRO depends on the hardware performing TCP checksum offload, and the
TCP checksum cannot be verified for IP fragments in isolation. So I
think drivers should not be passing fragments into inet_lro or should
reject them in its get_frag_header() or get_skb_header() method.
Certainly sfc doesn't pass fragments into inet_lro because they have
not been checksummed.

The fragments, definitely will not have checksums offloaded, but
what about the first packet in the chain? I haven't verified in
ixgbe or igb whether or not it could try and aggregate a packet
with MF set if it was the first fragment in a series of IP fragments.--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/