On 6/12/23 16:31, Andrii Nakryiko wrote:
On Mon, Jun 12, 2023 at 2:56 PM Kui-Feng Lee <sinquersw@xxxxxxxxx> wrote:
On 6/12/23 13:31, Alexei Starovoitov wrote:
On Mon, Jun 12, 2023 at 12:16 PM Kui-Feng Lee <thinker.li@xxxxxxxxx> wrote:
+static int close_connection(int *closing_fd, int *peer_fd, int *listen_fd)
+{
+ int err;
+
+ /* Half shutdown to make sure the closing socket having a chance to
+ * receive a FIN from the client.
+ */
+ err = shutdown(*closing_fd, SHUT_WR);
+ if (CHECK(err, "shutdown closing_fd", "failed: %d\n", err))
+ return -1;
+ usleep(100000);
+ err = close(*peer_fd);
+ if (CHECK(err, "close peer_fd", "failed: %d\n", err))
+ return -1;
+ *peer_fd = -1;
+ usleep(100000);
+ err = close(*closing_fd);
usleep() won't guarantee it. The test will be flaky.
Can you make it reliable?
What if it checks a counter of packets going through the filter?
Will try a couple times until it is too long, one second
for example.
+
+/* Run accept() on a socket in the cgroup to receive a new connection. */
+#define EGRESS_ACCEPT \
+ case SYN_RECV_SENDING_SYN_ACK: \
+ if (tcph.fin || !tcph.syn || tcph.rst || !tcph.ack) \
+ g_unexpected++; \
+ else \
+ g_sock_state = SYN_RECV; \
+ break
+
+#define INGRESS_ACCEPT \
+ case INIT: \
+ if (!tcph.syn || tcph.fin || tcph.rst || tcph.ack) \
+ g_unexpected++; \
+ else \
+ g_sock_state = SYN_RECV_SENDING_SYN_ACK; \
+ break; \
+ case SYN_RECV: \
+ if (tcph.fin || tcph.syn || tcph.rst || !tcph.ack) \
+ g_unexpected++; \
+ else \
+ g_sock_state = ESTABLISHED; \
+ break
+
The macros make the code difficult to follow.
Could you do it with normal functions?
Sure!
please don't use CHECK() macros for new code, use ASSERT_xxx() family
Got it!
