Skip to content

Commit

Permalink
icmp: Don't discard first reply sequence for a given echo ID
Browse files Browse the repository at this point in the history
In pasta mode, ICMP and ICMPv6 echo sockets relay back to us any
reply we send: we're on the same host as the target, after all. We
discard them by comparing the last sequence we sent with the sequence
we receive.

However, on the first reply for a given identifier, the sequence
might be zero, depending on the implementation of ping(8): we need
another value to indicate we haven't sent any sequence number, yet.

Use -1 as initialiser in the echo identifier map.

This is visible with Busybox's ping, and was reported by Paul on the
integration at containers/podman#16141, with:

  $ podman run --net=pasta alpine ping -c 2 192.168.188.1

...where only the second reply would be routed back.

Reported-by: Paul Holzinger <[email protected]>
Fixes: 33482d5 ("passt: Add PASTA mode, major rework")
Signed-off-by: Stefano Brivio <[email protected]>
Reviewed-by: David Gibson <[email protected]>
  • Loading branch information
sbrivio-rh committed Oct 26, 2022
1 parent b062ee4 commit f212044
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
16 changes: 14 additions & 2 deletions icmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@
/**
* struct icmp_id_sock - Tracking information for single ICMP echo identifier
* @sock: Bound socket for identifier
* @seq: Last sequence number sent to tap, host order
* @seq: Last sequence number sent to tap, host order, -1: not sent yet
* @ts: Last associated activity from tap, seconds
*/
struct icmp_id_sock {
int sock;
uint16_t seq;
int seq;
time_t ts;
};

Expand Down Expand Up @@ -273,6 +273,7 @@ static void icmp_timer_one(const struct ctx *c, int v6, uint16_t id,
epoll_ctl(c->epollfd, EPOLL_CTL_DEL, id_map->sock, NULL);
close(id_map->sock);
id_map->sock = 0;
id_map->seq = -1;
}

/**
Expand Down Expand Up @@ -301,3 +302,14 @@ void icmp_timer(const struct ctx *c, const struct timespec *ts)
goto v6;
}
}

/**
* icmp_init() - Initialise sequences in ID map to -1 (no sequence sent yet)
*/
void icmp_init(void)
{
unsigned i;

for (i = 0; i < ICMP_NUM_IDS; i++)
icmp_id_map[V4][i].seq = icmp_id_map[V6][i].seq = -1;
}
1 change: 1 addition & 0 deletions icmp.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref,
int icmp_tap_handler(const struct ctx *c, int af, const void *addr,
const struct pool *p, const struct timespec *now);
void icmp_timer(const struct ctx *c, const struct timespec *ts);
void icmp_init(void);

/**
* union icmp_epoll_ref - epoll reference portion for ICMP tracking
Expand Down
3 changes: 3 additions & 0 deletions passt.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ int main(int argc, char **argv)
if ((!c.no_udp && udp_init(&c)) || (!c.no_tcp && tcp_init(&c)))
exit(EXIT_FAILURE);

if (!c.no_icmp)
icmp_init();

proto_update_l2_buf(c.mac_guest, c.mac, &c.ip4.addr);

if (c.ifi4 && !c.no_dhcp)
Expand Down

0 comments on commit f212044

Please sign in to comment.