From: Alex Elder <hidden> Date: 2021-06-11 19:06:55
I'm posting a large series an two smaller parts; this is part 1.
The RMNet driver handles MAP (or QMAP) protocol traffic. There are
several versions of this protocol. Version 1 supports multiplexing,
as well as aggregation of packets in a single buffer. Version 4
adds the ability to perform checksum offload. And version 5
implements checksum offload in a different way from version 4.
This series involves only MAPv4 protocol checksum offload, and only
in the download (RX) direction. It affects handling of checksums
computed by hardware for UDP datagrams and TCP segments, carried
over both IPv4 and IPv6.
MAP packets arriving on an RMNet port implementing MAPv4 checksum
offload are passed to rmnet_map_checksum_downlink_packet() for
handling.
The packet is then passed to rmnet_map_ipv4_dl_csum_trailer() or
rmnet_map_ipv6_dl_csum_trailer(), depending contents of the MAP
payload. These two functions interpret checksum metadata to
determine whether the checksum in the received packet matches that
calculated by the hardware.
It is these two functions that are the subject of this series (parts
1 and 2). The bulk of these functions are transformed--in a lot of
small steps--from an extremely difficult-to-follow block of checksum
processing code into a fairly simple, heavily commented equivalent.
-Alex
Alex Elder (8):
net: qualcomm: rmnet: use ip_is_fragment()
net: qualcomm: rmnet: eliminate some ifdefs
net: qualcomm: rmnet: get rid of some local variables
net: qualcomm: rmnet: simplify rmnet_map_get_csum_field()
net: qualcomm: rmnet: IPv4 header has zero checksum
net: qualcomm: rmnet: clarify a bit of code
net: qualcomm: rmnet: avoid unnecessary byte-swapping
net: qualcomm: rmnet: avoid unnecessary IPv6 byte-swapping
.../ethernet/qualcomm/rmnet/rmnet_config.h | 1 +
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 179 +++++++++---------
.../net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 1 +
3 files changed, 92 insertions(+), 89 deletions(-)
--
2.27.0
From: Alex Elder <hidden> Date: 2021-06-11 19:05:41
In rmnet_map_ipv4_dl_csum_trailer() use ip_is_fragment() to
determine whether a socket buffer contains a packet fragment.
Signed-off-by: Alex Elder <redacted>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
From: Alex Elder <hidden> Date: 2021-06-11 19:05:45
If IPV6 is not enabled in the kernel configuration, the RMNet
checksum code indicates a buffer containing an IPv6 packet is not
supported. The same thing happens if a buffer contains something
other than an IPv4 or IPv6 packet.
We can rearrange things a bit in two functions so that some #ifdef
calls can simply be eliminated.
Signed-off-by: Alex Elder <redacted>
---
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 54 ++++++++-----------
1 file changed, 23 insertions(+), 31 deletions(-)
From: Alex Elder <hidden> Date: 2021-06-11 19:05:54
The value passed as an argument to rmnet_map_ipv4_ul_csum_header()
is always an IPv4 header. Rather than using a local variable, just
have the type of the argument reflect the proper type.
In rmnet_map_ipv6_ul_csum_header() things are defined a little
differently, but make the same basic change there.
Signed-off-by: Alex Elder <redacted>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
From: Alex Elder <hidden> Date: 2021-06-11 19:06:03
The checksum fields of the TCP and UDP header structures already
have type __sum16. We don't support any other protocol headers, so
we can simplify rmnet_map_get_csum_field(), getting rid of the local
variable entirely and just returning the appropriate address.
Signed-off-by: Alex Elder <redacted>
---
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 20 +++++--------------
1 file changed, 5 insertions(+), 15 deletions(-)
From: Alex Elder <hidden> Date: 2021-06-11 19:06:06
In rmnet_map_ipv4_dl_csum_trailer(), an illegal checksum subtraction
is done, subtracting hdr_csum (in host byte order) from csum_value (in
network byte order). Despite being illegal, it generally works,
because it turns out the value subtracted is (or should be) always 0,
which has the same representation in either byte order.
Doing illegal operations is not good form though, so fix this by
verifying the IP header checksum early in that function. If its
checksum is non-zero, the packet will be bad, so just return an
error. This will cause the packet to passed to the IP layer where
it can be dropped.
Thereafter, there is no need subtract the IP header checksum from
the checksum value in the trailer because we know it is zero.
Add a comment explaining this.
This type of packet error is different from other types, so add a
new statistics counter to track this condition.
Signed-off-by: Alex Elder <redacted>
---
.../ethernet/qualcomm/rmnet/rmnet_config.h | 1 +
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 41 ++++++++++++-------
.../net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 1 +
3 files changed, 29 insertions(+), 14 deletions(-)
@@ -33,13 +33,21 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,structrmnet_map_dl_csum_trailer*csum_trailer,structrmnet_priv*priv){-__sum16*csum_field,csum_temp,pseudo_csum,hdr_csum,ip_payload_csum;-u16csum_value,csum_value_final;-structiphdr*ip4h;-void*txporthdr;+structiphdr*ip4h=(structiphdr*)skb->data;+void*txporthdr=skb->data+ip4h->ihl*4;+__sum16*csum_field,csum_temp,pseudo_csum;+__sum16ip_payload_csum;+u16csum_value_final;__be16addend;-ip4h=(structiphdr*)(skb->data);+/* Computing the checksum over just the IPv4 header--including its+*checksumfield--shouldyield0.Ifitdoesn't,theIPheader+*isbad,soreturnanerrorandlettheIPlayerdropit.+*/+if(ip_fast_csum(ip4h,ip4h->ihl)){+priv->stats.csum_ip4_header_bad++;+return-EINVAL;+}/* We don't support checksum offload on IPv4 fragments */if(ip_is_fragment(ip4h)){
@@ -47,25 +55,30 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,return-EOPNOTSUPP;}-txporthdr=skb->data+ip4h->ihl*4;-+/* Checksum offload is only supported for UDP and TCP protocols */csum_field=rmnet_map_get_csum_field(ip4h->protocol,txporthdr);-if(!csum_field){priv->stats.csum_err_invalid_transport++;return-EPROTONOSUPPORT;}-/* RFC 768 - Skip IPv4 UDP packets where sender checksum field is 0 */-if(*csum_field==0&&ip4h->protocol==IPPROTO_UDP){+/* RFC 768: UDP checksum is optional for IPv4, and is 0 if unused */+if(!*csum_field&&ip4h->protocol==IPPROTO_UDP){priv->stats.csum_skipped++;return0;}-csum_value=~ntohs(csum_trailer->csum_value);-hdr_csum=~ip_fast_csum(ip4h,(int)ip4h->ihl);-ip_payload_csum=csum16_sub((__force__sum16)csum_value,-(__force__be16)hdr_csum);+/* The checksum value in the trailer is computed over the entire+*IPpacket,includingtheIPheaderandpayload.Toderivethe+*transportchecksumfromthis,wefirstsubractthecontribution+*oftheIPheaderfromthetrailerchecksum.Wethenaddthe+*checksumcomputedoverthepseudoheader.+*+*WeverifiedabovethattheIPheadercontributeszerotothe+*trailerchecksum.Thereforethechecksuminthetraileris+*justthechecksumcomputedovertheIPpayload.+*/+ip_payload_csum=(__force__sum16)~ntohs(csum_trailer->csum_value);pseudo_csum=~csum_tcpudp_magic(ip4h->saddr,ip4h->daddr,ntohs(ip4h->tot_len)-ip4h->ihl*4,
From: Alex Elder <hidden> Date: 2021-06-11 19:06:49
In rmnet_map_ipv6_dl_csum_trailer() there is an especially involved
line of code that determines the ones' complement sum of the IPv6
packet header (in host byte order). Simplify that by storing the
result of computing just the header checksum in a local variable,
then using that in the original assignment.
Use the size of the IPv6 header structure as the number of bytes to
checksum, rather than computing the offset to the transport header.
And use ip_fast_csum() rather than ipa_compute_csum(), knowing that
the size of an IPv6 header (40 bytes) is a multiple of 4 bytes
greater than 16.
Add some comments to match rmnet_map_ipv4_dl_csum_trailer().
Signed-off-by: Alex Elder <redacted>
---
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 26 ++++++++++++-------
1 file changed, 16 insertions(+), 10 deletions(-)
@@ -120,27 +120,33 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,structrmnet_map_dl_csum_trailer*csum_trailer,structrmnet_priv*priv){-__sum16*csum_field,ip6_payload_csum,pseudo_csum,csum_temp;+structipv6hdr*ip6h=(structipv6hdr*)skb->data;+void*txporthdr=skb->data+sizeof(*ip6h);+__sum16*csum_field,pseudo_csum,csum_temp;u16csum_value,csum_value_final;__be16ip6_hdr_csum,addend;-structipv6hdr*ip6h;-void*txporthdr;+__sum16ip6_payload_csum;+__be16ip_header_csum;u32length;-ip6h=(structipv6hdr*)(skb->data);--txporthdr=skb->data+sizeof(structipv6hdr);+/* Checksum offload is only supported for UDP and TCP protocols;+*thepacketcannotincludeanyIPv6extensionheaders+*/csum_field=rmnet_map_get_csum_field(ip6h->nexthdr,txporthdr);-if(!csum_field){priv->stats.csum_err_invalid_transport++;return-EPROTONOSUPPORT;}+/* The checksum value in the trailer is computed over the entire+*IPpacket,includingtheIPheaderandpayload.Toderivethe+*transportchecksumfromthis,wefirstsubractthecontribution+*oftheIPheaderfromthetrailerchecksum.Wethenaddthe+*checksumcomputedoverthepseudoheader.+*/csum_value=~ntohs(csum_trailer->csum_value);-ip6_hdr_csum=(__force__be16)-~ntohs((__force__be16)ip_compute_csum(ip6h,-(int)(txporthdr-(void*)(skb->data))));+ip_header_csum=(__force__be16)ip_fast_csum(ip6h,sizeof(*ip6h)/4);+ip6_hdr_csum=(__force__be16)~ntohs(ip_header_csum);ip6_payload_csum=csum16_sub((__force__sum16)csum_value,ip6_hdr_csum);
From: Alex Elder <hidden> Date: 2021-06-11 19:06:52
Internet checksums are used for IPv4 header checksum, as well as TCP
segment and UDP datagram checksums. Such a checksum represents the
negated sum of adjacent pairs of bytes, using ones' complement
arithmetic.
One property of the Internet checkum is byte order independence [1].
Specifically, the sum of byte-swapped pairs is equal to the result
of byte swapping the sum of those same pairs when not byte-swapped.
So for example if a, b, c, d, y, and z are hexadecimal digits, and
PLUS represents ones' complement addition:
If: ab PLUS cd = yz
Then: ba PLUS dc = zy
For this reason, there is no need to swap the order of bytes in the
checksum value held in a message header, nor the one in the QMAPv4
trailer, in order to operate on them.
In other words, we can determine whether the hardware-computed
checksum matches the one in the message header without any byte
swaps.
(This patch leaves in place all existing type casts.)
[1] https://tools.ietf.org/html/rfc1071
Signed-off-by: Alex Elder <redacted>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
From: Alex Elder <hidden> Date: 2021-06-11 19:07:04
In the previous patch IPv4 download checksum offload code was
updated to avoid unnecessary byte swapping, based on properties of
the Internet checksum algorithm. This patch makes comparable
changes to the IPv6 download checksum offload handling.
Signed-off-by: Alex Elder <redacted>
---
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
@@ -123,10 +123,11 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,structipv6hdr*ip6h=(structipv6hdr*)skb->data;void*txporthdr=skb->data+sizeof(*ip6h);__sum16*csum_field,pseudo_csum,csum_temp;-u16csum_value,csum_value_final;__be16ip6_hdr_csum,addend;__sum16ip6_payload_csum;__be16ip_header_csum;+u16csum_value_final;+__be16csum_value;u32length;/* Checksum offload is only supported for UDP and TCP protocols;