On Tue, 27 Mar 2018 23:06:38 +0530
Atul Gupta <atul.gupta@xxxxxxxxxxx> wrote:
> +static u8 tcp_state_to_flowc_state(u8 state)
> +{
> + u8 ret = FW_FLOWC_MNEM_TCPSTATE_ESTABLISHED;
> +
> + switch (state) {
> + case TCP_ESTABLISHED:
> + ret = FW_FLOWC_MNEM_TCPSTATE_ESTABLISHED;
> + break;
> + case TCP_CLOSE_WAIT:
> + ret = FW_FLOWC_MNEM_TCPSTATE_CLOSEWAIT;
> + break;
> + case TCP_FIN_WAIT1:
> + ret = FW_FLOWC_MNEM_TCPSTATE_FINWAIT1;
> + break;
> + case TCP_CLOSING:
> + ret = FW_FLOWC_MNEM_TCPSTATE_CLOSING;
> + break;
> + case TCP_LAST_ACK:
> + ret = FW_FLOWC_MNEM_TCPSTATE_LASTACK;
> + break;
> + case TCP_FIN_WAIT2:
> + ret = FW_FLOWC_MNEM_TCPSTATE_FINWAIT2;
> + break;
Can't you just return those values right away instead?
> [...]
>
> +static u64 tlstx_seq_number(struct chtls_hws *hws)
> +{
> + return hws->tx_seq_no++;
> +}
The name of this function, as I also had commented earlier, is
misleading, because you are also incrementing the sequence number.
> [...]
>
> +static void mark_urg(struct tcp_sock *tp, int flags,
> + struct sk_buff *skb)
> +{
> + if (unlikely(flags & MSG_OOB)) {
> + tp->snd_up = tp->write_seq;
> + ULP_SKB_CB(skb)->flags = ULPCB_FLAG_URG |
> + ULPCB_FLAG_BARRIER |
> + ULPCB_FLAG_NO_APPEND |
> + ULPCB_FLAG_NEED_HDR;
Is this indentation the result of a previous 'if' clause which is now
gone?
> [...]
>
> +/*
> + * Returns true if a TCP socket is corked.
> + */
> +static int corked(const struct tcp_sock *tp, int flags)
> +{
> + return (flags & MSG_MORE) | (tp->nonagle & TCP_NAGLE_CORK);
I guess you meant || here. Shouldn't this be a bool?
> +}
> +
> +/*
> + * Returns true if a send should try to push new data.
> + */
> +static int send_should_push(struct sock *sk, int flags)
> +{
> + return should_push(sk) && !corked(tcp_sk(sk), flags);
> +}
If it returns true, I guess it should be a bool.
> [...]
--
Stefano
