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
