Hi Sergey Shtylyov, Thanks for the feedback. > >> Don't we need to set skb->csum_level? > > > > As per my knowledge, it is not needed. I may be wrong. Why do you > > think it is needed? > > * CHECKSUM_UNNECESSARY is applicable to following protocols: > * TCP: IPv6 and IPv4. > * UDP: IPv4 and IPv6. A device may apply CHECKSUM_UNNECESSARY to a > * zero UDP checksum for either IPv4 or IPv6, the networking stack > * may perform further validation in this case. > * GRE: only if the checksum is present in the header. > * SCTP: indicates the CRC in SCTP header has been validated. > * FCOE: indicates the CRC in FC frame has been validated. > * > * skb->csum_level indicates the number of consecutive checksums found > in > * the packet minus one that have been verified as CHECKSUM_UNNECESSARY. > * For instance if a device receives an IPv6->UDP->GRE->IPv4->TCP packet > * and a device is able to verify the checksums for UDP (possibly zero), > * GRE (checksum flag is set) and TCP, skb->csum_level would be set to > * two. If the device were only able to verify the UDP checksum and not > * GRE, either because it doesn't support GRE checksum or because GRE > * checksum is bad, skb->csum_level would be set to zero (TCP checksum > is > * not considered in this case). > > It would seem we should set this field to 1 if the TCP/UDP checksum was > successfully verified? I guess it is for encapsulated packets. For just IP and UDP/TCP Checksum it is not required. See https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/3com/3c59x.c#L2669 https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/amazon/ena/ena_netdev.c#L1626 https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/atheros/alx/main.c#L272 https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/ti/am65-cpsw-nuss.c#L711 > > >> [...] > >>> @@ -2518,6 +2593,8 @@ static const struct ravb_hw_info gbeth_hw_info = > { > >>> .emac_init = ravb_emac_init_gbeth, > >>> .gstrings_stats = ravb_gstrings_stats_gbeth, > >>> .gstrings_size = sizeof(ravb_gstrings_stats_gbeth), > >>> + .net_hw_features = NETIF_F_RXCSUM, > >>> + .net_features = NETIF_F_RXCSUM, > >> > >> What about NETIF_F_IP_CSUM, BTW? > > > > Why is it needed? Can you please clarify? > > Ignore me -- this one seems to be used for the TX path. > I just had to learn how the checksum offloading works while reviewing > your patches... :-) OK. For other comments. I will test and respond. Cheers, Biju
