* [PATCH net-next 1/8] net: qualcomm: rmnet: remove some local variables
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
@ 2021-06-12 14:37 ` Alex Elder
2021-06-12 14:37 ` [PATCH net-next 2/8] net: qualcomm: rmnet: rearrange some NOTs Alex Elder
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alex Elder @ 2021-06-12 14:37 UTC (permalink / raw)
To: subashab, stranche, davem, kuba; +Cc: bjorn.andersson, netdev, linux-kernel
In rmnet_map_ipv4_dl_csum_trailer(), remove the "csum_temp" and
"addend" local variables, and simplify a few lines of code.
Remove the "csum_temp", "csum_value", "ip6_hdr_csum", and "addend"
local variables in rmnet_map_ipv6_dl_csum_trailer(), and simplify a
few lines of code there as well.
Signed-off-by: Alex Elder <elder@linaro.org>
---
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 37 +++++++------------
1 file changed, 13 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index d4d23ab446ef5..3e6feef0fd252 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -35,10 +35,9 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
{
struct iphdr *ip4h = (struct iphdr *)skb->data;
void *txporthdr = skb->data + ip4h->ihl * 4;
- __sum16 *csum_field, csum_temp, pseudo_csum;
+ __sum16 *csum_field, pseudo_csum;
__sum16 ip_payload_csum;
- u16 csum_value_final;
- __be16 addend;
+ __sum16 csum_value_final;
/* Computing the checksum over just the IPv4 header--including its
* checksum field--should yield 0. If it doesn't, the IP header
@@ -83,14 +82,11 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
pseudo_csum = ~csum_tcpudp_magic(ip4h->saddr, ip4h->daddr,
ntohs(ip4h->tot_len) - ip4h->ihl * 4,
ip4h->protocol, 0);
- addend = (__force __be16)pseudo_csum;
- pseudo_csum = csum16_add(ip_payload_csum, addend);
+ pseudo_csum = csum16_add(ip_payload_csum, (__force __be16)pseudo_csum);
- addend = (__force __be16)*csum_field;
- csum_temp = ~csum16_sub(pseudo_csum, addend);
- csum_value_final = (__force u16)csum_temp;
+ csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
- if (unlikely(csum_value_final == 0)) {
+ if (unlikely(!csum_value_final)) {
switch (ip4h->protocol) {
case IPPROTO_UDP:
/* RFC 768 - DL4 1's complement rule for UDP csum 0 */
@@ -105,7 +101,7 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
}
}
- if (csum_value_final == (__force u16)*csum_field) {
+ if (csum_value_final == *csum_field) {
priv->stats.csum_ok++;
return 0;
} else {
@@ -122,12 +118,10 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
{
struct ipv6hdr *ip6h = (struct ipv6hdr *)skb->data;
void *txporthdr = skb->data + sizeof(*ip6h);
- __sum16 *csum_field, pseudo_csum, csum_temp;
- __be16 ip6_hdr_csum, addend;
+ __sum16 *csum_field, pseudo_csum;
__sum16 ip6_payload_csum;
__be16 ip_header_csum;
- u16 csum_value_final;
- __be16 csum_value;
+ __sum16 csum_value_final;
u32 length;
/* Checksum offload is only supported for UDP and TCP protocols;
@@ -145,23 +139,18 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
* of the IP header from the trailer checksum. We then add the
* checksum computed over the pseudo header.
*/
- csum_value = ~csum_trailer->csum_value;
ip_header_csum = (__force __be16)ip_fast_csum(ip6h, sizeof(*ip6h) / 4);
- ip6_hdr_csum = (__force __be16)~ip_header_csum;
- ip6_payload_csum = csum16_sub((__force __sum16)csum_value,
- ip6_hdr_csum);
+ ip6_payload_csum = csum16_sub((__force __sum16)~csum_trailer->csum_value,
+ ~ip_header_csum);
length = (ip6h->nexthdr == IPPROTO_UDP) ?
ntohs(((struct udphdr *)txporthdr)->len) :
ntohs(ip6h->payload_len);
pseudo_csum = ~csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
length, ip6h->nexthdr, 0);
- addend = (__force __be16)pseudo_csum;
- pseudo_csum = csum16_add(ip6_payload_csum, addend);
+ pseudo_csum = csum16_add(ip6_payload_csum, (__force __be16)pseudo_csum);
- addend = (__force __be16)*csum_field;
- csum_temp = ~csum16_sub(pseudo_csum, addend);
- csum_value_final = (__force u16)csum_temp;
+ csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
if (unlikely(csum_value_final == 0)) {
switch (ip6h->nexthdr) {
@@ -180,7 +169,7 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
}
}
- if (csum_value_final == (__force u16)*csum_field) {
+ if (csum_value_final == *csum_field) {
priv->stats.csum_ok++;
return 0;
} else {
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 2/8] net: qualcomm: rmnet: rearrange some NOTs
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
2021-06-12 14:37 ` [PATCH net-next 1/8] net: qualcomm: rmnet: remove some local variables Alex Elder
@ 2021-06-12 14:37 ` Alex Elder
2021-06-12 14:37 ` [PATCH net-next 3/8] net: qualcomm: rmnet: show that an intermediate sum is zero Alex Elder
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alex Elder @ 2021-06-12 14:37 UTC (permalink / raw)
To: subashab, stranche, davem, kuba; +Cc: bjorn.andersson, netdev, linux-kernel
With the ones' complement arithmetic, the sum of two negated values
is equal to the negation of the sum of the two original values [1].
Rearrange the calculation ip6_payload_sum using this property.
[1] https://tools.ietf.org/html/rfc1071
Signed-off-by: Alex Elder <elder@linaro.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 3e6feef0fd252..1b170e9189d8a 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -140,8 +140,8 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
* checksum computed over the pseudo header.
*/
ip_header_csum = (__force __be16)ip_fast_csum(ip6h, sizeof(*ip6h) / 4);
- ip6_payload_csum = csum16_sub((__force __sum16)~csum_trailer->csum_value,
- ~ip_header_csum);
+ ip6_payload_csum = ~csum16_sub((__force __sum16)csum_trailer->csum_value,
+ ip_header_csum);
length = (ip6h->nexthdr == IPPROTO_UDP) ?
ntohs(((struct udphdr *)txporthdr)->len) :
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 3/8] net: qualcomm: rmnet: show that an intermediate sum is zero
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
2021-06-12 14:37 ` [PATCH net-next 1/8] net: qualcomm: rmnet: remove some local variables Alex Elder
2021-06-12 14:37 ` [PATCH net-next 2/8] net: qualcomm: rmnet: rearrange some NOTs Alex Elder
@ 2021-06-12 14:37 ` Alex Elder
2021-06-12 14:37 ` [PATCH net-next 4/8] net: qualcomm: rmnet: return earlier for bad checksum Alex Elder
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alex Elder @ 2021-06-12 14:37 UTC (permalink / raw)
To: subashab, stranche, davem, kuba; +Cc: bjorn.andersson, netdev, linux-kernel
This patch simply demonstrates that a checksum value computed when
verifying an offloaded transport checksum value for both IPv4 and
IPv6 is (normally) 0. It can be squashed into the next patch.
Signed-off-by: Alex Elder <elder@linaro.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 1b170e9189d8a..51909b8fa8a80 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -84,6 +84,11 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
ip4h->protocol, 0);
pseudo_csum = csum16_add(ip_payload_csum, (__force __be16)pseudo_csum);
+ /* The trailer checksum *includes* the checksum in the transport
+ * header. Adding that to the pseudo checksum will yield 0xffff
+ * ("negative 0") if the message arrived intact.
+ */
+ WARN_ON((__sum16)~pseudo_csum);
csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
if (unlikely(!csum_value_final)) {
@@ -150,6 +155,10 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
length, ip6h->nexthdr, 0);
pseudo_csum = csum16_add(ip6_payload_csum, (__force __be16)pseudo_csum);
+ /* Adding the payload checksum to the pseudo checksum yields 0xffff
+ * ("negative 0") if the message arrived intact.
+ */
+ WARN_ON((__sum16)~pseudo_csum);
csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
if (unlikely(csum_value_final == 0)) {
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 4/8] net: qualcomm: rmnet: return earlier for bad checksum
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
` (2 preceding siblings ...)
2021-06-12 14:37 ` [PATCH net-next 3/8] net: qualcomm: rmnet: show that an intermediate sum is zero Alex Elder
@ 2021-06-12 14:37 ` Alex Elder
2021-06-12 14:37 ` [PATCH net-next 5/8] net: qualcomm: rmnet: remove unneeded code Alex Elder
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alex Elder @ 2021-06-12 14:37 UTC (permalink / raw)
To: subashab, stranche, davem, kuba; +Cc: bjorn.andersson, netdev, linux-kernel
In rmnet_map_ipv4_dl_csum_trailer(), if the sum of the trailer
checksum and the pseudo checksum is non-zero, checksum validation
has failed. We can return an error as soon as we know that.
We can do the same thing in rmnet_map_ipv6_dl_csum_trailer().
Add some comments that explain where we're headed.
Signed-off-by: Alex Elder <elder@linaro.org>
---
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 36 ++++++++++++++-----
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 51909b8fa8a80..a05124eb8602e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -76,6 +76,17 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
* We verified above that the IP header contributes zero to the
* trailer checksum. Therefore the checksum in the trailer is
* just the checksum computed over the IP payload.
+
+ * If the IP payload arrives intact, adding the pseudo header
+ * checksum to the IP payload checksum will yield 0xffff (negative
+ * zero). This means the trailer checksum and the pseudo checksum
+ * are additive inverses of each other. Put another way, the
+ * message passes the checksum test if the trailer checksum value
+ * is the negated pseudo header checksum.
+ *
+ * Knowing this, we don't even need to examine the transport
+ * header checksum value; it is already accounted for in the
+ * checksum value found in the trailer.
*/
ip_payload_csum = (__force __sum16)~csum_trailer->csum_value;
@@ -84,11 +95,11 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
ip4h->protocol, 0);
pseudo_csum = csum16_add(ip_payload_csum, (__force __be16)pseudo_csum);
- /* The trailer checksum *includes* the checksum in the transport
- * header. Adding that to the pseudo checksum will yield 0xffff
- * ("negative 0") if the message arrived intact.
- */
- WARN_ON((__sum16)~pseudo_csum);
+ /* The cast is required to ensure only the low 16 bits are examined */
+ if ((__sum16)~pseudo_csum) {
+ priv->stats.csum_validation_failed++;
+ return -EINVAL;
+ }
csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
if (unlikely(!csum_value_final)) {
@@ -143,6 +154,11 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
* transport checksum from this, we first subract the contribution
* of the IP header from the trailer checksum. We then add the
* checksum computed over the pseudo header.
+ *
+ * It's sufficient to compare the IP payload checksum with the
+ * negated pseudo checksum to determine whether the packet
+ * checksum was good. (See further explanation in comments
+ * in rmnet_map_ipv4_dl_csum_trailer()).
*/
ip_header_csum = (__force __be16)ip_fast_csum(ip6h, sizeof(*ip6h) / 4);
ip6_payload_csum = ~csum16_sub((__force __sum16)csum_trailer->csum_value,
@@ -155,10 +171,12 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
length, ip6h->nexthdr, 0);
pseudo_csum = csum16_add(ip6_payload_csum, (__force __be16)pseudo_csum);
- /* Adding the payload checksum to the pseudo checksum yields 0xffff
- * ("negative 0") if the message arrived intact.
- */
- WARN_ON((__sum16)~pseudo_csum);
+ /* The cast is required to ensure only the low 16 bits are examined */
+ if ((__sum16)~pseudo_csum) {
+ priv->stats.csum_validation_failed++;
+ return -EINVAL;
+ }
+
csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
if (unlikely(csum_value_final == 0)) {
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 5/8] net: qualcomm: rmnet: remove unneeded code
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
` (3 preceding siblings ...)
2021-06-12 14:37 ` [PATCH net-next 4/8] net: qualcomm: rmnet: return earlier for bad checksum Alex Elder
@ 2021-06-12 14:37 ` Alex Elder
2021-06-12 14:37 ` [PATCH net-next 6/8] net: qualcomm: rmnet: trailer value is a checksum Alex Elder
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alex Elder @ 2021-06-12 14:37 UTC (permalink / raw)
To: subashab, stranche, davem, kuba; +Cc: bjorn.andersson, netdev, linux-kernel
The previous patch makes rmnet_map_ipv4_dl_csum_trailer() return
early with an error if it is determined that the computed checksum
for the IP payload does not match what was expected.
If the computed checksum *does* match the expected value, the IP
payload (i.e., the transport message), can be considered good.
There is no need to do any further processing of the message.
This means a big block of code is unnecessary for validating the
transport checksum value, and can be removed.
Make comparable changes in rmnet_map_ipv6_dl_csum_trailer().
Signed-off-by: Alex Elder <elder@linaro.org>
---
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 75 ++++---------------
1 file changed, 14 insertions(+), 61 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index a05124eb8602e..033b8ad3d7356 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -37,7 +37,6 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
void *txporthdr = skb->data + ip4h->ihl * 4;
__sum16 *csum_field, pseudo_csum;
__sum16 ip_payload_csum;
- __sum16 csum_value_final;
/* Computing the checksum over just the IPv4 header--including its
* checksum field--should yield 0. If it doesn't, the IP header
@@ -93,37 +92,15 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
pseudo_csum = ~csum_tcpudp_magic(ip4h->saddr, ip4h->daddr,
ntohs(ip4h->tot_len) - ip4h->ihl * 4,
ip4h->protocol, 0);
- pseudo_csum = csum16_add(ip_payload_csum, (__force __be16)pseudo_csum);
/* The cast is required to ensure only the low 16 bits are examined */
- if ((__sum16)~pseudo_csum) {
+ if (ip_payload_csum != (__sum16)~pseudo_csum) {
priv->stats.csum_validation_failed++;
return -EINVAL;
}
- csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
- if (unlikely(!csum_value_final)) {
- switch (ip4h->protocol) {
- case IPPROTO_UDP:
- /* RFC 768 - DL4 1's complement rule for UDP csum 0 */
- csum_value_final = ~csum_value_final;
- break;
-
- case IPPROTO_TCP:
- /* DL4 Non-RFC compliant TCP checksum found */
- if (*csum_field == (__force __sum16)0xFFFF)
- csum_value_final = ~csum_value_final;
- break;
- }
- }
-
- if (csum_value_final == *csum_field) {
- priv->stats.csum_ok++;
- return 0;
- } else {
- priv->stats.csum_validation_failed++;
- return -EINVAL;
- }
+ priv->stats.csum_ok++;
+ return 0;
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -137,7 +114,6 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
__sum16 *csum_field, pseudo_csum;
__sum16 ip6_payload_csum;
__be16 ip_header_csum;
- __sum16 csum_value_final;
u32 length;
/* Checksum offload is only supported for UDP and TCP protocols;
@@ -154,11 +130,6 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
* transport checksum from this, we first subract the contribution
* of the IP header from the trailer checksum. We then add the
* checksum computed over the pseudo header.
- *
- * It's sufficient to compare the IP payload checksum with the
- * negated pseudo checksum to determine whether the packet
- * checksum was good. (See further explanation in comments
- * in rmnet_map_ipv4_dl_csum_trailer()).
*/
ip_header_csum = (__force __be16)ip_fast_csum(ip6h, sizeof(*ip6h) / 4);
ip6_payload_csum = ~csum16_sub((__force __sum16)csum_trailer->csum_value,
@@ -169,40 +140,22 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
ntohs(ip6h->payload_len);
pseudo_csum = ~csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
length, ip6h->nexthdr, 0);
- pseudo_csum = csum16_add(ip6_payload_csum, (__force __be16)pseudo_csum);
- /* The cast is required to ensure only the low 16 bits are examined */
- if ((__sum16)~pseudo_csum) {
+ /* It's sufficient to compare the IP payload checksum with the
+ * negated pseudo checksum to determine whether the packet
+ * checksum was good. (See further explanation in comments
+ * in rmnet_map_ipv4_dl_csum_trailer()).
+ *
+ * The cast is required to ensure only the low 16 bits are
+ * examined.
+ */
+ if (ip6_payload_csum != (__sum16)~pseudo_csum) {
priv->stats.csum_validation_failed++;
return -EINVAL;
}
- csum_value_final = ~csum16_sub(pseudo_csum, (__force __be16)*csum_field);
-
- if (unlikely(csum_value_final == 0)) {
- switch (ip6h->nexthdr) {
- case IPPROTO_UDP:
- /* RFC 2460 section 8.1
- * DL6 One's complement rule for UDP checksum 0
- */
- csum_value_final = ~csum_value_final;
- break;
-
- case IPPROTO_TCP:
- /* DL6 Non-RFC compliant TCP checksum found */
- if (*csum_field == (__force __sum16)0xFFFF)
- csum_value_final = ~csum_value_final;
- break;
- }
- }
-
- if (csum_value_final == *csum_field) {
- priv->stats.csum_ok++;
- return 0;
- } else {
- priv->stats.csum_validation_failed++;
- return -EINVAL;
- }
+ priv->stats.csum_ok++;
+ return 0;
}
#endif
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 6/8] net: qualcomm: rmnet: trailer value is a checksum
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
` (4 preceding siblings ...)
2021-06-12 14:37 ` [PATCH net-next 5/8] net: qualcomm: rmnet: remove unneeded code Alex Elder
@ 2021-06-12 14:37 ` Alex Elder
2021-06-12 14:37 ` [PATCH net-next 7/8] net: qualcomm: rmnet: drop some unary NOTs Alex Elder
2021-06-12 14:37 ` [PATCH net-next 8/8] net: qualcomm: rmnet: IPv6 payload length is simple Alex Elder
7 siblings, 0 replies; 9+ messages in thread
From: Alex Elder @ 2021-06-12 14:37 UTC (permalink / raw)
To: subashab, stranche, davem, kuba; +Cc: bjorn.andersson, netdev, linux-kernel
The csum_value field in the rmnet_map_dl_csum_trailer structure is a
"real" Internet checksum. It is a 16 bit value, in big endian format,
which represents an inverted ones' complement sum over pairs of bytes.
Make that clear by changing its type to __sum16.
This makes a typecast in rmnet_map_ipv4_dl_csum_trailer() and
another in rmnet_map_ipv6_dl_csum_trailer() unnecessary.
Signed-off-by: Alex Elder <elder@linaro.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 5 ++---
include/linux/if_rmnet.h | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 033b8ad3d7356..610c8b5a8f46a 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -87,7 +87,7 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
* header checksum value; it is already accounted for in the
* checksum value found in the trailer.
*/
- ip_payload_csum = (__force __sum16)~csum_trailer->csum_value;
+ ip_payload_csum = ~csum_trailer->csum_value;
pseudo_csum = ~csum_tcpudp_magic(ip4h->saddr, ip4h->daddr,
ntohs(ip4h->tot_len) - ip4h->ihl * 4,
@@ -132,8 +132,7 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
* checksum computed over the pseudo header.
*/
ip_header_csum = (__force __be16)ip_fast_csum(ip6h, sizeof(*ip6h) / 4);
- ip6_payload_csum = ~csum16_sub((__force __sum16)csum_trailer->csum_value,
- ip_header_csum);
+ ip6_payload_csum = ~csum16_sub(csum_trailer->csum_value, ip_header_csum);
length = (ip6h->nexthdr == IPPROTO_UDP) ?
ntohs(((struct udphdr *)txporthdr)->len) :
diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
index be17610a981ef..10e7521ecb6cd 100644
--- a/include/linux/if_rmnet.h
+++ b/include/linux/if_rmnet.h
@@ -25,7 +25,7 @@ struct rmnet_map_dl_csum_trailer {
u8 flags; /* MAP_CSUM_DL_VALID_FLAG */
__be16 csum_start_offset;
__be16 csum_length;
- __be16 csum_value;
+ __sum16 csum_value;
} __aligned(1);
/* rmnet_map_dl_csum_trailer flags field:
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 7/8] net: qualcomm: rmnet: drop some unary NOTs
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
` (5 preceding siblings ...)
2021-06-12 14:37 ` [PATCH net-next 6/8] net: qualcomm: rmnet: trailer value is a checksum Alex Elder
@ 2021-06-12 14:37 ` Alex Elder
2021-06-12 14:37 ` [PATCH net-next 8/8] net: qualcomm: rmnet: IPv6 payload length is simple Alex Elder
7 siblings, 0 replies; 9+ messages in thread
From: Alex Elder @ 2021-06-12 14:37 UTC (permalink / raw)
To: subashab, stranche, davem, kuba; +Cc: bjorn.andersson, netdev, linux-kernel
We compare a payload checksum with a pseudo checksum value for
equality in rmnet_map_ipv4_dl_csum_trailer(). Both of those values
are computed with a unary NOT (~) operation. The result of the
comparison is the same if we omit that NOT for both values.
Remove these operations in rmnet_map_ipv6_dl_csum_trailer() also.
Signed-off-by: Alex Elder <elder@linaro.org>
---
.../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 610c8b5a8f46a..ed4737d0043d6 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -87,11 +87,11 @@ rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
* header checksum value; it is already accounted for in the
* checksum value found in the trailer.
*/
- ip_payload_csum = ~csum_trailer->csum_value;
+ ip_payload_csum = csum_trailer->csum_value;
- pseudo_csum = ~csum_tcpudp_magic(ip4h->saddr, ip4h->daddr,
- ntohs(ip4h->tot_len) - ip4h->ihl * 4,
- ip4h->protocol, 0);
+ pseudo_csum = csum_tcpudp_magic(ip4h->saddr, ip4h->daddr,
+ ntohs(ip4h->tot_len) - ip4h->ihl * 4,
+ ip4h->protocol, 0);
/* The cast is required to ensure only the low 16 bits are examined */
if (ip_payload_csum != (__sum16)~pseudo_csum) {
@@ -132,13 +132,13 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
* checksum computed over the pseudo header.
*/
ip_header_csum = (__force __be16)ip_fast_csum(ip6h, sizeof(*ip6h) / 4);
- ip6_payload_csum = ~csum16_sub(csum_trailer->csum_value, ip_header_csum);
+ ip6_payload_csum = csum16_sub(csum_trailer->csum_value, ip_header_csum);
length = (ip6h->nexthdr == IPPROTO_UDP) ?
ntohs(((struct udphdr *)txporthdr)->len) :
ntohs(ip6h->payload_len);
- pseudo_csum = ~csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
- length, ip6h->nexthdr, 0);
+ pseudo_csum = csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
+ length, ip6h->nexthdr, 0);
/* It's sufficient to compare the IP payload checksum with the
* negated pseudo checksum to determine whether the packet
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 8/8] net: qualcomm: rmnet: IPv6 payload length is simple
2021-06-12 14:37 [PATCH net-next 0/8] net: qualcomm: rmnet: MAPv4 download checksum cleanup, part 2 Alex Elder
` (6 preceding siblings ...)
2021-06-12 14:37 ` [PATCH net-next 7/8] net: qualcomm: rmnet: drop some unary NOTs Alex Elder
@ 2021-06-12 14:37 ` Alex Elder
7 siblings, 0 replies; 9+ messages in thread
From: Alex Elder @ 2021-06-12 14:37 UTC (permalink / raw)
To: subashab, stranche, davem, kuba; +Cc: bjorn.andersson, netdev, linux-kernel
We don't support any extension headers for IPv6 packets. Extension
headers therefore contribute 0 bytes to the payload length. As a
result we can just use the IPv6 payload length as the length used to
compute the pseudo header checksum for both UDP and TCP messages.
Signed-off-by: Alex Elder <elder@linaro.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index ed4737d0043d6..a6ce22f60a00c 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -114,7 +114,6 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
__sum16 *csum_field, pseudo_csum;
__sum16 ip6_payload_csum;
__be16 ip_header_csum;
- u32 length;
/* Checksum offload is only supported for UDP and TCP protocols;
* the packet cannot include any IPv6 extension headers
@@ -134,11 +133,9 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
ip_header_csum = (__force __be16)ip_fast_csum(ip6h, sizeof(*ip6h) / 4);
ip6_payload_csum = csum16_sub(csum_trailer->csum_value, ip_header_csum);
- length = (ip6h->nexthdr == IPPROTO_UDP) ?
- ntohs(((struct udphdr *)txporthdr)->len) :
- ntohs(ip6h->payload_len);
pseudo_csum = csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
- length, ip6h->nexthdr, 0);
+ ntohs(ip6h->payload_len),
+ ip6h->nexthdr, 0);
/* It's sufficient to compare the IP payload checksum with the
* negated pseudo checksum to determine whether the packet
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread