Quicly hotfix pcb - fix chksum verify #343
Conversation
lib/cnet/tcp/tcp_input.c
Outdated
| @@ -90,14 +92,14 @@ tcp_input_lookup(struct cne_node *node, pktmbuf_t *m, struct pcb_hd *hd) | |||
| pcb = cnet_pcb_lookup(hd, &key, BEST_MATCH); | |||
| if (likely(pcb)) { | |||
| int rc = TCP_INPUT_NEXT_PKT_DROP; | |||
|
|
|||
| #if 0 // Disable till cne_ipv4_udptcp_cksum_verify is fixed | |||
| if (is_pcb_dom_inet6(pcb)) | |||
| csum = cne_ipv6_udptcp_cksum_verify(&tip->ip6, &tip->tcp); | |||
| else | |||
| csum = cne_ipv4_udptcp_cksum_verify(&tip->ip4, &tip->tcp); | |||
| if (csum) | |||
There was a problem hiding this comment.
Also csum here needs to be signed, at the fns above return -1 on a fail...
There was a problem hiding this comment.
Yes I agree it should be signed, but maybe the function should be returning a 0 or 1 for fail/pass?
There was a problem hiding this comment.
maybe I should just rename the csum variable to something else as it's not really a csum returned by those functions
There was a problem hiding this comment.
Yes, I agree it is not a checksum. We can do that change now or after this one gets merged up to you or I can create a PR for that just that fix.
There was a problem hiding this comment.
I can push a separate PR to rename it after this no prob
maryamtahhan
force-pushed
the
quicly
branch
2 times, most recently
from
f4ea904 to
57c246b
Compare
|
I've only tested UDP and not TCP - would someone be able to check the TCP path? |
maryamtahhan
force-pushed
the
quicly
branch
from
d8c363c to
4ff78ab
Compare
|
@pbanicev Can you verify this works for you?
|
Signed-off-by: Pavo Banicevic <[email protected]>
Signed-off-by: Maryam Tahhan <[email protected]>
Signed-off-by: Maryam Tahhan <[email protected]>
Signed-off-by: Maryam Tahhan <[email protected]>
Signed-off-by: Maryam Tahhan <[email protected]>
Signed-off-by: Maryam Tahhan <[email protected]>
Signed-off-by: Maryam Tahhan <[email protected]>
maryamtahhan
force-pushed
the
quicly
branch
from
df3e5ba to
eaf636c
Compare
|
The code for TCP looks correct, but I can check it after the merge if that seems reasonable? |
seems reasonable :) |
maryamtahhan
changed the title
|
Thank you @maryamtahhan |
Depends on #340
Fixes the PCB lookup failure in the UDP input node for cnet and the failing csum verifications ...
Note: it looks like the same issue exists in the tcp input node also... so I applied the same fix
Note: Only tested UDP path with quicly client application...