On 09.10.2021 11:27, Biju Das wrote:
[...]
Fillup ravb_rx_gbeth() function to support RZ/G2L.
This patch also renames ravb_rcar_rx to ravb_rx_rcar to be
consistent with the naming convention used in sh_eth driver.
Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
Reviewed-by: Lad Prabhakar
<prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>[...]
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 37164a983156..42573eac82b9 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -720,6 +720,23 @@ static void ravb_get_tx_tstamp(struct
net_device
*ndev)
}
}
+static void ravb_rx_csum_gbeth(struct sk_buff *skb) {
+ u8 *hw_csum;
+
+ /* The hardware checksum is contained in sizeof(__sum16) (2)
bytes
+ * appended to packet data
+ */
+ if (unlikely(skb->len < sizeof(__sum16)))
+ return;
+ hw_csum = skb_tail_pointer(skb) - sizeof(__sum16);
[...]
Please check the section 30.5.6.1 checksum calculation handling>
And figure 30.25 the field of checksum attaching field
I have.
Also see Table 30.17 for checksum values for non-error conditions.
TCP/UDP/ICPM checksum is at last 2bytes.
What are you arguing with then? :-)
My point was that your code fetched the TCP/UDP/ICMP checksum
ISO the IP checksum because it subtracts sizeof(__sum16), while
should probably subtract sizeof(__wsum)
Agreed. My code missed IP4 checksum result. May be we need to
extract 2 checksum info from last 4 bytes. First checksum(2bytes)
is IP4 header checksum and next checksum(2 bytes) for TCP/UDP/ICMP
and use this info finding the non error case mentioned in Table
30.17.
For eg:-
IPV6 non error-condition --> "0xFFFF"-->IPV4HeaderCSum value and
"0x0000"
TCP/UDP/ICMP CSUM value
IPV4 non error-condition --> "0x0000"-->IPV4HeaderCSum value and
"0x0000"
TCP/UDP/ICMP CSUM value
Do you agree?
What I meant here is some thing like below, please let me know if you
have any issues with this, otherwise I would like to send the patch
with below changes.
Further improvements can happen later.
Please let me know.
+/* Hardware checksum status */
+#define IPV4_RX_CSUM_OK 0x00000000
+#define IPV6_RX_CSUM_OK 0xFFFF0000
Mhm, this should prolly come from the IP headers...
[...]
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index bbb42e5328e4..d9201fbbd472 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -722,16 +722,18 @@ static void ravb_get_tx_tstamp(struct
net_device *ndev)
static void ravb_rx_csum_gbeth(struct sk_buff *skb) {
- u16 *hw_csum;
+ u32 csum_result;
This is not against the patch currently under investigation. :-)
+ u8 *hw_csum;
/* The hardware checksum is contained in sizeof(__sum16) (2)
bytes
* appended to packet data
*/
- if (unlikely(skb->len < sizeof(__sum16)))
+ if (unlikely(skb->len < sizeof(__wsum)))
I think this usage of __wsum is valid (I remember that I suggested
it). We have 2 16-bit checksums here
I meant "I don't think", of course. :-)
Ok will use 2 * sizeof(__sum16) instead and extract IPV4 header csum and TCP/UDP/ICMP csum result.
I'm not sure how to deal with the later...
All error condition/unsupported cases will be passed to stack with CHECKSUM_NONE
and only non-error cases will be set as CHECKSUM_UNNCESSARY.
Does it sounds good to you?
No. The networking stack needs to know about the bad checksums too.
Regards,
Biju
[...]
MBR, Sergey
