Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ARP handling with UDP #2905

Merged
merged 1 commit into from
Sep 24, 2024
Merged

Fix ARP handling with UDP #2905

merged 1 commit into from
Sep 24, 2024

Conversation

scaprile
Copy link
Collaborator

No description provided.

@scaprile scaprile requested a review from cpq September 24, 2024 15:42
Comment on lines -4934 to 4935
#define MIP_TCP_ARP_MS 100 // Timeout for ARP response
#define MIP_ARP_RESP_MS 100 // Timeout for ARP response
#define MIP_TCP_SYN_MS 15000 // Timeout for connection establishment
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name itself induced the error...

Comment on lines -5287 to -5288
send_syn(c);
settmout(c, MIP_TTYPE_SYN);
Copy link
Collaborator Author

@scaprile scaprile Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UDP ARP lookup was causing bogus TCP SYNs

@@ -5869,18 +5870,20 @@ static void mg_tcpip_poll(struct mg_tcpip_if *ifp, uint64_t now) {

// Process timeouts
for (c = ifp->mgr->conns; c != NULL; c = c->next) {
if (c->is_udp || c->is_listening || c->is_resolving) continue;
if ((c->is_udp && !c->is_arplooking) || c->is_listening || c->is_resolving) continue;
struct connstate *s = (struct connstate *) (c + 1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARP lookups never timed out for UDP

Comment on lines +5878 to +5881
if (s->ttype == MIP_TTYPE_ARP) {
mg_error(c, "ARP timeout");
} else if (c->is_udp) {
continue;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First check for ARP and then exit if UDP

static void mac_resolved(struct mg_connection *c) {
if (c->is_udp) {
c->is_connecting = 0;
mg_call(c, MG_EV_CONNECT, NULL);
Copy link
Collaborator Author

@scaprile scaprile Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MG_EV_CONNECT was being fired for UDP connections only for remote hosts, when the destination MAC was set as the gateway MAC
is_connect wasn't being cleared either

Comment on lines +6086 to +6088
} else if (c->is_udp && (c->is_arplooking || c->is_resolving)) {
// Fail to send, no target MAC or IP
MG_VERBOSE(("still resolving..."));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your patch

@scaprile scaprile merged commit b2bb033 into master Sep 24, 2024
68 of 76 checks passed
@scaprile scaprile deleted the arpudp branch September 24, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant