Add support for receive checksum offloading. When enabled, pretend that the IPv4 and transport layer checksum is correct.
This is a prerequisite to add LRO support, which will be added after this. Then tap interfaces can be used by packetdrill to test the LRO code even in local mode.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
| sys/net/if_tuntap.c | ||
|---|---|---|
| 1822 | CSUM_SCTP_VALID == CSUM_SCTP_VALID. Is it a historical reason we maintains a CSUM_SCTP_VALID ? | |
| sys/net/if_tuntap.c | ||
|---|---|---|
| 1782 | The NULL check for eh seems to be redundant. It should never be NULL. | |
| sys/net/if_tuntap.c | ||
|---|---|---|
| 1782 | I found that also when integrating the change. I agree that it can't be NULL, that is why I'm not checking for it. However, removing the check here should be done in a separate small cleanup commit. I was planing to do that after this is in. | |
| 1822 | I guess you want to say that CSUM_DATA_VALID and CSUM_SCTP_VALID both map to CSUM_L4_VALID. So they are the same and the compiler can simplify the statement. However, there is no reason why thy must be mapped to the same value. That is why I added both. | |
| sys/net/if_tuntap.c | ||
|---|---|---|
| 980 | Only tap devices are going to have LRO then should we check TUN_L2 ? | |
| 1822 |
Yes, CSUM_DATA_VALID and CSUM_SCTP_VALID .
From the layer perspective, both TCP and SCTP are layer 4. For incoming packets, if the HW can do SCTP checksum offload, then set the flag CSUM_DATA_VALID should be enough. That's why I guess CSUM_SCTP_VALID is maintained for historical reason. | |
| sys/net/if_tuntap.c | ||
|---|---|---|
| 980 | Right now both the tun and tap interfaces can enable receive checksum offloading. LRO will only work for the tap device, since the LRO code requires an ethernet header to be present. | |
| 1822 | Some ethernet drivers prefer the information separately. So CSUM_SCTP_VALID is used. If you prefer, you can do an overall cleanup, but I would prefer to keep focussed in this commit. | |
| sys/net/if_tuntap.c | ||
|---|---|---|
| 980 |
I'd prefer consistency. I'm seeing inconsistency for tun and tap (of the feature RXSUM / RXCSUM_IPV6). | |
| 1822 | OK. | |
| sys/net/if_tuntap.c | ||
|---|---|---|
| 980 | Good point, thanks for catching that. I restricted setting the capabilities to the L2 device. I discussed this also with rgrimes. | |
Comment Actions
Looks good to me.
| sys/net/if_tuntap.c | ||
|---|---|---|
| 1796 | The last bits to check. #ifdef INET case ETHERTYPE_IP: #endif #ifdef INET6 case ETHERTYPE_IPV6 #endif | |
| sys/net/if_tuntap.c | ||
|---|---|---|
| 1796 | Done. Thanks for the review! | |