Messages in this thread Patch in this message | | | Date | Tue, 25 Jul 2023 19:02:52 +0200 | From | Simon Horman <> | Subject | Re: [PATCH v8.1 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing packets |
| |
On Fri, Jul 21, 2023 at 05:18:57PM +0100, Dmitry Safonov wrote:
...
Hi Dmitry,
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
...
> @@ -619,7 +621,33 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp, > opts->hash_location = (__u8 *)ptr; > ptr += 4; > } > +#ifdef CONFIG_TCP_AO > + if (unlikely(OPTION_AO & options) && tp) {
Smatch warns that here we check if tp is NULL, but later on in the same function (existing) code uses tp unconditionally.
That code looks like this:
if (unlikely(opts->num_sack_blocks)) { struct tcp_sack_block *sp = tp->rx_opt.dsack ? tp->duplicate_sack : tp->selective_acks;
I would recommend running Smatch. It points out a lot of interesting things.
> + struct tcp_ao_key *rnext_key; > + struct tcp_ao_info *ao_info; > + u8 maclen; > > + if (WARN_ON_ONCE(!ao_key)) > + goto out_ao; > + ao_info = rcu_dereference_check(tp->ao_info, > + lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
Checkpatch complains about indentation here.
Rather than point out each case in the series, could I ask you to run ./scripts/checkpatch.pl --strict over the patchset?
...
> @@ -1363,6 +1424,34 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, > md5, sk, skb); > } > #endif > +#ifdef CONFIG_TCP_AO > + if (ao) { > + u8 *traffic_key; > + void *tkey_buf = NULL; > + u32 disn; > + > + sk_gso_disable(sk); > + if (unlikely(tcb->tcp_flags & TCPHDR_SYN)) { > + if (tcb->tcp_flags & TCPHDR_ACK) > + disn = ao->risn;
Sparse complains that there is an endian missmatch between disn and ao->risn ?
Rather than point out every problem flagged by Sparse, could I ask you to run it over the series?
...
| |